Messages in this thread | | | Date | Thu, 19 Nov 2020 18:20:11 +0530 | From | gubbaven@codeauro ... | Subject | Re: [PATCH v1] Bluetooth: Use NVM files based on SoC ID for WCN3991 |
| |
Hi Matthias,
On 2020-09-16 23:32, Matthias Kaehlcke wrote: > Hi Venkata, > > I agree with Marcel that the version magic is confusing ... > > On Tue, Sep 15, 2020 at 09:13:25PM +0530, Venkata Lakshmi Narayana > Gubba wrote: >> 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)); >> + 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); >> + > > Can we at least do the leN_to_cpu conversions in qca_read_soc_version() > as previously to make this less clunky? > > And/or define a macro to extract 'soc_ver' to unclunkify this further. > [Venkata]: I will define a macro to extract soc_ver and will update in next patchset. >> + 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) > > The QCA_ prefix seems a bit verbose, given that this is a QCA driver > and > WCN3991 uniquely identifies the chip. Having the prefix just needlessly > clutters conditions, I suggest to just call it SOC_ID_WCN3991.
| |