2023-01-19 07:58:59

by Max Chou

[permalink] [raw]
Subject: [PATCH 1/1] Bluetooth: btrtl: Firmware format v2 support

From: Max Chou <[email protected]>

Realtek changes the format for the firmware file as v2. The driver
should implement the patch to extract the firmware data from the
firmware file.
It's compatible with the both previous format(v1) and new format(v2).

Signed-off-by: Allen Chen <[email protected]>
Signed-off-by: Alex Lu <[email protected]>
Tested-by: Hilda Wu <[email protected]>
Signed-off-by: Max Chou <[email protected]>
---
drivers/bluetooth/btrtl.c | 364 +++++++++++++++++++++++++++++++++-----
drivers/bluetooth/btrtl.h | 61 +++++++
2 files changed, 385 insertions(+), 40 deletions(-)

diff --git a/drivers/bluetooth/btrtl.c b/drivers/bluetooth/btrtl.c
index 69c3fe649ca7..2277cb4e50c2 100644
--- a/drivers/bluetooth/btrtl.c
+++ b/drivers/bluetooth/btrtl.c
@@ -18,6 +18,7 @@
#define VERSION "0.1"

#define RTL_EPATCH_SIGNATURE "Realtech"
+#define RTL_EPATCH_SIGNATURE_V2 "RTBTCore"
#define RTL_ROM_LMP_8723A 0x1200
#define RTL_ROM_LMP_8723B 0x8723
#define RTL_ROM_LMP_8821A 0x8821
@@ -38,6 +39,14 @@
.hci_ver = (hciv), \
.hci_bus = (bus)

+#define RTL_CHIP_SUBVER (&(struct rtl_vendor_cmd) {{0x10, 0x38, 0x04, 0x28, 0x80}})
+#define RTL_CHIP_REV (&(struct rtl_vendor_cmd) {{0x10, 0x3A, 0x04, 0x28, 0x80}})
+#define RTL_SEC_PROJ (&(struct rtl_vendor_cmd) {{0x10, 0xA4, 0xAD, 0x00, 0xb0}})
+
+#define RTL_PATCH_SNIPPETS 0x01
+#define RTL_PATCH_DUMMY_HEADER 0x02
+#define RTL_PATCH_SECURITY_HEADER 0x03
+
enum btrtl_chip_id {
CHIP_ID_8723A,
CHIP_ID_8723B,
@@ -75,6 +84,8 @@ struct btrtl_device_info {
int cfg_len;
bool drop_fw;
int project_id;
+ u8 key_id;
+ struct list_head patch_subsecs;
};

static const struct id_table ic_id_table[] = {
@@ -284,6 +295,242 @@ static int rtl_read_rom_version(struct hci_dev *hdev, u8 *version)
return 0;
}

+
+static int btrtl_vendor_read_reg16(struct hci_dev *hdev,
+ struct rtl_vendor_cmd *cmd, u8 *rp)
+{
+ struct sk_buff *skb;
+ int err = 0;
+
+ skb = __hci_cmd_sync(hdev, 0xfc61, sizeof(*cmd), cmd,
+ HCI_INIT_TIMEOUT);
+ if (IS_ERR(skb)) {
+ err = PTR_ERR(skb);
+ rtl_dev_err(hdev, "RTL: Read reg16 failed (%d)", err);
+ return err;
+ }
+
+ if (skb->len != 3 || skb->data[0]) {
+ bt_dev_err(hdev, "RTL: Read reg16 length mismatch");
+ kfree_skb(skb);
+ return -EIO;
+ }
+
+ if (rp)
+ memcpy(rp, skb->data + 1, 2);
+
+ kfree_skb(skb);
+
+ return 0;
+}
+
+static void btrtl_insert_ordered_subsec(struct rtl_subsection *node,
+ struct btrtl_device_info *btrtl_dev)
+{
+ struct list_head *pos;
+ struct list_head *next;
+ struct rtl_subsection *subsec;
+
+ list_for_each_safe(pos, next, &btrtl_dev->patch_subsecs) {
+ subsec = list_entry(pos, struct rtl_subsection, list);
+ if (subsec->prio >= node->prio)
+ break;
+ }
+ __list_add(&node->list, pos->prev, pos);
+}
+
+static int btrtl_parse_section(struct hci_dev *hdev,
+ struct btrtl_device_info *btrtl_dev, u32 opcode,
+ u8 *data, u32 len)
+{
+ struct rtl_section_hdr *hdr;
+ struct rtl_subsection *subsec;
+ struct rtl_common_subsec *common_subsec;
+ struct rtl_sec_hdr *sec_hdr;
+ int i;
+ u8 *ptr;
+ u8 *end = data + len;
+ u16 num_subsecs;
+ u32 subsec_len;
+ int rc = 0;
+
+ if (sizeof(*hdr) > len)
+ return -EINVAL;
+
+ hdr = (void *)data;
+ ptr = data + sizeof(*hdr);
+ num_subsecs = le16_to_cpu(hdr->num);
+ for (i = 0; i < num_subsecs; i++) {
+ common_subsec = (void *)ptr;
+ if (ptr + sizeof(*common_subsec) > end)
+ break;
+ ptr += sizeof(*common_subsec);
+
+ subsec_len = le32_to_cpu(common_subsec->len);
+ if (ptr + subsec_len > end)
+ break;
+ ptr += subsec_len;
+ }
+
+ if (i < num_subsecs)
+ return -EOVERFLOW;
+
+ ptr = data + sizeof(*hdr);
+ for (i = 0; i < num_subsecs; i++) {
+ common_subsec = (void *)ptr;
+ subsec_len = le32_to_cpu(common_subsec->len);
+ ptr += sizeof(*common_subsec);
+
+ if (common_subsec->eco != btrtl_dev->rom_version + 1) {
+ ptr += subsec_len;
+ continue;
+ }
+
+ switch (opcode) {
+ case RTL_PATCH_SECURITY_HEADER:
+ sec_hdr = (void *)common_subsec;
+ if (sec_hdr->key_id != btrtl_dev->key_id) {
+ ptr += subsec_len;
+ continue;
+ }
+ break;
+ }
+
+ subsec = kzalloc(sizeof(*subsec), GFP_KERNEL);
+ if (!subsec)
+ return -ENOMEM;
+ subsec->prio = common_subsec->prio;
+ subsec->len = subsec_len;
+ subsec->data = ptr;
+ btrtl_insert_ordered_subsec(subsec, btrtl_dev);
+
+ ptr += subsec_len;
+ rc += subsec_len;
+ }
+
+ return rc;
+}
+
+static int rtlbt_parse_firmware_v2(struct hci_dev *hdev,
+ struct btrtl_device_info *btrtl_dev,
+ unsigned char **_buf)
+{
+ struct rtl_epatch_header_v2 *hdr;
+ int rc;
+ u8 reg_val[2];
+ u8 key_id;
+ u32 num_sections;
+ struct rtl_section *section;
+ struct rtl_subsection *entry, *tmp;
+ u32 section_len;
+ u32 opcode;
+ int len = 0;
+ int i;
+ int secure = 0;
+ int dummy = 0;
+ u8 *ptr;
+ /* Cut the tail of the firmware. */
+ u8 *end = btrtl_dev->fw_data + btrtl_dev->fw_len - 7;
+
+ rc = btrtl_vendor_read_reg16(hdev, RTL_SEC_PROJ, reg_val);
+ if (rc < 0)
+ return -EIO;
+
+ key_id = reg_val[0];
+ rtl_dev_dbg(hdev, "%s: key id %u", __func__, key_id);
+ btrtl_dev->key_id = key_id;
+
+ hdr = (struct rtl_epatch_header_v2 *)btrtl_dev->fw_data;
+ num_sections = le32_to_cpu(hdr->num_sections);
+
+ rtl_dev_dbg(hdev, "FW version %08x-%08x", *((u32 *)hdr->fw_version),
+ *((u32 *)(hdr->fw_version + 4)));
+
+ ptr = btrtl_dev->fw_data + sizeof(*hdr);
+ for (i = 0; i < num_sections; i++) {
+ section = (void *)ptr;
+ if (ptr + sizeof(*section) > end)
+ break;
+ ptr += sizeof(*section);
+
+ section_len = le32_to_cpu(section->len);
+ if (ptr + section_len > end)
+ break;
+ ptr += section_len;
+ }
+
+ if (i < num_sections)
+ return -EOVERFLOW;
+
+ ptr = btrtl_dev->fw_data + sizeof(*hdr);
+ for (i = 0; i < num_sections; i++) {
+ section = (void *)ptr;
+ rtl_dev_dbg(hdev, "opcode 0x%04x", section->opcode);
+ ptr += sizeof(*section);
+
+ section_len = le32_to_cpu(section->len);
+ opcode = le32_to_cpu(section->opcode);
+ switch (opcode) {
+ case RTL_PATCH_SNIPPETS:
+ rc = btrtl_parse_section(hdev, btrtl_dev, opcode, ptr,
+ section_len);
+ break;
+ case RTL_PATCH_SECURITY_HEADER:
+ /* If key_id from chip is zero, ignore all security
+ * headers.
+ */
+ if (!key_id)
+ break;
+ rc = btrtl_parse_section(hdev, btrtl_dev, opcode, ptr,
+ section_len);
+ if (rc > 0)
+ secure += rc;
+
+ break;
+ case RTL_PATCH_DUMMY_HEADER:
+ if (key_id)
+ break;
+ rc = btrtl_parse_section(hdev, btrtl_dev, opcode, ptr,
+ section_len);
+ if (rc > 0)
+ dummy += rc;
+ break;
+ default:
+ rc = 0;
+ break;
+ }
+ if (rc < 0) {
+ rtl_dev_err(hdev, "RTL: Parse section (%u) err %d",
+ opcode, rc);
+ return rc;
+ }
+ len += rc;
+ ptr += section_len;
+ }
+
+ if (!len)
+ return -ENODATA;
+
+ bt_dev_info(hdev, "RTL: Patch (len %d) found", len);
+
+ /* Allocate mem and copy all found subsecs. */
+ ptr = kvmalloc(len, GFP_KERNEL);
+ if (!ptr)
+ return -ENOMEM;
+
+ len = 0;
+ list_for_each_entry_safe(entry, tmp, &btrtl_dev->patch_subsecs, list) {
+ rtl_dev_dbg(hdev, "RTL: len 0x%x", entry->len);
+ memcpy(ptr + len, entry->data, entry->len);
+ len += entry->len;
+ list_del(&entry->list);
+ kfree(entry);
+ }
+
+ *_buf = ptr;
+ return len;
+}
+
static int rtlbt_parse_firmware(struct hci_dev *hdev,
struct btrtl_device_info *btrtl_dev,
unsigned char **_buf)
@@ -317,7 +564,18 @@ static int rtlbt_parse_firmware(struct hci_dev *hdev,
{ RTL_ROM_LMP_8852A, 25 }, /* 8852C */
};

- min_size = sizeof(struct rtl_epatch_header) + sizeof(extension_sig) + 3;
+ if (btrtl_dev->fw_len <= 8)
+ return -EINVAL;
+
+ if (!memcmp(btrtl_dev->fw_data, RTL_EPATCH_SIGNATURE, 8))
+ min_size = sizeof(struct rtl_epatch_header) +
+ sizeof(extension_sig) + 3;
+ else if (!memcmp(btrtl_dev->fw_data, RTL_EPATCH_SIGNATURE_V2, 8))
+ min_size = sizeof(struct rtl_epatch_header_v2) +
+ sizeof(extension_sig) + 3;
+ else
+ return -EINVAL;
+
if (btrtl_dev->fw_len < min_size)
return -EINVAL;

@@ -382,12 +640,14 @@ static int rtlbt_parse_firmware(struct hci_dev *hdev,
return -EINVAL;
}

- epatch_info = (struct rtl_epatch_header *)btrtl_dev->fw_data;
- if (memcmp(epatch_info->signature, RTL_EPATCH_SIGNATURE, 8) != 0) {
+ if (memcmp(btrtl_dev->fw_data, RTL_EPATCH_SIGNATURE, 8) != 0) {
+ if (!memcmp(btrtl_dev->fw_data, RTL_EPATCH_SIGNATURE_V2, 8))
+ return rtlbt_parse_firmware_v2(hdev, btrtl_dev, _buf);
rtl_dev_err(hdev, "bad EPATCH signature");
return -EINVAL;
}

+ epatch_info = (struct rtl_epatch_header *)btrtl_dev->fw_data;
num_patches = le16_to_cpu(epatch_info->num_patches);
BT_DBG("fw_version=%x, num_patches=%d",
le32_to_cpu(epatch_info->fw_version), num_patches);
@@ -451,6 +711,7 @@ static int rtl_download_firmware(struct hci_dev *hdev,
int frag_len = RTL_FRAG_LEN;
int ret = 0;
int i;
+ int j = 0;
struct sk_buff *skb;
struct hci_rp_read_local_version *rp;

@@ -461,17 +722,16 @@ static int rtl_download_firmware(struct hci_dev *hdev,
for (i = 0; i < frag_num; i++) {
struct sk_buff *skb;

- BT_DBG("download fw (%d/%d)", i, frag_num);
-
- if (i > 0x7f)
- dl_cmd->index = (i & 0x7f) + 1;
- else
- dl_cmd->index = i;
+ dl_cmd->index = j++;
+ if (dl_cmd->index == 0x7f)
+ j = 1;

if (i == (frag_num - 1)) {
dl_cmd->index |= 0x80; /* data end */
frag_len = fw_len % RTL_FRAG_LEN;
}
+ rtl_dev_dbg(hdev, "download fw (%d/%d). index = %d", i,
+ frag_num, dl_cmd->index);
memcpy(dl_cmd->data, data, frag_len);

/* Send download command */
@@ -589,8 +849,16 @@ static int btrtl_setup_rtl8723b(struct hci_dev *hdev,

void btrtl_free(struct btrtl_device_info *btrtl_dev)
{
+ struct rtl_subsection *entry, *tmp;
+
kvfree(btrtl_dev->fw_data);
kvfree(btrtl_dev->cfg_data);
+
+ list_for_each_entry_safe(entry, tmp, &btrtl_dev->patch_subsecs, list) {
+ list_del(&entry->list);
+ kfree(entry);
+ }
+
kfree(btrtl_dev);
}
EXPORT_SYMBOL_GPL(btrtl_free);
@@ -604,9 +872,11 @@ struct btrtl_device_info *btrtl_initialize(struct hci_dev *hdev,
char cfg_name[40];
u16 hci_rev, lmp_subver;
u8 hci_ver;
+ u8 lmp_ver;
int ret;
u16 opcode;
u8 cmd[2];
+ u8 reg_val[2];

btrtl_dev = kzalloc(sizeof(*btrtl_dev), GFP_KERNEL);
if (!btrtl_dev) {
@@ -614,26 +884,56 @@ struct btrtl_device_info *btrtl_initialize(struct hci_dev *hdev,
goto err_alloc;
}

+ INIT_LIST_HEAD(&btrtl_dev->patch_subsecs);
+
+check_version:
+ ret = btrtl_vendor_read_reg16(hdev, RTL_CHIP_SUBVER, reg_val);
+ if (ret < 0)
+ goto err_free;
+ lmp_subver = le16_to_cpu(*((u16 *)reg_val));
+
+ if (lmp_subver == RTL_ROM_LMP_8822B) {
+ ret = btrtl_vendor_read_reg16(hdev, RTL_CHIP_REV, reg_val);
+ if (ret < 0)
+ goto err_free;
+ hci_rev = le16_to_cpu(*((u16 *)reg_val));
+
+ /* 8822E */
+ if (hci_rev == 0x000e) {
+ hci_ver = 0x0c;
+ lmp_ver = 0x0c;
+ btrtl_dev->ic_info = btrtl_match_ic(lmp_subver, hci_rev,
+ hci_ver, hdev->bus);
+ goto next;
+ }
+ }
+
skb = btrtl_read_local_version(hdev);
if (IS_ERR(skb)) {
ret = PTR_ERR(skb);
goto err_free;
}
-
resp = (struct hci_rp_read_local_version *)skb->data;
- rtl_dev_info(hdev, "examining hci_ver=%02x hci_rev=%04x lmp_ver=%02x lmp_subver=%04x",
- resp->hci_ver, resp->hci_rev,
- resp->lmp_ver, resp->lmp_subver);

- hci_ver = resp->hci_ver;
- hci_rev = le16_to_cpu(resp->hci_rev);
+ hci_ver = resp->hci_ver;
+ hci_rev = le16_to_cpu(resp->hci_rev);
+ lmp_ver = resp->lmp_ver;
lmp_subver = le16_to_cpu(resp->lmp_subver);

+ kfree_skb(skb);
+
btrtl_dev->ic_info = btrtl_match_ic(lmp_subver, hci_rev, hci_ver,
hdev->bus);

- if (!btrtl_dev->ic_info)
+next:
+ rtl_dev_info(hdev, "examining hci_ver=%02x hci_rev=%04x lmp_ver=%02x lmp_subver=%04x",
+ hci_ver, hci_rev,
+ lmp_ver, lmp_subver);
+
+ if (!btrtl_dev->ic_info && !btrtl_dev->drop_fw)
btrtl_dev->drop_fw = true;
+ else
+ btrtl_dev->drop_fw = false;

if (btrtl_dev->drop_fw) {
opcode = hci_opcode_pack(0x3f, 0x66);
@@ -642,41 +942,25 @@ struct btrtl_device_info *btrtl_initialize(struct hci_dev *hdev,

skb = bt_skb_alloc(sizeof(cmd), GFP_KERNEL);
if (!skb)
- goto out_free;
+ goto err_free;

skb_put_data(skb, cmd, sizeof(cmd));
hci_skb_pkt_type(skb) = HCI_COMMAND_PKT;

- hdev->send(hdev, skb);
+ ret = hdev->send(hdev, skb);
+ if (ret < 0) {
+ bt_dev_err(hdev, "sending frame failed (%d)", ret);
+ kfree_skb(skb);
+ goto err_free;
+ }

/* Ensure the above vendor command is sent to controller and
* process has done.
*/
msleep(200);

- /* Read the local version again. Expect to have the vanilla
- * version as cold boot.
- */
- skb = btrtl_read_local_version(hdev);
- if (IS_ERR(skb)) {
- ret = PTR_ERR(skb);
- goto err_free;
- }
-
- resp = (struct hci_rp_read_local_version *)skb->data;
- rtl_dev_info(hdev, "examining hci_ver=%02x hci_rev=%04x lmp_ver=%02x lmp_subver=%04x",
- resp->hci_ver, resp->hci_rev,
- resp->lmp_ver, resp->lmp_subver);
-
- hci_ver = resp->hci_ver;
- hci_rev = le16_to_cpu(resp->hci_rev);
- lmp_subver = le16_to_cpu(resp->lmp_subver);
-
- btrtl_dev->ic_info = btrtl_match_ic(lmp_subver, hci_rev, hci_ver,
- hdev->bus);
+ goto check_version;
}
-out_free:
- kfree_skb(skb);

if (!btrtl_dev->ic_info) {
rtl_dev_info(hdev, "unknown IC info, lmp subver %04x, hci rev %04x, hci ver %04x",
diff --git a/drivers/bluetooth/btrtl.h b/drivers/bluetooth/btrtl.h
index ebf0101c959b..9fbfe323120b 100644
--- a/drivers/bluetooth/btrtl.h
+++ b/drivers/bluetooth/btrtl.h
@@ -47,6 +47,67 @@ struct rtl_vendor_config {
struct rtl_vendor_config_entry entry[];
} __packed;

+struct rtl_epatch_header_v2 {
+ __u8 signature[8];
+ __u8 fw_version[8];
+ __le32 num_sections;
+} __packed;
+
+struct rtl_section {
+ __le32 opcode;
+ __le32 len;
+ u8 data[];
+} __packed;
+
+struct rtl_section_hdr {
+ __le16 num;
+ __le16 reserved;
+} __packed;
+
+struct rtl_common_subsec {
+ __u8 eco;
+ __u8 prio;
+ __u8 cb[2];
+ __le32 len;
+ __u8 data[];
+};
+
+struct rtl_snippet {
+ __u8 eco;
+ __u8 prio;
+ __u16 reserved;
+ __le32 len;
+ __u8 data[];
+} __packed;
+
+struct rtl_dummy_hdr {
+ __u8 eco;
+ __u8 prio;
+ __le16 reserved;
+ __le32 len;
+ __u8 data[];
+} __packed;
+
+struct rtl_sec_hdr {
+ __u8 eco;
+ __u8 prio;
+ __u8 key_id;
+ __u8 reserved;
+ __le32 len;
+ __u8 data[];
+} __packed;
+
+struct rtl_subsection {
+ struct list_head list;
+ u8 prio;
+ u32 len;
+ u8 *data;
+};
+
+struct rtl_vendor_cmd {
+ __u8 param[5];
+} __packed;
+
enum {
REALTEK_ALT6_CONTINUOUS_TX_CHIP,

--
2.34.1


2023-01-19 08:59:40

by bluez.test.bot

[permalink] [raw]
Subject: RE: [1/1] Bluetooth: btrtl: Firmware format v2 support

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=713556

---Test result---

Test Summary:
CheckPatch PASS 1.39 seconds
GitLint PASS 0.25 seconds
SubjectPrefix PASS 0.06 seconds
BuildKernel PASS 38.98 seconds
CheckAllWarning PASS 42.77 seconds
CheckSparse WARNING 49.12 seconds
CheckSmatch WARNING 130.11 seconds
BuildKernel32 PASS 37.73 seconds
TestRunnerSetup PASS 543.53 seconds
TestRunner_l2cap-tester PASS 18.83 seconds
TestRunner_iso-tester PASS 20.04 seconds
TestRunner_bnep-tester PASS 6.81 seconds
TestRunner_mgmt-tester PASS 126.67 seconds
TestRunner_rfcomm-tester PASS 10.59 seconds
TestRunner_sco-tester PASS 9.78 seconds
TestRunner_ioctl-tester PASS 11.47 seconds
TestRunner_mesh-tester PASS 8.51 seconds
TestRunner_smp-tester PASS 9.61 seconds
TestRunner_userchan-tester PASS 7.19 seconds
IncrementalBuild PASS 35.70 seconds

Details
##############################
Test: CheckSparse - WARNING
Desc: Run sparse tool with linux kernel
Output:
drivers/bluetooth/btrtl.c:893:22: warning: cast to restricted __le16drivers/bluetooth/btrtl.c:899:27: warning: cast to restricted __le16drivers/bluetooth/btrtl.c: note: in included file:drivers/bluetooth/btrtl.h:47:45: warning: array of flexible structures
##############################
Test: CheckSmatch - WARNING
Desc: Run smatch tool with source
Output:
drivers/bluetooth/btrtl.c: note: in included file:drivers/bluetooth/btrtl.h:47:45: warning: array of flexible structures


---
Regards,
Linux Bluetooth

2023-01-19 20:08:50

by Luiz Augusto von Dentz

[permalink] [raw]
Subject: Re: [PATCH 1/1] Bluetooth: btrtl: Firmware format v2 support

Hi Max,

On Wed, Jan 18, 2023 at 11:47 PM <[email protected]> wrote:
>
> From: Max Chou <[email protected]>
>
> Realtek changes the format for the firmware file as v2. The driver
> should implement the patch to extract the firmware data from the
> firmware file.
> It's compatible with the both previous format(v1) and new format(v2).
>
> Signed-off-by: Allen Chen <[email protected]>
> Signed-off-by: Alex Lu <[email protected]>
> Tested-by: Hilda Wu <[email protected]>
> Signed-off-by: Max Chou <[email protected]>
> ---
> drivers/bluetooth/btrtl.c | 364 +++++++++++++++++++++++++++++++++-----
> drivers/bluetooth/btrtl.h | 61 +++++++
> 2 files changed, 385 insertions(+), 40 deletions(-)
>
> diff --git a/drivers/bluetooth/btrtl.c b/drivers/bluetooth/btrtl.c
> index 69c3fe649ca7..2277cb4e50c2 100644
> --- a/drivers/bluetooth/btrtl.c
> +++ b/drivers/bluetooth/btrtl.c
> @@ -18,6 +18,7 @@
> #define VERSION "0.1"
>
> #define RTL_EPATCH_SIGNATURE "Realtech"
> +#define RTL_EPATCH_SIGNATURE_V2 "RTBTCore"
> #define RTL_ROM_LMP_8723A 0x1200
> #define RTL_ROM_LMP_8723B 0x8723
> #define RTL_ROM_LMP_8821A 0x8821
> @@ -38,6 +39,14 @@
> .hci_ver = (hciv), \
> .hci_bus = (bus)
>
> +#define RTL_CHIP_SUBVER (&(struct rtl_vendor_cmd) {{0x10, 0x38, 0x04, 0x28, 0x80}})
> +#define RTL_CHIP_REV (&(struct rtl_vendor_cmd) {{0x10, 0x3A, 0x04, 0x28, 0x80}})
> +#define RTL_SEC_PROJ (&(struct rtl_vendor_cmd) {{0x10, 0xA4, 0xAD, 0x00, 0xb0}})
> +
> +#define RTL_PATCH_SNIPPETS 0x01
> +#define RTL_PATCH_DUMMY_HEADER 0x02
> +#define RTL_PATCH_SECURITY_HEADER 0x03
> +
> enum btrtl_chip_id {
> CHIP_ID_8723A,
> CHIP_ID_8723B,
> @@ -75,6 +84,8 @@ struct btrtl_device_info {
> int cfg_len;
> bool drop_fw;
> int project_id;
> + u8 key_id;
> + struct list_head patch_subsecs;
> };
>
> static const struct id_table ic_id_table[] = {
> @@ -284,6 +295,242 @@ static int rtl_read_rom_version(struct hci_dev *hdev, u8 *version)
> return 0;
> }
>
> +
> +static int btrtl_vendor_read_reg16(struct hci_dev *hdev,
> + struct rtl_vendor_cmd *cmd, u8 *rp)
> +{
> + struct sk_buff *skb;
> + int err = 0;
> +
> + skb = __hci_cmd_sync(hdev, 0xfc61, sizeof(*cmd), cmd,
> + HCI_INIT_TIMEOUT);
> + if (IS_ERR(skb)) {
> + err = PTR_ERR(skb);
> + rtl_dev_err(hdev, "RTL: Read reg16 failed (%d)", err);
> + return err;
> + }
> +
> + if (skb->len != 3 || skb->data[0]) {
> + bt_dev_err(hdev, "RTL: Read reg16 length mismatch");
> + kfree_skb(skb);
> + return -EIO;
> + }
> +
> + if (rp)
> + memcpy(rp, skb->data + 1, 2);
> +
> + kfree_skb(skb);
> +
> + return 0;
> +}
> +
> +static void btrtl_insert_ordered_subsec(struct rtl_subsection *node,
> + struct btrtl_device_info *btrtl_dev)
> +{
> + struct list_head *pos;
> + struct list_head *next;
> + struct rtl_subsection *subsec;
> +
> + list_for_each_safe(pos, next, &btrtl_dev->patch_subsecs) {
> + subsec = list_entry(pos, struct rtl_subsection, list);
> + if (subsec->prio >= node->prio)
> + break;
> + }
> + __list_add(&node->list, pos->prev, pos);
> +}
> +
> +static int btrtl_parse_section(struct hci_dev *hdev,
> + struct btrtl_device_info *btrtl_dev, u32 opcode,
> + u8 *data, u32 len)
> +{
> + struct rtl_section_hdr *hdr;
> + struct rtl_subsection *subsec;
> + struct rtl_common_subsec *common_subsec;
> + struct rtl_sec_hdr *sec_hdr;
> + int i;
> + u8 *ptr;
> + u8 *end = data + len;

I rather use an skb for parsing the data instead of parsing via
pointer directly, that way you can use the likes of skb_pull_data
which makes the code a lot simpler to follow and less prone to
mistakes accessing data outside the buffer area.

> + u16 num_subsecs;
> + u32 subsec_len;
> + int rc = 0;
> +
> + if (sizeof(*hdr) > len)
> + return -EINVAL;
> +
> + hdr = (void *)data;
> + ptr = data + sizeof(*hdr);
> + num_subsecs = le16_to_cpu(hdr->num);
> + for (i = 0; i < num_subsecs; i++) {
> + common_subsec = (void *)ptr;
> + if (ptr + sizeof(*common_subsec) > end)
> + break;
> + ptr += sizeof(*common_subsec);
> +
> + subsec_len = le32_to_cpu(common_subsec->len);
> + if (ptr + subsec_len > end)
> + break;
> + ptr += subsec_len;
> + }
> +
> + if (i < num_subsecs)
> + return -EOVERFLOW;
> +
> + ptr = data + sizeof(*hdr);
> + for (i = 0; i < num_subsecs; i++) {
> + common_subsec = (void *)ptr;
> + subsec_len = le32_to_cpu(common_subsec->len);
> + ptr += sizeof(*common_subsec);
> +
> + if (common_subsec->eco != btrtl_dev->rom_version + 1) {
> + ptr += subsec_len;
> + continue;
> + }
> +
> + switch (opcode) {
> + case RTL_PATCH_SECURITY_HEADER:
> + sec_hdr = (void *)common_subsec;
> + if (sec_hdr->key_id != btrtl_dev->key_id) {
> + ptr += subsec_len;
> + continue;
> + }
> + break;
> + }
> +
> + subsec = kzalloc(sizeof(*subsec), GFP_KERNEL);
> + if (!subsec)
> + return -ENOMEM;
> + subsec->prio = common_subsec->prio;
> + subsec->len = subsec_len;
> + subsec->data = ptr;
> + btrtl_insert_ordered_subsec(subsec, btrtl_dev);
> +
> + ptr += subsec_len;
> + rc += subsec_len;
> + }
> +
> + return rc;
> +}
> +
> +static int rtlbt_parse_firmware_v2(struct hci_dev *hdev,
> + struct btrtl_device_info *btrtl_dev,
> + unsigned char **_buf)
> +{
> + struct rtl_epatch_header_v2 *hdr;
> + int rc;
> + u8 reg_val[2];
> + u8 key_id;
> + u32 num_sections;
> + struct rtl_section *section;
> + struct rtl_subsection *entry, *tmp;
> + u32 section_len;
> + u32 opcode;
> + int len = 0;
> + int i;
> + int secure = 0;
> + int dummy = 0;
> + u8 *ptr;
> + /* Cut the tail of the firmware. */
> + u8 *end = btrtl_dev->fw_data + btrtl_dev->fw_len - 7;
> +
> + rc = btrtl_vendor_read_reg16(hdev, RTL_SEC_PROJ, reg_val);
> + if (rc < 0)
> + return -EIO;
> +
> + key_id = reg_val[0];
> + rtl_dev_dbg(hdev, "%s: key id %u", __func__, key_id);
> + btrtl_dev->key_id = key_id;
> +
> + hdr = (struct rtl_epatch_header_v2 *)btrtl_dev->fw_data;
> + num_sections = le32_to_cpu(hdr->num_sections);
> +
> + rtl_dev_dbg(hdev, "FW version %08x-%08x", *((u32 *)hdr->fw_version),
> + *((u32 *)(hdr->fw_version + 4)));
> +
> + ptr = btrtl_dev->fw_data + sizeof(*hdr);
> + for (i = 0; i < num_sections; i++) {
> + section = (void *)ptr;
> + if (ptr + sizeof(*section) > end)
> + break;
> + ptr += sizeof(*section);
> +
> + section_len = le32_to_cpu(section->len);
> + if (ptr + section_len > end)
> + break;
> + ptr += section_len;
> + }
> +
> + if (i < num_sections)
> + return -EOVERFLOW;
> +
> + ptr = btrtl_dev->fw_data + sizeof(*hdr);
> + for (i = 0; i < num_sections; i++) {
> + section = (void *)ptr;
> + rtl_dev_dbg(hdev, "opcode 0x%04x", section->opcode);
> + ptr += sizeof(*section);
> +
> + section_len = le32_to_cpu(section->len);
> + opcode = le32_to_cpu(section->opcode);
> + switch (opcode) {
> + case RTL_PATCH_SNIPPETS:
> + rc = btrtl_parse_section(hdev, btrtl_dev, opcode, ptr,
> + section_len);
> + break;
> + case RTL_PATCH_SECURITY_HEADER:
> + /* If key_id from chip is zero, ignore all security
> + * headers.
> + */
> + if (!key_id)
> + break;
> + rc = btrtl_parse_section(hdev, btrtl_dev, opcode, ptr,
> + section_len);
> + if (rc > 0)
> + secure += rc;
> +
> + break;
> + case RTL_PATCH_DUMMY_HEADER:
> + if (key_id)
> + break;
> + rc = btrtl_parse_section(hdev, btrtl_dev, opcode, ptr,
> + section_len);
> + if (rc > 0)
> + dummy += rc;
> + break;
> + default:
> + rc = 0;
> + break;
> + }
> + if (rc < 0) {
> + rtl_dev_err(hdev, "RTL: Parse section (%u) err %d",
> + opcode, rc);
> + return rc;
> + }
> + len += rc;
> + ptr += section_len;
> + }
> +
> + if (!len)
> + return -ENODATA;
> +
> + bt_dev_info(hdev, "RTL: Patch (len %d) found", len);
> +
> + /* Allocate mem and copy all found subsecs. */
> + ptr = kvmalloc(len, GFP_KERNEL);
> + if (!ptr)
> + return -ENOMEM;
> +
> + len = 0;
> + list_for_each_entry_safe(entry, tmp, &btrtl_dev->patch_subsecs, list) {
> + rtl_dev_dbg(hdev, "RTL: len 0x%x", entry->len);
> + memcpy(ptr + len, entry->data, entry->len);
> + len += entry->len;
> + list_del(&entry->list);
> + kfree(entry);
> + }
> +
> + *_buf = ptr;
> + return len;
> +}
> +
> static int rtlbt_parse_firmware(struct hci_dev *hdev,
> struct btrtl_device_info *btrtl_dev,
> unsigned char **_buf)
> @@ -317,7 +564,18 @@ static int rtlbt_parse_firmware(struct hci_dev *hdev,
> { RTL_ROM_LMP_8852A, 25 }, /* 8852C */
> };
>
> - min_size = sizeof(struct rtl_epatch_header) + sizeof(extension_sig) + 3;
> + if (btrtl_dev->fw_len <= 8)
> + return -EINVAL;
> +
> + if (!memcmp(btrtl_dev->fw_data, RTL_EPATCH_SIGNATURE, 8))
> + min_size = sizeof(struct rtl_epatch_header) +
> + sizeof(extension_sig) + 3;
> + else if (!memcmp(btrtl_dev->fw_data, RTL_EPATCH_SIGNATURE_V2, 8))
> + min_size = sizeof(struct rtl_epatch_header_v2) +
> + sizeof(extension_sig) + 3;
> + else
> + return -EINVAL;
> +
> if (btrtl_dev->fw_len < min_size)
> return -EINVAL;
>
> @@ -382,12 +640,14 @@ static int rtlbt_parse_firmware(struct hci_dev *hdev,
> return -EINVAL;
> }
>
> - epatch_info = (struct rtl_epatch_header *)btrtl_dev->fw_data;
> - if (memcmp(epatch_info->signature, RTL_EPATCH_SIGNATURE, 8) != 0) {
> + if (memcmp(btrtl_dev->fw_data, RTL_EPATCH_SIGNATURE, 8) != 0) {
> + if (!memcmp(btrtl_dev->fw_data, RTL_EPATCH_SIGNATURE_V2, 8))
> + return rtlbt_parse_firmware_v2(hdev, btrtl_dev, _buf);
> rtl_dev_err(hdev, "bad EPATCH signature");
> return -EINVAL;
> }
>
> + epatch_info = (struct rtl_epatch_header *)btrtl_dev->fw_data;
> num_patches = le16_to_cpu(epatch_info->num_patches);
> BT_DBG("fw_version=%x, num_patches=%d",
> le32_to_cpu(epatch_info->fw_version), num_patches);
> @@ -451,6 +711,7 @@ static int rtl_download_firmware(struct hci_dev *hdev,
> int frag_len = RTL_FRAG_LEN;
> int ret = 0;
> int i;
> + int j = 0;
> struct sk_buff *skb;
> struct hci_rp_read_local_version *rp;
>
> @@ -461,17 +722,16 @@ static int rtl_download_firmware(struct hci_dev *hdev,
> for (i = 0; i < frag_num; i++) {
> struct sk_buff *skb;
>
> - BT_DBG("download fw (%d/%d)", i, frag_num);
> -
> - if (i > 0x7f)
> - dl_cmd->index = (i & 0x7f) + 1;
> - else
> - dl_cmd->index = i;
> + dl_cmd->index = j++;
> + if (dl_cmd->index == 0x7f)
> + j = 1;
>
> if (i == (frag_num - 1)) {
> dl_cmd->index |= 0x80; /* data end */
> frag_len = fw_len % RTL_FRAG_LEN;
> }
> + rtl_dev_dbg(hdev, "download fw (%d/%d). index = %d", i,
> + frag_num, dl_cmd->index);
> memcpy(dl_cmd->data, data, frag_len);
>
> /* Send download command */
> @@ -589,8 +849,16 @@ static int btrtl_setup_rtl8723b(struct hci_dev *hdev,
>
> void btrtl_free(struct btrtl_device_info *btrtl_dev)
> {
> + struct rtl_subsection *entry, *tmp;
> +
> kvfree(btrtl_dev->fw_data);
> kvfree(btrtl_dev->cfg_data);
> +
> + list_for_each_entry_safe(entry, tmp, &btrtl_dev->patch_subsecs, list) {
> + list_del(&entry->list);
> + kfree(entry);
> + }
> +
> kfree(btrtl_dev);
> }
> EXPORT_SYMBOL_GPL(btrtl_free);
> @@ -604,9 +872,11 @@ struct btrtl_device_info *btrtl_initialize(struct hci_dev *hdev,
> char cfg_name[40];
> u16 hci_rev, lmp_subver;
> u8 hci_ver;
> + u8 lmp_ver;
> int ret;
> u16 opcode;
> u8 cmd[2];
> + u8 reg_val[2];
>
> btrtl_dev = kzalloc(sizeof(*btrtl_dev), GFP_KERNEL);
> if (!btrtl_dev) {
> @@ -614,26 +884,56 @@ struct btrtl_device_info *btrtl_initialize(struct hci_dev *hdev,
> goto err_alloc;
> }
>
> + INIT_LIST_HEAD(&btrtl_dev->patch_subsecs);
> +
> +check_version:
> + ret = btrtl_vendor_read_reg16(hdev, RTL_CHIP_SUBVER, reg_val);
> + if (ret < 0)
> + goto err_free;
> + lmp_subver = le16_to_cpu(*((u16 *)reg_val));
> +
> + if (lmp_subver == RTL_ROM_LMP_8822B) {
> + ret = btrtl_vendor_read_reg16(hdev, RTL_CHIP_REV, reg_val);
> + if (ret < 0)
> + goto err_free;
> + hci_rev = le16_to_cpu(*((u16 *)reg_val));
> +
> + /* 8822E */
> + if (hci_rev == 0x000e) {
> + hci_ver = 0x0c;
> + lmp_ver = 0x0c;
> + btrtl_dev->ic_info = btrtl_match_ic(lmp_subver, hci_rev,
> + hci_ver, hdev->bus);
> + goto next;
> + }
> + }
> +
> skb = btrtl_read_local_version(hdev);
> if (IS_ERR(skb)) {
> ret = PTR_ERR(skb);
> goto err_free;
> }
> -
> resp = (struct hci_rp_read_local_version *)skb->data;
> - rtl_dev_info(hdev, "examining hci_ver=%02x hci_rev=%04x lmp_ver=%02x lmp_subver=%04x",
> - resp->hci_ver, resp->hci_rev,
> - resp->lmp_ver, resp->lmp_subver);
>
> - hci_ver = resp->hci_ver;
> - hci_rev = le16_to_cpu(resp->hci_rev);
> + hci_ver = resp->hci_ver;
> + hci_rev = le16_to_cpu(resp->hci_rev);
> + lmp_ver = resp->lmp_ver;
> lmp_subver = le16_to_cpu(resp->lmp_subver);
>
> + kfree_skb(skb);
> +
> btrtl_dev->ic_info = btrtl_match_ic(lmp_subver, hci_rev, hci_ver,
> hdev->bus);
>
> - if (!btrtl_dev->ic_info)
> +next:
> + rtl_dev_info(hdev, "examining hci_ver=%02x hci_rev=%04x lmp_ver=%02x lmp_subver=%04x",
> + hci_ver, hci_rev,
> + lmp_ver, lmp_subver);
> +
> + if (!btrtl_dev->ic_info && !btrtl_dev->drop_fw)
> btrtl_dev->drop_fw = true;
> + else
> + btrtl_dev->drop_fw = false;
>
> if (btrtl_dev->drop_fw) {
> opcode = hci_opcode_pack(0x3f, 0x66);
> @@ -642,41 +942,25 @@ struct btrtl_device_info *btrtl_initialize(struct hci_dev *hdev,
>
> skb = bt_skb_alloc(sizeof(cmd), GFP_KERNEL);
> if (!skb)
> - goto out_free;
> + goto err_free;
>
> skb_put_data(skb, cmd, sizeof(cmd));
> hci_skb_pkt_type(skb) = HCI_COMMAND_PKT;
>
> - hdev->send(hdev, skb);
> + ret = hdev->send(hdev, skb);
> + if (ret < 0) {
> + bt_dev_err(hdev, "sending frame failed (%d)", ret);
> + kfree_skb(skb);
> + goto err_free;
> + }
>
> /* Ensure the above vendor command is sent to controller and
> * process has done.
> */
> msleep(200);
>
> - /* Read the local version again. Expect to have the vanilla
> - * version as cold boot.
> - */
> - skb = btrtl_read_local_version(hdev);
> - if (IS_ERR(skb)) {
> - ret = PTR_ERR(skb);
> - goto err_free;
> - }
> -
> - resp = (struct hci_rp_read_local_version *)skb->data;
> - rtl_dev_info(hdev, "examining hci_ver=%02x hci_rev=%04x lmp_ver=%02x lmp_subver=%04x",
> - resp->hci_ver, resp->hci_rev,
> - resp->lmp_ver, resp->lmp_subver);
> -
> - hci_ver = resp->hci_ver;
> - hci_rev = le16_to_cpu(resp->hci_rev);
> - lmp_subver = le16_to_cpu(resp->lmp_subver);
> -
> - btrtl_dev->ic_info = btrtl_match_ic(lmp_subver, hci_rev, hci_ver,
> - hdev->bus);
> + goto check_version;
> }
> -out_free:
> - kfree_skb(skb);
>
> if (!btrtl_dev->ic_info) {
> rtl_dev_info(hdev, "unknown IC info, lmp subver %04x, hci rev %04x, hci ver %04x",
> diff --git a/drivers/bluetooth/btrtl.h b/drivers/bluetooth/btrtl.h
> index ebf0101c959b..9fbfe323120b 100644
> --- a/drivers/bluetooth/btrtl.h
> +++ b/drivers/bluetooth/btrtl.h
> @@ -47,6 +47,67 @@ struct rtl_vendor_config {
> struct rtl_vendor_config_entry entry[];
> } __packed;
>
> +struct rtl_epatch_header_v2 {
> + __u8 signature[8];
> + __u8 fw_version[8];
> + __le32 num_sections;
> +} __packed;
> +
> +struct rtl_section {
> + __le32 opcode;
> + __le32 len;
> + u8 data[];
> +} __packed;
> +
> +struct rtl_section_hdr {
> + __le16 num;
> + __le16 reserved;
> +} __packed;
> +
> +struct rtl_common_subsec {
> + __u8 eco;
> + __u8 prio;
> + __u8 cb[2];
> + __le32 len;
> + __u8 data[];
> +};
> +
> +struct rtl_snippet {
> + __u8 eco;
> + __u8 prio;
> + __u16 reserved;
> + __le32 len;
> + __u8 data[];
> +} __packed;
> +
> +struct rtl_dummy_hdr {
> + __u8 eco;
> + __u8 prio;
> + __le16 reserved;
> + __le32 len;
> + __u8 data[];
> +} __packed;
> +
> +struct rtl_sec_hdr {
> + __u8 eco;
> + __u8 prio;
> + __u8 key_id;
> + __u8 reserved;
> + __le32 len;
> + __u8 data[];
> +} __packed;
> +
> +struct rtl_subsection {
> + struct list_head list;
> + u8 prio;
> + u32 len;
> + u8 *data;
> +};
> +
> +struct rtl_vendor_cmd {
> + __u8 param[5];
> +} __packed;
> +
> enum {
> REALTEK_ALT6_CONTINUOUS_TX_CHIP,
>
> --
> 2.34.1
>


--
Luiz Augusto von Dentz

2023-01-20 04:54:32

by kernel test robot

[permalink] [raw]
Subject: Re: [PATCH 1/1] Bluetooth: btrtl: Firmware format v2 support

Hi,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on bluetooth/master]
[also build test WARNING on bluetooth-next/master linus/master v6.2-rc4 next-20230119]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url: https://github.com/intel-lab-lkp/linux/commits/max-chou-realtek-com/Bluetooth-btrtl-Firmware-format-v2-support/20230119-155205
base: https://git.kernel.org/pub/scm/linux/kernel/git/bluetooth/bluetooth.git master
patch link: https://lore.kernel.org/r/20230119074714.156283-1-max.chou%40realtek.com
patch subject: [PATCH 1/1] Bluetooth: btrtl: Firmware format v2 support
config: x86_64-randconfig-a003 (https://download.01.org/0day-ci/archive/20230120/[email protected]/config)
compiler: clang version 14.0.6 (https://github.com/llvm/llvm-project f28c006a5895fc0e329fe15fead81e37457cb1d1)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/intel-lab-lkp/linux/commit/ffdaef9bef66fbba642b544b0a1f35217dc17d6a
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review max-chou-realtek-com/Bluetooth-btrtl-Firmware-format-v2-support/20230119-155205
git checkout ffdaef9bef66fbba642b544b0a1f35217dc17d6a
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=x86_64 olddefconfig
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=x86_64 SHELL=/bin/bash drivers/bluetooth/

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <[email protected]>

All warnings (new ones prefixed by >>):

>> drivers/bluetooth/btrtl.c:430:6: warning: variable 'dummy' set but not used [-Wunused-but-set-variable]
int dummy = 0;
^
>> drivers/bluetooth/btrtl.c:429:6: warning: variable 'secure' set but not used [-Wunused-but-set-variable]
int secure = 0;
^
2 warnings generated.


vim +/dummy +430 drivers/bluetooth/btrtl.c

413
414 static int rtlbt_parse_firmware_v2(struct hci_dev *hdev,
415 struct btrtl_device_info *btrtl_dev,
416 unsigned char **_buf)
417 {
418 struct rtl_epatch_header_v2 *hdr;
419 int rc;
420 u8 reg_val[2];
421 u8 key_id;
422 u32 num_sections;
423 struct rtl_section *section;
424 struct rtl_subsection *entry, *tmp;
425 u32 section_len;
426 u32 opcode;
427 int len = 0;
428 int i;
> 429 int secure = 0;
> 430 int dummy = 0;
431 u8 *ptr;
432 /* Cut the tail of the firmware. */
433 u8 *end = btrtl_dev->fw_data + btrtl_dev->fw_len - 7;
434
435 rc = btrtl_vendor_read_reg16(hdev, RTL_SEC_PROJ, reg_val);
436 if (rc < 0)
437 return -EIO;
438
439 key_id = reg_val[0];
440 rtl_dev_dbg(hdev, "%s: key id %u", __func__, key_id);
441 btrtl_dev->key_id = key_id;
442
443 hdr = (struct rtl_epatch_header_v2 *)btrtl_dev->fw_data;
444 num_sections = le32_to_cpu(hdr->num_sections);
445
446 rtl_dev_dbg(hdev, "FW version %08x-%08x", *((u32 *)hdr->fw_version),
447 *((u32 *)(hdr->fw_version + 4)));
448
449 ptr = btrtl_dev->fw_data + sizeof(*hdr);
450 for (i = 0; i < num_sections; i++) {
451 section = (void *)ptr;
452 if (ptr + sizeof(*section) > end)
453 break;
454 ptr += sizeof(*section);
455
456 section_len = le32_to_cpu(section->len);
457 if (ptr + section_len > end)
458 break;
459 ptr += section_len;
460 }
461
462 if (i < num_sections)
463 return -EOVERFLOW;
464
465 ptr = btrtl_dev->fw_data + sizeof(*hdr);
466 for (i = 0; i < num_sections; i++) {
467 section = (void *)ptr;
468 rtl_dev_dbg(hdev, "opcode 0x%04x", section->opcode);
469 ptr += sizeof(*section);
470
471 section_len = le32_to_cpu(section->len);
472 opcode = le32_to_cpu(section->opcode);
473 switch (opcode) {
474 case RTL_PATCH_SNIPPETS:
475 rc = btrtl_parse_section(hdev, btrtl_dev, opcode, ptr,
476 section_len);
477 break;
478 case RTL_PATCH_SECURITY_HEADER:
479 /* If key_id from chip is zero, ignore all security
480 * headers.
481 */
482 if (!key_id)
483 break;
484 rc = btrtl_parse_section(hdev, btrtl_dev, opcode, ptr,
485 section_len);
486 if (rc > 0)
487 secure += rc;
488
489 break;
490 case RTL_PATCH_DUMMY_HEADER:
491 if (key_id)
492 break;
493 rc = btrtl_parse_section(hdev, btrtl_dev, opcode, ptr,
494 section_len);
495 if (rc > 0)
496 dummy += rc;
497 break;
498 default:
499 rc = 0;
500 break;
501 }
502 if (rc < 0) {
503 rtl_dev_err(hdev, "RTL: Parse section (%u) err %d",
504 opcode, rc);
505 return rc;
506 }
507 len += rc;
508 ptr += section_len;
509 }
510
511 if (!len)
512 return -ENODATA;
513
514 bt_dev_info(hdev, "RTL: Patch (len %d) found", len);
515
516 /* Allocate mem and copy all found subsecs. */
517 ptr = kvmalloc(len, GFP_KERNEL);
518 if (!ptr)
519 return -ENOMEM;
520
521 len = 0;
522 list_for_each_entry_safe(entry, tmp, &btrtl_dev->patch_subsecs, list) {
523 rtl_dev_dbg(hdev, "RTL: len 0x%x", entry->len);
524 memcpy(ptr + len, entry->data, entry->len);
525 len += entry->len;
526 list_del(&entry->list);
527 kfree(entry);
528 }
529
530 *_buf = ptr;
531 return len;
532 }
533

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

2023-01-20 04:54:36

by kernel test robot

[permalink] [raw]
Subject: Re: [PATCH 1/1] Bluetooth: btrtl: Firmware format v2 support

Hi,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on bluetooth/master]
[also build test WARNING on bluetooth-next/master linus/master v6.2-rc4 next-20230119]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url: https://github.com/intel-lab-lkp/linux/commits/max-chou-realtek-com/Bluetooth-btrtl-Firmware-format-v2-support/20230119-155205
base: https://git.kernel.org/pub/scm/linux/kernel/git/bluetooth/bluetooth.git master
patch link: https://lore.kernel.org/r/20230119074714.156283-1-max.chou%40realtek.com
patch subject: [PATCH 1/1] Bluetooth: btrtl: Firmware format v2 support
config: csky-randconfig-s043-20230119 (https://download.01.org/0day-ci/archive/20230120/[email protected]/config)
compiler: csky-linux-gcc (GCC) 12.1.0
reproduce:
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# apt-get install sparse
# sparse version: v0.6.4-39-gce1a6720-dirty
# https://github.com/intel-lab-lkp/linux/commit/ffdaef9bef66fbba642b544b0a1f35217dc17d6a
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review max-chou-realtek-com/Bluetooth-btrtl-Firmware-format-v2-support/20230119-155205
git checkout ffdaef9bef66fbba642b544b0a1f35217dc17d6a
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' O=build_dir ARCH=csky olddefconfig
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' O=build_dir ARCH=csky SHELL=/bin/bash drivers/bluetooth/

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <[email protected]>

sparse warnings: (new ones prefixed by >>)
>> drivers/bluetooth/btrtl.c:893:22: sparse: sparse: cast to restricted __le16
drivers/bluetooth/btrtl.c:899:27: sparse: sparse: cast to restricted __le16
drivers/bluetooth/btrtl.c: note: in included file:
drivers/bluetooth/btrtl.h:47:45: sparse: sparse: array of flexible structures

vim +893 drivers/bluetooth/btrtl.c

865
866 struct btrtl_device_info *btrtl_initialize(struct hci_dev *hdev,
867 const char *postfix)
868 {
869 struct btrtl_device_info *btrtl_dev;
870 struct sk_buff *skb;
871 struct hci_rp_read_local_version *resp;
872 char cfg_name[40];
873 u16 hci_rev, lmp_subver;
874 u8 hci_ver;
875 u8 lmp_ver;
876 int ret;
877 u16 opcode;
878 u8 cmd[2];
879 u8 reg_val[2];
880
881 btrtl_dev = kzalloc(sizeof(*btrtl_dev), GFP_KERNEL);
882 if (!btrtl_dev) {
883 ret = -ENOMEM;
884 goto err_alloc;
885 }
886
887 INIT_LIST_HEAD(&btrtl_dev->patch_subsecs);
888
889 check_version:
890 ret = btrtl_vendor_read_reg16(hdev, RTL_CHIP_SUBVER, reg_val);
891 if (ret < 0)
892 goto err_free;
> 893 lmp_subver = le16_to_cpu(*((u16 *)reg_val));
894
895 if (lmp_subver == RTL_ROM_LMP_8822B) {
896 ret = btrtl_vendor_read_reg16(hdev, RTL_CHIP_REV, reg_val);
897 if (ret < 0)
898 goto err_free;
899 hci_rev = le16_to_cpu(*((u16 *)reg_val));
900
901 /* 8822E */
902 if (hci_rev == 0x000e) {
903 hci_ver = 0x0c;
904 lmp_ver = 0x0c;
905 btrtl_dev->ic_info = btrtl_match_ic(lmp_subver, hci_rev,
906 hci_ver, hdev->bus);
907 goto next;
908 }
909 }
910
911 skb = btrtl_read_local_version(hdev);
912 if (IS_ERR(skb)) {
913 ret = PTR_ERR(skb);
914 goto err_free;
915 }
916 resp = (struct hci_rp_read_local_version *)skb->data;
917
918 hci_ver = resp->hci_ver;
919 hci_rev = le16_to_cpu(resp->hci_rev);
920 lmp_ver = resp->lmp_ver;
921 lmp_subver = le16_to_cpu(resp->lmp_subver);
922
923 kfree_skb(skb);
924
925 btrtl_dev->ic_info = btrtl_match_ic(lmp_subver, hci_rev, hci_ver,
926 hdev->bus);
927
928 next:
929 rtl_dev_info(hdev, "examining hci_ver=%02x hci_rev=%04x lmp_ver=%02x lmp_subver=%04x",
930 hci_ver, hci_rev,
931 lmp_ver, lmp_subver);
932
933 if (!btrtl_dev->ic_info && !btrtl_dev->drop_fw)
934 btrtl_dev->drop_fw = true;
935 else
936 btrtl_dev->drop_fw = false;
937
938 if (btrtl_dev->drop_fw) {
939 opcode = hci_opcode_pack(0x3f, 0x66);
940 cmd[0] = opcode & 0xff;
941 cmd[1] = opcode >> 8;
942
943 skb = bt_skb_alloc(sizeof(cmd), GFP_KERNEL);
944 if (!skb)
945 goto err_free;
946
947 skb_put_data(skb, cmd, sizeof(cmd));
948 hci_skb_pkt_type(skb) = HCI_COMMAND_PKT;
949
950 ret = hdev->send(hdev, skb);
951 if (ret < 0) {
952 bt_dev_err(hdev, "sending frame failed (%d)", ret);
953 kfree_skb(skb);
954 goto err_free;
955 }
956
957 /* Ensure the above vendor command is sent to controller and
958 * process has done.
959 */
960 msleep(200);
961
962 goto check_version;
963 }
964
965 if (!btrtl_dev->ic_info) {
966 rtl_dev_info(hdev, "unknown IC info, lmp subver %04x, hci rev %04x, hci ver %04x",
967 lmp_subver, hci_rev, hci_ver);
968 return btrtl_dev;
969 }
970
971 if (btrtl_dev->ic_info->has_rom_version) {
972 ret = rtl_read_rom_version(hdev, &btrtl_dev->rom_version);
973 if (ret)
974 goto err_free;
975 }
976
977 btrtl_dev->fw_len = rtl_load_file(hdev, btrtl_dev->ic_info->fw_name,
978 &btrtl_dev->fw_data);
979 if (btrtl_dev->fw_len < 0) {
980 rtl_dev_err(hdev, "firmware file %s not found",
981 btrtl_dev->ic_info->fw_name);
982 ret = btrtl_dev->fw_len;
983 goto err_free;
984 }
985
986 if (btrtl_dev->ic_info->cfg_name) {
987 if (postfix) {
988 snprintf(cfg_name, sizeof(cfg_name), "%s-%s.bin",
989 btrtl_dev->ic_info->cfg_name, postfix);
990 } else {
991 snprintf(cfg_name, sizeof(cfg_name), "%s.bin",
992 btrtl_dev->ic_info->cfg_name);
993 }
994 btrtl_dev->cfg_len = rtl_load_file(hdev, cfg_name,
995 &btrtl_dev->cfg_data);
996 if (btrtl_dev->ic_info->config_needed &&
997 btrtl_dev->cfg_len <= 0) {
998 rtl_dev_err(hdev, "mandatory config file %s not found",
999 btrtl_dev->ic_info->cfg_name);
1000 ret = btrtl_dev->cfg_len;
1001 goto err_free;
1002 }
1003 }
1004
1005 /* The following chips supports the Microsoft vendor extension,
1006 * therefore set the corresponding VsMsftOpCode.
1007 */
1008 if (btrtl_dev->ic_info->has_msft_ext)
1009 hci_set_msft_opcode(hdev, 0xFCF0);
1010
1011 return btrtl_dev;
1012
1013 err_free:
1014 btrtl_free(btrtl_dev);
1015 err_alloc:
1016 return ERR_PTR(ret);
1017 }
1018 EXPORT_SYMBOL_GPL(btrtl_initialize);
1019

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