public inbox for devicetree@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/3] Enable QoS configuration on QCS8300
@ 2025-11-28 15:01 Odelu Kukatla
  2025-11-28 15:01 ` [PATCH 1/3] dt-bindings: interconnect: add clocks property to enable QoS on qcs8300 Odelu Kukatla
                   ` (2 more replies)
  0 siblings, 3 replies; 20+ messages in thread
From: Odelu Kukatla @ 2025-11-28 15:01 UTC (permalink / raw)
  To: Georgi Djakov, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Bjorn Andersson, Konrad Dybcio
  Cc: Raviteja Laggyshetty, Odelu Kukatla, Dmitry Baryshkov,
	Bartosz Golaszewski, linux-arm-msm, linux-pm, devicetree,
	linux-kernel, Mike Tipton

This series enables QoS configuration for QNOC type device which
can be found on QCS8300 platform. It enables QoS configuration
for master ports with predefined priority and urgency forwarding.
This helps in prioritizing the traffic originating from different
interconnect masters at NOC (Network On Chip). 
The system may function normally without this feature. However,
enabling QoS helps optimize latency and bandwidth across subsystems
like CPU, GPU, and multimedia engines, which becomes important in
high-throughput scenarios. This is a feature aimed at performance
enhancement to improve system performance under concurrent workloads.

Odelu Kukatla (3):
  dt-bindings: interconnect: add clocks property to enable QoS on
    qcs8300
  interconnect: qcom: qcs8300: enable QoS configuration
  arm64: dts: qcom: qcs8300: Add clocks for QoS configuration

 .../interconnect/qcom,qcs8300-rpmh.yaml       |  53 ++-
 arch/arm64/boot/dts/qcom/monaco.dtsi          |   6 +
 drivers/interconnect/qcom/qcs8300.c           | 375 ++++++++++++++++++
 3 files changed, 428 insertions(+), 6 deletions(-)

-- 
2.17.1


^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH 1/3] dt-bindings: interconnect: add clocks property to enable QoS on qcs8300
  2025-11-28 15:01 [PATCH 0/3] Enable QoS configuration on QCS8300 Odelu Kukatla
@ 2025-11-28 15:01 ` Odelu Kukatla
  2025-11-28 16:19   ` Rob Herring (Arm)
                     ` (3 more replies)
  2025-11-28 15:01 ` [PATCH 2/3] interconnect: qcom: qcs8300: enable QoS configuration Odelu Kukatla
  2025-11-28 15:01 ` [PATCH 3/3] arm64: dts: qcom: qcs8300: Add clocks for " Odelu Kukatla
  2 siblings, 4 replies; 20+ messages in thread
From: Odelu Kukatla @ 2025-11-28 15:01 UTC (permalink / raw)
  To: Georgi Djakov, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Bjorn Andersson, Konrad Dybcio
  Cc: Raviteja Laggyshetty, Odelu Kukatla, Dmitry Baryshkov,
	Bartosz Golaszewski, linux-arm-msm, linux-pm, devicetree,
	linux-kernel, Mike Tipton

Add 'clocks' property to enable QoS configuration. This property
enables the necessary clocks for QoS configuration.

QoS configuration is essential for ensuring that latency sensitive
components such as CPUs and multimedia engines receive prioritized
access to memory and interconnect resources. This helps to manage
bandwidth and latency across subsystems, improving system responsiveness
and performance in concurrent workloads.

Both 'reg' and 'clocks' properties are optional. If either is missing,
QoS configuration will be skipped. This behavior is controlled by the
'qos_requires_clocks' flag in the driver, which ensures that QoS
configuration is bypassed when required clocks are not defined.

Signed-off-by: Odelu Kukatla <odelu.kukatla@oss.qualcomm.com>
---
 .../interconnect/qcom,qcs8300-rpmh.yaml       | 53 ++++++++++++++++---
 1 file changed, 47 insertions(+), 6 deletions(-)

diff --git a/Documentation/devicetree/bindings/interconnect/qcom,qcs8300-rpmh.yaml b/Documentation/devicetree/bindings/interconnect/qcom,qcs8300-rpmh.yaml
index e9f528d6d9a8..594e835d1845 100644
--- a/Documentation/devicetree/bindings/interconnect/qcom,qcs8300-rpmh.yaml
+++ b/Documentation/devicetree/bindings/interconnect/qcom,qcs8300-rpmh.yaml
@@ -35,6 +35,10 @@ properties:
   reg:
     maxItems: 1
 
+  clocks:
+    minItems: 1
+    maxItems: 4
+
 required:
   - compatible
 
@@ -45,14 +49,39 @@ allOf:
         compatible:
           contains:
             enum:
-              - qcom,qcs8300-clk-virt
-              - qcom,qcs8300-mc-virt
+              - qcom,qcs8300-aggre1-noc
     then:
       properties:
-        reg: false
-    else:
-      required:
-        - reg
+        clocks:
+          items:
+            - description: aggre UFS PHY AXI clock
+            - description: aggre QUP PRIM AXI clock
+            - description: aggre USB2 PRIM AXI clock
+            - description: aggre USB3 PRIM AXI clock
+
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - qcom,qcs8300-aggre2-noc
+    then:
+      properties:
+        clocks:
+          items:
+            - description: RPMH CC IPA clock
+
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - qcom,qcs8300-gem-noc
+    then:
+      properties:
+        clocks:
+          items:
+            - description: GCC DDRSS GPU AXI clock
 
 unevaluatedProperties: false
 
@@ -63,6 +92,7 @@ examples:
         reg = <0x9100000 0xf7080>;
         #interconnect-cells = <2>;
         qcom,bcm-voters = <&apps_bcm_voter>;
+        clocks = <&gcc_ddrss_gpu_axi_clk>;
     };
 
     clk_virt: interconnect-0 {
@@ -70,3 +100,14 @@ examples:
         #interconnect-cells = <2>;
         qcom,bcm-voters = <&apps_bcm_voter>;
     };
+
+    aggre1_noc: interconnect@16c0000 {
+        compatible = "qcom,qcs8300-aggre1-noc";
+        reg = <0x016c0000 0x17080>;
+        #interconnect-cells = <2>;
+        qcom,bcm-voters = <&apps_bcm_voter>;
+        clocks = <&gcc_aggre_ufs_phy_axi_clk>,
+                 <&gcc_aggre_noc_qupv3_axi_clk>,
+                 <&gcc_aggre_usb2_prim_axi_clk>,
+                 <&gcc_aggre_usb3_prim_axi_clk>;
+    };
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH 2/3] interconnect: qcom: qcs8300: enable QoS configuration
  2025-11-28 15:01 [PATCH 0/3] Enable QoS configuration on QCS8300 Odelu Kukatla
  2025-11-28 15:01 ` [PATCH 1/3] dt-bindings: interconnect: add clocks property to enable QoS on qcs8300 Odelu Kukatla
@ 2025-11-28 15:01 ` Odelu Kukatla
  2025-11-29  1:20   ` Dmitry Baryshkov
  2025-12-01 15:06   ` Konrad Dybcio
  2025-11-28 15:01 ` [PATCH 3/3] arm64: dts: qcom: qcs8300: Add clocks for " Odelu Kukatla
  2 siblings, 2 replies; 20+ messages in thread
From: Odelu Kukatla @ 2025-11-28 15:01 UTC (permalink / raw)
  To: Georgi Djakov, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Bjorn Andersson, Konrad Dybcio
  Cc: Raviteja Laggyshetty, Odelu Kukatla, Dmitry Baryshkov,
	Bartosz Golaszewski, linux-arm-msm, linux-pm, devicetree,
	linux-kernel, Mike Tipton

Enable QoS configuration for master ports with predefined priority
and urgency forwarding.

Signed-off-by: Odelu Kukatla <odelu.kukatla@oss.qualcomm.com>
---
 drivers/interconnect/qcom/qcs8300.c | 375 ++++++++++++++++++++++++++++
 1 file changed, 375 insertions(+)

diff --git a/drivers/interconnect/qcom/qcs8300.c b/drivers/interconnect/qcom/qcs8300.c
index 70a377bbcf29..3f4fe62148d3 100644
--- a/drivers/interconnect/qcom/qcs8300.c
+++ b/drivers/interconnect/qcom/qcs8300.c
@@ -186,6 +186,13 @@ static struct qcom_icc_node qxm_qup3 = {
 	.name = "qxm_qup3",
 	.channels = 1,
 	.buswidth = 8,
+	.qosbox = &(const struct qcom_icc_qosbox) {
+		.num_ports = 1,
+		.port_offsets = { 0x11000 },
+		.prio_fwd_disable = 1,
+		.prio = 2,
+		.urg_fwd = 0,
+	},
 	.num_links = 1,
 	.link_nodes = { &qns_a1noc_snoc },
 };
@@ -194,6 +201,13 @@ static struct qcom_icc_node xm_emac_0 = {
 	.name = "xm_emac_0",
 	.channels = 1,
 	.buswidth = 8,
+	.qosbox = &(const struct qcom_icc_qosbox) {
+		.num_ports = 1,
+		.port_offsets = { 0x12000 },
+		.prio_fwd_disable = 1,
+		.prio = 2,
+		.urg_fwd = 0,
+	},
 	.num_links = 1,
 	.link_nodes = { &qns_a1noc_snoc },
 };
@@ -202,6 +216,13 @@ static struct qcom_icc_node xm_sdc1 = {
 	.name = "xm_sdc1",
 	.channels = 1,
 	.buswidth = 8,
+	.qosbox = &(const struct qcom_icc_qosbox) {
+		.num_ports = 1,
+		.port_offsets = { 0x14000 },
+		.prio_fwd_disable = 1,
+		.prio = 2,
+		.urg_fwd = 0,
+	},
 	.num_links = 1,
 	.link_nodes = { &qns_a1noc_snoc },
 };
@@ -210,6 +231,13 @@ static struct qcom_icc_node xm_ufs_mem = {
 	.name = "xm_ufs_mem",
 	.channels = 1,
 	.buswidth = 8,
+	.qosbox = &(const struct qcom_icc_qosbox) {
+		.num_ports = 1,
+		.port_offsets = { 0x15000 },
+		.prio_fwd_disable = 1,
+		.prio = 2,
+		.urg_fwd = 0,
+	},
 	.num_links = 1,
 	.link_nodes = { &qns_a1noc_snoc },
 };
@@ -218,6 +246,13 @@ static struct qcom_icc_node xm_usb2_2 = {
 	.name = "xm_usb2_2",
 	.channels = 1,
 	.buswidth = 8,
+	.qosbox = &(const struct qcom_icc_qosbox) {
+		.num_ports = 1,
+		.port_offsets = { 0x16000 },
+		.prio_fwd_disable = 1,
+		.prio = 2,
+		.urg_fwd = 0,
+	},
 	.num_links = 1,
 	.link_nodes = { &qns_a1noc_snoc },
 };
@@ -226,6 +261,13 @@ static struct qcom_icc_node xm_usb3_0 = {
 	.name = "xm_usb3_0",
 	.channels = 1,
 	.buswidth = 8,
+	.qosbox = &(const struct qcom_icc_qosbox) {
+		.num_ports = 1,
+		.port_offsets = { 0x17000 },
+		.prio_fwd_disable = 1,
+		.prio = 2,
+		.urg_fwd = 0,
+	},
 	.num_links = 1,
 	.link_nodes = { &qns_a1noc_snoc },
 };
@@ -234,6 +276,13 @@ static struct qcom_icc_node qhm_qdss_bam = {
 	.name = "qhm_qdss_bam",
 	.channels = 1,
 	.buswidth = 4,
+	.qosbox = &(const struct qcom_icc_qosbox) {
+		.num_ports = 1,
+		.port_offsets = { 0x14000 },
+		.prio_fwd_disable = 1,
+		.prio = 2,
+		.urg_fwd = 0,
+	},
 	.num_links = 1,
 	.link_nodes = { &qns_a2noc_snoc },
 };
@@ -242,6 +291,13 @@ static struct qcom_icc_node qhm_qup0 = {
 	.name = "qhm_qup0",
 	.channels = 1,
 	.buswidth = 4,
+	.qosbox = &(const struct qcom_icc_qosbox) {
+		.num_ports = 1,
+		.port_offsets = { 0x17000 },
+		.prio_fwd_disable = 1,
+		.prio = 2,
+		.urg_fwd = 0,
+	},
 	.num_links = 1,
 	.link_nodes = { &qns_a2noc_snoc },
 };
@@ -250,6 +306,13 @@ static struct qcom_icc_node qhm_qup1 = {
 	.name = "qhm_qup1",
 	.channels = 1,
 	.buswidth = 4,
+	.qosbox = &(const struct qcom_icc_qosbox) {
+		.num_ports = 1,
+		.port_offsets = { 0x12000 },
+		.prio_fwd_disable = 1,
+		.prio = 2,
+		.urg_fwd = 0,
+	},
 	.num_links = 1,
 	.link_nodes = { &qns_a2noc_snoc },
 };
@@ -258,6 +321,13 @@ static struct qcom_icc_node qnm_cnoc_datapath = {
 	.name = "qnm_cnoc_datapath",
 	.channels = 1,
 	.buswidth = 8,
+	.qosbox = &(const struct qcom_icc_qosbox) {
+		.num_ports = 1,
+		.port_offsets = { 0x16000 },
+		.prio_fwd_disable = 1,
+		.prio = 2,
+		.urg_fwd = 0,
+	},
 	.num_links = 1,
 	.link_nodes = { &qns_a2noc_snoc },
 };
@@ -266,6 +336,13 @@ static struct qcom_icc_node qxm_crypto_0 = {
 	.name = "qxm_crypto_0",
 	.channels = 1,
 	.buswidth = 8,
+	.qosbox = &(const struct qcom_icc_qosbox) {
+		.num_ports = 1,
+		.port_offsets = { 0x18000 },
+		.prio_fwd_disable = 1,
+		.prio = 2,
+		.urg_fwd = 0,
+	},
 	.num_links = 1,
 	.link_nodes = { &qns_a2noc_snoc },
 };
@@ -274,6 +351,13 @@ static struct qcom_icc_node qxm_crypto_1 = {
 	.name = "qxm_crypto_1",
 	.channels = 1,
 	.buswidth = 8,
+	.qosbox = &(const struct qcom_icc_qosbox) {
+		.num_ports = 1,
+		.port_offsets = { 0x1a000 },
+		.prio_fwd_disable = 1,
+		.prio = 2,
+		.urg_fwd = 0,
+	},
 	.num_links = 1,
 	.link_nodes = { &qns_a2noc_snoc },
 };
@@ -282,6 +366,13 @@ static struct qcom_icc_node qxm_ipa = {
 	.name = "qxm_ipa",
 	.channels = 1,
 	.buswidth = 8,
+	.qosbox = &(const struct qcom_icc_qosbox) {
+		.num_ports = 1,
+		.port_offsets = { 0x11000 },
+		.prio_fwd_disable = 1,
+		.prio = 2,
+		.urg_fwd = 0,
+	},
 	.num_links = 1,
 	.link_nodes = { &qns_a2noc_snoc },
 };
@@ -290,6 +381,13 @@ static struct qcom_icc_node xm_qdss_etr_0 = {
 	.name = "xm_qdss_etr_0",
 	.channels = 1,
 	.buswidth = 8,
+	.qosbox = &(const struct qcom_icc_qosbox) {
+		.num_ports = 1,
+		.port_offsets = { 0x13000 },
+		.prio_fwd_disable = 1,
+		.prio = 2,
+		.urg_fwd = 0,
+	},
 	.num_links = 1,
 	.link_nodes = { &qns_a2noc_snoc },
 };
@@ -298,6 +396,13 @@ static struct qcom_icc_node xm_qdss_etr_1 = {
 	.name = "xm_qdss_etr_1",
 	.channels = 1,
 	.buswidth = 8,
+	.qosbox = &(const struct qcom_icc_qosbox) {
+		.num_ports = 1,
+		.port_offsets = { 0x19000 },
+		.prio_fwd_disable = 1,
+		.prio = 2,
+		.urg_fwd = 0,
+	},
 	.num_links = 1,
 	.link_nodes = { &qns_a2noc_snoc },
 };
@@ -390,6 +495,13 @@ static struct qcom_icc_node alm_gpu_tcu = {
 	.name = "alm_gpu_tcu",
 	.channels = 1,
 	.buswidth = 8,
+	.qosbox = &(const struct qcom_icc_qosbox) {
+		.num_ports = 1,
+		.port_offsets = { 0xaf000 },
+		.prio_fwd_disable = 1,
+		.prio = 1,
+		.urg_fwd = 0,
+	},
 	.num_links = 2,
 	.link_nodes = { &qns_gem_noc_cnoc, &qns_llcc },
 };
@@ -398,6 +510,13 @@ static struct qcom_icc_node alm_pcie_tcu = {
 	.name = "alm_pcie_tcu",
 	.channels = 1,
 	.buswidth = 8,
+	.qosbox = &(const struct qcom_icc_qosbox) {
+		.num_ports = 1,
+		.port_offsets = { 0xb0000 },
+		.prio_fwd_disable = 1,
+		.prio = 3,
+		.urg_fwd = 0,
+	},
 	.num_links = 2,
 	.link_nodes = { &qns_gem_noc_cnoc, &qns_llcc },
 };
@@ -406,6 +525,13 @@ static struct qcom_icc_node alm_sys_tcu = {
 	.name = "alm_sys_tcu",
 	.channels = 1,
 	.buswidth = 8,
+	.qosbox = &(const struct qcom_icc_qosbox) {
+		.num_ports = 1,
+		.port_offsets = { 0xb1000 },
+		.prio_fwd_disable = 1,
+		.prio = 6,
+		.urg_fwd = 0,
+	},
 	.num_links = 2,
 	.link_nodes = { &qns_gem_noc_cnoc, &qns_llcc },
 };
@@ -423,6 +549,13 @@ static struct qcom_icc_node qnm_cmpnoc0 = {
 	.name = "qnm_cmpnoc0",
 	.channels = 2,
 	.buswidth = 32,
+	.qosbox = &(const struct qcom_icc_qosbox) {
+		.num_ports = 2,
+		.port_offsets = { 0xf6000, 0xf7000 },
+		.prio_fwd_disable = 1,
+		.prio = 0,
+		.urg_fwd = 0,
+	},
 	.num_links = 2,
 	.link_nodes = { &qns_gem_noc_cnoc, &qns_llcc },
 };
@@ -448,6 +581,13 @@ static struct qcom_icc_node qnm_gpu = {
 	.name = "qnm_gpu",
 	.channels = 2,
 	.buswidth = 32,
+	.qosbox = &(const struct qcom_icc_qosbox) {
+		.num_ports = 2,
+		.port_offsets = { 0xf0000, 0xf1000 },
+		.prio_fwd_disable = 1,
+		.prio = 0,
+		.urg_fwd = 0,
+	},
 	.num_links = 2,
 	.link_nodes = { &qns_gem_noc_cnoc, &qns_llcc },
 };
@@ -456,6 +596,13 @@ static struct qcom_icc_node qnm_mnoc_hf = {
 	.name = "qnm_mnoc_hf",
 	.channels = 2,
 	.buswidth = 32,
+	.qosbox = &(const struct qcom_icc_qosbox) {
+		.num_ports = 2,
+		.port_offsets = { 0xf2000, 0xf3000 },
+		.prio_fwd_disable = 0,
+		.prio = 0,
+		.urg_fwd = 1,
+	},
 	.num_links = 2,
 	.link_nodes = { &qns_llcc, &qns_pcie },
 };
@@ -464,6 +611,13 @@ static struct qcom_icc_node qnm_mnoc_sf = {
 	.name = "qnm_mnoc_sf",
 	.channels = 2,
 	.buswidth = 32,
+	.qosbox = &(const struct qcom_icc_qosbox) {
+		.num_ports = 2,
+		.port_offsets = { 0xf4000, 0xf5000 },
+		.prio_fwd_disable = 0,
+		.prio = 0,
+		.urg_fwd = 1,
+	},
 	.num_links = 3,
 	.link_nodes = { &qns_gem_noc_cnoc, &qns_llcc,
 			&qns_pcie },
@@ -473,6 +627,13 @@ static struct qcom_icc_node qnm_pcie = {
 	.name = "qnm_pcie",
 	.channels = 1,
 	.buswidth = 32,
+	.qosbox = &(const struct qcom_icc_qosbox) {
+		.num_ports = 1,
+		.port_offsets = { 0xb3000 },
+		.prio_fwd_disable = 1,
+		.prio = 2,
+		.urg_fwd = 0,
+	},
 	.num_links = 2,
 	.link_nodes = { &qns_gem_noc_cnoc, &qns_llcc },
 };
@@ -481,6 +642,13 @@ static struct qcom_icc_node qnm_snoc_gc = {
 	.name = "qnm_snoc_gc",
 	.channels = 1,
 	.buswidth = 8,
+	.qosbox = &(const struct qcom_icc_qosbox) {
+		.num_ports = 1,
+		.port_offsets = { 0xb4000 },
+		.prio_fwd_disable = 0,
+		.prio = 0,
+		.urg_fwd = 1,
+	},
 	.num_links = 1,
 	.link_nodes = { &qns_llcc },
 };
@@ -489,6 +657,13 @@ static struct qcom_icc_node qnm_snoc_sf = {
 	.name = "qnm_snoc_sf",
 	.channels = 1,
 	.buswidth = 16,
+	.qosbox = &(const struct qcom_icc_qosbox) {
+		.num_ports = 1,
+		.port_offsets = { 0xb5000 },
+		.prio_fwd_disable = 0,
+		.prio = 0,
+		.urg_fwd = 1,
+	},
 	.num_links = 3,
 	.link_nodes = { &qns_gem_noc_cnoc, &qns_llcc,
 			&qns_pcie },
@@ -541,6 +716,13 @@ static struct qcom_icc_node qnm_camnoc_hf = {
 	.name = "qnm_camnoc_hf",
 	.channels = 1,
 	.buswidth = 32,
+	.qosbox = &(const struct qcom_icc_qosbox) {
+		.num_ports = 1,
+		.port_offsets = { 0xa000 },
+		.prio_fwd_disable = 0,
+		.prio = 0,
+		.urg_fwd = 1,
+	},
 	.num_links = 1,
 	.link_nodes = { &qns_mem_noc_hf },
 };
@@ -549,6 +731,13 @@ static struct qcom_icc_node qnm_camnoc_icp = {
 	.name = "qnm_camnoc_icp",
 	.channels = 1,
 	.buswidth = 8,
+	.qosbox = &(const struct qcom_icc_qosbox) {
+		.num_ports = 1,
+		.port_offsets = { 0x2a000 },
+		.prio_fwd_disable = 0,
+		.prio = 0,
+		.urg_fwd = 1,
+	},
 	.num_links = 1,
 	.link_nodes = { &qns_mem_noc_sf },
 };
@@ -557,6 +746,13 @@ static struct qcom_icc_node qnm_camnoc_sf = {
 	.name = "qnm_camnoc_sf",
 	.channels = 1,
 	.buswidth = 32,
+	.qosbox = &(const struct qcom_icc_qosbox) {
+		.num_ports = 1,
+		.port_offsets = { 0x2a080 },
+		.prio_fwd_disable = 0,
+		.prio = 0,
+		.urg_fwd = 1,
+	},
 	.num_links = 1,
 	.link_nodes = { &qns_mem_noc_sf },
 };
@@ -565,6 +761,13 @@ static struct qcom_icc_node qnm_mdp0_0 = {
 	.name = "qnm_mdp0_0",
 	.channels = 1,
 	.buswidth = 32,
+	.qosbox = &(const struct qcom_icc_qosbox) {
+		.num_ports = 1,
+		.port_offsets = { 0xa080 },
+		.prio_fwd_disable = 0,
+		.prio = 0,
+		.urg_fwd = 1,
+	},
 	.num_links = 1,
 	.link_nodes = { &qns_mem_noc_hf },
 };
@@ -573,6 +776,13 @@ static struct qcom_icc_node qnm_mdp0_1 = {
 	.name = "qnm_mdp0_1",
 	.channels = 1,
 	.buswidth = 32,
+	.qosbox = &(const struct qcom_icc_qosbox) {
+		.num_ports = 1,
+		.port_offsets = { 0xa180 },
+		.prio_fwd_disable = 0,
+		.prio = 0,
+		.urg_fwd = 1,
+	},
 	.num_links = 1,
 	.link_nodes = { &qns_mem_noc_hf },
 };
@@ -597,6 +807,13 @@ static struct qcom_icc_node qnm_video0 = {
 	.name = "qnm_video0",
 	.channels = 1,
 	.buswidth = 32,
+	.qosbox = &(const struct qcom_icc_qosbox) {
+		.num_ports = 1,
+		.port_offsets = { 0x2a100 },
+		.prio_fwd_disable = 0,
+		.prio = 0,
+		.urg_fwd = 1,
+	},
 	.num_links = 1,
 	.link_nodes = { &qns_mem_noc_sf },
 };
@@ -605,6 +822,13 @@ static struct qcom_icc_node qnm_video_cvp = {
 	.name = "qnm_video_cvp",
 	.channels = 1,
 	.buswidth = 32,
+	.qosbox = &(const struct qcom_icc_qosbox) {
+		.num_ports = 1,
+		.port_offsets = { 0x2a200 },
+		.prio_fwd_disable = 0,
+		.prio = 0,
+		.urg_fwd = 1,
+	},
 	.num_links = 1,
 	.link_nodes = { &qns_mem_noc_sf },
 };
@@ -613,6 +837,13 @@ static struct qcom_icc_node qnm_video_v_cpu = {
 	.name = "qnm_video_v_cpu",
 	.channels = 1,
 	.buswidth = 8,
+	.qosbox = &(const struct qcom_icc_qosbox) {
+		.num_ports = 1,
+		.port_offsets = { 0x2a280 },
+		.prio_fwd_disable = 0,
+		.prio = 0,
+		.urg_fwd = 1,
+	},
 	.num_links = 1,
 	.link_nodes = { &qns_mem_noc_sf },
 };
@@ -637,6 +868,13 @@ static struct qcom_icc_node xm_pcie3_0 = {
 	.name = "xm_pcie3_0",
 	.channels = 1,
 	.buswidth = 16,
+	.qosbox = &(const struct qcom_icc_qosbox) {
+		.num_ports = 1,
+		.port_offsets = { 0xb000 },
+		.prio_fwd_disable = 1,
+		.prio = 2,
+		.urg_fwd = 0,
+	},
 	.num_links = 1,
 	.link_nodes = { &qns_pcie_mem_noc },
 };
@@ -645,6 +883,13 @@ static struct qcom_icc_node xm_pcie3_1 = {
 	.name = "xm_pcie3_1",
 	.channels = 1,
 	.buswidth = 32,
+	.qosbox = &(const struct qcom_icc_qosbox) {
+		.num_ports = 1,
+		.port_offsets = { 0xc000 },
+		.prio_fwd_disable = 1,
+		.prio = 2,
+		.urg_fwd = 0,
+	},
 	.num_links = 1,
 	.link_nodes = { &qns_pcie_mem_noc },
 };
@@ -653,6 +898,13 @@ static struct qcom_icc_node qhm_gic = {
 	.name = "qhm_gic",
 	.channels = 1,
 	.buswidth = 4,
+	.qosbox = &(const struct qcom_icc_qosbox) {
+		.num_ports = 1,
+		.port_offsets = { 0x14000 },
+		.prio_fwd_disable = 1,
+		.prio = 2,
+		.urg_fwd = 0,
+	},
 	.num_links = 1,
 	.link_nodes = { &qns_gemnoc_sf },
 };
@@ -677,6 +929,13 @@ static struct qcom_icc_node qnm_lpass_noc = {
 	.name = "qnm_lpass_noc",
 	.channels = 1,
 	.buswidth = 16,
+	.qosbox = &(const struct qcom_icc_qosbox) {
+		.num_ports = 1,
+		.port_offsets = { 0x12000 },
+		.prio_fwd_disable = 0,
+		.prio = 0,
+		.urg_fwd = 1,
+	},
 	.num_links = 1,
 	.link_nodes = { &qns_gemnoc_sf },
 };
@@ -693,6 +952,13 @@ static struct qcom_icc_node qxm_pimem = {
 	.name = "qxm_pimem",
 	.channels = 1,
 	.buswidth = 8,
+	.qosbox = &(const struct qcom_icc_qosbox) {
+		.num_ports = 1,
+		.port_offsets = { 0x13000 },
+		.prio_fwd_disable = 1,
+		.prio = 2,
+		.urg_fwd = 0,
+	},
 	.num_links = 1,
 	.link_nodes = { &qns_gemnoc_gc },
 };
@@ -701,6 +967,13 @@ static struct qcom_icc_node xm_gic = {
 	.name = "xm_gic",
 	.channels = 1,
 	.buswidth = 8,
+	.qosbox = &(const struct qcom_icc_qosbox) {
+		.num_ports = 1,
+		.port_offsets = { 0x15000 },
+		.prio_fwd_disable = 1,
+		.prio = 2,
+		.urg_fwd = 0,
+	},
 	.num_links = 1,
 	.link_nodes = { &qns_gemnoc_gc },
 };
@@ -1599,11 +1872,21 @@ static struct qcom_icc_node * const aggre1_noc_nodes[] = {
 	[SLAVE_A1NOC_SNOC] = &qns_a1noc_snoc,
 };
 
+static const struct regmap_config qcs8300_aggre1_noc_regmap_config = {
+	.reg_bits = 32,
+	.reg_stride = 4,
+	.val_bits = 32,
+	.max_register = 0x17080,
+	.fast_io = true,
+};
+
 static const struct qcom_icc_desc qcs8300_aggre1_noc = {
+	.config = &qcs8300_aggre1_noc_regmap_config,
 	.nodes = aggre1_noc_nodes,
 	.num_nodes = ARRAY_SIZE(aggre1_noc_nodes),
 	.bcms = aggre1_noc_bcms,
 	.num_bcms = ARRAY_SIZE(aggre1_noc_bcms),
+	.qos_requires_clocks = true,
 };
 
 static struct qcom_icc_bcm * const aggre2_noc_bcms[] = {
@@ -1624,11 +1907,21 @@ static struct qcom_icc_node * const aggre2_noc_nodes[] = {
 	[SLAVE_A2NOC_SNOC] = &qns_a2noc_snoc,
 };
 
+static const struct regmap_config qcs8300_aggre2_noc_regmap_config = {
+	.reg_bits = 32,
+	.reg_stride = 4,
+	.val_bits = 32,
+	.max_register = 0x1a080,
+	.fast_io = true,
+};
+
 static const struct qcom_icc_desc qcs8300_aggre2_noc = {
+	.config = &qcs8300_aggre2_noc_regmap_config,
 	.nodes = aggre2_noc_nodes,
 	.num_nodes = ARRAY_SIZE(aggre2_noc_nodes),
 	.bcms = aggre2_noc_bcms,
 	.num_bcms = ARRAY_SIZE(aggre2_noc_bcms),
+	.qos_requires_clocks = true,
 };
 
 static struct qcom_icc_bcm * const clk_virt_bcms[] = {
@@ -1740,7 +2033,16 @@ static struct qcom_icc_node * const config_noc_nodes[] = {
 	[SLAVE_TCU] = &xs_sys_tcu_cfg,
 };
 
+static const struct regmap_config qcs8300_config_noc_regmap_config = {
+	.reg_bits = 32,
+	.reg_stride = 4,
+	.val_bits = 32,
+	.max_register = 0x13080,
+	.fast_io = true,
+};
+
 static const struct qcom_icc_desc qcs8300_config_noc = {
+	.config = &qcs8300_config_noc_regmap_config,
 	.nodes = config_noc_nodes,
 	.num_nodes = ARRAY_SIZE(config_noc_nodes),
 	.bcms = config_noc_bcms,
@@ -1753,7 +2055,16 @@ static struct qcom_icc_node * const dc_noc_nodes[] = {
 	[SLAVE_GEM_NOC_CFG] = &qns_gemnoc,
 };
 
+static const struct regmap_config qcs8300_dc_noc_regmap_config = {
+	.reg_bits = 32,
+	.reg_stride = 4,
+	.val_bits = 32,
+	.max_register = 0x5080,
+	.fast_io = true,
+};
+
 static const struct qcom_icc_desc qcs8300_dc_noc = {
+	.config = &qcs8300_dc_noc_regmap_config,
 	.nodes = dc_noc_nodes,
 	.num_nodes = ARRAY_SIZE(dc_noc_nodes),
 };
@@ -1786,11 +2097,21 @@ static struct qcom_icc_node * const gem_noc_nodes[] = {
 	[SLAVE_SERVICE_GEM_NOC2] = &srvc_sys_gemnoc_2,
 };
 
+static const struct regmap_config qcs8300_gem_noc_regmap_config = {
+	.reg_bits = 32,
+	.reg_stride = 4,
+	.val_bits = 32,
+	.max_register = 0xf7080,
+	.fast_io = true,
+};
+
 static const struct qcom_icc_desc qcs8300_gem_noc = {
+	.config = &qcs8300_gem_noc_regmap_config,
 	.nodes = gem_noc_nodes,
 	.num_nodes = ARRAY_SIZE(gem_noc_nodes),
 	.bcms = gem_noc_bcms,
 	.num_bcms = ARRAY_SIZE(gem_noc_bcms),
+	.qos_requires_clocks = true,
 };
 
 static struct qcom_icc_bcm * const gpdsp_anoc_bcms[] = {
@@ -1803,7 +2124,16 @@ static struct qcom_icc_node * const gpdsp_anoc_nodes[] = {
 	[SLAVE_GP_DSP_SAIL_NOC] = &qns_gp_dsp_sail_noc,
 };
 
+static const struct regmap_config qcs8300_gpdsp_anoc_regmap_config = {
+	.reg_bits = 32,
+	.reg_stride = 4,
+	.val_bits = 32,
+	.max_register = 0xd080,
+	.fast_io = true,
+};
+
 static const struct qcom_icc_desc qcs8300_gpdsp_anoc = {
+	.config = &qcs8300_gpdsp_anoc_regmap_config,
 	.nodes = gpdsp_anoc_nodes,
 	.num_nodes = ARRAY_SIZE(gpdsp_anoc_nodes),
 	.bcms = gpdsp_anoc_bcms,
@@ -1826,7 +2156,16 @@ static struct qcom_icc_node * const lpass_ag_noc_nodes[] = {
 	[SLAVE_SERVICE_LPASS_AG_NOC] = &srvc_niu_lpass_agnoc,
 };
 
+static const struct regmap_config qcs8300_lpass_ag_noc_regmap_config = {
+	.reg_bits = 32,
+	.reg_stride = 4,
+	.val_bits = 32,
+	.max_register = 0x17200,
+	.fast_io = true,
+};
+
 static const struct qcom_icc_desc qcs8300_lpass_ag_noc = {
+	.config = &qcs8300_lpass_ag_noc_regmap_config,
 	.nodes = lpass_ag_noc_nodes,
 	.num_nodes = ARRAY_SIZE(lpass_ag_noc_nodes),
 	.bcms = lpass_ag_noc_bcms,
@@ -1872,7 +2211,16 @@ static struct qcom_icc_node * const mmss_noc_nodes[] = {
 	[SLAVE_SERVICE_MNOC_SF] = &srvc_mnoc_sf,
 };
 
+static const struct regmap_config qcs8300_mmss_noc_regmap_config = {
+	.reg_bits = 32,
+	.reg_stride = 4,
+	.val_bits = 32,
+	.max_register = 0x40000,
+	.fast_io = true,
+};
+
 static const struct qcom_icc_desc qcs8300_mmss_noc = {
+	.config = &qcs8300_mmss_noc_regmap_config,
 	.nodes = mmss_noc_nodes,
 	.num_nodes = ARRAY_SIZE(mmss_noc_nodes),
 	.bcms = mmss_noc_bcms,
@@ -1892,7 +2240,16 @@ static struct qcom_icc_node * const nspa_noc_nodes[] = {
 	[SLAVE_SERVICE_NSP_NOC] = &service_nsp_noc,
 };
 
+static const struct regmap_config qcs8300_nspa_noc_regmap_config = {
+	.reg_bits = 32,
+	.reg_stride = 4,
+	.val_bits = 32,
+	.max_register = 0x16080,
+	.fast_io = true,
+};
+
 static const struct qcom_icc_desc qcs8300_nspa_noc = {
+	.config = &qcs8300_nspa_noc_regmap_config,
 	.nodes = nspa_noc_nodes,
 	.num_nodes = ARRAY_SIZE(nspa_noc_nodes),
 	.bcms = nspa_noc_bcms,
@@ -1909,7 +2266,16 @@ static struct qcom_icc_node * const pcie_anoc_nodes[] = {
 	[SLAVE_ANOC_PCIE_GEM_NOC] = &qns_pcie_mem_noc,
 };
 
+static const struct regmap_config qcs8300_pcie_anoc_regmap_config = {
+	.reg_bits = 32,
+	.reg_stride = 4,
+	.val_bits = 32,
+	.max_register = 0xc080,
+	.fast_io = true,
+};
+
 static const struct qcom_icc_desc qcs8300_pcie_anoc = {
+	.config = &qcs8300_pcie_anoc_regmap_config,
 	.nodes = pcie_anoc_nodes,
 	.num_nodes = ARRAY_SIZE(pcie_anoc_nodes),
 	.bcms = pcie_anoc_bcms,
@@ -1937,7 +2303,16 @@ static struct qcom_icc_node * const system_noc_nodes[] = {
 	[SLAVE_SERVICE_SNOC] = &srvc_snoc,
 };
 
+static const struct regmap_config qcs8300_system_noc_regmap_config = {
+	.reg_bits = 32,
+	.reg_stride = 4,
+	.val_bits = 32,
+	.max_register = 0x15080,
+	.fast_io = true,
+};
+
 static const struct qcom_icc_desc qcs8300_system_noc = {
+	.config = &qcs8300_system_noc_regmap_config,
 	.nodes = system_noc_nodes,
 	.num_nodes = ARRAY_SIZE(system_noc_nodes),
 	.bcms = system_noc_bcms,
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH 3/3] arm64: dts: qcom: qcs8300: Add clocks for QoS configuration
  2025-11-28 15:01 [PATCH 0/3] Enable QoS configuration on QCS8300 Odelu Kukatla
  2025-11-28 15:01 ` [PATCH 1/3] dt-bindings: interconnect: add clocks property to enable QoS on qcs8300 Odelu Kukatla
  2025-11-28 15:01 ` [PATCH 2/3] interconnect: qcom: qcs8300: enable QoS configuration Odelu Kukatla
@ 2025-11-28 15:01 ` Odelu Kukatla
  2025-11-29  1:16   ` Dmitry Baryshkov
                     ` (2 more replies)
  2 siblings, 3 replies; 20+ messages in thread
From: Odelu Kukatla @ 2025-11-28 15:01 UTC (permalink / raw)
  To: Georgi Djakov, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Bjorn Andersson, Konrad Dybcio
  Cc: Raviteja Laggyshetty, Odelu Kukatla, Dmitry Baryshkov,
	Bartosz Golaszewski, linux-arm-msm, linux-pm, devicetree,
	linux-kernel, Mike Tipton

Add clocks which need to be enabled for configuring QoS on
qcs8300 SoC.

Signed-off-by: Odelu Kukatla <odelu.kukatla@oss.qualcomm.com>
---
 arch/arm64/boot/dts/qcom/monaco.dtsi | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/monaco.dtsi b/arch/arm64/boot/dts/qcom/monaco.dtsi
index 816fa2af8a9a..6139511ea525 100644
--- a/arch/arm64/boot/dts/qcom/monaco.dtsi
+++ b/arch/arm64/boot/dts/qcom/monaco.dtsi
@@ -2226,6 +2226,10 @@
 			reg = <0x0 0x016c0000 0x0 0x17080>;
 			#interconnect-cells = <2>;
 			qcom,bcm-voters = <&apps_bcm_voter>;
+			clocks = <&gcc GCC_AGGRE_UFS_PHY_AXI_CLK>,
+				 <&gcc GCC_AGGRE_NOC_QUPV3_AXI_CLK>,
+				 <&gcc GCC_AGGRE_USB2_PRIM_AXI_CLK>,
+				 <&gcc GCC_AGGRE_USB3_PRIM_AXI_CLK>;
 		};
 
 		aggre2_noc: interconnect@1700000 {
@@ -2233,6 +2237,7 @@
 			reg = <0x0 0x01700000 0x0 0x1a080>;
 			#interconnect-cells = <2>;
 			qcom,bcm-voters = <&apps_bcm_voter>;
+			clocks = <&rpmhcc RPMH_IPA_CLK>;
 		};
 
 		pcie_anoc: interconnect@1760000 {
@@ -4560,6 +4565,7 @@
 			reg = <0x0 0x9100000 0x0 0xf7080>;
 			#interconnect-cells = <2>;
 			qcom,bcm-voters = <&apps_bcm_voter>;
+			clocks = <&gcc GCC_DDRSS_GPU_AXI_CLK>;
 		};
 
 		llcc: system-cache-controller@9200000 {
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* Re: [PATCH 1/3] dt-bindings: interconnect: add clocks property to enable QoS on qcs8300
  2025-11-28 15:01 ` [PATCH 1/3] dt-bindings: interconnect: add clocks property to enable QoS on qcs8300 Odelu Kukatla
@ 2025-11-28 16:19   ` Rob Herring (Arm)
  2025-11-29  9:33   ` Krzysztof Kozlowski
                     ` (2 subsequent siblings)
  3 siblings, 0 replies; 20+ messages in thread
From: Rob Herring (Arm) @ 2025-11-28 16:19 UTC (permalink / raw)
  To: Odelu Kukatla
  Cc: Mike Tipton, Dmitry Baryshkov, Raviteja Laggyshetty,
	Bartosz Golaszewski, Krzysztof Kozlowski, Konrad Dybcio,
	linux-arm-msm, Bjorn Andersson, devicetree, linux-kernel,
	linux-pm, Conor Dooley, Georgi Djakov


On Fri, 28 Nov 2025 20:31:04 +0530, Odelu Kukatla wrote:
> Add 'clocks' property to enable QoS configuration. This property
> enables the necessary clocks for QoS configuration.
> 
> QoS configuration is essential for ensuring that latency sensitive
> components such as CPUs and multimedia engines receive prioritized
> access to memory and interconnect resources. This helps to manage
> bandwidth and latency across subsystems, improving system responsiveness
> and performance in concurrent workloads.
> 
> Both 'reg' and 'clocks' properties are optional. If either is missing,
> QoS configuration will be skipped. This behavior is controlled by the
> 'qos_requires_clocks' flag in the driver, which ensures that QoS
> configuration is bypassed when required clocks are not defined.
> 
> Signed-off-by: Odelu Kukatla <odelu.kukatla@oss.qualcomm.com>
> ---
>  .../interconnect/qcom,qcs8300-rpmh.yaml       | 53 ++++++++++++++++---
>  1 file changed, 47 insertions(+), 6 deletions(-)
> 

My bot found errors running 'make dt_binding_check' on your patch:

yamllint warnings/errors:

dtschema/dtc warnings/errors:
Documentation/devicetree/bindings/thermal/thermal-sensor.example.dtb: /example-0/soc/thermal-sensor@c263000: failed to match any schema with compatible: ['qcom,sdm845-tsens', 'qcom,tsens-v2']
Documentation/devicetree/bindings/thermal/thermal-sensor.example.dtb: /example-0/soc/thermal-sensor@c263000: failed to match any schema with compatible: ['qcom,sdm845-tsens', 'qcom,tsens-v2']
Documentation/devicetree/bindings/thermal/thermal-sensor.example.dtb: /example-0/soc/thermal-sensor@c265000: failed to match any schema with compatible: ['qcom,sdm845-tsens', 'qcom,tsens-v2']
Documentation/devicetree/bindings/thermal/thermal-sensor.example.dtb: /example-0/soc/thermal-sensor@c265000: failed to match any schema with compatible: ['qcom,sdm845-tsens', 'qcom,tsens-v2']

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20251128150106.13849-2-odelu.kukatla@oss.qualcomm.com

The base for the series is generally the latest rc1. A different dependency
should be noted in *this* patch.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit after running the above command yourself. Note
that DT_SCHEMA_FILES can be set to your schema file to speed up checking
your schema. However, it must be unset to test all examples with your schema.


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 3/3] arm64: dts: qcom: qcs8300: Add clocks for QoS configuration
  2025-11-28 15:01 ` [PATCH 3/3] arm64: dts: qcom: qcs8300: Add clocks for " Odelu Kukatla
@ 2025-11-29  1:16   ` Dmitry Baryshkov
  2025-11-29  9:34   ` Krzysztof Kozlowski
  2025-12-01 15:09   ` Konrad Dybcio
  2 siblings, 0 replies; 20+ messages in thread
From: Dmitry Baryshkov @ 2025-11-29  1:16 UTC (permalink / raw)
  To: Odelu Kukatla
  Cc: Georgi Djakov, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Bjorn Andersson, Konrad Dybcio, Raviteja Laggyshetty,
	Bartosz Golaszewski, linux-arm-msm, linux-pm, devicetree,
	linux-kernel, Mike Tipton

On Fri, Nov 28, 2025 at 08:31:06PM +0530, Odelu Kukatla wrote:
> Add clocks which need to be enabled for configuring QoS on
> qcs8300 SoC.
> 
> Signed-off-by: Odelu Kukatla <odelu.kukatla@oss.qualcomm.com>
> ---
>  arch/arm64/boot/dts/qcom/monaco.dtsi | 6 ++++++
>  1 file changed, 6 insertions(+)
> 

Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>


-- 
With best wishes
Dmitry

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 2/3] interconnect: qcom: qcs8300: enable QoS configuration
  2025-11-28 15:01 ` [PATCH 2/3] interconnect: qcom: qcs8300: enable QoS configuration Odelu Kukatla
@ 2025-11-29  1:20   ` Dmitry Baryshkov
  2025-12-01 15:06   ` Konrad Dybcio
  1 sibling, 0 replies; 20+ messages in thread
From: Dmitry Baryshkov @ 2025-11-29  1:20 UTC (permalink / raw)
  To: Odelu Kukatla
  Cc: Georgi Djakov, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Bjorn Andersson, Konrad Dybcio, Raviteja Laggyshetty,
	Bartosz Golaszewski, linux-arm-msm, linux-pm, devicetree,
	linux-kernel, Mike Tipton

On Fri, Nov 28, 2025 at 08:31:05PM +0530, Odelu Kukatla wrote:
> Enable QoS configuration for master ports with predefined priority
> and urgency forwarding.
> 
> Signed-off-by: Odelu Kukatla <odelu.kukatla@oss.qualcomm.com>
> ---
>  drivers/interconnect/qcom/qcs8300.c | 375 ++++++++++++++++++++++++++++
>  1 file changed, 375 insertions(+)
> 

Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>


-- 
With best wishes
Dmitry

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 1/3] dt-bindings: interconnect: add clocks property to enable QoS on qcs8300
  2025-11-28 15:01 ` [PATCH 1/3] dt-bindings: interconnect: add clocks property to enable QoS on qcs8300 Odelu Kukatla
  2025-11-28 16:19   ` Rob Herring (Arm)
@ 2025-11-29  9:33   ` Krzysztof Kozlowski
  2025-11-29  9:37     ` Krzysztof Kozlowski
  2025-12-22 17:38     ` Odelu Kukatla
  2025-11-29  9:45   ` Krzysztof Kozlowski
  2025-12-23 19:55   ` Dmitry Baryshkov
  3 siblings, 2 replies; 20+ messages in thread
From: Krzysztof Kozlowski @ 2025-11-29  9:33 UTC (permalink / raw)
  To: Odelu Kukatla, Georgi Djakov, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Bjorn Andersson, Konrad Dybcio
  Cc: Raviteja Laggyshetty, Dmitry Baryshkov, Bartosz Golaszewski,
	linux-arm-msm, linux-pm, devicetree, linux-kernel, Mike Tipton

On 28/11/2025 16:01, Odelu Kukatla wrote:
> Add 'clocks' property to enable QoS configuration. This property
> enables the necessary clocks for QoS configuration.
> 
> QoS configuration is essential for ensuring that latency sensitive
> components such as CPUs and multimedia engines receive prioritized
> access to memory and interconnect resources. This helps to manage
> bandwidth and latency across subsystems, improving system responsiveness
> and performance in concurrent workloads.

I don't see how clocks property help here at all. Are you getting clock
rates in the driver of some other clocks to make QoS decisions?

> 
> Both 'reg' and 'clocks' properties are optional. If either is missing,

No! They are not. How they can be optional in the hardware? How SoC can
have for ONE GIVEN device optional reg, meaning one board with the same
Soc has the IO address space but other board with the same SoC does not
have it.

> QoS configuration will be skipped. This behavior is controlled by the
> 'qos_requires_clocks' flag in the driver, which ensures that QoS
> configuration is bypassed when required clocks are not defined.

This suggests that - driver is not helping. Please describe the
hardware, not your drivers.

> 
> Signed-off-by: Odelu Kukatla <odelu.kukatla@oss.qualcomm.com>
> ---
>  .../interconnect/qcom,qcs8300-rpmh.yaml       | 53 ++++++++++++++++---
>  1 file changed, 47 insertions(+), 6 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/interconnect/qcom,qcs8300-rpmh.yaml b/Documentation/devicetree/bindings/interconnect/qcom,qcs8300-rpmh.yaml
> index e9f528d6d9a8..594e835d1845 100644
> --- a/Documentation/devicetree/bindings/interconnect/qcom,qcs8300-rpmh.yaml
> +++ b/Documentation/devicetree/bindings/interconnect/qcom,qcs8300-rpmh.yaml
> @@ -35,6 +35,10 @@ properties:
>    reg:
>      maxItems: 1
>  
> +  clocks:
> +    minItems: 1
> +    maxItems: 4
> +
>  required:
>    - compatible
>  
> @@ -45,14 +49,39 @@ allOf:
>          compatible:
>            contains:
>              enum:
> -              - qcom,qcs8300-clk-virt
> -              - qcom,qcs8300-mc-virt
> +              - qcom,qcs8300-aggre1-noc
>      then:
>        properties:
> -        reg: false
> -    else:
> -      required:
> -        - reg

Why do you remove this? You cannot make random changes.

> +        clocks:
> +          items:
> +            - description: aggre UFS PHY AXI clock
> +            - description: aggre QUP PRIM AXI clock
> +            - description: aggre USB2 PRIM AXI clock
> +            - description: aggre USB3 PRIM AXI clock
> +
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            enum:
> +              - qcom,qcs8300-aggre2-noc
> +    then:
> +      properties:
> +        clocks:
> +          items:
> +            - description: RPMH CC IPA clock
> +
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            enum:
> +              - qcom,qcs8300-gem-noc
> +    then:
> +      properties:
> +        clocks:
> +          items:
> +            - description: GCC DDRSS GPU AXI clock

So all devices have clocks now? You made the schema less strict now,
removed pieces of it and you add more relaxed code telling every device
has a clock.

And none of this is explained in the commit msg.

>  
>  unevaluatedProperties: false
>  
> @@ -63,6 +92,7 @@ examples:
>          reg = <0x9100000 0xf7080>;
>          #interconnect-cells = <2>;
>          qcom,bcm-voters = <&apps_bcm_voter>;
> +        clocks = <&gcc_ddrss_gpu_axi_clk>;
>      };
>  
>      clk_virt: interconnect-0 {
> @@ -70,3 +100,14 @@ examples:
>          #interconnect-cells = <2>;
>          qcom,bcm-voters = <&apps_bcm_voter>;
>      };
> +
> +    aggre1_noc: interconnect@16c0000 {

No need for new example, it is the same as previous.

> +        compatible = "qcom,qcs8300-aggre1-noc";
> +        reg = <0x016c0000 0x17080>;
> +        #interconnect-cells = <2>;
> +        qcom,bcm-voters = <&apps_bcm_voter>;
> +        clocks = <&gcc_aggre_ufs_phy_axi_clk>,
> +                 <&gcc_aggre_noc_qupv3_axi_clk>,
> +                 <&gcc_aggre_usb2_prim_axi_clk>,
> +                 <&gcc_aggre_usb3_prim_axi_clk>;
> +    };


Best regards,
Krzysztof

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 3/3] arm64: dts: qcom: qcs8300: Add clocks for QoS configuration
  2025-11-28 15:01 ` [PATCH 3/3] arm64: dts: qcom: qcs8300: Add clocks for " Odelu Kukatla
  2025-11-29  1:16   ` Dmitry Baryshkov
@ 2025-11-29  9:34   ` Krzysztof Kozlowski
  2025-12-22 16:57     ` Odelu Kukatla
  2025-12-01 15:09   ` Konrad Dybcio
  2 siblings, 1 reply; 20+ messages in thread
From: Krzysztof Kozlowski @ 2025-11-29  9:34 UTC (permalink / raw)
  To: Odelu Kukatla, Georgi Djakov, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Bjorn Andersson, Konrad Dybcio
  Cc: Raviteja Laggyshetty, Dmitry Baryshkov, Bartosz Golaszewski,
	linux-arm-msm, linux-pm, devicetree, linux-kernel, Mike Tipton

On 28/11/2025 16:01, Odelu Kukatla wrote:
> Add clocks which need to be enabled for configuring QoS on
> qcs8300 SoC.
> 
> Signed-off-by: Odelu Kukatla <odelu.kukatla@oss.qualcomm.com>
> ---
>  arch/arm64/boot/dts/qcom/monaco.dtsi | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/qcom/monaco.dtsi b/arch/arm64/boot/dts/qcom/monaco.dtsi
> index 816fa2af8a9a..6139511ea525 100644
> --- a/arch/arm64/boot/dts/qcom/monaco.dtsi
> +++ b/arch/arm64/boot/dts/qcom/monaco.dtsi
> @@ -2226,6 +2226,10 @@
>  			reg = <0x0 0x016c0000 0x0 0x17080>;
>  			#interconnect-cells = <2>;
>  			qcom,bcm-voters = <&apps_bcm_voter>;
> +			clocks = <&gcc GCC_AGGRE_UFS_PHY_AXI_CLK>,
> +				 <&gcc GCC_AGGRE_NOC_QUPV3_AXI_CLK>,
> +				 <&gcc GCC_AGGRE_USB2_PRIM_AXI_CLK>,
> +				 <&gcc GCC_AGGRE_USB3_PRIM_AXI_CLK>;

Your binding said all interconnects have clocks, so please update all of
them. Not only three out of 10-or-whatever-in-total-you-have.


Best regards,
Krzysztof

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 1/3] dt-bindings: interconnect: add clocks property to enable QoS on qcs8300
  2025-11-29  9:33   ` Krzysztof Kozlowski
@ 2025-11-29  9:37     ` Krzysztof Kozlowski
  2025-12-22 17:38     ` Odelu Kukatla
  1 sibling, 0 replies; 20+ messages in thread
From: Krzysztof Kozlowski @ 2025-11-29  9:37 UTC (permalink / raw)
  To: Odelu Kukatla, Georgi Djakov, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Bjorn Andersson, Konrad Dybcio
  Cc: Raviteja Laggyshetty, Dmitry Baryshkov, Bartosz Golaszewski,
	linux-arm-msm, linux-pm, devicetree, linux-kernel, Mike Tipton

On 29/11/2025 10:33, Krzysztof Kozlowski wrote:
> On 28/11/2025 16:01, Odelu Kukatla wrote:
>> Add 'clocks' property to enable QoS configuration. This property
>> enables the necessary clocks for QoS configuration.
>>
>> QoS configuration is essential for ensuring that latency sensitive
>> components such as CPUs and multimedia engines receive prioritized
>> access to memory and interconnect resources. This helps to manage
>> bandwidth and latency across subsystems, improving system responsiveness
>> and performance in concurrent workloads.
> 
> I don't see how clocks property help here at all. Are you getting clock
> rates in the driver of some other clocks to make QoS decisions?
> 
>>
>> Both 'reg' and 'clocks' properties are optional. If either is missing,
> 
> No! They are not. How they can be optional in the hardware? How SoC can
> have for ONE GIVEN device optional reg, meaning one board with the same
> Soc has the IO address space but other board with the same SoC does not
> have it.
> 
>> QoS configuration will be skipped. This behavior is controlled by the
>> 'qos_requires_clocks' flag in the driver, which ensures that QoS
>> configuration is bypassed when required clocks are not defined.
> 
> This suggests that - driver is not helping. Please describe the
> hardware, not your drivers.
> 

And now I see you sent the same buggy code for sa8775p:
https://lore.kernel.org/all/20251001073344.6599-2-odelu.kukatla@oss.qualcomm.com/

And this was already merged!

No, you just make bindings worse.


Best regards,
Krzysztof

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 1/3] dt-bindings: interconnect: add clocks property to enable QoS on qcs8300
  2025-11-28 15:01 ` [PATCH 1/3] dt-bindings: interconnect: add clocks property to enable QoS on qcs8300 Odelu Kukatla
  2025-11-28 16:19   ` Rob Herring (Arm)
  2025-11-29  9:33   ` Krzysztof Kozlowski
@ 2025-11-29  9:45   ` Krzysztof Kozlowski
  2025-12-22 17:45     ` Odelu Kukatla
  2025-12-23 19:55   ` Dmitry Baryshkov
  3 siblings, 1 reply; 20+ messages in thread
From: Krzysztof Kozlowski @ 2025-11-29  9:45 UTC (permalink / raw)
  To: Odelu Kukatla, Georgi Djakov, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Bjorn Andersson, Konrad Dybcio
  Cc: Raviteja Laggyshetty, Dmitry Baryshkov, Bartosz Golaszewski,
	linux-arm-msm, linux-pm, devicetree, linux-kernel, Mike Tipton

On 28/11/2025 16:01, Odelu Kukatla wrote:
> Add 'clocks' property to enable QoS configuration. This property
> enables the necessary clocks for QoS configuration.
> 
> QoS configuration is essential for ensuring that latency sensitive
> components such as CPUs and multimedia engines receive prioritized
> access to memory and interconnect resources. This helps to manage
> bandwidth and latency across subsystems, improving system responsiveness
> and performance in concurrent workloads.


Same problems with your previous and other commits:

Please use subject prefixes matching the subsystem. You can get them for
example with `git log --oneline -- DIRECTORY_OR_FILE` on the directory
your patch is touching. For bindings, the preferred subjects are
explained here:
https://www.kernel.org/doc/html/latest/devicetree/bindings/submitting-patches.html#i-for-patch-submitters

"on qcs8300" is redundant. Prefix defines that, so use proper prefixes.
You do not add clocks for every interconnect...

Best regards,
Krzysztof

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 2/3] interconnect: qcom: qcs8300: enable QoS configuration
  2025-11-28 15:01 ` [PATCH 2/3] interconnect: qcom: qcs8300: enable QoS configuration Odelu Kukatla
  2025-11-29  1:20   ` Dmitry Baryshkov
@ 2025-12-01 15:06   ` Konrad Dybcio
  1 sibling, 0 replies; 20+ messages in thread
From: Konrad Dybcio @ 2025-12-01 15:06 UTC (permalink / raw)
  To: Odelu Kukatla, Georgi Djakov, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Bjorn Andersson, Konrad Dybcio
  Cc: Raviteja Laggyshetty, Dmitry Baryshkov, Bartosz Golaszewski,
	linux-arm-msm, linux-pm, devicetree, linux-kernel, Mike Tipton

On 11/28/25 4:01 PM, Odelu Kukatla wrote:
> Enable QoS configuration for master ports with predefined priority
> and urgency forwarding.
> 
> Signed-off-by: Odelu Kukatla <odelu.kukatla@oss.qualcomm.com>
> ---
>  drivers/interconnect/qcom/qcs8300.c | 375 ++++++++++++++++++++++++++++
>  1 file changed, 375 insertions(+)
> 
> diff --git a/drivers/interconnect/qcom/qcs8300.c b/drivers/interconnect/qcom/qcs8300.c
> index 70a377bbcf29..3f4fe62148d3 100644
> --- a/drivers/interconnect/qcom/qcs8300.c
> +++ b/drivers/interconnect/qcom/qcs8300.c
> @@ -186,6 +186,13 @@ static struct qcom_icc_node qxm_qup3 = {
>  	.name = "qxm_qup3",
>  	.channels = 1,
>  	.buswidth = 8,
> +	.qosbox = &(const struct qcom_icc_qosbox) {
> +		.num_ports = 1,
> +		.port_offsets = { 0x11000 },
> +		.prio_fwd_disable = 1,
> +		.prio = 2,
> +		.urg_fwd = 0,

FWIW prio_fwd_disable and urg_uwd are booleans, so true/false would be neat

I checked a couple nodes and things seem alright, hopefully the .max_register
values you set don't clip anything

Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>

Konrad

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 3/3] arm64: dts: qcom: qcs8300: Add clocks for QoS configuration
  2025-11-28 15:01 ` [PATCH 3/3] arm64: dts: qcom: qcs8300: Add clocks for " Odelu Kukatla
  2025-11-29  1:16   ` Dmitry Baryshkov
  2025-11-29  9:34   ` Krzysztof Kozlowski
@ 2025-12-01 15:09   ` Konrad Dybcio
  2 siblings, 0 replies; 20+ messages in thread
From: Konrad Dybcio @ 2025-12-01 15:09 UTC (permalink / raw)
  To: Odelu Kukatla, Georgi Djakov, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Bjorn Andersson, Konrad Dybcio
  Cc: Raviteja Laggyshetty, Dmitry Baryshkov, Bartosz Golaszewski,
	linux-arm-msm, linux-pm, devicetree, linux-kernel, Mike Tipton

On 11/28/25 4:01 PM, Odelu Kukatla wrote:
> Add clocks which need to be enabled for configuring QoS on
> qcs8300 SoC.
> 
> Signed-off-by: Odelu Kukatla <odelu.kukatla@oss.qualcomm.com>
> ---

I don't have a good reference for this, but it seems like there's
a lot more various AXI_CLKs (PCIe, ethernet, camera) - do we need
any of them too?

Konrad

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 3/3] arm64: dts: qcom: qcs8300: Add clocks for QoS configuration
  2025-11-29  9:34   ` Krzysztof Kozlowski
@ 2025-12-22 16:57     ` Odelu Kukatla
  0 siblings, 0 replies; 20+ messages in thread
From: Odelu Kukatla @ 2025-12-22 16:57 UTC (permalink / raw)
  To: Krzysztof Kozlowski, Georgi Djakov, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Bjorn Andersson, Konrad Dybcio
  Cc: Raviteja Laggyshetty, Dmitry Baryshkov, Bartosz Golaszewski,
	linux-arm-msm, linux-pm, devicetree, linux-kernel, Mike Tipton



On 11/29/2025 3:04 PM, Krzysztof Kozlowski wrote:
> On 28/11/2025 16:01, Odelu Kukatla wrote:
>> Add clocks which need to be enabled for configuring QoS on
>> qcs8300 SoC.
>>
>> Signed-off-by: Odelu Kukatla <odelu.kukatla@oss.qualcomm.com>
>> ---
>>  arch/arm64/boot/dts/qcom/monaco.dtsi | 6 ++++++
>>  1 file changed, 6 insertions(+)
>>
>> diff --git a/arch/arm64/boot/dts/qcom/monaco.dtsi b/arch/arm64/boot/dts/qcom/monaco.dtsi
>> index 816fa2af8a9a..6139511ea525 100644
>> --- a/arch/arm64/boot/dts/qcom/monaco.dtsi
>> +++ b/arch/arm64/boot/dts/qcom/monaco.dtsi
>> @@ -2226,6 +2226,10 @@
>>  			reg = <0x0 0x016c0000 0x0 0x17080>;
>>  			#interconnect-cells = <2>;
>>  			qcom,bcm-voters = <&apps_bcm_voter>;
>> +			clocks = <&gcc GCC_AGGRE_UFS_PHY_AXI_CLK>,
>> +				 <&gcc GCC_AGGRE_NOC_QUPV3_AXI_CLK>,
>> +				 <&gcc GCC_AGGRE_USB2_PRIM_AXI_CLK>,
>> +				 <&gcc GCC_AGGRE_USB3_PRIM_AXI_CLK>;
> 
> Your binding said all interconnects have clocks, so please update all of
> them. Not only three out of 10-or-whatever-in-total-you-have.
> 

Thanks Krzysztof!
Not all interconnects have clocks/MMIO, I will update the bindings as
you suggested in the fix:
https://lore.kernel.org/all/20251129094612.16838-2-krzysztof.kozlowski@oss.qualcomm.com/


Thanks,
Odelu

> 
> Best regards,
> Krzysztof


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 1/3] dt-bindings: interconnect: add clocks property to enable QoS on qcs8300
  2025-11-29  9:33   ` Krzysztof Kozlowski
  2025-11-29  9:37     ` Krzysztof Kozlowski
@ 2025-12-22 17:38     ` Odelu Kukatla
  2025-12-23 13:45       ` Krzysztof Kozlowski
  1 sibling, 1 reply; 20+ messages in thread
From: Odelu Kukatla @ 2025-12-22 17:38 UTC (permalink / raw)
  To: Krzysztof Kozlowski, Georgi Djakov, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Bjorn Andersson, Konrad Dybcio
  Cc: Raviteja Laggyshetty, Dmitry Baryshkov, Bartosz Golaszewski,
	linux-arm-msm, linux-pm, devicetree, linux-kernel, Mike Tipton



On 11/29/2025 3:03 PM, Krzysztof Kozlowski wrote:
> On 28/11/2025 16:01, Odelu Kukatla wrote:
>> Add 'clocks' property to enable QoS configuration. This property
>> enables the necessary clocks for QoS configuration.
>>
>> QoS configuration is essential for ensuring that latency sensitive
>> components such as CPUs and multimedia engines receive prioritized
>> access to memory and interconnect resources. This helps to manage
>> bandwidth and latency across subsystems, improving system responsiveness
>> and performance in concurrent workloads.
> 
> I don't see how clocks property help here at all. Are you getting clock
> rates in the driver of some other clocks to make QoS decisions?
> 

We don't need to get clock rate/frequency, just need to enable the
clock(s) for QoS register access for which we need to get the clock
handle in driver.

>>
>> Both 'reg' and 'clocks' properties are optional. If either is missing,
> 
> No! They are not. How they can be optional in the hardware? How SoC can
> have for ONE GIVEN device optional reg, meaning one board with the same
> Soc has the IO address space but other board with the same SoC does not
> have it.
> 

I agree, I will drop the “optional” wording and rework the schema so
that `reg` and clocks’ are required for interconnects that implement
these resources, and not allowed for the NOCs without MMIO/clocks and
the virtual providers.

>> QoS configuration will be skipped. This behavior is controlled by the
>> 'qos_requires_clocks' flag in the driver, which ensures that QoS
>> configuration is bypassed when required clocks are not defined.
> 
> This suggests that - driver is not helping. Please describe the
> hardware, not your drivers.
> 

I will remove the driver related description from the commit message and
ensure the binding text stays strictly about the hardware.

>>
>> Signed-off-by: Odelu Kukatla <odelu.kukatla@oss.qualcomm.com>
>> ---
>>  .../interconnect/qcom,qcs8300-rpmh.yaml       | 53 ++++++++++++++++---
>>  1 file changed, 47 insertions(+), 6 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/interconnect/qcom,qcs8300-rpmh.yaml b/Documentation/devicetree/bindings/interconnect/qcom,qcs8300-rpmh.yaml
>> index e9f528d6d9a8..594e835d1845 100644
>> --- a/Documentation/devicetree/bindings/interconnect/qcom,qcs8300-rpmh.yaml
>> +++ b/Documentation/devicetree/bindings/interconnect/qcom,qcs8300-rpmh.yaml
>> @@ -35,6 +35,10 @@ properties:
>>    reg:
>>      maxItems: 1
>>  
>> +  clocks:
>> +    minItems: 1
>> +    maxItems: 4
>> +
>>  required:
>>    - compatible
>>  
>> @@ -45,14 +49,39 @@ allOf:
>>          compatible:
>>            contains:
>>              enum:
>> -              - qcom,qcs8300-clk-virt
>> -              - qcom,qcs8300-mc-virt
>> +              - qcom,qcs8300-aggre1-noc
>>      then:
>>        properties:
>> -        reg: false
>> -    else:
>> -      required:
>> -        - reg
> 
> Why do you remove this? You cannot make random changes.
> 

My goal was to add clocks for the interconnects that need them, not to
relax the `reg` requirements or change the behavior for
`clk-virt`/`mc-virt`. I will restore the previous `reg` handling and
only add additional constraints for `clocks` where they are actually
required by the hardware.

>> +        clocks:
>> +          items:
>> +            - description: aggre UFS PHY AXI clock
>> +            - description: aggre QUP PRIM AXI clock
>> +            - description: aggre USB2 PRIM AXI clock
>> +            - description: aggre USB3 PRIM AXI clock
>> +
>> +  - if:
>> +      properties:
>> +        compatible:
>> +          contains:
>> +            enum:
>> +              - qcom,qcs8300-aggre2-noc
>> +    then:
>> +      properties:
>> +        clocks:
>> +          items:
>> +            - description: RPMH CC IPA clock
>> +
>> +  - if:
>> +      properties:
>> +        compatible:
>> +          contains:
>> +            enum:
>> +              - qcom,qcs8300-gem-noc
>> +    then:
>> +      properties:
>> +        clocks:
>> +          items:
>> +            - description: GCC DDRSS GPU AXI clock
> 
> So all devices have clocks now? You made the schema less strict now,
> removed pieces of it and you add more relaxed code telling every device
> has a clock.
> 

Not all interconnects have clocks/MMIO, I will update the bindings as
you suggested in the fix:
https://lore.kernel.org/all/20251129094612.16838-2-krzysztof.kozlowski@oss.qualcomm.com/


> And none of this is explained in the commit msg.
> 
>>  
>>  unevaluatedProperties: false
>>  
>> @@ -63,6 +92,7 @@ examples:
>>          reg = <0x9100000 0xf7080>;
>>          #interconnect-cells = <2>;
>>          qcom,bcm-voters = <&apps_bcm_voter>;
>> +        clocks = <&gcc_ddrss_gpu_axi_clk>;
>>      };
>>  
>>      clk_virt: interconnect-0 {
>> @@ -70,3 +100,14 @@ examples:
>>          #interconnect-cells = <2>;
>>          qcom,bcm-voters = <&apps_bcm_voter>;
>>      };
>> +
>> +    aggre1_noc: interconnect@16c0000 {
> 
> No need for new example, it is the same as previous.
> 


Understood. I will drop the additional example instead of duplicating
essentially the same content.
I’ll rework the patch accordingly and send a v2 that keeps the schema
strict.

>> +        compatible = "qcom,qcs8300-aggre1-noc";
>> +        reg = <0x016c0000 0x17080>;
>> +        #interconnect-cells = <2>;
>> +        qcom,bcm-voters = <&apps_bcm_voter>;
>> +        clocks = <&gcc_aggre_ufs_phy_axi_clk>,
>> +                 <&gcc_aggre_noc_qupv3_axi_clk>,
>> +                 <&gcc_aggre_usb2_prim_axi_clk>,
>> +                 <&gcc_aggre_usb3_prim_axi_clk>;
>> +    };
> 
> 
> Best regards,
> Krzysztof


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 1/3] dt-bindings: interconnect: add clocks property to enable QoS on qcs8300
  2025-11-29  9:45   ` Krzysztof Kozlowski
@ 2025-12-22 17:45     ` Odelu Kukatla
  0 siblings, 0 replies; 20+ messages in thread
From: Odelu Kukatla @ 2025-12-22 17:45 UTC (permalink / raw)
  To: Krzysztof Kozlowski, Georgi Djakov, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Bjorn Andersson, Konrad Dybcio
  Cc: Raviteja Laggyshetty, Dmitry Baryshkov, Bartosz Golaszewski,
	linux-arm-msm, linux-pm, devicetree, linux-kernel, Mike Tipton



On 11/29/2025 3:15 PM, Krzysztof Kozlowski wrote:
> On 28/11/2025 16:01, Odelu Kukatla wrote:
>> Add 'clocks' property to enable QoS configuration. This property
>> enables the necessary clocks for QoS configuration.
>>
>> QoS configuration is essential for ensuring that latency sensitive
>> components such as CPUs and multimedia engines receive prioritized
>> access to memory and interconnect resources. This helps to manage
>> bandwidth and latency across subsystems, improving system responsiveness
>> and performance in concurrent workloads.
> 
> 
> Same problems with your previous and other commits:
> 
> Please use subject prefixes matching the subsystem. You can get them for
> example with `git log --oneline -- DIRECTORY_OR_FILE` on the directory
> your patch is touching. For bindings, the preferred subjects are
> explained here:
> https://www.kernel.org/doc/html/latest/devicetree/bindings/submitting-patches.html#i-for-patch-submitters
> 
> "on qcs8300" is redundant. Prefix defines that, so use proper prefixes.
> You do not add clocks for every interconnect...
> 

Thanks for the review and providing pointers, I’ll address the prefix
issue in v2.

> Best regards,
> Krzysztof


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 1/3] dt-bindings: interconnect: add clocks property to enable QoS on qcs8300
  2025-12-22 17:38     ` Odelu Kukatla
@ 2025-12-23 13:45       ` Krzysztof Kozlowski
  2026-01-03 18:42         ` Odelu Kukatla
  0 siblings, 1 reply; 20+ messages in thread
From: Krzysztof Kozlowski @ 2025-12-23 13:45 UTC (permalink / raw)
  To: Odelu Kukatla, Georgi Djakov, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Bjorn Andersson, Konrad Dybcio
  Cc: Raviteja Laggyshetty, Dmitry Baryshkov, Bartosz Golaszewski,
	linux-arm-msm, linux-pm, devicetree, linux-kernel, Mike Tipton

On 22/12/2025 18:38, Odelu Kukatla wrote:
> 
> 
> On 11/29/2025 3:03 PM, Krzysztof Kozlowski wrote:
>> On 28/11/2025 16:01, Odelu Kukatla wrote:
>>> Add 'clocks' property to enable QoS configuration. This property
>>> enables the necessary clocks for QoS configuration.
>>>
>>> QoS configuration is essential for ensuring that latency sensitive
>>> components such as CPUs and multimedia engines receive prioritized
>>> access to memory and interconnect resources. This helps to manage
>>> bandwidth and latency across subsystems, improving system responsiveness
>>> and performance in concurrent workloads.
>>
>> I don't see how clocks property help here at all. Are you getting clock
>> rates in the driver of some other clocks to make QoS decisions?
>>
> 
> We don't need to get clock rate/frequency, just need to enable the
> clock(s) for QoS register access for which we need to get the clock
> handle in driver.

Not relevant what your driver does. Still getting clock does not improve
system responsiveness. If you claim otherwise give me an argument or any
sort of proof that providing clock has impact on system responsiveness.

IOW, don't feed us marketing. It's waste of our time.



> 
>>>
>>> Both 'reg' and 'clocks' properties are optional. If either is missing,
>>
>> No! They are not. How they can be optional in the hardware? How SoC can
>> have for ONE GIVEN device optional reg, meaning one board with the same
>> Soc has the IO address space but other board with the same SoC does not
>> have it.
>>
> 
> I agree, I will drop the “optional” wording and rework the schema so

So why were they optional in the first place? What is this patch describing?


Best regards,
Krzysztof

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 1/3] dt-bindings: interconnect: add clocks property to enable QoS on qcs8300
  2025-11-28 15:01 ` [PATCH 1/3] dt-bindings: interconnect: add clocks property to enable QoS on qcs8300 Odelu Kukatla
                     ` (2 preceding siblings ...)
  2025-11-29  9:45   ` Krzysztof Kozlowski
@ 2025-12-23 19:55   ` Dmitry Baryshkov
  2025-12-24 10:59     ` Odelu Kukatla
  3 siblings, 1 reply; 20+ messages in thread
From: Dmitry Baryshkov @ 2025-12-23 19:55 UTC (permalink / raw)
  To: Odelu Kukatla
  Cc: Georgi Djakov, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Bjorn Andersson, Konrad Dybcio, Raviteja Laggyshetty,
	Bartosz Golaszewski, linux-arm-msm, linux-pm, devicetree,
	linux-kernel, Mike Tipton

On Fri, Nov 28, 2025 at 08:31:04PM +0530, Odelu Kukatla wrote:
> Add 'clocks' property to enable QoS configuration. This property
> enables the necessary clocks for QoS configuration.
> 
> QoS configuration is essential for ensuring that latency sensitive
> components such as CPUs and multimedia engines receive prioritized
> access to memory and interconnect resources. This helps to manage
> bandwidth and latency across subsystems, improving system responsiveness
> and performance in concurrent workloads.
> 
> Both 'reg' and 'clocks' properties are optional. If either is missing,
> QoS configuration will be skipped. This behavior is controlled by the
> 'qos_requires_clocks' flag in the driver, which ensures that QoS
> configuration is bypassed when required clocks are not defined.
> 
> Signed-off-by: Odelu Kukatla <odelu.kukatla@oss.qualcomm.com>
> ---
>  .../interconnect/qcom,qcs8300-rpmh.yaml       | 53 ++++++++++++++++---
>  1 file changed, 47 insertions(+), 6 deletions(-)

As a generic feedback for Qualcomm interconnect drivers (please pass it
through the team):

Please ensure that QoS-related clocks are defined in the first driver
submission. DT bindings should describe the hardware and it's not that
the hardware has changed between the time the first patches were
submitted and this patchset.

I see a typical pattern that QoS support is being submitted several
months later. Why is it so? Why can't QoS be a part of the _same_
patchset?

-- 
With best wishes
Dmitry

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 1/3] dt-bindings: interconnect: add clocks property to enable QoS on qcs8300
  2025-12-23 19:55   ` Dmitry Baryshkov
@ 2025-12-24 10:59     ` Odelu Kukatla
  0 siblings, 0 replies; 20+ messages in thread
From: Odelu Kukatla @ 2025-12-24 10:59 UTC (permalink / raw)
  To: Dmitry Baryshkov
  Cc: Georgi Djakov, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Bjorn Andersson, Konrad Dybcio, Raviteja Laggyshetty,
	Bartosz Golaszewski, linux-arm-msm, linux-pm, devicetree,
	linux-kernel, Mike Tipton



On 12/24/2025 1:25 AM, Dmitry Baryshkov wrote:
> On Fri, Nov 28, 2025 at 08:31:04PM +0530, Odelu Kukatla wrote:
>> Add 'clocks' property to enable QoS configuration. This property
>> enables the necessary clocks for QoS configuration.
>>
>> QoS configuration is essential for ensuring that latency sensitive
>> components such as CPUs and multimedia engines receive prioritized
>> access to memory and interconnect resources. This helps to manage
>> bandwidth and latency across subsystems, improving system responsiveness
>> and performance in concurrent workloads.
>>
>> Both 'reg' and 'clocks' properties are optional. If either is missing,
>> QoS configuration will be skipped. This behavior is controlled by the
>> 'qos_requires_clocks' flag in the driver, which ensures that QoS
>> configuration is bypassed when required clocks are not defined.
>>
>> Signed-off-by: Odelu Kukatla <odelu.kukatla@oss.qualcomm.com>
>> ---
>>  .../interconnect/qcom,qcs8300-rpmh.yaml       | 53 ++++++++++++++++---
>>  1 file changed, 47 insertions(+), 6 deletions(-)
> 
> As a generic feedback for Qualcomm interconnect drivers (please pass it
> through the team):
> 
> Please ensure that QoS-related clocks are defined in the first driver
> submission. DT bindings should describe the hardware and it's not that
> the hardware has changed between the time the first patches were
> submitted and this patchset.
> 
> I see a typical pattern that QoS support is being submitted several
> months later. Why is it so? Why can't QoS be a part of the _same_
> patchset?
> 

Hi Dmitry,
Thanks for feedback. we are ensuring that QoS-related clocks are defined
in the first driver submission like recently for Glymur/Kaanapali, there
were part of first driver submission.
QCS8300 and QCS615 are the only chip-sets, we are sending QoS patches
separately.

Thanks,
Odelu



^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 1/3] dt-bindings: interconnect: add clocks property to enable QoS on qcs8300
  2025-12-23 13:45       ` Krzysztof Kozlowski
@ 2026-01-03 18:42         ` Odelu Kukatla
  0 siblings, 0 replies; 20+ messages in thread
From: Odelu Kukatla @ 2026-01-03 18:42 UTC (permalink / raw)
  To: Krzysztof Kozlowski, Georgi Djakov, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Bjorn Andersson, Konrad Dybcio
  Cc: Raviteja Laggyshetty, Dmitry Baryshkov, Bartosz Golaszewski,
	linux-arm-msm, linux-pm, devicetree, linux-kernel, Mike Tipton



On 12/23/2025 7:15 PM, Krzysztof Kozlowski wrote:
> On 22/12/2025 18:38, Odelu Kukatla wrote:
>>
>>
>> On 11/29/2025 3:03 PM, Krzysztof Kozlowski wrote:
>>> On 28/11/2025 16:01, Odelu Kukatla wrote:
>>>> Add 'clocks' property to enable QoS configuration. This property
>>>> enables the necessary clocks for QoS configuration.
>>>>
>>>> QoS configuration is essential for ensuring that latency sensitive
>>>> components such as CPUs and multimedia engines receive prioritized
>>>> access to memory and interconnect resources. This helps to manage
>>>> bandwidth and latency across subsystems, improving system responsiveness
>>>> and performance in concurrent workloads.
>>>
>>> I don't see how clocks property help here at all. Are you getting clock
>>> rates in the driver of some other clocks to make QoS decisions?
>>>
>>
>> We don't need to get clock rate/frequency, just need to enable the
>> clock(s) for QoS register access for which we need to get the clock
>> handle in driver.
> 
> Not relevant what your driver does. Still getting clock does not improve
> system responsiveness. If you claim otherwise give me an argument or any
> sort of proof that providing clock has impact on system responsiveness.
> 
> IOW, don't feed us marketing. It's waste of our time.
> 
> 

Hi Krzysztof,
Thanks for the feedback.
You are right that the clocks property itself does not improve system
responsiveness. The QoS registers are inside a block whose interface is
clock-gated, so the driver must enable the required clock before
accessing those registers. The clocks property is only there to let the
driver obtain that clock handle and enable the required clock; without
that, the QoS registers are not accessible. The actual QoS behaviour is
determined entirely by the values written to those registers, not by the
presence of the clocks property in DT.

About the “optional” wording: that was incorrect on my side. I will:
1.update the binding so that the `clocks` property accurately reflects
the hardware and does not allow describing non-existing hardware, and
2.drop the “optional” wording and the performance oriented description
from the commit message, keeping it to a minimal statement that the
clock is needed to access the QoS registers.

I’ll send a v2 with these changes.

Best regards,
Odelu

> 
>>
>>>>
>>>> Both 'reg' and 'clocks' properties are optional. If either is missing,
>>>
>>> No! They are not. How they can be optional in the hardware? How SoC can
>>> have for ONE GIVEN device optional reg, meaning one board with the same
>>> Soc has the IO address space but other board with the same SoC does not
>>> have it.
>>>
>>
>> I agree, I will drop the “optional” wording and rework the schema so
> 
> So why were they optional in the first place? What is this patch describing?
> 
> 
> Best regards,
> Krzysztof


^ permalink raw reply	[flat|nested] 20+ messages in thread

end of thread, other threads:[~2026-01-03 18:42 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-11-28 15:01 [PATCH 0/3] Enable QoS configuration on QCS8300 Odelu Kukatla
2025-11-28 15:01 ` [PATCH 1/3] dt-bindings: interconnect: add clocks property to enable QoS on qcs8300 Odelu Kukatla
2025-11-28 16:19   ` Rob Herring (Arm)
2025-11-29  9:33   ` Krzysztof Kozlowski
2025-11-29  9:37     ` Krzysztof Kozlowski
2025-12-22 17:38     ` Odelu Kukatla
2025-12-23 13:45       ` Krzysztof Kozlowski
2026-01-03 18:42         ` Odelu Kukatla
2025-11-29  9:45   ` Krzysztof Kozlowski
2025-12-22 17:45     ` Odelu Kukatla
2025-12-23 19:55   ` Dmitry Baryshkov
2025-12-24 10:59     ` Odelu Kukatla
2025-11-28 15:01 ` [PATCH 2/3] interconnect: qcom: qcs8300: enable QoS configuration Odelu Kukatla
2025-11-29  1:20   ` Dmitry Baryshkov
2025-12-01 15:06   ` Konrad Dybcio
2025-11-28 15:01 ` [PATCH 3/3] arm64: dts: qcom: qcs8300: Add clocks for " Odelu Kukatla
2025-11-29  1:16   ` Dmitry Baryshkov
2025-11-29  9:34   ` Krzysztof Kozlowski
2025-12-22 16:57     ` Odelu Kukatla
2025-12-01 15:09   ` Konrad Dybcio

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox