Merge pull request #17683 from chrysn-pull-requests/mtd-granularity

mtd: Introduce write granularity

boards/native/board_init.c

@@ -27,6 +27,7 @@ mtd_native_dev_t mtd0_dev = {
         .sector_count = MTD_SECTOR_NUM,
         .pages_per_sector = MTD_SECTOR_SIZE / MTD_PAGE_SIZE,
         .page_size = MTD_PAGE_SIZE,
+        .write_size = MTD_WRITE_SIZE,
     },
     .fname = MTD_NATIVE_FILENAME,
 };

boards/native/include/board.h

@@ -76,6 +76,11 @@ void _native_LED_RED_TOGGLE(void);
 #ifndef MTD_SECTOR_NUM
 #define MTD_SECTOR_NUM          (2048)
 #endif
+/** Advertised write size. While the file system backend supports single byte
+ * granularity, this can be increased to mimic other media. */
+#ifndef MTD_WRITE_SIZE
+#define MTD_WRITE_SIZE          (1)
+#endif
 #ifndef MTD_NATIVE_FILENAME
 #define MTD_NATIVE_FILENAME     "MEMORY.bin"
 #endif

cpu/esp_common/periph/flash.c

@@ -127,6 +127,7 @@ void spi_flash_drive_init (void)
     _flash_driver.write_page = &_flash_write_page;
     _flash_driver.erase = &_flash_erase;
     _flash_driver.power = &_flash_power;
+    _flash_driver.flags = MTD_DRIVER_FLAG_CLEARING_OVERWRITE;
 
     /* first, set the beginning of flash to 0x0 to read partition table */
     _flash_beg  = 0x0;
@@ -200,6 +201,9 @@ void spi_flash_drive_init (void)
 
     _flash_dev.pages_per_sector = _flashchip->sector_size / _flashchip->page_size;
     _flash_dev.page_size = _flashchip->page_size;
+    /* Emulation for smaller / unaligned writes is present, but at reduced
+     * performance */
+    _flash_dev.write_size = 4;
 
     DEBUG("%s flashchip chip_size=%d block_size=%d sector_size=%d page_size=%d\n", __func__,
           _flashchip->chip_size, _flashchip->block_size,

drivers/at24cxxx/mtd/mtd.c

@@ -41,6 +41,7 @@ static int _mtd_at24cxxx_init(mtd_dev_t *mtd)
     mtd->pages_per_sector = 1;
     mtd->sector_count = DEV(mtd)->params.eeprom_size /
                         DEV(mtd)->params.page_size;
+    mtd->write_size = 1;
     return 0;
 }
 

drivers/at25xxx/mtd/mtd.c

@@ -38,6 +38,7 @@ static int mtd_at25xxx_init(mtd_dev_t *dev)
         dev->pages_per_sector = 1;
         dev->page_size        = mtd_at25xxx->params->page_size;
         dev->sector_count     = mtd_at25xxx->params->size / mtd_at25xxx->params->page_size;
+        dev->write_size       = 1;
         return 0;
     }
     return -EIO;

drivers/include/mtd.h

@@ -22,7 +22,7 @@
  *
  * MTD devices expose a block based erase and write interface. In that, they
  * are the distinct from block devices (like hard disks) on which individual
- * bytes can be overridden. The [Linux MTD FAQ](http://www.linux-mtd.infradead.org/faq/general.html)
+ * bytes can be overwritten. The [Linux MTD FAQ](http://www.linux-mtd.infradead.org/faq/general.html)
  * has a convenient comparison (beware though of terminology differences
  * outlined below). They can be erased (with some granularity, often wearing
  * out the erased area a bit), and erased areas can be written to (sometimes
@@ -44,11 +44,25 @@
  *
  *   Pages are a subdivision of sectors.
  *
+ * * The **write size** is the minimum size of writes to the device, and also
+ *   the required alignment of writes.
+ *
+ *   The write size is a divider of the page. It is often between 1 to 4 bytes
+ *   long, but may be up to the full page size.
+ *
  * * The device's **flags** indicate features, eg. whether a memory location
  *   can be overwritten without erasing it first.
  *
- * Note that some properties of the backend are currently not advertised to the
+ * Unless a flag (such as @ref MTD_DRIVER_FLAG_DIRECT_WRITE or @ref
- * user (see the documentation of @ref mtd_write).
+ * MTD_DRIVER_FLAG_CLEARING_OVERWRITE) allows it, this MTD API does not allow
+ * memory areas to be written to twice between erase operations. Drivers are
+ * not expected to count write accesses, and neither do this module's
+ * functions: The performance impact would be too great. It is up to the
+ * application to only write to erased memory once. Failure to do so may damage
+ * hardware.
+ *
+ * This MTD API currently does not specify which value will be read from an
+ * erased sector.
  *
  * @file
  *
@@ -96,6 +110,7 @@ typedef struct {
     uint32_t sector_count;     /**< Number of sector in the MTD */
     uint32_t pages_per_sector; /**< Number of pages by sector in the MTD */
     uint32_t page_size;        /**< Size of the pages in the MTD */
+    uint32_t write_size;       /**< Minimum size and alignment of writes to the device */
 #if defined(MODULE_MTD_WRITE_PAGE) || DOXYGEN
     void *work_area;           /**< sector-sized buffer (only present when @ref mtd_write_page is enabled) */
 #endif
@@ -105,13 +120,22 @@ typedef struct {
  * @brief   MTD driver can write any data to the storage without erasing it first.
  *
  * If this is set, a write completely overrides the previous values.
- *
- * Its absence makes no statement on whether or not writes to memory areas that
- * have been written to previously are allowed, and if so, whether previously
- * written bits should be written again or not written.
  */
 #define MTD_DRIVER_FLAG_DIRECT_WRITE    (1 << 0)
 
+/**
+ * @brief   MTD driver supports arbitrary clearing overwrites
+ *
+ * If this is set, (arbitrarily) many writes are permitted per write size, and
+ * the result is the old value bitwise-AND the written value.
+ *
+ * This property is common for managed flash memories. (By comparison, the raw
+ * flash often used internally by MCUs may not allow overwrites, or may allow
+ * them with the same semantics, but only for a limited number of writes
+ * between erasures; there is currently no flag describing these any further).
+ */
+#define MTD_DRIVER_FLAG_CLEARING_OVERWRITE    (1 << 1)
+
 /**
  * @brief   MTD driver interface
  *
@@ -315,8 +339,9 @@ int mtd_read_page(mtd_dev_t *mtd, void *dest, uint32_t page, uint32_t offset, ui
  * @brief   Write data to a MTD device
  *
  * @p addr + @p count must be inside a page boundary. @p addr can be anywhere
- * but the buffer cannot overlap two pages. Though some devices might enforce alignment
+ * but the buffer cannot overlap two pages.
- * on both @p addr and @p buf.
+ *
+ * Both parameters must be multiples of the device's write size.
  *
  * @param      mtd   the device to write to
  * @param[in]  src   the buffer to write
@@ -342,6 +367,8 @@ int mtd_write(mtd_dev_t *mtd, const void *src, uint32_t addr, uint32_t count);
  *
  * This performs a raw write, no automatic read-modify-write cycle is performed.
  *
+ * Both @p offset and @p size must be multiples of the device's write size.
+ *
  * @p offset must be smaller than the page size
  *
  * @param      mtd      the device to write to

drivers/include/mtd_flashpage.h

@@ -43,6 +43,9 @@ extern "C"
         .sector_count = FLASHPAGE_NUMOF,            \
         .pages_per_sector = _pages_per_sector,      \
         .page_size = FLASHPAGE_SIZE / _pages_per_sector, \
+        .write_size = FLASHPAGE_WRITE_BLOCK_SIZE >= FLASHPAGE_WRITE_BLOCK_ALIGNMENT \
+            ? FLASHPAGE_WRITE_BLOCK_SIZE            \
+            : FLASHPAGE_WRITE_BLOCK_ALIGNMENT,      \
     },                                              \
 }
 

drivers/include/mtd_mapper.h

@@ -39,6 +39,7 @@
  *         .sector_count = SECTOR_COUNT / 2,
  *         .pages_per_sector = PAGE_PER_SECTOR,
  *         .page_size = PAGE_SIZE,
+ *         .write_size = WRITE_SIZE,
  *     },
  *     .parent = &parent,
  *     .sector = SECTOR_COUNT / 2

drivers/mtd/mtd.c

@@ -36,6 +36,15 @@ int mtd_init(mtd_dev_t *mtd)
 
     int res = -ENOTSUP;
 
+    /* Drivers preceding the introduction of write_size need to set it. While
+     * this assert breaks applications that previously worked, it is likely
+     * that these applications silently assumed a certain write size and would
+     * break when switching the MTD backend. When tripping over this assert,
+     * please update your driver to produce a correct value *and* place a check
+     * in your application for whether the backend allows sufficiently small
+     * writes. */
+    assert(mtd->write_size != 0);
+
     if (mtd->driver->init) {
         res = mtd->driver->init(mtd);
     }

drivers/mtd_mapper/mtd_mapper.c

@@ -78,6 +78,7 @@ static int _init(mtd_dev_t *mtd)
     assert(backing_mtd->page_size == region->mtd.page_size);
     assert(backing_mtd->pages_per_sector == region->mtd.pages_per_sector);
     assert(backing_mtd->sector_count >= region->mtd.sector_count);
+    assert(backing_mtd->write_size == region->mtd.write_size);
 
     /* offset + region size must not exceed the backing device */
     assert(region->sector + region->mtd.sector_count <= backing_mtd->sector_count);

drivers/mtd_mci/mtd_mci.c

@@ -48,6 +48,7 @@ static int mtd_mci_init(mtd_dev_t *dev)
 
     dev->pages_per_sector = 1;
     dev->page_size = SD_HC_BLOCK_SIZE;
+    dev->write_size = SD_HC_BLOCK_SIZE;
     mci_ioctl(GET_SECTOR_COUNT, &dev->sector_count);
 
     DEBUG("sector size: %lu\n", dev->page_size);

drivers/mtd_sdcard/mtd_sdcard.c

@@ -45,6 +45,7 @@ static int mtd_sdcard_init(mtd_dev_t *dev)
 
         /* sdcard_spi always uses the fixed block size of SD-HC cards */
         dev->page_size        = SD_HC_BLOCK_SIZE;
+        dev->write_size       = SD_HC_BLOCK_SIZE;
         return 0;
     }
     return -EIO;

drivers/mtd_spi_nor/mtd_spi_nor.c

@@ -521,6 +521,9 @@ static int mtd_spi_nor_init(mtd_dev_t *mtd)
         mtd->sector_count = mtd_spi_nor_get_size(&dev->jedec_id)
                           / (mtd->pages_per_sector * mtd->page_size);
     }
+    /* SPI NOR is byte addressable; instances don't need to configure that */
+    assert(mtd->write_size <= 1);
+    mtd->write_size = 1;
     _set_addr_width(mtd);
 
     DEBUG("mtd_spi_nor_init: %" PRIu32 " bytes "

tests/mtd_mapper/main.c

@@ -37,6 +37,9 @@
 #ifndef PAGE_SIZE
 #define PAGE_SIZE 64
 #endif
+#ifndef WRITE_SIZE
+#define WRITE_SIZE 4
+#endif
 
 #define MEMORY_PAGE_COUNT    PAGE_PER_SECTOR * SECTOR_COUNT
 
@@ -180,6 +183,7 @@ static mtd_dev_t dev = {
     .sector_count = SECTOR_COUNT,
     .pages_per_sector = PAGE_PER_SECTOR,
     .page_size = PAGE_SIZE,
+    .write_size = WRITE_SIZE,
 };
 
 static mtd_mapper_parent_t _parent = MTD_PARENT_INIT(&dev);
@@ -190,6 +194,7 @@ static mtd_mapper_region_t _region_a = {
         .sector_count = SECTOR_COUNT / 2,
         .pages_per_sector = PAGE_PER_SECTOR,
         .page_size = PAGE_SIZE,
+        .write_size = WRITE_SIZE,
     },
     .parent = &_parent,
     .sector = 0,
@@ -201,6 +206,7 @@ static mtd_mapper_region_t _region_b = {
         .sector_count = SECTOR_COUNT / 2,
         .pages_per_sector = PAGE_PER_SECTOR,
         .page_size = PAGE_SIZE,
+        .write_size = WRITE_SIZE,
     },
     .parent = &_parent,
     .sector = SECTOR_COUNT / 2,

tests/pkg_littlefs/main.c

@@ -115,6 +115,7 @@ static mtd_dev_t dev = {
     .sector_count = SECTOR_COUNT,
     .pages_per_sector = PAGE_PER_SECTOR,
     .page_size = PAGE_SIZE,
+    .write_size = 1,
 };
 
 static mtd_dev_t *_dev = (mtd_dev_t*) &dev;

tests/pkg_littlefs2/main.c

@@ -115,6 +115,7 @@ static mtd_dev_t dev = {
     .sector_count = SECTOR_COUNT,
     .pages_per_sector = PAGE_PER_SECTOR,
     .page_size = PAGE_SIZE,
+    .write_size = 1,
 };
 
 static mtd_dev_t *_dev = (mtd_dev_t*) &dev;

tests/pkg_spiffs/main.c

@@ -112,6 +112,7 @@ static mtd_dev_t dev = {
     .sector_count = SECTOR_COUNT,
     .pages_per_sector = PAGE_PER_SECTOR,
     .page_size = PAGE_SIZE,
+    .write_size = 1,
 };
 
 static mtd_dev_t *_dev = (mtd_dev_t*) &dev;

tests/unittests/tests-mtd/tests-mtd.c

@@ -39,6 +39,9 @@
 #ifndef PAGE_SIZE
 #define PAGE_SIZE 128
 #endif
+#ifndef WRITE_SIZE
+#define WRITE_SIZE 1
+#endif
 
 static uint8_t dummy_memory[PAGE_PER_SECTOR * PAGE_SIZE * SECTOR_COUNT];
 
@@ -115,6 +118,7 @@ static mtd_dev_t _dev = {
     .sector_count = SECTOR_COUNT,
     .pages_per_sector = PAGE_PER_SECTOR,
     .page_size = PAGE_SIZE,
+    .write_size = WRITE_SIZE,
 };
 
 static mtd_dev_t *dev = (mtd_dev_t*) &_dev;