From f7952f22dc5c7601d1f5bd4401848398234a0166 Mon Sep 17 00:00:00 2001 From: William Vinnicombe Date: Fri, 22 Nov 2024 10:29:53 +0000 Subject: [PATCH] Review feedback Add copyright headers Put ptinfo buffer on the stack, and break when partition is found Improve function description --- src/rp2_common/pico_bootrom/include/pico/bootrom.h | 13 +++++++------ src/rp2_common/pico_cyw43_driver/cyw43_driver.c | 10 ++++------ src/rp2_common/pico_cyw43_driver/cyw43_firmware.py | 6 ++++++ .../include/cyw43_partition_firmware.h | 5 +++++ src/rp2_common/pico_cyw43_driver/wifi_firmware.S | 6 ++++++ 5 files changed, 28 insertions(+), 12 deletions(-) diff --git a/src/rp2_common/pico_bootrom/include/pico/bootrom.h b/src/rp2_common/pico_bootrom/include/pico/bootrom.h index 36de93e1c..d42248d7b 100644 --- a/src/rp2_common/pico_bootrom/include/pico/bootrom.h +++ b/src/rp2_common/pico_bootrom/include/pico/bootrom.h @@ -1071,16 +1071,17 @@ 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 +/*! \brief Pick A/B partition without disturbing any in progress update or TBYB boot * \ingroup pico_bootrom * - * This will call `rom_pick_ab_partition` with the current `flash_update_boot_window_base`, while performing extra checks to prevent disrupting a main image TBYB. - * It requires the same minimum workarea size as `rom_pick_ab_partition`. + * This will call `rom_pick_ab_partition` using the `flash_update_boot_window_base` from the current boot, while performing extra checks to prevent disrupting + * a main image TBYB boot. It requires the same minimum workarea size as `rom_pick_ab_partition`. * \see rom_pick_ab_partition() * - * 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 - * occur when `explicit_buy` is called - this function saves and restores that address to prevent this issue, and returns `BOOTROM_ERROR_NOT_PERMITTED` if the - * partition chosen by `pick_ab_partition` also requires a flash erase version downgrade (as you can't erase 2 partitions with one `explicit_buy` call). + * For example, if an `explicit_buy` is pending then calling `pick_ab_partition` would normally clear the saved flash erase address for the version downgrade, + * so the required erase of the other partition would not occur when `explicit_buy` is called - this function saves and restores that address to prevent this + * issue, and returns `BOOTROM_ERROR_NOT_PERMITTED` if the partition chosen by `pick_ab_partition` also requires a flash erase version downgrade (as you can't + * erase 2 partitions with one `explicit_buy` call). * * It also checks that the chosen partition contained a valid image (eg a signed image when using secure boot), and returns `BOOTROM_ERROR_NOT_FOUND` * if it does not. diff --git a/src/rp2_common/pico_cyw43_driver/cyw43_driver.c b/src/rp2_common/pico_cyw43_driver/cyw43_driver.c index 52d2beaa6..edd6d87f3 100644 --- a/src/rp2_common/pico_cyw43_driver/cyw43_driver.c +++ b/src/rp2_common/pico_cyw43_driver/cyw43_driver.c @@ -121,9 +121,8 @@ static void cyw43_sleep_timeout_reached(async_context_t *context, __unused async bool cyw43_driver_init(async_context_t *context) { #if CYW43_USE_PARTITION_FIRMWARE - const int buf_words = (16 * 4) + 1; // maximum of 16 partitions, each with maximum of 4 words returned, plus 1 - uint32_t* buffer = malloc(buf_words * 4); - int ret = rom_get_partition_table_info(buffer, buf_words, PT_INFO_PARTITION_LOCATION_AND_FLAGS | PT_INFO_PARTITION_ID); + uint32_t buffer[(16 * 4) + 1] = {}; // maximum of 16 partitions, each with maximum of 4 words returned, plus 1 + int ret = rom_get_partition_table_info(buffer, count_of(buffer), PT_INFO_PARTITION_LOCATION_AND_FLAGS | PT_INFO_PARTITION_ID); assert(buffer[0] == (PT_INFO_PARTITION_LOCATION_AND_FLAGS | PT_INFO_PARTITION_ID)); @@ -141,6 +140,7 @@ bool cyw43_driver_init(async_context_t *context) { id |= ((uint64_t)(buffer[i++]) << 32ull); if (id == CYW43_WIFI_FW_PARTITION_ID) { picked_p = p; + break; } } @@ -162,7 +162,7 @@ bool cyw43_driver_init(async_context_t *context) { } CYW43_DEBUG("Chosen CYW43 firmware in partition %d\n", picked_p); - int ret = rom_get_partition_table_info(buffer, buf_words, PT_INFO_PARTITION_LOCATION_AND_FLAGS | PT_INFO_SINGLE_PARTITION | (picked_p << 24)); + int ret = rom_get_partition_table_info(buffer, count_of(buffer), PT_INFO_PARTITION_LOCATION_AND_FLAGS | PT_INFO_SINGLE_PARTITION | (picked_p << 24)); assert(buffer[0] == (PT_INFO_PARTITION_LOCATION_AND_FLAGS | PT_INFO_SINGLE_PARTITION)); assert(ret == 3); @@ -179,9 +179,7 @@ bool cyw43_driver_init(async_context_t *context) { cyw43_clm_len = *(uint32_t*)(XIP_NOCACHE_NOALLOC_NOTRANSLATE_BASE + saddr + 4); fw_data = XIP_NOCACHE_NOALLOC_NOTRANSLATE_BASE + saddr + 8; } - free(buffer); } else { - free(buffer); CYW43_DEBUG("No partition table, so cannot get firmware from partition - get_partition_table_info returned %d\n", ret); return false; } diff --git a/src/rp2_common/pico_cyw43_driver/cyw43_firmware.py b/src/rp2_common/pico_cyw43_driver/cyw43_firmware.py index e32fe80ff..e669a169d 100644 --- a/src/rp2_common/pico_cyw43_driver/cyw43_firmware.py +++ b/src/rp2_common/pico_cyw43_driver/cyw43_firmware.py @@ -1,3 +1,9 @@ +#!/usr/bin/env python3 +# +# Copyright (c) 2024 Raspberry Pi (Trading) Ltd. +# +# SPDX-License-Identifier: BSD-3-Clause + import sys with open(sys.argv[1], "r") as f: diff --git a/src/rp2_common/pico_cyw43_driver/include/cyw43_partition_firmware.h b/src/rp2_common/pico_cyw43_driver/include/cyw43_partition_firmware.h index afec44245..5df9eb5e8 100644 --- a/src/rp2_common/pico_cyw43_driver/include/cyw43_partition_firmware.h +++ b/src/rp2_common/pico_cyw43_driver/include/cyw43_partition_firmware.h @@ -1,3 +1,8 @@ +/* + * Copyright (c) 2024 Raspberry Pi (Trading) Ltd. + * + * SPDX-License-Identifier: BSD-3-Clause + */ extern int cyw43_wifi_fw_len; extern int cyw43_clm_len; diff --git a/src/rp2_common/pico_cyw43_driver/wifi_firmware.S b/src/rp2_common/pico_cyw43_driver/wifi_firmware.S index bca9fddbf..412f43739 100644 --- a/src/rp2_common/pico_cyw43_driver/wifi_firmware.S +++ b/src/rp2_common/pico_cyw43_driver/wifi_firmware.S @@ -1,3 +1,9 @@ +/* + * Copyright (c) 2024 Raspberry Pi (Trading) Ltd. + * + * SPDX-License-Identifier: BSD-3-Clause + */ + #include "boot/picobin.h" #if PICO_CRT0_IMAGE_TYPE_TBYB