Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

drivers/mtd_sdmmc: always enable the erase function #20180

Merged
merged 5 commits into from
Dec 14, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 24 additions & 0 deletions drivers/include/mtd.h
Original file line number Diff line number Diff line change
Expand Up @@ -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 overwritten. The [Linux MTD FAQ](http://www.linux-mtd.infradead.org/faq/general.html)

Check warning on line 25 in drivers/include/mtd.h

View workflow job for this annotation

GitHub Actions / static-tests

line is longer than 100 characters
* 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
Expand Down Expand Up @@ -73,6 +73,7 @@
#ifndef MTD_H
#define MTD_H

#include <stdbool.h>
#include <stddef.h>
#include <stdint.h>

Expand Down Expand Up @@ -115,7 +116,7 @@
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) */

Check warning on line 119 in drivers/include/mtd.h

View workflow job for this annotation

GitHub Actions / static-tests

line is longer than 100 characters
#endif
} mtd_dev_t;

Expand Down Expand Up @@ -505,6 +506,29 @@
*/
int mtd_erase_sector(mtd_dev_t *mtd, uint32_t sector, uint32_t num);

/**
* @brief Write data to a MTD device with whole sector writes
*
* The MTD layer will take care of splitting up the transaction into multiple
* writes if it is required by the underlying storage media.
*
* The sectors will be erased before writing if needed.
*
* @param mtd Device to write to
* @param[in] src Buffer to write
* @param[in] sector Sector number to start writing to
* @param[in] num Number of sectors to write
*
* @retval n number of bytes written on success
* @retval <0 value on error
* @retval -ENODEV if @p mtd is not a valid device
* @retval -ENOTSUP if operation is not supported on @p mtd
* @retval -EOVERFLOW if @p addr or @p count are not valid, i.e. outside memory,
* @retval -EIO if I/O error occurred
* @retval -EINVAL if parameters are invalid
*/
int mtd_write_sector(mtd_dev_t *mtd, const void *src, uint32_t sector, uint32_t num);

/**
* @brief Set power mode on a MTD device
*
Expand Down
22 changes: 0 additions & 22 deletions drivers/include/mtd_sdmmc.h
Original file line number Diff line number Diff line change
Expand Up @@ -45,28 +45,6 @@ typedef struct {
uint8_t sdmmc_idx; /**< SD/MMC peripheral index */
} mtd_sdmmc_t;

/**
* @defgroup drivers_mtd_sdmmc_config SD Memory Card driver compile configuration
* @ingroup config_drivers_storage
* @{
*/
/**
* @brief Enable SD Memory Card Erase
*
* SD Memory Cards and MMCs/eMMCs handle sector erase internally
* so it's possible to directly write to the card without erasing
* the sector first.
*
* @note An erase call will NOT touch the content if `CONFIG_MTD_SDMMC_ERASE`
* is not set, so enable this feature to ensure overriding the data.
*
* @pre This feature requires the `mtd_write_page` module.
*/
#ifdef DOXYGEN
#define CONFIG_MTD_SDMMC_ERASE
#endif
/** @} */

/**
* @brief sdcard device operations table for mtd
*/
Expand Down
14 changes: 14 additions & 0 deletions drivers/mtd/mtd.c
Original file line number Diff line number Diff line change
Expand Up @@ -393,6 +393,20 @@ int mtd_erase_sector(mtd_dev_t *mtd, uint32_t sector, uint32_t count)
return mtd->driver->erase_sector(mtd, sector, count);
}

int mtd_write_sector(mtd_dev_t *mtd, const void *data, uint32_t sector,
uint32_t count)
{
if (!(mtd->driver->flags & MTD_DRIVER_FLAG_DIRECT_WRITE)) {
int res = mtd_erase_sector(mtd, sector, count);
if (res) {
return res;
}
}

uint32_t page = sector * mtd->pages_per_sector;
return mtd_write_page_raw(mtd, data, page, 0, page * mtd->page_size);
}

int mtd_power(mtd_dev_t *mtd, enum mtd_power_state power)
{
if (!mtd || !mtd->driver) {
Expand Down
9 changes: 0 additions & 9 deletions drivers/mtd_sdmmc/Kconfig
Original file line number Diff line number Diff line change
Expand Up @@ -23,13 +23,4 @@ config SDMMC_GENERIC_MTD_OFFSET
the auto-configured SD Memory Card(s) or MMCs/eMMCs from
mtd_sdmmc_default will come after them.

config MTD_SDMMC_ERASE
bool "Enable SD Memory Card erase"
help
Enable this to erase sector before a data write operation (SD Memory
Cards only).
SD Memory Cards and MMCs/eMMCs handle sector erase internally
so it's possible to directly write to the card without erasing
the sector first hence this feature is disabled by default.

endif # KCONFIG_USEMODULE_MTD_SDMMC
30 changes: 2 additions & 28 deletions drivers/mtd_sdmmc/mtd_sdmmc.c
Original file line number Diff line number Diff line change
Expand Up @@ -141,35 +141,8 @@ static int mtd_sdmmc_write_page(mtd_dev_t *dev, const void *buff, uint32_t page,

static int mtd_sdmmc_erase_sector(mtd_dev_t *dev, uint32_t sector, uint32_t count)
{
#if IS_ACTIVE(CONFIG_MTD_SDMMC_ERASE) && IS_USED(MODULE_MTD_WRITE_PAGE)
mtd_sdmmc_t *mtd_sd = (mtd_sdmmc_t*)dev;

DEBUG("mtd_sdmmc_erase_sector: sector: %" PRIu32 " count: %" PRIu32 "\n",
sector, count);

if (dev->work_area == NULL) {
DEBUG("mtd_sdmmc_erase_sector: no work area\n");
return -ENOTSUP;
}
memset(dev->work_area, 0, SDMMC_SDHC_BLOCK_SIZE);
while (count) {
if (sdmmc_write_blocks(mtd_sd->sdmmc, sector, SDMMC_SDHC_BLOCK_SIZE,
1, dev->work_area, NULL)) {
return -EIO;
}
--count;
++sector;
}
Comment on lines -154 to -162
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What I don't understand is why you added this fallback implementation when you have a proper sdmmc_erase_blocks().

This was only needed for sdcard_spi because it does not implement the erase command at all.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What I don't understand is why you added this fallback implementation when you have a proper sdmmc_erase_blocks().

To be honest, I don't remember. Probably, I just used mtd_sdcard_spi as a template to implement mtd_sdmmc so that it is just a copy of it. I didn't know the MTD_DRIVER_FLAG_DIRECT_WRITE flag.

#else
(void)dev;
(void)sector;
(void)count;
mtd_sdmmc_t *mtd_sd = (mtd_sdmmc_t*)dev;
if (IS_ACTIVE(CONFIG_MTD_SDMMC_ERASE)) {
return sdmmc_erase_blocks(mtd_sd->sdmmc, sector, count);
}
#endif
return 0;
return sdmmc_erase_blocks(mtd_sd->sdmmc, sector, count);
}

static int mtd_sdmmc_power(mtd_dev_t *dev, enum mtd_power_state power)
Expand Down Expand Up @@ -218,6 +191,7 @@ const mtd_desc_t mtd_sdmmc_driver = {
.write_page = mtd_sdmmc_write_page,
.erase_sector = mtd_sdmmc_erase_sector,
.power = mtd_sdmmc_power,
.flags = MTD_DRIVER_FLAG_DIRECT_WRITE,
};

#if IS_USED(MODULE_MTD_SDMMC_DEFAULT)
Expand Down
17 changes: 3 additions & 14 deletions pkg/fatfs/fatfs_diskio/mtd/mtd_diskio.c
Original file line number Diff line number Diff line change
Expand Up @@ -130,24 +130,13 @@ DRESULT disk_read(BYTE pdrv, BYTE *buff, DWORD sector, UINT count)
*/
DRESULT disk_write(BYTE pdrv, const BYTE *buff, DWORD sector, UINT count)
{
mtd_dev_t *mtd = fatfs_mtd_devs[pdrv];
DEBUG("disk_write: %d, %lu, %d\n", pdrv, (long unsigned)sector, count);
if ((pdrv >= FF_VOLUMES) || (fatfs_mtd_devs[pdrv]->driver == NULL)) {
if ((pdrv >= FF_VOLUMES) || (mtd->driver == NULL)) {
return RES_PARERR;
}

/* erase memory before writing to it */
int res = mtd_erase_sector(fatfs_mtd_devs[pdrv], sector, count);

if (res != 0) {
return RES_ERROR; /* erase failed! */
}

uint32_t sector_size = fatfs_mtd_devs[pdrv]->page_size
* fatfs_mtd_devs[pdrv]->pages_per_sector;

res = mtd_write_page_raw(fatfs_mtd_devs[pdrv], buff,
sector, 0, count * sector_size);

int res = mtd_write_sector(mtd, buff, sector, count);
if (res != 0) {
return RES_ERROR;
}
Expand Down
14 changes: 3 additions & 11 deletions pkg/lwext4/fs/lwext4_fs.c
Original file line number Diff line number Diff line change
Expand Up @@ -94,20 +94,12 @@ static int blockdev_bwrite(struct ext4_blockdev *bdev, const void *buf,
{
mtd_dev_t *dev = bdev->bdif->p_user;

uint32_t page = blk_id * dev->pages_per_sector;
uint32_t size = blk_cnt * dev->pages_per_sector * dev->page_size;

assert(blk_id <= UINT32_MAX);

DEBUG("lwext4: erase %"PRIu32" sectors starting with %"PRIu64"\n", blk_cnt, blk_id);
int res = mtd_erase_sector(dev, blk_id, blk_cnt);
if (res) {
return -res;
}

DEBUG("lwext4: write %"PRIu32" bytes to page %"PRIu32"\n", size, page);
DEBUG("lwext4: write %"PRIu32" bytes to sector %"PRIu32"\n",
blk_cnt * dev->pages_per_sector * dev->page_size, (uint32_t)blk_id);

return -mtd_write_page_raw(dev, buf, page, 0, size);
return -mtd_write_sector(dev, buf, blk_id, blk_cnt);
}

static int prepare(lwext4_desc_t *fs, const char *mount_point)
Expand Down
Loading