Best practices to fit changes onto flash? (WAS: rationale for writing settings onto flash on change only)

[ia]

TLDR

UPDATE 2: This message has been started just as a pull request but it turned into very deep rabbit hole for me. But I would like to keep the original part as is.

TL;DR: by the "price" of using extra sizeof(systemSettingsType) bytes in RAM, this patch (presumably) reduces write operations into internal flash, hence prolonging lifecycle for it by reducing flash wear-out effect.

According to datasheet for STM32F103 (which is used for TS80P model), it has limit of 10'000 write cycles (page 57, table 29) which is usually "standard" amount for generic purpose MCU which is not small amount. However, as far as I'm aware (and correct me if I'm wrong!) MCU flash doesn't have modern wear leveling algorithms (unlike modern consumer-grade flash storage chips, therefore this commit could be useful from described perspective).

UPDATE 1: All builds have been successfull except only one: TS80P / ZH_CN. It ran out of ROM by 44 bytes. See below.

Intro

Hello. First of all, I would like to thank everyone who is involved in IronOS because this is really incredible project on so many levels. But the longer I was scouting through the code just out of my own curiosity, the more I was getting some thoughts to share. So in addition to my words of appreciation, I started to think about helping the project back not only by my words but by commits.

Please, keep in mind that I'm not a professional in:

• thermal physics;
• low level MCU programming;
• embedded C/C++.

So I'm fully open to any criticism but even if my patch(es) will be declined, I would like to hear an explanation why (if possible).

Rationale

Current code flow

• saveSettings() function is being called every time on exit from settings menu unconditionally;
• flash_save_buffer() platform-dependent function is being called by saveSettings() unconditionally;
• flash_save_buffer() implementations (regardless the platform) not only overwrite flash storage unconditionally with settings' buffer but zeroing storage before re-writing settings.

Proposal

Implementation of tiny check inside of saveSettings() before calling flash_save_buffer() will reduce unnecessary writings onto flash. Suggested code flow:

• load current settings from flash into additional memory buffer as currentSettings;
• compare already stored currentSettings with systemSettings buffer which is going to be stored;
• if they are the same data, then don't re-flash/re-write internal storage;
• if they are not the same data, then save updated settings to internal storage.

This approach may be too straightforward, but implementing a check for every setting (has it been changed in menu?) may make code base a bit messy.

Testing

Although I make was going to make pull-request with this patch for dev branch (which is the main active development branch of the project - as far as I understand), I did successfully mostly successfully the following tests:

• the patch has been applied to the source code tree based on v2.21 release tag - to test the patch on the stable version;
• the builds have been compiled for every supported device with every supported language using custom build script on alpine linux 16.3 inside lxc container;
• TS80P_EN.hex has been tested successfully on my own personal soldering iron for a couple of hours;
• from user perspective nothing has been changed - once the settings are being changed, they are being saved successfully even between power-ups;
• v2.21 source code tree without this patch produces the same warning for using memcpy inside loadSettings during compilation just like with this patch, therefore the patch has nothing to do with this warning:

./Core/BSP/Miniware/flash.c:41:66: warning: 'memcpy' reading 88 bytes from a region of size 0 [-Wstringop-overread]
   41 | void flash_read_buffer(uint8_t *buffer, const uint16_t length) { memcpy(buffer, (uint8_t*)SETTINGS_START_PAGE, length); }

TS80P / ZH_CN build problem

Here the problem starts.

During the final checks before asking for pull request, I discovered that there is the issue with fitting TS80P-ZH_CN into flash:

/usr/lib/gcc/arm-none-eabi/11.2.0/../../../../arm-none-eabi/bin/ld: Hexfile/TS80P_ZH_CN.elf section `.data' will not fit in region `ROM'
/usr/lib/gcc/arm-none-eabi/11.2.0/../../../../arm-none-eabi/bin/ld: region `ROM' overflowed by 44 bytes
Memory region         Used Size  Region Size  %age Used
             RAM:       13152 B        20 KB     64.22%
             ROM:       47148 B        46 KB    100.09%
collect2: error: ld returned 1 exit status
make: *** [Makefile:563: Hexfile/TS80P_ZH_CN.elf] Error 1

Since the main documentation is the code, after digging around and with a hint from flash.c, I have the following understanding of flash map for TS100/TS80/TS80P models:

start	size	in KB	content
0	4000	16KB	bootloader
4000	B800	46KB	app
F800	400	1KB	--empty gap--
FC00	400	1KB	settings

Is this correct?
And is my understanding correct that if I change this line to flash_size=63k then the flash map will be like this:

start	size	in KB	content
0	4000	16KB	bootloader
4000	BC00	47KB	app
FC00	400	1KB	settings

Now I have even more questions:

• Do you use the gap as additional measure to avoid possible intersection overwriting if something goes wrong?
• But is it safe to remove this gap? And what are the pitfalls of removing it?
• Is there any ways to compress app data (including translation) more "tight"?
• And what do you think about my patch for saveSettings() anyway?

The way how I see the main problem: IronOS gets so rich with features, and some languages are so complicated that eventually it may not be fitting into flash on some devices anymore with new patches & features. Are there any tricks & hacks for compressing binaries even more? I'm not talking about options of optimizations for compilers since it's -Os already (as far as I noticed) but maybe there is something else can be done.

If the original suggestion will be considered valuable and someone will help me to fix the ROM size issue, then I would be happy to apply for a proper feature pull-request of my patch.

I can't wait to hear any feedback about this. Thank you a lot for your time especially if you've read all of this.

[Ralim]

Hello and welcome👋🏼

This is indeed wise, originally I made the call that most users wont change their settings over 10k times 🙃 and took the quicker path to the code.

I think this is the wiser approach, though rather than reading the entire struct into memory, you could just call a memcmp() over the ROM address and the struct in ram (which may be less code). Otherwise logic stands :)

The 1K flash gap is there for the bootup logo / animation so it cant be used for application code.
Its location is also fixed, we we cant relocate / resize it trivially.

Things that would shrink the size of the binary:

• Move from STM HAL to libopencm3
• Optimise implementation of some of the c++ static classes to remove any leftover c++ overheads
• Probably small optimisations to be made somewhere

The first two of those has been on my list forever to do, but effort > reward.
Sadly you hit the nail on the head where these larger languages and packing lots of features bites us in code size.

[ia]

Hello and welcome👋🏼

Thank you.

most users wont change their settings over 10k times

I fully agree with you. Although I just thought such optimization could be useful.

you could just call a memcmp() over the ROM address and the struct in ram (which may be less code)

What a smart approach!

The 1K flash gap is there for the bootup logo / animation so it cant be used for application code.
Its location is also fixed, we we cant relocate / resize it trivially.

Aaaaha!!! Somehow I couldn't figure this out easily - I forgot about animation data. Thanks for this technical input!

Things that would shrink the size of the binary: [...]

Unfortunately, I do not have enough of skills to achieve the first two (not on my own at least). I tried what I could do:

• memcmp'ed SETTINGS_START_PAGE
• removed sections related to BLE and Hall sensor right from Translation.ZH_CN.cpp manually
• experimented with other things

No luck.

TS100/TS80/TS80P models are almost identical (from the point of firmware) but IronOS firmware for TS100/TS80 takes ~10% space less. As far as I understand, this delta comes from usb-pd stack for TS80P so optimization of usb-pd project may be useful in such cases as well.

The first two of those has been on my list forever to do, but effort > reward.

As far as I understand something will have to be done eventually because all the features will stop to fit otherwise.

Sadly you hit the nail on the head where these larger languages and packing lots of features bites us in code size.

Got it. Well, here is my suggestion then: if size optimization task will get more or less clear ToDo list with what/how/where, then you can count on extra pair of hands. Until then I close this task not to bother others with its open status.

Thank you a lot for the response and for clarifications.