Restore EEPROM address to prior released location

[earlephilhower]

When the FS_END was adjusted to end on a full block (i.e. rounded down)
to avoid filesystem issues, but _FS_end was changed. The EEPROM library
used _FS_end to implicitly calculate the start of the EEPROM data, so
this means after the _FS_end fix, EEPROM data written with prior
releases would "disappear."

Avoid the issue by explicitly calculating the EEPROM start location in
the linker, using the same formula as prior release.

Fixes #6531

[mcspr]

Hey
If adding another symbol is a way to go, perhaps we can also add _EEPROM_end?

Back in april when #5989 was proposed, I also patched to my home-brew ld script generator for ESPurna with the same fs_end calculation, but have not noticed any EEPROM issues because my 4m1m script reserved 16K for EEPROM (for wear-leveling writes) 🤷‍♂

[d-a-v]

No difference with 2.5.2, using
for i in eagle.flash.*; do echo $i; git diff 2.5.2 $i | egrep -ie 'eprom|^-.*_end'; done

[earlephilhower]

@mcspr there's no real clean way to get what you're trying to do. The way EEPROM works is to erase and write starting from the EEPROM beginning, and it erases/rewrites the sector on each commit, so there is no way to do wear leveling, w/o going to a new EEPROM setup (which wouldn't be compat with existing setups, sadly) that iterated over a set of sectors...

[mcspr]

True, but that is exactly what is happening for that specific firmware because of the small shim library working on top of the eeprom logic - https://github.com/xoseperez/eeprom_rotate - where it calculates free space between sdk-reserved space and fs_end and then uses a different sector for each write.

Given that right now there is also interest to create bootloader config sector (#6538), maybe there needs to be a consistent approach to add sort-of partitions

[earlephilhower]

Partitions are part of the 3.0.0 stuff (one of the reasons for the move to esptool), and on the ESP32 are defined by Espressif. I'm not following their 8266 SDK RTOS 3.0, but that's better left for another issue.

I can add an _EEPROM_end, sure, but for compatibility it can only ever be _EEPROM_end = _EEPROM_start + 4096, so for now I'm not seeing the utility. It would also need some minor rework to the lib itself, because the lib now assume a 1-sector erase/write.

For now I'd like to have this just be a bugfix ASAP for 2.6. We can work out the best way of getting your setup working in another issue (or track in @davisonja 's PR) or new PR since EEPROM wear leveling is a breaking change...

[mcspr]

Btw, EEPROM at runtime specifically I think it does not even need to know where fs is and could just count -20KB (-5 sectors) from the ESP::getFlashChipSize()?

There is no specific problem with the PR, only that without -DNO_GLOBAL_EEPROM my builds would fail with undefined reference err because I need to keep custom .ld script up-to-date. But fix is pretty easy, just adding new symbol or INCLUDE the up-steam .ld script somewhere in the custom one. Perhaps I am also overthinking it and should use a different approach altogether instead of moving fs_end address down (as the only reason of that is to simplify sector calculations by using last one as a base point and counting down by a specified index)

It was not meant to derail the discussion by mentioning Rotate library, but I have not thought about adding it here before. I could try to update the existing class to optionally support it, hopefully without breaking too much.

[earlephilhower]

Great. Maybe it would make sense to put your suggestion in an issue and we can mark it for 3.0 consideration. That way we won't forget!

[d-a-v]

This rotate library was proposed here as a PR and instead moved to an arduino library #4493
doc (should be moved to the 'Other libraries' paragraph below?)
https://github.com/jwrw/ESP_EEPROM

[mcspr]

One quirk is that both want extern _SPIFFS_end, which is replaced with _FS_end.
xoseperez/eeprom_rotate#6 had mentioned this some time ago, but I forgot about it...
I hot-fixed it via the .ld scripts too, but maybe there should be PROVIDE ( _SPIFFS_... = _FS_... ) for each one in the local.eagle.app.v6.common.ld to avoid breaking such libs?

[davisonja]

I'd have thought when _SPIFFS_ got replaced it happened everywhere. A provide would seem like simple patch

…

On Sat, 21 Sep 2019, 09:14 Max Prokhorov, ***@***.***> wrote: One quirk is that both want extern _SPIFFS_end, which is replaced with _FS_end. xoseperez/eeprom_rotate#6 <xoseperez/eeprom_rotate#6> had mentioned this some time ago, but I forgot about it... I hot-fixed it via the .ld scripts too, but maybe there should be PROVIDE ( _SPIFFS_... = _FS_... ) for each one in the local.eagle.app.v6.common.ld to avoid breaking such libs? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#6537?email_source=notifications&email_token=AABX6AYFIZTEL3VGED3WJYTQKU4KFA5CNFSM4IYPLR3KYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD7H4PNY#issuecomment-533710775>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AABX6A52PORRUV3AKNXXFADQKU4KFANCNFSM4IYPLR3A> .