Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Generic wear-leveling algorithm #16996

Merged
merged 11 commits into from
Jun 26, 2022
Merged

Generic wear-leveling algorithm #16996

merged 11 commits into from
Jun 26, 2022

Conversation

tzarc
Copy link
Member

@tzarc tzarc commented May 3, 2022 •
edited
Loading

Description

Just getting the ball rolling -- this is an adaptation of the wear leveling algorithms within QMK.
Currently we support 2-byte and 4-byte writes for "emulated EEPROM", but with Keychron (and others) using STM32L4, there was a need for 8-byte writes.

#15050 intended to add support for 8-byte EEPROM emulation; it has not been (and likely will not be) merged as the requested changes and tests were not implemented.

This PR intends to begin to rectify the situation -- decoupling the wear leveling algorithm from the underlying reads/writes, also allowing for it to be adapted for other things like external SPI NOR flash.

This PR is inclusive of the algorithm and tests only. No documentation is required as the configuration and the like will be at the driver level.

NOTE: For people wanting persistence support on their Keychron boards, they'll have to wait a while longer. This is laying the foundations, it does not implement concrete support for their boards. Same goes for GMMK and their later boards with external flash.

TODO

  • Better data corruption detection (checksums? rolling codes?) -- Cancelled, no bits to spare.
  • Add tests that include failures

Types of Changes

  • Core
  • Bugfix
  • New feature
  • Enhancement/optimization
  • Keyboard (addition or update)
  • Keymap/layout/userspace (addition or update)
  • Documentation

Checklist

  • My code follows the code style of this project: C, Python
  • I have read the PR Checklist document and have made the appropriate changes.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • I have tested the changes and verified that they work and don't break anything (as well as I can manage).

@tzarc tzarc requested a review from a team May 3, 2022 19:32
@github-actions github-actions bot added the core label May 3, 2022
@tzarc tzarc force-pushed the wear-leveling branch 2 times, most recently from 7988c3b to 1b2b23b Compare May 3, 2022 19:44
@github-actions github-actions bot added cli qmk cli command python labels May 3, 2022
@github-actions github-actions bot removed cli qmk cli command python labels May 3, 2022
@tzarc tzarc mentioned this pull request May 15, 2022
Merged
14 tasks
@tzarc tzarc marked this pull request as ready for review June 5, 2022 22:50
@tzarc tzarc mentioned this pull request Jun 13, 2022
Merged
14 tasks
drashna
drashna approved these changes Jun 13, 2022
View reviewed changes
Copy link
Member

@drashna drashna left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

__attribute__((weak)) ✅

@drashna drashna requested a review from a team June 13, 2022 05:58
KarlK90
KarlK90 approved these changes Jun 22, 2022
View reviewed changes
Copy link
Member

@KarlK90 KarlK90 left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

__attribute__((weak)) ✔️

Tested in hardware on the GD32VF103 using #17376

quantum/wear_leveling/wear_leveling_internal.h Outdated Show resolved Hide resolved
@KarlK90
Copy link
Member

KarlK90 commented Jun 22, 2022 •
edited
Loading

What is the plan with the TODO items in the description?

  • Better data corruption detection (checksums? rolling codes?)
  • Add tests that include failures

@KarlK90 KarlK90 requested a review from a team June 22, 2022 19:32
@tzarc
Copy link
Member Author

tzarc commented Jun 24, 2022

What is the plan with the TODO items in the description?

  • Corruption detection: Nothing. We don't really have any bits to spare, so can't even do a parity bit. To address this, we would have to halve the address ranges.
  • Failure-triggering tests: Added some, more coming.

@tzarc
Copy link
Member Author

tzarc commented Jun 24, 2022

@drashna @KarlK90 bunch more tests added, only a small logic change to check lock/unlock state.... found by tests!
Want to give it another look?

KarlK90
KarlK90 reviewed Jun 25, 2022
View reviewed changes
quantum/wear_leveling/wear_leveling.c Outdated Show resolved Hide resolved
quantum/wear_leveling/wear_leveling.c Outdated Show resolved Hide resolved
@KarlK90
Copy link
Member

KarlK90 commented Jun 25, 2022 •
edited
Loading

I wonder if we could have some pretty basic error detection for the consolidated data. This would work by having the CRC8 or FNV hash of that blob as the header before the write log. At least when the cache is build from that consolidated data + write log we can then detect corruption in the former. For the log entries this doesn't work of course.

@tzarc
Copy link
Member Author

tzarc commented Jun 26, 2022

I wonder if we could have some pretty basic error detection for the consolidated data. This would work by having the CRC8 or FNV hash of that blob as the header before the write log. At least when the cache is build from that consolidated data + write log we can then detect corruption in the former. For the log entries this doesn't work of course.

Added FNV1a_64 that gets written after a consolidation occurs.

KarlK90
KarlK90 approved these changes Jun 26, 2022
View reviewed changes
Copy link
Member

@KarlK90 KarlK90 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for addressing my findings so quickly and picking up the fnv hash idea. The changes look good for me now!

@tzarc tzarc merged commit 01ecf33 into qmk:develop Jun 26, 2022
@tzarc tzarc deleted the wear-leveling branch June 26, 2022 21:18
0xcharly pushed a commit to Bastardkb/bastardkb-qmk that referenced this pull request Jul 4, 2022
@tzarc @KarlK90 @zykrah
Generic wear-leveling algorithm (qmk#16996)
68e4a2c 
* Initial import of wear-leveling algorithm.

* Alignment.

* Docs tweaks.

* Lock/unlock.

* Update quantum/wear_leveling/wear_leveling_internal.h

Co-authored-by: Stefan Kerkmann <karlk90@pm.me>

* More tests, fix issue with consolidation when unlocked.

* More tests.

* Review comments.

* Add plumbing for FNV1a.

* Another test checking that checksum mismatch clears the cache.

* Check that the write log still gets played back.

Co-authored-by: Stefan Kerkmann <karlk90@pm.me>
@tzarc tzarc mentioned this pull request Jul 11, 2022
Closed
14 tasks
nolanseaton pushed a commit to nolanseaton/qmk_firmware that referenced this pull request Jan 23, 2023
@tzarc @KarlK90 @nolanseaton
Generic wear-leveling algorithm (qmk#16996)
11fbee4 
* Initial import of wear-leveling algorithm.

* Alignment.

* Docs tweaks.

* Lock/unlock.

* Update quantum/wear_leveling/wear_leveling_internal.h

Co-authored-by: Stefan Kerkmann <karlk90@pm.me>

* More tests, fix issue with consolidation when unlocked.

* More tests.

* Review comments.

* Add plumbing for FNV1a.

* Another test checking that checksum mismatch clears the cache.

* Check that the write log still gets played back.

Co-authored-by: Stefan Kerkmann <karlk90@pm.me>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@KarlK90 KarlK90 KarlK90 approved these changes

@drashna drashna drashna approved these changes

Assignees
No one assigned
Labels
core dependencies
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@tzarc @KarlK90 @drashna