#2082 breaks user firmware update

[scaprile]

We have a firmware update in which we copy a new image onto an older image (erase, write), then reset. We've been running this with SDK 2.0.0 in evb-pico2-alike boards.
With #2082 in SDK 2.1.0, flash_range_erase() adds this:

pico-sdk/src/rp2_common/hardware_flash/flash.c

Lines 141 to 143 in 389dc03

	#if PICO_RP2350
	flash_rp2350_restore_qmi_cs1(&qmi_save);
	#endif

Here, flash_rp2350_restore_qmi_cs1() calls flash_devinfo_get_cs_size(), which calls flash_devinfo_ptr(), function that is not declared as __no_inline_not_in_flash_func and hence is in flash.
Should this function somehow not have been erased, yet, it will call rom_data_lookup(), also resident in flash.

Is this change actually necessary when one is going to erase everything ?
Is there a workaround that I can easily apply ? (instead of patching flash_range_erase() or writing our own version of it, of course)

Regards

[peterharperuk]

Sorry Luke - I might need some guidance on this.

[will-v-pi]

I think this was fixed in #2116 - with that fix the rom_data_lookup is inlined, so it gets inlined into flash_devinfo_get_cs_size thus preventing flash_rp2350_restore_qmi_cs1 from touching the flash

[scaprile]

Nope, it gets inlined into flash_devinfo_ptr() but this function is still in flash

pico-sdk/src/rp2_common/hardware_flash/flash.c

Lines 288 to 296 in 3d746b3

	#if !PICO_RP2040
	// This is a static symbol because the layout of FLASH_DEVINFO is liable to change from device to
	// device, so fields must have getters/setters.
	static io_rw_16 * flash_devinfo_ptr(void) {
	// Note the lookup returns a pointer to a 32-bit pointer literal in the ROM
	io_rw_16 **p = (io_rw_16 **) rom_data_lookup_inline(ROM_DATA_FLASH_DEVINFO16_PTR);
	assert(p);
	return *p;
	}

pico-sdk/src/rp2_common/hardware_flash/flash.c

Lines 305 to 310 in 3d746b3

	// This is a RAM function because may be called during flash programming to enable save/restore of
	// QMI window 1 registers on RP2350:
	flash_devinfo_size_t __no_inline_not_in_flash_func(flash_devinfo_get_cs_size)(uint cs) {
	invalid_params_if(HARDWARE_FLASH, cs > 1);
	io_ro_16 *devinfo = (io_ro_16 *) flash_devinfo_ptr();
	if (cs == 0u) {

declaring flash_devinfo_ptr() as __no_inline_not_in_flash_func does the trick:

diff --git a/src/rp2_common/hardware_flash/flash.c b/src/rp2_common/hardware_flash/flash.c
index 26d47a6..adae078 100644
--- a/src/rp2_common/hardware_flash/flash.c
+++ b/src/rp2_common/hardware_flash/flash.c
@@ -288,7 +288,7 @@ void flash_get_unique_id(uint8_t *id_out) {
 #if !PICO_RP2040
 // This is a static symbol because the layout of FLASH_DEVINFO is liable to change from device to
 // device, so fields must have getters/setters.
-static io_rw_16 * flash_devinfo_ptr(void) {
+static io_rw_16 * __no_inline_not_in_flash_func(flash_devinfo_ptr)(void) {
     // Note the lookup returns a pointer to a 32-bit pointer literal in the ROM
     io_rw_16 **p = (io_rw_16 **) rom_data_lookup_inline(ROM_DATA_FLASH_DEVINFO16_PTR);
     assert(p);

Didn't take a look at the other functions, so there might be some others needing this massage.
Regards

[kilograham]

fix merged into develop