Open m-mcgowan opened 1 year ago
@hathach can you take a look at this PR, let us know if the changes are acceptable or if not, what we need to do to get it into a good spot for merging into main?
sorry, I was a bit busy for now, will check this out whenever I could !!
Hey @hathach have you had a chance to review this yet?
@hathach Is there any way I can help you with reviewing this? I'm happy to walk you though the PR and talk through the changes, the motivation behind them etc. It would be great to have this merged, so anything I can do to help just let me know. Thanks!
sorry for late response, I am in the middle of reviewing this, however look like the additional code cause the flash to be overflowed. Let me think how to shrink the size of bootloader quickly first, then we can merge to this and continue with the PR.
LD feather_nrf52840_express_bootloader-0.7.0-16-g047531a-dirty.out
/home/hathach/.local/xPacks/@xpack-dev-tools/arm-none-eabi-gcc/11.2.1-1.2.2/.content/bin/../lib/gcc/arm-none-eabi/11.2.1/../../../../arm-none-eabi/bin/ld: _build/build-feather_nrf52840_express/feather_nrf52840_express_bootloader-0.7.0-16-g047531a-dirty.out section `.text' will not fit in region `FLASH'
/home/hathach/.local/xPacks/@xpack-dev-tools/arm-none-eabi-gcc/11.2.1-1.2.2/.content/bin/../lib/gcc/arm-none-eabi/11.2.1/../../../../arm-none-eabi/bin/ld: region FLASH overflowed with .data and user data
/home/hathach/.local/xPacks/@xpack-dev-tools/arm-none-eabi-gcc/11.2.1-1.2.2/.content/bin/../lib/gcc/arm-none-eabi/11.2.1/../../../../arm-none-eabi/bin/ld: section .bootloaderConfig LMA [00000000000fd800,00000000000fd857] overlaps section .text LMA [00000000000f4000,00000000000fd9e7]
/home/hathach/.local/xPacks/@xpack-dev-tools/arm-none-eabi-gcc/11.2.1-1.2.2/.content/bin/../lib/gcc/arm-none-eabi/11.2.1/../../../../arm-none-eabi/bin/ld: region `FLASH' overflowed by 728 bytes
collect2: error: ld returned 1 exit status
PS: oops, never mind, it is due to my setup with debug option. All is good now, but I am making an PR to optimize space anyway. Will continue with reviewing
I need to push changes to your fork to update PR, would you mind give me the write permission ?
I would also appreciate the DFU over UART pins for the Feather nRF52840 Express, so thanks for the work guys! One request though if possible... could a new "Magic Number" also be added to the GPREGRET which points directly to DFU over UART?
In my scenario I communicate with the feather through UART exclusively. In my application code, I would then put in a uart command to set GPREGRET to the "DFU_MAGIC_SERIAL_UART_ONLY_RESET" and trigger a reset. This would avoid requiring an additional pull on the DFU pin.
First time Github poster, so I hope this request isn't completely out of line.
Overview
This PR implements changes in the bootloader to enable MCU to MCU updates, in a similar manner to the built-in ROM bootloaders provided on STM32L4 and ESP32 devices. The host MCU can programmatically enter bootloader mode by asserting a DFU pin on reset, which places the device in DFU bootloader mode, with the DFU protocol running over UART rather than USB serial.
Additionally, the bootloader protocol has been extended with commands to allow erasing and writing to QSPI flash. This can be used to flash a pre-prepared FAT filesystem image to QSPI flash, such as for use with CircuitPython, enabling factory programming of CircuitPython scripts along with the CircuitPython runtime.
All changes are opt-in only and are intended to be fully backwards compatible, meaning no changes to boards that don't have the new configuration flags defined.
Configuration Changes
Boards that support DFU activation via a pin should define
DFU_PIN_ACTIVATION=1
.The button pin, DFU activation pin and FRST pin configurations now specify a pull direction to allow active high or low depending upon how that GPIO is used. The defaults reflect was was previously being used (
BUTTON_PULL
).New optional defines
PIN_DFU_ACTIVATE
andPIN_DFU_ACTIVATE_PULL
allow a particular board to define the DFU activation pin and pull direction. When these defines are present, DFU pin activation is compile-time enabled. These defines are present only for the Adafruit nRF52480 express board, which uses BUTTON_2 - feather pin D2. This was previously the FRST signal, but factory reset functionality has been removed prior to this PR, and so it was safe to reuse this.The define
DFU_EXTERNAL_FLASH
enables the protocol extensions for programming the QSPI flash. It is defined as 0 by default, and defined as 1 only for the adafruit_nRF52480_express board.When DFU activation pin is enabled, both USART and USB CDC serial are both compiled in, and selected at runtime. When the DFU pin is not asserted, the bootloader functions as it did before - USB CDC and MSD is enabled for use with adafruit-nrfjprog and UF2. UART is still used by default for boards with the MCU subvariant
nrf52
, as was the case prior to this PR.Other Implementation Details
I added some critical sections around the hci memory pool and hci transport code, to prevent race conditions accessing shared state between the main program thread and code running on UART interrupts.
When DFU is started via the GPIO, the device is not restarted when the application is flashed, in case the MCU wishes to flash more binaries, such as the QSPI filesystem.
bootloader_app_start()
previously both registered the app and launched the app. The function has been factored out into functions so the app can be registered without necessarily starting it.Testing
The code has been tested extensively on the Adafruit feather nrf52840 express, but not on any other boards. With the conditional compilation around most of the new DFU activation pin functionality and QSPI flash, regressions to other boards are not expected, although testing these would be prudent.