kilograham
commented
1 year ago typedef struct cyw43_firmware_funcs {
const cyw43_firmware_details_t* (*firmware_details)(void); ///< get wifi firmware details
int (*start_wifi_fw)(const cyw43_firmware_details_t *fw_details); ///< start wifi firmware loading
int (*start_bt_fw)(const cyw43_firmware_details_t *fw_details); ///< start bt firmware loading
const uint8_t* (*get_wifi_fw)(const uint8_t *addr, size_t sz_in, uint8_t *buffer, size_t buffer_sz); ///< get block of wifi firmware data
const uint8_t* (*get_bt_fw)(const uint8_t *addr, size_t sz_in, uint8_t *buffer, size_t buffer_sz); ///< get block of bt firmware data
const uint8_t* (*get_nvram)(const uint8_t *addr, size_t sz_in, uint8_t *buffer, size_t buffer_sz); ///< get block of nvram data
const uint8_t* (*get_clm)(const uint8_t *addr, size_t sz_in, uint8_t *buffer, size_t buffer_sz); ///< get block of clm data
void (*end)(void); ///< end firmware loading
} cyw43_firmware_funcs_t;So my initial impressions:
- Kinda weird that the get_wifi_fw etc don't take the fw_details (or some context)
- Moving on from 1) you would then not need separate wifi/bt/nvram/clm funcs
- I notice that all of these get_ functions are assumed to pass sequential addresses in order, so passing the address arg seems superfluous
- it seems oddly specific to single out wifi ant bt specially (and not nvram and clm)
Also, looking at
typedef struct cyw43_firmware_details {
size_t raw_wifi_fw_len; ///< Size in bytes of the combined wifi firmware data before extraction
size_t wifi_fw_len; ///< Size of the wifi firmware in bytes after extraction
size_t clm_len; ///< Size of the clm blob in bytes after extraction
const uint8_t *wifi_fw_addr;///< Pointer to the raw wifi firmware
const uint8_t *clm_addr; ///< Pointer to the raw clm blob in uncompressed firmware
size_t wifi_nvram_len; ///< Size of nvram data
const uint8_t *wifi_nvram_addr; ///< Pointer to nvram data
size_t raw_bt_fw_len; ///< size of bluetooth firmware data before extraction
size_t bt_fw_len; ///< size of bluetooth firmware data after extraction
const uint8_t *bt_fw_addr; ///< Pointer to the bluetooth firmware
} cyw43_firmware_details_t;I wonder if some of these (e.g. wifi_fw_addr) belong in here... I think the firmware details (and I haven't totally scrutinized the firmware loading code) should be limited to meaningful data about the firmware itself (sizes and wot not)
These are just suggestions, perhaps @dpgeorge has some thoughts too
Maybe something like this
typedef struct cyw43_firmware_funcs {
const cyw43_firmware_details_t* (*firmware_details)(void); ///< get wifi firmware details
int (*start_fw_stream)(const cyw43_firmware_details_t *fw_details, enum cyw43_fw_type which_firmware, void **streaming_context); ///< start fw streaming
// returns number of bytes copied
int (*stream_fw)(void *streaming_context, uint8_t *buffer, size_t buffer_sz); ///< get block of firmware data
void (*end_fw_stream)(void *streaming_context context); ///< end firmware loading
} cyw43_firmware_funcs_t;you can still use a static streaming_context if you like (we can document that we won't read multiple fws concurrently/overlapped)
peterharperuk
dpgeorge
This is a fairly big change which enables firmware compression.
Firmware compression is enabled with a flag CYW43_ENABLE_FIRMWARE_COMPRESSION.
Firmware details and functions for reading firmware are stored in function pointers in cyw43_get_firmware_funcs_default. This can be replaced by changing the macro cyw43_get_firmware_funcs
One of these functions returns the firmware details cyw43_firmware_details_func
New firmware header files have been added that have a compressed (gzip) version of the firmware.
If firmware is compressed it reads the firmware with uzlib, see cyw43_gz_read.c
As the firmware header files are quite complicated now, I have a project to generate them. This also includes a test/demo that demonstrates how firmware could now be stored in flash, see https://github.com/peterharperuk/cyw43-firmware