Flash leveling for some STM32 boards (and more later?)

[randellhodges]

This is currently a draft. It works on my board. I welcome contributions to include other variations of the STM32 platform, if it is something the community would like to pursue.

My C/C++ skills are beginner level so I'm sure things can "better".

Description

The STM32 library, specifically framework-adruinoststm32 as used by the SKR PRO 1.1 has a simple flash eeprom implementation that writes a buffer in the same location on all the STM32 boards.

This PR is the basis (more work will be needed if we want to try to cover everything) of adding LPC style wear leveling to the flash operations.

Currently, only STM32F407xx and STM32F446xx have the option of enabling this new feature. To enabled:

#define EEPROM_SETTINGS
#define FLASH_EEPROM_EMULATION
#define FLASH_EEPROM_LEVELING

If you do not add #define FLASH_EEPROM_LEVELING then it will just use the platforms version of eeprom saving. It supports more versions, but again, only writes to the same flash locations.

Why isn't everything supported?

The STM32 platform is a circus. Some chips have a concept of sectors and some pages. Really, there isn't that much difference between the 2. A sector and a page are still the smallest unit you can erase. It is just that a page is generally around 2 or 4kB in size, whereas a sector is between 16kB and 128kB. The ones at the end of the flash that we can use are always (?) 128kB.

To complicate matters further, some chips have multiple banks. From what I can gather, there are different ways to use the banks.

In order to support everything, we'd have to look more into stm32_eeprom.c (where I took inspiration) and add in all the different #define they have.

The hard part, is, for those that use small pages of 2kB, we'd have to keep track of multiple pages. If we want to keep the 4kB eeprom, then we'd have to manage 2 pages per slot.

This simple implementation defines a maximum number of slots by dividing the sector by the eeprom size. The out of the box defines means we have 32 slots.

What's next?

The LPC code included disabling interrupts during operations. I am unsure if the STM32 platform needs that or not. I don't really see anything in the platform eeprom version, nor on the internet about it.

More boards. I know there is talk of merging the other STM32 HALs. At a minimum we might want to add in support for the F103 board (a pages board). This would, I think, cover over 80% of the current STM32 boards.

Benefits

Flash doesn't wear out as fast.

Credit

Most of the code is a mashup of the HAL_STM32 and HAL_LPC1768 and the framework-arduinoststm32/cores/arduino/stm32/stm32_eeprom.* (and other files).

[thinkyhead]

Note that we only write the differences when writing to EEPROM.

[sjasonsmith]

I haven't looked back at what the LPC code does, but do we need to check every uint32_t value to find the first unused one? Could one single location in each slot be checked? I suppose it is possible to have only UBL meshes saved, without ever saving EEPROM configuration, so that might be tricked than it sounds.

[sjasonsmith]

Ideally the last word written (in time, not necessary location) in each slot would be a "valid" indicator, that could be used to ensure a slot is only used if it was written completely. That would allow ignoring a corrupt slot, where power was lost while writing the flash. That of course could not protect you when the entire sector is erased. More than one sector would be needed to provide fault tolerance in that case.

It probably doesn't make sense to tie up this feature for that, unless it could already be easily implemented without requiring EEPROM layout changes that would result in issues elsewhere.

[sjasonsmith]

ignoring a corrupt slot

If you chose to implement this it would also add complexity where you'd have to skip corrupt slots when choosing your destination, rather than simply rolling to the next slot.

[randellhodges]

I haven't looked back at what the LPC code does, but do we need to check every uint32_t value to find the first unused one? Could one single location in each slot be checked? I suppose it is possible to have only UBL meshes saved, without ever saving EEPROM configuration, so that might be tricked than it sounds.

The configuration in the 4k buffer is offset by 100, with the print counter information stored before it. We could, probably look around offset 100 where it stores the checksum.

The LPC code calls a blanksector function and doesn't have that type of loop inside the HAL. I can dig into that function and check.

[randellhodges]

One thing that'll make things tricky is if we adapt this code across the other chips. The sector concept of the F4 chips, where they have 8 to 12 "blocks" between 16k and 128k is easy to work with because we can use one "sector" and subdivide.

The F103 chips, which currently uses a completely different HAL for now, uses a "page" concept. Only difference is the number and size. Instead of subdividing what I'll just call a flash "unit" into multiple slots, we'd have to subdivide a slot into multiple units (or pages).

The datasheets show chips with pages of 1, 2, and 4k. It could get convoluted quickly.

Would we want to get more variations of these STM32 chips supported with this type of wear leveling and then add the corruption check add it first?

[sjasonsmith]

Note that we only write the differences when writing to EEPROM.

That is not an option with flash. You can't set bits when you write to flash. You have to erase the page/sector to set all the bits, and writes can only clear them. That's where the real value in this change comes in, since it will reduce the erase frequency on an SKR Pro by 32x.

[randellhodges]

I have read something about maybe combining the STM32 HALs. Is that on a roadmap, or just something that will happen later.

HAL_STM32
HAL_STM32_F4_F7 (is this here to stay?)
HAL_STM32F1

I keep talking about supporting other variations of the STM32 chips but my changes have only been applied to HAL_STM32, which I think is only used by the SKR PRO 1.1 and the Fysetc S6.

Thoughts path forward? Keep this PR small and only apply it, as I have, to those specific chips? STM32F407xx and STM32F446xx or set it up for a more generic approach?

A generic approach in a single file will get ugly fast. Is there precedence/example of having the logic in the same HAL but in different files?

Aside: I have an F103 device coming, so I will play around with leveling for that but that code, for now, will go in HAL_STM32F1.

[sjasonsmith]

I don’t think anything currently functions I. The F4_F7 HAL, and those boards need to move into the STM32 HAL.

Rumba32 currently uses the STM32 HAL. It is easy not to notice because they build from Arduino IDE.

The F1 HAL is already widely used, and I think it is less certain whether they should move to the shared HAL.

As for whether wear leveling would be implemented for F1 chips, you probably wouldn’t want to do more than swap between two locations, due to the small page size.

[randellhodges]

Pushed a few changes

1. Available for all STM32F4 chips (note: it is still an opt in option at this time)
2. Debug_out based on ENABLED(EEPROM_CHITCHAT)
3. access_start only fills the buffer once
4. flash address start/end calculated based on sector number/size
5. Removed commented out irq code.
6. Adjusted position of static_assert and added another one
7. only flagging the eeprom dirty if the incoming byte is actually different. This means, if a person spammed M500 over and over it isn't going to load/save into the flash all the time

Thoughts on changes 3 and 7?

I need to do more in depth testing. It passes some quick smoke tests, but I haven't tried it with mesh data yet.

[thinkyhead]

Changes look good to me. If this isn't blowing up anything then it should be safe to merge for wider testing.

[randellhodges]

It'll be a bit longer before my printer is back together and I am actually using my SKR PRO 1.1. It wasn't blowing anything up when I was bench testing the code.

The fact that, for now, it is an opt in option FLASH_EEPROM_LEVELING, I would say go for the merge. If anyone has problems they can either submit a pull request for a fix or disable the leveling option :)

[thinkyhead]

I don’t think anything currently functions I. The F4_F7 HAL, and those boards need to move into the STM32 HAL.

It's important that we preserve the ability to compile for STM32 in Arduino IDE. The status of this capability needs a review.

[sjasonsmith]

It's important that we preserve the ability to compile for STM32 in Arduino IDE. The status of this capability needs a review.

The temporary SoftwareSerial implementation I added prevents building from Arduino, since I don't think there is any way to instruct it to exclude the SoftwareSerial library. The need for our own copy should go away when Arduino_Core_STM32 1.8 releases. I think that is supposed to be this month sometime.

I wasn't able to find any preprocessor constant which would tell us we are building inside Arduino. Maybe it exists, but for now the workaround if you don't need SoftwareSerial is just to delete the SoftwareSerial.cpp file from our own HAL folder. I believe the Rumba2 is more built for SPI-enabled drivers, so hopefully that is workable for now.

[thinkyhead]

Specifically, for the STM32 boards that Arduino IDE supports we were targeting the https://github.com/danieleff/STM32GENERIC platform as an alternative to https://github.com/stm32duino/Arduino_Core_STM32.

I wasn't able to find any preprocessor constant…

To skip compilation in Arduino IDE without deleting a file, either…

• Use #ifdef with a define not existing in the Arduino IDE environment,
• Use #ifdef with a define only existing in the PlatformIO environment, or
• Use #ifndef with a define only existing in the Arduino IDE environment.

We can easily provide the second one, if needed.

[sjasonsmith]

Specifically, for the STM32 boards that Arduino IDE supports we were targeting the https://github.com/danieleff/STM32GENERIC platform as an alternative to https://github.com/stm32duino/Arduino_Core_STM32.

The instructions on the Rumba32 website have you install "STM32 Cores", which seems to correspond to Arduino_Core_STM32. There has been at least one issue opened regarding the SoftwareSerial conflict that made it clear that HAL_STM32 (which uses Arduino_Core_STM32) was being used for Rumba32.

That is the only STM32 board I've head of anyone building inside Arduino, so I can't speak for the others.

Use #ifndef with a define only existing in the Arduino IDE environment.

This is what I was looking for and didn't find. Certainly we could easily add a DEFINE in the PIO ini file and use that.

[thinkyhead]

Although users are more savvy about PlatformIO, it would be great if users could choose Arduino IDE to build Marlin 2 targeting any of the 32-bit boards that Arduino IDE supports (some of which are STM32-based).

This is what I was looking for and didn't find.

These are used in Marlin and only exist for PlatformIO…

PIO_PLATFORM_VERSION
PIO_FRAMEWORK_VERSION

[randellhodges]

Was the conversation more about my question about HAL_STM32_F4_F7 or this PR in general?

Thoughts on reversing the logic of FLASH_EEPROM_LEVELING to a disable define? Then it will be on by default with quick way for users to disable if it turns out to be problematic?

[sjasonsmith]

Was the conversation more about my question about HAL_STM32_F4_F7 or this PR in general?

I think we got a bit off topic with the Arduino discussion. It probably came up becaused I mentioned the Rumba32 using HAL_STM32. I posted PR #16266 to address the SoftwareSerial incompatibility I mentioned.

I don't think any of the boards this benefits can be built inside Arduino, so I doubt you introduced problems for them.