Add ability to specify which clocks are re-initialized in 'clock_init()', after microPython 'lightsleep()' #1668

Closed
opened 2024-03-14 08:07:22 -05:00 by mungewell · 11 comments
mungewell commented 2024-03-14 08:07:22 -05:00 (Migrated from github.com)

From thread:
https://github.com/orgs/micropython/discussions/14087

I am looking to keep the PIOs running during 'lightsleep()', as this will help my project save power. The CPU can pre-fill the FIFOs and then sleep, whilst the PIOs are processing the data.

ATM microPython 'lightsleep()' function calls the 'clock_init()' func after the CPU re-awakes, which causes the clocks to be reinitialized. This is 'bad' for me, as it overwrites the prior 'machine.freq(x)' call, but also potentially causes a glitch on the SYS clock - and therefore glitch on the PIO clocks.

This patch allows the passing of previously used sleep_en0 and sleep_en1 values, which are used to determine whether SYS_CLOCK should be reinitialized, or skipped:

20240314_sdk_keep_pios_running.patch.txt

From thread: https://github.com/orgs/micropython/discussions/14087 I am looking to keep the PIOs running during 'lightsleep()', as this will help my project save power. The CPU can pre-fill the FIFOs and then sleep, whilst the PIOs are processing the data. ATM microPython 'lightsleep()' function calls the 'clock_init()' func after the CPU re-awakes, which causes the clocks to be reinitialized. This is 'bad' for me, as it overwrites the prior 'machine.freq(x)' call, but also potentially causes a glitch on the SYS clock - and therefore glitch on the PIO clocks. This patch allows the passing of previously used `sleep_en0` and `sleep_en1` values, which are used to determine whether SYS_CLOCK should be reinitialized, or skipped: [20240314_sdk_keep_pios_running.patch.txt](https://github.com/raspberrypi/pico-sdk/files/14602541/20240314_sdk_keep_pios_running.patch.txt)
lurch commented 2024-03-14 08:58:41 -05:00 (Migrated from github.com)

I see that your patch is modifying src/rp2040/hardware_regs/include/hardware/regs/clocks.h - FYI everything inside src/rp2040/hardware_regs is autogenerated, so you'll have to move the changes you made there to some other file 😉

And I also think we generally prefer Pull Requests, rather than patch files attached to Issues.

I see that your patch is modifying `src/rp2040/hardware_regs/include/hardware/regs/clocks.h` - FYI everything inside `src/rp2040/hardware_regs` is autogenerated, so you'll have to move the changes you made there to some other file :wink: And I also think we generally prefer Pull Requests, rather than patch files attached to Issues.
mungewell commented 2024-03-14 19:25:27 -05:00 (Migrated from github.com)

I presume you mean that they are generated from pico-sdk/src/rp2040/hardware_regs/rp2040.svd.

How is that done? Can someone point me at a script? What I had added was a 'collation' of the 'SYS_CLK' dependants, not sure how to auto-gen that....

No problem for a PR, if I can figure it out.

I presume you mean that they are generated from `pico-sdk/src/rp2040/hardware_regs/rp2040.svd`. How is that done? Can someone point me at a script? What I had added was a 'collation' of the 'SYS_CLK' dependants, not sure how to auto-gen that.... No problem for a PR, if I can figure it out.
lurch commented 2024-03-15 08:09:50 -05:00 (Migrated from github.com)

I presume you mean that they are generated from pico-sdk/src/rp2040/hardware_regs/rp2040.svd.

Not quite. Both src/rp2040/hardware_regs/include/hardware/regs/ and src/rp2040/hardware_regs/rp2040.svd are generated from the same internal data-files. (which are also used to generate the register tables in https://datasheets.raspberrypi.com/rp2040/rp2040-datasheet.pdf )

How is that done? Can someone point me at a script?

This is done with internal (private) tooling.

> I presume you mean that they are generated from `pico-sdk/src/rp2040/hardware_regs/rp2040.svd`. Not quite. Both `src/rp2040/hardware_regs/include/hardware/regs/` and `src/rp2040/hardware_regs/rp2040.svd` are generated from the same internal data-files. (which are also used to generate the register tables in https://datasheets.raspberrypi.com/rp2040/rp2040-datasheet.pdf ) > How is that done? Can someone point me at a script? This is done with internal (private) tooling.
mungewell commented 2024-11-07 23:33:42 -06:00 (Migrated from github.com)

I am back working on my project for which this is for. I note that the init code has changed substantially in the Pico-SDK and the patch is no longer valid. But....

MicroPython has taken ownship of the clock init, and I have implement a patch in that domain.
https://github.com/micropython/micropython/pull/16183

I am back working on my project for which this is for. I note that the init code has changed substantially in the Pico-SDK and the patch is no longer valid. But.... MicroPython has taken _ownship_ of the clock init, and I have implement a patch in that domain. https://github.com/micropython/micropython/pull/16183
lurch commented 2024-11-11 07:49:19 -06:00 (Migrated from github.com)

Okay - if the issue is being handled over on the micropython side of things, can this pico-sdk issue be closed? Or is this still a nice-to-have in the pico-sdk world?

Okay - if the issue is being handled over on the micropython side of things, can this pico-sdk issue be closed? Or is this still a nice-to-have in the pico-sdk world?
mungewell commented 2024-11-11 14:27:22 -06:00 (Migrated from github.com)

To be honest, it seems that nobody else is crazy enough to be doing stuff like this... ;-)

Since I can implement in MicroPython world, and there's enough of a 'bread crumb trail' to there, I am closing the issue. Thanks.

To be honest, it seems that nobody else is crazy enough to be doing stuff like this... ;-) Since I can implement in MicroPython world, and there's enough of a 'bread crumb trail' to there, I am closing the issue. Thanks.
projectgus commented 2024-12-19 23:43:58 -06:00 (Migrated from github.com)

Sorry I'm turning up very late to this issue from the MicroPython side, but I have realised that it's much more sustainable to have this use case supported in pico-sdk than for us to play catch-up with a patched SDK clocks.c file. I should have followed up back in July, when we split it out - my bad. 😞

Is something like this request still viable, where a variant of clocks_init() allows specifying which clocks to skip reinitialisation for?

Our current use case in MicroPython is to keep the USB clock and PLL running during lightsleep and not re-initialise on wake to avoid glitches, but obviously @mungewell has a use case for the PIO clock, and there are probably many others too.

Sorry I'm turning up very late to this issue from the MicroPython side, but I have realised that it's much more sustainable to have this use case supported in pico-sdk than for us to play catch-up with a patched SDK clocks.c file. I should have followed up back in July, when we split it out - my bad. :disappointed: Is something like this request still viable, where a variant of `clocks_init()` allows specifying which clocks to skip reinitialisation for? Our current use case in MicroPython is to keep the USB clock and PLL running during lightsleep and not re-initialise on wake to avoid glitches, but obviously @mungewell has a use case for the PIO clock, and there are probably many others too.
lurch commented 2024-12-20 04:17:05 -06:00 (Migrated from github.com)

Your issue #1746 is still open, if you want to add more details to that 🙂

Your issue #1746 is still open, if you want to add more details to that :slightly_smiling_face:
projectgus commented 2024-12-20 04:39:50 -06:00 (Migrated from github.com)

Oh, I did raise this back in the middle of the year 🤦 . I should give myself more credit for thinking ahead (albeit not for having a good memory.) Thanks, @lurch 😄

Oh, I did raise this back in the middle of the year :facepalm: . I should give myself more credit for thinking ahead (albeit not for having a good memory.) Thanks, @lurch :smile:
peterharperuk commented 2024-12-20 05:38:44 -06:00 (Migrated from github.com)

@projectgus It's not been forgotten about. We had a meeting about a new long overdue "sleep" API recently and the Micropython requirement to keep the USB (or whatever) clock running was discussed.

@projectgus It's not been forgotten about. We had a meeting about a new long overdue "sleep" API recently and the Micropython requirement to keep the USB (or whatever) clock running was discussed.
projectgus commented 2024-12-20 18:25:29 -06:00 (Migrated from github.com)

@peterharperuk Well that's even better that you folks remembered it while I forgot. 😆

@peterharperuk Well that's even better that you folks remembered it while I forgot. :laughing:
Sign in to join this conversation.
No milestone
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
gh-raspberrypi/pico-sdk#1668
No description provided.