From 75f8249405ea32905e2d3790d27f3d3a038be2a9 Mon Sep 17 00:00:00 2001 From: Marco Casaroli Date: Wed, 7 Oct 2026 09:15:59 +0200 Subject: [PATCH] arch/arm/rp23xx: Erase and program the flash in small steps. The flash MTD driver disabled interrupts for a whole request. A multi-block erase or a large write kept them off for seconds. Erase one 64K block (or one 4K sector where the range is not block aligned) and program one 256 byte page per step. Enable interrupts and release the other core between steps. A single block erase is still long, but that is the limit of the flash. Also, on SMP: - Do not send the pause call to the CPU that does the operation. nxsched_smp_call_single_async() runs it at once on that CPU. - Keep the isolation data in a static, not on the stack. The other CPU spins on it while the flash is busy. Assisted-by: Claude Code:claude-opus-5-5 Signed-off-by: Marco Casaroli --- Documentation/platforms/arm/rp23xx/index.rst | 6 +- arch/arm/src/rp23xx/Kconfig | 5 +- arch/arm/src/rp23xx/rp23xx_flash_mtd.c | 144 ++++++++++++------- 3 files changed, 97 insertions(+), 58 deletions(-) diff --git a/Documentation/platforms/arm/rp23xx/index.rst b/Documentation/platforms/arm/rp23xx/index.rst index 69345e5af5e..81a9fb5e4f3 100644 --- a/Documentation/platforms/arm/rp23xx/index.rst +++ b/Documentation/platforms/arm/rp23xx/index.rst @@ -326,8 +326,10 @@ offset fails at boot instead of corrupting the running firmware. Erase and program go through the bootrom flash routines. Because those operations stall instruction fetch from the same flash, the driver runs them -from SRAM with interrupts disabled and, on SMP builds, the other core parked; -expect interrupt latency to suffer for the duration of a write. Afterwards the +from SRAM with interrupts disabled and, on SMP builds, the other core parked. +It does so for one 64K block erase or one 256 byte page program at a time, and +enables interrupts between them; expect interrupt latency to suffer for that +long (a block erase can take hundreds of milliseconds). Afterwards the QSPI interface is put back into execute-in-place mode -- by default with a copy of the XIP setup function that the bootrom leaves in boot RAM, which restores the read mode found at boot, or, with diff --git a/arch/arm/src/rp23xx/Kconfig b/arch/arm/src/rp23xx/Kconfig index dc3af5774ce..1f166d81cd0 100644 --- a/arch/arm/src/rp23xx/Kconfig +++ b/arch/arm/src/rp23xx/Kconfig @@ -1220,8 +1220,9 @@ config RP23XX_FLASH_MTD Note that erasing or programming this flash stalls all instruction fetches from it. The driver therefore runs those operations from - SRAM with interrupts disabled and the other core parked, which will - visibly affect interrupt latency for the duration. + SRAM with interrupts disabled and the other core parked, one 64K + block erase or one 256 byte page program at a time. This will + visibly affect interrupt latency. if RP23XX_FLASH_MTD diff --git a/arch/arm/src/rp23xx/rp23xx_flash_mtd.c b/arch/arm/src/rp23xx/rp23xx_flash_mtd.c index d229dca87f1..07f1484435c 100644 --- a/arch/arm/src/rp23xx/rp23xx_flash_mtd.c +++ b/arch/arm/src/rp23xx/rp23xx_flash_mtd.c @@ -39,7 +39,8 @@ * boot by the linker script, which is the same mechanism the Pico SDK * spells __not_in_flash_func(), * 2. runs with interrupts disabled, because an ISR vector or handler - * living in flash would be fetched mid-erase, and + * living in flash would be fetched mid-erase -- one 64K block erase + * or one 256 byte page program at a time, and * 3. parks the other core, because it is very likely executing from * flash as well. * @@ -153,6 +154,16 @@ struct rp23xx_flash_dev_s mutex_t lock; }; +/* One flash operation. It is static, so that it is in SRAM. */ + +struct rp23xx_flash_op_s +{ + CODE void (*func)(FAR struct rp23xx_flash_op_s *op); + uint32_t addr; + FAR const uint8_t *data; + size_t count; +}; + /* QSPI state saved over a flash operation */ struct rp23xx_qspi_state_s @@ -232,6 +243,12 @@ static struct rp23xx_flash_dev_s g_flash_dev = static bool g_initialized = false; +static struct rp23xx_flash_op_s g_flash_op; + +#ifdef CONFIG_SMP +static struct smp_isolation_s g_smp_isolation; +#endif + static struct { connect_internal_flash_f connect_internal_flash; @@ -340,11 +357,11 @@ static void enter_smp_isolation(struct smp_isolation_s *const data) spin_lock(&cpu_data->cpu_wait); spin_lock(&cpu_data->cpu_pause); spin_unlock(&cpu_data->cpu_resume); - } - nxsched_smp_call_init(&cpu_data->call_data, pause_cpu_handler, - cpu_data); - nxsched_smp_call_single_async(other_cpuid, &cpu_data->call_data); + nxsched_smp_call_init(&cpu_data->call_data, pause_cpu_handler, + cpu_data); + nxsched_smp_call_single_async(other_cpuid, &cpu_data->call_data); + } } /* Wait until every other CPU has actually parked */ @@ -481,24 +498,18 @@ static void RAM_CODE(rp23xx_flash_end)(struct rp23xx_qspi_state_s *state) * Name: do_erase * * Description: - * Erase a byte range. Runs from RAM with interrupts already disabled and - * the other core already parked. + * Erase one sector or block. Runs from RAM with interrupts disabled and + * the other core parked. * ****************************************************************************/ -static void RAM_CODE(do_erase)(uint32_t addr, size_t count) +static void RAM_CODE(do_erase)(FAR struct rp23xx_flash_op_s *op) { struct rp23xx_qspi_state_s state; rp23xx_flash_begin(&state); - - /* The bootrom erases whole 64K blocks where address and length allow it - * and falls back to 4K sectors otherwise. - */ - - g_rom.flash_range_erase(addr, count, FLASH_BLOCK_SIZE, + g_rom.flash_range_erase(op->addr, op->count, FLASH_BLOCK_SIZE, FLASH_BLOCK_ERASE_CMD); - rp23xx_flash_end(&state); } @@ -506,32 +517,59 @@ static void RAM_CODE(do_erase)(uint32_t addr, size_t count) * Name: do_write ****************************************************************************/ -static void RAM_CODE(do_write)(uint32_t addr, const uint8_t *data, - size_t count) +static void RAM_CODE(do_write)(FAR struct rp23xx_flash_op_s *op) { struct rp23xx_qspi_state_s state; rp23xx_flash_begin(&state); - g_rom.flash_range_program(addr, data, count); + g_rom.flash_range_program(op->addr, op->data, op->count); rp23xx_flash_end(&state); } +/**************************************************************************** + * Name: rp23xx_flash_run + * + * Description: + * Run g_flash_op with interrupts disabled and the other core parked. + * The caller holds the device lock. + * + ****************************************************************************/ + +static void rp23xx_flash_run(void) +{ + irqstate_t flags; + +#ifdef CONFIG_SMP + init_smp_isolation(&g_smp_isolation); + enter_smp_isolation(&g_smp_isolation); +#endif + + flags = enter_critical_section(); + g_flash_op.func(&g_flash_op); + leave_critical_section(flags); + +#ifdef CONFIG_SMP + leave_smp_isolation(&g_smp_isolation); +#endif +} + /**************************************************************************** * Name: rp23xx_flash_erase + * + * Description: + * Erase one 64K block or 4K sector at a time, so that interrupts are + * disabled for one block erase at most. + * ****************************************************************************/ static int rp23xx_flash_erase(struct mtd_dev_s *dev, off_t startblock, size_t nblocks) { struct rp23xx_flash_dev_s *priv = (struct rp23xx_flash_dev_s *)dev; - irqstate_t flags; + uint32_t addr; + uint32_t end; int ret; -#ifdef CONFIG_SMP - struct smp_isolation_s smp_isolation; - init_smp_isolation(&smp_isolation); -#endif - if (startblock < 0 || startblock + nblocks > FS_SECTORS) { return -EINVAL; @@ -545,20 +583,23 @@ static int rp23xx_flash_erase(struct mtd_dev_s *dev, off_t startblock, return ret; } -#ifdef CONFIG_SMP - enter_smp_isolation(&smp_isolation); -#endif + addr = FS_OFFSET + startblock * FLASH_SECTOR_SIZE; + end = addr + nblocks * FLASH_SECTOR_SIZE; - flags = enter_critical_section(); + while (addr < end) + { + g_flash_op.func = do_erase; + g_flash_op.addr = addr; + g_flash_op.count = FLASH_SECTOR_SIZE; - do_erase(FS_OFFSET + startblock * FLASH_SECTOR_SIZE, - nblocks * FLASH_SECTOR_SIZE); + if ((addr % FLASH_BLOCK_SIZE) == 0 && end - addr >= FLASH_BLOCK_SIZE) + { + g_flash_op.count = FLASH_BLOCK_SIZE; + } - leave_critical_section(flags); - -#ifdef CONFIG_SMP - leave_smp_isolation(&smp_isolation); -#endif + rp23xx_flash_run(); + addr += g_flash_op.count; + } nxmutex_unlock(&priv->lock); return nblocks; @@ -601,20 +642,20 @@ static ssize_t rp23xx_flash_bread(struct mtd_dev_s *dev, off_t startblock, /**************************************************************************** * Name: rp23xx_flash_bwrite + * + * Description: + * Program one page at a time, so that interrupts are disabled for one + * page program at most. + * ****************************************************************************/ static ssize_t rp23xx_flash_bwrite(struct mtd_dev_s *dev, off_t startblock, size_t nblocks, const uint8_t *buffer) { struct rp23xx_flash_dev_s *priv = (struct rp23xx_flash_dev_s *)dev; - irqstate_t flags; + size_t i; int ret; -#ifdef CONFIG_SMP - struct smp_isolation_s smp_isolation; - init_smp_isolation(&smp_isolation); -#endif - if (startblock < 0 || startblock + nblocks > FS_PAGES) { return -EINVAL; @@ -626,20 +667,15 @@ static ssize_t rp23xx_flash_bwrite(struct mtd_dev_s *dev, off_t startblock, return ret; } -#ifdef CONFIG_SMP - enter_smp_isolation(&smp_isolation); -#endif + for (i = 0; i < nblocks; i++) + { + g_flash_op.func = do_write; + g_flash_op.addr = FS_OFFSET + (startblock + i) * FLASH_PAGE_SIZE; + g_flash_op.data = buffer + i * FLASH_PAGE_SIZE; + g_flash_op.count = FLASH_PAGE_SIZE; - flags = enter_critical_section(); - - do_write(FS_OFFSET + startblock * FLASH_PAGE_SIZE, buffer, - nblocks * FLASH_PAGE_SIZE); - - leave_critical_section(flags); - -#ifdef CONFIG_SMP - leave_smp_isolation(&smp_isolation); -#endif + rp23xx_flash_run(); + } finfo("write page %ld count %zu\n", (long)startblock, nblocks);