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

[BUG] Fix tinyuF2 BootLoader make error when emulating EEPROM #16586

Closed
wants to merge 1 commit into from
Closed

[BUG] Fix tinyuF2 BootLoader make error when emulating EEPROM #16586

wants to merge 1 commit into from

Conversation

jiaxin96
Copy link
Contributor

@jiaxin96 jiaxin96 commented Mar 8, 2022

Description

Types of Changes

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

Issues Fixed or Closed by This PR

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
Copy link
Member

tzarc commented Mar 8, 2022

This won't work -- TinyUF2 uses a binary of about 24kB last I checked, which means the second half of the bootloader is in the same location as the emulated EEPROM implementation -- clearing EEPROM will wipe out the last 8kB of the bootloader.

@tzarc
Copy link
Member

tzarc commented Mar 8, 2022

So I confirmed -- the bootloader binary is 14kB, and tbh feels a bit tight to assume that it'll never grow to 16kB in size.
Realistically I think the emulated EEPROM implementation for TinyUF2-based boards should be moved to sector 4 instead of sector 2, occupying the range of [48k,64k), given that the bootloader expects user code to be at the 64k location. This would ensure that the largest amount of space is left for the bootloader to grow over time, as well as ensuring that the user program isn't impacted.

Copy link
Member

@zvecr zvecr left a comment

Choose a reason for hiding this comment

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

.ld files are not configured for eeprom emulation so this will not work correctly.

Sorry, not ld files, config defaults https://github.com/qmk/qmk_firmware/blob/master/platforms/chibios/eeprom_stm32_defs.h#L65

@jiaxin96
Copy link
Contributor Author

jiaxin96 commented Mar 8, 2022

I checked the LD file (https://github.com/qmk/qmk_firmware/blob/65eb0f939fb35de66e6772ca358311f2c12344f6/platforms/chibios/boards/BLACKPILL_STM32_F401/ld/STM32F401xC_tinyuf2.ld#L22) and it takes up 64K size, but I tried to compile the tinyuf2 file myself and can figure out that the minimum size of tinyuF2-bootloader on STM32F401CCU6 chip is 16K.

Opening data file [D:\PRJ\KBs\YR_KB_FM_REPO\UF2_BL\tinyuf2-stm32f401_blackpill_16m_lite.hex] ...
 - Data file opened successfully (15112 bytes, 2 ranges, CRC of data = 0x1319386F, CRC of file = 0x6F086E43)
Opening data file [D:\PRJ\KBs\YR_KB_FM_REPO\UF2_BL\tinyuf2-stm32f401_blackpill_16m.hex] ...
 - Data file opened successfully (15120 bytes, 2 ranges, CRC of data = 0x3288D226, CRC of file = 0xFF70D3F5)

@jiaxin96 jiaxin96 closed this Mar 8, 2022
@jiaxin96 jiaxin96 reopened this Mar 8, 2022
@jiaxin96
Copy link
Contributor Author

jiaxin96 commented Mar 9, 2022

I think the starting address should start from base + 64K,which is 0x08010000, to avoid conflicting with tinyuf2 BL.(in https://github.com/qmk/qmk_firmware/blob/master/platforms/chibios/eeprom_stm32_defs.h#L65)

#define FEE_PAGE_BASE_ADDRESS 0x08010000

By a way, currently, using BOOTLOADER = tinyuf2 for STM32F401 in rules.mk cannot compile.

@jiaxin96 jiaxin96 closed this Mar 9, 2022
@jiaxin96 jiaxin96 reopened this Mar 9, 2022
@tzarc
Copy link
Member

tzarc commented Mar 9, 2022

base + 64k is where tinyuf2 jumps to for execution, so it's a non-starter too.

@jiaxin96 jiaxin96 requested a review from zvecr March 9, 2022 05:56
@jiaxin96 jiaxin96 marked this pull request as draft March 9, 2022 06:00
@shuihuohuooo
Copy link

鸭子哥, 我是你粉丝

@sigprof
Copy link
Contributor

sigprof commented Mar 13, 2022

See also this comment thread: #16287 (comment) — the STeMCell controller is intended to use the tinyuf2 bootloader too; that PR attempts to place the EEPROM storage at 32K, leaving 32K for the bootloader; one remaining 16K flash sector won't be used by the current EEPROM implementation (although maybe someone would want to enhance that implementation to be safe against unexpected poweroffs, and that would require using at least 2 flash sectors).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants