2023-02-16 09:23:00

by Tim Jiang

[permalink] [raw]
Subject: [PATCH v2] Bluetooth: hci_qca: Add support for Qualcomm Bluetooth SoC QCA2066

This patch adds support for QCA2066 firmware patch and nvm downloading.

Signed-off-by: Tim Jiang <[email protected]>
---
drivers/bluetooth/btqca.c | 78 +++++++++++++++++++++++++++++++++++++
drivers/bluetooth/btqca.h | 2 +
drivers/bluetooth/hci_qca.c | 6 +++
3 files changed, 86 insertions(+)

diff --git a/drivers/bluetooth/btqca.c b/drivers/bluetooth/btqca.c
index c9064d34d830..e1127532c5a8 100644
--- a/drivers/bluetooth/btqca.c
+++ b/drivers/bluetooth/btqca.c
@@ -205,6 +205,50 @@ static int qca_send_reset(struct hci_dev *hdev)
return 0;
}

+static int qca_read_fw_board_id(struct hci_dev *hdev, u16 *bid)
+{
+ u8 cmd;
+ struct sk_buff *skb;
+ struct edl_event_hdr *edl;
+ int err = 0;
+ int bid_len;
+
+ bt_dev_dbg(hdev, "QCA read board ID");
+
+ cmd = EDL_GET_BID_REQ_CMD;
+ skb = __hci_cmd_sync_ev(hdev, EDL_PATCH_CMD_OPCODE, EDL_PATCH_CMD_LEN,
+ &cmd, 0, HCI_INIT_TIMEOUT);
+ if (IS_ERR(skb)) {
+ err = PTR_ERR(skb);
+ bt_dev_err(hdev, "Reading QCA board ID failed (%d)",
+ err);
+ return err;
+ }
+
+ edl = skb_pull_data(skb, sizeof(*edl));
+ if (!edl) {
+ bt_dev_err(hdev, "QCA read board ID with no header");
+ err = -EILSEQ;
+ goto out;
+ }
+
+ if (edl->cresp != EDL_CMD_REQ_RES_EVT ||
+ edl->rtype != EDL_GET_BID_REQ_CMD) {
+ bt_dev_err(hdev, "QCA Wrong packet received %d %d", edl->cresp,
+ edl->rtype);
+ err = -EIO;
+ goto out;
+ }
+
+ bid_len = edl->data[0];
+ *bid = (edl->data[1] << 8) + edl->data[2];
+ bt_dev_info(hdev, "%s: bid len = %x, bid = %x", __func__, bid_len, *bid);
+
+out:
+ kfree_skb(skb);
+ return err;
+}
+
int qca_send_pre_shutdown_cmd(struct hci_dev *hdev)
{
struct sk_buff *skb;
@@ -574,6 +618,31 @@ int qca_set_bdaddr_rome(struct hci_dev *hdev, const bdaddr_t *bdaddr)
}
EXPORT_SYMBOL_GPL(qca_set_bdaddr_rome);

+static void qca_generate_nvm_name(struct hci_dev *hdev, char *fwname,
+ size_t max_size, struct qca_btsoc_version ver, u16 bid)
+{
+ u8 rom_ver = 0;
+ u32 soc_ver;
+ const char *variant;
+
+ soc_ver = get_soc_ver(ver.soc_id, ver.rom_ver);
+ rom_ver = ((soc_ver & 0x00000f00) >> 0x04) | (soc_ver & 0x0000000f);
+
+ if ((ver.soc_id & 0x0000ff00) == 0x1200) /*hsp gf chip*/
+ variant = "g";
+ else
+ variant = "";
+
+ if (bid == 0x0) {
+ snprintf(fwname, max_size, "qca/hpnv%02x%s.bin", rom_ver, variant);
+ } else {
+ snprintf(fwname, max_size, "qca/hpnv%02x%s.%x",
+ rom_ver, variant, bid);
+ }
+
+ bt_dev_info(hdev, "%s: nvm name is %s", __func__, fwname);
+}
+
int qca_uart_setup(struct hci_dev *hdev, uint8_t baudrate,
enum qca_btsoc_type soc_type, struct qca_btsoc_version ver,
const char *firmware_name)
@@ -582,6 +651,7 @@ int qca_uart_setup(struct hci_dev *hdev, uint8_t baudrate,
int err;
u8 rom_ver = 0;
u32 soc_ver;
+ u16 bid = 0;

bt_dev_dbg(hdev, "QCA setup on UART");

@@ -607,6 +677,9 @@ int qca_uart_setup(struct hci_dev *hdev, uint8_t baudrate,
} else if (soc_type == QCA_QCA6390) {
snprintf(config.fwname, sizeof(config.fwname),
"qca/htbtfw%02x.tlv", rom_ver);
+ } else if (soc_type == QCA_QCA2066) {
+ snprintf(config.fwname, sizeof(config.fwname),
+ "qca/hpbtfw%02x.tlv", rom_ver);
} else if (soc_type == QCA_WCN6750) {
/* Choose mbn file by default.If mbn file is not found
* then choose tlv file
@@ -628,6 +701,9 @@ int qca_uart_setup(struct hci_dev *hdev, uint8_t baudrate,
/* Give the controller some time to get ready to receive the NVM */
msleep(10);

+ if (soc_type == QCA_QCA2066)
+ qca_read_fw_board_id(hdev, &bid);
+
/* Download NVM configuration */
config.type = TLV_TYPE_NVM;
if (firmware_name)
@@ -645,6 +721,8 @@ int qca_uart_setup(struct hci_dev *hdev, uint8_t baudrate,
else if (soc_type == QCA_QCA6390)
snprintf(config.fwname, sizeof(config.fwname),
"qca/htnv%02x.bin", rom_ver);
+ else if (soc_type == QCA_QCA2066)
+ qca_generate_nvm_name(hdev, config.fwname, sizeof(config.fwname), ver, bid);
else if (soc_type == QCA_WCN6750)
snprintf(config.fwname, sizeof(config.fwname),
"qca/msnv%02x.bin", rom_ver);
diff --git a/drivers/bluetooth/btqca.h b/drivers/bluetooth/btqca.h
index 61e9a50e66ae..e762c403284a 100644
--- a/drivers/bluetooth/btqca.h
+++ b/drivers/bluetooth/btqca.h
@@ -13,6 +13,7 @@
#define EDL_PATCH_TLV_REQ_CMD (0x1E)
#define EDL_GET_BUILD_INFO_CMD (0x20)
#define EDL_NVM_ACCESS_SET_REQ_CMD (0x01)
+#define EDL_GET_BID_REQ_CMD (0x23)
#define EDL_PATCH_CONFIG_CMD (0x28)
#define MAX_SIZE_PER_TLV_SEGMENT (243)
#define QCA_PRE_SHUTDOWN_CMD (0xFC08)
@@ -147,6 +148,7 @@ enum qca_btsoc_type {
QCA_WCN3991,
QCA_QCA6390,
QCA_WCN6750,
+ QCA_QCA2066,
};

#if IS_ENABLED(CONFIG_BT_QCA)
diff --git a/drivers/bluetooth/hci_qca.c b/drivers/bluetooth/hci_qca.c
index 3df8c3606e93..84769089ddff 100644
--- a/drivers/bluetooth/hci_qca.c
+++ b/drivers/bluetooth/hci_qca.c
@@ -1866,6 +1866,11 @@ static const struct qca_device_data qca_soc_data_qca6390 = {
.num_vregs = 0,
};

+static const struct qca_device_data qca_soc_data_qca2066 = {
+ .soc_type = QCA_QCA2066,
+ .num_vregs = 0,
+};
+
static const struct qca_device_data qca_soc_data_wcn6750 = {
.soc_type = QCA_WCN6750,
.vregs = (struct qca_vreg []) {
@@ -2335,6 +2340,7 @@ static const struct of_device_id qca_bluetooth_of_match[] = {
{ .compatible = "qcom,wcn3991-bt", .data = &qca_soc_data_wcn3991},
{ .compatible = "qcom,wcn3998-bt", .data = &qca_soc_data_wcn3998},
{ .compatible = "qcom,wcn6750-bt", .data = &qca_soc_data_wcn6750},
+ { .compatible = "qcom,qca2066-bt", .data = &qca_soc_data_qca2066},
{ /* sentinel */ }
};
MODULE_DEVICE_TABLE(of, qca_bluetooth_of_match);
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project



2023-02-16 09:57:27

by bluez.test.bot

[permalink] [raw]
Subject: RE: [v2] Bluetooth: hci_qca: Add support for Qualcomm Bluetooth SoC QCA2066

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

---Test result---

Test Summary:
CheckPatch FAIL 1.36 seconds
GitLint PASS 0.28 seconds
SubjectPrefix PASS 0.10 seconds
BuildKernel PASS 31.60 seconds
CheckAllWarning PASS 34.23 seconds
CheckSparse WARNING 39.23 seconds
CheckSmatch PASS 106.34 seconds
BuildKernel32 PASS 30.48 seconds
TestRunnerSetup PASS 436.94 seconds
TestRunner_l2cap-tester PASS 16.55 seconds
TestRunner_iso-tester PASS 17.21 seconds
TestRunner_bnep-tester PASS 5.64 seconds
TestRunner_mgmt-tester PASS 111.67 seconds
TestRunner_rfcomm-tester PASS 9.04 seconds
TestRunner_sco-tester PASS 8.31 seconds
TestRunner_ioctl-tester PASS 9.72 seconds
TestRunner_mesh-tester PASS 7.13 seconds
TestRunner_smp-tester PASS 8.20 seconds
TestRunner_userchan-tester PASS 5.95 seconds
IncrementalBuild PASS 28.61 seconds

Details
##############################
Test: CheckPatch - FAIL
Desc: Run checkpatch.pl script
Output:
[v2] Bluetooth: hci_qca: Add support for Qualcomm Bluetooth SoC QCA2066
WARNING: DT compatible string "qcom,qca2066-bt" appears un-documented -- check ./Documentation/devicetree/bindings/
#263: FILE: drivers/bluetooth/hci_qca.c:2343:
+ { .compatible = "qcom,qca2066-bt", .data = &qca_soc_data_qca2066},

total: 0 errors, 1 warnings, 146 lines checked

NOTE: For some of the reported defects, checkpatch may be able to
mechanically convert to the typical style using --fix or --fix-inplace.

/github/workspace/src/src/13142752.patch has style problems, please review.

NOTE: Ignored message types: UNKNOWN_COMMIT_ID

NOTE: If any of the errors are false positives, please report
them to the maintainer, see CHECKPATCH in MAINTAINERS.


##############################
Test: CheckSparse - WARNING
Desc: Run sparse tool with linux kernel
Output:
drivers/bluetooth/btqca.c:631:17: warning: restricted __le32 degrades to integerdrivers/bluetooth/btqca.c:713:24: warning: restricted __le32 degrades to integer


---
Regards,
Linux Bluetooth

2023-02-16 11:38:08

by Paul Menzel

[permalink] [raw]
Subject: Re: [PATCH v2] Bluetooth: hci_qca: Add support for Qualcomm Bluetooth SoC QCA2066

Dear Tim,


Thank you for the patch.

Am 16.02.23 um 10:22 schrieb Tim Jiang:
> This patch adds support for QCA2066 firmware patch and nvm downloading.

(Nit: Imperative mood with *Add support …* would make it shorter, and is
recommended.)

Please elaborate a little. Please mention the datasheet name and
revision, and how it was tested.

> Signed-off-by: Tim Jiang <[email protected]>
> ---
> drivers/bluetooth/btqca.c | 78 +++++++++++++++++++++++++++++++++++++
> drivers/bluetooth/btqca.h | 2 +
> drivers/bluetooth/hci_qca.c | 6 +++
> 3 files changed, 86 insertions(+)
>
> diff --git a/drivers/bluetooth/btqca.c b/drivers/bluetooth/btqca.c
> index c9064d34d830..e1127532c5a8 100644
> --- a/drivers/bluetooth/btqca.c
> +++ b/drivers/bluetooth/btqca.c
> @@ -205,6 +205,50 @@ static int qca_send_reset(struct hci_dev *hdev)
> return 0;
> }
>
> +static int qca_read_fw_board_id(struct hci_dev *hdev, u16 *bid)
> +{
> + u8 cmd;
> + struct sk_buff *skb;
> + struct edl_event_hdr *edl;
> + int err = 0;
> + int bid_len;
> +
> + bt_dev_dbg(hdev, "QCA read board ID");
> +
> + cmd = EDL_GET_BID_REQ_CMD;
> + skb = __hci_cmd_sync_ev(hdev, EDL_PATCH_CMD_OPCODE, EDL_PATCH_CMD_LEN,
> + &cmd, 0, HCI_INIT_TIMEOUT);
> + if (IS_ERR(skb)) {
> + err = PTR_ERR(skb);
> + bt_dev_err(hdev, "Reading QCA board ID failed (%d)",
> + err);

Should fit in one line?

> + return err;
> + }
> +
> + edl = skb_pull_data(skb, sizeof(*edl));
> + if (!edl) {
> + bt_dev_err(hdev, "QCA read board ID with no header");

Please phrase these errors more user understandable. Maybe even with a
suggestion, how to fix it.

> + err = -EILSEQ;
> + goto out;
> + }
> +
> + if (edl->cresp != EDL_CMD_REQ_RES_EVT ||
> + edl->rtype != EDL_GET_BID_REQ_CMD) {
> + bt_dev_err(hdev, "QCA Wrong packet received %d %d", edl->cresp,
> + edl->rtype);

Same.

> + err = -EIO;
> + goto out;
> + }
> +
> + bid_len = edl->data[0];
> + *bid = (edl->data[1] << 8) + edl->data[2];
> + bt_dev_info(hdev, "%s: bid len = %x, bid = %x", __func__, bid_len, *bid);
> +
> +out:
> + kfree_skb(skb);
> + return err;
> +}
> +
> int qca_send_pre_shutdown_cmd(struct hci_dev *hdev)
> {
> struct sk_buff *skb;
> @@ -574,6 +618,31 @@ int qca_set_bdaddr_rome(struct hci_dev *hdev, const bdaddr_t *bdaddr)
> }
> EXPORT_SYMBOL_GPL(qca_set_bdaddr_rome);
>
> +static void qca_generate_nvm_name(struct hci_dev *hdev, char *fwname,
> + size_t max_size, struct qca_btsoc_version ver, u16 bid)
> +{
> + u8 rom_ver = 0;
> + u32 soc_ver;
> + const char *variant;
> +
> + soc_ver = get_soc_ver(ver.soc_id, ver.rom_ver);
> + rom_ver = ((soc_ver & 0x00000f00) >> 0x04) | (soc_ver & 0x0000000f);
> +
> + if ((ver.soc_id & 0x0000ff00) == 0x1200) /*hsp gf chip*/

Missing spaces in the comment. Can macros/enums be used?

> + variant = "g";
> + else
> + variant = "";
> +
> + if (bid == 0x0) {
> + snprintf(fwname, max_size, "qca/hpnv%02x%s.bin", rom_ver, variant);
> + } else {
> + snprintf(fwname, max_size, "qca/hpnv%02x%s.%x",
> + rom_ver, variant, bid);
> + }

The coding style requires no spaces for oneline bodies.

> +
> + bt_dev_info(hdev, "%s: nvm name is %s", __func__, fwname);
> +}
> +
> int qca_uart_setup(struct hci_dev *hdev, uint8_t baudrate,
> enum qca_btsoc_type soc_type, struct qca_btsoc_version ver,
> const char *firmware_name)
> @@ -582,6 +651,7 @@ int qca_uart_setup(struct hci_dev *hdev, uint8_t baudrate,
> int err;
> u8 rom_ver = 0;
> u32 soc_ver;
> + u16 bid = 0;

Use boardid as name?

>
> bt_dev_dbg(hdev, "QCA setup on UART");
>
> @@ -607,6 +677,9 @@ int qca_uart_setup(struct hci_dev *hdev, uint8_t baudrate,
> } else if (soc_type == QCA_QCA6390) {
> snprintf(config.fwname, sizeof(config.fwname),
> "qca/htbtfw%02x.tlv", rom_ver);
> + } else if (soc_type == QCA_QCA2066) {
> + snprintf(config.fwname, sizeof(config.fwname),
> + "qca/hpbtfw%02x.tlv", rom_ver);
> } else if (soc_type == QCA_WCN6750) {
> /* Choose mbn file by default.If mbn file is not found

Missing space after dot/period.

> * then choose tlv file
> @@ -628,6 +701,9 @@ int qca_uart_setup(struct hci_dev *hdev, uint8_t baudrate,
> /* Give the controller some time to get ready to receive the NVM */
> msleep(10);
>
> + if (soc_type == QCA_QCA2066)
> + qca_read_fw_board_id(hdev, &bid);
> +
> /* Download NVM configuration */
> config.type = TLV_TYPE_NVM;
> if (firmware_name)
> @@ -645,6 +721,8 @@ int qca_uart_setup(struct hci_dev *hdev, uint8_t baudrate,
> else if (soc_type == QCA_QCA6390)
> snprintf(config.fwname, sizeof(config.fwname),
> "qca/htnv%02x.bin", rom_ver);
> + else if (soc_type == QCA_QCA2066)
> + qca_generate_nvm_name(hdev, config.fwname, sizeof(config.fwname), ver, bid);

Sort the branches lexicographically?

> else if (soc_type == QCA_WCN6750)
> snprintf(config.fwname, sizeof(config.fwname),
> "qca/msnv%02x.bin", rom_ver);
> diff --git a/drivers/bluetooth/btqca.h b/drivers/bluetooth/btqca.h
> index 61e9a50e66ae..e762c403284a 100644
> --- a/drivers/bluetooth/btqca.h
> +++ b/drivers/bluetooth/btqca.h
> @@ -13,6 +13,7 @@
> #define EDL_PATCH_TLV_REQ_CMD (0x1E)
> #define EDL_GET_BUILD_INFO_CMD (0x20)
> #define EDL_NVM_ACCESS_SET_REQ_CMD (0x01)
> +#define EDL_GET_BID_REQ_CMD (0x23)
> #define EDL_PATCH_CONFIG_CMD (0x28)
> #define MAX_SIZE_PER_TLV_SEGMENT (243)
> #define QCA_PRE_SHUTDOWN_CMD (0xFC08)
> @@ -147,6 +148,7 @@ enum qca_btsoc_type {
> QCA_WCN3991,
> QCA_QCA6390,
> QCA_WCN6750,
> + QCA_QCA2066,

Sort it?

> };
>
> #if IS_ENABLED(CONFIG_BT_QCA)
> diff --git a/drivers/bluetooth/hci_qca.c b/drivers/bluetooth/hci_qca.c
> index 3df8c3606e93..84769089ddff 100644
> --- a/drivers/bluetooth/hci_qca.c
> +++ b/drivers/bluetooth/hci_qca.c
> @@ -1866,6 +1866,11 @@ static const struct qca_device_data qca_soc_data_qca6390 = {
> .num_vregs = 0,
> };
>
> +static const struct qca_device_data qca_soc_data_qca2066 = {
> + .soc_type = QCA_QCA2066,
> + .num_vregs = 0,
> +};
> +
> static const struct qca_device_data qca_soc_data_wcn6750 = {
> .soc_type = QCA_WCN6750,
> .vregs = (struct qca_vreg []) {
> @@ -2335,6 +2340,7 @@ static const struct of_device_id qca_bluetooth_of_match[] = {
> { .compatible = "qcom,wcn3991-bt", .data = &qca_soc_data_wcn3991},
> { .compatible = "qcom,wcn3998-bt", .data = &qca_soc_data_wcn3998},
> { .compatible = "qcom,wcn6750-bt", .data = &qca_soc_data_wcn6750},
> + { .compatible = "qcom,qca2066-bt", .data = &qca_soc_data_qca2066},

Sort it?

> { /* sentinel */ }
> };
> MODULE_DEVICE_TABLE(of, qca_bluetooth_of_match);


Kind regards,

Paul

2023-02-18 04:15:55

by Steev Klimaszewski

[permalink] [raw]
Subject: Re: [PATCH v2] Bluetooth: hci_qca: Add support for Qualcomm Bluetooth SoC QCA2066

>This patch adds support for QCA2066 firmware patch and nvm downloading.

>Signed-off-by: Tim Jiang <[email protected]>
>---
> drivers/bluetooth/btqca.c | 78 +++++++++++++++++++++++++++++++++++++
> drivers/bluetooth/btqca.h | 2 +
> drivers/bluetooth/hci_qca.c | 6 +++
> 3 files changed, 86 insertions(+)

>diff --git a/drivers/bluetooth/btqca.c b/drivers/bluetooth/btqca.c
>index c9064d34d830..e1127532c5a8 100644
>--- a/drivers/bluetooth/btqca.c
>+++ b/drivers/bluetooth/btqca.c
>@@ -205,6 +205,50 @@ static int qca_send_reset(struct hci_dev *hdev)
> return 0;
> }
>
>+static int qca_read_fw_board_id(struct hci_dev *hdev, u16 *bid)
>+{
>+ u8 cmd;
>+ struct sk_buff *skb;
>+ struct edl_event_hdr *edl;
>+ int err = 0;
>+ int bid_len;
>+
>+ bt_dev_dbg(hdev, "QCA read board ID");
>+
>+ cmd = EDL_GET_BID_REQ_CMD;
>+ skb = __hci_cmd_sync_ev(hdev, EDL_PATCH_CMD_OPCODE, EDL_PATCH_CMD_LEN,
>+ &cmd, 0, HCI_INIT_TIMEOUT);
>+ if (IS_ERR(skb)) {
>+ err = PTR_ERR(skb);
>+ bt_dev_err(hdev, "Reading QCA board ID failed (%d)",
>+ err);
>+ return err;
>+ }
>+
>+ edl = skb_pull_data(skb, sizeof(*edl));
>+ if (!edl) {
>+ bt_dev_err(hdev, "QCA read board ID with no header");
>+ err = -EILSEQ;
>+ goto out;
>+ }
>+
>+ if (edl->cresp != EDL_CMD_REQ_RES_EVT ||
>+ edl->rtype != EDL_GET_BID_REQ_CMD) {
>+ bt_dev_err(hdev, "QCA Wrong packet received %d %d", edl->cresp,
>+ edl->rtype);
>+ err = -EIO;
>+ goto out;
>+ }
>+
>+ bid_len = edl->data[0];
>+ *bid = (edl->data[1] << 8) + edl->data[2];

Is this actually correct? I ask because here I have the WCN6855 in the Lenovo
Thinkpad X13s, and when attempting to use this driver, I end up with a
board id of 08c, and according to the firmware, we should have a board id of
b8c.

>+ bt_dev_info(hdev, "%s: bid len = %x, bid = %x", __func__, bid_len, *bid);

Does this print serve any use for end users? Perhaps it could be combined with
the QCA Read Board ID above, so it is in line with the other prints from the
driver?

>+
>+out:
>+ kfree_skb(skb);
>+ return err;
>+}
>+
> int qca_send_pre_shutdown_cmd(struct hci_dev *hdev)
> {
> struct sk_buff *skb;
>@@ -574,6 +618,31 @@ int qca_set_bdaddr_rome(struct hci_dev *hdev, const bdaddr_t *bdaddr)
> }
> EXPORT_SYMBOL_GPL(qca_set_bdaddr_rome);
>
>+static void qca_generate_nvm_name(struct hci_dev *hdev, char *fwname,
>+ size_t max_size, struct qca_btsoc_version ver, u16 bid)
>+{
>+ u8 rom_ver = 0;
>+ u32 soc_ver;
>+ const char *variant;
>+
>+ soc_ver = get_soc_ver(ver.soc_id, ver.rom_ver);
>+ rom_ver = ((soc_ver & 0x00000f00) >> 0x04) | (soc_ver & 0x0000000f);
>+
>+ if ((ver.soc_id & 0x0000ff00) == 0x1200) /*hsp gf chip*/
>+ variant = "g";
>+ else
>+ variant = "";
>+
>+ if (bid == 0x0) {
>+ snprintf(fwname, max_size, "qca/hpnv%02x%s.bin", rom_ver, variant);
>+ } else {
>+ snprintf(fwname, max_size, "qca/hpnv%02x%s.%x",
>+ rom_ver, variant, bid);
>+ }
>+
>+ bt_dev_info(hdev, "%s: nvm name is %s", __func__, fwname);

Does this print serve any purpose for end users?

>+}
>+
> int qca_uart_setup(struct hci_dev *hdev, uint8_t baudrate,
> enum qca_btsoc_type soc_type, struct qca_btsoc_version ver,
> const char *firmware_name)
>@@ -582,6 +651,7 @@ int qca_uart_setup(struct hci_dev *hdev, uint8_t baudrate,
> int err;
> u8 rom_ver = 0;
> u32 soc_ver;
>+ u16 bid = 0;
>
> bt_dev_dbg(hdev, "QCA setup on UART");
>
>@@ -607,6 +677,9 @@ int qca_uart_setup(struct hci_dev *hdev, uint8_t baudrate,
> } else if (soc_type == QCA_QCA6390) {
> snprintf(config.fwname, sizeof(config.fwname),
> "qca/htbtfw%02x.tlv", rom_ver);
>+ } else if (soc_type == QCA_QCA2066) {
>+ snprintf(config.fwname, sizeof(config.fwname),
>+ "qca/hpbtfw%02x.tlv", rom_ver);
> } else if (soc_type == QCA_WCN6750) {
> /* Choose mbn file by default.If mbn file is not found
> * then choose tlv file
>@@ -628,6 +701,9 @@ int qca_uart_setup(struct hci_dev *hdev, uint8_t baudrate,
> /* Give the controller some time to get ready to receive the NVM */
> msleep(10);
>
>+ if (soc_type == QCA_QCA2066)
>+ qca_read_fw_board_id(hdev, &bid);
>+
> /* Download NVM configuration */
> config.type = TLV_TYPE_NVM;
> if (firmware_name)
>@@ -645,6 +721,8 @@ int qca_uart_setup(struct hci_dev *hdev, uint8_t baudrate,
> else if (soc_type == QCA_QCA6390)
> snprintf(config.fwname, sizeof(config.fwname),
> "qca/htnv%02x.bin", rom_ver);
>+ else if (soc_type == QCA_QCA2066)
>+ qca_generate_nvm_name(hdev, config.fwname, sizeof(config.fwname), ver, bid);
> else if (soc_type == QCA_WCN6750)
> snprintf(config.fwname, sizeof(config.fwname),
> "qca/msnv%02x.bin", rom_ver);
>diff --git a/drivers/bluetooth/btqca.h b/drivers/bluetooth/btqca.h
>index 61e9a50e66ae..e762c403284a 100644
>--- a/drivers/bluetooth/btqca.h
>+++ b/drivers/bluetooth/btqca.h
>@@ -13,6 +13,7 @@
> #define EDL_PATCH_TLV_REQ_CMD (0x1E)
> #define EDL_GET_BUILD_INFO_CMD (0x20)
> #define EDL_NVM_ACCESS_SET_REQ_CMD (0x01)
>+#define EDL_GET_BID_REQ_CMD (0x23)
> #define EDL_PATCH_CONFIG_CMD (0x28)
> #define MAX_SIZE_PER_TLV_SEGMENT (243)
> #define QCA_PRE_SHUTDOWN_CMD (0xFC08)
>@@ -147,6 +148,7 @@ enum qca_btsoc_type {
> QCA_WCN3991,
> QCA_QCA6390,
> QCA_WCN6750,
>+ QCA_QCA2066,
> };
>
> #if IS_ENABLED(CONFIG_BT_QCA)
>diff --git a/drivers/bluetooth/hci_qca.c b/drivers/bluetooth/hci_qca.c
>index 3df8c3606e93..84769089ddff 100644
>--- a/drivers/bluetooth/hci_qca.c
>+++ b/drivers/bluetooth/hci_qca.c
>@@ -1866,6 +1866,11 @@ static const struct qca_device_data qca_soc_data_qca6390 = {
> .num_vregs = 0,
> };
>
>+static const struct qca_device_data qca_soc_data_qca2066 = {
>+ .soc_type = QCA_QCA2066,
>+ .num_vregs = 0,
>+};
>+
> static const struct qca_device_data qca_soc_data_wcn6750 = {
> .soc_type = QCA_WCN6750,
> .vregs = (struct qca_vreg []) {
>@@ -2335,6 +2340,7 @@ static const struct of_device_id qca_bluetooth_of_match[] = {
> { .compatible = "qcom,wcn3991-bt", .data = &qca_soc_data_wcn3991},
> { .compatible = "qcom,wcn3998-bt", .data = &qca_soc_data_wcn3998},
> { .compatible = "qcom,wcn6750-bt", .data = &qca_soc_data_wcn6750},
>+ { .compatible = "qcom,qca2066-bt", .data = &qca_soc_data_qca2066},
> { /* sentinel */ }
> };
> MODULE_DEVICE_TABLE(of, qca_bluetooth_of_match);

I'm not sure how to phrase this, and just thinking out loud - if the qca2066 is
the same as the wcn6855, just with an additional antenna, perhaps there should
be some way to indicate it? In my patchset, I had a print of the name "Setting
up wcn6855" and locally, I combined your patch with mine, although dropping the
above qca2066 hunks in favor of my already written wcn6855 hunks, and then
combined the two in the print so it says "Setting up qca2066/wcn6855" - is there
any way to differentiate between what Qualcomm considers a qca2066 and what is
considered the wcn6855?

Below is dmesg output, and so if we go by the wifi device check, we end up
seeing that we get

chip_id 0x2
chip_family 0xb
board_id 0x8c
soc_id 0x400c0210

[ 9.724834] ath11k_pci 0006:01:00.0: chip_id 0x2 chip_family 0xb board_id 0x8c soc_id 0x400c0210
[ 9.724850] ath11k_pci 0006:01:00.0: fw_version 0x110b196e fw_build_timestamp 2022-12-22 12:54 fw_build_id WLAN.HSP.1.1-03125-QCAHSPSWPL_V1_V2_SILICONZ_LITE-3.6510.23
[ 9.742346] ath11k_pci 0006:01:00.0: failed to fetch board data for bus=pci,vendor=17cb,device=1103,subsystem-vendor=17cb,subsystem-device=0108,qmi-chip-id=2,qmi-board-id=140 from ath11k/WCN6855/hw2.1/board-2.bin
[ 9.742358] ath11k_pci 0006:01:00.0: failed to fetch board.bin from WCN6855/hw2.1
[ 9.742361] ath11k_pci 0006:01:00.0: qmi failed to fetch board file: -2
[ 9.742363] ath11k_pci 0006:01:00.0: failed to load board data file: -2

But with your driver (mine too for that matter, but then it's just doing the
same bid bits)...

[ 9.081003] Bluetooth: hci0: Frame reassembly failed (-84)
[ 9.141992] Bluetooth: hci0: QCA Product ID :0x00000013
[ 9.141999] Bluetooth: hci0: QCA SOC Version :0x400c0210
[ 9.142003] Bluetooth: hci0: QCA ROM Version :0x00000201
[ 9.142007] Bluetooth: hci0: QCA Patch Version:0x000038e6
[ 9.170612] Bluetooth: hci0: QCA controller version 0x02100201
[ 9.170620] Bluetooth: hci0: QCA Downloading qca/hpbtfw21.tlv


[ 9.760537] Bluetooth: hci0: QCA board ID len 2,id = 0 8c
[ 9.760547] Bluetooth: hci0: QCA Downloading qca/hpnv21.08c
[ 9.760638] bluetooth hci0: Direct firmware load for qca/hpnv21.08c failed with error -2
[ 9.760640] Bluetooth: hci0: QCA Failed to request file: qca/hpnv21.08c (-2)
[ 9.760643] Bluetooth: hci0: QCA Failed to download NVM (-2)

Is there another way to check the board id or board family? Because it's not
coming up with the correct one here.

--steev

2023-02-20 07:43:35

by Tim Jiang

[permalink] [raw]
Subject: RE: [PATCH v2] Bluetooth: hci_qca: Add support for Qualcomm Bluetooth SoC QCA2066

Steve, sorry I does not know why I can not send out the email as have some warning , so I removed the code part, you can refer to my inline comments.

Regards.
Tim

>This patch adds support for QCA2066 firmware patch and nvm downloading.

Is this actually correct? I ask because here I have the WCN6855 in the Lenovo Thinkpad X13s, and when attempting to use this driver, I end up with a board id of 08c, and according to the firmware, we should have a board id of b8c.

[Tim] correct, I does not know where you get the WCN6855 firmware, for our side, we will use different name rule for android and linux , for android , we will add "b" in the front of board id, for linux we will not add this bit.

I'm not sure how to phrase this, and just thinking out loud - if the qca2066 is the same as the wcn6855, just with an additional antenna, perhaps there should be some way to indicate it? In my patchset, I had a print of the name "Setting up wcn6855" and locally, I combined your patch with mine, although dropping the above qca2066 hunks in favor of my already written wcn6855 hunks, and then combined the two in the print so it says "Setting up qca2066/wcn6855" - is there any way to differentiate between what qti considers a qca2066 and what is considered the wcn6855?
[Tim] steve ,as I mentioned previously, we have different board id for qca2066 and wcn6855 , and then which will use different nvm config files based on board id.

Below is dmesg output, and so if we go by the wifi device check, we end up seeing that we get

chip_id 0x2
chip_family 0xb
board_id 0x8c
soc_id 0x400c0210

[ 9.724834] ath11k_pci 0006:01:00.0: chip_id 0x2 chip_family 0xb board_id 0x8c soc_id 0x400c0210
[ 9.724850] ath11k_pci 0006:01:00.0: fw_version 0x110b196e fw_build_timestamp 2022-12-22 12:54 fw_build_id WLAN.HSP.1.1-03125-QCAHSPSWPL_V1_V2_SILICONZ_LITE-3.6510.23
[ 9.742346] ath11k_pci 0006:01:00.0: failed to fetch board data for bus=pci,vendor=17cb,device=1103,subsystem-vendor=17cb,subsystem-device=0108,qmi-chip-id=2,qmi-board-id=140 from ath11k/WCN6855/hw2.1/board-2.bin
[ 9.742358] ath11k_pci 0006:01:00.0: failed to fetch board.bin from WCN6855/hw2.1
[ 9.742361] ath11k_pci 0006:01:00.0: qmi failed to fetch board file: -2
[ 9.742363] ath11k_pci 0006:01:00.0: failed to load board data file: -2

But with your driver (mine too for that matter, but then it's just doing the same bid bits)...

[ 9.081003] Bluetooth: hci0: Frame reassembly failed (-84)
[ 9.141992] Bluetooth: hci0: QCA Product ID :0x00000013
[ 9.141999] Bluetooth: hci0: QCA SOC Version :0x400c0210
[ 9.142003] Bluetooth: hci0: QCA ROM Version :0x00000201
[ 9.142007] Bluetooth: hci0: QCA Patch Version:0x000038e6
[ 9.170612] Bluetooth: hci0: QCA controller version 0x02100201
[ 9.170620] Bluetooth: hci0: QCA Downloading qca/hpbtfw21.tlv


[ 9.760537] Bluetooth: hci0: QCA board ID len 2,id = 0 8c
[ 9.760547] Bluetooth: hci0: QCA Downloading qca/hpnv21.08c
[ 9.760638] bluetooth hci0: Direct firmware load for qca/hpnv21.08c failed with error -2
[ 9.760640] Bluetooth: hci0: QCA Failed to request file: qca/hpnv21.08c (-2)
[ 9.760643] Bluetooth: hci0: QCA Failed to download NVM (-2)

Is there another way to check the board id or board family? Because it's not coming up with the correct one here.
[Tim] as my comments above, read board id way is correct, only we have different name rule for android and linux .

--steev

2023-02-21 05:28:17

by Steev Klimaszewski

[permalink] [raw]
Subject: Re: [PATCH v2] Bluetooth: hci_qca: Add support for Qualcomm Bluetooth SoC QCA2066

Hi Tim,

On Mon, Feb 20, 2023 at 1:04 AM Tim Jiang (QUIC)
<[email protected]> wrote:
>
> Steve, sorry I does not know why I can not send out the email as have some warning , so I removed the code part, you can refer to my inline comments.
>

No worries, I understand!

> Regards.
> Tim
>
> >This patch adds support for QCA2066 firmware patch and nvm downloading.
>
> Is this actually correct? I ask because here I have the WCN6855 in the Lenovo Thinkpad X13s, and when attempting to use this driver, I end up with a board id of 08c, and according to the firmware, we should have a board id of b8c.
>
> [Tim] correct, I does not know where you get the WCN6855 firmware, for our side, we will use different name rule for android and linux , for android , we will add "b" in the front of board id, for linux we will not add this bit.

So, the Thinkpad X13s is... well, it's a Thinkpad, and it comes with
Windows. So the firmware that we have, comes from.... them? qualcomm?
I honestly don't know the provenance of it, just that I grabbed the
files from the Windows partition:

steev@wintermute:~/firmware/new/qcbtfmuart_hsp8280.inf_arm64_69bd85311531b34a$
ls
bsrc_bt.bin hpnv10.b03 hpnv20.b80 hpnv20.b8f hpnv21.ba2 hpnv21g.b9f
hpnv21g.bb8 hpbtfw10.tlv hpnv10.b04 hpnv20.b82 hpnv20.bin hpnv21.ba3
hpnv21g.ba0 hpnv21g.bin hpbtfw10.ver hpnv10.b06 hpnv20.b84 hpnv21.b8c
hpnv21.ba4 hpnv21g.ba1 qcbtfmuart_hsp8280.cat hpbtfw20.tlv hpnv10.b07
hpnv20.b85 hpnv21.b9f hpnv21.bb8 hpnv21g.ba2 qcbtfmuart_hsp8280.inf
hpbtfw20.ver hpnv10.b08 hpnv20.b8c hpnv21.ba0 hpnv21.bin hpnv21g.ba3
qcbtfmuart_hsp8280.PNF hpbtfw21.tlv hpnv10.bin hpnv20.b8e hpnv21.ba1
hpnv21g.b8c hpnv21g.ba4 qcbtfmuart_hsp8280.sys

So, all of the nvm patches that are provided by them with the windows
driver, all start with 'b'.

> I'm not sure how to phrase this, and just thinking out loud - if the qca2066 is the same as the wcn6855, just with an additional antenna, perhaps there should be some way to indicate it? In my patchset, I had a print of the name "Setting up wcn6855" and locally, I combined your patch with mine, although dropping the above qca2066 hunks in favor of my already written wcn6855 hunks, and then combined the two in the print so it says "Setting up qca2066/wcn6855" - is there any way to differentiate between what qti considers a qca2066 and what is considered the wcn6855?
> [Tim] steve ,as I mentioned previously, we have different board id for qca2066 and wcn6855 , and then which will use different nvm config files based on board id.
>
I get that; I'm thinking of end users, who may know that they have a
wcn6855, but if the messages are qca2066, they may not understand that
it's the same chip.

> Below is dmesg output, and so if we go by the wifi device check, we end up seeing that we get
>
> chip_id 0x2
> chip_family 0xb
> board_id 0x8c
> soc_id 0x400c0210
>
> [ 9.724834] ath11k_pci 0006:01:00.0: chip_id 0x2 chip_family 0xb board_id 0x8c soc_id 0x400c0210
> [ 9.724850] ath11k_pci 0006:01:00.0: fw_version 0x110b196e fw_build_timestamp 2022-12-22 12:54 fw_build_id WLAN.HSP.1.1-03125-QCAHSPSWPL_V1_V2_SILICONZ_LITE-3.6510.23
> [ 9.742346] ath11k_pci 0006:01:00.0: failed to fetch board data for bus=pci,vendor=17cb,device=1103,subsystem-vendor=17cb,subsystem-device=0108,qmi-chip-id=2,qmi-board-id=140 from ath11k/WCN6855/hw2.1/board-2.bin
> [ 9.742358] ath11k_pci 0006:01:00.0: failed to fetch board.bin from WCN6855/hw2.1
> [ 9.742361] ath11k_pci 0006:01:00.0: qmi failed to fetch board file: -2
> [ 9.742363] ath11k_pci 0006:01:00.0: failed to load board data file: -2
>
> But with your driver (mine too for that matter, but then it's just doing the same bid bits)...
>
> [ 9.081003] Bluetooth: hci0: Frame reassembly failed (-84)
> [ 9.141992] Bluetooth: hci0: QCA Product ID :0x00000013
> [ 9.141999] Bluetooth: hci0: QCA SOC Version :0x400c0210
> [ 9.142003] Bluetooth: hci0: QCA ROM Version :0x00000201
> [ 9.142007] Bluetooth: hci0: QCA Patch Version:0x000038e6
> [ 9.170612] Bluetooth: hci0: QCA controller version 0x02100201
> [ 9.170620] Bluetooth: hci0: QCA Downloading qca/hpbtfw21.tlv
>
>
> [ 9.760537] Bluetooth: hci0: QCA board ID len 2,id = 0 8c
> [ 9.760547] Bluetooth: hci0: QCA Downloading qca/hpnv21.08c
> [ 9.760638] bluetooth hci0: Direct firmware load for qca/hpnv21.08c failed with error -2
> [ 9.760640] Bluetooth: hci0: QCA Failed to request file: qca/hpnv21.08c (-2)
> [ 9.760643] Bluetooth: hci0: QCA Failed to download NVM (-2)
>
> Is there another way to check the board id or board family? Because it's not coming up with the correct one here.
> [Tim] as my comments above, the read board id way is correct, only we have different name rule for android and linux .

Right, I get that reading the board id is the right way - and if you
see in my Windows firmware above (the firmware submitted to
linux-firmware by you only seems to have nvm patches that start with
'3', and no others..) I'm saying, should we also read the chip_family
somehow, since the WiFi firmware seems to properly notice that we are
chip_family 0xb, however, the bluetooth driver just returns '0'. And
neither the submitted firmware, nor the firmware that Lenovo/Qualcomm
ship to Windows users, have nvm patches that start with 0.

An additional note, neither my driver, nor yours, seems to work with
BLE. I cannot connect a pair of AirPods to my Thinkpad in Linux, but
I am able to do just fine in the Windows installation. I'm not sure
if this is a known limitation already in linux or not.

>
> --steev

2023-02-21 06:51:31

by Tim Jiang

[permalink] [raw]
Subject: RE: [PATCH v2] Bluetooth: hci_qca: Add support for Qualcomm Bluetooth SoC QCA2066

Steev , inline comments!

> >This patch adds support for QCA2066 firmware patch and nvm downloading.
>
> Is this actually correct? I ask because here I have the WCN6855 in the Lenovo Thinkpad X13s, and when attempting to use this driver, I end up with a board id of 08c, and according to the firmware, we should have a board id of b8c.
>
> [Tim] correct, I does not know where you get the WCN6855 firmware, for our side, we will use different name rule for android and linux , for android , we will add "b" in the front of board id, for linux we will not add this bit.

So, the Thinkpad X13s is... well, it's a Thinkpad, and it comes with Windows. So the firmware that we have, comes from.... them? qc?
[Tim] yeah , for wos , btfw name seems also the same with android. I think come from qc
I honestly don't know the provenance of it, just that I grabbed the files from the Windows partition:

So, all of the nvm patches that are provided by them with the windows driver, all start with 'b'.

> I'm not sure how to phrase this, and just thinking out loud - if the qca2066 is the same as the wcn6855, just with an additional antenna, perhaps there should be some way to indicate it? In my patchset, I had a print of the name "Setting up wcn6855" and locally, I combined your patch with mine, although dropping the above qca2066 hunks in favor of my already written wcn6855 hunks, and then combined the two in the print so it says "Setting up qca2066/wcn6855" - is there any way to differentiate between what qti considers a qca2066 and what is considered the wcn6855?
> [Tim] steve ,as I mentioned previously, we have different board id for qca2066 and wcn6855 , and then which will use different nvm config files based on board id.
>
I get that; I'm thinking of end users, who may know that they have a wcn6855, but if the messages are qca2066, they may not understand that it's the same chip.
[Tim] I see, wcn6855 and qca2066 is belong to "hsp" btchip family , so maybe we can use "hsp" to instead.

> Below is dmesg output, and so if we go by the wifi device check, we
> end up seeing that we get
>
> chip_id 0x2
> chip_family 0xb
> board_id 0x8c
> soc_id 0x400c0210
>
>
> But with your driver (mine too for that matter, but then it's just doing the same bid bits)...
>
> [ 9.081003] Bluetooth: hci0: Frame reassembly failed (-84)
> [ 9.141992] Bluetooth: hci0: QCA Product ID :0x00000013
> [ 9.141999] Bluetooth: hci0: QCA SOC Version :0x400c0210
> [ 9.142003] Bluetooth: hci0: QCA ROM Version :0x00000201
> [ 9.142007] Bluetooth: hci0: QCA Patch Version:0x000038e6
> [ 9.170612] Bluetooth: hci0: QCA controller version 0x02100201
> [ 9.170620] Bluetooth: hci0: QCA Downloading qca/hpbtfw21.tlv
>
>
> [ 9.760537] Bluetooth: hci0: QCA board ID len 2,id = 0 8c
> [ 9.760547] Bluetooth: hci0: QCA Downloading qca/hpnv21.08c
> [ 9.760638] bluetooth hci0: Direct firmware load for qca/hpnv21.08c failed with error -2
> [ 9.760640] Bluetooth: hci0: QCA Failed to request file: qca/hpnv21.08c (-2)
> [ 9.760643] Bluetooth: hci0: QCA Failed to download NVM (-2)
>
> Is there another way to check the board id or board family? Because it's not coming up with the correct one here.
> [Tim] as my comments above, the read board id way is correct, only we have different name rule for android and linux .

Right, I get that reading the board id is the right way - and if you see in my Windows firmware above (the firmware submitted to linux-firmware by you only seems to have nvm patches that start with '3', and no others..) I'm saying, should we also read the chip_family somehow, since the WiFi firmware seems to properly notice that we are chip_family 0xb, however, the bluetooth driver just returns '0'. And neither the submitted firmware, nor the firmware that Lenovo/Qualcomm ship to Windows users, have nvm patches that start with 0.

[Tim] we detect the chip_family by detect product_id and soc_id , such as for "hsp" chip family, if product id equal 0x13 ,and soc_id & 0xffff0000 == 0x400c , we assume this chip is "hsp". You can refer the log as above.

An additional note, neither my driver, nor yours, seems to work with BLE. I cannot connect a pair of AirPods to my Thinkpad in Linux, but I am able to do just fine in the Windows installation. I'm not sure if this is a known limitation already in linux or not.
[Tim] I am not sure, maybe have IOT issue, need to check it case by case.

>
> --steev