[PATCH v1 00/10] Enable cpufreq for IPQ5332 & IPQ9574

[PATCH v1 00/10] Enable cpufreq for IPQ5332 & IPQ9574

[Varadarajan Narayanan]

Depends On:
https://lore.kernel.org/linux-arm-msm/1693474133-10467-1-git-send-email-quic_varada@quicinc.com/
https://lore.kernel.org/linux-arm-msm/20230904-gpll_cleanup-v1-0-de2c448f1188@quicinc.com/

This patch series aims to enable cpufreq for IPQ5332 and IPQ9574.
For IPQ5332, a minor enhancement to Stromer Plus ops and a safe
source switch is needed before cpu freq can be enabled.

These are also included in this series. Posting this as a single
series. Please let me know if this is not correct, will split in
the subsequent revisions.

Passed the following DT related validations
make W=1 ARCH=arm64 -j16 DT_CHECKER_FLAGS='-v -m' dt_binding_check DT_SCHEMA_FILES=qcom
make W=1 ARCH=arm64 -j16 CHECK_DTBS=y DT_SCHEMA_FILES=qcom dtbs_check

For IPQ5332:
~~~~~~~~~~~
	* This patch series introduces stromer plus ops which
	  builds on stromer ops and implements a different
	  set_rate and determine_rate.

	  A different set_rate is needed since stromer plus PLLs
	  do not support dynamic frequency scaling. To switch
	  between frequencies, we have to shut down the PLL,
	  configure the L and ALPHA values and turn on again. So
	  introduce the separate set of ops for Stromer Plus PLL.

	* Update ipq_pll_stromer_plus to use clk_alpha_pll_stromer_plus_ops
	  instead of clk_alpha_pll_stromer_ops.

	* Set 'l' value to a value that is supported on all SKUs.

	* Provide safe source switch for a53pll

	* Include IPQ5332 in cpufreq nvmem framework

	* Add OPP details to device tree

For IPQ9574:
~~~~~~~~~~~
	* Include IPQ9574 in cpufreq nvmem framework

	* Add OPP details to device tree

Varadarajan Narayanan (10):
  clk: qcom: clk-alpha-pll: introduce stromer plus ops
  clk: qcom: apss-ipq-pll: Use stromer plus ops for stromer plus pll
  clk: qcom: apss-ipq-pll: Fix 'l' value for ipq5332_pll_config
  clk: qcom: apss-ipq6018: ipq5332: add safe source switch for a53pll
  dt-bindings: cpufreq: qcom-cpufreq-nvmem: document IPQ5332
  cpufreq: qti: Enable cpufreq for ipq53xx
  arm64: dts: qcom: ipq5332: populate the opp table based on the eFuse
  dt-bindings: cpufreq: qcom-cpufreq-nvmem: document IPQ9574
  cpufreq: qti: Introduce cpufreq for ipq95xx
  arm64: dts: qcom: ipq9574: populate the opp table based on the eFuse

 .../bindings/cpufreq/qcom-cpufreq-nvmem.yaml       |  2 +
 arch/arm64/boot/dts/qcom/ipq5332.dtsi              | 34 ++++++++++-
 arch/arm64/boot/dts/qcom/ipq9574.dtsi              | 38 +++++++++++-
 drivers/clk/qcom/apss-ipq-pll.c                    |  4 +-
 drivers/clk/qcom/apss-ipq6018.c                    | 54 ++++++++++++++++-
 drivers/clk/qcom/clk-alpha-pll.c                   | 68 ++++++++++++++++++++++
 drivers/clk/qcom/clk-alpha-pll.h                   |  1 +
 drivers/cpufreq/cpufreq-dt-platdev.c               |  2 +
 drivers/cpufreq/qcom-cpufreq-nvmem.c               | 35 +++++++++++
 9 files changed, 231 insertions(+), 7 deletions(-)

-- 
2.7.4

[PATCH v1 01/10] clk: qcom: clk-alpha-pll: introduce stromer plus ops

[Varadarajan Narayanan]

Stromer plus APSS PLL does not support dynamic frequency scaling.
To switch between frequencies, we have to shut down the PLL,
configure the L and ALPHA values and turn on again. So introduce the
separate set of ops for Stromer Plus PLL.

Signed-off-by: Kathiravan T <quic_kathirav@quicinc.com>
Signed-off-by: Varadarajan Narayanan <quic_varada@quicinc.com>
---
 drivers/clk/qcom/clk-alpha-pll.c | 68 ++++++++++++++++++++++++++++++++++++++++
 drivers/clk/qcom/clk-alpha-pll.h |  1 +
 2 files changed, 69 insertions(+)

diff --git a/drivers/clk/qcom/clk-alpha-pll.c b/drivers/clk/qcom/clk-alpha-pll.c
index e4ef645..2ef81f7 100644
--- a/drivers/clk/qcom/clk-alpha-pll.c
+++ b/drivers/clk/qcom/clk-alpha-pll.c
@@ -2479,3 +2479,71 @@ const struct clk_ops clk_alpha_pll_stromer_ops = {
 	.set_rate = clk_alpha_pll_stromer_set_rate,
 };
 EXPORT_SYMBOL_GPL(clk_alpha_pll_stromer_ops);
+
+static int clk_alpha_pll_stromer_plus_determine_rate(struct clk_hw *hw,
+						     struct clk_rate_request *req)
+{
+	struct clk_alpha_pll *pll = to_clk_alpha_pll(hw);
+	u32 l, alpha_width = pll_alpha_width(pll);
+	u64 a;
+
+	req->rate = alpha_pll_round_rate(req->rate, req->best_parent_rate, &l,
+					 &a, alpha_width);
+	return 0;
+}
+
+static int clk_alpha_pll_stromer_plus_set_rate(struct clk_hw *hw,
+					       unsigned long rate,
+					       unsigned long prate)
+{
+	struct clk_alpha_pll *pll = to_clk_alpha_pll(hw);
+	u32 l, alpha_width = pll_alpha_width(pll);
+	int ret;
+	u64 a;
+
+	rate = alpha_pll_round_rate(rate, prate, &l, &a, alpha_width);
+
+	regmap_write(pll->clkr.regmap, PLL_MODE(pll), 0);
+
+	/* Delay of 2 output clock ticks required until output is disabled */
+	udelay(1);
+
+	regmap_write(pll->clkr.regmap, PLL_L_VAL(pll), l);
+
+	if (alpha_width > ALPHA_BITWIDTH)
+		a <<= alpha_width - ALPHA_BITWIDTH;
+
+	regmap_write(pll->clkr.regmap, PLL_ALPHA_VAL(pll), a);
+	regmap_write(pll->clkr.regmap, PLL_ALPHA_VAL_U(pll),
+					a >> ALPHA_BITWIDTH);
+
+	regmap_write(pll->clkr.regmap, PLL_MODE(pll), PLL_BYPASSNL);
+
+	/* Wait five micro seconds or more */
+	udelay(5);
+	regmap_update_bits(pll->clkr.regmap, PLL_MODE(pll), PLL_RESET_N,
+			   PLL_RESET_N);
+
+	/* The lock time should be less than 50 micro seconds worst case */
+	udelay(50);
+
+	ret = wait_for_pll_enable_lock(pll);
+	if (ret) {
+		pr_err("alpha pll running in 800 MHz with source GPLL0\n");
+		return ret;
+	}
+	regmap_update_bits(pll->clkr.regmap, PLL_MODE(pll), PLL_OUTCTRL,
+			   PLL_OUTCTRL);
+
+	return 0;
+}
+
+const struct clk_ops clk_alpha_pll_stromer_plus_ops = {
+	.enable = clk_alpha_pll_enable,
+	.disable = clk_alpha_pll_disable,
+	.is_enabled = clk_alpha_pll_is_enabled,
+	.recalc_rate = clk_alpha_pll_recalc_rate,
+	.determine_rate = clk_alpha_pll_stromer_plus_determine_rate,
+	.set_rate = clk_alpha_pll_stromer_plus_set_rate,
+};
+EXPORT_SYMBOL_GPL(clk_alpha_pll_stromer_plus_ops);
diff --git a/drivers/clk/qcom/clk-alpha-pll.h b/drivers/clk/qcom/clk-alpha-pll.h
index e4bd863..903fbab 100644
--- a/drivers/clk/qcom/clk-alpha-pll.h
+++ b/drivers/clk/qcom/clk-alpha-pll.h
@@ -152,6 +152,7 @@ extern const struct clk_ops clk_alpha_pll_postdiv_ops;
 extern const struct clk_ops clk_alpha_pll_huayra_ops;
 extern const struct clk_ops clk_alpha_pll_postdiv_ro_ops;
 extern const struct clk_ops clk_alpha_pll_stromer_ops;
+extern const struct clk_ops clk_alpha_pll_stromer_plus_ops;
 
 extern const struct clk_ops clk_alpha_pll_fabia_ops;
 extern const struct clk_ops clk_alpha_pll_fixed_fabia_ops;
-- 
2.7.4

[PATCH v1 02/10] clk: qcom: apss-ipq-pll: Use stromer plus ops for stromer plus pll

[Varadarajan Narayanan]

The set rate and determine rate operations are different between
Stromer and Stromer Plus PLLs. Hence, use stromer plus ops for
ipq_pll_stromer_plus.

Signed-off-by: Kathiravan T <quic_kathirav@quicinc.com>
Signed-off-by: Varadarajan Narayanan <quic_varada@quicinc.com>
---
 drivers/clk/qcom/apss-ipq-pll.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/clk/qcom/apss-ipq-pll.c b/drivers/clk/qcom/apss-ipq-pll.c
index e170331..18c4ffe 100644
--- a/drivers/clk/qcom/apss-ipq-pll.c
+++ b/drivers/clk/qcom/apss-ipq-pll.c
@@ -68,7 +68,7 @@ static struct clk_alpha_pll ipq_pll_stromer_plus = {
 				.fw_name = "xo",
 			},
 			.num_parents = 1,
-			.ops = &clk_alpha_pll_stromer_ops,
+			.ops = &clk_alpha_pll_stromer_plus_ops,
 		},
 	},
 };
-- 
2.7.4

[PATCH v1 03/10] clk: qcom: apss-ipq-pll: Fix 'l' value for ipq5332_pll_config

[Varadarajan Narayanan]

The earlier 'l' value of 0x3e is for 1.5GHz. Not all SKUs support
this frequency. Hence set it to 0x2d to get 1.1GHz which is
supported in all SKUs.

Fixes: c7ef7fbb1ccf ("clk: qcom: apss-ipq-pll: add support for IPQ5332")
Signed-off-by: Kathiravan T <quic_kathirav@quicinc.com>
Signed-off-by: Varadarajan Narayanan <quic_varada@quicinc.com>
---
 drivers/clk/qcom/apss-ipq-pll.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/clk/qcom/apss-ipq-pll.c b/drivers/clk/qcom/apss-ipq-pll.c
index 18c4ffe..41279e5 100644
--- a/drivers/clk/qcom/apss-ipq-pll.c
+++ b/drivers/clk/qcom/apss-ipq-pll.c
@@ -74,7 +74,7 @@ static struct clk_alpha_pll ipq_pll_stromer_plus = {
 };
 
 static const struct alpha_pll_config ipq5332_pll_config = {
-	.l = 0x3e,
+	.l = 0x2d,
 	.config_ctl_val = 0x4001075b,
 	.config_ctl_hi_val = 0x304,
 	.main_output_mask = BIT(0),
-- 
2.7.4

[PATCH v1 04/10] clk: qcom: apss-ipq6018: ipq5332: add safe source switch for a53pll

[Varadarajan Narayanan]

Stromer Plus PLL found on IPQ53xx doesn't support dynamic
frequency scaling. To achieve the same, we need to park the APPS
PLL source to GPLL0, re configure the PLL and then switch the
source to APSS_PLL_EARLY.

To support this, register a clock notifier to get the PRE_RATE
and POST_RATE notification. Change the APSS PLL source to GPLL0
when PRE_RATE notification is received, then configure the PLL
and then change back the source to APSS_PLL_EARLY.

Signed-off-by: Kathiravan T <quic_kathirav@quicinc.com>
Signed-off-by: Varadarajan Narayanan <quic_varada@quicinc.com>
---
 drivers/clk/qcom/apss-ipq6018.c | 54 ++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 53 insertions(+), 1 deletion(-)

diff --git a/drivers/clk/qcom/apss-ipq6018.c b/drivers/clk/qcom/apss-ipq6018.c
index 4e13a08..ffb6ab5 100644
--- a/drivers/clk/qcom/apss-ipq6018.c
+++ b/drivers/clk/qcom/apss-ipq6018.c
@@ -9,8 +9,11 @@
 #include <linux/clk-provider.h>
 #include <linux/regmap.h>
 #include <linux/module.h>
+#include <linux/clk.h>
+#include <linux/soc/qcom/smem.h>
 
 #include <dt-bindings/clock/qcom,apss-ipq.h>
+#include <dt-bindings/arm/qcom,ids.h>
 
 #include "common.h"
 #include "clk-regmap.h"
@@ -84,15 +87,64 @@ static const struct qcom_cc_desc apss_ipq6018_desc = {
 	.num_clks = ARRAY_SIZE(apss_ipq6018_clks),
 };
 
+static int cpu_clk_notifier_fn(struct notifier_block *nb, unsigned long action,
+				void *data)
+{
+	u8 index;
+	int err;
+
+	if (action == PRE_RATE_CHANGE)
+		index = P_GPLL0;
+	else if (action == POST_RATE_CHANGE)
+		index = P_APSS_PLL_EARLY;
+	else
+		return 0;
+
+	err = clk_rcg2_mux_closest_ops.set_parent(&apcs_alias0_clk_src.clkr.hw,
+						  index);
+
+	return notifier_from_errno(err);
+}
+
+static struct notifier_block cpu_clk_notifier = {
+	.notifier_call = cpu_clk_notifier_fn,
+};
+
 static int apss_ipq6018_probe(struct platform_device *pdev)
 {
 	struct regmap *regmap;
+	u32 soc_id;
+	int ret;
+
+	ret = qcom_smem_get_soc_id(&soc_id);
+	if (ret)
+		return ret;
 
 	regmap = dev_get_regmap(pdev->dev.parent, NULL);
 	if (!regmap)
 		return -ENODEV;
 
-	return qcom_cc_really_probe(pdev, &apss_ipq6018_desc, regmap);
+	ret = qcom_cc_really_probe(pdev, &apss_ipq6018_desc, regmap);
+	if (ret)
+		return ret;
+
+	switch (soc_id) {
+	/*
+	 * Only below variants of IPQ53xx support scaling
+	 */
+	case QCOM_ID_IPQ5332:
+	case QCOM_ID_IPQ5322:
+	case QCOM_ID_IPQ5300:
+		ret = clk_notifier_register(apcs_alias0_clk_src.clkr.hw.clk,
+						&cpu_clk_notifier);
+		if (ret)
+			return ret;
+		break;
+	default:
+		break;
+	}
+
+	return 0;
 }
 
 static struct platform_driver apss_ipq6018_driver = {
-- 
2.7.4

[PATCH v1 05/10] dt-bindings: cpufreq: qcom-cpufreq-nvmem: document IPQ5332

[Varadarajan Narayanan]

Document IPQ5332 compatible for Qcom NVMEM CPUFreq driver.

Signed-off-by: Varadarajan Narayanan <quic_varada@quicinc.com>
---
 Documentation/devicetree/bindings/cpufreq/qcom-cpufreq-nvmem.yaml | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/cpufreq/qcom-cpufreq-nvmem.yaml b/Documentation/devicetree/bindings/cpufreq/qcom-cpufreq-nvmem.yaml
index 7e1bb99..75e996a 100644
--- a/Documentation/devicetree/bindings/cpufreq/qcom-cpufreq-nvmem.yaml
+++ b/Documentation/devicetree/bindings/cpufreq/qcom-cpufreq-nvmem.yaml
@@ -27,6 +27,7 @@ select:
         enum:
           - qcom,apq8064
           - qcom,apq8096
+          - qcom,ipq5332
           - qcom,ipq8064
           - qcom,ipq8074
           - qcom,msm8939
-- 
2.7.4

[PATCH v1 06/10] cpufreq: qti: Enable cpufreq for ipq53xx

[Varadarajan Narayanan]

IPQ53xx 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 ipq53xx 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: Kathiravan T <quic_kathirav@quicinc.com>
Signed-off-by: Varadarajan Narayanan <quic_varada@quicinc.com>
---
 drivers/cpufreq/cpufreq-dt-platdev.c |  1 +
 drivers/cpufreq/qcom-cpufreq-nvmem.c | 15 +++++++++++++++
 2 files changed, 16 insertions(+)

diff --git a/drivers/cpufreq/cpufreq-dt-platdev.c b/drivers/cpufreq/cpufreq-dt-platdev.c
index 02ec58a..f0c45d4 100644
--- a/drivers/cpufreq/cpufreq-dt-platdev.c
+++ b/drivers/cpufreq/cpufreq-dt-platdev.c
@@ -178,6 +178,7 @@ static const struct of_device_id blocklist[] __initconst = {
 	{ .compatible = "ti,am625", },
 	{ .compatible = "ti,am62a7", },
 
+	{ .compatible = "qcom,ipq5332", },
 	{ .compatible = "qcom,ipq8064", },
 	{ .compatible = "qcom,apq8064", },
 	{ .compatible = "qcom,msm8974", },
diff --git a/drivers/cpufreq/qcom-cpufreq-nvmem.c b/drivers/cpufreq/qcom-cpufreq-nvmem.c
index 84d7033..49d21b0 100644
--- a/drivers/cpufreq/qcom-cpufreq-nvmem.c
+++ b/drivers/cpufreq/qcom-cpufreq-nvmem.c
@@ -146,6 +146,20 @@ static int qcom_cpufreq_kryo_name_version(struct device *cpu_dev,
 		return PTR_ERR(speedbin);
 
 	switch (msm_id) {
+	case QCOM_ID_IPQ5332:
+	case QCOM_ID_IPQ5322:
+	case QCOM_ID_IPQ5312:
+	case QCOM_ID_IPQ5302:
+	case QCOM_ID_IPQ5300:
+		/* Fuse Value    Freq    BIT to set
+		 * ---------------------------------
+		 *   2’b00     No Limit     BIT(0)
+		 *   2’b01     1.5 GHz      BIT(1)
+		 *   2’b10     1.2 Ghz      BIT(2)
+		 *   2’b11     1.0 GHz      BIT(3)
+		 */
+		drv->versions = 1 << (unsigned int)(*speedbin);
+		break;
 	case QCOM_ID_MSM8996:
 	case QCOM_ID_APQ8096:
 		drv->versions = 1 << (unsigned int)(*speedbin);
@@ -359,6 +373,7 @@ static const struct of_device_id qcom_cpufreq_match_list[] __initconst = {
 	{ .compatible = "qcom,apq8096", .data = &match_data_kryo },
 	{ .compatible = "qcom,msm8996", .data = &match_data_kryo },
 	{ .compatible = "qcom,qcs404", .data = &match_data_qcs404 },
+	{ .compatible = "qcom,ipq5332", .data = &match_data_kryo },
 	{ .compatible = "qcom,ipq8064", .data = &match_data_krait },
 	{ .compatible = "qcom,apq8064", .data = &match_data_krait },
 	{ .compatible = "qcom,msm8974", .data = &match_data_krait },
-- 
2.7.4

[PATCH v1 07/10] arm64: dts: qcom: ipq5332: populate the opp table based on the eFuse

[Varadarajan Narayanan]

IPQ53xx 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.

Add support to read the eFuse and populate the OPPs based on it.

Signed-off-by: Kathiravan T <quic_kathirav@quicinc.com>
Signed-off-by: Varadarajan Narayanan <quic_varada@quicinc.com>
---
 arch/arm64/boot/dts/qcom/ipq5332.dtsi | 34 +++++++++++++++++++++++++++++++---
 1 file changed, 31 insertions(+), 3 deletions(-)

diff --git a/arch/arm64/boot/dts/qcom/ipq5332.dtsi b/arch/arm64/boot/dts/qcom/ipq5332.dtsi
index 82761ae..3ca3f34 100644
--- a/arch/arm64/boot/dts/qcom/ipq5332.dtsi
+++ b/arch/arm64/boot/dts/qcom/ipq5332.dtsi
@@ -91,11 +91,34 @@
 	};
 
 	cpu_opp_table: opp-table-cpu {
-		compatible = "operating-points-v2";
+		compatible = "operating-points-v2-kryo-cpu";
 		opp-shared;
+		nvmem-cells = <&cpu_speed_bin>;
+		nvmem-cell-names = "speed_bin";
+
+		/*
+		 * Listed all supported CPU frequencies and opp-supported-hw
+		 * values to select CPU frequencies based on the limits fused.
+		 * ------------------------------------------------------------
+		 * Frequency     BIT3   BIT2   BIT1    BIT0    opp-supported-hw
+		 *              1.0GHz 1.2GHz 1.5GHz No Limit
+		 * ------------------------------------------------------------
+		 * 1100000000     1      1      1       1            0xF
+		 * 1500000000     0      0      1       1            0x3
+		 * -----------------------------------------------------------
+		 */
+
+		opp-1100000000 {
+			opp-hz = /bits/ 64 <1100000000>;
+			opp-microvolt = <850000>;
+			opp-supported-hw = <0xF>;
+			clock-latency-ns = <200000>;
+		};
 
-		opp-1488000000 {
-			opp-hz = /bits/ 64 <1488000000>;
+		opp-1500000000 {
+			opp-hz = /bits/ 64 <1500000000>;
+			opp-microvolt = <950000>;
+			opp-supported-hw = <0x3>;
 			clock-latency-ns = <200000>;
 		};
 	};
@@ -150,6 +173,11 @@
 			reg = <0x000a4000 0x721>;
 			#address-cells = <1>;
 			#size-cells = <1>;
+
+			cpu_speed_bin: cpu_speed_bin@1d {
+				reg = <0x1d 0x2>;
+				bits = <7 2>;
+			};
 		};
 
 		rng: rng@e3000 {
-- 
2.7.4

[PATCH v1 08/10] dt-bindings: cpufreq: qcom-cpufreq-nvmem: document IPQ9574

[Varadarajan Narayanan]

Document IPQ9574 compatible for Qcom NVMEM CPUFreq driver.

Signed-off-by: Varadarajan Narayanan <quic_varada@quicinc.com>
---
 Documentation/devicetree/bindings/cpufreq/qcom-cpufreq-nvmem.yaml | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/cpufreq/qcom-cpufreq-nvmem.yaml b/Documentation/devicetree/bindings/cpufreq/qcom-cpufreq-nvmem.yaml
index 75e996a..a6534bf 100644
--- a/Documentation/devicetree/bindings/cpufreq/qcom-cpufreq-nvmem.yaml
+++ b/Documentation/devicetree/bindings/cpufreq/qcom-cpufreq-nvmem.yaml
@@ -30,6 +30,7 @@ select:
           - qcom,ipq5332
           - qcom,ipq8064
           - qcom,ipq8074
+          - qcom,ipq9574
           - qcom,msm8939
           - qcom,msm8960
           - qcom,msm8974
-- 
2.7.4

[PATCH v1 09/10] cpufreq: qti: Introduce cpufreq for ipq95xx

[Varadarajan Narayanan]

IPQ95xx SoCs have different OPPs available for the CPU based on
the SoC variant. This can be determined from an eFuse register
present in the silicon.

Added support for ipq95xx 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.

Signed-off-by: Praveenkumar I <ipkumar@codeaurora.org>
Signed-off-by: Kathiravan T <quic_kathirav@quicinc.com>
Signed-off-by: Varadarajan Narayanan <quic_varada@quicinc.com>
---
 drivers/cpufreq/cpufreq-dt-platdev.c |  1 +
 drivers/cpufreq/qcom-cpufreq-nvmem.c | 20 ++++++++++++++++++++
 2 files changed, 21 insertions(+)

diff --git a/drivers/cpufreq/cpufreq-dt-platdev.c b/drivers/cpufreq/cpufreq-dt-platdev.c
index f0c45d4..4ab29c0 100644
--- a/drivers/cpufreq/cpufreq-dt-platdev.c
+++ b/drivers/cpufreq/cpufreq-dt-platdev.c
@@ -180,6 +180,7 @@ static const struct of_device_id blocklist[] __initconst = {
 
 	{ .compatible = "qcom,ipq5332", },
 	{ .compatible = "qcom,ipq8064", },
+	{ .compatible = "qcom,ipq9574", },
 	{ .compatible = "qcom,apq8064", },
 	{ .compatible = "qcom,msm8974", },
 	{ .compatible = "qcom,msm8960", },
diff --git a/drivers/cpufreq/qcom-cpufreq-nvmem.c b/drivers/cpufreq/qcom-cpufreq-nvmem.c
index 49d21b0..de70225 100644
--- a/drivers/cpufreq/qcom-cpufreq-nvmem.c
+++ b/drivers/cpufreq/qcom-cpufreq-nvmem.c
@@ -168,6 +168,25 @@ static int qcom_cpufreq_kryo_name_version(struct device *cpu_dev,
 	case QCOM_ID_APQ8096SG:
 		drv->versions = 1 << ((unsigned int)(*speedbin) + 4);
 		break;
+	case QCOM_ID_IPQ9514:
+	case QCOM_ID_IPQ9550:
+	case QCOM_ID_IPQ9554:
+	case QCOM_ID_IPQ9570:
+	case QCOM_ID_IPQ9574:
+		/* Fuse Value    Freq    BIT to set
+		 * ---------------------------------
+		 *   2’b00     No Limit     BIT(0)
+		 *   2’b10     1.8 GHz      BIT(1)
+		 *   2’b01     1.5 Ghz      BIT(2)
+		 *   2’b11     1.2 GHz      BIT(3)
+		 */
+		if ((unsigned int)(*speedbin) == 2)
+			drv->versions = BIT(1);
+		else if ((unsigned int)(*speedbin) == 1)
+			drv->versions = BIT(2);
+		else
+			drv->versions = 1 << (unsigned int)(*speedbin);
+		break;
 	default:
 		BUG();
 		break;
@@ -375,6 +394,7 @@ static const struct of_device_id qcom_cpufreq_match_list[] __initconst = {
 	{ .compatible = "qcom,qcs404", .data = &match_data_qcs404 },
 	{ .compatible = "qcom,ipq5332", .data = &match_data_kryo },
 	{ .compatible = "qcom,ipq8064", .data = &match_data_krait },
+	{ .compatible = "qcom,ipq9574", .data = &match_data_kryo },
 	{ .compatible = "qcom,apq8064", .data = &match_data_krait },
 	{ .compatible = "qcom,msm8974", .data = &match_data_krait },
 	{ .compatible = "qcom,msm8960", .data = &match_data_krait },
-- 
2.7.4

[PATCH v1 10/10] arm64: dts: qcom: ipq9574: populate the opp table based on the eFuse

[Varadarajan Narayanan]

IPQ95xx SoCs have different OPPs available for the CPU based on
SoC variant. This can be determined from an eFuse register
present in the silicon.

Add support to read the eFuse and populate the OPPs based on it.

Signed-off-by: Kathiravan T <quic_kathirav@quicinc.com>
Signed-off-by: Varadarajan Narayanan <quic_varada@quicinc.com>
---
 arch/arm64/boot/dts/qcom/ipq9574.dtsi | 38 ++++++++++++++++++++++++++++++++++-
 1 file changed, 37 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/boot/dts/qcom/ipq9574.dtsi b/arch/arm64/boot/dts/qcom/ipq9574.dtsi
index 89edb4b..ad6f1c6 100644
--- a/arch/arm64/boot/dts/qcom/ipq9574.dtsi
+++ b/arch/arm64/boot/dts/qcom/ipq9574.dtsi
@@ -106,42 +106,73 @@
 	};
 
 	cpu_opp_table: opp-table-cpu {
-		compatible = "operating-points-v2";
+		compatible = "operating-points-v2-kryo-cpu";
 		opp-shared;
+		nvmem-cells = <&cpu_speed_bin>;
+		nvmem-cell-names = "speed_bin";
+
+		/* Listed all supported CPU frequencies and opp-supported-hw
+		 * values to select CPU frequencies based on the limits fused.
+		 * ------------------------------------------------------------
+		 * Frequency     BIT3   BIT2   BIT1    BIT0    opp-supported-hw
+		 *              1.2GHz 1.5GHz 1.8GHz No Limit
+		 * ------------------------------------------------------------
+		 * 936000000      1      1      1       1            0xF
+		 * 1104000000     1      1      1       1            0xF
+		 * 1200000000     1      0      0       0            0x8
+		 * 1416000000     0      1      1       1            0x7
+		 * 1488000000     0      1      0       0            0x4
+		 * 1800000000     0      0      1       1            0x3
+		 * 2208000000     0      0      0       1            0x1
+		 * -----------------------------------------------------------
+		 */
 
 		opp-936000000 {
 			opp-hz = /bits/ 64 <936000000>;
 			opp-microvolt = <725000>;
+			opp-supported-hw = <0xf>;
 			clock-latency-ns = <200000>;
 		};
 
 		opp-1104000000 {
 			opp-hz = /bits/ 64 <1104000000>;
 			opp-microvolt = <787500>;
+			opp-supported-hw = <0xf>;
+			clock-latency-ns = <200000>;
+		};
+
+		opp-1200000000 {
+			opp-hz = /bits/ 64 <1200000000>;
+			opp-microvolt = <862500>;
+			opp-supported-hw = <0x8>;
 			clock-latency-ns = <200000>;
 		};
 
 		opp-1416000000 {
 			opp-hz = /bits/ 64 <1416000000>;
 			opp-microvolt = <862500>;
+			opp-supported-hw = <0x7>;
 			clock-latency-ns = <200000>;
 		};
 
 		opp-1488000000 {
 			opp-hz = /bits/ 64 <1488000000>;
 			opp-microvolt = <925000>;
+			opp-supported-hw = <0x7>;
 			clock-latency-ns = <200000>;
 		};
 
 		opp-1800000000 {
 			opp-hz = /bits/ 64 <1800000000>;
 			opp-microvolt = <987500>;
+			opp-supported-hw = <0x3>;
 			clock-latency-ns = <200000>;
 		};
 
 		opp-2208000000 {
 			opp-hz = /bits/ 64 <2208000000>;
 			opp-microvolt = <1062500>;
+			opp-supported-hw = <0x1>;
 			clock-latency-ns = <200000>;
 		};
 	};
@@ -223,6 +254,11 @@
 			reg = <0x000a4000 0x5a1>;
 			#address-cells = <1>;
 			#size-cells = <1>;
+
+			cpu_speed_bin: cpu_speed_bin@15 {
+				reg = <0x15 0x2>;
+				bits = <7 2>;
+			};
 		};
 
 		cryptobam: dma-controller@704000 {
-- 
2.7.4

Re: [PATCH v1 07/10] arm64: dts: qcom: ipq5332: populate the opp table based on the eFuse

[Krzysztof Kozlowski]

On 07/09/2023 07:21, Varadarajan Narayanan wrote:
> IPQ53xx 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.
> 
> Add support to read the eFuse and populate the OPPs based on it.
> 
> Signed-off-by: Kathiravan T <quic_kathirav@quicinc.com>


>  	};
> @@ -150,6 +173,11 @@
>  			reg = <0x000a4000 0x721>;
>  			#address-cells = <1>;
>  			#size-cells = <1>;
> +
> +			cpu_speed_bin: cpu_speed_bin@1d {

No underscores in node names. I am pretty sure I repeated it multiple
times already...

Best regards,
Krzysztof

Re: [PATCH v1 05/10] dt-bindings: cpufreq: qcom-cpufreq-nvmem: document IPQ5332

[Krzysztof Kozlowski]

On 07/09/2023 07:21, Varadarajan Narayanan wrote:
> Document IPQ5332 compatible for Qcom NVMEM CPUFreq driver.
> 
> Signed-off-by: Varadarajan Narayanan <quic_varada@quicinc.com>
> ---

Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

Best regards,
Krzysztof

Re: [PATCH v1 10/10] arm64: dts: qcom: ipq9574: populate the opp table based on the eFuse

[Krzysztof Kozlowski]

On 07/09/2023 07:21, Varadarajan Narayanan wrote:
> IPQ95xx SoCs have different OPPs available for the CPU based on
> SoC variant. This can be determined from an eFuse register
> present in the silicon.
> 
> Add support to read the eFuse and populate the OPPs based on it.
> 

...

>  	};
> @@ -223,6 +254,11 @@
>  			reg = <0x000a4000 0x5a1>;
>  			#address-cells = <1>;
>  			#size-cells = <1>;
> +
> +			cpu_speed_bin: cpu_speed_bin@15 {

No underscores in node names

Best regards,
Krzysztof

Re: [PATCH v1 08/10] dt-bindings: cpufreq: qcom-cpufreq-nvmem: document IPQ9574

[Krzysztof Kozlowski]

On 07/09/2023 07:21, Varadarajan Narayanan wrote:
> Document IPQ9574 compatible for Qcom NVMEM CPUFreq driver.
> 
> Signed-off-by: Varadarajan Narayanan <quic_varada@quicinc.com>
> ---
>  Documentation/devicetree/bindings/cpufreq/qcom-cpufreq-nvmem.yaml | 1 +
>  1 file changed, 1 insertion(+)


Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

Best regards,
Krzysztof

Re: [PATCH v1 01/10] clk: qcom: clk-alpha-pll: introduce stromer plus ops

[Konrad Dybcio]

On 7.09.2023 07:21, Varadarajan Narayanan wrote:
> Stromer plus APSS PLL does not support dynamic frequency scaling.
> To switch between frequencies, we have to shut down the PLL,
> configure the L and ALPHA values and turn on again. So introduce the
> separate set of ops for Stromer Plus PLL.
> 
> Signed-off-by: Kathiravan T <quic_kathirav@quicinc.com>
> Signed-off-by: Varadarajan Narayanan <quic_varada@quicinc.com>
> ---
[...]

> +
> +	/* Wait five micro seconds or more */
> +	udelay(5);
> +	regmap_update_bits(pll->clkr.regmap, PLL_MODE(pll), PLL_RESET_N,
> +			   PLL_RESET_N);
> +
> +	/* The lock time should be less than 50 micro seconds worst case */
> +	udelay(50);
https://www.kernel.org/doc/Documentation/timers/timers-howto.txt

> +
> +	ret = wait_for_pll_enable_lock(pll);
> +	if (ret) {
> +		pr_err("alpha pll running in 800 MHz with source GPLL0\n");
> +		return ret;
> +	}
Would that not be SoC-specific information?

Konrad

Re: [PATCH v1 03/10] clk: qcom: apss-ipq-pll: Fix 'l' value for ipq5332_pll_config

[Konrad Dybcio]

On 7.09.2023 07:21, Varadarajan Narayanan wrote:
> The earlier 'l' value of 0x3e is for 1.5GHz. Not all SKUs support
> this frequency. Hence set it to 0x2d to get 1.1GHz which is
> supported in all SKUs.
> 
> Fixes: c7ef7fbb1ccf ("clk: qcom: apss-ipq-pll: add support for IPQ5332")
> Signed-off-by: Kathiravan T <quic_kathirav@quicinc.com>
> Signed-off-by: Varadarajan Narayanan <quic_varada@quicinc.com>
> ---
Reviewed-by: Konrad Dybcio <konrad.dybcio@linaro.org>

Konrad

Re: [PATCH v1 04/10] clk: qcom: apss-ipq6018: ipq5332: add safe source switch for a53pll

[Konrad Dybcio]

On 7.09.2023 07:21, Varadarajan Narayanan wrote:
> Stromer Plus PLL found on IPQ53xx doesn't support dynamic
> frequency scaling. To achieve the same, we need to park the APPS
> PLL source to GPLL0, re configure the PLL and then switch the
> source to APSS_PLL_EARLY.
> 
> To support this, register a clock notifier to get the PRE_RATE
> and POST_RATE notification. Change the APSS PLL source to GPLL0
> when PRE_RATE notification is received, then configure the PLL
> and then change back the source to APSS_PLL_EARLY.
> 
> Signed-off-by: Kathiravan T <quic_kathirav@quicinc.com>
> Signed-off-by: Varadarajan Narayanan <quic_varada@quicinc.com>
> ---
>  drivers/clk/qcom/apss-ipq6018.c | 54 ++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 53 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/clk/qcom/apss-ipq6018.c b/drivers/clk/qcom/apss-ipq6018.c
> index 4e13a08..ffb6ab5 100644
> --- a/drivers/clk/qcom/apss-ipq6018.c
> +++ b/drivers/clk/qcom/apss-ipq6018.c
> @@ -9,8 +9,11 @@
>  #include <linux/clk-provider.h>
>  #include <linux/regmap.h>
>  #include <linux/module.h>
> +#include <linux/clk.h>
> +#include <linux/soc/qcom/smem.h>
>  
>  #include <dt-bindings/clock/qcom,apss-ipq.h>
> +#include <dt-bindings/arm/qcom,ids.h>
>  
>  #include "common.h"
>  #include "clk-regmap.h"
> @@ -84,15 +87,64 @@ static const struct qcom_cc_desc apss_ipq6018_desc = {
>  	.num_clks = ARRAY_SIZE(apss_ipq6018_clks),
>  };
>  
> +static int cpu_clk_notifier_fn(struct notifier_block *nb, unsigned long action,
> +				void *data)
> +{
> +	u8 index;
> +	int err;
> +
> +	if (action == PRE_RATE_CHANGE)
> +		index = P_GPLL0;
> +	else if (action == POST_RATE_CHANGE)
> +		index = P_APSS_PLL_EARLY;
> +	else
> +		return 0;
> +
> +	err = clk_rcg2_mux_closest_ops.set_parent(&apcs_alias0_clk_src.clkr.hw,
> +						  index);
Adding a variable for clk_hw within the apcs_alias0 clock would
make this easier to digest, I think.

And if we wanna be even less error-prone, you can reference the
ops of this clock in an indirect way.

> +
> +	return notifier_from_errno(err);
> +}
> +
> +static struct notifier_block cpu_clk_notifier = {
> +	.notifier_call = cpu_clk_notifier_fn,
> +};
> +
>  static int apss_ipq6018_probe(struct platform_device *pdev)
>  {
>  	struct regmap *regmap;
> +	u32 soc_id;
> +	int ret;
> +
> +	ret = qcom_smem_get_soc_id(&soc_id);
> +	if (ret)
> +		return ret;
>  
>  	regmap = dev_get_regmap(pdev->dev.parent, NULL);
>  	if (!regmap)
>  		return -ENODEV;
>  
> -	return qcom_cc_really_probe(pdev, &apss_ipq6018_desc, regmap);
> +	ret = qcom_cc_really_probe(pdev, &apss_ipq6018_desc, regmap);
> +	if (ret)
> +		return ret;
> +
> +	switch (soc_id) {
> +	/*
> +	 * Only below variants of IPQ53xx support scaling
> +	 */
1. /* Keep this in a 1-line comment */

2. why? explain the reasoning in the commit message

Konrad
> +	case QCOM_ID_IPQ5332:
> +	case QCOM_ID_IPQ5322:
> +	case QCOM_ID_IPQ5300:
> +		ret = clk_notifier_register(apcs_alias0_clk_src.clkr.hw.clk,
> +						&cpu_clk_notifier);
> +		if (ret)
> +			return ret;
> +		break;
> +	default:
> +		break;
> +	}
> +
> +	return 0;
>  }
>  
>  static struct platform_driver apss_ipq6018_driver = {

Re: [PATCH v1 06/10] cpufreq: qti: Enable cpufreq for ipq53xx

[Konrad Dybcio]

On 7.09.2023 07:21, Varadarajan Narayanan wrote:
> IPQ53xx 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 ipq53xx 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: Kathiravan T <quic_kathirav@quicinc.com>
> Signed-off-by: Varadarajan Narayanan <quic_varada@quicinc.com>
> ---
>  drivers/cpufreq/cpufreq-dt-platdev.c |  1 +
>  drivers/cpufreq/qcom-cpufreq-nvmem.c | 15 +++++++++++++++
>  2 files changed, 16 insertions(+)
> 
> diff --git a/drivers/cpufreq/cpufreq-dt-platdev.c b/drivers/cpufreq/cpufreq-dt-platdev.c
> index 02ec58a..f0c45d4 100644
> --- a/drivers/cpufreq/cpufreq-dt-platdev.c
> +++ b/drivers/cpufreq/cpufreq-dt-platdev.c
Viresh probably picks up patches for both files, so it should
be fine, but I'd say it's 'eeeh' to edit 2 separate drivers at
once.

> @@ -178,6 +178,7 @@ static const struct of_device_id blocklist[] __initconst = {
>  	{ .compatible = "ti,am625", },
>  	{ .compatible = "ti,am62a7", },
>  
> +	{ .compatible = "qcom,ipq5332", },
>  	{ .compatible = "qcom,ipq8064", },
>  	{ .compatible = "qcom,apq8064", },
>  	{ .compatible = "qcom,msm8974", },
> diff --git a/drivers/cpufreq/qcom-cpufreq-nvmem.c b/drivers/cpufreq/qcom-cpufreq-nvmem.c
> index 84d7033..49d21b0 100644
> --- a/drivers/cpufreq/qcom-cpufreq-nvmem.c
> +++ b/drivers/cpufreq/qcom-cpufreq-nvmem.c
> @@ -146,6 +146,20 @@ static int qcom_cpufreq_kryo_name_version(struct device *cpu_dev,
>  		return PTR_ERR(speedbin);
>  
>  	switch (msm_id) {
> +	case QCOM_ID_IPQ5332:
> +	case QCOM_ID_IPQ5322:
> +	case QCOM_ID_IPQ5312:
> +	case QCOM_ID_IPQ5302:
> +	case QCOM_ID_IPQ5300:
> +		/* Fuse Value    Freq    BIT to set
> +		 * ---------------------------------
> +		 *   2’b00     No Limit     BIT(0)
> +		 *   2’b01     1.5 GHz      BIT(1)
> +		 *   2’b10     1.2 Ghz      BIT(2)
> +		 *   2’b11     1.0 GHz      BIT(3)
I think the last column is a bit excessive, it says exactly
the same as the line below.

Actually, with this data being present in the devicetree, perhaps
this comment could be entirely skipped.

Konrad
> +		 */
> +		drv->versions = 1 << (unsigned int)(*speedbin);
> +		break;
>  	case QCOM_ID_MSM8996:
>  	case QCOM_ID_APQ8096:
>  		drv->versions = 1 << (unsigned int)(*speedbin);
> @@ -359,6 +373,7 @@ static const struct of_device_id qcom_cpufreq_match_list[] __initconst = {
>  	{ .compatible = "qcom,apq8096", .data = &match_data_kryo },
>  	{ .compatible = "qcom,msm8996", .data = &match_data_kryo },
>  	{ .compatible = "qcom,qcs404", .data = &match_data_qcs404 },
> +	{ .compatible = "qcom,ipq5332", .data = &match_data_kryo },
>  	{ .compatible = "qcom,ipq8064", .data = &match_data_krait },
>  	{ .compatible = "qcom,apq8064", .data = &match_data_krait },
>  	{ .compatible = "qcom,msm8974", .data = &match_data_krait },

Re: [PATCH v1 04/10] clk: qcom: apss-ipq6018: ipq5332: add safe source switch for a53pll

[kernel test robot]

Hi Varadarajan,

kernel test robot noticed the following build errors:

[auto build test ERROR on linus/master]
[also build test ERROR on v6.5 next-20230907]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Varadarajan-Narayanan/clk-qcom-clk-alpha-pll-introduce-stromer-plus-ops/20230907-132537
base:   linus/master
patch link:    https://lore.kernel.org/r/5e3c29df2b42cceb8072b00546a78e1b99b2d374.1693996662.git.quic_varada%40quicinc.com
patch subject: [PATCH v1 04/10] clk: qcom: apss-ipq6018: ipq5332: add safe source switch for a53pll
config: s390-randconfig-r031-20230907 (https://download.01.org/0day-ci/archive/20230907/202309071656.f4IVOEO6-lkp@intel.com/config)
compiler: clang version 16.0.4 (https://github.com/llvm/llvm-project.git ae42196bc493ffe877a7e3dff8be32035dea4d07)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20230907/202309071656.f4IVOEO6-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202309071656.f4IVOEO6-lkp@intel.com/

All errors (new ones prefixed by >>):

   In file included from drivers/clk/qcom/apss-ipq6018.c:10:
   In file included from include/linux/regmap.h:20:
   In file included from include/linux/iopoll.h:14:
   In file included from include/linux/io.h:13:
   In file included from arch/s390/include/asm/io.h:78:
   include/asm-generic/io.h:547:31: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           val = __raw_readb(PCI_IOBASE + addr);
                             ~~~~~~~~~~ ^
   include/asm-generic/io.h:560:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           val = __le16_to_cpu((__le16 __force)__raw_readw(PCI_IOBASE + addr));
                                                           ~~~~~~~~~~ ^
   include/uapi/linux/byteorder/big_endian.h:37:59: note: expanded from macro '__le16_to_cpu'
   #define __le16_to_cpu(x) __swab16((__force __u16)(__le16)(x))
                                                             ^
   include/uapi/linux/swab.h:102:54: note: expanded from macro '__swab16'
   #define __swab16(x) (__u16)__builtin_bswap16((__u16)(x))
                                                        ^
   In file included from drivers/clk/qcom/apss-ipq6018.c:10:
   In file included from include/linux/regmap.h:20:
   In file included from include/linux/iopoll.h:14:
   In file included from include/linux/io.h:13:
   In file included from arch/s390/include/asm/io.h:78:
   include/asm-generic/io.h:573:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           val = __le32_to_cpu((__le32 __force)__raw_readl(PCI_IOBASE + addr));
                                                           ~~~~~~~~~~ ^
   include/uapi/linux/byteorder/big_endian.h:35:59: note: expanded from macro '__le32_to_cpu'
   #define __le32_to_cpu(x) __swab32((__force __u32)(__le32)(x))
                                                             ^
   include/uapi/linux/swab.h:115:54: note: expanded from macro '__swab32'
   #define __swab32(x) (__u32)__builtin_bswap32((__u32)(x))
                                                        ^
   In file included from drivers/clk/qcom/apss-ipq6018.c:10:
   In file included from include/linux/regmap.h:20:
   In file included from include/linux/iopoll.h:14:
   In file included from include/linux/io.h:13:
   In file included from arch/s390/include/asm/io.h:78:
   include/asm-generic/io.h:584:33: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           __raw_writeb(value, PCI_IOBASE + addr);
                               ~~~~~~~~~~ ^
   include/asm-generic/io.h:594:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           __raw_writew((u16 __force)cpu_to_le16(value), PCI_IOBASE + addr);
                                                         ~~~~~~~~~~ ^
   include/asm-generic/io.h:604:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           __raw_writel((u32 __force)cpu_to_le32(value), PCI_IOBASE + addr);
                                                         ~~~~~~~~~~ ^
   include/asm-generic/io.h:692:20: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           readsb(PCI_IOBASE + addr, buffer, count);
                  ~~~~~~~~~~ ^
   include/asm-generic/io.h:700:20: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           readsw(PCI_IOBASE + addr, buffer, count);
                  ~~~~~~~~~~ ^
   include/asm-generic/io.h:708:20: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           readsl(PCI_IOBASE + addr, buffer, count);
                  ~~~~~~~~~~ ^
   include/asm-generic/io.h:717:21: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           writesb(PCI_IOBASE + addr, buffer, count);
                   ~~~~~~~~~~ ^
   include/asm-generic/io.h:726:21: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           writesw(PCI_IOBASE + addr, buffer, count);
                   ~~~~~~~~~~ ^
   include/asm-generic/io.h:735:21: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           writesl(PCI_IOBASE + addr, buffer, count);
                   ~~~~~~~~~~ ^
>> drivers/clk/qcom/apss-ipq6018.c:94:11: error: use of undeclared identifier 'P_GPLL0'
                   index = P_GPLL0;
                           ^
   12 warnings and 1 error generated.


vim +/P_GPLL0 +94 drivers/clk/qcom/apss-ipq6018.c

    86	
    87	static int cpu_clk_notifier_fn(struct notifier_block *nb, unsigned long action,
    88					void *data)
    89	{
    90		u8 index;
    91		int err;
    92	
    93		if (action == PRE_RATE_CHANGE)
  > 94			index = P_GPLL0;
    95		else if (action == POST_RATE_CHANGE)
    96			index = P_APSS_PLL_EARLY;
    97		else
    98			return 0;
    99	
   100		err = clk_rcg2_mux_closest_ops.set_parent(&apcs_alias0_clk_src.clkr.hw,
   101							  index);
   102	
   103		return notifier_from_errno(err);
   104	}
   105	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH v1 02/10] clk: qcom: apss-ipq-pll: Use stromer plus ops for stromer plus pll

[Dmitry Baryshkov]

On Thu, 7 Sept 2023 at 08:22, Varadarajan Narayanan
<quic_varada@quicinc.com> wrote:
>
> The set rate and determine rate operations are different between
> Stromer and Stromer Plus PLLs. Hence, use stromer plus ops for
> ipq_pll_stromer_plus.
>
> Signed-off-by: Kathiravan T <quic_kathirav@quicinc.com>
> Signed-off-by: Varadarajan Narayanan <quic_varada@quicinc.com>

Fixes tag?

> ---
>  drivers/clk/qcom/apss-ipq-pll.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/clk/qcom/apss-ipq-pll.c b/drivers/clk/qcom/apss-ipq-pll.c
> index e170331..18c4ffe 100644
> --- a/drivers/clk/qcom/apss-ipq-pll.c
> +++ b/drivers/clk/qcom/apss-ipq-pll.c
> @@ -68,7 +68,7 @@ static struct clk_alpha_pll ipq_pll_stromer_plus = {
>                                 .fw_name = "xo",
>                         },
>                         .num_parents = 1,
> -                       .ops = &clk_alpha_pll_stromer_ops,
> +                       .ops = &clk_alpha_pll_stromer_plus_ops,
>                 },
>         },
>  };
> --
> 2.7.4
>


-- 
With best wishes
Dmitry

Re: [PATCH v1 01/10] clk: qcom: clk-alpha-pll: introduce stromer plus ops

[Dmitry Baryshkov]

On Thu, 7 Sept 2023 at 08:22, Varadarajan Narayanan
<quic_varada@quicinc.com> wrote:
>
> Stromer plus APSS PLL does not support dynamic frequency scaling.
> To switch between frequencies, we have to shut down the PLL,
> configure the L and ALPHA values and turn on again. So introduce the
> separate set of ops for Stromer Plus PLL.
>
> Signed-off-by: Kathiravan T <quic_kathirav@quicinc.com>
> Signed-off-by: Varadarajan Narayanan <quic_varada@quicinc.com>
> ---
>  drivers/clk/qcom/clk-alpha-pll.c | 68 ++++++++++++++++++++++++++++++++++++++++
>  drivers/clk/qcom/clk-alpha-pll.h |  1 +
>  2 files changed, 69 insertions(+)
>
> diff --git a/drivers/clk/qcom/clk-alpha-pll.c b/drivers/clk/qcom/clk-alpha-pll.c
> index e4ef645..2ef81f7 100644
> --- a/drivers/clk/qcom/clk-alpha-pll.c
> +++ b/drivers/clk/qcom/clk-alpha-pll.c
> @@ -2479,3 +2479,71 @@ const struct clk_ops clk_alpha_pll_stromer_ops = {
>         .set_rate = clk_alpha_pll_stromer_set_rate,
>  };
>  EXPORT_SYMBOL_GPL(clk_alpha_pll_stromer_ops);
> +
> +static int clk_alpha_pll_stromer_plus_determine_rate(struct clk_hw *hw,
> +                                                    struct clk_rate_request *req)
> +{
> +       struct clk_alpha_pll *pll = to_clk_alpha_pll(hw);
> +       u32 l, alpha_width = pll_alpha_width(pll);
> +       u64 a;
> +
> +       req->rate = alpha_pll_round_rate(req->rate, req->best_parent_rate, &l,
> +                                        &a, alpha_width);
> +       return 0;
> +}

What is the plL_alpha_width on stromer_plus? Does
clk_alpha_pll_stromer_determine_rate() work for you?

> +
> +static int clk_alpha_pll_stromer_plus_set_rate(struct clk_hw *hw,
> +                                              unsigned long rate,
> +                                              unsigned long prate)
> +{
> +       struct clk_alpha_pll *pll = to_clk_alpha_pll(hw);
> +       u32 l, alpha_width = pll_alpha_width(pll);
> +       int ret;
> +       u64 a;
> +
> +       rate = alpha_pll_round_rate(rate, prate, &l, &a, alpha_width);
> +
> +       regmap_write(pll->clkr.regmap, PLL_MODE(pll), 0);
> +
> +       /* Delay of 2 output clock ticks required until output is disabled */
> +       udelay(1);
> +
> +       regmap_write(pll->clkr.regmap, PLL_L_VAL(pll), l);
> +
> +       if (alpha_width > ALPHA_BITWIDTH)
> +               a <<= alpha_width - ALPHA_BITWIDTH;
> +
> +       regmap_write(pll->clkr.regmap, PLL_ALPHA_VAL(pll), a);
> +       regmap_write(pll->clkr.regmap, PLL_ALPHA_VAL_U(pll),
> +                                       a >> ALPHA_BITWIDTH);
> +
> +       regmap_write(pll->clkr.regmap, PLL_MODE(pll), PLL_BYPASSNL);
> +
> +       /* Wait five micro seconds or more */
> +       udelay(5);
> +       regmap_update_bits(pll->clkr.regmap, PLL_MODE(pll), PLL_RESET_N,
> +                          PLL_RESET_N);
> +
> +       /* The lock time should be less than 50 micro seconds worst case */
> +       udelay(50);
> +
> +       ret = wait_for_pll_enable_lock(pll);
> +       if (ret) {
> +               pr_err("alpha pll running in 800 MHz with source GPLL0\n");
> +               return ret;
> +       }
> +       regmap_update_bits(pll->clkr.regmap, PLL_MODE(pll), PLL_OUTCTRL,
> +                          PLL_OUTCTRL);
> +
> +       return 0;
> +}
> +
> +const struct clk_ops clk_alpha_pll_stromer_plus_ops = {
> +       .enable = clk_alpha_pll_enable,
> +       .disable = clk_alpha_pll_disable,
> +       .is_enabled = clk_alpha_pll_is_enabled,
> +       .recalc_rate = clk_alpha_pll_recalc_rate,
> +       .determine_rate = clk_alpha_pll_stromer_plus_determine_rate,
> +       .set_rate = clk_alpha_pll_stromer_plus_set_rate,
> +};
> +EXPORT_SYMBOL_GPL(clk_alpha_pll_stromer_plus_ops);
> diff --git a/drivers/clk/qcom/clk-alpha-pll.h b/drivers/clk/qcom/clk-alpha-pll.h
> index e4bd863..903fbab 100644
> --- a/drivers/clk/qcom/clk-alpha-pll.h
> +++ b/drivers/clk/qcom/clk-alpha-pll.h
> @@ -152,6 +152,7 @@ extern const struct clk_ops clk_alpha_pll_postdiv_ops;
>  extern const struct clk_ops clk_alpha_pll_huayra_ops;
>  extern const struct clk_ops clk_alpha_pll_postdiv_ro_ops;
>  extern const struct clk_ops clk_alpha_pll_stromer_ops;
> +extern const struct clk_ops clk_alpha_pll_stromer_plus_ops;
>
>  extern const struct clk_ops clk_alpha_pll_fabia_ops;
>  extern const struct clk_ops clk_alpha_pll_fixed_fabia_ops;
> --
> 2.7.4
>


-- 
With best wishes
Dmitry

Re: [PATCH v1 03/10] clk: qcom: apss-ipq-pll: Fix 'l' value for ipq5332_pll_config

[Dmitry Baryshkov]

On Thu, 7 Sept 2023 at 08:22, Varadarajan Narayanan
<quic_varada@quicinc.com> wrote:
>
> The earlier 'l' value of 0x3e is for 1.5GHz. Not all SKUs support
> this frequency. Hence set it to 0x2d to get 1.1GHz which is
> supported in all SKUs.
>
> Fixes: c7ef7fbb1ccf ("clk: qcom: apss-ipq-pll: add support for IPQ5332")
> Signed-off-by: Kathiravan T <quic_kathirav@quicinc.com>
> Signed-off-by: Varadarajan Narayanan <quic_varada@quicinc.com>

Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>


-- 
With best wishes
Dmitry

Re: [PATCH v1 04/10] clk: qcom: apss-ipq6018: ipq5332: add safe source switch for a53pll

[Dmitry Baryshkov]

On Thu, 7 Sept 2023 at 08:22, Varadarajan Narayanan
<quic_varada@quicinc.com> wrote:
>
> Stromer Plus PLL found on IPQ53xx doesn't support dynamic
> frequency scaling. To achieve the same, we need to park the APPS
> PLL source to GPLL0, re configure the PLL and then switch the
> source to APSS_PLL_EARLY.
>
> To support this, register a clock notifier to get the PRE_RATE
> and POST_RATE notification. Change the APSS PLL source to GPLL0
> when PRE_RATE notification is received, then configure the PLL
> and then change back the source to APSS_PLL_EARLY.

This means that we are changing the parents behind the back of CCF,
which is not great.

>
> Signed-off-by: Kathiravan T <quic_kathirav@quicinc.com>
> Signed-off-by: Varadarajan Narayanan <quic_varada@quicinc.com>
> ---
>  drivers/clk/qcom/apss-ipq6018.c | 54 ++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 53 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/clk/qcom/apss-ipq6018.c b/drivers/clk/qcom/apss-ipq6018.c
> index 4e13a08..ffb6ab5 100644
> --- a/drivers/clk/qcom/apss-ipq6018.c
> +++ b/drivers/clk/qcom/apss-ipq6018.c
> @@ -9,8 +9,11 @@
>  #include <linux/clk-provider.h>
>  #include <linux/regmap.h>
>  #include <linux/module.h>
> +#include <linux/clk.h>
> +#include <linux/soc/qcom/smem.h>
>
>  #include <dt-bindings/clock/qcom,apss-ipq.h>
> +#include <dt-bindings/arm/qcom,ids.h>
>
>  #include "common.h"
>  #include "clk-regmap.h"
> @@ -84,15 +87,64 @@ static const struct qcom_cc_desc apss_ipq6018_desc = {
>         .num_clks = ARRAY_SIZE(apss_ipq6018_clks),
>  };
>
> +static int cpu_clk_notifier_fn(struct notifier_block *nb, unsigned long action,
> +                               void *data)
> +{
> +       u8 index;
> +       int err;
> +
> +       if (action == PRE_RATE_CHANGE)
> +               index = P_GPLL0;

I don't see P_GPLL0 being supported in the ipq6018 driver.

> +       else if (action == POST_RATE_CHANGE)
> +               index = P_APSS_PLL_EARLY;

You also have to handle ABORT_RATE_CHANGE here.

> +       else
> +               return 0;
> +
> +       err = clk_rcg2_mux_closest_ops.set_parent(&apcs_alias0_clk_src.clkr.hw,
> +                                                 index);
> +
> +       return notifier_from_errno(err);
> +}
> +
> +static struct notifier_block cpu_clk_notifier = {
> +       .notifier_call = cpu_clk_notifier_fn,
> +};
> +
>  static int apss_ipq6018_probe(struct platform_device *pdev)
>  {
>         struct regmap *regmap;
> +       u32 soc_id;
> +       int ret;
> +
> +       ret = qcom_smem_get_soc_id(&soc_id);
> +       if (ret)
> +               return ret;
>
>         regmap = dev_get_regmap(pdev->dev.parent, NULL);
>         if (!regmap)
>                 return -ENODEV;
>
> -       return qcom_cc_really_probe(pdev, &apss_ipq6018_desc, regmap);
> +       ret = qcom_cc_really_probe(pdev, &apss_ipq6018_desc, regmap);
> +       if (ret)
> +               return ret;
> +
> +       switch (soc_id) {
> +       /*
> +        * Only below variants of IPQ53xx support scaling
> +        */
> +       case QCOM_ID_IPQ5332:
> +       case QCOM_ID_IPQ5322:
> +       case QCOM_ID_IPQ5300:

Please use compat strings instead of using the soc-id.

> +               ret = clk_notifier_register(apcs_alias0_clk_src.clkr.hw.clk,
> +                                               &cpu_clk_notifier);
> +               if (ret)
> +                       return ret;
> +               break;
> +       default:
> +               break;
> +       }
> +
> +       return 0;
>  }
>
>  static struct platform_driver apss_ipq6018_driver = {
> --
> 2.7.4
>


--
With best wishes

Dmitry

Re: [PATCH v1 06/10] cpufreq: qti: Enable cpufreq for ipq53xx

[Dmitry Baryshkov]

On Thu, 7 Sept 2023 at 08:23, Varadarajan Narayanan
<quic_varada@quicinc.com> wrote:
>
> IPQ53xx 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 ipq53xx 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: Kathiravan T <quic_kathirav@quicinc.com>
> Signed-off-by: Varadarajan Narayanan <quic_varada@quicinc.com>
> ---
>  drivers/cpufreq/cpufreq-dt-platdev.c |  1 +
>  drivers/cpufreq/qcom-cpufreq-nvmem.c | 15 +++++++++++++++
>  2 files changed, 16 insertions(+)
>
> diff --git a/drivers/cpufreq/cpufreq-dt-platdev.c b/drivers/cpufreq/cpufreq-dt-platdev.c
> index 02ec58a..f0c45d4 100644
> --- a/drivers/cpufreq/cpufreq-dt-platdev.c
> +++ b/drivers/cpufreq/cpufreq-dt-platdev.c
> @@ -178,6 +178,7 @@ static const struct of_device_id blocklist[] __initconst = {
>         { .compatible = "ti,am625", },
>         { .compatible = "ti,am62a7", },
>
> +       { .compatible = "qcom,ipq5332", },
>         { .compatible = "qcom,ipq8064", },
>         { .compatible = "qcom,apq8064", },
>         { .compatible = "qcom,msm8974", },
> diff --git a/drivers/cpufreq/qcom-cpufreq-nvmem.c b/drivers/cpufreq/qcom-cpufreq-nvmem.c
> index 84d7033..49d21b0 100644
> --- a/drivers/cpufreq/qcom-cpufreq-nvmem.c
> +++ b/drivers/cpufreq/qcom-cpufreq-nvmem.c
> @@ -146,6 +146,20 @@ static int qcom_cpufreq_kryo_name_version(struct device *cpu_dev,
>                 return PTR_ERR(speedbin);
>
>         switch (msm_id) {
> +       case QCOM_ID_IPQ5332:
> +       case QCOM_ID_IPQ5322:
> +       case QCOM_ID_IPQ5312:
> +       case QCOM_ID_IPQ5302:
> +       case QCOM_ID_IPQ5300:

msm8996 was bad enough. Can we use compat strings instead? Or make
this a default for qcom_cpufreq_kryo_name_version()?

> +               /* Fuse Value    Freq    BIT to set
> +                * ---------------------------------
> +                *   2’b00     No Limit     BIT(0)
> +                *   2’b01     1.5 GHz      BIT(1)
> +                *   2’b10     1.2 Ghz      BIT(2)
> +                *   2’b11     1.0 GHz      BIT(3)
> +                */
> +               drv->versions = 1 << (unsigned int)(*speedbin);
> +               break;
>         case QCOM_ID_MSM8996:
>         case QCOM_ID_APQ8096:
>                 drv->versions = 1 << (unsigned int)(*speedbin);
> @@ -359,6 +373,7 @@ static const struct of_device_id qcom_cpufreq_match_list[] __initconst = {
>         { .compatible = "qcom,apq8096", .data = &match_data_kryo },
>         { .compatible = "qcom,msm8996", .data = &match_data_kryo },
>         { .compatible = "qcom,qcs404", .data = &match_data_qcs404 },
> +       { .compatible = "qcom,ipq5332", .data = &match_data_kryo },
>         { .compatible = "qcom,ipq8064", .data = &match_data_krait },
>         { .compatible = "qcom,apq8064", .data = &match_data_krait },
>         { .compatible = "qcom,msm8974", .data = &match_data_krait },
> --
> 2.7.4
>


-- 
With best wishes
Dmitry

Re: [PATCH v1 07/10] arm64: dts: qcom: ipq5332: populate the opp table based on the eFuse

[Dmitry Baryshkov]

On Thu, 7 Sept 2023 at 08:23, Varadarajan Narayanan
<quic_varada@quicinc.com> wrote:
>
> IPQ53xx 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.
>
> Add support to read the eFuse and populate the OPPs based on it.
>
> Signed-off-by: Kathiravan T <quic_kathirav@quicinc.com>
> Signed-off-by: Varadarajan Narayanan <quic_varada@quicinc.com>
> ---
>  arch/arm64/boot/dts/qcom/ipq5332.dtsi | 34 +++++++++++++++++++++++++++++++---
>  1 file changed, 31 insertions(+), 3 deletions(-)
>
> diff --git a/arch/arm64/boot/dts/qcom/ipq5332.dtsi b/arch/arm64/boot/dts/qcom/ipq5332.dtsi
> index 82761ae..3ca3f34 100644
> --- a/arch/arm64/boot/dts/qcom/ipq5332.dtsi
> +++ b/arch/arm64/boot/dts/qcom/ipq5332.dtsi
> @@ -91,11 +91,34 @@
>         };
>
>         cpu_opp_table: opp-table-cpu {
> -               compatible = "operating-points-v2";
> +               compatible = "operating-points-v2-kryo-cpu";
>                 opp-shared;
> +               nvmem-cells = <&cpu_speed_bin>;
> +               nvmem-cell-names = "speed_bin";
> +
> +               /*
> +                * Listed all supported CPU frequencies and opp-supported-hw
> +                * values to select CPU frequencies based on the limits fused.
> +                * ------------------------------------------------------------
> +                * Frequency     BIT3   BIT2   BIT1    BIT0    opp-supported-hw
> +                *              1.0GHz 1.2GHz 1.5GHz No Limit
> +                * ------------------------------------------------------------
> +                * 1100000000     1      1      1       1            0xF
> +                * 1500000000     0      0      1       1            0x3
> +                * -----------------------------------------------------------
> +                */

This can probably go to the commit message instead.

> +
> +               opp-1100000000 {
> +                       opp-hz = /bits/ 64 <1100000000>;

But your table shows 1.0 GHz and 1.2 GHz instead of 1.1 GHz

> +                       opp-microvolt = <850000>;
> +                       opp-supported-hw = <0xF>;
> +                       clock-latency-ns = <200000>;
> +               };
>
> -               opp-1488000000 {
> -                       opp-hz = /bits/ 64 <1488000000>;
> +               opp-1500000000 {
> +                       opp-hz = /bits/ 64 <1500000000>;

So, 1.488 GHz or 1.5 GHz?

> +                       opp-microvolt = <950000>;

Which regulator is controlled by this microvolt?

> +                       opp-supported-hw = <0x3>;
>                         clock-latency-ns = <200000>;
>                 };
>         };
> @@ -150,6 +173,11 @@
>                         reg = <0x000a4000 0x721>;
>                         #address-cells = <1>;
>                         #size-cells = <1>;
> +
> +                       cpu_speed_bin: cpu_speed_bin@1d {
> +                               reg = <0x1d 0x2>;
> +                               bits = <7 2>;
> +                       };
>                 };
>
>                 rng: rng@e3000 {
> --
> 2.7.4
>


-- 
With best wishes
Dmitry

Re: [PATCH v1 09/10] cpufreq: qti: Introduce cpufreq for ipq95xx

[Dmitry Baryshkov]

On Thu, 7 Sept 2023 at 08:24, Varadarajan Narayanan
<quic_varada@quicinc.com> wrote:
>
> IPQ95xx SoCs have different OPPs available for the CPU based on
> the SoC variant. This can be determined from an eFuse register
> present in the silicon.
>
> Added support for ipq95xx 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.
>
> Signed-off-by: Praveenkumar I <ipkumar@codeaurora.org>
> Signed-off-by: Kathiravan T <quic_kathirav@quicinc.com>
> Signed-off-by: Varadarajan Narayanan <quic_varada@quicinc.com>
> ---
>  drivers/cpufreq/cpufreq-dt-platdev.c |  1 +
>  drivers/cpufreq/qcom-cpufreq-nvmem.c | 20 ++++++++++++++++++++
>  2 files changed, 21 insertions(+)
>
> diff --git a/drivers/cpufreq/cpufreq-dt-platdev.c b/drivers/cpufreq/cpufreq-dt-platdev.c
> index f0c45d4..4ab29c0 100644
> --- a/drivers/cpufreq/cpufreq-dt-platdev.c
> +++ b/drivers/cpufreq/cpufreq-dt-platdev.c
> @@ -180,6 +180,7 @@ static const struct of_device_id blocklist[] __initconst = {
>
>         { .compatible = "qcom,ipq5332", },
>         { .compatible = "qcom,ipq8064", },
> +       { .compatible = "qcom,ipq9574", },
>         { .compatible = "qcom,apq8064", },
>         { .compatible = "qcom,msm8974", },
>         { .compatible = "qcom,msm8960", },
> diff --git a/drivers/cpufreq/qcom-cpufreq-nvmem.c b/drivers/cpufreq/qcom-cpufreq-nvmem.c
> index 49d21b0..de70225 100644
> --- a/drivers/cpufreq/qcom-cpufreq-nvmem.c
> +++ b/drivers/cpufreq/qcom-cpufreq-nvmem.c
> @@ -168,6 +168,25 @@ static int qcom_cpufreq_kryo_name_version(struct device *cpu_dev,
>         case QCOM_ID_APQ8096SG:
>                 drv->versions = 1 << ((unsigned int)(*speedbin) + 4);
>                 break;
> +       case QCOM_ID_IPQ9514:
> +       case QCOM_ID_IPQ9550:
> +       case QCOM_ID_IPQ9554:
> +       case QCOM_ID_IPQ9570:
> +       case QCOM_ID_IPQ9574:
> +               /* Fuse Value    Freq    BIT to set
> +                * ---------------------------------
> +                *   2’b00     No Limit     BIT(0)
> +                *   2’b10     1.8 GHz      BIT(1)
> +                *   2’b01     1.5 Ghz      BIT(2)
> +                *   2’b11     1.2 GHz      BIT(3)
> +                */
> +               if ((unsigned int)(*speedbin) == 2)
> +                       drv->versions = BIT(1);
> +               else if ((unsigned int)(*speedbin) == 1)
> +                       drv->versions = BIT(2);
> +               else
> +                       drv->versions = 1 << (unsigned int)(*speedbin);

If you change the order of speedbins 1 and 2 in DT, you can use 1 <<
speedbin for all the kinds,

> +               break;
>         default:
>                 BUG();
>                 break;
> @@ -375,6 +394,7 @@ static const struct of_device_id qcom_cpufreq_match_list[] __initconst = {
>         { .compatible = "qcom,qcs404", .data = &match_data_qcs404 },
>         { .compatible = "qcom,ipq5332", .data = &match_data_kryo },
>         { .compatible = "qcom,ipq8064", .data = &match_data_krait },
> +       { .compatible = "qcom,ipq9574", .data = &match_data_kryo },
>         { .compatible = "qcom,apq8064", .data = &match_data_krait },
>         { .compatible = "qcom,msm8974", .data = &match_data_krait },
>         { .compatible = "qcom,msm8960", .data = &match_data_krait },
> --
> 2.7.4
>


-- 
With best wishes
Dmitry

Re: [PATCH v1 06/10] cpufreq: qti: Enable cpufreq for ipq53xx

[Bryan O'Donoghue]

On 07/09/2023 06:21, Varadarajan Narayanan wrote:
> IPQ53xx 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 ipq53xx 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: Kathiravan T <quic_kathirav@quicinc.com>
> Signed-off-by: Varadarajan Narayanan <quic_varada@quicinc.com>
> ---
>   drivers/cpufreq/cpufreq-dt-platdev.c |  1 +
>   drivers/cpufreq/qcom-cpufreq-nvmem.c | 15 +++++++++++++++
>   2 files changed, 16 insertions(+)
> 
> diff --git a/drivers/cpufreq/cpufreq-dt-platdev.c b/drivers/cpufreq/cpufreq-dt-platdev.c
> index 02ec58a..f0c45d4 100644
> --- a/drivers/cpufreq/cpufreq-dt-platdev.c
> +++ b/drivers/cpufreq/cpufreq-dt-platdev.c
> @@ -178,6 +178,7 @@ static const struct of_device_id blocklist[] __initconst = {
>   	{ .compatible = "ti,am625", },
>   	{ .compatible = "ti,am62a7", },
>   
> +	{ .compatible = "qcom,ipq5332", },
>   	{ .compatible = "qcom,ipq8064", },
>   	{ .compatible = "qcom,apq8064", },
>   	{ .compatible = "qcom,msm8974", },
> diff --git a/drivers/cpufreq/qcom-cpufreq-nvmem.c b/drivers/cpufreq/qcom-cpufreq-nvmem.c
> index 84d7033..49d21b0 100644
> --- a/drivers/cpufreq/qcom-cpufreq-nvmem.c
> +++ b/drivers/cpufreq/qcom-cpufreq-nvmem.c
> @@ -146,6 +146,20 @@ static int qcom_cpufreq_kryo_name_version(struct device *cpu_dev,
>   		return PTR_ERR(speedbin);
>   
>   	switch (msm_id) {
> +	case QCOM_ID_IPQ5332:
> +	case QCOM_ID_IPQ5322:
> +	case QCOM_ID_IPQ5312:
> +	case QCOM_ID_IPQ5302:
> +	case QCOM_ID_IPQ5300:
> +		/* Fuse Value    Freq    BIT to set
> +		 * ---------------------------------
> +		 *   2’b00     No Limit     BIT(0)
> +		 *   2’b01     1.5 GHz      BIT(1)
> +		 *   2’b10     1.2 Ghz      BIT(2)
> +		 *   2’b11     1.0 GHz      BIT(3)
> +		 */
> +		drv->versions = 1 << (unsigned int)(*speedbin);
> +		break;

I like that you've included a comment however, the switch is an ordered 
list and these values should come after QCOM_ID_APQ8096SG

#define QCOM_ID_MSM8996                 246
#define QCOM_ID_APQ8096                 291
#define QCOM_ID_MSM8996SG               305
#define QCOM_ID_APQ8096SG               312
#define QCOM_ID_IPQ5332                 592
...
#define QCOM_ID_IPQ5300                 624

>   	case QCOM_ID_MSM8996:
>   	case QCOM_ID_APQ8096:
>   		drv->versions = 1 << (unsigned int)(*speedbin);


> @@ -359,6 +373,7 @@ static const struct of_device_id qcom_cpufreq_match_list[] __initconst = {
>   	{ .compatible = "qcom,apq8096", .data = &match_data_kryo },
>   	{ .compatible = "qcom,msm8996", .data = &match_data_kryo },
>   	{ .compatible = "qcom,qcs404", .data = &match_data_qcs404 },
> +	{ .compatible = "qcom,ipq5332", .data = &match_data_kryo },
>   	{ .compatible = "qcom,ipq8064", .data = &match_data_krait },
>   	{ .compatible = "qcom,apq8064", .data = &match_data_krait },
>   	{ .compatible = "qcom,msm8974", .data = &match_data_krait },

Other than that.

Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>

---
bod

Re: [PATCH v1 04/10] clk: qcom: apss-ipq6018: ipq5332: add safe source switch for a53pll

[Varadarajan Narayanan]

On Thu, Sep 07, 2023 at 04:54:56PM +0300, Dmitry Baryshkov wrote:
> On Thu, 7 Sept 2023 at 08:22, Varadarajan Narayanan
> <quic_varada@quicinc.com> wrote:
> >
> > Stromer Plus PLL found on IPQ53xx doesn't support dynamic
> > frequency scaling. To achieve the same, we need to park the APPS
> > PLL source to GPLL0, re configure the PLL and then switch the
> > source to APSS_PLL_EARLY.
> >
> > To support this, register a clock notifier to get the PRE_RATE
> > and POST_RATE notification. Change the APSS PLL source to GPLL0
> > when PRE_RATE notification is received, then configure the PLL
> > and then change back the source to APSS_PLL_EARLY.
>
> This means that we are changing the parents behind the back of CCF,
> which is not great.

Unfortunately, we are not aware of any other way to do this.
Please let me know if there is a better way to do this, will
implement that and post a revision.

> > Signed-off-by: Kathiravan T <quic_kathirav@quicinc.com>
> > Signed-off-by: Varadarajan Narayanan <quic_varada@quicinc.com>
> > ---
> >  drivers/clk/qcom/apss-ipq6018.c | 54 ++++++++++++++++++++++++++++++++++++++++-
> >  1 file changed, 53 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/clk/qcom/apss-ipq6018.c b/drivers/clk/qcom/apss-ipq6018.c
> > index 4e13a08..ffb6ab5 100644
> > --- a/drivers/clk/qcom/apss-ipq6018.c
> > +++ b/drivers/clk/qcom/apss-ipq6018.c
> > @@ -9,8 +9,11 @@
> >  #include <linux/clk-provider.h>
> >  #include <linux/regmap.h>
> >  #include <linux/module.h>
> > +#include <linux/clk.h>
> > +#include <linux/soc/qcom/smem.h>
> >
> >  #include <dt-bindings/clock/qcom,apss-ipq.h>
> > +#include <dt-bindings/arm/qcom,ids.h>
> >
> >  #include "common.h"
> >  #include "clk-regmap.h"
> > @@ -84,15 +87,64 @@ static const struct qcom_cc_desc apss_ipq6018_desc = {
> >         .num_clks = ARRAY_SIZE(apss_ipq6018_clks),
> >  };
> >
> > +static int cpu_clk_notifier_fn(struct notifier_block *nb, unsigned long action,
> > +                               void *data)
> > +{
> > +       u8 index;
> > +       int err;
> > +
> > +       if (action == PRE_RATE_CHANGE)
> > +               index = P_GPLL0;
>
> I don't see P_GPLL0 being supported in the ipq6018 driver.

This comes from one of the dependency patches mentioned in the
cover letter [https://lore.kernel.org/linux-arm-msm/20230904-gpll_cleanup-v1-0-de2c448f1188@quicinc.com/].
Please refer to patch https://lore.kernel.org/linux-arm-msm/20230904-gpll_cleanup-v1-6-de2c448f1188@quicinc.com/.

>
> > +       else if (action == POST_RATE_CHANGE)
> > +               index = P_APSS_PLL_EARLY;
>
> You also have to handle ABORT_RATE_CHANGE here.

ok.

> > +       else
> > +               return 0;
> > +
> > +       err = clk_rcg2_mux_closest_ops.set_parent(&apcs_alias0_clk_src.clkr.hw,
> > +                                                 index);
> > +
> > +       return notifier_from_errno(err);
> > +}
> > +
> > +static struct notifier_block cpu_clk_notifier = {
> > +       .notifier_call = cpu_clk_notifier_fn,
> > +};
> > +
> >  static int apss_ipq6018_probe(struct platform_device *pdev)
> >  {
> >         struct regmap *regmap;
> > +       u32 soc_id;
> > +       int ret;
> > +
> > +       ret = qcom_smem_get_soc_id(&soc_id);
> > +       if (ret)
> > +               return ret;
> >
> >         regmap = dev_get_regmap(pdev->dev.parent, NULL);
> >         if (!regmap)
> >                 return -ENODEV;
> >
> > -       return qcom_cc_really_probe(pdev, &apss_ipq6018_desc, regmap);
> > +       ret = qcom_cc_really_probe(pdev, &apss_ipq6018_desc, regmap);
> > +       if (ret)
> > +               return ret;
> > +
> > +       switch (soc_id) {
> > +       /*
> > +        * Only below variants of IPQ53xx support scaling
> > +        */
> > +       case QCOM_ID_IPQ5332:
> > +       case QCOM_ID_IPQ5322:
> > +       case QCOM_ID_IPQ5300:
>
> Please use compat strings instead of using the soc-id.

We have a common compatible string for all IPQ53xx CPUs across
boards.  The CPU variant is identified from fuse settings. Not
sure how we can differentiate between the variants using compat
strings. Can you kindly help.

Thanks
varada

> > +               ret = clk_notifier_register(apcs_alias0_clk_src.clkr.hw.clk,
> > +                                               &cpu_clk_notifier);
> > +               if (ret)
> > +                       return ret;
> > +               break;
> > +       default:
> > +               break;
> > +       }
> > +
> > +       return 0;
> >  }
> >
> >  static struct platform_driver apss_ipq6018_driver = {
> > --
> > 2.7.4
> >
>
>
> --
> With best wishes
>
> Dmitry

Re: [PATCH v1 08/10] dt-bindings: cpufreq: qcom-cpufreq-nvmem: document IPQ9574

[Viresh Kumar]

On 07-09-23, 08:04, Krzysztof Kozlowski wrote:
> On 07/09/2023 07:21, Varadarajan Narayanan wrote:
> > Document IPQ9574 compatible for Qcom NVMEM CPUFreq driver.
> > 
> > Signed-off-by: Varadarajan Narayanan <quic_varada@quicinc.com>
> > ---
> >  Documentation/devicetree/bindings/cpufreq/qcom-cpufreq-nvmem.yaml | 1 +
> >  1 file changed, 1 insertion(+)
> 
> 
> Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

Applied. Thanks.

-- 
viresh

Re: [PATCH v1 05/10] dt-bindings: cpufreq: qcom-cpufreq-nvmem: document IPQ5332

[Viresh Kumar]

On 07-09-23, 08:03, Krzysztof Kozlowski wrote:
> On 07/09/2023 07:21, Varadarajan Narayanan wrote:
> > Document IPQ5332 compatible for Qcom NVMEM CPUFreq driver.
> > 
> > Signed-off-by: Varadarajan Narayanan <quic_varada@quicinc.com>
> > ---
> 
> Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

Applied. Thanks.

-- 
viresh

Re: [PATCH v1 01/10] clk: qcom: clk-alpha-pll: introduce stromer plus ops

[Varadarajan Narayanan]

On Thu, Sep 07, 2023 at 04:39:33PM +0300, Dmitry Baryshkov wrote:
> On Thu, 7 Sept 2023 at 08:22, Varadarajan Narayanan
> <quic_varada@quicinc.com> wrote:
> >
> > Stromer plus APSS PLL does not support dynamic frequency scaling.
> > To switch between frequencies, we have to shut down the PLL,
> > configure the L and ALPHA values and turn on again. So introduce the
> > separate set of ops for Stromer Plus PLL.
> >
> > Signed-off-by: Kathiravan T <quic_kathirav@quicinc.com>
> > Signed-off-by: Varadarajan Narayanan <quic_varada@quicinc.com>
> > ---
> >  drivers/clk/qcom/clk-alpha-pll.c | 68 ++++++++++++++++++++++++++++++++++++++++
> >  drivers/clk/qcom/clk-alpha-pll.h |  1 +
> >  2 files changed, 69 insertions(+)
> >
> > diff --git a/drivers/clk/qcom/clk-alpha-pll.c b/drivers/clk/qcom/clk-alpha-pll.c
> > index e4ef645..2ef81f7 100644
> > --- a/drivers/clk/qcom/clk-alpha-pll.c
> > +++ b/drivers/clk/qcom/clk-alpha-pll.c
> > @@ -2479,3 +2479,71 @@ const struct clk_ops clk_alpha_pll_stromer_ops = {
> >         .set_rate = clk_alpha_pll_stromer_set_rate,
> >  };
> >  EXPORT_SYMBOL_GPL(clk_alpha_pll_stromer_ops);
> > +
> > +static int clk_alpha_pll_stromer_plus_determine_rate(struct clk_hw *hw,
> > +                                                    struct clk_rate_request *req)
> > +{
> > +       struct clk_alpha_pll *pll = to_clk_alpha_pll(hw);
> > +       u32 l, alpha_width = pll_alpha_width(pll);
> > +       u64 a;
> > +
> > +       req->rate = alpha_pll_round_rate(req->rate, req->best_parent_rate, &l,
> > +                                        &a, alpha_width);
> > +       return 0;
> > +}
>
> What is the plL_alpha_width on stromer_plus? Does
> clk_alpha_pll_stromer_determine_rate() work for you?

pll_alpha_width is 4. I tested with clk_alpha_pll_stromer_determine_rate()
and it works. Will change and post a new version.

Thanks
Varada

> > +
> > +static int clk_alpha_pll_stromer_plus_set_rate(struct clk_hw *hw,
> > +                                              unsigned long rate,
> > +                                              unsigned long prate)
> > +{
> > +       struct clk_alpha_pll *pll = to_clk_alpha_pll(hw);
> > +       u32 l, alpha_width = pll_alpha_width(pll);
> > +       int ret;
> > +       u64 a;
> > +
> > +       rate = alpha_pll_round_rate(rate, prate, &l, &a, alpha_width);
> > +
> > +       regmap_write(pll->clkr.regmap, PLL_MODE(pll), 0);
> > +
> > +       /* Delay of 2 output clock ticks required until output is disabled */
> > +       udelay(1);
> > +
> > +       regmap_write(pll->clkr.regmap, PLL_L_VAL(pll), l);
> > +
> > +       if (alpha_width > ALPHA_BITWIDTH)
> > +               a <<= alpha_width - ALPHA_BITWIDTH;
> > +
> > +       regmap_write(pll->clkr.regmap, PLL_ALPHA_VAL(pll), a);
> > +       regmap_write(pll->clkr.regmap, PLL_ALPHA_VAL_U(pll),
> > +                                       a >> ALPHA_BITWIDTH);
> > +
> > +       regmap_write(pll->clkr.regmap, PLL_MODE(pll), PLL_BYPASSNL);
> > +
> > +       /* Wait five micro seconds or more */
> > +       udelay(5);
> > +       regmap_update_bits(pll->clkr.regmap, PLL_MODE(pll), PLL_RESET_N,
> > +                          PLL_RESET_N);
> > +
> > +       /* The lock time should be less than 50 micro seconds worst case */
> > +       udelay(50);
> > +
> > +       ret = wait_for_pll_enable_lock(pll);
> > +       if (ret) {
> > +               pr_err("alpha pll running in 800 MHz with source GPLL0\n");
> > +               return ret;
> > +       }
> > +       regmap_update_bits(pll->clkr.regmap, PLL_MODE(pll), PLL_OUTCTRL,
> > +                          PLL_OUTCTRL);
> > +
> > +       return 0;
> > +}
> > +
> > +const struct clk_ops clk_alpha_pll_stromer_plus_ops = {
> > +       .enable = clk_alpha_pll_enable,
> > +       .disable = clk_alpha_pll_disable,
> > +       .is_enabled = clk_alpha_pll_is_enabled,
> > +       .recalc_rate = clk_alpha_pll_recalc_rate,
> > +       .determine_rate = clk_alpha_pll_stromer_plus_determine_rate,
> > +       .set_rate = clk_alpha_pll_stromer_plus_set_rate,
> > +};
> > +EXPORT_SYMBOL_GPL(clk_alpha_pll_stromer_plus_ops);
> > diff --git a/drivers/clk/qcom/clk-alpha-pll.h b/drivers/clk/qcom/clk-alpha-pll.h
> > index e4bd863..903fbab 100644
> > --- a/drivers/clk/qcom/clk-alpha-pll.h
> > +++ b/drivers/clk/qcom/clk-alpha-pll.h
> > @@ -152,6 +152,7 @@ extern const struct clk_ops clk_alpha_pll_postdiv_ops;
> >  extern const struct clk_ops clk_alpha_pll_huayra_ops;
> >  extern const struct clk_ops clk_alpha_pll_postdiv_ro_ops;
> >  extern const struct clk_ops clk_alpha_pll_stromer_ops;
> > +extern const struct clk_ops clk_alpha_pll_stromer_plus_ops;
> >
> >  extern const struct clk_ops clk_alpha_pll_fabia_ops;
> >  extern const struct clk_ops clk_alpha_pll_fixed_fabia_ops;
> > --
> > 2.7.4
> >
>
>
> --
> With best wishes
> Dmitry

Re: [PATCH v1 04/10] clk: qcom: apss-ipq6018: ipq5332: add safe source switch for a53pll

[Varadarajan Narayanan]

On Thu, Sep 07, 2023 at 10:31:55AM +0200, Konrad Dybcio wrote:
> On 7.09.2023 07:21, Varadarajan Narayanan wrote:
> > Stromer Plus PLL found on IPQ53xx doesn't support dynamic
> > frequency scaling. To achieve the same, we need to park the APPS
> > PLL source to GPLL0, re configure the PLL and then switch the
> > source to APSS_PLL_EARLY.
> >
> > To support this, register a clock notifier to get the PRE_RATE
> > and POST_RATE notification. Change the APSS PLL source to GPLL0
> > when PRE_RATE notification is received, then configure the PLL
> > and then change back the source to APSS_PLL_EARLY.
> >
> > Signed-off-by: Kathiravan T <quic_kathirav@quicinc.com>
> > Signed-off-by: Varadarajan Narayanan <quic_varada@quicinc.com>
> > ---
> >  drivers/clk/qcom/apss-ipq6018.c | 54 ++++++++++++++++++++++++++++++++++++++++-
> >  1 file changed, 53 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/clk/qcom/apss-ipq6018.c b/drivers/clk/qcom/apss-ipq6018.c
> > index 4e13a08..ffb6ab5 100644
> > --- a/drivers/clk/qcom/apss-ipq6018.c
> > +++ b/drivers/clk/qcom/apss-ipq6018.c
> > @@ -9,8 +9,11 @@
> >  #include <linux/clk-provider.h>
> >  #include <linux/regmap.h>
> >  #include <linux/module.h>
> > +#include <linux/clk.h>
> > +#include <linux/soc/qcom/smem.h>
> >
> >  #include <dt-bindings/clock/qcom,apss-ipq.h>
> > +#include <dt-bindings/arm/qcom,ids.h>
> >
> >  #include "common.h"
> >  #include "clk-regmap.h"
> > @@ -84,15 +87,64 @@ static const struct qcom_cc_desc apss_ipq6018_desc = {
> >  	.num_clks = ARRAY_SIZE(apss_ipq6018_clks),
> >  };
> >
> > +static int cpu_clk_notifier_fn(struct notifier_block *nb, unsigned long action,
> > +				void *data)
> > +{
> > +	u8 index;
> > +	int err;
> > +
> > +	if (action == PRE_RATE_CHANGE)
> > +		index = P_GPLL0;
> > +	else if (action == POST_RATE_CHANGE)
> > +		index = P_APSS_PLL_EARLY;
> > +	else
> > +		return 0;
> > +
> > +	err = clk_rcg2_mux_closest_ops.set_parent(&apcs_alias0_clk_src.clkr.hw,
> > +						  index);
> Adding a variable for clk_hw within the apcs_alias0 clock would
> make this easier to digest, I think.
>
> And if we wanna be even less error-prone, you can reference the
> ops of this clock in an indirect way.

Will change it as

	struct clk_hw *hw;

	hw = &apcs_alias0_clk_src.clkr.hw;
	err = hw->init->ops->set_parent(hw, index);

> > +	return notifier_from_errno(err);
> > +}
> > +
> > +static struct notifier_block cpu_clk_notifier = {
> > +	.notifier_call = cpu_clk_notifier_fn,
> > +};
> > +
> >  static int apss_ipq6018_probe(struct platform_device *pdev)
> >  {
> >  	struct regmap *regmap;
> > +	u32 soc_id;
> > +	int ret;
> > +
> > +	ret = qcom_smem_get_soc_id(&soc_id);
> > +	if (ret)
> > +		return ret;
> >
> >  	regmap = dev_get_regmap(pdev->dev.parent, NULL);
> >  	if (!regmap)
> >  		return -ENODEV;
> >
> > -	return qcom_cc_really_probe(pdev, &apss_ipq6018_desc, regmap);
> > +	ret = qcom_cc_really_probe(pdev, &apss_ipq6018_desc, regmap);
> > +	if (ret)
> > +		return ret;
> > +
> > +	switch (soc_id) {
> > +	/*
> > +	 * Only below variants of IPQ53xx support scaling
> > +	 */
> 1. /* Keep this in a 1-line comment */

Ok

> 2. why? explain the reasoning in the commit message

Ok

Thanks
Varada

> > +	case QCOM_ID_IPQ5332:
> > +	case QCOM_ID_IPQ5322:
> > +	case QCOM_ID_IPQ5300:
> > +		ret = clk_notifier_register(apcs_alias0_clk_src.clkr.hw.clk,
> > +						&cpu_clk_notifier);
> > +		if (ret)
> > +			return ret;
> > +		break;
> > +	default:
> > +		break;
> > +	}
> > +
> > +	return 0;
> >  }
> >
> >  static struct platform_driver apss_ipq6018_driver = {

Re: [PATCH v1 04/10] clk: qcom: apss-ipq6018: ipq5332: add safe source switch for a53pll

[Dmitry Baryshkov]

On Fri, 29 Sept 2023 at 10:33, Varadarajan Narayanan
<quic_varada@quicinc.com> wrote:
>
> On Thu, Sep 07, 2023 at 10:31:55AM +0200, Konrad Dybcio wrote:
> > On 7.09.2023 07:21, Varadarajan Narayanan wrote:
> > > Stromer Plus PLL found on IPQ53xx doesn't support dynamic
> > > frequency scaling. To achieve the same, we need to park the APPS
> > > PLL source to GPLL0, re configure the PLL and then switch the
> > > source to APSS_PLL_EARLY.
> > >
> > > To support this, register a clock notifier to get the PRE_RATE
> > > and POST_RATE notification. Change the APSS PLL source to GPLL0
> > > when PRE_RATE notification is received, then configure the PLL
> > > and then change back the source to APSS_PLL_EARLY.
> > >
> > > Signed-off-by: Kathiravan T <quic_kathirav@quicinc.com>
> > > Signed-off-by: Varadarajan Narayanan <quic_varada@quicinc.com>
> > > ---
> > >  drivers/clk/qcom/apss-ipq6018.c | 54 ++++++++++++++++++++++++++++++++++++++++-
> > >  1 file changed, 53 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/clk/qcom/apss-ipq6018.c b/drivers/clk/qcom/apss-ipq6018.c
> > > index 4e13a08..ffb6ab5 100644
> > > --- a/drivers/clk/qcom/apss-ipq6018.c
> > > +++ b/drivers/clk/qcom/apss-ipq6018.c
> > > @@ -9,8 +9,11 @@
> > >  #include <linux/clk-provider.h>
> > >  #include <linux/regmap.h>
> > >  #include <linux/module.h>
> > > +#include <linux/clk.h>
> > > +#include <linux/soc/qcom/smem.h>
> > >
> > >  #include <dt-bindings/clock/qcom,apss-ipq.h>
> > > +#include <dt-bindings/arm/qcom,ids.h>
> > >
> > >  #include "common.h"
> > >  #include "clk-regmap.h"
> > > @@ -84,15 +87,64 @@ static const struct qcom_cc_desc apss_ipq6018_desc = {
> > >     .num_clks = ARRAY_SIZE(apss_ipq6018_clks),
> > >  };
> > >
> > > +static int cpu_clk_notifier_fn(struct notifier_block *nb, unsigned long action,
> > > +                           void *data)
> > > +{
> > > +   u8 index;
> > > +   int err;
> > > +
> > > +   if (action == PRE_RATE_CHANGE)
> > > +           index = P_GPLL0;
> > > +   else if (action == POST_RATE_CHANGE)
> > > +           index = P_APSS_PLL_EARLY;
> > > +   else
> > > +           return 0;
> > > +
> > > +   err = clk_rcg2_mux_closest_ops.set_parent(&apcs_alias0_clk_src.clkr.hw,
> > > +                                             index);
> > Adding a variable for clk_hw within the apcs_alias0 clock would
> > make this easier to digest, I think.
> >
> > And if we wanna be even less error-prone, you can reference the
> > ops of this clock in an indirect way.
>
> Will change it as
>
>         struct clk_hw *hw;
>
>         hw = &apcs_alias0_clk_src.clkr.hw;
>         err = hw->init->ops->set_parent(hw, index);

You can not do this, hw->init is cleared during registration.

>
> > > +   return notifier_from_errno(err);
> > > +}
> > > +
> > > +static struct notifier_block cpu_clk_notifier = {
> > > +   .notifier_call = cpu_clk_notifier_fn,
> > > +};
> > > +
> > >  static int apss_ipq6018_probe(struct platform_device *pdev)
> > >  {
> > >     struct regmap *regmap;
> > > +   u32 soc_id;
> > > +   int ret;
> > > +
> > > +   ret = qcom_smem_get_soc_id(&soc_id);
> > > +   if (ret)
> > > +           return ret;
> > >
> > >     regmap = dev_get_regmap(pdev->dev.parent, NULL);
> > >     if (!regmap)
> > >             return -ENODEV;
> > >
> > > -   return qcom_cc_really_probe(pdev, &apss_ipq6018_desc, regmap);
> > > +   ret = qcom_cc_really_probe(pdev, &apss_ipq6018_desc, regmap);
> > > +   if (ret)
> > > +           return ret;
> > > +
> > > +   switch (soc_id) {
> > > +   /*
> > > +    * Only below variants of IPQ53xx support scaling
> > > +    */
> > 1. /* Keep this in a 1-line comment */
>
> Ok
>
> > 2. why? explain the reasoning in the commit message
>
> Ok
>
> Thanks
> Varada
>
> > > +   case QCOM_ID_IPQ5332:
> > > +   case QCOM_ID_IPQ5322:
> > > +   case QCOM_ID_IPQ5300:
> > > +           ret = clk_notifier_register(apcs_alias0_clk_src.clkr.hw.clk,
> > > +                                           &cpu_clk_notifier);
> > > +           if (ret)
> > > +                   return ret;
> > > +           break;
> > > +   default:
> > > +           break;
> > > +   }
> > > +
> > > +   return 0;
> > >  }
> > >
> > >  static struct platform_driver apss_ipq6018_driver = {



-- 
With best wishes
Dmitry

Re: [PATCH v1 07/10] arm64: dts: qcom: ipq5332: populate the opp table based on the eFuse

[Varadarajan Narayanan]

On Thu, Sep 07, 2023 at 04:59:28PM +0300, Dmitry Baryshkov wrote:
> On Thu, 7 Sept 2023 at 08:23, Varadarajan Narayanan
> <quic_varada@quicinc.com> wrote:
> >
> > IPQ53xx 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.
> >
> > Add support to read the eFuse and populate the OPPs based on it.
> >
> > Signed-off-by: Kathiravan T <quic_kathirav@quicinc.com>
> > Signed-off-by: Varadarajan Narayanan <quic_varada@quicinc.com>
> > ---
> >  arch/arm64/boot/dts/qcom/ipq5332.dtsi | 34 +++++++++++++++++++++++++++++++---
> >  1 file changed, 31 insertions(+), 3 deletions(-)
> >
> > diff --git a/arch/arm64/boot/dts/qcom/ipq5332.dtsi b/arch/arm64/boot/dts/qcom/ipq5332.dtsi
> > index 82761ae..3ca3f34 100644
> > --- a/arch/arm64/boot/dts/qcom/ipq5332.dtsi
> > +++ b/arch/arm64/boot/dts/qcom/ipq5332.dtsi
> > @@ -91,11 +91,34 @@
> >         };
> >
> >         cpu_opp_table: opp-table-cpu {
> > -               compatible = "operating-points-v2";
> > +               compatible = "operating-points-v2-kryo-cpu";
> >                 opp-shared;
> > +               nvmem-cells = <&cpu_speed_bin>;
> > +               nvmem-cell-names = "speed_bin";
> > +
> > +               /*
> > +                * Listed all supported CPU frequencies and opp-supported-hw
> > +                * values to select CPU frequencies based on the limits fused.
> > +                * ------------------------------------------------------------
> > +                * Frequency     BIT3   BIT2   BIT1    BIT0    opp-supported-hw
> > +                *              1.0GHz 1.2GHz 1.5GHz No Limit
> > +                * ------------------------------------------------------------
> > +                * 1100000000     1      1      1       1            0xF
> > +                * 1500000000     0      0      1       1            0x3
> > +                * -----------------------------------------------------------
> > +                */
>
> This can probably go to the commit message instead.

Ok

> > +
> > +               opp-1100000000 {
> > +                       opp-hz = /bits/ 64 <1100000000>;
>
> But your table shows 1.0 GHz and 1.2 GHz instead of 1.1 GHz

Will update it.

> > +                       opp-microvolt = <850000>;
> > +                       opp-supported-hw = <0xF>;
> > +                       clock-latency-ns = <200000>;
> > +               };
> >
> > -               opp-1488000000 {
> > -                       opp-hz = /bits/ 64 <1488000000>;
> > +               opp-1500000000 {
> > +                       opp-hz = /bits/ 64 <1500000000>;
>
> So, 1.488 GHz or 1.5 GHz?

1.5 GHz

> > +                       opp-microvolt = <950000>;
>
> Which regulator is controlled by this microvolt?

Based on the SKU, the XBL sets up the regulator to provide 950000uV
on CPUs capable of running 1.5G and 850000uV on other SKUs. Linux
doesn't control it.

Thanks
Varada
> > +                       opp-supported-hw = <0x3>;
> >                         clock-latency-ns = <200000>;
> >                 };
> >         };
> > @@ -150,6 +173,11 @@
> >                         reg = <0x000a4000 0x721>;
> >                         #address-cells = <1>;
> >                         #size-cells = <1>;
> > +
> > +                       cpu_speed_bin: cpu_speed_bin@1d {
> > +                               reg = <0x1d 0x2>;
> > +                               bits = <7 2>;
> > +                       };
> >                 };
> >
> >                 rng: rng@e3000 {
> > --
> > 2.7.4
> >
>
>
> --
> With best wishes
> Dmitry

Re: [PATCH v1 07/10] arm64: dts: qcom: ipq5332: populate the opp table based on the eFuse

[Dmitry Baryshkov]

On Thu, 5 Oct 2023 at 12:58, Varadarajan Narayanan
<quic_varada@quicinc.com> wrote:
>
> On Thu, Sep 07, 2023 at 04:59:28PM +0300, Dmitry Baryshkov wrote:
> > On Thu, 7 Sept 2023 at 08:23, Varadarajan Narayanan
> > <quic_varada@quicinc.com> wrote:
> > >
> > > IPQ53xx 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.
> > >
> > > Add support to read the eFuse and populate the OPPs based on it.
> > >
> > > Signed-off-by: Kathiravan T <quic_kathirav@quicinc.com>
> > > Signed-off-by: Varadarajan Narayanan <quic_varada@quicinc.com>
> > > ---
> > >  arch/arm64/boot/dts/qcom/ipq5332.dtsi | 34 +++++++++++++++++++++++++++++++---
> > >  1 file changed, 31 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/arch/arm64/boot/dts/qcom/ipq5332.dtsi b/arch/arm64/boot/dts/qcom/ipq5332.dtsi
> > > index 82761ae..3ca3f34 100644
> > > --- a/arch/arm64/boot/dts/qcom/ipq5332.dtsi
> > > +++ b/arch/arm64/boot/dts/qcom/ipq5332.dtsi
> > > @@ -91,11 +91,34 @@
> > >         };
> > >
> > >         cpu_opp_table: opp-table-cpu {
> > > -               compatible = "operating-points-v2";
> > > +               compatible = "operating-points-v2-kryo-cpu";
> > >                 opp-shared;
> > > +               nvmem-cells = <&cpu_speed_bin>;
> > > +               nvmem-cell-names = "speed_bin";
> > > +
> > > +               /*
> > > +                * Listed all supported CPU frequencies and opp-supported-hw
> > > +                * values to select CPU frequencies based on the limits fused.
> > > +                * ------------------------------------------------------------
> > > +                * Frequency     BIT3   BIT2   BIT1    BIT0    opp-supported-hw
> > > +                *              1.0GHz 1.2GHz 1.5GHz No Limit
> > > +                * ------------------------------------------------------------
> > > +                * 1100000000     1      1      1       1            0xF
> > > +                * 1500000000     0      0      1       1            0x3
> > > +                * -----------------------------------------------------------
> > > +                */
> >
> > This can probably go to the commit message instead.
>
> Ok
>
> > > +
> > > +               opp-1100000000 {
> > > +                       opp-hz = /bits/ 64 <1100000000>;
> >
> > But your table shows 1.0 GHz and 1.2 GHz instead of 1.1 GHz
>
> Will update it.
>
> > > +                       opp-microvolt = <850000>;
> > > +                       opp-supported-hw = <0xF>;
> > > +                       clock-latency-ns = <200000>;
> > > +               };
> > >
> > > -               opp-1488000000 {
> > > -                       opp-hz = /bits/ 64 <1488000000>;
> > > +               opp-1500000000 {
> > > +                       opp-hz = /bits/ 64 <1500000000>;
> >
> > So, 1.488 GHz or 1.5 GHz?
>
> 1.5 GHz
>
> > > +                       opp-microvolt = <950000>;
> >
> > Which regulator is controlled by this microvolt?
>
> Based on the SKU, the XBL sets up the regulator to provide 950000uV
> on CPUs capable of running 1.5G and 850000uV on other SKUs. Linux
> doesn't control it.

Then why do you need this property here in the first place?


-- 
With best wishes
Dmitry

Re: [PATCH v1 07/10] arm64: dts: qcom: ipq5332: populate the opp table based on the eFuse

[Varadarajan Narayanan]

On Thu, Oct 05, 2023 at 02:39:43PM +0300, Dmitry Baryshkov wrote:
> On Thu, 5 Oct 2023 at 12:58, Varadarajan Narayanan
> <quic_varada@quicinc.com> wrote:
> >
> > On Thu, Sep 07, 2023 at 04:59:28PM +0300, Dmitry Baryshkov wrote:
> > > On Thu, 7 Sept 2023 at 08:23, Varadarajan Narayanan
> > > <quic_varada@quicinc.com> wrote:
> > > >
> > > > IPQ53xx 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.
> > > >
> > > > Add support to read the eFuse and populate the OPPs based on it.
> > > >
> > > > Signed-off-by: Kathiravan T <quic_kathirav@quicinc.com>
> > > > Signed-off-by: Varadarajan Narayanan <quic_varada@quicinc.com>
> > > > ---
> > > >  arch/arm64/boot/dts/qcom/ipq5332.dtsi | 34 +++++++++++++++++++++++++++++++---
> > > >  1 file changed, 31 insertions(+), 3 deletions(-)
> > > >
> > > > diff --git a/arch/arm64/boot/dts/qcom/ipq5332.dtsi b/arch/arm64/boot/dts/qcom/ipq5332.dtsi
> > > > index 82761ae..3ca3f34 100644
> > > > --- a/arch/arm64/boot/dts/qcom/ipq5332.dtsi
> > > > +++ b/arch/arm64/boot/dts/qcom/ipq5332.dtsi
> > > > @@ -91,11 +91,34 @@
> > > >         };
> > > >
> > > >         cpu_opp_table: opp-table-cpu {
> > > > -               compatible = "operating-points-v2";
> > > > +               compatible = "operating-points-v2-kryo-cpu";
> > > >                 opp-shared;
> > > > +               nvmem-cells = <&cpu_speed_bin>;
> > > > +               nvmem-cell-names = "speed_bin";
> > > > +
> > > > +               /*
> > > > +                * Listed all supported CPU frequencies and opp-supported-hw
> > > > +                * values to select CPU frequencies based on the limits fused.
> > > > +                * ------------------------------------------------------------
> > > > +                * Frequency     BIT3   BIT2   BIT1    BIT0    opp-supported-hw
> > > > +                *              1.0GHz 1.2GHz 1.5GHz No Limit
> > > > +                * ------------------------------------------------------------
> > > > +                * 1100000000     1      1      1       1            0xF
> > > > +                * 1500000000     0      0      1       1            0x3
> > > > +                * -----------------------------------------------------------
> > > > +                */
> > >
> > > This can probably go to the commit message instead.
> >
> > Ok
> >
> > > > +
> > > > +               opp-1100000000 {
> > > > +                       opp-hz = /bits/ 64 <1100000000>;
> > >
> > > But your table shows 1.0 GHz and 1.2 GHz instead of 1.1 GHz
> >
> > Will update it.
> >
> > > > +                       opp-microvolt = <850000>;
> > > > +                       opp-supported-hw = <0xF>;
> > > > +                       clock-latency-ns = <200000>;
> > > > +               };
> > > >
> > > > -               opp-1488000000 {
> > > > -                       opp-hz = /bits/ 64 <1488000000>;
> > > > +               opp-1500000000 {
> > > > +                       opp-hz = /bits/ 64 <1500000000>;
> > >
> > > So, 1.488 GHz or 1.5 GHz?
> >
> > 1.5 GHz
> >
> > > > +                       opp-microvolt = <950000>;
> > >
> > > Which regulator is controlled by this microvolt?
> >
> > Based on the SKU, the XBL sets up the regulator to provide 950000uV
> > on CPUs capable of running 1.5G and 850000uV on other SKUs. Linux
> > doesn't control it.
>
> Then why do you need this property here in the first place?

I get these errors without this property

[    1.018065] cpu cpu0: opp_parse_microvolt: opp-microvolt missing although OPP managing regulators
[    1.018074] cpu cpu0: _of_add_opp_table_v2: Failed to add OPP, -22

Thanks
Varada

Re: [PATCH v1 07/10] arm64: dts: qcom: ipq5332: populate the opp table based on the eFuse

[Dmitry Baryshkov]

On Thu, 5 Oct 2023 at 17:42, Varadarajan Narayanan
<quic_varada@quicinc.com> wrote:
>
> On Thu, Oct 05, 2023 at 02:39:43PM +0300, Dmitry Baryshkov wrote:
> > On Thu, 5 Oct 2023 at 12:58, Varadarajan Narayanan
> > <quic_varada@quicinc.com> wrote:
> > >
> > > On Thu, Sep 07, 2023 at 04:59:28PM +0300, Dmitry Baryshkov wrote:
> > > > On Thu, 7 Sept 2023 at 08:23, Varadarajan Narayanan
> > > > <quic_varada@quicinc.com> wrote:
> > > > >
> > > > > IPQ53xx 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.
> > > > >
> > > > > Add support to read the eFuse and populate the OPPs based on it.
> > > > >
> > > > > Signed-off-by: Kathiravan T <quic_kathirav@quicinc.com>
> > > > > Signed-off-by: Varadarajan Narayanan <quic_varada@quicinc.com>
> > > > > ---
> > > > >  arch/arm64/boot/dts/qcom/ipq5332.dtsi | 34 +++++++++++++++++++++++++++++++---
> > > > >  1 file changed, 31 insertions(+), 3 deletions(-)
> > > > >
> > > > > diff --git a/arch/arm64/boot/dts/qcom/ipq5332.dtsi b/arch/arm64/boot/dts/qcom/ipq5332.dtsi
> > > > > index 82761ae..3ca3f34 100644
> > > > > --- a/arch/arm64/boot/dts/qcom/ipq5332.dtsi
> > > > > +++ b/arch/arm64/boot/dts/qcom/ipq5332.dtsi
> > > > > @@ -91,11 +91,34 @@
> > > > >         };
> > > > >
> > > > >         cpu_opp_table: opp-table-cpu {
> > > > > -               compatible = "operating-points-v2";
> > > > > +               compatible = "operating-points-v2-kryo-cpu";
> > > > >                 opp-shared;
> > > > > +               nvmem-cells = <&cpu_speed_bin>;
> > > > > +               nvmem-cell-names = "speed_bin";
> > > > > +
> > > > > +               /*
> > > > > +                * Listed all supported CPU frequencies and opp-supported-hw
> > > > > +                * values to select CPU frequencies based on the limits fused.
> > > > > +                * ------------------------------------------------------------
> > > > > +                * Frequency     BIT3   BIT2   BIT1    BIT0    opp-supported-hw
> > > > > +                *              1.0GHz 1.2GHz 1.5GHz No Limit
> > > > > +                * ------------------------------------------------------------
> > > > > +                * 1100000000     1      1      1       1            0xF
> > > > > +                * 1500000000     0      0      1       1            0x3
> > > > > +                * -----------------------------------------------------------
> > > > > +                */
> > > >
> > > > This can probably go to the commit message instead.
> > >
> > > Ok
> > >
> > > > > +
> > > > > +               opp-1100000000 {
> > > > > +                       opp-hz = /bits/ 64 <1100000000>;
> > > >
> > > > But your table shows 1.0 GHz and 1.2 GHz instead of 1.1 GHz
> > >
> > > Will update it.
> > >
> > > > > +                       opp-microvolt = <850000>;
> > > > > +                       opp-supported-hw = <0xF>;
> > > > > +                       clock-latency-ns = <200000>;
> > > > > +               };
> > > > >
> > > > > -               opp-1488000000 {
> > > > > -                       opp-hz = /bits/ 64 <1488000000>;
> > > > > +               opp-1500000000 {
> > > > > +                       opp-hz = /bits/ 64 <1500000000>;
> > > >
> > > > So, 1.488 GHz or 1.5 GHz?
> > >
> > > 1.5 GHz
> > >
> > > > > +                       opp-microvolt = <950000>;
> > > >
> > > > Which regulator is controlled by this microvolt?
> > >
> > > Based on the SKU, the XBL sets up the regulator to provide 950000uV
> > > on CPUs capable of running 1.5G and 850000uV on other SKUs. Linux
> > > doesn't control it.
> >
> > Then why do you need this property here in the first place?
>
> I get these errors without this property
>
> [    1.018065] cpu cpu0: opp_parse_microvolt: opp-microvolt missing although OPP managing regulators

But you have said that "Linux doesn't control it" [the regulator]!

> [    1.018074] cpu cpu0: _of_add_opp_table_v2: Failed to add OPP, -22

-- 
With best wishes
Dmitry

Re: [PATCH v1 07/10] arm64: dts: qcom: ipq5332: populate the opp table based on the eFuse

[Varadarajan Narayanan]

On Thu, Oct 05, 2023 at 10:39:55PM +0300, Dmitry Baryshkov wrote:
> On Thu, 5 Oct 2023 at 17:42, Varadarajan Narayanan
> <quic_varada@quicinc.com> wrote:
> >
> > On Thu, Oct 05, 2023 at 02:39:43PM +0300, Dmitry Baryshkov wrote:
> > > On Thu, 5 Oct 2023 at 12:58, Varadarajan Narayanan
> > > <quic_varada@quicinc.com> wrote:
> > > >
> > > > On Thu, Sep 07, 2023 at 04:59:28PM +0300, Dmitry Baryshkov wrote:
> > > > > On Thu, 7 Sept 2023 at 08:23, Varadarajan Narayanan
> > > > > <quic_varada@quicinc.com> wrote:
> > > > > >
> > > > > > IPQ53xx 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.
> > > > > >
> > > > > > Add support to read the eFuse and populate the OPPs based on it.
> > > > > >
> > > > > > Signed-off-by: Kathiravan T <quic_kathirav@quicinc.com>
> > > > > > Signed-off-by: Varadarajan Narayanan <quic_varada@quicinc.com>
> > > > > > ---
> > > > > >  arch/arm64/boot/dts/qcom/ipq5332.dtsi | 34 +++++++++++++++++++++++++++++++---
> > > > > >  1 file changed, 31 insertions(+), 3 deletions(-)
> > > > > >
> > > > > > diff --git a/arch/arm64/boot/dts/qcom/ipq5332.dtsi b/arch/arm64/boot/dts/qcom/ipq5332.dtsi
> > > > > > index 82761ae..3ca3f34 100644
> > > > > > --- a/arch/arm64/boot/dts/qcom/ipq5332.dtsi
> > > > > > +++ b/arch/arm64/boot/dts/qcom/ipq5332.dtsi
> > > > > > @@ -91,11 +91,34 @@
> > > > > >         };
> > > > > >
> > > > > >         cpu_opp_table: opp-table-cpu {
> > > > > > -               compatible = "operating-points-v2";
> > > > > > +               compatible = "operating-points-v2-kryo-cpu";
> > > > > >                 opp-shared;
> > > > > > +               nvmem-cells = <&cpu_speed_bin>;
> > > > > > +               nvmem-cell-names = "speed_bin";
> > > > > > +
> > > > > > +               /*
> > > > > > +                * Listed all supported CPU frequencies and opp-supported-hw
> > > > > > +                * values to select CPU frequencies based on the limits fused.
> > > > > > +                * ------------------------------------------------------------
> > > > > > +                * Frequency     BIT3   BIT2   BIT1    BIT0    opp-supported-hw
> > > > > > +                *              1.0GHz 1.2GHz 1.5GHz No Limit
> > > > > > +                * ------------------------------------------------------------
> > > > > > +                * 1100000000     1      1      1       1            0xF
> > > > > > +                * 1500000000     0      0      1       1            0x3
> > > > > > +                * -----------------------------------------------------------
> > > > > > +                */
> > > > >
> > > > > This can probably go to the commit message instead.
> > > >
> > > > Ok
> > > >
> > > > > > +
> > > > > > +               opp-1100000000 {
> > > > > > +                       opp-hz = /bits/ 64 <1100000000>;
> > > > >
> > > > > But your table shows 1.0 GHz and 1.2 GHz instead of 1.1 GHz
> > > >
> > > > Will update it.
> > > >
> > > > > > +                       opp-microvolt = <850000>;
> > > > > > +                       opp-supported-hw = <0xF>;
> > > > > > +                       clock-latency-ns = <200000>;
> > > > > > +               };
> > > > > >
> > > > > > -               opp-1488000000 {
> > > > > > -                       opp-hz = /bits/ 64 <1488000000>;
> > > > > > +               opp-1500000000 {
> > > > > > +                       opp-hz = /bits/ 64 <1500000000>;
> > > > >
> > > > > So, 1.488 GHz or 1.5 GHz?
> > > >
> > > > 1.5 GHz
> > > >
> > > > > > +                       opp-microvolt = <950000>;
> > > > >
> > > > > Which regulator is controlled by this microvolt?
> > > >
> > > > Based on the SKU, the XBL sets up the regulator to provide 950000uV
> > > > on CPUs capable of running 1.5G and 850000uV on other SKUs. Linux
> > > > doesn't control it.
> > >
> > > Then why do you need this property here in the first place?
> >
> > I get these errors without this property
> >
> > [    1.018065] cpu cpu0: opp_parse_microvolt: opp-microvolt missing although OPP managing regulators
>
> But you have said that "Linux doesn't control it" [the regulator]!

Got confused between the ipq9574 and ipq5332 patches.
Have removed and addressed other comments too and
posted v2 - https://lore.kernel.org/linux-arm-msm/cover.1697101543.git.quic_varada@quicinc.com/

Please take a look.

Thanks
Varada
>
> > [    1.018074] cpu cpu0: _of_add_opp_table_v2: Failed to add OPP, -22