kern/60671: cpufreq_dt ignores min/max voltage ranges in opp-microvolt, breaking DVFS

["stefano%dragas.it@localhost via gnats" <gnats-admin%NetBSD.org@localhost>]

>Number:         60671
>Category:       kern
>Synopsis:       cpufreq_dt ignores min/max voltage ranges in opp-microvolt, breaking DVFS
>Confidential:   no
>Severity:       serious
>Priority:       medium
>Responsible:    kern-bug-people
>State:          open
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Mon Aug 31 06:15:00 +0000 2026
>Originator:     Stefano Marinelli
>Release:        NetBSD 11.0_STABLE / netbsd-11
>Organization:
>Environment:
NetBSD opionenb 11.0_STABLE NetBSD 11.0_STABLE (GENERIC) #0: Tue Aug 25 20:15:20 UTC 2026  mkrepro%mkrepro.NetBSD.org@localhost:/usr/src/sys/arch/evbarm/compile/GENERIC evbarm
>Description:
The FDT cpufreq driver only stores the first value of the
"opp-microvolt" property when parsing operating-points-v2.

An OPP may specify three values:

opp-microvolt = <target min max>;

However, cpufreq_dt currently only stores opp_uv[0], and later requests
that exact voltage from the regulator using:

fdtbus_regulator_set_voltage(sc->sc_supply, new_uv, new_uv);

This causes CPU frequency transitions to fail when the regulator does not
provide the exact target voltage, but does provide a valid voltage within
the range specified by the OPP.

This can be reproduced on the Allwinner H3 Orange Pi boards.

For example, the available CPU frequencies are:

machdep.cpufreq.cpu0.available = 1008 816 648

On these boards the OPP table contains voltage ranges, while the GPIO
regulator provides a limited set of discrete voltages.

As a consequence, with the unmodified kernel:

# sysctl -w machdep.cpufreq.cpu0.target=648
sysctl: machdep.cpufreq.cpu0.target: Invalid argument

Interestingly, the frequency may already have changed before the
regulator operation fails:

# sysctl machdep.cpufreq.cpu0.current
machdep.cpufreq.cpu0.current = 648

Going upwards can fail before the clock transition:

# sysctl -w machdep.cpufreq.cpu0.target=816
machdep.cpufreq.cpu0.target: 648 -> 816

# sysctl -w machdep.cpufreq.cpu0.target=1008
sysctl: machdep.cpufreq.cpu0.target: Invalid argument

# sysctl machdep.cpufreq.cpu0.current
machdep.cpufreq.cpu0.current = 816

This also breaks estd. estd correctly selects a different CPU frequency,
but when the sysctl operation returns an error it exits with:

estd: Cannot set CPU frequency (maybe you aren't root?)

The problem appears to be that cpufreq_dt loses the min/max voltage
information supplied by operating-points-v2.
>How-To-Repeat:
On an Orange Pi One running NetBSD 11_STABLE:

# sysctl machdep.cpufreq
machdep.cpufreq.cpu0.available = 1008 816 648

Stop estd if it is running (it will crash anyway), then try:

# sysctl -w machdep.cpufreq.cpu0.target=816
# sysctl -w machdep.cpufreq.cpu0.target=648
# sysctl -w machdep.cpufreq.cpu0.target=816
# sysctl -w machdep.cpufreq.cpu0.target=1008

Some transitions return EINVAL.

Running estd and generating CPU load also reproduces the problem, as
estd exits when one of these transitions returns an error.
>Fix:
Modify cpufreq_dt so that it preserves the target, minimum and maximum
values from:

opp-microvolt = <target min max>;

and pass the valid voltage range to fdtbus_regulator_set_voltage()
instead of requiring the exact target voltage.

For legacy OPP tables containing only one voltage, min and max remain
equal to the target voltage, preserving the existing behaviour.

With this change applied to netbsd-11, I rebuilt a GENERIC
evbarm/earmv7hf kernel and tested it on an Orange Pi One.

All transitions now complete successfully:

1008 -> 816
 816 -> 648
 648 -> 816
 816 -> 1008

All sysctl operations return success, and estd can dynamically scale
between 648, 816 and 1008 MHz without terminating.

Patch follows:

diff --git a/sys/dev/fdt/cpufreq_dt.c b/sys/dev/fdt/cpufreq_dt.c
index d80eb90a5..741db01da 100644
--- a/sys/dev/fdt/cpufreq_dt.c
+++ b/sys/dev/fdt/cpufreq_dt.c
@@ -55,6 +55,8 @@ static kmutex_t cpufreq_dt_tables_lock;
 struct cpufreq_dt_opp {
 	u_int			freq_khz;
 	u_int			voltage_uv;
+	u_int			voltage_min_uv;
+	u_int			voltage_max_uv;
 	u_int			latency_ns;
 };

@@ -94,6 +96,7 @@ cpufreq_dt_set_rate(struct cpufreq_dt_softc *sc, u_int freq_khz)
 {
 	struct cpufreq_dt_opp *opp = NULL;
 	u_int old_rate, new_rate, old_uv, new_uv;
+	u_int new_min_uv, new_max_uv;
 	uint64_t xc;
 	int error;
 	ssize_t n;
@@ -109,6 +112,8 @@ cpufreq_dt_set_rate(struct cpufreq_dt_softc *sc, u_int freq_khz)
 	old_rate = clk_get_rate(sc->sc_clk);
 	new_rate = freq_khz * 1000;
 	new_uv = opp->voltage_uv;
+	new_min_uv = opp->voltage_min_uv;
+	new_max_uv = opp->voltage_max_uv;

 	if (old_rate == new_rate)
 		return 0;
@@ -120,7 +125,7 @@ cpufreq_dt_set_rate(struct cpufreq_dt_softc *sc, u_int freq_khz)

 		if (new_uv > old_uv) {
 			error = fdtbus_regulator_set_voltage(sc->sc_supply,
-			    new_uv, new_uv);
+			    new_min_uv, new_max_uv);
 			if (error != 0)
 				return error;
 		}
@@ -137,7 +142,7 @@ cpufreq_dt_set_rate(struct cpufreq_dt_softc *sc, u_int freq_khz)
 	if (sc->sc_supply != NULL) {
 		if (new_uv < old_uv) {
 			error = fdtbus_regulator_set_voltage(sc->sc_supply,
-			    new_uv, new_uv);
+			    new_min_uv, new_max_uv);
 			if (error != 0)
 				return error;
 		}
@@ -351,6 +356,10 @@ cpufreq_dt_parse_opp(struct cpufreq_dt_softc *sc)
 	for (i = 0; i < sc->sc_nopp; i++, opp += 2) {
 		sc->sc_opp[i].freq_khz = be32toh(opp[0]);
 		sc->sc_opp[i].voltage_uv = be32toh(opp[1]);
+		sc->sc_opp[i].voltage_min_uv =
+		    sc->sc_opp[i].voltage_uv;
+		sc->sc_opp[i].voltage_max_uv =
+		    sc->sc_opp[i].voltage_uv;
 	}

 	return 0;
@@ -444,11 +453,22 @@ cpufreq_dt_parse_opp_v2(struct cpufreq_dt_softc *sc)
 		if (of_getprop_uint64(opp_node, "opp-hz", &opp_hz) != 0)
 			return EINVAL;
 		opp_uv = fdtbus_get_prop(opp_node, "opp-microvolt", &len);
-		if (opp_uv == NULL || len < 1)
+		if (opp_uv == NULL || len < sizeof(*opp_uv))
 			return EINVAL;
 		/* Table is in reverse order */
 		sc->sc_opp[index].freq_khz = (u_int)(opp_hz / 1000);
 		sc->sc_opp[index].voltage_uv = be32toh(opp_uv[0]);
+		if (len >= 3 * sizeof(*opp_uv)) {
+			sc->sc_opp[index].voltage_min_uv =
+			    be32toh(opp_uv[1]);
+			sc->sc_opp[index].voltage_max_uv =
+			    be32toh(opp_uv[2]);
+		} else {
+			sc->sc_opp[index].voltage_min_uv =
+			    sc->sc_opp[index].voltage_uv;
+			sc->sc_opp[index].voltage_max_uv =
+			    sc->sc_opp[index].voltage_uv;
+		}
 		of_getprop_uint32(opp_node, "clock-latency-ns", &sc->sc_opp[index].latency_ns);
 		--index;
 	}


Tested-by: Stefano Marinelli <stefano%dragas.it@localhost>