Feature: Fancy DFU on blackpill-f4#1717
Conversation
|
If you could please rebase this PR now the other two precursors are merged, we'll get to try and review it and give it an honest look. |
dragonmux
added
the
Foreign Host Board
ALTracer
force-pushed
the
feature/blackpill-f4-dfu-fancy
branch
from
83bab96 to
094d436
Compare
dragonmux
left a comment
dragonmux
left a comment
There was a problem hiding this comment.
We've got a couple of queries and some stylistic items to address, but we think this is looking pretty decent and is close to being merge-able.
dragonmux
left a comment
dragonmux
left a comment
There was a problem hiding this comment.
This all looks good to us now - if you can do final pass with squashing the fixup commits, we'll get this merged.
ALTracer
force-pushed
the
feature/blackpill-f4-dfu-fancy
branch
from
619f745 to
2f57c58
Compare
|
Propagated the "unsigned type" and commentaries to relevant commits, it wasn't as simple as a fixup autosquash. |
|
For |
* Set up SysTick interrupt to blink both LED_IDLE_RUN and LED_BOOTLOADER
* Drop the toggling of LED_BOOTLOADER in favor of SET_IDLE_STATE() * Tone down the blinking from 10 Hz, 50% duty cycle to 1 Hz, 10% duty cycle
… BMP DFU * Implement the dfu_event callback and make it inhibit the show for 1 second * Toggle the platform Idle LED on every callback hit (usually 1 KiB)
* Previous timings corresponded to max delays per datasheets "guaranteed by characterization results" * It is safe to back off the host for less time and then deal with polling * Use typical times (which happen to be twice as short for erase and 25x for write) from same datasheets
* Drop RCC_OTGFS enabling because it happens in stm32f107_usbd_init, and move RCC_CRC up to GPIOs * Drop GPIO_OSPEEDR setting of PA1/13/14 because that mapping is obsolete * Bump OSPEEDR setting from 2MHz to 25MHz for GPIO TAP pins * Reduce default "max" frequency for robust UX
ALTracer
force-pushed
the
feature/blackpill-f4-dfu-fancy
branch
from
2f57c58 to
aec136d
Compare
dragonmux
left a comment
dragonmux
left a comment
There was a problem hiding this comment.
This all LGTM now, so merging. Thank you for the contribution!
2 participants
Detailed description
blackpill-f4family of platforms (and probably other STM32F4 boards).The first couple commits belong to the serialno PR this PR is based on (or blocks on). I fail to see what mechanism checks that the usbdfu binary fits under the allocated space (8 1-KiB sectors for F1, first 16 KiB sector for F4, etc.), but I noticed that flipping the DFU_SERIAL_LENGTH from 9 to 13 pulled sprintf and bloated the dfu binary past first sector. See that PR for details.
The
dfu_f4.cchanges are required and beneficial for BMPBootloader to work how I tested it to, but can be split into its separate PR if needed. I could submit similar delay tunings fordfu_f1.cI am using on F103CB boards.The last commit is a general platform cleanup, but it can be amended or rebased away depending on situation.
Measurements for reference:
ST MaskROM DFU ~ 7.75 KiB/s (fine for flashing BMPBootloader)
BMPBootloader on
main~12 KiB/s (excessive delays)BMPBootloader on this PR branch ~ 43 KiB/s (up to 54 KiB/s if fitting under 0x08020000 and tuning delays further)
4x reduced upload time (down to ~3 seconds) is worth it for developers reflashing their boards often.
Your checklist for this pull request
make PROBE_HOST=native)make PROBE_HOST=hosted)Closing issues
Closes #1516, or so I suppose (after a possible removal of
LED_BOOTLOADERfor good)