nuttx
nuttx copied to clipboard
apache
RFQ: progmem.h: Modify interface to avoid terms like page, sector, and block, as these terms are used interchangeably by different silicon vendors.
Summary
Modify the progmem interface to avoid terms like page, sector, and block, as these terms are used interchangeably by different silicon vendors.
I have a bootloader that uses the progmem interface to program FLASH. I expected, given the common interface, that it would be possible to use the exact same code across multiple platforms. However, due to the interpretations of terms like page, sector, and block, the underlying drivers are inconsistent in their implementations, and therefore this currently does not work.
Some drivers refer to the smallest erasable unit of memory as a page, while others refer to it as a sector. Prior to this change, the up_progmem_pagesize function was used to get the smallest writable unit of memory - or, at least on some platforms that was the case. Clearly, the term page has different meanings, to different people.
This change attempts to reconcile this issue by modifying the common progmem interface and removing all references to page, sector, and block, since these terms can't be used consistently across vendors. Instead, descriptive terms are attempted to be used.
Additionally, some platforms were exposing additional chip specific functions such as stm32_flash_unlock, or stm32_flash_writeprotect that were not exposed by the common interface. A generic version of these functions has been added to the progmem interface.
I should mention that this is not the first time I've run into issues because of this inconsistency - and I am not alone. @davids5 and the PX4 community has worked around these issues as well.
I am looking for feedback on the naming and once we are happy with the interface change, I will go through each platform and adjust the drivers so that things are consistent.
Impact
Testing
"I think that mapping the "nuttx specified" naming should be done in the implementing code. For example, in stm32h7 stm_flash.c ( which @davids5 mentioned ), the mapping seems to be done correctly. STM32H7 name "SECTOR" really means smallest erasable unit (block) and "PAGE" means the write-granule (page). I guess there are drivers which map these differently (wrong) still; so these should also be fixed in order not to get burned."
Well, to be fair. It is really the STM32H7 that did not follow what all the other drivers did. It is the one that really introduced the disparity. In all other drivers, the getaddress and getpage calls returned what erasable unit you were in. On the H7 though, you got back the index for what writable unit you were in. And the claim that one is right and one is wrong is EXACTLY why I don't want to use sector or page terminology.
I hear what you are saying that we may need to expand the interface to include functions for both erasable and writable, but I refuse to accept that reintroducing the term page or sector as a good idea, because it's how the drivers got out of line in the first place.
@jlaitine As I mentioned, the H7 was the only platform that interpreted getpage as the index of writable units instead of erasable units. Can you explain why this was necessary? It stands out to me that no one needed this information before and makes me wonder what the use-case is for this info is.
@jlaitine As I mentioned, the H7 was the only platform that interpreted getpage as the index of writable units instead of erasable units. Can you explain why this was necessary? It stands out to me that no one needed this information before and makes me wonder what the use-case is for this info is.
I don't think the above is true; you need to look into other devices which use the progmem interface on NAND flash (with ECC correction).
I am not saying that everything in stm32h7 flash driver is good :) But it is extremely important that we maintain clear distinction between the eraseblocks and write/read unit. Otherwise it is impossible to use the interface with NAND flash devices, where the smallest read/write unit actually matters. On NOR flash devices it pretty much makes no difference. On some NAND flash devices the smallest erasable unit is so small that you can actually mix these up by also reading/writing the same size.
On stm32h7 there is no room for compromise, since it has got a 128KB (!) smallest erase size and 32 byte read/write size. However, I have seen it mapped to both PX4 flashparams and also nuttx littlefs (latter turned out to make no sense, but worked in principle). So the it can't be all broken...
It is also that this distinction is there on purpose in progmem.h; see the topmost commit touching it: " commit 0f18e8cc328f1a420135531eb2afe57b27d3332f Author: EunBong Song [email protected] Date: Fri Sep 21 03:18:38 2018 +0000 "
Also check out the commit history of "drivers/mtd/mtd_progmem.c"
I don't object naming changes as such, if there is a common view that it is important. I just 1) see the need of distinction between the "eraseblock" and "write/read unit" for NAND flash support and 2) personally liked the nuttx-way of naming these "block" and "page".
