GrumpyOldPizza / ArduinoCore-stm32l0

Arduino Core for STM32L0
125 stars 67 forks source link

USB does not disconnect on USBDevice.detach() #171

Open matthijskooijman opened 3 years ago

matthijskooijman commented 3 years ago

It turns out that stopping the USB peripheral does not automatically reset the D+ pullup, so stopping USB (e.g. through USBDevice.detach()) would let the pullup enabled, making the USB host think the board was still connected (but any subsequent communication would fail). Additionally, this would use around 300μA of current.

I've found this is a bug in the USB HAL, which is fixed by v1.11.3: https://github.com/STMicroelectronics/STM32CubeL0/commit/8b26821a5b4831fc85df5c450c81568a744a1027

I tried backporting the single change that seems to fix this, which works as expected (the diff is slightly different from the linked upstream commit, probably because this core is using an even older HAL version, I suspect):

--- a/system/STM32L0xx/Source/USB/HAL/Src/stm32l0xx_hal_pcd.c
+++ b/system/STM32L0xx/Source/USB/HAL/Src/stm32l0xx_hal_pcd.c
@@ -240,7 +240,7 @@ HAL_StatusTypeDef HAL_PCD_DeInit(PCD_HandleTypeDef *hpcd)
   hpcd->State = HAL_PCD_STATE_BUSY;

   /* Stop Device */
-  (void)HAL_PCD_Stop(hpcd);
+  (void)USB_StopDevice(hpcd->Instance);

 #if (USE_HAL_PCD_REGISTER_CALLBACKS == 1U)
   if (hpcd->MspDeInitCallback == NULL)
@@ -998,7 +998,7 @@ HAL_StatusTypeDef HAL_PCD_Stop(PCD_HandleTypeDef *hpcd)
   __HAL_LOCK(hpcd);
   __HAL_PCD_DISABLE(hpcd);

-  (void)USB_StopDevice(hpcd->Instance);
+  (void)USB_DevDisconnect(hpcd->Instance);

   __HAL_UNLOCK(hpcd);

However, it's probably better to just update the HAL completely. Since I'm not sure what the procedure for that is, I'll leave it at this issue rather than a full PR.

GrumpyOldPizza commented 3 years ago

No clue what HAL version is in use right now. USB is using a few files from the HAL. Many of them are modified. IMHO the correct fix is to get rid of the last HAL references anyway.

Thanx for tracking that down.

matthijskooijman commented 3 years ago

Oh, I thought you actually recently (in c845a3b4b90b8a8fe95c6fedd4fa927dae14ba45) replaced the USB HAL and updated it in c845a3b4b90b8a8fe95c6fedd4fa927dae14ba45. The commits sound like you're using the USB HAL verbatim, though I can't tell for sure from the messages. Anyway, I've applied the above fix to our version of the core, I'll leave it to you to (decide how to) apply it here.

GrumpyOldPizza commented 3 years ago

Not quite verbatim. There are a lots of issues I have with Core/HAL for USB. GIven my limited time, I am just ignoring those for now. BCD is not working as should, Double Buffered transmit/receive is not working ... there is some nitpicking issues with larger transmit sizes ... a huge ton of race conditions that I have not attended to. A house of cards overall.

matthijskooijman commented 3 years ago

Given that your HAL is not a verbatim copy, I guess updating it to a new version completely is not the way forward, so I submitted the backported fix (which I already had as a separate commit in my own tree) in a PR, in case that's helpful: #176.