Add cyw43_spi_set_clock_divisor() to allow CYW43 to work with over/underclocked RP2040 #1521

2023-10-21T19:13:40-05:00

ai6aj commented

2023-10-21 19:13:40 -05:00

(Migrated from github.com)

Fixes issues #1297 and #1392

This is a straightforward patch to allow the Pico W to work with the WiFi/BT module when heavily over- or underclocked. The previous code had the SPI clock divisor hardcoded to a value of 2, this allows the programmer to explicitly set it to any value they'd like.

In both instances, simply set the desired clock divisor PRIOR to initializing the WiFi module:

set_sys_clock_khz(48000, true);      // cyw43 init will hang or fail, see issue #1392
cyw43_spi_set_clock_divisor(1,0);   // Speed up the SPI bus so it will work
if (cyw43_arch_init_with_country(CYW43_COUNTRY_USA)) { ...

or

set_sys_clock_khz(266000, true);      // cyw43 init will fail for reasons similar to #1297
cyw43_spi_set_clock_divisor(4,0);     // Slow down the SPI bus 
if (cyw43_arch_init_with_country(CYW43_COUNTRY_USA)) { ...

Fixes issues #1297 and #1392 This is a straightforward patch to allow the Pico W to work with the WiFi/BT module when heavily over- or underclocked. The previous code had the SPI clock divisor hardcoded to a value of 2, this allows the programmer to explicitly set it to any value they'd like. In both instances, simply set the desired clock divisor PRIOR to initializing the WiFi module: set_sys_clock_khz(48000, true); // cyw43 init will hang or fail, see issue #1392 cyw43_spi_set_clock_divisor(1,0); // Speed up the SPI bus so it will work if (cyw43_arch_init_with_country(CYW43_COUNTRY_USA)) { ... or set_sys_clock_khz(266000, true); // cyw43 init will fail for reasons similar to #1297 cyw43_spi_set_clock_divisor(4,0); // Slow down the SPI bus if (cyw43_arch_init_with_country(CYW43_COUNTRY_USA)) { ...

🎉 1

lurch commented

2023-10-22 07:20:12 -05:00

(Migrated from github.com)

I guess this is similar-but-different to #1499 ?

ai6aj commented

2023-10-22 13:05:01 -05:00

(Migrated from github.com)

It's similar to #1499, but having an actual SDK call means that Micropython users (and Lua, JS, etc.) aren't stuck with whatever SPI clock speed was chosen at compile time. That patch looks like it's entirely compatible with this one and I rather like the option to do it either way.

👍 1

peterharperuk (Migrated from github.com) reviewed

2023-10-23 04:13:39 -05:00

src/rp2_common/pico_cyw43_driver/cyw43_bus_pio_spi.c

				`@ -84,6 +84,16 @@ typedef struct {`

coudn't these be static const?

peterharperuk (Migrated from github.com) reviewed

2023-10-23 04:16:59 -05:00

src/rp2_common/pico_cyw43_driver/cyw43_bus_pio_spi.c

				`@ -84,6 +84,16 @@ typedef struct {`

I wonder if that's the right name for the function. Maybe cyw43_set_pio_clock_divisor?

ai6aj (Migrated from github.com) reviewed

2023-10-23 12:25:57 -05:00

src/rp2_common/pico_cyw43_driver/cyw43_bus_pio_spi.c

				`@ -84,6 +84,16 @@ typedef struct {`

They are set by cyw43_spi_set_clock_divisor() so they can't be const, but they should be static. I've made the changes in the latest commit.

ai6aj (Migrated from github.com) reviewed

2023-10-23 13:34:26 -05:00

src/rp2_common/pico_cyw43_driver/cyw43_bus_pio_spi.c

				`@ -84,6 +84,16 @@ typedef struct {`

Good call, done

FrankLaterza commented

2024-01-11 22:13:33 -06:00

(Migrated from github.com)

Any updates on this? Has anyone found an alternative I could use so users don't have to edit the SDK when they want to overclock their pi pico w?

peterharperuk commented

2024-01-12 05:00:42 -06:00

(Migrated from github.com)

@FrankLaterza does this change work for you?

FrankLaterza commented

2024-01-12 07:58:54 -06:00

(Migrated from github.com)

After overclocking to 400MHZ this change worked perfect for me.

kilograham (Migrated from github.com) reviewed

2024-01-12 16:30:27 -06:00

kilograham (Migrated from github.com) left a comment

Is this something that needs to be dynamic? I'd vote for making this a PICO_ define which defaults to the old values

ai6aj commented

2024-01-12 17:30:43 -06:00

(Migrated from github.com)

Is this something that needs to be dynamic? I'd vote for making this a PICO_ define which defaults to the old values

If the system clock wasn't dynamically adjustable I'd agree that it should be a simple #define, but Micropython and other VMs expose set_system_clock_mhz to the user and need to be able to adjust the SPI clock at runtime. Otherwise you end up with weird cases like "the Pico W works fine in Micropython when under- or overclocked, unless you want to use WiFi, in which case you'll have to rebuild the entire VM from scratch."

> Is this something that needs to be dynamic? I'd vote for making this a PICO_ define which defaults to the old values If the system clock wasn't dynamically adjustable I'd agree that it should be a simple #define, but Micropython and other VMs expose set_system_clock_mhz to the user and need to be able to adjust the SPI clock at runtime. Otherwise you end up with weird cases like "the Pico W works fine in Micropython when under- or overclocked, unless you want to use WiFi, in which case you'll have to rebuild the entire VM from scratch."

peterharperuk commented

2024-08-09 04:57:48 -05:00

(Migrated from github.com)

Your change has more or less been implemented. You need to define CYW43_PIO_CLOCK_DIV_DYNAMIC and then cyw43_set_pio_clock_divisor is available. Or else at build time you can use CYW43_PIO_CLOCK_DIV_INT and CYW43_PIO_CLOCK_DIV_FRAC. This is all in pico_cyw43_driver. There's an example here https://github.com/raspberrypi/pico-examples/blob/master/pico_w/wifi/blink/picow_blink_slow_clock.c