[PATCH 0/4] Add Qualcomm's IPQ9650 APSS Clock Driver Support

[PATCH 0/4] Add Qualcomm's IPQ9650 APSS Clock Driver Support

[Kathiravan Thirumoorthy]

The APSS clock controller on IPQ9650 provides the CPU and L3 clocks. The
four Cortex-A55 cores (silver), the Cortex-A78 core (gold) and the L3
are each clocked by their own Zonda PLL, selected through a glitch-free mux
whose parents are XO, GPLL0 and the corresponding PLL. Since the L3 has
to scale along with the CPUs, the controller also registers an icc-clk
provider in which both CPU clusters vote on the L3 clock.

Signed-off-by: Kathiravan Thirumoorthy <kathiravan.thirumoorthy@oss.qualcomm.com>
---
Kathiravan Thirumoorthy (4):
      interconnect: icc-clk: support N:1 master-to-slave topology
      clk: qcom: clk-alpha-pll: Skip Zonda PLL configuration if already enabled
      dt-bindings: clock: ipq9650-apss-clk: Add IPQ9650 apss clock controller
      clk: qcom: apss-ipq9650: Add IPQ9650 APSS clock controller driver

 .../bindings/clock/qcom,ipq9650-apss-clk.yaml      |  58 ++++
 drivers/clk/qcom/Kconfig                           |   9 +
 drivers/clk/qcom/Makefile                          |   1 +
 drivers/clk/qcom/apss-ipq9650.c                    | 385 +++++++++++++++++++++
 drivers/clk/qcom/clk-alpha-pll.c                   |   4 +
 drivers/interconnect/icc-clk.c                     |  45 ++-
 include/dt-bindings/clock/qcom,apss-ipq.h          |   5 +
 include/dt-bindings/interconnect/qcom,ipq9650.h    |   4 +
 8 files changed, 504 insertions(+), 7 deletions(-)
---
base-commit: eea3fef32a9cf36abcb5975a5a594e4135a6b026
change-id: 20260925-ipq9650_apss_clk-ff825e337d11

Best regards,
--  
Kathiravan Thirumoorthy <kathiravan.thirumoorthy@oss.qualcomm.com>

[PATCH 1/4] interconnect: icc-clk: support N:1 master-to-slave topology

[Kathiravan Thirumoorthy]

icc_clk_register() assumes each icc_clk_data entry describes a unique
master/slave pair and creates both nodes for every entry. When several
masters share the same slave_id, the shared slave node is created and
added again on each later entry. icc_node_create() returns the existing
node, so icc_node_add() hits WARN_ON(node->provider):

  WARNING: drivers/interconnect/core.c:1103 at icc_node_add+0x10c/0x120
  Call trace:
   icc_node_add+0x10c/0x120 (P)
   icc_clk_register+0x1a4/0x214
   devm_icc_clk_register+0x18/0x80
   qcom_cc_really_probe+0x4c8/0x514
   apss_ipq9650_probe+0xc0/0x108

This topology is needed on IPQ9650, where the silver and gold CPU
clusters both vote on the single L3 clock through a shared SLAVE_L3
node.

The onecell table is also filled in registration order, so xlate only
works when the node IDs happen to match that order. Size the table by
the highest node ID, index it by ID, and skip slave creation when the
node is already registered.

icc_clk_set() also passes only the requesting node's bandwidth to
clk_set_rate(), so the last writer wins. A low vote from one master can
then lower the clock below what another master asked for. Before setting
the rate, take the highest peak bandwidth across all nodes that share
the clock.

Signed-off-by: Kathiravan Thirumoorthy <kathiravan.thirumoorthy@oss.qualcomm.com>
---
 drivers/interconnect/icc-clk.c | 45 +++++++++++++++++++++++++++++++++++-------
 1 file changed, 38 insertions(+), 7 deletions(-)

diff --git a/drivers/interconnect/icc-clk.c b/drivers/interconnect/icc-clk.c
index 93c030608d3e..567d071fade8 100644
--- a/drivers/interconnect/icc-clk.c
+++ b/drivers/interconnect/icc-clk.c
@@ -24,7 +24,10 @@ struct icc_clk_provider {
 
 static int icc_clk_set(struct icc_node *src, struct icc_node *dst)
 {
+	unsigned long rate = icc_units_to_bps(src->peak_bw);
+	struct icc_provider *provider = src->provider;
 	struct icc_clk_node *qn = src->data;
+	struct icc_node *node;
 	int ret;
 
 	if (!qn || !qn->clk)
@@ -45,7 +48,21 @@ static int icc_clk_set(struct icc_node *src, struct icc_node *dst)
 		qn->enabled = true;
 	}
 
-	return clk_set_rate(qn->clk, icc_units_to_bps(src->peak_bw));
+	/*
+	 * Multiple master nodes can share the same underlying clock (N:1
+	 * topology, e.g. several CPU clusters voting on one L3 clock).
+	 * Aggregate the peak bandwidth across all of them before setting
+	 * the rate, otherwise the last caller wins and can undervote what
+	 * another master already asked for.
+	 */
+	list_for_each_entry(node, &provider->nodes, node_list) {
+		struct icc_clk_node *n = node->data;
+
+		if (n && clk_is_match(n->clk, qn->clk))
+			rate = max(rate, icc_units_to_bps(node->peak_bw));
+	}
+
+	return clk_set_rate(qn->clk, rate);
 }
 
 static int icc_clk_get_bw(struct icc_node *node, u32 *avg, u32 *peak)
@@ -82,12 +99,17 @@ struct icc_provider *icc_clk_register(struct device *dev,
 	struct icc_provider *provider;
 	struct icc_onecell_data *onecell;
 	struct icc_node *node;
-	int ret, i, j;
+	unsigned int max_id = 0;
+	int ret, i;
 
-	onecell = devm_kzalloc(dev, struct_size(onecell, nodes, 2 * num_clocks), GFP_KERNEL);
+	/* Find the highest node ID to size the xlate lookup table */
+	for (i = 0; i < num_clocks; i++)
+		max_id = max(max_id, max(data[i].master_id, data[i].slave_id));
+
+	onecell = devm_kzalloc(dev, struct_size(onecell, nodes, max_id + 1), GFP_KERNEL);
 	if (!onecell)
 		return ERR_PTR(-ENOMEM);
-	onecell->num_nodes = 2 * num_clocks;
+	onecell->num_nodes = max_id + 1;
 
 	qp = devm_kzalloc(dev, struct_size(qp, clocks, num_clocks), GFP_KERNEL);
 	if (!qp)
@@ -106,7 +128,7 @@ struct icc_provider *icc_clk_register(struct device *dev,
 
 	icc_provider_init(provider);
 
-	for (i = 0, j = 0; i < num_clocks; i++) {
+	for (i = 0; i < num_clocks; i++) {
 		qp->clocks[i].clk = data[i].clk;
 
 		node = icc_node_create(first_id + data[i].master_id);
@@ -126,7 +148,16 @@ struct icc_provider *icc_clk_register(struct device *dev,
 		icc_node_add(node, provider);
 		/* link to the next node, slave */
 		icc_link_create(node, first_id + data[i].slave_id);
-		onecell->nodes[j++] = node;
+		onecell->nodes[data[i].master_id] = node;
+
+		/*
+		 * Slave node may already be registered by an earlier
+		 * master in an N:1 topology (multiple masters sharing
+		 * the same slave, e.g. several CPU clusters voting on
+		 * one L3 clock).
+		 */
+		if (onecell->nodes[data[i].slave_id])
+			continue;
 
 		node = icc_node_create(first_id + data[i].slave_id);
 		if (IS_ERR(node)) {
@@ -143,7 +174,7 @@ struct icc_provider *icc_clk_register(struct device *dev,
 
 		/* no data for slave node */
 		icc_node_add(node, provider);
-		onecell->nodes[j++] = node;
+		onecell->nodes[data[i].slave_id] = node;
 	}
 
 	ret = icc_provider_register(provider);

-- 
2.34.1

[PATCH 2/4] clk: qcom: clk-alpha-pll: Skip Zonda PLL configuration if already enabled

[Kathiravan Thirumoorthy]

On IPQ9650, the bootloader configures and enables the APSS Zonda PLL
that provide the clocks for the A55 CPU cores and L3 interconnect. When
the APSS clock driver probes, it attempts to reconfigure the already
running PLLs. As part of the reconfiguration sequence, the PLL output is
disabled and the PLL is placed into standby before being re-enabled.
Since the CPUs are actively clocked from these PLLs, removing the clock
during this window causes the system to hang.

Avoid reconfiguring PLLs that are already enabled and return early,
similar to clk_trion_pll_configure().

Signed-off-by: Kathiravan Thirumoorthy <kathiravan.thirumoorthy@oss.qualcomm.com>
---
 drivers/clk/qcom/clk-alpha-pll.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/clk/qcom/clk-alpha-pll.c b/drivers/clk/qcom/clk-alpha-pll.c
index 60173b076cc5..3641b2da92cd 100644
--- a/drivers/clk/qcom/clk-alpha-pll.c
+++ b/drivers/clk/qcom/clk-alpha-pll.c
@@ -2162,6 +2162,10 @@ EXPORT_SYMBOL_GPL(clk_alpha_pll_postdiv_lucid_5lpe_ops);
 void clk_zonda_pll_configure(struct clk_alpha_pll *pll, struct regmap *regmap,
 			     const struct alpha_pll_config *config)
 {
+	/* Check if PLL is already enabled */
+	if (trion_pll_is_enabled(pll, regmap))
+		return;
+
 	clk_alpha_pll_write_config(regmap, PLL_L_VAL(pll), config->l);
 	clk_alpha_pll_write_config(regmap, PLL_ALPHA_VAL(pll), config->alpha);
 	clk_alpha_pll_write_config(regmap, PLL_CONFIG_CTL(pll), config->config_ctl_val);

-- 
2.34.1

[PATCH 3/4] dt-bindings: clock: ipq9650-apss-clk: Add IPQ9650 apss clock controller

[Kathiravan Thirumoorthy]

The APSS clock controller on IPQ9650 provides the CPU and L3 clocks and
has a register space separate from the GCC. The four Cortex-A55 cores,
the Cortex-A78 core and the L3 are each clocked by their own Zonda PLL,
selected through a glitch-free mux whose parents are XO, GPLL0 and the
corresponding PLL. The L3 needs to be scaled along with the CPUs, so the
controller is also an interconnect provider.

Add the binding, the clock indices for the Cortex-A78 (gold) cluster to
the shared APSS header, and the interconnect IDs for the CPU and L3
nodes.

Signed-off-by: Kathiravan Thirumoorthy <kathiravan.thirumoorthy@oss.qualcomm.com>
---
 .../bindings/clock/qcom,ipq9650-apss-clk.yaml      | 58 ++++++++++++++++++++++
 include/dt-bindings/clock/qcom,apss-ipq.h          |  5 ++
 include/dt-bindings/interconnect/qcom,ipq9650.h    |  4 ++
 3 files changed, 67 insertions(+)

diff --git a/Documentation/devicetree/bindings/clock/qcom,ipq9650-apss-clk.yaml b/Documentation/devicetree/bindings/clock/qcom,ipq9650-apss-clk.yaml
new file mode 100644
index 000000000000..6b47183edd56
--- /dev/null
+++ b/Documentation/devicetree/bindings/clock/qcom,ipq9650-apss-clk.yaml
@@ -0,0 +1,58 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/clock/qcom,ipq9650-apss-clk.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Qualcomm APSS IPQ9650 Clock Controller
+
+maintainers:
+  - Kathiravan Thirumoorthy <kathiravan.thirumoorthy@oss.qualcomm.com>
+
+description:
+  The APSS clock controller provides the CPU and L3 clocks on IPQ9650 and
+  has a register space separate from the GCC. The four Cortex-A55 cores,
+  the Cortex-A78 core and the L3 are each clocked by their own Zonda PLL.
+  Each clock is selected through a glitch-free mux (GFM) whose parents are
+  XO, GPLL0 and the corresponding PLL.
+
+properties:
+  compatible:
+    enum:
+      - qcom,ipq9650-apss-clk
+
+  reg:
+    maxItems: 1
+
+  clocks:
+    items:
+      - description: Reference to the XO clock.
+      - description: Reference to the GPLL0 clock.
+
+  '#clock-cells':
+    const: 1
+
+  '#interconnect-cells':
+    const: 1
+
+required:
+  - compatible
+  - reg
+  - clocks
+  - '#clock-cells'
+  - '#interconnect-cells'
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/clock/qcom,ipq9650-gcc.h>
+
+    apss_clk: clock-controller@fa80000 {
+      compatible = "qcom,ipq9650-apss-clk";
+      reg = <0x0fa80000 0x30000>;
+      clocks = <&xo_board>,
+               <&gcc GPLL0>;
+      #clock-cells = <1>;
+      #interconnect-cells = <1>;
+    };
diff --git a/include/dt-bindings/clock/qcom,apss-ipq.h b/include/dt-bindings/clock/qcom,apss-ipq.h
index 0bb41e5efdef..c9b04f2dbb6c 100644
--- a/include/dt-bindings/clock/qcom,apss-ipq.h
+++ b/include/dt-bindings/clock/qcom,apss-ipq.h
@@ -15,4 +15,9 @@
 #define L3_CLK_SRC				6
 #define L3_CORE_CLK				7
 
+/* IPQ9650 Gold cluster clock indices */
+#define APSS_GOLD_PLL				8
+#define APSS_GOLD_PLL_POSTDIV			9
+#define APSS_GOLD_CORE_CLK			10
+
 #endif
diff --git a/include/dt-bindings/interconnect/qcom,ipq9650.h b/include/dt-bindings/interconnect/qcom,ipq9650.h
index 023a3878cc08..1a77609de832 100644
--- a/include/dt-bindings/interconnect/qcom,ipq9650.h
+++ b/include/dt-bindings/interconnect/qcom,ipq9650.h
@@ -25,4 +25,8 @@
 #define MASTER_SNOC_USB			20
 #define SLAVE_SNOC_USB			21
 
+#define MASTER_CPU_SILVER		0
+#define SLAVE_L3			1
+#define MASTER_CPU_GOLD			2
+
 #endif /* INTERCONNECT_QCOM_IPQ9650_H */

-- 
2.34.1

[PATCH 4/4] clk: qcom: apss-ipq9650: Add IPQ9650 APSS clock controller driver

[Kathiravan Thirumoorthy]

The APSS clock controller on IPQ9650 provides the CPU and L3 clocks.
The four Cortex-A55 cores (silver), the Cortex-A78 core (gold) and the
L3 are each clocked by their own Zonda PLL. Each clock is selected
through a glitch-free mux (GFM) whose parents are XO, GPLL0 and the
corresponding PLL. The gold mux can also select the PLL's post-divider
output.

The L3 needs to be scaled along with the CPUs, so register an icc-clk
provider in which both CPU clusters vote on the L3 core clock.

Signed-off-by: Kathiravan Thirumoorthy <kathiravan.thirumoorthy@oss.qualcomm.com>
---
 drivers/clk/qcom/Kconfig        |   9 +
 drivers/clk/qcom/Makefile       |   1 +
 drivers/clk/qcom/apss-ipq9650.c | 385 ++++++++++++++++++++++++++++++++++++++++
 3 files changed, 395 insertions(+)

diff --git a/drivers/clk/qcom/Kconfig b/drivers/clk/qcom/Kconfig
index 27d0ab24d50a..a17f661c3102 100644
--- a/drivers/clk/qcom/Kconfig
+++ b/drivers/clk/qcom/Kconfig
@@ -599,6 +599,15 @@ config IPQ_APSS_6018
 	  Say Y if you want to support CPU frequency scaling on
 	  ipq based devices.
 
+config IPQ_APSS_9650
+	tristate "IPQ9650 APSS Clock Controller"
+	depends on ARM64 || COMPILE_TEST
+	default y if IPQ_GCC_9650
+	help
+	  Support for APSS Clock controller on Qualcomm IPQ9650 platform.
+	  Say Y if you want to support CPU frequency scaling on IPQ9650 based
+	  devices.
+
 config IPQ_CMN_PLL
 	tristate "IPQ CMN PLL Clock Controller"
 	depends on ARM64 || COMPILE_TEST
diff --git a/drivers/clk/qcom/Makefile b/drivers/clk/qcom/Makefile
index 6affb5073f26..ec3d8512a812 100644
--- a/drivers/clk/qcom/Makefile
+++ b/drivers/clk/qcom/Makefile
@@ -68,6 +68,7 @@ obj-$(CONFIG_CLK_QCM2290_GPUCC) += gpucc-qcm2290.o
 obj-$(CONFIG_IPQ_APSS_PLL) += apss-ipq-pll.o
 obj-$(CONFIG_IPQ_APSS_5424) += apss-ipq5424.o
 obj-$(CONFIG_IPQ_APSS_6018) += apss-ipq6018.o
+obj-$(CONFIG_IPQ_APSS_9650) += apss-ipq9650.o
 obj-$(CONFIG_IPQ_CMN_PLL) += ipq-cmn-pll.o
 obj-$(CONFIG_IPQ_GCC_4019) += gcc-ipq4019.o
 obj-$(CONFIG_IPQ_GCC_5018) += gcc-ipq5018.o
diff --git a/drivers/clk/qcom/apss-ipq9650.c b/drivers/clk/qcom/apss-ipq9650.c
new file mode 100644
index 000000000000..7602f3896fe3
--- /dev/null
+++ b/drivers/clk/qcom/apss-ipq9650.c
@@ -0,0 +1,385 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) Qualcomm Technologies, Inc. and/or its subsidiaries.
+ */
+
+#include <linux/clk-provider.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+#include <linux/interconnect-provider.h>
+
+#include <dt-bindings/clock/qcom,apss-ipq.h>
+#include <dt-bindings/interconnect/qcom,ipq9650.h>
+
+#include "clk-alpha-pll.h"
+#include "clk-branch.h"
+#include "clk-regmap.h"
+#include "common.h"
+
+enum {
+	DT_XO,
+	DT_GPLL0,
+};
+
+static const struct pll_vco silver_pll_vco[] = {
+	{ 816000000, 1968000000, 0 },
+};
+
+static const struct pll_vco gold_pll_vco[] = {
+	{ 696000000, 1992000000, 0 },
+};
+
+static const struct pll_vco l3_pll_vco[] = {
+	{ 696000000, 1512000000, 0 },
+};
+
+static const struct alpha_pll_config silver_apss_pll_config = {
+	.l = 0x23,
+	.config_ctl_val = 0x08200920,
+	.config_ctl_hi_val = 0x05008001,
+	.config_ctl_hi1_val = 0x04000000,
+	.user_ctl_val = 0x01000009,
+};
+
+static const struct alpha_pll_config gold_apss_pll_config = {
+	.l = 0x26,
+	.config_ctl_val = 0x08200920,
+	.config_ctl_hi_val = 0x05008001,
+	.config_ctl_hi1_val = 0x04000000,
+	.user_ctl_val = 0x01000009,
+};
+
+static const struct alpha_pll_config l3_pll_config = {
+	.l = 0x1e,
+	.config_ctl_val = 0x08200920,
+	.config_ctl_hi_val = 0x05008001,
+	.config_ctl_hi1_val = 0x04000000,
+	.user_ctl_val = 0x01000009,
+};
+
+static struct clk_alpha_pll silver_apss_pll = {
+	.offset = 0x0,
+	.config = &silver_apss_pll_config,
+	.vco_table = silver_pll_vco,
+	.num_vco = ARRAY_SIZE(silver_pll_vco),
+	.regs = clk_alpha_pll_regs[CLK_ALPHA_PLL_TYPE_ZONDA],
+	.flags = SUPPORTS_DYNAMIC_UPDATE,
+	.clkr = {
+		.enable_reg = 0x0,
+		.enable_mask = BIT(0),
+		.hw.init = &(struct clk_init_data){
+			.name = "silver_apss_pll",
+			.parent_data = &(const struct clk_parent_data){
+				.index = DT_XO,
+			},
+			.num_parents = 1,
+			.flags = CLK_IS_CRITICAL,
+			.ops = &clk_alpha_pll_zonda_ops,
+		},
+	},
+};
+
+static struct clk_alpha_pll gold_apss_pll = {
+	.offset = 0x10000,
+	.config = &gold_apss_pll_config,
+	.vco_table = gold_pll_vco,
+	.num_vco = ARRAY_SIZE(gold_pll_vco),
+	.regs = clk_alpha_pll_regs[CLK_ALPHA_PLL_TYPE_ZONDA],
+	.flags = SUPPORTS_DYNAMIC_UPDATE,
+	.clkr = {
+		.enable_reg = 0x10000,
+		.enable_mask = BIT(0),
+		.hw.init = &(struct clk_init_data){
+			.name = "gold_apss_pll",
+			.parent_data = &(const struct clk_parent_data){
+				.index = DT_XO,
+			},
+			.num_parents = 1,
+			.flags = CLK_IS_CRITICAL,
+			.ops = &clk_alpha_pll_zonda_ops,
+		},
+	},
+};
+
+static const struct clk_div_table gold_apss_pll_post_div_table[] = {
+	{ 0x0, 1 },
+	{ 0x1, 2 },
+	{ }
+};
+
+static struct clk_alpha_pll_postdiv gold_apss_pll_postdiv = {
+	.offset = 0x10000,
+	.post_div_shift = 8,
+	.post_div_table = gold_apss_pll_post_div_table,
+	.num_post_div = ARRAY_SIZE(gold_apss_pll_post_div_table),
+	.width = 2,
+	.regs = clk_alpha_pll_regs[CLK_ALPHA_PLL_TYPE_ZONDA],
+	.clkr.hw.init = &(struct clk_init_data){
+		.name = "gold_apss_pll_postdiv",
+		.parent_hws = (const struct clk_hw*[]){
+			&gold_apss_pll.clkr.hw,
+		},
+		.num_parents = 1,
+		.flags = CLK_SET_RATE_PARENT,
+		.ops = &clk_alpha_pll_postdiv_zonda_ops,
+	},
+};
+
+static struct clk_alpha_pll l3_pll = {
+	.offset = 0x20000,
+	.config = &l3_pll_config,
+	.vco_table = l3_pll_vco,
+	.num_vco = ARRAY_SIZE(l3_pll_vco),
+	.regs = clk_alpha_pll_regs[CLK_ALPHA_PLL_TYPE_ZONDA],
+	.flags = SUPPORTS_DYNAMIC_UPDATE,
+	.clkr = {
+		.enable_reg = 0x20000,
+		.enable_mask = BIT(0),
+		.hw.init = &(struct clk_init_data){
+			.name = "l3_pll",
+			.parent_data = &(const struct clk_parent_data){
+				.index = DT_XO,
+			},
+			.num_parents = 1,
+			.flags = CLK_IS_CRITICAL,
+			.ops = &clk_alpha_pll_zonda_ops,
+		},
+	},
+};
+
+struct clk_gfm {
+	u32 mux_reg;
+	u32 mux_mask;
+	struct clk_hw hw;
+	void __iomem *base;
+};
+
+#define to_clk_gfm(_hw) container_of(_hw, struct clk_gfm, hw)
+
+static u8 clk_gfm_get_parent(struct clk_hw *hw)
+{
+	struct clk_gfm *gfm = to_clk_gfm(hw);
+	u32 val;
+
+	val = readl(gfm->base + gfm->mux_reg);
+	return val & gfm->mux_mask;
+}
+
+static int clk_gfm_set_parent(struct clk_hw *hw, u8 index)
+{
+	struct clk_gfm *gfm = to_clk_gfm(hw);
+	u32 val;
+
+	val = readl(gfm->base + gfm->mux_reg);
+	val &= ~gfm->mux_mask;
+	val |= (index & gfm->mux_mask);
+	writel(val, gfm->base + gfm->mux_reg);
+
+	return 0;
+}
+
+static const struct clk_ops clk_gfm_ops = {
+	.get_parent = clk_gfm_get_parent,
+	.set_parent = clk_gfm_set_parent,
+	.determine_rate = __clk_mux_determine_rate_closest,
+};
+
+static struct clk_gfm silver_apss_pll_gfmux = {
+	.mux_reg = 0x84,
+	.mux_mask = 0xf,
+	.hw.init = &(struct clk_init_data) {
+		.name = "silver_apss_pll_gfmux",
+		.parent_data = (const struct clk_parent_data[]) {
+			{ .index = DT_XO },
+			{ .index = DT_GPLL0 },
+			{ .hw = &silver_apss_pll.clkr.hw },
+		},
+		.num_parents = 3,
+		.ops = &clk_gfm_ops,
+		.flags = CLK_SET_RATE_PARENT,
+	},
+};
+
+static struct clk_gfm gold_apss_pll_gfmux = {
+	.mux_reg = 0x10084,
+	.mux_mask = 0xf,
+	.hw.init = &(struct clk_init_data) {
+		.name = "gold_apss_pll_gfmux",
+		.parent_data = (const struct clk_parent_data[]) {
+			{ .index = DT_XO },
+			{ .index = DT_GPLL0 },
+			{ .hw = &gold_apss_pll.clkr.hw },
+			{ .hw = &gold_apss_pll_postdiv.clkr.hw },
+		},
+		.num_parents = 4,
+		.ops = &clk_gfm_ops,
+		.flags = CLK_SET_RATE_PARENT,
+	},
+};
+
+static struct clk_gfm l3_pll_gfmux = {
+	.mux_reg = 0x20084,
+	.mux_mask = 0xf,
+	.hw.init = &(struct clk_init_data) {
+		.name = "l3_pll_gfmux",
+		.parent_data = (const struct clk_parent_data[]) {
+			{ .index = DT_XO },
+			{ .index = DT_GPLL0 },
+			{ .hw = &l3_pll.clkr.hw },
+		},
+		.num_parents = 3,
+		.ops = &clk_gfm_ops,
+		.flags = CLK_SET_RATE_PARENT,
+	},
+};
+
+static struct clk_branch apss_silver_core_clk = {
+	.halt_reg = 0x8c,
+	.clkr = {
+		.enable_reg = 0x8c,
+		.enable_mask = BIT(0),
+		.hw.init = &(struct clk_init_data) {
+			.name = "apss_silver_clk",
+			.parent_hws = (const struct clk_hw*[]) {
+				&silver_apss_pll_gfmux.hw,
+			},
+			.num_parents = 1,
+			.flags = CLK_SET_RATE_PARENT | CLK_IS_CRITICAL,
+			.ops = &clk_branch2_ops,
+		},
+	},
+};
+
+static struct clk_branch apss_gold_core_clk = {
+	.halt_reg = 0x1008c,
+	.clkr = {
+		.enable_reg = 0x1008c,
+		.enable_mask = BIT(0),
+		.hw.init = &(struct clk_init_data) {
+			.name = "apss_gold_clk",
+			.parent_hws = (const struct clk_hw*[]) {
+				&gold_apss_pll_gfmux.hw,
+			},
+			.num_parents = 1,
+			.flags = CLK_SET_RATE_PARENT | CLK_IS_CRITICAL,
+			.ops = &clk_branch2_ops,
+		},
+	},
+};
+
+static struct clk_branch l3_core_clk = {
+	.halt_reg = 0x2008c,
+	.clkr = {
+		.enable_reg = 0x2008c,
+		.enable_mask = BIT(0),
+		.hw.init = &(struct clk_init_data) {
+			.name = "l3_core_clk",
+			.parent_hws = (const struct clk_hw*[]) {
+				&l3_pll_gfmux.hw,
+			},
+			.num_parents = 1,
+			.flags = CLK_SET_RATE_PARENT | CLK_IS_CRITICAL,
+			.ops = &clk_branch2_ops,
+		},
+	},
+};
+
+static struct clk_hw *apss_hws[] = {
+	&silver_apss_pll_gfmux.hw,
+	&l3_pll_gfmux.hw,
+	&gold_apss_pll_gfmux.hw,
+};
+
+static struct clk_regmap *apss_clks[] = {
+	[APSS_PLL_EARLY] = &silver_apss_pll.clkr,
+	[APSS_SILVER_CORE_CLK] = &apss_silver_core_clk.clkr,
+	[L3_PLL] = &l3_pll.clkr,
+	[L3_CORE_CLK] = &l3_core_clk.clkr,
+	[APSS_GOLD_PLL] = &gold_apss_pll.clkr,
+	[APSS_GOLD_PLL_POSTDIV] = &gold_apss_pll_postdiv.clkr,
+	[APSS_GOLD_CORE_CLK] = &apss_gold_core_clk.clkr,
+};
+
+static const struct regmap_config apss_regmap_config = {
+	.reg_bits = 32,
+	.reg_stride = 4,
+	.val_bits = 32,
+	.max_register = 0x2fffc,
+	.fast_io = true,
+};
+
+static struct clk_alpha_pll *ipq9650_apss_plls[] = {
+	&silver_apss_pll,
+	&gold_apss_pll,
+	&l3_pll,
+};
+
+static const struct qcom_cc_driver_data ipq9650_apss_driver_data = {
+	.alpha_plls = ipq9650_apss_plls,
+	.num_alpha_plls = ARRAY_SIZE(ipq9650_apss_plls),
+};
+
+#define IPQ_APPS_PLL_ID			(9650 * 3)	/* some unique value */
+
+static const struct qcom_icc_hws_data icc_cpu_l3[] = {
+	{ MASTER_CPU_SILVER, SLAVE_L3, L3_CORE_CLK },
+	{ MASTER_CPU_GOLD, SLAVE_L3, L3_CORE_CLK },
+};
+
+static const struct qcom_cc_desc apss_desc = {
+	.config = &apss_regmap_config,
+	.clks = apss_clks,
+	.num_clks = ARRAY_SIZE(apss_clks),
+	.icc_hws = icc_cpu_l3,
+	.num_icc_hws = ARRAY_SIZE(icc_cpu_l3),
+	.icc_first_node_id = IPQ_APPS_PLL_ID,
+	.clk_hws = apss_hws,
+	.num_clk_hws = ARRAY_SIZE(apss_hws),
+	.driver_data = &ipq9650_apss_driver_data,
+};
+
+static int apss_ipq9650_probe(struct platform_device *pdev)
+{
+	const struct qcom_cc_desc *desc = &apss_desc;
+	struct regmap *regmap;
+	struct clk_gfm *gfm;
+	void __iomem *base;
+	int i;
+
+	base = devm_platform_ioremap_resource(pdev, 0);
+	if (IS_ERR(base))
+		return PTR_ERR(base);
+
+	regmap = devm_regmap_init_mmio(&pdev->dev, base,
+				       &apss_regmap_config);
+	if (IS_ERR(regmap))
+		return PTR_ERR(regmap);
+
+	for (i = 0; i < desc->num_clk_hws; i++) {
+		gfm = to_clk_gfm(apss_hws[i]);
+		gfm->base = base;
+	}
+
+	return qcom_cc_really_probe(&pdev->dev, desc, regmap);
+}
+
+static const struct of_device_id apss_ipq9650_match_table[] = {
+	{ .compatible = "qcom,ipq9650-apss-clk" },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, apss_ipq9650_match_table);
+
+static struct platform_driver apss_ipq9650_driver = {
+	.probe = apss_ipq9650_probe,
+	.driver = {
+		.name = "qcom,ipq9650-apss-clk",
+		.of_match_table = apss_ipq9650_match_table,
+		.sync_state = icc_sync_state,
+	},
+};
+
+module_platform_driver(apss_ipq9650_driver);
+
+MODULE_DESCRIPTION("Qualcomm IPQ9650 APSS Clock Driver");
+MODULE_LICENSE("GPL");

-- 
2.34.1

Re: [PATCH 1/4] interconnect: icc-clk: support N:1 master-to-slave topology

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] icc: list corruption and data race in icc_clk_set() during concurrent unbind
- [Medium] icc: bypassed rate aggregation leaves shared clock at high rate
- [Medium] icc: missing error check for icc_link_create() in icc_clk_register()
--

commit fb3619debbc961d128e9125a7da752adeb391118
Author: Kathiravan Thirumoorthy <kathiravan.thirumoorthy@oss.qualcomm.com>

    interconnect: icc-clk: support N:1 master-to-slave topology

    icc_clk_register() assumes each icc_clk_data entry describes a unique
    master/slave pair and creates both nodes for every entry. When several
    masters share the same slave_id, the shared slave node is created and
    added again on each later entry. icc_node_create() returns the existing
    node, so icc_node_add() hits WARN_ON(node->provider):

[ ... ]

    This topology is needed on IPQ9650, where the silver and gold CPU
    clusters both vote on the single L3 clock through a shared SLAVE_L3
    node.
    
    The onecell table is also filled in registration order, so xlate only
    works when the node IDs happen to match that order. Size the table by
    the highest node ID, index it by ID, and skip slave creation when the
    node is already registered.
    
    icc_clk_set() also passes only the requesting node's bandwidth to
    clk_set_rate(), so the last writer wins. A low vote from one master can
    then lower the clock below what another master asked for. Before setting
    the rate, take the highest peak bandwidth across all nodes that share
    the clock.

> diff --git a/drivers/interconnect/icc-clk.c b/drivers/interconnect/icc-clk.c
> index 93c030608d3e0..567d071fade8e 100644
> --- a/drivers/interconnect/icc-clk.c
> +++ b/drivers/interconnect/icc-clk.c
> @@ -24,7 +24,10 @@ struct icc_clk_provider {
>  
>  static int icc_clk_set(struct icc_node *src, struct icc_node *dst)
>  {
> +	unsigned long rate = icc_units_to_bps(src->peak_bw);
> +	struct icc_provider *provider = src->provider;
>  	struct icc_clk_node *qn = src->data;
> +	struct icc_node *node;
>  	int ret;
>  
>  	if (!qn || !qn->clk)
>  		return 0;
>  
>  	if (!src->peak_bw) {
>  		if (qn->enabled)
>  			clk_disable_unprepare(qn->clk);
>  		qn->enabled = false;
>  
>  		return 0;
>  	}

[Severity: Medium]
Will this early return circumvent the newly added aggregation logic when a
master sets its bandwidth to zero?

When multiple masters share the same clock in an N:1 topology, bypassing the
rate aggregation loop due to an early return causes the underlying shared
clock to remain running at the previously requested high rate instead of
stepping down to the maximum of the remaining masters' votes. The clock stays
unnecessarily high until another node updates its vote.

>  
>  	if (!qn->enabled) {
>  		ret = clk_prepare_enable(qn->clk);
>  		if (ret)
>  			return ret;
>  		qn->enabled = true;
>  	}
>  
> -	return clk_set_rate(qn->clk, icc_units_to_bps(src->peak_bw));
> +	/*
> +	 * Multiple master nodes can share the same underlying clock (N:1
> +	 * topology, e.g. several CPU clusters voting on one L3 clock).
> +	 * Aggregate the peak bandwidth across all of them before setting
> +	 * the rate, otherwise the last caller wins and can undervote what
> +	 * another master already asked for.
> +	 */
> +	list_for_each_entry(node, &provider->nodes, node_list) {

[Severity: High]
Does this list iteration race with concurrent unbind?

icc_clk_set() executes as the provider->set callback holding only icc_bw_lock.
Concurrently, if an interconnect provider is unbound via sysfs,
icc_node_del() removes nodes from provider->nodes using list_del() while
holding only icc_lock:

drivers/interconnect/core.c:icc_node_del() {
    mutex_lock(&icc_lock);
    list_del(&node->node_list);
    mutex_unlock(&icc_lock);
}

Because the locks are disjoint, this list traversal can race with list_del(),
leading to iteration over poisoned pointers.

> +		struct icc_clk_node *n = node->data;
> +
> +		if (n && clk_is_match(n->clk, qn->clk))
> +			rate = max(rate, icc_units_to_bps(node->peak_bw));
> +	}
> +
> +	return clk_set_rate(qn->clk, rate);
>  }

[ ... ]

> @@ -126,7 +148,16 @@ struct icc_provider *icc_clk_register(struct device *dev,
>  		icc_node_add(node, provider);
>  		/* link to the next node, slave */
>  		icc_link_create(node, first_id + data[i].slave_id);
> -		onecell->nodes[j++] = node;
> +		onecell->nodes[data[i].master_id] = node;

[Severity: Medium]
Should the return value of icc_link_create() be checked here?

If icc_link_create() fails to allocate memory for the links, it returns
-ENOMEM. Because this return value is ignored, the interconnect graph links
might silently fail to form. This can break the master-to-slave interconnect
topology, leading to ignored bandwidth requests while driver initialization
appears to succeed.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261007-ipq9650_apss_clk-v1-0-fd4c2b3cbdea@oss.qualcomm.com?part=1

Re: [PATCH 2/4] clk: qcom: clk-alpha-pll: Skip Zonda PLL configuration if already enabled

[Abel Vesa]

On 26-10-07 12:20:45, Kathiravan Thirumoorthy wrote:
> On IPQ9650, the bootloader configures and enables the APSS Zonda PLL
> that provide the clocks for the A55 CPU cores and L3 interconnect. When
> the APSS clock driver probes, it attempts to reconfigure the already
> running PLLs. As part of the reconfiguration sequence, the PLL output is
> disabled and the PLL is placed into standby before being re-enabled.
> Since the CPUs are actively clocked from these PLLs, removing the clock
> during this window causes the system to hang.
> 
> Avoid reconfiguring PLLs that are already enabled and return early,
> similar to clk_trion_pll_configure().
> 
> Signed-off-by: Kathiravan Thirumoorthy <kathiravan.thirumoorthy@oss.qualcomm.com>

Reviewed-by: Abel Vesa <abel.vesa@oss.qualcomm.com>