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.

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

massdrop alt/ctrl: support saving into nvm #6068

Merged
merged 5 commits into from
Sep 29, 2021
Merged

Conversation

daltona
Copy link

@daltona daltona commented Jun 4, 2019

Use SAMD51 virtual eeprom to store eeprom in nvm instead of ram buffer so it is persistent accross reboots.
In order to have this working it is required to set PSZ and SBLK values in the NVM user row, I believe that can be done with the mdloader, but not having the source code, I am not able to implement this, I would be happy to do so.
I've tested by manualy updating the NVM user row connecting a JLINK probe on my keyboard.

Signed-off-by: Alexandre d Alton alex@alexdalton.org

@drashna
Copy link
Member

drashna commented Jun 4, 2019

@patrickmt you should absolutely see this.

@daltona daltona force-pushed the alt-eeprom branch 2 times, most recently from 22b28ff to ca9816a Compare June 7, 2019 11:38
@drashna drashna requested review from a team and removed request for fredizzimo July 30, 2019 07:53
@drashna
Copy link
Member

drashna commented Jul 30, 2019

Anyone want to test this out?

@XScorpion2 @reywood @abishalom @MatthewRobo @valen214

@reywood
Copy link
Contributor

reywood commented Jul 30, 2019

I wouldn’t be able to test the nvm part, but could test that it behaves normally without those bits set.

@XScorpion2
Copy link
Contributor

I'll test this out later tonight I hope

@zvecr
Copy link
Member

zvecr commented Jul 30, 2019

To fully understand the situation, does the proposed change require mdloader changes or external hardware to function? If this code was merged as-is, what would users who dont have access to the above see? If its runtime failures, I dont see how this can be merged yet.

@reywood
Copy link
Contributor

reywood commented Jul 30, 2019

@zvecr Looks like the code will work with or without changes to mdloader. Though you wouldn't see the benefits until mdloader is changed.

@zvecr
Copy link
Member

zvecr commented Jul 30, 2019

@reywood that's good. Let's see what the testing brings.

@XScorpion2
Copy link
Contributor

XScorpion2 commented Aug 1, 2019

Well, so far my CTRL has yet to crap itself from these changes. Also types just fine, except my fingers are now too use to a grid layout...
Would really like to get an updated mdloader to try the whole saving portion of this pr.

@drashna
Copy link
Member

drashna commented Aug 1, 2019

@patrickmt

@daltona
Copy link
Author

daltona commented Aug 12, 2019

I can try to add the code that will automatically update the configuration area, but this is too dangerous in my opinion as it cannot be recovered without a change in mdloader or JTAG hardware.

having mdloader changed would allow to securely update the configuration area without risking to lose some important data that is stored in it.

@XScorpion2
Copy link
Contributor

Please no, last thing that needs to be floating around is test code that could break a popular kb. We can wait for an official mdloader change.

@daltona
Copy link
Author

daltona commented Aug 12, 2019

fully agree.
Well I figured out that there is host part of mdloader in the form of source code, so I might be able to implement the NVM configuration change in here.
I'll implement this, try and report back.

@daltona
Copy link
Author

daltona commented Aug 13, 2019

This patch to mdloader allows to enable smarteeprom feature in order to test configuration persistence.
daltona/mdloader@0aaf775
tested on a mac.

@XScorpion2
Copy link
Contributor

XScorpion2 commented Aug 17, 2019

@daltona tried your modified mdloader on windows with no luck. It still does not save out rgb matrix state. Did I use the right option?
.\mdloader_windows.exe -f -D -r .\massdrop_ctrl_xulkal.bin --restart

Interestingly enough, when I swap the order of the params, it spits out that another command conflicts with download:
.\mdloader_windows.exe -f -r -D .\massdrop_ctrl_xulkal.bin --restart

And the command list does not mention the -r option at all. Tried -r option by itself:
.\mdloader_windows.exe -f -r --restart
This at least spit out that it was updating the user row, but still not saving any state.

Opening port 'COM12'... Success!
Found MCU: SAMD51J18A
Bootloader version: v2.18Jun 22 2018 17:28:08
user row: 0xfe9a9239 0xaeecff80 0xffffffff 0xffffffff
SmartEEPROM not configured, proceed
dsu statusb: 00000010
nvm config: 00000004

Running it again states it is already configured:

Opening port 'COM12'... Success!
Found MCU: SAMD51J18A
Bootloader version: v2.18Jun 22 2018 17:28:08
user row: 0xfe9a9239 0xaeecff92 0xffffffff 0xffffffff
SmartEEPROM already configured

Final edit:
I just realized I forgot to switch to this feature branch....

Switching to the correct branch makes it work! Nice! 👍

@fauxpark
Copy link
Member

It's certainly possible to write your own bootloader for ATSAM devices, the problem is that you will need extra hardware to replace it, in this case a J-Link and a Tag-Connect cable: https://learn.adafruit.com/how-to-program-samd-bootloaders
image
Which for most users then becomes completely useless once you've done it.

@kaeltis
Copy link
Contributor

kaeltis commented May 25, 2021

I see, thanks for explaining. I thought the applet-mdflash.bin was the bootloader, but that makes more sense :)

Apart from the bootloader (that's definitely different between boards, mine is v2.20 Mar 27 2019 10:04:47 [ctrl]), would it be of any use (or even possible) to replace the applet-mdflash.bin with our own, assuming their bootloader is based on the official SAM-BA Bootloader (https://www.microchip.com/DevelopmentTools/ProductDetails/PartNO/SAM-BA%20In-system%20Programmer)? Looking through the mdloader releases, the applet hasn't changed at all.

@fauxpark
Copy link
Member

As far as I can tell, the applet simply contains routines for programming external memories; it is uploaded to the chip before flashing proper starts, so that the bootloader knows where to shove the data it's receiving. From the SAM-BA docs:

Applets are binary programs that can be loaded through the sam-ba connection into some RAM region of the target before being executed. Applets extend the SAM-BA monitor to provide more features. The main usage of applets is to program the bootstrap into some Non-Volatile Memory (NVM).

So it would make sense that the mdloader's applet file hasn't changed, as it is fairly specific to the hardware configuration. In any case, I don't think it would be useful.

Signed-off-by: Alexandre d Alton <alex@alexdalton.org>
- Use define for SmartEEPROM buffer address
- Check buffer overflow
- Do not perform operation when timeout occurs

Signed-off-by: Alexandre d'Alton <alex@alexdalton.org>
Signed-off-by: Alexandre d'Alton <alex@alexdalton.org>
@tzarc tzarc merged commit 90797d9 into qmk:develop Sep 29, 2021
cadusk pushed a commit to cadusk/qmk_firmware that referenced this pull request Sep 29, 2021
* qmk/develop: (21 commits)
  massdrop alt/ctrl: support saving into nvm (qmk#6068)
  Added power tracking api (qmk#12691)
  Mechlovin Hannah60RGB touch-up (qmk#14646)
  [Core] Fix "6kro enable" and clarify naming (qmk#14563)
  [Keymap] Update to Drashna Code (qmk#14644)
  [Keyboard] add support for Quark_LP (qmk#14552)
  [Keyboard] Update Grandiceps to Rev2 (qmk#14618)
  [Keymap] Jonavin murphpad keymap update (qmk#14637)
  [Keyboard] Updates for Tractyl Manuform config (qmk#14641)
  [Keyboard] Fix end of file issue for Owlab suit80 (qmk#14640)
  [Keyboard] Add support for PaladinPad, Arya pcb and move keyboards by KapCave into their own directory (qmk#14194)
  [Keymap] Keychron Q1 user keymap (qmk#14636)
  Fix for mechlovin/adelais/standard_led/arm/rev4 (qmk#14639)
  convert checkerboards/quark_squared:via rules.mk to Unix line endings (qmk#14638)
  Mechlovin Delphine: add LAYOUT_numpad_6x4 (qmk#14635)
  Move "firmware size check skipped" note to message.mk (qmk#14632)
  [Keymap] fix NKRO - switch to get_mods() and refactor encoder action code (qmk#14278)
  [Keyboard] Yampad VIA support (qmk#14397)
  [Keyboard] Add OwLab Suit80 (qmk#14362)
  [Keymap] arkag userspace/keymap -- new macro and minor preonic keymap change (qmk#14623)
  ...
@daltona
Copy link
Author

daltona commented Sep 30, 2021

@zvecr Thank you for completing this and allowing it to be merged !

@zvecr
Copy link
Member

zvecr commented Oct 5, 2021

Thanks go to you for putting in the bulk of the work!

@vmalloc
Copy link

vmalloc commented Jan 23, 2022

@zvecr @daltona Just to make sure I'm up to speed - what's the bottom line for mdloader? This PR being merged means the qmk side supports persistence, but this can't actually be used until Drop updates their mdloader binaries?

@nicoelayda
Copy link

@vmalloc mdloader officially supports persistence starting from v1.0.6 https://github.com/Massdrop/mdloader/releases/tag/1.0.6

@vmalloc
Copy link

vmalloc commented Jan 24, 2022

@nicoelayda awesome! I missed that bit - updated mdloader and it works great. Thanks for all the hard work you guys put into this!

ptrxyz pushed a commit to ptrxyz/qmk_firmware that referenced this pull request Apr 9, 2022
* support saving into SmartEEPROM

Signed-off-by: Alexandre d Alton <alex@alexdalton.org>

* atsam: update smarteeprom implementation

- Use define for SmartEEPROM buffer address
- Check buffer overflow
- Do not perform operation when timeout occurs

Signed-off-by: Alexandre d'Alton <alex@alexdalton.org>

* return 0 instead of ff for invalid address or timeout

Signed-off-by: Alexandre d'Alton <alex@alexdalton.org>

* clang-format

* Add extra bounds checks

Co-authored-by: zvecr <git@zvecr.com>
BorisTestov pushed a commit to BorisTestov/qmk_firmware that referenced this pull request May 23, 2024
* support saving into SmartEEPROM

Signed-off-by: Alexandre d Alton <alex@alexdalton.org>

* atsam: update smarteeprom implementation

- Use define for SmartEEPROM buffer address
- Check buffer overflow
- Do not perform operation when timeout occurs

Signed-off-by: Alexandre d'Alton <alex@alexdalton.org>

* return 0 instead of ff for invalid address or timeout

Signed-off-by: Alexandre d'Alton <alex@alexdalton.org>

* clang-format

* Add extra bounds checks

Co-authored-by: zvecr <git@zvecr.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.