STMicroelectronics / STM32CubeG0

STM32Cube MCU Full Package for the STM32G0 series - (HAL + LL Drivers, CMSIS Core, CMSIS Device, MW libraries plus a set of Projects running on all boards provided by ST (Nucleo, Evaluation and Discovery Kits))
Other
165 stars 75 forks source link

VBUS race condition where USB TypeC 'attach' event is never seen by the application #6

Closed intercreate closed 3 years ago

intercreate commented 4 years ago

Overview Possible issue in the Middleware USB-PD G0 Device section

Describe the set-up

Describe the bug Possible issue in USB PD Middleware Device for G0. At attach sensing time, the library should check for VBUS to stabilize to Vsafe5V before moving forward. However, I believe there's an incorrect conditional for this check.

How To Reproduce

  1. Have a system with relatively fast VBUS ramp (less than CAD_TPDDEBOUCE_THRESHOLD)
  2. Attached a USB PD compliant device
  3. No attach event occurs.

Additional context This patch gets me past this issue.

iff --git a/Middlewares/ST/STM32_USBPD_Library/Devices/STM32G0XX/src/usbpd_cad_hw_if.c b/Middlewares/ST/STM32_USBPD_Library/Devices/STM32G0XX/src/usbpd_cad_hw_if.c
index 0209bb8..3068df8 100644
--- a/Middlewares/ST/STM32_USBPD_Library/Devices/STM32G0XX/src/usbpd_cad_hw_if.c
+++ b/Middlewares/ST/STM32_USBPD_Library/Devices/STM32G0XX/src/usbpd_cad_hw_if.c
@@ -766,7 +766,7 @@ static uint32_t ManageStateAttachedWait_SRC(uint8_t PortNum, USBPD_CAD_EVENT *pE

   if ((_handle->CurrentHWcondition != HW_Detachment) && (_handle->CurrentHWcondition != HW_PwrCable_NoSink_Attachment))
   {
-    if ((BSP_PWR_VBUSGetVoltage(PortNum) > BSP_PWR_LOW_VBUS_THRESHOLD)
+    if ((BSP_PWR_VBUSGetVoltage(PortNum) < BSP_PWR_LOW_VBUS_THRESHOLD)^M
         && (USBPD_PORTPOWERROLE_SRC == Ports[PortNum].params->PE_PowerRole))
     {
       /* reset the timing because VBUS threshold not yet reach */
intercreate commented 4 years ago

There doesn't seem to be any notification to the user that an attach event timed out. I would expect one from USBPD_DPM_Notification() maybe?

Also, as a reminder, USBPD_VbusEnable() library callback is called before this conditional statement, hence there is a race condition.

ALABSTM commented 4 years ago

Hi InterCreate,

Thank you for this one new aspect you pointed out. I let @bouattay check what you described and provide answers.

With regards,

ALABSTM commented 4 years ago

Hi InterCreate,

Our development teams said your issue is valid and that it is already known. It has been fixed internally and shall be made available in the frame of a future firmware package release. Unfortunately, we cannot share a date for the moment. We will be back to you as soon as this is done.

With regards,

ALABSTM commented 4 years ago

ST Internal Reference: 59814