[RFC PATCH 2/6] dt-bindings: net: wireless: ath12k: describe WSI property for QCN9274

Raj Kumar Bhagat posted 6 patches 1 month ago
There is a newer version of this series
[RFC PATCH 2/6] dt-bindings: net: wireless: ath12k: describe WSI property for QCN9274
Posted by Raj Kumar Bhagat 1 month ago
QCN9274 device has WSI support. WSI stands for WLAN Serial Interface.
It is used for the exchange of specific control information across
radios based on the doorbell mechanism. This WSI connection is
essential to exchange control information among these devices

Hence, describe WSI interface supported in QCN9274 with the following
properties:

 - qcom,wsi-group-id: It represents the identifier assigned to the WSI
   connection. All the ath12k devices connected to same WSI connection
   have the same wsi-group-id.

 - qcom,wsi-index: It represents the identifier assigned to ath12k
   device in the order of the WSI connection.

 - qcom,wsi-num-devices: Number of devices connected through WSI in
   the same group ID.

Signed-off-by: Raj Kumar Bhagat <quic_rajkbhag@quicinc.com>
---
 .../bindings/net/wireless/qcom,ath12k.yaml    | 61 +++++++++++++++++++
 1 file changed, 61 insertions(+)

diff --git a/Documentation/devicetree/bindings/net/wireless/qcom,ath12k.yaml b/Documentation/devicetree/bindings/net/wireless/qcom,ath12k.yaml
index ecf38af747f7..6c8f97865075 100644
--- a/Documentation/devicetree/bindings/net/wireless/qcom,ath12k.yaml
+++ b/Documentation/devicetree/bindings/net/wireless/qcom,ath12k.yaml
@@ -19,6 +19,7 @@ properties:
   compatible:
     enum:
       - pci17cb,1107  # WCN7850
+      - pci17cb,1109  # QCN9274
 
   reg:
     maxItems: 1
@@ -50,6 +51,41 @@ properties:
   vddpcie1p8-supply:
     description: VDD_PCIE_1P8 supply regulator handle
 
+  wsi:
+    type: object
+    description:
+      The ath12k devices (QCN9274) feature WSI support. WSI stands for
+      WLAN Serial Interface. It is used for the exchange of specific
+      control information across radios based on the doorbell mechanism.
+      This WSI connection is essential to exchange control information
+      among these devices.
+
+    properties:
+      qcom,wsi-group-id:
+        $ref: /schemas/types.yaml#/definitions/uint32
+        description:
+          It represents the identifier assigned to the WSI connection. All
+          the ath12k devices connected to same WSI connection have the
+          same wsi-group-id.
+
+      qcom,wsi-index:
+        $ref: /schemas/types.yaml#/definitions/uint32
+        description:
+          It represents the identifier assigned to ath12k device in the
+          order of the WSI connection.
+
+      qcom,wsi-num-devices:
+        $ref: /schemas/types.yaml#/definitions/uint32
+        description:
+          Number of devices connected through WSI in the same group ID.
+
+    required:
+      - qcom,wsi-group-id
+      - qcom,wsi-index
+      - qcom,wsi-num-devices
+
+    additionalProperties: false
+
 required:
   - compatible
   - reg
@@ -108,3 +144,28 @@ examples:
             };
         };
     };
+
+  - |
+    pcie {
+        #address-cells = <3>;
+        #size-cells = <2>;
+
+        pcie@0 {
+            device_type = "pci";
+            reg = <0x0 0x0 0x0 0x0 0x0>;
+            #address-cells = <3>;
+            #size-cells = <2>;
+            ranges;
+
+            wifi@0 {
+                compatible = "pci17cb,1109";
+                reg = <0x0 0x0 0x0 0x0 0x0>;
+
+                wsi {
+                    qcom,wsi-group-id = <0>;
+                    qcom,wsi-index = <0>;
+                    qcom,wsi-num-devices = <3>;
+                };
+            };
+        };
+    };
-- 
2.34.1
Re: [RFC PATCH 2/6] dt-bindings: net: wireless: ath12k: describe WSI property for QCN9274
Posted by Krzysztof Kozlowski 1 month ago
On 23/10/2024 08:03, Raj Kumar Bhagat wrote:
> QCN9274 device has WSI support. WSI stands for WLAN Serial Interface.
> It is used for the exchange of specific control information across
> radios based on the doorbell mechanism. This WSI connection is
> essential to exchange control information among these devices
> 
> Hence, describe WSI interface supported in QCN9274 with the following
> properties:
> 
>  - qcom,wsi-group-id: It represents the identifier assigned to the WSI
>    connection. All the ath12k devices connected to same WSI connection
>    have the same wsi-group-id.
> 
>  - qcom,wsi-index: It represents the identifier assigned to ath12k
>    device in the order of the WSI connection.
> 
>  - qcom,wsi-num-devices: Number of devices connected through WSI in
>    the same group ID.
> 
> Signed-off-by: Raj Kumar Bhagat <quic_rajkbhag@quicinc.com>
> ---
>  .../bindings/net/wireless/qcom,ath12k.yaml    | 61 +++++++++++++++++++
>  1 file changed, 61 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/net/wireless/qcom,ath12k.yaml b/Documentation/devicetree/bindings/net/wireless/qcom,ath12k.yaml
> index ecf38af747f7..6c8f97865075 100644
> --- a/Documentation/devicetree/bindings/net/wireless/qcom,ath12k.yaml
> +++ b/Documentation/devicetree/bindings/net/wireless/qcom,ath12k.yaml
> @@ -19,6 +19,7 @@ properties:
>    compatible:
>      enum:
>        - pci17cb,1107  # WCN7850
> +      - pci17cb,1109  # QCN9274

Missing supplies. How does the device take power? Everything through
standard PCI pins? Are you sure? Please submit complete binding, so with
all required properties.

Best regards,
Krzysztof
Re: [RFC PATCH 2/6] dt-bindings: net: wireless: ath12k: describe WSI property for QCN9274
Posted by Raj Kumar Bhagat 1 month ago
On 10/23/2024 12:30 PM, Krzysztof Kozlowski wrote:
> On 23/10/2024 08:03, Raj Kumar Bhagat wrote:
>> QCN9274 device has WSI support. WSI stands for WLAN Serial Interface.
>> It is used for the exchange of specific control information across
>> radios based on the doorbell mechanism. This WSI connection is
>> essential to exchange control information among these devices
>>
>> Hence, describe WSI interface supported in QCN9274 with the following
>> properties:
>>
>>  - qcom,wsi-group-id: It represents the identifier assigned to the WSI
>>    connection. All the ath12k devices connected to same WSI connection
>>    have the same wsi-group-id.
>>
>>  - qcom,wsi-index: It represents the identifier assigned to ath12k
>>    device in the order of the WSI connection.
>>
>>  - qcom,wsi-num-devices: Number of devices connected through WSI in
>>    the same group ID.
>>
>> Signed-off-by: Raj Kumar Bhagat <quic_rajkbhag@quicinc.com>
>> ---
>>  .../bindings/net/wireless/qcom,ath12k.yaml    | 61 +++++++++++++++++++
>>  1 file changed, 61 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/net/wireless/qcom,ath12k.yaml b/Documentation/devicetree/bindings/net/wireless/qcom,ath12k.yaml
>> index ecf38af747f7..6c8f97865075 100644
>> --- a/Documentation/devicetree/bindings/net/wireless/qcom,ath12k.yaml
>> +++ b/Documentation/devicetree/bindings/net/wireless/qcom,ath12k.yaml
>> @@ -19,6 +19,7 @@ properties:
>>    compatible:
>>      enum:
>>        - pci17cb,1107  # WCN7850
>> +      - pci17cb,1109  # QCN9274
> 
> Missing supplies. How does the device take power? Everything through
> standard PCI pins? Are you sure? Please submit complete binding, so with
> all required properties.
> 

QCN9274 gets powered from the standard Pcie (3.3 V) supply. No additional
regulators are required.
Re: [RFC PATCH 2/6] dt-bindings: net: wireless: ath12k: describe WSI property for QCN9274
Posted by Krzysztof Kozlowski 1 month ago
On 25/10/2024 12:08, Raj Kumar Bhagat wrote:
>>> Signed-off-by: Raj Kumar Bhagat <quic_rajkbhag@quicinc.com>
>>> ---
>>>  .../bindings/net/wireless/qcom,ath12k.yaml    | 61 +++++++++++++++++++
>>>  1 file changed, 61 insertions(+)
>>>
>>> diff --git a/Documentation/devicetree/bindings/net/wireless/qcom,ath12k.yaml b/Documentation/devicetree/bindings/net/wireless/qcom,ath12k.yaml
>>> index ecf38af747f7..6c8f97865075 100644
>>> --- a/Documentation/devicetree/bindings/net/wireless/qcom,ath12k.yaml
>>> +++ b/Documentation/devicetree/bindings/net/wireless/qcom,ath12k.yaml
>>> @@ -19,6 +19,7 @@ properties:
>>>    compatible:
>>>      enum:
>>>        - pci17cb,1107  # WCN7850
>>> +      - pci17cb,1109  # QCN9274
>>
>> Missing supplies. How does the device take power? Everything through
>> standard PCI pins? Are you sure? Please submit complete binding, so with
>> all required properties.
>>
> 
> QCN9274 gets powered from the standard Pcie (3.3 V) supply. No additional
> regulators are required.

OK, just keep in mind that if you come later with PMU approach to solve
your sequencing problem, we can just NAK it based on above explanation
that PMU approach is not required.

Existing binding lists required supplies on purpose, that's not
coincidence. But of course I don't know if it applies to your case, so
above confirms that it does not apply.

Best regards,
Krzysztof
Re: [RFC PATCH 2/6] dt-bindings: net: wireless: ath12k: describe WSI property for QCN9274
Posted by Krzysztof Kozlowski 1 month ago
On 23/10/2024 08:03, Raj Kumar Bhagat wrote:
> QCN9274 device has WSI support. WSI stands for WLAN Serial Interface.
> It is used for the exchange of specific control information across
> radios based on the doorbell mechanism. This WSI connection is
> essential to exchange control information among these devices
> 
> Hence, describe WSI interface supported in QCN9274 with the following
> properties:
> 
>  - qcom,wsi-group-id: It represents the identifier assigned to the WSI
>    connection. All the ath12k devices connected to same WSI connection
>    have the same wsi-group-id.
> 
>  - qcom,wsi-index: It represents the identifier assigned to ath12k
>    device in the order of the WSI connection.
> 
>  - qcom,wsi-num-devices: Number of devices connected through WSI in
>    the same group ID.

You should have separate binding.

> 
> Signed-off-by: Raj Kumar Bhagat <quic_rajkbhag@quicinc.com>
> ---
>  .../bindings/net/wireless/qcom,ath12k.yaml    | 61 +++++++++++++++++++
>  1 file changed, 61 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/net/wireless/qcom,ath12k.yaml b/Documentation/devicetree/bindings/net/wireless/qcom,ath12k.yaml
> index ecf38af747f7..6c8f97865075 100644
> --- a/Documentation/devicetree/bindings/net/wireless/qcom,ath12k.yaml
> +++ b/Documentation/devicetree/bindings/net/wireless/qcom,ath12k.yaml
> @@ -19,6 +19,7 @@ properties:
>    compatible:
>      enum:
>        - pci17cb,1107  # WCN7850
> +      - pci17cb,1109  # QCN9274
>  
>    reg:
>      maxItems: 1
> @@ -50,6 +51,41 @@ properties:
>    vddpcie1p8-supply:
>      description: VDD_PCIE_1P8 supply regulator handle
>  
> +  wsi:
> +    type: object
> +    description:
> +      The ath12k devices (QCN9274) feature WSI support. WSI stands for
> +      WLAN Serial Interface. It is used for the exchange of specific
> +      control information across radios based on the doorbell mechanism.
> +      This WSI connection is essential to exchange control information
> +      among these devices.
> +
> +    properties:
> +      qcom,wsi-group-id:
> +        $ref: /schemas/types.yaml#/definitions/uint32
> +        description:
> +          It represents the identifier assigned to the WSI connection. All
> +          the ath12k devices connected to same WSI connection have the
> +          same wsi-group-id.

Why it cannot be implied by compatible?

> +
> +      qcom,wsi-index:
> +        $ref: /schemas/types.yaml#/definitions/uint32
> +        description:
> +          It represents the identifier assigned to ath12k device in the
> +          order of the WSI connection.

No, we do not have indices in DTS.

> +
> +      qcom,wsi-num-devices:
> +        $ref: /schemas/types.yaml#/definitions/uint32
> +        description:
> +          Number of devices connected through WSI in the same group ID.

Wait, why? Number of devices is visible from DTS. You are missing some
diagram showing this but it looks like you stuff multiple nodes into one
node.



> +
> +    required:
> +      - qcom,wsi-group-id
> +      - qcom,wsi-index
> +      - qcom,wsi-num-devices
> +
> +    additionalProperties: false
> +
>  required:
>    - compatible
>    - reg
> @@ -108,3 +144,28 @@ examples:
>              };
>          };
>      };
> +
> +  - |
> +    pcie {
> +        #address-cells = <3>;
> +        #size-cells = <2>;
> +
> +        pcie@0 {
> +            device_type = "pci";
> +            reg = <0x0 0x0 0x0 0x0 0x0>;
> +            #address-cells = <3>;
> +            #size-cells = <2>;
> +            ranges;
> +
> +            wifi@0 {
> +                compatible = "pci17cb,1109";
> +                reg = <0x0 0x0 0x0 0x0 0x0>;
> +
> +                wsi {
> +                    qcom,wsi-group-id = <0>;
> +                    qcom,wsi-index = <0>;
> +                    qcom,wsi-num-devices = <3>;

So what are the other 2 devices? Where are they documented?

Best regards,
Krzysztof
Re: [RFC PATCH 2/6] dt-bindings: net: wireless: ath12k: describe WSI property for QCN9274
Posted by Raj Kumar Bhagat 1 month ago
On 10/23/2024 12:08 PM, Krzysztof Kozlowski wrote:
> On 23/10/2024 08:03, Raj Kumar Bhagat wrote:
>> QCN9274 device has WSI support. WSI stands for WLAN Serial Interface.
>> It is used for the exchange of specific control information across
>> radios based on the doorbell mechanism. This WSI connection is
>> essential to exchange control information among these devices
>>
>> Hence, describe WSI interface supported in QCN9274 with the following
>> properties:
>>
>>  - qcom,wsi-group-id: It represents the identifier assigned to the WSI
>>    connection. All the ath12k devices connected to same WSI connection
>>    have the same wsi-group-id.
>>
>>  - qcom,wsi-index: It represents the identifier assigned to ath12k
>>    device in the order of the WSI connection.
>>
>>  - qcom,wsi-num-devices: Number of devices connected through WSI in
>>    the same group ID.
> 
> You should have separate binding.
> 

Based on the discussion in previous patch [1/6]. If required we will have
separate binding.

>>
>> Signed-off-by: Raj Kumar Bhagat <quic_rajkbhag@quicinc.com>
>> ---
>>  .../bindings/net/wireless/qcom,ath12k.yaml    | 61 +++++++++++++++++++
>>  1 file changed, 61 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/net/wireless/qcom,ath12k.yaml b/Documentation/devicetree/bindings/net/wireless/qcom,ath12k.yaml
>> index ecf38af747f7..6c8f97865075 100644
>> --- a/Documentation/devicetree/bindings/net/wireless/qcom,ath12k.yaml
>> +++ b/Documentation/devicetree/bindings/net/wireless/qcom,ath12k.yaml
>> @@ -19,6 +19,7 @@ properties:
>>    compatible:
>>      enum:
>>        - pci17cb,1107  # WCN7850
>> +      - pci17cb,1109  # QCN9274
>>  
>>    reg:
>>      maxItems: 1
>> @@ -50,6 +51,41 @@ properties:
>>    vddpcie1p8-supply:
>>      description: VDD_PCIE_1P8 supply regulator handle
>>  
>> +  wsi:
>> +    type: object
>> +    description:
>> +      The ath12k devices (QCN9274) feature WSI support. WSI stands for
>> +      WLAN Serial Interface. It is used for the exchange of specific
>> +      control information across radios based on the doorbell mechanism.
>> +      This WSI connection is essential to exchange control information
>> +      among these devices.
>> +
>> +    properties:
>> +      qcom,wsi-group-id:
>> +        $ref: /schemas/types.yaml#/definitions/uint32
>> +        description:
>> +          It represents the identifier assigned to the WSI connection. All
>> +          the ath12k devices connected to same WSI connection have the
>> +          same wsi-group-id.
> 
> Why it cannot be implied by compatible?
> 
>> +
>> +      qcom,wsi-index:
>> +        $ref: /schemas/types.yaml#/definitions/uint32
>> +        description:
>> +          It represents the identifier assigned to ath12k device in the
>> +          order of the WSI connection.
> 
> No, we do not have indices in DTS.
> 
>> +
>> +      qcom,wsi-num-devices:
>> +        $ref: /schemas/types.yaml#/definitions/uint32
>> +        description:
>> +          Number of devices connected through WSI in the same group ID.
> 
> Wait, why? Number of devices is visible from DTS. You are missing some
> diagram showing this but it looks like you stuff multiple nodes into one
> node.
> 
> 

To discuss the above comments, let me provide a diagram that is part of the
commit log of DTS patch [6/6].

The WSI connection in RDP433 is represented below:

          +-------+        +-------+        +-------+
          | pcie2 |        | pcie3 |        | pcie1 |
          |       |        |       |        |       |
   +----->|  wsi  |------->|  wsi  |------->|  wsi  |-----+
   |      | idx 0 |        | idx 1 |        | idx 2 |     |
   |      +-------+        +-------+        +-------+     |
   +------------------------------------------------------+

The above three blocks represent the QCN9274 WiFi devices connected to their
respective PCI slots. The dotted line represents the WSI connection that connects
these three devices together. Hence, the WSI interface is part of the QCN9274 device.

To describe this WSI hardware connection in the device tree, we are adding three
properties inside the WSI object:

1. qcom,wsi-group-id:
   In the above diagram, we have one WSI connection connecting all three devices.
   Hence, “qcom,wsi-group-id” for all three devices can be 0.

   This cannot be implied by the compatible property, as explained below:
   Let’s take the case of a platform that can have four QCN9274 WiFi devices. Below
   is one possibility of a WSI connection:

         +-------+       +-------+          +-------+      +-------+
         | pcie2 |       | pcie3 |          | pcie1 |      | pcie0 |
         |       |       |       |          |       |      |       |
   +---->|  wsi  |------>|  wsi  |--+   +-->|  wsi  |----->|  wsi  |----+
   |     | idx 0 |       | idx 1 |  |   |   | idx 0 |      | idx 1 |    |
   |     +-------+       +-------+  |   |   +-------+      +-------+    |
   +--------------------------------+   +-------------------------------+

   In this case, QCN9274 devices connected in PCIe2 and PCIe3 will have the same
   “qcom,wsi-group-id”. This group-id will be different from the “qcom,wsi-group-id”
   of QCN9274 devices connected at PCIe1 and PCIe0.

2. qcom,wsi-index:
   This is a unique identifier of the device within the same group. The value of
   wsi-idx is represented in both the above cases (RDP433 and the 4 WiFi device
   platform) in the diagram itself.

3. qcom,wsi-num-devices:
   Represents the number of devices connected through WSI within the same WSI group to
   which the device belongs.
   
   In the case of RDP433, all devices will have this number as 3.
   For the second example with four WiFi devices but with two WSI connections, the
   value of “qcom,wsi-num-devices” for each device will be 2.

> 
>> +
>> +    required:
>> +      - qcom,wsi-group-id
>> +      - qcom,wsi-index
>> +      - qcom,wsi-num-devices
>> +
>> +    additionalProperties: false
>> +
>>  required:
>>    - compatible
>>    - reg
>> @@ -108,3 +144,28 @@ examples:
>>              };
>>          };
>>      };
>> +
>> +  - |
>> +    pcie {
>> +        #address-cells = <3>;
>> +        #size-cells = <2>;
>> +
>> +        pcie@0 {
>> +            device_type = "pci";
>> +            reg = <0x0 0x0 0x0 0x0 0x0>;
>> +            #address-cells = <3>;
>> +            #size-cells = <2>;
>> +            ranges;
>> +
>> +            wifi@0 {
>> +                compatible = "pci17cb,1109";
>> +                reg = <0x0 0x0 0x0 0x0 0x0>;
>> +
>> +                wsi {
>> +                    qcom,wsi-group-id = <0>;
>> +                    qcom,wsi-index = <0>;
>> +                    qcom,wsi-num-devices = <3>;
> 
> So what are the other 2 devices? Where are they documented?
> 

In the example, we have represented only one WiFi node. Although it indicates
that there are a total of three devices in the same WSI connection with group-id
0, we can add all three WiFi nodes in the next version if required.

Re: [RFC PATCH 2/6] dt-bindings: net: wireless: ath12k: describe WSI property for QCN9274
Posted by Krzysztof Kozlowski 1 month ago
On 23/10/2024 14:22, Raj Kumar Bhagat wrote:
> 
> The above three blocks represent the QCN9274 WiFi devices connected to their
> respective PCI slots. The dotted line represents the WSI connection that connects
> these three devices together. Hence, the WSI interface is part of the QCN9274 device.
> 
> To describe this WSI hardware connection in the device tree, we are adding three
> properties inside the WSI object:
> 
> 1. qcom,wsi-group-id:
>    In the above diagram, we have one WSI connection connecting all three devices.
>    Hence, “qcom,wsi-group-id” for all three devices can be 0.
> 
>    This cannot be implied by the compatible property, as explained below:
>    Let’s take the case of a platform that can have four QCN9274 WiFi devices. Below
>    is one possibility of a WSI connection:
> 
>          +-------+       +-------+          +-------+      +-------+
>          | pcie2 |       | pcie3 |          | pcie1 |      | pcie0 |
>          |       |       |       |          |       |      |       |
>    +---->|  wsi  |------>|  wsi  |--+   +-->|  wsi  |----->|  wsi  |----+
>    |     | idx 0 |       | idx 1 |  |   |   | idx 0 |      | idx 1 |    |
>    |     +-------+       +-------+  |   |   +-------+      +-------+    |
>    +--------------------------------+   +-------------------------------+
> 
>    In this case, QCN9274 devices connected in PCIe2 and PCIe3 will have the same
>    “qcom,wsi-group-id”. This group-id will be different from the “qcom,wsi-group-id”
>    of QCN9274 devices connected at PCIe1 and PCIe0.

Thanks, this explains why group-id cannot be same...

> 
> 2. qcom,wsi-index:
>    This is a unique identifier of the device within the same group. The value of
>    wsi-idx is represented in both the above cases (RDP433 and the 4 WiFi device
>    platform) in the diagram itself.

But still any device-indexing is in general not accepted (and was
mentioned during reviews multiple times).

This looks like circular list, so phandle will be enough. You only need
to mark devices being part of the same chain.

Actually graph with endpoints would be more suitable, assuming above
diagram represents connections.

Please include that diagram in binding description.

> 
> 3. qcom,wsi-num-devices:
>    Represents the number of devices connected through WSI within the same WSI group to
>    which the device belongs.
>    
>    In the case of RDP433, all devices will have this number as 3.
>    For the second example with four WiFi devices but with two WSI connections, the
>    value of “qcom,wsi-num-devices” for each device will be 2.

Not needed, just iterate over the graph children.

Best regards,
Krzysztof

Re: [RFC PATCH 2/6] dt-bindings: net: wireless: ath12k: describe WSI property for QCN9274
Posted by Raj Kumar Bhagat 1 month ago
On 10/23/2024 11:27 PM, Krzysztof Kozlowski wrote:
> On 23/10/2024 14:22, Raj Kumar Bhagat wrote:
>> The above three blocks represent the QCN9274 WiFi devices connected to their
>> respective PCI slots. The dotted line represents the WSI connection that connects
>> these three devices together. Hence, the WSI interface is part of the QCN9274 device.
>>
>> To describe this WSI hardware connection in the device tree, we are adding three
>> properties inside the WSI object:
>>
>> 1. qcom,wsi-group-id:
>>    In the above diagram, we have one WSI connection connecting all three devices.
>>    Hence, “qcom,wsi-group-id” for all three devices can be 0.
>>
>>    This cannot be implied by the compatible property, as explained below:
>>    Let’s take the case of a platform that can have four QCN9274 WiFi devices. Below
>>    is one possibility of a WSI connection:
>>
>>          +-------+       +-------+          +-------+      +-------+
>>          | pcie2 |       | pcie3 |          | pcie1 |      | pcie0 |
>>          |       |       |       |          |       |      |       |
>>    +---->|  wsi  |------>|  wsi  |--+   +-->|  wsi  |----->|  wsi  |----+
>>    |     | idx 0 |       | idx 1 |  |   |   | idx 0 |      | idx 1 |    |
>>    |     +-------+       +-------+  |   |   +-------+      +-------+    |
>>    +--------------------------------+   +-------------------------------+
>>
>>    In this case, QCN9274 devices connected in PCIe2 and PCIe3 will have the same
>>    “qcom,wsi-group-id”. This group-id will be different from the “qcom,wsi-group-id”
>>    of QCN9274 devices connected at PCIe1 and PCIe0.
> Thanks, this explains why group-id cannot be same...
> 
>> 2. qcom,wsi-index:
>>    This is a unique identifier of the device within the same group. The value of
>>    wsi-idx is represented in both the above cases (RDP433 and the 4 WiFi device
>>    platform) in the diagram itself.
> But still any device-indexing is in general not accepted (and was
> mentioned during reviews multiple times).
> 
> This looks like circular list, so phandle will be enough. You only need
> to mark devices being part of the same chain.
> 
> Actually graph with endpoints would be more suitable, assuming above
> diagram represents connections.
> 

Thanks for suggesting "graph will endpoints" approach for representing WSI
connections.

I will check on this and come back.

> Please include that diagram in binding description.
> 

Sure will include the above diagram in next version.

>> 3. qcom,wsi-num-devices:
>>    Represents the number of devices connected through WSI within the same WSI group to
>>    which the device belongs.
>>    
>>    In the case of RDP433, all devices will have this number as 3.
>>    For the second example with four WiFi devices but with two WSI connections, the
>>    value of “qcom,wsi-num-devices” for each device will be 2.
> Not needed, just iterate over the graph children.

Thanks, based on "graph with endpoints" implementation will have this as well.
Re: [RFC PATCH 2/6] dt-bindings: net: wireless: ath12k: describe WSI property for QCN9274
Posted by Krzysztof Kozlowski 1 month ago
On 23/10/2024 08:38, Krzysztof Kozlowski wrote:
> On 23/10/2024 08:03, Raj Kumar Bhagat wrote:
>> QCN9274 device has WSI support. WSI stands for WLAN Serial Interface.
>> It is used for the exchange of specific control information across
>> radios based on the doorbell mechanism. This WSI connection is
>> essential to exchange control information among these devices
>>
>> Hence, describe WSI interface supported in QCN9274 with the following
>> properties:
>>
>>  - qcom,wsi-group-id: It represents the identifier assigned to the WSI
>>    connection. All the ath12k devices connected to same WSI connection
>>    have the same wsi-group-id.
>>
>>  - qcom,wsi-index: It represents the identifier assigned to ath12k
>>    device in the order of the WSI connection.
>>
>>  - qcom,wsi-num-devices: Number of devices connected through WSI in
>>    the same group ID.
> 
> You should have separate binding.
> 
>>
>> Signed-off-by: Raj Kumar Bhagat <quic_rajkbhag@quicinc.com>
>> ---
>>  .../bindings/net/wireless/qcom,ath12k.yaml    | 61 +++++++++++++++++++
>>  1 file changed, 61 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/net/wireless/qcom,ath12k.yaml b/Documentation/devicetree/bindings/net/wireless/qcom,ath12k.yaml
>> index ecf38af747f7..6c8f97865075 100644
>> --- a/Documentation/devicetree/bindings/net/wireless/qcom,ath12k.yaml
>> +++ b/Documentation/devicetree/bindings/net/wireless/qcom,ath12k.yaml
>> @@ -19,6 +19,7 @@ properties:
>>    compatible:
>>      enum:
>>        - pci17cb,1107  # WCN7850
>> +      - pci17cb,1109  # QCN9274
>>  
>>    reg:
>>      maxItems: 1
>> @@ -50,6 +51,41 @@ properties:
>>    vddpcie1p8-supply:
>>      description: VDD_PCIE_1P8 supply regulator handle
>>  
>> +  wsi:
>> +    type: object

Plus this entire wsi has to be dropped - it's useless. No bus here.

Best regards,
Krzysztof