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

removed delay before init of cyw43 #10038

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

bablokb
Copy link

@bablokb bablokb commented Feb 7, 2025

This PR removes the one second delay before the initialization of the CYW43. I don't think it is necessary, since nobody else is using a delay. Neither MicroPython nor any of the examples distributed by the RPi foundation (pico-examples, pico-extras and so on). I also searched the net and did not find any code that adds this delay.

Removing it cuts the boot-time of the Pico-W by almost 50%.

@eightycc
Copy link

eightycc commented Feb 7, 2025

cyw43_spi_reset() implements the correct delays around powering down then up the CYW43's WiFi section during initialization in cyw43_ll_bus_init(), so this patch looks good to me. Maybe @jepler has some insight into the circumstances around adding the 1 second delay?

@jepler
Copy link
Member

jepler commented Feb 7, 2025

We added it in #7313 to fix a problem reported in #7112.

I never saw or reproduced the problem, and we have no real idea how many boards/chips might be affected. However, we had at least 2 community members in that thread reporting a problem that the delay appeared to fix.

I do not know what we would do to be confident that it was OK to remove or decrease the delay, unless someone had in hand a device that reproduced the problem.

@bablokb
Copy link
Author

bablokb commented Feb 7, 2025

There is a merge from MicroPython on Oct 16, 2023 that defines CYW43_EVENT_POLL_HOOK (see

#define CYW43_EVENT_POLL_HOOK usb_background();
).

And according to the cyw43-driver source, this hook is necessary to perform background tasks "during long blocking operations such as WiFi initialisation". So this sounds like the solution of the initial problem #7112 (which hints at USB-devices not enumerating correctly).

@eightycc
Copy link

eightycc commented Feb 7, 2025

I do not know what we would do to be confident that it was OK to remove or decrease the delay, unless someone had in hand a device that reproduced the problem.

Thanks for the background info. Starting up the CYW43 is touchy. I'm now convinced we shouldn't change it.

@dhalbert
Copy link
Collaborator

dhalbert commented Feb 7, 2025

The one-second delay came from a test UF2 I gave to one of the folks in #7112. It was meant to make it completely clear that a delay would help. There is some looking in that issue at the CYW43 datasheet where it looks like 85 msec is the startup time. But the real test is to find a Pico W that fails without the delay, and then experiment. I was unable to reproduce the original problem, so it seems sample-based.

@eightycc
Copy link

eightycc commented Feb 7, 2025

Perhaps we could remove the delay and replace it with a retry of cyw43_arch_init_with_country() on a fail? For testing purposes, I'm thinking we can force a failure by decreasing the delays in cyw43_spi_reset().

@bablokb
Copy link
Author

bablokb commented Feb 7, 2025

If I read #7112 correctly, the OS did not see the Pico-W, probably because the USB-enumeration failed. But now, after merging MicroPython as linked above, that should not happen again because usb_background() takes care of this?!

Just to give you an idea why this 1s delay is important: I am running a project which does long-term environmental monitoring in schools in Africa, far away from any wall-plug. With all kinds of tricks we can now run about 2 years on a pair of AA-batteries. Current draw is mainly driven by on-time and this 1s delay during boot accounts for about 20% off the on-time during a single measurement cycle. So removing this delay makes a huge difference of a about half a year of operation.

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 this pull request may close these issues.

4 participants