Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
matthijskooijman opened this issue Dec 5, 2020 · 4 comments · May be fixed by #176
Open

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

matthijskooijman opened this issue Dec 5, 2020 · 4 comments · May be fixed by #176

Comments

@matthijskooijman
Copy link

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: STMicroelectronics/STM32CubeL0@8b26821

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
Copy link
Owner

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
Copy link
Author

Oh, I thought you actually recently (in c845a3b) replaced the USB HAL and updated it in c845a3b. 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
Copy link
Owner

GrumpyOldPizza commented Dec 5, 2020

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 added a commit to meetjestad/mjs_boards that referenced this issue Dec 5, 2020
It turns out that stopping the USB peripheral does not automatically
reset this 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.

This is a bug in the USB HAL used. This commit backports a small change
from the HAL v1.11.3 to fix this.

See: GrumpyOldPizza/ArduinoCore-stm32l0#171
matthijskooijman added a commit to matthijskooijman/ArduinoCore-stm32l0 that referenced this issue Mar 1, 2021
It turns out that stopping the USB peripheral does not automatically
reset this 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.

This is a bug in the USB HAL used. This commit backports a small change
from the HAL v1.11.3 to fix this.

This fixes GrumpyOldPizza#171
@matthijskooijman matthijskooijman linked a pull request Mar 1, 2021 that will close this issue
@matthijskooijman
Copy link
Author

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants