-
Notifications
You must be signed in to change notification settings - Fork 2k
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
mtd: Introduce write granularity #17683
Changes from all commits
df9226f
9ec4249
882f76a
35b57af
52ea93e
d764e03
88f4f7c
4714864
38d0ec5
b458d52
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Seems to be correct since address and length "should" be 4-byte aligned. |
||
|
||
DEBUG("%s flashchip chip_size=%d block_size=%d sector_size=%d page_size=%d\n", __func__, | ||
_flashchip->chip_size, _flashchip->block_size, | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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 | ||
* user (see the documentation of @ref mtd_write). | ||
* Unless a flag (such as @ref MTD_DRIVER_FLAG_DIRECT_WRITE or @ref | ||
* 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. | ||
jue89 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
* | ||
* 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 */ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thinking about it, doesn't this also apply for reads? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. (Weird, did GitHub reject my mail? If delayed, sorry for the duplicate.) It would (albeit as a separate parameter; eg. some STM32 flashes are So we could, but unlike the write case this is barely actionable. For *: Might cost an arm and a leg on the stack, or not if the driver (say) |
||
#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) | ||
Comment on lines
+126
to
+137
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is this information useful, yet? We don't know the erased state of the underlying storage ( |
||
|
||
/** | ||
* @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 | ||
* on both @p addr and @p buf. | ||
* but the buffer cannot overlap two pages. | ||
* | ||
* 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 | ||
|
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -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); | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can't we instead just
Suggested change
Seems pretty redundant to require the user to set this. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We could, but I value consistency more highly here (page size and pages per sector are set manually as well). |
||||||
|
||||||
/* offset + region size must not exceed the backing device */ | ||||||
assert(region->sector + region->mtd.sector_count <= backing_mtd->sector_count); | ||||||
|
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -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; | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
mci_ioctl(GET_SECTOR_COUNT, &dev->sector_count); | ||||||
|
||||||
DEBUG("sector size: %lu\n", dev->page_size); | ||||||
|
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -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; | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
return 0; | ||||||
} | ||||||
return -EIO; | ||||||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems to be correct, ESP32 SoCs use NOR flash chips.