Re: [PATCH 1/4] arm64: dts: rockchip: list all CPU supplies on ArmSoM Sige5

On Wednesday, 4 June 2025 21:12:35 Central European Summer Time Alexey Charkov wrote:

On Wed, Jun 4, 2025 at 10:38 PM Nicolas Frattaroli
[off-list ref] wrote:

quoted

On Tuesday, 3 June 2025 19:01:13 Central European Summer Time Alexey Charkov wrote:

quoted

List both CPU supply regulators which drive the little and big CPU
clusters, respectively, so that cpufreq can pick them up.

Signed-off-by: Alexey Charkov <alchark@gmail.com>
---
 .../boot/dts/rockchip/rk3576-armsom-sige5.dts      | 28 ++++++++++++++++++++++
 1 file changed, 28 insertions(+)

diff --git a/arch/arm64/boot/dts/rockchip/rk3576-armsom-sige5.dts b/arch/arm64/boot/dts/rockchip/rk3576-armsom-sige5.dts
index b09e789c75c47fec7cf7e9810ab0dcca32d9404a..d9c129be55a0d997e04e6d677cdc98fb50353418 100644
--- a/arch/arm64/boot/dts/rockchip/rk3576-armsom-sige5.dts
+++ b/arch/arm64/boot/dts/rockchip/rk3576-armsom-sige5.dts

@@ -207,6 +207,22 @@ vcc_3v3_ufs_s0: regulator-vcc-ufs-s0 {
      };
 };

+&cpu_b0 {
+     cpu-supply = <&vdd_cpu_big_s0>;
+};
+
+&cpu_b1 {
+     cpu-supply = <&vdd_cpu_big_s0>;
+};
+
+&cpu_b2 {
+     cpu-supply = <&vdd_cpu_big_s0>;
+};
+
+&cpu_b3 {
+     cpu-supply = <&vdd_cpu_big_s0>;
+};
+
 &combphy0_ps {
      status = "okay";
 };

@@ -215,6 +231,18 @@ &cpu_l0 {
      cpu-supply = <&vdd_cpu_lit_s0>;
 };

+&cpu_l1 {
+     cpu-supply = <&vdd_cpu_lit_s0>;
+};
+
+&cpu_l2 {
+     cpu-supply = <&vdd_cpu_lit_s0>;
+};
+
+&cpu_l3 {
+     cpu-supply = <&vdd_cpu_lit_s0>;
+};
+
 &gmac0 {
      phy-mode = "rgmii-id";
      clock_in_out = "output";

Reviewed-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
Tested-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>

quoted

so that cpufreq can pick them up.

Fwiw, even without this patch they're picked up by cpufreq-dt for me:

    user@trixie:~$ sudo cpupower frequency-info
      analyzing CPU 5:
      driver: cpufreq-dt
      CPUs which run at the same hardware frequency: 4 5 6 7
      CPUs which need to have their frequency coordinated by software: 4 5 6 7
      maximum transition latency: 40.0 us
      hardware limits: 408 MHz - 2.30 GHz
      available frequency steps:  408 MHz, 600 MHz, 816 MHz, 1.01 GHz, 1.20 GHz, 1.42 GHz, 1.61 GHz, 1.80 GHz, 2.02 GHz, 2.21 GHz, 2.30 GHz
      available cpufreq governors: ondemand userspace performance schedutil
      current policy: frequency should be within 408 MHz and 2.30 GHz.
                      The governor "schedutil" may decide which speed to use
                      within this range.
      current CPU frequency: 1.61 GHz (asserted by call to hardware)
    user@trixie:~$ uname -a
    Linux trixie 6.15.0-11173-g546b1c9e93c2 #2 SMP PREEMPT Wed Jun  4 20:32:52 CEST 2025 aarch64 GNU/Linux

Frequencies are fine, but I don't think the more power hungry big CPU
cluster gets any voltage scaling without it. Once I try to load the
system enough that the governor decides to bump the big cluster
frequency up, the regulator stays at 850000 microvolts, causing random
reboots when the whole cluster starts starving. With the patch,
voltage oscillates between 700000-737000 microvolts in idle and jumps
up to 950000 under load, and the system seems stable.

Okay, that sounds pretty serious and in this case you should add the
following tag at the end of the commit message, usually as the first
thing after the line break:

  Fixes: 40f742b07ab2 ("arm64: dts: rockchip: Add rk3576-armsom-sige5 board")

That way, our stable kernel robot overlords will pick it up and
backport it to the kernels that already contain the mentioned commit.

Here's what I used to monitor the voltage (there must be a better way
to do it, but it works):
sige5 ~ # watch cat `grep -r . /sys/class/regulator/*/name | grep
vdd_cpu_big_s0 | sed -e 's/name.*//'`/microvolts

And in another terminal:
sige5 ~ # stress-ng -c8

This might warrant a note in the commit message I guess :)

Yes, definitely mention stuff like this in the commit message.

Thanks for your review and testing!

Best regards,
Alexey

Kind regards,
Nicolas Frattaroli