Flash operations overwrite PSRAM QMI configuration registers #1983

New issue

Closed

opened 2024-10-12 14:30:03 -05:00 by earlephilhower · 6 comments

earlephilhower commented

2024-10-12 14:30:03 -05:00

(Migrated from github.com)

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 https://github.com/earlephilhower/arduino-pico/issues/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.

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 https://github.com/earlephilhower/arduino-pico/issues/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.

👍 1 👀 1

asumagic commented

2024-10-22 03:29:50 -05:00

(Migrated from github.com)

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.

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 `#ifdef`s in the SDK.

asumagic commented

2024-10-22 04:33:13 -05:00

(Migrated from github.com)

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.

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 commented

2024-10-22 11:25:41 -05:00

(Migrated from github.com)

Looks like CircuitPython also does the manual save-and-restore around their flash ops:
github.com/adafruit/circuitpython@45e0cf10cc/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.

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](https://forums.raspberrypi.com/viewtopic.php?t=378249).

Wren6991 commented

2024-11-04 04:51:19 -06:00

(Migrated from github.com)

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.

> 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 commented

2024-11-04 06:35:16 -06:00

(Migrated from github.com)

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.

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 commented

2024-11-21 16:52:46 -06:00

(Migrated from github.com)