Re: [PATCH v5] Bluetooth: btrtl: Add firmware format v3 support

[Paul Menzel]

Dear Hilda,


Thank you for your patch.

Am 24.06.26 um 07:33 schrieb Hilda Wu:
> Realtek updated its Bluetooth firmware format to v3.

Please name the specification document, and state whether it’s 
publically accessible.

> This patch extends the btrtl driver to recognise and parse the new v3 file
> format, including:
> - New signature string and image ID definitions
> - Extension of btrtl_device_info to store v3-specific metadata
> - Logic to extract and load firmware data out of v3 images
> - Maintains compatibility with existing v2 firmware format
>
> This is required for future Realtek Bluetooth chips that ship with
> v3 firmware.

Please add a blank line between paragraphs or do not wrap the line after 
the sentence.

> The RTL8922D is the first IC to use firmware format V3, so the following
> example uses the RTL8922D's log as expected fw format v3 output:
>
> Bluetooth: btrtl_read_chip_id() hci0: RTL: chip_id status=0x00 id=0x37
> Bluetooth: btrtl_initialize() hci0: RTL: examining hci_ver=0d
> hci_rev=000d lmp_ver=0d lmp_subver=8922

Please do not wrap the lines of the pasted lines. If you keep the 
timestamp in the front, `checkpatch.pl` should not complain.

> Bluetooth: rtl_read_rom_version() hci0: RTL: rom_version status=0 version=1
> Bluetooth: btrtl_initialize() hci0: RTL: btrtl_initialize: key id 0
> Bluetooth: rtl_load_file() hci0: RTL: loading rtl_bt/rtl8922du_fw.bin
> Bluetooth: rtl_load_file() hci0: RTL: loading rtl_bt/rtl8922du_config.bin
> Bluetooth: rtlbt_parse_firmware_v3() hci0: RTL: key id 0
> Bluetooth: rtlbt_parse_section_v3() hci0: RTL: image (f000:00), chip id
> 55, cut 0x02, len 00007185
> Bluetooth: rtlbt_parse_section_v3() hci0: RTL: image version: 35fd7908
> Bluetooth: rtlbt_parse_config() hci0: RTL: config file:
> rtl_bt/rtl8922du_config_f000.bin
> Bluetooth: rtlbt_parse_section_v3() hci0: RTL: image (f002:00), chip id
> 55, cut 0x02, len 000078f5
> Bluetooth: rtlbt_parse_section_v3() hci0: RTL: image version: 47b6874d
> Bluetooth: rtlbt_parse_config() hci0: RTL: config file:
> rtl_bt/rtl8922du_config_f002.bin
> Bluetooth: rtlbt_parse_firmware_v3() hci0: RTL: image payload total len:
> 0x0000ea7a
> Bluetooth: rtl_finalize_download() hci0: RTL: Watchdog reset status 00
> Bluetooth: rtl_finalize_download() hci0: RTL: fw version 0x47b6874d

How big in the firmware file, and how long did it take to load it?

> Signed-off-by: Alex Lu <alex_lu@xxxxxxxxxxxxxx>
> Signed-off-by: Zoey Zhou <zoey_zhou@xxxxxxxxxxxxxx>
> Signed-off-by: Hilda Wu <hildawu@xxxxxxxxxxx>
>
> ---
> V4 -> V5::
> - Add independent support for RTL8922D section
> - Introduce macros to improve code readability
> - Document firmware format v3 and its differences
> - Align implementation with reviewer feedback
>
> V3 -> V4:
> - Rework skb->data access and add clarifying comments
> - Fix latent issues
>
> V2 -> V3:
> - Address coccinelle warning
>
> V1 -> V2:
> - Add missing symbols
> - Resolve build warnings
> ---
>   drivers/bluetooth/btrtl.c | 698 +++++++++++++++++++++++++++++++++++++-
>   drivers/bluetooth/btrtl.h | 102 ++++++

The files get bigger. Would it be good to split the firmware handling 
out into a separate file?

>   drivers/bluetooth/btusb.c |   3 +
>   3 files changed, 786 insertions(+), 17 deletions(-)

For easier review, would it be possible to split out the refactoring 
like defining the macros FW_TYPE_V0, … and using them?

[…]


Kind regards,

Paul