Fix flash wear leveling sector calculation
Description
Currently the calculation of the last usable flash sector doesn't work properly if last_sector starts beyond the end of the actual flash size:
https://github.com/qmk/qmk_firmware/blob/172c3496756fb5468c778c2536b4334f25eb883b/platforms/chibios/drivers/wear_leveling/wear_leveling_efl.c#L87-L92
Here the last_sector is calculated from the first_sector but also i is included in the calculation. But i is incremented in each loop iteration and thus an increasing offset is added.
The solution here is to first calculate the number of the last usable sector and then use another loop to calculate the first sector.
Types of Changes
- [ ] Core
- [x] Bugfix
- [ ] New feature
- [ ] Enhancement/optimization
- [ ] Keyboard (addition or update)
- [ ] Keymap/layout (addition or update)
- [ ] Documentation
Issues Fixed or Closed by This PR
Checklist
- [x] My code follows the code style of this project: C, Python
- [x] 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.
- [x] I have read the CONTRIBUTING document.
- [ ] I have added tests to cover my changes.
- [x] I have tested the changes and verified that they work and don't break anything (as well as I can manage).
Thanks for the review. I fixed the issues you mentioned and added some additional checks.
One remaining issue is that flash sectors that are used for the firmware code could be used by the wear_leveling driver. I don't see any checks to prevent that. But I'm not that experienced with QMK and don't know what would be the best way to do this.
Error handling is a bit problematic. Exiting with chSysHalt is better than just crashing. But maybe compile time checks could be used to prevent some of the error cases.
Hey, is there anything missing to get this merged?
Hey, is there anything missing to get this merged?
From you, nothing at this stage. Needs two approving reviews for merge; we'll get to it when we can.
Thanks for finally merging this :+1: