STM32 flash driver cleanup and support for STM32L0 #25245
Conversation
drivers/flash/flash_stm32_generic.c
Outdated
There was a problem hiding this comment.
With the introduction of #25947 the WRITE_BLOCK_SIZE should be also assigned to flash_parameters.write_block_size , specific for the flash device.
There was a problem hiding this comment.
Wasn't this done already in the #25947? The code looked corrected when I rebased.
drivers/flash/flash_stm32_generic.c
Outdated
There was a problem hiding this comment.
Similar situation as with WRITE_BLOCK_SIZE
There was a problem hiding this comment.
Similar situation as with
WRITE_BLOCK_SIZE
I have updated flash_stm32.c to reflect this.
|
@andysan, from your code I would conclude that the L0/L1 don't have flash but an eeprom instead. If this is the case flash support for these devices is best added as a layer on top of the eeprom driver. Doing it like this would allow support for both a eeprom section and flash section on these devices. Also the flash erase can in that case be implemented as a write of 0xff. @de-nordic, @nvlsianpu, @pabigot, @carlescufi, the excellent work done in #25947 might have been unnecessary. |
I think you are right about the L0/L1 using an EEPROM for program memory. However, the manual refers to this as a flash, presumably because it requires explicit erase of 128 byte pages. In practice, that can't be implemented using the EEPROM API. IMO, the fact that we have different APIs for EEPROMs and FLASH memories is very problematic. It seems like the only real difference between the APIs is that the EEPROM API doesn't support explicit erase. Assuming that only NVMs with implicit erase operations are implemented using the EEPROM API, that's functionally OK. However, many of these devices support explicit erase operations which make future write operations. |
Maybe we will encounter actual flash devices in the future that do use an erase value different from 0xFF :) we'll be ready for it. |
Or we will end up merging bot APIs under one common for NVM API. |
I think that would be ideal. The current approach makes it really annoying to write code that either uses a flash or a data EEPROM since most subsystems seem to use the flash api. I imagine we would currently have to use the EEPROM API for things like FRAM or ReRAM, which seems a bit weird since they aren’t EEPROMs. It would be nice if the api could expose direct access to memory mapped devices as well to avoid additional copying. |
Please feel free to start a proposal to deprecate the EEPROM API and fold it into the flash API. We can continue the conversation there, and then we can bring it up in the API meeting. Thanks! |
disregarding the "unnecessary", I am happy :) |
After reading through some of the stm32 datasheets I can also remove the "unnecessary", the flash on these devices should be considered flash devices with an erase value of 0x00. Given their properties the flash page size could be defined as either the page size (128 bytes/ 256 bytes) or the sector size (4kB). Probably they are not a good fit for nvs (very slow word writes 3.2ms if the datasheet is correct) and it might be better to define them as flash with a write_block_size of a half page (64 bytes / 128 bytes) and to use them with fcb. I am also in favour of grouping support of NV devices under a NVM api, it is possible to add the type under |
|
@andysan I'll do some testing, but I don't have the boards right now. Should be done next week. |
erwango
left a comment
erwango
left a comment
There was a problem hiding this comment.
Tested ok on F0/F1/F3 targets
|
BK Failures: |
|
@nvlsianpu not at all |
Several STM32 chips have identical chip-specific code that has been duplicated in different source files. Unify the F0x, F1x, and F3x to use a single implementation. Signed-off-by: Andreas Sandberg <[email protected]>
Add support for STM32L0X using the generic STM32 backend. This is quite a significant change since the L0 series uses a slightly different flash controller. Refactor the generic backend to better support different block sizes and the L0's register interface. Signed-off-by: Andreas Sandberg <[email protected]>
I just pushed a rebase to trigger a rebuild to make sure it's all clean. |
|
One new unrelated CI failure:
One new unrelated CI failure: |
|
@superna9999 @nashif CI failures are not related to PR content. Can this be merged? |
10 participants
Several STM32 chips have identical chip-specific code that has been duplicated in different source files. To avoid adding another nearly identical backend, unify the F0x, F1x, and F3x (some of these only differed in copyright!) to use a single implementation and extend this to support the L0.
Some of the other implementations seem nearly identical as well, so we could likely get rid of the custom G0, G4, L4, and WBX implementation.
Testing: Tested with littlefs example on a b_l072z_lrwan1. Built the flash shell for an stm32f3_disco board. I don't have access to any boards to test the F0, F1, and F3 implementation.