Request: Variant of clocks_init() that allows selectively enabling PLLs (and/or clocks) #1746

New issue

Open

opened 2024-06-25 01:54:51 -05:00 by projectgus · 2 comments

projectgus commented

2024-06-25 01:54:51 -05:00

(Migrated from github.com)

This is a use case we found in MicroPython, as we added support for retaining the USB peripheral clock in SLEEP mode.

The mp_machine_lightsleep function selectively disables a range of clocks and PLLs, goes to SLEEP mode, and then calls pico-sdk clocks_init() on wake to restore them. Calling the init function again is useful as it saves a bunch of binary size and removes the potential for a mismatch between the clock config "on init" and "on wake".

However, we recently added support for keeping the USB peripheral clock and PLL enabled in sleep (XOSC also enabled). This works most of the time, but intermittently the USB peripheral would hang on wake as clocks_init() is re-enabling the USB PLL and presumably this can glitch the USB clock.

Our solution for now has been to copy clocks_init() as void clocks_init_optional_usb(bool init_usb). The new argument can be false if the USB PLL is already running. See https://github.com/micropython/micropython/pull/15301 for more details.

However, wanted to raise it here as a request. Perhaps there is a broader use for pico-sdk API to have something like clocks_init_flags(unsigned PLL_FLAGS) function where the default clocks_init() becomes clocks_init_flags(ALL_PLL_FLAGS) (or similar).

This is a use case we found in MicroPython, as we added support for retaining the USB peripheral clock in SLEEP mode. The [mp_machine_lightsleep function](https://github.com/micropython/micropython/blob/a84c7a0ed931667cda44a8e27888f066e085d5eb/ports/rp2/modmachine.c#L109) selectively disables a range of clocks and PLLs, goes to SLEEP mode, and then calls pico-sdk `clocks_init()` on wake to restore them. Calling the init function again is useful as it saves a bunch of binary size and removes the potential for a mismatch between the clock config "on init" and "on wake". However, we recently added support for keeping the USB peripheral clock and PLL enabled in sleep (XOSC also enabled). This works most of the time, but intermittently the USB peripheral would hang on wake as `clocks_init()` is re-enabling the USB PLL and presumably this can glitch the USB clock. Our solution for now has been to copy `clocks_init()` as [void clocks_init_optional_usb(bool init_usb)](https://github.com/micropython/micropython/pull/15301/commits/5dcffb53ab953e8264be33e7cb56280b7e20c345). The new argument can be false if the USB PLL is already running. See https://github.com/micropython/micropython/pull/15301 for more details. However, wanted to raise it here as a request. Perhaps there is a broader use for pico-sdk API to have something like `clocks_init_flags(unsigned PLL_FLAGS)` function where the default `clocks_init()` becomes `clocks_init_flags(ALL_PLL_FLAGS)` (or similar).

lurch commented

2024-06-25 13:29:43 -05:00

(Migrated from github.com)

Sounds like this might be related to #1668 and #1672 ?

projectgus commented

2024-06-25 17:22:47 -05:00

(Migrated from github.com)

@lurch It is, I'm sorry I should have searched first. Feel free to close in favour of either of those.

EDIT: Actually, not quite - that patch as-written doesn't meet our immediate needs as it doesn't cover PLL_USB (only PLL_SYS). But I think that approach could be extended pretty easily to also cover PLL_USB.

@lurch It is, I'm sorry I should have searched first. Feel free to close in favour of either of those. EDIT: Actually, not quite - that patch as-written doesn't meet our immediate needs as it doesn't cover PLL_USB (only PLL_SYS). But I think that approach could be extended pretty easily to also cover PLL_USB.