| Message ID | 20230804055426.6806-1-max.chou@realtek.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Bluetooth: btrtl: Load FW v2 otherwise FW v1 for RTL8852C | expand |
| Context | Check | Description |
|---|---|---|
| tedd_an/pre-ci_am | success | Success |
| tedd_an/CheckPatch | success | CheckPatch PASS |
| tedd_an/GitLint | success | Gitlint PASS |
| tedd_an/SubjectPrefix | success | Gitlint PASS |
| tedd_an/BuildKernel | success | BuildKernel PASS |
| tedd_an/CheckAllWarning | success | CheckAllWarning PASS |
| tedd_an/CheckSparse | success | CheckSparse PASS |
| tedd_an/CheckSmatch | success | CheckSparse PASS |
| tedd_an/BuildKernel32 | success | BuildKernel32 PASS |
| tedd_an/TestRunnerSetup | success | TestRunnerSetup PASS |
| tedd_an/TestRunner_l2cap-tester | success | TestRunner PASS |
| tedd_an/TestRunner_iso-tester | success | TestRunner PASS |
| tedd_an/TestRunner_bnep-tester | success | TestRunner PASS |
| tedd_an/TestRunner_mgmt-tester | success | TestRunner PASS |
| tedd_an/TestRunner_rfcomm-tester | success | TestRunner PASS |
| tedd_an/TestRunner_sco-tester | success | TestRunner PASS |
| tedd_an/TestRunner_ioctl-tester | success | TestRunner PASS |
| tedd_an/TestRunner_mesh-tester | success | TestRunner PASS |
| tedd_an/TestRunner_smp-tester | success | TestRunner PASS |
| tedd_an/TestRunner_userchan-tester | success | TestRunner PASS |
| tedd_an/IncrementalBuild | success | Incremental Build PASS |
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
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");
[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");
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
>
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.
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");
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");
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
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");