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

mimxrt/mboot: Adds bootloader support. #8229

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

alphaFred
Copy link
Contributor

@alphaFred alphaFred commented Jan 30, 2022

Adds bootloader for mimxrt port with DFU support.

  • Allows uniform firmware upgrade of all supported MIMXRT boards via USB DFU mode. Hence no button needs to be pressed to switch to the bootloader. Just a dfu detach request has to be sent to the USB control endpoint.
  • Prevents erasure of the virtual file system.
  • Post processing is designed such that the bootloader and all required metadata is patched into the firmware .elf file. Therefore there are no special steps required to debug the firmware.
  • The above mentioned design also allows to still flash the firmware via "traditional" ways (JLink etc.) without extra precautions. As an added benefit the bootloader is always installed automatically since it is integrated into the firmware .bin/.hex.

Signed-off-by: Philipp Ebensberger

@alphaFred
Copy link
Contributor Author

@robert-hh thanks a lot for all the support and testing - this one was quite a journey
@dpgeorge I know it is a big change but most of it is in adding the bootloader itself. Changes in the firmware focus on the makefile and linker script. Thanks a lot for your review in advance and best regards.

@alphaFred
Copy link
Contributor Author

@dpgeorge I have added some more information to the commit message.

Maybe as a hint for you, if you want to try the bootloader out. You can just flash the .hex file on your Teensy. There are no extra steps to take. Then you will have the "normal" firmware plus the bootloader installed on your board. From then on any firmware upgrade does not require you to press the darn button on that board anymore 😄.

I have tested the bootloader on four different boards with my Jenkins test automation setup. In addition @robert-hh has also tested it hundreds of times, too. I guess I can say that we are quite confident that it works reliably.

@andrewleech
Copy link
Sponsor Contributor

Hi, this is a really interesting feature!
The bootloader code itself, is it based on an existing / external project or brand new custom?

I've done a fair bit of work with the stm32 port mboot bootloader, there's a couple of features here that sound quite useful.

Just a dfu detach request has to be sent to the USB control endpoint.

Do you mean this detach request is sent when the device is in "normal" runtime mode to trigger a reboot to bootloader? This sounds quite useful, I've wanted something like this on stm32. Is the micropython app currently running given a chance to shutdown cleanly here though?

Post processing is designed such that the bootloader and all required metadata is patched into the firmware .elf file

Merging the elf / bin / hex files is handy, that could certainly simplify initial deployment certainly, rather than having to ensure both hex files are delivered / flashed correctly, and yeah it'd be nice to debug both the bootloader & main firmware in the one session.

On the flip side, mboot has a number of other features like signed/encrypted firmware updates or flashing a dfu from internal vfs filesystem, etc...


I'd love to see micropython as a platform moving towards having a common bootloader codebase that could be used on many / most ports with a common set of features.
The fact that TinyUSB is used here can certainly help to work towards this, as it is being used on an increasing number of ports (for good reason).
I think It'd be nice to have much of the core logic for this bootloader abstracted into the lib folder, with the port specific functions for things like flash access provided by individual port interfaces.

@alphaFred
Copy link
Contributor Author

Hi @andrewleech

Thanks for your feedback. The bootloader has be written by myself from the ground up. To be fair I had tremendous help by @robert-hh who supported with a lot of discussions, testing and provided hardware.

The only thing you need to do in order to detach from the firmware is to add a dfu-runtime descriptor to the MicroPython firmware. There is only one callback provided by TinyUSB which needs to be filled out. In my implementation I set a flag in an no-init RAM section ( to let the bootloader know it has to start dfu mode) and trigger a soft reset. Most certainly this could be improved to properly shut down MicroPython, too.

I also thought about allowing a firmware upgrade from VFS but I did not yet implement that feature.

Regarding debugging you can only debug the firmware with the "combined" .elf file. Probably it is possible to also add the bootloader symbols but I did not try it out.

Regarding your platform idea. I would say that code wise it is already 80% portable. The flash access for instance is included from the mimxrt port. However there are some intricate parts like for instance the Linker Script which would require more attention. Nonetheless there are still many features missing.
I am definitely willing to work towards such a goal if there is the need and desire.

In any case I will concentrate to improve the bootloader for the mimxrt port which is kind of my baby 😄.

@dpgeorge
Copy link
Member

dpgeorge commented Feb 2, 2022

It would definitely be a good idea to try and unify the bootloaders across the different ports, so that features can be reused and they are functionally similar (from a user's point of view).

@alphaFred what would you say to renaming this bootloader mboot? Then at least from the outside it is unified with the stm32 version because they have the same name 😄 From there we can eventually factor out common parts.

@andrewleech
Copy link
Sponsor Contributor

Yes the consolidation doesn't have to happen at once, can certainly be an iterative process :-)

Similar to at least making the name match though, it's probably worth looking at / copying the USE_MBOOT make flag in stm32 too @alphaFred - this is essentially used as a switch the enable / disable the bootloader in the build.

If at least the make / board defines are broadly compatible between the two, that will go a long way towards making it easy for users to migrate between ports, and means that any future refactoring ot code into common libraries will have less breaking changes for existing users.

@alphaFred
Copy link
Contributor Author

@alphaFred what would you say to renaming this bootloader mboot?

That would be possible 👍.

@alphaFred
Copy link
Contributor Author

Similar to at least making the name match though, it's probably worth looking at / copying the USE_MBOOT make flag in stm32 too @alphaFred - this is essentially used as a switch the enable / disable the bootloader in the build.

@robert-hh and myself had quite a long discussion about how to make integration of the bootloader as seamless as possible. In the beginning I also had a flag which would allow building the port with/without bootloader. We abandoned that idea because in that case we would have to provide twice the number of firmware binaries. We thought that there is no harm in always shipping the bootloader. From a users perspective the firmware still behaves the same,just with added features.

Therefore I am a bit reluctant to add such a switch to be honest. @dpgeorge Would you say that make option is required for the PR to be merged?

@dpgeorge
Copy link
Member

dpgeorge commented Feb 2, 2022

Would you say that make option is required for the PR to be merged?

No, not required. But I'd need to do a full review first.

If the bootloader is always enabled on all builds for all boards then that does keep things simple (stm32 might have gone that way if mboot was there from the beginning...).

@alphaFred
Copy link
Contributor Author

alphaFred commented Feb 2, 2022

No, not required.

Puh I was kinda nervous, thinking about the effort to go back to splitting the whole thing up again 😄.

But I'd need to do a full review first.

Yes of course. And I am looking forward to that whenever you can find the time.

Quite frankly there is no difference for a user. The VFS size and (memory location) remained the same. So for someone using the port now it is just the same as with previous updates to the firmware. Take a debugger and flash the binary. Your VFS will remain. Except on Teensy I believe but don't nail me on that one.
But from then on you can upgrade to future firmware releases with the help of the bootloader which makes it much more comfortable.

@robert-hh
Copy link
Contributor

Except on Teensy

One of the improvements for Teensy with the new bootloader is, that the VFS is not touched. The PJRC Teensy bootloader erases the whole flash, while the new bootloader keeps the file system intact.

@alphaFred
Copy link
Contributor Author

@dpgeorge I changed the name to mboot and renamed everything accordingly.

@alphaFred
Copy link
Contributor Author

@dpgeorge just rebased PR to not get too far behind. No issues. Still ready for review.

@alphaFred
Copy link
Contributor Author

@dpgeorge rebased and integrated changes merged in by @robert-hh for MIMXRT1015 support.

All supported boards have been retested by me and/or Robert. Everything is still working fine.

@alphaFred alphaFred changed the title mimxrt/bootloader: Adds bootloader support. mimxrt/mboot: Adds bootloader support. Jul 3, 2022
@dpgeorge
Copy link
Member

I had a play with this PR and it works well. When the board is running uPy, using dfu-util -s 0:leave will trigger a reset of the board into the bootloader, which is neat.

But I could not find a firmware.dfu file to program the board with over USB DFU...? I would have thought that once the bootloader is on you could program just the application from firmware.dfu. Did I miss something?

Also, it would be good if this PR could be split into a few commits. For example:

  • refactoring flash code into flash.c and flash.h
  • refactoring usb_phy0_init() code
  • adding mboot/ code
  • adding machine.bootloader() function
  • adding the TUD_DFU_RT_DESCRIPTOR() to the application

ports/mimxrt/mboot/main.c Show resolved Hide resolved
ports/mimxrt/mboot/main.c Outdated Show resolved Hide resolved
ports/mimxrt/mboot/main.c Outdated Show resolved Hide resolved
ports/mimxrt/mboot/main.c Outdated Show resolved Hide resolved
ports/mimxrt/mboot/main.c Outdated Show resolved Hide resolved
ports/mimxrt/mboot/main.c Outdated Show resolved Hide resolved
@dpgeorge
Copy link
Member

When building this I get a compiler warning, which turns into an error with -Werror:

mboot/main.c: In function 'main':
mboot/main.c:134:68: error: array subscript 'fw_header_t {aka struct _fw_header_t}[0]' is partly outside array bounds of 'uint32_t[1]' {aka 'long unsigned int[1]'} [-Werror=array-bounds]
  134 |     firmware_entry_func entry_func = (firmware_entry_func)fw_header->entry_addr;
      |                                                           ~~~~~~~~~^~~~~~~~~~~~
mboot/main.c:65:17: note: while referencing '__firmware_start'
   65 | extern uint32_t __firmware_start;  // Linker symbol
      |                 ^~~~~~~~~~~~~~~~

@alphaFred
Copy link
Contributor Author

@dpgeorge first of all thank you for your review. I will take care of the points you made.

Regarding firmware.dfu. @robert-hh and myself discussed that topic a lot. In the end we aimed for making it very easy. Therefore the generated firmware binaries are constructed in such a way that they always contain the bootloader and firmware. There is no "special" file which could only be used with the dfu bootloader. The bootloader knows about the structure of the binary and dismisses the the first 32k/256k (depending on the flash type).
Thus there is no need to provide three binaries (bootloader, firmware, and firmware.dfu) but rather a single one. Whenever you flash a binary from this commit on you get the bootloader automatically on your board. If you do not want to use it your workflow does not change to the way it was before.

I hope that makes sense. Feel free to comment on my approach. I am always open for feedback.

I have to thank Robert again for his help!

Splits `mimxrt_flash.c` into low-level driver functions and MicroPython
code.

Signed-off-by: Philipp Ebensberger
Refactors USB phy0 initialization routine.

Signed-off-by: Philipp Ebensberger
Adds DFU bootloader for mimxrt port. The bootloader binary is packaged
into the "normal" firmware binary. Therefore when setting up a new board
the bootloader will be available automatically by just loading the
normal firmware binary onto it.

Signed-off-by: Philipp Ebensberger
Adds runtime DFU descriptor to firmware. Allows to reset into DFU
bootloader.

Signed-off-by: Philipp Ebensberger
Adds method to jump into DFU bootloader via MicroPython code.

Signed-off-by: Philipp Ebensberger
Fixes code formatting with curly brackets on `for` loops and adds
`void` argument to empty function declarations and definitions.

Signed-off-by: Philipp Ebensberger
@alphaFred
Copy link
Contributor Author

alphaFred commented Jul 19, 2022

When building this I get a compiler warning, which turns into an error with -Werror:

mboot/main.c: In function 'main':
mboot/main.c:134:68: error: array subscript 'fw_header_t {aka struct _fw_header_t}[0]' is partly outside array bounds of 'uint32_t[1]' {aka 'long unsigned int[1]'} [-Werror=array-bounds]
  134 |     firmware_entry_func entry_func = (firmware_entry_func)fw_header->entry_addr;
      |                                                           ~~~~~~~~~^~~~~~~~~~~~
mboot/main.c:65:17: note: while referencing '__firmware_start'
   65 | extern uint32_t __firmware_start;  // Linker symbol
      |                 ^~~~~~~~~~~~~~~~

I can not replicate the error. In the CI pipeline there is also no issue. Maybe you use a different (newer?) gcc toolchain? I use the same version (9.2.1) like our CI pipeline.

@robert-hh
Copy link
Contributor

I did not see that error either when did a confirmation compile & test yesterday. But I had similar errors (arry out-of-bound) with gcc 11.2-2022.02. So I moved back to gcc 10.3-2021.07-

@robert-hh
Copy link
Contributor

@dpgeorge I did not look if it is mentioned somewhere, but once a firmware with the bootloader is installed, the intended command for further firmware updates is:

dfu-util -D <firmware_file> -R

After a second or two, updating will start. No need to activate the bootloader before at the board's REPL. That does not exclude using the previous firmware upload mechanisms as the fallback option.

@ojg78
Copy link

ojg78 commented Sep 18, 2023

Has this pr passed into the mists of time or is it still under consideration?

@alphaFred
Copy link
Contributor Author

I guess it depends. I certainly would love for it to be merged. But at the moment I have very little spare time in order to update the PR TO latest main branch to begin with.

@alphaFred
Copy link
Contributor Author

And I should add that I am under the impression that it would be more appreciated if the update mechanism would be UF2.

@projectgus
Copy link
Contributor

This is an automated heads-up that we've just merged a Pull Request
that removes the STATIC macro from MicroPython's C API.

See #13763

A search suggests this PR might apply the STATIC macro to some C code. If it
does, then next time you rebase the PR (or merge from master) then you should
please replace all the STATIC keywords with static.

Although this is an automated message, feel free to @-reply to me directly if
you have any questions about this.

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.

None yet

6 participants