STM32H7 flash-based params

[rippetoej]

Problem
The current implementation of flash-based params is not compatible with the STM32H7's embedded flash memory. There are two main issues:

1. The flashfs performs writes as small as 16 bits (when marking a file entry as erased), but the H7 flash perform writes via 256 bit flash words. Smaller writes can be performed via a force-write mechanism, but the 10 bit ECC is computed on the entire 256 bit word. Trying to modify the unproggramed bits in this flash word at a later time leads to an ECC error. From the reference manual:

Using a force-write operation prevents the application from updating later the missing bits with something else than 1, because it is likely that it will lead to permanent ECC error.

The current NuttX flash driver requires the write address and buffer contents be aligned on 256 bit boundaries, but the current flashfs makes no attempt to align writes according to device write page size.

2. Writing to a flash location in a none-erased state, will raise an ECC error flag since the contents of a previously written flash word have been modified. The current flashfs implementation violates this by changing the contents of the file status flag when marking it as erased.

Solutions

I've started looking into possible solutions, but wanted to verify something wasn't already in the works. Some ideas I have

1. Utilize the NuttX MTD driver - I don't know much about this driver, but it seems like a good choice for building a flash-based file system.

2. Build a small, MTD-like FS - This could make use of the progmem interface in NuttX to make a small, efficient FS with the sole purpose of storing and retrieving the single param 'file' in flash. The current flashfs could probably be extended to work on read, write, and erase pages instead of assuming byte level read and write access. I have only looked at the STM32F4 and STM32H7 flash drivers in linux, but I already see issues with how the various drivers are dealing with read/write pages and erase blocks (as NuttX calls them). While the H7 driver correctly reports these numbers, the F4 driver does not. Going this route might require modifications to a variety of NuttX flash drivers.

There is also the issue of making sure that whatever is implemented does not force users to abandon their currently saved parameters when changing over. So there are obviously a lot of things to consider here. Does anyone have any ideas on how we can solve this? Any comments?

[julianoes]

FYI @davids5

[davids5]

The issue with MTD is backing store RAM to allow a re-write of of a sector via read-erase-write(updated). The sector size is 128K. This is why the code is structured as it is and does not use backing store.
I think the simple thing is to obey the blocks size. See the boot loader code. It has a write cache that is 256 bits. Then requires the bin file to be padded to multiple of 8 bytes.

[rippetoej]

The issue with MTD is backing store RAM to allow a re-write of of a sector via read-erase-write(updated). The sector size is 128K. This is why the code is structured as it is and does not use backing store.

That makes sense.

I think the simple thing is to obey the blocks size. See the boot loader code. It has a write cache that is 256 bits. Then requires the bin file to be padded to multiple of 8 bytes.

Obeying the write block size is a start, but it is only part of the problem. The flashfs also rewrites a 2 byte flag in the file header when marking an old entry as erased. Doing this will on the H7 will trigger an ECC error. If we want to keep that functionality in place for existing chips, a workaround for the H7 could be coded that does not mark entries as erased. The address of the latest file could be stored as a static variable in RAM. The issue here is that you need to know how to find the latest entry on startup, which is problematic if you have use more than one sector for param storage and there is no flag to mark entries as erased.

[dagar]

Honestly even with the cost of backing ram I still think we'd be better off overall leaning on an existing filesystem (looking at SmartFS https://cwiki.apache.org/confluence/display/NUTTX/Using+SmartFS) and gutting a lot of this custom code. The way it's tangled into both parameters and dataman is a mess that's gotten in the way of further improvements to those components.

• https://github.com/PX4/Firmware/tree/master/src/lib/parameters/flashparams
• https://github.com/PX4/Firmware/blob/master/src/modules/dataman/dataman.cpp

[rippetoej]

I can see the benefit of using a small, purpose-built filesystem for flash params since it only ever stores one 'file'. With that said, I can also see the benefits to moving over to something like SmartFS. I'm not familiar enough with SmartFS to know what the backing RAM cost would be to use something like that. It's certainly overkill for the purposes of flash params.

[dagar]

I can see the benefit of using a small, purpose-built filesystem for flash params since it only ever stores one 'file'.

I agree on a technical basis, but it's more of a pragmatic maintainability comment. In PX4 none of the properly supported boards use the flashfs parameter (or dataman) storage. The custom flashfs code has no maintainer, no testing, and very little active usage. At the same time there are multiple robust flash filesystem options available to us already in NuttX that have significant adoption. We have a hardware test rack with the majority of boards people actually use and it doesn't have a single board with flash based parameter storage.

On top of that there are things like the parameter system and dataman that are critical pieces of the PX4 platform with valuable improvements I'd like to make, but they both have custom flashfs support tangled in that makes it fragile to any real change.

If someone could properly abstract a "small, purpose-built filesystem for flash params" and write thorough testing I'd be fine with it, but otherwise I'd much rather purge the technical debt at the cost of wasting memory (that we can actually afford pretty comfortably these days).

Does that make sense?

tl;dr It saves memory we don't need on boards we don't use while getting in the way of improving things used by everyone all the time (the parameter system and dataman).

[rippetoej]

In PX4 none of the properly supported boards use the flashfs parameter (or dataman) storage. The custom flashfs code has no maintainer, no testing, and very little active usage. At the same time there are multiple robust flash filesystem options available to us already in NuttX that have significant adoption. We have a hardware test rack with the majority of boards people actually use and it doesn't have a single board with flash based parameter storage.

Is there community demand for flashfs storage? I guess what I am wondering is, if nobody is truly using it, why is flashfs storage still hanging around?

I completely understand and agree with what you are saying with regards to maintainability and would love to facilitate something that helps push the code in that direction. I will have some time in the coming weeks to work on this, but I'm not sure what direction I should take things. I'm going to play around with some ideas and keep this issue open for now.

[dagar]

Is there community demand for flashfs storage? I guess what I am wondering is, if nobody is truly using it, why is flashfs storage still hanging around?

Small, but growing (kakutef7 and other inexpensive racing boards). If we had a really solid solution I think a lot of future boards could consider dropping the FRAM to reduce cost.

[taylornelms15]

Honestly even with the cost of backing ram I still think we'd be better off overall leaning on an existing filesystem (looking at SmartFS https://cwiki.apache.org/confluence/display/NUTTX/Using+SmartFS) and gutting a lot of this custom code. The way it's tangled into both parameters and dataman is a mess that's gotten in the way of further improvements to those components.

* https://github.com/PX4/Firmware/tree/master/src/lib/parameters/flashparams

* https://github.com/PX4/Firmware/blob/master/src/modules/dataman/dataman.cpp

For reference/posterity, it looks like SmartFS still writes multiple times to the same "page" (32-byte flash word) in normal operation when using the mtd progmem_initialize function on NuttX, which throws up an ECC error from the stm32h7 chip. I'm having a hard time finding existing filesystems that respect the stm32h7's stipulation of:

It is not recommended to overwrite a Flash word that is not virgin. The result may lead to an
inconsistent ECC code that will be systematically reported by the embedded Flash memory,

[spiderkeys]

@taylornelms15 out of curiosity, have you had a chance to look at how Ardupilot is handling this on Chibi OS for the H743? We use the Matek H743 Wing and ported our application from Ardupilot to PX4 and flash-based params was one of the things that was working in Ardupilot that we lost in the transition. I haven't had the time to dig in myself, but it might provide some ideas for a path forward.

[taylornelms15]

@spiderkeys thanks for the tip; it looks like they have a specialized methodology for executing their flash storage in this case, and are able to respect the limitations of the STM32H7 program flash by, essentially, throwing more bytes at the bookkeeping at the top of each record they store. I'm currently poking around at modifying the flashfs.c methods to try something similar.

[davids5]

Yes - that is the solution. use 32 byte alignment and the invalid flag now has to be a 32 byte value

[davids5]

Please create a separate flashfs32 for this