zephyrproject-rtos / zephyr

Primary Git Repository for the Zephyr Project. Zephyr is a new generation, scalable, optimized, secure RTOS for multiple hardware architectures.
https://docs.zephyrproject.org
Apache License 2.0
10.86k stars 6.62k forks source link

Including STM32Cube's USB PD support to Zephyr #27525

Closed intercreate closed 3 years ago

intercreate commented 4 years ago

Is your enhancement proposal related to a problem? Please describe. STM32Cube SDK has USB PD library support. Example here. This is currently not included in the hal/stm32/stm32cube module

Describe the solution you'd like Add the source code of this library, one for each STM32Cube. This does not include integration into the RTOS (yet!), but should be eventually.

Describe alternatives you've considered Simply copying all files under STM32_USBPD_Library file in the application area (outside of zephyr) and building using cmake.

Additional context I would LOVE to see the Devices/STM32xxx directory integrated into Zephyr and using it's clean API to hardware, while still allowing the application to use the ST supplied static libraries for USB PD state management. But for now, it would be a big help if the STM32_USBPD_Library were included in the stm32cube module.

thoughts? I'm happy to issue a PR if I can find the script or whatever that dictates what files are copied over from the official STM32Cube lib to the stm32 repo.

carlescufi commented 4 years ago

@jfischer-phytec-iot @jettr @sjg20 are there any plans for work on USB Power Delivery in Zephyr itself? This is a USB feature that the current Zephyr USB stack doesn't support, hence the request for integrating the STM32 library. @intercreate are you interested in PD only, or do you want to combine that with Zephyr's USB stack?

erwango commented 4 years ago

@intercreate Txs for your interest in STM32 PD. One thing to consider before importing code in module/hal/stm32 is that this code needs to be maintained and tested and this is something hard to do when there is nothing on zephyr side to exercise the code. So, we'd need a matching sample to use it on zephyr side.

intercreate commented 4 years ago

HI @carlescufi , Since USB PD is its own independent spec with its own stack, it should be considered separate from the USB stack... The Zephyr USB stack as it stands is great, btw! 👍

HI @erwango , Can you elaborate on the level of testing needed to meet zephyr quality standards? Is it just a matter of adding an example project for the specific Nucleo board to the Zephyr repo? (There are at least 2 Nucleo boards that zephyr supports and where the MCU supports USB PD)

thank you for your responses!

carlescufi commented 4 years ago

@intercreate I think what @erwango means is that if you propose to include the USB PD code in the module, you need to also add a library/subsystem in Zephyr that somehow (indirectly, behind a generic API) uses it, or at the very least a new sample which is STM32-specific, that uses the STM32 HAL functions directly. You can place that sample in samples/boards/...

jfischer-no commented 4 years ago

I thought we had an open enhancement request for USB PD but I found only #18579.

jettr commented 4 years ago

We do have plans to port our existing USB-PD stack over to Zephyr. You can see our existing implementation here https://source.chromium.org/chromiumos/chromiumos/codesearch/+/master:src/platform/ec/common/usbc/

I would like to keep the USB-PD stack as a separate subsystem instead of mixing it with the existing USB data stack.

jfischer-no commented 4 years ago

I would like to keep the USB-PD stack as a separate subsystem instead of mixing it with the existing USB data stack.

Nobody wants to mix USB PD with USB device or host stack, but we will always have the problem with confusing nomenclature. Maybe we should call it PD for USB, subsys/pd-usb orsubsys/pd_usb?

intercreate commented 4 years ago

@jettr Awesome! Thanks for posting. Can you comment on how compliant this chromium code is to the USB PD spec?

@jfischer-phytec-iot I'm surprised to learn that nobody in the Zephyr community wants to treat USB PD separate from USB as they're separate specs.. just like how I2C is separate from UART. PD has dedicated wires, a complete different phy, different protocol... it's not USB2.0.

USB TypeC and USB PD is more tricky and not as clear cut. They are also different specs, but the both talk about each other a lot in their respective specs :).. and both use the same physical transport (CC line).

jfischer-no commented 4 years ago

@jfischer-phytec-iot I'm surprised to learn that nobody in the Zephyr community wants to treat USB PD separate from USB as they're separate specs.. just like how I2C is separate from UART. PD has dedicated wires, a complete different phy, different protocol... it's not USB2.0.

Where have I said the opposite? I have revised my comment to be more precise.

intercreate commented 4 years ago

@jfischer-phytec-iot ah! thanks for clarifying. I'm glad we all agree 👍

jettr commented 4 years ago

The chromium code aims to follow the state machine layout as described in the Type-C (e.g. Figure 4-16) and PD (e.g. Figure 6-54) USB specifications. It also aims to be 100% compliant with the test specifications. Admittedly, we are not at 100% compliance via testing but we are actively working on that.

Also, it is worth mentioning that we typically depend on USB Type-C Port Controller (TCPC) ICs, as defined by the TCPCI specification, to implement the physical layer (e.g. BMC encoding). The embedded controller typically communicates with these TCPCs over i2c to send and receive PD packets. Some embedded controllers have the TCPCs embedded within the EC as hardware modules though. So the PD stack depends on the TCPC interface; we do not plan on porting over a physical layer for USB-PD (e.g. BMC encoding on the CC lines).

erwango commented 3 years ago

@intercreate, @jfischer-no, @jettr if sticking with existing request "Including STM32Cube's USB PD support to Zephyr", I'd tend to close current issue, as we agree this won't be the way to go. An new issue could be created for USB PD support if required. Is that fine for you ?

jfischer-no commented 3 years ago

@erwango yes

intercreate commented 3 years ago

@erwango , yes... I am looking forward to @jettr getting the chromium OS code in there!

raveslave commented 3 years ago

@jettr great to see this coming! agree that PD is a completely separate thing.
...in most our products, we have the PD stack (talking to the PD PHY) + power state machine (talking to chargers, pmic's, vbus switches etc) running on a low-power ARM, while the USB stack itself runs completely decoupled on a larger mcu/dsp or such.

some topics that probably would need separate issues:

  1. Discuss how to best bridge the needed events in designs using
    • multi core MCUs (more and more common with the m33's where one might be low-power domain doing PD)
    • two MCU's sitting on the same board (could be a nordic for ble and always-on duties) + a more power-hungry MCU with USB HS host/device capabilities.
  2. how the pd subsys would bridge in a nice way with the power management subsystem. some examples might be:
    • a PD transition could be triggered by a regulator detecting undervoltage on vbus (in sink mode) vs. over-current (in source mode)
    • it could also be signals coming from user-interactions changing states in the power-management parts like, battery-power vs bus/dc-power, then the PD would have to present different options as well as react to these runtime.
    • we also have to support several pd-phy instances for designs implementing multiple type-c ports (more and more common with one usb-c for data, and one dedicated for power, coming from a pd compliant ac/dc supply)

Zephyr doesn't (afaik) implement a structured messaging protocol for multi-core systems when it comes to power-management. In this case, it would span multiple subsystems (pd, power, usb) as events from any peripheral need to control how the usb peripheral is configured, torn down, switch to host-mode, etc... to internal power sequencing when doing DFP+Source (legacy "OTG") where PD has to run in tight sync with the Power Management state-machine controlling regulators for sinking or sourcing VBUS current... to legacy cables (non PD) USB scenarios where d+/d- enumeration decides available sink current which I believe should end up in the same unified 'manager' to avoid duplication of two parallel state-machines.

ps. found an old issue I made around PD
https://github.com/zephyrproject-rtos/zephyr/issues/18579

raveslave commented 3 years ago

@jettr would you mind sharing what PD phy's you are using today?

jettr commented 3 years ago

You can find more information by looking at our open-source drivers: https://source.chromium.org/chromiumos/chromiumos/codesearch/+/master:src/platform/ec/driver/tcpm/

A non-representative example list follow:

We do have a PHY FW stack that we have used for STM32 chips, but we prefer to use a TCPC IC (discrete or embedded within EC) to handle the PHY translation.

raveslave commented 3 years ago

Hi Jeff, just checking in if there are any development for the zephyr tcpm work

jettr commented 3 years ago

We are still a ways off from upstreaming a native TCPM port to Zephyr. @keith-zephyr will know more about schedule and priorities at list point

keith-zephyr commented 3 years ago

We haven't started any of the upstream Zephyr TCPM work yet. We do have the TCPM stack working with Zephyr as the kernel, but this still very tightly coupled with our EC application. The time frame to de-couple the TCPM stack from our EC application is ways out.

You can see some of the groundwork regarding the KConfig options that have been created here: https://chromium.googlesource.com/chromiumos/platform/ec/+/621f34cc/zephyr/Kconfig.usbc#

raveslave commented 3 years ago

thx for update, looking forward!

davedesro commented 3 years ago

Hi there. 'intercreate' here. (I converted the github account to an organization recently). So, since posting this originally, I have successfully used the STM32 USBPD libraries with Zephyr using hacked up out of tree cmake scripts. It works, sure.. but after going through it, I don't believe it is a good setup to be integrated into Zephyr. For starters, it requires CMSIS RTOS v1 api, but the header used is modified. Also, the binaries support GCC, but has a bunch of IAR sections. They do support FreeRTOS 'out of the box'. I (and I bet STMicro) wouldn't recommend anyone to use it on any other RTOS.

Fast forward to today, we are working on a project using STM32G0 (with integrated USB PD phy). The project requires two independent USB PD instances. We're super duper interested in the chromiumos efforts, although we will not be using a TCPM. I have implemented BMC encoding a while ago and (for better or worse) know enough of the STM32 architecture. I'm not too worried about those lower level parts.

But man, USB PD is such a complex spec.. lots to consider. One of the more time consuming parts, I think, is with implementing all those crazy protocol layer FSMs and TypeC FSMs.

@keith-zephyr @jettr is this something you're working on by chance 🤞??

In any case, I'd love to have us here at intercreate work with you on getting something in zephyr

jettr commented 3 years ago

Unfortunately porting the PD stack to upstream Zephyr is still a ways off on our timeline (> 1 year). You are more than welcome to use/reuse the chromium version of our PD stack in the meantime though (it does compile with a Zephry adapter layer, i.e. we use that chromium code with a zephyr build)

davedesro commented 3 years ago

@jettr , yeah, no problem on the timeline. We are already looking through your code with the plan to use it in our application area. This will be good enough for our near term deliverables. We'll report back if we find anything interesting along the way.

I'm curious, though, are the protocol layer FSMs and TypeC FSMs implemented in your code base?

jettr commented 3 years ago

are the protocol layer FSMs and TypeC FSMs implemented in your code base?

They sure are! You can find them all in the usbc folder. Specifically here is a link to the protocol layer and another for the TypeC layer.

rohand87 commented 3 years ago

Hi @davedesro I am also trying to use STM32Cube's USB-PD stack with Zephyr. I am using a STM32G4 controller. Could you provide some pointers of how you made it to work with the STM32G0? The USB-PD core library is provided as a *.a file. Which file did you use and how did you compile that with Zephyr. Any guidance will be helpful for me. Thanks

keith-zephyr commented 3 years ago

For those folowing this issue, I added a comment to https://github.com/zephyrproject-rtos/zephyr/issues/18579 requesting feedback for USB-C features to prioritize.

davedesro commented 3 years ago

Hi @rohand87 Unfortunately I can't provide pointers. I can say that it's possible with both G0 and G4 hardware.

You can use the ST supplied 'Device/' files directly to configure the ucpd hardware (dmas, timers) and disable them in Zephyr's environment (Zephyr wouldn't know about the hardware being used). This is hacky and dangerous.. so I don't recommend it unless you know what you're doing and your schedule demands this approach. Integrating this in Zephyr instead would be a time consuming task mainly because of ST's dependency of the files under Device/

Besides that, to integrate the static library, amongst other things, you can add the static library like this

target_link_libraries(
  app PUBLIC
  ${CMAKE_CURRENT_SOURCE_DIR}/Core/lib/USBPDCORE_PD2_CONFIG_MINSNK_CM0PLUS_wc32.a
  ${ZEPHYR_CURRENT_LIBRARY}
  ) 

zephyr_library_sources(
  Core/src/usbpd_dpm_core.c
  )

It's worth pointing out that the library has a bunch of .iar sections in it, that the zephyr linker may complain about.

I hope this helps.

rohand87 commented 3 years ago

Hi @davedesro Thank you for your help. Now I am able to build the USB PD stack with my Zephyr application. There is a with a bunch of iar orphan section warnings like you said. I am using the USBPDCORE_PD3_CONFIG_MINSNK_CM4_wc32.a library.

While running I am facing the following issue. The function USBPD_CAD_Process() hangs and none of the callback functions are not getting called. This is a function inside the static library so I cannot debug. Did you face this issue? Any idea why this might happen?

How did you implement the USBPD_CAD_Task and USBPD_PE_Task threads? I converted them to Zephyr threads.