-
Notifications
You must be signed in to change notification settings - Fork 972
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 Wi-Fi firmware partition support for Pico 2 W #1969
base: develop
Are you sure you want to change the base?
Conversation
pinging @peterharperuk as he did a lot of the Wifi-related stuff for Pico 1. |
d6121e9
to
508071f
Compare
Works nicely. I think we need to fix the Pico W (rp2040) build errors in src/rp2_common/pico_cyw43_driver/cyw43_driver.c |
Have added in separate PR #2054 |
I've added a check that it's not RP2040 in the CMake function, so it'll throw a fatal error at that point if you try to build a binary with |
If the Bazel checks are getting in your way, you can add the two new files to a |
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.
Tested this and it seems good.
Thanks, have done - I couldn't actually assign the issue to you, but have tagged you in it |
Is that still going to be happening? I guess choosing randomly-generated IDs is much less likely to produce an accidental collision than a "nice" ID like 0x123456789abcdef0 ? And should those IDs be added to a header-file, to allow other UF2s to make use of this Wi-Fi firmware partition? |
Yes - I was looking for ideas. We could go with |
* It requires the same minimum workarea size as `rom_pick_ab_partition`. | ||
* | ||
* Also checks that the chosen partition contained a valid image | ||
* For example, if an `explicit_buy` is pending then calling `pick_ab_partition` would normally clear the saved `flash_erase_addr` so the required erase would not |
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 doxygen mentions both rom_pick_ab_partition
and pick_ab_partition
- are they referring to the same function or are they different?
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.
rom_…
is the SDK function wrapper for the pick_ab_partition
function - so I was using that to refer to it in the SDK (ie the function this calls internally, and where more docs are), but using pick_ab_partition
to refer to the behaviour of the underlying bootrom function
I've got no opinions on this so it's probably something that you and @peterharperuk ought to decide. Would MicroPython be able to use the same Wifi firmware partition as C-SDK code? (or is that another one of my stupidly naive questions? 😆 ) |
// printf("IMAGE_DEF_TBYB_FLAGGED %08x\n", (uint32_t)&(scan_workarea->parsed_block_loops[0].image_def.core.tbyb_flagged) - (uint32_t)scan_workarea); | ||
// printf("IMAGE_DEF_BASE %08x\n", (uint32_t)&(scan_workarea->parsed_block_loops[0].image_def.core.enclosing_window.base) - (uint32_t)scan_workarea); | ||
// printf("IMAGE_DEF_REL_BLOCK_OFFSET %08x\n", (uint32_t)&(scan_workarea->parsed_block_loops[0].image_def.core.window_rel_block_offset) - (uint32_t)scan_workarea); | ||
#define VERSION_DOWNGRADE_ERASE_ADDR *(uint32_t*)0x400e0338 |
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.
arguably these are not addresses. i would leave the leading * off and use in code references
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.
Technically they are addresses, eg the version downgrade erase address (ie the address in flash to erase) is stored at 0x400e0338
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.
Page 31 of the RP2350 datasheet says BOOTRAM_BASE 0x400e0000
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.
yeah, more of a stylistic thing I guess; it's weird to see a CONSTANT as a lhs
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.
but i'm ok
@@ -1071,6 +1071,29 @@ static inline int rom_get_last_boot_type(void) { | |||
*/ | |||
int rom_add_flash_runtime_partition(uint32_t start_offset, uint32_t size, uint32_t permissions); | |||
|
|||
/*! \brief Pick A/B partition with TBYB guards |
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.
I still find the function description a bit confusing:
- maybe something along the lines of "Pick A/B partition for an arbitrary pair of partitions without disturbing any in process update A/B or TBYB boot" (needs improvement still)...
- not sure people know what a flash_erase_addr is maybe describe it in words (e.g. the erase of xxx because yyy"
- I'd say "using the flash_update_boot_window_base from the current boot"
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.
I think "Pick A/B partition without disturbing any in progress update or TBYB boot" works for the description - pick_ab_partition is always for an arbitrary pair of partitions, so that probably doesn't need including in the description?
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.
Ah, I was (wrongly) thinking this new API existed because pick_ab_partition was for the regular boot partitions, rather than because that API is somewhat broken. Perhaps just add a warning to the other API referencing this one
Enabled using the pico_use_partition_firmware(exe_name) cmake function
…ifi only firmware variants
RP2040 does not support partitions, so throw fatal_error at the CMake stage
… family Adds CYW43_WIFI_FW_PARTITION_ID define to override the partition ID
Add copyright headers Put ptinfo buffer on the stack, and break when partition is found Improve function description
RCP mask constants still need to be accessed from Risc-V
f7952f2
to
1e7e0c7
Compare
I've noticed an issue with the Risc-V build of the firmware UF2 - for some reason the compilation is putting a programme header in the ELF with offset of |
… CYW43_WIFI_FW_PARTITION_ID->CYW43_FIRMWARE_PARTITION_ID
1792796
to
f2511a9
Compare
The Risc-V compiler doesn't link the firmware blobs as expected, so use Arm ELFs instead
I've now got it working for Risc-V by storing copies of the firmware ELFs in the SDK, and then copying those into the build when necessary. It also needs increased bootrom stack size. This is tested and working fine. However, I'm not sure if this is actually a solution that we want to put in the 2.1.0 SDK release, so it might be best if we don't merge this for 2.1.0 and I can investigate further to fix the Risc-V firmware build? |
This adds the ability to store and load the Wi-Fi firmware for Pico 2 W in a partition. It can be enabled by adding
pico_use_partition_firmware(<exe_name>)
to your CMakeLists.txt, which will embed a compatible partition table in the binary, and output lots of firmware UF2s to use (all called<exe_name>_firmware_..._.uf2
, eg..._firmware_w
for Wi-Fi only,..._firmware_wb
for Wi-Fi and Bluetooth,..._firmware_w_tbyb
for TBYB). You can also create your own partition table and use that.A Wi-Fi firmware partition is detected as having the ID
0x123456789abcdef0
, and the UF2 family_is for Wi-Fi firmware blobs is0x12345678
- these should probably both be changed to something else before merging? The default firmware partition starts at 3500K into the flash - should this be changed to depend on PICO_FLASH_SIZE_BYTES? The default firmware partition is also duplicated with A/B partitions in the same location in flash - this is required to ensure a signature check is performed before loading the Wi-Fi firmware, as there's no way to call the bootrom to check the signature of a single partition (unless chaining into it), you can only callpick_ab_partition
.The Wi-Fi firmware blob is marked in it's image_def as an RP2350 Risc-V executable, and the partition is marked as
ignored_during_riscv_boot
- this ensures that it can work with TBYB (as TBYB only works for executable image_defs), and that signature checks are performed before loading the firmware when Secure Boot is enabled (because signature checks are performed for all executable image_defs in a partition that is not markedignored_during_arm_boot
). This is slightly clunky, but seems to work robustly.Supercedes #1850, as it now includes that function in this PR, and shows a use for it.