Re: [PATCH v1] Bluetooth: Use NVM files based on SoC ID for WCN3991

2020-11-19 Thread gubbaven

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 


---
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(>data, >data[1], sizeof(*ver));
-
-   ver = (struct qca_btsoc_version *)(edl->data);
+   memcpy(ver, >data[1], sizeof(*ver));


any reason to use >data[1] and not just edl->data + 1?

[Venkata]:
I will use edl->data + 1 and will update in next patchset.



+   else
+   memcpy(ver, >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) & 0x);
-   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) & 0x);


Please indent this properly.


[Venkat]:
Sure.I will update in next patchset.

I am also confused about the 0x 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 

Re: [PATCH v1] Bluetooth: Use NVM files based on SoC ID for WCN3991

2020-11-19 Thread gubbaven

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 


---
 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(>data, >data[1], sizeof(*ver));
-
-   ver = (struct qca_btsoc_version *)(edl->data);
+   memcpy(ver, >data[1], sizeof(*ver));
+   else
+   memcpy(ver, >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) & 0x);
-   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) & 0x);
+


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


Re: [PATCH v1] Bluetooth: Use NVM files based on SoC ID for WCN3991

2020-09-16 Thread Marcel Holtmann
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 
> ---
> 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(>data, >data[1], sizeof(*ver));
> -
> - ver = (struct qca_btsoc_version *)(edl->data);
> + memcpy(ver, >data[1], sizeof(*ver));

any reason to use >data[1] and not just edl->data + 1?

> + else
> + memcpy(ver, >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) & 0x);
> - 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) & 0x);

Please indent this properly.

I am also confused about the 0x 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_SUCCESS0x00
> 
> +#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, 

Re: [PATCH v1] Bluetooth: Use NVM files based on SoC ID for WCN3991

2020-09-16 Thread Matthias Kaehlcke
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 
> ---
>  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(>data, >data[1], sizeof(*ver));
> -
> - ver = (struct qca_btsoc_version *)(edl->data);
> + memcpy(ver, >data[1], sizeof(*ver));
> + else
> + memcpy(ver, >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) & 0x);
> - 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) & 0x);
> +

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.

> + 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_OPCODE0xFC00
>  #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 

[PATCH v1] Bluetooth: Use NVM files based on SoC ID for WCN3991

2020-09-15 Thread Venkata Lakshmi Narayana Gubba
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 
---
 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(>data, >data[1], sizeof(*ver));
-
-   ver = (struct qca_btsoc_version *)(edl->data);
+   memcpy(ver, >data[1], sizeof(*ver));
+   else
+   memcpy(ver, >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) & 0x);
-   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) & 0x);
+
+   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