[RFC PATCH 0/6] wifi: ath12k: Add WSI node for QCN9274 in RDP433 for MLO

[RFC PATCH 0/6] wifi: ath12k: Add WSI node for QCN9274 in RDP433 for MLO

[Raj Kumar Bhagat]

The RDP433 is a Qualcomm Reference Design Platform based on the
IPQ9574. It features three QCN9274 WiFi devices connected to PCIe1,
PCIe2, and PCIe3. These devices are also interconnected via a WLAN
Serial Interface (WSI) connection. This WSI connection is essential
to exchange control information among these devices.

This patch series describes the WSI interface found in QCN9274 and
uses this device-tree node in the Ath12k driver to provide details
of adjacent devices in Multi Link Operation (MLO) among multiple
QCN9274 devices.

NOTES:
1. As ath12k MLO patches are not ready yet, this patchset does not apply
   to the ath.git ath-next branch and that's why the patchset is marked
   as RFC. These are the work-in-progress patches we have at the moment.
   The full set of MLO patches is available at:
   https://git.kernel.org/pub/scm/linux/kernel/git/ath/ath.git/log/?h=ath12k-mlo-qcn9274

2. The dependency marked below applies only to the DTS patch. The
   dt-bindings patches do not have this dependency.

Depends-On: [PATCH V7 0/4] Add PCIe support for IPQ9574
Link: https://lore.kernel.org/linux-pci/20240801054803.3015572-1-quic_srichara@quicinc.com/

Aditya Kumar Singh (1):
  wifi: ath12k: assign unique hardware link IDs during QMI host cap

Harshitha Prem (1):
  wifi: ath12k: parse multiple device information from device tree

Karthikeyan Periyasamy (1):
  wifi: ath12k: Send partner device details in QMI MLO capability

Raj Kumar Bhagat (3):
  dt-bindings: net: wireless: update required properties for ath12k PCI
    module
  dt-bindings: net: wireless: ath12k: describe WSI property for QCN9274
  arm64: dts: qcom: ipq9574: Add WiFi nodes for RDP433

 .../bindings/net/wireless/qcom,ath12k.yaml    |  90 +++++++++++++--
 arch/arm64/boot/dts/qcom/ipq9574-rdp433.dts   |  62 +++++++++-
 drivers/net/wireless/ath/ath12k/core.c        | 106 ++++++++++++++++--
 drivers/net/wireless/ath/ath12k/core.h        |   2 +
 drivers/net/wireless/ath/ath12k/qmi.c         |  98 +++++++++++++---
 5 files changed, 322 insertions(+), 36 deletions(-)


base-commit: 7603a9349b2fc64152a734f253cf8d8e5befb6db
prerequisite-patch-id: d1334693a2e8da65ae7b458ee4adb459850ad2e7
prerequisite-patch-id: 87f73b342f67c2636390a7da1294cee90f1fff48
prerequisite-patch-id: 46d8302766527d16cdd90c59ded6cbae0ec4ad70
prerequisite-patch-id: b17db6783b1c35f3e8812f621730fe0a1a57a14e
-- 
2.34.1

[RFC PATCH 1/6] dt-bindings: net: wireless: update required properties for ath12k PCI module

[Raj Kumar Bhagat]

The current device-tree bindings for the Ath12K module list many
WCN7850-specific properties as required. However, these properties are
not applicable to other Ath12K devices.

Hence, remove WCN7850-specific properties from the required section,
retaining only generic properties valid across all Ath12K devices.
WCN7850-specific properties will remain required based on the device's
compatible enum.

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

diff --git a/Documentation/devicetree/bindings/net/wireless/qcom,ath12k.yaml b/Documentation/devicetree/bindings/net/wireless/qcom,ath12k.yaml
index 1b5884015b15..ecf38af747f7 100644
--- a/Documentation/devicetree/bindings/net/wireless/qcom,ath12k.yaml
+++ b/Documentation/devicetree/bindings/net/wireless/qcom,ath12k.yaml
@@ -1,5 +1,6 @@
 # SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
 # Copyright (c) 2024 Linaro Limited
+# Copyright (c) 2024 Qualcomm Innovation Center, Inc. All rights reserved.
 %YAML 1.2
 ---
 $id: http://devicetree.org/schemas/net/wireless/qcom,ath12k.yaml#
@@ -52,18 +53,28 @@ properties:
 required:
   - compatible
   - reg
-  - vddaon-supply
-  - vddwlcx-supply
-  - vddwlmx-supply
-  - vddrfacmn-supply
-  - vddrfa0p8-supply
-  - vddrfa1p2-supply
-  - vddrfa1p8-supply
-  - vddpcie0p9-supply
-  - vddpcie1p8-supply
 
 additionalProperties: false
 
+allOf:
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - pci17cb,1107
+    then:
+      required:
+        - vddaon-supply
+        - vddwlcx-supply
+        - vddwlmx-supply
+        - vddrfacmn-supply
+        - vddrfa0p8-supply
+        - vddrfa1p2-supply
+        - vddrfa1p8-supply
+        - vddpcie0p9-supply
+        - vddpcie1p8-supply
+
 examples:
   - |
     #include <dt-bindings/clock/qcom,rpmh.h>
-- 
2.34.1

Re: [RFC PATCH 1/6] dt-bindings: net: wireless: update required properties for ath12k PCI module

[Krzysztof Kozlowski]

On 23/10/2024 08:03, Raj Kumar Bhagat wrote:
> The current device-tree bindings for the Ath12K module list many
> WCN7850-specific properties as required. However, these properties are
> not applicable to other Ath12K devices.
> 
> Hence, remove WCN7850-specific properties from the required section,
> retaining only generic properties valid across all Ath12K devices.
> WCN7850-specific properties will remain required based on the device's
> compatible enum.

Just not true. These apply to all devices described in this binding.

NAK.

Don't send patches for your downstream stuff.


Best regards,
Krzysztof

Re: [RFC PATCH 1/6] dt-bindings: net: wireless: update required properties for ath12k PCI module

[Raj Kumar Bhagat]

On 10/23/2024 12:05 PM, Krzysztof Kozlowski wrote:
> On 23/10/2024 08:03, Raj Kumar Bhagat wrote:
>> The current device-tree bindings for the Ath12K module list many
>> WCN7850-specific properties as required. However, these properties are
>> not applicable to other Ath12K devices.
>>
>> Hence, remove WCN7850-specific properties from the required section,
>> retaining only generic properties valid across all Ath12K devices.
>> WCN7850-specific properties will remain required based on the device's
>> compatible enum.
> Just not true. These apply to all devices described in this binding.
> 
> NAK.
> 
> Don't send patches for your downstream stuff.

This is not for downstream. This series is the per-requisite for ath12k
MLO support in upstream.

In the subsequent patch [2/6] we are adding new device (QCN9274) in this
binding that do not require the WCN7850 specific properties.

This is a refactoring patch for the next patch [2/6].

Re: [RFC PATCH 1/6] dt-bindings: net: wireless: update required properties for ath12k PCI module

[Krzysztof Kozlowski]

On 23/10/2024 08:45, Raj Kumar Bhagat wrote:
> On 10/23/2024 12:05 PM, Krzysztof Kozlowski wrote:
>> On 23/10/2024 08:03, Raj Kumar Bhagat wrote:
>>> The current device-tree bindings for the Ath12K module list many
>>> WCN7850-specific properties as required. However, these properties are
>>> not applicable to other Ath12K devices.
>>>
>>> Hence, remove WCN7850-specific properties from the required section,
>>> retaining only generic properties valid across all Ath12K devices.
>>> WCN7850-specific properties will remain required based on the device's
>>> compatible enum.
>> Just not true. These apply to all devices described in this binding.
>>
>> NAK.
>>
>> Don't send patches for your downstream stuff.
> 
> This is not for downstream. This series is the per-requisite for ath12k
> MLO support in upstream.
> 
> In the subsequent patch [2/6] we are adding new device (QCN9274) in this
> binding that do not require the WCN7850 specific properties.
> 
> This is a refactoring patch for the next patch [2/6].

It's just wrong. Not true. At this point of patch there are no other
devices. Don't refactor uselessly introducing incorrect hardware
description.

Best regards,
Krzysztof

Re: [RFC PATCH 1/6] dt-bindings: net: wireless: update required properties for ath12k PCI module

[Raj Kumar Bhagat]

On 10/23/2024 12:17 PM, Krzysztof Kozlowski wrote:
> On 23/10/2024 08:45, Raj Kumar Bhagat wrote:
>> On 10/23/2024 12:05 PM, Krzysztof Kozlowski wrote:
>>> On 23/10/2024 08:03, Raj Kumar Bhagat wrote:
>>>> The current device-tree bindings for the Ath12K module list many
>>>> WCN7850-specific properties as required. However, these properties are
>>>> not applicable to other Ath12K devices.
>>>>
>>>> Hence, remove WCN7850-specific properties from the required section,
>>>> retaining only generic properties valid across all Ath12K devices.
>>>> WCN7850-specific properties will remain required based on the device's
>>>> compatible enum.
>>> Just not true. These apply to all devices described in this binding.
>>>
>>> NAK.
>>>
>>> Don't send patches for your downstream stuff.
>> This is not for downstream. This series is the per-requisite for ath12k
>> MLO support in upstream.
>>
>> In the subsequent patch [2/6] we are adding new device (QCN9274) in this
>> binding that do not require the WCN7850 specific properties.
>>
>> This is a refactoring patch for the next patch [2/6].
> It's just wrong. Not true. At this point of patch there are no other
> devices. Don't refactor uselessly introducing incorrect hardware

Ok then, If we squash this patch with the next patch [2/6], that actually adding
the new device, then this patch changes are valid right?

Re: [RFC PATCH 1/6] dt-bindings: net: wireless: update required properties for ath12k PCI module

[Krzysztof Kozlowski]

On 23/10/2024 08:53, Raj Kumar Bhagat wrote:
> On 10/23/2024 12:17 PM, Krzysztof Kozlowski wrote:
>> On 23/10/2024 08:45, Raj Kumar Bhagat wrote:
>>> On 10/23/2024 12:05 PM, Krzysztof Kozlowski wrote:
>>>> On 23/10/2024 08:03, Raj Kumar Bhagat wrote:
>>>>> The current device-tree bindings for the Ath12K module list many
>>>>> WCN7850-specific properties as required. However, these properties are
>>>>> not applicable to other Ath12K devices.
>>>>>
>>>>> Hence, remove WCN7850-specific properties from the required section,
>>>>> retaining only generic properties valid across all Ath12K devices.
>>>>> WCN7850-specific properties will remain required based on the device's
>>>>> compatible enum.
>>>> Just not true. These apply to all devices described in this binding.
>>>>
>>>> NAK.
>>>>
>>>> Don't send patches for your downstream stuff.
>>> This is not for downstream. This series is the per-requisite for ath12k
>>> MLO support in upstream.
>>>
>>> In the subsequent patch [2/6] we are adding new device (QCN9274) in this
>>> binding that do not require the WCN7850 specific properties.
>>>
>>> This is a refactoring patch for the next patch [2/6].
>> It's just wrong. Not true. At this point of patch there are no other
>> devices. Don't refactor uselessly introducing incorrect hardware
> 
> Ok then, If we squash this patch with the next patch [2/6], that actually adding
> the new device, then this patch changes are valid right?

Yes, except I asked to have separate binding for devices with different
interface (WSI). You add unrelated devices to same binding, growing it
into something tricky to manage. Your second patch misses if:then
disallwing all this WSI stuff for existing device... and then you should
notice there is absolutely *nothing* in common.

Best regards,
Krzysztof

Re: [RFC PATCH 1/6] dt-bindings: net: wireless: update required properties for ath12k PCI module

[Raj Kumar Bhagat]

On 10/23/2024 12:29 PM, Krzysztof Kozlowski wrote:
> On 23/10/2024 08:53, Raj Kumar Bhagat wrote:
>> On 10/23/2024 12:17 PM, Krzysztof Kozlowski wrote:
>>> On 23/10/2024 08:45, Raj Kumar Bhagat wrote:
>>>> On 10/23/2024 12:05 PM, Krzysztof Kozlowski wrote:
>>>>> On 23/10/2024 08:03, Raj Kumar Bhagat wrote:
>>>>>> The current device-tree bindings for the Ath12K module list many
>>>>>> WCN7850-specific properties as required. However, these properties are
>>>>>> not applicable to other Ath12K devices.
>>>>>>
>>>>>> Hence, remove WCN7850-specific properties from the required section,
>>>>>> retaining only generic properties valid across all Ath12K devices.
>>>>>> WCN7850-specific properties will remain required based on the device's
>>>>>> compatible enum.
>>>>> Just not true. These apply to all devices described in this binding.
>>>>>
>>>>> NAK.
>>>>>
>>>>> Don't send patches for your downstream stuff.
>>>> This is not for downstream. This series is the per-requisite for ath12k
>>>> MLO support in upstream.
>>>>
>>>> In the subsequent patch [2/6] we are adding new device (QCN9274) in this
>>>> binding that do not require the WCN7850 specific properties.
>>>>
>>>> This is a refactoring patch for the next patch [2/6].
>>> It's just wrong. Not true. At this point of patch there are no other
>>> devices. Don't refactor uselessly introducing incorrect hardware
>> Ok then, If we squash this patch with the next patch [2/6], that actually adding
>> the new device, then this patch changes are valid right?
> Yes, except I asked to have separate binding for devices with different
> interface (WSI). You add unrelated devices to same binding, growing it
> into something tricky to manage. Your second patch misses if:then
> disallwing all this WSI stuff for existing device... and then you should
> notice there is absolutely *nothing* in common.
> 

I understand your point about having separate bindings if there are no common
properties. However, the title and description of this binding indicate that it
is intended for Qualcomm ath12k wireless devices with a PCI bus. Given this, the
QCN9274 seems to fit within the same binding.

Additionally, there will likely be more properties added in the future that could
be common. For example, the “qcom,ath12k-calibration-variant” property (which the
ath12k host currently doesn’t support reading and using, hence we are not adding it
now) could be a common property.

If you still recommend creating a separate binding for the QCN9274, we are open to
working on that.

Thank you for your guidance.

Re: [RFC PATCH 1/6] dt-bindings: net: wireless: update required properties for ath12k PCI module

[Krzysztof Kozlowski]

On 23/10/2024 12:28, Raj Kumar Bhagat wrote:
> On 10/23/2024 12:29 PM, Krzysztof Kozlowski wrote:
>> On 23/10/2024 08:53, Raj Kumar Bhagat wrote:
>>> On 10/23/2024 12:17 PM, Krzysztof Kozlowski wrote:
>>>> On 23/10/2024 08:45, Raj Kumar Bhagat wrote:
>>>>> On 10/23/2024 12:05 PM, Krzysztof Kozlowski wrote:
>>>>>> On 23/10/2024 08:03, Raj Kumar Bhagat wrote:
>>>>>>> The current device-tree bindings for the Ath12K module list many
>>>>>>> WCN7850-specific properties as required. However, these properties are
>>>>>>> not applicable to other Ath12K devices.
>>>>>>>
>>>>>>> Hence, remove WCN7850-specific properties from the required section,
>>>>>>> retaining only generic properties valid across all Ath12K devices.
>>>>>>> WCN7850-specific properties will remain required based on the device's
>>>>>>> compatible enum.
>>>>>> Just not true. These apply to all devices described in this binding.
>>>>>>
>>>>>> NAK.
>>>>>>
>>>>>> Don't send patches for your downstream stuff.
>>>>> This is not for downstream. This series is the per-requisite for ath12k
>>>>> MLO support in upstream.
>>>>>
>>>>> In the subsequent patch [2/6] we are adding new device (QCN9274) in this
>>>>> binding that do not require the WCN7850 specific properties.
>>>>>
>>>>> This is a refactoring patch for the next patch [2/6].
>>>> It's just wrong. Not true. At this point of patch there are no other
>>>> devices. Don't refactor uselessly introducing incorrect hardware
>>> Ok then, If we squash this patch with the next patch [2/6], that actually adding
>>> the new device, then this patch changes are valid right?
>> Yes, except I asked to have separate binding for devices with different
>> interface (WSI). You add unrelated devices to same binding, growing it
>> into something tricky to manage. Your second patch misses if:then
>> disallwing all this WSI stuff for existing device... and then you should
>> notice there is absolutely *nothing* in common.
>>
> 
> I understand your point about having separate bindings if there are no common
> properties. However, the title and description of this binding indicate that it
> is intended for Qualcomm ath12k wireless devices with a PCI bus. Given this, the
> QCN9274 seems to fit within the same binding.

Feel free to fix it. Or add common schema used by multiple bindings.

> 
> Additionally, there will likely be more properties added in the future that could
> be common. For example, the “qcom,ath12k-calibration-variant” property (which the

You are supposed to add them now, not later. See writing bindings. They
are supposed to be complete.

> ath12k host currently doesn’t support reading and using, hence we are not adding it
> now) could be a common property.

What is "host"? Either the device has this property or not. Whether host
supports something does not really matter, right? You have hardware
property or you have it *not*.

> 
> If you still recommend creating a separate binding for the QCN9274, we are open to
> working on that.


Best regards,
Krzysztof

Re: [RFC PATCH 1/6] dt-bindings: net: wireless: update required properties for ath12k PCI module

[Raj Kumar Bhagat]

On 10/23/2024 5:38 PM, Krzysztof Kozlowski wrote:
> On 23/10/2024 12:28, Raj Kumar Bhagat wrote:
>> On 10/23/2024 12:29 PM, Krzysztof Kozlowski wrote:
>>> On 23/10/2024 08:53, Raj Kumar Bhagat wrote:
>>>> On 10/23/2024 12:17 PM, Krzysztof Kozlowski wrote:
>>>>> On 23/10/2024 08:45, Raj Kumar Bhagat wrote:
>>>>>> On 10/23/2024 12:05 PM, Krzysztof Kozlowski wrote:
>>>>>>> On 23/10/2024 08:03, Raj Kumar Bhagat wrote:
>>>>>>>> The current device-tree bindings for the Ath12K module list many
>>>>>>>> WCN7850-specific properties as required. However, these properties are
>>>>>>>> not applicable to other Ath12K devices.
>>>>>>>>
>>>>>>>> Hence, remove WCN7850-specific properties from the required section,
>>>>>>>> retaining only generic properties valid across all Ath12K devices.
>>>>>>>> WCN7850-specific properties will remain required based on the device's
>>>>>>>> compatible enum.
>>>>>>> Just not true. These apply to all devices described in this binding.
>>>>>>>
>>>>>>> NAK.
>>>>>>>
>>>>>>> Don't send patches for your downstream stuff.
>>>>>> This is not for downstream. This series is the per-requisite for ath12k
>>>>>> MLO support in upstream.
>>>>>>
>>>>>> In the subsequent patch [2/6] we are adding new device (QCN9274) in this
>>>>>> binding that do not require the WCN7850 specific properties.
>>>>>>
>>>>>> This is a refactoring patch for the next patch [2/6].
>>>>> It's just wrong. Not true. At this point of patch there are no other
>>>>> devices. Don't refactor uselessly introducing incorrect hardware
>>>> Ok then, If we squash this patch with the next patch [2/6], that actually adding
>>>> the new device, then this patch changes are valid right?
>>> Yes, except I asked to have separate binding for devices with different
>>> interface (WSI). You add unrelated devices to same binding, growing it
>>> into something tricky to manage. Your second patch misses if:then
>>> disallwing all this WSI stuff for existing device... and then you should
>>> notice there is absolutely *nothing* in common.
>>>
>> I understand your point about having separate bindings if there are no common
>> properties. However, the title and description of this binding indicate that it
>> is intended for Qualcomm ath12k wireless devices with a PCI bus. Given this, the
>> QCN9274 seems to fit within the same binding.
> Feel free to fix it. Or add common schema used by multiple bindings.
> 
>> Additionally, there will likely be more properties added in the future that could
>> be common. For example, the “qcom,ath12k-calibration-variant” property (which the
> You are supposed to add them now, not later. See writing bindings. They
> are supposed to be complete.
> 

Sure will add "qcom,ath12k-calibration-variant" in next version.

>> ath12k host currently doesn’t support reading and using, hence we are not adding it
>> now) could be a common property.
> What is "host"? Either the device has this property or not. Whether host
> supports something does not really matter, right? You have hardware
> property or you have it *not*.

Ah, my bad. I meant to say “ath12k driver”.

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

[Raj Kumar Bhagat]

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

[Krzysztof Kozlowski]

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

[Krzysztof Kozlowski]

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

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

[Raj Kumar Bhagat]

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

[Krzysztof Kozlowski]

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

[Raj Kumar Bhagat]

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

[Krzysztof Kozlowski]

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

[Raj Kumar Bhagat]

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

[Krzysztof Kozlowski]

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

[RFC PATCH 3/6] wifi: ath12k: parse multiple device information from device tree

[Raj Kumar Bhagat]

From: Harshitha Prem <quic_hprem@quicinc.com>

Currently, single device is part of device group abstraction but for multi
link operation, multiple devices have to be combined together. Information
of how many devices involved in grouping can be parsed from device tree and
it would have the valid group id information as well.

Add changes to parse devices involved and group id from device tree file
to form device group

Tested-on: QCN9274 hw2.0 PCI WLAN.WBE.1.0.1-00029-QCAHKSWPL_SILICONZ-1

Signed-off-by: Harshitha Prem <quic_hprem@quicinc.com>
Co-developed-by: Aditya Kumar Singh <quic_adisi@quicinc.com>
Signed-off-by: Aditya Kumar Singh <quic_adisi@quicinc.com>
Signed-off-by: Kalle Valo <quic_kvalo@quicinc.com>
---
 drivers/net/wireless/ath/ath12k/core.c | 106 ++++++++++++++++++++++---
 drivers/net/wireless/ath/ath12k/core.h |   2 +
 2 files changed, 98 insertions(+), 10 deletions(-)

diff --git a/drivers/net/wireless/ath/ath12k/core.c b/drivers/net/wireless/ath/ath12k/core.c
index 3f0f44cbdb4c..b7b2d552205c 100644
--- a/drivers/net/wireless/ath/ath12k/core.c
+++ b/drivers/net/wireless/ath/ath12k/core.c
@@ -1403,6 +1403,7 @@ ath12k_core_hw_group_alloc(u8 id, u8 max_devices)
 	ag->num_devices = max_devices;
 	list_add(&ag->list, &ath12k_hw_group_list);
 	mutex_init(&ag->mutex_lock);
+	ag->mlo_capable = false;
 
 	return ag;
 }
@@ -1417,23 +1418,94 @@ static void ath12k_core_hw_group_free(struct ath12k_hw_group *ag)
 	mutex_unlock(&ath12k_ag_list_lock);
 }
 
+/* This function needs to be used only when dt has multi chip grouping information */
+static struct ath12k_hw_group *ath12k_core_hw_group_find_by_id(u8 group_id)
+{
+	struct ath12k_hw_group *ag;
+
+	/* group ids will be unique only for multi chip group */
+	list_for_each_entry(ag, &ath12k_hw_group_list, list) {
+		if (group_id == ag->id && ag->num_devices > 1)
+			return ag;
+	}
+
+	return NULL;
+}
+
 static struct ath12k_hw_group *ath12k_core_assign_hw_group(struct ath12k_base *ab)
 {
 	struct ath12k_hw_group *ag;
-	u32 group_id = ATH12K_INVALID_GROUP_ID;
+	u32 group_id = ATH12K_INVALID_GROUP_ID, num_devices;
+	struct device *dev = ab->dev;
+	struct device_node *mlo;
 
 	lockdep_assert_held(&ath12k_ag_list_lock);
 
-	/* The grouping of multiple devices will be done based on device tree file.
-	 * TODO: device tree file parsing to know about the devices involved in group.
-	 *
-	 * The platforms that do not have any valid group information would have each
-	 * device to be part of its own invalid group.
+	/* The grouping of multiple devices will be done based on device
+	 * tree file.
 	 *
-	 * Currently, we are not parsing any device tree information and hence, grouping
-	 * of multiple devices is not involved. Thus, single device is added to device
-	 * group.
+	 * The platforms that do not have any valid group information would
+	 * have each device to be part of its own invalid group.
 	 */
+	mlo = of_get_child_by_name(dev->of_node, "wsi");
+	if (!mlo) {
+		goto invalid_group;
+	} else {
+		if (of_property_read_u32(mlo, "qcom,wsi-group-id", &group_id)) {
+			ath12k_err(ab, "wsi-group-id not found\n");
+			goto invalid_group;
+		}
+	}
+
+	if (of_property_read_u32(mlo, "qcom,wsi-num-devices", &num_devices)) {
+		ath12k_err(ab, "wsi-num-devices not found\n");
+		group_id = ATH12K_INVALID_GROUP_ID;
+		goto invalid_group;
+	}
+
+	if (num_devices > ATH12K_MAX_SOCS) {
+		ath12k_warn(ab, "num_devices advertised %d is more than limit %d\n",
+			    num_devices, ATH12K_MAX_SOCS);
+		group_id = ATH12K_INVALID_GROUP_ID;
+		goto invalid_group;
+	}
+
+	if (of_property_read_u32(mlo, "qcom,wsi-index", &ab->wsi_index)) {
+		ath12k_err(ab, "qcom,wsi-index not found\n");
+		group_id = ATH12K_INVALID_GROUP_ID;
+		goto invalid_group;
+	}
+
+	ath12k_dbg(ab, ATH12K_DBG_BOOT,
+		   "WSI info: group-id: %d, num-devices: %d, index: %d",
+		   group_id, num_devices, ab->wsi_index);
+
+	/* Currently only one group of multiple devices are supported,
+	 * since we use group id ATH12K_INVALID_GROUP_ID for single
+	 * device group which didn't have dt entry, there could be many
+	 * groups with same group id, i.e ATH12K_INVALID_GROUP_ID. So
+	 * default group id of ATH12K_INVALID_GROUP_ID combined with
+	 * num devices in ath12k_hw_group determines if the group is
+	 * multi device or single device group
+	 */
+	ag = ath12k_core_hw_group_find_by_id(group_id);
+	if (!ag) {
+		ag = ath12k_core_hw_group_alloc(group_id, num_devices);
+		if (!ag) {
+			ath12k_warn(ab, "unable to create new hw group\n");
+			return NULL;
+		}
+		goto exit;
+	} else if (test_bit(ATH12K_GROUP_FLAG_UNREGISTER, &ag->flags)) {
+		ath12k_dbg(ab, ATH12K_DBG_BOOT, "group id %d in unregister state\n",
+			   ag->id);
+		group_id = ATH12K_INVALID_GROUP_ID;
+		goto invalid_group;
+	} else {
+		goto exit;
+	}
+
+invalid_group:
 	ag = ath12k_core_hw_group_alloc(group_id, 1);
 	if (!ag) {
 		ath12k_warn(ab, "unable to create new hw group\n");
@@ -1441,10 +1513,16 @@ static struct ath12k_hw_group *ath12k_core_assign_hw_group(struct ath12k_base *a
 	}
 	ath12k_dbg(ab, ATH12K_DBG_BOOT, "Single device is added to hardware group\n");
 
+exit:
+	if (ag->num_probed >= ag->num_devices) {
+		ath12k_warn(ab, "unable to add new device to group, max limit reached\n");
+		group_id = ATH12K_INVALID_GROUP_ID;
+		goto invalid_group;
+	}
+
 	ab->device_id = ag->num_probed++;
 	ag->ab[ab->device_id] = ab;
 	ab->ag = ag;
-	ag->mlo_capable = false;
 
 	return ag;
 }
@@ -1511,6 +1589,14 @@ static void ath12k_core_hw_group_cleanup(struct ath12k_hw_group *ag)
 		return;
 
 	mutex_lock(&ag->mutex_lock);
+
+	if (test_bit(ATH12K_GROUP_FLAG_UNREGISTER, &ag->flags)) {
+		mutex_unlock(&ag->mutex_lock);
+		return;
+	}
+
+	set_bit(ATH12K_GROUP_FLAG_UNREGISTER, &ag->flags);
+
 	ath12k_core_hw_group_stop(ag);
 
 	for (i = 0; i < ag->num_devices; i++) {
diff --git a/drivers/net/wireless/ath/ath12k/core.h b/drivers/net/wireless/ath/ath12k/core.h
index cde35616ba56..09fc1c29368f 100644
--- a/drivers/net/wireless/ath/ath12k/core.h
+++ b/drivers/net/wireless/ath/ath12k/core.h
@@ -211,6 +211,7 @@ enum ath12k_scan_state {
 
 enum ath12k_hw_group_flags {
 	ATH12K_GROUP_FLAG_REGISTERED,
+	ATH12K_GROUP_FLAG_UNREGISTER,
 };
 
 enum ath12k_dev_flags {
@@ -1026,6 +1027,7 @@ struct ath12k_base {
 	struct notifier_block panic_nb;
 
 	struct ath12k_hw_group *ag;
+	u32 wsi_index;
 
 	/* must be last */
 	u8 drv_priv[] __aligned(sizeof(void *));
-- 
2.34.1

Re: [RFC PATCH 3/6] wifi: ath12k: parse multiple device information from device tree

[Krzysztof Kozlowski]

On 23/10/2024 08:03, Raj Kumar Bhagat wrote:
> From: Harshitha Prem <quic_hprem@quicinc.com>
> 
> Currently, single device is part of device group abstraction but for multi
> link operation, multiple devices have to be combined together. Information
> of how many devices involved in grouping can be parsed from device tree and
> it would have the valid group id information as well.
> 
> Add changes to parse devices involved and group id from device tree file
> to form device group
> 
> Tested-on: QCN9274 hw2.0 PCI WLAN.WBE.1.0.1-00029-QCAHKSWPL_SILICONZ-1
> 
> Signed-off-by: Harshitha Prem <quic_hprem@quicinc.com>
> Co-developed-by: Aditya Kumar Singh <quic_adisi@quicinc.com>
> Signed-off-by: Aditya Kumar Singh <quic_adisi@quicinc.com>
> Signed-off-by: Kalle Valo <quic_kvalo@quicinc.com>

Missing SoB.

What does Kalle SoB do here?

Please read carefully submitting patches.


Best regards,
Krzysztof

[RFC PATCH 4/6] wifi: ath12k: Send partner device details in QMI MLO capability

[Raj Kumar Bhagat]

From: Karthikeyan Periyasamy <quic_periyasa@quicinc.com>

Currently, QMI MLO host capability is sent with the details of
local links and hw_link id only for particular device but in case
of multi device group abstraction, it has to include the details
of hw_link_id, num_local_links of every partner device that is
involved in the group during QMI MLO capability exchange.

Add changes to send partner device details in QMI MLO capability
exchange.

Tested-on: QCN9274 hw2.0 PCI WLAN.WBE.1.0.1-00029-QCAHKSWPL_SILICONZ-1

Signed-off-by: Karthikeyan Periyasamy <quic_periyasa@quicinc.com>
Signed-off-by: Harshitha Prem <quic_hprem@quicinc.com>
Signed-off-by: Kalle Valo <quic_kvalo@quicinc.com>
---
 drivers/net/wireless/ath/ath12k/qmi.c | 86 ++++++++++++++++++++++-----
 1 file changed, 70 insertions(+), 16 deletions(-)

diff --git a/drivers/net/wireless/ath/ath12k/qmi.c b/drivers/net/wireless/ath/ath12k/qmi.c
index 5ebfe13b5313..689171b7b19f 100644
--- a/drivers/net/wireless/ath/ath12k/qmi.c
+++ b/drivers/net/wireless/ath/ath12k/qmi.c
@@ -2016,17 +2016,19 @@ static const struct qmi_elem_info qmi_wlanfw_wlan_ini_resp_msg_v01_ei[] = {
 	},
 };
 
-static void ath12k_host_cap_parse_mlo(struct ath12k_base *ab,
-				      struct qmi_wlanfw_host_cap_req_msg_v01 *req)
+static int ath12k_host_cap_parse_mlo(struct ath12k_base *ab,
+				     struct qmi_wlanfw_host_cap_req_msg_v01 *req)
 {
 	struct wlfw_host_mlo_chip_info_s_v01 *info;
+	struct ath12k_hw_group *ag = ab->ag;
+	struct ath12k_base *partner_ab;
 	u8 hw_link_id = 0;
-	int i;
+	int i, j, ret;
 
-	if (!ab->ag->mlo_capable) {
+	if (!ag->mlo_capable) {
 		ath12k_dbg(ab, ATH12K_DBG_QMI,
 			   "MLO is disabled hence skip QMI MLO cap");
-		return;
+		return 0;
 	}
 
 	if (!ab->qmi.num_radios || ab->qmi.num_radios == U8_MAX) {
@@ -2035,7 +2037,13 @@ static void ath12k_host_cap_parse_mlo(struct ath12k_base *ab,
 		ath12k_dbg(ab, ATH12K_DBG_QMI,
 			   "skip QMI MLO cap due to invalid num_radio %d\n",
 			   ab->qmi.num_radios);
-		return;
+		return 0;
+	}
+
+	if (ab->device_id == ATH12K_INVALID_DEVICE_ID) {
+		ath12k_err(ab, "failed to send MLO cap due to invalid device id\n");
+		ret = -EINVAL;
+		return ret;
 	}
 
 	req->mlo_capable_valid = 1;
@@ -2043,27 +2051,71 @@ static void ath12k_host_cap_parse_mlo(struct ath12k_base *ab,
 	req->mlo_chip_id_valid = 1;
 	req->mlo_chip_id = ab->device_id;
 	req->mlo_group_id_valid = 1;
-	req->mlo_group_id = 0;
+	req->mlo_group_id = ag->id;
 	req->max_mlo_peer_valid = 1;
 	/* Max peer number generally won't change for the same device
 	 * but needs to be synced with host driver.
 	 */
 	req->max_mlo_peer = ab->hw_params->max_mlo_peer;
 	req->mlo_num_chips_valid = 1;
-	req->mlo_num_chips = 1;
+	req->mlo_num_chips = ag->num_devices;
 
-	info = &req->mlo_chip_info[0];
-	info->chip_id = ab->device_id;
-	info->num_local_links = ab->qmi.num_radios;
+	mutex_lock(&ag->mutex_lock);
+	for (i = 0; i < ag->num_devices; i++) {
+		info = &req->mlo_chip_info[i];
+		partner_ab = ag->ab[i];
+
+		if (partner_ab->device_id == ATH12K_INVALID_DEVICE_ID) {
+			ath12k_err(ab, "failed to send MLO cap due to invalid partner device id\n");
+			ret = -EINVAL;
+			goto device_cleanup;
+		}
+
+		info->chip_id = partner_ab->device_id;
+		info->num_local_links = partner_ab->qmi.num_radios;
 
-	for (i = 0; i < info->num_local_links; i++) {
-		info->hw_link_id[i] = hw_link_id;
-		info->valid_mlo_link_id[i] = 1;
+		ath12k_dbg(ab, ATH12K_DBG_QMI, "MLO device id %d num_link %d\n",
+			   info->chip_id, info->num_local_links);
 
-		hw_link_id++;
+		for (j = 0; j < info->num_local_links; j++) {
+			info->hw_link_id[j] = hw_link_id;
+			info->valid_mlo_link_id[j] = 1;
+
+			hw_link_id++;
+		}
 	}
 
+	if (hw_link_id <= 0)
+		ag->mlo_capable = false;
+
 	req->mlo_chip_info_valid = 1;
+
+	mutex_unlock(&ag->mutex_lock);
+	return 0;
+
+device_cleanup:
+	for (i = i - 1; i >= 0; i--) {
+		info = &req->mlo_chip_info[i];
+
+		memset(info, 0, sizeof(*info));
+	}
+
+	req->mlo_num_chips = 0;
+	req->mlo_num_chips_valid = 0;
+
+	req->max_mlo_peer = 0;
+	req->max_mlo_peer_valid = 0;
+	req->mlo_group_id = 0;
+	req->mlo_group_id_valid = 0;
+	req->mlo_chip_id = 0;
+	req->mlo_chip_id_valid = 0;
+	req->mlo_capable = 0;
+	req->mlo_capable_valid = 0;
+
+	ag->mlo_capable = false;
+	mutex_unlock(&ag->mutex_lock);
+
+	return ret;
 }
 
 static int ath12k_qmi_host_cap_send(struct ath12k_base *ab)
@@ -2111,7 +2163,9 @@ static int ath12k_qmi_host_cap_send(struct ath12k_base *ab)
 		req.nm_modem |= PLATFORM_CAP_PCIE_GLOBAL_RESET;
 	}
 
-	ath12k_host_cap_parse_mlo(ab, &req);
+	ret = ath12k_host_cap_parse_mlo(ab, &req);
+	if (ret < 0)
+		goto out;
 
 	ret = qmi_txn_init(&ab->qmi.handle, &txn,
 			   qmi_wlanfw_host_cap_resp_msg_v01_ei, &resp);
-- 
2.34.1

[RFC PATCH 5/6] wifi: ath12k: assign unique hardware link IDs during QMI host cap

[Raj Kumar Bhagat]

From: Aditya Kumar Singh <quic_adisi@quicinc.com>

Currently, in the QMI host capability, the device index, the number of
local links, and the corresponding hardware link IDs are sent. The hardware
link ID assignment is based on the local variable hw_link_id, which starts
from 0 and ranges up to num_local_links in the device. Starting from 0 is
not ideal because it can result in the same link ID being assigned to
different devices in certain scenarios (for example split mac). Hence, for
MLO to function seamlessly, the hardware link IDs across devices need to
be unique.

To address this, a previous change already read the device ID from the
Device Tree (DT) and stored it. This device ID will now be used as the
starting index for the hardware link IDs. This ensures that the hardware
link IDs assigned are unique across all devices.

While at it, add debug prints to clearly show the MLO capability
advertisement sent during QMI host capability exchange.

Signed-off-by: Aditya Kumar Singh <quic_adisi@quicinc.com>
Signed-off-by: Kalle Valo <quic_kvalo@quicinc.com>
---
 drivers/net/wireless/ath/ath12k/qmi.c | 18 +++++++++++++++---
 1 file changed, 15 insertions(+), 3 deletions(-)

diff --git a/drivers/net/wireless/ath/ath12k/qmi.c b/drivers/net/wireless/ath/ath12k/qmi.c
index 689171b7b19f..24aa74fa1c85 100644
--- a/drivers/net/wireless/ath/ath12k/qmi.c
+++ b/drivers/net/wireless/ath/ath12k/qmi.c
@@ -2060,6 +2060,12 @@ static int ath12k_host_cap_parse_mlo(struct ath12k_base *ab,
 	req->mlo_num_chips_valid = 1;
 	req->mlo_num_chips = ag->num_devices;
 
+	ath12k_dbg(ab, ATH12K_DBG_QMI, "MLO Capability advertisement:");
+	ath12k_dbg(ab, ATH12K_DBG_QMI, " * device_id: %d", req->mlo_chip_id);
+	ath12k_dbg(ab, ATH12K_DBG_QMI, " * group_id: %d", req->mlo_group_id);
+	ath12k_dbg(ab, ATH12K_DBG_QMI, " * num_devices: %d", req->mlo_num_chips);
+	ath12k_dbg(ab, ATH12K_DBG_QMI, " * Devices info:");
+
 	mutex_lock(&ag->mutex_lock);
 	for (i = 0; i < ag->num_devices; i++) {
 		info = &req->mlo_chip_info[i];
@@ -2074,13 +2080,19 @@ static int ath12k_host_cap_parse_mlo(struct ath12k_base *ab,
 		info->chip_id = partner_ab->device_id;
 		info->num_local_links = partner_ab->qmi.num_radios;
 
-		ath12k_dbg(ab, ATH12K_DBG_QMI, "MLO device id %d num_link %d\n",
-			   info->chip_id, info->num_local_links);
+		ath12k_dbg(ab, ATH12K_DBG_QMI, "   * device_id: %d",
+			   info->chip_id);
+		ath12k_dbg(ab, ATH12K_DBG_QMI, "     * num_links: %d",
+			   info->num_local_links);
 
 		for (j = 0; j < info->num_local_links; j++) {
-			info->hw_link_id[j] = hw_link_id;
+			info->hw_link_id[j] = partner_ab->wsi_index + j;
 			info->valid_mlo_link_id[j] = 1;
 
+			ath12k_dbg(ab, ATH12K_DBG_QMI,
+				   "       * hw_link_id: %d\n",
+				   info->hw_link_id[j]);
+
 			hw_link_id++;
 		}
 	}
-- 
2.34.1

[RFC PATCH 6/6] arm64: dts: qcom: ipq9574: Add WiFi nodes for RDP433

[Raj Kumar Bhagat]

The RDP433 is a Qualcomm Reference Design Platform based on the
IPQ9574. It has three QCN9274 WiFi devices connected to PCIe1, PCIe2,
and PCIe3. These devices are also connected among themselves via
WSI connection. This WSI connection is essential to exchange control
information among these devices

The WSI connection in RDP433 is represented below:

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

Based on the above, the WSI properties for QCN9274 at pcie2 are:
 qcom,wsi-group-id = 0
 qcom,wsi-index = 0
 qcom,wsi-num-devices = 3;

Hence, add WiFi nodes with WSI properties for all three QCN9274
devices connected to RDP433.

Signed-off-by: Raj Kumar Bhagat <quic_rajkbhag@quicinc.com>
---
 arch/arm64/boot/dts/qcom/ipq9574-rdp433.dts | 62 ++++++++++++++++++++-
 1 file changed, 61 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/boot/dts/qcom/ipq9574-rdp433.dts b/arch/arm64/boot/dts/qcom/ipq9574-rdp433.dts
index 165ebbb59511..2241e20ad42a 100644
--- a/arch/arm64/boot/dts/qcom/ipq9574-rdp433.dts
+++ b/arch/arm64/boot/dts/qcom/ipq9574-rdp433.dts
@@ -3,7 +3,7 @@
  * IPQ9574 RDP433 board device tree source
  *
  * Copyright (c) 2020-2021 The Linux Foundation. All rights reserved.
- * Copyright (c) 2023 Qualcomm Innovation Center, Inc. All rights reserved.
+ * Copyright (c) 2023-2024 Qualcomm Innovation Center, Inc. All rights reserved.
  */
 
 /dts-v1/;
@@ -27,6 +27,26 @@ &pcie1 {
 	perst-gpios = <&tlmm 26 GPIO_ACTIVE_LOW>;
 	wake-gpios = <&tlmm 27 GPIO_ACTIVE_LOW>;
 	status = "okay";
+
+	pcie@0 {
+		device_type = "pci";
+		reg = <0x0 0x0 0x0 0x0 0x0>;
+		#address-cells = <3>;
+		#size-cells = <2>;
+		ranges;
+
+		wifi1@0 {
+			compatible = "pci17cb,1109";
+			reg = <0x0 0x0 0x0 0x0 0x0>;
+			status = "okay";
+
+			wsi {
+				qcom,wsi-group-id = <0>;
+				qcom,wsi-index = <2>;
+				qcom,wsi-num-devices = <3>;
+			};
+		};
+	};
 };
 
 &pcie2_phy {
@@ -40,6 +60,26 @@ &pcie2 {
 	perst-gpios = <&tlmm 29 GPIO_ACTIVE_LOW>;
 	wake-gpios = <&tlmm 30 GPIO_ACTIVE_LOW>;
 	status = "okay";
+
+	pcie@0 {
+		device_type = "pci";
+		reg = <0x0 0x0 0x0 0x0 0x0>;
+		#address-cells = <3>;
+		#size-cells = <2>;
+		ranges;
+
+		wifi2@0 {
+			compatible = "pci17cb,1109";
+			reg = <0x0 0x0 0x0 0x0 0x0>;
+			status = "okay";
+
+			wsi {
+				qcom,wsi-group-id = <0>;
+				qcom,wsi-index = <0>;
+				qcom,wsi-num-devices = <3>;
+			};
+		};
+	};
 };
 
 &pcie3_phy {
@@ -53,6 +93,26 @@ &pcie3 {
 	perst-gpios = <&tlmm 32 GPIO_ACTIVE_LOW>;
 	wake-gpios = <&tlmm 33 GPIO_ACTIVE_LOW>;
 	status = "okay";
+
+	pcie@0 {
+		device_type = "pci";
+		reg = <0x0 0x0 0x0 0x0 0x0>;
+		#address-cells = <3>;
+		#size-cells = <2>;
+		ranges;
+
+		wifi3@0 {
+			compatible = "pci17cb,1109";
+			reg = <0x0 0x0 0x0 0x0 0x0>;
+			status = "okay";
+
+			wsi {
+				qcom,wsi-group-id = <0>;
+				qcom,wsi-index = <1>;
+				qcom,wsi-num-devices = <3>;
+			};
+		};
+	};
 };
 
 &sdhc_1 {
-- 
2.34.1

Re: [RFC PATCH 6/6] arm64: dts: qcom: ipq9574: Add WiFi nodes for RDP433

[Krzysztof Kozlowski]

On 23/10/2024 08:03, Raj Kumar Bhagat wrote:
> The RDP433 is a Qualcomm Reference Design Platform based on the
> IPQ9574. It has three QCN9274 WiFi devices connected to PCIe1, PCIe2,
> and PCIe3. These devices are also connected among themselves via
> WSI connection. This WSI connection is essential to exchange control
> information among these devices
> 
> The WSI connection in RDP433 is represented below:
> 
>           +-------+        +-------+        +-------+
>           | pcie2 |        | pcie3 |        | pcie1 |
>           |       |        |       |        |       |
>    +----->|  wsi  |------->|  wsi  |------->|  wsi  |-----+
>    |      | idx 0 |        | idx 1 |        | idx 2 |     |
>    |      +-------+        +-------+        +-------+     |
>    +------------------------------------------------------+
> 
> Based on the above, the WSI properties for QCN9274 at pcie2 are:
>  qcom,wsi-group-id = 0
>  qcom,wsi-index = 0
>  qcom,wsi-num-devices = 3;
> 
> Hence, add WiFi nodes with WSI properties for all three QCN9274
> devices connected to RDP433.
> 
> Signed-off-by: Raj Kumar Bhagat <quic_rajkbhag@quicinc.com>
> ---
>  arch/arm64/boot/dts/qcom/ipq9574-rdp433.dts | 62 ++++++++++++++++++++-
>  1 file changed, 61 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/arm64/boot/dts/qcom/ipq9574-rdp433.dts b/arch/arm64/boot/dts/qcom/ipq9574-rdp433.dts
> index 165ebbb59511..2241e20ad42a 100644
> --- a/arch/arm64/boot/dts/qcom/ipq9574-rdp433.dts
> +++ b/arch/arm64/boot/dts/qcom/ipq9574-rdp433.dts
> @@ -3,7 +3,7 @@
>   * IPQ9574 RDP433 board device tree source
>   *
>   * Copyright (c) 2020-2021 The Linux Foundation. All rights reserved.
> - * Copyright (c) 2023 Qualcomm Innovation Center, Inc. All rights reserved.
> + * Copyright (c) 2023-2024 Qualcomm Innovation Center, Inc. All rights reserved.
>   */
>  
>  /dts-v1/;
> @@ -27,6 +27,26 @@ &pcie1 {
>  	perst-gpios = <&tlmm 26 GPIO_ACTIVE_LOW>;
>  	wake-gpios = <&tlmm 27 GPIO_ACTIVE_LOW>;
>  	status = "okay";
> +
> +	pcie@0 {
> +		device_type = "pci";
> +		reg = <0x0 0x0 0x0 0x0 0x0>;
> +		#address-cells = <3>;
> +		#size-cells = <2>;
> +		ranges;
> +
> +		wifi1@0 {
> +			compatible = "pci17cb,1109";
> +			reg = <0x0 0x0 0x0 0x0 0x0>;
> +			status = "okay";

Why?

> +
> +			wsi {
> +				qcom,wsi-group-id = <0>;

So all devices have the same group id? No other group id? So hard-code
it at 0?

> +				qcom,wsi-index = <2>;
> +				qcom,wsi-num-devices = <3>;

All this looks opposite how we organize usually DTS. Instead of phandles
you pass some sort of indices. Instead of re-using some existing
properties for remoteproc-related stuff, you add three more.


Best regards,
Krzysztof