Fix BootloaderCDC flashing over 32k #170
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Without the explicit cast, avr-gcc generates incorrect asm which sets only the lower bytes of the u32, leading the upper bytes to be set to 0xFFFF when the input value has MSB set. This results in flashing past 32k bytes failing. Explicit casting corrects this behaviour in testing.
|
From my experience this cast looks correct and required. I guess that most USB MCUs tested simply do not have that much space, so the issue never popped up. |
|
Oops - yes, with the shift it will implicitly up-cast the uint8_t to a uint16_t, but no further. Thanks! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
When loading the bytes from the USB serial stream that represent the current position in words, BootloaderCDC uses a trick which is too clever for GCC in order to populate a u32 and double the integer value.
In my testing, when the size of the flashed item exceeds 32k, the asm generated by avr-gcc (both v5 and v10) incorrectly manipulates the u32, leading to its upper 2 bytes being set to 0xFF. This then fails the
Address < BOOT_START_ADDRtest inIsPageAddressValid.This PR adds an explicit u32 cast to the function calls, leading to avr-gcc treating the u32 correctly and populating all 4 bytes.
Disclaimer: I am not familiar with AVR ASM and have based this on functional testing as well as an approximate analysis.