* [PATCH] Bluetooth: btrtl: Load FW v2 otherwise FW v1 for RTL8852C
@ 2023-08-04 5:54 max.chou
2023-08-04 6:42 ` bluez.test.bot
` (4 more replies)
0 siblings, 5 replies; 9+ messages in thread
From: max.chou @ 2023-08-04 5:54 UTC (permalink / raw)
To: marcel
Cc: johan.hedberg, luiz.dentz, linux-bluetooth, linux-kernel, alex_lu,
hildawu, karenhsu, kidman, max.chou, juerg.haefliger, vicamo.yang,
Riley.Kao
From: Max Chou <max.chou@realtek.com>
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 <juerg.haefliger@canonical.com>
Tested-by: Hilda Wu <hildawu@realtek.com>
Signed-off-by: Max Chou <max.chou@realtek.com>
---
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
^ permalink raw reply related [flat|nested] 9+ messages in thread
* RE: Bluetooth: btrtl: Load FW v2 otherwise FW v1 for RTL8852C
2023-08-04 5:54 [PATCH] Bluetooth: btrtl: Load FW v2 otherwise FW v1 for RTL8852C max.chou
@ 2023-08-04 6:42 ` bluez.test.bot
2023-08-04 8:51 ` [PATCH] " Juerg Haefliger
` (3 subsequent siblings)
4 siblings, 0 replies; 9+ messages in thread
From: bluez.test.bot @ 2023-08-04 6:42 UTC (permalink / raw)
To: linux-bluetooth, max.chou
[-- Attachment #1: Type: text/plain, Size: 1427 bytes --]
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
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] Bluetooth: btrtl: Load FW v2 otherwise FW v1 for RTL8852C
2023-08-04 5:54 [PATCH] Bluetooth: btrtl: Load FW v2 otherwise FW v1 for RTL8852C max.chou
2023-08-04 6:42 ` bluez.test.bot
@ 2023-08-04 8:51 ` Juerg Haefliger
2023-08-04 8:54 ` Linux regression tracking (Thorsten Leemhuis)
` (2 subsequent siblings)
4 siblings, 0 replies; 9+ messages in thread
From: Juerg Haefliger @ 2023-08-04 8:51 UTC (permalink / raw)
To: max.chou
Cc: marcel, johan.hedberg, luiz.dentz, linux-bluetooth, linux-kernel,
alex_lu, hildawu, karenhsu, kidman, vicamo.yang, Riley.Kao
[-- Attachment #1: Type: text/plain, Size: 12391 bytes --]
Thanks for this Max,
Comments inlined.
> From: Max Chou <max.chou@realtek.com>
>
> 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 <juerg.haefliger@canonical.com>
> Tested-by: Hilda Wu <hildawu@realtek.com>
> Signed-off-by: Max Chou <max.chou@realtek.com>
> ---
> 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");
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] Bluetooth: btrtl: Load FW v2 otherwise FW v1 for RTL8852C
2023-08-04 5:54 [PATCH] Bluetooth: btrtl: Load FW v2 otherwise FW v1 for RTL8852C max.chou
2023-08-04 6:42 ` bluez.test.bot
2023-08-04 8:51 ` [PATCH] " Juerg Haefliger
@ 2023-08-04 8:54 ` Linux regression tracking (Thorsten Leemhuis)
2023-08-04 9:14 ` You-Sheng Yang
[not found] ` <202308040950.3749oOuG5031679@rtits1.realtek.com.tw>
4 siblings, 0 replies; 9+ messages in thread
From: Linux regression tracking (Thorsten Leemhuis) @ 2023-08-04 8:54 UTC (permalink / raw)
To: max.chou, marcel
Cc: johan.hedberg, luiz.dentz, linux-bluetooth, linux-kernel, alex_lu,
hildawu, karenhsu, kidman, juerg.haefliger, vicamo.yang,
Riley.Kao, Josh Boyer, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Linux kernel regressions list
[CCing the linux-firmware maintainer, the network maintainers, and the
regressions lists]
On 04.08.23 07:54, max.chou@realtek.com wrote:
> From: Max Chou <max.chou@realtek.com>
>
> 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/commit/?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/commit/?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
```
> 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?
> Suggested-by: Juerg Haefliger <juerg.haefliger@canonical.com>
> Tested-by: Hilda Wu <hildawu@realtek.com>
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: stable@vger.kernel.org
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 <max.chou@realtek.com>
> ---
> 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");
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] Bluetooth: btrtl: Load FW v2 otherwise FW v1 for RTL8852C
2023-08-04 5:54 [PATCH] Bluetooth: btrtl: Load FW v2 otherwise FW v1 for RTL8852C max.chou
` (2 preceding siblings ...)
2023-08-04 8:54 ` Linux regression tracking (Thorsten Leemhuis)
@ 2023-08-04 9:14 ` You-Sheng Yang
2023-08-04 9:30 ` Max Chou
[not found] ` <202308040950.3749oOuG5031679@rtits1.realtek.com.tw>
4 siblings, 1 reply; 9+ messages in thread
From: You-Sheng Yang @ 2023-08-04 9:14 UTC (permalink / raw)
To: max.chou
Cc: marcel, johan.hedberg, luiz.dentz, linux-bluetooth, linux-kernel,
alex_lu, hildawu, karenhsu, kidman, juerg.haefliger, Riley.Kao
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 <max.chou@realtek.com> wrote:
>
> From: Max Chou <max.chou@realtek.com>
>
> 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 <juerg.haefliger@canonical.com>
> Tested-by: Hilda Wu <hildawu@realtek.com>
> Signed-off-by: Max Chou <max.chou@realtek.com>
> ---
> 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
^ permalink raw reply [flat|nested] 9+ messages in thread
* RE: [PATCH] Bluetooth: btrtl: Load FW v2 otherwise FW v1 for RTL8852C
2023-08-04 9:14 ` You-Sheng Yang
@ 2023-08-04 9:30 ` Max Chou
0 siblings, 0 replies; 9+ messages in thread
From: Max Chou @ 2023-08-04 9:30 UTC (permalink / raw)
To: You-Sheng Yang
Cc: marcel@holtmann.org, johan.hedberg@gmail.com,
luiz.dentz@gmail.com, linux-bluetooth@vger.kernel.org,
linux-kernel@vger.kernel.org, alex_lu@realsil.com.cn, Hilda Wu,
Karen Hsu, KidmanLee, juerg.haefliger@canonical.com,
Riley.Kao@dell.com
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 <vicamo.yang@canonical.com>
> Sent: Friday, August 4, 2023 5:14 PM
> To: Max Chou <max.chou@realtek.com>
> Cc: marcel@holtmann.org; johan.hedberg@gmail.com; luiz.dentz@gmail.com;
> linux-bluetooth@vger.kernel.org; linux-kernel@vger.kernel.org;
> alex_lu@realsil.com.cn; Hilda Wu <hildawu@realtek.com>; Karen Hsu
> <karenhsu@realtek.com>; KidmanLee <kidman@realtek.com>;
> juerg.haefliger@canonical.com; Riley.Kao@dell.com
> 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 <max.chou@realtek.com> wrote:
> >
> > From: Max Chou <max.chou@realtek.com>
> >
> > 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 <juerg.haefliger@canonical.com>
> > Tested-by: Hilda Wu <hildawu@realtek.com>
> > Signed-off-by: Max Chou <max.chou@realtek.com>
> > ---
> > 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.
^ permalink raw reply [flat|nested] 9+ messages in thread
* RE: [PATCH] Bluetooth: btrtl: Load FW v2 otherwise FW v1 for RTL8852C
[not found] ` <202308040950.3749oOuG5031679@rtits1.realtek.com.tw>
@ 2023-08-04 10:15 ` Max Chou
2023-08-04 10:31 ` Linux regression tracking (Thorsten Leemhuis)
2023-08-04 10:28 ` Max Chou
1 sibling, 1 reply; 9+ messages in thread
From: Max Chou @ 2023-08-04 10:15 UTC (permalink / raw)
To: Linux regressions mailing list, marcel@holtmann.org
Cc: johan.hedberg@gmail.com, luiz.dentz@gmail.com,
linux-bluetooth@vger.kernel.org, linux-kernel@vger.kernel.org,
alex_lu@realsil.com.cn, Hilda Wu, Karen Hsu, KidmanLee,
juerg.haefliger@canonical.com, vicamo.yang@canonical.com,
Riley.Kao@dell.com, Josh Boyer, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni
Dear All,
Please check my comment inline. Thanks.
BRs,
Max
> -----Original Message-----
> From: Linux regression tracking (Thorsten Leemhuis)
> <regressions@leemhuis.info>
> Sent: Friday, August 4, 2023 4:55 PM
> To: Max Chou <max.chou@realtek.com>; marcel@holtmann.org
> Cc: johan.hedberg@gmail.com; luiz.dentz@gmail.com;
> linux-bluetooth@vger.kernel.org; linux-kernel@vger.kernel.org;
> alex_lu@realsil.com.cn; Hilda Wu <hildawu@realtek.com>; Karen Hsu
> <karenhsu@realtek.com>; KidmanLee <kidman@realtek.com>;
> juerg.haefliger@canonical.com; vicamo.yang@canonical.com;
> Riley.Kao@dell.com; Josh Boyer <jwboyer@kernel.org>; David S. Miller
> <davem@davemloft.net>; Eric Dumazet <edumazet@google.com>; Jakub
> Kicinski <kuba@kernel.org>; Paolo Abeni <pabeni@redhat.com>; Linux kernel
> regressions list <regressions@lists.linux.dev>
> 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, max.chou@realtek.com wrote:
> > From: Max Chou <max.chou@realtek.com>
> >
> > 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 <juerg.haefliger@canonical.com>
> > Tested-by: Hilda Wu <hildawu@realtek.com>
>
> 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: stable@vger.kernel.org
>
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 <max.chou@realtek.com>
> > ---
> > 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");
^ permalink raw reply [flat|nested] 9+ messages in thread
* RE: [PATCH] Bluetooth: btrtl: Load FW v2 otherwise FW v1 for RTL8852C
[not found] ` <202308040950.3749oOuG5031679@rtits1.realtek.com.tw>
2023-08-04 10:15 ` Max Chou
@ 2023-08-04 10:28 ` Max Chou
1 sibling, 0 replies; 9+ messages in thread
From: Max Chou @ 2023-08-04 10:28 UTC (permalink / raw)
To: Linux regressions mailing list, marcel@holtmann.org
Cc: johan.hedberg@gmail.com, luiz.dentz@gmail.com,
linux-bluetooth@vger.kernel.org, linux-kernel@vger.kernel.org,
alex_lu@realsil.com.cn, Hilda Wu, Karen Hsu, KidmanLee,
juerg.haefliger@canonical.com, vicamo.yang@canonical.com,
Riley.Kao@dell.com, Josh Boyer, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni
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)
> <regressions@leemhuis.info>
> Sent: Friday, August 4, 2023 4:55 PM
> To: Max Chou <max.chou@realtek.com>; marcel@holtmann.org
> Cc: johan.hedberg@gmail.com; luiz.dentz@gmail.com;
> linux-bluetooth@vger.kernel.org; linux-kernel@vger.kernel.org;
> alex_lu@realsil.com.cn; Hilda Wu <hildawu@realtek.com>; Karen Hsu
> <karenhsu@realtek.com>; KidmanLee <kidman@realtek.com>;
> juerg.haefliger@canonical.com; vicamo.yang@canonical.com;
> Riley.Kao@dell.com; Josh Boyer <jwboyer@kernel.org>; David S. Miller
> <davem@davemloft.net>; Eric Dumazet <edumazet@google.com>; Jakub
> Kicinski <kuba@kernel.org>; Paolo Abeni <pabeni@redhat.com>; Linux kernel
> regressions list <regressions@lists.linux.dev>
> 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, max.chou@realtek.com wrote:
> > From: Max Chou <max.chou@realtek.com>
> >
> > 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 <juerg.haefliger@canonical.com>
> > Tested-by: Hilda Wu <hildawu@realtek.com>
>
> 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: stable@vger.kernel.org
>
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 <max.chou@realtek.com>
> > ---
> > 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");
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] Bluetooth: btrtl: Load FW v2 otherwise FW v1 for RTL8852C
2023-08-04 10:15 ` Max Chou
@ 2023-08-04 10:31 ` Linux regression tracking (Thorsten Leemhuis)
0 siblings, 0 replies; 9+ messages in thread
From: Linux regression tracking (Thorsten Leemhuis) @ 2023-08-04 10:31 UTC (permalink / raw)
To: Max Chou, Linux regressions mailing list, marcel@holtmann.org
Cc: johan.hedberg@gmail.com, luiz.dentz@gmail.com,
linux-bluetooth@vger.kernel.org, linux-kernel@vger.kernel.org,
alex_lu@realsil.com.cn, Hilda Wu, Karen Hsu, KidmanLee,
juerg.haefliger@canonical.com, vicamo.yang@canonical.com,
Riley.Kao@dell.com, Josh Boyer, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni
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, max.chou@realtek.com wrote:
>>> From: Max Chou <max.chou@realtek.com>
>>>
>>> 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
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2023-08-04 10:32 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-08-04 5:54 [PATCH] Bluetooth: btrtl: Load FW v2 otherwise FW v1 for RTL8852C max.chou
2023-08-04 6:42 ` bluez.test.bot
2023-08-04 8:51 ` [PATCH] " Juerg Haefliger
2023-08-04 8:54 ` Linux regression tracking (Thorsten Leemhuis)
2023-08-04 9:14 ` You-Sheng Yang
2023-08-04 9:30 ` Max Chou
[not found] ` <202308040950.3749oOuG5031679@rtits1.realtek.com.tw>
2023-08-04 10:15 ` Max Chou
2023-08-04 10:31 ` Linux regression tracking (Thorsten Leemhuis)
2023-08-04 10:28 ` Max Chou
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox