[PATCH v4 0/3] Expand firmware-name property to load specific

[PATCH v4 0/3] Expand firmware-name property to load specific

[Cheng Jiang]

Expand the firmware-name property to specify the names of NVM and
rampatch firmware to load.

This update will support loading specific firmware (nvm and rampatch)
for certain chips, like the QCA6698 Bluetooth chip, which shares the
same IP core as the WCN6855 but has different RF components and RAM
sizes, requiring new firmware files.

Different connectivity boards may be attached to the same platform. For
example, QCA6698-based boards can support either a two-antenna or
three-antenna solution, both of which work on the sa8775p-ride platform.
Due to differences in connectivity boards and variations in RF
performance from different foundries, different NVM configurations are
used based on the board ID.

So In firmware-name, if the NVM file has an extension, the NVM file will
be used. Otherwise, the system will first try the .bNN (board ID) file,
and if that fails, it will fall back to the .bin file.

Possible configurations:
firmware-name = "QCA6698/hpnv21.bin", "QCA6698/hpbtfw21.tlv";
firmware-name = "QCA6698/hpnv21", "QCA6698/hpbtfw21.tlv";
firmware-name = "QCA6698/hpnv21.bin";

---
v4:
  1. Split nvm and rampatch changes to 2 commits
  2. Code fix according to review comments

v3:
  1.Expand firmware-name property to specify the nvm and rampatch to
  load.
  2.Change the driver to support two items in firmware-name and choose
  the NVM file according to board id if there is no extension in NVM
  file.
  3.Update the dts file to idendify the firmware for QCA6698.
---

Cheng Jiang (4):
  dt-bindings: net: bluetooth: qca: Expand firmware-name property
  Bluetooth: qca: Add support in firmware-name to load board specific
    nvm
  Bluetooth: qca: Expand firmware-name to load specific rampatch
  arm64: dts: qcom: sa8775p-ride: Add firmware-name in BT node

 .../net/bluetooth/qualcomm-bluetooth.yaml     |   5 +-
 arch/arm64/boot/dts/qcom/sa8775p-ride.dtsi    |   1 +
 drivers/bluetooth/btqca.c                     | 149 +++++++++++++-----
 drivers/bluetooth/btqca.h                     |   5 +-
 drivers/bluetooth/hci_qca.c                   |  22 ++-
 5 files changed, 133 insertions(+), 49 deletions(-)


base-commit: ebe1b11614e079c5e366ce9bd3c8f44ca0fbcc1b
-- 
2.25.1

[PATCH v4 1/4] dt-bindings: net: bluetooth: qca: Expand firmware-name property

[Cheng Jiang]

Expand the firmware-name property to specify the names of NVM and
rampatch firmware to load. This update will support loading specific
firmware (nvm and rampatch) for certain chips, like the QCA6698
Bluetooth chip, which shares the same IP core as the WCN6855 but has
different RF components and RAM sizes, requiring new firmware files.

We might use different connectivity boards on the same platform. For
example, QCA6698-based boards can support either a two-antenna or
three-antenna solution, both of which work on the sa8775p-ride platform.
Due to differences in connectivity boards and variations in RF
performance from different foundries, different NVM configurations are
used based on the board ID.

So In firmware-name, if the NVM file has an extension, the NVM file will
be used. Otherwise, the system will first try the .bNN (board ID) file,
and if that fails, it will fall back to the .bin file.

Possible configurations:
firmware-name = "QCA6698/hpnv21.bin", "QCA6698/hpbtfw21.tlv";
firmware-name = "QCA6698/hpnv21", "QCA6698/hpbtfw21.tlv";
firmware-name = "QCA6698/hpnv21.bin";

Signed-off-by: Cheng Jiang <quic_chejiang@quicinc.com>
---
 .../bindings/net/bluetooth/qualcomm-bluetooth.yaml           | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/net/bluetooth/qualcomm-bluetooth.yaml b/Documentation/devicetree/bindings/net/bluetooth/qualcomm-bluetooth.yaml
index 7bb68311c..2782d2325 100644
--- a/Documentation/devicetree/bindings/net/bluetooth/qualcomm-bluetooth.yaml
+++ b/Documentation/devicetree/bindings/net/bluetooth/qualcomm-bluetooth.yaml
@@ -101,7 +101,10 @@ properties:
   max-speed: true
 
   firmware-name:
-    description: specify the name of nvm firmware to load
+    description:
+      If one item is present, specify the name of the NVM firmware to load.
+      If two items are present, the first item specifies the name of the NVM,
+      and the second specifies the name of the rampatch firmware to load.
 
   local-bd-address: true
 
-- 
2.25.1

Re: [PATCH v4 1/4] dt-bindings: net: bluetooth: qca: Expand firmware-name property

[Krzysztof Kozlowski]

On Tue, Dec 10, 2024 at 11:16:33PM +0800, Cheng Jiang wrote:
> Expand the firmware-name property to specify the names of NVM and
> rampatch firmware to load. This update will support loading specific
> firmware (nvm and rampatch) for certain chips, like the QCA6698
> Bluetooth chip, which shares the same IP core as the WCN6855 but has
> different RF components and RAM sizes, requiring new firmware files.
> 
> We might use different connectivity boards on the same platform. For
> example, QCA6698-based boards can support either a two-antenna or
> three-antenna solution, both of which work on the sa8775p-ride platform.
> Due to differences in connectivity boards and variations in RF
> performance from different foundries, different NVM configurations are
> used based on the board ID.
> 
> So In firmware-name, if the NVM file has an extension, the NVM file will
> be used. Otherwise, the system will first try the .bNN (board ID) file,
> and if that fails, it will fall back to the .bin file.
> 
> Possible configurations:
> firmware-name = "QCA6698/hpnv21.bin", "QCA6698/hpbtfw21.tlv";
> firmware-name = "QCA6698/hpnv21", "QCA6698/hpbtfw21.tlv";
> firmware-name = "QCA6698/hpnv21.bin";
> 
> Signed-off-by: Cheng Jiang <quic_chejiang@quicinc.com>
> ---
>  .../bindings/net/bluetooth/qualcomm-bluetooth.yaml           | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/devicetree/bindings/net/bluetooth/qualcomm-bluetooth.yaml b/Documentation/devicetree/bindings/net/bluetooth/qualcomm-bluetooth.yaml
> index 7bb68311c..2782d2325 100644
> --- a/Documentation/devicetree/bindings/net/bluetooth/qualcomm-bluetooth.yaml
> +++ b/Documentation/devicetree/bindings/net/bluetooth/qualcomm-bluetooth.yaml
> @@ -101,7 +101,10 @@ properties:
>    max-speed: true
>  
>    firmware-name:
> -    description: specify the name of nvm firmware to load
> +    description:
> +      If one item is present, specify the name of the NVM firmware to load.
> +      If two items are present, the first item specifies the name of the NVM,
> +      and the second specifies the name of the rampatch firmware to load.

Don't repeat constraints in free form text. Use proper constraints so
you can validate your DTS. And then actually do validate your DTS...

Best regards,
Krzysztof

Re: [PATCH v4 1/4] dt-bindings: net: bluetooth: qca: Expand firmware-name property

[Cheng Jiang (IOE)]

Hi Krzysztof,

On 12/11/2024 4:53 PM, Krzysztof Kozlowski wrote:
> On Tue, Dec 10, 2024 at 11:16:33PM +0800, Cheng Jiang wrote:
>> Expand the firmware-name property to specify the names of NVM and
>> rampatch firmware to load. This update will support loading specific
>> firmware (nvm and rampatch) for certain chips, like the QCA6698
>> Bluetooth chip, which shares the same IP core as the WCN6855 but has
>> different RF components and RAM sizes, requiring new firmware files.
>>
>> We might use different connectivity boards on the same platform. For
>> example, QCA6698-based boards can support either a two-antenna or
>> three-antenna solution, both of which work on the sa8775p-ride platform.
>> Due to differences in connectivity boards and variations in RF
>> performance from different foundries, different NVM configurations are
>> used based on the board ID.
>>
>> So In firmware-name, if the NVM file has an extension, the NVM file will
>> be used. Otherwise, the system will first try the .bNN (board ID) file,
>> and if that fails, it will fall back to the .bin file.
>>
>> Possible configurations:
>> firmware-name = "QCA6698/hpnv21.bin", "QCA6698/hpbtfw21.tlv";
>> firmware-name = "QCA6698/hpnv21", "QCA6698/hpbtfw21.tlv";
>> firmware-name = "QCA6698/hpnv21.bin";
>>
>> Signed-off-by: Cheng Jiang <quic_chejiang@quicinc.com>
>> ---
>>  .../bindings/net/bluetooth/qualcomm-bluetooth.yaml           | 5 ++++-
>>  1 file changed, 4 insertions(+), 1 deletion(-)
>>
>> diff --git a/Documentation/devicetree/bindings/net/bluetooth/qualcomm-bluetooth.yaml b/Documentation/devicetree/bindings/net/bluetooth/qualcomm-bluetooth.yaml
>> index 7bb68311c..2782d2325 100644
>> --- a/Documentation/devicetree/bindings/net/bluetooth/qualcomm-bluetooth.yaml
>> +++ b/Documentation/devicetree/bindings/net/bluetooth/qualcomm-bluetooth.yaml
>> @@ -101,7 +101,10 @@ properties:
>>    max-speed: true
>>  
>>    firmware-name:
>> -    description: specify the name of nvm firmware to load
>> +    description:
>> +      If one item is present, specify the name of the NVM firmware to load.
>> +      If two items are present, the first item specifies the name of the NVM,
>> +      and the second specifies the name of the rampatch firmware to load.
> 
> Don't repeat constraints in free form text. Use proper constraints so
> you can validate your DTS. And then actually do validate your DTS...
> 
It seems unnecessary to add this description, so I will drop this change. Is that okay?

> Best regards,
> Krzysztof
> 

Re: [PATCH v4 1/4] dt-bindings: net: bluetooth: qca: Expand firmware-name property

[Krzysztof Kozlowski]

On 11/12/2024 10:39, Cheng Jiang (IOE) wrote:
>>>
>>> diff --git a/Documentation/devicetree/bindings/net/bluetooth/qualcomm-bluetooth.yaml b/Documentation/devicetree/bindings/net/bluetooth/qualcomm-bluetooth.yaml
>>> index 7bb68311c..2782d2325 100644
>>> --- a/Documentation/devicetree/bindings/net/bluetooth/qualcomm-bluetooth.yaml
>>> +++ b/Documentation/devicetree/bindings/net/bluetooth/qualcomm-bluetooth.yaml
>>> @@ -101,7 +101,10 @@ properties:
>>>    max-speed: true
>>>  
>>>    firmware-name:
>>> -    description: specify the name of nvm firmware to load
>>> +    description:
>>> +      If one item is present, specify the name of the NVM firmware to load.
>>> +      If two items are present, the first item specifies the name of the NVM,
>>> +      and the second specifies the name of the rampatch firmware to load.
>>
>> Don't repeat constraints in free form text. Use proper constraints so
>> you can validate your DTS. And then actually do validate your DTS...
>>
> It seems unnecessary to add this description, so I will drop this change. Is that okay?

You need to list the items and describe them. See how all other bindings
do it.

Best regards,
Krzysztof

Re: [PATCH v4 1/4] dt-bindings: net: bluetooth: qca: Expand firmware-name property

[Cheng Jiang (IOE)]

Hi Krzysztof,

On 12/11/2024 5:48 PM, Krzysztof Kozlowski wrote:
> On 11/12/2024 10:39, Cheng Jiang (IOE) wrote:
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/net/bluetooth/qualcomm-bluetooth.yaml b/Documentation/devicetree/bindings/net/bluetooth/qualcomm-bluetooth.yaml
>>>> index 7bb68311c..2782d2325 100644
>>>> --- a/Documentation/devicetree/bindings/net/bluetooth/qualcomm-bluetooth.yaml
>>>> +++ b/Documentation/devicetree/bindings/net/bluetooth/qualcomm-bluetooth.yaml
>>>> @@ -101,7 +101,10 @@ properties:
>>>>    max-speed: true
>>>>  
>>>>    firmware-name:
>>>> -    description: specify the name of nvm firmware to load
>>>> +    description:
>>>> +      If one item is present, specify the name of the NVM firmware to load.
>>>> +      If two items are present, the first item specifies the name of the NVM,
>>>> +      and the second specifies the name of the rampatch firmware to load.
>>>
>>> Don't repeat constraints in free form text. Use proper constraints so
>>> you can validate your DTS. And then actually do validate your DTS...
>>>
>> It seems unnecessary to add this description, so I will drop this change. Is that okay?
> 
> You need to list the items and describe them. See how all other bindings
> do it.
> 
The firmware names are not fixed strings; they vary depending on the chip, board, or platform.

How about the following description? Thank you!

  firmware-name:
    $ref: /schemas/types.yaml#/definitions/string
    description: |
      List of firmware names. The first item is the name of the NVM firmware
      to load. The second item is the name of the rampatch firmware to load,
      if present.
    minItems: 1
    maxItems: 2

> Best regards,
> Krzysztof

Re: [PATCH v4 1/4] dt-bindings: net: bluetooth: qca: Expand firmware-name property

[Dmitry Baryshkov]

On Wed, Dec 11, 2024 at 06:16:44PM +0800, Cheng Jiang (IOE) wrote:
> Hi Krzysztof,
> 
> On 12/11/2024 5:48 PM, Krzysztof Kozlowski wrote:
> > On 11/12/2024 10:39, Cheng Jiang (IOE) wrote:
> >>>>
> >>>> diff --git a/Documentation/devicetree/bindings/net/bluetooth/qualcomm-bluetooth.yaml b/Documentation/devicetree/bindings/net/bluetooth/qualcomm-bluetooth.yaml
> >>>> index 7bb68311c..2782d2325 100644
> >>>> --- a/Documentation/devicetree/bindings/net/bluetooth/qualcomm-bluetooth.yaml
> >>>> +++ b/Documentation/devicetree/bindings/net/bluetooth/qualcomm-bluetooth.yaml
> >>>> @@ -101,7 +101,10 @@ properties:
> >>>>    max-speed: true
> >>>>  
> >>>>    firmware-name:
> >>>> -    description: specify the name of nvm firmware to load
> >>>> +    description:
> >>>> +      If one item is present, specify the name of the NVM firmware to load.
> >>>> +      If two items are present, the first item specifies the name of the NVM,
> >>>> +      and the second specifies the name of the rampatch firmware to load.
> >>>
> >>> Don't repeat constraints in free form text. Use proper constraints so
> >>> you can validate your DTS. And then actually do validate your DTS...
> >>>
> >> It seems unnecessary to add this description, so I will drop this change. Is that okay?
> > 
> > You need to list the items and describe them. See how all other bindings
> > do it.
> > 
> The firmware names are not fixed strings; they vary depending on the chip, board, or platform.
> 
> How about the following description? Thank you!
> 
>   firmware-name:
>     $ref: /schemas/types.yaml#/definitions/string
>     description: |
>       List of firmware names. The first item is the name of the NVM firmware
>       to load. The second item is the name of the rampatch firmware to load,
>       if present.
>     minItems: 1
>     maxItems: 2

I think this is better:

firmware-name:
  minItems: 1
  items:
    - description: NVM firmware to load (extend the desription)
    - description: rampatch (extend the description)

> > Best regards,
> > Krzysztof
> 

-- 
With best wishes
Dmitry

Re: [PATCH v4 1/4] dt-bindings: net: bluetooth: qca: Expand firmware-name property

[Cheng Jiang (IOE)]

Hi Krzysztof,

On 12/12/2024 3:28 AM, Dmitry Baryshkov wrote:
> On Wed, Dec 11, 2024 at 06:16:44PM +0800, Cheng Jiang (IOE) wrote:
>> Hi Krzysztof,
>>
>> On 12/11/2024 5:48 PM, Krzysztof Kozlowski wrote:
>>> On 11/12/2024 10:39, Cheng Jiang (IOE) wrote:
>>>>>>
>>>>>> diff --git a/Documentation/devicetree/bindings/net/bluetooth/qualcomm-bluetooth.yaml b/Documentation/devicetree/bindings/net/bluetooth/qualcomm-bluetooth.yaml
>>>>>> index 7bb68311c..2782d2325 100644
>>>>>> --- a/Documentation/devicetree/bindings/net/bluetooth/qualcomm-bluetooth.yaml
>>>>>> +++ b/Documentation/devicetree/bindings/net/bluetooth/qualcomm-bluetooth.yaml
>>>>>> @@ -101,7 +101,10 @@ properties:
>>>>>>    max-speed: true
>>>>>>  
>>>>>>    firmware-name:
>>>>>> -    description: specify the name of nvm firmware to load
>>>>>> +    description:
>>>>>> +      If one item is present, specify the name of the NVM firmware to load.
>>>>>> +      If two items are present, the first item specifies the name of the NVM,
>>>>>> +      and the second specifies the name of the rampatch firmware to load.
>>>>>
>>>>> Don't repeat constraints in free form text. Use proper constraints so
>>>>> you can validate your DTS. And then actually do validate your DTS...
>>>>>
>>>> It seems unnecessary to add this description, so I will drop this change. Is that okay?
>>>
>>> You need to list the items and describe them. See how all other bindings
>>> do it.
>>>
>> The firmware names are not fixed strings; they vary depending on the chip, board, or platform.
>>
>> How about the following description? Thank you!
>>
>>   firmware-name:
>>     $ref: /schemas/types.yaml#/definitions/string
>>     description: |
>>       List of firmware names. The first item is the name of the NVM firmware
>>       to load. The second item is the name of the rampatch firmware to load,
>>       if present.
>>     minItems: 1
>>     maxItems: 2
> 
> I think this is better:
> 
> firmware-name:
>   minItems: 1
>   items:
>     - description: NVM firmware to load (extend the desription)
>     - description: rampatch (extend the description)
> 
Ack. Thank you very much for the suggestion. 

>>> Best regards,
>>> Krzysztof
>>
> 

Re: [PATCH v4 1/4] dt-bindings: net: bluetooth: qca: Expand firmware-name property

[Krzysztof Kozlowski]

On 11/12/2024 11:16, Cheng Jiang (IOE) wrote:
> Hi Krzysztof,
> 
> On 12/11/2024 5:48 PM, Krzysztof Kozlowski wrote:
>> On 11/12/2024 10:39, Cheng Jiang (IOE) wrote:
>>>>>
>>>>> diff --git a/Documentation/devicetree/bindings/net/bluetooth/qualcomm-bluetooth.yaml b/Documentation/devicetree/bindings/net/bluetooth/qualcomm-bluetooth.yaml
>>>>> index 7bb68311c..2782d2325 100644
>>>>> --- a/Documentation/devicetree/bindings/net/bluetooth/qualcomm-bluetooth.yaml
>>>>> +++ b/Documentation/devicetree/bindings/net/bluetooth/qualcomm-bluetooth.yaml
>>>>> @@ -101,7 +101,10 @@ properties:
>>>>>    max-speed: true
>>>>>  
>>>>>    firmware-name:
>>>>> -    description: specify the name of nvm firmware to load
>>>>> +    description:
>>>>> +      If one item is present, specify the name of the NVM firmware to load.
>>>>> +      If two items are present, the first item specifies the name of the NVM,
>>>>> +      and the second specifies the name of the rampatch firmware to load.
>>>>
>>>> Don't repeat constraints in free form text. Use proper constraints so
>>>> you can validate your DTS. And then actually do validate your DTS...
>>>>
>>> It seems unnecessary to add this description, so I will drop this change. Is that okay?
>>
>> You need to list the items and describe them. See how all other bindings
>> do it.
>>
> The firmware names are not fixed strings; they vary depending on the chip, board, or platform.

Instead of replying immediately and pushing this back again on us, read
other bindings. There are nowhere "fixed strings".


Best regards,
Krzysztof

Re: [PATCH v4 1/4] dt-bindings: net: bluetooth: qca: Expand firmware-name property

[Cheng Jiang (IOE)]

Hi Krzysztof,

On 12/12/2024 3:16 PM, Krzysztof Kozlowski wrote:
> On 11/12/2024 11:16, Cheng Jiang (IOE) wrote:
>> Hi Krzysztof,
>>
>> On 12/11/2024 5:48 PM, Krzysztof Kozlowski wrote:
>>> On 11/12/2024 10:39, Cheng Jiang (IOE) wrote:
>>>>>>
>>>>>> diff --git a/Documentation/devicetree/bindings/net/bluetooth/qualcomm-bluetooth.yaml b/Documentation/devicetree/bindings/net/bluetooth/qualcomm-bluetooth.yaml
>>>>>> index 7bb68311c..2782d2325 100644
>>>>>> --- a/Documentation/devicetree/bindings/net/bluetooth/qualcomm-bluetooth.yaml
>>>>>> +++ b/Documentation/devicetree/bindings/net/bluetooth/qualcomm-bluetooth.yaml
>>>>>> @@ -101,7 +101,10 @@ properties:
>>>>>>    max-speed: true
>>>>>>  
>>>>>>    firmware-name:
>>>>>> -    description: specify the name of nvm firmware to load
>>>>>> +    description:
>>>>>> +      If one item is present, specify the name of the NVM firmware to load.
>>>>>> +      If two items are present, the first item specifies the name of the NVM,
>>>>>> +      and the second specifies the name of the rampatch firmware to load.
>>>>>
>>>>> Don't repeat constraints in free form text. Use proper constraints so
>>>>> you can validate your DTS. And then actually do validate your DTS...
>>>>>
>>>> It seems unnecessary to add this description, so I will drop this change. Is that okay?
>>>
>>> You need to list the items and describe them. See how all other bindings
>>> do it.
>>>
>> The firmware names are not fixed strings; they vary depending on the chip, board, or platform.
> 
> Instead of replying immediately and pushing this back again on us, read
> other bindings. There are nowhere "fixed strings".
> 
Ack. I will take care of it next time. Thank you!
> 
> Best regards,
> Krzysztof

[PATCH v4 2/4] Bluetooth: qca: Add support in firmware-name to load board specific nvm

[Cheng Jiang]

Different connectivity boards may be attached to the same platform. For
example, QCA6698-based boards can support either a two-antenna or
three-antenna solution, both of which work on the sa8775p-ride platform.
Due to differences in connectivity boards and variations in RF
performance from different foundries, different NVM configurations are
used based on the board ID.

Therefore, in the firmware-name property, if the NVM file has an
extension, the NVM file will be used. Otherwise, the system will first
try the .bNN (board ID) file, and if that fails, it will fall back to
the .bin file.

Possible configurations:
firmware-name = "QCA6698/hpnv21";
firmware-name = "QCA6698/hpnv21.bin";

Signed-off-by: Cheng Jiang <quic_chejiang@quicinc.com>
---
 drivers/bluetooth/btqca.c | 67 +++++++++++++++++++++++++++++++++++++--
 1 file changed, 65 insertions(+), 2 deletions(-)

diff --git a/drivers/bluetooth/btqca.c b/drivers/bluetooth/btqca.c
index dfbbac922..deb2b1753 100644
--- a/drivers/bluetooth/btqca.c
+++ b/drivers/bluetooth/btqca.c
@@ -272,6 +272,27 @@ int qca_send_pre_shutdown_cmd(struct hci_dev *hdev)
 }
 EXPORT_SYMBOL_GPL(qca_send_pre_shutdown_cmd);
 
+static bool qca_get_alt_nvm_path(char *path, size_t max_size)
+{
+	char fwname[64];
+	const char *suffix;
+
+	suffix = strrchr(path, '.');
+
+	/* nvm file name has a suffix, replace with .bin */
+	if (suffix && suffix != path && *(suffix + 1) != '\0' && strchr(suffix, '/') == NULL) {
+		strscpy(fwname, path, suffix - path + 1);
+		snprintf(fwname + (suffix - path), sizeof(fwname) - (suffix - path), ".bin");
+		/* If nvm file is already the default one, return false to skip the retry. */
+		if (strcmp(fwname, path) == 0)
+			return false;
+
+		snprintf(path, max_size, "%s", fwname);
+		return true;
+	}
+	return false;
+}
+
 static int qca_tlv_check_data(struct hci_dev *hdev,
 			       struct qca_fw_config *config,
 			       u8 *fw_data, size_t fw_size,
@@ -564,6 +585,19 @@ static int qca_download_firmware(struct hci_dev *hdev,
 					   config->fwname, ret);
 				return ret;
 			}
+		}
+		/* For nvm, if desired nvm file is not present and it's not the
+		 * default nvm file(ends with .bin), try to load the default nvm.
+		 */
+		else if (config->type == TLV_TYPE_NVM &&
+			 qca_get_alt_nvm_path(config->fwname, sizeof(config->fwname))) {
+			bt_dev_info(hdev, "QCA Downloading %s", config->fwname);
+			ret = request_firmware(&fw, config->fwname, &hdev->dev);
+			if (ret) {
+				bt_dev_err(hdev, "QCA Failed to request file: %s (%d)",
+					   config->fwname, ret);
+				return ret;
+			}
 		} else {
 			bt_dev_err(hdev, "QCA Failed to request file: %s (%d)",
 				   config->fwname, ret);
@@ -730,6 +764,26 @@ static inline void qca_get_nvm_name_generic(struct qca_fw_config *cfg,
 			 "qca/%snv%02x.b%02x", stem, rom_ver, bid);
 }
 
+static void qca_get_nvm_name_by_board(char *fwname, size_t max_size,
+		const char *firmware_name, struct qca_btsoc_version ver,
+		enum qca_btsoc_type soc_type, u16 bid)
+{
+	const char *variant;
+
+	/* Set the variant to empty by default */
+	variant = "";
+	/* hsp gf chip */
+	if (soc_type == QCA_WCN6855) {
+		if ((le32_to_cpu(ver.soc_id) & QCA_HSP_GF_SOC_MASK) == QCA_HSP_GF_SOC_ID)
+			variant = "g";
+	}
+
+	if (bid == 0x0 || bid == 0xffff)
+		snprintf(fwname, max_size, "qca/%s%s.bin", firmware_name, variant);
+	else
+		snprintf(fwname, max_size, "qca/%s%s.b%02x", firmware_name, variant, bid);
+}
+
 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)
@@ -739,6 +793,7 @@ int qca_uart_setup(struct hci_dev *hdev, uint8_t baudrate,
 	u8 rom_ver = 0;
 	u32 soc_ver;
 	u16 boardid = 0;
+	const char *suffix;
 
 	bt_dev_dbg(hdev, "QCA setup on UART");
 
@@ -816,8 +871,16 @@ int qca_uart_setup(struct hci_dev *hdev, uint8_t baudrate,
 	/* Download NVM configuration */
 	config.type = TLV_TYPE_NVM;
 	if (firmware_name) {
-		snprintf(config.fwname, sizeof(config.fwname),
-			 "qca/%s", firmware_name);
+		/* The firmware name has suffix, use it directly */
+		suffix = strrchr(firmware_name, '.');
+		if (suffix && suffix != firmware_name &&
+			*(suffix + 1) != '\0' && strchr(suffix, '/') == NULL) {
+			snprintf(config.fwname, sizeof(config.fwname), "qca/%s", firmware_name);
+		} else {
+			qca_read_fw_board_id(hdev, &boardid);
+			qca_get_nvm_name_by_board(config.fwname, sizeof(config.fwname),
+				 firmware_name, ver, soc_type, boardid);
+		}
 	} else {
 		switch (soc_type) {
 		case QCA_WCN3990:
-- 
2.25.1

Re: [PATCH v4 2/4] Bluetooth: qca: Add support in firmware-name to load board specific nvm

[Dmitry Baryshkov]

On Tue, Dec 10, 2024 at 11:16:34PM +0800, Cheng Jiang wrote:
> Different connectivity boards may be attached to the same platform. For
> example, QCA6698-based boards can support either a two-antenna or
> three-antenna solution, both of which work on the sa8775p-ride platform.
> Due to differences in connectivity boards and variations in RF
> performance from different foundries, different NVM configurations are
> used based on the board ID.
> 
> Therefore, in the firmware-name property, if the NVM file has an
> extension, the NVM file will be used. Otherwise, the system will first
> try the .bNN (board ID) file, and if that fails, it will fall back to
> the .bin file.
> 
> Possible configurations:
> firmware-name = "QCA6698/hpnv21";
> firmware-name = "QCA6698/hpnv21.bin";
> 
> Signed-off-by: Cheng Jiang <quic_chejiang@quicinc.com>
> ---
>  drivers/bluetooth/btqca.c | 67 +++++++++++++++++++++++++++++++++++++--
>  1 file changed, 65 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/bluetooth/btqca.c b/drivers/bluetooth/btqca.c
> index dfbbac922..deb2b1753 100644
> --- a/drivers/bluetooth/btqca.c
> +++ b/drivers/bluetooth/btqca.c
> @@ -272,6 +272,27 @@ int qca_send_pre_shutdown_cmd(struct hci_dev *hdev)
>  }
>  EXPORT_SYMBOL_GPL(qca_send_pre_shutdown_cmd);
>  
> +static bool qca_get_alt_nvm_path(char *path, size_t max_size)
> +{
> +	char fwname[64];
> +	const char *suffix;
> +
> +	suffix = strrchr(path, '.');
> +
> +	/* nvm file name has a suffix, replace with .bin */
> +	if (suffix && suffix != path && *(suffix + 1) != '\0' && strchr(suffix, '/') == NULL) {
> +		strscpy(fwname, path, suffix - path + 1);
> +		snprintf(fwname + (suffix - path), sizeof(fwname) - (suffix - path), ".bin");
> +		/* If nvm file is already the default one, return false to skip the retry. */
> +		if (strcmp(fwname, path) == 0)
> +			return false;
> +
> +		snprintf(path, max_size, "%s", fwname);
> +		return true;
> +	}
> +	return false;
> +}
> +
>  static int qca_tlv_check_data(struct hci_dev *hdev,
>  			       struct qca_fw_config *config,
>  			       u8 *fw_data, size_t fw_size,
> @@ -564,6 +585,19 @@ static int qca_download_firmware(struct hci_dev *hdev,
>  					   config->fwname, ret);
>  				return ret;
>  			}
> +		}
> +		/* For nvm, if desired nvm file is not present and it's not the
> +		 * default nvm file(ends with .bin), try to load the default nvm.
> +		 */
> +		else if (config->type == TLV_TYPE_NVM &&
> +			 qca_get_alt_nvm_path(config->fwname, sizeof(config->fwname))) {
> +			bt_dev_info(hdev, "QCA Downloading %s", config->fwname);
> +			ret = request_firmware(&fw, config->fwname, &hdev->dev);
> +			if (ret) {
> +				bt_dev_err(hdev, "QCA Failed to request file: %s (%d)",
> +					   config->fwname, ret);
> +				return ret;
> +			}
>  		} else {
>  			bt_dev_err(hdev, "QCA Failed to request file: %s (%d)",
>  				   config->fwname, ret);
> @@ -730,6 +764,26 @@ static inline void qca_get_nvm_name_generic(struct qca_fw_config *cfg,
>  			 "qca/%snv%02x.b%02x", stem, rom_ver, bid);
>  }
>  
> +static void qca_get_nvm_name_by_board(char *fwname, size_t max_size,
> +		const char *firmware_name, struct qca_btsoc_version ver,
> +		enum qca_btsoc_type soc_type, u16 bid)
> +{
> +	const char *variant;
> +
> +	/* Set the variant to empty by default */
> +	variant = "";
> +	/* hsp gf chip */
> +	if (soc_type == QCA_WCN6855) {
> +		if ((le32_to_cpu(ver.soc_id) & QCA_HSP_GF_SOC_MASK) == QCA_HSP_GF_SOC_ID)
> +			variant = "g";
> +	}
> +
> +	if (bid == 0x0 || bid == 0xffff)
> +		snprintf(fwname, max_size, "qca/%s%s.bin", firmware_name, variant);
> +	else
> +		snprintf(fwname, max_size, "qca/%s%s.b%02x", firmware_name, variant, bid);

So, we have qca_generate_hsp_nvm_name(), qca_get_nvm_name_generic(), now
you are adding a third one. Can we please have a single function that
handles that?

> +}
> +
>  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)
> @@ -739,6 +793,7 @@ int qca_uart_setup(struct hci_dev *hdev, uint8_t baudrate,
>  	u8 rom_ver = 0;
>  	u32 soc_ver;
>  	u16 boardid = 0;
> +	const char *suffix;
>  
>  	bt_dev_dbg(hdev, "QCA setup on UART");
>  
> @@ -816,8 +871,16 @@ int qca_uart_setup(struct hci_dev *hdev, uint8_t baudrate,
>  	/* Download NVM configuration */
>  	config.type = TLV_TYPE_NVM;
>  	if (firmware_name) {
> -		snprintf(config.fwname, sizeof(config.fwname),
> -			 "qca/%s", firmware_name);
> +		/* The firmware name has suffix, use it directly */
> +		suffix = strrchr(firmware_name, '.');
> +		if (suffix && suffix != firmware_name &&
> +			*(suffix + 1) != '\0' && strchr(suffix, '/') == NULL) {

The have-suffix code should be extracted to a helper function. You have
two copies of it.

> +			snprintf(config.fwname, sizeof(config.fwname), "qca/%s", firmware_name);
> +		} else {
> +			qca_read_fw_board_id(hdev, &boardid);
> +			qca_get_nvm_name_by_board(config.fwname, sizeof(config.fwname),
> +				 firmware_name, ver, soc_type, boardid);
> +		}
>  	} else {
>  		switch (soc_type) {
>  		case QCA_WCN3990:
> -- 
> 2.25.1
> 

-- 
With best wishes
Dmitry

Re: [PATCH v4 2/4] Bluetooth: qca: Add support in firmware-name to load board specific nvm

[Cheng Jiang (IOE)]

Hi Dmitry, 

On 12/10/2024 11:28 PM, Dmitry Baryshkov wrote:
> On Tue, Dec 10, 2024 at 11:16:34PM +0800, Cheng Jiang wrote:
>> Different connectivity boards may be attached to the same platform. For
>> example, QCA6698-based boards can support either a two-antenna or
>> three-antenna solution, both of which work on the sa8775p-ride platform.
>> Due to differences in connectivity boards and variations in RF
>> performance from different foundries, different NVM configurations are
>> used based on the board ID.
>>
>> Therefore, in the firmware-name property, if the NVM file has an
>> extension, the NVM file will be used. Otherwise, the system will first
>> try the .bNN (board ID) file, and if that fails, it will fall back to
>> the .bin file.
>>
>> Possible configurations:
>> firmware-name = "QCA6698/hpnv21";
>> firmware-name = "QCA6698/hpnv21.bin";
>>
>> Signed-off-by: Cheng Jiang <quic_chejiang@quicinc.com>
>> ---
>>  drivers/bluetooth/btqca.c | 67 +++++++++++++++++++++++++++++++++++++--
>>  1 file changed, 65 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/bluetooth/btqca.c b/drivers/bluetooth/btqca.c
>> index dfbbac922..deb2b1753 100644
>> --- a/drivers/bluetooth/btqca.c
>> +++ b/drivers/bluetooth/btqca.c
>> @@ -272,6 +272,27 @@ int qca_send_pre_shutdown_cmd(struct hci_dev *hdev)
>>  }
>>  EXPORT_SYMBOL_GPL(qca_send_pre_shutdown_cmd);
>>  
>> +static bool qca_get_alt_nvm_path(char *path, size_t max_size)
>> +{
>> +	char fwname[64];
>> +	const char *suffix;
>> +
>> +	suffix = strrchr(path, '.');
>> +
>> +	/* nvm file name has a suffix, replace with .bin */
>> +	if (suffix && suffix != path && *(suffix + 1) != '\0' && strchr(suffix, '/') == NULL) {
>> +		strscpy(fwname, path, suffix - path + 1);
>> +		snprintf(fwname + (suffix - path), sizeof(fwname) - (suffix - path), ".bin");
>> +		/* If nvm file is already the default one, return false to skip the retry. */
>> +		if (strcmp(fwname, path) == 0)
>> +			return false;
>> +
>> +		snprintf(path, max_size, "%s", fwname);
>> +		return true;
>> +	}
>> +	return false;
>> +}
>> +
>>  static int qca_tlv_check_data(struct hci_dev *hdev,
>>  			       struct qca_fw_config *config,
>>  			       u8 *fw_data, size_t fw_size,
>> @@ -564,6 +585,19 @@ static int qca_download_firmware(struct hci_dev *hdev,
>>  					   config->fwname, ret);
>>  				return ret;
>>  			}
>> +		}
>> +		/* For nvm, if desired nvm file is not present and it's not the
>> +		 * default nvm file(ends with .bin), try to load the default nvm.
>> +		 */
>> +		else if (config->type == TLV_TYPE_NVM &&
>> +			 qca_get_alt_nvm_path(config->fwname, sizeof(config->fwname))) {
>> +			bt_dev_info(hdev, "QCA Downloading %s", config->fwname);
>> +			ret = request_firmware(&fw, config->fwname, &hdev->dev);
>> +			if (ret) {
>> +				bt_dev_err(hdev, "QCA Failed to request file: %s (%d)",
>> +					   config->fwname, ret);
>> +				return ret;
>> +			}
>>  		} else {
>>  			bt_dev_err(hdev, "QCA Failed to request file: %s (%d)",
>>  				   config->fwname, ret);
>> @@ -730,6 +764,26 @@ static inline void qca_get_nvm_name_generic(struct qca_fw_config *cfg,
>>  			 "qca/%snv%02x.b%02x", stem, rom_ver, bid);
>>  }
>>  
>> +static void qca_get_nvm_name_by_board(char *fwname, size_t max_size,
>> +		const char *firmware_name, struct qca_btsoc_version ver,
>> +		enum qca_btsoc_type soc_type, u16 bid)
>> +{
>> +	const char *variant;
>> +
>> +	/* Set the variant to empty by default */
>> +	variant = "";
>> +	/* hsp gf chip */
>> +	if (soc_type == QCA_WCN6855) {
>> +		if ((le32_to_cpu(ver.soc_id) & QCA_HSP_GF_SOC_MASK) == QCA_HSP_GF_SOC_ID)
>> +			variant = "g";
>> +	}
>> +
>> +	if (bid == 0x0 || bid == 0xffff)
>> +		snprintf(fwname, max_size, "qca/%s%s.bin", firmware_name, variant);
>> +	else
>> +		snprintf(fwname, max_size, "qca/%s%s.b%02x", firmware_name, variant, bid);
> 
> So, we have qca_generate_hsp_nvm_name(), qca_get_nvm_name_generic(), now
> you are adding a third one. Can we please have a single function that
> handles that?
> 
Ack, will use a single function to handle this.
>> +}
>> +
>>  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)
>> @@ -739,6 +793,7 @@ int qca_uart_setup(struct hci_dev *hdev, uint8_t baudrate,
>>  	u8 rom_ver = 0;
>>  	u32 soc_ver;
>>  	u16 boardid = 0;
>> +	const char *suffix;
>>  
>>  	bt_dev_dbg(hdev, "QCA setup on UART");
>>  
>> @@ -816,8 +871,16 @@ int qca_uart_setup(struct hci_dev *hdev, uint8_t baudrate,
>>  	/* Download NVM configuration */
>>  	config.type = TLV_TYPE_NVM;
>>  	if (firmware_name) {
>> -		snprintf(config.fwname, sizeof(config.fwname),
>> -			 "qca/%s", firmware_name);
>> +		/* The firmware name has suffix, use it directly */
>> +		suffix = strrchr(firmware_name, '.');
>> +		if (suffix && suffix != firmware_name &&
>> +			*(suffix + 1) != '\0' && strchr(suffix, '/') == NULL) {
> 
> The have-suffix code should be extracted to a helper function. You have
> two copies of it.
> 
Ack.
>> +			snprintf(config.fwname, sizeof(config.fwname), "qca/%s", firmware_name);
>> +		} else {
>> +			qca_read_fw_board_id(hdev, &boardid);
>> +			qca_get_nvm_name_by_board(config.fwname, sizeof(config.fwname),
>> +				 firmware_name, ver, soc_type, boardid);
>> +		}
>>  	} else {
>>  		switch (soc_type) {
>>  		case QCA_WCN3990:
>> -- 
>> 2.25.1
>>
> 

[PATCH v4 3/4] Bluetooth: qca: Expand firmware-name to load specific rampatch

[Cheng Jiang]

The firmware-name property has been expanded to specify the names of NVM
and rampatch firmware for certain chips, such as the QCA6698 Bluetooth
chip. Although it shares the same IP core as the WCN6855, the QCA6698
has different RF components and RAM sizes, necessitating new firmware
files. This change allows for the configuration of NVM and rampatch in
DT.

Possible configurations:
firmware-name = QCA6698/hpnv21.bin, QCA6698/hpbtfw21.tlv;
firmware-name = QCA6698/hpnv21, QCA6698/hpbtfw21.tlv;

Signed-off-by: Cheng Jiang <quic_chejiang@quicinc.com>
---
 drivers/bluetooth/btqca.c   | 82 +++++++++++++++++++------------------
 drivers/bluetooth/btqca.h   |  5 ++-
 drivers/bluetooth/hci_qca.c | 22 +++++++---
 3 files changed, 63 insertions(+), 46 deletions(-)

diff --git a/drivers/bluetooth/btqca.c b/drivers/bluetooth/btqca.c
index deb2b1753..8dfc2c084 100644
--- a/drivers/bluetooth/btqca.c
+++ b/drivers/bluetooth/btqca.c
@@ -786,7 +786,7 @@ static void qca_get_nvm_name_by_board(char *fwname, size_t max_size,
 
 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)
+		   const char *firmware_name, const char *rampatch_name)
 {
 	struct qca_fw_config config = {};
 	int err;
@@ -816,44 +816,48 @@ int qca_uart_setup(struct hci_dev *hdev, uint8_t baudrate,
 
 	/* Download rampatch file */
 	config.type = TLV_TYPE_PATCH;
-	switch (soc_type) {
-	case QCA_WCN3990:
-	case QCA_WCN3991:
-	case QCA_WCN3998:
-		snprintf(config.fwname, sizeof(config.fwname),
-			 "qca/crbtfw%02x.tlv", rom_ver);
-		break;
-	case QCA_WCN3988:
-		snprintf(config.fwname, sizeof(config.fwname),
-			 "qca/apbtfw%02x.tlv", rom_ver);
-		break;
-	case QCA_QCA2066:
-		snprintf(config.fwname, sizeof(config.fwname),
-			 "qca/hpbtfw%02x.tlv", rom_ver);
-		break;
-	case QCA_QCA6390:
-		snprintf(config.fwname, sizeof(config.fwname),
-			 "qca/htbtfw%02x.tlv", rom_ver);
-		break;
-	case QCA_WCN6750:
-		/* Choose mbn file by default.If mbn file is not found
-		 * then choose tlv file
-		 */
-		config.type = ELF_TYPE_PATCH;
-		snprintf(config.fwname, sizeof(config.fwname),
-			 "qca/msbtfw%02x.mbn", rom_ver);
-		break;
-	case QCA_WCN6855:
-		snprintf(config.fwname, sizeof(config.fwname),
-			 "qca/hpbtfw%02x.tlv", rom_ver);
-		break;
-	case QCA_WCN7850:
-		snprintf(config.fwname, sizeof(config.fwname),
-			 "qca/hmtbtfw%02x.tlv", rom_ver);
-		break;
-	default:
-		snprintf(config.fwname, sizeof(config.fwname),
-			 "qca/rampatch_%08x.bin", soc_ver);
+	if (rampatch_name) {
+		snprintf(config.fwname, sizeof(config.fwname), "qca/%s", rampatch_name);
+	} else {
+		switch (soc_type) {
+		case QCA_WCN3990:
+		case QCA_WCN3991:
+		case QCA_WCN3998:
+			snprintf(config.fwname, sizeof(config.fwname),
+				 "qca/crbtfw%02x.tlv", rom_ver);
+			break;
+		case QCA_WCN3988:
+			snprintf(config.fwname, sizeof(config.fwname),
+				 "qca/apbtfw%02x.tlv", rom_ver);
+			break;
+		case QCA_QCA2066:
+			snprintf(config.fwname, sizeof(config.fwname),
+				 "qca/hpbtfw%02x.tlv", rom_ver);
+			break;
+		case QCA_QCA6390:
+			snprintf(config.fwname, sizeof(config.fwname),
+				 "qca/htbtfw%02x.tlv", rom_ver);
+			break;
+		case QCA_WCN6750:
+			/* Choose mbn file by default.If mbn file is not found
+			 * then choose tlv file
+			 */
+			config.type = ELF_TYPE_PATCH;
+			snprintf(config.fwname, sizeof(config.fwname),
+				 "qca/msbtfw%02x.mbn", rom_ver);
+			break;
+		case QCA_WCN6855:
+			snprintf(config.fwname, sizeof(config.fwname),
+				 "qca/hpbtfw%02x.tlv", rom_ver);
+			break;
+		case QCA_WCN7850:
+			snprintf(config.fwname, sizeof(config.fwname),
+				 "qca/hmtbtfw%02x.tlv", rom_ver);
+			break;
+		default:
+			snprintf(config.fwname, sizeof(config.fwname),
+				 "qca/rampatch_%08x.bin", soc_ver);
+		}
 	}
 
 	err = qca_download_firmware(hdev, &config, soc_type, rom_ver);
diff --git a/drivers/bluetooth/btqca.h b/drivers/bluetooth/btqca.h
index bb5207d7a..9d28c8800 100644
--- a/drivers/bluetooth/btqca.h
+++ b/drivers/bluetooth/btqca.h
@@ -161,7 +161,7 @@ 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, struct qca_btsoc_version ver,
-		   const char *firmware_name);
+		   const char *firmware_name, const char *rampatch_name);
 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);
@@ -176,7 +176,8 @@ 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,
 				 struct qca_btsoc_version ver,
-				 const char *firmware_name)
+				 const char *firmware_name,
+				 const char *rampatch_name)
 {
 	return -EOPNOTSUPP;
 }
diff --git a/drivers/bluetooth/hci_qca.c b/drivers/bluetooth/hci_qca.c
index 37129e6cb..5d75087cc 100644
--- a/drivers/bluetooth/hci_qca.c
+++ b/drivers/bluetooth/hci_qca.c
@@ -228,7 +228,7 @@ struct qca_serdev {
 	u32 init_speed;
 	u32 oper_speed;
 	bool bdaddr_property_broken;
-	const char *firmware_name;
+	const char *firmware_name[2];
 };
 
 static int qca_regulator_enable(struct qca_serdev *qcadev);
@@ -258,7 +258,18 @@ static const char *qca_get_firmware_name(struct hci_uart *hu)
 	if (hu->serdev) {
 		struct qca_serdev *qsd = serdev_device_get_drvdata(hu->serdev);
 
-		return qsd->firmware_name;
+		return qsd->firmware_name[0];
+	} else {
+		return NULL;
+	}
+}
+
+static const char *qca_get_rampatch_name(struct hci_uart *hu)
+{
+	if (hu->serdev) {
+		struct qca_serdev *qsd = serdev_device_get_drvdata(hu->serdev);
+
+		return qsd->firmware_name[1];
 	} else {
 		return NULL;
 	}
@@ -1855,6 +1866,7 @@ static int qca_setup(struct hci_uart *hu)
 	unsigned int retries = 0;
 	enum qca_btsoc_type soc_type = qca_soc_type(hu);
 	const char *firmware_name = qca_get_firmware_name(hu);
+	const char *rampatch_name = qca_get_rampatch_name(hu);
 	int ret;
 	struct qca_btsoc_version ver;
 	struct qca_serdev *qcadev;
@@ -1963,7 +1975,7 @@ static int qca_setup(struct hci_uart *hu)
 
 	/* Setup patch / NVM configurations */
 	ret = qca_uart_setup(hdev, qca_baudrate, soc_type, ver,
-			firmware_name);
+			firmware_name, rampatch_name);
 	if (!ret) {
 		clear_bit(QCA_IBS_DISABLED, &qca->flags);
 		qca_debugfs_init(hdev);
@@ -2309,8 +2321,8 @@ static int qca_serdev_probe(struct serdev_device *serdev)
 	qcadev->serdev_hu.serdev = serdev;
 	data = device_get_match_data(&serdev->dev);
 	serdev_device_set_drvdata(serdev, qcadev);
-	device_property_read_string(&serdev->dev, "firmware-name",
-					 &qcadev->firmware_name);
+	device_property_read_string_array(&serdev->dev, "firmware-name",
+					 qcadev->firmware_name, ARRAY_SIZE(qcadev->firmware_name));
 	device_property_read_u32(&serdev->dev, "max-speed",
 				 &qcadev->oper_speed);
 	if (!qcadev->oper_speed)
-- 
2.25.1

[PATCH v4 4/4] arm64: dts: qcom: sa8775p-ride: Add firmware-name in BT node

[Cheng Jiang]

The sa8775p-ride platform uses the QCA6698 Bluetooth chip. While the
QCA6698 shares the same IP core as the WCN6855, it has different RF
components and RAM sizes, requiring new firmware files. Use the
firmware-name property to specify the NVM and rampatch firmware to load.

Signed-off-by: Cheng Jiang <quic_chejiang@quicinc.com>
---
 arch/arm64/boot/dts/qcom/sa8775p-ride.dtsi | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm64/boot/dts/qcom/sa8775p-ride.dtsi b/arch/arm64/boot/dts/qcom/sa8775p-ride.dtsi
index 3fc62e123..e7fe53d95 100644
--- a/arch/arm64/boot/dts/qcom/sa8775p-ride.dtsi
+++ b/arch/arm64/boot/dts/qcom/sa8775p-ride.dtsi
@@ -857,6 +857,7 @@ &uart17 {
 
 	bluetooth {
 		compatible = "qcom,wcn6855-bt";
+		firmware-name = "QCA6698/hpnv21", "QCA6698/hpbtfw21.tlv";
 
 		vddrfacmn-supply = <&vreg_pmu_rfa_cmn>;
 		vddaon-supply = <&vreg_pmu_aon_0p59>;
-- 
2.25.1

Re: [PATCH v4 0/3] Expand firmware-name property to load specific

[Krzysztof Kozlowski]

On Tue, Dec 10, 2024 at 11:16:32PM +0800, Cheng Jiang wrote:
> Expand the firmware-name property to specify the names of NVM and
> rampatch firmware to load.
> 
> This update will support loading specific firmware (nvm and rampatch)
> for certain chips, like the QCA6698 Bluetooth chip, which shares the
> same IP core as the WCN6855 but has different RF components and RAM
> sizes, requiring new firmware files.
> 
> Different connectivity boards may be attached to the same platform. For
> example, QCA6698-based boards can support either a two-antenna or
> three-antenna solution, both of which work on the sa8775p-ride platform.
> Due to differences in connectivity boards and variations in RF
> performance from different foundries, different NVM configurations are
> used based on the board ID.
> 
> So In firmware-name, if the NVM file has an extension, the NVM file will
> be used. Otherwise, the system will first try the .bNN (board ID) file,
> and if that fails, it will fall back to the .bin file.
> 
> Possible configurations:
> firmware-name = "QCA6698/hpnv21.bin", "QCA6698/hpbtfw21.tlv";
> firmware-name = "QCA6698/hpnv21", "QCA6698/hpbtfw21.tlv";
> firmware-name = "QCA6698/hpnv21.bin";
> 
> ---
> v4:
>   1. Split nvm and rampatch changes to 2 commits
>   2. Code fix according to review comments

Which comments? What exactly did you fix? This cannot be vague.

Best regards,
Krzysztof

Re: [PATCH v4 0/3] Expand firmware-name property to load specific

[Cheng Jiang (IOE)]

Hi Krzysztof,

On 12/11/2024 4:53 PM, Krzysztof Kozlowski wrote:
> On Tue, Dec 10, 2024 at 11:16:32PM +0800, Cheng Jiang wrote:
>> Expand the firmware-name property to specify the names of NVM and
>> rampatch firmware to load.
>>
>> This update will support loading specific firmware (nvm and rampatch)
>> for certain chips, like the QCA6698 Bluetooth chip, which shares the
>> same IP core as the WCN6855 but has different RF components and RAM
>> sizes, requiring new firmware files.
>>
>> Different connectivity boards may be attached to the same platform. For
>> example, QCA6698-based boards can support either a two-antenna or
>> three-antenna solution, both of which work on the sa8775p-ride platform.
>> Due to differences in connectivity boards and variations in RF
>> performance from different foundries, different NVM configurations are
>> used based on the board ID.
>>
>> So In firmware-name, if the NVM file has an extension, the NVM file will
>> be used. Otherwise, the system will first try the .bNN (board ID) file,
>> and if that fails, it will fall back to the .bin file.
>>
>> Possible configurations:
>> firmware-name = "QCA6698/hpnv21.bin", "QCA6698/hpbtfw21.tlv";
>> firmware-name = "QCA6698/hpnv21", "QCA6698/hpbtfw21.tlv";
>> firmware-name = "QCA6698/hpnv21.bin";
>>
>> ---
>> v4:
>>   1. Split nvm and rampatch changes to 2 commits
>>   2. Code fix according to review comments
> 
> Which comments? What exactly did you fix? This cannot be vague.
> 
Ack.
> Best regards,
> Krzysztof
>