lkml.org 
[lkml]   [2023]   [Feb]   [16]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    SubjectRe: [PATCH v2] Bluetooth: hci_qca: Add support for Qualcomm Bluetooth SoC QCA2066
    From
    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 <quic_tjiang@quicinc.com>
    > ---
    > 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

    \
     
     \ /
      Last update: 2023-03-27 00:25    [W:4.690 / U:0.024 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site