Skip to contentSkip to conversation

Enable cpufreq for IPQ5332 & IPQ9574

PATCH Superseded Varadarajan Narayanan 4 participants 15 messages Oct 12, 2023 – Oct 18, 2023

Subsystems (7)

v2 of 6 Superseded Latest is v6 →

lore
Share & tools
lore
#0 lore raw

v2 cover Enable cpufreq for IPQ5332 & IPQ9574

Depends On:
https://lore.kernel.org/lkml/[email protected]/T/

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

Removed 2 patches from V1 as they have been merged
* dt-bindings: cpufreq: qcom-cpufreq-nvmem: document IPQ5332
* dt-bindings: cpufreq: qcom-cpufreq-nvmem: document IPQ9574

Varadarajan Narayanan (8):
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
cpufreq: qti: Enable cpufreq for ipq53xx
arm64: dts: qcom: ipq5332: populate the opp table based on the eFuse
cpufreq: qti: Introduce cpufreq for ipq95xx
arm64: dts: qcom: ipq9574: populate the opp table based on the eFuse

 arch/arm64/boot/dts/qcom/ipq5332.dtsi | 19 ++++++++++--
 arch/arm64/boot/dts/qcom/ipq9574.dtsi | 21 ++++++++++++-
 drivers/clk/qcom/apss-ipq-pll.c       |  4 +--
 drivers/clk/qcom/apss-ipq6018.c       | 53 +++++++++++++++++++++++++++++++-
 drivers/clk/qcom/clk-alpha-pll.c      | 57 +++++++++++++++++++++++++++++++++++
 drivers/clk/qcom/clk-alpha-pll.h      |  1 +
 drivers/cpufreq/cpufreq-dt-platdev.c  |  2 ++
 drivers/cpufreq/qcom-cpufreq-nvmem.c  | 16 ++++++++++
 8 files changed, 166 insertions(+), 7 deletions(-)
Signature · 1 line
2.7.4
#1 lore raw

PATCH v2 1/8 clk: qcom: clk-alpha-pll: introduce stromer plus ops

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.

---
v2: Use clk_alpha_pll_stromer_determine_rate, instead of adding new
clk_alpha_pll_stromer_plus_determine_rate as the alpha pll width is same for both

Fix review comments
udelay(50) -> usleep_range(50, 60) Remove SoC-specific from print message

---
 drivers/clk/qcom/clk-alpha-pll.c | 57 ++++++++++++++++++++++++++++++++++++++++
 drivers/clk/qcom/clk-alpha-pll.h |  1 +
 2 files changed, 58 insertions(+)
drivers/clk/qcom/clk-alpha-pll.c +57 -0
index 4edbf77..5221b6c 100644
@@ -2508,3 +2508,60 @@ 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_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 */
+	usleep_range(50, 60);
+
+	ret = wait_for_pll_enable_lock(pll);
+	if (ret) {
+		pr_err("Wait for PLL enable lock failed [%s] %d\n",
+		       clk_hw_get_name(hw), ret);
+		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_determine_rate,
+	.set_rate = clk_alpha_pll_stromer_plus_set_rate,
+};
+EXPORT_SYMBOL_GPL(clk_alpha_pll_stromer_plus_ops);
drivers/clk/qcom/clk-alpha-pll.h +1 -0
index 3b24a66..a1a75bb 100644
@@ -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;
Signature · 1 line
2.7.4
maintainer
#2 lore raw

Re: [PATCH v2 1/8] clk: qcom: clk-alpha-pll: introduce stromer plus ops

Quoting Varadarajan Narayanan (2023-10-12 02:26:17)

Quoted 4 lines

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.

Does this assume the PLL is always on?

Quoted 27 lines

---
v2: Use clk_alpha_pll_stromer_determine_rate, instead of adding new
clk_alpha_pll_stromer_plus_determine_rate as the alpha pll width is same for both

Fix review comments
udelay(50) -> usleep_range(50, 60) Remove SoC-specific from print message

---
 drivers/clk/qcom/clk-alpha-pll.c | 57 ++++++++++++++++++++++++++++++++++++++++
 drivers/clk/qcom/clk-alpha-pll.h |  1 +
 2 files changed, 58 insertions(+)
drivers/clk/qcom/clk-alpha-pll.c +3 -0
index 4edbf77..5221b6c 100644
@@ -2508,3 +2508,60 @@ 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_set_rate(struct clk_hw *hw,
+                                              unsigned long rate,

drivers/clk/qcom/clk-alpha-pll.c const struct clk_ops clk_alpha_pll_stromer_ops = { · line 2523

Quoted 10 lines
drivers/clk/qcom/clk-alpha-pll.c +10 -0
+                                              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);

There's a theoretical problem here if I understand correctly. A call to clk_enable() can happen while clk_set_rate() is in progress or vice versa. Probably we need some sort of spinlock for this PLL that synchronizes any enable/disable with the rate change so that when we restore the enable bit the clk isn't enabled when it was supposed to be off.

#3 lore raw

Re: [PATCH v2 1/8] clk: qcom: clk-alpha-pll: introduce stromer plus ops

Quoted 7 lines · Stephen Boyd

Quoting Varadarajan Narayanan (2023-10-12 02:26:17)

Quoted 4 lines

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.

Does this assume the PLL is always on?

Yes once the PLL is configured by apss-ipq-pll driver, it is always on.

Quoted 35 lines
Quoted 35 lines

---
v2: Use clk_alpha_pll_stromer_determine_rate, instead of adding new
clk_alpha_pll_stromer_plus_determine_rate as the alpha pll width is same for both

Fix review comments
udelay(50) -> usleep_range(50, 60) Remove SoC-specific from print message

---
 drivers/clk/qcom/clk-alpha-pll.c | 57 ++++++++++++++++++++++++++++++++++++++++
 drivers/clk/qcom/clk-alpha-pll.h |  1 +
 2 files changed, 58 insertions(+)
drivers/clk/qcom/clk-alpha-pll.c +12 -0
index 4edbf77..5221b6c 100644
@@ -2508,3 +2508,60 @@ 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_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);
+
Quoted 8 lines

drivers/clk/qcom/clk-alpha-pll.c const struct clk_ops clk_alpha_pll_stromer_ops = { · line 2523

Quoted 1 line
drivers/clk/qcom/clk-alpha-pll.c +1 -0
+       regmap_write(pll->clkr.regmap, PLL_MODE(pll), 0);

There's a theoretical problem here if I understand correctly. A call to clk_enable() can happen while clk_set_rate() is in progress or vice versa. Probably we need some sort of spinlock for this PLL that synchronizes any enable/disable with the rate change so that when we restore the enable bit the clk isn't enabled when it was supposed to be off.

Since the PLL is always on, should we worry about enable/disable? If you feel it is better to synchronize with a spin lock, will add and post a new revision. Please let me know.

Thanks
Varada

#4 lore raw

Re: [PATCH v2 1/8] clk: qcom: clk-alpha-pll: introduce stromer plus ops

Quoted 56 lines · Varadarajan Narayanan
Quoted 7 lines · Stephen Boyd

Quoting Varadarajan Narayanan (2023-10-12 02:26:17)

Quoted 4 lines

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.

Does this assume the PLL is always on?

Yes once the PLL is configured by apss-ipq-pll driver, it is always on.

Quoted 35 lines
Quoted 35 lines

---
v2: Use clk_alpha_pll_stromer_determine_rate, instead of adding new
clk_alpha_pll_stromer_plus_determine_rate as the alpha pll width is same for both

Fix review comments
udelay(50) -> usleep_range(50, 60) Remove SoC-specific from print message

---
 drivers/clk/qcom/clk-alpha-pll.c | 57 ++++++++++++++++++++++++++++++++++++++++
 drivers/clk/qcom/clk-alpha-pll.h |  1 +
 2 files changed, 58 insertions(+)
drivers/clk/qcom/clk-alpha-pll.c +12 -0
index 4edbf77..5221b6c 100644
@@ -2508,3 +2508,60 @@ 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_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);
+
Quoted 8 lines

drivers/clk/qcom/clk-alpha-pll.c const struct clk_ops clk_alpha_pll_stromer_ops = { · line 2523

Quoted 1 line
drivers/clk/qcom/clk-alpha-pll.c +1 -0
+       regmap_write(pll->clkr.regmap, PLL_MODE(pll), 0);

There's a theoretical problem here if I understand correctly. A call to clk_enable() can happen while clk_set_rate() is in progress or vice versa. Probably we need some sort of spinlock for this PLL that synchronizes any enable/disable with the rate change so that when we restore the enable bit the clk isn't enabled when it was supposed to be off.

Quoted 3 lines

Since the PLL is always on, should we worry about enable/disable? If you feel it is better to synchronize with a spin lock, will add and post a new revision. Please let me know.

Probably another option might be to change stromer PLL ops to use prepare/unprepare instead of enable/disable. This way the clk_prepare_lock() in clk_set_rate() will take care of locking.

Quoted 3 lines

Thanks
Varada

Signature · 2 lines
With best wishes
Dmitry
#5 lore raw

Re: [PATCH v2 1/8] clk: qcom: clk-alpha-pll: introduce stromer plus ops

Quoted 58 lines · Dmitry Baryshkov
Quoted 56 lines · Varadarajan Narayanan
Quoted 7 lines · Stephen Boyd

Quoting Varadarajan Narayanan (2023-10-12 02:26:17)

Quoted 4 lines

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.

Does this assume the PLL is always on?

Yes once the PLL is configured by apss-ipq-pll driver, it is always on.

Quoted 43 lines
Quoted 26 lines

---
v2: Use clk_alpha_pll_stromer_determine_rate, instead of adding new
clk_alpha_pll_stromer_plus_determine_rate as the alpha pll width is same for both

Fix review comments
udelay(50) -> usleep_range(50, 60) Remove SoC-specific from print message

---
 drivers/clk/qcom/clk-alpha-pll.c | 57 ++++++++++++++++++++++++++++++++++++++++
 drivers/clk/qcom/clk-alpha-pll.h |  1 +
 2 files changed, 58 insertions(+)
drivers/clk/qcom/clk-alpha-pll.c +3 -0
index 4edbf77..5221b6c 100644
@@ -2508,3 +2508,60 @@ 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_set_rate(struct clk_hw *hw,
+                                              unsigned long rate,

drivers/clk/qcom/clk-alpha-pll.c const struct clk_ops clk_alpha_pll_stromer_ops = { · line 2523

Quoted 10 lines
drivers/clk/qcom/clk-alpha-pll.c +10 -0
+                                              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);

There's a theoretical problem here if I understand correctly. A call to clk_enable() can happen while clk_set_rate() is in progress or vice versa. Probably we need some sort of spinlock for this PLL that synchronizes any enable/disable with the rate change so that when we restore the enable bit the clk isn't enabled when it was supposed to be off.

Quoted 7 lines
Quoted 3 lines

Since the PLL is always on, should we worry about enable/disable? If you feel it is better to synchronize with a spin lock, will add and post a new revision. Please let me know.

Probably another option might be to change stromer PLL ops to use prepare/unprepare instead of enable/disable. This way the clk_prepare_lock() in clk_set_rate() will take care of locking.

Thanks for the suggestion. Have posted v3 with this and addressing Stephen Boyd's other comments. Please take a look.
(https://lore.kernel.org/linux-arm-msm/[email protected]/)

Thanks
Varada

#6 lore raw

PATCH v2 2/8 clk: qcom: apss-ipq-pll: Use stromer plus ops for stromer plus pll

In reply to 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.

Fixes c7ef7fbb1ccf ("clk: qcom: apss-ipq-pll: add support for IPQ5332")

---
v2: Add Fixes tag

---
 drivers/clk/qcom/apss-ipq-pll.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/clk/qcom/apss-ipq-pll.c +1 -1
index e170331..18c4ffe 100644
@@ -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,
 		},
 	},
 };
Signature · 1 line
2.7.4
#7 lore raw

PATCH v2 3/8 clk: qcom: apss-ipq-pll: Fix 'l' value for ipq5332_pll_config

In reply to 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")
---
 drivers/clk/qcom/apss-ipq-pll.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/clk/qcom/apss-ipq-pll.c +1 -1
index 18c4ffe..41279e5 100644
@@ -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),
Signature · 1 line
2.7.4
#8 lore raw

PATCH v2 4/8 clk: qcom: apss-ipq6018: ipq5332: add safe source switch for a53pll

In reply to 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.

Additionally, not all SKUs of IPQ53xx support scaling. Hence, do the above to the SKUs that support scaling.

---
v2: Handle ABORT_RATE_CHANGE
Use local variable for apcs_alias0_clk_src.clkr.hw Use single line comment instead of multi line style

---
 drivers/clk/qcom/apss-ipq6018.c | 53 ++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 52 insertions(+), 1 deletion(-)
drivers/clk/qcom/apss-ipq6018.c +52 -1
index 4e13a08..c05c2b2 100644
@@ -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,63 @@ 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)
+{
+	struct clk_hw *hw;
+	u8 index;
+	int err;
+
+	if (action == PRE_RATE_CHANGE)
+		index = P_GPLL0;
+	else if ((action == POST_RATE_CHANGE) || (action == ABORT_RATE_CHANGE))
+		index = P_APSS_PLL_EARLY;
+	else
+		return 0;
+
+	hw = &apcs_alias0_clk_src.clkr.hw;
+	err = clk_rcg2_mux_closest_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 */
+	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 = {
Signature · 1 line
2.7.4
maintainer
#9 lore raw

Re: [PATCH v2 4/8] clk: qcom: apss-ipq6018: ipq5332: add safe source switch for a53pll

Quoting Varadarajan Narayanan (2023-10-12 02:26:20)

Quoted 8 lines
drivers/clk/qcom/apss-ipq6018.c +0 -0
index 4e13a08..c05c2b2 100644
@@ -84,15 +87,63 @@ static const struct qcom_cc_desc apss_ipq6018_desc = {
        .num_clks = ARRAY_SIZE(apss_ipq6018_clks),
 };
 

drivers/clk/qcom/apss-ipq6018.c static const struct qcom_cc_desc apss_ipq6018_desc = { · line 99

Quoted 10 lines
drivers/clk/qcom/apss-ipq6018.c +10 -0
+static int cpu_clk_notifier_fn(struct notifier_block *nb, unsigned long action,
+                               void *data)
+{
+       struct clk_hw *hw;
+       u8 index;
+       int err;
+
+       if (action == PRE_RATE_CHANGE)
+               index = P_GPLL0;
+       else if ((action == POST_RATE_CHANGE) || (action == ABORT_RATE_CHANGE))

This has too many parenthesis.

drivers/clk/qcom/apss-ipq6018.c static const struct qcom_cc_desc apss_ipq6018_desc = { · line 102

Quoted 3 lines
drivers/clk/qcom/apss-ipq6018.c +3 -0
+               index = P_APSS_PLL_EARLY;
+       else
+               return 0;

Maybe 'return NOTIFY_OK' instead?

drivers/clk/qcom/apss-ipq6018.c static const struct qcom_cc_desc apss_ipq6018_desc = { · line 110

Quoted 8 lines
drivers/clk/qcom/apss-ipq6018.c +8 -0
+
+       hw = &apcs_alias0_clk_src.clkr.hw;
+       err = clk_rcg2_mux_closest_ops.set_parent(hw, index);
+
+       return notifier_from_errno(err);
+}
+
+static struct notifier_block cpu_clk_notifier = {

Instead of a static global can this be allocated with kzalloc?

Quoted 4 lines
drivers/clk/qcom/apss-ipq6018.c +3 -0
+       .notifier_call = cpu_clk_notifier_fn,
+};
+
 static int apss_ipq6018_probe(struct platform_device *pdev)
#10 lore raw

Re: [PATCH v2 4/8] clk: qcom: apss-ipq6018: ipq5332: add safe source switch for a53pll

In reply to Varadarajan Narayanan
:::::: :::::: Manual check reason: "git am base is a link in commit message" ::::::

::::::
:::::: Manual check reason: "git am base is a link in commit message" ::::::

BCC: [email protected]
CC: [email protected]
In-Reply-To: <000c61a028814f08a9fc6d1d5c446e8dad11a650.1697101543.git.quic_varada@quicinc.com>
References: <000c61a028814f08a9fc6d1d5c446e8dad11a650.1697101543.git.quic_varada@quicinc.com>
TO: Varadarajan Narayanan <[email protected]>
TO: [email protected]
TO: [email protected]
TO: [email protected]
TO: [email protected]
TO: [email protected]
TO: [email protected]
TO: [email protected]
TO: [email protected]
TO: [email protected]
TO: [email protected]
TO: [email protected]
TO: [email protected]
TO: [email protected]
TO: [email protected]
TO: [email protected]
TO: [email protected]
TO: [email protected]
CC: Varadarajan Narayanan <[email protected]>

Hi Varadarajan,

kernel test robot noticed the following build errors:

[auto build test ERROR on linus/master]
[also build test ERROR on v6.6-rc6 next-20231018]
[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/20231017-104355
base:   linus/master
patch link:    https://lore.kernel.org/r/000c61a028814f08a9fc6d1d5c446e8dad11a650.1697101543.git.quic_varada%40quicinc.com
patch subject: [PATCH v2 4/8] clk: qcom: apss-ipq6018: ipq5332: add safe source switch for a53pll

:::::: branch date: 29 hours ago
:::::: commit date: 29 hours ago
config: loongarch-randconfig-001-20231018 (https://download.01.org/0day-ci/archive/20231018/[email protected]/config)
compiler: loongarch64-linux-gcc (GCC) 13.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20231018/[email protected]/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 <[email protected]>
| Closes: https://lore.kernel.org/r/[email protected]/

All errors (new ones prefixed by >>):

loongarch64-linux-ld: drivers/clk/qcom/apss-ipq6018.o: in function `apss_ipq6018_probe':

>> apss-ipq6018.c:(.text+0xd0): undefined reference to `qcom_smem_get_soc_id'
Signature · 2 lines
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
#11 lore raw

PATCH v2 5/8 cpufreq: qti: Enable cpufreq for ipq53xx

In reply to 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.

---
v2: Move IPQ53xx after APQ8096SG

---
 drivers/cpufreq/cpufreq-dt-platdev.c | 1 +
 drivers/cpufreq/qcom-cpufreq-nvmem.c | 8 ++++++++
 2 files changed, 9 insertions(+)
drivers/cpufreq/cpufreq-dt-platdev.c +1 -0
index 2016d47..5804063 100644
@@ -179,6 +179,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", },
drivers/cpufreq/qcom-cpufreq-nvmem.c +8 -0
index 84d7033..520b79a 100644
@@ -154,6 +154,13 @@ 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_IPQ5332:
+	case QCOM_ID_IPQ5322:
+	case QCOM_ID_IPQ5312:
+	case QCOM_ID_IPQ5302:
+	case QCOM_ID_IPQ5300:
+		drv->versions = 1 << (unsigned int)(*speedbin);
+		break;
 	default:
 		BUG();
 		break;
@@ -359,6 +366,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 },
Signature · 1 line
2.7.4
#12 lore raw

PATCH v2 6/8 arm64: dts: qcom: ipq5332: populate the opp table based on the eFuse

In reply to 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.

	------------------------------------------------
	Frequency	BIT2	BIT1	opp-supported-hw
			1.1GHz	1.5GHz
	------------------------------------------------
	1100000000	1	1	0xf
	1500000000	0	1	0x3
	------------------------------------------------

---
v2: Fix inconsistencies in comment and move it to commit log
as suggested
Remove opp-microvolt entries as no regulator is managed by Linux cpu_speed_bin -> cpu-speed-bin in node name
Remove "nvmem-cell-names" due to dtbs_check error

---
 arch/arm64/boot/dts/qcom/ipq5332.dtsi | 19 ++++++++++++++++---
 1 file changed, 16 insertions(+), 3 deletions(-)
arch/arm64/boot/dts/qcom/ipq5332.dtsi +16 -3
index 4206f05..a0dcba3 100644
@@ -91,11 +91,19 @@
 	};
 
 	cpu_opp_table: opp-table-cpu {
-		compatible = "operating-points-v2";
+		compatible = "operating-points-v2-kryo-cpu";
 		opp-shared;
+		nvmem-cells = <&cpu_speed_bin>;
 
-		opp-1488000000 {
-			opp-hz = /bits/ 64 <1488000000>;
+		opp-1100000000 {
+			opp-hz = /bits/ 64 <1100000000>;
+			opp-supported-hw = <0xF>;
+			clock-latency-ns = <200000>;
+		};
+
+		opp-1500000000 {
+			opp-hz = /bits/ 64 <1500000000>;
+			opp-supported-hw = <0x3>;
 			clock-latency-ns = <200000>;
 		};
 	};
@@ -163,6 +171,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 {
Signature · 1 line
2.7.4
#13 lore raw

PATCH v2 7/8 cpufreq: qti: Introduce cpufreq for ipq95xx

In reply to 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.

---
v2: Simplify bin selection by tweaking the order in dts

---
 drivers/cpufreq/cpufreq-dt-platdev.c | 1 +
 drivers/cpufreq/qcom-cpufreq-nvmem.c | 8 ++++++++
 2 files changed, 9 insertions(+)
drivers/cpufreq/cpufreq-dt-platdev.c +1 -0
index 5804063..16b763f 100644
@@ -181,6 +181,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", },
drivers/cpufreq/qcom-cpufreq-nvmem.c +8 -0
index 520b79a..4768d07 100644
@@ -161,6 +161,13 @@ static int qcom_cpufreq_kryo_name_version(struct device *cpu_dev,
 	case QCOM_ID_IPQ5300:
 		drv->versions = 1 << (unsigned int)(*speedbin);
 		break;
+	case QCOM_ID_IPQ9514:
+	case QCOM_ID_IPQ9550:
+	case QCOM_ID_IPQ9554:
+	case QCOM_ID_IPQ9570:
+	case QCOM_ID_IPQ9574:
+		drv->versions = 1 << (unsigned int)(*speedbin);
+		break;
 	default:
 		BUG();
 		break;
@@ -368,6 +375,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 },
Signature · 1 line
2.7.4
#14 lore raw

PATCH v2 8/8 arm64: dts: qcom: ipq9574: populate the opp table based on the eFuse

In reply to 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.

Frequency 1.2GHz 1.8GHz 1.5GHz No opp-supported-hw
Limit
------------------------------------------------------------
936000000 1 1 1 1 0xf
1104000000 1 1 1 1 0xf
1200000000 1 1 1 1 0xf
1416000000 0 1 1 1 0x7
1488000000 0 1 1 1 0x7
1800000000 0 1 0 1 0x5
2208000000 0 0 0 1 0x1
-----------------------------------------------------------

---
v2: cpu_speed_bin -> cpu-speed-bin in node name
Move comment to commit log

---
 arch/arm64/boot/dts/qcom/ipq9574.dtsi | 21 ++++++++++++++++++++-
 1 file changed, 20 insertions(+), 1 deletion(-)
arch/arm64/boot/dts/qcom/ipq9574.dtsi +20 -1
index cc84f25..5f83ee4 100644
@@ -106,42 +106,56 @@
 	};
 
 	cpu_opp_table: opp-table-cpu {
-		compatible = "operating-points-v2";
+		compatible = "operating-points-v2-kryo-cpu";
 		opp-shared;
+		nvmem-cells = <&cpu_speed_bin>;
 
 		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 = <0xf>;
 			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 = <0x5>;
 			clock-latency-ns = <200000>;
 		};
 
 		opp-2208000000 {
 			opp-hz = /bits/ 64 <2208000000>;
 			opp-microvolt = <1062500>;
+			opp-supported-hw = <0x1>;
 			clock-latency-ns = <200000>;
 		};
 	};
@@ -223,6 +237,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 {
Signature · 1 line
2.7.4