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

Add UF2 output file generation #31066

Merged

Conversation

petejohanson
Copy link
Contributor

Various devices are now shipping w/ UF2 compatible bootloaders. We've been hacking this into Zephyr ourselves w/ per-board post build commands for ZMK, but this adds proper upstream support for building UF2 files as part of the output, and adds support for the seeeduino_xiao board which has the BOSSA UF2 bootloader.

Tested locally building the samples/basic/blinky sample, double tapping reset, then cp build/xiao/zephyr/zephyr.uf2 /run/media/peter/Arduino and it is happily blinking away.

Adds new BUILD_OUTPUT_UF2_FAMILY_ID Kconfig setting for the family ID needed: https://github.com/Microsoft/uf2#family-id

The only thing I have an open question about would be the west.yml changes. I assume before merging the standard is to fork the upstream project into zephyrproject-rtos?

TIA!

Copy link
Collaborator

@tejlmand tejlmand left a comment

Choose a reason for hiding this comment

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

Thanks for this PR.

The only thing I have an open question about would be the west.yml changes. I assume before merging the standard is to fork the upstream project into zephyrproject-rtos?

That is correct.
You can find some more information here:
https://docs.zephyrproject.org/latest/guides/modules.html#module-repositories

So we need a fork @nashif and @carlescufi can you take care of that ?

And in that fork we would need a zephyr/module.yml and a CMakeLists.txt file.

If you wait for: #30904, then you can place all Zephyr glue code in the main Zephyr repo, and only the zephyr/module.yml file will be needed in the fork.

CMakeLists.txt Show resolved Hide resolved
Kconfig.zephyr Show resolved Hide resolved
west.yml Outdated
@@ -144,6 +146,10 @@ manifest:
repo-path: mcuboot
path: modules/tee/tfm-mcuboot
revision: 1.7.0-rc1
- name: uf2
repo-path: uf2
remote: microsoft
Copy link
Collaborator

@tejlmand tejlmand Jan 12, 2021

Choose a reason for hiding this comment

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

we would need a fork into Zephyr organization, as described:
https://docs.zephyrproject.org/latest/guides/modules.html#module-repositories

  • All modules included in the default manifest shall be hosted in repositories under the zephyrproject-rtos GitHub organization.
  • The module repository codebase shall include a module.yml file in a zephyr/ folder at the root of the repository.
  • ...

Copy link
Collaborator

Choose a reason for hiding this comment

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

as well as this:

Suggested change
remote: microsoft

@pabigot
Copy link
Collaborator

pabigot commented Jan 12, 2021

AFAICT this would enable Zephyr on a large number of Adafruit Nordic boards @carlescufi.

If true, I'd very much like to see this merged. Let me know if there's something I can do to help.

@petejohanson
Copy link
Contributor Author

AFAICT this would enable Zephyr on a large number of Adafruit Nordic boards @carlescufi.

If true, I'd very much like to see this merged. Let me know if there's something I can do to help.

That is correct. This is based on our initial hacky post build command work we've done in https://github.com/zmkfirmware/zmk for now to support boards using that exact bootloader. Trying to "do this right" by getting this merged into Zephyr proper.

@pabigot
Copy link
Collaborator

pabigot commented Jan 16, 2021

Will review after @tejlmand's blocking change requests are addressed, as testing before that won't be much use.

@petejohanson
Copy link
Contributor Author

My plan is to rework this once #30904 is in.

@carlescufi
Copy link
Member

My plan is to rework this once #30904 is in.

It's in now @petejohanson

@tejlmand
Copy link
Collaborator

My plan is to rework this once #30904 is in.

It's in now @petejohanson

@carlescufi I think we also need a Zephyr fork, as mentioned: #31066 (review)

@petejohanson
Copy link
Contributor Author

@tejlmand @pabigot @carlescufi @pabigot

Ok, I've just reworked this based on the new modules work.

Since we don't yet have the fork of the uf2 repo, the west.yml changes are still as is.

This assumes the following:

  1. Checkout this PR.
  2. Run west update
  3. mkdir tools/uf2/zephyr
  4. Add tools/uf2/zephyr/module.yml file with cmake-ext and kconfig-ext set to True
  5. Build w/ the seeeduino_xiao, e.g. west build -d build/xiao -b seeeduino_xiao samples/basic/blinky

I've only updated the XIAO for this, since it is one known good board in upstream Zephyr that comes OOTB w/ a UF2 enabled bootloader.

If everyone is happy, and can get the uf2 repo forked into Zephyr's org, I can update the west.yml appropriately.

Thanks again!

@petejohanson
Copy link
Contributor Author

Any thoughts on this?

@pabigot
Copy link
Collaborator

pabigot commented Jan 31, 2021

Any thoughts on this?

A lot of us are pretty focused on the stabilization phase for 2.5 right now, so we may not get to this until 2.5.0 closer to being ready for the release in the second week of February. (At least, I probably won't look at this until then.)

@carlescufi
Copy link
Member

Any thoughts on this?

Could you please state which repository exactly you need forked, what it provides to Zephyr and which changes do you expect to have in it?

@petejohanson
Copy link
Contributor Author

Any thoughts on this?

Could you please state which repository exactly you need forked, what it provides to Zephyr and which changes do you expect to have in it?

Repo: https://github.com/microsoft/uf2
Why needed: It contains a python script, https://github.com/microsoft/uf2/blob/master/utils/uf2conv.py that can convert a bin/hex to a UF2 file. This is as authoritative a conversion tool that I've found, and being python, is easy for us to use.

The only modification needed is the creation of one file zephyr/module.yml which should contain;

build:
  cmake-ext: True
  kconfig-ext: True

This is based on the work in #30904.

With this in place, the extra CMakeLists.txt added in this PR will work along w/ the few extra Kconfig settings, to build UF2 files as an extra post build step.

Copy link
Collaborator

@tejlmand tejlmand left a comment

Choose a reason for hiding this comment

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

Thanks for this.

Looks good, only a couple of small cleanups needed when the fork is in place.

if(CONFIG_BUILD_OUTPUT_BIN AND CONFIG_BUILD_OUTPUT_UF2)
set_property(GLOBAL APPEND PROPERTY
extra_post_build_commands
COMMAND ${PYTHON_EXECUTABLE} ${ZEPHYR_BASE}/../tools/uf2/utils/uf2conv.py
Copy link
Collaborator

Choose a reason for hiding this comment

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

When we have this as a Zephyr module, you should use:

Suggested change
COMMAND ${PYTHON_EXECUTABLE} ${ZEPHYR_BASE}/../tools/uf2/utils/uf2conv.py
COMMAND ${PYTHON_EXECUTABLE} ${ZEPHYR_UF2_MODULE_DIR}/utils/uf2conv.py

west.yml Outdated
Comment on lines 24 to 25
- name: microsoft
url-base: https://github.com/microsoft
Copy link
Collaborator

Choose a reason for hiding this comment

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

when the fork is in place, this can be removed.

Suggested change
- name: microsoft
url-base: https://github.com/microsoft

west.yml Outdated
@@ -144,6 +146,10 @@ manifest:
repo-path: mcuboot
path: modules/tee/tfm-mcuboot
revision: 1.7.0-rc1
- name: uf2
repo-path: uf2
remote: microsoft
Copy link
Collaborator

Choose a reason for hiding this comment

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

as well as this:

Suggested change
remote: microsoft

@tejlmand
Copy link
Collaborator

tejlmand commented Feb 2, 2021

@carlescufi please make the zephyr/module.yml file as:

name: uf2

build:
  cmake-ext: True
  kconfig-ext: True

@carlescufi
Copy link
Member

carlescufi commented Feb 5, 2021

Repo: https://github.com/microsoft/uf2
Why needed: It contains a python script, https://github.com/microsoft/uf2/blob/master/utils/uf2conv.py that can convert a bin/hex to a UF2 file. This is as authoritative a conversion tool that I've found, and being python, is easy for us to use.

Thanks for explaining. But does it make sense to fork a whole repo just for that single script in a utils/ folder? It's also a pretty simple script, why not just take it and place it in our scripts/ folder in the main tree?
@mbolivar-nordic and @tejlmand thoughts?

@petejohanson
Copy link
Contributor Author

I don't particularly care. The only advantage I would see would be easier integration of future changes upstream. If you take a look, that script had updates 15 days ago to add the RP2040 family, so changes to support more families would require occasional updates to the version we integration into Zephyr.

Whether those updates take the form of a repo fork, or a copy of a single file into Zephyr, I'll defer to others.

@petejohanson
Copy link
Contributor Author

I think this is a good enhancement that will make Adafruit boards a lot easier to use in Zephyr.

There's quite a few other boards supporting UF2 natively now, including the RP2040, which support UF2 from their default/onboard bootloader.

Please fix the commit messages:

feat(build): Add UF2 family IDs for registered SoCs.

is not how we format these.

soc: Add UF2 family IDs for supported SoCs

It's also unusual to make the commit messages lists of changes. Free-form text is standard.

The Purpose: and Maintained-By: lines should be removed from the cmake: commit; the purpose should be in the description, and once it's in Zephyr its Zephyr that maintains it. The commit message could/should document that the expectation is this is unmodified from the source repository version.

Will do.

@pabigot
Copy link
Collaborator

pabigot commented Feb 16, 2021

I think this is a good enhancement that will make Adafruit boards a lot easier to use in Zephyr.

There's quite a few other boards supporting UF2 natively now, including the RP2040, which support UF2 from their default/onboard bootloader.

OK. BTW my block is solely on this stuff:

Please fix the commit messages: ...
Will do.

For the other point of contention (defaulting BUILD_OUTPUT_UF2_FAMILY_ID based on SOC) , based on all discussion so far just putting the defaults as text identifiers in Kconfig.zepyhyr where config BUILD_OUTPUT_UF2_FAMILY_ID is introduced seems the most maintainable. It's not like it's hundreds of lines.

But if it has to be squirreled away in an include file under modules/ that's not something I'd block on. (I would block on using the hex constants instead of the text identifiers from the uf2 script, unless we actually need the hex constants explicitly.)

@petejohanson
Copy link
Contributor Author

But if it has to be squirreled away in an include file under modules/ that's not something I'd block on. (I would block on using the hex constants instead of the text identifiers from the uf2 script, unless we actually need the hex constants explicitly.)

I was going to comment on this. It seems like going with the family name alias, instead of the actual hex value ties us to an implementation detail of uf2conv.py, which seemed good to avoid. The actual hex value seems to be a fixed thing, is the friendly name alias?

I don't care strongly either way, just a thought.

Should a wait for concensus on where to put the defaults?

@pabigot
Copy link
Collaborator

pabigot commented Feb 16, 2021

I was going to comment on this. It seems like going with the family name alias, instead of the actual hex value ties us to an implementation detail of uf2conv.py, which seemed good to avoid. The actual hex value seems to be a fixed thing, is the friendly name alias?

Is there a registry of known identifiers that is more canonical than the uf2conv script? In a quick search I didn't find one.

Is there an alternative to uf2conv.py that has any market share?

If the answers are "no" and "no", then for now I'd say that script is the reference definition, and using the names it supports should be pretty stable.

Should a wait for concensus on where to put the defaults?

Couldn't say. I'm not sure if @tejlmand is open to anything other than what he's proposed, but that solution is something I'm willing to accept. So I guess just run with it and see if we can get this merged before my Clue boards arrive this week.

@petejohanson
Copy link
Contributor Author

I was going to comment on this. It seems like going with the family name alias, instead of the actual hex value ties us to an implementation detail of uf2conv.py, which seemed good to avoid. The actual hex value seems to be a fixed thing, is the friendly name alias?

Is there a registry of known identifiers that is more canonical than the uf2conv script? In a quick search I didn't find one.

Actually, yes. The toplevel README.md in the uf2 repository lists more chosen IDs than have aliases in the uf2conv.py script. In particular, the ESP32 family ID (0x1c5f21b0) doesn't have a corresponding alias. This may be oversight, on the part of the maintainers there, but it points to a bit of a disconnect.

Is there an alternative to uf2conv.py that has any market share?

No that I've seen, but I haven't gone looking.

If the answers are "no" and "no", then for now I'd say that script is the reference definition, and using the names it supports should be pretty stable.

Should a wait for concensus on where to put the defaults?

Couldn't say. I'm not sure if @tejlmand is open to anything other than what he's proposed, but that solution is something I'm willing to accept. So I guess just run with it and see if we can get this merged before my Clue boards arrive this week.

I'm going to update to put them inline in Kconfig.zephyr to start, it's easy enough to then move that to a separate (single) file if requested.

@petejohanson
Copy link
Contributor Author

@pabigot @tejlmand Ok, pushed a rebased/updated version.

  1. Defaults put in Kconfig.zephyr. Let me know if there is consensus on a different location, I'll happily move it.
  2. Fixes up commit message based on @pabigot feedback.
  3. Does not switch to the "friendly name" for the family ID defaults, pending some continued discussion w/ @pabigot. I don't have a strong opinion on this, just answering some follow up questions before switching. @pabigot if you still want me to switch to the name alias for those the script supports, let me know.

@pabigot
Copy link
Collaborator

pabigot commented Feb 16, 2021

I'm fine with the hex constants under the argument that the canonical source for the mapping is the identified URL. You need to add a signed-off-by to that commit.

Also my comments on commit format also applied to the third commit, which still has feat(board) with a list-format body.

Add ability to build a UF2 (https://github.com/Microsoft/uf2)
image as an additional output type. This leverages the code
partition offset for the UF2 base address, and a configurable
UF2 family ID.

Includes an unmodified (except for headers for licensing, pylit
disabling) version of the uf2conv.py script copied the UF2
format specification repository, used to convert the bin to UF2.

Origin: UF2 file format specification reference utilies
License: MIT
URL: https://github.com/microsoft/uf2/blob/master/utils/uf2conv.py
commit: 587abb8b909266e9b468d6284f2fbd425235d1b5
Signed-off-by: Pete Johanson <peter@peterjohanson.com>
@petejohanson
Copy link
Contributor Author

I'm fine with the hex constants under the argument that the canonical source for the mapping is the identified URL. You need to add a signed-off-by to that commit.

Also my comments on commit format also applied to the third commit, which still has feat(board) with a list-format body.

Thanks. missed that when I was rebasing. Hopefully all good now, from a commit message perspective.

@tejlmand
Copy link
Collaborator

1. Defaults put in `Kconfig.zephyr`. Let me know if there is consensus on a different location, I'll happily move it.

I think this is a good current solution.
I'm not really fond of any of the discussed alternative, and even though it does clutter Kconfig.zephyr, it's ok for now.

In Toolchain WG, it has been discussed to make a more proper Kconfig tree for the toolchain and compiler settings in general, and that work itself will clean up Kconfig.zephyr, so let's have the UF2 directly there, and we can see if the build outputs in general will deserve its own Kconfig.

Copy link
Collaborator

@tejlmand tejlmand left a comment

Choose a reason for hiding this comment

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

Looking good. Thanks for the patience.

Just one small comment.

Kconfig.zephyr Outdated
if BUILD_OUTPUT_UF2

config BUILD_OUTPUT_UF2_FAMILY_ID
# Prompt less config to setup a list of default UF2 Family IDs
Copy link
Collaborator

Choose a reason for hiding this comment

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

It should no longer be prompt less, as I suppose users should still be able to provide a custom string if current soc is not known. So you probably want to re-add.

Suggested change
# Prompt less config to setup a list of default UF2 Family IDs
string "UF2 Device Family"

Note: The prompt less was only in case we wanted to have the defaults located outside of Kconfig.zephyr

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok. Prompt restored.

Defaults from https://github.com/Microsoft/uf2#family-list

Signed-off-by: Pete Johanson <peter@peterjohanson.com>
Default BOSSA bootloader supports UF2 OOTB, so enabling
UF2 output by default.

Signed-off-by: Pete Johanson <peter@peterjohanson.com>
Copy link
Collaborator

@tejlmand tejlmand left a comment

Choose a reason for hiding this comment

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

lgtm

Copy link
Collaborator

@pabigot pabigot left a comment

Choose a reason for hiding this comment

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

Implementation looks good now; thanks.

AFAICT this would enable Zephyr on a large number of [Adafruit Nordic boards]
That is correct.

For some definition of "large". From this:

Only Adafruit Express boards and the Trinket M0 and Gemma M0 boards ship with the UF2 bootloader installed. Feather M0 Basic, Feather M0 Adalogger, and similar boards use a regular Arduino-compatible bootloader, which does not show a boardnameBOOT drive.

So it's not clear whether this helps me.

@petejohanson
Copy link
Contributor Author

Implementation looks good now; thanks.

AFAICT this would enable Zephyr on a large number of [Adafruit Nordic boards]
That is correct.

For some definition of "large". From this:

Only Adafruit Express boards and the Trinket M0 and Gemma M0 boards ship with the UF2 bootloader installed. Feather M0 Basic, Feather M0 Adalogger, and similar boards use a regular Arduino-compatible bootloader, which does not show a boardnameBOOT drive.

So it's not clear whether this helps me.

There's been some decent activity on https://github.com/adafruit/tinyuf2 recently, so maybe hope moving forward more of their boards will ship w/ uf2 out of the box?

@MaureenHelm
Copy link
Member

Implementation looks good now; thanks.

AFAICT this would enable Zephyr on a large number of [Adafruit Nordic boards]
That is correct.

For some definition of "large". From this:

Only Adafruit Express boards and the Trinket M0 and Gemma M0 boards ship with the UF2 bootloader installed. Feather M0 Basic, Feather M0 Adalogger, and similar boards use a regular Arduino-compatible bootloader, which does not show a boardnameBOOT drive.

So it's not clear whether this helps me.

There's been some decent activity on adafruit/tinyuf2 recently, so maybe hope moving forward more of their boards will ship w/ uf2 out of the box?

cc: @tannewt

@pabigot
Copy link
Collaborator

pabigot commented Feb 19, 2021

There's been some decent activity on https://github.com/adafruit/tinyuf2 recently, so maybe hope moving forward more of their boards will ship w/ uf2 out of the box?

That could help.

I really wanted Zephyr on the Clue, but it only supports DFU/serial programming, and the bootloader is placed in high flash with a configuration register overwritten to make the system start there, so the chance of bricking the device during porting is really high. SWD pins are provided only on unlabeled test points, so there seems no solution but to throw the boards into a box or donate them to somebody who understands Arduino or CircuitPython.

Too bad: it would have been a really cool Zephyr platform.

@tannewt
Copy link

tannewt commented Feb 20, 2021

Only Adafruit Express boards and the Trinket M0 and Gemma M0 boards ship with the UF2 bootloader installed. Feather M0 Basic, Feather M0 Adalogger, and similar boards use a regular Arduino-compatible bootloader, which does not show a boardnameBOOT drive.

So it's not clear whether this helps me.

Sorry about this text. It's old. All nRF boards from Adafruit should have a UF2 bootloader.

There's been some decent activity on https://github.com/adafruit/tinyuf2 recently, so maybe hope moving forward more of their boards will ship w/ uf2 out of the box?

Generally we ship UF2 bootloaders on all of our boards. All SAMD51 and nRF boards will. The text above is for M0, SAMD21 boards only. Basically ones designed before UF2 may not have it. (It depends on if the test jig gets updated.)

It's less urgent for us on boards with ROM bootloaders (ESP32-S2, iMX RT, STM32) so the first batches may not have it because the UF2 bootloader isn't ready yet. Our long term goal is to always have UF2 to ensure a consistent experience for all of our boards. (Once UF2 is ready, it depends on updating the test jig.)

tinyuf2 is my hope for the future. We have separate SAMD and nRF implementations that have been a pain to keep consistent. tinyuf2 builds on top of tinyusb to bring the uf2 bootloader to all of the same platforms. (CircuitPython builds on tinyusb for the same purpose.)

I really wanted Zephyr on the Clue, but it only supports DFU/serial programming, and the bootloader is placed in high flash with a configuration register overwritten to make the system start there, so the chance of bricking the device during porting is really high.

Mine has a UF2 bootloader. Just double tap reset. The UF2 bootloader (source) may assume the soft device is present though.

If you do brick it during porting I can get you a new one.

SWD pins are provided only on unlabeled test points, so there seems no solution but to throw the boards into a box or donate them to somebody who understands Arduino or CircuitPython.

The Feather nRF52840 is the best board if you want 2x5 SWD. It should work similar enough to the clue if you need SWD to debug a bad crash. If you need SWD on the clue you can solder from the test points to a larger pad and then use a micro:bit breakout to get it to a bread board. Eagle files for it are here: https://github.com/adafruit/Adafruit-CLUE-PCB (To figure out which test point is which.)

You should try CircuitPython if you haven't. It's very easy to get started. This is the setup guide: learn.adafruit.com/welcome-to-circuitpython and the API overview is here: https://learn.adafruit.com/circuitpython-essentials/

Too bad: it would have been a really cool Zephyr platform.

It would be!

@tannewt
Copy link

tannewt commented Feb 20, 2021

Is there an alternative to uf2conv.py that has any market share?

Not that I know of. We use uf2conv.py. Raspberry Pi has their own converter I think.

Copy link

@tannewt tannewt left a comment

Choose a reason for hiding this comment

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

I skimmed the code and it looks reasonable to me.

@carlescufi
Copy link
Member

Scancode error:

Error: * scan/scripts/uf2conv.py is not apache-2.0 licensed: mit

Since this is a script that does not end in the firmware, I believe this can be considered acceptable.

@carlescufi carlescufi merged commit 32a28f9 into zephyrproject-rtos:master Feb 23, 2021
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.

8 participants