STM32 USB PHY not working with UCPD node disabled and Type-C plugged into host supporting USB PD

[JPHutchins]

Describe the bug

STM32 devices, like L5, have a USB PHY and UCPD (USB power delivery) system. We have encountered an issue wherein disabling the UCPD node (and disabling the STM provided USBPD system or the Zephyr USBPD system) while leaving the USB PHY enabled causes the STM32 to not enumerate on some host devices supporting USB PD on their Type-C ports.

This is a critical issue for MCUBoot USB functionality because it adds a USB PD requirement to MCUBoot - a potentially large flash footprint for a bootloader.

To Reproduce

Put the Zephyr shell on USB CDC ACM, disable the ucpd node, and verify:

• enumeration on host Type-C ports that DO NOT have USB PD
• no enumeration on host Type-C ports that do have USB PD

Expected behavior

The USB PHY should not depend on the presence of some USB PD system.

Impact

Prevents FW upgrades from within MCUBoot from newer laptops that have Type-C with PD. Users must use a Type-C -> Type-A cable or adapter to defeat the bug.

Fix

It is relatively simple to fix and this is a patch that I would propose on all STM32 systems that have USB PD.

diff --git a/soc/arm/st_stm32/stm32l5/soc.c b/soc/arm/st_stm32/stm32l5/soc.c
index 1db1bf0411..ac22e0e7a7 100644
--- a/soc/arm/st_stm32/stm32l5/soc.c
+++ b/soc/arm/st_stm32/stm32l5/soc.c
@@ -58,8 +58,10 @@ static int stm32l5_init(void)
    LL_APB1_GRP1_EnableClock(LL_APB1_GRP1_PERIPH_PWR);
    LL_PWR_SetRegulVoltageScaling(LL_PWR_REGU_VOLTAGE_SCALE0);

-   /* Disable USB Type-C dead battery pull-down behavior */
-   LL_PWR_DisableUCPDDeadBattery();
+   if (IS_ENABLED(CONFIG_DT_HAS_ST_STM32_UCPD_ENABLED) || !IS_ENABLED(CONFIG_USB_DEVICE_DRIVER)) {
+       /* Disable USB Type-C dead battery pull-down behavior */
+       LL_PWR_DisableUCPDDeadBattery();
+   }

    return 0;
 }

The logic is that the UCPDDeadBattery Mode (AKA Rd 5.1K pulldown) should ONLY BE DISABLED if:

• a USB PD system is in place (the UCPD node is enabled)

• or the user does not require USB PHY anyway

  So, in a normal MCUBoot case where

• UCPD node is disabled

• USB PHY is enabled

  LL_PWR_DisableUCPDDeadBattery(); will not get called, leaving the 5.1K pulldown in place so that the host sees a "regular" USB sink.

  However, this investigation has brought up some important questions. The docstring from ST about LL_PWR_DisableUCPDDeadBattery(); seems to indicate that it is misused in soc.c:

  /**
  * @brief  Disable the USB Type-C and power delivery dead battery pull-down behavior
  *         on UCPD CC1 and CC2 pins.
  * @note   After exiting reset, the USB Type-C dead battery behavior is enabled,
  *         which may have a pull-down effect on CC1 and CC2 pins. It is recommended
  *         to disable it in all cases, either to stop this pull-down or to hand over
  *         control to the UCPD (which should therefore be initialized before doing the disable).
  * @rmtoll CR3          UCPD_DBDIS           LL_PWR_DisableUCPDDeadBattery
  * @retval None
  */
  __STATIC_INLINE void LL_PWR_DisableUCPDDeadBattery(void)
  {
  SET_BIT(PWR->CR3, PWR_CR3_UCPD_DBDIS);
  }

  The important bit is "hand over control to the UCPD (which should therefore be initialized before doing the disable)". By placing this function in soc.c, it is certainly occurring before either the ST X-CUBE-USB-PD library or the Zephyr USB PD driver have been initialized. In fact, my interpretation is that disabling of dead battery mode should be driver dependent - the UCPD driver (or lack thereof) should decide whether or not to disable dead battery mode according to the users' configuration.

The commit adding this, https://github.com/zephyrproject-rtos/zephyr/commit/9e30ab58ea50ee8467adde26909eced0c9f85d52, has this message:

soc: arm: stm32l5 with USB-C PD cannot use CC1 and CC2 pins by default
With this patch, the UCPD1 _CC1 and _CC2 pins
are disabling the USB Type-C and Power Delivery Dead Battery.

Signed-off-by: Francois Ramu <francois.ramu@st.com>

Which is pretty clear! Yet, I think that it is a mistake to call this function in the soc.c files without regard for the products requirements.

@FRASTM please take a look when you have a chance, thank you!

Cheers, JP

[erwango]

• check if this need to be extended to other series as well (u5, ..)

[FRASTM]

The same change should apply to

• STM32G4
• STM32H5
• STM32L5
• STM32U5