2023-08-04 06:28:20

by Max Chou

[permalink] [raw]
Subject: [PATCH] Bluetooth: btrtl: Load FW v2 otherwise FW v1 for RTL8852C

From: Max Chou <[email protected]>

In the commit of linux-firmware project, rtl8852cu_fw.bin is updated as
FW v2 format[1]. Consider the case that if driver did not be updated for
FW v2 supported[2], it can not use FW v2.
By Canonical's suggestion, older driver should be able to load FW v1,
so rtl8852cu_fw.bin will be revert to the previous commit as FW v1 and
add rtl8852cu_fw_v2.bin as FW v2. This item will be started on
linux-firmware project.

In this commit, the driver prefers to load FW v2 if available. Fallback to
FW v1 otherwise.

To do on linux-firmware project.
rtl_bt/rtl8852cu_fw.bin: FW v1 (stay at ver. 0xD7B8_FABF)
rtl_bt/rtl8852cu_fw_v2.bin: FW v2 (to be maintained)

[1]'9a24ce5e29b1 ("Bluetooth: btrtl: Firmware format v2 support")'
[2]'55e7448533e7 ("rtl_bt: Update RTL8852C BT USB firmware
to 0x040D_7225")'

Suggested-by: Juerg Haefliger <[email protected]>
Tested-by: Hilda Wu <[email protected]>
Signed-off-by: Max Chou <[email protected]>
---
drivers/bluetooth/btrtl.c | 68 +++++++++++++++++++++++++--------------
1 file changed, 44 insertions(+), 24 deletions(-)

diff --git a/drivers/bluetooth/btrtl.c b/drivers/bluetooth/btrtl.c
index ddae6524106d..8bfa86dd12f7 100644
--- a/drivers/bluetooth/btrtl.c
+++ b/drivers/bluetooth/btrtl.c
@@ -104,7 +104,7 @@ static const struct id_table ic_id_table[] = {
{ IC_INFO(RTL_ROM_LMP_8723A, 0xb, 0x6, HCI_USB),
.config_needed = false,
.has_rom_version = false,
- .fw_name = "rtl_bt/rtl8723a_fw.bin",
+ .fw_name = "rtl_bt/rtl8723a_fw",
.cfg_name = NULL,
.hw_info = "rtl8723au" },

@@ -112,7 +112,7 @@ static const struct id_table ic_id_table[] = {
{ IC_INFO(RTL_ROM_LMP_8723B, 0xb, 0x6, HCI_UART),
.config_needed = true,
.has_rom_version = true,
- .fw_name = "rtl_bt/rtl8723bs_fw.bin",
+ .fw_name = "rtl_bt/rtl8723bs_fw",
.cfg_name = "rtl_bt/rtl8723bs_config",
.hw_info = "rtl8723bs" },

@@ -120,7 +120,7 @@ static const struct id_table ic_id_table[] = {
{ IC_INFO(RTL_ROM_LMP_8723B, 0xb, 0x6, HCI_USB),
.config_needed = false,
.has_rom_version = true,
- .fw_name = "rtl_bt/rtl8723b_fw.bin",
+ .fw_name = "rtl_bt/rtl8723b_fw",
.cfg_name = "rtl_bt/rtl8723b_config",
.hw_info = "rtl8723bu" },

@@ -132,7 +132,7 @@ static const struct id_table ic_id_table[] = {
.hci_bus = HCI_UART,
.config_needed = true,
.has_rom_version = true,
- .fw_name = "rtl_bt/rtl8723cs_cg_fw.bin",
+ .fw_name = "rtl_bt/rtl8723cs_cg_fw",
.cfg_name = "rtl_bt/rtl8723cs_cg_config",
.hw_info = "rtl8723cs-cg" },

@@ -144,7 +144,7 @@ static const struct id_table ic_id_table[] = {
.hci_bus = HCI_UART,
.config_needed = true,
.has_rom_version = true,
- .fw_name = "rtl_bt/rtl8723cs_vf_fw.bin",
+ .fw_name = "rtl_bt/rtl8723cs_vf_fw",
.cfg_name = "rtl_bt/rtl8723cs_vf_config",
.hw_info = "rtl8723cs-vf" },

@@ -156,7 +156,7 @@ static const struct id_table ic_id_table[] = {
.hci_bus = HCI_UART,
.config_needed = true,
.has_rom_version = true,
- .fw_name = "rtl_bt/rtl8723cs_xx_fw.bin",
+ .fw_name = "rtl_bt/rtl8723cs_xx_fw",
.cfg_name = "rtl_bt/rtl8723cs_xx_config",
.hw_info = "rtl8723cs" },

@@ -164,7 +164,7 @@ static const struct id_table ic_id_table[] = {
{ IC_INFO(RTL_ROM_LMP_8723B, 0xd, 0x8, HCI_USB),
.config_needed = true,
.has_rom_version = true,
- .fw_name = "rtl_bt/rtl8723d_fw.bin",
+ .fw_name = "rtl_bt/rtl8723d_fw",
.cfg_name = "rtl_bt/rtl8723d_config",
.hw_info = "rtl8723du" },

@@ -172,7 +172,7 @@ static const struct id_table ic_id_table[] = {
{ IC_INFO(RTL_ROM_LMP_8723B, 0xd, 0x8, HCI_UART),
.config_needed = true,
.has_rom_version = true,
- .fw_name = "rtl_bt/rtl8723ds_fw.bin",
+ .fw_name = "rtl_bt/rtl8723ds_fw",
.cfg_name = "rtl_bt/rtl8723ds_config",
.hw_info = "rtl8723ds" },

@@ -180,7 +180,7 @@ static const struct id_table ic_id_table[] = {
{ IC_INFO(RTL_ROM_LMP_8821A, 0xa, 0x6, HCI_USB),
.config_needed = false,
.has_rom_version = true,
- .fw_name = "rtl_bt/rtl8821a_fw.bin",
+ .fw_name = "rtl_bt/rtl8821a_fw",
.cfg_name = "rtl_bt/rtl8821a_config",
.hw_info = "rtl8821au" },

@@ -189,7 +189,7 @@ static const struct id_table ic_id_table[] = {
.config_needed = false,
.has_rom_version = true,
.has_msft_ext = true,
- .fw_name = "rtl_bt/rtl8821c_fw.bin",
+ .fw_name = "rtl_bt/rtl8821c_fw",
.cfg_name = "rtl_bt/rtl8821c_config",
.hw_info = "rtl8821cu" },

@@ -198,7 +198,7 @@ static const struct id_table ic_id_table[] = {
.config_needed = true,
.has_rom_version = true,
.has_msft_ext = true,
- .fw_name = "rtl_bt/rtl8821cs_fw.bin",
+ .fw_name = "rtl_bt/rtl8821cs_fw",
.cfg_name = "rtl_bt/rtl8821cs_config",
.hw_info = "rtl8821cs" },

@@ -206,7 +206,7 @@ static const struct id_table ic_id_table[] = {
{ IC_INFO(RTL_ROM_LMP_8761A, 0xa, 0x6, HCI_USB),
.config_needed = false,
.has_rom_version = true,
- .fw_name = "rtl_bt/rtl8761a_fw.bin",
+ .fw_name = "rtl_bt/rtl8761a_fw",
.cfg_name = "rtl_bt/rtl8761a_config",
.hw_info = "rtl8761au" },

@@ -215,7 +215,7 @@ static const struct id_table ic_id_table[] = {
.config_needed = false,
.has_rom_version = true,
.has_msft_ext = true,
- .fw_name = "rtl_bt/rtl8761b_fw.bin",
+ .fw_name = "rtl_bt/rtl8761b_fw",
.cfg_name = "rtl_bt/rtl8761b_config",
.hw_info = "rtl8761btv" },

@@ -223,7 +223,7 @@ static const struct id_table ic_id_table[] = {
{ IC_INFO(RTL_ROM_LMP_8761A, 0xb, 0xa, HCI_USB),
.config_needed = false,
.has_rom_version = true,
- .fw_name = "rtl_bt/rtl8761bu_fw.bin",
+ .fw_name = "rtl_bt/rtl8761bu_fw",
.cfg_name = "rtl_bt/rtl8761bu_config",
.hw_info = "rtl8761bu" },

@@ -232,7 +232,7 @@ static const struct id_table ic_id_table[] = {
.config_needed = true,
.has_rom_version = true,
.has_msft_ext = true,
- .fw_name = "rtl_bt/rtl8822cs_fw.bin",
+ .fw_name = "rtl_bt/rtl8822cs_fw",
.cfg_name = "rtl_bt/rtl8822cs_config",
.hw_info = "rtl8822cs" },

@@ -241,7 +241,7 @@ static const struct id_table ic_id_table[] = {
.config_needed = true,
.has_rom_version = true,
.has_msft_ext = true,
- .fw_name = "rtl_bt/rtl8822cs_fw.bin",
+ .fw_name = "rtl_bt/rtl8822cs_fw",
.cfg_name = "rtl_bt/rtl8822cs_config",
.hw_info = "rtl8822cs" },

@@ -250,7 +250,7 @@ static const struct id_table ic_id_table[] = {
.config_needed = false,
.has_rom_version = true,
.has_msft_ext = true,
- .fw_name = "rtl_bt/rtl8822cu_fw.bin",
+ .fw_name = "rtl_bt/rtl8822cu_fw",
.cfg_name = "rtl_bt/rtl8822cu_config",
.hw_info = "rtl8822cu" },

@@ -259,7 +259,7 @@ static const struct id_table ic_id_table[] = {
.config_needed = true,
.has_rom_version = true,
.has_msft_ext = true,
- .fw_name = "rtl_bt/rtl8822b_fw.bin",
+ .fw_name = "rtl_bt/rtl8822b_fw",
.cfg_name = "rtl_bt/rtl8822b_config",
.hw_info = "rtl8822bu" },

@@ -268,7 +268,7 @@ static const struct id_table ic_id_table[] = {
.config_needed = false,
.has_rom_version = true,
.has_msft_ext = true,
- .fw_name = "rtl_bt/rtl8852au_fw.bin",
+ .fw_name = "rtl_bt/rtl8852au_fw",
.cfg_name = "rtl_bt/rtl8852au_config",
.hw_info = "rtl8852au" },

@@ -277,7 +277,7 @@ static const struct id_table ic_id_table[] = {
.config_needed = true,
.has_rom_version = true,
.has_msft_ext = true,
- .fw_name = "rtl_bt/rtl8852bs_fw.bin",
+ .fw_name = "rtl_bt/rtl8852bs_fw",
.cfg_name = "rtl_bt/rtl8852bs_config",
.hw_info = "rtl8852bs" },

@@ -286,7 +286,7 @@ static const struct id_table ic_id_table[] = {
.config_needed = false,
.has_rom_version = true,
.has_msft_ext = true,
- .fw_name = "rtl_bt/rtl8852bu_fw.bin",
+ .fw_name = "rtl_bt/rtl8852bu_fw",
.cfg_name = "rtl_bt/rtl8852bu_config",
.hw_info = "rtl8852bu" },

@@ -295,7 +295,7 @@ static const struct id_table ic_id_table[] = {
.config_needed = false,
.has_rom_version = true,
.has_msft_ext = true,
- .fw_name = "rtl_bt/rtl8852cu_fw.bin",
+ .fw_name = "rtl_bt/rtl8852cu_fw",
.cfg_name = "rtl_bt/rtl8852cu_config",
.hw_info = "rtl8852cu" },

@@ -304,7 +304,7 @@ static const struct id_table ic_id_table[] = {
.config_needed = false,
.has_rom_version = true,
.has_msft_ext = false,
- .fw_name = "rtl_bt/rtl8851bu_fw.bin",
+ .fw_name = "rtl_bt/rtl8851bu_fw",
.cfg_name = "rtl_bt/rtl8851bu_config",
.hw_info = "rtl8851bu" },
};
@@ -1045,10 +1045,12 @@ struct btrtl_device_info *btrtl_initialize(struct hci_dev *hdev,
struct sk_buff *skb;
struct hci_rp_read_local_version *resp;
struct hci_command_hdr *cmd;
+ char fw_name[40];
char cfg_name[40];
u16 hci_rev, lmp_subver;
u8 hci_ver, lmp_ver, chip_type = 0;
int ret;
+ int fw_load_retry = 0;
u8 reg_val[2];

btrtl_dev = kzalloc(sizeof(*btrtl_dev), GFP_KERNEL);
@@ -1154,9 +1156,26 @@ struct btrtl_device_info *btrtl_initialize(struct hci_dev *hdev,
goto err_free;
}

- btrtl_dev->fw_len = rtl_load_file(hdev, btrtl_dev->ic_info->fw_name,
+fw_name_load:
+ if (btrtl_dev->ic_info->fw_name) {
+ if (lmp_subver == RTL_ROM_LMP_8852A && hci_rev == 0x000c &&
+ fw_load_retry == 0) {
+ fw_load_retry = 1;
+ snprintf(fw_name, sizeof(fw_name), "%s_v2.bin",
+ btrtl_dev->ic_info->fw_name);
+ } else {
+ fw_load_retry = 0;
+ snprintf(fw_name, sizeof(fw_name), "%s.bin",
+ btrtl_dev->ic_info->fw_name);
+ }
+ btrtl_dev->fw_len = rtl_load_file(hdev, fw_name,
&btrtl_dev->fw_data);
+ }
+
if (btrtl_dev->fw_len < 0) {
+ if (fw_load_retry == 1)
+ goto fw_name_load;
+
rtl_dev_err(hdev, "firmware file %s not found",
btrtl_dev->ic_info->fw_name);
ret = btrtl_dev->fw_len;
@@ -1491,4 +1510,5 @@ MODULE_FIRMWARE("rtl_bt/rtl8852bs_config.bin");
MODULE_FIRMWARE("rtl_bt/rtl8852bu_fw.bin");
MODULE_FIRMWARE("rtl_bt/rtl8852bu_config.bin");
MODULE_FIRMWARE("rtl_bt/rtl8852cu_fw.bin");
+MODULE_FIRMWARE("rtl_bt/rtl8852cu_fw_v2.bin");
MODULE_FIRMWARE("rtl_bt/rtl8852cu_config.bin");
--
2.34.1



2023-08-04 06:48:41

by bluez.test.bot

[permalink] [raw]
Subject: RE: Bluetooth: btrtl: Load FW v2 otherwise FW v1 for RTL8852C

This is automated email and please do not reply to this email!

Dear submitter,

Thank you for submitting the patches to the linux bluetooth mailing list.
This is a CI test results with your patch series:
PW Link:https://patchwork.kernel.org/project/bluetooth/list/?series=772909

---Test result---

Test Summary:
CheckPatch PASS 0.79 seconds
GitLint PASS 0.31 seconds
SubjectPrefix PASS 0.10 seconds
BuildKernel PASS 39.99 seconds
CheckAllWarning PASS 43.73 seconds
CheckSparse PASS 49.47 seconds
CheckSmatch PASS 133.81 seconds
BuildKernel32 PASS 38.42 seconds
TestRunnerSetup PASS 584.50 seconds
TestRunner_l2cap-tester PASS 27.97 seconds
TestRunner_iso-tester PASS 65.61 seconds
TestRunner_bnep-tester PASS 12.80 seconds
TestRunner_mgmt-tester PASS 240.32 seconds
TestRunner_rfcomm-tester PASS 19.06 seconds
TestRunner_sco-tester PASS 19.72 seconds
TestRunner_ioctl-tester PASS 21.54 seconds
TestRunner_mesh-tester PASS 15.89 seconds
TestRunner_smp-tester PASS 16.95 seconds
TestRunner_userchan-tester PASS 13.11 seconds
IncrementalBuild PASS 36.24 seconds



---
Regards,
Linux Bluetooth

2023-08-04 09:33:52

by Juerg Haefliger

[permalink] [raw]
Subject: Re: [PATCH] Bluetooth: btrtl: Load FW v2 otherwise FW v1 for RTL8852C

Thanks for this Max,

Comments inlined.


> From: Max Chou <[email protected]>
>
> In the commit of linux-firmware project, rtl8852cu_fw.bin is updated as
> FW v2 format[1]. Consider the case that if driver did not be updated for
> FW v2 supported[2], it can not use FW v2.
> By Canonical's suggestion, older driver should be able to load FW v1,
> so rtl8852cu_fw.bin will be revert to the previous commit as FW v1 and
> add rtl8852cu_fw_v2.bin as FW v2. This item will be started on
> linux-firmware project.
>
> In this commit, the driver prefers to load FW v2 if available. Fallback to
> FW v1 otherwise.
>
> To do on linux-firmware project.
> rtl_bt/rtl8852cu_fw.bin: FW v1 (stay at ver. 0xD7B8_FABF)
> rtl_bt/rtl8852cu_fw_v2.bin: FW v2 (to be maintained)
>
> [1]'9a24ce5e29b1 ("Bluetooth: btrtl: Firmware format v2 support")'
> [2]'55e7448533e7 ("rtl_bt: Update RTL8852C BT USB firmware
> to 0x040D_7225")'
>
> Suggested-by: Juerg Haefliger <[email protected]>
> Tested-by: Hilda Wu <[email protected]>
> Signed-off-by: Max Chou <[email protected]>
> ---
> drivers/bluetooth/btrtl.c | 68 +++++++++++++++++++++++++--------------
> 1 file changed, 44 insertions(+), 24 deletions(-)
>
> diff --git a/drivers/bluetooth/btrtl.c b/drivers/bluetooth/btrtl.c
> index ddae6524106d..8bfa86dd12f7 100644
> --- a/drivers/bluetooth/btrtl.c
> +++ b/drivers/bluetooth/btrtl.c
> @@ -104,7 +104,7 @@ static const struct id_table ic_id_table[] = {
> { IC_INFO(RTL_ROM_LMP_8723A, 0xb, 0x6, HCI_USB),
> .config_needed = false,
> .has_rom_version = false,
> - .fw_name = "rtl_bt/rtl8723a_fw.bin",
> + .fw_name = "rtl_bt/rtl8723a_fw",
> .cfg_name = NULL,
> .hw_info = "rtl8723au" },
>
> @@ -112,7 +112,7 @@ static const struct id_table ic_id_table[] = {
> { IC_INFO(RTL_ROM_LMP_8723B, 0xb, 0x6, HCI_UART),
> .config_needed = true,
> .has_rom_version = true,
> - .fw_name = "rtl_bt/rtl8723bs_fw.bin",
> + .fw_name = "rtl_bt/rtl8723bs_fw",
> .cfg_name = "rtl_bt/rtl8723bs_config",
> .hw_info = "rtl8723bs" },
>
> @@ -120,7 +120,7 @@ static const struct id_table ic_id_table[] = {
> { IC_INFO(RTL_ROM_LMP_8723B, 0xb, 0x6, HCI_USB),
> .config_needed = false,
> .has_rom_version = true,
> - .fw_name = "rtl_bt/rtl8723b_fw.bin",
> + .fw_name = "rtl_bt/rtl8723b_fw",
> .cfg_name = "rtl_bt/rtl8723b_config",
> .hw_info = "rtl8723bu" },
>
> @@ -132,7 +132,7 @@ static const struct id_table ic_id_table[] = {
> .hci_bus = HCI_UART,
> .config_needed = true,
> .has_rom_version = true,
> - .fw_name = "rtl_bt/rtl8723cs_cg_fw.bin",
> + .fw_name = "rtl_bt/rtl8723cs_cg_fw",
> .cfg_name = "rtl_bt/rtl8723cs_cg_config",
> .hw_info = "rtl8723cs-cg" },
>
> @@ -144,7 +144,7 @@ static const struct id_table ic_id_table[] = {
> .hci_bus = HCI_UART,
> .config_needed = true,
> .has_rom_version = true,
> - .fw_name = "rtl_bt/rtl8723cs_vf_fw.bin",
> + .fw_name = "rtl_bt/rtl8723cs_vf_fw",
> .cfg_name = "rtl_bt/rtl8723cs_vf_config",
> .hw_info = "rtl8723cs-vf" },
>
> @@ -156,7 +156,7 @@ static const struct id_table ic_id_table[] = {
> .hci_bus = HCI_UART,
> .config_needed = true,
> .has_rom_version = true,
> - .fw_name = "rtl_bt/rtl8723cs_xx_fw.bin",
> + .fw_name = "rtl_bt/rtl8723cs_xx_fw",
> .cfg_name = "rtl_bt/rtl8723cs_xx_config",
> .hw_info = "rtl8723cs" },
>
> @@ -164,7 +164,7 @@ static const struct id_table ic_id_table[] = {
> { IC_INFO(RTL_ROM_LMP_8723B, 0xd, 0x8, HCI_USB),
> .config_needed = true,
> .has_rom_version = true,
> - .fw_name = "rtl_bt/rtl8723d_fw.bin",
> + .fw_name = "rtl_bt/rtl8723d_fw",
> .cfg_name = "rtl_bt/rtl8723d_config",
> .hw_info = "rtl8723du" },
>
> @@ -172,7 +172,7 @@ static const struct id_table ic_id_table[] = {
> { IC_INFO(RTL_ROM_LMP_8723B, 0xd, 0x8, HCI_UART),
> .config_needed = true,
> .has_rom_version = true,
> - .fw_name = "rtl_bt/rtl8723ds_fw.bin",
> + .fw_name = "rtl_bt/rtl8723ds_fw",
> .cfg_name = "rtl_bt/rtl8723ds_config",
> .hw_info = "rtl8723ds" },
>
> @@ -180,7 +180,7 @@ static const struct id_table ic_id_table[] = {
> { IC_INFO(RTL_ROM_LMP_8821A, 0xa, 0x6, HCI_USB),
> .config_needed = false,
> .has_rom_version = true,
> - .fw_name = "rtl_bt/rtl8821a_fw.bin",
> + .fw_name = "rtl_bt/rtl8821a_fw",
> .cfg_name = "rtl_bt/rtl8821a_config",
> .hw_info = "rtl8821au" },
>
> @@ -189,7 +189,7 @@ static const struct id_table ic_id_table[] = {
> .config_needed = false,
> .has_rom_version = true,
> .has_msft_ext = true,
> - .fw_name = "rtl_bt/rtl8821c_fw.bin",
> + .fw_name = "rtl_bt/rtl8821c_fw",
> .cfg_name = "rtl_bt/rtl8821c_config",
> .hw_info = "rtl8821cu" },
>
> @@ -198,7 +198,7 @@ static const struct id_table ic_id_table[] = {
> .config_needed = true,
> .has_rom_version = true,
> .has_msft_ext = true,
> - .fw_name = "rtl_bt/rtl8821cs_fw.bin",
> + .fw_name = "rtl_bt/rtl8821cs_fw",
> .cfg_name = "rtl_bt/rtl8821cs_config",
> .hw_info = "rtl8821cs" },
>
> @@ -206,7 +206,7 @@ static const struct id_table ic_id_table[] = {
> { IC_INFO(RTL_ROM_LMP_8761A, 0xa, 0x6, HCI_USB),
> .config_needed = false,
> .has_rom_version = true,
> - .fw_name = "rtl_bt/rtl8761a_fw.bin",
> + .fw_name = "rtl_bt/rtl8761a_fw",
> .cfg_name = "rtl_bt/rtl8761a_config",
> .hw_info = "rtl8761au" },
>
> @@ -215,7 +215,7 @@ static const struct id_table ic_id_table[] = {
> .config_needed = false,
> .has_rom_version = true,
> .has_msft_ext = true,
> - .fw_name = "rtl_bt/rtl8761b_fw.bin",
> + .fw_name = "rtl_bt/rtl8761b_fw",
> .cfg_name = "rtl_bt/rtl8761b_config",
> .hw_info = "rtl8761btv" },
>
> @@ -223,7 +223,7 @@ static const struct id_table ic_id_table[] = {
> { IC_INFO(RTL_ROM_LMP_8761A, 0xb, 0xa, HCI_USB),
> .config_needed = false,
> .has_rom_version = true,
> - .fw_name = "rtl_bt/rtl8761bu_fw.bin",
> + .fw_name = "rtl_bt/rtl8761bu_fw",
> .cfg_name = "rtl_bt/rtl8761bu_config",
> .hw_info = "rtl8761bu" },
>
> @@ -232,7 +232,7 @@ static const struct id_table ic_id_table[] = {
> .config_needed = true,
> .has_rom_version = true,
> .has_msft_ext = true,
> - .fw_name = "rtl_bt/rtl8822cs_fw.bin",
> + .fw_name = "rtl_bt/rtl8822cs_fw",
> .cfg_name = "rtl_bt/rtl8822cs_config",
> .hw_info = "rtl8822cs" },
>
> @@ -241,7 +241,7 @@ static const struct id_table ic_id_table[] = {
> .config_needed = true,
> .has_rom_version = true,
> .has_msft_ext = true,
> - .fw_name = "rtl_bt/rtl8822cs_fw.bin",
> + .fw_name = "rtl_bt/rtl8822cs_fw",
> .cfg_name = "rtl_bt/rtl8822cs_config",
> .hw_info = "rtl8822cs" },
>
> @@ -250,7 +250,7 @@ static const struct id_table ic_id_table[] = {
> .config_needed = false,
> .has_rom_version = true,
> .has_msft_ext = true,
> - .fw_name = "rtl_bt/rtl8822cu_fw.bin",
> + .fw_name = "rtl_bt/rtl8822cu_fw",
> .cfg_name = "rtl_bt/rtl8822cu_config",
> .hw_info = "rtl8822cu" },
>
> @@ -259,7 +259,7 @@ static const struct id_table ic_id_table[] = {
> .config_needed = true,
> .has_rom_version = true,
> .has_msft_ext = true,
> - .fw_name = "rtl_bt/rtl8822b_fw.bin",
> + .fw_name = "rtl_bt/rtl8822b_fw",
> .cfg_name = "rtl_bt/rtl8822b_config",
> .hw_info = "rtl8822bu" },
>
> @@ -268,7 +268,7 @@ static const struct id_table ic_id_table[] = {
> .config_needed = false,
> .has_rom_version = true,
> .has_msft_ext = true,
> - .fw_name = "rtl_bt/rtl8852au_fw.bin",
> + .fw_name = "rtl_bt/rtl8852au_fw",
> .cfg_name = "rtl_bt/rtl8852au_config",
> .hw_info = "rtl8852au" },
>
> @@ -277,7 +277,7 @@ static const struct id_table ic_id_table[] = {
> .config_needed = true,
> .has_rom_version = true,
> .has_msft_ext = true,
> - .fw_name = "rtl_bt/rtl8852bs_fw.bin",
> + .fw_name = "rtl_bt/rtl8852bs_fw",
> .cfg_name = "rtl_bt/rtl8852bs_config",
> .hw_info = "rtl8852bs" },
>
> @@ -286,7 +286,7 @@ static const struct id_table ic_id_table[] = {
> .config_needed = false,
> .has_rom_version = true,
> .has_msft_ext = true,
> - .fw_name = "rtl_bt/rtl8852bu_fw.bin",
> + .fw_name = "rtl_bt/rtl8852bu_fw",
> .cfg_name = "rtl_bt/rtl8852bu_config",
> .hw_info = "rtl8852bu" },
>
> @@ -295,7 +295,7 @@ static const struct id_table ic_id_table[] = {
> .config_needed = false,
> .has_rom_version = true,
> .has_msft_ext = true,
> - .fw_name = "rtl_bt/rtl8852cu_fw.bin",
> + .fw_name = "rtl_bt/rtl8852cu_fw",
> .cfg_name = "rtl_bt/rtl8852cu_config",
> .hw_info = "rtl8852cu" },
>
> @@ -304,7 +304,7 @@ static const struct id_table ic_id_table[] = {
> .config_needed = false,
> .has_rom_version = true,
> .has_msft_ext = false,
> - .fw_name = "rtl_bt/rtl8851bu_fw.bin",
> + .fw_name = "rtl_bt/rtl8851bu_fw",
> .cfg_name = "rtl_bt/rtl8851bu_config",
> .hw_info = "rtl8851bu" },
> };
> @@ -1045,10 +1045,12 @@ struct btrtl_device_info *btrtl_initialize(struct hci_dev *hdev,
> struct sk_buff *skb;
> struct hci_rp_read_local_version *resp;
> struct hci_command_hdr *cmd;
> + char fw_name[40];
> char cfg_name[40];
> u16 hci_rev, lmp_subver;
> u8 hci_ver, lmp_ver, chip_type = 0;
> int ret;
> + int fw_load_retry = 0;
> u8 reg_val[2];
>
> btrtl_dev = kzalloc(sizeof(*btrtl_dev), GFP_KERNEL);
> @@ -1154,9 +1156,26 @@ struct btrtl_device_info *btrtl_initialize(struct hci_dev *hdev,
> goto err_free;
> }
>
> - btrtl_dev->fw_len = rtl_load_file(hdev, btrtl_dev->ic_info->fw_name,
> +fw_name_load:
> + if (btrtl_dev->ic_info->fw_name) {
> + if (lmp_subver == RTL_ROM_LMP_8852A && hci_rev == 0x000c &&
> + fw_load_retry == 0) {
> + fw_load_retry = 1;
> + snprintf(fw_name, sizeof(fw_name), "%s_v2.bin",
> + btrtl_dev->ic_info->fw_name);
> + } else {
> + fw_load_retry = 0;
> + snprintf(fw_name, sizeof(fw_name), "%s.bin",
> + btrtl_dev->ic_info->fw_name);
> + }
> + btrtl_dev->fw_len = rtl_load_file(hdev, fw_name,
> &btrtl_dev->fw_data);
> + }
> +
> if (btrtl_dev->fw_len < 0) {
> + if (fw_load_retry == 1)
> + goto fw_name_load;
> +
> rtl_dev_err(hdev, "firmware file %s not found",
> btrtl_dev->ic_info->fw_name);
> ret = btrtl_dev->fw_len;

This makes my head hurt ;-) Can we make this more readable, something like
(untested):

@@ -1154,8 +1155,26 @@ struct btrtl_device_info *btrtl_initialize(struct hci_dev *hdev,
goto err_free;
}

- btrtl_dev->fw_len = rtl_load_file(hdev, btrtl_dev->ic_info->fw_name,
- &btrtl_dev->fw_data);
+ if (!btrtl_dev->ic_info->fw_name) {
+ ret = -ENOMEM;
+ goto err_free;
+ }
+
+ btrtl_dev->fw_len = -EIO;
+ if (lmp_subver == RTL_ROM_LMP_8852A && hci_rev == 0x000c) {
+ snprintf(fw_name, sizeof(fw_name), "%s_v2.bin",
+ btrtl_dev->ic_info->fw_name);
+ btrtl_dev->fw_len = rtl_load_file(hdev, fw_name,
+ &btrtl_dev->fw_data);
+ }
+
+ if (btrtl_dev->fw_len < 0) {
+ snprintf(fw_name, sizeof(fw_name), "%s.bin",
+ btrtl_dev->ic_info->fw_name);
+ btrtl_dev->fw_len = rtl_load_file(hdev, fw_name,
+ &btrtl_dev->fw_data);
+ }
+
if (btrtl_dev->fw_len < 0) {
rtl_dev_err(hdev, "firmware file %s not found",
btrtl_dev->ic_info->fw_name);


...Juerg


> @@ -1491,4 +1510,5 @@ MODULE_FIRMWARE("rtl_bt/rtl8852bs_config.bin");
> MODULE_FIRMWARE("rtl_bt/rtl8852bu_fw.bin");
> MODULE_FIRMWARE("rtl_bt/rtl8852bu_config.bin");
> MODULE_FIRMWARE("rtl_bt/rtl8852cu_fw.bin");
> +MODULE_FIRMWARE("rtl_bt/rtl8852cu_fw_v2.bin");
> MODULE_FIRMWARE("rtl_bt/rtl8852cu_config.bin");


Attachments:
(No filename) (849.00 B)
OpenPGP digital signature

2023-08-04 09:39:50

by You-Sheng Yang

[permalink] [raw]
Subject: Re: [PATCH] Bluetooth: btrtl: Load FW v2 otherwise FW v1 for RTL8852C

By the way, what will you do to linux-firmware commit 53e48f93f
("rtl_bt: Add firmware and config files for RTL8851B")? It is also a
V2 firmware. If it's not subjected to the rename here, there will be
some V2 firmware blobs with V1 names, which may add some more troubles
in the future.

$ hexdump -C rtl_bt/rtl8851bu_fw.bin | head -n1
00000000 52 54 42 54 43 6f 72 65 41 b0 34 01 44 ba 01 00 |RTBTCoreA.4.D...|

On Fri, Aug 4, 2023 at 1:54 PM <[email protected]> wrote:
>
> From: Max Chou <[email protected]>
>
> In the commit of linux-firmware project, rtl8852cu_fw.bin is updated as
> FW v2 format[1]. Consider the case that if driver did not be updated for
> FW v2 supported[2], it can not use FW v2.
> By Canonical's suggestion, older driver should be able to load FW v1,
> so rtl8852cu_fw.bin will be revert to the previous commit as FW v1 and
> add rtl8852cu_fw_v2.bin as FW v2. This item will be started on
> linux-firmware project.
>
> In this commit, the driver prefers to load FW v2 if available. Fallback to
> FW v1 otherwise.
>
> To do on linux-firmware project.
> rtl_bt/rtl8852cu_fw.bin: FW v1 (stay at ver. 0xD7B8_FABF)
> rtl_bt/rtl8852cu_fw_v2.bin: FW v2 (to be maintained)
>
> [1]'9a24ce5e29b1 ("Bluetooth: btrtl: Firmware format v2 support")'
> [2]'55e7448533e7 ("rtl_bt: Update RTL8852C BT USB firmware
> to 0x040D_7225")'
>
> Suggested-by: Juerg Haefliger <[email protected]>
> Tested-by: Hilda Wu <[email protected]>
> Signed-off-by: Max Chou <[email protected]>
> ---
> drivers/bluetooth/btrtl.c | 68 +++++++++++++++++++++++++--------------
> 1 file changed, 44 insertions(+), 24 deletions(-)
>
> diff --git a/drivers/bluetooth/btrtl.c b/drivers/bluetooth/btrtl.c
> index ddae6524106d..8bfa86dd12f7 100644
> --- a/drivers/bluetooth/btrtl.c
> +++ b/drivers/bluetooth/btrtl.c
> @@ -104,7 +104,7 @@ static const struct id_table ic_id_table[] = {
> { IC_INFO(RTL_ROM_LMP_8723A, 0xb, 0x6, HCI_USB),
> .config_needed = false,
> .has_rom_version = false,
> - .fw_name = "rtl_bt/rtl8723a_fw.bin",
> + .fw_name = "rtl_bt/rtl8723a_fw",
> .cfg_name = NULL,
> .hw_info = "rtl8723au" },
>
> @@ -112,7 +112,7 @@ static const struct id_table ic_id_table[] = {
> { IC_INFO(RTL_ROM_LMP_8723B, 0xb, 0x6, HCI_UART),
> .config_needed = true,
> .has_rom_version = true,
> - .fw_name = "rtl_bt/rtl8723bs_fw.bin",
> + .fw_name = "rtl_bt/rtl8723bs_fw",
> .cfg_name = "rtl_bt/rtl8723bs_config",
> .hw_info = "rtl8723bs" },
>
> @@ -120,7 +120,7 @@ static const struct id_table ic_id_table[] = {
> { IC_INFO(RTL_ROM_LMP_8723B, 0xb, 0x6, HCI_USB),
> .config_needed = false,
> .has_rom_version = true,
> - .fw_name = "rtl_bt/rtl8723b_fw.bin",
> + .fw_name = "rtl_bt/rtl8723b_fw",
> .cfg_name = "rtl_bt/rtl8723b_config",
> .hw_info = "rtl8723bu" },
>
> @@ -132,7 +132,7 @@ static const struct id_table ic_id_table[] = {
> .hci_bus = HCI_UART,
> .config_needed = true,
> .has_rom_version = true,
> - .fw_name = "rtl_bt/rtl8723cs_cg_fw.bin",
> + .fw_name = "rtl_bt/rtl8723cs_cg_fw",
> .cfg_name = "rtl_bt/rtl8723cs_cg_config",
> .hw_info = "rtl8723cs-cg" },
>
> @@ -144,7 +144,7 @@ static const struct id_table ic_id_table[] = {
> .hci_bus = HCI_UART,
> .config_needed = true,
> .has_rom_version = true,
> - .fw_name = "rtl_bt/rtl8723cs_vf_fw.bin",
> + .fw_name = "rtl_bt/rtl8723cs_vf_fw",
> .cfg_name = "rtl_bt/rtl8723cs_vf_config",
> .hw_info = "rtl8723cs-vf" },
>
> @@ -156,7 +156,7 @@ static const struct id_table ic_id_table[] = {
> .hci_bus = HCI_UART,
> .config_needed = true,
> .has_rom_version = true,
> - .fw_name = "rtl_bt/rtl8723cs_xx_fw.bin",
> + .fw_name = "rtl_bt/rtl8723cs_xx_fw",
> .cfg_name = "rtl_bt/rtl8723cs_xx_config",
> .hw_info = "rtl8723cs" },
>
> @@ -164,7 +164,7 @@ static const struct id_table ic_id_table[] = {
> { IC_INFO(RTL_ROM_LMP_8723B, 0xd, 0x8, HCI_USB),
> .config_needed = true,
> .has_rom_version = true,
> - .fw_name = "rtl_bt/rtl8723d_fw.bin",
> + .fw_name = "rtl_bt/rtl8723d_fw",
> .cfg_name = "rtl_bt/rtl8723d_config",
> .hw_info = "rtl8723du" },
>
> @@ -172,7 +172,7 @@ static const struct id_table ic_id_table[] = {
> { IC_INFO(RTL_ROM_LMP_8723B, 0xd, 0x8, HCI_UART),
> .config_needed = true,
> .has_rom_version = true,
> - .fw_name = "rtl_bt/rtl8723ds_fw.bin",
> + .fw_name = "rtl_bt/rtl8723ds_fw",
> .cfg_name = "rtl_bt/rtl8723ds_config",
> .hw_info = "rtl8723ds" },
>
> @@ -180,7 +180,7 @@ static const struct id_table ic_id_table[] = {
> { IC_INFO(RTL_ROM_LMP_8821A, 0xa, 0x6, HCI_USB),
> .config_needed = false,
> .has_rom_version = true,
> - .fw_name = "rtl_bt/rtl8821a_fw.bin",
> + .fw_name = "rtl_bt/rtl8821a_fw",
> .cfg_name = "rtl_bt/rtl8821a_config",
> .hw_info = "rtl8821au" },
>
> @@ -189,7 +189,7 @@ static const struct id_table ic_id_table[] = {
> .config_needed = false,
> .has_rom_version = true,
> .has_msft_ext = true,
> - .fw_name = "rtl_bt/rtl8821c_fw.bin",
> + .fw_name = "rtl_bt/rtl8821c_fw",
> .cfg_name = "rtl_bt/rtl8821c_config",
> .hw_info = "rtl8821cu" },
>
> @@ -198,7 +198,7 @@ static const struct id_table ic_id_table[] = {
> .config_needed = true,
> .has_rom_version = true,
> .has_msft_ext = true,
> - .fw_name = "rtl_bt/rtl8821cs_fw.bin",
> + .fw_name = "rtl_bt/rtl8821cs_fw",
> .cfg_name = "rtl_bt/rtl8821cs_config",
> .hw_info = "rtl8821cs" },
>
> @@ -206,7 +206,7 @@ static const struct id_table ic_id_table[] = {
> { IC_INFO(RTL_ROM_LMP_8761A, 0xa, 0x6, HCI_USB),
> .config_needed = false,
> .has_rom_version = true,
> - .fw_name = "rtl_bt/rtl8761a_fw.bin",
> + .fw_name = "rtl_bt/rtl8761a_fw",
> .cfg_name = "rtl_bt/rtl8761a_config",
> .hw_info = "rtl8761au" },
>
> @@ -215,7 +215,7 @@ static const struct id_table ic_id_table[] = {
> .config_needed = false,
> .has_rom_version = true,
> .has_msft_ext = true,
> - .fw_name = "rtl_bt/rtl8761b_fw.bin",
> + .fw_name = "rtl_bt/rtl8761b_fw",
> .cfg_name = "rtl_bt/rtl8761b_config",
> .hw_info = "rtl8761btv" },
>
> @@ -223,7 +223,7 @@ static const struct id_table ic_id_table[] = {
> { IC_INFO(RTL_ROM_LMP_8761A, 0xb, 0xa, HCI_USB),
> .config_needed = false,
> .has_rom_version = true,
> - .fw_name = "rtl_bt/rtl8761bu_fw.bin",
> + .fw_name = "rtl_bt/rtl8761bu_fw",
> .cfg_name = "rtl_bt/rtl8761bu_config",
> .hw_info = "rtl8761bu" },
>
> @@ -232,7 +232,7 @@ static const struct id_table ic_id_table[] = {
> .config_needed = true,
> .has_rom_version = true,
> .has_msft_ext = true,
> - .fw_name = "rtl_bt/rtl8822cs_fw.bin",
> + .fw_name = "rtl_bt/rtl8822cs_fw",
> .cfg_name = "rtl_bt/rtl8822cs_config",
> .hw_info = "rtl8822cs" },
>
> @@ -241,7 +241,7 @@ static const struct id_table ic_id_table[] = {
> .config_needed = true,
> .has_rom_version = true,
> .has_msft_ext = true,
> - .fw_name = "rtl_bt/rtl8822cs_fw.bin",
> + .fw_name = "rtl_bt/rtl8822cs_fw",
> .cfg_name = "rtl_bt/rtl8822cs_config",
> .hw_info = "rtl8822cs" },
>
> @@ -250,7 +250,7 @@ static const struct id_table ic_id_table[] = {
> .config_needed = false,
> .has_rom_version = true,
> .has_msft_ext = true,
> - .fw_name = "rtl_bt/rtl8822cu_fw.bin",
> + .fw_name = "rtl_bt/rtl8822cu_fw",
> .cfg_name = "rtl_bt/rtl8822cu_config",
> .hw_info = "rtl8822cu" },
>
> @@ -259,7 +259,7 @@ static const struct id_table ic_id_table[] = {
> .config_needed = true,
> .has_rom_version = true,
> .has_msft_ext = true,
> - .fw_name = "rtl_bt/rtl8822b_fw.bin",
> + .fw_name = "rtl_bt/rtl8822b_fw",
> .cfg_name = "rtl_bt/rtl8822b_config",
> .hw_info = "rtl8822bu" },
>
> @@ -268,7 +268,7 @@ static const struct id_table ic_id_table[] = {
> .config_needed = false,
> .has_rom_version = true,
> .has_msft_ext = true,
> - .fw_name = "rtl_bt/rtl8852au_fw.bin",
> + .fw_name = "rtl_bt/rtl8852au_fw",
> .cfg_name = "rtl_bt/rtl8852au_config",
> .hw_info = "rtl8852au" },
>
> @@ -277,7 +277,7 @@ static const struct id_table ic_id_table[] = {
> .config_needed = true,
> .has_rom_version = true,
> .has_msft_ext = true,
> - .fw_name = "rtl_bt/rtl8852bs_fw.bin",
> + .fw_name = "rtl_bt/rtl8852bs_fw",
> .cfg_name = "rtl_bt/rtl8852bs_config",
> .hw_info = "rtl8852bs" },
>
> @@ -286,7 +286,7 @@ static const struct id_table ic_id_table[] = {
> .config_needed = false,
> .has_rom_version = true,
> .has_msft_ext = true,
> - .fw_name = "rtl_bt/rtl8852bu_fw.bin",
> + .fw_name = "rtl_bt/rtl8852bu_fw",
> .cfg_name = "rtl_bt/rtl8852bu_config",
> .hw_info = "rtl8852bu" },
>
> @@ -295,7 +295,7 @@ static const struct id_table ic_id_table[] = {
> .config_needed = false,
> .has_rom_version = true,
> .has_msft_ext = true,
> - .fw_name = "rtl_bt/rtl8852cu_fw.bin",
> + .fw_name = "rtl_bt/rtl8852cu_fw",
> .cfg_name = "rtl_bt/rtl8852cu_config",
> .hw_info = "rtl8852cu" },
>
> @@ -304,7 +304,7 @@ static const struct id_table ic_id_table[] = {
> .config_needed = false,
> .has_rom_version = true,
> .has_msft_ext = false,
> - .fw_name = "rtl_bt/rtl8851bu_fw.bin",
> + .fw_name = "rtl_bt/rtl8851bu_fw",
> .cfg_name = "rtl_bt/rtl8851bu_config",
> .hw_info = "rtl8851bu" },
> };
> @@ -1045,10 +1045,12 @@ struct btrtl_device_info *btrtl_initialize(struct hci_dev *hdev,
> struct sk_buff *skb;
> struct hci_rp_read_local_version *resp;
> struct hci_command_hdr *cmd;
> + char fw_name[40];
> char cfg_name[40];
> u16 hci_rev, lmp_subver;
> u8 hci_ver, lmp_ver, chip_type = 0;
> int ret;
> + int fw_load_retry = 0;
> u8 reg_val[2];
>
> btrtl_dev = kzalloc(sizeof(*btrtl_dev), GFP_KERNEL);
> @@ -1154,9 +1156,26 @@ struct btrtl_device_info *btrtl_initialize(struct hci_dev *hdev,
> goto err_free;
> }
>
> - btrtl_dev->fw_len = rtl_load_file(hdev, btrtl_dev->ic_info->fw_name,
> +fw_name_load:
> + if (btrtl_dev->ic_info->fw_name) {
> + if (lmp_subver == RTL_ROM_LMP_8852A && hci_rev == 0x000c &&
> + fw_load_retry == 0) {
> + fw_load_retry = 1;
> + snprintf(fw_name, sizeof(fw_name), "%s_v2.bin",
> + btrtl_dev->ic_info->fw_name);
> + } else {
> + fw_load_retry = 0;
> + snprintf(fw_name, sizeof(fw_name), "%s.bin",
> + btrtl_dev->ic_info->fw_name);
> + }
> + btrtl_dev->fw_len = rtl_load_file(hdev, fw_name,
> &btrtl_dev->fw_data);
> + }
> +
> if (btrtl_dev->fw_len < 0) {
> + if (fw_load_retry == 1)
> + goto fw_name_load;
> +
> rtl_dev_err(hdev, "firmware file %s not found",
> btrtl_dev->ic_info->fw_name);
> ret = btrtl_dev->fw_len;
> @@ -1491,4 +1510,5 @@ MODULE_FIRMWARE("rtl_bt/rtl8852bs_config.bin");
> MODULE_FIRMWARE("rtl_bt/rtl8852bu_fw.bin");
> MODULE_FIRMWARE("rtl_bt/rtl8852bu_config.bin");
> MODULE_FIRMWARE("rtl_bt/rtl8852cu_fw.bin");
> +MODULE_FIRMWARE("rtl_bt/rtl8852cu_fw_v2.bin");
> MODULE_FIRMWARE("rtl_bt/rtl8852cu_config.bin");
> --
> 2.34.1
>


--
Regards,
You-Sheng Yang

2023-08-04 09:47:20

by Max Chou

[permalink] [raw]
Subject: RE: [PATCH] Bluetooth: btrtl: Load FW v2 otherwise FW v1 for RTL8852C

Hi! You-Sheng,
The first commit for the FW file of RTL8851B is FW v2 format, so I believe it's no worry about that.

BRs,
Max


> -----Original Message-----
> From: You-Sheng Yang <[email protected]>
> Sent: Friday, August 4, 2023 5:14 PM
> To: Max Chou <[email protected]>
> Cc: [email protected]; [email protected]; [email protected];
> [email protected]; [email protected];
> [email protected]; Hilda Wu <[email protected]>; Karen Hsu
> <[email protected]>; KidmanLee <[email protected]>;
> [email protected]; [email protected]
> Subject: Re: [PATCH] Bluetooth: btrtl: Load FW v2 otherwise FW v1 for
> RTL8852C
>
>
> External mail.
>
>
>
> By the way, what will you do to linux-firmware commit 53e48f93f
> ("rtl_bt: Add firmware and config files for RTL8851B")? It is also a
> V2 firmware. If it's not subjected to the rename here, there will be some V2
> firmware blobs with V1 names, which may add some more troubles in the
> future.
>
> $ hexdump -C rtl_bt/rtl8851bu_fw.bin | head -n1
> 00000000 52 54 42 54 43 6f 72 65 41 b0 34 01 44 ba 01 00
> |RTBTCoreA.4.D...|
>
> On Fri, Aug 4, 2023 at 1:54 PM <[email protected]> wrote:
> >
> > From: Max Chou <[email protected]>
> >
> > In the commit of linux-firmware project, rtl8852cu_fw.bin is updated
> > as FW v2 format[1]. Consider the case that if driver did not be
> > updated for FW v2 supported[2], it can not use FW v2.
> > By Canonical's suggestion, older driver should be able to load FW v1,
> > so rtl8852cu_fw.bin will be revert to the previous commit as FW v1 and
> > add rtl8852cu_fw_v2.bin as FW v2. This item will be started on
> > linux-firmware project.
> >
> > In this commit, the driver prefers to load FW v2 if available.
> > Fallback to FW v1 otherwise.
> >
> > To do on linux-firmware project.
> > rtl_bt/rtl8852cu_fw.bin: FW v1 (stay at ver. 0xD7B8_FABF)
> > rtl_bt/rtl8852cu_fw_v2.bin: FW v2 (to be maintained)
> >
> > [1]'9a24ce5e29b1 ("Bluetooth: btrtl: Firmware format v2 support")'
> > [2]'55e7448533e7 ("rtl_bt: Update RTL8852C BT USB firmware
> > to 0x040D_7225")'
> >
> > Suggested-by: Juerg Haefliger <[email protected]>
> > Tested-by: Hilda Wu <[email protected]>
> > Signed-off-by: Max Chou <[email protected]>
> > ---
> > drivers/bluetooth/btrtl.c | 68
> > +++++++++++++++++++++++++--------------
> > 1 file changed, 44 insertions(+), 24 deletions(-)
> >
> > diff --git a/drivers/bluetooth/btrtl.c b/drivers/bluetooth/btrtl.c
> > index ddae6524106d..8bfa86dd12f7 100644
> > --- a/drivers/bluetooth/btrtl.c
> > +++ b/drivers/bluetooth/btrtl.c
> > @@ -104,7 +104,7 @@ static const struct id_table ic_id_table[] = {
> > { IC_INFO(RTL_ROM_LMP_8723A, 0xb, 0x6, HCI_USB),
> > .config_needed = false,
> > .has_rom_version = false,
> > - .fw_name = "rtl_bt/rtl8723a_fw.bin",
> > + .fw_name = "rtl_bt/rtl8723a_fw",
> > .cfg_name = NULL,
> > .hw_info = "rtl8723au" },
> >
> > @@ -112,7 +112,7 @@ static const struct id_table ic_id_table[] = {
> > { IC_INFO(RTL_ROM_LMP_8723B, 0xb, 0x6, HCI_UART),
> > .config_needed = true,
> > .has_rom_version = true,
> > - .fw_name = "rtl_bt/rtl8723bs_fw.bin",
> > + .fw_name = "rtl_bt/rtl8723bs_fw",
> > .cfg_name = "rtl_bt/rtl8723bs_config",
> > .hw_info = "rtl8723bs" },
> >
> > @@ -120,7 +120,7 @@ static const struct id_table ic_id_table[] = {
> > { IC_INFO(RTL_ROM_LMP_8723B, 0xb, 0x6, HCI_USB),
> > .config_needed = false,
> > .has_rom_version = true,
> > - .fw_name = "rtl_bt/rtl8723b_fw.bin",
> > + .fw_name = "rtl_bt/rtl8723b_fw",
> > .cfg_name = "rtl_bt/rtl8723b_config",
> > .hw_info = "rtl8723bu" },
> >
> > @@ -132,7 +132,7 @@ static const struct id_table ic_id_table[] = {
> > .hci_bus = HCI_UART,
> > .config_needed = true,
> > .has_rom_version = true,
> > - .fw_name = "rtl_bt/rtl8723cs_cg_fw.bin",
> > + .fw_name = "rtl_bt/rtl8723cs_cg_fw",
> > .cfg_name = "rtl_bt/rtl8723cs_cg_config",
> > .hw_info = "rtl8723cs-cg" },
> >
> > @@ -144,7 +144,7 @@ static const struct id_table ic_id_table[] = {
> > .hci_bus = HCI_UART,
> > .config_needed = true,
> > .has_rom_version = true,
> > - .fw_name = "rtl_bt/rtl8723cs_vf_fw.bin",
> > + .fw_name = "rtl_bt/rtl8723cs_vf_fw",
> > .cfg_name = "rtl_bt/rtl8723cs_vf_config",
> > .hw_info = "rtl8723cs-vf" },
> >
> > @@ -156,7 +156,7 @@ static const struct id_table ic_id_table[] = {
> > .hci_bus = HCI_UART,
> > .config_needed = true,
> > .has_rom_version = true,
> > - .fw_name = "rtl_bt/rtl8723cs_xx_fw.bin",
> > + .fw_name = "rtl_bt/rtl8723cs_xx_fw",
> > .cfg_name = "rtl_bt/rtl8723cs_xx_config",
> > .hw_info = "rtl8723cs" },
> >
> > @@ -164,7 +164,7 @@ static const struct id_table ic_id_table[] = {
> > { IC_INFO(RTL_ROM_LMP_8723B, 0xd, 0x8, HCI_USB),
> > .config_needed = true,
> > .has_rom_version = true,
> > - .fw_name = "rtl_bt/rtl8723d_fw.bin",
> > + .fw_name = "rtl_bt/rtl8723d_fw",
> > .cfg_name = "rtl_bt/rtl8723d_config",
> > .hw_info = "rtl8723du" },
> >
> > @@ -172,7 +172,7 @@ static const struct id_table ic_id_table[] = {
> > { IC_INFO(RTL_ROM_LMP_8723B, 0xd, 0x8, HCI_UART),
> > .config_needed = true,
> > .has_rom_version = true,
> > - .fw_name = "rtl_bt/rtl8723ds_fw.bin",
> > + .fw_name = "rtl_bt/rtl8723ds_fw",
> > .cfg_name = "rtl_bt/rtl8723ds_config",
> > .hw_info = "rtl8723ds" },
> >
> > @@ -180,7 +180,7 @@ static const struct id_table ic_id_table[] = {
> > { IC_INFO(RTL_ROM_LMP_8821A, 0xa, 0x6, HCI_USB),
> > .config_needed = false,
> > .has_rom_version = true,
> > - .fw_name = "rtl_bt/rtl8821a_fw.bin",
> > + .fw_name = "rtl_bt/rtl8821a_fw",
> > .cfg_name = "rtl_bt/rtl8821a_config",
> > .hw_info = "rtl8821au" },
> >
> > @@ -189,7 +189,7 @@ static const struct id_table ic_id_table[] = {
> > .config_needed = false,
> > .has_rom_version = true,
> > .has_msft_ext = true,
> > - .fw_name = "rtl_bt/rtl8821c_fw.bin",
> > + .fw_name = "rtl_bt/rtl8821c_fw",
> > .cfg_name = "rtl_bt/rtl8821c_config",
> > .hw_info = "rtl8821cu" },
> >
> > @@ -198,7 +198,7 @@ static const struct id_table ic_id_table[] = {
> > .config_needed = true,
> > .has_rom_version = true,
> > .has_msft_ext = true,
> > - .fw_name = "rtl_bt/rtl8821cs_fw.bin",
> > + .fw_name = "rtl_bt/rtl8821cs_fw",
> > .cfg_name = "rtl_bt/rtl8821cs_config",
> > .hw_info = "rtl8821cs" },
> >
> > @@ -206,7 +206,7 @@ static const struct id_table ic_id_table[] = {
> > { IC_INFO(RTL_ROM_LMP_8761A, 0xa, 0x6, HCI_USB),
> > .config_needed = false,
> > .has_rom_version = true,
> > - .fw_name = "rtl_bt/rtl8761a_fw.bin",
> > + .fw_name = "rtl_bt/rtl8761a_fw",
> > .cfg_name = "rtl_bt/rtl8761a_config",
> > .hw_info = "rtl8761au" },
> >
> > @@ -215,7 +215,7 @@ static const struct id_table ic_id_table[] = {
> > .config_needed = false,
> > .has_rom_version = true,
> > .has_msft_ext = true,
> > - .fw_name = "rtl_bt/rtl8761b_fw.bin",
> > + .fw_name = "rtl_bt/rtl8761b_fw",
> > .cfg_name = "rtl_bt/rtl8761b_config",
> > .hw_info = "rtl8761btv" },
> >
> > @@ -223,7 +223,7 @@ static const struct id_table ic_id_table[] = {
> > { IC_INFO(RTL_ROM_LMP_8761A, 0xb, 0xa, HCI_USB),
> > .config_needed = false,
> > .has_rom_version = true,
> > - .fw_name = "rtl_bt/rtl8761bu_fw.bin",
> > + .fw_name = "rtl_bt/rtl8761bu_fw",
> > .cfg_name = "rtl_bt/rtl8761bu_config",
> > .hw_info = "rtl8761bu" },
> >
> > @@ -232,7 +232,7 @@ static const struct id_table ic_id_table[] = {
> > .config_needed = true,
> > .has_rom_version = true,
> > .has_msft_ext = true,
> > - .fw_name = "rtl_bt/rtl8822cs_fw.bin",
> > + .fw_name = "rtl_bt/rtl8822cs_fw",
> > .cfg_name = "rtl_bt/rtl8822cs_config",
> > .hw_info = "rtl8822cs" },
> >
> > @@ -241,7 +241,7 @@ static const struct id_table ic_id_table[] = {
> > .config_needed = true,
> > .has_rom_version = true,
> > .has_msft_ext = true,
> > - .fw_name = "rtl_bt/rtl8822cs_fw.bin",
> > + .fw_name = "rtl_bt/rtl8822cs_fw",
> > .cfg_name = "rtl_bt/rtl8822cs_config",
> > .hw_info = "rtl8822cs" },
> >
> > @@ -250,7 +250,7 @@ static const struct id_table ic_id_table[] = {
> > .config_needed = false,
> > .has_rom_version = true,
> > .has_msft_ext = true,
> > - .fw_name = "rtl_bt/rtl8822cu_fw.bin",
> > + .fw_name = "rtl_bt/rtl8822cu_fw",
> > .cfg_name = "rtl_bt/rtl8822cu_config",
> > .hw_info = "rtl8822cu" },
> >
> > @@ -259,7 +259,7 @@ static const struct id_table ic_id_table[] = {
> > .config_needed = true,
> > .has_rom_version = true,
> > .has_msft_ext = true,
> > - .fw_name = "rtl_bt/rtl8822b_fw.bin",
> > + .fw_name = "rtl_bt/rtl8822b_fw",
> > .cfg_name = "rtl_bt/rtl8822b_config",
> > .hw_info = "rtl8822bu" },
> >
> > @@ -268,7 +268,7 @@ static const struct id_table ic_id_table[] = {
> > .config_needed = false,
> > .has_rom_version = true,
> > .has_msft_ext = true,
> > - .fw_name = "rtl_bt/rtl8852au_fw.bin",
> > + .fw_name = "rtl_bt/rtl8852au_fw",
> > .cfg_name = "rtl_bt/rtl8852au_config",
> > .hw_info = "rtl8852au" },
> >
> > @@ -277,7 +277,7 @@ static const struct id_table ic_id_table[] = {
> > .config_needed = true,
> > .has_rom_version = true,
> > .has_msft_ext = true,
> > - .fw_name = "rtl_bt/rtl8852bs_fw.bin",
> > + .fw_name = "rtl_bt/rtl8852bs_fw",
> > .cfg_name = "rtl_bt/rtl8852bs_config",
> > .hw_info = "rtl8852bs" },
> >
> > @@ -286,7 +286,7 @@ static const struct id_table ic_id_table[] = {
> > .config_needed = false,
> > .has_rom_version = true,
> > .has_msft_ext = true,
> > - .fw_name = "rtl_bt/rtl8852bu_fw.bin",
> > + .fw_name = "rtl_bt/rtl8852bu_fw",
> > .cfg_name = "rtl_bt/rtl8852bu_config",
> > .hw_info = "rtl8852bu" },
> >
> > @@ -295,7 +295,7 @@ static const struct id_table ic_id_table[] = {
> > .config_needed = false,
> > .has_rom_version = true,
> > .has_msft_ext = true,
> > - .fw_name = "rtl_bt/rtl8852cu_fw.bin",
> > + .fw_name = "rtl_bt/rtl8852cu_fw",
> > .cfg_name = "rtl_bt/rtl8852cu_config",
> > .hw_info = "rtl8852cu" },
> >
> > @@ -304,7 +304,7 @@ static const struct id_table ic_id_table[] = {
> > .config_needed = false,
> > .has_rom_version = true,
> > .has_msft_ext = false,
> > - .fw_name = "rtl_bt/rtl8851bu_fw.bin",
> > + .fw_name = "rtl_bt/rtl8851bu_fw",
> > .cfg_name = "rtl_bt/rtl8851bu_config",
> > .hw_info = "rtl8851bu" },
> > };
> > @@ -1045,10 +1045,12 @@ struct btrtl_device_info *btrtl_initialize(struct
> hci_dev *hdev,
> > struct sk_buff *skb;
> > struct hci_rp_read_local_version *resp;
> > struct hci_command_hdr *cmd;
> > + char fw_name[40];
> > char cfg_name[40];
> > u16 hci_rev, lmp_subver;
> > u8 hci_ver, lmp_ver, chip_type = 0;
> > int ret;
> > + int fw_load_retry = 0;
> > u8 reg_val[2];
> >
> > btrtl_dev = kzalloc(sizeof(*btrtl_dev), GFP_KERNEL); @@
> > -1154,9 +1156,26 @@ struct btrtl_device_info *btrtl_initialize(struct hci_dev
> *hdev,
> > goto err_free;
> > }
> >
> > - btrtl_dev->fw_len = rtl_load_file(hdev,
> btrtl_dev->ic_info->fw_name,
> > +fw_name_load:
> > + if (btrtl_dev->ic_info->fw_name) {
> > + if (lmp_subver == RTL_ROM_LMP_8852A && hci_rev ==
> 0x000c &&
> > + fw_load_retry == 0) {
> > + fw_load_retry = 1;
> > + snprintf(fw_name, sizeof(fw_name),
> "%s_v2.bin",
> > + btrtl_dev->ic_info->fw_name);
> > + } else {
> > + fw_load_retry = 0;
> > + snprintf(fw_name, sizeof(fw_name), "%s.bin",
> > + btrtl_dev->ic_info->fw_name);
> > + }
> > + btrtl_dev->fw_len = rtl_load_file(hdev, fw_name,
> > &btrtl_dev->fw_data);
> > + }
> > +
> > if (btrtl_dev->fw_len < 0) {
> > + if (fw_load_retry == 1)
> > + goto fw_name_load;
> > +
> > rtl_dev_err(hdev, "firmware file %s not found",
> > btrtl_dev->ic_info->fw_name);
> > ret = btrtl_dev->fw_len; @@ -1491,4 +1510,5 @@
> > MODULE_FIRMWARE("rtl_bt/rtl8852bs_config.bin");
> > MODULE_FIRMWARE("rtl_bt/rtl8852bu_fw.bin");
> > MODULE_FIRMWARE("rtl_bt/rtl8852bu_config.bin");
> > MODULE_FIRMWARE("rtl_bt/rtl8852cu_fw.bin");
> > +MODULE_FIRMWARE("rtl_bt/rtl8852cu_fw_v2.bin");
> > MODULE_FIRMWARE("rtl_bt/rtl8852cu_config.bin");
> > --
> > 2.34.1
> >
>
>
> --
> Regards,
> You-Sheng Yang
>
> ------Please consider the environment before printing this e-mail.

2023-08-04 10:40:50

by Max Chou

[permalink] [raw]
Subject: RE: [PATCH] Bluetooth: btrtl: Load FW v2 otherwise FW v1 for RTL8852C

Dear All,
Please check my comment inline. Thanks.

BRs,
Max


> -----Original Message-----
> From: Linux regression tracking (Thorsten Leemhuis)
> <[email protected]>
> Sent: Friday, August 4, 2023 4:55 PM
> To: Max Chou <[email protected]>; [email protected]
> Cc: [email protected]; [email protected];
> [email protected]; [email protected];
> [email protected]; Hilda Wu <[email protected]>; Karen Hsu
> <[email protected]>; KidmanLee <[email protected]>;
> [email protected]; [email protected];
> [email protected]; Josh Boyer <[email protected]>; David S. Miller
> <[email protected]>; Eric Dumazet <[email protected]>; Jakub
> Kicinski <[email protected]>; Paolo Abeni <[email protected]>; Linux kernel
> regressions list <[email protected]>
> Subject: Re: [PATCH] Bluetooth: btrtl: Load FW v2 otherwise FW v1 for
> RTL8852C
>
>
> External mail.
>
>
>
> [CCing the linux-firmware maintainer, the network maintainers, and the
> regressions lists]
>
> On 04.08.23 07:54, [email protected] wrote:
> > From: Max Chou <[email protected]>
> >
> > In the commit of linux-firmware project, rtl8852cu_fw.bin is updated
> > as FW v2 format[1]. Consider the case that if driver did not be updated fo>
> FW v2 supported[2], it can not use FW v2.
> >
> > By Canonical's suggestion, older driver should be able to load FW v1,
>
> Well, that's not only Canonical suggestion, that is how things are supposed to
> be handled in general. See
> Documentation/driver-api/firmware/firmware-usage-guidelines.rst (alt:
> https://docs.kernel.org/driver-api/firmware/firmware-usage-guidelines.html
> ) for details.
>
> We had a similar situation in March already with a Wifi driver:
> https://git.kernel.org/pub/scm/linux/kernel/git/firmware/linux-firmware.git/c
> ommit/?id=416a66cae796939d7d096988c72c641656c81c5a
>
> Makes me wonder: is there a problem here we should try harder to avoid?
> Especially as the problem in this case would have been simple to predict by
> reading the changelog of the linux-firmware change. To quote from
> https://git.kernel.org/pub/scm/linux/kernel/git/firmware/linux-firmware.git/c
> ommit/?id=55e7448533e7
>
> ```
> Note: This firmware patch needs to be used with driver
> "btrtl: Firmware format v2 support". Please update the driver with this commit:
> 9a24ce5e29b15c4c6b0c89c04f9df6ce14addefa
> ```


Actually, we thought this note is enough. If users update linux-firmware,
backport will be work.
However, the commit avoid the firmware guideline.
"At the same time updated firmware files must not cause any regressions for
users of older kernel releases."
We will revert the commit and send a new commit for rtl8852cu_fw_v2.bin




>
> > so rtl8852cu_fw.bin will be revert to the previous commit as FW v1 and
> > add rtl8852cu_fw_v2.bin as FW v2. This item will be started on
> > linux-firmware project.
> >
> > In this commit, the driver prefers to load FW v2 if available.
> > Fallback to FW v1 otherwise.
> >
> > To do on linux-firmware project.
> > rtl_bt/rtl8852cu_fw.bin: FW v1 (stay at ver. 0xD7B8_FABF)
> > rtl_bt/rtl8852cu_fw_v2.bin: FW v2 (to be maintained)
> >
> > [1]'9a24ce5e29b1 ("Bluetooth: btrtl: Firmware format v2 support")'
> > [2]'55e7448533e7 ("rtl_bt: Update RTL8852C BT USB firmware
> > to 0x040D_7225")'
>
> You seem to have mixed up [1] and [2] here, or did my brain go sideways
> somewhere?

We will revert '55e7448533e7 ("rtl_bt: Update RTL8852C BT USB firmware
to 0x040D_7225")' and add a new commit for rtl8852cu_fw_v2.bin.

>
> > Suggested-by: Juerg Haefliger <[email protected]>
> > Tested-by: Hilda Wu <[email protected]>
>
> All that sounds like this should also have these tags to to ensure this makes it
> back too all trees that applied 9a24ce5e29b:
>
> Fixes: 9a24ce5e29b ("Bluetooth: btrtl: Firmware format v2 support")
> Cc: [email protected]
>

Roger that

> Ciao, Thorsten (wearing his 'the Linux kernel's regression tracker' hat)
> --
> Everything you wanna know about Linux kernel regression tracking:
> https://linux-regtracking.leemhuis.info/about/#tldr
> If I did something stupid, please tell me, as explained on that page.
>
> > Signed-off-by: Max Chou <[email protected]>
> > ---
> > drivers/bluetooth/btrtl.c | 68
> > +++++++++++++++++++++++++--------------
> > 1 file changed, 44 insertions(+), 24 deletions(-)
> >
> > diff --git a/drivers/bluetooth/btrtl.c b/drivers/bluetooth/btrtl.c
> > index ddae6524106d..8bfa86dd12f7 100644
> > --- a/drivers/bluetooth/btrtl.c
> > +++ b/drivers/bluetooth/btrtl.c
> > @@ -104,7 +104,7 @@ static const struct id_table ic_id_table[] = {
> > { IC_INFO(RTL_ROM_LMP_8723A, 0xb, 0x6, HCI_USB),
> > .config_needed = false,
> > .has_rom_version = false,
> > - .fw_name = "rtl_bt/rtl8723a_fw.bin",
> > + .fw_name = "rtl_bt/rtl8723a_fw",
> > .cfg_name = NULL,
> > .hw_info = "rtl8723au" },
> >
> > @@ -112,7 +112,7 @@ static const struct id_table ic_id_table[] = {
> > { IC_INFO(RTL_ROM_LMP_8723B, 0xb, 0x6, HCI_UART),
> > .config_needed = true,
> > .has_rom_version = true,
> > - .fw_name = "rtl_bt/rtl8723bs_fw.bin",
> > + .fw_name = "rtl_bt/rtl8723bs_fw",
> > .cfg_name = "rtl_bt/rtl8723bs_config",
> > .hw_info = "rtl8723bs" },
> >
> > @@ -120,7 +120,7 @@ static const struct id_table ic_id_table[] = {
> > { IC_INFO(RTL_ROM_LMP_8723B, 0xb, 0x6, HCI_USB),
> > .config_needed = false,
> > .has_rom_version = true,
> > - .fw_name = "rtl_bt/rtl8723b_fw.bin",
> > + .fw_name = "rtl_bt/rtl8723b_fw",
> > .cfg_name = "rtl_bt/rtl8723b_config",
> > .hw_info = "rtl8723bu" },
> >
> > @@ -132,7 +132,7 @@ static const struct id_table ic_id_table[] = {
> > .hci_bus = HCI_UART,
> > .config_needed = true,
> > .has_rom_version = true,
> > - .fw_name = "rtl_bt/rtl8723cs_cg_fw.bin",
> > + .fw_name = "rtl_bt/rtl8723cs_cg_fw",
> > .cfg_name = "rtl_bt/rtl8723cs_cg_config",
> > .hw_info = "rtl8723cs-cg" },
> >
> > @@ -144,7 +144,7 @@ static const struct id_table ic_id_table[] = {
> > .hci_bus = HCI_UART,
> > .config_needed = true,
> > .has_rom_version = true,
> > - .fw_name = "rtl_bt/rtl8723cs_vf_fw.bin",
> > + .fw_name = "rtl_bt/rtl8723cs_vf_fw",
> > .cfg_name = "rtl_bt/rtl8723cs_vf_config",
> > .hw_info = "rtl8723cs-vf" },
> >
> > @@ -156,7 +156,7 @@ static const struct id_table ic_id_table[] = {
> > .hci_bus = HCI_UART,
> > .config_needed = true,
> > .has_rom_version = true,
> > - .fw_name = "rtl_bt/rtl8723cs_xx_fw.bin",
> > + .fw_name = "rtl_bt/rtl8723cs_xx_fw",
> > .cfg_name = "rtl_bt/rtl8723cs_xx_config",
> > .hw_info = "rtl8723cs" },
> >
> > @@ -164,7 +164,7 @@ static const struct id_table ic_id_table[] = {
> > { IC_INFO(RTL_ROM_LMP_8723B, 0xd, 0x8, HCI_USB),
> > .config_needed = true,
> > .has_rom_version = true,
> > - .fw_name = "rtl_bt/rtl8723d_fw.bin",
> > + .fw_name = "rtl_bt/rtl8723d_fw",
> > .cfg_name = "rtl_bt/rtl8723d_config",
> > .hw_info = "rtl8723du" },
> >
> > @@ -172,7 +172,7 @@ static const struct id_table ic_id_table[] = {
> > { IC_INFO(RTL_ROM_LMP_8723B, 0xd, 0x8, HCI_UART),
> > .config_needed = true,
> > .has_rom_version = true,
> > - .fw_name = "rtl_bt/rtl8723ds_fw.bin",
> > + .fw_name = "rtl_bt/rtl8723ds_fw",
> > .cfg_name = "rtl_bt/rtl8723ds_config",
> > .hw_info = "rtl8723ds" },
> >
> > @@ -180,7 +180,7 @@ static const struct id_table ic_id_table[] = {
> > { IC_INFO(RTL_ROM_LMP_8821A, 0xa, 0x6, HCI_USB),
> > .config_needed = false,
> > .has_rom_version = true,
> > - .fw_name = "rtl_bt/rtl8821a_fw.bin",
> > + .fw_name = "rtl_bt/rtl8821a_fw",
> > .cfg_name = "rtl_bt/rtl8821a_config",
> > .hw_info = "rtl8821au" },
> >
> > @@ -189,7 +189,7 @@ static const struct id_table ic_id_table[] = {
> > .config_needed = false,
> > .has_rom_version = true,
> > .has_msft_ext = true,
> > - .fw_name = "rtl_bt/rtl8821c_fw.bin",
> > + .fw_name = "rtl_bt/rtl8821c_fw",
> > .cfg_name = "rtl_bt/rtl8821c_config",
> > .hw_info = "rtl8821cu" },
> >
> > @@ -198,7 +198,7 @@ static const struct id_table ic_id_table[] = {
> > .config_needed = true,
> > .has_rom_version = true,
> > .has_msft_ext = true,
> > - .fw_name = "rtl_bt/rtl8821cs_fw.bin",
> > + .fw_name = "rtl_bt/rtl8821cs_fw",
> > .cfg_name = "rtl_bt/rtl8821cs_config",
> > .hw_info = "rtl8821cs" },
> >
> > @@ -206,7 +206,7 @@ static const struct id_table ic_id_table[] = {
> > { IC_INFO(RTL_ROM_LMP_8761A, 0xa, 0x6, HCI_USB),
> > .config_needed = false,
> > .has_rom_version = true,
> > - .fw_name = "rtl_bt/rtl8761a_fw.bin",
> > + .fw_name = "rtl_bt/rtl8761a_fw",
> > .cfg_name = "rtl_bt/rtl8761a_config",
> > .hw_info = "rtl8761au" },
> >
> > @@ -215,7 +215,7 @@ static const struct id_table ic_id_table[] = {
> > .config_needed = false,
> > .has_rom_version = true,
> > .has_msft_ext = true,
> > - .fw_name = "rtl_bt/rtl8761b_fw.bin",
> > + .fw_name = "rtl_bt/rtl8761b_fw",
> > .cfg_name = "rtl_bt/rtl8761b_config",
> > .hw_info = "rtl8761btv" },
> >
> > @@ -223,7 +223,7 @@ static const struct id_table ic_id_table[] = {
> > { IC_INFO(RTL_ROM_LMP_8761A, 0xb, 0xa, HCI_USB),
> > .config_needed = false,
> > .has_rom_version = true,
> > - .fw_name = "rtl_bt/rtl8761bu_fw.bin",
> > + .fw_name = "rtl_bt/rtl8761bu_fw",
> > .cfg_name = "rtl_bt/rtl8761bu_config",
> > .hw_info = "rtl8761bu" },
> >
> > @@ -232,7 +232,7 @@ static const struct id_table ic_id_table[] = {
> > .config_needed = true,
> > .has_rom_version = true,
> > .has_msft_ext = true,
> > - .fw_name = "rtl_bt/rtl8822cs_fw.bin",
> > + .fw_name = "rtl_bt/rtl8822cs_fw",
> > .cfg_name = "rtl_bt/rtl8822cs_config",
> > .hw_info = "rtl8822cs" },
> >
> > @@ -241,7 +241,7 @@ static const struct id_table ic_id_table[] = {
> > .config_needed = true,
> > .has_rom_version = true,
> > .has_msft_ext = true,
> > - .fw_name = "rtl_bt/rtl8822cs_fw.bin",
> > + .fw_name = "rtl_bt/rtl8822cs_fw",
> > .cfg_name = "rtl_bt/rtl8822cs_config",
> > .hw_info = "rtl8822cs" },
> >
> > @@ -250,7 +250,7 @@ static const struct id_table ic_id_table[] = {
> > .config_needed = false,
> > .has_rom_version = true,
> > .has_msft_ext = true,
> > - .fw_name = "rtl_bt/rtl8822cu_fw.bin",
> > + .fw_name = "rtl_bt/rtl8822cu_fw",
> > .cfg_name = "rtl_bt/rtl8822cu_config",
> > .hw_info = "rtl8822cu" },
> >
> > @@ -259,7 +259,7 @@ static const struct id_table ic_id_table[] = {
> > .config_needed = true,
> > .has_rom_version = true,
> > .has_msft_ext = true,
> > - .fw_name = "rtl_bt/rtl8822b_fw.bin",
> > + .fw_name = "rtl_bt/rtl8822b_fw",
> > .cfg_name = "rtl_bt/rtl8822b_config",
> > .hw_info = "rtl8822bu" },
> >
> > @@ -268,7 +268,7 @@ static const struct id_table ic_id_table[] = {
> > .config_needed = false,
> > .has_rom_version = true,
> > .has_msft_ext = true,
> > - .fw_name = "rtl_bt/rtl8852au_fw.bin",
> > + .fw_name = "rtl_bt/rtl8852au_fw",
> > .cfg_name = "rtl_bt/rtl8852au_config",
> > .hw_info = "rtl8852au" },
> >
> > @@ -277,7 +277,7 @@ static const struct id_table ic_id_table[] = {
> > .config_needed = true,
> > .has_rom_version = true,
> > .has_msft_ext = true,
> > - .fw_name = "rtl_bt/rtl8852bs_fw.bin",
> > + .fw_name = "rtl_bt/rtl8852bs_fw",
> > .cfg_name = "rtl_bt/rtl8852bs_config",
> > .hw_info = "rtl8852bs" },
> >
> > @@ -286,7 +286,7 @@ static const struct id_table ic_id_table[] = {
> > .config_needed = false,
> > .has_rom_version = true,
> > .has_msft_ext = true,
> > - .fw_name = "rtl_bt/rtl8852bu_fw.bin",
> > + .fw_name = "rtl_bt/rtl8852bu_fw",
> > .cfg_name = "rtl_bt/rtl8852bu_config",
> > .hw_info = "rtl8852bu" },
> >
> > @@ -295,7 +295,7 @@ static const struct id_table ic_id_table[] = {
> > .config_needed = false,
> > .has_rom_version = true,
> > .has_msft_ext = true,
> > - .fw_name = "rtl_bt/rtl8852cu_fw.bin",
> > + .fw_name = "rtl_bt/rtl8852cu_fw",
> > .cfg_name = "rtl_bt/rtl8852cu_config",
> > .hw_info = "rtl8852cu" },
> >
> > @@ -304,7 +304,7 @@ static const struct id_table ic_id_table[] = {
> > .config_needed = false,
> > .has_rom_version = true,
> > .has_msft_ext = false,
> > - .fw_name = "rtl_bt/rtl8851bu_fw.bin",
> > + .fw_name = "rtl_bt/rtl8851bu_fw",
> > .cfg_name = "rtl_bt/rtl8851bu_config",
> > .hw_info = "rtl8851bu" },
> > };
> > @@ -1045,10 +1045,12 @@ struct btrtl_device_info *btrtl_initialize(struct
> hci_dev *hdev,
> > struct sk_buff *skb;
> > struct hci_rp_read_local_version *resp;
> > struct hci_command_hdr *cmd;
> > + char fw_name[40];
> > char cfg_name[40];
> > u16 hci_rev, lmp_subver;
> > u8 hci_ver, lmp_ver, chip_type = 0;
> > int ret;
> > + int fw_load_retry = 0;
> > u8 reg_val[2];
> >
> > btrtl_dev = kzalloc(sizeof(*btrtl_dev), GFP_KERNEL); @@ -1154,9
> > +1156,26 @@ struct btrtl_device_info *btrtl_initialize(struct hci_dev *hdev,
> > goto err_free;
> > }
> >
> > - btrtl_dev->fw_len = rtl_load_file(hdev, btrtl_dev->ic_info->fw_name,
> > +fw_name_load:
> > + if (btrtl_dev->ic_info->fw_name) {
> > + if (lmp_subver == RTL_ROM_LMP_8852A && hci_rev ==
> 0x000c &&
> > + fw_load_retry == 0) {
> > + fw_load_retry = 1;
> > + snprintf(fw_name, sizeof(fw_name), "%s_v2.bin",
> > + btrtl_dev->ic_info->fw_name);
> > + } else {
> > + fw_load_retry = 0;
> > + snprintf(fw_name, sizeof(fw_name), "%s.bin",
> > + btrtl_dev->ic_info->fw_name);
> > + }
> > + btrtl_dev->fw_len = rtl_load_file(hdev, fw_name,
> > &btrtl_dev->fw_data);
> > + }
> > +
> > if (btrtl_dev->fw_len < 0) {
> > + if (fw_load_retry == 1)
> > + goto fw_name_load;
> > +
> > rtl_dev_err(hdev, "firmware file %s not found",
> > btrtl_dev->ic_info->fw_name);
> > ret = btrtl_dev->fw_len; @@ -1491,4 +1510,5 @@
> > MODULE_FIRMWARE("rtl_bt/rtl8852bs_config.bin");
> > MODULE_FIRMWARE("rtl_bt/rtl8852bu_fw.bin");
> > MODULE_FIRMWARE("rtl_bt/rtl8852bu_config.bin");
> > MODULE_FIRMWARE("rtl_bt/rtl8852cu_fw.bin");
> > +MODULE_FIRMWARE("rtl_bt/rtl8852cu_fw_v2.bin");
> > MODULE_FIRMWARE("rtl_bt/rtl8852cu_config.bin");

2023-08-04 10:45:10

by Max Chou

[permalink] [raw]
Subject: RE: [PATCH] Bluetooth: btrtl: Load FW v2 otherwise FW v1 for RTL8852C

Dear All,
Please check my comment inline. Thanks.

P.S. Fix the typo for the previous one.

BRs,
Max

> -----Original Message-----
> From: Linux regression tracking (Thorsten Leemhuis)
> <[email protected]>
> Sent: Friday, August 4, 2023 4:55 PM
> To: Max Chou <[email protected]>; [email protected]
> Cc: [email protected]; [email protected];
> [email protected]; [email protected];
> [email protected]; Hilda Wu <[email protected]>; Karen Hsu
> <[email protected]>; KidmanLee <[email protected]>;
> [email protected]; [email protected];
> [email protected]; Josh Boyer <[email protected]>; David S. Miller
> <[email protected]>; Eric Dumazet <[email protected]>; Jakub
> Kicinski <[email protected]>; Paolo Abeni <[email protected]>; Linux kernel
> regressions list <[email protected]>
> Subject: Re: [PATCH] Bluetooth: btrtl: Load FW v2 otherwise FW v1 for
> RTL8852C
>
>
> External mail.
>
>
>
> [CCing the linux-firmware maintainer, the network maintainers, and the
> regressions lists]
>
> On 04.08.23 07:54, [email protected] wrote:
> > From: Max Chou <[email protected]>
> >
> > In the commit of linux-firmware project, rtl8852cu_fw.bin is updated
> > as FW v2 format[1]. Consider the case that if driver did not be updated fo>
> FW v2 supported[2], it can not use FW v2.
> >
> > By Canonical's suggestion, older driver should be able to load FW v1,
>
> Well, that's not only Canonical suggestion, that is how things are supposed to
> be handled in general. See
> Documentation/driver-api/firmware/firmware-usage-guidelines.rst (alt:
> https://docs.kernel.org/driver-api/firmware/firmware-usage-guidelines.html
> ) for details.
>
> We had a similar situation in March already with a Wifi driver:
> https://git.kernel.org/pub/scm/linux/kernel/git/firmware/linux-firmware.git/c
> ommit/?id=416a66cae796939d7d096988c72c641656c81c5a
>
> Makes me wonder: is there a problem here we should try harder to avoid?
> Especially as the problem in this case would have been simple to predict by
> reading the changelog of the linux-firmware change. To quote from
> https://git.kernel.org/pub/scm/linux/kernel/git/firmware/linux-firmware.git/c
> ommit/?id=55e7448533e7
>
> ```
> Note: This firmware patch needs to be used with driver
> "btrtl: Firmware format v2 support". Please update the driver with this commit:
> 9a24ce5e29b15c4c6b0c89c04f9df6ce14addefa
> ```


Actually, we thought this note is enough. If users update linux-firmware, backport will be work.
However, the commit violate the firmware guideline.
"At the same time updated firmware files must not cause any regressions for users of older kernel releases."
We will revert the commit and send a new commit for rtl8852cu_fw_v2.bin


>
> > so rtl8852cu_fw.bin will be revert to the previous commit as FW v1 and
> > add rtl8852cu_fw_v2.bin as FW v2. This item will be started on
> > linux-firmware project.
> >
> > In this commit, the driver prefers to load FW v2 if available.
> > Fallback to FW v1 otherwise.
> >
> > To do on linux-firmware project.
> > rtl_bt/rtl8852cu_fw.bin: FW v1 (stay at ver. 0xD7B8_FABF)
> > rtl_bt/rtl8852cu_fw_v2.bin: FW v2 (to be maintained)
> >
> > [1]'9a24ce5e29b1 ("Bluetooth: btrtl: Firmware format v2 support")'
> > [2]'55e7448533e7 ("rtl_bt: Update RTL8852C BT USB firmware
> > to 0x040D_7225")'
>
> You seem to have mixed up [1] and [2] here, or did my brain go sideways
> somewhere?



We will revert '55e7448533e7 ("rtl_bt: Update RTL8852C BT USB firmware to 0x040D_7225")'
and add a new commit for rtl8852cu_fw_v2.bin.


>
> > Suggested-by: Juerg Haefliger <[email protected]>
> > Tested-by: Hilda Wu <[email protected]>
>
> All that sounds like this should also have these tags to to ensure this makes it
> back too all trees that applied 9a24ce5e29b:
>
> Fixes: 9a24ce5e29b ("Bluetooth: btrtl: Firmware format v2 support")
> Cc: [email protected]
>


Roger that



> Ciao, Thorsten (wearing his 'the Linux kernel's regression tracker' hat)
> --
> Everything you wanna know about Linux kernel regression tracking:
> https://linux-regtracking.leemhuis.info/about/#tldr
> If I did something stupid, please tell me, as explained on that page.
>
> > Signed-off-by: Max Chou <[email protected]>
> > ---
> > drivers/bluetooth/btrtl.c | 68
> > +++++++++++++++++++++++++--------------
> > 1 file changed, 44 insertions(+), 24 deletions(-)
> >
> > diff --git a/drivers/bluetooth/btrtl.c b/drivers/bluetooth/btrtl.c
> > index ddae6524106d..8bfa86dd12f7 100644
> > --- a/drivers/bluetooth/btrtl.c
> > +++ b/drivers/bluetooth/btrtl.c
> > @@ -104,7 +104,7 @@ static const struct id_table ic_id_table[] = {
> > { IC_INFO(RTL_ROM_LMP_8723A, 0xb, 0x6, HCI_USB),
> > .config_needed = false,
> > .has_rom_version = false,
> > - .fw_name = "rtl_bt/rtl8723a_fw.bin",
> > + .fw_name = "rtl_bt/rtl8723a_fw",
> > .cfg_name = NULL,
> > .hw_info = "rtl8723au" },
> >
> > @@ -112,7 +112,7 @@ static const struct id_table ic_id_table[] = {
> > { IC_INFO(RTL_ROM_LMP_8723B, 0xb, 0x6, HCI_UART),
> > .config_needed = true,
> > .has_rom_version = true,
> > - .fw_name = "rtl_bt/rtl8723bs_fw.bin",
> > + .fw_name = "rtl_bt/rtl8723bs_fw",
> > .cfg_name = "rtl_bt/rtl8723bs_config",
> > .hw_info = "rtl8723bs" },
> >
> > @@ -120,7 +120,7 @@ static const struct id_table ic_id_table[] = {
> > { IC_INFO(RTL_ROM_LMP_8723B, 0xb, 0x6, HCI_USB),
> > .config_needed = false,
> > .has_rom_version = true,
> > - .fw_name = "rtl_bt/rtl8723b_fw.bin",
> > + .fw_name = "rtl_bt/rtl8723b_fw",
> > .cfg_name = "rtl_bt/rtl8723b_config",
> > .hw_info = "rtl8723bu" },
> >
> > @@ -132,7 +132,7 @@ static const struct id_table ic_id_table[] = {
> > .hci_bus = HCI_UART,
> > .config_needed = true,
> > .has_rom_version = true,
> > - .fw_name = "rtl_bt/rtl8723cs_cg_fw.bin",
> > + .fw_name = "rtl_bt/rtl8723cs_cg_fw",
> > .cfg_name = "rtl_bt/rtl8723cs_cg_config",
> > .hw_info = "rtl8723cs-cg" },
> >
> > @@ -144,7 +144,7 @@ static const struct id_table ic_id_table[] = {
> > .hci_bus = HCI_UART,
> > .config_needed = true,
> > .has_rom_version = true,
> > - .fw_name = "rtl_bt/rtl8723cs_vf_fw.bin",
> > + .fw_name = "rtl_bt/rtl8723cs_vf_fw",
> > .cfg_name = "rtl_bt/rtl8723cs_vf_config",
> > .hw_info = "rtl8723cs-vf" },
> >
> > @@ -156,7 +156,7 @@ static const struct id_table ic_id_table[] = {
> > .hci_bus = HCI_UART,
> > .config_needed = true,
> > .has_rom_version = true,
> > - .fw_name = "rtl_bt/rtl8723cs_xx_fw.bin",
> > + .fw_name = "rtl_bt/rtl8723cs_xx_fw",
> > .cfg_name = "rtl_bt/rtl8723cs_xx_config",
> > .hw_info = "rtl8723cs" },
> >
> > @@ -164,7 +164,7 @@ static const struct id_table ic_id_table[] = {
> > { IC_INFO(RTL_ROM_LMP_8723B, 0xd, 0x8, HCI_USB),
> > .config_needed = true,
> > .has_rom_version = true,
> > - .fw_name = "rtl_bt/rtl8723d_fw.bin",
> > + .fw_name = "rtl_bt/rtl8723d_fw",
> > .cfg_name = "rtl_bt/rtl8723d_config",
> > .hw_info = "rtl8723du" },
> >
> > @@ -172,7 +172,7 @@ static const struct id_table ic_id_table[] = {
> > { IC_INFO(RTL_ROM_LMP_8723B, 0xd, 0x8, HCI_UART),
> > .config_needed = true,
> > .has_rom_version = true,
> > - .fw_name = "rtl_bt/rtl8723ds_fw.bin",
> > + .fw_name = "rtl_bt/rtl8723ds_fw",
> > .cfg_name = "rtl_bt/rtl8723ds_config",
> > .hw_info = "rtl8723ds" },
> >
> > @@ -180,7 +180,7 @@ static const struct id_table ic_id_table[] = {
> > { IC_INFO(RTL_ROM_LMP_8821A, 0xa, 0x6, HCI_USB),
> > .config_needed = false,
> > .has_rom_version = true,
> > - .fw_name = "rtl_bt/rtl8821a_fw.bin",
> > + .fw_name = "rtl_bt/rtl8821a_fw",
> > .cfg_name = "rtl_bt/rtl8821a_config",
> > .hw_info = "rtl8821au" },
> >
> > @@ -189,7 +189,7 @@ static const struct id_table ic_id_table[] = {
> > .config_needed = false,
> > .has_rom_version = true,
> > .has_msft_ext = true,
> > - .fw_name = "rtl_bt/rtl8821c_fw.bin",
> > + .fw_name = "rtl_bt/rtl8821c_fw",
> > .cfg_name = "rtl_bt/rtl8821c_config",
> > .hw_info = "rtl8821cu" },
> >
> > @@ -198,7 +198,7 @@ static const struct id_table ic_id_table[] = {
> > .config_needed = true,
> > .has_rom_version = true,
> > .has_msft_ext = true,
> > - .fw_name = "rtl_bt/rtl8821cs_fw.bin",
> > + .fw_name = "rtl_bt/rtl8821cs_fw",
> > .cfg_name = "rtl_bt/rtl8821cs_config",
> > .hw_info = "rtl8821cs" },
> >
> > @@ -206,7 +206,7 @@ static const struct id_table ic_id_table[] = {
> > { IC_INFO(RTL_ROM_LMP_8761A, 0xa, 0x6, HCI_USB),
> > .config_needed = false,
> > .has_rom_version = true,
> > - .fw_name = "rtl_bt/rtl8761a_fw.bin",
> > + .fw_name = "rtl_bt/rtl8761a_fw",
> > .cfg_name = "rtl_bt/rtl8761a_config",
> > .hw_info = "rtl8761au" },
> >
> > @@ -215,7 +215,7 @@ static const struct id_table ic_id_table[] = {
> > .config_needed = false,
> > .has_rom_version = true,
> > .has_msft_ext = true,
> > - .fw_name = "rtl_bt/rtl8761b_fw.bin",
> > + .fw_name = "rtl_bt/rtl8761b_fw",
> > .cfg_name = "rtl_bt/rtl8761b_config",
> > .hw_info = "rtl8761btv" },
> >
> > @@ -223,7 +223,7 @@ static const struct id_table ic_id_table[] = {
> > { IC_INFO(RTL_ROM_LMP_8761A, 0xb, 0xa, HCI_USB),
> > .config_needed = false,
> > .has_rom_version = true,
> > - .fw_name = "rtl_bt/rtl8761bu_fw.bin",
> > + .fw_name = "rtl_bt/rtl8761bu_fw",
> > .cfg_name = "rtl_bt/rtl8761bu_config",
> > .hw_info = "rtl8761bu" },
> >
> > @@ -232,7 +232,7 @@ static const struct id_table ic_id_table[] = {
> > .config_needed = true,
> > .has_rom_version = true,
> > .has_msft_ext = true,
> > - .fw_name = "rtl_bt/rtl8822cs_fw.bin",
> > + .fw_name = "rtl_bt/rtl8822cs_fw",
> > .cfg_name = "rtl_bt/rtl8822cs_config",
> > .hw_info = "rtl8822cs" },
> >
> > @@ -241,7 +241,7 @@ static const struct id_table ic_id_table[] = {
> > .config_needed = true,
> > .has_rom_version = true,
> > .has_msft_ext = true,
> > - .fw_name = "rtl_bt/rtl8822cs_fw.bin",
> > + .fw_name = "rtl_bt/rtl8822cs_fw",
> > .cfg_name = "rtl_bt/rtl8822cs_config",
> > .hw_info = "rtl8822cs" },
> >
> > @@ -250,7 +250,7 @@ static const struct id_table ic_id_table[] = {
> > .config_needed = false,
> > .has_rom_version = true,
> > .has_msft_ext = true,
> > - .fw_name = "rtl_bt/rtl8822cu_fw.bin",
> > + .fw_name = "rtl_bt/rtl8822cu_fw",
> > .cfg_name = "rtl_bt/rtl8822cu_config",
> > .hw_info = "rtl8822cu" },
> >
> > @@ -259,7 +259,7 @@ static const struct id_table ic_id_table[] = {
> > .config_needed = true,
> > .has_rom_version = true,
> > .has_msft_ext = true,
> > - .fw_name = "rtl_bt/rtl8822b_fw.bin",
> > + .fw_name = "rtl_bt/rtl8822b_fw",
> > .cfg_name = "rtl_bt/rtl8822b_config",
> > .hw_info = "rtl8822bu" },
> >
> > @@ -268,7 +268,7 @@ static const struct id_table ic_id_table[] = {
> > .config_needed = false,
> > .has_rom_version = true,
> > .has_msft_ext = true,
> > - .fw_name = "rtl_bt/rtl8852au_fw.bin",
> > + .fw_name = "rtl_bt/rtl8852au_fw",
> > .cfg_name = "rtl_bt/rtl8852au_config",
> > .hw_info = "rtl8852au" },
> >
> > @@ -277,7 +277,7 @@ static const struct id_table ic_id_table[] = {
> > .config_needed = true,
> > .has_rom_version = true,
> > .has_msft_ext = true,
> > - .fw_name = "rtl_bt/rtl8852bs_fw.bin",
> > + .fw_name = "rtl_bt/rtl8852bs_fw",
> > .cfg_name = "rtl_bt/rtl8852bs_config",
> > .hw_info = "rtl8852bs" },
> >
> > @@ -286,7 +286,7 @@ static const struct id_table ic_id_table[] = {
> > .config_needed = false,
> > .has_rom_version = true,
> > .has_msft_ext = true,
> > - .fw_name = "rtl_bt/rtl8852bu_fw.bin",
> > + .fw_name = "rtl_bt/rtl8852bu_fw",
> > .cfg_name = "rtl_bt/rtl8852bu_config",
> > .hw_info = "rtl8852bu" },
> >
> > @@ -295,7 +295,7 @@ static const struct id_table ic_id_table[] = {
> > .config_needed = false,
> > .has_rom_version = true,
> > .has_msft_ext = true,
> > - .fw_name = "rtl_bt/rtl8852cu_fw.bin",
> > + .fw_name = "rtl_bt/rtl8852cu_fw",
> > .cfg_name = "rtl_bt/rtl8852cu_config",
> > .hw_info = "rtl8852cu" },
> >
> > @@ -304,7 +304,7 @@ static const struct id_table ic_id_table[] = {
> > .config_needed = false,
> > .has_rom_version = true,
> > .has_msft_ext = false,
> > - .fw_name = "rtl_bt/rtl8851bu_fw.bin",
> > + .fw_name = "rtl_bt/rtl8851bu_fw",
> > .cfg_name = "rtl_bt/rtl8851bu_config",
> > .hw_info = "rtl8851bu" },
> > };
> > @@ -1045,10 +1045,12 @@ struct btrtl_device_info *btrtl_initialize(struct
> hci_dev *hdev,
> > struct sk_buff *skb;
> > struct hci_rp_read_local_version *resp;
> > struct hci_command_hdr *cmd;
> > + char fw_name[40];
> > char cfg_name[40];
> > u16 hci_rev, lmp_subver;
> > u8 hci_ver, lmp_ver, chip_type = 0;
> > int ret;
> > + int fw_load_retry = 0;
> > u8 reg_val[2];
> >
> > btrtl_dev = kzalloc(sizeof(*btrtl_dev), GFP_KERNEL); @@ -1154,9
> > +1156,26 @@ struct btrtl_device_info *btrtl_initialize(struct hci_dev *hdev,
> > goto err_free;
> > }
> >
> > - btrtl_dev->fw_len = rtl_load_file(hdev, btrtl_dev->ic_info->fw_name,
> > +fw_name_load:
> > + if (btrtl_dev->ic_info->fw_name) {
> > + if (lmp_subver == RTL_ROM_LMP_8852A && hci_rev ==
> 0x000c &&
> > + fw_load_retry == 0) {
> > + fw_load_retry = 1;
> > + snprintf(fw_name, sizeof(fw_name), "%s_v2.bin",
> > + btrtl_dev->ic_info->fw_name);
> > + } else {
> > + fw_load_retry = 0;
> > + snprintf(fw_name, sizeof(fw_name), "%s.bin",
> > + btrtl_dev->ic_info->fw_name);
> > + }
> > + btrtl_dev->fw_len = rtl_load_file(hdev, fw_name,
> > &btrtl_dev->fw_data);
> > + }
> > +
> > if (btrtl_dev->fw_len < 0) {
> > + if (fw_load_retry == 1)
> > + goto fw_name_load;
> > +
> > rtl_dev_err(hdev, "firmware file %s not found",
> > btrtl_dev->ic_info->fw_name);
> > ret = btrtl_dev->fw_len; @@ -1491,4 +1510,5 @@
> > MODULE_FIRMWARE("rtl_bt/rtl8852bs_config.bin");
> > MODULE_FIRMWARE("rtl_bt/rtl8852bu_fw.bin");
> > MODULE_FIRMWARE("rtl_bt/rtl8852bu_config.bin");
> > MODULE_FIRMWARE("rtl_bt/rtl8852cu_fw.bin");
> > +MODULE_FIRMWARE("rtl_bt/rtl8852cu_fw_v2.bin");
> > MODULE_FIRMWARE("rtl_bt/rtl8852cu_config.bin");

Subject: Re: [PATCH] Bluetooth: btrtl: Load FW v2 otherwise FW v1 for RTL8852C

On 04.08.23 12:15, Max Chou wrote:
>> -----Original Message-----
>> [CCing the linux-firmware maintainer, the network maintainers, and the
>> regressions lists]
>>
>> On 04.08.23 07:54, [email protected] wrote:
>>> From: Max Chou <[email protected]>
>>>
>>> In the commit of linux-firmware project, rtl8852cu_fw.bin is updated
>>> as FW v2 format[1]. Consider the case that if driver did not be updated fo>
>> FW v2 supported[2], it can not use FW v2.
>>>
>>> By Canonical's suggestion, older driver should be able to load FW v1,
>>
>> Well, that's not only Canonical suggestion, that is how things are supposed to
>> be handled in general. See
>> Documentation/driver-api/firmware/firmware-usage-guidelines.rst (alt:
>> https://docs.kernel.org/driver-api/firmware/firmware-usage-guidelines.html
>> ) for details.
>>
>> We had a similar situation in March already with a Wifi driver:
>> https://git.kernel.org/pub/scm/linux/kernel/git/firmware/linux-firmware.git/c
>> ommit/?id=416a66cae796939d7d096988c72c641656c81c5a
>>
>> Makes me wonder: is there a problem here we should try harder to avoid?
>> Especially as the problem in this case would have been simple to predict by
>> reading the changelog of the linux-firmware change. To quote from
>> https://git.kernel.org/pub/scm/linux/kernel/git/firmware/linux-firmware.git/c
>> ommit/?id=55e7448533e7
>>
>> ```
>> Note: This firmware patch needs to be used with driver
>> "btrtl: Firmware format v2 support". Please update the driver with this commit:
>> 9a24ce5e29b15c4c6b0c89c04f9df6ce14addefa
>> ```
>
> Actually, we thought this note is enough. [...]

Yeah, no big deal, error are made and dealt with, and you are working on
that. Thx for that. It's just that the episode got got me thinking:

It's the second time such error is made within a a few months, hence I
wonder if we on the side of the Linux kernel and/or Linux-firmware
should change something to ideally prevent this from happening.

Ciao, Thorsten