From: gubbaven@codeaurora.org
To: Marcel Holtmann <marcel@holtmann.org>
Cc: Johan Hedberg <johan.hedberg@gmail.com>,
Matthias Kaehlcke <mka@chromium.org>,
open list <linux-kernel@vger.kernel.org>,
linux-bluetooth <linux-bluetooth@vger.kernel.org>,
Hemantg <hemantg@codeaurora.org>,
MSM <linux-arm-msm@vger.kernel.org>,
Balakrishna Godavarthi <bgodavar@codeaurora.org>,
Rocky Liao <rjliao@codeaurora.org>,
hbandi@codeaurora.org, abhishekpandit@chromium.org
Subject: Re: [PATCH v1] Bluetooth: Use NVM files based on SoC ID for WCN3991
Date: Thu, 19 Nov 2020 18:22:55 +0530 [thread overview]
Message-ID: <b98756e1e34b4dbde0b0374c50765a33@codeaurora.org> (raw)
In-Reply-To: <A07A6C72-55B6-4DE6-BA4A-987ED4DB88E0@holtmann.org>
Hi Marcel,
On 2020-09-16 19:57, Marcel Holtmann wrote:
> Hi Venkata,
>
>> This change will allow to use different NVM file based
>> on WCN3991 BT SoC ID.Need to use different NVM file based on
>> fab location for WCN3991 BT SoC.
>>
>> Signed-off-by: Venkata Lakshmi Narayana Gubba
>> <gubbaven@codeaurora.org>
>> ---
>> drivers/bluetooth/btqca.c | 41
>> +++++++++++++++++++++++++----------------
>> drivers/bluetooth/btqca.h | 13 ++++++++-----
>> drivers/bluetooth/hci_qca.c | 11 +++++------
>> 3 files changed, 38 insertions(+), 27 deletions(-)
>>
>> diff --git a/drivers/bluetooth/btqca.c b/drivers/bluetooth/btqca.c
>> index ce9dcff..a7e72f1 100644
>> --- a/drivers/bluetooth/btqca.c
>> +++ b/drivers/bluetooth/btqca.c
>> @@ -14,12 +14,11 @@
>>
>> #define VERSION "0.1"
>>
>> -int qca_read_soc_version(struct hci_dev *hdev, u32 *soc_version,
>> +int qca_read_soc_version(struct hci_dev *hdev, struct
>> qca_btsoc_version *ver,
>> enum qca_btsoc_type soc_type)
>> {
>> struct sk_buff *skb;
>> struct edl_event_hdr *edl;
>> - struct qca_btsoc_version *ver;
>> char cmd;
>> int err = 0;
>> u8 event_type = HCI_EV_VENDOR;
>> @@ -70,9 +69,9 @@ int qca_read_soc_version(struct hci_dev *hdev, u32
>> *soc_version,
>> }
>>
>> if (soc_type >= QCA_WCN3991)
>> - memmove(&edl->data, &edl->data[1], sizeof(*ver));
>> -
>> - ver = (struct qca_btsoc_version *)(edl->data);
>> + memcpy(ver, &edl->data[1], sizeof(*ver));
>
> any reason to use &edl->data[1] and not just edl->data + 1?
[Venkata]:
I will use edl->data + 1 and will update in next patchset.
>
>> + else
>> + memcpy(ver, &edl->data, sizeof(*ver));
>>
>> bt_dev_info(hdev, "QCA Product ID :0x%08x",
>> le32_to_cpu(ver->product_id));
>> @@ -83,13 +82,7 @@ int qca_read_soc_version(struct hci_dev *hdev, u32
>> *soc_version,
>> bt_dev_info(hdev, "QCA Patch Version:0x%08x",
>> le16_to_cpu(ver->patch_ver));
>>
>> - /* QCA chipset version can be decided by patch and SoC
>> - * version, combination with upper 2 bytes from SoC
>> - * and lower 2 bytes from patch will be used.
>> - */
>> - *soc_version = (le32_to_cpu(ver->soc_id) << 16) |
>> - (le16_to_cpu(ver->rom_ver) & 0x0000ffff);
>> - if (*soc_version == 0)
>> + if (le32_to_cpu(ver->soc_id) == 0 || le16_to_cpu(ver->rom_ver) == 0)
>> err = -EILSEQ;
>>
>> out:
>> @@ -446,15 +439,25 @@ int qca_set_bdaddr_rome(struct hci_dev *hdev,
>> const bdaddr_t *bdaddr)
>> EXPORT_SYMBOL_GPL(qca_set_bdaddr_rome);
>>
>> int qca_uart_setup(struct hci_dev *hdev, uint8_t baudrate,
>> - enum qca_btsoc_type soc_type, u32 soc_ver,
>> + enum qca_btsoc_type soc_type, struct qca_btsoc_version ver,
>> const char *firmware_name)
>> {
>> struct qca_fw_config config;
>> int err;
>> u8 rom_ver = 0;
>> + u32 soc_ver;
>>
>> bt_dev_dbg(hdev, "QCA setup on UART");
>>
>> + /* QCA chipset version can be decided by patch and SoC
>> + * version, combination with upper 2 bytes from SoC
>> + * and lower 2 bytes from patch will be used.
>> + */
>> + soc_ver = (le32_to_cpu(ver.soc_id) << 16) |
>> + (le16_to_cpu(ver.rom_ver) & 0x0000ffff);
>
> Please indent this properly.
>
[Venkat]:
Sure.I will update in next patchset.
> I am also confused about the 0x0000ffff since it is just 16-bit value
> in the first place. Also where do you want to shift the 32-bit value
> to.
>
> Frankly, I find all this version magic kinda questionable.
>
>> +
>> + bt_dev_info(hdev, "QCA controller version 0x%08x", soc_ver);
>> +
>> config.user_baud_rate = baudrate;
>>
>> /* Download rampatch file */
>> @@ -491,9 +494,15 @@ int qca_uart_setup(struct hci_dev *hdev, uint8_t
>> baudrate,
>> if (firmware_name)
>> snprintf(config.fwname, sizeof(config.fwname),
>> "qca/%s", firmware_name);
>> - else if (qca_is_wcn399x(soc_type))
>> - snprintf(config.fwname, sizeof(config.fwname),
>> - "qca/crnv%02x.bin", rom_ver);
>> + else if (qca_is_wcn399x(soc_type)) {
>> + if (ver.soc_id == QCA_WCN3991_SOC_ID) {
>> + snprintf(config.fwname, sizeof(config.fwname),
>> + "qca/crnv%02xu.bin", rom_ver);
>> + } else {
>> + snprintf(config.fwname, sizeof(config.fwname),
>> + "qca/crnv%02x.bin", rom_ver);
>> + }
>> + }
>> else if (soc_type == QCA_QCA6390)
>> snprintf(config.fwname, sizeof(config.fwname),
>> "qca/htnv%02x.bin", rom_ver);
>> diff --git a/drivers/bluetooth/btqca.h b/drivers/bluetooth/btqca.h
>> index d81b74c..d01a9f5 100644
>> --- a/drivers/bluetooth/btqca.h
>> +++ b/drivers/bluetooth/btqca.h
>> @@ -34,6 +34,8 @@
>> #define QCA_HCI_CC_OPCODE 0xFC00
>> #define QCA_HCI_CC_SUCCESS 0x00
>>
>> +#define QCA_WCN3991_SOC_ID (0x40014320)
>> +
>> enum qca_baudrate {
>> QCA_BAUDRATE_115200 = 0,
>> QCA_BAUDRATE_57600,
>> @@ -136,9 +138,9 @@ enum qca_btsoc_type {
>>
>> int qca_set_bdaddr_rome(struct hci_dev *hdev, const bdaddr_t *bdaddr);
>> int qca_uart_setup(struct hci_dev *hdev, uint8_t baudrate,
>> - enum qca_btsoc_type soc_type, u32 soc_ver,
>> + enum qca_btsoc_type soc_type, struct qca_btsoc_version ver,
>> const char *firmware_name);
>> -int qca_read_soc_version(struct hci_dev *hdev, u32 *soc_version,
>> +int qca_read_soc_version(struct hci_dev *hdev, struct
>> qca_btsoc_version *ver,
>> enum qca_btsoc_type);
>> int qca_set_bdaddr(struct hci_dev *hdev, const bdaddr_t *bdaddr);
>> int qca_send_pre_shutdown_cmd(struct hci_dev *hdev);
>> @@ -155,13 +157,14 @@ static inline int qca_set_bdaddr_rome(struct
>> hci_dev *hdev, const bdaddr_t *bdad
>> }
>>
>> static inline int qca_uart_setup(struct hci_dev *hdev, uint8_t
>> baudrate,
>> - enum qca_btsoc_type soc_type, u32 soc_ver,
>> - const char *firmware_name)
>> + enum qca_btsoc_type soc_type, struct qca_btsoc_version ver,
>> + const char *firmware_name)
>
> Please use proper indentation.
>
[Venkat]:
Sure.I will update in next patchset.
>> {
>> return -EOPNOTSUPP;
>> }
>>
>> -static inline int qca_read_soc_version(struct hci_dev *hdev, u32
>> *soc_version,
>> +static inline int qca_read_soc_version(struct hci_dev *hdev,
>> + struct qca_btsoc_version *ver,
>> enum qca_btsoc_type)
>> {
>> return -EOPNOTSUPP;
>> diff --git a/drivers/bluetooth/hci_qca.c b/drivers/bluetooth/hci_qca.c
>> index 244b8fe..4c32c60 100644
>> --- a/drivers/bluetooth/hci_qca.c
>> +++ b/drivers/bluetooth/hci_qca.c
>> @@ -1649,7 +1649,7 @@ static int qca_setup(struct hci_uart *hu)
>> enum qca_btsoc_type soc_type = qca_soc_type(hu);
>> const char *firmware_name = qca_get_firmware_name(hu);
>> int ret;
>> - int soc_ver = 0;
>> + struct qca_btsoc_version ver;
>>
>> ret = qca_check_speeds(hu);
>> if (ret)
>> @@ -1678,7 +1678,7 @@ static int qca_setup(struct hci_uart *hu)
>> if (qca_is_wcn399x(soc_type)) {
>> set_bit(HCI_QUIRK_USE_BDADDR_PROPERTY, &hdev->quirks);
>>
>> - ret = qca_read_soc_version(hdev, &soc_ver, soc_type);
>> + ret = qca_read_soc_version(hdev, &ver, soc_type);
>> if (ret)
>> return ret;
>> } else {
>> @@ -1697,15 +1697,14 @@ static int qca_setup(struct hci_uart *hu)
>>
>> if (!qca_is_wcn399x(soc_type)) {
>> /* Get QCA version information */
>> - ret = qca_read_soc_version(hdev, &soc_ver, soc_type);
>> + ret = qca_read_soc_version(hdev, &ver, soc_type);
>> if (ret)
>> return ret;
>> }
>>
>> - bt_dev_info(hdev, "QCA controller version 0x%08x", soc_ver);
>> /* Setup patch / NVM configurations */
>> - ret = qca_uart_setup(hdev, qca_baudrate, soc_type, soc_ver,
>> - firmware_name);
>> + ret = qca_uart_setup(hdev, qca_baudrate, soc_type, ver,
>> + firmware_name);
>> if (!ret) {
>> set_bit(QCA_IBS_ENABLED, &qca->flags);
>> qca_debugfs_init(hdev);
>
> Regards
>
> Marcel
next prev parent reply other threads:[~2020-11-19 12:53 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-09-15 15:43 [PATCH v1] Bluetooth: Use NVM files based on SoC ID for WCN3991 Venkata Lakshmi Narayana Gubba
2020-09-16 14:27 ` Marcel Holtmann
2020-11-19 12:52 ` gubbaven [this message]
2020-09-16 18:02 ` Matthias Kaehlcke
2020-11-19 12:50 ` gubbaven
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=b98756e1e34b4dbde0b0374c50765a33@codeaurora.org \
--to=gubbaven@codeaurora.org \
--cc=abhishekpandit@chromium.org \
--cc=bgodavar@codeaurora.org \
--cc=hbandi@codeaurora.org \
--cc=hemantg@codeaurora.org \
--cc=johan.hedberg@gmail.com \
--cc=linux-arm-msm@vger.kernel.org \
--cc=linux-bluetooth@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=marcel@holtmann.org \
--cc=mka@chromium.org \
--cc=rjliao@codeaurora.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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.