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

core: make the full 4096 bytes of EEPROM work on Teensy 3.6 #12947

Merged
merged 1 commit into from
Nov 1, 2021

Conversation

stapelberg
Copy link
Contributor

Description

This commit updates QMK’s copy of the the teensy3 Arduino core code with the
necessary changes to make the Teensy 3.6 work.

Aside from different values for the partitioning, HSRUN mode must be left
temporarily while using the EEPROM.

fixes kinx-project/kint#8

related to kinx-project/kint#10

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).

@github-actions github-actions bot added the core label May 19, 2021
@drashna drashna requested a review from a team May 19, 2021 23:35
@fauxpark
Copy link
Member

@firetech this might be of interest?

@firetech
Copy link
Contributor

firetech commented May 20, 2021

Is this safe on other K20x boards? The current code says that partitioning may only be done once (current code is partitioning for 32 bytes), and that the board may be bricked if one tries to change the partitioning. How does this change get around that?

@stapelberg
Copy link
Contributor Author

Is this safe on other K20x boards?

Note that my code only changes behavior for the MK66F18, not the other K20x.

The current code says that partitioning may only be done once (current code is partitioning for 32 bytes), and that the board may be bricked if one tries to change the partitioning. How does this change get around that?

I am a bit puzzled about that comment to be honest. I have not found confirmation of the danger of bricking the board within NXP’s documentation. NXP talks about “the lifetime of the application”, but programming a new application to the flash with different EEPROM settings should work fine. Plus, a flash mass erase should be sufficient to get back into a good state.

Perhaps the author of the comment misunderstood something?

In any case, the Teensy 3.6 comes with 4096 bytes of EEPROM, and their Arduino code configures the EEPROM for 4096 bytes, so if anything, we need to be consistent with that if we want to use the most cautious approach.

@firetech
Copy link
Contributor

Might be that the original author was worried about a possible flashed bootloader getting wiped? I tried changing the EEPROM size on my Ergodox Infinity, and it didn't get bricked, but it didn't boot either until I changed it back.

@tzarc
Copy link
Member

tzarc commented May 20, 2021

Pretty sure I wrote that comment.

image

"Once in the entire lifetime of the device."

@stapelberg
Copy link
Contributor Author

That’s from NXP AN4282, yes?

In the MK66F datasheet, they say “application” instead of “device”:

lifetime

I wonder if NXP got it wrong in the earlier app note, or if the limitation is specific to K20x and was lifted for the later MK66F?

@fauxpark
Copy link
Member

The wording suggests to me that it's just warning about repartitioning blowing away any existing data, which is fair but wouldn't brick the chip, I don't think. Though, it is a little confusing that they use "needs to be done once" and then later "must be done only once".

@tzarc
Copy link
Member

tzarc commented May 20, 2021

That’s from NXP AN4282, yes?

Correct.

I wonder if NXP got it wrong in the earlier app note, or if the limitation is specific to K20x and was lifted for the later MK66F?

Wouldn't be the first time datasheets and appnotes were inaccurate. Perhaps it'd be worth approaching them for clarification?

@stapelberg
Copy link
Contributor Author

stapelberg commented May 20, 2021

Perhaps it'd be worth approaching them for clarification?

Yep, I have started a thread in the NXP forums: https://community.nxp.com/t5/Kinetis-Microcontrollers/FlexNVM-EEPROM-partitioning-only-once-More-than-once/m-p/1279546#M60576

@drashna drashna requested a review from tzarc May 22, 2021 06:22
@stapelberg
Copy link
Contributor Author

Yep, I have started a thread in the NXP forums: https://community.nxp.com/t5/Kinetis-Microcontrollers/FlexNVM-EEPROM-partitioning-only-once-More-than-once/m-p/1279546#M60576

NXP replied:

“It is suggested that the partition is done only once in the lifetime of the device. If it is done more than once the recorded data is lost and we do not guarantee you get the expected endurance.
Before executing a partition program the FlexNVM and D-Flash IFR needs to be in an erased state.”

I’m not sure why the endurance suffers from re-partitioning, but it certainly does not sound like the device will be bricked :)

@firetech
Copy link
Contributor

I’m not sure why the endurance suffers from re-partitioning

I would guess that they simply mean that repartitioning doesn't give you "new" endurance, so the calculations for expected endurance are no longer valid.

@stapelberg
Copy link
Contributor Author

That makes sense, thanks!

@firetech
Copy link
Contributor

firetech commented May 26, 2021

This is getting a bit off-topic, I guess, but is there any way to read out the current partitioning? If so, writing code to handle erasing of the FlexNVM if someone changes EEPROM_SIZE might be possible?

As said earlier, I tried to change EEPROM_SIZE on my Ergodox Infinity, but it wouldn't boot until I changed it back. Not entirely sure why, but given what NXP said, I would guess it was because the FlexNVM wasn't "in an erased state".

EDIT: I've done some investigation, and it doesn't seem like erasing FlexNVM and D-Flash IFR is possible without erasing the entire flash (the "Erase All Blocks" command), which I guess would be bad?

@stapelberg
Copy link
Contributor Author

without erasing the entire flash (the "Erase All Blocks" command), which I guess would be bad?

Maybe this post/thread is interesting: https://forum.pjrc.com/threads/25189-Want-to-reconfigure-Teensy3-FlexRam-FlexNVM-for-EEPROM-and-data-flash?p=64074&viewfull=1#post64074 — on the Teensy 3 at least, erasing all is fine, as long as you ensure the flash is unlocked afterwards (after erase it seems to be locked?)

@firetech
Copy link
Contributor

Doesn't the bootloader reside in (P-)flash? I see no mention of a bootloader in the reference manual, so I think erasing flash could be a bit dangerous if you don't have a JTAG programmer or similar...

@stapelberg
Copy link
Contributor Author

On the Teensy, the bootloader is programmed into a separate chip: https://www.pjrc.com/store/ic_mkl02.html

@firetech
Copy link
Contributor

Quite certain the Ergodox Infinity doesn't use such a chip. There's no flash in its BOM, and the MK20 pins it's supposed to use (the Infinity has an MK20DX256 per half) are used for the downstream UART interface for communication between halves. I guess this means its bootloader (and QMK) lives in the P-flash?

@stapelberg
Copy link
Contributor Author

It looks to me like the Ergodox Infinity is using a MK20DX128VLF5 (as per the screenshot in https://input.club/configurator-setup/, and the BOM in https://github.com/kiibohd/pcb/blob/master/IC60/IC60%20PCBA%20BOM%20-%20Sheet%201.pdf).

The X in MK20DX means “Program flash and FlexMemory” as per table 2.3 in datasheet https://www.nxp.com/docs/en/data-sheet/K20P48M50SF0.pdf. So these devices come with a separate persistent memory block that is independent of program flash, search that datasheet for FlexNVM and/or EEPROM for details.

@firetech
Copy link
Contributor

firetech commented Jun 2, 2021

It looks to me like the Ergodox Infinity is using a MK20DX128VLF5 (as per the screenshot in https://input.club/configurator-setup/, and the BOM in https://github.com/kiibohd/pcb/blob/master/IC60/IC60%20PCBA%20BOM%20-%20Sheet%201.pdf).

That's the BOM for the Input:Club 60%, correct BOM is https://github.com/kiibohd/pcb/blob/master/ICED%20-%20Left/ICED%20PCBA%20BOM%20-%20Sheet1.pdf or https://github.com/kiibohd/pcb/blob/master/ICED%20-%20Right/ICED%20PCBA%20BOM%20-%20Sheet1.pdf (probably the same file). It's a MK20DX256VLH7 per half.

@firetech
Copy link
Contributor

firetech commented Jun 2, 2021

Was a bit quick to answer...

The X in MK20DX means “Program flash and FlexMemory” as per table 2.3 in datasheet https://www.nxp.com/docs/en/data-sheet/K20P48M50SF0.pdf. So these devices come with a separate persistent memory block that is independent of program flash, search that datasheet for FlexNVM and/or EEPROM for details.

FlexMemory is what's used for "EEPROM". Quite sure the bootloader and QMK resides in program flash, since QMK sets up the entirety of FlexMemory to be used for EEPROM, but only 32 bytes of storage, for maximized write endurance.

@stapelberg
Copy link
Contributor Author

FlexMemory is what's used for "EEPROM". Quite sure the bootloader and QMK resides in program flash, since QMK sets up the entirety of FlexMemory to be used for EEPROM, but only 32 bytes of storage, for maximized write endurance.

Yes, that matches my understanding.

I don’t know what bootloader is used on the mk20 with the ergodox, so I’m not sure how to tell it which memory blocks to erase, but reading http://www.pemicro.com/forums/forum.cfm?forum_topic_id=4604 gives me the impression that you can preserve/erase all blocks independently, in principle.

Are there any questions left we need to clarify before merging this PR?

@fauxpark
Copy link
Member

fauxpark commented Jun 8, 2021

@stapelberg the Input:Club boards all use this bootloader.

@stapelberg
Copy link
Contributor Author

Thanks for the pointer to the bootloader, but I still think the whole K20x discussion is not relevant to my code change.

I’m only changing behavior for the MK66F18, not the other K20x.

Can we move this PR forward and get the MK66F18 (Teensy 3.6) change in? Y’all can handle K20x separately.

@stale
Copy link

stale bot commented Aug 13, 2021

Thank you for your contribution!
This pull request has been automatically marked as stale because it has not had activity in the last 45 days. It will be closed in 30 days if no further activity occurs. Please feel free to give a status update now, or re-open when it's ready.
For maintainers: Please label with awaiting review, breaking_change, in progress, or on hold to prevent the issue from being re-flagged.

@stapelberg
Copy link
Contributor Author

Perhaps I have insufficiently tested this PR before. In my tests just now, the keyboard hangs with the EEPROM changes. Will need to look into it when I find the time…

Figured it out! Needed to add the CPU/BUS clock frequencies we use (120 MHz/60 MHz) to the HSRUN disable code, otherwise it wouldn’t actually leave HSRUN and just hang.

This only affected micro controllers on which the code never successfully ran, i.e. where EECONFIG_MAGIC_NUMBER was not written to EEPROM already. For testing, temporarily changing EECONFIG_MAGIC_NUMBER to a different value will trigger the EEPROM initialization code path (where the hang occurred).

This commit updates QMK’s copy of the the teensy3 Arduino core code with the
necessary changes to make the Teensy 3.6 work.

Aside from different values for the partitioning, HSRUN mode must be left
temporarily while using the EEPROM.

fixes kinx-project/kint#8

related to kinx-project/kint#10
@stapelberg
Copy link
Contributor Author

@tzarc @zvecr @drashna This should be good to go now, can you take another look please?

Thank you!

@stapelberg
Copy link
Contributor Author

Friendly ping? :)

@zvecr zvecr merged commit 7f8faa4 into qmk:develop Nov 1, 2021
@tzarc tzarc mentioned this pull request Nov 2, 2021
14 tasks
tzarc added a commit to tzarc/qmk_firmware that referenced this pull request Nov 2, 2021
zvecr pushed a commit that referenced this pull request Nov 2, 2021
* Fix build failures caused by #12947. Unknown if this actually works.

* qmk format-c
zemackdaddy added a commit to zemackdaddy/qmk_firmware that referenced this pull request Jan 2, 2022
stapelberg added a commit to stapelberg/qmk_firmware that referenced this pull request Jan 7, 2022
tzarc pushed a commit that referenced this pull request Jan 9, 2022
zemackdaddy added a commit to zemackdaddy/qmk_firmware that referenced this pull request Jan 11, 2022
ryphon pushed a commit to ryphon/qmk_firmware that referenced this pull request Jan 12, 2022
ptrxyz pushed a commit to ptrxyz/qmk_firmware that referenced this pull request Apr 9, 2022
This commit updates QMK’s copy of the the teensy3 Arduino core code with the
necessary changes to make the Teensy 3.6 work.

Aside from different values for the partitioning, HSRUN mode must be left
temporarily while using the EEPROM.

fixes kinx-project/kint#8

related to kinx-project/kint#10
ptrxyz pushed a commit to ptrxyz/qmk_firmware that referenced this pull request Apr 9, 2022
* Fix build failures caused by qmk#12947. Unknown if this actually works.

* qmk format-c
magdalipka pushed a commit to magdalipka/qmk_firmware that referenced this pull request Jul 20, 2023
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.

6 participants