-
-
Notifications
You must be signed in to change notification settings - Fork 26
Build universal UF2 file compatible with RP2040 and RP2350 #53
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
base: master
Are you sure you want to change the base?
Conversation
9ary
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks okay overall, just a few nits and a proper solution for your shell issue.
| elif family == "data": | ||
| family_id = 0xE48BFF58 # DATA family ID compatible with RP2350 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If this is unused, I would rather not include it. As far as I understand, this is related to flash partitions on the RP2350 and newer. I haven't looked at the PicoBoot side of things to know whether you're actually using it, but since you're not shipping this file here, I'm assuming you aren't.
| def main(): | ||
| if len(sys.argv) not in range(3, 4 + 1): | ||
| print(f"Usage: {sys.argv[0]} <output> <executable> [<IPL ROM>]") | ||
| print(f"Usage: {sys.argv[0]} <output> <executable> [<IPL ROM>/<family>]") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"Family" is a bit too vague, something like "UF2 board family" would be more descriptive.
|
|
||
| elif output.endswith(".uf2"): | ||
| if len(sys.argv) < 4: | ||
| print("Missing family argument!") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ditto.
| install: true, | ||
| install_dir: '/', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
BTW. I've also added DOL file to build artifacts as it's very convenient for testing and for building PicoBoot in CI pipeline.
From a UX perspective, shipping the dol is a bit problematic because it isn't really useful on its own. Not sure how to handle this but let's at least make it clear that it isn't meant for end user consumption.
Can you explain a bit more what you're doing with the dol?
| pico_universal = custom_target( | ||
| 'pico_universal', | ||
| input: [pico, pico2], | ||
| output: 'gekkoboot_universal.uf2', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Two things.
- This should be named
gekkoboot_pico_universal.uf2(or even better,gekkoboot_picoboot_universal.uf2). Regardless of compatibility with various Pico models, this is still specific to PicoBoot and its memory layout. If we were to support other modchip firmware, I'd prefer shipping separate files for them to minimize confusion. - Since there are multiple files for the PicoBoot modchip family, they should all be grouped in a directory. But in reality, I would rather unship the individual files even if they are built internally, and only keep the fat binary in the release archive. Having only one file saves the user some guesswork as to which one they need.
| 'pico_universal', | ||
| input: [pico, pico2], | ||
| output: 'gekkoboot_universal.uf2', | ||
| command: ['sh', '-c', 'cat "$1" "$2" > "$3"', 'sh', '@INPUT0@', '@INPUT1@', '@OUTPUT@'], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The redirect operator didn't work because Meson doesn't run build commands in a shell. Anything involving shells is woefully unportable so they should be avoided at all costs.
The correct solution here is to use capture. See the packer build script for examples of how it's used:
Lines 44 to 50 in c8f7e10
| bin_size = custom_target( | |
| name + '_bin_size', | |
| input: bin, | |
| output: name + '.bin_size.ld', | |
| capture: true, | |
| command: [stat, '--format=executable_bin_size = %s;', '@INPUT@'], | |
| ) |
Try this:
| command: ['sh', '-c', 'cat "$1" "$2" > "$3"', 'sh', '@INPUT0@', '@INPUT1@', '@OUTPUT@'], | |
| command: [cat, '@INPUT@'], | |
| capture: true, |
@INPUT@ means the entire list of inputs and it's safe to use here.
Also, use find_program() instead of naming cat directly in your command:
Lines 21 to 22 in c8f7e10
| objcopy = find_program('objcopy') | |
| elf2dol = find_program('elf2dol') |
PicoBoot is now compatible with Pico 2 boards thanks to webhdx/PicoBoot#129 This requires new payload UF2 files to be created. A nice thing about Pico and UF2 is that we can have a single UF2 file with blocks targeted at different MCUs. It lets us create a single gekkoboot payload compatible with both Pico and Pico 2 boards.
I updated
dol2ipl.pyscript to take family id used in uf2 file and updated mason build to produce concatenated uf2 file.I'm not particularly satisied with:
But meson would treat
>output redirection operator as a string value so I had to figure out a workaround for this.BTW. I've also added DOL file to build artifacts as it's very convenient for testing and for building PicoBoot in CI pipeline.