From 4034f9616983d8c653ded05476e93ee5525c6320 Mon Sep 17 00:00:00 2001 From: Benjamin Valentin Date: Wed, 6 May 2020 23:46:43 +0200 Subject: [PATCH 01/10] mtd: add page addressed operations Currently read(), write() and erase() all use 32 bit addressing. This is a problem when writing to media > 4 GiB, e.g. SD cards. The current implementation would wrap around after 4 GiB and corrupt data. To avoid this, add functions to the MTD subsystem that allow for page-wise addressing. This is how most of the underling storage drivers and the file-systems above work anyway. In the future we should then deprecate the 32-bit functions if all drivers are converted. --- drivers/include/mtd.h | 121 ++++++++++++++++++++++++++++++++ drivers/mtd/mtd.c | 158 ++++++++++++++++++++++++++++++++++++++---- 2 files changed, 267 insertions(+), 12 deletions(-) diff --git a/drivers/include/mtd.h b/drivers/include/mtd.h index 5ec7c18271..8431fef24e 100644 --- a/drivers/include/mtd.h +++ b/drivers/include/mtd.h @@ -101,6 +101,27 @@ struct mtd_desc { uint32_t addr, uint32_t size); + /** + * @brief Read from the Memory Technology Device (MTD) using + * pagewise addressing. + * + * @p offset should not exceed the page size + * + * @param[in] dev Pointer to the selected driver + * @param[out] buff Pointer to the data buffer to store read data + * @param[in] page Page number to start reading from + * @param[in] offset Byte offset from the start of the page + * @param[in] size Number of bytes + * + * @return number of bytes read on success + * @return < 0 value on error + */ + int (*read_page)(mtd_dev_t *dev, + void *buff, + uint32_t page, + uint32_t offset, + uint32_t size); + /** * @brief Write to the Memory Technology Device (MTD) * @@ -120,6 +141,27 @@ struct mtd_desc { uint32_t addr, uint32_t size); + /** + * @brief Write to the Memory Technology Device (MTD) using + * pagewise addressing. + * + * @p offset should not exceed the page size + * + * @param[in] dev Pointer to the selected driver + * @param[out] buff Pointer to the data to be written + * @param[in] page Page number to start writing to + * @param[in] offset Byte offset from the start of the page + * @param[in] size Number of bytes + * + * @return bytes written on success + * @return < 0 value on error + */ + int (*write_page)(mtd_dev_t *dev, + const void *buff, + uint32_t page, + uint32_t offset, + uint32_t size); + /** * @brief Erase sector(s) over the Memory Technology Device (MTD) * @@ -136,6 +178,21 @@ struct mtd_desc { uint32_t addr, uint32_t size); + /** + * @brief Erase sector(s) of the Memory Technology Device (MTD) + * + * @param[in] dev Pointer to the selected driver + * @param[in] sector the first sector number to erase + + * @param[in] count Number of sectors to erase + * + * @return 0 on success + * @return < 0 value on error + */ + int (*erase_sector)(mtd_dev_t *dev, + uint32_t sector, + uint32_t count); + /** * @brief Control power of Memory Technology Device (MTD) * @@ -176,6 +233,29 @@ int mtd_init(mtd_dev_t *mtd); */ int mtd_read(mtd_dev_t *mtd, void *dest, uint32_t addr, uint32_t count); +/** + * @brief Read data from a MTD device with pagewise addressing + * + * The MTD layer will take care of splitting up the transaction into multiple + * reads if it is required by the underlying storage media. + * + * @p offset must be smaller than the page size + * + * @param mtd the device to read from + * @param[out] dest the buffer to fill in + * @param[in] page Page number to start reading from + * @param[in] offset offset from the start of the page (in bytes) + * @param[in] size the number of bytes to read + * + * @return 0 on success + * @return < 0 if an error occurred + * @return -ENODEV if @p mtd is not a valid device + * @return -ENOTSUP if operation is not supported on @p mtd + * @return -EOVERFLOW if @p addr or @p count are not valid, i.e. outside memory + * @return -EIO if I/O error occurred + */ +int mtd_read_page(mtd_dev_t *mtd, void *dest, uint32_t page, uint32_t offset, uint32_t size); + /** * @brief Write data to a MTD device * @@ -199,6 +279,31 @@ int mtd_read(mtd_dev_t *mtd, void *dest, uint32_t addr, uint32_t count); */ int mtd_write(mtd_dev_t *mtd, const void *src, uint32_t addr, uint32_t count); +/** + * @brief Write data to a MTD device with pagewise addressing + * + * The MTD layer will take care of splitting up the transaction into multiple + * writes if it is required by the underlying storage media. + * + * @p offset must be smaller than the page size + * + * + * @param mtd the device to write to + * @param[in] src the buffer to write + * @param[in] page Page number to start writing to + * @param[in] offset byte offset from the start of the page + * @param[in] size the number of bytes to write + * + * @return 0 on success + * @return < 0 if an error occurred + * @return -ENODEV if @p mtd is not a valid device + * @return -ENOTSUP if operation is not supported on @p mtd + * @return -EOVERFLOW if @p addr or @p count are not valid, i.e. outside memory, + * @return -EIO if I/O error occurred + * @return -EINVAL if parameters are invalid + */ +int mtd_write_page(mtd_dev_t *mtd, const void *src, uint32_t page, uint32_t offset, uint32_t size); + /** * @brief Erase sectors of a MTD device * @@ -217,6 +322,22 @@ int mtd_write(mtd_dev_t *mtd, const void *src, uint32_t addr, uint32_t count); */ int mtd_erase(mtd_dev_t *mtd, uint32_t addr, uint32_t count); +/** + * @brief Erase sectors of a MTD device + * + * @param mtd the device to erase + * @param[in] sector the first sector number to erase + * @param[in] num the number of sectors to erase + * + * @return 0 if erase successful + * @return < 0 if an error occurred + * @return -ENODEV if @p mtd is not a valid device + * @return -ENOTSUP if operation is not supported on @p mtd + * @return -EOVERFLOW if @p addr or @p sector are not valid, i.e. outside memory + * @return -EIO if I/O error occurred + */ +int mtd_erase_sector(mtd_dev_t *mtd, uint32_t sector, uint32_t num); + /** * @brief Set power mode on a MTD device * diff --git a/drivers/mtd/mtd.c b/drivers/mtd/mtd.c index 7f8d8570a8..e19c233693 100644 --- a/drivers/mtd/mtd.c +++ b/drivers/mtd/mtd.c @@ -18,8 +18,12 @@ * @author Vincent Dupont */ +#include #include +#include +#include +#include "bitarithm.h" #include "mtd.h" int mtd_init(mtd_dev_t *mtd) @@ -42,12 +46,62 @@ int mtd_read(mtd_dev_t *mtd, void *dest, uint32_t addr, uint32_t count) return -ENODEV; } - if (mtd->driver->read) { - return mtd->driver->read(mtd, dest, addr, count); + /* page size is always a power of two */ + const uint32_t page_shift = bitarithm_msb(mtd->page_size); + const uint32_t page_mask = mtd->page_size - 1; + + return mtd_read_page(mtd, dest, addr >> page_shift, addr & page_mask, count); +} + +int mtd_read_page(mtd_dev_t *mtd, void *dest, uint32_t page, uint32_t offset, + uint32_t count) +{ + if (!mtd || !mtd->driver) { + return -ENODEV; } - else { - return -ENOTSUP; + + if (mtd->driver->read_page == NULL) { + /* TODO: remove when all backends implement read_page */ + if (mtd->driver->read) { + return mtd->driver->read(mtd, dest, mtd->page_size * page + offset, count); + } else { + return -ENOTSUP; + } } + + /* Implementation assumes page size is <= INT_MAX and a power of two. */ + /* We didn't find hardware yet where this is not true. */ + assert(mtd->page_size <= INT_MAX); + assert(bitarithm_bits_set(mtd->page_size) == 1); + + /* page size is always a power of two */ + const uint32_t page_shift = bitarithm_msb(mtd->page_size); + const uint32_t page_mask = mtd->page_size - 1; + + page += offset >> page_shift; + offset = offset & page_mask; + + char *_dst = dest; + + while (count) { + int read_bytes = mtd->driver->read_page(mtd, _dst, page, offset, count); + + if (read_bytes < 0) { + return read_bytes; + } + + count -= read_bytes; + + if (count == 0) { + break; + } + + _dst += read_bytes; + page += (offset + read_bytes) >> page_shift; + offset = (offset + read_bytes) & page_mask; + } + + return 0; } int mtd_write(mtd_dev_t *mtd, const void *src, uint32_t addr, uint32_t count) @@ -56,12 +110,62 @@ int mtd_write(mtd_dev_t *mtd, const void *src, uint32_t addr, uint32_t count) return -ENODEV; } - if (mtd->driver->write) { - return mtd->driver->write(mtd, src, addr, count); + /* page size is always a power of two */ + const uint32_t page_shift = bitarithm_msb(mtd->page_size); + const uint32_t page_mask = mtd->page_size - 1; + + return mtd_write_page(mtd, src, addr >> page_shift, addr & page_mask, count); +} + +int mtd_write_page(mtd_dev_t *mtd, const void *src, uint32_t page, uint32_t offset, + uint32_t count) +{ + if (!mtd || !mtd->driver) { + return -ENODEV; } - else { - return -ENOTSUP; + + if (mtd->driver->write_page == NULL) { + /* TODO: remove when all backends implement write_page */ + if (mtd->driver->write) { + return mtd->driver->write(mtd, src, mtd->page_size * page + offset, count); + } else { + return -ENOTSUP; + } } + + /* Implementation assumes page size is <= INT_MAX and a power of two. */ + /* We didn't find hardware yet where this is not true. */ + assert(mtd->page_size <= INT_MAX); + assert(bitarithm_bits_set(mtd->page_size) == 1); + + /* page size is always a power of two */ + const uint32_t page_shift = bitarithm_msb(mtd->page_size); + const uint32_t page_mask = mtd->page_size - 1; + + page += offset >> page_shift; + offset = offset & page_mask; + + const char *_src = src; + + while (count) { + int written = mtd->driver->write_page(mtd, _src, page, offset, count); + + if (written < 0) { + return written; + } + + count -= written; + + if (count == 0) { + break; + } + + _src += written; + page += (offset + written) >> page_shift; + offset = (offset + written) & page_mask; + } + + return 0; } int mtd_erase(mtd_dev_t *mtd, uint32_t addr, uint32_t count) @@ -70,12 +174,42 @@ int mtd_erase(mtd_dev_t *mtd, uint32_t addr, uint32_t count) return -ENODEV; } - if (mtd->driver->erase) { - return mtd->driver->erase(mtd, addr, count); + uint32_t sector_size = mtd->pages_per_sector * mtd->page_size; + + if (count % sector_size) { + return -EOVERFLOW; } - else { - return -ENOTSUP; + + if (addr % sector_size) { + return -EOVERFLOW; } + + return mtd_erase_sector(mtd, addr / sector_size, count / sector_size); +} + +int mtd_erase_sector(mtd_dev_t *mtd, uint32_t sector, uint32_t count) +{ + if (!mtd || !mtd->driver) { + return -ENODEV; + } + + if (sector >= mtd->sector_count) { + return -EOVERFLOW; + } + + if (mtd->driver->erase_sector == NULL) { + /* TODO: remove when all backends implement erase_sector */ + if (mtd->driver->erase) { + uint32_t sector_size = mtd->pages_per_sector * mtd->page_size; + return mtd->driver->erase(mtd, + sector * sector_size, + count * sector_size); + } else { + return -ENOTSUP; + } + } + + return mtd->driver->erase_sector(mtd, sector, count); } int mtd_power(mtd_dev_t *mtd, enum mtd_power_state power) From 7d291b570710351962830b9101cfb7ca234f05d3 Mon Sep 17 00:00:00 2001 From: Benjamin Valentin Date: Wed, 6 May 2020 23:49:29 +0200 Subject: [PATCH 02/10] drivers/mtd_sdard: implement page-wise read() and write() SD cards are usually larger than 4 GiB, so using 32 bit addressing will break once they are filled to a certain extend. Implement read_page() and write_page() which at a page size of 512 should work for SD cards of up to 2 TiB. The read() and write() functions will ignore any offset that is not aligned with the page boundary, so do the same for the new functions, but at least report an error. --- drivers/mtd_sdcard/mtd_sdcard.c | 70 +++++++++++++++++++++++++-------- 1 file changed, 54 insertions(+), 16 deletions(-) diff --git a/drivers/mtd_sdcard/mtd_sdcard.c b/drivers/mtd_sdcard/mtd_sdcard.c index a55258974d..1fa1e4eb32 100644 --- a/drivers/mtd_sdcard/mtd_sdcard.c +++ b/drivers/mtd_sdcard/mtd_sdcard.c @@ -29,22 +29,6 @@ #include #include -static int mtd_sdcard_init(mtd_dev_t *mtd); -static int mtd_sdcard_read(mtd_dev_t *mtd, void *dest, uint32_t addr, - uint32_t size); -static int mtd_sdcard_write(mtd_dev_t *mtd, const void *src, uint32_t addr, - uint32_t size); -static int mtd_sdcard_erase(mtd_dev_t *mtd, uint32_t addr, uint32_t size); -static int mtd_sdcard_power(mtd_dev_t *mtd, enum mtd_power_state power); - -const mtd_desc_t mtd_sdcard_driver = { - .init = mtd_sdcard_init, - .read = mtd_sdcard_read, - .write = mtd_sdcard_write, - .erase = mtd_sdcard_erase, - .power = mtd_sdcard_power, -}; - static int mtd_sdcard_init(mtd_dev_t *dev) { DEBUG("mtd_sdcard_init\n"); @@ -79,6 +63,28 @@ static int mtd_sdcard_read(mtd_dev_t *dev, void *buff, uint32_t addr, return -EIO; } +static int mtd_sdcard_read_page(mtd_dev_t *dev, void *buff, uint32_t page, + uint32_t offset, uint32_t size) +{ + DEBUG("mtd_sdcard_read_page: page:%" PRIu32 " offset:%" PRIu32 " size:%" PRIu32 "\n", + page, offset, size); + + if (offset) { + return -ENOTSUP; + } + + mtd_sdcard_t *mtd_sd = (mtd_sdcard_t*)dev; + sd_rw_response_t err; + int res = sdcard_spi_read_blocks(mtd_sd->sd_card, page, + buff, SD_HC_BLOCK_SIZE, + size / SD_HC_BLOCK_SIZE, &err); + + if (err != SD_RW_OK) { + return -EIO; + } + return res * SD_HC_BLOCK_SIZE; +} + static int mtd_sdcard_write(mtd_dev_t *dev, const void *buff, uint32_t addr, uint32_t size) { @@ -95,6 +101,28 @@ static int mtd_sdcard_write(mtd_dev_t *dev, const void *buff, uint32_t addr, return -EIO; } +static int mtd_sdcard_write_page(mtd_dev_t *dev, const void *buff, uint32_t page, + uint32_t offset, uint32_t size) +{ + DEBUG("mtd_sdcard_write_page: page:%" PRIu32 " offset:%" PRIu32 " size:%" PRIu32 "\n", + page, offset, size); + + if (offset) { + return -ENOTSUP; + } + + mtd_sdcard_t *mtd_sd = (mtd_sdcard_t*)dev; + sd_rw_response_t err; + int res = sdcard_spi_write_blocks(mtd_sd->sd_card, page, + buff, SD_HC_BLOCK_SIZE, + size / SD_HC_BLOCK_SIZE, &err); + + if (err != SD_RW_OK) { + return -EIO; + } + return res * SD_HC_BLOCK_SIZE; +} + static int mtd_sdcard_erase(mtd_dev_t *dev, uint32_t addr, uint32_t size) @@ -121,3 +149,13 @@ static int mtd_sdcard_power(mtd_dev_t *dev, enum mtd_power_state power) (make use of sdcard_spi_params_t.power pin) */ return -ENOTSUP; /* currently not supported */ } + +const mtd_desc_t mtd_sdcard_driver = { + .init = mtd_sdcard_init, + .read = mtd_sdcard_read, + .read_page = mtd_sdcard_read_page, + .write = mtd_sdcard_write, + .write_page = mtd_sdcard_write_page, + .erase = mtd_sdcard_erase, + .power = mtd_sdcard_power, +}; From c436c39ea8dd1e78b72dd2b504f8370c9b43631e Mon Sep 17 00:00:00 2001 From: Benjamin Valentin Date: Wed, 6 May 2020 23:52:16 +0200 Subject: [PATCH 03/10] pkg/fatfs: use page-wise read() and write() The FAT file system operates on sectors (pages), so use the new mtd_write_page() and mtd_read_page() functions. This also avoids the wrap-around after reading/writing past 4 GiB. --- pkg/fatfs/fatfs_diskio/mtd/mtd_diskio.c | 24 +++++++++++------------- 1 file changed, 11 insertions(+), 13 deletions(-) diff --git a/pkg/fatfs/fatfs_diskio/mtd/mtd_diskio.c b/pkg/fatfs/fatfs_diskio/mtd/mtd_diskio.c index d218276de1..687f71aa1e 100644 --- a/pkg/fatfs/fatfs_diskio/mtd/mtd_diskio.c +++ b/pkg/fatfs/fatfs_diskio/mtd/mtd_diskio.c @@ -95,15 +95,15 @@ DRESULT disk_read(BYTE pdrv, BYTE *buff, DWORD sector, UINT count) return RES_PARERR; } - uint32_t nread = count * fatfs_mtd_devs[pdrv]->page_size; - int res = mtd_read(fatfs_mtd_devs[pdrv], buff, - sector * fatfs_mtd_devs[pdrv]->page_size, - nread); + uint32_t sector_size = fatfs_mtd_devs[pdrv]->page_size + * fatfs_mtd_devs[pdrv]->pages_per_sector; + + int res = mtd_read_page(fatfs_mtd_devs[pdrv], buff, + sector, 0, count * sector_size); if (res != 0) { return RES_ERROR; } - assert((nread / fatfs_mtd_devs[pdrv]->page_size) == count); return RES_OK; } @@ -127,23 +127,21 @@ DRESULT disk_write(BYTE pdrv, const BYTE *buff, DWORD sector, UINT count) } /* erase memory before writing to it */ - int res = mtd_erase(fatfs_mtd_devs[pdrv], - sector * fatfs_mtd_devs[pdrv]->page_size, - count * fatfs_mtd_devs[pdrv]->page_size); + int res = mtd_erase_sector(fatfs_mtd_devs[pdrv], sector, count); if (res != 0) { return RES_ERROR; /* erase failed! */ } - uint32_t nwrite = count * fatfs_mtd_devs[pdrv]->page_size; - res = mtd_write(fatfs_mtd_devs[pdrv], buff, - sector * fatfs_mtd_devs[pdrv]->page_size, - nwrite); + uint32_t sector_size = fatfs_mtd_devs[pdrv]->page_size + * fatfs_mtd_devs[pdrv]->pages_per_sector; + + res = mtd_write_page(fatfs_mtd_devs[pdrv], buff, + sector, 0, count * sector_size); if (res != 0) { return RES_ERROR; } - assert((nwrite / fatfs_mtd_devs[pdrv]->page_size) == count); return RES_OK; } From 68a47b63e32058f4cf0832889334ef2d45dd4796 Mon Sep 17 00:00:00 2001 From: Benjamin Valentin Date: Thu, 7 May 2020 15:07:47 +0200 Subject: [PATCH 04/10] drivers/mtd_spi_nor: implement mtd_spi_nor_write_page() --- drivers/mtd_spi_nor/mtd_spi_nor.c | 50 +++++++++++++++++++++++++------ 1 file changed, 41 insertions(+), 9 deletions(-) diff --git a/drivers/mtd_spi_nor/mtd_spi_nor.c b/drivers/mtd_spi_nor/mtd_spi_nor.c index 5828acc8a2..d75f689aa6 100644 --- a/drivers/mtd_spi_nor/mtd_spi_nor.c +++ b/drivers/mtd_spi_nor/mtd_spi_nor.c @@ -56,6 +56,8 @@ #define MBIT_AS_BYTES ((1024 * 1024) / 8) +#define MIN(a, b) ((a) > (b) ? (b) : (a)) + /** * @brief JEDEC memory manufacturer ID codes. * @@ -75,14 +77,6 @@ static int mtd_spi_nor_write(mtd_dev_t *mtd, const void *src, uint32_t addr, uin static int mtd_spi_nor_erase(mtd_dev_t *mtd, uint32_t addr, uint32_t size); static int mtd_spi_nor_power(mtd_dev_t *mtd, enum mtd_power_state power); -const mtd_desc_t mtd_spi_nor_driver = { - .init = mtd_spi_nor_init, - .read = mtd_spi_nor_read, - .write = mtd_spi_nor_write, - .erase = mtd_spi_nor_erase, - .power = mtd_spi_nor_power, -}; - static void mtd_spi_acquire(const mtd_spi_nor_t *dev) { spi_acquire(dev->params->spi, dev->params->cs, @@ -464,7 +458,7 @@ static int mtd_spi_nor_read(mtd_dev_t *mtd, void *dest, uint32_t addr, uint32_t DEBUG("mtd_spi_nor_read: %p, %p, 0x%" PRIx32 ", 0x%" PRIx32 "\n", (void *)mtd, dest, addr, size); const mtd_spi_nor_t *dev = (mtd_spi_nor_t *)mtd; - size_t chipsize = mtd->page_size * mtd->pages_per_sector * mtd->sector_count; + uint32_t chipsize = mtd->page_size * mtd->pages_per_sector * mtd->sector_count; if (addr > chipsize) { return -EOVERFLOW; } @@ -521,6 +515,35 @@ static int mtd_spi_nor_write(mtd_dev_t *mtd, const void *src, uint32_t addr, uin return 0; } +static int mtd_spi_nor_write_page(mtd_dev_t *mtd, const void *src, uint32_t page, uint32_t offset, + uint32_t size) +{ + const mtd_spi_nor_t *dev = (mtd_spi_nor_t *)mtd; + + DEBUG("mtd_spi_nor_write_page: %p, %p, 0x%" PRIx32 ", 0x%" PRIx32 ", 0x%" PRIx32 "\n", + (void *)mtd, src, page, offset, size); + + uint32_t remaining = mtd->page_size - offset; + size = MIN(remaining, size); + + be_uint32_t addr_be = byteorder_htonl(page * mtd->page_size + offset); + + mtd_spi_acquire(dev); + + /* write enable */ + mtd_spi_cmd(dev, dev->params->opcode->wren); + + /* Page program */ + mtd_spi_cmd_addr_write(dev, dev->params->opcode->page_program, addr_be, src, size); + + /* waiting for the command to complete before returning */ + wait_for_write_complete(dev, 0); + + mtd_spi_release(dev); + + return size; +} + static int mtd_spi_nor_erase(mtd_dev_t *mtd, uint32_t addr, uint32_t size) { DEBUG("mtd_spi_nor_erase: %p, 0x%" PRIx32 ", 0x%" PRIx32 "\n", @@ -619,3 +642,12 @@ static int mtd_spi_nor_power(mtd_dev_t *mtd, enum mtd_power_state power) return 0; } + +const mtd_desc_t mtd_spi_nor_driver = { + .init = mtd_spi_nor_init, + .read = mtd_spi_nor_read, + .write = mtd_spi_nor_write, + .write_page = mtd_spi_nor_write_page, + .erase = mtd_spi_nor_erase, + .power = mtd_spi_nor_power, +}; From c2492209d8d0a4b498ed5a5fb28a3e9a0863cac8 Mon Sep 17 00:00:00 2001 From: Benjamin Valentin Date: Thu, 7 May 2020 15:10:07 +0200 Subject: [PATCH 05/10] pkg/littlefs2: use page-addressed MTD operations --- pkg/littlefs2/fs/littlefs2_fs.c | 23 +++++------------------ 1 file changed, 5 insertions(+), 18 deletions(-) diff --git a/pkg/littlefs2/fs/littlefs2_fs.c b/pkg/littlefs2/fs/littlefs2_fs.c index 314548ff2c..e9dbc03887 100644 --- a/pkg/littlefs2/fs/littlefs2_fs.c +++ b/pkg/littlefs2/fs/littlefs2_fs.c @@ -72,7 +72,8 @@ static int _dev_read(const struct lfs_config *c, lfs_block_t block, DEBUG("lfs_read: c=%p, block=%" PRIu32 ", off=%" PRIu32 ", buf=%p, size=%" PRIu32 "\n", (void *)c, block, off, buffer, size); - return mtd_read(mtd, buffer, ((fs->base_addr + block) * c->block_size) + off, size); + return mtd_read_page(mtd, buffer, (fs->base_addr + block) * mtd->pages_per_sector, + off, size); } static int _dev_write(const struct lfs_config *c, lfs_block_t block, @@ -84,17 +85,8 @@ static int _dev_write(const struct lfs_config *c, lfs_block_t block, DEBUG("lfs_write: c=%p, block=%" PRIu32 ", off=%" PRIu32 ", buf=%p, size=%" PRIu32 "\n", (void *)c, block, off, buffer, size); - const uint8_t *buf = buffer; - uint32_t addr = ((fs->base_addr + block) * c->block_size) + off; - for (const uint8_t *part = buf; part < buf + size; part += c->prog_size, - addr += c->prog_size) { - int ret = mtd_write(mtd, part, addr, c->prog_size); - if (ret != 0) { - return ret; - } - } - - return 0; + return mtd_write_page(mtd, buffer, (fs->base_addr + block) * mtd->pages_per_sector, + off, size); } static int _dev_erase(const struct lfs_config *c, lfs_block_t block) @@ -104,12 +96,7 @@ static int _dev_erase(const struct lfs_config *c, lfs_block_t block) DEBUG("lfs_erase: c=%p, block=%" PRIu32 "\n", (void *)c, block); - int ret = mtd_erase(mtd, ((fs->base_addr + block) * c->block_size), c->block_size); - if (ret >= 0) { - return 0; - } - - return ret; + return mtd_erase_sector(mtd, fs->base_addr + block, 1); } static int _dev_sync(const struct lfs_config *c) From 7c8fe862ce246e605e6e878cba750a7c4bf9cd8c Mon Sep 17 00:00:00 2001 From: Benjamin Valentin Date: Thu, 7 May 2020 17:52:08 +0200 Subject: [PATCH 06/10] drivers/at25xxx: export page-wise write function Export the _write_page() function so it can be used by the MTD subsystem. --- drivers/at25xxx/at25xxx.c | 56 +++++++++++++++++++++++++++++++-------- drivers/at25xxx/mtd/mtd.c | 10 +++++++ drivers/include/at25xxx.h | 16 +++++++++++ 3 files changed, 71 insertions(+), 11 deletions(-) diff --git a/drivers/at25xxx/at25xxx.c b/drivers/at25xxx/at25xxx.c index 33509b9cf0..7e53b8831c 100644 --- a/drivers/at25xxx/at25xxx.c +++ b/drivers/at25xxx/at25xxx.c @@ -26,6 +26,7 @@ #include "at25xxx.h" #include "at25xxx_constants.h" #include "at25xxx_params.h" +#include "bitarithm.h" #include "byteorder.h" #include "xtimer.h" @@ -81,12 +82,14 @@ static inline int _wait_until_eeprom_ready(const at25xxx_t *dev) return tries == 0 ? -ETIMEDOUT : 0; } -static ssize_t _write_page(const at25xxx_t *dev, uint32_t pos, const void *data, size_t len) +static int _at25xxx_write_page(const at25xxx_t *dev, uint32_t page, uint32_t offset, const void *data, size_t len) { + assert(offset < PAGE_SIZE); + /* write no more than to the end of the current page to prevent wrap-around */ - size_t remaining = PAGE_SIZE - (pos & (PAGE_SIZE - 1)); + size_t remaining = PAGE_SIZE - offset; len = min(len, remaining); - pos = _pos(CMD_WRITE, pos); + uint32_t pos = _pos(CMD_WRITE, page * PAGE_SIZE + offset); /* wait for previous write to finish - may take up to 5 ms */ int res = _wait_until_eeprom_ready(dev); @@ -111,6 +114,17 @@ static ssize_t _write_page(const at25xxx_t *dev, uint32_t pos, const void *data, return len; } +int at25xxx_write_page(const at25xxx_t *dev, uint32_t page, uint32_t offset, const void *data, size_t len) +{ + int res; + + getbus(dev); + res = _at25xxx_write_page(dev, page, offset, data, len); + spi_release(dev->params.spi); + + return res; +} + int at25xxx_write(const at25xxx_t *dev, uint32_t pos, const void *data, size_t len) { int res = 0; @@ -120,18 +134,32 @@ int at25xxx_write(const at25xxx_t *dev, uint32_t pos, const void *data, size_t l return -ERANGE; } + /* page size is always a power of two */ + const uint32_t page_shift = bitarithm_msb(PAGE_SIZE); + const uint32_t page_mask = PAGE_SIZE - 1; + + uint32_t page = pos >> page_shift; + uint32_t offset = pos & page_mask; + getbus(dev); while (len) { - ssize_t written = _write_page(dev, pos, d, len); + ssize_t written = _at25xxx_write_page(dev, page, offset, d, len); + if (written < 0) { res = written; break; } len -= written; - pos += written; - d += written; + + if (len == 0) { + break; + } + + d += written; + page += (offset + written) >> page_shift; + offset = (offset + written) & page_mask; } spi_release(dev->params.spi); @@ -177,7 +205,6 @@ uint8_t at25xxx_read_byte(const at25xxx_t *dev, uint32_t pos) int at25xxx_set(const at25xxx_t *dev, uint32_t pos, uint8_t val, size_t len) { uint8_t data[AT225XXXX_SET_BUF_SIZE]; - size_t total = 0; if (pos + len > dev->params.size) { return -ERANGE; @@ -185,13 +212,20 @@ int at25xxx_set(const at25xxx_t *dev, uint32_t pos, uint8_t val, size_t len) memset(data, val, sizeof(data)); + /* page size is always a power of two */ + const uint32_t page_shift = bitarithm_msb(PAGE_SIZE); + const uint32_t page_mask = PAGE_SIZE - 1; + + uint32_t page = pos >> page_shift; + uint32_t offset = pos & page_mask; + getbus(dev); while (len) { - size_t written = _write_page(dev, pos, data, min(len, sizeof(data))); - len -= written; - pos += written; - total += written; + size_t written = _at25xxx_write_page(dev, page, offset, data, min(len, sizeof(data))); + len -= written; + page += (offset + written) >> page_shift; + offset = (offset + written) & page_mask; } spi_release(dev->params.spi); diff --git a/drivers/at25xxx/mtd/mtd.c b/drivers/at25xxx/mtd/mtd.c index 10b6dc2244..1c70573b4d 100644 --- a/drivers/at25xxx/mtd/mtd.c +++ b/drivers/at25xxx/mtd/mtd.c @@ -57,6 +57,15 @@ static int mtd_at25xxx_write(mtd_dev_t *dev, const void *buff, uint32_t addr, ui return at25xxx_write(mtd_at25xxx_->at25xxx_eeprom, addr, buff, size); } +static int mtd_at25xxx_write_page(mtd_dev_t *dev, const void *src, uint32_t page, uint32_t offset, + uint32_t size) +{ + DEBUG("[mtd_at25xxx] write_page: page:%" PRIu32 " offset:%" PRIu32 " size:%" PRIu32 "\n", + page, offset, size); + mtd_at25xxx_t *mtd_at25xxx_ = (mtd_at25xxx_t*)dev; + return at25xxx_write_page(mtd_at25xxx_->at25xxx_eeprom, page, offset, src, size); +} + static int mtd_at25xxx_erase(mtd_dev_t *dev, uint32_t addr, uint32_t size) { DEBUG("[mtd_at25xxx] mtd_at25xxx_erase: addr:%" PRIu32 " size:%" PRIu32 "\n", addr, size); @@ -78,6 +87,7 @@ const mtd_desc_t mtd_at25xxx_driver = { .init = mtd_at25xxx_init, .read = mtd_at25xxx_read, .write = mtd_at25xxx_write, + .write_page = mtd_at25xxx_write_page, .erase = mtd_at25xxx_erase, .power = mtd_at25xxx_power, }; diff --git a/drivers/include/at25xxx.h b/drivers/include/at25xxx.h index e4bd598234..7d8119defc 100644 --- a/drivers/include/at25xxx.h +++ b/drivers/include/at25xxx.h @@ -109,6 +109,22 @@ void at25xxx_write_byte(const at25xxx_t *dev, uint32_t pos, uint8_t data); */ int at25xxx_write(const at25xxx_t *dev, uint32_t pos, const void *data, size_t len); +/** + * @brief Sequentially write @p len bytes to a given @p page. + * The function will write up to the page boundary and then return. + * + * @param[in] dev AT25XXX device handle + * @param[in] page page of EEPROM memory + * @param[in] offset offset from the start of the page, must be < page size + * @param[in] data write buffer + * @param[in] len requested length to be written + * + * @return number of bytes written on success + * @return error on failure + */ +int at25xxx_write_page(const at25xxx_t *dev, uint32_t page, uint32_t offset, + const void *data, size_t len); + /** * @brief Set @p len bytes from a given position @p pos to the * value @p val From f3500257b1b573881651d6f4bf55181502c56aa1 Mon Sep 17 00:00:00 2001 From: Benjamin Valentin Date: Thu, 7 May 2020 20:05:08 +0200 Subject: [PATCH 07/10] pkg/littlefs: use page-addressed MTD operations --- pkg/littlefs/fs/littlefs_fs.c | 23 +++++------------------ 1 file changed, 5 insertions(+), 18 deletions(-) diff --git a/pkg/littlefs/fs/littlefs_fs.c b/pkg/littlefs/fs/littlefs_fs.c index 570be591c0..b053ab754e 100644 --- a/pkg/littlefs/fs/littlefs_fs.c +++ b/pkg/littlefs/fs/littlefs_fs.c @@ -72,7 +72,8 @@ static int _dev_read(const struct lfs_config *c, lfs_block_t block, DEBUG("lfs_read: c=%p, block=%" PRIu32 ", off=%" PRIu32 ", buf=%p, size=%" PRIu32 "\n", (void *)c, block, off, buffer, size); - return mtd_read(mtd, buffer, ((fs->base_addr + block) * c->block_size) + off, size); + return mtd_read_page(mtd, buffer, (fs->base_addr + block) * mtd->pages_per_sector, + off, size); } static int _dev_write(const struct lfs_config *c, lfs_block_t block, @@ -84,17 +85,8 @@ static int _dev_write(const struct lfs_config *c, lfs_block_t block, DEBUG("lfs_write: c=%p, block=%" PRIu32 ", off=%" PRIu32 ", buf=%p, size=%" PRIu32 "\n", (void *)c, block, off, buffer, size); - const uint8_t *buf = buffer; - uint32_t addr = ((fs->base_addr + block) * c->block_size) + off; - for (const uint8_t *part = buf; part < buf + size; part += c->prog_size, - addr += c->prog_size) { - int ret = mtd_write(mtd, part, addr, c->prog_size); - if (ret != 0) { - return ret; - } - } - - return 0; + return mtd_write_page(mtd, buffer, (fs->base_addr + block) * mtd->pages_per_sector, + off, size); } static int _dev_erase(const struct lfs_config *c, lfs_block_t block) @@ -104,12 +96,7 @@ static int _dev_erase(const struct lfs_config *c, lfs_block_t block) DEBUG("lfs_erase: c=%p, block=%" PRIu32 "\n", (void *)c, block); - int ret = mtd_erase(mtd, ((fs->base_addr + block) * c->block_size), c->block_size); - if (ret >= 0) { - return 0; - } - - return ret; + return mtd_erase_sector(mtd, fs->base_addr + block, 1); } static int _dev_sync(const struct lfs_config *c) From 277452807baa1f0a96ecf9f9997aacf3d864213c Mon Sep 17 00:00:00 2001 From: Benjamin Valentin Date: Thu, 7 May 2020 23:26:02 +0200 Subject: [PATCH 08/10] cpu/esp_common: flash: implement write_page() --- cpu/esp_common/periph/flash.c | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/cpu/esp_common/periph/flash.c b/cpu/esp_common/periph/flash.c index c777f027ef..718df800aa 100644 --- a/cpu/esp_common/periph/flash.c +++ b/cpu/esp_common/periph/flash.c @@ -58,8 +58,8 @@ /* the external pointer to the system MTD device */ mtd_dev_t* mtd0 = 0; -mtd_dev_t _flash_dev; -mtd_desc_t _flash_driver; +static mtd_dev_t _flash_dev; +static mtd_desc_t _flash_driver; #ifdef MCU_ESP8266 @@ -77,6 +77,8 @@ extern uint32_t spi_flash_get_id(void); static int _flash_init (mtd_dev_t *dev); static int _flash_read (mtd_dev_t *dev, void *buff, uint32_t addr, uint32_t size); static int _flash_write (mtd_dev_t *dev, const void *buff, uint32_t addr, uint32_t size); +static int _flash_write_page (mtd_dev_t *dev, const void *buff, uint32_t page, + uint32_t offset, uint32_t size); static int _flash_erase (mtd_dev_t *dev, uint32_t addr, uint32_t size); static int _flash_power (mtd_dev_t *dev, enum mtd_power_state power); @@ -120,6 +122,7 @@ void spi_flash_drive_init (void) _flash_driver.init = &_flash_init; _flash_driver.read = &_flash_read; _flash_driver.write = &_flash_write; + _flash_driver.write_page = &_flash_write_page; _flash_driver.erase = &_flash_erase; _flash_driver.power = &_flash_power; @@ -528,6 +531,16 @@ static int _flash_write (mtd_dev_t *dev, const void *buff, uint32_t addr, uint32 return (spi_flash_write(_flash_beg + addr, buff, size) == ESP_OK) ? 0 : -EIO; } +static int _flash_write_page (mtd_dev_t *dev, const void *buff, uint32_t page, uint32_t offset, + uint32_t size) +{ + uint32_t addr = _flash_beg + page * _flashchip->page_size + offset; + uint32_t remaining = _flashchip->page_size - offset; + size = MIN(size, remaining); + + return (spi_flash_write(addr, buff, size) == ESP_OK) ? (int) size : -EIO; +} + static int _flash_erase (mtd_dev_t *dev, uint32_t addr, uint32_t size) { DEBUG("%s dev=%p addr=%08x size=%u\n", __func__, dev, addr, size); From 02f37b998a2b2e78e5cf23d293df3348c987b9cb Mon Sep 17 00:00:00 2001 From: Benjamin Valentin Date: Wed, 24 Jun 2020 23:52:47 +0200 Subject: [PATCH 09/10] drivers/at24cxxx: export page-wise write function Export the _write_page() function so it can be used by the MTD subsystem. --- drivers/at24cxxx/at24cxxx.c | 82 ++++++++++++++++++++++++++----------- drivers/at24cxxx/mtd/mtd.c | 7 ++++ drivers/include/at24cxxx.h | 17 ++++++++ 3 files changed, 81 insertions(+), 25 deletions(-) diff --git a/drivers/at24cxxx/at24cxxx.c b/drivers/at24cxxx/at24cxxx.c index 53601fda44..d8de2d9022 100644 --- a/drivers/at24cxxx/at24cxxx.c +++ b/drivers/at24cxxx/at24cxxx.c @@ -103,6 +103,42 @@ int _read(const at24cxxx_t *dev, uint32_t pos, void *data, size_t len) return check; } +static +int _write_page(const at24cxxx_t *dev, uint32_t pos, const void *data, size_t len) +{ + int check; + uint8_t polls = DEV_MAX_POLLS; + uint8_t dev_addr; + uint16_t _pos; + uint8_t flags = 0; + + if (DEV_EEPROM_SIZE > 2048) { + /* 2 bytes word address length if more than 11 bits are + used for addressing */ + /* append page address bits to device address (if any) */ + dev_addr = (DEV_I2C_ADDR | ((pos & 0xFF0000) >> 16)); + _pos = (pos & 0xFFFF); + flags = I2C_REG16; + } + else { + /* append page address bits to device address (if any) */ + dev_addr = (DEV_I2C_ADDR | ((pos & 0xFF00) >> 8)); + _pos = pos & 0xFF; + } + + while (-ENXIO == (check = i2c_write_regs(DEV_I2C_BUS, dev_addr, + _pos, data, len, flags))) { + if (--polls == 0) { + break; + } + xtimer_usleep(AT24CXXX_POLL_DELAY_US); + } + + DEBUG("[at24cxxx] i2c_write_regs(): %d; polls: %d\n", check, polls); + + return check; +} + static int _write(const at24cxxx_t *dev, uint32_t pos, const void *data, size_t len) { @@ -111,31 +147,9 @@ int _write(const at24cxxx_t *dev, uint32_t pos, const void *data, size_t len) while (len) { size_t clen = MIN(len, DEV_PAGE_SIZE - MOD_POW2(pos, DEV_PAGE_SIZE)); - uint8_t polls = DEV_MAX_POLLS; - uint8_t dev_addr; - uint16_t _pos; - uint8_t flags = 0; - if (DEV_EEPROM_SIZE > 2048) { - /* 2 bytes word address length if more than 11 bits are - used for addressing */ - /* append page address bits to device address (if any) */ - dev_addr = (DEV_I2C_ADDR | ((pos & 0xFF0000) >> 16)); - _pos = (pos & 0xFFFF); - flags = I2C_REG16; - } - else { - /* append page address bits to device address (if any) */ - dev_addr = (DEV_I2C_ADDR | ((pos & 0xFF00) >> 8)); - _pos = pos & 0xFF; - } - while (-ENXIO == (check = i2c_write_regs(DEV_I2C_BUS, dev_addr, - _pos, cdata, clen, flags))) { - if (--polls == 0) { - break; - } - xtimer_usleep(AT24CXXX_POLL_DELAY_US); - } - DEBUG("[at24cxxx] i2c_write_regs(): %d; polls: %d\n", check, polls); + + check = _write_page(dev, pos, cdata, clen); + if (!check) { len -= clen; pos += clen; @@ -243,6 +257,24 @@ int at24cxxx_write(const at24cxxx_t *dev, uint32_t pos, const void *data, return check; } +int at24cxxx_write_page(const at24cxxx_t *dev, uint32_t page, uint32_t offset, + const void *data, size_t len) +{ + int check; + + assert(offset < DEV_PAGE_SIZE); + + /* write no more than to the end of the current page to prevent wrap-around */ + size_t remaining = DEV_PAGE_SIZE - offset; + len = MIN(len, remaining); + + i2c_acquire(DEV_I2C_BUS); + check = _write_page(dev, page * DEV_PAGE_SIZE + offset, data, len); + i2c_release(DEV_I2C_BUS); + + return check ? check : (int) len; +} + int at24cxxx_set(const at24cxxx_t *dev, uint32_t pos, uint8_t val, size_t len) { diff --git a/drivers/at24cxxx/mtd/mtd.c b/drivers/at24cxxx/mtd/mtd.c index dd36e54a26..1a69469d6c 100644 --- a/drivers/at24cxxx/mtd/mtd.c +++ b/drivers/at24cxxx/mtd/mtd.c @@ -56,6 +56,12 @@ static int _mtd_at24cxxx_write(mtd_dev_t *mtd, const void *src, uint32_t addr, return at24cxxx_write(DEV(mtd), addr, src, size) == AT24CXXX_OK ? 0 : -EIO; } +static int mtd_at24cxxx_write_page(mtd_dev_t *mtd, const void *src, uint32_t page, + uint32_t offset, uint32_t size) +{ + return at24cxxx_write_page(DEV(mtd), page, offset, src, size); +} + static int _mtd_at24cxxx_erase(mtd_dev_t *mtd, uint32_t addr, uint32_t size) { return at24cxxx_clear(DEV(mtd), addr, size) == AT24CXXX_OK ? 0 : -EIO; @@ -72,6 +78,7 @@ const mtd_desc_t mtd_at24cxxx_driver = { .init = _mtd_at24cxxx_init, .read = _mtd_at24cxxx_read, .write = _mtd_at24cxxx_write, + .write_page = mtd_at24cxxx_write_page, .erase = _mtd_at24cxxx_erase, .power = _mtd_at24cxxx_power }; diff --git a/drivers/include/at24cxxx.h b/drivers/include/at24cxxx.h index 5e6874df48..a291550c31 100644 --- a/drivers/include/at24cxxx.h +++ b/drivers/include/at24cxxx.h @@ -129,6 +129,23 @@ int at24cxxx_write_byte(const at24cxxx_t *dev, uint32_t pos, uint8_t data); int at24cxxx_write(const at24cxxx_t *dev, uint32_t pos, const void *data, size_t len); +/** + * @brief Sequentially write @p len bytes to a given @p page. + * The function will write up to the page boundary and then return + * the number of bytes written up to that. + * + * @param[in] dev AT24CXXX device handle + * @param[in] page page of EEPROM memory + * @param[in] offset offset from the start of the page, must be < page size + * @param[in] data write buffer + * @param[in] len requested length to be written + * + * @return number of bytes written on success + * @return error on failure + */ +int at24cxxx_write_page(const at24cxxx_t *dev, uint32_t page, uint32_t offset, + const void *data, size_t len); + /** * @brief Set @p len bytes from a given position @p pos to the * value @p val From 4d41d7f0699f87397f639364d40a865e2e9da16f Mon Sep 17 00:00:00 2001 From: Benjamin Valentin Date: Tue, 18 Aug 2020 14:59:56 +0200 Subject: [PATCH 10/10] tests/pkg_fatfs_vfs: blacklist nucleo-l031k6 --- tests/pkg_fatfs_vfs/Makefile.ci | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/pkg_fatfs_vfs/Makefile.ci b/tests/pkg_fatfs_vfs/Makefile.ci index 41426e51ea..bfa6164a4f 100644 --- a/tests/pkg_fatfs_vfs/Makefile.ci +++ b/tests/pkg_fatfs_vfs/Makefile.ci @@ -7,6 +7,7 @@ BOARD_INSUFFICIENT_MEMORY := \ msb-430 \ msb-430h \ nucleo-f031k6 \ + nucleo-l031k6 \ nucleo-f042k6 \ stm32f030f4-demo \ telosb \