All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v3] Bluetooth: btrtl: Load FW v2 otherwise FW v1 for RTL8852C
@ 2023-08-07  1:44 max.chou
  2023-08-07  1:46 ` kernel test robot
                   ` (2 more replies)
  0 siblings, 3 replies; 5+ messages in thread
From: max.chou @ 2023-08-07  1:44 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, stable, jwboyer, davem, edumazet, kuba, pabeni,
	regressions

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.

Note that the driver has supported to extract the data for v1 and v2
since the commit[1].
The previous FW format of RTL8852C is v1. After the commit[2], the FW
format was changed to v2. Only RTL8852C suffered the different FW formats,
so we will use rtl8852cu_fw.bin for the original commit as FW v1 and
rtl8852cu_fw_v2.bin for the future maintained as FW v2. Other Realtek
chips will not been impacted by this patch.

To do on linux-firmware project after this commit.
1. revert '55e7448533e7 ("rtl_bt: Update RTL8852C BT USB firmware
   to 0x040D_7225")'
   => rtl_bt/rtl8852cu_fw.bin: FW v1 (stay at ver. 0xD7B8_FABF)
2. Add a new commit for rtl8852cu_fw_v2.bin
   =>rtl_bt/rtl8852cu_fw_v2.bin: FW v2 (to be maintained)

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

Fixes: '9a24ce5e29b ("Bluetooth: btrtl: Firmware format v2 support")'
Suggested-by: Juerg Haefliger <juerg.haefliger@canonical.com>
Tested-by: Hilda Wu <hildawu@realtek.com>
Signed-off-by: Max Chou <max.chou@realtek.com>

---
Changes in v2:
- Fix commit log for CheckPatch FAIL

Changes in v2:
- Tuning the code for more readable. Thanks Juerg!
- Modify the commit log.
---
 drivers/bluetooth/btrtl.c | 70 +++++++++++++++++++++++++--------------
 1 file changed, 45 insertions(+), 25 deletions(-)

diff --git a/drivers/bluetooth/btrtl.c b/drivers/bluetooth/btrtl.c
index ddae6524106d..84c2c2e1122f 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,6 +1045,7 @@ 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;
@@ -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);
@@ -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] 5+ messages in thread

* Re: [PATCH v3] Bluetooth: btrtl: Load FW v2 otherwise FW v1 for RTL8852C
  2023-08-07  1:44 [PATCH v3] Bluetooth: btrtl: Load FW v2 otherwise FW v1 for RTL8852C max.chou
@ 2023-08-07  1:46 ` kernel test robot
  2023-08-07  2:26 ` [v3] " bluez.test.bot
  2023-08-07 10:06 ` [PATCH v3] " Juerg Haefliger
  2 siblings, 0 replies; 5+ messages in thread
From: kernel test robot @ 2023-08-07  1:46 UTC (permalink / raw)
  To: max.chou; +Cc: stable, oe-kbuild-all

Hi,

Thanks for your patch.

FYI: kernel test robot notices the stable kernel rule is not satisfied.

Rule: 'Cc: stable@vger.kernel.org' or 'commit <sha1> upstream.'
Subject: [PATCH v3] Bluetooth: btrtl: Load FW v2 otherwise FW v1 for RTL8852C
Link: https://lore.kernel.org/stable/20230807014415.12358-1-max.chou%40realtek.com

The check is based on https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki




^ permalink raw reply	[flat|nested] 5+ messages in thread

* RE: [v3] Bluetooth: btrtl: Load FW v2 otherwise FW v1 for RTL8852C
  2023-08-07  1:44 [PATCH v3] Bluetooth: btrtl: Load FW v2 otherwise FW v1 for RTL8852C max.chou
  2023-08-07  1:46 ` kernel test robot
@ 2023-08-07  2:26 ` bluez.test.bot
  2023-08-07 10:06 ` [PATCH v3] " Juerg Haefliger
  2 siblings, 0 replies; 5+ messages in thread
From: bluez.test.bot @ 2023-08-07  2:26 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=773464

---Test result---

Test Summary:
CheckPatch                    PASS      0.76 seconds
GitLint                       PASS      0.35 seconds
SubjectPrefix                 PASS      0.12 seconds
BuildKernel                   PASS      34.58 seconds
CheckAllWarning               PASS      37.81 seconds
CheckSparse                   PASS      43.01 seconds
CheckSmatch                   PASS      114.49 seconds
BuildKernel32                 PASS      32.61 seconds
TestRunnerSetup               PASS      497.78 seconds
TestRunner_l2cap-tester       PASS      23.96 seconds
TestRunner_iso-tester         PASS      47.03 seconds
TestRunner_bnep-tester        PASS      10.81 seconds
TestRunner_mgmt-tester        PASS      218.10 seconds
TestRunner_rfcomm-tester      PASS      16.38 seconds
TestRunner_sco-tester         PASS      19.54 seconds
TestRunner_ioctl-tester       PASS      18.47 seconds
TestRunner_mesh-tester        PASS      13.67 seconds
TestRunner_smp-tester         PASS      14.72 seconds
TestRunner_userchan-tester    PASS      11.40 seconds
IncrementalBuild              PASS      31.59 seconds



---
Regards,
Linux Bluetooth


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v3] Bluetooth: btrtl: Load FW v2 otherwise FW v1 for RTL8852C
  2023-08-07  1:44 [PATCH v3] Bluetooth: btrtl: Load FW v2 otherwise FW v1 for RTL8852C max.chou
  2023-08-07  1:46 ` kernel test robot
  2023-08-07  2:26 ` [v3] " bluez.test.bot
@ 2023-08-07 10:06 ` Juerg Haefliger
  2023-08-07 10:40   ` Linux regression tracking (Thorsten Leemhuis)
  2 siblings, 1 reply; 5+ messages in thread
From: Juerg Haefliger @ 2023-08-07 10:06 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,
	stable, jwboyer, davem, edumazet, kuba, pabeni, regressions

[-- Attachment #1: Type: text/plain, Size: 12199 bytes --]

On Mon, 7 Aug 2023 09:44:15 +0800
<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.
> 
> Note that the driver has supported to extract the data for v1 and v2
> since the commit[1].
> The previous FW format of RTL8852C is v1. After the commit[2], the FW
> format was changed to v2. Only RTL8852C suffered the different FW formats,
> so we will use rtl8852cu_fw.bin for the original commit as FW v1 and
> rtl8852cu_fw_v2.bin for the future maintained as FW v2. Other Realtek
> chips will not been impacted by this patch.
> 
> To do on linux-firmware project after this commit.
> 1. revert '55e7448533e7 ("rtl_bt: Update RTL8852C BT USB firmware
>    to 0x040D_7225")'
>    => rtl_bt/rtl8852cu_fw.bin: FW v1 (stay at ver. 0xD7B8_FABF)  
> 2. Add a new commit for rtl8852cu_fw_v2.bin
>    =>rtl_bt/rtl8852cu_fw_v2.bin: FW v2 (to be maintained)  
> 
> Reference:
> [1]'9a24ce5e29b1 ("Bluetooth: btrtl: Firmware format v2 support")'
> [2]'55e7448533e7 ("rtl_bt: Update RTL8852C BT USB firmware
>     to 0x040D_7225")'

IMO this commit message is too confusing. I don't think you need nor should
refer to linux-firmware commits and what happened in that repo. This commit
simply fixes the v2 support by trying to load a v2 file for certain chips
with a fall-back if fw is not found (or failed to load). That's all that
needs to be mentioned in the commit message.

> Fixes: '9a24ce5e29b ("Bluetooth: btrtl: Firmware format v2 support")'

No '' quotes.

...Juerg


> Suggested-by: Juerg Haefliger <juerg.haefliger@canonical.com>
> Tested-by: Hilda Wu <hildawu@realtek.com>
> Signed-off-by: Max Chou <max.chou@realtek.com>
> 
> ---
> Changes in v2:
> - Fix commit log for CheckPatch FAIL
> 
> Changes in v2:
> - Tuning the code for more readable. Thanks Juerg!
> - Modify the commit log.
> ---
>  drivers/bluetooth/btrtl.c | 70 +++++++++++++++++++++++++--------------
>  1 file changed, 45 insertions(+), 25 deletions(-)
> 
> diff --git a/drivers/bluetooth/btrtl.c b/drivers/bluetooth/btrtl.c
> index ddae6524106d..84c2c2e1122f 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,6 +1045,7 @@ 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;
> @@ -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);
> @@ -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] 5+ messages in thread

* Re: [PATCH v3] Bluetooth: btrtl: Load FW v2 otherwise FW v1 for RTL8852C
  2023-08-07 10:06 ` [PATCH v3] " Juerg Haefliger
@ 2023-08-07 10:40   ` Linux regression tracking (Thorsten Leemhuis)
  0 siblings, 0 replies; 5+ messages in thread
From: Linux regression tracking (Thorsten Leemhuis) @ 2023-08-07 10:40 UTC (permalink / raw)
  To: Juerg Haefliger, max.chou
  Cc: marcel, johan.hedberg, luiz.dentz, linux-bluetooth, linux-kernel,
	alex_lu, hildawu, karenhsu, kidman, vicamo.yang, Riley.Kao,
	stable, jwboyer, davem, edumazet, kuba, pabeni, regressions

On 07.08.23 12:06, Juerg Haefliger wrote:
> On Mon, 7 Aug 2023 09:44:15 +0800
> <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.
>>
>> Note that the driver has supported to extract the data for v1 and v2
>> since the commit[1].
>> The previous FW format of RTL8852C is v1. After the commit[2], the FW
>> format was changed to v2. Only RTL8852C suffered the different FW formats,
>> so we will use rtl8852cu_fw.bin for the original commit as FW v1 and
>> rtl8852cu_fw_v2.bin for the future maintained as FW v2. Other Realtek
>> chips will not been impacted by this patch.
>>
>> To do on linux-firmware project after this commit.
>> 1. revert '55e7448533e7 ("rtl_bt: Update RTL8852C BT USB firmware
>>    to 0x040D_7225")'
>>    => rtl_bt/rtl8852cu_fw.bin: FW v1 (stay at ver. 0xD7B8_FABF)  
>> 2. Add a new commit for rtl8852cu_fw_v2.bin
>>    =>rtl_bt/rtl8852cu_fw_v2.bin: FW v2 (to be maintained)  
>>
>> Reference:
>> [1]'9a24ce5e29b1 ("Bluetooth: btrtl: Firmware format v2 support")'
>> [2]'55e7448533e7 ("rtl_bt: Update RTL8852C BT USB firmware
>>     to 0x040D_7225")'
> 
> IMO this commit message is too confusing. I don't think you need nor should
> refer to linux-firmware commits and what happened in that repo. This commit
> simply fixes the v2 support by trying to load a v2 file for certain chips
> with a fall-back if fw is not found (or failed to load). That's all that
> needs to be mentioned in the commit message.

/me totally agrees that this in confusing the way it is and needs to be
simplified, but explaining things briefly with maybe a Link: to the
linux-firmware commit in question might nevertheless be a good idea (YMMV)

>> Fixes: '9a24ce5e29b ("Bluetooth: btrtl: Firmware format v2 support")'
> 
> No '' quotes.

Max Chou, you could have just picked the two tags I mentioned earlier
just without the space in front of it[1]. If you do so the 0-day robot
will also stop complaining, as you apparently misunderstood what I meant
with stable tag; [2] explains this.

Ciao, Thorsten

[1]
https://lore.kernel.org/all/f11ba0ef-0bb1-b925-3088-2ba7a8d17393@leemhuis.info/
[2] https://docs.kernel.org/process/stable-kernel-rules.html

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2023-08-07 10:41 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-08-07  1:44 [PATCH v3] Bluetooth: btrtl: Load FW v2 otherwise FW v1 for RTL8852C max.chou
2023-08-07  1:46 ` kernel test robot
2023-08-07  2:26 ` [v3] " bluez.test.bot
2023-08-07 10:06 ` [PATCH v3] " Juerg Haefliger
2023-08-07 10:40   ` Linux regression tracking (Thorsten Leemhuis)

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.