MT29F4G08 NAND Flash Driver#100683
Conversation
etienne-lms
left a comment
There was a problem hiding this comment.
Minor comments only.
a5ce021 to
7514a70
Compare
7514a70 to
3aeb26d
Compare
| (void)FMC_NAND_CommonSpace_Timing_Init(dev_data->instance, &timing, | ||
| dev_data->init.NandBank); | ||
| (void)FMC_NAND_AttributeSpace_Timing_Init(dev_data->instance, &timing, | ||
| dev_data->init.NandBank); |
There was a problem hiding this comment.
In theory, we're not supposed to call directly these apis, which are not meant to be public, but API internal to the HAL driver. As such there is no guarantee on these APIs (portability, stability, ..).
Can you detail why you chose not to call HAL_NAND_Init() directly ?
There was a problem hiding this comment.
The STM32 HAL NAND driver does not provide the entire functionality we need (DMA support, status read, etc.). Additionally, there is no intend to further develop it. This is the reasoning I got from ST's official support: "The NAND is not so popular since the xSPI memories we see the FMC usage less and less."
There was a problem hiding this comment.
The STM32 HAL NAND driver does not provide the entire functionality we need (DMA support, status read, etc.).
Ok, that's clear. However, I need to check internally if FMC_NAND_() usage is safe.
| *(int *)out = | ||
| ((ret == 0) && (spare_area[0] != 0xFF)) ? NAND_BLOCK_BAD : NAND_BLOCK_GOOD; | ||
| break; | ||
| } |
There was a problem hiding this comment.
I don't think these braces (in both case sequences) are useful. There indentation could be confusing. Could you remove them?
There was a problem hiding this comment.
The advantage is to define operation specific variables in the block. Also, the indentation remains the same.
| struct nand_flash_address address; | ||
| uint8_t spare_area[config->spare_area_size]; | ||
|
|
||
| if (block * config->block_size >= config->flash_size) { |
There was a problem hiding this comment.
| if (block * config->block_size >= config->flash_size) { | |
| off_t offset; | |
| if (u32_mul_overflow(block, config->block_size, &offset) || (offset >= config->flash_size)) { |
and below:
- address = flash_mt29f4g08_calculate_address(config, block * config->block_size);
+ address = flash_mt29f4g08_calculate_address(config, offset);There was a problem hiding this comment.
Nice point. Thanks :)
| /* Get page data into buffer */ | ||
| for (size_t index = 0; index < dev_data->page_size; index++) { | ||
| config->page_buffer[index] = sys_read8(NAND_DEVICE); | ||
| } |
There was a problem hiding this comment.
Using config->page_buffer allocated in cache memory is useful when using DMAs, but here, maybe worth the straight load data read into the likely cached data buffer.
If using this scheme, you could avoid flash_stm32_fmc_nand_page_buffer_XXX[] allocations.
There was a problem hiding this comment.
What could work is something like:
/* Read chunk of page data directly into output buffer */
for (size_t index = 0; index < dev_data->page_size; index++) {
if (index >= page_offset && index < (page_offset + chunk)) {
*data = sys_read8(NAND_DEVICE);
data++;
} else {
(void)sys_read8(NAND_DEVICE);
}
}
In our case, this speeds up reading a page from 3.6 MiBps to 4.5 MiBps. Is this what you have imagined?
3aeb26d to
90bf138
Compare
|
I also implemented the suggestions from #100858 (comment) moving the extended operations to the generic definition. The filename did no more make sense why it is now called nand_flash.h for NAND flash specific data structures and maybe functions in future. |
90bf138 to
3076c54
Compare
Some API extensions are foreseen for bad block management by a flash translation layer (FTL). Signed-off-by: Pascal Linder <pascal.linder@zuehlke.com>
Introduces a NAND flash driver for Micron's MT29F4G08 memory device that is configurable via devicetree. A generic flash controller driver communicates to the flexible memory controller of STM32 MCUs. Any other NAND flash driver can be implemented to work via FMC in the future. The chip-specific driver provides the flash API for upper layers and enables the on-die ECC feature. As it is the first NAND flash driver in Zephyr, some API extensions are foreseen for bad block management by a flash translation layer (FTL). Signed-off-by: Pascal Linder <pascal.linder@zuehlke.com> Signed-off-by: Tim Pambor <tim.pambor@codewrights.de>
Renames overlays to be board-specific and adds a build test for the newly introduced NAND flash driver for MT29F4G08 via STM32 FMC. Signed-off-by: Pascal Linder <pascal.linder@zuehlke.com>
3076c54 to
a47feb5
Compare
| # SPDX-License-Identifier: Apache-2.0 | ||
|
|
||
| description: | | ||
| NAND flash driver for Micron MT29F4G08 flash memory. |
There was a problem hiding this comment.
| NAND flash driver for Micron MT29F4G08 flash memory. | |
| Micron MT29F4G08 NAND flash memory. |
|
|
||
| compatible: "micron,mt29f4g08" | ||
|
|
||
| properties: |
There was a problem hiding this comment.
Please add a description to all the properties.
|
| flash_mt29f4g08_calculate_address(config, offset); | ||
|
|
||
| ret = flash_stm32_fmc_nand_read_page_chunk(controller, &address, page_offset, chunk, | ||
| (uint8_t *)data); |
There was a problem hiding this comment.
Sorry if this was asked before but should not this driver be controller-independent instead? It seems every vendor will have to maintain their own variant of flash_vendor_mt29f4g08.c.
There was a problem hiding this comment.
First, the split between flash_stm32_fmc_nand.c and flash_stm32_fmc_mt29f4g08.c is due to the MT29F4G08 driver making use of on‑die ECC, meaning error detection and correction are handled inside the NAND device itself. This is a vendor‑specific feature. As a result, supporting on‑die ECC reliably requires a NAND flash vendor‑specific flash driver rather than a fully generic one.
It would indeed be possible to add a controller‑specific but flash chip vendor‑agnostic driver such as a hypothetical flash_stm32_fmc_onfi.c. Such a driver could work with ONFI‑compliant NAND devices across different vendors. However, to stay generic, it would need to rely on host ECC (calculated in the MCU) or software ECC, and therefore could not take advantage of on‑die ECC features provided by some flashes.
The reason these drivers are NAND controller‑dependent is timing. Parallel NAND flashes have fairly strict and sometimes subtle timing requirements (setup/hold times, access times, etc.), and each controller exposes its own mechanism and parameters for generating those timings. In practice, it is very difficult to define a standardized, controller‑independent interface that maps cleanly onto the timing configuration model of every NAND controller. For the same reason, we also have vendor‑specific bindings for e.g. SDRAM, where the timing parameters are inherently tied to the memory controller’s register layout and capabilities. See, e.g.:
zephyr/dts/bindings/memory-controllers/st,stm32-fmc-sdram.yaml
Lines 142 to 166 in 32823f3
There was a problem hiding this comment.
First, the split between
flash_stm32_fmc_nand.candflash_stm32_fmc_mt29f4g08.cis due to the MT29F4G08 driver making use of on‑die ECC, meaning error detection and correction are handled inside the NAND device itself. This is a vendor‑specific feature. As a result, supporting on‑die ECC reliably requires a NAND flash vendor‑specific flash driver rather than a fully generic one.
It would indeed be possible to add a controller‑specific but flash chip vendor‑agnostic driver such as a hypothetical
flash_stm32_fmc_onfi.c. Such a driver could work with ONFI‑compliant NAND devices across different vendors. However, to stay generic, it would need to rely on host ECC (calculated in the MCU) or software ECC, and therefore could not take advantage of on‑die ECC features provided by some flashes.
The reason these drivers are NAND controller‑dependent is timing. Parallel NAND flashes have fairly strict and sometimes subtle timing requirements (setup/hold times, access times, etc.), and each controller exposes its own mechanism and parameters for generating those timings. In practice, it is very difficult to define a standardized, controller‑independent interface that maps cleanly onto the timing configuration model of every NAND controller. For the same reason, we also have vendor‑specific bindings for e.g. SDRAM, where the timing parameters are inherently tied to the memory controller’s register layout and capabilities. See, e.g.:
zephyr/dts/bindings/memory-controllers/st,stm32-fmc-sdram.yaml
Lines 142 to 166 in 32823f3
I did not actually mean that there should be single driver that covers all NAND chips. I was asking if it is possible to move the calls to STM32 FMC inside flash_stm32_fmc_mt29f4g08.c behind some common API so that it becomes vendor-agnostic, such as flash_mt29f4g08.c. Otherwise, we are going to end up with flash_vendorA_mt29f4g08.c, flash_vendorB_mt29f4g08.c whenever someone decides to add support for their controller.
There was a problem hiding this comment.
I did not actually mean that there should be single driver that covers all NAND chips. I was asking if it is possible to move the calls to STM32 FMC inside
flash_stm32_fmc_mt29f4g08.cbehind some common API so that it becomes vendor-agnostic, such asflash_mt29f4g08.c. Otherwise, we are going to end up withflash_vendorA_mt29f4g08.c,flash_vendorB_mt29f4g08.cwhenever someone decides to add support for their controller.
Yes, I wanted to make clear both decisions, why it is flash-chip dependent and why it is flash-controller dependent as others might have the same question. The second part was about the flash-controller dependency.
The reason these drivers are NAND controller‑dependent is timing. Parallel NAND flashes have fairly strict and sometimes subtle timing requirements (setup/hold times, access times, etc.), and each controller exposes its own mechanism and parameters for generating those timings. In practice, it is very difficult to define a standardized, controller‑independent interface that maps cleanly onto the timing configuration model of every NAND controller. For the same reason, we also have vendor‑specific bindings for e.g. SDRAM, where the timing parameters are inherently tied to the memory controller’s register layout and capabilities. See, e.g.:
zephyr/dts/bindings/memory-controllers/st,stm32-fmc-sdram.yaml
Lines 142 to 166 in 32823f3
|
|
||
| /* Feature data. */ | ||
| uint8_t feature_data[4]; | ||
| }; |
There was a problem hiding this comment.
Is this header common for both serial/parallel nand? This feature structure looks for parallel nand with ONFI definition.
|
Thanks for this NTFL driver and do we have any real board testing available for this? and what testing scope shall we defined? |
| const struct flash_mt29f4g08_config *config = dev->config; | ||
| const struct device *controller = config->controller; | ||
| int ret; | ||
|
|
There was a problem hiding this comment.
Chrck 'len==0' first, if call is not going to do any work do not fail it on other checks.
| const struct flash_mt29f4g08_config *config = dev->config; | ||
| const struct device *controller = config->controller; | ||
| int ret; | ||
|
|
There was a problem hiding this comment.
Check size against 0 first, it is pointless to fail call when monwork is going to happen.
| reg: | ||
| type: int | ||
| required: true | ||
|
|
||
| page-size: | ||
| type: int | ||
| required: true | ||
|
|
||
| spare-area-size: | ||
| type: int | ||
| required: true | ||
|
|
||
| block-size: | ||
| type: int | ||
| required: true | ||
|
|
||
| plane-size: | ||
| type: int | ||
| required: true | ||
|
|
||
| flash-size: | ||
| type: int | ||
| required: true | ||
|
|
||
| setup-time: | ||
| type: int | ||
| default: 0 | ||
|
|
||
| wait-setup-time: | ||
| type: int | ||
| default: 0 | ||
|
|
||
| hold-setup-time: | ||
| type: int | ||
| default: 0 | ||
|
|
||
| hiz-setup-time: | ||
| type: int | ||
| default: 0 |
There was a problem hiding this comment.
Missing descriptions, units are not explained.
|
|
||
| /** | ||
| * Checks whether a block is marked as bad. As input it takes the address of the block | ||
| * (off_t *). As output it returns @ref flash_block_status (enum flash_block_status *). | ||
| */ | ||
| FLASH_IS_BAD_BLOCK, | ||
|
|
||
| /** | ||
| * Marks a block as bad. As input it takes the address of the block (off_t *). There is no | ||
| * output. | ||
| */ | ||
| FLASH_MARK_BAD_BLOCK, |
There was a problem hiding this comment.
Please explicitly number these. It is easier to debug when i do not have to count enums, and future depreciation, if happens, would be easier.
| const struct flash_mt29f4g08_config *config = dev->config; | ||
| const struct device *controller = config->controller; | ||
| int ret; | ||
|
|
There was a problem hiding this comment.
Check len first, and return 0 if no work is going to happen.
|
Currently, no capacity to work on this. |



Introduces a NAND flash driver for Micron's MT29F4G08 memory device that is configurable via devicetree. A generic flash controller driver communicates to the flexible memory controller of STM32 MCUs. Any other NAND flash driver can be implemented to work via FMC in the future. The chip-specific driver provides the flash API for upper layers and enables the on-die ECC feature.
The flash controller is strongly inspired by the STM32 HAL NAND driver following the ONFI specification. However, it optionally uses DMA to transfer a read page. In future, an additional function can be implemented for entire page reads. At the moment this is not required by upper layers and the direct memory transfer is somehow slower to a yet unknown problem.
As it is the first NAND flash driver in Zephyr, some API extensions are foreseen for bad block management by a flash translation layer (FTL). As proposed in #99908, we are currently working on a NFTL disk driver module based on Dhara. A first version is implemented and available here for testing.
As an optional enhancement, a bad block map could be kept in memory such that the driver does not need to ask the memory device for bad blocks on every request.