Add Wi-Fi firmware partition support for Pico 2 W #1969

will-v-pi · 2024-10-03T13:45:49Z

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 is 0x12345678 - 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 call pick_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 marked ignored_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.

Enabled using the pico_use_partition_firmware(exe_name) cmake function

lurch · 2024-10-03T14:53:10Z

pinging @peterharperuk as he did a lot of the Wifi-related stuff for Pico 1.

src/rp2_common/pico_cyw43_driver/CMakeLists.txt

…ifi only firmware variants

src/rp2_common/pico_cyw43_driver/cyw43_driver.c

…bootrom_headers

peterharperuk · 2024-11-19T15:02:37Z

Works nicely. I think we need to fix the Pico W (rp2040) build errors in src/rp2_common/pico_cyw43_driver/cyw43_driver.c
It would be nice if it gave an error if you're a numpty like me and put pico_use_partition_firmware after pico_add_extra_outputs, but that could be improved later?

will-v-pi · 2024-11-19T15:13:40Z

It would be nice if it gave an error if you're a numpty like me and put pico_use_partition_firmware after pico_add_extra_outputs, but that could be improved later?

Have added in separate PR #2054

RP2040 does not support partitions, so throw fatal_error at the CMake stage

will-v-pi · 2024-11-19T15:28:41Z

I think we need to fix the Pico W (rp2040) build errors in src/rp2_common/pico_cyw43_driver/cyw43_driver.c

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 pico_use_partition_firmware for RP2040

armandomontanez · 2024-11-19T16:46:37Z

If the Bazel checks are getting in your way, you can add the two new files to a filegroup (which will silence the error) and file an issue+leave a TODO to support this in Bazel and assign it to me.

peterharperuk

Tested this and it seems good.

src/rp2_common/pico_bootrom/include/pico/bootrom.h

will-v-pi · 2024-11-19T17:10:45Z

If the Bazel checks are getting in your way, you can add the two new files to a filegroup (which will silence the error) and file an issue+leave a TODO to support this in Bazel and assign it to me.

Thanks, have done - I couldn't actually assign the issue to you, but have tagged you in it

src/rp2_common/pico_bootrom/include/pico/bootrom.h

lurch · 2024-11-19T23:01:56Z

A Wi-Fi firmware partition is detected as having the ID 0x123456789abcdef0, and the UF2 family_is for Wi-Fi firmware blobs is 0x12345678 - these should probably both be changed to something else before merging?

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?

will-v-pi · 2024-11-20T10:57:11Z

A Wi-Fi firmware partition is detected as having the ID 0x123456789abcdef0, and the UF2 family_is for Wi-Fi firmware blobs is 0x12345678 - these should probably both be changed to something else before merging?

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 0x776966696669726d (hex for wififirm) for the partition ID, and just use the data family_id we already have?

lurch · 2024-11-20T12:45:17Z

src/rp2_common/pico_bootrom/include/pico/bootrom.h

+ * 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


This doxygen mentions both rom_pick_ab_partition and pick_ab_partition - are they referring to the same function or are they different?

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

src/rp2_common/pico_bootrom/include/pico/bootrom.h

lurch · 2024-11-20T13:06:44Z

Yes - I was looking for ideas. We could go with 0x776966696669726d (hex for wififirm) for the partition ID, and just use the data family_id we already have?

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? 😆 )

will-v-pi · 2024-11-20T13:27:44Z

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? 😆 )

Yes, they definitely could - there's no C-SDK specific code in the Wifi firmware partition, it's just a raw binary blob, so should work with MicroPython too.

… family Adds CYW43_WIFI_FW_PARTITION_ID define to override the partition ID

lurch · 2024-11-20T14:26:08Z

On a vaguely-related note, the doxygen at https://github.com/raspberrypi/pico-sdk/blob/develop/src/rp2_common/pico_bootrom/include/pico/bootrom.h#L834 talks about an "explicit buy flag" - is that the same thing as the TBYB flag referred to elsewhere? (If so, it might be worth editing that doxygen for consistency?)

kilograham · 2024-11-21T17:37:21Z

src/rp2_common/pico_cyw43_driver/cyw43_driver.c

+                id |= buffer[i++];
+                id |= ((uint64_t)(buffer[i++]) << 32ull);
+                if (id == CYW43_WIFI_FW_PARTITION_ID) {
+                    picked_p = p;


is it deliberate not to break here?

No - I will add a break

kilograham · 2024-11-21T17:37:29Z

src/rp2_common/pico_cyw43_driver/cyw43_driver.c

+                id |= buffer[i++];
+                id |= ((uint64_t)(buffer[i++]) << 32ull);
+                if (id == CYW43_WIFI_FW_PARTITION_ID) {
+                    picked_p = p;


is it deliberate not to break here?

kilograham · 2024-11-21T17:38:29Z

src/rp2_common/pico_cyw43_driver/cyw43_driver.c

+                if (picked_p == BOOTROM_ERROR_NOT_FOUND) {
+                    CYW43_DEBUG("Chosen CYW43 firmware partition was not verified\n");
+                } else if (picked_p == BOOTROM_ERROR_NOT_PERMITTED) {
+                    CYW43_DEBUG("Too many update boots going on at once\n");


not sure what this error message means!

It means there are multiple partitions being updated at once, so if you update the boot partition with a TBYB image and the firmware partition in the same boot you get this error.

I can't actually think of a scenario when this might happen, because you can only have one flash_update_base, but I left in the error handling for it in case it does (as I did get it to happen during my early testing, although the code has changed a lot since then)

kilograham

new source files should have copyright/license

kilograham · 2024-11-21T17:47:26Z

src/rp2_common/pico_bootrom/bootrom.c

+// 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


arguably these are not addresses. i would leave the leading * off and use in code references

Technically they are addresses, eg the version downgrade erase address (ie the address in flash to erase) is stored at 0x400e0338

Page 31 of the RP2350 datasheet says BOOTRAM_BASE 0x400e0000

yeah, more of a stylistic thing I guess; it's weird to see a CONSTANT as a lhs

kilograham · 2024-11-21T23:12:05Z

src/rp2_common/pico_bootrom/include/pico/bootrom.h

@@ -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


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"

kilograham · 2024-11-21T23:12:45Z

src/rp2_common/pico_cyw43_driver/cyw43_driver.c

+    #include "boot/picobin.h"
+    #include <stdlib.h>
+
+    // PICO_CONFIG: CYW43_WIFI_FW_PARTITION_ID, ID of Wi-Fi firmware partition, type=int, default=0x776966696669726d (wififirm), group=pico_cyw43_driver


I would put this in the header, it seems like other people might need to know the value

kilograham · 2024-11-21T23:19:54Z

src/rp2_common/pico_cyw43_driver/CMakeLists.txt

    pico_promote_common_scope_vars()
+
+    function(pico_use_partition_firmware TARGET)


I think in the future as mentioned elsewhere we might want to allow more params to configure how we do things (partition size/loc, versioninfo, partition ID) but this is fine too add via CMake args later

That said, the name in the global namespace is a bit non-specific. perhaps `pico_configure_wifi_firmware_partition' (or select instead of configure

kilograham · 2024-11-21T23:25:39Z

src/rp2_common/pico_cyw43_driver/cyw43_driver.c

+
+    // PICO_CONFIG: CYW43_WIFI_FW_PARTITION_ID, ID of Wi-Fi firmware partition, type=int, default=0x776966696669726d (wififirm), group=pico_cyw43_driver
+    #ifndef CYW43_WIFI_FW_PARTITION_ID
+    #define CYW43_WIFI_FW_PARTITION_ID 0x776966696669726d // wififirm


I was humming an hah-ing about this, as i don't know whether we want

a) this to be generic (so that different WiFi chips can use the same ID)
b) non-specific with version info, so people can search for the right partition

I think for now this is OK (even though the code is cyw43_ specific) as we can always change it in the future

I suspect that in the future we're likely to want to pass the CYW43_WIFI_FW_PARTITION_ID from the CMake in which case the CYW43_USE_PARTITION_FIRMWARE is perhaps superfluous, but i'm fine with it. (note i guess we won't pass ID from CMake for now as it would require dynamic JSON)

Please however rename CYW43_WIFI_FW_PARTITION_ID to CYW43_WIFI_FIRMWARE_PARTITION_ID as FW_ is not used as a contraction in any other defines... and I could go with CYW43_USE_FIRMWARE_PARTITION

Finally, is this WiFi only; in which case either one or the other define should gain/lose _WIFI

will-v-pi added 3 commits October 3, 2024 12:06

Add rom_pick_ab_update_partition function

cda08b7

Fix TBYB flag - it's bool, not uint32_t

9260cf7

Add partitions for Wi-Fi firmware

9efa702

Enabled using the pico_use_partition_firmware(exe_name) cmake function

will-v-pi requested a review from kilograham October 3, 2024 13:45

This was referenced Oct 3, 2024

Add TBYB safe rom_pick_ab_partition function #1850

Closed

Pico 2 W OTA Update Example raspberrypi/pico-examples#559

Draft

peterharperuk reviewed Oct 8, 2024

View reviewed changes

src/rp2_common/pico_cyw43_driver/CMakeLists.txt Outdated Show resolved Hide resolved

peterharperuk reviewed Oct 8, 2024

View reviewed changes

src/rp2_common/pico_cyw43_driver/CMakeLists.txt Outdated Show resolved Hide resolved

Use common scope var for PICO_CYW43_DRIVER_CURRENT_PATH, and remove w…

6b69108

…ifi only firmware variants

peterharperuk reviewed Oct 8, 2024

View reviewed changes

src/rp2_common/pico_cyw43_driver/cyw43_driver.c Show resolved Hide resolved

peterharperuk reviewed Oct 8, 2024

View reviewed changes

src/rp2_common/pico_cyw43_driver/cyw43_driver.c Show resolved Hide resolved

peterharperuk reviewed Oct 8, 2024

View reviewed changes

src/rp2_common/pico_cyw43_driver/cyw43_driver.c Outdated Show resolved Hide resolved

will-v-pi added 2 commits October 15, 2024 16:59

Use FLASH_SECTOR_SIZE instead of 4096

0c91aab

Attempt to fix bazel build in same way as pico_flash works with pico_…

508071f

…bootrom_headers

will-v-pi force-pushed the pico2-w-pts branch from d6121e9 to 508071f Compare October 15, 2024 16:36

will-v-pi marked this pull request as ready for review October 17, 2024 09:01

will-v-pi added this to the 2.0.1 milestone Oct 30, 2024

Throw FATAL_ERROR when using RP2040

ab77cf9

RP2040 does not support partitions, so throw fatal_error at the CMake stage

peterharperuk reviewed Nov 19, 2024

View reviewed changes

peterharperuk previously approved these changes Nov 19, 2024

View reviewed changes

Add new sources to bazel filegroup

7cfcf04

will-v-pi dismissed peterharperuk’s stale review via 7cfcf04 November 19, 2024 16:58

will-v-pi mentioned this pull request Nov 19, 2024

[Bazel] Support storing Wi-Fi firmware in partitions #2055

Open

lurch reviewed Nov 19, 2024

View reviewed changes

src/rp2_common/pico_bootrom/include/pico/bootrom.h Show resolved Hide resolved

Add Bazel todo referencing raspberrypi#2055

3e84228

lurch reviewed Nov 19, 2024

View reviewed changes

src/rp2_common/pico_bootrom/include/pico/bootrom.h Outdated Show resolved Hide resolved

Improve rom_pick_ab_update_partition description

0511287

lurch reviewed Nov 20, 2024

View reviewed changes

src/rp2_common/pico_bootrom/include/pico/bootrom.h Outdated Show resolved Hide resolved

lurch reviewed Nov 20, 2024

View reviewed changes

src/rp2_common/pico_bootrom/include/pico/bootrom.h Outdated Show resolved Hide resolved

lurch reviewed Nov 20, 2024

View reviewed changes

src/rp2_common/pico_bootrom/include/pico/bootrom.h Show resolved Hide resolved

will-v-pi added 2 commits November 20, 2024 13:46

Fix docs typos, add \see, and rename brief

b600894

Change partition ID to 0x776966696669726d (wififirm) and use data UF2…

6cf28a3

… family Adds CYW43_WIFI_FW_PARTITION_ID define to override the partition ID

kilograham reviewed Nov 21, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Wi-Fi firmware partition support for Pico 2 W #1969

Add Wi-Fi firmware partition support for Pico 2 W #1969

will-v-pi commented Oct 3, 2024

lurch commented Oct 3, 2024

peterharperuk commented Nov 19, 2024

will-v-pi commented Nov 19, 2024

will-v-pi commented Nov 19, 2024

armandomontanez commented Nov 19, 2024 •

edited

Loading

peterharperuk left a comment

will-v-pi commented Nov 19, 2024

lurch commented Nov 19, 2024

will-v-pi commented Nov 20, 2024

lurch Nov 20, 2024

will-v-pi Nov 20, 2024

lurch commented Nov 20, 2024

will-v-pi commented Nov 20, 2024

lurch commented Nov 20, 2024

kilograham Nov 21, 2024

will-v-pi Nov 21, 2024

kilograham Nov 21, 2024

kilograham Nov 21, 2024

will-v-pi Nov 21, 2024

kilograham left a comment

kilograham Nov 21, 2024

will-v-pi Nov 21, 2024

lurch Nov 21, 2024

kilograham Nov 21, 2024

kilograham Nov 21, 2024

kilograham Nov 21, 2024

kilograham Nov 21, 2024

kilograham Nov 21, 2024

kilograham Nov 21, 2024

kilograham Nov 21, 2024

kilograham Nov 21, 2024

		pico_promote_common_scope_vars()

		function(pico_use_partition_firmware TARGET)

Add Wi-Fi firmware partition support for Pico 2 W #1969

Are you sure you want to change the base?

Add Wi-Fi firmware partition support for Pico 2 W #1969

Conversation

will-v-pi commented Oct 3, 2024

lurch commented Oct 3, 2024

peterharperuk commented Nov 19, 2024

will-v-pi commented Nov 19, 2024

will-v-pi commented Nov 19, 2024

armandomontanez commented Nov 19, 2024 • edited Loading

peterharperuk left a comment

Choose a reason for hiding this comment

will-v-pi commented Nov 19, 2024

lurch commented Nov 19, 2024

will-v-pi commented Nov 20, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lurch commented Nov 20, 2024

will-v-pi commented Nov 20, 2024

lurch commented Nov 20, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kilograham left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

armandomontanez commented Nov 19, 2024 •

edited

Loading