Re: [PATCH v3 1/2] cpufreq: spacemit: Add K1 cpufreq driver

Hi Viresh,

Thanks for pointing it out.

On Mon Jun 15, 2026 at 2:34 PM CST, Viresh Kumar wrote:
> Hi Shuwei,
>
> On 15-06-26, 14:12, Shuwei Wu wrote:
>> The clusters have separate clocks, but they share the same voltage supply.[1]
>> So two independent policies would be unsafe: one policy could lower the shared
>> voltage while the other cluster is still running at a higher frequency.
>
> No, both will vote for the regulator contraints using CPU device and the
> regulator core will make sure it doesn't break any of them. This is what all
> frameworks do, regulator, clk, etc.
>
>> This means they can't use different policies.
>
> This is incorrect.
>
>> From a hardware perspective, the eight cores of the K1 are homogeneous,
>> so using the same policy for them is relatively reasonable.
>
> Right, but this is inefficient. One cluster can be idle, or in low freq mode
> while other can be in higher. They MUST be two policies.

Yes, you're right. Two policies are not inherently unsafe.
The regulator framework handles multi-consumer voting correctly.

The actual problem was using single value opp-microvolt. With one cluster at
a low frequency holding a precise [950000, 950000] on the shared rail, and the
other requesting [1050000, 1050000] for 1.6 GHz, the regulator finds no
intersection:

~ # echo 1600000 > /sys/devices/system/cpu/cpu0/cpufreq/scaling_setspeed
[  544.744195] buck1: Restricting voltage, 1050000-950000uV
[  544.757073] cpu cpu0: _set_opp_voltage: failed to set
              voltage (1050000 1050000 1050000 mV): -22
[  544.771304] cpufreq: __target_index: Failed to change
              cpu frequency: -22

The fix is to use the <target min max> triplet for the lower OPPs so the
regulator always has a valid intersection:

-    cpu_opp_table: opp-table-cpu {
+    cluster0_opp_table: opp-table-cluster0 {
         compatible = "operating-points-v2";
         opp-shared;
         opp-614400000 {
-            opp-microvolt = <950000>;
+            opp-microvolt = <950000 950000 1050000>;
         };
         opp-819000000 {
-            opp-microvolt = <950000>;
+            opp-microvolt = <950000 950000 1050000>;
         };
         ...
         opp-1600000000 {
             opp-microvolt = <1050000>;
         };
     };
+    cluster1_opp_table: opp-table-cluster1 {
+        // same OPP entries and voltage ranges as above
+    };
-    &cpu_0 { operating-points-v2 = <&cpu_opp_table>; };
+    &cpu_0 { operating-points-v2 = <&cluster0_opp_table>; };
     ...
-    &cpu_7 { operating-points-v2 = <&cpu_opp_table>; };
+    &cpu_3 { operating-points-v2 = <&cluster0_opp_table>; };
+    &cpu_4 { operating-points-v2 = <&cluster1_opp_table>; };
     ...
+    &cpu_7 { operating-points-v2 = <&cluster1_opp_table>; };

This way the low frequency cluster accepts up to 1.05 V on the rail.
That is safe: high voltage at low frequency costs power but does not cause
instability.

With two cpufreq-dt policies (one per cluster) and these ranges, neither cluster
blocks the other. Tested on BPI-F3 and OrangePi Rv2 boards, works as expected.

Does this look good to you, or would you prefer a different approach?
>
>> I used a K1-specific driver because cpufreq-dt only manages one CPU clock
>> through the CPU device used for the OPP transition.
>> On K1, the policy needs to control two independent cluster clocks and one shared
>> regulator, so the driver has to update the second cluster clock explicitly and
>> keep the ordering safe: raise voltage before raising either cluster, and lower
>> both cluster clocks before lowering the shared voltage.
>> 
>> [1] https://lore.kernel.org/spacemit/[email protected]/

-- 
Best regards,
Shuwei Wu