Merge pull request #18608 from benpicco/mtd_flashpage-aux

drivers/mtd_flashpage: allow to define AUX slot on flash

bootloaders/riotboot_common.mk

@@ -26,4 +26,4 @@ include $(RIOTBASE)/Makefile.include
 # limit riotboot bootloader size
 # TODO: Manage to set this variable for boards which already embed a
 # bootloader, currently it will be overwritten
-ROM_LEN := $(RIOTBOOT_LEN)
+FW_ROM_LEN := $(RIOTBOOT_LEN)

cpu/cortexm_common/Makefile.features

@@ -7,6 +7,7 @@ FEATURES_PROVIDED += cpu_core_cortexm
 FEATURES_PROVIDED += dbgpin
 FEATURES_PROVIDED += libstdcpp
 FEATURES_PROVIDED += newlib
+FEATURES_PROVIDED += periph_flashpage_aux
 FEATURES_PROVIDED += periph_pm
 FEATURES_PROVIDED += puf_sram
 FEATURES_PROVIDED += picolibc

cpu/cortexm_common/ldscripts/cortexm_base.ld

@@ -41,6 +41,7 @@ _backup_ram_len = DEFINED( _backup_ram_len ) ? _backup_ram_len : 0x0 ;
 MEMORY
 {
     bkup_ram (w!rx) : ORIGIN = _backup_ram_start_addr, LENGTH = _backup_ram_len
+    rom_aux  (rx)   : ORIGIN = _rom_start_addr + _slot_aux_offset, LENGTH = _slot_aux_len
 }
 
 /* Section Definitions */

cpu/cortexm_common/ldscripts/cortexm_rom_offset.ld

@@ -21,6 +21,8 @@
  */
 
 _rom_offset = DEFINED( _rom_offset ) ? _rom_offset : 0x0;
-_fw_rom_length = DEFINED( _fw_rom_length ) ? _fw_rom_length : _rom_length - _rom_offset;
+_slot_aux_len = DEFINED( _slot_aux_len ) ? _slot_aux_len  : 0x0;
+_fw_rom_length = DEFINED( _fw_rom_length ) ? _fw_rom_length : _rom_length - _rom_offset - _slot_aux_len;
 
-ASSERT((_fw_rom_length <= _rom_length - _rom_offset), "Specified firmware size does not fit in ROM");
+ASSERT(_fw_rom_length <= _rom_length - _rom_offset - _slot_aux_len, "Specified firmware size does not fit in ROM");
+ASSERT(_fw_rom_length <= _rom_length, "Specified firmware size does not fit in ROM");

cpu/riscv_common/Makefile.features

@@ -8,6 +8,7 @@ FEATURES_PROVIDED += cpp
 FEATURES_PROVIDED += libstdcpp
 FEATURES_PROVIDED += newlib
 FEATURES_PROVIDED += periph_coretimer
+FEATURES_PROVIDED += periph_flashpage_aux
 FEATURES_PROVIDED += puf_sram
 FEATURES_PROVIDED += rust_target
 FEATURES_PROVIDED += ssp

cpu/riscv_common/Makefile.include

@@ -14,10 +14,8 @@ ifneq (,$(ROM_START_ADDR)$(RAM_START_ADDR)$(ROM_LEN)$(RAM_LEN))
   LINKFLAGS += $(LINKFLAGPREFIX)--defsym=_ram_start_addr=$(RAM_START_ADDR)
   LINKFLAGS += $(LINKFLAGPREFIX)--defsym=_rom_length=$(ROM_LEN)
   LINKFLAGS += $(LINKFLAGPREFIX)--defsym=_ram_length=$(RAM_LEN)
-  LINKFLAGS += $(if $(ROM_OFFSET),$(LINKFLAGPREFIX)--defsym=_rom_offset=$(ROM_OFFSET) \
-                                 ,$(LINKFLAGPREFIX)--defsym=_rom_offset=0x0)
-  LINKFLAGS += $(if $(FW_ROM_LEN),$(LINKFLAGPREFIX)--defsym=_fw_rom_length=$(FW_ROM_LEN) \
-                                 ,$(LINKFLAGPREFIX)--defsym=_fw_rom_length=$(ROM_LEN))
+  LINKFLAGS += $(if $(ROM_OFFSET),$(LINKFLAGPREFIX)--defsym=_rom_offset=$(ROM_OFFSET))
+  LINKFLAGS += $(if $(FW_ROM_LEN),$(LINKFLAGPREFIX)--defsym=_fw_rom_length=$(FW_ROM_LEN))
 endif
 
 ifneq (,$(ITIM_START_ADDR))

cpu/riscv_common/ldscripts/riscv.ld

@@ -25,6 +25,7 @@ MEMORY
   flash (rxai!w) : ORIGIN = _rom_start_addr + _rom_offset, LENGTH = _fw_rom_length
   ram   (wxa!ri) : ORIGIN = _ram_start_addr, LENGTH = _ram_length
   itim  (wxa!ri) : ORIGIN = _itim_start_addr, LENGTH = _itim_length
+  rom_aux (rx)   : ORIGIN = _rom_start_addr + _slot_aux_offset, LENGTH = _slot_aux_len
 }
 
 INCLUDE riscv_base.ld

cpu/riscv_common/ldscripts/riscv_vars.ld

@@ -21,3 +21,10 @@
 
 _itim_start_addr = DEFINED( _itim_start_addr ) ? _itim_start_addr : 0x0;
 _itim_length = DEFINED( _itim_length ) ? _itim_length : 0x0;
+
+_rom_offset = DEFINED( _rom_offset ) ? _rom_offset : 0x0;
+_slot_aux_len = DEFINED( _slot_aux_len ) ? _slot_aux_len  : 0x0;
+_fw_rom_length = DEFINED( _fw_rom_length ) ? _fw_rom_length : _rom_length - _rom_offset - _slot_aux_len;
+
+ASSERT(_fw_rom_length <= _rom_length - _rom_offset - _slot_aux_len, "Specified firmware size does not fit in ROM");
+ASSERT(_fw_rom_length <= _rom_length, "Specified firmware size does not fit in ROM");

dist/tools/insufficient_memory/create_makefile.ci.sh

@@ -57,6 +57,7 @@ for BOARD in $(EXTERNAL_BOARD_DIRS="" make  --no-print-directory info-boards-sup
                 -e "not within region" \
                 -e "wraps around address space" \
                 -e "overlaps section" \
+                -e "does not fit in ROM" \
                 "$TMPFILE" > /dev/null; then
             printf "${CBIG}%s${CRESET}\n" "too big"
             BOARDS="${BOARDS} ${BOARD}"

drivers/Makefile.dep

@@ -136,6 +136,10 @@ ifneq (,$(filter pcf857%,$(USEMODULE)))
   USEMODULE += pcf857x
 endif
 
+ifneq (,$(filter periph_flashpage_aux,$(FEATURES_USED)))
+  FEATURES_REQUIRED += periph_flashpage_pagewise
+endif
+
 ifneq (,$(filter periph_ptp_timer periph_ptp_speed_adjustment,$(FEATURES_USED)))
   FEATURES_REQUIRED += periph_ptp
 endif

drivers/include/mtd_flashpage.h

@@ -35,7 +35,7 @@ extern "C"
 #endif
 
 /**
- * @brief   Macro helper to initialize a mtd_t with flash-age driver
+ * @brief   Macro helper to initialize a mtd_t with flashpage driver
  */
 #define MTD_FLASHPAGE_INIT_VAL(_pages_per_sector) { \
     .base = {                                       \
@@ -47,6 +47,23 @@ extern "C"
     },                                              \
 }
 
+/**
+ * @brief   Macro helper to initialize a mtd_t with a portion of the flash
+ *
+ * @param[in] start     Start address of the flash section
+ * @param[in] len       Size of the flash section in bytes
+ */
+#define MTD_FLASHPAGE_AUX_INIT_VAL(start, len) {    \
+    .base = {                                       \
+        .driver = &mtd_flashpage_driver,            \
+        .sector_count = len / FLASHPAGE_SIZE,       \
+        .pages_per_sector = 1,                      \
+        .page_size = FLASHPAGE_SIZE,                \
+        .write_size = 1,                            \
+    },                                              \
+    .offset = start / FLASHPAGE_SIZE,               \
+}
+
 /**
  * @brief   Flashpage MTD device operations table
  */
@@ -62,6 +79,40 @@ typedef struct {
                              flash */
 } mtd_flashpage_t;
 
+#if CONFIG_SLOT_AUX_LEN || DOXYGEN
+/**
+ * @brief   MTD device representing the auxiliary flash slot
+ */
+extern mtd_flashpage_t mtd_flash_aux_slot;
+
+/**
+ * @brief   Generic MTD device backed by the auxiliary flash slot
+ */
+extern mtd_dev_t *mtd_aux;
+#endif
+
+/**
+ * @brief   Size of the auxiliary slot on the internal flash
+ *          Must align with the flash page size.
+ *
+ * @note    Don't set this config value directly!
+ *          Instead set `SLOT_AUX_LEN` in the board's `Makefile.include`
+ *
+ * This value is fixed and can not be changed in the field / via firmware
+ * updates. Changing this value requires re-flashing of riotboot.
+ *
+ */
+#ifndef CONFIG_SLOT_AUX_LEN
+#define CONFIG_SLOT_AUX_LEN 0
+#endif
+
+/**
+ * @brief   Default MTD offset for the AUX slot
+ */
+#ifndef CONFIG_SLOT_AUX_MTD_OFFSET
+#define CONFIG_SLOT_AUX_MTD_OFFSET  1
+#endif
+
 #ifdef __cplusplus
 }
 #endif

drivers/mtd_flashpage/mtd_flashpage.c

@@ -159,3 +159,13 @@ const mtd_desc_t mtd_flashpage_driver = {
     .write_page = _write_page,
     .erase_sector = _erase_sector,
 };
+
+#if CONFIG_SLOT_AUX_LEN
+mtd_flashpage_t mtd_flash_aux_slot = MTD_FLASHPAGE_AUX_INIT_VAL(CONFIG_SLOT_AUX_OFFSET,
+                                                                CONFIG_SLOT_AUX_LEN);
+MTD_XFA_ADD(mtd_flash_aux_slot, CONFIG_SLOT_AUX_MTD_OFFSET);
+mtd_dev_t *mtd_aux = &mtd_flash_aux_slot.base;
+
+static_assert(CONFIG_SLOT_AUX_OFFSET % FLASHPAGE_SIZE == 0, "AUX slot must align with page");
+static_assert(CONFIG_SLOT_AUX_LEN % FLASHPAGE_SIZE == 0, "AUX slot must align with page");
+#endif

features.yaml

@@ -695,6 +695,9 @@ groups:
       help: The Flashpage peripheral supports pagewise writing.
     - name: periph_flashpage_rwee
       help: The Flashpage peripheral is of the Read While Write.
+    - name: periph_flashpage_aux
+      help: It is possible to partition off a part of the internal flash for an
+            auxiliary slot.
 
   - title: Other Peripheral Storage Features
     features:

makefiles/features_existing.inc.mk

@@ -147,6 +147,7 @@ FEATURES_EXISTING := \
     periph_eeprom \
     periph_eth \
     periph_flashpage \
+    periph_flashpage_aux \
     periph_flashpage_in_address_space \
     periph_flashpage_pagewise \
     periph_flashpage_rwee \

sys/Makefile.include

@@ -110,6 +110,9 @@ ifneq (,$(filter scanf_float,$(USEMODULE)))
   endif
 endif
 
+# we need to include this before the riotboot include
+include $(RIOTBASE)/sys/slot_aux/Makefile.include
+
 ifneq (,$(filter riotboot,$(FEATURES_USED)))
   include $(RIOTBASE)/sys/riotboot/Makefile.include
 endif

sys/riotboot/Makefile.include

@@ -9,7 +9,7 @@ RIOTBOOT_HDR_LEN ?= 0x100
 NUM_SLOTS ?= 2
 
 # Take the whole flash minus RIOTBOOT_LEN and divide it by NUM_SLOTS
-SLOT0_LEN ?= $(shell printf "0x%x" $$((($(ROM_LEN:%K=%*1024)-$(RIOTBOOT_LEN)) / $(NUM_SLOTS))))
+SLOT0_LEN ?= $(shell printf "0x%x" $$((($(ROM_LEN:%K=%*1024)-$(RIOTBOOT_LEN)-$(SLOT_AUX_LEN)) / $(NUM_SLOTS))))
 SLOT1_LEN ?= $(SLOT0_LEN)
 SLOT0_LEN := $(SLOT0_LEN)
 SLOT1_LEN := $(SLOT1_LEN)

sys/slot_aux/Makefile.include

@@ -0,0 +1,15 @@
+# This adds an optional AUX slot to flash that can be used to store persistent data.
+# It will work with and without riotboot, but the size of the slot must remain fixed across
+# firmware versions.
+
+# Size of the AUX slot in Bytes - must align with flash page size
+SLOT_AUX_LEN ?= 0
+SLOT_AUX_OFFSET ?= $(shell echo $$(($(ROM_LEN:%K=%*1024)-$(SLOT_AUX_LEN))))
+
+LINKFLAGS += $(LINKFLAGPREFIX)--defsym=_slot_aux_offset=$(SLOT_AUX_OFFSET)
+LINKFLAGS += $(LINKFLAGPREFIX)--defsym=_slot_aux_len=$(SLOT_AUX_LEN)
+
+ifneq (0, $(SLOT_AUX_LEN))
+  CFLAGS += -DCONFIG_SLOT_AUX_OFFSET=$(SLOT_AUX_OFFSET)
+  CFLAGS += -DCONFIG_SLOT_AUX_LEN=$(SLOT_AUX_LEN)
+endif

tests/drivers/mtd_flashpage/Makefile

@@ -4,4 +4,11 @@ USEMODULE += mtd_flashpage
 USEMODULE += mtd_write_page
 USEMODULE += embunit
 
+FEATURES_OPTIONAL += periph_flashpage_aux
+
+# define an auxiliary slot on the internal flash
+# NOTE: This should typically be set by the board as it can not be changed in the field.
+# This is only defined here for the sake of the test.
+SLOT_AUX_LEN := 0x8000
+
 include $(RIOTBASE)/Makefile.include

tests/drivers/mtd_flashpage/Makefile.ci

@@ -0,0 +1,12 @@
+BOARD_INSUFFICIENT_MEMORY := \
+    nucleo-c031c6 \
+    nucleo-f031k6 \
+    nucleo-f042k6 \
+    nucleo-l011k4 \
+    nucleo-l031k6 \
+    samd10-xmini \
+    stk3200 \
+    stm32g0316-disco \
+    stm32f030f4-demo \
+    weact-g030f6 \
+    #

tests/drivers/mtd_flashpage/main.c

@@ -11,6 +11,7 @@
  *
  * @file
  */
+#include <stdbool.h>
 #include <string.h>
 #include <errno.h>
 
@@ -217,6 +218,80 @@ static void test_mtd_write_read_page(void)
 #endif
 }
 
+#ifdef MODULE_PERIPH_FLASHPAGE_AUX
+static bool mem_is_all_set(const uint8_t *buf, uint8_t c, size_t n)
+{
+    for (const uint8_t *end = buf + n; buf != end; ++buf) {
+        if (*buf != c) {
+            return false;
+        }
+    }
+
+    return true;
+}
+
+static void test_mtd_aux_slot(void)
+{
+    mtd_dev_t *dev_aux = mtd_aux;
+
+    mtd_init(dev_aux);
+
+    uint32_t sector = dev_aux->sector_count - 2;
+
+    static uint8_t buffer[FLASHPAGE_SIZE];
+
+    uint32_t page_0 = dev_aux->pages_per_sector * sector;
+    uint32_t page_1 = dev_aux->pages_per_sector * (sector + 1);
+    uint32_t page_size = dev_aux->page_size;
+
+    /* write dummy data to sectors */
+    memset(buffer, 0x23, dev_aux->page_size);
+    TEST_ASSERT_EQUAL_INT(mtd_write_page_raw(dev_aux, buffer, page_0, 0, page_size), 0);
+    TEST_ASSERT_EQUAL_INT(mtd_write_page_raw(dev_aux, buffer, page_1, 0, page_size), 0);
+
+    /* erase two sectors and check if they have been erased */
+    TEST_ASSERT_EQUAL_INT(mtd_erase_sector(dev_aux, sector, 2), 0);
+    TEST_ASSERT_EQUAL_INT(mtd_read_page(dev_aux, buffer, page_0, 0, page_size), 0);
+    TEST_ASSERT(mem_is_all_set(buffer, 0xFF, page_size) || mem_is_all_set(buffer, 0x00, page_size));
+    TEST_ASSERT_EQUAL_INT(mtd_read_page(dev_aux, buffer, page_1, 0, page_size), 0);
+    TEST_ASSERT(mem_is_all_set(buffer, 0xFF, page_size) || mem_is_all_set(buffer, 0x00, page_size));
+
+    /* write test data & read it back */
+    const char test_str[] = "0123456789";
+    uint32_t offset = 5;
+
+    TEST_ASSERT_EQUAL_INT(mtd_write_page_raw(dev_aux, test_str, page_0, offset, sizeof(test_str)), 0);
+    TEST_ASSERT_EQUAL_INT(mtd_read_page(dev_aux, buffer, page_0, offset, sizeof(test_str)), 0);
+    TEST_ASSERT_EQUAL_INT(memcmp(test_str, buffer, sizeof(test_str)), 0);
+
+    /* write across page boundary */
+    offset = page_size - sizeof(test_str) / 2;
+    TEST_ASSERT_EQUAL_INT(mtd_write_page_raw(dev_aux, test_str, page_0, offset, sizeof(test_str)), 0);
+    TEST_ASSERT_EQUAL_INT(mtd_read_page(dev_aux, buffer, page_0, offset, sizeof(test_str)), 0);
+    TEST_ASSERT_EQUAL_INT(memcmp(test_str, buffer, sizeof(test_str)), 0);
+
+    /* write across sector boundary */
+    offset = page_size - sizeof(test_str) / 2
+           + (dev_aux->pages_per_sector - 1) * page_size;
+    TEST_ASSERT_EQUAL_INT(mtd_write_page_raw(dev_aux, test_str, page_0, offset, sizeof(test_str)), 0);
+    TEST_ASSERT_EQUAL_INT(mtd_read_page(dev_aux, buffer, page_0, offset, sizeof(test_str)), 0);
+    TEST_ASSERT_EQUAL_INT(memcmp(test_str, buffer, sizeof(test_str)), 0);
+
+    /* overwrite first test string, rely on MTD for read-modify-write */
+    const char test_str_2[] = "Hello World!";
+    offset = 5;
+    TEST_ASSERT_EQUAL_INT(mtd_write_page(dev_aux, test_str_2, page_0, offset, sizeof(test_str_2)), 0);
+    TEST_ASSERT_EQUAL_INT(mtd_read_page(dev_aux, buffer, page_0, offset, sizeof(test_str_2)), 0);
+    TEST_ASSERT_EQUAL_INT(memcmp(test_str_2, buffer, sizeof(test_str_2)), 0);
+
+    /* test write_page across sectors */
+    offset = dev_aux->pages_per_sector * dev_aux->page_size - 2;
+    TEST_ASSERT_EQUAL_INT(mtd_write_page(dev_aux, test_str, page_0, offset, sizeof(test_str)), 0);
+    TEST_ASSERT_EQUAL_INT(mtd_read_page(dev_aux, buffer, page_0, offset, sizeof(test_str)), 0);
+    TEST_ASSERT_EQUAL_INT(memcmp(test_str, buffer, sizeof(test_str)), 0);
+}
+#endif
+
 Test *tests_mtd_flashpage_tests(void)
 {
     EMB_UNIT_TESTFIXTURES(fixtures) {
@@ -225,6 +300,9 @@ Test *tests_mtd_flashpage_tests(void)
         new_TestFixture(test_mtd_write_erase),
         new_TestFixture(test_mtd_write_read),
         new_TestFixture(test_mtd_write_read_page),
+#ifdef MODULE_PERIPH_FLASHPAGE_AUX
+        new_TestFixture(test_mtd_aux_slot),
+#endif
     };
 
     EMB_UNIT_TESTCALLER(mtd_flashpage_tests, setup, teardown, fixtures);