Add Wi-Fi firmware partition support for Pico 2 W

[will-v-pi]

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_wifi_firmware_partition(<exe_name>) to your CMakeLists.txt, which will embed a compatible partition table in the binary, and output two firmware UF2s to use (<exe_name>_firmware.uf2 for regular, and <exe_name>_firmware_tbyb.uf2 for TBYB). You can also create your own partition table and use that.

A Wi-Fi firmware partition is detected as having the ID 0x776966696669726d, and the UF2 family ID for Wi-Fi firmware blobs is 0xe48bff55. 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.

[lurch]

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

[peterharperuk]

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]

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

[will-v-pi]

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]

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.

[will-v-pi]

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

[lurch]

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]

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]

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? :laughing: )

[will-v-pi]

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.

[lurch]

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

[will-v-pi]

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 0x0000 in the file loading to 0x1ffff000, when the Arm compiler correctly puts this at offset 0x1000 in the file loading to 0x20000000. @Wren6991 or @kilograham any ideas why the Arm and Risc-V compilers might be compiling the wifi_firmware.S file differently?

[will-v-pi]

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?

[richard9999999999]

I see that default firmware partition ID is defined as follows: #define CYW43_FIRMWARE_PARTITION_ID 0x776966696669726d // wififirm SDK uses this value as default to find the partition

..Also in pt.json, this long value is used "id": "0x776966696669726d",

However when I check by picotool, I see following number set as partition ID:

 partition 1 (A):       0036b000->003a7000 S(rw) NSBOOT(rw) NS(rw), id=000000006669726d, "Wi-Fi Firmware", uf2 { data },
                        arm_boot 1, riscv_boot 0

So upper 32 bits are zeros.. And I need to set this truncated value to CYW43_FIRMWARE_PARTITION_ID to get it working.

Did I miss something?

[richard9999999999]

One more note:

function(pico_use_wifi_firmware_partition TARGET)
        if (PICO_PLATFORM STREQUAL "rp2040")
                message(FATAL_ERROR "RP2040 does not support storing wi-fi firmware in partitions")
        endif()
        target_compile_definitions(${TARGET} PRIVATE CYW43_USE_FIRMWARE_PARTITION=1)
        pico_embed_pt_in_binary(${TARGET} ${PICO_CYW43_DRIVER_CURRENT_PATH}/wifi_pt.json)

seems like that partitioning of binary is always done according wifi_pt.json file. In many cases user prefers to have his own partitioning (when e.g. custom bootloader is used). Could'n this be done via preprocessor macro define e.g. something like "CYW43_EMBED_FIRMWARE_PARTITION=0"?

[will-v-pi]

Rebased onto develop, which contains the rom_pick_ab_update_partition function

[will-v-pi]

However when I check by picotool, I see following number set as partition ID:

This looks like a bug in your build of picotool - for me it shows the correct full ID, and the ID in the actual partition table in flash should also be correct. If you're still seeing this with the latest picotool develop branch, could you raise an issue against that repo with details of how you're building picotool?

In many cases user prefers to have his own partitioning (when e.g. custom bootloader is used)

I will push a commit to only set the partition table if it has not already been defined, and if the user defines it afterwards then that will override this definition

[richard9999999999]

This looks like a bug in your build of picotool

The picotool used to embed the pt into my binary is loaded and built "automatically" into project's "build" folder. I will wait till this your change will be committed into the dev branch and then I will check again..

[richard9999999999]

The "picotool version" shows "picotool v2.1.2-develop (Windows, GNU-8.1.0, Release)"

[richard9999999999]

I will push a commit to only set the partition table if it has not already been defined, and if the user defines it afterwards then that will override this definition This solution does not fit to my case: I use encrypted bootloader and all partitioning (including wifi firmware partition) is defined in the bootloader binary (so in the different binary). Thus in this case we cannot decide wifi partition has been included or not using yours: get_target_property(picotool_embed_pt ${TARGET} PICOTOOL_EMBED_PT)

[will-v-pi]

That use case will work fine - the embedded partition table in these binaries will be ignored when they’re booted from partitions by the bootloader (as opposed to when they’re booted from slots, see 5.1.12-15 in the datasheet for more details)

[richard9999999999]

OK so I will check that. Thanks much!

[richard9999999999]

One more note (possibly bug), which I found while debugging my problem with truncated partition ID:

file cyw43_driver.c

    } else {
        CYW43_DEBUG("No partition table, so cannot get firmware from partition - get_partition_table_info returned %d\n", ret);
        return false;
    }

above else is for "no partition table found" case. However the error case when we have some partitions but not with correct ID is not handled and it continues in initialization. I mean missing else for: if (picked_p >= 0) {

[richard9999999999]

That use case will work fine - the embedded partition table in these binaries will be ignored when they’re booted from partitions by the bootloader (as opposed to when they’re booted from slots, see 5.1.12-15 in the datasheet for more details)

Unfortunately in the encrypted bootloader sample like scenario (like my project) the wifi partition in the executable binary seems causes malfunction. So I have to exclude the wifi partition from the executable (by hack of your branch) and add it into the bootloader only.

[will-v-pi]

That use case will work fine - the embedded partition table in these binaries will be ignored when they’re booted from partitions by the bootloader (as opposed to when they’re booted from slots, see 5.1.12-15 in the datasheet for more details)

Unfortunately in the encrypted bootloader sample like scenario (like my project) the wifi partition in the executable binary seems causes malfunction. So I have to exclude the wifi partition from the executable (by hack of your branch) and add it into the bootloader only.

I've fixed this, so you can now pass NO_EMBEDDED_PT to the function and it will not embed the partition table

[richard9999999999]

I've fixed this, so you can now pass NO_EMBEDDED_PT to the function and it will not embed the partition table

Thank you, I am looking forward to see all those great changes in develop and in new SDK release later..

[kilograham]

i had imagined that we would use a new family-id so we can target specifically (even if it is just for generic wifi firmware)

[will-v-pi]

Would we want to use the next family ID for this then (0xe48bff5c), and add a define into the SDK and picotool for it (WIFI_FIRMWARE_FAMILY_ID)?

[kilograham]

Yes, we can do that since it is well known

[kilograham]

@peterharperuk any suggestions on naming - i assume firmware does and is always likely to include both Wifi and BT (or at least there wouldn't usually be two separates)