diff mbox series

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

Message ID 20241210151636.2474809-2-quic_chejiang@quicinc.com (mailing list archive)
State Superseded
Headers show
Series Expand firmware-name property to load specific | expand

Commit Message

Cheng Jiang (IOE) Dec. 10, 2024, 3:16 p.m. UTC
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(-)

Comments

Krzysztof Kozlowski Dec. 11, 2024, 8:53 a.m. UTC | #1
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
Cheng Jiang (IOE) Dec. 11, 2024, 9:39 a.m. UTC | #2
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
>
Krzysztof Kozlowski Dec. 11, 2024, 9:48 a.m. UTC | #3
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
Cheng Jiang (IOE) Dec. 11, 2024, 10:16 a.m. UTC | #4
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
Dmitry Baryshkov Dec. 11, 2024, 7:28 p.m. UTC | #5
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
>
Cheng Jiang (IOE) Dec. 12, 2024, 5:56 a.m. UTC | #6
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
>>
>
Krzysztof Kozlowski Dec. 12, 2024, 7:16 a.m. UTC | #7
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
Cheng Jiang (IOE) Dec. 12, 2024, 10:16 a.m. UTC | #8
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
diff mbox series

Patch

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