ERROR: FLASH CLKDIV not calculated properly

[GitHubsSilverBullet]

Good morning.
Long story short: I'm rather sure that there's a severe error in the new automagic flash divider calculation in ›ports/rp2/rp2_flash.c‹
One that rears its ugly head only under certain circumstances. Bit me, nevertheless.

Currently the formula assumes a flash clock of 133MHz which is wrong.
The standard(original) PICO board uses a Winbond W25Q16. While it apparently tolerates higher clock frequencies way above the declared 80MHz clock operation, other FLASH chips do not. Which results in hanging systems once a certain CPU frequency is requested.
I'd like to propose a change which not only recognizes the limitations declared in the datasheet, it also corrects another (quite minor) error in the original formula.
(Plus I checked some other datasheet in which either the same max. frequency or a slightly higher one is given. So those 80MHz should be a safe bet for the moment.)

diff --git a/ports/rp2/rp2_flash.c b/ports/rp2/rp2_flash.c
index a9beabf05..78274d7be 100644
--- a/ports/rp2/rp2_flash.c
+++ b/ports/rp2/rp2_flash.c
@@ -104,9 +104,10 @@ static bool use_multicore_lockout(void) {
 // and core1 locked out if relevant.
 static void __no_inline_not_in_flash_func(rp2_flash_set_timing_internal)(int clock_hz) {
 
-    // Use the minimum divisor assuming a 133MHz flash.
-    const int max_flash_freq = 133000000;
-    int divisor = (clock_hz + max_flash_freq - 1) / max_flash_freq;
+    // as per winbond datasheet W25Q80/16/32   80MHz clock operation
+    // smallest divisor is 1 for frequencies up to and including 80MHz
+    const int max_flash_freq = 80000000;
+    int divisor = 1 + ((clock_hz-1) / max_flash_freq);
 
     #if PICO_RP2350
     // Make sure flash is deselected - QMI doesn't appear to have a busy flag(!)

Maybe someone can have a look at it and verify.

Thanks,
Good night.

[robert-hh]

The standard(original) PICO board uses a Winbond W25Q16. While it apparently tolerates higher clock frequencies way above the declared 80MHz clock operation, other FLASH chips do not.

There are several variants of the Winbond W25Q16. For instance:

the W25Q16DV is at 3.3V good for 104 MHz. 80Mhz only for command 0x03.
the W25Q16JV is at 3.3V good for 133MHz. 50 MHz only for command 0x03. Same spec for W26Q32JV and W25Q64JV.

The RP2 PICO uses according to the schematic a W25Q16JV device and does not use the command 0x03. So 133Mhz is within specs. Looking at the original boards I have here, that matches the schematics.

[GitHubsSilverBullet]

Still leaves all the other FLASH chips unaccounted for. (And my original winbond datasheet mentions 80MHz and nowhere a higher clock rate. Maybe newer ones are different, will search and investigate)
Anyway, I have a handful of boards with a different FLASH which simply lockup entirely.
Only applying the changes and rebuilding the firmware fixed that.

[GitHubsSilverBullet]

Just saw that the suggested change has found its way into the rp2_flash.c and that MICROPY_HW_FLASH_MAX_FREQ is now configurable.

Thanks!