mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH] cpufreq: dt: Set the initial OPP to apply its voltage constraints
@ 2026-11-11 13:04 Natanael Copa
  2026-11-12 12:54 ` Zhongqiu Han
  0 siblings, 1 reply; 2+ messages in thread
From: Natanael Copa @ 2026-11-11 13:04 UTC (permalink / raw)
  To: Rafael J . Wysocki, Viresh Kumar
  Cc: Nishanth Menon, Stephen Boyd, Milan P . Stanić,
	Natanael Copa, linux-pm, linux-kernel, Yixun Lan, Troy Mitchell,
	Shuwei Wu, Aurelien Jarno, spacemit, linux-riscv, stable

The cpufreq core skips requests for the current frequency, so a policy
has no voltage constraint on its supply until its first frequency
change. Another policy sharing the supply can then lower the voltage
too far.

On the SpacemiT K1, both clusters share one supply and boot at 1.6 GHz.
When policy0 scales down, the supply drops to 0.95 V while policy4
still runs at 1.6 GHz, causing random crashes during boot.

Set the OPP for the current clock rate of all CPUs before registering
the driver.

Fixes: 3b9c34556fc1 ("riscv: dts: spacemit: Add cpu scaling for K1 SoC")
Cc: stable@vger.kernel.org
Assisted-by: Claude:claude-opus-5-5
Assisted-by: Claude:claude-fable-5-2
Signed-off-by: Natanael Copa <ncopa@alpinelinux.org>
---
Reproducer, booted with cpufreq.default_governor=performance:

  # echo 614400 > /sys/devices/system/cpu/cpufreq/policy0/scaling_max_freq

buck1 drops to 950000 uV while policy4 is still at 1600000 kHz, which
crashed my board within seconds.

Tested on a Banana Pi BPI-F3 with v7.3.3 and v7.4-rc2. Without the
patch, v7.3.3 crashed on every boot with schedutil (three). With the
patch, the reproducer keeps buck1 at 1050000 uV, and five boots with
schedutil were clean on each kernel. Not tested on other SoCs.

Other OPP users that never set their current OPP have the same gap. If
you would rather fix this in the OPP core, I can rework it.

Found and fixed with the help of an AI assistant (Claude). I reviewed
and tested the result.

 drivers/cpufreq/cpufreq-dt.c | 26 ++++++++++++++++++++++++++
 1 file changed, 26 insertions(+)

diff --git a/drivers/cpufreq/cpufreq-dt.c b/drivers/cpufreq/cpufreq-dt.c
index eb15650f99bf..2646dda10aac 100644
--- a/drivers/cpufreq/cpufreq-dt.c
+++ b/drivers/cpufreq/cpufreq-dt.c
@@ -153,6 +153,30 @@ static struct cpufreq_driver dt_cpufreq_driver = {
 	.suspend = cpufreq_generic_suspend,
 };
 
+/*
+ * Set the OPP for the current clock rate, so that its voltage constraints
+ * apply before any policy can change frequency. Otherwise another policy
+ * sharing the supply could lower the voltage below what this one needs.
+ */
+static void dt_cpufreq_set_initial_opp(struct device *cpu_dev)
+{
+	struct clk *clk = clk_get(cpu_dev, NULL);
+	unsigned long rate;
+	int ret;
+
+	if (IS_ERR(clk))
+		return;
+
+	rate = clk_get_rate(clk);
+	clk_put(clk);
+	if (!rate)
+		return;
+
+	ret = dev_pm_opp_set_rate(cpu_dev, rate);
+	if (ret)
+		dev_warn(cpu_dev, "failed to set initial OPP: %d\n", ret);
+}
+
 static int dt_cpufreq_early_init(struct device *dev, int cpu)
 {
 	struct private_data *priv;
@@ -251,6 +275,8 @@ static int dt_cpufreq_early_init(struct device *dev, int cpu)
 		goto out;
 	}
 
+	dt_cpufreq_set_initial_opp(cpu_dev);
+
 	list_add(&priv->node, &priv_list);
 	return 0;
 

base-commit: 067c5c24cef23d913b67eaaf71ae67e7169a6620
-- 
2.56.1


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

* Re: [PATCH] cpufreq: dt: Set the initial OPP to apply its voltage constraints
  2026-11-11 13:04 [PATCH] cpufreq: dt: Set the initial OPP to apply its voltage constraints Natanael Copa
@ 2026-11-12 12:54 ` Zhongqiu Han
  0 siblings, 0 replies; 2+ messages in thread
From: Zhongqiu Han @ 2026-11-12 12:54 UTC (permalink / raw)
  To: Natanael Copa, Rafael J . Wysocki, Viresh Kumar
  Cc: Nishanth Menon, Stephen Boyd, Milan P . Stanić,
	linux-pm, linux-kernel, Yixun Lan, Troy Mitchell, Shuwei Wu,
	Aurelien Jarno, spacemit, linux-riscv, stable, Zhongqiu Han

Hi Natanael,
Thanks for the patch.

On 11/11/2026 9:04 PM, Natanael Copa wrote:
> The cpufreq core skips requests for the current frequency, so a policy
> has no voltage constraint on its supply until its first frequency
> change. Another policy sharing the supply can then lower the voltage
> too far.
> 
> On the SpacemiT K1, both clusters share one supply and boot at 1.6 GHz.
> When policy0 scales down, the supply drops to 0.95 V while policy4
> still runs at 1.6 GHz, causing random crashes during boot.
> 
> Set the OPP for the current clock rate of all CPUs before registering
> the driver.
> 
> Fixes: 3b9c34556fc1 ("riscv: dts: spacemit: Add cpu scaling for K1 SoC")
> Cc: stable@vger.kernel.org
> Assisted-by: Claude:claude-opus-5-5
> Assisted-by: Claude:claude-fable-5-2
> Signed-off-by: Natanael Copa <ncopa@alpinelinux.org>
> ---
> Reproducer, booted with cpufreq.default_governor=performance:
> 
>    # echo 614400 > /sys/devices/system/cpu/cpufreq/policy0/scaling_max_freq
> 
> buck1 drops to 950000 uV while policy4 is still at 1600000 kHz, which
> crashed my board within seconds.
> 
> Tested on a Banana Pi BPI-F3 with v7.3.3 and v7.4-rc2. Without the
> patch, v7.3.3 crashed on every boot with schedutil (three). With the
> patch, the reproducer keeps buck1 at 1050000 uV, and five boots with
> schedutil were clean on each kernel. Not tested on other SoCs.
> 
> Other OPP users that never set their current OPP have the same gap. If
> you would rather fix this in the OPP core, I can rework it.

Which other users do you have in mind? Apart from K1 I couldn't find
another in-tree cpufreq-dt platform where several policies share one
cpu-supply. MediaTek CPU/CCI has the similar issue, and
mtk_cpufreq_set_target() handles it by not going below the boot
voltage until the CCI driver is ready, so I'd keep this in cpufreq-dt.
But I'll defer to Viresh on this.

> 
> Found and fixed with the help of an AI assistant (Claude). I reviewed
> and tested the result.
> 
>   drivers/cpufreq/cpufreq-dt.c | 26 ++++++++++++++++++++++++++
>   1 file changed, 26 insertions(+)
> 
> diff --git a/drivers/cpufreq/cpufreq-dt.c b/drivers/cpufreq/cpufreq-dt.c
> index eb15650f99bf..2646dda10aac 100644
> --- a/drivers/cpufreq/cpufreq-dt.c
> +++ b/drivers/cpufreq/cpufreq-dt.c
> @@ -153,6 +153,30 @@ static struct cpufreq_driver dt_cpufreq_driver = {
>   	.suspend = cpufreq_generic_suspend,
>   };
>   
> +/*
> + * Set the OPP for the current clock rate, so that its voltage constraints
> + * apply before any policy can change frequency. Otherwise another policy
> + * sharing the supply could lower the voltage below what this one needs.
> + */
> +static void dt_cpufreq_set_initial_opp(struct device *cpu_dev)
> +{
> +	struct clk *clk = clk_get(cpu_dev, NULL);
> +	unsigned long rate;
> +	int ret;
> +
> +	if (IS_ERR(clk))
> +		return;
> +
> +	rate = clk_get_rate(clk);
> +	clk_put(clk);
> +	if (!rate)
> +		return;
> +
> +	ret = dev_pm_opp_set_rate(cpu_dev, rate);
> +	if (ret)
> +		dev_warn(cpu_dev, "failed to set initial OPP: %d\n", ret);
> +}
> +
>   static int dt_cpufreq_early_init(struct device *dev, int cpu)
>   {
>   	struct private_data *priv;
> @@ -251,6 +275,8 @@ static int dt_cpufreq_early_init(struct device *dev, int cpu)
>   		goto out;
>   	}
>   
> +	dt_cpufreq_set_initial_opp(cpu_dev);

Does this need to apply to all cpufreq-dt platforms? Could we detect
this dynamically by checking whether the policies share the same cpu
supply, and only do it then?


> +
>   	list_add(&priv->node, &priv_list);
>   	return 0;
>   
> 
> base-commit: 067c5c24cef23d913b67eaaf71ae67e7169a6620


Besides, Sashiko reported some issues, would you please have a look?

https://sashiko.dev/#/patchset/20261111130430.10359-1-ncopa%40alpinelinux.org


-- 
Thx and BRs,
Zhongqiu Han

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

end of thread, other threads:[~2026-11-12 12:55 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-11-11 13:04 [PATCH] cpufreq: dt: Set the initial OPP to apply its voltage constraints Natanael Copa
2026-11-12 12:54 ` Zhongqiu Han

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

all inboxes | Powered by JetHome®