[PATCH 0/4] Enable cpufreq for IPQ5424

[PATCH 0/4] Enable cpufreq for IPQ5424

[Sricharan R]

From: Sricharan Ramabadhran <quic_srichara@quicinc.com>

CPU on Qualcomm ipq5424 is clocked by huayra PLL with RCG support.
Add support for the APSS PLL, RCG and clock enable for ipq5424.
The PLL, RCG register space are clubbed. Hence adding new APSS driver
for both PLL and RCG/CBC control. Also the L3 cache has a separate pll
and needs to be scaled along with the CPU.

Md Sadre Alam (1):
  cpufreq: qcom-nvmem: Enable cpufreq for ipq5424

Sricharan Ramabadhran (3):
  dt-bindings: clock: ipq5424-apss-clk: Add ipq5424 apss clock
    controller
  clk: qcom: apss-ipq5424: Add ipq5424 apss clock controller
  arm64: dts: qcom: ipq5424: Enable cpufreq support

 .../bindings/clock/qcom,ipq5424-apss-clk.yaml |  57 +++
 arch/arm64/boot/dts/qcom/ipq5424.dtsi         |  71 ++++
 drivers/clk/qcom/Kconfig                      |   7 +
 drivers/clk/qcom/Makefile                     |   1 +
 drivers/clk/qcom/apss-ipq5424.c               | 373 ++++++++++++++++++
 drivers/cpufreq/cpufreq-dt-platdev.c          |   1 +
 drivers/cpufreq/qcom-cpufreq-nvmem.c          |   5 +
 include/dt-bindings/clock/qcom,apss-ipq.h     |   6 +
 8 files changed, 521 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/clock/qcom,ipq5424-apss-clk.yaml
 create mode 100644 drivers/clk/qcom/apss-ipq5424.c

-- 
2.34.1

[PATCH 1/4] dt-bindings: clock: ipq5424-apss-clk: Add ipq5424 apss clock controller

[Sricharan R]

From: Sricharan Ramabadhran <quic_srichara@quicinc.com>

The CPU core in ipq5424 is clocked by a huayra PLL with RCG support.
The RCG and PLL have a separate register space from the GCC.
Also the L3 cache has a separate pll and needs to be scaled along
with the CPU.

Co-developed-by: Md Sadre Alam <quic_mdalam@quicinc.com>
Signed-off-by: Md Sadre Alam <quic_mdalam@quicinc.com>
Signed-off-by: Sricharan Ramabadhran <quic_srichara@quicinc.com>
---
 .../bindings/clock/qcom,ipq5424-apss-clk.yaml | 57 +++++++++++++++++++
 include/dt-bindings/clock/qcom,apss-ipq.h     |  6 ++
 2 files changed, 63 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/clock/qcom,ipq5424-apss-clk.yaml

diff --git a/Documentation/devicetree/bindings/clock/qcom,ipq5424-apss-clk.yaml b/Documentation/devicetree/bindings/clock/qcom,ipq5424-apss-clk.yaml
new file mode 100644
index 000000000000..df7cfb82bac3
--- /dev/null
+++ b/Documentation/devicetree/bindings/clock/qcom,ipq5424-apss-clk.yaml
@@ -0,0 +1,57 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/clock/qcom,ipq5424-apss-clk.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Qualcomm APSS IPQ5424 Clock Controller
+
+maintainers:
+  - Sricharan Ramabadhran <quic_srichara@quicinc.com>
+  - Md Sadre Alam <quic_mdalam@quicinc.com>
+
+description: |
+  The CPU core in ipq5424 is clocked by a huayra PLL with RCG support.
+  The RCG and PLL have a separate register space from the GCC.
+
+properties:
+  compatible:
+    enum:
+      - qcom,ipq5424-apss-clk
+  reg:
+    maxItems: 1
+
+  clocks:
+    items:
+      - description: Reference to the XO clock.
+      - description: Reference to the GPLL0 clock.
+
+  clock-names:
+    items:
+      - const: xo
+      - const: gpll0
+
+  '#clock-cells':
+    const: 1
+
+required:
+  - compatible
+  - reg
+  - clocks
+  - clock-names
+  - '#clock-cells'
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/clock/qcom,ipq5424-gcc.h>
+
+    apss_clk: apss-clock@fa80000 {
+      compatible = "qcom,ipq5424-apss-clk";
+      reg = <0x0fa80000 0x20000>;
+      clocks = <&xo_board>, <&gcc GPLL0>;
+      clock-names = "xo", "gpll0";
+      #clock-cells = <1>;
+    };
+
diff --git a/include/dt-bindings/clock/qcom,apss-ipq.h b/include/dt-bindings/clock/qcom,apss-ipq.h
index 77b6e05492e2..0bb41e5efdef 100644
--- a/include/dt-bindings/clock/qcom,apss-ipq.h
+++ b/include/dt-bindings/clock/qcom,apss-ipq.h
@@ -8,5 +8,11 @@
 
 #define APCS_ALIAS0_CLK_SRC			0
 #define APCS_ALIAS0_CORE_CLK			1
+#define APSS_PLL_EARLY				2
+#define APSS_SILVER_CLK_SRC			3
+#define APSS_SILVER_CORE_CLK			4
+#define L3_PLL					5
+#define L3_CLK_SRC				6
+#define L3_CORE_CLK				7
 
 #endif
-- 
2.34.1

[PATCH 2/4] clk: qcom: apss-ipq5424: Add ipq5424 apss clock controller

[Sricharan R]

From: Sricharan Ramabadhran <quic_srichara@quicinc.com>

CPU on Qualcomm ipq5424 is clocked by huayra PLL with RCG support.
Add support for the APSS PLL, RCG and clock enable for ipq5424.
The PLL, RCG register space are clubbed. Hence adding new APSS driver
for both PLL and RCG/CBC control. Also the L3 cache has a separate pll
and needs to be scaled along with the CPU.

Co-developed-by: Md Sadre Alam <quic_mdalam@quicinc.com>
Signed-off-by: Md Sadre Alam <quic_mdalam@quicinc.com>
Signed-off-by: Sricharan Ramabadhran <quic_srichara@quicinc.com>
---
 drivers/clk/qcom/Kconfig        |   7 +
 drivers/clk/qcom/Makefile       |   1 +
 drivers/clk/qcom/apss-ipq5424.c | 373 ++++++++++++++++++++++++++++++++
 3 files changed, 381 insertions(+)
 create mode 100644 drivers/clk/qcom/apss-ipq5424.c

diff --git a/drivers/clk/qcom/Kconfig b/drivers/clk/qcom/Kconfig
index ef89d686cbc4..9a03257d67e0 100644
--- a/drivers/clk/qcom/Kconfig
+++ b/drivers/clk/qcom/Kconfig
@@ -190,6 +190,13 @@ config IPQ_APSS_6018
 	  Say Y if you want to support CPU frequency scaling on
 	  ipq based devices.
 
+config IPQ_APSS_5424
+	tristate "IPQ APSS Clock Controller"
+	help
+	  Support for APSS Clock controller on Qualcom IPQ5424 platform.
+	  Say Y if you want to support CPU frequency scaling on ipq based
+	  devices.
+
 config IPQ_GCC_4019
 	tristate "IPQ4019 Global Clock Controller"
 	help
diff --git a/drivers/clk/qcom/Makefile b/drivers/clk/qcom/Makefile
index b09dbdc210eb..db15514e7367 100644
--- a/drivers/clk/qcom/Makefile
+++ b/drivers/clk/qcom/Makefile
@@ -28,6 +28,7 @@ obj-$(CONFIG_CLK_X1E80100_GPUCC) += gpucc-x1e80100.o
 obj-$(CONFIG_CLK_X1E80100_TCSRCC) += tcsrcc-x1e80100.o
 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_GCC_4019) += gcc-ipq4019.o
 obj-$(CONFIG_IPQ_GCC_5018) += gcc-ipq5018.o
diff --git a/drivers/clk/qcom/apss-ipq5424.c b/drivers/clk/qcom/apss-ipq5424.c
new file mode 100644
index 000000000000..2bd6ee7575dc
--- /dev/null
+++ b/drivers/clk/qcom/apss-ipq5424.c
@@ -0,0 +1,373 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2018, The Linux Foundation. All rights reserved.
+ * Copyright (c) 2024, Qualcomm Innovation Center, Inc. All rights reserved.
+ */
+
+#include <linux/clk.h>
+#include <linux/clk-provider.h>
+#include <linux/err.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+
+#include <dt-bindings/clock/qcom,apss-ipq.h>
+#include <dt-bindings/arm/qcom,ids.h>
+
+#include "clk-alpha-pll.h"
+#include "clk-branch.h"
+#include "clk-rcg.h"
+#include "clk-regmap.h"
+#include "common.h"
+
+#define GPLL0_CLK_RATE		800000000
+#define CPU_NOM_CLK_RATE	1416000000
+#define CPU_TURBO_CLK_RATE	1800000000
+#define L3_NOM_CLK_RATE		984000000
+#define L3_TURBO_CLK_RATE	1272000000
+
+enum {
+	P_XO,
+	P_GPLL0,
+	P_APSS_PLL_EARLY,
+	P_L3_PLL,
+};
+
+struct apss_clk {
+	struct notifier_block cpu_clk_notifier;
+	struct clk_hw *hw;
+	struct device *dev;
+	struct clk *l3_clk;
+};
+
+/*
+ * IPQ5424 Huayra PLL offsets are different from the one mentioned in the
+ * clk-alpha-pll.c, hence define the IPQ5424 offsets here
+ */
+static const u8 ipq5424_pll_offsets[][PLL_OFF_MAX_REGS] = {
+	[CLK_ALPHA_PLL_TYPE_HUAYRA] =  {
+		[PLL_OFF_L_VAL] = 0x04,
+		[PLL_OFF_ALPHA_VAL] = 0x08,
+		[PLL_OFF_USER_CTL] = 0x0c,
+		[PLL_OFF_CONFIG_CTL] = 0x10,
+		[PLL_OFF_CONFIG_CTL_U] = 0x14,
+		[PLL_OFF_CONFIG_CTL_U1] = 0x18,
+		[PLL_OFF_TEST_CTL] = 0x1c,
+		[PLL_OFF_TEST_CTL_U] = 0x20,
+		[PLL_OFF_TEST_CTL_U1] = 0x24,
+		[PLL_OFF_STATUS] = 0x38,
+	},
+};
+
+static struct clk_alpha_pll ipq5424_apss_pll = {
+	.offset = 0x0,
+	.regs = ipq5424_pll_offsets[CLK_ALPHA_PLL_TYPE_HUAYRA],
+	.flags = SUPPORTS_DYNAMIC_UPDATE,
+	.clkr = {
+		.enable_reg = 0x0,
+		.enable_mask = BIT(0),
+		.hw.init = &(struct clk_init_data){
+			.name = "apss_pll",
+			.parent_data = &(const struct clk_parent_data) {
+				.fw_name = "xo-board-clk",
+			},
+			.parent_names = (const char *[]){ "xo-board-clk"},
+			.num_parents = 1,
+			.ops = &clk_alpha_pll_huayra_ops,
+		},
+	},
+};
+
+static const struct clk_parent_data parents_apss_silver_clk_src[] = {
+	{ .fw_name = "xo-board-clk" },
+	{ .fw_name = "gpll0" },
+	{ .hw = &ipq5424_apss_pll.clkr.hw },
+};
+
+static const struct parent_map parents_apss_silver_clk_src_map[] = {
+	{ P_XO, 0 },
+	{ P_GPLL0, 4 },
+	{ P_APSS_PLL_EARLY, 5 },
+};
+
+static const struct freq_tbl ftbl_apss_clk_src[] = {
+	F(GPLL0_CLK_RATE, P_GPLL0, 1, 0, 0),
+	F(CPU_NOM_CLK_RATE, P_APSS_PLL_EARLY, 1, 0, 0),
+	F(CPU_TURBO_CLK_RATE, P_APSS_PLL_EARLY, 1, 0, 0),
+	{ }
+};
+
+static struct clk_rcg2 apss_silver_clk_src = {
+	.cmd_rcgr = 0x0080,
+	.freq_tbl = ftbl_apss_clk_src,
+	.hid_width = 5,
+	.parent_map = parents_apss_silver_clk_src_map,
+	.clkr.hw.init = &(struct clk_init_data){
+		.name = "apss_silver_clk_src",
+		.parent_data = parents_apss_silver_clk_src,
+		.num_parents = ARRAY_SIZE(parents_apss_silver_clk_src),
+		.ops = &clk_rcg2_ops,
+		.flags = CLK_SET_RATE_PARENT,
+	},
+};
+
+static struct clk_branch apss_silver_core_clk = {
+	.halt_reg = 0x008c,
+	.clkr = {
+		.enable_reg = 0x008c,
+		.enable_mask = BIT(0),
+		.hw.init = &(struct clk_init_data){
+			.name = "apss_silver_core_clk",
+			.parent_hws = (const struct clk_hw *[]){
+				&apss_silver_clk_src.clkr.hw },
+			.num_parents = 1,
+			.flags = CLK_SET_RATE_PARENT | CLK_IS_CRITICAL,
+			.ops = &clk_branch2_ops,
+		},
+	},
+};
+
+static struct clk_alpha_pll ipq5424_l3_pll = {
+	.offset = 0x10000,
+	.regs = ipq5424_pll_offsets[CLK_ALPHA_PLL_TYPE_HUAYRA],
+	.flags = SUPPORTS_DYNAMIC_UPDATE,
+	.clkr = {
+		.enable_reg = 0x0,
+		.enable_mask = BIT(0),
+		.hw.init = &(struct clk_init_data){
+			.name = "l3_pll",
+			.parent_data = &(const struct clk_parent_data) {
+				.fw_name = "xo-board-clk",
+			},
+			.parent_names = (const char *[]){ "xo-board-clk"},
+			.num_parents = 1,
+			.ops = &clk_alpha_pll_huayra_ops,
+		},
+	},
+};
+
+static const struct clk_parent_data parents_l3_clk_src[] = {
+	{ .fw_name = "xo-board-clk" },
+	{ .fw_name = "gpll0" },
+	{ .hw = &ipq5424_l3_pll.clkr.hw },
+};
+
+static const struct parent_map parents_l3_clk_src_map[] = {
+	{ P_XO, 0 },
+	{ P_GPLL0, 4 },
+	{ P_L3_PLL, 5 },
+};
+
+static const struct freq_tbl ftbl_l3_clk_src[] = {
+	F(GPLL0_CLK_RATE, P_GPLL0, 1, 0, 0),
+	F(L3_NOM_CLK_RATE, P_L3_PLL, 1, 0, 0),
+	F(L3_TURBO_CLK_RATE, P_L3_PLL, 1, 0, 0),
+	{ }
+};
+
+static struct clk_rcg2 l3_clk_src = {
+	.cmd_rcgr = 0x10080,
+	.freq_tbl = ftbl_l3_clk_src,
+	.hid_width = 5,
+	.parent_map = parents_l3_clk_src_map,
+	.clkr.hw.init = &(struct clk_init_data){
+		.name = "l3_clk_src",
+		.parent_data = parents_l3_clk_src,
+		.num_parents = ARRAY_SIZE(parents_l3_clk_src),
+		.ops = &clk_rcg2_ops,
+		.flags = CLK_SET_RATE_PARENT,
+	},
+};
+
+static struct clk_branch l3_core_clk = {
+	.halt_reg = 0x1008c,
+	.clkr = {
+		.enable_reg = 0x1008c,
+		.enable_mask = BIT(0),
+		.hw.init = &(struct clk_init_data){
+			.name = "l3_clk",
+			.parent_hws = (const struct clk_hw *[]){
+				&l3_clk_src.clkr.hw },
+			.num_parents = 1,
+			.flags = CLK_SET_RATE_PARENT | CLK_IS_CRITICAL,
+			.ops = &clk_branch2_ops,
+		},
+	},
+};
+
+static const struct regmap_config apss_ipq5424_regmap_config = {
+	.reg_bits       = 32,
+	.reg_stride     = 4,
+	.val_bits       = 32,
+	.max_register   = 0x20000,
+	.fast_io        = true,
+};
+
+static struct clk_regmap *apss_ipq5424_clks[] = {
+	[APSS_PLL_EARLY] = &ipq5424_apss_pll.clkr,
+	[APSS_SILVER_CLK_SRC] = &apss_silver_clk_src.clkr,
+	[APSS_SILVER_CORE_CLK] = &apss_silver_core_clk.clkr,
+	[L3_PLL] = &ipq5424_l3_pll.clkr,
+	[L3_CLK_SRC] = &l3_clk_src.clkr,
+	[L3_CORE_CLK] = &l3_core_clk.clkr,
+
+};
+
+static const struct qcom_cc_desc apss_ipq5424_desc = {
+	.config = &apss_ipq5424_regmap_config,
+	.clks = apss_ipq5424_clks,
+	.num_clks = ARRAY_SIZE(apss_ipq5424_clks),
+};
+
+static const struct alpha_pll_config apss_pll_config = {
+	.l = 0x3b,
+	.config_ctl_val = 0x08200920,
+	.config_ctl_hi_val = 0x05008001,
+	.config_ctl_hi1_val = 0x04000000,
+	.test_ctl_val = 0x0,
+	.test_ctl_hi_val = 0x0,
+	.test_ctl_hi1_val = 0x0,
+	.user_ctl_val = 0x1,
+	.early_output_mask = BIT(3),
+	.aux2_output_mask = BIT(2),
+	.aux_output_mask = BIT(1),
+	.main_output_mask = BIT(0),
+};
+
+static const struct alpha_pll_config l3_pll_config = {
+	.l = 0x29,
+	.config_ctl_val = 0x08200920,
+	.config_ctl_hi_val = 0x05008001,
+	.config_ctl_hi1_val = 0x04000000,
+	.test_ctl_val = 0x0,
+	.test_ctl_hi_val = 0x0,
+	.test_ctl_hi1_val = 0x0,
+	.user_ctl_val = 0x1,
+	.early_output_mask = BIT(3),
+	.aux2_output_mask = BIT(2),
+	.aux_output_mask = BIT(1),
+	.main_output_mask = BIT(0),
+};
+
+static unsigned long get_l3_clk_from_tbl(unsigned long rate)
+{
+	struct clk_rcg2 *l3_rcg2 = container_of(&l3_clk_src.clkr, struct clk_rcg2, clkr);
+	u8 max_clk = sizeof(ftbl_apss_clk_src) / sizeof(struct freq_tbl);
+	u8 loop;
+
+	for (loop = 0; loop < max_clk; loop++)
+		if (ftbl_apss_clk_src[loop].freq == rate)
+			return l3_rcg2->freq_tbl[loop].freq;
+	return 0;
+}
+
+static int cpu_clk_notifier_fn(struct notifier_block *nb, unsigned long action,
+			       void *data)
+{
+	struct apss_clk *apss_ipq5424_cfg = container_of(nb, struct apss_clk, cpu_clk_notifier);
+	struct clk_notifier_data *cnd = (struct clk_notifier_data *)data;
+	struct device *dev = apss_ipq5424_cfg->dev;
+	unsigned long rate = 0, l3_rate;
+	int err = 0;
+
+	dev_dbg(dev, "action:%ld old_rate:%ld new_rate:%ld\n", action,
+		cnd->old_rate, cnd->new_rate);
+
+	switch (action) {
+	case PRE_RATE_CHANGE:
+		if (cnd->old_rate < cnd->new_rate)
+			rate = cnd->new_rate;
+	break;
+	case POST_RATE_CHANGE:
+		if (cnd->old_rate > cnd->new_rate)
+			rate = cnd->new_rate;
+	break;
+	};
+
+	if (!rate)
+		goto notif_ret;
+
+	l3_rate = get_l3_clk_from_tbl(rate);
+	if (!l3_rate) {
+		dev_err(dev, "Failed to get l3 clock rate from l3_tbl\n");
+		return NOTIFY_BAD;
+	}
+
+	err = clk_set_rate(apss_ipq5424_cfg->l3_clk, l3_rate);
+	if (err) {
+		dev_err(dev, "Failed to set l3 clock rate(%ld) err(%d)\n", l3_rate, err);
+		return NOTIFY_BAD;
+	}
+
+notif_ret:
+	return NOTIFY_OK;
+}
+
+static int apss_ipq5424_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct apss_clk *apss_ipq5424_cfg;
+	struct regmap *regmap;
+	void __iomem *base;
+	int ret;
+
+	apss_ipq5424_cfg = devm_kzalloc(&pdev->dev, sizeof(struct apss_clk), GFP_KERNEL);
+	if (IS_ERR_OR_NULL(apss_ipq5424_cfg))
+		return PTR_ERR(apss_ipq5424_cfg);
+
+	base = devm_platform_ioremap_resource(pdev, 0);
+	if (IS_ERR(base))
+		return PTR_ERR(base);
+
+	regmap = devm_regmap_init_mmio(dev, base, &apss_ipq5424_regmap_config);
+	if (!regmap)
+		return PTR_ERR(regmap);
+
+	clk_alpha_pll_configure(&ipq5424_l3_pll, regmap, &l3_pll_config);
+
+	clk_alpha_pll_configure(&ipq5424_apss_pll, regmap, &apss_pll_config);
+
+	ret = qcom_cc_really_probe(dev, &apss_ipq5424_desc, regmap);
+	if (ret)
+		return ret;
+
+	dev_dbg(&pdev->dev, "Registered APSS & L3 clock provider\n");
+
+	apss_ipq5424_cfg->dev = dev;
+	apss_ipq5424_cfg->hw = &apss_silver_clk_src.clkr.hw;
+	apss_ipq5424_cfg->cpu_clk_notifier.notifier_call = cpu_clk_notifier_fn;
+
+	apss_ipq5424_cfg->l3_clk = clk_hw_get_clk(&l3_core_clk.clkr.hw, "l3_clk");
+	if (IS_ERR(apss_ipq5424_cfg->l3_clk)) {
+		dev_err(&pdev->dev, "Failed to get L3 clk, %ld\n",
+			PTR_ERR(apss_ipq5424_cfg->l3_clk));
+		return PTR_ERR(apss_ipq5424_cfg->l3_clk);
+	}
+
+	ret = devm_clk_notifier_register(&pdev->dev, apss_ipq5424_cfg->hw->clk,
+					 &apss_ipq5424_cfg->cpu_clk_notifier);
+	if (ret)
+		return ret;
+
+	return 0;
+}
+
+static const struct of_device_id apss_ipq5424_match_table[] = {
+	{ .compatible = "qcom,ipq5424-apss-clk" },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, apss_ipq5424_match_table);
+
+static struct platform_driver apss_ipq5424_driver = {
+	.probe = apss_ipq5424_probe,
+	.driver = {
+		.name   = "apss-ipq5424-clk",
+		.of_match_table = apss_ipq5424_match_table,
+	},
+};
+
+module_platform_driver(apss_ipq5424_driver);
+
+MODULE_DESCRIPTION("QCOM APSS IPQ5424 CLK Driver");
+MODULE_LICENSE("GPL v2");
-- 
2.34.1

[PATCH 3/4] cpufreq: qcom-nvmem: Enable cpufreq for ipq5424

[Sricharan R]

From: Md Sadre Alam <quic_mdalam@quicinc.com>

IPQ5424 have different OPPs available for the CPU based on
SoC variant. This can be determined through use of an eFuse
register present in the silicon.

Added support for ipq5424 on nvmem driver which helps to
determine OPPs at runtime based on the eFuse register which
has the CPU frequency limits. opp-supported-hw dt binding
can be used to indicate the available OPPs for each limit.

nvmem driver also creates the "cpufreq-dt" platform_device after
passing the version matching data to the OPP framework so that the
cpufreq-dt handles the actual cpufreq implementation.

Signed-off-by: Md Sadre Alam <quic_mdalam@quicinc.com>
Signed-off-by: Sricharan Ramabadhran <quic_srichara@quicinc.com>
---
 drivers/cpufreq/cpufreq-dt-platdev.c | 1 +
 drivers/cpufreq/qcom-cpufreq-nvmem.c | 5 +++++
 2 files changed, 6 insertions(+)

diff --git a/drivers/cpufreq/cpufreq-dt-platdev.c b/drivers/cpufreq/cpufreq-dt-platdev.c
index 9c198bd4f7e9..4045bc3ce805 100644
--- a/drivers/cpufreq/cpufreq-dt-platdev.c
+++ b/drivers/cpufreq/cpufreq-dt-platdev.c
@@ -187,6 +187,7 @@ static const struct of_device_id blocklist[] __initconst = {
 	{ .compatible = "ti,am62p5", },
 
 	{ .compatible = "qcom,ipq5332", },
+	{ .compatible = "qcom,ipq5424", },
 	{ .compatible = "qcom,ipq6018", },
 	{ .compatible = "qcom,ipq8064", },
 	{ .compatible = "qcom,ipq8074", },
diff --git a/drivers/cpufreq/qcom-cpufreq-nvmem.c b/drivers/cpufreq/qcom-cpufreq-nvmem.c
index 3a8ed723a23e..102f7f1b031c 100644
--- a/drivers/cpufreq/qcom-cpufreq-nvmem.c
+++ b/drivers/cpufreq/qcom-cpufreq-nvmem.c
@@ -200,6 +200,10 @@ static int qcom_cpufreq_kryo_name_version(struct device *cpu_dev,
 	case QCOM_ID_IPQ9574:
 		drv->versions = 1 << (unsigned int)(*speedbin);
 		break;
+	case QCOM_ID_IPQ5424:
+	case QCOM_ID_IPQ5404:
+		drv->versions =  (*speedbin != 0x3b) ? BIT(0) : BIT(1);
+		break;
 	case QCOM_ID_MSM8996SG:
 	case QCOM_ID_APQ8096SG:
 		drv->versions = 1 << ((unsigned int)(*speedbin) + 4);
@@ -591,6 +595,7 @@ static const struct of_device_id qcom_cpufreq_match_list[] __initconst __maybe_u
 	{ .compatible = "qcom,msm8996", .data = &match_data_kryo },
 	{ .compatible = "qcom,qcs404", .data = &match_data_qcs404 },
 	{ .compatible = "qcom,ipq5332", .data = &match_data_kryo },
+	{ .compatible = "qcom,ipq5424", .data = &match_data_kryo },
 	{ .compatible = "qcom,ipq6018", .data = &match_data_ipq6018 },
 	{ .compatible = "qcom,ipq8064", .data = &match_data_ipq8064 },
 	{ .compatible = "qcom,ipq8074", .data = &match_data_ipq8074 },
-- 
2.34.1

[PATCH 4/4] arm64: dts: qcom: ipq5424: Enable cpufreq support

[Sricharan R]

From: Sricharan Ramabadhran <quic_srichara@quicinc.com>

Add the qfprom, cpu clocks, A53 PLL and cpu-opp-table required for
CPU clock scaling.

Signed-off-by: Sricharan Ramabadhran <quic_srichara@quicinc.com>
---
 arch/arm64/boot/dts/qcom/ipq5424.dtsi | 71 +++++++++++++++++++++++++++
 1 file changed, 71 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/ipq5424.dtsi b/arch/arm64/boot/dts/qcom/ipq5424.dtsi
index 577b88cd5172..3c07f7c28c4a 100644
--- a/arch/arm64/boot/dts/qcom/ipq5424.dtsi
+++ b/arch/arm64/boot/dts/qcom/ipq5424.dtsi
@@ -7,6 +7,7 @@
  */
 
 #include <dt-bindings/interrupt-controller/arm-gic.h>
+#include <dt-bindings/clock/qcom,apss-ipq.h>
 #include <dt-bindings/clock/qcom,ipq5424-gcc.h>
 #include <dt-bindings/reset/qcom,ipq5424-gcc.h>
 #include <dt-bindings/gpio/gpio.h>
@@ -38,6 +39,11 @@ cpu0: cpu@0 {
 			reg = <0x0>;
 			enable-method = "psci";
 			next-level-cache = <&l2_0>;
+			clocks = <&apss_clk APSS_SILVER_CORE_CLK>,
+				 <&apss_clk L3_CORE_CLK>;
+			clock-names = "cpu", "l3_core";
+			operating-points-v2 = <&cpu_opp_table>;
+
 			l2_0: l2-cache {
 				compatible = "cache";
 				cache-level = <2>;
@@ -58,6 +64,10 @@ cpu1: cpu@100 {
 			enable-method = "psci";
 			reg = <0x100>;
 			next-level-cache = <&l2_100>;
+			clocks = <&apss_clk APSS_SILVER_CORE_CLK>,
+				 <&apss_clk L3_CORE_CLK>;
+			clock-names = "cpu", "l3_core";
+			operating-points-v2 = <&cpu_opp_table>;
 
 			l2_100: l2-cache {
 				compatible = "cache";
@@ -73,6 +83,10 @@ cpu2: cpu@200 {
 			enable-method = "psci";
 			reg = <0x200>;
 			next-level-cache = <&l2_200>;
+			clocks = <&apss_clk APSS_SILVER_CORE_CLK>,
+				 <&apss_clk L3_CORE_CLK>;
+			clock-names = "cpu", "l3_core";
+			operating-points-v2 = <&cpu_opp_table>;
 
 			l2_200: l2-cache {
 				compatible = "cache";
@@ -88,6 +102,10 @@ cpu3: cpu@300 {
 			enable-method = "psci";
 			reg = <0x300>;
 			next-level-cache = <&l2_300>;
+			clocks = <&apss_clk APSS_SILVER_CORE_CLK>,
+				 <&apss_clk L3_CORE_CLK>;
+			clock-names = "cpu", "l3_core";
+			operating-points-v2 = <&cpu_opp_table>;
 
 			l2_300: l2-cache {
 				compatible = "cache";
@@ -98,6 +116,39 @@ l2_300: l2-cache {
 		};
 	};
 
+	cpu_opp_table: opp-table-cpu {
+		compatible = "operating-points-v2-kryo-cpu";
+		opp-shared;
+		nvmem-cells = <&cpu_speed_bin>;
+
+		/*
+		 * CPU supports two frequencies and the fuse has LValue instead
+		 * of limits. As only two frequencies are supported, considering
+		 * zero Lvalue as no limit and Lvalue as 1.4GHz limit.
+		 * ------------------------------------------------------------
+		 * Frequency     BIT1    BIT0    opp-supported-hw
+		 *	      1.4GHz  No Limit
+		 * ------------------------------------------------------------
+		 * 1416000000      1       1	    0x3
+		 * 1800000000      0       1	    0x1
+		 * ------------------------------------------------------------
+		 */
+
+		opp-1416000000 {
+			opp-hz = /bits/ 64 <1416000000>;
+			opp-microvolt = <1>;
+			opp-supported-hw = <0x3>;
+			clock-latency-ns = <200000>;
+		};
+
+		opp-1800000000 {
+			opp-hz = /bits/ 64 <1800000000>;
+			opp-microvolt = <2>;
+			opp-supported-hw = <0x1>;
+			clock-latency-ns = <200000>;
+		};
+	};
+
 	memory@80000000 {
 		device_type = "memory";
 		/* We expect the bootloader to fill in the size */
@@ -151,6 +202,18 @@ soc@0 {
 		#size-cells = <2>;
 		ranges = <0 0 0 0 0x10 0>;
 
+		qfprom@a6000 {
+			compatible = "qcom,qfprom";
+			reg = <0x0 0xa6000 0x0 0x1000>;
+			#address-cells = <1>;
+			#size-cells = <1>;
+
+			cpu_speed_bin: cpu-speed-bin@234 {
+				reg = <0x234 0x1>;
+				bits = <0 8>;
+			};
+		};
+
 		tlmm: pinctrl@1000000 {
 			compatible = "qcom,ipq5424-tlmm";
 			reg = <0 0x01000000 0 0x300000>;
@@ -363,6 +426,14 @@ frame@f42d000 {
 			};
 		};
 
+		apss_clk: apss-clock@fa80000 {
+			compatible = "qcom,ipq5424-apss-clk";
+			reg = <0x0 0x0fa80000 0x0 0x20000>;
+			clocks = <&xo_board>, <&gcc GPLL0>;
+			clock-names = "xo", "gpll0";
+			#clock-cells = <1>;
+		};
+
 		tmel_qmp: qmp@32090000 {
 			compatible = "qcom,ipq5424-tmel-qmp", "qcom,tmel-qmp";
 			reg = <0 0x32090000 0 0x2000>;
-- 
2.34.1

Re: [PATCH 3/4] cpufreq: qcom-nvmem: Enable cpufreq for ipq5424

[Konrad Dybcio]

On 27.01.2025 10:31 AM, Sricharan R wrote:
> From: Md Sadre Alam <quic_mdalam@quicinc.com>
> 
> IPQ5424 have different OPPs available for the CPU based on
> SoC variant. This can be determined through use of an eFuse
> register present in the silicon.
> 
> Added support for ipq5424 on nvmem driver which helps to
> determine OPPs at runtime based on the eFuse register which
> has the CPU frequency limits. opp-supported-hw dt binding
> can be used to indicate the available OPPs for each limit.
> 
> nvmem driver also creates the "cpufreq-dt" platform_device after
> passing the version matching data to the OPP framework so that the
> cpufreq-dt handles the actual cpufreq implementation.
> 
> Signed-off-by: Md Sadre Alam <quic_mdalam@quicinc.com>
> Signed-off-by: Sricharan Ramabadhran <quic_srichara@quicinc.com>
> ---
>  drivers/cpufreq/cpufreq-dt-platdev.c | 1 +
>  drivers/cpufreq/qcom-cpufreq-nvmem.c | 5 +++++
>  2 files changed, 6 insertions(+)
> 
> diff --git a/drivers/cpufreq/cpufreq-dt-platdev.c b/drivers/cpufreq/cpufreq-dt-platdev.c
> index 9c198bd4f7e9..4045bc3ce805 100644
> --- a/drivers/cpufreq/cpufreq-dt-platdev.c
> +++ b/drivers/cpufreq/cpufreq-dt-platdev.c
> @@ -187,6 +187,7 @@ static const struct of_device_id blocklist[] __initconst = {
>  	{ .compatible = "ti,am62p5", },
>  
>  	{ .compatible = "qcom,ipq5332", },
> +	{ .compatible = "qcom,ipq5424", },
>  	{ .compatible = "qcom,ipq6018", },
>  	{ .compatible = "qcom,ipq8064", },
>  	{ .compatible = "qcom,ipq8074", },
> diff --git a/drivers/cpufreq/qcom-cpufreq-nvmem.c b/drivers/cpufreq/qcom-cpufreq-nvmem.c
> index 3a8ed723a23e..102f7f1b031c 100644
> --- a/drivers/cpufreq/qcom-cpufreq-nvmem.c
> +++ b/drivers/cpufreq/qcom-cpufreq-nvmem.c
> @@ -200,6 +200,10 @@ static int qcom_cpufreq_kryo_name_version(struct device *cpu_dev,
>  	case QCOM_ID_IPQ9574:
>  		drv->versions = 1 << (unsigned int)(*speedbin);
>  		break;
> +	case QCOM_ID_IPQ5424:
> +	case QCOM_ID_IPQ5404:
> +		drv->versions =  (*speedbin != 0x3b) ? BIT(0) : BIT(1);

Perhaps:

drv->versions =  (*speedbin == 0x3b) ? BIT(1) : BIT(0);

But ultimately both work:

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

Konrad

Re: [PATCH 1/4] dt-bindings: clock: ipq5424-apss-clk: Add ipq5424 apss clock controller

[Krzysztof Kozlowski]

On 27/01/2025 10:31, Sricharan R wrote:
> From: Sricharan Ramabadhran <quic_srichara@quicinc.com>
> 
> The CPU core in ipq5424 is clocked by a huayra PLL with RCG support.
> The RCG and PLL have a separate register space from the GCC.
> Also the L3 cache has a separate pll and needs to be scaled along
> with the CPU.
> 
> Co-developed-by: Md Sadre Alam <quic_mdalam@quicinc.com>
> Signed-off-by: Md Sadre Alam <quic_mdalam@quicinc.com>
> Signed-off-by: Sricharan Ramabadhran <quic_srichara@quicinc.com>

Considering that there were multiple conflicting patches coming from
Qualcomm around IPQ SoCs and that we are in the merge window, I will
skip this patch.

I suspect this duplicates the other chip as well, but that's your task
to sync up internally.

Best regards,
Krzysztof

Re: [PATCH 2/4] clk: qcom: apss-ipq5424: Add ipq5424 apss clock controller

[Varadarajan Narayanan]

On Mon, Jan 27, 2025 at 03:01:26PM +0530, Sricharan R wrote:
> From: Sricharan Ramabadhran <quic_srichara@quicinc.com>
>
> CPU on Qualcomm ipq5424 is clocked by huayra PLL with RCG support.
> Add support for the APSS PLL, RCG and clock enable for ipq5424.
> The PLL, RCG register space are clubbed. Hence adding new APSS driver
> for both PLL and RCG/CBC control. Also the L3 cache has a separate pll
> and needs to be scaled along with the CPU.
>
> Co-developed-by: Md Sadre Alam <quic_mdalam@quicinc.com>
> Signed-off-by: Md Sadre Alam <quic_mdalam@quicinc.com>
> Signed-off-by: Sricharan Ramabadhran <quic_srichara@quicinc.com>
> ---
>  drivers/clk/qcom/Kconfig        |   7 +
>  drivers/clk/qcom/Makefile       |   1 +
>  drivers/clk/qcom/apss-ipq5424.c | 373 ++++++++++++++++++++++++++++++++
>  3 files changed, 381 insertions(+)
>  create mode 100644 drivers/clk/qcom/apss-ipq5424.c
>
> diff --git a/drivers/clk/qcom/Kconfig b/drivers/clk/qcom/Kconfig
> index ef89d686cbc4..9a03257d67e0 100644
> --- a/drivers/clk/qcom/Kconfig
> +++ b/drivers/clk/qcom/Kconfig
> @@ -190,6 +190,13 @@ config IPQ_APSS_6018
>  	  Say Y if you want to support CPU frequency scaling on
>  	  ipq based devices.
>
> +config IPQ_APSS_5424
> +	tristate "IPQ APSS Clock Controller"
> +	help
> +	  Support for APSS Clock controller on Qualcom IPQ5424 platform.
> +	  Say Y if you want to support CPU frequency scaling on ipq based
> +	  devices.
> +
>  config IPQ_GCC_4019
>  	tristate "IPQ4019 Global Clock Controller"
>  	help
> diff --git a/drivers/clk/qcom/Makefile b/drivers/clk/qcom/Makefile
> index b09dbdc210eb..db15514e7367 100644
> --- a/drivers/clk/qcom/Makefile
> +++ b/drivers/clk/qcom/Makefile
> @@ -28,6 +28,7 @@ obj-$(CONFIG_CLK_X1E80100_GPUCC) += gpucc-x1e80100.o
>  obj-$(CONFIG_CLK_X1E80100_TCSRCC) += tcsrcc-x1e80100.o
>  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_GCC_4019) += gcc-ipq4019.o
>  obj-$(CONFIG_IPQ_GCC_5018) += gcc-ipq5018.o
> diff --git a/drivers/clk/qcom/apss-ipq5424.c b/drivers/clk/qcom/apss-ipq5424.c
> new file mode 100644
> index 000000000000..2bd6ee7575dc
> --- /dev/null
> +++ b/drivers/clk/qcom/apss-ipq5424.c
> @@ -0,0 +1,373 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (c) 2018, The Linux Foundation. All rights reserved.
> + * Copyright (c) 2024, Qualcomm Innovation Center, Inc. All rights reserved.

2025

> + */
> +
> +#include <linux/clk.h>
> +#include <linux/clk-provider.h>
> +#include <linux/err.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/platform_device.h>
> +#include <linux/regmap.h>
> +
> +#include <dt-bindings/clock/qcom,apss-ipq.h>
> +#include <dt-bindings/arm/qcom,ids.h>
> +
> +#include "clk-alpha-pll.h"
> +#include "clk-branch.h"
> +#include "clk-rcg.h"
> +#include "clk-regmap.h"
> +#include "common.h"
> +
> +#define GPLL0_CLK_RATE		800000000
> +#define CPU_NOM_CLK_RATE	1416000000
> +#define CPU_TURBO_CLK_RATE	1800000000
> +#define L3_NOM_CLK_RATE		984000000
> +#define L3_TURBO_CLK_RATE	1272000000
> +
> +enum {
> +	P_XO,
> +	P_GPLL0,
> +	P_APSS_PLL_EARLY,
> +	P_L3_PLL,
> +};
> +
> +struct apss_clk {
> +	struct notifier_block cpu_clk_notifier;
> +	struct clk_hw *hw;
> +	struct device *dev;
> +	struct clk *l3_clk;
> +};
> +
> +/*
> + * IPQ5424 Huayra PLL offsets are different from the one mentioned in the
> + * clk-alpha-pll.c, hence define the IPQ5424 offsets here
> + */

This seems to be same as clk_alpha_pll_regs[CLK_ALPHA_PLL_TYPE_HUAYRA_2290]
in clk-alpha-pll.c. Please see if that can be used here.

> +static const u8 ipq5424_pll_offsets[][PLL_OFF_MAX_REGS] = {
> +	[CLK_ALPHA_PLL_TYPE_HUAYRA] =  {
> +		[PLL_OFF_L_VAL] = 0x04,
> +		[PLL_OFF_ALPHA_VAL] = 0x08,
> +		[PLL_OFF_USER_CTL] = 0x0c,
> +		[PLL_OFF_CONFIG_CTL] = 0x10,
> +		[PLL_OFF_CONFIG_CTL_U] = 0x14,
> +		[PLL_OFF_CONFIG_CTL_U1] = 0x18,
> +		[PLL_OFF_TEST_CTL] = 0x1c,
> +		[PLL_OFF_TEST_CTL_U] = 0x20,
> +		[PLL_OFF_TEST_CTL_U1] = 0x24,
> +		[PLL_OFF_STATUS] = 0x38,
> +	},
> +};
> +
> +static struct clk_alpha_pll ipq5424_apss_pll = {
> +	.offset = 0x0,
> +	.regs = ipq5424_pll_offsets[CLK_ALPHA_PLL_TYPE_HUAYRA],
> +	.flags = SUPPORTS_DYNAMIC_UPDATE,
> +	.clkr = {
> +		.enable_reg = 0x0,
> +		.enable_mask = BIT(0),
> +		.hw.init = &(struct clk_init_data){
> +			.name = "apss_pll",
> +			.parent_data = &(const struct clk_parent_data) {
> +				.fw_name = "xo-board-clk",
> +			},
> +			.parent_names = (const char *[]){ "xo-board-clk"},
> +			.num_parents = 1,
> +			.ops = &clk_alpha_pll_huayra_ops,
> +		},
> +	},
> +};
> +
> +static const struct clk_parent_data parents_apss_silver_clk_src[] = {
> +	{ .fw_name = "xo-board-clk" },
> +	{ .fw_name = "gpll0" },
> +	{ .hw = &ipq5424_apss_pll.clkr.hw },
> +};
> +
> +static const struct parent_map parents_apss_silver_clk_src_map[] = {
> +	{ P_XO, 0 },
> +	{ P_GPLL0, 4 },
> +	{ P_APSS_PLL_EARLY, 5 },
> +};
> +
> +static const struct freq_tbl ftbl_apss_clk_src[] = {
> +	F(GPLL0_CLK_RATE, P_GPLL0, 1, 0, 0),
> +	F(CPU_NOM_CLK_RATE, P_APSS_PLL_EARLY, 1, 0, 0),
> +	F(CPU_TURBO_CLK_RATE, P_APSS_PLL_EARLY, 1, 0, 0),
> +	{ }
> +};
> +
> +static struct clk_rcg2 apss_silver_clk_src = {
> +	.cmd_rcgr = 0x0080,
> +	.freq_tbl = ftbl_apss_clk_src,
> +	.hid_width = 5,
> +	.parent_map = parents_apss_silver_clk_src_map,
> +	.clkr.hw.init = &(struct clk_init_data){
> +		.name = "apss_silver_clk_src",
> +		.parent_data = parents_apss_silver_clk_src,
> +		.num_parents = ARRAY_SIZE(parents_apss_silver_clk_src),
> +		.ops = &clk_rcg2_ops,
> +		.flags = CLK_SET_RATE_PARENT,
> +	},
> +};
> +
> +static struct clk_branch apss_silver_core_clk = {
> +	.halt_reg = 0x008c,
> +	.clkr = {
> +		.enable_reg = 0x008c,
> +		.enable_mask = BIT(0),
> +		.hw.init = &(struct clk_init_data){
> +			.name = "apss_silver_core_clk",
> +			.parent_hws = (const struct clk_hw *[]){
> +				&apss_silver_clk_src.clkr.hw },
> +			.num_parents = 1,
> +			.flags = CLK_SET_RATE_PARENT | CLK_IS_CRITICAL,
> +			.ops = &clk_branch2_ops,
> +		},
> +	},
> +};
> +
> +static struct clk_alpha_pll ipq5424_l3_pll = {
> +	.offset = 0x10000,
> +	.regs = ipq5424_pll_offsets[CLK_ALPHA_PLL_TYPE_HUAYRA],
> +	.flags = SUPPORTS_DYNAMIC_UPDATE,
> +	.clkr = {
> +		.enable_reg = 0x0,
> +		.enable_mask = BIT(0),
> +		.hw.init = &(struct clk_init_data){
> +			.name = "l3_pll",
> +			.parent_data = &(const struct clk_parent_data) {
> +				.fw_name = "xo-board-clk",
> +			},
> +			.parent_names = (const char *[]){ "xo-board-clk"},
> +			.num_parents = 1,
> +			.ops = &clk_alpha_pll_huayra_ops,
> +		},
> +	},
> +};
> +
> +static const struct clk_parent_data parents_l3_clk_src[] = {
> +	{ .fw_name = "xo-board-clk" },
> +	{ .fw_name = "gpll0" },
> +	{ .hw = &ipq5424_l3_pll.clkr.hw },
> +};
> +
> +static const struct parent_map parents_l3_clk_src_map[] = {
> +	{ P_XO, 0 },
> +	{ P_GPLL0, 4 },
> +	{ P_L3_PLL, 5 },
> +};
> +
> +static const struct freq_tbl ftbl_l3_clk_src[] = {
> +	F(GPLL0_CLK_RATE, P_GPLL0, 1, 0, 0),
> +	F(L3_NOM_CLK_RATE, P_L3_PLL, 1, 0, 0),
> +	F(L3_TURBO_CLK_RATE, P_L3_PLL, 1, 0, 0),
> +	{ }
> +};
> +
> +static struct clk_rcg2 l3_clk_src = {
> +	.cmd_rcgr = 0x10080,
> +	.freq_tbl = ftbl_l3_clk_src,
> +	.hid_width = 5,
> +	.parent_map = parents_l3_clk_src_map,
> +	.clkr.hw.init = &(struct clk_init_data){
> +		.name = "l3_clk_src",
> +		.parent_data = parents_l3_clk_src,
> +		.num_parents = ARRAY_SIZE(parents_l3_clk_src),
> +		.ops = &clk_rcg2_ops,
> +		.flags = CLK_SET_RATE_PARENT,
> +	},
> +};
> +
> +static struct clk_branch l3_core_clk = {
> +	.halt_reg = 0x1008c,
> +	.clkr = {
> +		.enable_reg = 0x1008c,
> +		.enable_mask = BIT(0),
> +		.hw.init = &(struct clk_init_data){
> +			.name = "l3_clk",
> +			.parent_hws = (const struct clk_hw *[]){
> +				&l3_clk_src.clkr.hw },
> +			.num_parents = 1,
> +			.flags = CLK_SET_RATE_PARENT | CLK_IS_CRITICAL,
> +			.ops = &clk_branch2_ops,
> +		},
> +	},
> +};
> +
> +static const struct regmap_config apss_ipq5424_regmap_config = {
> +	.reg_bits       = 32,
> +	.reg_stride     = 4,
> +	.val_bits       = 32,
> +	.max_register   = 0x20000,
> +	.fast_io        = true,
> +};
> +
> +static struct clk_regmap *apss_ipq5424_clks[] = {
> +	[APSS_PLL_EARLY] = &ipq5424_apss_pll.clkr,
> +	[APSS_SILVER_CLK_SRC] = &apss_silver_clk_src.clkr,
> +	[APSS_SILVER_CORE_CLK] = &apss_silver_core_clk.clkr,
> +	[L3_PLL] = &ipq5424_l3_pll.clkr,
> +	[L3_CLK_SRC] = &l3_clk_src.clkr,
> +	[L3_CORE_CLK] = &l3_core_clk.clkr,
> +
> +};
> +
> +static const struct qcom_cc_desc apss_ipq5424_desc = {
> +	.config = &apss_ipq5424_regmap_config,
> +	.clks = apss_ipq5424_clks,
> +	.num_clks = ARRAY_SIZE(apss_ipq5424_clks),
> +};
> +
> +static const struct alpha_pll_config apss_pll_config = {
> +	.l = 0x3b,
> +	.config_ctl_val = 0x08200920,
> +	.config_ctl_hi_val = 0x05008001,
> +	.config_ctl_hi1_val = 0x04000000,
> +	.test_ctl_val = 0x0,
> +	.test_ctl_hi_val = 0x0,
> +	.test_ctl_hi1_val = 0x0,
> +	.user_ctl_val = 0x1,
> +	.early_output_mask = BIT(3),
> +	.aux2_output_mask = BIT(2),
> +	.aux_output_mask = BIT(1),
> +	.main_output_mask = BIT(0),
> +};
> +
> +static const struct alpha_pll_config l3_pll_config = {
> +	.l = 0x29,
> +	.config_ctl_val = 0x08200920,
> +	.config_ctl_hi_val = 0x05008001,
> +	.config_ctl_hi1_val = 0x04000000,
> +	.test_ctl_val = 0x0,
> +	.test_ctl_hi_val = 0x0,
> +	.test_ctl_hi1_val = 0x0,
> +	.user_ctl_val = 0x1,
> +	.early_output_mask = BIT(3),
> +	.aux2_output_mask = BIT(2),
> +	.aux_output_mask = BIT(1),
> +	.main_output_mask = BIT(0),
> +};
> +
> +static unsigned long get_l3_clk_from_tbl(unsigned long rate)
> +{
> +	struct clk_rcg2 *l3_rcg2 = container_of(&l3_clk_src.clkr, struct clk_rcg2, clkr);
> +	u8 max_clk = sizeof(ftbl_apss_clk_src) / sizeof(struct freq_tbl);
> +	u8 loop;
> +
> +	for (loop = 0; loop < max_clk; loop++)
> +		if (ftbl_apss_clk_src[loop].freq == rate)
> +			return l3_rcg2->freq_tbl[loop].freq;
> +	return 0;
> +}
> +
> +static int cpu_clk_notifier_fn(struct notifier_block *nb, unsigned long action,
> +			       void *data)
> +{
> +	struct apss_clk *apss_ipq5424_cfg = container_of(nb, struct apss_clk, cpu_clk_notifier);
> +	struct clk_notifier_data *cnd = (struct clk_notifier_data *)data;
> +	struct device *dev = apss_ipq5424_cfg->dev;
> +	unsigned long rate = 0, l3_rate;
> +	int err = 0;

No need to init 'err' here.

> +
> +	dev_dbg(dev, "action:%ld old_rate:%ld new_rate:%ld\n", action,
> +		cnd->old_rate, cnd->new_rate);
> +
> +	switch (action) {
> +	case PRE_RATE_CHANGE:
> +		if (cnd->old_rate < cnd->new_rate)
> +			rate = cnd->new_rate;
> +	break;
> +	case POST_RATE_CHANGE:
> +		if (cnd->old_rate > cnd->new_rate)
> +			rate = cnd->new_rate;
> +	break;
> +	};
> +
> +	if (!rate)
> +		goto notif_ret;
> +
> +	l3_rate = get_l3_clk_from_tbl(rate);
> +	if (!l3_rate) {
> +		dev_err(dev, "Failed to get l3 clock rate from l3_tbl\n");
> +		return NOTIFY_BAD;
> +	}
> +
> +	err = clk_set_rate(apss_ipq5424_cfg->l3_clk, l3_rate);
> +	if (err) {
> +		dev_err(dev, "Failed to set l3 clock rate(%ld) err(%d)\n", l3_rate, err);
> +		return NOTIFY_BAD;
> +	}
> +
> +notif_ret:
> +	return NOTIFY_OK;
> +}
> +
> +static int apss_ipq5424_probe(struct platform_device *pdev)
> +{
> +	struct device *dev = &pdev->dev;
> +	struct apss_clk *apss_ipq5424_cfg;
> +	struct regmap *regmap;
> +	void __iomem *base;
> +	int ret;
> +
> +	apss_ipq5424_cfg = devm_kzalloc(&pdev->dev, sizeof(struct apss_clk), GFP_KERNEL);
> +	if (IS_ERR_OR_NULL(apss_ipq5424_cfg))
> +		return PTR_ERR(apss_ipq5424_cfg);
> +
> +	base = devm_platform_ioremap_resource(pdev, 0);
> +	if (IS_ERR(base))
> +		return PTR_ERR(base);
> +
> +	regmap = devm_regmap_init_mmio(dev, base, &apss_ipq5424_regmap_config);
> +	if (!regmap)
> +		return PTR_ERR(regmap);
> +
> +	clk_alpha_pll_configure(&ipq5424_l3_pll, regmap, &l3_pll_config);
> +
> +	clk_alpha_pll_configure(&ipq5424_apss_pll, regmap, &apss_pll_config);
> +
> +	ret = qcom_cc_really_probe(dev, &apss_ipq5424_desc, regmap);
> +	if (ret)
> +		return ret;
> +
> +	dev_dbg(&pdev->dev, "Registered APSS & L3 clock provider\n");
> +
> +	apss_ipq5424_cfg->dev = dev;
> +	apss_ipq5424_cfg->hw = &apss_silver_clk_src.clkr.hw;
> +	apss_ipq5424_cfg->cpu_clk_notifier.notifier_call = cpu_clk_notifier_fn;
> +
> +	apss_ipq5424_cfg->l3_clk = clk_hw_get_clk(&l3_core_clk.clkr.hw, "l3_clk");
> +	if (IS_ERR(apss_ipq5424_cfg->l3_clk)) {
> +		dev_err(&pdev->dev, "Failed to get L3 clk, %ld\n",
> +			PTR_ERR(apss_ipq5424_cfg->l3_clk));
> +		return PTR_ERR(apss_ipq5424_cfg->l3_clk);
> +	}
> +
> +	ret = devm_clk_notifier_register(&pdev->dev, apss_ipq5424_cfg->hw->clk,
> +					 &apss_ipq5424_cfg->cpu_clk_notifier);

Use return devm_clk_notifier_register(...) and below lines can be skipped.

Thanks
Varada

> +	if (ret)
> +		return ret;
> +
> +	return 0;
> +}
> +
> +static const struct of_device_id apss_ipq5424_match_table[] = {
> +	{ .compatible = "qcom,ipq5424-apss-clk" },
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(of, apss_ipq5424_match_table);
> +
> +static struct platform_driver apss_ipq5424_driver = {
> +	.probe = apss_ipq5424_probe,
> +	.driver = {
> +		.name   = "apss-ipq5424-clk",
> +		.of_match_table = apss_ipq5424_match_table,
> +	},
> +};
> +
> +module_platform_driver(apss_ipq5424_driver);
> +
> +MODULE_DESCRIPTION("QCOM APSS IPQ5424 CLK Driver");
> +MODULE_LICENSE("GPL v2");
>
> --
> 2.34.1
>

Re: [PATCH 1/4] dt-bindings: clock: ipq5424-apss-clk: Add ipq5424 apss clock controller

[Sricharan Ramabadhran]

On 1/28/2025 1:04 PM, Krzysztof Kozlowski wrote:
> On 27/01/2025 10:31, Sricharan R wrote:
>> From: Sricharan Ramabadhran <quic_srichara@quicinc.com>
>>
>> The CPU core in ipq5424 is clocked by a huayra PLL with RCG support.
>> The RCG and PLL have a separate register space from the GCC.
>> Also the L3 cache has a separate pll and needs to be scaled along
>> with the CPU.
>>
>> Co-developed-by: Md Sadre Alam <quic_mdalam@quicinc.com>
>> Signed-off-by: Md Sadre Alam <quic_mdalam@quicinc.com>
>> Signed-off-by: Sricharan Ramabadhran <quic_srichara@quicinc.com>
> 
> Considering that there were multiple conflicting patches coming from
> Qualcomm around IPQ SoCs and that we are in the merge window, I will
> skip this patch.
> 
> I suspect this duplicates the other chip as well, but that's your task
> to sync up internally.
> 
ok, but this .yaml is specific to IPQ5424 and would not conflict with
IPQ5332. That said, will post it after merge window as a part of
V3 (for other patch changes) to avoid any confusion.

Regards,
  Sricharan

Re: [PATCH 2/4] clk: qcom: apss-ipq5424: Add ipq5424 apss clock controller

[Konrad Dybcio]

On 27.01.2025 10:31 AM, Sricharan R wrote:
> From: Sricharan Ramabadhran <quic_srichara@quicinc.com>
> 
> CPU on Qualcomm ipq5424 is clocked by huayra PLL with RCG support.
> Add support for the APSS PLL, RCG and clock enable for ipq5424.
> The PLL, RCG register space are clubbed. Hence adding new APSS driver
> for both PLL and RCG/CBC control. Also the L3 cache has a separate pll
> and needs to be scaled along with the CPU.
> 
> Co-developed-by: Md Sadre Alam <quic_mdalam@quicinc.com>
> Signed-off-by: Md Sadre Alam <quic_mdalam@quicinc.com>
> Signed-off-by: Sricharan Ramabadhran <quic_srichara@quicinc.com>
> ---

[...]

> +#define GPLL0_CLK_RATE		800000000
> +#define CPU_NOM_CLK_RATE	1416000000
> +#define CPU_TURBO_CLK_RATE	1800000000
> +#define L3_NOM_CLK_RATE		984000000
> +#define L3_TURBO_CLK_RATE	1272000000

Please inline these values

> +
> +enum {
> +	P_XO,
> +	P_GPLL0,
> +	P_APSS_PLL_EARLY,
> +	P_L3_PLL,
> +};
> +
> +struct apss_clk {
> +	struct notifier_block cpu_clk_notifier;
> +	struct clk_hw *hw;
> +	struct device *dev;
> +	struct clk *l3_clk;
> +};
> +

> +static struct clk_branch l3_core_clk = {
> +	.halt_reg = 0x1008c,
> +	.clkr = {
> +		.enable_reg = 0x1008c,
> +		.enable_mask = BIT(0),
> +		.hw.init = &(struct clk_init_data){
> +			.name = "l3_clk",
> +			.parent_hws = (const struct clk_hw *[]){
> +				&l3_clk_src.clkr.hw },

	&l3_clk_src.clkr.hw
},

> +static unsigned long get_l3_clk_from_tbl(unsigned long rate)
> +{
> +	struct clk_rcg2 *l3_rcg2 = container_of(&l3_clk_src.clkr, struct clk_rcg2, clkr);
> +	u8 max_clk = sizeof(ftbl_apss_clk_src) / sizeof(struct freq_tbl);
> +	u8 loop;
> +
> +	for (loop = 0; loop < max_clk; loop++)
> +		if (ftbl_apss_clk_src[loop].freq == rate)
> +			return l3_rcg2->freq_tbl[loop].freq;

This looks extremely explosive if anyone makes changes to the driver..

Use an OPP table in the devicetree instead

And please add a newline before the return statement

> +	return 0;
> +}
> +
> +static int cpu_clk_notifier_fn(struct notifier_block *nb, unsigned long action,
> +			       void *data)
> +{
> +	struct apss_clk *apss_ipq5424_cfg = container_of(nb, struct apss_clk, cpu_clk_notifier);
> +	struct clk_notifier_data *cnd = (struct clk_notifier_data *)data;
> +	struct device *dev = apss_ipq5424_cfg->dev;
> +	unsigned long rate = 0, l3_rate;
> +	int err = 0;

Please use 'ret'

> +
> +	dev_dbg(dev, "action:%ld old_rate:%ld new_rate:%ld\n", action,
> +		cnd->old_rate, cnd->new_rate);
> +
> +	switch (action) {
> +	case PRE_RATE_CHANGE:
> +		if (cnd->old_rate < cnd->new_rate)
> +			rate = cnd->new_rate;
> +	break;

Why are the breaks indented like this?

> +	case POST_RATE_CHANGE:
> +		if (cnd->old_rate > cnd->new_rate)
> +			rate = cnd->new_rate;
> +	break;
> +	};
> +
> +	if (!rate)
> +		goto notif_ret;

In cases like these, just return directly instead of jumping

> +
> +	l3_rate = get_l3_clk_from_tbl(rate);
> +	if (!l3_rate) {
> +		dev_err(dev, "Failed to get l3 clock rate from l3_tbl\n");
> +		return NOTIFY_BAD;
> +	}
> +
> +	err = clk_set_rate(apss_ipq5424_cfg->l3_clk, l3_rate);
> +	if (err) {
> +		dev_err(dev, "Failed to set l3 clock rate(%ld) err(%d)\n", l3_rate, err);
> +		return NOTIFY_BAD;
> +	}
> +
> +notif_ret:
> +	return NOTIFY_OK;
> +}
> +
> +static int apss_ipq5424_probe(struct platform_device *pdev)
> +{
> +	struct device *dev = &pdev->dev;
> +	struct apss_clk *apss_ipq5424_cfg;
> +	struct regmap *regmap;
> +	void __iomem *base;
> +	int ret;
> +
> +	apss_ipq5424_cfg = devm_kzalloc(&pdev->dev, sizeof(struct apss_clk), GFP_KERNEL);

Since there is no "config" in there, something like "ipq5424_apsscc" would be
more fitting

> +	if (IS_ERR_OR_NULL(apss_ipq5424_cfg))
> +		return PTR_ERR(apss_ipq5424_cfg);

https://elixir.bootlin.com/linux/v6.13/source/include/linux/device.h#L326-L329
|_
   > elixir.bootlin.com/linux/v6.13/source/drivers/base/devres.c#L819-L820

It can never throw an errno, just check for if (!apss...)

> +
> +	base = devm_platform_ioremap_resource(pdev, 0);
> +	if (IS_ERR(base))
> +		return PTR_ERR(base);
> +
> +	regmap = devm_regmap_init_mmio(dev, base, &apss_ipq5424_regmap_config);
> +	if (!regmap)
> +		return PTR_ERR(regmap);

devm_platform_get_and_ioremap_resource()

> +
> +	clk_alpha_pll_configure(&ipq5424_l3_pll, regmap, &l3_pll_config);
> +
> +	clk_alpha_pll_configure(&ipq5424_apss_pll, regmap, &apss_pll_config);
> +
> +	ret = qcom_cc_really_probe(dev, &apss_ipq5424_desc, regmap);
> +	if (ret)
> +		return ret;
> +
> +	dev_dbg(&pdev->dev, "Registered APSS & L3 clock provider\n");
> +
> +	apss_ipq5424_cfg->dev = dev;
> +	apss_ipq5424_cfg->hw = &apss_silver_clk_src.clkr.hw;
> +	apss_ipq5424_cfg->cpu_clk_notifier.notifier_call = cpu_clk_notifier_fn;
> +
> +	apss_ipq5424_cfg->l3_clk = clk_hw_get_clk(&l3_core_clk.clkr.hw, "l3_clk");
> +	if (IS_ERR(apss_ipq5424_cfg->l3_clk)) {
> +		dev_err(&pdev->dev, "Failed to get L3 clk, %ld\n",
> +			PTR_ERR(apss_ipq5424_cfg->l3_clk));
> +		return PTR_ERR(apss_ipq5424_cfg->l3_clk);
> +	}

Now that you'll use OPP, you can drop all this getting.. maybe even the
apss_ipq5424_cfg struct could be let go
> +
> +	ret = devm_clk_notifier_register(&pdev->dev, apss_ipq5424_cfg->hw->clk,
> +					 &apss_ipq5424_cfg->cpu_clk_notifier);
> +	if (ret)
> +		return ret;
> +
> +	return 0;

Just return ret instead

> +}
> +
> +static const struct of_device_id apss_ipq5424_match_table[] = {
> +	{ .compatible = "qcom,ipq5424-apss-clk" },
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(of, apss_ipq5424_match_table);
> +
> +static struct platform_driver apss_ipq5424_driver = {
> +	.probe = apss_ipq5424_probe,
> +	.driver = {
> +		.name   = "apss-ipq5424-clk",
> +		.of_match_table = apss_ipq5424_match_table,
> +	},
> +};
> +
> +module_platform_driver(apss_ipq5424_driver);
> +
> +MODULE_DESCRIPTION("QCOM APSS IPQ5424 CLK Driver");
> +MODULE_LICENSE("GPL v2");

Please don't skip running 'checkpatch'.

Konrad

Re: [PATCH 1/4] dt-bindings: clock: ipq5424-apss-clk: Add ipq5424 apss clock controller

[Krzysztof Kozlowski]

On 28/01/2025 12:15, Sricharan Ramabadhran wrote:
> 
> 
> On 1/28/2025 1:04 PM, Krzysztof Kozlowski wrote:
>> On 27/01/2025 10:31, Sricharan R wrote:
>>> From: Sricharan Ramabadhran <quic_srichara@quicinc.com>
>>>
>>> The CPU core in ipq5424 is clocked by a huayra PLL with RCG support.
>>> The RCG and PLL have a separate register space from the GCC.
>>> Also the L3 cache has a separate pll and needs to be scaled along
>>> with the CPU.
>>>
>>> Co-developed-by: Md Sadre Alam <quic_mdalam@quicinc.com>
>>> Signed-off-by: Md Sadre Alam <quic_mdalam@quicinc.com>
>>> Signed-off-by: Sricharan Ramabadhran <quic_srichara@quicinc.com>
>>
>> Considering that there were multiple conflicting patches coming from
>> Qualcomm around IPQ SoCs and that we are in the merge window, I will
>> skip this patch.
>>
>> I suspect this duplicates the other chip as well, but that's your task
>> to sync up internally.
>>
> ok, but this .yaml is specific to IPQ5424 and would not conflict with
> IPQ5332. That said, will post it after merge window as a part of
> V3 (for other patch changes) to avoid any confusion.


But maybe it is the same on ipq5332? or similar? Other works were
totally de-synced and you ask community to sync them. That's not how it
works.

Best regards,
Krzysztof

Re: [PATCH 2/4] clk: qcom: apss-ipq5424: Add ipq5424 apss clock controller

[Sricharan Ramabadhran]

On 1/28/2025 1:18 PM, Varadarajan Narayanan wrote:

>> diff --git a/drivers/clk/qcom/Makefile b/drivers/clk/qcom/Makefile
>> index b09dbdc210eb..db15514e7367 100644
>> --- a/drivers/clk/qcom/Makefile
>> +++ b/drivers/clk/qcom/Makefile
>> @@ -28,6 +28,7 @@ obj-$(CONFIG_CLK_X1E80100_GPUCC) += gpucc-x1e80100.o
>>   obj-$(CONFIG_CLK_X1E80100_TCSRCC) += tcsrcc-x1e80100.o
>>   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_GCC_4019) += gcc-ipq4019.o
>>   obj-$(CONFIG_IPQ_GCC_5018) += gcc-ipq5018.o
>> diff --git a/drivers/clk/qcom/apss-ipq5424.c b/drivers/clk/qcom/apss-ipq5424.c
>> new file mode 100644
>> index 000000000000..2bd6ee7575dc
>> --- /dev/null
>> +++ b/drivers/clk/qcom/apss-ipq5424.c
>> @@ -0,0 +1,373 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/*
>> + * Copyright (c) 2018, The Linux Foundation. All rights reserved.
>> + * Copyright (c) 2024, Qualcomm Innovation Center, Inc. All rights reserved.
> 
> 2025
> 
ok.

>> + */
>> +
>> +#include <linux/clk.h>
>> +#include <linux/clk-provider.h>
>> +#include <linux/err.h>
>> +#include <linux/kernel.h>
>> +#include <linux/module.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/regmap.h>
>> +
>> +#include <dt-bindings/clock/qcom,apss-ipq.h>
>> +#include <dt-bindings/arm/qcom,ids.h>
>> +
>> +#include "clk-alpha-pll.h"
>> +#include "clk-branch.h"
>> +#include "clk-rcg.h"
>> +#include "clk-regmap.h"
>> +#include "common.h"
>> +
>> +#define GPLL0_CLK_RATE		800000000
>> +#define CPU_NOM_CLK_RATE	1416000000
>> +#define CPU_TURBO_CLK_RATE	1800000000
>> +#define L3_NOM_CLK_RATE		984000000
>> +#define L3_TURBO_CLK_RATE	1272000000
>> +
>> +enum {
>> +	P_XO,
>> +	P_GPLL0,
>> +	P_APSS_PLL_EARLY,
>> +	P_L3_PLL,
>> +};
>> +
>> +struct apss_clk {
>> +	struct notifier_block cpu_clk_notifier;
>> +	struct clk_hw *hw;
>> +	struct device *dev;
>> +	struct clk *l3_clk;
>> +};
>> +
>> +/*
>> + * IPQ5424 Huayra PLL offsets are different from the one mentioned in the
>> + * clk-alpha-pll.c, hence define the IPQ5424 offsets here
>> + */
> 
> This seems to be same as clk_alpha_pll_regs[CLK_ALPHA_PLL_TYPE_HUAYRA_2290]
> in clk-alpha-pll.c. Please see if that can be used here.
> 
ok

>> +static const u8 ipq5424_pll_offsets[][PLL_OFF_MAX_REGS] = {
>> +	[CLK_ALPHA_PLL_TYPE_HUAYRA] =  {
>> +		[PLL_OFF_L_VAL] = 0x04,
>> +		[PLL_OFF_ALPHA_VAL] = 0x08,
>> +		[PLL_OFF_USER_CTL] = 0x0c,
>> +		[PLL_OFF_CONFIG_CTL] = 0x10,
>> +		[PLL_OFF_CONFIG_CTL_U] = 0x14,
>> +		[PLL_OFF_CONFIG_CTL_U1] = 0x18,
>> +		[PLL_OFF_TEST_CTL] = 0x1c,
>> +		[PLL_OFF_TEST_CTL_U] = 0x20,
>> +		[PLL_OFF_TEST_CTL_U1] = 0x24,
>> +		[PLL_OFF_STATUS] = 0x38,
>> +	},
>> +};
>> +
>> +static struct clk_alpha_pll ipq5424_apss_pll = {
>> +	.offset = 0x0,
>> +	.regs = ipq5424_pll_offsets[CLK_ALPHA_PLL_TYPE_HUAYRA],
>> +	.flags = SUPPORTS_DYNAMIC_UPDATE,
>> +	.clkr = {
>> +		.enable_reg = 0x0,
>> +		.enable_mask = BIT(0),
>> +		.hw.init = &(struct clk_init_data){
>> +			.name = "apss_pll",
>> +			.parent_data = &(const struct clk_parent_data) {
>> +				.fw_name = "xo-board-clk",
>> +			},
>> +			.parent_names = (const char *[]){ "xo-board-clk"},
>> +			.num_parents = 1,
>> +			.ops = &clk_alpha_pll_huayra_ops,
>> +		},
>> +	},
>> +};
>> +
>> +static const struct clk_parent_data parents_apss_silver_clk_src[] = {
>> +	{ .fw_name = "xo-board-clk" },
>> +	{ .fw_name = "gpll0" },
>> +	{ .hw = &ipq5424_apss_pll.clkr.hw },
>> +};
>> +
>> +static const struct parent_map parents_apss_silver_clk_src_map[] = {
>> +	{ P_XO, 0 },
>> +	{ P_GPLL0, 4 },
>> +	{ P_APSS_PLL_EARLY, 5 },
>> +};
>> +
>> +static const struct freq_tbl ftbl_apss_clk_src[] = {
>> +	F(GPLL0_CLK_RATE, P_GPLL0, 1, 0, 0),
>> +	F(CPU_NOM_CLK_RATE, P_APSS_PLL_EARLY, 1, 0, 0),
>> +	F(CPU_TURBO_CLK_RATE, P_APSS_PLL_EARLY, 1, 0, 0),
>> +	{ }
>> +};
>> +
>> +static struct clk_rcg2 apss_silver_clk_src = {
>> +	.cmd_rcgr = 0x0080,
>> +	.freq_tbl = ftbl_apss_clk_src,
>> +	.hid_width = 5,
>> +	.parent_map = parents_apss_silver_clk_src_map,
>> +	.clkr.hw.init = &(struct clk_init_data){
>> +		.name = "apss_silver_clk_src",
>> +		.parent_data = parents_apss_silver_clk_src,
>> +		.num_parents = ARRAY_SIZE(parents_apss_silver_clk_src),
>> +		.ops = &clk_rcg2_ops,
>> +		.flags = CLK_SET_RATE_PARENT,
>> +	},
>> +};
>> +
>> +static struct clk_branch apss_silver_core_clk = {
>> +	.halt_reg = 0x008c,
>> +	.clkr = {
>> +		.enable_reg = 0x008c,
>> +		.enable_mask = BIT(0),
>> +		.hw.init = &(struct clk_init_data){
>> +			.name = "apss_silver_core_clk",
>> +			.parent_hws = (const struct clk_hw *[]){
>> +				&apss_silver_clk_src.clkr.hw },
>> +			.num_parents = 1,
>> +			.flags = CLK_SET_RATE_PARENT | CLK_IS_CRITICAL,
>> +			.ops = &clk_branch2_ops,
>> +		},
>> +	},
>> +};
>> +
>> +static struct clk_alpha_pll ipq5424_l3_pll = {
>> +	.offset = 0x10000,
>> +	.regs = ipq5424_pll_offsets[CLK_ALPHA_PLL_TYPE_HUAYRA],
>> +	.flags = SUPPORTS_DYNAMIC_UPDATE,
>> +	.clkr = {
>> +		.enable_reg = 0x0,
>> +		.enable_mask = BIT(0),
>> +		.hw.init = &(struct clk_init_data){
>> +			.name = "l3_pll",
>> +			.parent_data = &(const struct clk_parent_data) {
>> +				.fw_name = "xo-board-clk",
>> +			},
>> +			.parent_names = (const char *[]){ "xo-board-clk"},
>> +			.num_parents = 1,
>> +			.ops = &clk_alpha_pll_huayra_ops,
>> +		},
>> +	},
>> +};
>> +
>> +static const struct clk_parent_data parents_l3_clk_src[] = {
>> +	{ .fw_name = "xo-board-clk" },
>> +	{ .fw_name = "gpll0" },
>> +	{ .hw = &ipq5424_l3_pll.clkr.hw },
>> +};
>> +
>> +static const struct parent_map parents_l3_clk_src_map[] = {
>> +	{ P_XO, 0 },
>> +	{ P_GPLL0, 4 },
>> +	{ P_L3_PLL, 5 },
>> +};
>> +
>> +static const struct freq_tbl ftbl_l3_clk_src[] = {
>> +	F(GPLL0_CLK_RATE, P_GPLL0, 1, 0, 0),
>> +	F(L3_NOM_CLK_RATE, P_L3_PLL, 1, 0, 0),
>> +	F(L3_TURBO_CLK_RATE, P_L3_PLL, 1, 0, 0),
>> +	{ }
>> +};
>> +
>> +static struct clk_rcg2 l3_clk_src = {
>> +	.cmd_rcgr = 0x10080,
>> +	.freq_tbl = ftbl_l3_clk_src,
>> +	.hid_width = 5,
>> +	.parent_map = parents_l3_clk_src_map,
>> +	.clkr.hw.init = &(struct clk_init_data){
>> +		.name = "l3_clk_src",
>> +		.parent_data = parents_l3_clk_src,
>> +		.num_parents = ARRAY_SIZE(parents_l3_clk_src),
>> +		.ops = &clk_rcg2_ops,
>> +		.flags = CLK_SET_RATE_PARENT,
>> +	},
>> +};
>> +
>> +static struct clk_branch l3_core_clk = {
>> +	.halt_reg = 0x1008c,
>> +	.clkr = {
>> +		.enable_reg = 0x1008c,
>> +		.enable_mask = BIT(0),
>> +		.hw.init = &(struct clk_init_data){
>> +			.name = "l3_clk",
>> +			.parent_hws = (const struct clk_hw *[]){
>> +				&l3_clk_src.clkr.hw },
>> +			.num_parents = 1,
>> +			.flags = CLK_SET_RATE_PARENT | CLK_IS_CRITICAL,
>> +			.ops = &clk_branch2_ops,
>> +		},
>> +	},
>> +};
>> +
>> +static const struct regmap_config apss_ipq5424_regmap_config = {
>> +	.reg_bits       = 32,
>> +	.reg_stride     = 4,
>> +	.val_bits       = 32,
>> +	.max_register   = 0x20000,
>> +	.fast_io        = true,
>> +};
>> +
>> +static struct clk_regmap *apss_ipq5424_clks[] = {
>> +	[APSS_PLL_EARLY] = &ipq5424_apss_pll.clkr,
>> +	[APSS_SILVER_CLK_SRC] = &apss_silver_clk_src.clkr,
>> +	[APSS_SILVER_CORE_CLK] = &apss_silver_core_clk.clkr,
>> +	[L3_PLL] = &ipq5424_l3_pll.clkr,
>> +	[L3_CLK_SRC] = &l3_clk_src.clkr,
>> +	[L3_CORE_CLK] = &l3_core_clk.clkr,
>> +
>> +};
>> +
>> +static const struct qcom_cc_desc apss_ipq5424_desc = {
>> +	.config = &apss_ipq5424_regmap_config,
>> +	.clks = apss_ipq5424_clks,
>> +	.num_clks = ARRAY_SIZE(apss_ipq5424_clks),
>> +};
>> +
>> +static const struct alpha_pll_config apss_pll_config = {
>> +	.l = 0x3b,
>> +	.config_ctl_val = 0x08200920,
>> +	.config_ctl_hi_val = 0x05008001,
>> +	.config_ctl_hi1_val = 0x04000000,
>> +	.test_ctl_val = 0x0,
>> +	.test_ctl_hi_val = 0x0,
>> +	.test_ctl_hi1_val = 0x0,
>> +	.user_ctl_val = 0x1,
>> +	.early_output_mask = BIT(3),
>> +	.aux2_output_mask = BIT(2),
>> +	.aux_output_mask = BIT(1),
>> +	.main_output_mask = BIT(0),
>> +};
>> +
>> +static const struct alpha_pll_config l3_pll_config = {
>> +	.l = 0x29,
>> +	.config_ctl_val = 0x08200920,
>> +	.config_ctl_hi_val = 0x05008001,
>> +	.config_ctl_hi1_val = 0x04000000,
>> +	.test_ctl_val = 0x0,
>> +	.test_ctl_hi_val = 0x0,
>> +	.test_ctl_hi1_val = 0x0,
>> +	.user_ctl_val = 0x1,
>> +	.early_output_mask = BIT(3),
>> +	.aux2_output_mask = BIT(2),
>> +	.aux_output_mask = BIT(1),
>> +	.main_output_mask = BIT(0),
>> +};
>> +
>> +static unsigned long get_l3_clk_from_tbl(unsigned long rate)
>> +{
>> +	struct clk_rcg2 *l3_rcg2 = container_of(&l3_clk_src.clkr, struct clk_rcg2, clkr);
>> +	u8 max_clk = sizeof(ftbl_apss_clk_src) / sizeof(struct freq_tbl);
>> +	u8 loop;
>> +
>> +	for (loop = 0; loop < max_clk; loop++)
>> +		if (ftbl_apss_clk_src[loop].freq == rate)
>> +			return l3_rcg2->freq_tbl[loop].freq;
>> +	return 0;
>> +}
>> +
>> +static int cpu_clk_notifier_fn(struct notifier_block *nb, unsigned long action,
>> +			       void *data)
>> +{
>> +	struct apss_clk *apss_ipq5424_cfg = container_of(nb, struct apss_clk, cpu_clk_notifier);
>> +	struct clk_notifier_data *cnd = (struct clk_notifier_data *)data;
>> +	struct device *dev = apss_ipq5424_cfg->dev;
>> +	unsigned long rate = 0, l3_rate;
>> +	int err = 0;
> 
> No need to init 'err' here.
> 
ok

>> +
>> +	dev_dbg(dev, "action:%ld old_rate:%ld new_rate:%ld\n", action,
>> +		cnd->old_rate, cnd->new_rate);
>> +
>> +	switch (action) {
>> +	case PRE_RATE_CHANGE:
>> +		if (cnd->old_rate < cnd->new_rate)
>> +			rate = cnd->new_rate;
>> +	break;
>> +	case POST_RATE_CHANGE:
>> +		if (cnd->old_rate > cnd->new_rate)
>> +			rate = cnd->new_rate;
>> +	break;
>> +	};
>> +
>> +	if (!rate)
>> +		goto notif_ret;
>> +
>> +	l3_rate = get_l3_clk_from_tbl(rate);
>> +	if (!l3_rate) {
>> +		dev_err(dev, "Failed to get l3 clock rate from l3_tbl\n");
>> +		return NOTIFY_BAD;
>> +	}
>> +
>> +	err = clk_set_rate(apss_ipq5424_cfg->l3_clk, l3_rate);
>> +	if (err) {
>> +		dev_err(dev, "Failed to set l3 clock rate(%ld) err(%d)\n", l3_rate, err);
>> +		return NOTIFY_BAD;
>> +	}
>> +
>> +notif_ret:
>> +	return NOTIFY_OK;
>> +}
>> +
>> +static int apss_ipq5424_probe(struct platform_device *pdev)
>> +{
>> +	struct device *dev = &pdev->dev;
>> +	struct apss_clk *apss_ipq5424_cfg;
>> +	struct regmap *regmap;
>> +	void __iomem *base;
>> +	int ret;
>> +
>> +	apss_ipq5424_cfg = devm_kzalloc(&pdev->dev, sizeof(struct apss_clk), GFP_KERNEL);
>> +	if (IS_ERR_OR_NULL(apss_ipq5424_cfg))
>> +		return PTR_ERR(apss_ipq5424_cfg);
>> +
>> +	base = devm_platform_ioremap_resource(pdev, 0);
>> +	if (IS_ERR(base))
>> +		return PTR_ERR(base);
>> +
>> +	regmap = devm_regmap_init_mmio(dev, base, &apss_ipq5424_regmap_config);
>> +	if (!regmap)
>> +		return PTR_ERR(regmap);
>> +
>> +	clk_alpha_pll_configure(&ipq5424_l3_pll, regmap, &l3_pll_config);
>> +
>> +	clk_alpha_pll_configure(&ipq5424_apss_pll, regmap, &apss_pll_config);
>> +
>> +	ret = qcom_cc_really_probe(dev, &apss_ipq5424_desc, regmap);
>> +	if (ret)
>> +		return ret;
>> +
>> +	dev_dbg(&pdev->dev, "Registered APSS & L3 clock provider\n");
>> +
>> +	apss_ipq5424_cfg->dev = dev;
>> +	apss_ipq5424_cfg->hw = &apss_silver_clk_src.clkr.hw;
>> +	apss_ipq5424_cfg->cpu_clk_notifier.notifier_call = cpu_clk_notifier_fn;
>> +
>> +	apss_ipq5424_cfg->l3_clk = clk_hw_get_clk(&l3_core_clk.clkr.hw, "l3_clk");
>> +	if (IS_ERR(apss_ipq5424_cfg->l3_clk)) {
>> +		dev_err(&pdev->dev, "Failed to get L3 clk, %ld\n",
>> +			PTR_ERR(apss_ipq5424_cfg->l3_clk));
>> +		return PTR_ERR(apss_ipq5424_cfg->l3_clk);
>> +	}
>> +
>> +	ret = devm_clk_notifier_register(&pdev->dev, apss_ipq5424_cfg->hw->clk,
>> +					 &apss_ipq5424_cfg->cpu_clk_notifier);
> 
> Use return devm_clk_notifier_register(...) and below lines can be skipped.
> 
ok

Regards,
  Sricharan

Re: [PATCH 2/4] clk: qcom: apss-ipq5424: Add ipq5424 apss clock controller

[Sricharan Ramabadhran]

On 1/28/2025 5:29 PM, Konrad Dybcio wrote:
> On 27.01.2025 10:31 AM, Sricharan R wrote:
>> From: Sricharan Ramabadhran <quic_srichara@quicinc.com>
>>
>> CPU on Qualcomm ipq5424 is clocked by huayra PLL with RCG support.
>> Add support for the APSS PLL, RCG and clock enable for ipq5424.
>> The PLL, RCG register space are clubbed. Hence adding new APSS driver
>> for both PLL and RCG/CBC control. Also the L3 cache has a separate pll
>> and needs to be scaled along with the CPU.
>>
>> Co-developed-by: Md Sadre Alam <quic_mdalam@quicinc.com>
>> Signed-off-by: Md Sadre Alam <quic_mdalam@quicinc.com>
>> Signed-off-by: Sricharan Ramabadhran <quic_srichara@quicinc.com>
>> ---
> 
> [...]
> 
>> +#define GPLL0_CLK_RATE		800000000
>> +#define CPU_NOM_CLK_RATE	1416000000
>> +#define CPU_TURBO_CLK_RATE	1800000000
>> +#define L3_NOM_CLK_RATE		984000000
>> +#define L3_TURBO_CLK_RATE	1272000000
> 
> Please inline these values
> 
ok.

>> +
>> +enum {
>> +	P_XO,
>> +	P_GPLL0,
>> +	P_APSS_PLL_EARLY,
>> +	P_L3_PLL,
>> +};
>> +
>> +struct apss_clk {
>> +	struct notifier_block cpu_clk_notifier;
>> +	struct clk_hw *hw;
>> +	struct device *dev;
>> +	struct clk *l3_clk;
>> +};
>> +
> 
>> +static struct clk_branch l3_core_clk = {
>> +	.halt_reg = 0x1008c,
>> +	.clkr = {
>> +		.enable_reg = 0x1008c,
>> +		.enable_mask = BIT(0),
>> +		.hw.init = &(struct clk_init_data){
>> +			.name = "l3_clk",
>> +			.parent_hws = (const struct clk_hw *[]){
>> +				&l3_clk_src.clkr.hw },
> 
> 	&l3_clk_src.clkr.hw
> },
> 
>> +static unsigned long get_l3_clk_from_tbl(unsigned long rate)
>> +{
>> +	struct clk_rcg2 *l3_rcg2 = container_of(&l3_clk_src.clkr, struct clk_rcg2, clkr);
>> +	u8 max_clk = sizeof(ftbl_apss_clk_src) / sizeof(struct freq_tbl);
>> +	u8 loop;
>> +
>> +	for (loop = 0; loop < max_clk; loop++)
>> +		if (ftbl_apss_clk_src[loop].freq == rate)
>> +			return l3_rcg2->freq_tbl[loop].freq;
> 
> This looks extremely explosive if anyone makes changes to the driver..
> 
> Use an OPP table in the devicetree instead
> 
ok, already using OPPtable for cpu. To understand better, since L3 clk
is separate that needs to be scaled along with cpu, are you suggesting
to use dev_pm_opp_find_freq here for indexing ?

> And please add a newline before the return statement
> 
ok

>> +	return 0;
>> +}
>> +
>> +static int cpu_clk_notifier_fn(struct notifier_block *nb, unsigned long action,
>> +			       void *data)
>> +{
>> +	struct apss_clk *apss_ipq5424_cfg = container_of(nb, struct apss_clk, cpu_clk_notifier);
>> +	struct clk_notifier_data *cnd = (struct clk_notifier_data *)data;
>> +	struct device *dev = apss_ipq5424_cfg->dev;
>> +	unsigned long rate = 0, l3_rate;
>> +	int err = 0;
> 
> Please use 'ret'
> 
ok

>> +
>> +	dev_dbg(dev, "action:%ld old_rate:%ld new_rate:%ld\n", action,
>> +		cnd->old_rate, cnd->new_rate);
>> +
>> +	switch (action) {
>> +	case PRE_RATE_CHANGE:
>> +		if (cnd->old_rate < cnd->new_rate)
>> +			rate = cnd->new_rate;
>> +	break;
> 
> Why are the breaks indented like this?
> 
ok, will fix

>> +	case POST_RATE_CHANGE:
>> +		if (cnd->old_rate > cnd->new_rate)
>> +			rate = cnd->new_rate;
>> +	break;
>> +	};
>> +
>> +	if (!rate)
>> +		goto notif_ret;
> 
> In cases like these, just return directly instead of jumping
> 
ok

>> +
>> +	l3_rate = get_l3_clk_from_tbl(rate);
>> +	if (!l3_rate) {
>> +		dev_err(dev, "Failed to get l3 clock rate from l3_tbl\n");
>> +		return NOTIFY_BAD;
>> +	}
>> +
>> +	err = clk_set_rate(apss_ipq5424_cfg->l3_clk, l3_rate);
>> +	if (err) {
>> +		dev_err(dev, "Failed to set l3 clock rate(%ld) err(%d)\n", l3_rate, err);
>> +		return NOTIFY_BAD;
>> +	}
>> +
>> +notif_ret:
>> +	return NOTIFY_OK;
>> +}
>> +
>> +static int apss_ipq5424_probe(struct platform_device *pdev)
>> +{
>> +	struct device *dev = &pdev->dev;
>> +	struct apss_clk *apss_ipq5424_cfg;
>> +	struct regmap *regmap;
>> +	void __iomem *base;
>> +	int ret;
>> +
>> +	apss_ipq5424_cfg = devm_kzalloc(&pdev->dev, sizeof(struct apss_clk), GFP_KERNEL);
> 
> Since there is no "config" in there, something like "ipq5424_apsscc" would be
> more fitting
> 
ok

>> +	if (IS_ERR_OR_NULL(apss_ipq5424_cfg))
>> +		return PTR_ERR(apss_ipq5424_cfg);
> 
> https://elixir.bootlin.com/linux/v6.13/source/include/linux/device.h#L326-L329
> |_
>     > elixir.bootlin.com/linux/v6.13/source/drivers/base/devres.c#L819-L820
> 
> It can never throw an errno, just check for if (!apss...)
> 
ok

>> +
>> +	base = devm_platform_ioremap_resource(pdev, 0);
>> +	if (IS_ERR(base))
>> +		return PTR_ERR(base);
>> +
>> +	regmap = devm_regmap_init_mmio(dev, base, &apss_ipq5424_regmap_config);
>> +	if (!regmap)
>> +		return PTR_ERR(regmap);
> 
> devm_platform_get_and_ioremap_resource()
> 
ok

>> +
>> +	clk_alpha_pll_configure(&ipq5424_l3_pll, regmap, &l3_pll_config);
>> +
>> +	clk_alpha_pll_configure(&ipq5424_apss_pll, regmap, &apss_pll_config);
>> +
>> +	ret = qcom_cc_really_probe(dev, &apss_ipq5424_desc, regmap);
>> +	if (ret)
>> +		return ret;
>> +
>> +	dev_dbg(&pdev->dev, "Registered APSS & L3 clock provider\n");
>> +
>> +	apss_ipq5424_cfg->dev = dev;
>> +	apss_ipq5424_cfg->hw = &apss_silver_clk_src.clkr.hw;
>> +	apss_ipq5424_cfg->cpu_clk_notifier.notifier_call = cpu_clk_notifier_fn;
>> +
>> +	apss_ipq5424_cfg->l3_clk = clk_hw_get_clk(&l3_core_clk.clkr.hw, "l3_clk");
>> +	if (IS_ERR(apss_ipq5424_cfg->l3_clk)) {
>> +		dev_err(&pdev->dev, "Failed to get L3 clk, %ld\n",
>> +			PTR_ERR(apss_ipq5424_cfg->l3_clk));
>> +		return PTR_ERR(apss_ipq5424_cfg->l3_clk);
>> +	}
> 
> Now that you'll use OPP, you can drop all this getting.. maybe even the
> apss_ipq5424_cfg struct could be let go

ok, is the suggestion here to use devm_pm_opp_set_config ?

>> +
>> +	ret = devm_clk_notifier_register(&pdev->dev, apss_ipq5424_cfg->hw->clk,
>> +					 &apss_ipq5424_cfg->cpu_clk_notifier);
>> +	if (ret)
>> +		return ret;
>> +
>> +	return 0;
> 
> Just return ret instead
> 
ok

>> +}
>> +
>> +static const struct of_device_id apss_ipq5424_match_table[] = {
>> +	{ .compatible = "qcom,ipq5424-apss-clk" },
>> +	{ }
>> +};
>> +MODULE_DEVICE_TABLE(of, apss_ipq5424_match_table);
>> +
>> +static struct platform_driver apss_ipq5424_driver = {
>> +	.probe = apss_ipq5424_probe,
>> +	.driver = {
>> +		.name   = "apss-ipq5424-clk",
>> +		.of_match_table = apss_ipq5424_match_table,
>> +	},
>> +};
>> +
>> +module_platform_driver(apss_ipq5424_driver);
>> +
>> +MODULE_DESCRIPTION("QCOM APSS IPQ5424 CLK Driver");
>> +MODULE_LICENSE("GPL v2");
> 
> Please don't skip running 'checkpatch'.

Infact ran it with --strict as well. But thought "GPL V2" was correct
and let it. Anyways will change.

Regards,
  Sricharan

Re: [PATCH 1/4] dt-bindings: clock: ipq5424-apss-clk: Add ipq5424 apss clock controller

[Sricharan Ramabadhran]

On 1/28/2025 6:00 PM, Krzysztof Kozlowski wrote:
> On 28/01/2025 12:15, Sricharan Ramabadhran wrote:
>>
>>
>> On 1/28/2025 1:04 PM, Krzysztof Kozlowski wrote:
>>> On 27/01/2025 10:31, Sricharan R wrote:
>>>> From: Sricharan Ramabadhran <quic_srichara@quicinc.com>
>>>>
>>>> The CPU core in ipq5424 is clocked by a huayra PLL with RCG support.
>>>> The RCG and PLL have a separate register space from the GCC.
>>>> Also the L3 cache has a separate pll and needs to be scaled along
>>>> with the CPU.
>>>>
>>>> Co-developed-by: Md Sadre Alam <quic_mdalam@quicinc.com>
>>>> Signed-off-by: Md Sadre Alam <quic_mdalam@quicinc.com>
>>>> Signed-off-by: Sricharan Ramabadhran <quic_srichara@quicinc.com>
>>>
>>> Considering that there were multiple conflicting patches coming from
>>> Qualcomm around IPQ SoCs and that we are in the merge window, I will
>>> skip this patch.
>>>
>>> I suspect this duplicates the other chip as well, but that's your task
>>> to sync up internally.
>>>
>> ok, but this .yaml is specific to IPQ5424 and would not conflict with
>> IPQ5332. That said, will post it after merge window as a part of
>> V3 (for other patch changes) to avoid any confusion.
> 
> 
> But maybe it is the same on ipq5332? or similar? Other works were
> totally de-synced and you ask community to sync them. That's not how it
> works.

This is different from ipq5332, hence we cannot re-use and would have
no conflicts as well.

Regards,
  Sricharan

Re: [PATCH 1/4] dt-bindings: clock: ipq5424-apss-clk: Add ipq5424 apss clock controller

[Konrad Dybcio]

On 28.01.2025 8:34 AM, Krzysztof Kozlowski wrote:
> On 27/01/2025 10:31, Sricharan R wrote:
>> From: Sricharan Ramabadhran <quic_srichara@quicinc.com>
>>
>> The CPU core in ipq5424 is clocked by a huayra PLL with RCG support.
>> The RCG and PLL have a separate register space from the GCC.
>> Also the L3 cache has a separate pll and needs to be scaled along
>> with the CPU.
>>
>> Co-developed-by: Md Sadre Alam <quic_mdalam@quicinc.com>
>> Signed-off-by: Md Sadre Alam <quic_mdalam@quicinc.com>
>> Signed-off-by: Sricharan Ramabadhran <quic_srichara@quicinc.com>
> 
> Considering that there were multiple conflicting patches coming from
> Qualcomm around IPQ SoCs and that we are in the merge window, I will
> skip this patch.

I think you confused this with something else, I don't see any other IPQ
clock patches

Konrad

Re: [PATCH 2/4] clk: qcom: apss-ipq5424: Add ipq5424 apss clock controller

[Konrad Dybcio]

On 30.01.2025 11:03 AM, Sricharan Ramabadhran wrote:
> 
> 
> On 1/28/2025 5:29 PM, Konrad Dybcio wrote:
>> On 27.01.2025 10:31 AM, Sricharan R wrote:
>>> From: Sricharan Ramabadhran <quic_srichara@quicinc.com>
>>>
>>> CPU on Qualcomm ipq5424 is clocked by huayra PLL with RCG support.
>>> Add support for the APSS PLL, RCG and clock enable for ipq5424.
>>> The PLL, RCG register space are clubbed. Hence adding new APSS driver
>>> for both PLL and RCG/CBC control. Also the L3 cache has a separate pll
>>> and needs to be scaled along with the CPU.
>>>
>>> Co-developed-by: Md Sadre Alam <quic_mdalam@quicinc.com>
>>> Signed-off-by: Md Sadre Alam <quic_mdalam@quicinc.com>
>>> Signed-off-by: Sricharan Ramabadhran <quic_srichara@quicinc.com>
>>> ---

[...]

>>> +    clk_alpha_pll_configure(&ipq5424_l3_pll, regmap, &l3_pll_config);
>>> +
>>> +    clk_alpha_pll_configure(&ipq5424_apss_pll, regmap, &apss_pll_config);
>>> +
>>> +    ret = qcom_cc_really_probe(dev, &apss_ipq5424_desc, regmap);
>>> +    if (ret)
>>> +        return ret;
>>> +
>>> +    dev_dbg(&pdev->dev, "Registered APSS & L3 clock provider\n");
>>> +
>>> +    apss_ipq5424_cfg->dev = dev;
>>> +    apss_ipq5424_cfg->hw = &apss_silver_clk_src.clkr.hw;
>>> +    apss_ipq5424_cfg->cpu_clk_notifier.notifier_call = cpu_clk_notifier_fn;
>>> +
>>> +    apss_ipq5424_cfg->l3_clk = clk_hw_get_clk(&l3_core_clk.clkr.hw, "l3_clk");
>>> +    if (IS_ERR(apss_ipq5424_cfg->l3_clk)) {
>>> +        dev_err(&pdev->dev, "Failed to get L3 clk, %ld\n",
>>> +            PTR_ERR(apss_ipq5424_cfg->l3_clk));
>>> +        return PTR_ERR(apss_ipq5424_cfg->l3_clk);
>>> +    }
>>
>> Now that you'll use OPP, you can drop all this getting.. maybe even the
>> apss_ipq5424_cfg struct could be let go
> 
> ok, is the suggestion here to use devm_pm_opp_set_config ?

Since what you tried to do here is binding CPU and L3 frequencies together,
yeah, we can just scale two clocks from OPP.

On some newer platforms using the epss-l3 driver, or on msm8996 with a more
complex setup, we expose the L3 voter as an interconnect, but here it would
seem that we directly control the clock that feeds it.

Konrad

Re: [PATCH 1/4] dt-bindings: clock: ipq5424-apss-clk: Add ipq5424 apss clock controller

[Krzysztof Kozlowski]

On 01/02/2025 16:21, Konrad Dybcio wrote:
> On 28.01.2025 8:34 AM, Krzysztof Kozlowski wrote:
>> On 27/01/2025 10:31, Sricharan R wrote:
>>> From: Sricharan Ramabadhran <quic_srichara@quicinc.com>
>>>
>>> The CPU core in ipq5424 is clocked by a huayra PLL with RCG support.
>>> The RCG and PLL have a separate register space from the GCC.
>>> Also the L3 cache has a separate pll and needs to be scaled along
>>> with the CPU.
>>>
>>> Co-developed-by: Md Sadre Alam <quic_mdalam@quicinc.com>
>>> Signed-off-by: Md Sadre Alam <quic_mdalam@quicinc.com>
>>> Signed-off-by: Sricharan Ramabadhran <quic_srichara@quicinc.com>
>>
>> Considering that there were multiple conflicting patches coming from
>> Qualcomm around IPQ SoCs and that we are in the merge window, I will
>> skip this patch.
> 
> I think you confused this with something else, I don't see any other IPQ
> clock patches

The conflicts were not about clocks, but I just don't want to spend my
time to figure out whether clocks also have conflicting work or not.

Best regards,
Krzysztof

Re: [PATCH 2/4] clk: qcom: apss-ipq5424: Add ipq5424 apss clock controller

[Sricharan Ramabadhran]

On 2/1/2025 8:55 PM, Konrad Dybcio wrote:
> On 30.01.2025 11:03 AM, Sricharan Ramabadhran wrote:
>>
>>
>> On 1/28/2025 5:29 PM, Konrad Dybcio wrote:
>>> On 27.01.2025 10:31 AM, Sricharan R wrote:
>>>> From: Sricharan Ramabadhran <quic_srichara@quicinc.com>
>>>>
>>>> CPU on Qualcomm ipq5424 is clocked by huayra PLL with RCG support.
>>>> Add support for the APSS PLL, RCG and clock enable for ipq5424.
>>>> The PLL, RCG register space are clubbed. Hence adding new APSS driver
>>>> for both PLL and RCG/CBC control. Also the L3 cache has a separate pll
>>>> and needs to be scaled along with the CPU.
>>>>
>>>> Co-developed-by: Md Sadre Alam <quic_mdalam@quicinc.com>
>>>> Signed-off-by: Md Sadre Alam <quic_mdalam@quicinc.com>
>>>> Signed-off-by: Sricharan Ramabadhran <quic_srichara@quicinc.com>
>>>> ---
> 
> [...]
> 
>>>> +    clk_alpha_pll_configure(&ipq5424_l3_pll, regmap, &l3_pll_config);
>>>> +
>>>> +    clk_alpha_pll_configure(&ipq5424_apss_pll, regmap, &apss_pll_config);
>>>> +
>>>> +    ret = qcom_cc_really_probe(dev, &apss_ipq5424_desc, regmap);
>>>> +    if (ret)
>>>> +        return ret;
>>>> +
>>>> +    dev_dbg(&pdev->dev, "Registered APSS & L3 clock provider\n");
>>>> +
>>>> +    apss_ipq5424_cfg->dev = dev;
>>>> +    apss_ipq5424_cfg->hw = &apss_silver_clk_src.clkr.hw;
>>>> +    apss_ipq5424_cfg->cpu_clk_notifier.notifier_call = cpu_clk_notifier_fn;
>>>> +
>>>> +    apss_ipq5424_cfg->l3_clk = clk_hw_get_clk(&l3_core_clk.clkr.hw, "l3_clk");
>>>> +    if (IS_ERR(apss_ipq5424_cfg->l3_clk)) {
>>>> +        dev_err(&pdev->dev, "Failed to get L3 clk, %ld\n",
>>>> +            PTR_ERR(apss_ipq5424_cfg->l3_clk));
>>>> +        return PTR_ERR(apss_ipq5424_cfg->l3_clk);
>>>> +    }
>>>
>>> Now that you'll use OPP, you can drop all this getting.. maybe even the
>>> apss_ipq5424_cfg struct could be let go
>>
>> ok, is the suggestion here to use devm_pm_opp_set_config ?
> 
> Since what you tried to do here is binding CPU and L3 frequencies together,
> yeah, we can just scale two clocks from OPP.
> 
> On some newer platforms using the epss-l3 driver, or on msm8996 with a more
> complex setup, we expose the L3 voter as an interconnect, but here it would
> seem that we directly control the clock that feeds it.

ok, will update and check.

Regards,
Sricharan

Re: [PATCH 2/4] clk: qcom: apss-ipq5424: Add ipq5424 apss clock controller

[Konrad Dybcio]

On 4.02.2025 7:28 AM, Sricharan Ramabadhran wrote:
> 
> 
> On 2/1/2025 8:55 PM, Konrad Dybcio wrote:
>> On 30.01.2025 11:03 AM, Sricharan Ramabadhran wrote:
>>>
>>>
>>> On 1/28/2025 5:29 PM, Konrad Dybcio wrote:
>>>> On 27.01.2025 10:31 AM, Sricharan R wrote:
>>>>> From: Sricharan Ramabadhran <quic_srichara@quicinc.com>
>>>>>
>>>>> CPU on Qualcomm ipq5424 is clocked by huayra PLL with RCG support.
>>>>> Add support for the APSS PLL, RCG and clock enable for ipq5424.
>>>>> The PLL, RCG register space are clubbed. Hence adding new APSS driver
>>>>> for both PLL and RCG/CBC control. Also the L3 cache has a separate pll
>>>>> and needs to be scaled along with the CPU.
>>>>>
>>>>> Co-developed-by: Md Sadre Alam <quic_mdalam@quicinc.com>
>>>>> Signed-off-by: Md Sadre Alam <quic_mdalam@quicinc.com>
>>>>> Signed-off-by: Sricharan Ramabadhran <quic_srichara@quicinc.com>
>>>>> ---
>>
>> [...]
>>
>>>>> +    clk_alpha_pll_configure(&ipq5424_l3_pll, regmap, &l3_pll_config);
>>>>> +
>>>>> +    clk_alpha_pll_configure(&ipq5424_apss_pll, regmap, &apss_pll_config);
>>>>> +
>>>>> +    ret = qcom_cc_really_probe(dev, &apss_ipq5424_desc, regmap);
>>>>> +    if (ret)
>>>>> +        return ret;
>>>>> +
>>>>> +    dev_dbg(&pdev->dev, "Registered APSS & L3 clock provider\n");
>>>>> +
>>>>> +    apss_ipq5424_cfg->dev = dev;
>>>>> +    apss_ipq5424_cfg->hw = &apss_silver_clk_src.clkr.hw;
>>>>> +    apss_ipq5424_cfg->cpu_clk_notifier.notifier_call = cpu_clk_notifier_fn;
>>>>> +
>>>>> +    apss_ipq5424_cfg->l3_clk = clk_hw_get_clk(&l3_core_clk.clkr.hw, "l3_clk");
>>>>> +    if (IS_ERR(apss_ipq5424_cfg->l3_clk)) {
>>>>> +        dev_err(&pdev->dev, "Failed to get L3 clk, %ld\n",
>>>>> +            PTR_ERR(apss_ipq5424_cfg->l3_clk));
>>>>> +        return PTR_ERR(apss_ipq5424_cfg->l3_clk);
>>>>> +    }
>>>>
>>>> Now that you'll use OPP, you can drop all this getting.. maybe even the
>>>> apss_ipq5424_cfg struct could be let go
>>>
>>> ok, is the suggestion here to use devm_pm_opp_set_config ?
>>
>> Since what you tried to do here is binding CPU and L3 frequencies together,
>> yeah, we can just scale two clocks from OPP.
>>
>> On some newer platforms using the epss-l3 driver, or on msm8996 with a more
>> complex setup, we expose the L3 voter as an interconnect, but here it would
>> seem that we directly control the clock that feeds it.
> 
> ok, will update and check.

+Dmitry

Giving it yet another thought, we now have infrastructure in clk/qcom/common.c
to register icc clocks. We can register the L3 one as such and make the
description like:

cpu0: cpu@0 {
	[...]
	interconnects = <&apss L3_CLK>;
	[...]
};

cpu_opp_table: opp-table {
	opp-1234000 {
		opp-hz = /bits/ 64 <1234000>;
		opp-peak-kBps = <SOME_L3_FREQ>;
	};
};

as that will both match how we modeled msm8996 & require less code changes

Dmitry, Bjorn?

Konrad

Re: [PATCH 4/4] arm64: dts: qcom: ipq5424: Enable cpufreq support

[Konrad Dybcio]

On 27.01.2025 10:31 AM, Sricharan R wrote:
> From: Sricharan Ramabadhran <quic_srichara@quicinc.com>

subject: you're not enabling support, you're either enabling cpufreq (the
feature), or adding support for it

> Add the qfprom, cpu clocks, A53 PLL and cpu-opp-table required for
> CPU clock scaling.
> 
> Signed-off-by: Sricharan Ramabadhran <quic_srichara@quicinc.com>
> ---

[...]

> +	cpu_opp_table: opp-table-cpu {
> +		compatible = "operating-points-v2-kryo-cpu";
> +		opp-shared;
> +		nvmem-cells = <&cpu_speed_bin>;
> +
> +		/*
> +		 * CPU supports two frequencies and the fuse has LValue instead
> +		 * of limits. As only two frequencies are supported, considering
> +		 * zero Lvalue as no limit and Lvalue as 1.4GHz limit.
> +		 * ------------------------------------------------------------
> +		 * Frequency     BIT1    BIT0    opp-supported-hw
> +		 *	      1.4GHz  No Limit
> +		 * ------------------------------------------------------------
> +		 * 1416000000      1       1	    0x3
> +		 * 1800000000      0       1	    0x1
> +		 * ------------------------------------------------------------
> +		 */

This is trivially inferred from the nodes below

> +
> +		opp-1416000000 {
> +			opp-hz = /bits/ 64 <1416000000>;
> +			opp-microvolt = <1>;
> +			opp-supported-hw = <0x3>;
> +			clock-latency-ns = <200000>;
> +		};
> +
> +		opp-1800000000 {
> +			opp-hz = /bits/ 64 <1800000000>;
> +			opp-microvolt = <2>;
> +			opp-supported-hw = <0x1>;
> +			clock-latency-ns = <200000>;
> +		};
> +	};
> +
>  	memory@80000000 {
>  		device_type = "memory";
>  		/* We expect the bootloader to fill in the size */
> @@ -151,6 +202,18 @@ soc@0 {
>  		#size-cells = <2>;
>  		ranges = <0 0 0 0 0x10 0>;
>  
> +		qfprom@a6000 {
> +			compatible = "qcom,qfprom";
> +			reg = <0x0 0xa6000 0x0 0x1000>;

Please pad the address part to 8 hex digits with leading zeroes

Konrad