Skip to content

rp2: Fix USB PLL glitch during wake from light sleep. #15301

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

Merged
merged 3 commits into from
Jun 25, 2024

Conversation

projectgus
Copy link
Contributor

@projectgus projectgus commented Jun 19, 2024

Follow-up to #15111, that change works most of the time but has an intermittent bug where USB doesn't resume as expected after waking from light sleep. Thanks to @GitHubsSilverBullet who reported it here: https://github.com/orgs/micropython/discussions/14401#discussioncomment-9675017

Turns out waking calls clocks_init() which will re-initialise the USB PLL. Most of the time this is OK but occasionally it seems like the clock glitches the USB peripheral and it stops working until the next hard reset.

Adds a machine.lightsleep() test that consistently hangs in the first two dozen iterations of a single test run on rp2 without this fix. Passed over 100 full test runs in a row with this fix.

The test is currently rp2-only as it seems similar lightsleep USB issues exist on other ports (both pyboard and ESP32-S3 native USB don't send any data to the host after waking, until they receive something from the host first.)

EDIT: Added one more small commit to always disable USB if there is no timeout (i.e. entering DORMANT mode where the external oscillator is disabled.)

This work was funded through GitHub Sponsors.

Copy link

codecov bot commented Jun 19, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 98.42%. Comparing base (cfa55b4) to head (f60c71d).

Additional details and impacted files
@@           Coverage Diff           @@
##           master   #15301   +/-   ##
=======================================
  Coverage   98.42%   98.42%           
=======================================
  Files         161      161           
  Lines       21248    21248           
=======================================
  Hits        20914    20914           
  Misses        334      334           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link

Code size report:

   bare-arm:    +0 +0.000% 
minimal x86:    +0 +0.000% 
   unix x64:    +0 +0.000% standard
      stm32:    +0 +0.000% PYBV10
     mimxrt:    +0 +0.000% TEENSY40
        rp2:    +0 +0.000% RPI_PICO_W
       samd:    +0 +0.000% ADAFRUIT_ITSYBITSY_M4_EXPRESS

@dpgeorge
Copy link
Member

Maybe we should report this to pico-sdk, and potentially they can add this argument to clocks_init()?

But for now, patching it here is OK.

Can you please split out the addition of our copy of clocks_init() to a separate commit and update the top-level LICENSE? See 8438c87

Adapts pico-sdk clocks_init() into clocks_init_optional_usb() which takes
an argument to initialise USB clocks or not.

To avoid a code size increase the SDK clocks_init() function is linker
wrapped to become clocks_init_optional_usb(true).

This work was funded through GitHub Sponsors.

Signed-off-by: Angus Gratton <angus@redyak.com.au>
Follow-up to a84c7a0, this commit works most of the time but has an
intermittent bug where USB doesn't resume as expected after waking from
light sleep.

Turns out waking calls clocks_init() which will re-initialise the USB PLL.
Most of the time this is OK but occasionally it seems like the clock
glitches the USB peripheral and it stops working until the next hard reset.

Adds a machine.lightsleep() test that consistently hangs in the first
two dozen iterations on rp2 without this fix. Passed over 100 times in a
row with this fix.

The test is currently rp2-only as it seems similar lightsleep USB issues
exist on other ports (both pyboard and ESP32-S3 native USB don't send any
data to the host after waking, until they receive something from the host
first.)

This work was funded through GitHub Sponsors.

Signed-off-by: Angus Gratton <angus@redyak.com.au>
@projectgus projectgus force-pushed the bugfix/rp2_usb_lightsleep branch from c1fa5e3 to 068d9bf Compare June 25, 2024 06:26
@projectgus
Copy link
Contributor Author

Can you please split out the addition of our copy of clocks_init() to a separate commit and update the top-level LICENSE? See 8438c87

No worries, done. PTAL!

In this mode, XOSC is stopped so can't really keep
the USB PLL enabled.

Signed-off-by: Angus Gratton <angus@redyak.com.au>
@projectgus
Copy link
Contributor Author

Added one more small commit to always disable USB if going to DORMANT mode with no external oscillator.

Have also raised a feature request with pico-sdk, see link above.

@dpgeorge
Copy link
Member

All looks good, thank you!

@dpgeorge dpgeorge merged commit f60c71d into micropython:master Jun 25, 2024
28 checks passed
@projectgus projectgus deleted the bugfix/rp2_usb_lightsleep branch June 26, 2024 00:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants