From 641899ef2db06d90e8c44da80e6ea047839adfa0 Mon Sep 17 00:00:00 2001 From: erikkaashoek Date: Mon, 28 Sep 2026 09:28:35 +0200 Subject: [PATCH] fix: SDSC card size, browser stack use and firmware update build checks Found by an adversarial review of last week's changes. - disk_ioctl(GET_SECTOR_COUNT): add READ_BL_LEN and C_SIZE_MULT of CSD v1 cards (<= 2 GB) instead of ORing them. USB DISK mode reported half or a quarter of the card size. Mask the reserved bits of C_SIZE for CSD v2. - fw_update_ui() is not inlined into browser_open_file(): its locals enlarged the browser's stack frame from 280 to 416 bytes for every file type, leaving little headroom on the sweep thread stack. - Firmware update: the image trailer is a C constant built from the fw_update.h values, the linker script only places it; the RAM range check uses the linker's RAM symbols; SD_SPI_SPEED and SD_INIT_SPI_SPEED move to spi.h and are shared with the flasher, with a build check that the LCD uses the same speed; the linker script checks that the flasher's write limit (SAVE_CONFIG_ADDR) is the end of flash0. --- STM32F303xC.ld | 9 ++++----- fw_update.c | 28 ++++++++++++++++++++-------- ili9341.c | 14 +++++++------- spi.h | 4 ++++ ui.c | 3 ++- 5 files changed, 37 insertions(+), 21 deletions(-) diff --git a/STM32F303xC.ld b/STM32F303xC.ld index edd6c34..62c1d44 100644 --- a/STM32F303xC.ld +++ b/STM32F303xC.ld @@ -160,13 +160,10 @@ INCLUDE rules_data.ld SECTIONS { - /* Firmware image trailer (fw_update.h fw_trailer_t), must be the last data in flash: - end magic "FEND", image size, CRC32 (0xFFFFFFFF = not set) */ + /* Firmware image trailer (fw_trailer in fw_update.c), must be the last data in flash */ .fwtrailer : ALIGN(4) { - LONG(0x444E4546) - LONG(__fw_image_size__) - LONG(0xFFFFFFFF) + KEEP(*(.fwtrailer)) __fw_image_end__ = .; } > flash0 @@ -182,3 +179,5 @@ __fw_image_size__ = __fw_image_end__ - ORIGIN(flash0); ASSERT(ADDR(.fwinfo) == ORIGIN(flash0) + SIZEOF(vectors), "firmware header must follow the vectors") ASSERT(LOADADDR(.fwtrailer) >= LOADADDR(.data) + SIZEOF(.data), "firmware trailer must be the last data in flash") ASSERT(LOADADDR(.fwtrailer) >= LOADADDR(.ram4_init) + SIZEOF(.ram4_init), "firmware trailer must be the last data in flash") +/* The SD firmware update (fw_update.c) programs flash up to FWU_APP_END = SAVE_CONFIG_ADDR (nanovna.h) */ +ASSERT(!DEFINED(__fwu_app_end__) || __fwu_app_end__ == ORIGIN(flash0) + LENGTH(flash0), "FWU_APP_END (SAVE_CONFIG_ADDR) must be the end of flash0") diff --git a/fw_update.c b/fw_update.c index 0d51c52..8344e96 100644 --- a/fw_update.c +++ b/fw_update.c @@ -25,11 +25,13 @@ #endif //***************************************************************************** -// Image header of this firmware, placed after the vector table by the linker script +// Image header and trailer of this firmware. The linker script places the header after the +// vector table and the trailer at the end of the image. //***************************************************************************** extern const char __fw_image_size__[]; // linker script symbol, its address is the image size _Static_assert(sizeof(VERSION) <= sizeof(((fw_header_t *)0)->version), "VERSION too long for fw_header_t"); +_Static_assert(sizeof(fw_trailer_t) == 12, "fw_trailer_t must be 12 bytes"); __attribute__((used, section(".fwinfo"))) const fw_header_t fw_header = { @@ -43,6 +45,13 @@ const fw_header_t fw_header = { .version = VERSION, }; +__attribute__((used, section(".fwtrailer"))) +const fw_trailer_t fw_trailer = { + .magic = FW_TRAILER_MAGIC, + .image_size = (uint32_t)__fw_image_size__, + .crc = FW_CRC_UNSET, +}; + //***************************************************************************** // Limits and layout //***************************************************************************** @@ -51,9 +60,14 @@ const fw_header_t fw_header = { #define FWU_MAX_PAGES ((FWU_APP_END - FWU_APP_START) / FLASH_PAGESIZE) #define FWU_MAX_EXTENTS 64 // contiguous sector runs of the image file #define FWU_SECTOR_SIZE 512 -#define FWU_RAM_START 0x20000000U -#define FWU_RAM_END (0x20000000U + 40*1024) #define FWU_MIN_BATTERY_MV 3500 + +extern const char __ram0_start__[], __ram0_end__[]; // RAM of this MCU, from the linker script + +// FWU_APP_END as a symbol, STM32F303xC.ld checks that it is the end of flash0 +#define FWU_STR_(x) #x +#define FWU_STR(x) FWU_STR_(x) +__asm__(".global __fwu_app_end__\n\t.set __fwu_app_end__, " FWU_STR(FWU_APP_END)); #define FWU_PAGE_BUF ((uint8_t *)spi_buffer) // 2 KB page buffer in SRAM (4 KB spi_buffer) #define FWU_PAGE0_BUF ((uint8_t *)spi_buffer + FLASH_PAGESIZE) // page 0, read before the erase starts @@ -223,8 +237,6 @@ FWU_TEXT static void fwu_bar(uint32_t done, uint32_t color) { #define FWU_CMD55 (0x40 + 55) // APP_CMD #define FWU_CMD58 (0x40 + 58) // READ_OCR #define FWU_ACMD41 (0x40 + 41) // SD_SEND_OP_COND -#define FWU_SD_SPEED SPI_BR_DIV2 // as SD_SPI_SPEED and LCD_SPI_SPEED in ili9341.c -#define FWU_SD_INIT_SPEED SPI_BR_DIV256 // as SD_INIT_SPI_SPEED in ili9341.c FWU_TEXT static bool fwu_sd_wait_ready(uint32_t ms) { uint32_t t0 = FWU_NOW(); @@ -265,7 +277,7 @@ FWU_TEXT static bool fwu_sd_init(void) { fwu_spi_flush(); FWU_LCD_CS_HIGH(); FWU_SD_CS_HIGH(); - SPI_BR_SET(SPI1, FWU_SD_INIT_SPEED); + SPI_BR_SET(SPI1, SD_INIT_SPI_SPEED); for (int i = 0; i < 10; i++) // 80 clocks with the card not selected fwu_spi_xfer(0xFF); FWU_SD_CS_LOW(); @@ -302,7 +314,7 @@ FWU_TEXT static bool fwu_sd_init(void) { } FWU_SD_CS_HIGH(); fwu_spi_xfer(0xFF); - SPI_BR_SET(SPI1, FWU_SD_SPEED); + SPI_BR_SET(SPI1, SD_SPI_SPEED); if (ok) fwu_job.byte_addr = byte_addr; return ok; @@ -652,7 +664,7 @@ int fwu_read_header(const char *path, fw_header_t *hdr) { size > FWU_APP_END - FWU_APP_START || size > fsize) return FWU_E_SIZE; uint32_t sp = w[0], reset = w[1]; - if (sp <= FWU_RAM_START || sp > FWU_RAM_END || !(reset & 1) || + if (sp <= (uint32_t)__ram0_start__ || sp > (uint32_t)__ram0_end__ || !(reset & 1) || reset < FWU_APP_START + hofs + hdr->hdr_size || reset >= FWU_APP_START + size) return FWU_E_VECTOR; return FWU_OK; diff --git a/ili9341.c b/ili9341.c index 7517e87..97bd724 100644 --- a/ili9341.c +++ b/ili9341.c @@ -70,10 +70,10 @@ #define __USE_DISPLAY_DMA_RX__ #endif -// Define SD SPI speed on work -#define SD_SPI_SPEED SPI_BR_DIV2 -// Define SD SPI speed on initialization (100-400kHz need) -#define SD_INIT_SPI_SPEED SPI_BR_DIV256 +// SD_SPI_SPEED and SD_INIT_SPI_SPEED are in spi.h, the SD firmware update uses them too +#if defined(__SD_FIRMWARE_UPDATE__) && (LCD_SPI_SPEED != SD_SPI_SPEED) +#error "fw_update.c draws its progress bar at the SD card speed, LCD_SPI_SPEED must equal SD_SPI_SPEED" +#endif // Disable DMA rx on disabled DMA tx #ifdef __USE_DISPLAY_DMA__ @@ -1754,12 +1754,12 @@ DRESULT disk_ioctl(BYTE pdrv, BYTE cmd, void* buff) { if ((SD_SendCmd(CMD9, 0) == 0) && SD_RxDataBlock(csd, 16, SD_TOKEN_START_BLOCK)) { uint32_t n, csize; if ((csd[0] >> 6) == 1) { // SDC V2 - csize = ((uint32_t)csd[7]<<16)|((uint32_t)csd[8]<< 8)|((uint32_t)csd[9]<< 0); + csize = ((uint32_t)(csd[7]&0x3F)<<16)|((uint32_t)csd[8]<< 8)|((uint32_t)csd[9]<< 0); n = 10; } - else { // MMC or SDC V1 + else { // MMC or SDC V1: (C_SIZE+1) << (READ_BL_LEN + C_SIZE_MULT + 2) bytes csize = ((uint32_t)csd[8]>>6)|((uint32_t)csd[7]<<2)|((uint32_t)(csd[6]&0x03)<<10); - n = ((csd[5]&0x0F)|((csd[10]&0x80)>>7)|((csd[9]&0x03)<<1)) + 2 - 9; + n = (csd[5]&0x0F) + (((csd[10]&0x80)>>7)|((csd[9]&0x03)<<1)) + 2 - 9; } *(uint32_t*)buff = (csize+1)<CR1 = (spi->CR1& ~(SPI_CR1_BR))|br) +// SD card SPI speed on work and on initialization (100-400kHz need), for ili9341.c and fw_update.c +#define SD_SPI_SPEED SPI_BR_DIV2 +#define SD_INIT_SPI_SPEED SPI_BR_DIV256 + //***************************************************** // SPI bus activity macros //***************************************************** diff --git a/ui.c b/ui.c index fe25476..b54ee49 100644 --- a/ui.c +++ b/ui.c @@ -4544,7 +4544,8 @@ static bool fw_update_confirm(const fw_header_t *hdr) { } // Check the file, ask, check the image and update. Returns only on cancel or error. -static void fw_update_ui(const char *fname) { +// Not inlined: its locals would otherwise enlarge the stack frame of browser_open_file() for every file type +__attribute__((noinline)) static void fw_update_ui(const char *fname) { fw_header_t hdr; int err = fwu_begin(); if (err == FWU_OK)