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

[RFC PATCH v2 0/5] 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 details of WSI connection for 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/

v2:
- "Graph with endpoint” is used to define the actual WSI connection in
  the DT binding.
- The qcom,wsi-index and qcom,wsi-num-devices properties are dropped
  from the binding. These are now determined in the driver using the
  graph with endpoint.
- The qcom,wsi-master property is added to the binding to define the
  WSI master.
- The qcom,ath12k-calibration-variant property is added to the binding
  for ath12k devices.
- DTS changes are made based on the “graph with endpoint” binding.
- The ath12k driver is updated to read graph nodes, determine the number
  of devices in WSI, and assign the wsi-index for each device.
- The ath12k driver now assigns hardware link IDs based on the order of
  WSI connections.

v1: https://patchwork.kernel.org/project/linux-wireless/cover/20241023060352.605019-1-quic_rajkbhag@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 (2):
  dt-bindings: net: wireless: ath12k: describe WSI properties for
    QCN9274
  arm64: dts: qcom: ipq9574: Add WiFi nodes for RDP433

 .../bindings/net/wireless/qcom,ath12k.yaml    | 241 +++++++++++++++++-
 arch/arm64/boot/dts/qcom/ipq9574-rdp433.dts   | 116 ++++++++-
 drivers/net/wireless/ath/ath12k/core.c        | 154 ++++++++++-
 drivers/net/wireless/ath/ath12k/core.h        |  10 +
 drivers/net/wireless/ath/ath12k/qmi.c         | 121 +++++++--
 5 files changed, 605 insertions(+), 37 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 v2 1/5] dt-bindings: net: wireless: ath12k: describe WSI properties 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-master: Indicates if this device is the WSI master.

 - ports: This is a graph ports schema that has two ports: TX (port@0)
   and RX (port@1). This represents the actual WSI connection among
   multiple devices.

Also, describe the ath12k device property
"qcom,ath12k-calibration-variant". This is a common property among
ath12k devices.

Signed-off-by: Raj Kumar Bhagat <quic_rajkbhag@quicinc.com>
---
 .../bindings/net/wireless/qcom,ath12k.yaml    | 241 +++++++++++++++++-
 1 file changed, 232 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..42bcd73dd159 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#
@@ -18,10 +19,17 @@ properties:
   compatible:
     enum:
       - pci17cb,1107  # WCN7850
+      - pci17cb,1109  # QCN9274
 
   reg:
     maxItems: 1
 
+  qcom,ath12k-calibration-variant:
+    $ref: /schemas/types.yaml#/definitions/string
+    description: |
+      string to uniquely identify variant of the calibration data for designs
+      with colliding bus and device ids
+
   vddaon-supply:
     description: VDD_AON supply regulator handle
 
@@ -49,21 +57,100 @@ 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.
+
+      Diagram to represent one WSI connection (one WSI group) among
+      three devices.
+
+               +-------+        +-------+        +-------+
+               | pcie2 |        | pcie3 |        | pcie1 |
+               |       |        |       |        |       |
+        +----->|  wsi  |------->|  wsi  |------->|  wsi  |-----+
+        |      | grp 0 |        | grp 0 |        | grp 2 |     |
+        |      +-------+        +-------+        +-------+     |
+        +------------------------------------------------------+
+
+      Diagram to represent two WSI connections (two separate WSI groups)
+      among four devices.
+
+           +-------+    +-------+          +-------+    +-------+
+           | pcie2 |    | pcie3 |          | pcie1 |    | pcie0 |
+           |       |    |       |          |       |    |       |
+       +-->|  wsi  |--->|  wsi  |--+   +-->|  wsi  |--->|  wsi  |--+
+       |   | grp 0 |    | grp 0 |  |   |   | grp 1 |    | grp 1 |  |
+       |   +-------+    +-------+  |   |   +-------+    +-------+  |
+       +---------------------------+   +---------------------------+
+
+    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-master:
+        type: boolean
+        description:
+          Indicates if this device is the WSI master.
+
+      ports:
+        $ref: /schemas/graph.yaml#/properties/ports
+        description:
+          These ports are used to connect multiple WSI supported devices to
+          form the WSI group.
+
+        properties:
+          port@0:
+            $ref: /schemas/graph.yaml#/properties/port
+            description:
+              This is the TX port of WSI interface. It is attached to the RX
+              port of the next device in the WSI connection.
+
+          port@1:
+            $ref: /schemas/graph.yaml#/properties/port
+            description:
+              This is the RX port of WSI interface. It is attached to the TX
+              port of the previous device in the WSI connection.
+
+    required:
+      - qcom,wsi-group-id
+      - ports
+
+    additionalProperties: false
+
 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>
@@ -97,3 +184,139 @@ examples:
             };
         };
     };
+
+  - |
+    pcie1 {
+        #address-cells = <3>;
+        #size-cells = <2>;
+
+        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>;
+
+                qcom,ath12k-calibration-variant = "RDP433_1";
+
+                wsi {
+                    qcom,wsi-group-id = <0>;
+
+                    ports {
+                        #address-cells = <1>;
+                        #size-cells = <0>;
+
+                        port@0 {
+                            reg = <0>;
+
+                            wifi1_wsi_tx: endpoint {
+                                remote-endpoint = <&wifi2_wsi_rx>;
+                            };
+                        };
+
+                        port@1 {
+                            reg = <1>;
+
+                            wifi1_wsi_rx: endpoint {
+                                remote-endpoint = <&wifi3_wsi_tx>;
+                            };
+                        };
+                    };
+                };
+            };
+        };
+    };
+
+    pcie2 {
+        #address-cells = <3>;
+        #size-cells = <2>;
+
+        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>;
+
+                qcom,ath12k-calibration-variant = "RDP433_2";
+
+                wsi {
+                    qcom,wsi-group-id = <0>;
+
+                    ports {
+                        #address-cells = <1>;
+                        #size-cells = <0>;
+
+                        port@0 {
+                            reg = <0>;
+
+                            wifi2_wsi_tx: endpoint {
+                                remote-endpoint = <&wifi3_wsi_rx>;
+                            };
+                        };
+
+                        port@1 {
+                            reg = <1>;
+
+                            wifi2_wsi_rx: endpoint {
+                                remote-endpoint = <&wifi1_wsi_tx>;
+                            };
+                        };
+                    };
+                };
+            };
+        };
+    };
+
+    pcie3 {
+        #address-cells = <3>;
+        #size-cells = <2>;
+
+        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>;
+
+                qcom,ath12k-calibration-variant = "RDP433_3";
+
+                wsi {
+                    qcom,wsi-group-id = <0>;
+
+                    ports {
+                        #address-cells = <1>;
+                        #size-cells = <0>;
+
+                        port@0 {
+                            reg = <0>;
+
+                            wifi3_wsi_tx: endpoint {
+                                remote-endpoint = <&wifi1_wsi_rx>;
+                            };
+                        };
+
+                        port@1 {
+                            reg = <1>;
+
+                            wifi3_wsi_rx: endpoint {
+                                remote-endpoint = <&wifi2_wsi_tx>;
+                            };
+                        };
+                    };
+                };
+            };
+        };
+    };
-- 
2.34.1

[RFC PATCH v2 2/5] 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>
Co-developed-by: Raj Kumar Bhagat <quic_rajkbhag@quicinc.com>
Signed-off-by: Raj Kumar Bhagat <quic_rajkbhag@quicinc.com>
---
 drivers/net/wireless/ath/ath12k/core.c | 154 +++++++++++++++++++++++--
 drivers/net/wireless/ath/ath12k/core.h |   8 ++
 2 files changed, 151 insertions(+), 11 deletions(-)

diff --git a/drivers/net/wireless/ath/ath12k/core.c b/drivers/net/wireless/ath/ath12k/core.c
index 3f0f44cbdb4c..91950ee50bf3 100644
--- a/drivers/net/wireless/ath/ath12k/core.c
+++ b/drivers/net/wireless/ath/ath12k/core.c
@@ -9,6 +9,7 @@
 #include <linux/remoteproc.h>
 #include <linux/firmware.h>
 #include <linux/of.h>
+#include <linux/of_graph.h>
 #include "core.h"
 #include "dp_tx.h"
 #include "dp_rx.h"
@@ -1403,6 +1404,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,34 +1419,156 @@ 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 int ath12k_core_get_wsi_info(struct ath12k_base *ab,
+				    struct ath12k_wsi_info *wsi_info)
+{
+	struct device_node *wsi_dev, *next_wsi_dev;
+	struct device_node *tx_endpoint, *next_rx_endpoint;
+	int device_count = 0, wsi_master_index = -1;
+
+	wsi_dev = of_get_child_by_name(ab->dev->of_node, "wsi");
+	if (!wsi_dev)
+		return -ENODEV;
+
+	if (of_property_read_u32(wsi_dev, "qcom,wsi-group-id",
+				 &wsi_info->group_id)) {
+		of_node_put(wsi_dev);
+		return -EINVAL;
+	}
+
+	next_wsi_dev = wsi_dev;
+
+	do {
+		if (of_property_read_bool(next_wsi_dev, "qcom,wsi-master"))
+			wsi_master_index = device_count;
+
+		tx_endpoint = of_graph_get_endpoint_by_regs(next_wsi_dev, 0, -1);
+		if (!tx_endpoint) {
+			of_node_put(next_wsi_dev);
+			return -ENODEV;
+		}
+
+		next_rx_endpoint = of_graph_get_remote_endpoint(tx_endpoint);
+		if (!next_rx_endpoint) {
+			of_node_put(next_wsi_dev);
+			of_node_put(tx_endpoint);
+			return -ENODEV;
+		}
+
+		of_node_put(tx_endpoint);
+		of_node_put(next_wsi_dev);
+		next_wsi_dev = of_graph_get_port_parent(next_rx_endpoint);
+		if (!next_wsi_dev) {
+			of_node_put(next_rx_endpoint);
+			return -ENODEV;
+		}
+		of_node_put(next_rx_endpoint);
+
+		device_count++;
+		if (device_count > ATH12K_MAX_SOCS) {
+			ath12k_warn(ab, "device count in DT %d is more than limit %d\n",
+				    device_count, ATH12K_MAX_SOCS);
+			of_node_put(next_wsi_dev);
+			return -EINVAL;
+		}
+	} while (wsi_dev != next_wsi_dev);
+
+	of_node_put(next_wsi_dev);
+	if (wsi_master_index == -1) {
+		ath12k_dbg(ab, ATH12K_DBG_BOOT, "no WSI master defined in DT");
+		return -EINVAL;
+	}
+
+	wsi_info->num_devices = device_count;
+	wsi_info->index = (device_count - wsi_master_index) % device_count;
+	return 0;
+}
+
 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;
+	struct ath12k_wsi_info *wsi = &ab->wsi_info;
+	int ret;
 
 	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.
+	 */
+	ret = ath12k_core_get_wsi_info(ab, wsi);
+	if (ret) {
+		ath12k_dbg(ab, ATH12K_DBG_BOOT,
+			   "unable to get complete WSI Info from DT, err = %d", ret);
+		wsi->group_id = ATH12K_INVALID_GROUP_ID;
+		wsi->num_devices = 1;
+		wsi->index = 0;
+		goto invalid_group;
+	}
+
+	ath12k_dbg(ab, ATH12K_DBG_BOOT,
+		   "WSI info: group-id: %d, num-devices: %d, index: %d",
+		   wsi->group_id, wsi->num_devices, 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_alloc(group_id, 1);
+	ag = ath12k_core_hw_group_find_by_id(wsi->group_id);
+	if (!ag) {
+		ag = ath12k_core_hw_group_alloc(wsi->group_id, wsi->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);
+		wsi->group_id = ATH12K_INVALID_GROUP_ID;
+		goto invalid_group;
+	} else {
+		goto exit;
+	}
+
+invalid_group:
+	ag = ath12k_core_hw_group_alloc(wsi->group_id, 1);
 	if (!ag) {
 		ath12k_warn(ab, "unable to create new hw group\n");
 		return NULL;
 	}
 	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");
+		wsi->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 +1635,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..6ade7a3cf6ff 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 {
@@ -845,6 +846,12 @@ struct ath12k_hw_group {
 	bool mlo_capable;
 };
 
+struct ath12k_wsi_info {
+	u32 group_id;
+	u32 num_devices;
+	u32 index;
+};
+
 /* Master structure to hold the hw data which may be used in core module */
 struct ath12k_base {
 	enum ath12k_hw_rev hw_rev;
@@ -1026,6 +1033,7 @@ struct ath12k_base {
 	struct notifier_block panic_nb;
 
 	struct ath12k_hw_group *ag;
+	struct ath12k_wsi_info wsi_info;
 
 	/* must be last */
 	u8 drv_priv[] __aligned(sizeof(void *));
-- 
2.34.1

[RFC PATCH v2 3/5] 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: Raj Kumar Bhagat <quic_rajkbhag@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 v2 4/5] 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 (e.g., split MAC).
Additionally, for multi link operations the firmware expects the hardware
link IDs in the same order as the Wireless Serial Interface (WSI)
connection.

Hence, for MLO to function seamlessly, the hardware link IDs across
devices need to be unique and should follow the order of the WSI
connection.

To address this, a previous change read the WSI index from the Device Tree
(DT) and stored it. Use this WSI index to determine the starting hardware
link IDs for each device, ensuring uniqueness and correct order across all
devices.

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

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

Signed-off-by: Aditya Kumar Singh <quic_adisi@quicinc.com>
Co-developed-by: Raj Kumar Bhagat <quic_rajkbhag@quicinc.com>
Signed-off-by: Raj Kumar Bhagat <quic_rajkbhag@quicinc.com>
---
 drivers/net/wireless/ath/ath12k/core.h |  2 ++
 drivers/net/wireless/ath/ath12k/qmi.c  | 41 ++++++++++++++++++++++++--
 2 files changed, 40 insertions(+), 3 deletions(-)

diff --git a/drivers/net/wireless/ath/ath12k/core.h b/drivers/net/wireless/ath/ath12k/core.h
index 6ade7a3cf6ff..a4f772ad96c0 100644
--- a/drivers/net/wireless/ath/ath12k/core.h
+++ b/drivers/net/wireless/ath/ath12k/core.h
@@ -844,12 +844,14 @@ struct ath12k_hw_group {
 	struct ath12k_hw *ah[ATH12K_GROUP_MAX_RADIO];
 	u8 num_hw;
 	bool mlo_capable;
+	bool hw_link_id_init_done;
 };
 
 struct ath12k_wsi_info {
 	u32 group_id;
 	u32 num_devices;
 	u32 index;
+	u32 hw_link_id_base;
 };
 
 /* Master structure to hold the hw data which may be used in core module */
diff --git a/drivers/net/wireless/ath/ath12k/qmi.c b/drivers/net/wireless/ath/ath12k/qmi.c
index 689171b7b19f..7ecb539f60f2 100644
--- a/drivers/net/wireless/ath/ath12k/qmi.c
+++ b/drivers/net/wireless/ath/ath12k/qmi.c
@@ -2016,6 +2016,25 @@ static const struct qmi_elem_info qmi_wlanfw_wlan_ini_resp_msg_v01_ei[] = {
 	},
 };
 
+static void ath12k_host_cap_hw_link_id_init(struct ath12k_hw_group *ag)
+{
+	struct ath12k_base *ab, *partner_ab;
+	int i, j, hw_id_base;
+
+	for (i = 0; i < ag->num_devices; i++) {
+		hw_id_base = 0;
+		ab = ag->ab[i];
+		for (j = 0; j < ag->num_devices; j++) {
+			partner_ab = ag->ab[j];
+			if (partner_ab->wsi_info.index >= ab->wsi_info.index)
+				continue;
+			hw_id_base += partner_ab->qmi.num_radios;
+		}
+		ab->wsi_info.hw_link_id_base = hw_id_base;
+	}
+	ag->hw_link_id_init_done = true;
+}
+
 static int ath12k_host_cap_parse_mlo(struct ath12k_base *ab,
 				     struct qmi_wlanfw_host_cap_req_msg_v01 *req)
 {
@@ -2060,7 +2079,17 @@ 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);
+
+	if (!ag->hw_link_id_init_done)
+		ath12k_host_cap_hw_link_id_init(ag);
+
 	for (i = 0; i < ag->num_devices; i++) {
 		info = &req->mlo_chip_info[i];
 		partner_ab = ag->ab[i];
@@ -2074,13 +2103,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_info.hw_link_id_base + 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 v2 5/5] 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  |-----+
   |      | grp 0 |        | grp 0 |        | grp 0 |     |
   |      +-------+        +-------+        +-------+     |
   +------------------------------------------------------+

Based on the above, the WSI properties for QCN9274 at pcie2 are
(considering QCN9274 at pcie2 is WSI master):

 qcom,wsi-group-id = 0
 qcom,wsi-master
 ports:
    tx-port (port@0): endpoint at pcie3 RX port.
    rx-port (port@1): endpoint at pcie1 TX port.

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 | 116 +++++++++++++++++++-
 1 file changed, 115 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..d0ecaefe5b41 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,44 @@ &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>;
+
+			wsi {
+				qcom,wsi-group-id = <0>;
+
+				ports {
+					#address-cells = <1>;
+					#size-cells = <0>;
+
+					port@0 {
+						reg = <0>;
+
+						wifi1_wsi_tx: endpoint {
+							remote-endpoint = <&wifi2_wsi_rx>;
+						};
+					};
+
+					port@1 {
+						reg = <1>;
+
+						wifi1_wsi_rx: endpoint {
+							remote-endpoint = <&wifi3_wsi_tx>;
+						};
+					};
+				};
+			};
+		};
+	};
 };
 
 &pcie2_phy {
@@ -40,6 +78,45 @@ &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>;
+
+			wsi {
+				qcom,wsi-group-id = <0>;
+				qcom,wsi-master;
+
+				ports {
+					#address-cells = <1>;
+					#size-cells = <0>;
+
+					port@0 {
+						reg = <0>;
+
+						wifi2_wsi_tx: endpoint {
+							remote-endpoint = <&wifi3_wsi_rx>;
+						};
+					};
+
+					port@1 {
+						reg = <1>;
+
+						wifi2_wsi_rx: endpoint {
+							remote-endpoint = <&wifi1_wsi_tx>;
+						};
+					};
+				};
+			};
+		};
+	};
 };
 
 &pcie3_phy {
@@ -53,6 +130,43 @@ &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>;
+
+			wsi {
+				qcom,wsi-group-id = <0>;
+
+				ports {
+					#address-cells = <1>;
+					#size-cells = <0>;
+
+					port@0 {
+						reg = <0>;
+
+						wifi3_wsi_tx: endpoint {
+							remote-endpoint = <&wifi1_wsi_rx>;
+						};
+					};
+
+					port@1 {
+						reg = <1>;
+						wifi3_wsi_rx: endpoint {
+							remote-endpoint = <&wifi2_wsi_tx>;
+						};
+					};
+				};
+			};
+		};
+	};
 };
 
 &sdhc_1 {
-- 
2.34.1

Re: [RFC PATCH v2 1/5] dt-bindings: net: wireless: ath12k: describe WSI properties for QCN9274

[Krzysztof Kozlowski]

On 29/10/2024 18:30, 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-master: Indicates if this device is the WSI master.
> 
>  - ports: This is a graph ports schema that has two ports: TX (port@0)
>    and RX (port@1). This represents the actual WSI connection among
>    multiple devices.

Describe the hardware, not the contents of the patch/binding. We see it
easily, but what we do not see is the hardware.

> 
> Also, describe the ath12k device property
> "qcom,ath12k-calibration-variant". This is a common property among
> ath12k devices.

Why do you describe it? What you do is easily visible. We do not see why.

> 
> Signed-off-by: Raj Kumar Bhagat <quic_rajkbhag@quicinc.com>
> ---
>  .../bindings/net/wireless/qcom,ath12k.yaml    | 241 +++++++++++++++++-
>  1 file changed, 232 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..42bcd73dd159 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#
> @@ -18,10 +19,17 @@ properties:
>    compatible:
>      enum:
>        - pci17cb,1107  # WCN7850
> +      - pci17cb,1109  # QCN9274

I asked for separate binding because it is quite a different device.
Unless it is not... but then commit msg is quite not precise here.

>  
>    reg:
>      maxItems: 1
>  
> +  qcom,ath12k-calibration-variant:
> +    $ref: /schemas/types.yaml#/definitions/string
> +    description: |

Do not need '|' unless you need to preserve formatting.

> +      string to uniquely identify variant of the calibration data for designs
> +      with colliding bus and device ids
> +
>    vddaon-supply:
>      description: VDD_AON supply regulator handle
>  
> @@ -49,21 +57,100 @@ properties:
>    vddpcie1p8-supply:
>      description: VDD_PCIE_1P8 supply regulator handle
>  
> +  wsi:

Not much improved here. I asked to drop the node.

> +    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.
> +
> +      Diagram to represent one WSI connection (one WSI group) among
> +      three devices.
> +
> +               +-------+        +-------+        +-------+
> +               | pcie2 |        | pcie3 |        | pcie1 |
> +               |       |        |       |        |       |
> +        +----->|  wsi  |------->|  wsi  |------->|  wsi  |-----+
> +        |      | grp 0 |        | grp 0 |        | grp 2 |     |
> +        |      +-------+        +-------+        +-------+     |
> +        +------------------------------------------------------+
> +
> +      Diagram to represent two WSI connections (two separate WSI groups)
> +      among four devices.
> +
> +           +-------+    +-------+          +-------+    +-------+
> +           | pcie2 |    | pcie3 |          | pcie1 |    | pcie0 |
> +           |       |    |       |          |       |    |       |
> +       +-->|  wsi  |--->|  wsi  |--+   +-->|  wsi  |--->|  wsi  |--+
> +       |   | grp 0 |    | grp 0 |  |   |   | grp 1 |    | grp 1 |  |
> +       |   +-------+    +-------+  |   |   +-------+    +-------+  |
> +       +---------------------------+   +---------------------------+
> +
> +    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.

That's not needed according to description. Entire group is defined by
graph.

> +
> +      qcom,wsi-master:
> +        type: boolean
> +        description:
> +          Indicates if this device is the WSI master.
> +

This copies property name. Why being master is important?

Also, use some different name: see preferred names in kernel coding style.

> +      ports:
> +        $ref: /schemas/graph.yaml#/properties/ports
> +        description:
> +          These ports are used to connect multiple WSI supported devices to
> +          form the WSI group.
> +
> +        properties:
> +          port@0:
> +            $ref: /schemas/graph.yaml#/properties/port
> +            description:
> +              This is the TX port of WSI interface. It is attached to the RX
> +              port of the next device in the WSI connection.
> +
> +          port@1:
> +            $ref: /schemas/graph.yaml#/properties/port
> +            description:
> +              This is the RX port of WSI interface. It is attached to the TX
> +              port of the previous device in the WSI connection.
> +
> +    required:
> +      - qcom,wsi-group-id
> +      - ports
> +
> +    additionalProperties: false
> +
>  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

Commit says WSI applies only to new variant, so properties should be
disallowed... or just follow my feedback last time: separate binding.

> +
>  examples:
>    - |
>      #include <dt-bindings/clock/qcom,rpmh.h>
> @@ -97,3 +184,139 @@ examples:
>              };
>          };
>      };
> +
> +  - |
> +    pcie1 {

pcie {
and keep all nodes here

> +        #address-cells = <3>;
> +        #size-cells = <2>;
> +
> +        pcie@0 {
> +            device_type = "pci";
> +            reg = <0x0 0x0 0x0 0x0 0x0>;
> +            #address-cells = <3>;
> +            #size-cells = <2>;
> +            ranges;
> +
> +            wifi1@0 {

wifi@

Same in other places.

> +                compatible = "pci17cb,1109";
> +                reg = <0x0 0x0 0x0 0x0 0x0>;
> +
> +                qcom,ath12k-calibration-variant = "RDP433_1";
> +
> +                wsi {

No resources here? Not a bus? You already got comment about it.


Best regards,
Krzysztof

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

[Krzysztof Kozlowski]

On 29/10/2024 18:30, 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  |-----+
>    |      | grp 0 |        | grp 0 |        | grp 0 |     |
>    |      +-------+        +-------+        +-------+     |
>    +------------------------------------------------------+
> 
> Based on the above, the WSI properties for QCN9274 at pcie2 are
> (considering QCN9274 at pcie2 is WSI master):
> 
>  qcom,wsi-group-id = 0
>  qcom,wsi-master
>  ports:
>     tx-port (port@0): endpoint at pcie3 RX port.
>     rx-port (port@1): endpoint at pcie1 TX port.
> 
> 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 | 116 +++++++++++++++++++-
>  1 file changed, 115 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..d0ecaefe5b41 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,44 @@ &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 {

Node names should be generic. See also an explanation and list of
examples (not exhaustive) in DT specification:
https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation

Best regards,
Krzysztof

Re: [RFC PATCH v2 1/5] dt-bindings: net: wireless: ath12k: describe WSI properties for QCN9274

[Jeff Johnson]

On 10/29/2024 10:30 AM, 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-master: Indicates if this device is the WSI master.
> 
>  - ports: This is a graph ports schema that has two ports: TX (port@0)
>    and RX (port@1). This represents the actual WSI connection among
>    multiple devices.
> 
> Also, describe the ath12k device property
> "qcom,ath12k-calibration-variant". This is a common property among
> ath12k devices.
> 
> Signed-off-by: Raj Kumar Bhagat <quic_rajkbhag@quicinc.com>
> ---
>  .../bindings/net/wireless/qcom,ath12k.yaml    | 241 +++++++++++++++++-
>  1 file changed, 232 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..42bcd73dd159 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#
> @@ -18,10 +19,17 @@ properties:
>    compatible:
>      enum:
>        - pci17cb,1107  # WCN7850
> +      - pci17cb,1109  # QCN9274
>  
>    reg:
>      maxItems: 1
>  
> +  qcom,ath12k-calibration-variant:
> +    $ref: /schemas/types.yaml#/definitions/string
> +    description: |
> +      string to uniquely identify variant of the calibration data for designs
> +      with colliding bus and device ids
> +
>    vddaon-supply:
>      description: VDD_AON supply regulator handle
>  
> @@ -49,21 +57,100 @@ 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.
> +
> +      Diagram to represent one WSI connection (one WSI group) among
> +      three devices.
> +
> +               +-------+        +-------+        +-------+
> +               | pcie2 |        | pcie3 |        | pcie1 |

is there a reason to not have these in some order?

> +               |       |        |       |        |       |
> +        +----->|  wsi  |------->|  wsi  |------->|  wsi  |-----+
> +        |      | grp 0 |        | grp 0 |        | grp 2 |     |

s/grp 2/grp 0/???                                          ^ typo?

> +        |      +-------+        +-------+        +-------+     |
> +        +------------------------------------------------------+
> +
> +      Diagram to represent two WSI connections (two separate WSI groups)
> +      among four devices.
> +
> +           +-------+    +-------+          +-------+    +-------+
> +           | pcie2 |    | pcie3 |          | pcie1 |    | pcie0 |

again seems strange to not have any logical (to me) order

> +           |       |    |       |          |       |    |       |
> +       +-->|  wsi  |--->|  wsi  |--+   +-->|  wsi  |--->|  wsi  |--+
> +       |   | grp 0 |    | grp 0 |  |   |   | grp 1 |    | grp 1 |  |
> +       |   +-------+    +-------+  |   |   +-------+    +-------+  |
> +       +---------------------------+   +---------------------------+
> +
> +    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-master:
> +        type: boolean
> +        description:
> +          Indicates if this device is the WSI master.
> +
> +      ports:
> +        $ref: /schemas/graph.yaml#/properties/ports
> +        description:
> +          These ports are used to connect multiple WSI supported devices to
> +          form the WSI group.
> +
> +        properties:
> +          port@0:
> +            $ref: /schemas/graph.yaml#/properties/port
> +            description:
> +              This is the TX port of WSI interface. It is attached to the RX
> +              port of the next device in the WSI connection.
> +
> +          port@1:
> +            $ref: /schemas/graph.yaml#/properties/port
> +            description:
> +              This is the RX port of WSI interface. It is attached to the TX
> +              port of the previous device in the WSI connection.
> +
> +    required:
> +      - qcom,wsi-group-id
> +      - ports
> +
> +    additionalProperties: false
> +
>  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>
> @@ -97,3 +184,139 @@ examples:
>              };
>          };
>      };
> +
> +  - |

in the description above you have two different diagrams:
- one that shows 3 pcie* devices in a single group with apparently one port
per device
- one that shows 4 pcie* devices split into two groups of two, again with
apparently one port per device

but in the representation that follows you describe three pcie* devices, each
with two distinct ports, all 6 of which are part of group 0.

can we have diagrams that match the actual bindings. does the real product
actually have 6 ports in one group?

> +    pcie1 {
> +        #address-cells = <3>;
> +        #size-cells = <2>;
> +
> +        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>;
> +
> +                qcom,ath12k-calibration-variant = "RDP433_1";
> +
> +                wsi {
> +                    qcom,wsi-group-id = <0>;
> +
> +                    ports {
> +                        #address-cells = <1>;
> +                        #size-cells = <0>;
> +
> +                        port@0 {
> +                            reg = <0>;
> +
> +                            wifi1_wsi_tx: endpoint {
> +                                remote-endpoint = <&wifi2_wsi_rx>;
> +                            };
> +                        };
> +
> +                        port@1 {
> +                            reg = <1>;
> +
> +                            wifi1_wsi_rx: endpoint {
> +                                remote-endpoint = <&wifi3_wsi_tx>;
> +                            };
> +                        };
> +                    };
> +                };
> +            };
> +        };
> +    };
> +
> +    pcie2 {
> +        #address-cells = <3>;
> +        #size-cells = <2>;
> +
> +        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>;
> +
> +                qcom,ath12k-calibration-variant = "RDP433_2";
> +
> +                wsi {
> +                    qcom,wsi-group-id = <0>;
> +
> +                    ports {
> +                        #address-cells = <1>;
> +                        #size-cells = <0>;
> +
> +                        port@0 {
> +                            reg = <0>;
> +
> +                            wifi2_wsi_tx: endpoint {
> +                                remote-endpoint = <&wifi3_wsi_rx>;
> +                            };
> +                        };
> +
> +                        port@1 {
> +                            reg = <1>;
> +
> +                            wifi2_wsi_rx: endpoint {
> +                                remote-endpoint = <&wifi1_wsi_tx>;
> +                            };
> +                        };
> +                    };
> +                };
> +            };
> +        };
> +    };
> +
> +    pcie3 {
> +        #address-cells = <3>;
> +        #size-cells = <2>;
> +
> +        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>;
> +
> +                qcom,ath12k-calibration-variant = "RDP433_3";
> +
> +                wsi {
> +                    qcom,wsi-group-id = <0>;
> +
> +                    ports {
> +                        #address-cells = <1>;
> +                        #size-cells = <0>;
> +
> +                        port@0 {
> +                            reg = <0>;
> +
> +                            wifi3_wsi_tx: endpoint {
> +                                remote-endpoint = <&wifi1_wsi_rx>;
> +                            };
> +                        };
> +
> +                        port@1 {
> +                            reg = <1>;
> +
> +                            wifi3_wsi_rx: endpoint {
> +                                remote-endpoint = <&wifi2_wsi_tx>;
> +                            };
> +                        };
> +                    };
> +                };
> +            };
> +        };
> +    };

Re: [RFC PATCH v2 1/5] dt-bindings: net: wireless: ath12k: describe WSI properties for QCN9274

[Jeff Johnson]

On 10/30/2024 12:04 PM, Jeff Johnson wrote:
> in the description above you have two different diagrams:
> - one that shows 3 pcie* devices in a single group with apparently one port
> per device
> - one that shows 4 pcie* devices split into two groups of two, again with
> apparently one port per device
> 
> but in the representation that follows you describe three pcie* devices, each
> with two distinct ports, all 6 of which are part of group 0.
> 
> can we have diagrams that match the actual bindings. does the real product
> actually have 6 ports in one group?

After stepping away and then coming back and reading the dts change I now
understand that each device has two ports, a tx and an rx port.

/jeff

Re: [RFC PATCH v2 1/5] dt-bindings: net: wireless: ath12k: describe WSI properties for QCN9274

[Raj Kumar Bhagat]

On 10/29/2024 11:22 PM, Krzysztof Kozlowski wrote:
> On 29/10/2024 18:30, 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-master: Indicates if this device is the WSI master.
>>
>>  - ports: This is a graph ports schema that has two ports: TX (port@0)
>>    and RX (port@1). This represents the actual WSI connection among
>>    multiple devices.
> 
> Describe the hardware, not the contents of the patch/binding. We see it
> easily, but what we do not see is the hardware.
> 

sure will update the commit log.

>>
>> Also, describe the ath12k device property
>> "qcom,ath12k-calibration-variant". This is a common property among
>> ath12k devices.
> 
> Why do you describe it? What you do is easily visible. We do not see why.
> 

will remove this description in next version.

>>
>> Signed-off-by: Raj Kumar Bhagat <quic_rajkbhag@quicinc.com>
>> ---
>>  .../bindings/net/wireless/qcom,ath12k.yaml    | 241 +++++++++++++++++-
>>  1 file changed, 232 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..42bcd73dd159 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#
>> @@ -18,10 +19,17 @@ properties:
>>    compatible:
>>      enum:
>>        - pci17cb,1107  # WCN7850
>> +      - pci17cb,1109  # QCN9274
> 
> I asked for separate binding because it is quite a different device.
> Unless it is not... but then commit msg is quite not precise here.
> 

sure, will create a separate binding, may be "qcom,ath12k_wsi.yaml".
This will be for ath21k PCI device with WSI interface.

>>  
>>    reg:
>>      maxItems: 1
>>  
>> +  qcom,ath12k-calibration-variant:
>> +    $ref: /schemas/types.yaml#/definitions/string
>> +    description: |
> 
> Do not need '|' unless you need to preserve formatting.
> 

thanks will remove "|"

>> +      string to uniquely identify variant of the calibration data for designs
>> +      with colliding bus and device ids
>> +
>>    vddaon-supply:
>>      description: VDD_AON supply regulator handle
>>  
>> @@ -49,21 +57,100 @@ properties:
>>    vddpcie1p8-supply:
>>      description: VDD_PCIE_1P8 supply regulator handle
>>  
>> +  wsi:
> 
> Not much improved here. I asked to drop the node.
> 

In next version will remove "wsi". The properties under wsi (ports,
qcom,wsi-master, etc) will be directly under ath12k device node.

>> +    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.
>> +
>> +      Diagram to represent one WSI connection (one WSI group) among
>> +      three devices.
>> +
>> +               +-------+        +-------+        +-------+
>> +               | pcie2 |        | pcie3 |        | pcie1 |
>> +               |       |        |       |        |       |
>> +        +----->|  wsi  |------->|  wsi  |------->|  wsi  |-----+
>> +        |      | grp 0 |        | grp 0 |        | grp 2 |     |
>> +        |      +-------+        +-------+        +-------+     |
>> +        +------------------------------------------------------+
>> +
>> +      Diagram to represent two WSI connections (two separate WSI groups)
>> +      among four devices.
>> +
>> +           +-------+    +-------+          +-------+    +-------+
>> +           | pcie2 |    | pcie3 |          | pcie1 |    | pcie0 |
>> +           |       |    |       |          |       |    |       |
>> +       +-->|  wsi  |--->|  wsi  |--+   +-->|  wsi  |--->|  wsi  |--+
>> +       |   | grp 0 |    | grp 0 |  |   |   | grp 1 |    | grp 1 |  |
>> +       |   +-------+    +-------+  |   |   +-------+    +-------+  |
>> +       +---------------------------+   +---------------------------+
>> +
>> +    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.
> 
> That's not needed according to description. Entire group is defined by
> graph.
> 

So this mean "qcom,wsi-group-id" to be dropped and we can assign the
group ID (in ath12k driver implementation) by using the graph?

>> +
>> +      qcom,wsi-master:
>> +        type: boolean
>> +        description:
>> +          Indicates if this device is the WSI master.
>> +
> 
> This copies property name. Why being master is important?
> 

The master device in the WSI group aids (is capable) to synchronize the Timing
Synchronization Function (TSF) clock across all devices in the group. Will include
this information in next version.

> Also, use some different name: see preferred names in kernel coding style.
> 

Thanks for pointing out, will use "qcom,wsi-controller"

>> +      ports:
>> +        $ref: /schemas/graph.yaml#/properties/ports
>> +        description:
>> +          These ports are used to connect multiple WSI supported devices to
>> +          form the WSI group.
>> +
>> +        properties:
>> +          port@0:
>> +            $ref: /schemas/graph.yaml#/properties/port
>> +            description:
>> +              This is the TX port of WSI interface. It is attached to the RX
>> +              port of the next device in the WSI connection.
>> +
>> +          port@1:
>> +            $ref: /schemas/graph.yaml#/properties/port
>> +            description:
>> +              This is the RX port of WSI interface. It is attached to the TX
>> +              port of the previous device in the WSI connection.
>> +
>> +    required:
>> +      - qcom,wsi-group-id
>> +      - ports
>> +
>> +    additionalProperties: false
>> +
>>  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
> 
> Commit says WSI applies only to new variant, so properties should be
> disallowed... or just follow my feedback last time: separate binding.
> 

Sure, we will have separate binding in next version.

>> +
>>  examples:
>>    - |
>>      #include <dt-bindings/clock/qcom,rpmh.h>
>> @@ -97,3 +184,139 @@ examples:
>>              };
>>          };
>>      };
>> +
>> +  - |
>> +    pcie1 {
> 
> pcie {
> and keep all nodes here
> 

sure

>> +        #address-cells = <3>;
>> +        #size-cells = <2>;
>> +
>> +        pcie@0 {
>> +            device_type = "pci";
>> +            reg = <0x0 0x0 0x0 0x0 0x0>;
>> +            #address-cells = <3>;
>> +            #size-cells = <2>;
>> +            ranges;
>> +
>> +            wifi1@0 {
> 
> wifi@
> 
> Same in other places.
> 

Thanks, will update.

>> +                compatible = "pci17cb,1109";
>> +                reg = <0x0 0x0 0x0 0x0 0x0>;
>> +
>> +                qcom,ath12k-calibration-variant = "RDP433_1";
>> +
>> +                wsi {
> 
> No resources here? Not a bus? You already got comment about it.
> 

sure will remove wsi node and directly define ports and other properties
inside wifi.

Re: [RFC PATCH v2 1/5] dt-bindings: net: wireless: ath12k: describe WSI properties for QCN9274

[Raj Kumar Bhagat]

On 10/31/2024 12:34 AM, Jeff Johnson wrote:
> On 10/29/2024 10:30 AM, 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-master: Indicates if this device is the WSI master.
>>
>>  - ports: This is a graph ports schema that has two ports: TX (port@0)
>>    and RX (port@1). This represents the actual WSI connection among
>>    multiple devices.
>>
>> Also, describe the ath12k device property
>> "qcom,ath12k-calibration-variant". This is a common property among
>> ath12k devices.
>>
>> Signed-off-by: Raj Kumar Bhagat <quic_rajkbhag@quicinc.com>
>> ---
>>  .../bindings/net/wireless/qcom,ath12k.yaml    | 241 +++++++++++++++++-
>>  1 file changed, 232 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..42bcd73dd159 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#
>> @@ -18,10 +19,17 @@ properties:
>>    compatible:
>>      enum:
>>        - pci17cb,1107  # WCN7850
>> +      - pci17cb,1109  # QCN9274
>>  
>>    reg:
>>      maxItems: 1
>>  
>> +  qcom,ath12k-calibration-variant:
>> +    $ref: /schemas/types.yaml#/definitions/string
>> +    description: |
>> +      string to uniquely identify variant of the calibration data for designs
>> +      with colliding bus and device ids
>> +
>>    vddaon-supply:
>>      description: VDD_AON supply regulator handle
>>  
>> @@ -49,21 +57,100 @@ 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.
>> +
>> +      Diagram to represent one WSI connection (one WSI group) among
>> +      three devices.
>> +
>> +               +-------+        +-------+        +-------+
>> +               | pcie2 |        | pcie3 |        | pcie1 |
> is there a reason to not have these in some order?
> 

This could be made in same order. In next version will update.
But in actual hardware the pcie and wsi connection may not be same order.

>> +               |       |        |       |        |       |
>> +        +----->|  wsi  |------->|  wsi  |------->|  wsi  |-----+
>> +        |      | grp 0 |        | grp 0 |        | grp 2 |     |
> s/grp 2/grp 0/???                                          ^ typo?
> 

Thanks for pointing out, this is a typo. will update in next version.
>> +        |      +-------+        +-------+        +-------+     |
>> +        +------------------------------------------------------+
>> +
>> +      Diagram to represent two WSI connections (two separate WSI groups)
>> +      among four devices.
>> +
>> +           +-------+    +-------+          +-------+    +-------+
>> +           | pcie2 |    | pcie3 |          | pcie1 |    | pcie0 |
> again seems strange to not have any logical (to me) order

Will keep this example diagram in pcie order in next version.

Re: [RFC PATCH v2 1/5] dt-bindings: net: wireless: ath12k: describe WSI properties for QCN9274

[Krzysztof Kozlowski]

On 04/11/2024 19:44, Raj Kumar Bhagat wrote:
>>>  $id: http://devicetree.org/schemas/net/wireless/qcom,ath12k.yaml#
>>> @@ -18,10 +19,17 @@ properties:
>>>    compatible:
>>>      enum:
>>>        - pci17cb,1107  # WCN7850
>>> +      - pci17cb,1109  # QCN9274
>>
>> I asked for separate binding because it is quite a different device.
>> Unless it is not... but then commit msg is quite not precise here.
>>
> 
> sure, will create a separate binding, may be "qcom,ath12k_wsi.yaml".

Underscores are not allowed in compatibles and the file name follows
compatible name, so use hyphen.
>>> +    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.
>>> +
>>> +      Diagram to represent one WSI connection (one WSI group) among
>>> +      three devices.
>>> +
>>> +               +-------+        +-------+        +-------+
>>> +               | pcie2 |        | pcie3 |        | pcie1 |
>>> +               |       |        |       |        |       |
>>> +        +----->|  wsi  |------->|  wsi  |------->|  wsi  |-----+
>>> +        |      | grp 0 |        | grp 0 |        | grp 2 |     |
>>> +        |      +-------+        +-------+        +-------+     |
>>> +        +------------------------------------------------------+
>>> +
>>> +      Diagram to represent two WSI connections (two separate WSI groups)
>>> +      among four devices.
>>> +
>>> +           +-------+    +-------+          +-------+    +-------+
>>> +           | pcie2 |    | pcie3 |          | pcie1 |    | pcie0 |
>>> +           |       |    |       |          |       |    |       |
>>> +       +-->|  wsi  |--->|  wsi  |--+   +-->|  wsi  |--->|  wsi  |--+
>>> +       |   | grp 0 |    | grp 0 |  |   |   | grp 1 |    | grp 1 |  |
>>> +       |   +-------+    +-------+  |   |   +-------+    +-------+  |
>>> +       +---------------------------+   +---------------------------+
>>> +
>>> +    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.
>>
>> That's not needed according to description. Entire group is defined by
>> graph.
>>
> 
> So this mean "qcom,wsi-group-id" to be dropped and we can assign the
> group ID (in ath12k driver implementation) by using the graph?

Yes

> 
Best regards,
Krzysztof