Flash operations overwrite PSRAM QMI configuration registers

[earlephilhower]

PSRAM not accessible by the core after a flash_range_erase or flash_range_program.

Inspection of the QMI interface before and after a call to these SDK functions shows that the 2nd memory region control register (PSRAM, qmi_hw->m[1]) have had their values modified, rendering the interface inoperative. If the original values are restored by the application manually, communication succeeds and PSRAM data is accessible.

In the instance of the PSRAM on the Pimoroni PGA2350, we see the following behavior:

Before flash write

qmi_hw->m[1].timing = 61a48102
qmi_hw->m[1].rfmt   = 000612aa
qmi_hw->m[1].rcmd   = 000000eb
qmi_hw->m[1].wfmt   = 000012aa
qmi_hw->m[1].wcmd   = 00000038
xip_ctrl_hw->ctrl   = 00000883

After flash_range_erase / flash_range_program

qmi_hw->m[1].timing = 60007203
qmi_hw->m[1].rfmt   = 000492a8
qmi_hw->m[1].rcmd   = 000000eb
qmi_hw->m[1].wfmt   = 000012aa
qmi_hw->m[1].wcmd   = 00000038
xip_ctrl_hw->ctrl   = 00000883

A more detailed description and test code (Arduino based, sorry, because PSRAM setup is not in the SDK yet) is being tracked in earlephilhower/arduino-pico#2537 .

As a workaround, by using GCC to wrap the flash functions and manually saving/restoring the registers, we're able to both access PSRAM and write to flash. I haven't gone through the RP2350 ROM code, but I would guess as part of the XIP restart after a flash command it is setting what it things are "safe" values to the QMI regions which is great for chip reset, but not so good for when things are already configured.

[asumagic]

I suspect this is due to flash_enable_xip_via_boot2(); in flash_range_erase/flash_range_program, which AFAICT boots the boot2 which resets QMI timings. I encountered this earlier but you beat me to making an issue :)

An alternative is to copy the code from flash_range_erase/flash_range_program on RP2350 (making sure to use __no_inline_not_in_flash_func) and ripping out flash_exit_xip_func(); and flash_enable_xip_via_boot2();, because I think it is not a required step on RP2350?
I could be wrong, but I haven't encountered strange behavior by doing this.
If true, the fix might be a matter of a few #ifdefs in the SDK.

[asumagic]

Also, I would suggest to update the documentation for the hardware_flash lib functions accordingly, as it currently doesn't hint at this behavior as far as I could tell.

[earlephilhower]

Looks like CircuitPython also does the manual save-and-restore around their flash ops:
https://github.com/adafruit/circuitpython/blob/45e0cf10ccffeb4f5e0bfcb42c8823b648a5f732/ports/raspberrypi/supervisor/internal_flash.c#L49-L67

I'll look into their cache clean code as well to see if it can help with my other PSRAM/flash cache invalidate issue on the RPI forums.

[Wren6991]

I suspect this is due to flash_enable_xip_via_boot2(); in flash_range_erase/flash_range_program, which AFAICT boots the boot2 which resets QMI timings

It's actually the other way round: the default boot2 (XIP setup func) does not configure QMI window 1 (there was an expectation at some point that it might), but the bootrom configures both windows to safe defaults (slow 03h) at the start of flash_exit_xip().

I think it would be reasonable for the SDK to save/restore the QMI registers for window 1 over the ROM calls, but this is actually a problem if window 1 received an XIP exit sequence (enabled by FLASH_DEVINFO_CS1_SIZE being nonzero), since the old register settings will no longer be valid if they used a non-serial-prefixed mode.

[Wren6991]

The best-effort fix might be to save/restore qmi_hw->m[1] across ROM calls if and only if FLASH_DEVINFO_CS1_SIZE is zero.

That would avoid trashing people's registers if they are ignoring the ROM support for the second chip select, but avoid breaking the ROM's reinitialisation of the m[1] registers in the case where the ROM has issued an XIP exit sequence to the second chip select.

The ideal fix is still to (re-) initialise both QSPI devices in the XIP setup, but that's a more substantial piece of work to cover all of the possible cases and combinations.

[kilograham]

merged into develop