[PATCH 0/3] Update, encapsulate and expand the RK356x SoC dtsi files

[PATCH 0/3] Update, encapsulate and expand the RK356x SoC dtsi files

[Dragan Simic]

This series tackles the Rockchip RK356x SoC dtsi files in a few different
ways.  First, it updates the lower and upper voltage limits and the exact
voltages for the Rockchip RK356x CPU OPPs, using the most conservative
per-OPP values for different SoC bins.  This is rather similar to the
already performed adjustment of the GPU OPP voltages. [1]

Next, this series prepares the RK356x SoC dtsi files for per-variant OPPs,
with the RK3566T being the first new RK356x SoC variant to be introduced.
This follows the approach used for the RK3588 SoC variants. [2]

Lastly, this series introduces new SoC dtsi for the RK3566T variant, which
is capable of operating at the CPU and GPU OPPs/frequencies lower than the
"full-fat" RK3566 variant's.  The RK3566T is found on some of the already
supported boards and rather importantly, this stops the CPU cores and the
GPU from being overclocked on these boards.

[1] https://lore.kernel.org/linux-rockchip/cover.1719763100.git.dsimic@manjaro.org/T/#m786f0e0a45377d29aea826f05c95b5052a8bb3d9
[2] https://lore.kernel.org/all/9ffedc0e2ca7f167d9d795b2a8f43cb9f56a653b.1717923308.git.dsimic@manjaro.org/T/#u

Dragan Simic (3):
  arm64: dts: rockchip: Update CPU OPP voltages in RK356x SoC dtsi
  arm64: dts: rockchip: Prepare RK356x SoC dtsi files for per-variant
    OPPs
  arm64: dts: rockchip: Add new SoC dtsi for the RK3566T variant

 .../{rk3566.dtsi => rk3566-base.dtsi}         |   2 +-
 .../dts/rockchip/rk3566-radxa-zero-3.dtsi     |   2 +-
 .../boot/dts/rockchip/rk3566-rock-3c.dts      |   2 +-
 arch/arm64/boot/dts/rockchip/rk3566.dtsi      | 116 ++++++++++++++----
 arch/arm64/boot/dts/rockchip/rk3566t.dtsi     |  90 ++++++++++++++
 arch/arm64/boot/dts/rockchip/rk3568.dtsi      | 113 ++++++++++++++++-
 .../{rk356x.dtsi => rk356x-base.dtsi}         |  81 ------------
 7 files changed, 294 insertions(+), 112 deletions(-)
 copy arch/arm64/boot/dts/rockchip/{rk3566.dtsi => rk3566-base.dtsi} (95%)
 create mode 100644 arch/arm64/boot/dts/rockchip/rk3566t.dtsi
 rename arch/arm64/boot/dts/rockchip/{rk356x.dtsi => rk356x-base.dtsi} (96%)

[PATCH 1/3] arm64: dts: rockchip: Update CPU OPP voltages in RK356x SoC dtsi

[Dragan Simic]

Update the lower/upper voltage limits and the exact voltages for the Rockchip
RK356x CPU OPPs, using the most conservative values (i.e. the highest per-OPP
voltages) found in the vendor kernel source. [1]

Using the most conservative per-OPP voltages ensures reliable CPU operation
regardless of the actual CPU binning, with the downside of possibly using
a bit more power for the CPU cores than absolutely needed.

Additionally, fill in the missing "clock-latency-ns" CPU OPP properties, using
the values found in the vendor kernel source. [1]

[1] https://raw.githubusercontent.com/rockchip-linux/kernel/f8b9431ee38ed561650be7092ab93f564598daa9/arch/arm64/boot/dts/rockchip/rk3568.dtsi

Related-to: eb665b1c06bc ("arm64: dts: rockchip: Update GPU OPP voltages in RK356x SoC dtsi")
Signed-off-by: Dragan Simic <dsimic@manjaro.org>
---
 arch/arm64/boot/dts/rockchip/rk3568.dtsi |  1 +
 arch/arm64/boot/dts/rockchip/rk356x.dtsi | 18 ++++++++++++------
 2 files changed, 13 insertions(+), 6 deletions(-)

diff --git a/arch/arm64/boot/dts/rockchip/rk3568.dtsi b/arch/arm64/boot/dts/rockchip/rk3568.dtsi
index 0946310e8c12..5c54898f6ed1 100644
--- a/arch/arm64/boot/dts/rockchip/rk3568.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3568.dtsi
@@ -273,6 +273,7 @@ &cpu0_opp_table {
 	opp-1992000000 {
 		opp-hz = /bits/ 64 <1992000000>;
 		opp-microvolt = <1150000 1150000 1150000>;
+		clock-latency-ns = <40000>;
 	};
 };
 
diff --git a/arch/arm64/boot/dts/rockchip/rk356x.dtsi b/arch/arm64/boot/dts/rockchip/rk356x.dtsi
index 0ee0ada6f0ab..534593f2ed0b 100644
--- a/arch/arm64/boot/dts/rockchip/rk356x.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk356x.dtsi
@@ -134,39 +134,45 @@ cpu0_opp_table: opp-table-0 {
 
 		opp-408000000 {
 			opp-hz = /bits/ 64 <408000000>;
-			opp-microvolt = <900000 900000 1150000>;
+			opp-microvolt = <850000 850000 1150000>;
 			clock-latency-ns = <40000>;
 		};
 
 		opp-600000000 {
 			opp-hz = /bits/ 64 <600000000>;
-			opp-microvolt = <900000 900000 1150000>;
+			opp-microvolt = <850000 850000 1150000>;
+			clock-latency-ns = <40000>;
 		};
 
 		opp-816000000 {
 			opp-hz = /bits/ 64 <816000000>;
-			opp-microvolt = <900000 900000 1150000>;
+			opp-microvolt = <850000 850000 1150000>;
+			clock-latency-ns = <40000>;
 			opp-suspend;
 		};
 
 		opp-1104000000 {
 			opp-hz = /bits/ 64 <1104000000>;
 			opp-microvolt = <900000 900000 1150000>;
+			clock-latency-ns = <40000>;
 		};
 
 		opp-1416000000 {
 			opp-hz = /bits/ 64 <1416000000>;
-			opp-microvolt = <900000 900000 1150000>;
+			opp-microvolt = <1025000 1025000 1150000>;
+			clock-latency-ns = <40000>;
 		};
 
 		opp-1608000000 {
 			opp-hz = /bits/ 64 <1608000000>;
-			opp-microvolt = <975000 975000 1150000>;
+			opp-microvolt = <1100000 1100000 1150000>;
+			clock-latency-ns = <40000>;
 		};
 
 		opp-1800000000 {
 			opp-hz = /bits/ 64 <1800000000>;
-			opp-microvolt = <1050000 1050000 1150000>;
+			opp-microvolt = <1150000 1150000 1150000>;
+			clock-latency-ns = <40000>;
 		};
 	};
 

[PATCH 2/3] arm64: dts: rockchip: Prepare RK356x SoC dtsi files for per-variant OPPs

[Dragan Simic]

Rename the Rockchip RK356x SoC dtsi files and, consequently, adjust their
contents appropriately, to prepare them for the ability to specify different
CPU and GPU OPPs for each of the supported RK356x SoC variants.

The first new RK356x SoC variant to be introduced is the RK3566T, which the
Pine64 Quartz64 Zero SBC is officially based on. [1]  Some other SBCs are
also based on the RK3566T variant, including Radxa ROCK 3C and ZERO 3E/3W,
but the slight trouble is that Radxa doesn't state that officially.  Though,
it's rather easy to spot the RK3566T on such boards, because their official
specifications state that the maximum frequency for the Cortex-A55 cores is
lower than the "full-fat" RK3566's 1.8 GHz. [2][3][4]

These changes follow the approach used for the Rockchip RK3588 SoC variants,
which was introduced and described further in commit def88eb4d836 ("arm64:
dts: rockchip: Prepare RK3588 SoC dtsi files for per-variant OPPs").  Please
see that commit for a more detailed explanation.

No functional changes are introduced, which was validated by decompiling and
comparing all affected board dtb files before and after these changes.  In
more detail, the affected dtb files have some of their blocks shuffled around
a bit and some of their phandles have different values, as a result of the
changes to the order in which the building blocks from the parent dtsi files
are included, but they effectively remain the same as the originals.

[1] https://wiki.pine64.org/wiki/Quartz64
[2] https://dl.radxa.com/rock3/docs/hw/3c/radxa_rock3c_product_brief.pdf
[3] https://dl.radxa.com/zero3/docs/hw/3e/radxa_zero_3e_product_brief.pdf
[4] https://dl.radxa.com/zero3/docs/hw/3w/radxa_zero_3w_product_brief.pdf

Related-to: def88eb4d836 ("arm64: dts: rockchip: Prepare RK3588 SoC dtsi files for per-variant OPPs")
Signed-off-by: Dragan Simic <dsimic@manjaro.org>
---
 .../{rk3566.dtsi => rk3566-base.dtsi}         |   2 +-
 arch/arm64/boot/dts/rockchip/rk3566.dtsi      | 116 ++++++++++++++----
 arch/arm64/boot/dts/rockchip/rk3568.dtsi      | 114 +++++++++++++++--
 .../{rk356x.dtsi => rk356x-base.dtsi}         |  87 -------------
 4 files changed, 202 insertions(+), 117 deletions(-)
 copy arch/arm64/boot/dts/rockchip/{rk3566.dtsi => rk3566-base.dtsi} (95%)
 rename arch/arm64/boot/dts/rockchip/{rk356x.dtsi => rk356x-base.dtsi} (96%)

diff --git a/arch/arm64/boot/dts/rockchip/rk3566.dtsi b/arch/arm64/boot/dts/rockchip/rk3566-base.dtsi
similarity index 95%
copy from arch/arm64/boot/dts/rockchip/rk3566.dtsi
copy to arch/arm64/boot/dts/rockchip/rk3566-base.dtsi
index 6c4b17d27bdc..e56e0b6ba941 100644
--- a/arch/arm64/boot/dts/rockchip/rk3566.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3566-base.dtsi
@@ -1,6 +1,6 @@
 // SPDX-License-Identifier: (GPL-2.0+ OR MIT)
 
-#include "rk356x.dtsi"
+#include "rk356x-base.dtsi"
 
 / {
 	compatible = "rockchip,rk3566";
diff --git a/arch/arm64/boot/dts/rockchip/rk3566.dtsi b/arch/arm64/boot/dts/rockchip/rk3566.dtsi
index 6c4b17d27bdc..3fcca79279f7 100644
--- a/arch/arm64/boot/dts/rockchip/rk3566.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3566.dtsi
@@ -1,35 +1,107 @@
 // SPDX-License-Identifier: (GPL-2.0+ OR MIT)
 
-#include "rk356x.dtsi"
+#include "rk3566-base.dtsi"
 
 / {
-	compatible = "rockchip,rk3566";
+	cpu0_opp_table: opp-table-0 {
+		compatible = "operating-points-v2";
+		opp-shared;
+
+		opp-408000000 {
+			opp-hz = /bits/ 64 <408000000>;
+			opp-microvolt = <850000 850000 1150000>;
+			clock-latency-ns = <40000>;
+		};
+
+		opp-600000000 {
+			opp-hz = /bits/ 64 <600000000>;
+			opp-microvolt = <850000 850000 1150000>;
+			clock-latency-ns = <40000>;
+		};
+
+		opp-816000000 {
+			opp-hz = /bits/ 64 <816000000>;
+			opp-microvolt = <850000 850000 1150000>;
+			clock-latency-ns = <40000>;
+			opp-suspend;
+		};
+
+		opp-1104000000 {
+			opp-hz = /bits/ 64 <1104000000>;
+			opp-microvolt = <900000 900000 1150000>;
+			clock-latency-ns = <40000>;
+		};
+
+		opp-1416000000 {
+			opp-hz = /bits/ 64 <1416000000>;
+			opp-microvolt = <1025000 1025000 1150000>;
+			clock-latency-ns = <40000>;
+		};
+
+		opp-1608000000 {
+			opp-hz = /bits/ 64 <1608000000>;
+			opp-microvolt = <1100000 1100000 1150000>;
+			clock-latency-ns = <40000>;
+		};
+
+		opp-1800000000 {
+			opp-hz = /bits/ 64 <1800000000>;
+			opp-microvolt = <1150000 1150000 1150000>;
+			clock-latency-ns = <40000>;
+		};
+	};
+
+	gpu_opp_table: opp-table-1 {
+		compatible = "operating-points-v2";
+
+		opp-200000000 {
+			opp-hz = /bits/ 64 <200000000>;
+			opp-microvolt = <850000 850000 1000000>;
+		};
+
+		opp-300000000 {
+			opp-hz = /bits/ 64 <300000000>;
+			opp-microvolt = <850000 850000 1000000>;
+		};
+
+		opp-400000000 {
+			opp-hz = /bits/ 64 <400000000>;
+			opp-microvolt = <850000 850000 1000000>;
+		};
+
+		opp-600000000 {
+			opp-hz = /bits/ 64 <600000000>;
+			opp-microvolt = <900000 900000 1000000>;
+		};
+
+		opp-700000000 {
+			opp-hz = /bits/ 64 <700000000>;
+			opp-microvolt = <950000 950000 1000000>;
+		};
+
+		opp-800000000 {
+			opp-hz = /bits/ 64 <800000000>;
+			opp-microvolt = <1000000 1000000 1000000>;
+		};
+	};
 };
 
-&pipegrf {
-	compatible = "rockchip,rk3566-pipe-grf", "syscon";
+&cpu0 {
+	operating-points-v2 = <&cpu0_opp_table>;
 };
 
-&power {
-	power-domain@RK3568_PD_PIPE {
-		reg = <RK3568_PD_PIPE>;
-		clocks = <&cru PCLK_PIPE>;
-		pm_qos = <&qos_pcie2x1>,
-			 <&qos_sata1>,
-			 <&qos_sata2>,
-			 <&qos_usb3_0>,
-			 <&qos_usb3_1>;
-		#power-domain-cells = <0>;
-	};
+&cpu1 {
+	operating-points-v2 = <&cpu0_opp_table>;
+};
+
+&cpu2 {
+	operating-points-v2 = <&cpu0_opp_table>;
 };
 
-&usb_host0_xhci {
-	phys = <&usb2phy0_otg>;
-	phy-names = "usb2-phy";
-	extcon = <&usb2phy0>;
-	maximum-speed = "high-speed";
+&cpu3 {
+	operating-points-v2 = <&cpu0_opp_table>;
 };
 
-&vop {
-	compatible = "rockchip,rk3566-vop";
+&gpu {
+	operating-points-v2 = <&gpu_opp_table>;
 };
diff --git a/arch/arm64/boot/dts/rockchip/rk3568.dtsi b/arch/arm64/boot/dts/rockchip/rk3568.dtsi
index 5c54898f6ed1..ecaefe208e3e 100644
--- a/arch/arm64/boot/dts/rockchip/rk3568.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3568.dtsi
@@ -3,11 +3,99 @@
  * Copyright (c) 2021 Rockchip Electronics Co., Ltd.
  */
 
-#include "rk356x.dtsi"
+#include "rk356x-base.dtsi"
 
 / {
 	compatible = "rockchip,rk3568";
 
+	cpu0_opp_table: opp-table-0 {
+		compatible = "operating-points-v2";
+		opp-shared;
+
+		opp-408000000 {
+			opp-hz = /bits/ 64 <408000000>;
+			opp-microvolt = <850000 850000 1150000>;
+			clock-latency-ns = <40000>;
+		};
+
+		opp-600000000 {
+			opp-hz = /bits/ 64 <600000000>;
+			opp-microvolt = <850000 850000 1150000>;
+			clock-latency-ns = <40000>;
+		};
+
+		opp-816000000 {
+			opp-hz = /bits/ 64 <816000000>;
+			opp-microvolt = <850000 850000 1150000>;
+			clock-latency-ns = <40000>;
+			opp-suspend;
+		};
+
+		opp-1104000000 {
+			opp-hz = /bits/ 64 <1104000000>;
+			opp-microvolt = <900000 900000 1150000>;
+			clock-latency-ns = <40000>;
+		};
+
+		opp-1416000000 {
+			opp-hz = /bits/ 64 <1416000000>;
+			opp-microvolt = <1025000 1025000 1150000>;
+			clock-latency-ns = <40000>;
+		};
+
+		opp-1608000000 {
+			opp-hz = /bits/ 64 <1608000000>;
+			opp-microvolt = <1100000 1100000 1150000>;
+			clock-latency-ns = <40000>;
+		};
+
+		opp-1800000000 {
+			opp-hz = /bits/ 64 <1800000000>;
+			opp-microvolt = <1150000 1150000 1150000>;
+			clock-latency-ns = <40000>;
+		};
+
+		opp-1992000000 {
+			opp-hz = /bits/ 64 <1992000000>;
+			opp-microvolt = <1150000 1150000 1150000>;
+			clock-latency-ns = <40000>;
+		};
+	};
+
+	gpu_opp_table: opp-table-1 {
+		compatible = "operating-points-v2";
+
+		opp-200000000 {
+			opp-hz = /bits/ 64 <200000000>;
+			opp-microvolt = <850000 850000 1000000>;
+		};
+
+		opp-300000000 {
+			opp-hz = /bits/ 64 <300000000>;
+			opp-microvolt = <850000 850000 1000000>;
+		};
+
+		opp-400000000 {
+			opp-hz = /bits/ 64 <400000000>;
+			opp-microvolt = <850000 850000 1000000>;
+		};
+
+		opp-600000000 {
+			opp-hz = /bits/ 64 <600000000>;
+			opp-microvolt = <900000 900000 1000000>;
+		};
+
+		opp-700000000 {
+			opp-hz = /bits/ 64 <700000000>;
+			opp-microvolt = <950000 950000 1000000>;
+		};
+
+		opp-800000000 {
+			opp-hz = /bits/ 64 <800000000>;
+			opp-microvolt = <1000000 1000000 1000000>;
+		};
+	};
+
 	sata0: sata@fc000000 {
 		compatible = "rockchip,rk3568-dwc-ahci", "snps,dwc-ahci";
 		reg = <0 0xfc000000 0 0x1000>;
@@ -269,12 +357,24 @@ combphy0: phy@fe820000 {
 	};
 };
 
-&cpu0_opp_table {
-	opp-1992000000 {
-		opp-hz = /bits/ 64 <1992000000>;
-		opp-microvolt = <1150000 1150000 1150000>;
-		clock-latency-ns = <40000>;
-	};
+&cpu0 {
+	operating-points-v2 = <&cpu0_opp_table>;
+};
+
+&cpu1 {
+	operating-points-v2 = <&cpu0_opp_table>;
+};
+
+&cpu2 {
+	operating-points-v2 = <&cpu0_opp_table>;
+};
+
+&cpu3 {
+	operating-points-v2 = <&cpu0_opp_table>;
+};
+
+&gpu {
+	operating-points-v2 = <&gpu_opp_table>;
 };
 
 &pipegrf {
diff --git a/arch/arm64/boot/dts/rockchip/rk356x.dtsi b/arch/arm64/boot/dts/rockchip/rk356x-base.dtsi
similarity index 96%
rename from arch/arm64/boot/dts/rockchip/rk356x.dtsi
rename to arch/arm64/boot/dts/rockchip/rk356x-base.dtsi
index 534593f2ed0b..62be06f3b863 100644
--- a/arch/arm64/boot/dts/rockchip/rk356x.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk356x-base.dtsi
@@ -56,7 +56,6 @@ cpu0: cpu@0 {
 			clocks = <&scmi_clk 0>;
 			#cooling-cells = <2>;
 			enable-method = "psci";
-			operating-points-v2 = <&cpu0_opp_table>;
 			i-cache-size = <0x8000>;
 			i-cache-line-size = <64>;
 			i-cache-sets = <128>;
@@ -72,7 +71,6 @@ cpu1: cpu@100 {
 			reg = <0x0 0x100>;
 			#cooling-cells = <2>;
 			enable-method = "psci";
-			operating-points-v2 = <&cpu0_opp_table>;
 			i-cache-size = <0x8000>;
 			i-cache-line-size = <64>;
 			i-cache-sets = <128>;
@@ -88,7 +86,6 @@ cpu2: cpu@200 {
 			reg = <0x0 0x200>;
 			#cooling-cells = <2>;
 			enable-method = "psci";
-			operating-points-v2 = <&cpu0_opp_table>;
 			i-cache-size = <0x8000>;
 			i-cache-line-size = <64>;
 			i-cache-sets = <128>;
@@ -104,7 +101,6 @@ cpu3: cpu@300 {
 			reg = <0x0 0x300>;
 			#cooling-cells = <2>;
 			enable-method = "psci";
-			operating-points-v2 = <&cpu0_opp_table>;
 			i-cache-size = <0x8000>;
 			i-cache-line-size = <64>;
 			i-cache-sets = <128>;
@@ -128,54 +124,6 @@ l3_cache: l3-cache {
 		cache-sets = <512>;
 	};
 
-	cpu0_opp_table: opp-table-0 {
-		compatible = "operating-points-v2";
-		opp-shared;
-
-		opp-408000000 {
-			opp-hz = /bits/ 64 <408000000>;
-			opp-microvolt = <850000 850000 1150000>;
-			clock-latency-ns = <40000>;
-		};
-
-		opp-600000000 {
-			opp-hz = /bits/ 64 <600000000>;
-			opp-microvolt = <850000 850000 1150000>;
-			clock-latency-ns = <40000>;
-		};
-
-		opp-816000000 {
-			opp-hz = /bits/ 64 <816000000>;
-			opp-microvolt = <850000 850000 1150000>;
-			clock-latency-ns = <40000>;
-			opp-suspend;
-		};
-
-		opp-1104000000 {
-			opp-hz = /bits/ 64 <1104000000>;
-			opp-microvolt = <900000 900000 1150000>;
-			clock-latency-ns = <40000>;
-		};
-
-		opp-1416000000 {
-			opp-hz = /bits/ 64 <1416000000>;
-			opp-microvolt = <1025000 1025000 1150000>;
-			clock-latency-ns = <40000>;
-		};
-
-		opp-1608000000 {
-			opp-hz = /bits/ 64 <1608000000>;
-			opp-microvolt = <1100000 1100000 1150000>;
-			clock-latency-ns = <40000>;
-		};
-
-		opp-1800000000 {
-			opp-hz = /bits/ 64 <1800000000>;
-			opp-microvolt = <1150000 1150000 1150000>;
-			clock-latency-ns = <40000>;
-		};
-	};
-
 	display_subsystem: display-subsystem {
 		compatible = "rockchip,display-subsystem";
 		ports = <&vop_out>;
@@ -196,40 +144,6 @@ scmi_clk: protocol@14 {
 		};
 	};
 
-	gpu_opp_table: opp-table-1 {
-		compatible = "operating-points-v2";
-
-		opp-200000000 {
-			opp-hz = /bits/ 64 <200000000>;
-			opp-microvolt = <850000 850000 1000000>;
-		};
-
-		opp-300000000 {
-			opp-hz = /bits/ 64 <300000000>;
-			opp-microvolt = <850000 850000 1000000>;
-		};
-
-		opp-400000000 {
-			opp-hz = /bits/ 64 <400000000>;
-			opp-microvolt = <850000 850000 1000000>;
-		};
-
-		opp-600000000 {
-			opp-hz = /bits/ 64 <600000000>;
-			opp-microvolt = <900000 900000 1000000>;
-		};
-
-		opp-700000000 {
-			opp-hz = /bits/ 64 <700000000>;
-			opp-microvolt = <950000 950000 1000000>;
-		};
-
-		opp-800000000 {
-			opp-hz = /bits/ 64 <800000000>;
-			opp-microvolt = <1000000 1000000 1000000>;
-		};
-	};
-
 	hdmi_sound: hdmi-sound {
 		compatible = "simple-audio-card";
 		simple-audio-card,name = "HDMI";
@@ -635,7 +549,6 @@ gpu: gpu@fde60000 {
 		clocks = <&scmi_clk 1>, <&cru CLK_GPU>;
 		clock-names = "gpu", "bus";
 		#cooling-cells = <2>;
-		operating-points-v2 = <&gpu_opp_table>;
 		power-domains = <&power RK3568_PD_GPU>;
 		status = "disabled";
 	};

[PATCH 3/3] arm64: dts: rockchip: Add new SoC dtsi for the RK3566T variant

[Dragan Simic]

Add new SoC dtsi file for the RK3566T variant of the Rockchip RK3566 SoC.
The difference between the RK3566T variant and the "full-fat" RK3566 variant
is in fewer supported CPU and GPU OPPs on the RK3566T, and in the absence of
a functional NPU, which we currently don't have to worry about.

Examples of the boards based on the RK3566T include the Pine64 Quartz64 Zero
SBC, [2] the Radxa ROCK 3C and the Radxa ZERO 3E/3W SBCs.  Unfortunately,
Radxa doesn't mention the use of RK3566T officially, but its official SBC
specifications do state that the maximum frequency for the Cortex-A55 cores
on those SBCs is lower than the "full-fat" RK3566's 1.8 GHz, which makes
spotting the presence of the RK3566T SoC variant rather easy. [3][4][5]  An
additional, helpful cue is that Radxa handles the CPU and GPU OPPs for the
RK3566T variant separately in its downstream kernel. [6]

The CPU and GPU OPPs supported on the RK3566T SoC variant are taken from the
vendor kernel source, [1] which uses the values of the "opp-supported-hw" OPP
properties to determine which ones are supported on a particular SoC variant.
The actual values of the "opp-supported-hw" properties make it rather easy
to see what OPPs are supported on the RK3566T SoC variant, but that, rather
unfortunately, clashes with the maximum frequencies advertised officially
for the Cortex-A55 CPU cores on the above-mentioned SBCs. [2][3][4][5]  The
vendor kernel source indicates that the maximum frequency for the CPU cores
is 1.4 GHz, while the SBC specifications state that to be 1.6 GHz.  Unless
that discrepancy is resolved somehow, let's take the safe approach and use
the lower maximum frequency for the CPU cores.

Update the dts files of the currently supported RK3566T-based boards to use
the new SoC dtsi for the RK3566T variant.  This actually takes the CPU cores
and the GPUs found on these boards out of their earlier overclocks, but it
also means that the officially advertised specifications [2][3][4][5] of the
highest supported frequencies for the Cortex-A55 CPU cores on these boards
may actually be wrong, as already explained above.

The correctness of the introduced changes was validated by decompiling and
comparing all affected board dtb files before and after these changes.

[1] https://raw.githubusercontent.com/rockchip-linux/kernel/f8b9431ee38ed561650be7092ab93f564598daa9/arch/arm64/boot/dts/rockchip/rk3568.dtsi
[2] https://wiki.pine64.org/wiki/Quartz64
[3] https://dl.radxa.com/rock3/docs/hw/3c/radxa_rock3c_product_brief.pdf
[4] https://dl.radxa.com/zero3/docs/hw/3e/radxa_zero_3e_product_brief.pdf
[5] https://dl.radxa.com/zero3/docs/hw/3w/radxa_zero_3w_product_brief.pdf
[6] https://github.com/radxa/kernel/commit/2dfd51da472e7ebb5ef0d3db78f902454af826b8

Cc: TL Lim <tllim@pine64.org>
Cc: Marek Kraus <gamiee@pine64.org>
Cc: Tom Cubie <tom@radxa.com>
Cc: FUKAUMI Naoki <naoki@radxa.com>
Helped-by: Nicolas Frattaroli <frattaroli.nicolas@gmail.com>
Helped-by: Jonas Karlman <jonas@kwiboo.se>
Signed-off-by: Dragan Simic <dsimic@manjaro.org>
---
 .../dts/rockchip/rk3566-radxa-zero-3.dtsi     |  2 +-
 .../boot/dts/rockchip/rk3566-rock-3c.dts      |  2 +-
 arch/arm64/boot/dts/rockchip/rk3566t.dtsi     | 90 +++++++++++++++++++
 3 files changed, 92 insertions(+), 2 deletions(-)
 create mode 100644 arch/arm64/boot/dts/rockchip/rk3566t.dtsi

diff --git a/arch/arm64/boot/dts/rockchip/rk3566-radxa-zero-3.dtsi b/arch/arm64/boot/dts/rockchip/rk3566-radxa-zero-3.dtsi
index de390d92c35e..1ee5d96a46a1 100644
--- a/arch/arm64/boot/dts/rockchip/rk3566-radxa-zero-3.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3566-radxa-zero-3.dtsi
@@ -3,7 +3,7 @@
 #include <dt-bindings/gpio/gpio.h>
 #include <dt-bindings/leds/common.h>
 #include <dt-bindings/soc/rockchip,vop2.h>
-#include "rk3566.dtsi"
+#include "rk3566t.dtsi"
 
 / {
 	chosen {
diff --git a/arch/arm64/boot/dts/rockchip/rk3566-rock-3c.dts b/arch/arm64/boot/dts/rockchip/rk3566-rock-3c.dts
index f2cc086e5001..9a8f4f774dbc 100644
--- a/arch/arm64/boot/dts/rockchip/rk3566-rock-3c.dts
+++ b/arch/arm64/boot/dts/rockchip/rk3566-rock-3c.dts
@@ -5,7 +5,7 @@
 #include <dt-bindings/leds/common.h>
 #include <dt-bindings/pinctrl/rockchip.h>
 #include <dt-bindings/soc/rockchip,vop2.h>
-#include "rk3566.dtsi"
+#include "rk3566t.dtsi"
 
 / {
 	model = "Radxa ROCK 3C";
diff --git a/arch/arm64/boot/dts/rockchip/rk3566t.dtsi b/arch/arm64/boot/dts/rockchip/rk3566t.dtsi
new file mode 100644
index 000000000000..cd89bd3b125b
--- /dev/null
+++ b/arch/arm64/boot/dts/rockchip/rk3566t.dtsi
@@ -0,0 +1,90 @@
+// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
+
+#include "rk3566-base.dtsi"
+
+/ {
+	cpu0_opp_table: opp-table-0 {
+		compatible = "operating-points-v2";
+		opp-shared;
+
+		opp-408000000 {
+			opp-hz = /bits/ 64 <408000000>;
+			opp-microvolt = <850000 850000 1150000>;
+			clock-latency-ns = <40000>;
+		};
+
+		opp-600000000 {
+			opp-hz = /bits/ 64 <600000000>;
+			opp-microvolt = <850000 850000 1150000>;
+			clock-latency-ns = <40000>;
+		};
+
+		opp-816000000 {
+			opp-hz = /bits/ 64 <816000000>;
+			opp-microvolt = <850000 850000 1150000>;
+			clock-latency-ns = <40000>;
+			opp-suspend;
+		};
+
+		opp-1104000000 {
+			opp-hz = /bits/ 64 <1104000000>;
+			opp-microvolt = <900000 900000 1150000>;
+			clock-latency-ns = <40000>;
+		};
+
+		opp-1416000000 {
+			opp-hz = /bits/ 64 <1416000000>;
+			opp-microvolt = <1025000 1025000 1150000>;
+			clock-latency-ns = <40000>;
+		};
+	};
+
+	gpu_opp_table: opp-table-1 {
+		compatible = "operating-points-v2";
+
+		opp-200000000 {
+			opp-hz = /bits/ 64 <200000000>;
+			opp-microvolt = <850000 850000 1000000>;
+		};
+
+		opp-300000000 {
+			opp-hz = /bits/ 64 <300000000>;
+			opp-microvolt = <850000 850000 1000000>;
+		};
+
+		opp-400000000 {
+			opp-hz = /bits/ 64 <400000000>;
+			opp-microvolt = <850000 850000 1000000>;
+		};
+
+		opp-600000000 {
+			opp-hz = /bits/ 64 <600000000>;
+			opp-microvolt = <900000 900000 1000000>;
+		};
+
+		opp-700000000 {
+			opp-hz = /bits/ 64 <700000000>;
+			opp-microvolt = <950000 950000 1000000>;
+		};
+	};
+};
+
+&cpu0 {
+	operating-points-v2 = <&cpu0_opp_table>;
+};
+
+&cpu1 {
+	operating-points-v2 = <&cpu0_opp_table>;
+};
+
+&cpu2 {
+	operating-points-v2 = <&cpu0_opp_table>;
+};
+
+&cpu3 {
+	operating-points-v2 = <&cpu0_opp_table>;
+};
+
+&gpu {
+	operating-points-v2 = <&gpu_opp_table>;
+};

Re: [PATCH 1/3] arm64: dts: rockchip: Update CPU OPP voltages in RK356x SoC dtsi

[Diederik de Haas]

[-- Attachment #1: Type: text/plain, Size: 2738 bytes --]

Hi Dragan,

On Sat Oct 12, 2024 at 7:04 PM CEST, Dragan Simic wrote:
> Update the lower/upper voltage limits and the exact voltages for the Rockchip
> RK356x CPU OPPs, using the most conservative values (i.e. the highest per-OPP
> voltages) found in the vendor kernel source. [1]
>
> Using the most conservative per-OPP voltages ensures reliable CPU operation
> regardless of the actual CPU binning, with the downside of possibly using
> a bit more power for the CPU cores than absolutely needed.
>
> Additionally, fill in the missing "clock-latency-ns" CPU OPP properties, using
> the values found in the vendor kernel source. [1]
>
> [1] https://raw.githubusercontent.com/rockchip-linux/kernel/f8b9431ee38ed561650be7092ab93f564598daa9/arch/arm64/boot/dts/rockchip/rk3568.dtsi
>
> Related-to: eb665b1c06bc ("arm64: dts: rockchip: Update GPU OPP voltages in RK356x SoC dtsi")
> Signed-off-by: Dragan Simic <dsimic@manjaro.org>
> ---
>  arch/arm64/boot/dts/rockchip/rk3568.dtsi |  1 +
>  arch/arm64/boot/dts/rockchip/rk356x.dtsi | 18 ++++++++++++------
>  2 files changed, 13 insertions(+), 6 deletions(-)
>
> diff --git a/arch/arm64/boot/dts/rockchip/rk3568.dtsi b/arch/arm64/boot/dts/rockchip/rk3568.dtsi
> index 0946310e8c12..5c54898f6ed1 100644
> --- a/arch/arm64/boot/dts/rockchip/rk3568.dtsi
> +++ b/arch/arm64/boot/dts/rockchip/rk3568.dtsi
> @@ -273,6 +273,7 @@ &cpu0_opp_table {
>  	opp-1992000000 {
>  		opp-hz = /bits/ 64 <1992000000>;
>  		opp-microvolt = <1150000 1150000 1150000>;
> +		clock-latency-ns = <40000>;
>  	};
>  };
>  
> diff --git a/arch/arm64/boot/dts/rockchip/rk356x.dtsi b/arch/arm64/boot/dts/rockchip/rk356x.dtsi
> index 0ee0ada6f0ab..534593f2ed0b 100644
> --- a/arch/arm64/boot/dts/rockchip/rk356x.dtsi
> +++ b/arch/arm64/boot/dts/rockchip/rk356x.dtsi
> @@ -134,39 +134,45 @@ cpu0_opp_table: opp-table-0 {
>  
>  		opp-408000000 {
>  			opp-hz = /bits/ 64 <408000000>;
> -			opp-microvolt = <900000 900000 1150000>;
> +			opp-microvolt = <850000 850000 1150000>;
>  			clock-latency-ns = <40000>;
>  		};
>  
>  		opp-600000000 {
>  			opp-hz = /bits/ 64 <600000000>;
> -			opp-microvolt = <900000 900000 1150000>;
> +			opp-microvolt = <850000 850000 1150000>;
> +			clock-latency-ns = <40000>;
>  		};
>  
>  		opp-816000000 {
>  			opp-hz = /bits/ 64 <816000000>;
> -			opp-microvolt = <900000 900000 1150000>;
> +			opp-microvolt = <850000 850000 1150000>;
> +			clock-latency-ns = <40000>;
>  			opp-suspend;
>  		};

While it felt a bit much to send a patch just to remove the blank lines
between the opp nodes, this sounds like an excellent opportunity to make
it consistent with the opp list in other DT files?

Cheers,
  Diederik

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH 2/3] arm64: dts: rockchip: Prepare RK356x SoC dtsi files for per-variant OPPs

[Diederik de Haas]

[-- Attachment #1: Type: text/plain, Size: 14868 bytes --]

Hi Dragan,

On Sat Oct 12, 2024 at 7:04 PM CEST, Dragan Simic wrote:
> Rename the Rockchip RK356x SoC dtsi files and, consequently, adjust their
> contents appropriately, to prepare them for the ability to specify different
> CPU and GPU OPPs for each of the supported RK356x SoC variants.
>
> The first new RK356x SoC variant to be introduced is the RK3566T, which the
> Pine64 Quartz64 Zero SBC is officially based on. [1]  Some other SBCs are
> also based on the RK3566T variant, including Radxa ROCK 3C and ZERO 3E/3W,
> but the slight trouble is that Radxa doesn't state that officially.  Though,
> it's rather easy to spot the RK3566T on such boards, because their official
> specifications state that the maximum frequency for the Cortex-A55 cores is
> lower than the "full-fat" RK3566's 1.8 GHz. [2][3][4]

I think we changed terminology from "full-fat" to something else in the
rk3588 variant? Would be nice to be consisten.

> These changes follow the approach used for the Rockchip RK3588 SoC variants,
> which was introduced and described further in commit def88eb4d836 ("arm64:
> dts: rockchip: Prepare RK3588 SoC dtsi files for per-variant OPPs").  Please
> see that commit for a more detailed explanation.
>
> No functional changes are introduced, which was validated by decompiling and

No functional changes ...

> comparing all affected board dtb files before and after these changes.  In
> more detail, the affected dtb files have some of their blocks shuffled around
> a bit and some of their phandles have different values, as a result of the
> changes to the order in which the building blocks from the parent dtsi files
> are included, but they effectively remain the same as the originals.
>
> [1] https://wiki.pine64.org/wiki/Quartz64
> [2] https://dl.radxa.com/rock3/docs/hw/3c/radxa_rock3c_product_brief.pdf
> [3] https://dl.radxa.com/zero3/docs/hw/3e/radxa_zero_3e_product_brief.pdf
> [4] https://dl.radxa.com/zero3/docs/hw/3w/radxa_zero_3w_product_brief.pdf
>
> Related-to: def88eb4d836 ("arm64: dts: rockchip: Prepare RK3588 SoC dtsi files for per-variant OPPs")
> Signed-off-by: Dragan Simic <dsimic@manjaro.org>
> ---
>  .../{rk3566.dtsi => rk3566-base.dtsi}         |   2 +-
>  arch/arm64/boot/dts/rockchip/rk3566.dtsi      | 116 ++++++++++++++----
>  arch/arm64/boot/dts/rockchip/rk3568.dtsi      | 114 +++++++++++++++--
>  .../{rk356x.dtsi => rk356x-base.dtsi}         |  87 -------------
>  4 files changed, 202 insertions(+), 117 deletions(-)
>  copy arch/arm64/boot/dts/rockchip/{rk3566.dtsi => rk3566-base.dtsi} (95%)
>  rename arch/arm64/boot/dts/rockchip/{rk356x.dtsi => rk356x-base.dtsi} (96%)
>
> diff --git a/arch/arm64/boot/dts/rockchip/rk3566.dtsi b/arch/arm64/boot/dts/rockchip/rk3566-base.dtsi
> similarity index 95%
> copy from arch/arm64/boot/dts/rockchip/rk3566.dtsi
> copy to arch/arm64/boot/dts/rockchip/rk3566-base.dtsi
> index 6c4b17d27bdc..e56e0b6ba941 100644
> --- a/arch/arm64/boot/dts/rockchip/rk3566.dtsi
> +++ b/arch/arm64/boot/dts/rockchip/rk3566-base.dtsi
> @@ -1,6 +1,6 @@
>  // SPDX-License-Identifier: (GPL-2.0+ OR MIT)
>  
> -#include "rk356x.dtsi"
> +#include "rk356x-base.dtsi"
>  
>  / {
>  	compatible = "rockchip,rk3566";
> diff --git a/arch/arm64/boot/dts/rockchip/rk3566.dtsi b/arch/arm64/boot/dts/rockchip/rk3566.dtsi
> index 6c4b17d27bdc..3fcca79279f7 100644
> --- a/arch/arm64/boot/dts/rockchip/rk3566.dtsi
> +++ b/arch/arm64/boot/dts/rockchip/rk3566.dtsi
> @@ -1,35 +1,107 @@
>  // SPDX-License-Identifier: (GPL-2.0+ OR MIT)
>  
> -#include "rk356x.dtsi"
> +#include "rk3566-base.dtsi"
>  
>  / {
> -	compatible = "rockchip,rk3566";
> +	cpu0_opp_table: opp-table-0 {
> +		compatible = "operating-points-v2";
> +		opp-shared;
> +
> +		opp-408000000 {
> +			opp-hz = /bits/ 64 <408000000>;
> +			opp-microvolt = <850000 850000 1150000>;
> +			clock-latency-ns = <40000>;
> +		};
> +
> +		opp-600000000 {
> +			opp-hz = /bits/ 64 <600000000>;
> +			opp-microvolt = <850000 850000 1150000>;
> +			clock-latency-ns = <40000>;
> +		};
> +
> +		opp-816000000 {
> +			opp-hz = /bits/ 64 <816000000>;
> +			opp-microvolt = <850000 850000 1150000>;
> +			clock-latency-ns = <40000>;
> +			opp-suspend;
> +		};

Just like with patch 1 of this series, drop the blank line?

> +
> +		opp-1104000000 {
> +			opp-hz = /bits/ 64 <1104000000>;
> +			opp-microvolt = <900000 900000 1150000>;
> +			clock-latency-ns = <40000>;
> +		};
> +
> +		opp-1416000000 {
> +			opp-hz = /bits/ 64 <1416000000>;
> +			opp-microvolt = <1025000 1025000 1150000>;
> +			clock-latency-ns = <40000>;
> +		};
> +
> +		opp-1608000000 {
> +			opp-hz = /bits/ 64 <1608000000>;
> +			opp-microvolt = <1100000 1100000 1150000>;
> +			clock-latency-ns = <40000>;
> +		};
> +
> +		opp-1800000000 {
> +			opp-hz = /bits/ 64 <1800000000>;
> +			opp-microvolt = <1150000 1150000 1150000>;
> +			clock-latency-ns = <40000>;
> +		};
> +	};
> +
> +	gpu_opp_table: opp-table-1 {
> +		compatible = "operating-points-v2";
> +
> +		opp-200000000 {
> +			opp-hz = /bits/ 64 <200000000>;
> +			opp-microvolt = <850000 850000 1000000>;
> +		};
> +
> +		opp-300000000 {
> +			opp-hz = /bits/ 64 <300000000>;
> +			opp-microvolt = <850000 850000 1000000>;
> +		};
> +
> +		opp-400000000 {
> +			opp-hz = /bits/ 64 <400000000>;
> +			opp-microvolt = <850000 850000 1000000>;
> +		};
> +
> +		opp-600000000 {
> +			opp-hz = /bits/ 64 <600000000>;
> +			opp-microvolt = <900000 900000 1000000>;
> +		};
> +
> +		opp-700000000 {
> +			opp-hz = /bits/ 64 <700000000>;
> +			opp-microvolt = <950000 950000 1000000>;
> +		};
> +
> +		opp-800000000 {
> +			opp-hz = /bits/ 64 <800000000>;
> +			opp-microvolt = <1000000 1000000 1000000>;
> +		};
> +	};
>  };
>  
> -&pipegrf {
> -	compatible = "rockchip,rk3566-pipe-grf", "syscon";

This seems unrelated?

> +&cpu0 {
> +	operating-points-v2 = <&cpu0_opp_table>;
>  };
>  
> -&power {
> -	power-domain@RK3568_PD_PIPE {
> -		reg = <RK3568_PD_PIPE>;
> -		clocks = <&cru PCLK_PIPE>;
> -		pm_qos = <&qos_pcie2x1>,
> -			 <&qos_sata1>,
> -			 <&qos_sata2>,
> -			 <&qos_usb3_0>,
> -			 <&qos_usb3_1>;
> -		#power-domain-cells = <0>;
> -	};

This seems unrelated to me and possibly a functional change?
If this was intended, then a description in the commit message would be
nice why this is appropriate and possibly moved to a separate patch?

> +&cpu1 {
> +	operating-points-v2 = <&cpu0_opp_table>;
> +};
> +
> +&cpu2 {
> +	operating-points-v2 = <&cpu0_opp_table>;
>  };
>  
> -&usb_host0_xhci {
> -	phys = <&usb2phy0_otg>;
> -	phy-names = "usb2-phy";
> -	extcon = <&usb2phy0>;
> -	maximum-speed = "high-speed";

This also looks unrelated and a functional change?

> +&cpu3 {
> +	operating-points-v2 = <&cpu0_opp_table>;
>  };
>  
> -&vop {
> -	compatible = "rockchip,rk3566-vop";

This also looks unrelated?

Cheers,
  Diederik

> +&gpu {
> +	operating-points-v2 = <&gpu_opp_table>;
>  };
> diff --git a/arch/arm64/boot/dts/rockchip/rk3568.dtsi b/arch/arm64/boot/dts/rockchip/rk3568.dtsi
> index 5c54898f6ed1..ecaefe208e3e 100644
> --- a/arch/arm64/boot/dts/rockchip/rk3568.dtsi
> +++ b/arch/arm64/boot/dts/rockchip/rk3568.dtsi
> @@ -3,11 +3,99 @@
>   * Copyright (c) 2021 Rockchip Electronics Co., Ltd.
>   */
>  
> -#include "rk356x.dtsi"
> +#include "rk356x-base.dtsi"
>  
>  / {
>  	compatible = "rockchip,rk3568";
>  
> +	cpu0_opp_table: opp-table-0 {
> +		compatible = "operating-points-v2";
> +		opp-shared;
> +
> +		opp-408000000 {
> +			opp-hz = /bits/ 64 <408000000>;
> +			opp-microvolt = <850000 850000 1150000>;
> +			clock-latency-ns = <40000>;
> +		};
> +
> +		opp-600000000 {
> +			opp-hz = /bits/ 64 <600000000>;
> +			opp-microvolt = <850000 850000 1150000>;
> +			clock-latency-ns = <40000>;
> +		};
> +
> +		opp-816000000 {
> +			opp-hz = /bits/ 64 <816000000>;
> +			opp-microvolt = <850000 850000 1150000>;
> +			clock-latency-ns = <40000>;
> +			opp-suspend;
> +		};
> +
> +		opp-1104000000 {
> +			opp-hz = /bits/ 64 <1104000000>;
> +			opp-microvolt = <900000 900000 1150000>;
> +			clock-latency-ns = <40000>;
> +		};
> +
> +		opp-1416000000 {
> +			opp-hz = /bits/ 64 <1416000000>;
> +			opp-microvolt = <1025000 1025000 1150000>;
> +			clock-latency-ns = <40000>;
> +		};
> +
> +		opp-1608000000 {
> +			opp-hz = /bits/ 64 <1608000000>;
> +			opp-microvolt = <1100000 1100000 1150000>;
> +			clock-latency-ns = <40000>;
> +		};
> +
> +		opp-1800000000 {
> +			opp-hz = /bits/ 64 <1800000000>;
> +			opp-microvolt = <1150000 1150000 1150000>;
> +			clock-latency-ns = <40000>;
> +		};
> +
> +		opp-1992000000 {
> +			opp-hz = /bits/ 64 <1992000000>;
> +			opp-microvolt = <1150000 1150000 1150000>;
> +			clock-latency-ns = <40000>;
> +		};
> +	};
> +
> +	gpu_opp_table: opp-table-1 {
> +		compatible = "operating-points-v2";
> +
> +		opp-200000000 {
> +			opp-hz = /bits/ 64 <200000000>;
> +			opp-microvolt = <850000 850000 1000000>;
> +		};
> +
> +		opp-300000000 {
> +			opp-hz = /bits/ 64 <300000000>;
> +			opp-microvolt = <850000 850000 1000000>;
> +		};
> +
> +		opp-400000000 {
> +			opp-hz = /bits/ 64 <400000000>;
> +			opp-microvolt = <850000 850000 1000000>;
> +		};
> +
> +		opp-600000000 {
> +			opp-hz = /bits/ 64 <600000000>;
> +			opp-microvolt = <900000 900000 1000000>;
> +		};
> +
> +		opp-700000000 {
> +			opp-hz = /bits/ 64 <700000000>;
> +			opp-microvolt = <950000 950000 1000000>;
> +		};
> +
> +		opp-800000000 {
> +			opp-hz = /bits/ 64 <800000000>;
> +			opp-microvolt = <1000000 1000000 1000000>;
> +		};
> +	};
> +
>  	sata0: sata@fc000000 {
>  		compatible = "rockchip,rk3568-dwc-ahci", "snps,dwc-ahci";
>  		reg = <0 0xfc000000 0 0x1000>;
> @@ -269,12 +357,24 @@ combphy0: phy@fe820000 {
>  	};
>  };
>  
> -&cpu0_opp_table {
> -	opp-1992000000 {
> -		opp-hz = /bits/ 64 <1992000000>;
> -		opp-microvolt = <1150000 1150000 1150000>;
> -		clock-latency-ns = <40000>;
> -	};
> +&cpu0 {
> +	operating-points-v2 = <&cpu0_opp_table>;
> +};
> +
> +&cpu1 {
> +	operating-points-v2 = <&cpu0_opp_table>;
> +};
> +
> +&cpu2 {
> +	operating-points-v2 = <&cpu0_opp_table>;
> +};
> +
> +&cpu3 {
> +	operating-points-v2 = <&cpu0_opp_table>;
> +};
> +
> +&gpu {
> +	operating-points-v2 = <&gpu_opp_table>;
>  };
>  
>  &pipegrf {
> diff --git a/arch/arm64/boot/dts/rockchip/rk356x.dtsi b/arch/arm64/boot/dts/rockchip/rk356x-base.dtsi
> similarity index 96%
> rename from arch/arm64/boot/dts/rockchip/rk356x.dtsi
> rename to arch/arm64/boot/dts/rockchip/rk356x-base.dtsi
> index 534593f2ed0b..62be06f3b863 100644
> --- a/arch/arm64/boot/dts/rockchip/rk356x.dtsi
> +++ b/arch/arm64/boot/dts/rockchip/rk356x-base.dtsi
> @@ -56,7 +56,6 @@ cpu0: cpu@0 {
>  			clocks = <&scmi_clk 0>;
>  			#cooling-cells = <2>;
>  			enable-method = "psci";
> -			operating-points-v2 = <&cpu0_opp_table>;
>  			i-cache-size = <0x8000>;
>  			i-cache-line-size = <64>;
>  			i-cache-sets = <128>;
> @@ -72,7 +71,6 @@ cpu1: cpu@100 {
>  			reg = <0x0 0x100>;
>  			#cooling-cells = <2>;
>  			enable-method = "psci";
> -			operating-points-v2 = <&cpu0_opp_table>;
>  			i-cache-size = <0x8000>;
>  			i-cache-line-size = <64>;
>  			i-cache-sets = <128>;
> @@ -88,7 +86,6 @@ cpu2: cpu@200 {
>  			reg = <0x0 0x200>;
>  			#cooling-cells = <2>;
>  			enable-method = "psci";
> -			operating-points-v2 = <&cpu0_opp_table>;
>  			i-cache-size = <0x8000>;
>  			i-cache-line-size = <64>;
>  			i-cache-sets = <128>;
> @@ -104,7 +101,6 @@ cpu3: cpu@300 {
>  			reg = <0x0 0x300>;
>  			#cooling-cells = <2>;
>  			enable-method = "psci";
> -			operating-points-v2 = <&cpu0_opp_table>;
>  			i-cache-size = <0x8000>;
>  			i-cache-line-size = <64>;
>  			i-cache-sets = <128>;
> @@ -128,54 +124,6 @@ l3_cache: l3-cache {
>  		cache-sets = <512>;
>  	};
>  
> -	cpu0_opp_table: opp-table-0 {
> -		compatible = "operating-points-v2";
> -		opp-shared;
> -
> -		opp-408000000 {
> -			opp-hz = /bits/ 64 <408000000>;
> -			opp-microvolt = <850000 850000 1150000>;
> -			clock-latency-ns = <40000>;
> -		};
> -
> -		opp-600000000 {
> -			opp-hz = /bits/ 64 <600000000>;
> -			opp-microvolt = <850000 850000 1150000>;
> -			clock-latency-ns = <40000>;
> -		};
> -
> -		opp-816000000 {
> -			opp-hz = /bits/ 64 <816000000>;
> -			opp-microvolt = <850000 850000 1150000>;
> -			clock-latency-ns = <40000>;
> -			opp-suspend;
> -		};
> -
> -		opp-1104000000 {
> -			opp-hz = /bits/ 64 <1104000000>;
> -			opp-microvolt = <900000 900000 1150000>;
> -			clock-latency-ns = <40000>;
> -		};
> -
> -		opp-1416000000 {
> -			opp-hz = /bits/ 64 <1416000000>;
> -			opp-microvolt = <1025000 1025000 1150000>;
> -			clock-latency-ns = <40000>;
> -		};
> -
> -		opp-1608000000 {
> -			opp-hz = /bits/ 64 <1608000000>;
> -			opp-microvolt = <1100000 1100000 1150000>;
> -			clock-latency-ns = <40000>;
> -		};
> -
> -		opp-1800000000 {
> -			opp-hz = /bits/ 64 <1800000000>;
> -			opp-microvolt = <1150000 1150000 1150000>;
> -			clock-latency-ns = <40000>;
> -		};
> -	};
> -
>  	display_subsystem: display-subsystem {
>  		compatible = "rockchip,display-subsystem";
>  		ports = <&vop_out>;
> @@ -196,40 +144,6 @@ scmi_clk: protocol@14 {
>  		};
>  	};
>  
> -	gpu_opp_table: opp-table-1 {
> -		compatible = "operating-points-v2";
> -
> -		opp-200000000 {
> -			opp-hz = /bits/ 64 <200000000>;
> -			opp-microvolt = <850000 850000 1000000>;
> -		};
> -
> -		opp-300000000 {
> -			opp-hz = /bits/ 64 <300000000>;
> -			opp-microvolt = <850000 850000 1000000>;
> -		};
> -
> -		opp-400000000 {
> -			opp-hz = /bits/ 64 <400000000>;
> -			opp-microvolt = <850000 850000 1000000>;
> -		};
> -
> -		opp-600000000 {
> -			opp-hz = /bits/ 64 <600000000>;
> -			opp-microvolt = <900000 900000 1000000>;
> -		};
> -
> -		opp-700000000 {
> -			opp-hz = /bits/ 64 <700000000>;
> -			opp-microvolt = <950000 950000 1000000>;
> -		};
> -
> -		opp-800000000 {
> -			opp-hz = /bits/ 64 <800000000>;
> -			opp-microvolt = <1000000 1000000 1000000>;
> -		};
> -	};
> -
>  	hdmi_sound: hdmi-sound {
>  		compatible = "simple-audio-card";
>  		simple-audio-card,name = "HDMI";
> @@ -635,7 +549,6 @@ gpu: gpu@fde60000 {
>  		clocks = <&scmi_clk 1>, <&cru CLK_GPU>;
>  		clock-names = "gpu", "bus";
>  		#cooling-cells = <2>;
> -		operating-points-v2 = <&gpu_opp_table>;
>  		power-domains = <&power RK3568_PD_GPU>;
>  		status = "disabled";
>  	};
>
> _______________________________________________
> Linux-rockchip mailing list
> Linux-rockchip@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-rockchip


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH 3/3] arm64: dts: rockchip: Add new SoC dtsi for the RK3566T variant

[Diederik de Haas]

[-- Attachment #1: Type: text/plain, Size: 7233 bytes --]

Hi Dragan,

On Sat Oct 12, 2024 at 7:04 PM CEST, Dragan Simic wrote:
> Add new SoC dtsi file for the RK3566T variant of the Rockchip RK3566 SoC.
> The difference between the RK3566T variant and the "full-fat" RK3566 variant
> is in fewer supported CPU and GPU OPPs on the RK3566T, and in the absence of
> a functional NPU, which we currently don't have to worry about.
>
> Examples of the boards based on the RK3566T include the Pine64 Quartz64 Zero
> SBC, [2] the Radxa ROCK 3C and the Radxa ZERO 3E/3W SBCs.  Unfortunately,
> Radxa doesn't mention the use of RK3566T officially, but its official SBC
> specifications do state that the maximum frequency for the Cortex-A55 cores
> on those SBCs is lower than the "full-fat" RK3566's 1.8 GHz, which makes
> spotting the presence of the RK3566T SoC variant rather easy. [3][4][5]  An
> additional, helpful cue is that Radxa handles the CPU and GPU OPPs for the
> RK3566T variant separately in its downstream kernel. [6]
>
> The CPU and GPU OPPs supported on the RK3566T SoC variant are taken from the
> vendor kernel source, [1] which uses the values of the "opp-supported-hw" OPP
> properties to determine which ones are supported on a particular SoC variant.
> The actual values of the "opp-supported-hw" properties make it rather easy
> to see what OPPs are supported on the RK3566T SoC variant, but that, rather
> unfortunately, clashes with the maximum frequencies advertised officially
> for the Cortex-A55 CPU cores on the above-mentioned SBCs. [2][3][4][5]  The
> vendor kernel source indicates that the maximum frequency for the CPU cores
> is 1.4 GHz, while the SBC specifications state that to be 1.6 GHz.  Unless
> that discrepancy is resolved somehow, let's take the safe approach and use
> the lower maximum frequency for the CPU cores.
>
> Update the dts files of the currently supported RK3566T-based boards to use
> the new SoC dtsi for the RK3566T variant.  This actually takes the CPU cores
> and the GPUs found on these boards out of their earlier overclocks, but it
> also means that the officially advertised specifications [2][3][4][5] of the
> highest supported frequencies for the Cortex-A55 CPU cores on these boards
> may actually be wrong, as already explained above.
>
> The correctness of the introduced changes was validated by decompiling and
> comparing all affected board dtb files before and after these changes.
>
> [1] https://raw.githubusercontent.com/rockchip-linux/kernel/f8b9431ee38ed561650be7092ab93f564598daa9/arch/arm64/boot/dts/rockchip/rk3568.dtsi
> [2] https://wiki.pine64.org/wiki/Quartz64
> [3] https://dl.radxa.com/rock3/docs/hw/3c/radxa_rock3c_product_brief.pdf
> [4] https://dl.radxa.com/zero3/docs/hw/3e/radxa_zero_3e_product_brief.pdf
> [5] https://dl.radxa.com/zero3/docs/hw/3w/radxa_zero_3w_product_brief.pdf
> [6] https://github.com/radxa/kernel/commit/2dfd51da472e7ebb5ef0d3db78f902454af826b8
>
> Cc: TL Lim <tllim@pine64.org>
> Cc: Marek Kraus <gamiee@pine64.org>
> Cc: Tom Cubie <tom@radxa.com>
> Cc: FUKAUMI Naoki <naoki@radxa.com>
> Helped-by: Nicolas Frattaroli <frattaroli.nicolas@gmail.com>
> Helped-by: Jonas Karlman <jonas@kwiboo.se>
> Signed-off-by: Dragan Simic <dsimic@manjaro.org>
> ---
>  .../dts/rockchip/rk3566-radxa-zero-3.dtsi     |  2 +-
>  .../boot/dts/rockchip/rk3566-rock-3c.dts      |  2 +-
>  arch/arm64/boot/dts/rockchip/rk3566t.dtsi     | 90 +++++++++++++++++++
>  3 files changed, 92 insertions(+), 2 deletions(-)
>  create mode 100644 arch/arm64/boot/dts/rockchip/rk3566t.dtsi
>
> diff --git a/arch/arm64/boot/dts/rockchip/rk3566-radxa-zero-3.dtsi b/arch/arm64/boot/dts/rockchip/rk3566-radxa-zero-3.dtsi
> index de390d92c35e..1ee5d96a46a1 100644
> --- a/arch/arm64/boot/dts/rockchip/rk3566-radxa-zero-3.dtsi
> +++ b/arch/arm64/boot/dts/rockchip/rk3566-radxa-zero-3.dtsi
> @@ -3,7 +3,7 @@
>  #include <dt-bindings/gpio/gpio.h>
>  #include <dt-bindings/leds/common.h>
>  #include <dt-bindings/soc/rockchip,vop2.h>
> -#include "rk3566.dtsi"
> +#include "rk3566t.dtsi"
>  
>  / {
>  	chosen {
> diff --git a/arch/arm64/boot/dts/rockchip/rk3566-rock-3c.dts b/arch/arm64/boot/dts/rockchip/rk3566-rock-3c.dts
> index f2cc086e5001..9a8f4f774dbc 100644
> --- a/arch/arm64/boot/dts/rockchip/rk3566-rock-3c.dts
> +++ b/arch/arm64/boot/dts/rockchip/rk3566-rock-3c.dts
> @@ -5,7 +5,7 @@
>  #include <dt-bindings/leds/common.h>
>  #include <dt-bindings/pinctrl/rockchip.h>
>  #include <dt-bindings/soc/rockchip,vop2.h>
> -#include "rk3566.dtsi"
> +#include "rk3566t.dtsi"
>  
>  / {
>  	model = "Radxa ROCK 3C";
> diff --git a/arch/arm64/boot/dts/rockchip/rk3566t.dtsi b/arch/arm64/boot/dts/rockchip/rk3566t.dtsi
> new file mode 100644
> index 000000000000..cd89bd3b125b
> --- /dev/null
> +++ b/arch/arm64/boot/dts/rockchip/rk3566t.dtsi
> @@ -0,0 +1,90 @@
> +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
> +
> +#include "rk3566-base.dtsi"
> +
> +/ {
> +	cpu0_opp_table: opp-table-0 {
> +		compatible = "operating-points-v2";
> +		opp-shared;
> +
> +		opp-408000000 {
> +			opp-hz = /bits/ 64 <408000000>;
> +			opp-microvolt = <850000 850000 1150000>;
> +			clock-latency-ns = <40000>;
> +		};
> +
> +		opp-600000000 {
> +			opp-hz = /bits/ 64 <600000000>;
> +			opp-microvolt = <850000 850000 1150000>;
> +			clock-latency-ns = <40000>;
> +		};
> +
> +		opp-816000000 {
> +			opp-hz = /bits/ 64 <816000000>;
> +			opp-microvolt = <850000 850000 1150000>;
> +			clock-latency-ns = <40000>;
> +			opp-suspend;
> +		};
> +

For consistency, no blank lines between the opp nodes would be nice ;)

Cheers,
  Diederik

> +		opp-1104000000 {
> +			opp-hz = /bits/ 64 <1104000000>;
> +			opp-microvolt = <900000 900000 1150000>;
> +			clock-latency-ns = <40000>;
> +		};
> +
> +		opp-1416000000 {
> +			opp-hz = /bits/ 64 <1416000000>;
> +			opp-microvolt = <1025000 1025000 1150000>;
> +			clock-latency-ns = <40000>;
> +		};
> +	};
> +
> +	gpu_opp_table: opp-table-1 {
> +		compatible = "operating-points-v2";
> +
> +		opp-200000000 {
> +			opp-hz = /bits/ 64 <200000000>;
> +			opp-microvolt = <850000 850000 1000000>;
> +		};
> +
> +		opp-300000000 {
> +			opp-hz = /bits/ 64 <300000000>;
> +			opp-microvolt = <850000 850000 1000000>;
> +		};
> +
> +		opp-400000000 {
> +			opp-hz = /bits/ 64 <400000000>;
> +			opp-microvolt = <850000 850000 1000000>;
> +		};
> +
> +		opp-600000000 {
> +			opp-hz = /bits/ 64 <600000000>;
> +			opp-microvolt = <900000 900000 1000000>;
> +		};
> +
> +		opp-700000000 {
> +			opp-hz = /bits/ 64 <700000000>;
> +			opp-microvolt = <950000 950000 1000000>;
> +		};
> +	};
> +};
> +
> +&cpu0 {
> +	operating-points-v2 = <&cpu0_opp_table>;
> +};
> +
> +&cpu1 {
> +	operating-points-v2 = <&cpu0_opp_table>;
> +};
> +
> +&cpu2 {
> +	operating-points-v2 = <&cpu0_opp_table>;
> +};
> +
> +&cpu3 {
> +	operating-points-v2 = <&cpu0_opp_table>;
> +};
> +
> +&gpu {
> +	operating-points-v2 = <&gpu_opp_table>;
> +};
>
> _______________________________________________
> Linux-rockchip mailing list
> Linux-rockchip@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-rockchip


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH 1/3] arm64: dts: rockchip: Update CPU OPP voltages in RK356x SoC dtsi

[Dragan Simic]

Hello Diederik,

On 2024-10-12 21:27, Diederik de Haas wrote:
> On Sat Oct 12, 2024 at 7:04 PM CEST, Dragan Simic wrote:
>> Update the lower/upper voltage limits and the exact voltages for the 
>> Rockchip
>> RK356x CPU OPPs, using the most conservative values (i.e. the highest 
>> per-OPP
>> voltages) found in the vendor kernel source. [1]
>> 
>> Using the most conservative per-OPP voltages ensures reliable CPU 
>> operation
>> regardless of the actual CPU binning, with the downside of possibly 
>> using
>> a bit more power for the CPU cores than absolutely needed.
>> 
>> Additionally, fill in the missing "clock-latency-ns" CPU OPP 
>> properties, using
>> the values found in the vendor kernel source. [1]
>> 
>> [1] 
>> https://raw.githubusercontent.com/rockchip-linux/kernel/f8b9431ee38ed561650be7092ab93f564598daa9/arch/arm64/boot/dts/rockchip/rk3568.dtsi
>> 
>> Related-to: eb665b1c06bc ("arm64: dts: rockchip: Update GPU OPP 
>> voltages in RK356x SoC dtsi")
>> Signed-off-by: Dragan Simic <dsimic@manjaro.org>
>> ---
>>  arch/arm64/boot/dts/rockchip/rk3568.dtsi |  1 +
>>  arch/arm64/boot/dts/rockchip/rk356x.dtsi | 18 ++++++++++++------
>>  2 files changed, 13 insertions(+), 6 deletions(-)
>> 
>> diff --git a/arch/arm64/boot/dts/rockchip/rk3568.dtsi 
>> b/arch/arm64/boot/dts/rockchip/rk3568.dtsi
>> index 0946310e8c12..5c54898f6ed1 100644
>> --- a/arch/arm64/boot/dts/rockchip/rk3568.dtsi
>> +++ b/arch/arm64/boot/dts/rockchip/rk3568.dtsi
>> @@ -273,6 +273,7 @@ &cpu0_opp_table {
>>  	opp-1992000000 {
>>  		opp-hz = /bits/ 64 <1992000000>;
>>  		opp-microvolt = <1150000 1150000 1150000>;
>> +		clock-latency-ns = <40000>;
>>  	};
>>  };
>> 
>> diff --git a/arch/arm64/boot/dts/rockchip/rk356x.dtsi 
>> b/arch/arm64/boot/dts/rockchip/rk356x.dtsi
>> index 0ee0ada6f0ab..534593f2ed0b 100644
>> --- a/arch/arm64/boot/dts/rockchip/rk356x.dtsi
>> +++ b/arch/arm64/boot/dts/rockchip/rk356x.dtsi
>> @@ -134,39 +134,45 @@ cpu0_opp_table: opp-table-0 {
>> 
>>  		opp-408000000 {
>>  			opp-hz = /bits/ 64 <408000000>;
>> -			opp-microvolt = <900000 900000 1150000>;
>> +			opp-microvolt = <850000 850000 1150000>;
>>  			clock-latency-ns = <40000>;
>>  		};
>> 
>>  		opp-600000000 {
>>  			opp-hz = /bits/ 64 <600000000>;
>> -			opp-microvolt = <900000 900000 1150000>;
>> +			opp-microvolt = <850000 850000 1150000>;
>> +			clock-latency-ns = <40000>;
>>  		};
>> 
>>  		opp-816000000 {
>>  			opp-hz = /bits/ 64 <816000000>;
>> -			opp-microvolt = <900000 900000 1150000>;
>> +			opp-microvolt = <850000 850000 1150000>;
>> +			clock-latency-ns = <40000>;
>>  			opp-suspend;
>>  		};
> 
> While it felt a bit much to send a patch just to remove the blank lines
> between the opp nodes, this sounds like an excellent opportunity to 
> make
> it consistent with the opp list in other DT files?

Actually, my plan is to work on the SoC binning, which will involve
touching nearly every OPP in the Rockchip DTs, and will add much more
data to each OPP node.  Thus, having empty lines as the separators
between the OPP nodes is something we should actually want, because
not having them will actually reduce the readability after the size
of the individual OPP nodes is increased.

That's the reason why I opted for having the separator lines in this
patch series, i.e. because having them everywhere should be the final
outcome, and because in this case they were already present where the
OPPs were moved or copied from.

Re: [PATCH 2/3] arm64: dts: rockchip: Prepare RK356x SoC dtsi files for per-variant OPPs

[Dragan Simic]

Hello Diederik,

On 2024-10-12 21:41, Diederik de Haas wrote:
> On Sat Oct 12, 2024 at 7:04 PM CEST, Dragan Simic wrote:
>> Rename the Rockchip RK356x SoC dtsi files and, consequently, adjust 
>> their
>> contents appropriately, to prepare them for the ability to specify 
>> different
>> CPU and GPU OPPs for each of the supported RK356x SoC variants.
>> 
>> The first new RK356x SoC variant to be introduced is the RK3566T, 
>> which the
>> Pine64 Quartz64 Zero SBC is officially based on. [1]  Some other SBCs 
>> are
>> also based on the RK3566T variant, including Radxa ROCK 3C and ZERO 
>> 3E/3W,
>> but the slight trouble is that Radxa doesn't state that officially.  
>> Though,
>> it's rather easy to spot the RK3566T on such boards, because their 
>> official
>> specifications state that the maximum frequency for the Cortex-A55 
>> cores is
>> lower than the "full-fat" RK3566's 1.8 GHz. [2][3][4]
> 
> I think we changed terminology from "full-fat" to something else in the
> rk3588 variant? Would be nice to be consisten.

Back then, it was about the naming of one of the dtsi files, [*] not
about using "full-fat" in the commit description.  Using "full-fat"
in one of the file names was just part of the RFC, as a temporary
solution.  OTOH, frankly, I don't feel like using "full-fat" in this
commit description is inappropriate or inconsistent.

[*] 
https://lore.kernel.org/linux-rockchip/673dcf47596e7bc8ba065034e339bb1bbf9cdcb0.1716948159.git.dsimic@manjaro.org/T/#u

>> These changes follow the approach used for the Rockchip RK3588 SoC 
>> variants,
>> which was introduced and described further in commit def88eb4d836 
>> ("arm64:
>> dts: rockchip: Prepare RK3588 SoC dtsi files for per-variant OPPs").  
>> Please
>> see that commit for a more detailed explanation.
>> 
>> No functional changes are introduced, which was validated by 
>> decompiling and
> 
> No functional changes ...

This will be covered later in my response...

>> comparing all affected board dtb files before and after these changes. 
>>  In
>> more detail, the affected dtb files have some of their blocks shuffled 
>> around
>> a bit and some of their phandles have different values, as a result of 
>> the
>> changes to the order in which the building blocks from the parent dtsi 
>> files
>> are included, but they effectively remain the same as the originals.
>> 
>> [1] https://wiki.pine64.org/wiki/Quartz64
>> [2] 
>> https://dl.radxa.com/rock3/docs/hw/3c/radxa_rock3c_product_brief.pdf
>> [3] 
>> https://dl.radxa.com/zero3/docs/hw/3e/radxa_zero_3e_product_brief.pdf
>> [4] 
>> https://dl.radxa.com/zero3/docs/hw/3w/radxa_zero_3w_product_brief.pdf
>> 
>> Related-to: def88eb4d836 ("arm64: dts: rockchip: Prepare RK3588 SoC 
>> dtsi files for per-variant OPPs")
>> Signed-off-by: Dragan Simic <dsimic@manjaro.org>
>> ---
>>  .../{rk3566.dtsi => rk3566-base.dtsi}         |   2 +-
>>  arch/arm64/boot/dts/rockchip/rk3566.dtsi      | 116 
>> ++++++++++++++----
>>  arch/arm64/boot/dts/rockchip/rk3568.dtsi      | 114 +++++++++++++++--
>>  .../{rk356x.dtsi => rk356x-base.dtsi}         |  87 -------------
>>  4 files changed, 202 insertions(+), 117 deletions(-)
>>  copy arch/arm64/boot/dts/rockchip/{rk3566.dtsi => rk3566-base.dtsi} 
>> (95%)
>>  rename arch/arm64/boot/dts/rockchip/{rk356x.dtsi => rk356x-base.dtsi} 
>> (96%)
>> 
>> diff --git a/arch/arm64/boot/dts/rockchip/rk3566.dtsi 
>> b/arch/arm64/boot/dts/rockchip/rk3566-base.dtsi
>> similarity index 95%
>> copy from arch/arm64/boot/dts/rockchip/rk3566.dtsi
>> copy to arch/arm64/boot/dts/rockchip/rk3566-base.dtsi
>> index 6c4b17d27bdc..e56e0b6ba941 100644
>> --- a/arch/arm64/boot/dts/rockchip/rk3566.dtsi
>> +++ b/arch/arm64/boot/dts/rockchip/rk3566-base.dtsi
>> @@ -1,6 +1,6 @@
>>  // SPDX-License-Identifier: (GPL-2.0+ OR MIT)
>> 
>> -#include "rk356x.dtsi"
>> +#include "rk356x-base.dtsi"
>> 
>>  / {
>>  	compatible = "rockchip,rk3566";
>> diff --git a/arch/arm64/boot/dts/rockchip/rk3566.dtsi 
>> b/arch/arm64/boot/dts/rockchip/rk3566.dtsi
>> index 6c4b17d27bdc..3fcca79279f7 100644
>> --- a/arch/arm64/boot/dts/rockchip/rk3566.dtsi
>> +++ b/arch/arm64/boot/dts/rockchip/rk3566.dtsi
>> @@ -1,35 +1,107 @@
>>  // SPDX-License-Identifier: (GPL-2.0+ OR MIT)
>> 
>> -#include "rk356x.dtsi"
>> +#include "rk3566-base.dtsi"
>> 
>>  / {
>> -	compatible = "rockchip,rk3566";
>> +	cpu0_opp_table: opp-table-0 {
>> +		compatible = "operating-points-v2";
>> +		opp-shared;
>> +
>> +		opp-408000000 {
>> +			opp-hz = /bits/ 64 <408000000>;
>> +			opp-microvolt = <850000 850000 1150000>;
>> +			clock-latency-ns = <40000>;
>> +		};
>> +
>> +		opp-600000000 {
>> +			opp-hz = /bits/ 64 <600000000>;
>> +			opp-microvolt = <850000 850000 1150000>;
>> +			clock-latency-ns = <40000>;
>> +		};
>> +
>> +		opp-816000000 {
>> +			opp-hz = /bits/ 64 <816000000>;
>> +			opp-microvolt = <850000 850000 1150000>;
>> +			clock-latency-ns = <40000>;
>> +			opp-suspend;
>> +		};
> 
> Just like with patch 1 of this series, drop the blank line?

I believe I've already explained the reasoning behind that. [**]

[**] 
https://lore.kernel.org/linux-rockchip/0a1f13d06ec3668c136997e72d0aea44@manjaro.org/

>> +
>> +		opp-1104000000 {
>> +			opp-hz = /bits/ 64 <1104000000>;
>> +			opp-microvolt = <900000 900000 1150000>;
>> +			clock-latency-ns = <40000>;
>> +		};
>> +
>> +		opp-1416000000 {
>> +			opp-hz = /bits/ 64 <1416000000>;
>> +			opp-microvolt = <1025000 1025000 1150000>;
>> +			clock-latency-ns = <40000>;
>> +		};
>> +
>> +		opp-1608000000 {
>> +			opp-hz = /bits/ 64 <1608000000>;
>> +			opp-microvolt = <1100000 1100000 1150000>;
>> +			clock-latency-ns = <40000>;
>> +		};
>> +
>> +		opp-1800000000 {
>> +			opp-hz = /bits/ 64 <1800000000>;
>> +			opp-microvolt = <1150000 1150000 1150000>;
>> +			clock-latency-ns = <40000>;
>> +		};
>> +	};
>> +
>> +	gpu_opp_table: opp-table-1 {
>> +		compatible = "operating-points-v2";
>> +
>> +		opp-200000000 {
>> +			opp-hz = /bits/ 64 <200000000>;
>> +			opp-microvolt = <850000 850000 1000000>;
>> +		};
>> +
>> +		opp-300000000 {
>> +			opp-hz = /bits/ 64 <300000000>;
>> +			opp-microvolt = <850000 850000 1000000>;
>> +		};
>> +
>> +		opp-400000000 {
>> +			opp-hz = /bits/ 64 <400000000>;
>> +			opp-microvolt = <850000 850000 1000000>;
>> +		};
>> +
>> +		opp-600000000 {
>> +			opp-hz = /bits/ 64 <600000000>;
>> +			opp-microvolt = <900000 900000 1000000>;
>> +		};
>> +
>> +		opp-700000000 {
>> +			opp-hz = /bits/ 64 <700000000>;
>> +			opp-microvolt = <950000 950000 1000000>;
>> +		};
>> +
>> +		opp-800000000 {
>> +			opp-hz = /bits/ 64 <800000000>;
>> +			opp-microvolt = <1000000 1000000 1000000>;
>> +		};
>> +	};
>>  };
>> 
>> -&pipegrf {
>> -	compatible = "rockchip,rk3566-pipe-grf", "syscon";
> 
> This seems unrelated?

Yes, it looks completely out of place, but it's just the way this
diff ended up looking like.  It's actually fine.

>> +&cpu0 {
>> +	operating-points-v2 = <&cpu0_opp_table>;
>>  };
>> 
>> -&power {
>> -	power-domain@RK3568_PD_PIPE {
>> -		reg = <RK3568_PD_PIPE>;
>> -		clocks = <&cru PCLK_PIPE>;
>> -		pm_qos = <&qos_pcie2x1>,
>> -			 <&qos_sata1>,
>> -			 <&qos_sata2>,
>> -			 <&qos_usb3_0>,
>> -			 <&qos_usb3_1>;
>> -		#power-domain-cells = <0>;
>> -	};
> 
> This seems unrelated to me and possibly a functional change?
> If this was intended, then a description in the commit message would be
> nice why this is appropriate and possibly moved to a separate patch?

Just another instance of the diff ending up looking strange,
while there are actually no such changes.

>> +&cpu1 {
>> +	operating-points-v2 = <&cpu0_opp_table>;
>> +};
>> +
>> +&cpu2 {
>> +	operating-points-v2 = <&cpu0_opp_table>;
>>  };
>> 
>> -&usb_host0_xhci {
>> -	phys = <&usb2phy0_otg>;
>> -	phy-names = "usb2-phy";
>> -	extcon = <&usb2phy0>;
>> -	maximum-speed = "high-speed";
> 
> This also looks unrelated and a functional change?

Already explained above.

>> +&cpu3 {
>> +	operating-points-v2 = <&cpu0_opp_table>;
>>  };
>> 
>> -&vop {
>> -	compatible = "rockchip,rk3566-vop";
> 
> This also looks unrelated?

Already explained above.

Re: [PATCH 1/3] arm64: dts: rockchip: Update CPU OPP voltages in RK356x SoC dtsi

[Diederik de Haas]

[-- Attachment #1: Type: text/plain, Size: 4154 bytes --]

Hi Dragan,

On Sat Oct 12, 2024 at 9:45 PM CEST, Dragan Simic wrote:
> On 2024-10-12 21:27, Diederik de Haas wrote:
> > On Sat Oct 12, 2024 at 7:04 PM CEST, Dragan Simic wrote:
> >> Update the lower/upper voltage limits and the exact voltages for the 
> >> Rockchip
> >> RK356x CPU OPPs, using the most conservative values (i.e. the highest 
> >> per-OPP
> >> voltages) found in the vendor kernel source. [1]
> >> 
> >> Using the most conservative per-OPP voltages ensures reliable CPU 
> >> operation
> >> regardless of the actual CPU binning, with the downside of possibly 
> >> using
> >> a bit more power for the CPU cores than absolutely needed.
> >> 
> >> Additionally, fill in the missing "clock-latency-ns" CPU OPP 
> >> properties, using
> >> the values found in the vendor kernel source. [1]
> >> 
> >> [1] 
> >> https://raw.githubusercontent.com/rockchip-linux/kernel/f8b9431ee38ed561650be7092ab93f564598daa9/arch/arm64/boot/dts/rockchip/rk3568.dtsi
> >> 
> >> Related-to: eb665b1c06bc ("arm64: dts: rockchip: Update GPU OPP 
> >> voltages in RK356x SoC dtsi")
> >> Signed-off-by: Dragan Simic <dsimic@manjaro.org>
> >> ---
> >>  arch/arm64/boot/dts/rockchip/rk3568.dtsi |  1 +
> >>  arch/arm64/boot/dts/rockchip/rk356x.dtsi | 18 ++++++++++++------
> >>  2 files changed, 13 insertions(+), 6 deletions(-)
> >> 
> >> diff --git a/arch/arm64/boot/dts/rockchip/rk3568.dtsi 
> >> b/arch/arm64/boot/dts/rockchip/rk3568.dtsi
> >> index 0946310e8c12..5c54898f6ed1 100644
> >> --- a/arch/arm64/boot/dts/rockchip/rk3568.dtsi
> >> +++ b/arch/arm64/boot/dts/rockchip/rk3568.dtsi
> >> @@ -273,6 +273,7 @@ &cpu0_opp_table {
> >>  	opp-1992000000 {
> >>  		opp-hz = /bits/ 64 <1992000000>;
> >>  		opp-microvolt = <1150000 1150000 1150000>;
> >> +		clock-latency-ns = <40000>;
> >>  	};
> >>  };
> >> 
> >> diff --git a/arch/arm64/boot/dts/rockchip/rk356x.dtsi 
> >> b/arch/arm64/boot/dts/rockchip/rk356x.dtsi
> >> index 0ee0ada6f0ab..534593f2ed0b 100644
> >> --- a/arch/arm64/boot/dts/rockchip/rk356x.dtsi
> >> +++ b/arch/arm64/boot/dts/rockchip/rk356x.dtsi
> >> @@ -134,39 +134,45 @@ cpu0_opp_table: opp-table-0 {
> >> 
> >>  		opp-408000000 {
> >>  			opp-hz = /bits/ 64 <408000000>;
> >> -			opp-microvolt = <900000 900000 1150000>;
> >> +			opp-microvolt = <850000 850000 1150000>;
> >>  			clock-latency-ns = <40000>;
> >>  		};
> >> 
> >>  		opp-600000000 {
> >>  			opp-hz = /bits/ 64 <600000000>;
> >> -			opp-microvolt = <900000 900000 1150000>;
> >> +			opp-microvolt = <850000 850000 1150000>;
> >> +			clock-latency-ns = <40000>;
> >>  		};
> >> 
> >>  		opp-816000000 {
> >>  			opp-hz = /bits/ 64 <816000000>;
> >> -			opp-microvolt = <900000 900000 1150000>;
> >> +			opp-microvolt = <850000 850000 1150000>;
> >> +			clock-latency-ns = <40000>;
> >>  			opp-suspend;
> >>  		};
> > 
> > While it felt a bit much to send a patch just to remove the blank lines
> > between the opp nodes, this sounds like an excellent opportunity to 
> > make it consistent with the opp list in other DT files?
>
> Actually, my plan is to work on the SoC binning, which will involve
> touching nearly every OPP in the Rockchip DTs, and will add much more
> data to each OPP node.  Thus, having empty lines as the separators
> between the OPP nodes is something we should actually want, because

As indicated in the "arm64: dts: rockchip: Add dtsi file for RK3399S SoC
variant" patch series, I do prefer the separator lines ...

> not having them will actually reduce the readability after the size
> of the individual OPP nodes is increased.
>
> That's the reason why I opted for having the separator lines in this
> patch series, i.e. because having them everywhere should be the final
> outcome, and because in this case they were already present where the
> OPPs were moved or copied from.

... but you actually removed those lines in the other patch set.

While I'm looking forward to the extra data to the OPP nodes, I don't
think the amount of properties should determine whether it should have a
separator line or not.

My 0.02

Cheers,
  Diederik

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH 3/3] arm64: dts: rockchip: Add new SoC dtsi for the RK3566T variant

[Dragan Simic]

Hello Diederik,

On 2024-10-12 21:42, Diederik de Haas wrote:
> On Sat Oct 12, 2024 at 7:04 PM CEST, Dragan Simic wrote:
>> Add new SoC dtsi file for the RK3566T variant of the Rockchip RK3566 
>> SoC.
>> The difference between the RK3566T variant and the "full-fat" RK3566 
>> variant
>> is in fewer supported CPU and GPU OPPs on the RK3566T, and in the 
>> absence of
>> a functional NPU, which we currently don't have to worry about.
>> 
>> Examples of the boards based on the RK3566T include the Pine64 
>> Quartz64 Zero
>> SBC, [2] the Radxa ROCK 3C and the Radxa ZERO 3E/3W SBCs.  
>> Unfortunately,
>> Radxa doesn't mention the use of RK3566T officially, but its official 
>> SBC
>> specifications do state that the maximum frequency for the Cortex-A55 
>> cores
>> on those SBCs is lower than the "full-fat" RK3566's 1.8 GHz, which 
>> makes
>> spotting the presence of the RK3566T SoC variant rather easy. 
>> [3][4][5]  An
>> additional, helpful cue is that Radxa handles the CPU and GPU OPPs for 
>> the
>> RK3566T variant separately in its downstream kernel. [6]
>> 
>> The CPU and GPU OPPs supported on the RK3566T SoC variant are taken 
>> from the
>> vendor kernel source, [1] which uses the values of the 
>> "opp-supported-hw" OPP
>> properties to determine which ones are supported on a particular SoC 
>> variant.
>> The actual values of the "opp-supported-hw" properties make it rather 
>> easy
>> to see what OPPs are supported on the RK3566T SoC variant, but that, 
>> rather
>> unfortunately, clashes with the maximum frequencies advertised 
>> officially
>> for the Cortex-A55 CPU cores on the above-mentioned SBCs. [2][3][4][5] 
>>  The
>> vendor kernel source indicates that the maximum frequency for the CPU 
>> cores
>> is 1.4 GHz, while the SBC specifications state that to be 1.6 GHz.  
>> Unless
>> that discrepancy is resolved somehow, let's take the safe approach and 
>> use
>> the lower maximum frequency for the CPU cores.
>> 
>> Update the dts files of the currently supported RK3566T-based boards 
>> to use
>> the new SoC dtsi for the RK3566T variant.  This actually takes the CPU 
>> cores
>> and the GPUs found on these boards out of their earlier overclocks, 
>> but it
>> also means that the officially advertised specifications [2][3][4][5] 
>> of the
>> highest supported frequencies for the Cortex-A55 CPU cores on these 
>> boards
>> may actually be wrong, as already explained above.
>> 
>> The correctness of the introduced changes was validated by decompiling 
>> and
>> comparing all affected board dtb files before and after these changes.
>> 
>> [1] 
>> https://raw.githubusercontent.com/rockchip-linux/kernel/f8b9431ee38ed561650be7092ab93f564598daa9/arch/arm64/boot/dts/rockchip/rk3568.dtsi
>> [2] https://wiki.pine64.org/wiki/Quartz64
>> [3] 
>> https://dl.radxa.com/rock3/docs/hw/3c/radxa_rock3c_product_brief.pdf
>> [4] 
>> https://dl.radxa.com/zero3/docs/hw/3e/radxa_zero_3e_product_brief.pdf
>> [5] 
>> https://dl.radxa.com/zero3/docs/hw/3w/radxa_zero_3w_product_brief.pdf
>> [6] 
>> https://github.com/radxa/kernel/commit/2dfd51da472e7ebb5ef0d3db78f902454af826b8
>> 
>> Cc: TL Lim <tllim@pine64.org>
>> Cc: Marek Kraus <gamiee@pine64.org>
>> Cc: Tom Cubie <tom@radxa.com>
>> Cc: FUKAUMI Naoki <naoki@radxa.com>
>> Helped-by: Nicolas Frattaroli <frattaroli.nicolas@gmail.com>
>> Helped-by: Jonas Karlman <jonas@kwiboo.se>
>> Signed-off-by: Dragan Simic <dsimic@manjaro.org>
>> ---
>>  .../dts/rockchip/rk3566-radxa-zero-3.dtsi     |  2 +-
>>  .../boot/dts/rockchip/rk3566-rock-3c.dts      |  2 +-
>>  arch/arm64/boot/dts/rockchip/rk3566t.dtsi     | 90 
>> +++++++++++++++++++
>>  3 files changed, 92 insertions(+), 2 deletions(-)
>>  create mode 100644 arch/arm64/boot/dts/rockchip/rk3566t.dtsi
>> 
>> diff --git a/arch/arm64/boot/dts/rockchip/rk3566-radxa-zero-3.dtsi 
>> b/arch/arm64/boot/dts/rockchip/rk3566-radxa-zero-3.dtsi
>> index de390d92c35e..1ee5d96a46a1 100644
>> --- a/arch/arm64/boot/dts/rockchip/rk3566-radxa-zero-3.dtsi
>> +++ b/arch/arm64/boot/dts/rockchip/rk3566-radxa-zero-3.dtsi
>> @@ -3,7 +3,7 @@
>>  #include <dt-bindings/gpio/gpio.h>
>>  #include <dt-bindings/leds/common.h>
>>  #include <dt-bindings/soc/rockchip,vop2.h>
>> -#include "rk3566.dtsi"
>> +#include "rk3566t.dtsi"
>> 
>>  / {
>>  	chosen {
>> diff --git a/arch/arm64/boot/dts/rockchip/rk3566-rock-3c.dts 
>> b/arch/arm64/boot/dts/rockchip/rk3566-rock-3c.dts
>> index f2cc086e5001..9a8f4f774dbc 100644
>> --- a/arch/arm64/boot/dts/rockchip/rk3566-rock-3c.dts
>> +++ b/arch/arm64/boot/dts/rockchip/rk3566-rock-3c.dts
>> @@ -5,7 +5,7 @@
>>  #include <dt-bindings/leds/common.h>
>>  #include <dt-bindings/pinctrl/rockchip.h>
>>  #include <dt-bindings/soc/rockchip,vop2.h>
>> -#include "rk3566.dtsi"
>> +#include "rk3566t.dtsi"
>> 
>>  / {
>>  	model = "Radxa ROCK 3C";
>> diff --git a/arch/arm64/boot/dts/rockchip/rk3566t.dtsi 
>> b/arch/arm64/boot/dts/rockchip/rk3566t.dtsi
>> new file mode 100644
>> index 000000000000..cd89bd3b125b
>> --- /dev/null
>> +++ b/arch/arm64/boot/dts/rockchip/rk3566t.dtsi
>> @@ -0,0 +1,90 @@
>> +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
>> +
>> +#include "rk3566-base.dtsi"
>> +
>> +/ {
>> +	cpu0_opp_table: opp-table-0 {
>> +		compatible = "operating-points-v2";
>> +		opp-shared;
>> +
>> +		opp-408000000 {
>> +			opp-hz = /bits/ 64 <408000000>;
>> +			opp-microvolt = <850000 850000 1150000>;
>> +			clock-latency-ns = <40000>;
>> +		};
>> +
>> +		opp-600000000 {
>> +			opp-hz = /bits/ 64 <600000000>;
>> +			opp-microvolt = <850000 850000 1150000>;
>> +			clock-latency-ns = <40000>;
>> +		};
>> +
>> +		opp-816000000 {
>> +			opp-hz = /bits/ 64 <816000000>;
>> +			opp-microvolt = <850000 850000 1150000>;
>> +			clock-latency-ns = <40000>;
>> +			opp-suspend;
>> +		};
>> +
> 
> For consistency, no blank lines between the opp nodes would be nice ;)

I hope the way I already explained the background [*] provides
a satisfactory explanation for this choice. :)

[*] 
https://lore.kernel.org/linux-rockchip/0a1f13d06ec3668c136997e72d0aea44@manjaro.org/

Re: [PATCH 1/3] arm64: dts: rockchip: Update CPU OPP voltages in RK356x SoC dtsi

[Dragan Simic]

Hello Diederik,

On 2024-10-12 22:02, Diederik de Haas wrote:
> On Sat Oct 12, 2024 at 9:45 PM CEST, Dragan Simic wrote:
>> On 2024-10-12 21:27, Diederik de Haas wrote:
>> > On Sat Oct 12, 2024 at 7:04 PM CEST, Dragan Simic wrote:
>> >> Update the lower/upper voltage limits and the exact voltages for the
>> >> Rockchip
>> >> RK356x CPU OPPs, using the most conservative values (i.e. the highest
>> >> per-OPP
>> >> voltages) found in the vendor kernel source. [1]
>> >>
>> >> Using the most conservative per-OPP voltages ensures reliable CPU
>> >> operation
>> >> regardless of the actual CPU binning, with the downside of possibly
>> >> using
>> >> a bit more power for the CPU cores than absolutely needed.
>> >>
>> >> Additionally, fill in the missing "clock-latency-ns" CPU OPP
>> >> properties, using
>> >> the values found in the vendor kernel source. [1]
>> >>
>> >> [1]
>> >> https://raw.githubusercontent.com/rockchip-linux/kernel/f8b9431ee38ed561650be7092ab93f564598daa9/arch/arm64/boot/dts/rockchip/rk3568.dtsi
>> >>
>> >> Related-to: eb665b1c06bc ("arm64: dts: rockchip: Update GPU OPP
>> >> voltages in RK356x SoC dtsi")
>> >> Signed-off-by: Dragan Simic <dsimic@manjaro.org>
>> >> ---
>> >>  arch/arm64/boot/dts/rockchip/rk3568.dtsi |  1 +
>> >>  arch/arm64/boot/dts/rockchip/rk356x.dtsi | 18 ++++++++++++------
>> >>  2 files changed, 13 insertions(+), 6 deletions(-)
>> >>
>> >> diff --git a/arch/arm64/boot/dts/rockchip/rk3568.dtsi
>> >> b/arch/arm64/boot/dts/rockchip/rk3568.dtsi
>> >> index 0946310e8c12..5c54898f6ed1 100644
>> >> --- a/arch/arm64/boot/dts/rockchip/rk3568.dtsi
>> >> +++ b/arch/arm64/boot/dts/rockchip/rk3568.dtsi
>> >> @@ -273,6 +273,7 @@ &cpu0_opp_table {
>> >>  	opp-1992000000 {
>> >>  		opp-hz = /bits/ 64 <1992000000>;
>> >>  		opp-microvolt = <1150000 1150000 1150000>;
>> >> +		clock-latency-ns = <40000>;
>> >>  	};
>> >>  };
>> >>
>> >> diff --git a/arch/arm64/boot/dts/rockchip/rk356x.dtsi
>> >> b/arch/arm64/boot/dts/rockchip/rk356x.dtsi
>> >> index 0ee0ada6f0ab..534593f2ed0b 100644
>> >> --- a/arch/arm64/boot/dts/rockchip/rk356x.dtsi
>> >> +++ b/arch/arm64/boot/dts/rockchip/rk356x.dtsi
>> >> @@ -134,39 +134,45 @@ cpu0_opp_table: opp-table-0 {
>> >>
>> >>  		opp-408000000 {
>> >>  			opp-hz = /bits/ 64 <408000000>;
>> >> -			opp-microvolt = <900000 900000 1150000>;
>> >> +			opp-microvolt = <850000 850000 1150000>;
>> >>  			clock-latency-ns = <40000>;
>> >>  		};
>> >>
>> >>  		opp-600000000 {
>> >>  			opp-hz = /bits/ 64 <600000000>;
>> >> -			opp-microvolt = <900000 900000 1150000>;
>> >> +			opp-microvolt = <850000 850000 1150000>;
>> >> +			clock-latency-ns = <40000>;
>> >>  		};
>> >>
>> >>  		opp-816000000 {
>> >>  			opp-hz = /bits/ 64 <816000000>;
>> >> -			opp-microvolt = <900000 900000 1150000>;
>> >> +			opp-microvolt = <850000 850000 1150000>;
>> >> +			clock-latency-ns = <40000>;
>> >>  			opp-suspend;
>> >>  		};
>> >
>> > While it felt a bit much to send a patch just to remove the blank lines
>> > between the opp nodes, this sounds like an excellent opportunity to
>> > make it consistent with the opp list in other DT files?
>> 
>> Actually, my plan is to work on the SoC binning, which will involve
>> touching nearly every OPP in the Rockchip DTs, and will add much more
>> data to each OPP node.  Thus, having empty lines as the separators
>> between the OPP nodes is something we should actually want, because
> 
> As indicated in the "arm64: dts: rockchip: Add dtsi file for RK3399S 
> SoC
> variant" patch series, I do prefer the separator lines ...

Oh, I also prefer them.  I just opted for introducing fewer changes
in the RK3399S patch series, to keep the size of the series smaller,
and to end up with easier diffing of the SoC variant dtsi files.

>> not having them will actually reduce the readability after the size
>> of the individual OPP nodes is increased.
>> 
>> That's the reason why I opted for having the separator lines in this
>> patch series, i.e. because having them everywhere should be the final
>> outcome, and because in this case they were already present where the
>> OPPs were moved or copied from.
> 
> ... but you actually removed those lines in the other patch set.

Well, I didn't remove them in place, but en route to another, much
larger file, :) which originally didn't have them.  As described
above, that was my choice to keep the things a bit more consistent
(in the "RK3399 way"), and to reduce the amount of "code churn"
a bit.  I'll try to explain it further below.

> While I'm looking forward to the extra data to the OPP nodes, I don't
> think the amount of properties should determine whether it should have 
> a
> separator line or not.

True, the size of the node shouldn't be the determining factor.
The reason why I emphasized the node size was because other people
may be a bit "allergic" to whitespace changes and such "cosmetic"
code layout changes, so justifying such changes can only help.

See, just as you're "triggered" by whitespace inconsistency, some
other people may be "triggered" by cosmetic code changes.   It's
quite tough to strike some kind of balance there. :)

Re: [PATCH 3/3] arm64: dts: rockchip: Add new SoC dtsi for the RK3566T variant

[FUKAUMI Naoki]

Hi,

On 10/13/24 02:04, Dragan Simic wrote:
> Add new SoC dtsi file for the RK3566T variant of the Rockchip RK3566 SoC.
> The difference between the RK3566T variant and the "full-fat" RK3566 variant
> is in fewer supported CPU and GPU OPPs on the RK3566T, and in the absence of
> a functional NPU, which we currently don't have to worry about.
> 
> Examples of the boards based on the RK3566T include the Pine64 Quartz64 Zero
> SBC, [2] the Radxa ROCK 3C and the Radxa ZERO 3E/3W SBCs.  Unfortunately,
> Radxa doesn't mention the use of RK3566T officially, but its official SBC
> specifications do state that the maximum frequency for the Cortex-A55 cores
> on those SBCs is lower than the "full-fat" RK3566's 1.8 GHz, which makes
> spotting the presence of the RK3566T SoC variant rather easy. [3][4][5]  An
> additional, helpful cue is that Radxa handles the CPU and GPU OPPs for the
> RK3566T variant separately in its downstream kernel. [6]
> 
> The CPU and GPU OPPs supported on the RK3566T SoC variant are taken from the
> vendor kernel source, [1] which uses the values of the "opp-supported-hw" OPP
> properties to determine which ones are supported on a particular SoC variant.
> The actual values of the "opp-supported-hw" properties make it rather easy
> to see what OPPs are supported on the RK3566T SoC variant, but that, rather
> unfortunately, clashes with the maximum frequencies advertised officially
> for the Cortex-A55 CPU cores on the above-mentioned SBCs. [2][3][4][5]  The
> vendor kernel source indicates that the maximum frequency for the CPU cores
> is 1.4 GHz, while the SBC specifications state that to be 1.6 GHz.  Unless
> that discrepancy is resolved somehow, let's take the safe approach and use
> the lower maximum frequency for the CPU cores.
> 
> Update the dts files of the currently supported RK3566T-based boards to use
> the new SoC dtsi for the RK3566T variant.  This actually takes the CPU cores
> and the GPUs found on these boards out of their earlier overclocks, but it
> also means that the officially advertised specifications [2][3][4][5] of the
> highest supported frequencies for the Cortex-A55 CPU cores on these boards
> may actually be wrong, as already explained above.
> 
> The correctness of the introduced changes was validated by decompiling and
> comparing all affected board dtb files before and after these changes.
> 
> [1] https://raw.githubusercontent.com/rockchip-linux/kernel/f8b9431ee38ed561650be7092ab93f564598daa9/arch/arm64/boot/dts/rockchip/rk3568.dtsi
> [2] https://wiki.pine64.org/wiki/Quartz64
> [3] https://dl.radxa.com/rock3/docs/hw/3c/radxa_rock3c_product_brief.pdf
> [4] https://dl.radxa.com/zero3/docs/hw/3e/radxa_zero_3e_product_brief.pdf
> [5] https://dl.radxa.com/zero3/docs/hw/3w/radxa_zero_3w_product_brief.pdf
> [6] https://github.com/radxa/kernel/commit/2dfd51da472e7ebb5ef0d3db78f902454af826b8
> 
> Cc: TL Lim <tllim@pine64.org>
> Cc: Marek Kraus <gamiee@pine64.org>
> Cc: Tom Cubie <tom@radxa.com>
> Cc: FUKAUMI Naoki <naoki@radxa.com>
> Helped-by: Nicolas Frattaroli <frattaroli.nicolas@gmail.com>
> Helped-by: Jonas Karlman <jonas@kwiboo.se>
> Signed-off-by: Dragan Simic <dsimic@manjaro.org>
> ---
>   .../dts/rockchip/rk3566-radxa-zero-3.dtsi     |  2 +-
>   .../boot/dts/rockchip/rk3566-rock-3c.dts      |  2 +-
>   arch/arm64/boot/dts/rockchip/rk3566t.dtsi     | 90 +++++++++++++++++++
>   3 files changed, 92 insertions(+), 2 deletions(-)
>   create mode 100644 arch/arm64/boot/dts/rockchip/rk3566t.dtsi
> 
> diff --git a/arch/arm64/boot/dts/rockchip/rk3566-radxa-zero-3.dtsi b/arch/arm64/boot/dts/rockchip/rk3566-radxa-zero-3.dtsi
> index de390d92c35e..1ee5d96a46a1 100644
> --- a/arch/arm64/boot/dts/rockchip/rk3566-radxa-zero-3.dtsi
> +++ b/arch/arm64/boot/dts/rockchip/rk3566-radxa-zero-3.dtsi
> @@ -3,7 +3,7 @@
>   #include <dt-bindings/gpio/gpio.h>
>   #include <dt-bindings/leds/common.h>
>   #include <dt-bindings/soc/rockchip,vop2.h>
> -#include "rk3566.dtsi"
> +#include "rk3566t.dtsi"

could you drop this change for now?

We(Radxa) think we use RK3566.

and vendor kernel[6] refers efuse value to determine it's -T or not.
can you do similar way?

>   / {
>   	chosen {
> diff --git a/arch/arm64/boot/dts/rockchip/rk3566-rock-3c.dts b/arch/arm64/boot/dts/rockchip/rk3566-rock-3c.dts
> index f2cc086e5001..9a8f4f774dbc 100644
> --- a/arch/arm64/boot/dts/rockchip/rk3566-rock-3c.dts
> +++ b/arch/arm64/boot/dts/rockchip/rk3566-rock-3c.dts
> @@ -5,7 +5,7 @@
>   #include <dt-bindings/leds/common.h>
>   #include <dt-bindings/pinctrl/rockchip.h>
>   #include <dt-bindings/soc/rockchip,vop2.h>
> -#include "rk3566.dtsi"
> +#include "rk3566t.dtsi"

same here.

Best regards,

--
FUKAUMI Naoki
Radxa Computer (Shenzhen) Co., Ltd.

>   / {
>   	model = "Radxa ROCK 3C";
> diff --git a/arch/arm64/boot/dts/rockchip/rk3566t.dtsi b/arch/arm64/boot/dts/rockchip/rk3566t.dtsi
> new file mode 100644
> index 000000000000..cd89bd3b125b
> --- /dev/null
> +++ b/arch/arm64/boot/dts/rockchip/rk3566t.dtsi
> @@ -0,0 +1,90 @@
> +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
> +
> +#include "rk3566-base.dtsi"
> +
> +/ {
> +	cpu0_opp_table: opp-table-0 {
> +		compatible = "operating-points-v2";
> +		opp-shared;
> +
> +		opp-408000000 {
> +			opp-hz = /bits/ 64 <408000000>;
> +			opp-microvolt = <850000 850000 1150000>;
> +			clock-latency-ns = <40000>;
> +		};
> +
> +		opp-600000000 {
> +			opp-hz = /bits/ 64 <600000000>;
> +			opp-microvolt = <850000 850000 1150000>;
> +			clock-latency-ns = <40000>;
> +		};
> +
> +		opp-816000000 {
> +			opp-hz = /bits/ 64 <816000000>;
> +			opp-microvolt = <850000 850000 1150000>;
> +			clock-latency-ns = <40000>;
> +			opp-suspend;
> +		};
> +
> +		opp-1104000000 {
> +			opp-hz = /bits/ 64 <1104000000>;
> +			opp-microvolt = <900000 900000 1150000>;
> +			clock-latency-ns = <40000>;
> +		};
> +
> +		opp-1416000000 {
> +			opp-hz = /bits/ 64 <1416000000>;
> +			opp-microvolt = <1025000 1025000 1150000>;
> +			clock-latency-ns = <40000>;
> +		};
> +	};
> +
> +	gpu_opp_table: opp-table-1 {
> +		compatible = "operating-points-v2";
> +
> +		opp-200000000 {
> +			opp-hz = /bits/ 64 <200000000>;
> +			opp-microvolt = <850000 850000 1000000>;
> +		};
> +
> +		opp-300000000 {
> +			opp-hz = /bits/ 64 <300000000>;
> +			opp-microvolt = <850000 850000 1000000>;
> +		};
> +
> +		opp-400000000 {
> +			opp-hz = /bits/ 64 <400000000>;
> +			opp-microvolt = <850000 850000 1000000>;
> +		};
> +
> +		opp-600000000 {
> +			opp-hz = /bits/ 64 <600000000>;
> +			opp-microvolt = <900000 900000 1000000>;
> +		};
> +
> +		opp-700000000 {
> +			opp-hz = /bits/ 64 <700000000>;
> +			opp-microvolt = <950000 950000 1000000>;
> +		};
> +	};
> +};
> +
> +&cpu0 {
> +	operating-points-v2 = <&cpu0_opp_table>;
> +};
> +
> +&cpu1 {
> +	operating-points-v2 = <&cpu0_opp_table>;
> +};
> +
> +&cpu2 {
> +	operating-points-v2 = <&cpu0_opp_table>;
> +};
> +
> +&cpu3 {
> +	operating-points-v2 = <&cpu0_opp_table>;
> +};
> +
> +&gpu {
> +	operating-points-v2 = <&gpu_opp_table>;
> +};

Re: [PATCH 3/3] arm64: dts: rockchip: Add new SoC dtsi for the RK3566T variant

[Dragan Simic]

Hello Fukaumi,

On 2024-10-14 06:38, FUKAUMI Naoki wrote:
> On 10/13/24 02:04, Dragan Simic wrote:
>> Add new SoC dtsi file for the RK3566T variant of the Rockchip RK3566 
>> SoC.
>> The difference between the RK3566T variant and the "full-fat" RK3566 
>> variant
>> is in fewer supported CPU and GPU OPPs on the RK3566T, and in the 
>> absence of
>> a functional NPU, which we currently don't have to worry about.
>> 
>> Examples of the boards based on the RK3566T include the Pine64 
>> Quartz64 Zero
>> SBC, [2] the Radxa ROCK 3C and the Radxa ZERO 3E/3W SBCs.  
>> Unfortunately,
>> Radxa doesn't mention the use of RK3566T officially, but its official 
>> SBC
>> specifications do state that the maximum frequency for the Cortex-A55 
>> cores
>> on those SBCs is lower than the "full-fat" RK3566's 1.8 GHz, which 
>> makes
>> spotting the presence of the RK3566T SoC variant rather easy. 
>> [3][4][5]  An
>> additional, helpful cue is that Radxa handles the CPU and GPU OPPs for 
>> the
>> RK3566T variant separately in its downstream kernel. [6]
>> 
>> The CPU and GPU OPPs supported on the RK3566T SoC variant are taken 
>> from the
>> vendor kernel source, [1] which uses the values of the 
>> "opp-supported-hw" OPP
>> properties to determine which ones are supported on a particular SoC 
>> variant.
>> The actual values of the "opp-supported-hw" properties make it rather 
>> easy
>> to see what OPPs are supported on the RK3566T SoC variant, but that, 
>> rather
>> unfortunately, clashes with the maximum frequencies advertised 
>> officially
>> for the Cortex-A55 CPU cores on the above-mentioned SBCs. [2][3][4][5] 
>>  The
>> vendor kernel source indicates that the maximum frequency for the CPU 
>> cores
>> is 1.4 GHz, while the SBC specifications state that to be 1.6 GHz.  
>> Unless
>> that discrepancy is resolved somehow, let's take the safe approach and 
>> use
>> the lower maximum frequency for the CPU cores.
>> 
>> Update the dts files of the currently supported RK3566T-based boards 
>> to use
>> the new SoC dtsi for the RK3566T variant.  This actually takes the CPU 
>> cores
>> and the GPUs found on these boards out of their earlier overclocks, 
>> but it
>> also means that the officially advertised specifications [2][3][4][5] 
>> of the
>> highest supported frequencies for the Cortex-A55 CPU cores on these 
>> boards
>> may actually be wrong, as already explained above.
>> 
>> The correctness of the introduced changes was validated by decompiling 
>> and
>> comparing all affected board dtb files before and after these changes.
>> 
>> [1] 
>> https://raw.githubusercontent.com/rockchip-linux/kernel/f8b9431ee38ed561650be7092ab93f564598daa9/arch/arm64/boot/dts/rockchip/rk3568.dtsi
>> [2] https://wiki.pine64.org/wiki/Quartz64
>> [3] 
>> https://dl.radxa.com/rock3/docs/hw/3c/radxa_rock3c_product_brief.pdf
>> [4] 
>> https://dl.radxa.com/zero3/docs/hw/3e/radxa_zero_3e_product_brief.pdf
>> [5] 
>> https://dl.radxa.com/zero3/docs/hw/3w/radxa_zero_3w_product_brief.pdf
>> [6] 
>> https://github.com/radxa/kernel/commit/2dfd51da472e7ebb5ef0d3db78f902454af826b8
>> 
>> Cc: TL Lim <tllim@pine64.org>
>> Cc: Marek Kraus <gamiee@pine64.org>
>> Cc: Tom Cubie <tom@radxa.com>
>> Cc: FUKAUMI Naoki <naoki@radxa.com>
>> Helped-by: Nicolas Frattaroli <frattaroli.nicolas@gmail.com>
>> Helped-by: Jonas Karlman <jonas@kwiboo.se>
>> Signed-off-by: Dragan Simic <dsimic@manjaro.org>
>> ---
>>   .../dts/rockchip/rk3566-radxa-zero-3.dtsi     |  2 +-
>>   .../boot/dts/rockchip/rk3566-rock-3c.dts      |  2 +-
>>   arch/arm64/boot/dts/rockchip/rk3566t.dtsi     | 90 
>> +++++++++++++++++++
>>   3 files changed, 92 insertions(+), 2 deletions(-)
>>   create mode 100644 arch/arm64/boot/dts/rockchip/rk3566t.dtsi
>> 
>> diff --git a/arch/arm64/boot/dts/rockchip/rk3566-radxa-zero-3.dtsi 
>> b/arch/arm64/boot/dts/rockchip/rk3566-radxa-zero-3.dtsi
>> index de390d92c35e..1ee5d96a46a1 100644
>> --- a/arch/arm64/boot/dts/rockchip/rk3566-radxa-zero-3.dtsi
>> +++ b/arch/arm64/boot/dts/rockchip/rk3566-radxa-zero-3.dtsi
>> @@ -3,7 +3,7 @@
>>   #include <dt-bindings/gpio/gpio.h>
>>   #include <dt-bindings/leds/common.h>
>>   #include <dt-bindings/soc/rockchip,vop2.h>
>> -#include "rk3566.dtsi"
>> +#include "rk3566t.dtsi"
> 
> could you drop this change for now?

This patch is also going to be used for the upcoming board dts
for the Pine64 Quartz64 Zero, so there's no need for dropping it.
The Quartz64 Zero definitely uses the RK3566T.

> We (Radxa) think we use RK3566.

Well, the available documentation for the Radxa ROCK 3C and ZERO
3E/3W boards doesn't say so; instead, everything points to the
RK3566T being used.  The referenced commit in the Radxa downstream
kernel also indicates that RK3566T is used at least on some boards.

Also, some independent testing, by reading the efuses, has showed
that at least some ROCK 3C and ZERO 3E/3W boards actually have the
RK3566T, which means that we should handle them all as having the
RK3566T, to avoid overclocking the CPU cores and the GPU.  I'll
get back to this below.

> and vendor kernel[6] refers efuse value to determine it's -T or not.
> can you do similar way?

Unfortunately not at the moment, because that would require major
changes to the way OPPs are handled in the upstream kernel.  Maybe
we can do that at some point in the future, as part of my planned
work on supporting SoC binning.

With that in place, hopefully, we could handle any ROCK 3C and ZERO
3E/3W boards that actually have the "full-fat" RK3566 variant as
such, but until then, it's much safer to treat them all as having
the RK3566T, and avoid the possible overclocking.

>>   / {
>>   	chosen {
>> diff --git a/arch/arm64/boot/dts/rockchip/rk3566-rock-3c.dts 
>> b/arch/arm64/boot/dts/rockchip/rk3566-rock-3c.dts
>> index f2cc086e5001..9a8f4f774dbc 100644
>> --- a/arch/arm64/boot/dts/rockchip/rk3566-rock-3c.dts
>> +++ b/arch/arm64/boot/dts/rockchip/rk3566-rock-3c.dts
>> @@ -5,7 +5,7 @@
>>   #include <dt-bindings/leds/common.h>
>>   #include <dt-bindings/pinctrl/rockchip.h>
>>   #include <dt-bindings/soc/rockchip,vop2.h>
>> -#include "rk3566.dtsi"
>> +#include "rk3566t.dtsi"
> 
> same here.
> 
> Best regards,
> 
> --
> FUKAUMI Naoki
> Radxa Computer (Shenzhen) Co., Ltd.
> 
>>   / {
>>   	model = "Radxa ROCK 3C";
>> diff --git a/arch/arm64/boot/dts/rockchip/rk3566t.dtsi 
>> b/arch/arm64/boot/dts/rockchip/rk3566t.dtsi
>> new file mode 100644
>> index 000000000000..cd89bd3b125b
>> --- /dev/null
>> +++ b/arch/arm64/boot/dts/rockchip/rk3566t.dtsi
>> @@ -0,0 +1,90 @@
>> +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
>> +
>> +#include "rk3566-base.dtsi"
>> +
>> +/ {
>> +	cpu0_opp_table: opp-table-0 {
>> +		compatible = "operating-points-v2";
>> +		opp-shared;
>> +
>> +		opp-408000000 {
>> +			opp-hz = /bits/ 64 <408000000>;
>> +			opp-microvolt = <850000 850000 1150000>;
>> +			clock-latency-ns = <40000>;
>> +		};
>> +
>> +		opp-600000000 {
>> +			opp-hz = /bits/ 64 <600000000>;
>> +			opp-microvolt = <850000 850000 1150000>;
>> +			clock-latency-ns = <40000>;
>> +		};
>> +
>> +		opp-816000000 {
>> +			opp-hz = /bits/ 64 <816000000>;
>> +			opp-microvolt = <850000 850000 1150000>;
>> +			clock-latency-ns = <40000>;
>> +			opp-suspend;
>> +		};
>> +
>> +		opp-1104000000 {
>> +			opp-hz = /bits/ 64 <1104000000>;
>> +			opp-microvolt = <900000 900000 1150000>;
>> +			clock-latency-ns = <40000>;
>> +		};
>> +
>> +		opp-1416000000 {
>> +			opp-hz = /bits/ 64 <1416000000>;
>> +			opp-microvolt = <1025000 1025000 1150000>;
>> +			clock-latency-ns = <40000>;
>> +		};
>> +	};
>> +
>> +	gpu_opp_table: opp-table-1 {
>> +		compatible = "operating-points-v2";
>> +
>> +		opp-200000000 {
>> +			opp-hz = /bits/ 64 <200000000>;
>> +			opp-microvolt = <850000 850000 1000000>;
>> +		};
>> +
>> +		opp-300000000 {
>> +			opp-hz = /bits/ 64 <300000000>;
>> +			opp-microvolt = <850000 850000 1000000>;
>> +		};
>> +
>> +		opp-400000000 {
>> +			opp-hz = /bits/ 64 <400000000>;
>> +			opp-microvolt = <850000 850000 1000000>;
>> +		};
>> +
>> +		opp-600000000 {
>> +			opp-hz = /bits/ 64 <600000000>;
>> +			opp-microvolt = <900000 900000 1000000>;
>> +		};
>> +
>> +		opp-700000000 {
>> +			opp-hz = /bits/ 64 <700000000>;
>> +			opp-microvolt = <950000 950000 1000000>;
>> +		};
>> +	};
>> +};
>> +
>> +&cpu0 {
>> +	operating-points-v2 = <&cpu0_opp_table>;
>> +};
>> +
>> +&cpu1 {
>> +	operating-points-v2 = <&cpu0_opp_table>;
>> +};
>> +
>> +&cpu2 {
>> +	operating-points-v2 = <&cpu0_opp_table>;
>> +};
>> +
>> +&cpu3 {
>> +	operating-points-v2 = <&cpu0_opp_table>;
>> +};
>> +
>> +&gpu {
>> +	operating-points-v2 = <&gpu_opp_table>;
>> +};

Re: [PATCH 2/3] arm64: dts: rockchip: Prepare RK356x SoC dtsi files for per-variant OPPs

[Diederik de Haas]

[-- Attachment #1: Type: text/plain, Size: 1493 bytes --]

Hi Dragan,

On Sat Oct 12, 2024 at 9:41 PM CEST, Diederik de Haas wrote:
> On Sat Oct 12, 2024 at 7:04 PM CEST, Dragan Simic wrote:
> >  
> > -&pipegrf {
> > -	compatible = "rockchip,rk3566-pipe-grf", "syscon";
>
> This seems unrelated?
>
> > +&cpu0 {
> > +	operating-points-v2 = <&cpu0_opp_table>;
> >  };
> >  
> > -&power {
> > -	power-domain@RK3568_PD_PIPE {
> > -		reg = <RK3568_PD_PIPE>;
> > -		clocks = <&cru PCLK_PIPE>;
> > -		pm_qos = <&qos_pcie2x1>,
> > -			 <&qos_sata1>,
> > -			 <&qos_sata2>,
> > -			 <&qos_usb3_0>,
> > -			 <&qos_usb3_1>;
> > -		#power-domain-cells = <0>;
> > -	};
>
> This seems unrelated to me and possibly a functional change?
> If this was intended, then a description in the commit message would be
> nice why this is appropriate and possibly moved to a separate patch?
>
> > +&cpu1 {
> > +	operating-points-v2 = <&cpu0_opp_table>;
> > +};
> > +
> > +&cpu2 {
> > +	operating-points-v2 = <&cpu0_opp_table>;
> >  };
> >  
> > -&usb_host0_xhci {
> > -	phys = <&usb2phy0_otg>;
> > -	phy-names = "usb2-phy";
> > -	extcon = <&usb2phy0>;
> > -	maximum-speed = "high-speed";
>
> This also looks unrelated and a functional change?
>
> > +&cpu3 {
> > +	operating-points-v2 = <&cpu0_opp_table>;
> >  };
> >  
> > -&vop {
> > -	compatible = "rockchip,rk3566-vop";
>
> This also looks unrelated?

It turns out I was wrong.
The elements I thought were removed, aren't removed.

Sorry for the noise.

Diederik

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH 2/3] arm64: dts: rockchip: Prepare RK356x SoC dtsi files for per-variant OPPs

[Dragan Simic]

Hello Diederik,

On 2024-10-19 20:09, Diederik de Haas wrote:
> On Sat Oct 12, 2024 at 9:41 PM CEST, Diederik de Haas wrote:
>> On Sat Oct 12, 2024 at 7:04 PM CEST, Dragan Simic wrote:
>> >
>> > -&pipegrf {
>> > -	compatible = "rockchip,rk3566-pipe-grf", "syscon";
>> 
>> This seems unrelated?
>> 
>> > +&cpu0 {
>> > +	operating-points-v2 = <&cpu0_opp_table>;
>> >  };
>> >
>> > -&power {
>> > -	power-domain@RK3568_PD_PIPE {
>> > -		reg = <RK3568_PD_PIPE>;
>> > -		clocks = <&cru PCLK_PIPE>;
>> > -		pm_qos = <&qos_pcie2x1>,
>> > -			 <&qos_sata1>,
>> > -			 <&qos_sata2>,
>> > -			 <&qos_usb3_0>,
>> > -			 <&qos_usb3_1>;
>> > -		#power-domain-cells = <0>;
>> > -	};
>> 
>> This seems unrelated to me and possibly a functional change?
>> If this was intended, then a description in the commit message would 
>> be
>> nice why this is appropriate and possibly moved to a separate patch?
>> 
>> > +&cpu1 {
>> > +	operating-points-v2 = <&cpu0_opp_table>;
>> > +};
>> > +
>> > +&cpu2 {
>> > +	operating-points-v2 = <&cpu0_opp_table>;
>> >  };
>> >
>> > -&usb_host0_xhci {
>> > -	phys = <&usb2phy0_otg>;
>> > -	phy-names = "usb2-phy";
>> > -	extcon = <&usb2phy0>;
>> > -	maximum-speed = "high-speed";
>> 
>> This also looks unrelated and a functional change?
>> 
>> > +&cpu3 {
>> > +	operating-points-v2 = <&cpu0_opp_table>;
>> >  };
>> >
>> > -&vop {
>> > -	compatible = "rockchip,rk3566-vop";
>> 
>> This also looks unrelated?
> 
> It turns out I was wrong.
> The elements I thought were removed, aren't removed.
> 
> Sorry for the noise.

No worries.  I tried to tune and adjust the patch generation
parameters as best as possible, but some parts of the produced
patches still remained slightly confusing.

By the way, a few months ago there was a discussion on the Git
mailing list about making Git perform such parameter adjustment
magic itself, which back then seemed like a good idea to me,
but after dealing with a few rather complex patches myself, I
no longer think that's actually possible.

Re: [PATCH 3/3] arm64: dts: rockchip: Add new SoC dtsi for the RK3566T variant

[Dragan Simic]

Hello Fukaumi and Tom,

On 2024-10-14 07:16, Dragan Simic wrote:
> On 2024-10-14 06:38, FUKAUMI Naoki wrote:
>> On 10/13/24 02:04, Dragan Simic wrote:
>>> Add new SoC dtsi file for the RK3566T variant of the Rockchip RK3566 
>>> SoC.
>>> The difference between the RK3566T variant and the "full-fat" RK3566 
>>> variant
>>> is in fewer supported CPU and GPU OPPs on the RK3566T, and in the 
>>> absence of
>>> a functional NPU, which we currently don't have to worry about.
>>> 
>>> Examples of the boards based on the RK3566T include the Pine64 
>>> Quartz64 Zero
>>> SBC, [2] the Radxa ROCK 3C and the Radxa ZERO 3E/3W SBCs.  
>>> Unfortunately,
>>> Radxa doesn't mention the use of RK3566T officially, but its official 
>>> SBC
>>> specifications do state that the maximum frequency for the Cortex-A55 
>>> cores
>>> on those SBCs is lower than the "full-fat" RK3566's 1.8 GHz, which 
>>> makes
>>> spotting the presence of the RK3566T SoC variant rather easy. 
>>> [3][4][5]  An
>>> additional, helpful cue is that Radxa handles the CPU and GPU OPPs 
>>> for the
>>> RK3566T variant separately in its downstream kernel. [6]
>>> 
>>> The CPU and GPU OPPs supported on the RK3566T SoC variant are taken 
>>> from the
>>> vendor kernel source, [1] which uses the values of the 
>>> "opp-supported-hw" OPP
>>> properties to determine which ones are supported on a particular SoC 
>>> variant.
>>> The actual values of the "opp-supported-hw" properties make it rather 
>>> easy
>>> to see what OPPs are supported on the RK3566T SoC variant, but that, 
>>> rather
>>> unfortunately, clashes with the maximum frequencies advertised 
>>> officially
>>> for the Cortex-A55 CPU cores on the above-mentioned SBCs. 
>>> [2][3][4][5]  The
>>> vendor kernel source indicates that the maximum frequency for the CPU 
>>> cores
>>> is 1.4 GHz, while the SBC specifications state that to be 1.6 GHz.  
>>> Unless
>>> that discrepancy is resolved somehow, let's take the safe approach 
>>> and use
>>> the lower maximum frequency for the CPU cores.
>>> 
>>> Update the dts files of the currently supported RK3566T-based boards 
>>> to use
>>> the new SoC dtsi for the RK3566T variant.  This actually takes the 
>>> CPU cores
>>> and the GPUs found on these boards out of their earlier overclocks, 
>>> but it
>>> also means that the officially advertised specifications [2][3][4][5] 
>>> of the
>>> highest supported frequencies for the Cortex-A55 CPU cores on these 
>>> boards
>>> may actually be wrong, as already explained above.
>>> 
>>> The correctness of the introduced changes was validated by 
>>> decompiling and
>>> comparing all affected board dtb files before and after these 
>>> changes.
>>> 
>>> [1] 
>>> https://raw.githubusercontent.com/rockchip-linux/kernel/f8b9431ee38ed561650be7092ab93f564598daa9/arch/arm64/boot/dts/rockchip/rk3568.dtsi
>>> [2] https://wiki.pine64.org/wiki/Quartz64
>>> [3] 
>>> https://dl.radxa.com/rock3/docs/hw/3c/radxa_rock3c_product_brief.pdf
>>> [4] 
>>> https://dl.radxa.com/zero3/docs/hw/3e/radxa_zero_3e_product_brief.pdf
>>> [5] 
>>> https://dl.radxa.com/zero3/docs/hw/3w/radxa_zero_3w_product_brief.pdf
>>> [6] 
>>> https://github.com/radxa/kernel/commit/2dfd51da472e7ebb5ef0d3db78f902454af826b8
>>> 
>>> Cc: TL Lim <tllim@pine64.org>
>>> Cc: Marek Kraus <gamiee@pine64.org>
>>> Cc: Tom Cubie <tom@radxa.com>
>>> Cc: FUKAUMI Naoki <naoki@radxa.com>
>>> Helped-by: Nicolas Frattaroli <frattaroli.nicolas@gmail.com>
>>> Helped-by: Jonas Karlman <jonas@kwiboo.se>
>>> Signed-off-by: Dragan Simic <dsimic@manjaro.org>
>>> ---
>>>   .../dts/rockchip/rk3566-radxa-zero-3.dtsi     |  2 +-
>>>   .../boot/dts/rockchip/rk3566-rock-3c.dts      |  2 +-
>>>   arch/arm64/boot/dts/rockchip/rk3566t.dtsi     | 90 
>>> +++++++++++++++++++
>>>   3 files changed, 92 insertions(+), 2 deletions(-)
>>>   create mode 100644 arch/arm64/boot/dts/rockchip/rk3566t.dtsi
>>> 
>>> diff --git a/arch/arm64/boot/dts/rockchip/rk3566-radxa-zero-3.dtsi 
>>> b/arch/arm64/boot/dts/rockchip/rk3566-radxa-zero-3.dtsi
>>> index de390d92c35e..1ee5d96a46a1 100644
>>> --- a/arch/arm64/boot/dts/rockchip/rk3566-radxa-zero-3.dtsi
>>> +++ b/arch/arm64/boot/dts/rockchip/rk3566-radxa-zero-3.dtsi
>>> @@ -3,7 +3,7 @@
>>>   #include <dt-bindings/gpio/gpio.h>
>>>   #include <dt-bindings/leds/common.h>
>>>   #include <dt-bindings/soc/rockchip,vop2.h>
>>> -#include "rk3566.dtsi"
>>> +#include "rk3566t.dtsi"
>> 
>> could you drop this change for now?
> 
> This patch is also going to be used for the upcoming board dts
> for the Pine64 Quartz64 Zero, so there's no need for dropping it.
> The Quartz64 Zero definitely uses the RK3566T.
> 
>> We (Radxa) think we use RK3566.
> 
> Well, the available documentation for the Radxa ROCK 3C and ZERO
> 3E/3W boards doesn't say so; instead, everything points to the
> RK3566T being used.  The referenced commit in the Radxa downstream
> kernel also indicates that RK3566T is used at least on some boards.
> 
> Also, some independent testing, by reading the efuses, has showed
> that at least some ROCK 3C and ZERO 3E/3W boards actually have the
> RK3566T, which means that we should handle them all as having the
> RK3566T, to avoid overclocking the CPU cores and the GPU.  I'll
> get back to this below.
> 
>> and vendor kernel[6] refers efuse value to determine it's -T or not.
>> can you do similar way?
> 
> Unfortunately not at the moment, because that would require major
> changes to the way OPPs are handled in the upstream kernel.  Maybe
> we can do that at some point in the future, as part of my planned
> work on supporting SoC binning.
> 
> With that in place, hopefully, we could handle any ROCK 3C and ZERO
> 3E/3W boards that actually have the "full-fat" RK3566 variant as
> such, but until then, it's much safer to treat them all as having
> the RK3566T, and avoid the possible overclocking.

Just checking, and having the subsequent discussion on IRC in mind,
are you fine with the above-proposed approach?  Please let me know
if some further clarification is needed.

>>>   / {
>>>   	chosen {
>>> diff --git a/arch/arm64/boot/dts/rockchip/rk3566-rock-3c.dts 
>>> b/arch/arm64/boot/dts/rockchip/rk3566-rock-3c.dts
>>> index f2cc086e5001..9a8f4f774dbc 100644
>>> --- a/arch/arm64/boot/dts/rockchip/rk3566-rock-3c.dts
>>> +++ b/arch/arm64/boot/dts/rockchip/rk3566-rock-3c.dts
>>> @@ -5,7 +5,7 @@
>>>   #include <dt-bindings/leds/common.h>
>>>   #include <dt-bindings/pinctrl/rockchip.h>
>>>   #include <dt-bindings/soc/rockchip,vop2.h>
>>> -#include "rk3566.dtsi"
>>> +#include "rk3566t.dtsi"
>> 
>> same here.
>> 
>>>   / {
>>>   	model = "Radxa ROCK 3C";
>>> diff --git a/arch/arm64/boot/dts/rockchip/rk3566t.dtsi 
>>> b/arch/arm64/boot/dts/rockchip/rk3566t.dtsi
>>> new file mode 100644
>>> index 000000000000..cd89bd3b125b
>>> --- /dev/null
>>> +++ b/arch/arm64/boot/dts/rockchip/rk3566t.dtsi
>>> @@ -0,0 +1,90 @@
>>> +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
>>> +
>>> +#include "rk3566-base.dtsi"
>>> +
>>> +/ {
>>> +	cpu0_opp_table: opp-table-0 {
>>> +		compatible = "operating-points-v2";
>>> +		opp-shared;
>>> +
>>> +		opp-408000000 {
>>> +			opp-hz = /bits/ 64 <408000000>;
>>> +			opp-microvolt = <850000 850000 1150000>;
>>> +			clock-latency-ns = <40000>;
>>> +		};
>>> +
>>> +		opp-600000000 {
>>> +			opp-hz = /bits/ 64 <600000000>;
>>> +			opp-microvolt = <850000 850000 1150000>;
>>> +			clock-latency-ns = <40000>;
>>> +		};
>>> +
>>> +		opp-816000000 {
>>> +			opp-hz = /bits/ 64 <816000000>;
>>> +			opp-microvolt = <850000 850000 1150000>;
>>> +			clock-latency-ns = <40000>;
>>> +			opp-suspend;
>>> +		};
>>> +
>>> +		opp-1104000000 {
>>> +			opp-hz = /bits/ 64 <1104000000>;
>>> +			opp-microvolt = <900000 900000 1150000>;
>>> +			clock-latency-ns = <40000>;
>>> +		};
>>> +
>>> +		opp-1416000000 {
>>> +			opp-hz = /bits/ 64 <1416000000>;
>>> +			opp-microvolt = <1025000 1025000 1150000>;
>>> +			clock-latency-ns = <40000>;
>>> +		};
>>> +	};
>>> +
>>> +	gpu_opp_table: opp-table-1 {
>>> +		compatible = "operating-points-v2";
>>> +
>>> +		opp-200000000 {
>>> +			opp-hz = /bits/ 64 <200000000>;
>>> +			opp-microvolt = <850000 850000 1000000>;
>>> +		};
>>> +
>>> +		opp-300000000 {
>>> +			opp-hz = /bits/ 64 <300000000>;
>>> +			opp-microvolt = <850000 850000 1000000>;
>>> +		};
>>> +
>>> +		opp-400000000 {
>>> +			opp-hz = /bits/ 64 <400000000>;
>>> +			opp-microvolt = <850000 850000 1000000>;
>>> +		};
>>> +
>>> +		opp-600000000 {
>>> +			opp-hz = /bits/ 64 <600000000>;
>>> +			opp-microvolt = <900000 900000 1000000>;
>>> +		};
>>> +
>>> +		opp-700000000 {
>>> +			opp-hz = /bits/ 64 <700000000>;
>>> +			opp-microvolt = <950000 950000 1000000>;
>>> +		};
>>> +	};
>>> +};
>>> +
>>> +&cpu0 {
>>> +	operating-points-v2 = <&cpu0_opp_table>;
>>> +};
>>> +
>>> +&cpu1 {
>>> +	operating-points-v2 = <&cpu0_opp_table>;
>>> +};
>>> +
>>> +&cpu2 {
>>> +	operating-points-v2 = <&cpu0_opp_table>;
>>> +};
>>> +
>>> +&cpu3 {
>>> +	operating-points-v2 = <&cpu0_opp_table>;
>>> +};
>>> +
>>> +&gpu {
>>> +	operating-points-v2 = <&gpu_opp_table>;
>>> +};

Re: [PATCH 3/3] arm64: dts: rockchip: Add new SoC dtsi for the RK3566T variant

[FUKAUMI Naoki]

Hi Dragan Simic,

On 10/23/24 05:13, Dragan Simic wrote:
> Hello Fukaumi and Tom,
> 
> On 2024-10-14 07:16, Dragan Simic wrote:
>> On 2024-10-14 06:38, FUKAUMI Naoki wrote:
>>> On 10/13/24 02:04, Dragan Simic wrote:
>>>> Add new SoC dtsi file for the RK3566T variant of the Rockchip RK3566 
>>>> SoC.
>>>> The difference between the RK3566T variant and the "full-fat" RK3566 
>>>> variant
>>>> is in fewer supported CPU and GPU OPPs on the RK3566T, and in the 
>>>> absence of
>>>> a functional NPU, which we currently don't have to worry about.
>>>>
>>>> Examples of the boards based on the RK3566T include the Pine64 
>>>> Quartz64 Zero
>>>> SBC, [2] the Radxa ROCK 3C and the Radxa ZERO 3E/3W SBCs. 
>>>> Unfortunately,
>>>> Radxa doesn't mention the use of RK3566T officially, but its 
>>>> official SBC
>>>> specifications do state that the maximum frequency for the 
>>>> Cortex-A55 cores
>>>> on those SBCs is lower than the "full-fat" RK3566's 1.8 GHz, which 
>>>> makes
>>>> spotting the presence of the RK3566T SoC variant rather easy. 
>>>> [3][4][5]  An
>>>> additional, helpful cue is that Radxa handles the CPU and GPU OPPs 
>>>> for the
>>>> RK3566T variant separately in its downstream kernel. [6]
>>>>
>>>> The CPU and GPU OPPs supported on the RK3566T SoC variant are taken 
>>>> from the
>>>> vendor kernel source, [1] which uses the values of the 
>>>> "opp-supported-hw" OPP
>>>> properties to determine which ones are supported on a particular SoC 
>>>> variant.
>>>> The actual values of the "opp-supported-hw" properties make it 
>>>> rather easy
>>>> to see what OPPs are supported on the RK3566T SoC variant, but that, 
>>>> rather
>>>> unfortunately, clashes with the maximum frequencies advertised 
>>>> officially
>>>> for the Cortex-A55 CPU cores on the above-mentioned SBCs. 
>>>> [2][3][4][5]  The
>>>> vendor kernel source indicates that the maximum frequency for the 
>>>> CPU cores
>>>> is 1.4 GHz, while the SBC specifications state that to be 1.6 GHz. 
>>>> Unless
>>>> that discrepancy is resolved somehow, let's take the safe approach 
>>>> and use
>>>> the lower maximum frequency for the CPU cores.
>>>>
>>>> Update the dts files of the currently supported RK3566T-based boards 
>>>> to use
>>>> the new SoC dtsi for the RK3566T variant.  This actually takes the 
>>>> CPU cores
>>>> and the GPUs found on these boards out of their earlier overclocks, 
>>>> but it
>>>> also means that the officially advertised specifications 
>>>> [2][3][4][5] of the
>>>> highest supported frequencies for the Cortex-A55 CPU cores on these 
>>>> boards
>>>> may actually be wrong, as already explained above.
>>>>
>>>> The correctness of the introduced changes was validated by 
>>>> decompiling and
>>>> comparing all affected board dtb files before and after these changes.
>>>>
>>>> [1] 
>>>> https://raw.githubusercontent.com/rockchip-linux/kernel/f8b9431ee38ed561650be7092ab93f564598daa9/arch/arm64/boot/dts/rockchip/rk3568.dtsi
>>>> [2] https://wiki.pine64.org/wiki/Quartz64
>>>> [3] 
>>>> https://dl.radxa.com/rock3/docs/hw/3c/radxa_rock3c_product_brief.pdf
>>>> [4] 
>>>> https://dl.radxa.com/zero3/docs/hw/3e/radxa_zero_3e_product_brief.pdf
>>>> [5] 
>>>> https://dl.radxa.com/zero3/docs/hw/3w/radxa_zero_3w_product_brief.pdf
>>>> [6] 
>>>> https://github.com/radxa/kernel/commit/2dfd51da472e7ebb5ef0d3db78f902454af826b8
>>>>
>>>> Cc: TL Lim <tllim@pine64.org>
>>>> Cc: Marek Kraus <gamiee@pine64.org>
>>>> Cc: Tom Cubie <tom@radxa.com>
>>>> Cc: FUKAUMI Naoki <naoki@radxa.com>
>>>> Helped-by: Nicolas Frattaroli <frattaroli.nicolas@gmail.com>
>>>> Helped-by: Jonas Karlman <jonas@kwiboo.se>
>>>> Signed-off-by: Dragan Simic <dsimic@manjaro.org>
>>>> ---
>>>>   .../dts/rockchip/rk3566-radxa-zero-3.dtsi     |  2 +-
>>>>   .../boot/dts/rockchip/rk3566-rock-3c.dts      |  2 +-
>>>>   arch/arm64/boot/dts/rockchip/rk3566t.dtsi     | 90 
>>>> +++++++++++++++++++
>>>>   3 files changed, 92 insertions(+), 2 deletions(-)
>>>>   create mode 100644 arch/arm64/boot/dts/rockchip/rk3566t.dtsi
>>>>
>>>> diff --git a/arch/arm64/boot/dts/rockchip/rk3566-radxa-zero-3.dtsi 
>>>> b/arch/arm64/boot/dts/rockchip/rk3566-radxa-zero-3.dtsi
>>>> index de390d92c35e..1ee5d96a46a1 100644
>>>> --- a/arch/arm64/boot/dts/rockchip/rk3566-radxa-zero-3.dtsi
>>>> +++ b/arch/arm64/boot/dts/rockchip/rk3566-radxa-zero-3.dtsi
>>>> @@ -3,7 +3,7 @@
>>>>   #include <dt-bindings/gpio/gpio.h>
>>>>   #include <dt-bindings/leds/common.h>
>>>>   #include <dt-bindings/soc/rockchip,vop2.h>
>>>> -#include "rk3566.dtsi"
>>>> +#include "rk3566t.dtsi"
>>>
>>> could you drop this change for now?
>>
>> This patch is also going to be used for the upcoming board dts
>> for the Pine64 Quartz64 Zero, so there's no need for dropping it.
>> The Quartz64 Zero definitely uses the RK3566T.
>>
>>> We (Radxa) think we use RK3566.
>>
>> Well, the available documentation for the Radxa ROCK 3C and ZERO
>> 3E/3W boards doesn't say so; instead, everything points to the
>> RK3566T being used.  The referenced commit in the Radxa downstream
>> kernel also indicates that RK3566T is used at least on some boards.
>>
>> Also, some independent testing, by reading the efuses, has showed
>> that at least some ROCK 3C and ZERO 3E/3W boards actually have the
>> RK3566T, which means that we should handle them all as having the
>> RK3566T, to avoid overclocking the CPU cores and the GPU.  I'll
>> get back to this below.
>>
>>> and vendor kernel[6] refers efuse value to determine it's -T or not.
>>> can you do similar way?
>>
>> Unfortunately not at the moment, because that would require major
>> changes to the way OPPs are handled in the upstream kernel.  Maybe
>> we can do that at some point in the future, as part of my planned
>> work on supporting SoC binning.
>>
>> With that in place, hopefully, we could handle any ROCK 3C and ZERO
>> 3E/3W boards that actually have the "full-fat" RK3566 variant as
>> such, but until then, it's much safer to treat them all as having
>> the RK3566T, and avoid the possible overclocking.
> 
> Just checking, and having the subsequent discussion on IRC in mind,
> are you fine with the above-proposed approach?  Please let me know
> if some further clarification is needed.

we have no objection. please do safer way.

Best regards,

--
FUKAUMI Naoki
Radxa Computer (Shenzhen) Co., Ltd.

>>>>   / {
>>>>       chosen {
>>>> diff --git a/arch/arm64/boot/dts/rockchip/rk3566-rock-3c.dts 
>>>> b/arch/arm64/boot/dts/rockchip/rk3566-rock-3c.dts
>>>> index f2cc086e5001..9a8f4f774dbc 100644
>>>> --- a/arch/arm64/boot/dts/rockchip/rk3566-rock-3c.dts
>>>> +++ b/arch/arm64/boot/dts/rockchip/rk3566-rock-3c.dts
>>>> @@ -5,7 +5,7 @@
>>>>   #include <dt-bindings/leds/common.h>
>>>>   #include <dt-bindings/pinctrl/rockchip.h>
>>>>   #include <dt-bindings/soc/rockchip,vop2.h>
>>>> -#include "rk3566.dtsi"
>>>> +#include "rk3566t.dtsi"
>>>
>>> same here.
>>>
>>>>   / {
>>>>       model = "Radxa ROCK 3C";
>>>> diff --git a/arch/arm64/boot/dts/rockchip/rk3566t.dtsi 
>>>> b/arch/arm64/boot/dts/rockchip/rk3566t.dtsi
>>>> new file mode 100644
>>>> index 000000000000..cd89bd3b125b
>>>> --- /dev/null
>>>> +++ b/arch/arm64/boot/dts/rockchip/rk3566t.dtsi
>>>> @@ -0,0 +1,90 @@
>>>> +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
>>>> +
>>>> +#include "rk3566-base.dtsi"
>>>> +
>>>> +/ {
>>>> +    cpu0_opp_table: opp-table-0 {
>>>> +        compatible = "operating-points-v2";
>>>> +        opp-shared;
>>>> +
>>>> +        opp-408000000 {
>>>> +            opp-hz = /bits/ 64 <408000000>;
>>>> +            opp-microvolt = <850000 850000 1150000>;
>>>> +            clock-latency-ns = <40000>;
>>>> +        };
>>>> +
>>>> +        opp-600000000 {
>>>> +            opp-hz = /bits/ 64 <600000000>;
>>>> +            opp-microvolt = <850000 850000 1150000>;
>>>> +            clock-latency-ns = <40000>;
>>>> +        };
>>>> +
>>>> +        opp-816000000 {
>>>> +            opp-hz = /bits/ 64 <816000000>;
>>>> +            opp-microvolt = <850000 850000 1150000>;
>>>> +            clock-latency-ns = <40000>;
>>>> +            opp-suspend;
>>>> +        };
>>>> +
>>>> +        opp-1104000000 {
>>>> +            opp-hz = /bits/ 64 <1104000000>;
>>>> +            opp-microvolt = <900000 900000 1150000>;
>>>> +            clock-latency-ns = <40000>;
>>>> +        };
>>>> +
>>>> +        opp-1416000000 {
>>>> +            opp-hz = /bits/ 64 <1416000000>;
>>>> +            opp-microvolt = <1025000 1025000 1150000>;
>>>> +            clock-latency-ns = <40000>;
>>>> +        };
>>>> +    };
>>>> +
>>>> +    gpu_opp_table: opp-table-1 {
>>>> +        compatible = "operating-points-v2";
>>>> +
>>>> +        opp-200000000 {
>>>> +            opp-hz = /bits/ 64 <200000000>;
>>>> +            opp-microvolt = <850000 850000 1000000>;
>>>> +        };
>>>> +
>>>> +        opp-300000000 {
>>>> +            opp-hz = /bits/ 64 <300000000>;
>>>> +            opp-microvolt = <850000 850000 1000000>;
>>>> +        };
>>>> +
>>>> +        opp-400000000 {
>>>> +            opp-hz = /bits/ 64 <400000000>;
>>>> +            opp-microvolt = <850000 850000 1000000>;
>>>> +        };
>>>> +
>>>> +        opp-600000000 {
>>>> +            opp-hz = /bits/ 64 <600000000>;
>>>> +            opp-microvolt = <900000 900000 1000000>;
>>>> +        };
>>>> +
>>>> +        opp-700000000 {
>>>> +            opp-hz = /bits/ 64 <700000000>;
>>>> +            opp-microvolt = <950000 950000 1000000>;
>>>> +        };
>>>> +    };
>>>> +};
>>>> +
>>>> +&cpu0 {
>>>> +    operating-points-v2 = <&cpu0_opp_table>;
>>>> +};
>>>> +
>>>> +&cpu1 {
>>>> +    operating-points-v2 = <&cpu0_opp_table>;
>>>> +};
>>>> +
>>>> +&cpu2 {
>>>> +    operating-points-v2 = <&cpu0_opp_table>;
>>>> +};
>>>> +
>>>> +&cpu3 {
>>>> +    operating-points-v2 = <&cpu0_opp_table>;
>>>> +};
>>>> +
>>>> +&gpu {
>>>> +    operating-points-v2 = <&gpu_opp_table>;
>>>> +};
> 

Re: [PATCH 3/3] arm64: dts: rockchip: Add new SoC dtsi for the RK3566T variant

[Dragan Simic]

Hello Fukaumi,

On 2024-10-23 01:30, FUKAUMI Naoki wrote:
> On 10/23/24 05:13, Dragan Simic wrote:
>> On 2024-10-14 07:16, Dragan Simic wrote:
>>> On 2024-10-14 06:38, FUKAUMI Naoki wrote:
>>>> On 10/13/24 02:04, Dragan Simic wrote:
>>>>> Add new SoC dtsi file for the RK3566T variant of the Rockchip 
>>>>> RK3566 SoC.
>>>>> The difference between the RK3566T variant and the "full-fat" 
>>>>> RK3566 variant
>>>>> is in fewer supported CPU and GPU OPPs on the RK3566T, and in the 
>>>>> absence of
>>>>> a functional NPU, which we currently don't have to worry about.
>>>>> 
>>>>> Examples of the boards based on the RK3566T include the Pine64 
>>>>> Quartz64 Zero
>>>>> SBC, [2] the Radxa ROCK 3C and the Radxa ZERO 3E/3W SBCs. 
>>>>> Unfortunately,
>>>>> Radxa doesn't mention the use of RK3566T officially, but its 
>>>>> official SBC
>>>>> specifications do state that the maximum frequency for the 
>>>>> Cortex-A55 cores
>>>>> on those SBCs is lower than the "full-fat" RK3566's 1.8 GHz, which 
>>>>> makes
>>>>> spotting the presence of the RK3566T SoC variant rather easy. 
>>>>> [3][4][5]  An
>>>>> additional, helpful cue is that Radxa handles the CPU and GPU OPPs 
>>>>> for the
>>>>> RK3566T variant separately in its downstream kernel. [6]
>>>>> 
>>>>> The CPU and GPU OPPs supported on the RK3566T SoC variant are taken 
>>>>> from the
>>>>> vendor kernel source, [1] which uses the values of the 
>>>>> "opp-supported-hw" OPP
>>>>> properties to determine which ones are supported on a particular 
>>>>> SoC variant.
>>>>> The actual values of the "opp-supported-hw" properties make it 
>>>>> rather easy
>>>>> to see what OPPs are supported on the RK3566T SoC variant, but 
>>>>> that, rather
>>>>> unfortunately, clashes with the maximum frequencies advertised 
>>>>> officially
>>>>> for the Cortex-A55 CPU cores on the above-mentioned SBCs. 
>>>>> [2][3][4][5]  The
>>>>> vendor kernel source indicates that the maximum frequency for the 
>>>>> CPU cores
>>>>> is 1.4 GHz, while the SBC specifications state that to be 1.6 GHz. 
>>>>> Unless
>>>>> that discrepancy is resolved somehow, let's take the safe approach 
>>>>> and use
>>>>> the lower maximum frequency for the CPU cores.
>>>>> 
>>>>> Update the dts files of the currently supported RK3566T-based 
>>>>> boards to use
>>>>> the new SoC dtsi for the RK3566T variant.  This actually takes the 
>>>>> CPU cores
>>>>> and the GPUs found on these boards out of their earlier overclocks, 
>>>>> but it
>>>>> also means that the officially advertised specifications 
>>>>> [2][3][4][5] of the
>>>>> highest supported frequencies for the Cortex-A55 CPU cores on these 
>>>>> boards
>>>>> may actually be wrong, as already explained above.
>>>>> 
>>>>> The correctness of the introduced changes was validated by 
>>>>> decompiling and
>>>>> comparing all affected board dtb files before and after these 
>>>>> changes.
>>>>> 
>>>>> [1] 
>>>>> https://raw.githubusercontent.com/rockchip-linux/kernel/f8b9431ee38ed561650be7092ab93f564598daa9/arch/arm64/boot/dts/rockchip/rk3568.dtsi
>>>>> [2] https://wiki.pine64.org/wiki/Quartz64
>>>>> [3] 
>>>>> https://dl.radxa.com/rock3/docs/hw/3c/radxa_rock3c_product_brief.pdf
>>>>> [4] 
>>>>> https://dl.radxa.com/zero3/docs/hw/3e/radxa_zero_3e_product_brief.pdf
>>>>> [5] 
>>>>> https://dl.radxa.com/zero3/docs/hw/3w/radxa_zero_3w_product_brief.pdf
>>>>> [6] 
>>>>> https://github.com/radxa/kernel/commit/2dfd51da472e7ebb5ef0d3db78f902454af826b8
>>>>> 
>>>>> Cc: TL Lim <tllim@pine64.org>
>>>>> Cc: Marek Kraus <gamiee@pine64.org>
>>>>> Cc: Tom Cubie <tom@radxa.com>
>>>>> Cc: FUKAUMI Naoki <naoki@radxa.com>
>>>>> Helped-by: Nicolas Frattaroli <frattaroli.nicolas@gmail.com>
>>>>> Helped-by: Jonas Karlman <jonas@kwiboo.se>
>>>>> Signed-off-by: Dragan Simic <dsimic@manjaro.org>
>>>>> ---
>>>>>   .../dts/rockchip/rk3566-radxa-zero-3.dtsi     |  2 +-
>>>>>   .../boot/dts/rockchip/rk3566-rock-3c.dts      |  2 +-
>>>>>   arch/arm64/boot/dts/rockchip/rk3566t.dtsi     | 90 
>>>>> +++++++++++++++++++
>>>>>   3 files changed, 92 insertions(+), 2 deletions(-)
>>>>>   create mode 100644 arch/arm64/boot/dts/rockchip/rk3566t.dtsi
>>>>> 
>>>>> diff --git a/arch/arm64/boot/dts/rockchip/rk3566-radxa-zero-3.dtsi 
>>>>> b/arch/arm64/boot/dts/rockchip/rk3566-radxa-zero-3.dtsi
>>>>> index de390d92c35e..1ee5d96a46a1 100644
>>>>> --- a/arch/arm64/boot/dts/rockchip/rk3566-radxa-zero-3.dtsi
>>>>> +++ b/arch/arm64/boot/dts/rockchip/rk3566-radxa-zero-3.dtsi
>>>>> @@ -3,7 +3,7 @@
>>>>>   #include <dt-bindings/gpio/gpio.h>
>>>>>   #include <dt-bindings/leds/common.h>
>>>>>   #include <dt-bindings/soc/rockchip,vop2.h>
>>>>> -#include "rk3566.dtsi"
>>>>> +#include "rk3566t.dtsi"
>>>> 
>>>> could you drop this change for now?
>>> 
>>> This patch is also going to be used for the upcoming board dts
>>> for the Pine64 Quartz64 Zero, so there's no need for dropping it.
>>> The Quartz64 Zero definitely uses the RK3566T.
>>> 
>>>> We (Radxa) think we use RK3566.
>>> 
>>> Well, the available documentation for the Radxa ROCK 3C and ZERO
>>> 3E/3W boards doesn't say so; instead, everything points to the
>>> RK3566T being used.  The referenced commit in the Radxa downstream
>>> kernel also indicates that RK3566T is used at least on some boards.
>>> 
>>> Also, some independent testing, by reading the efuses, has showed
>>> that at least some ROCK 3C and ZERO 3E/3W boards actually have the
>>> RK3566T, which means that we should handle them all as having the
>>> RK3566T, to avoid overclocking the CPU cores and the GPU.  I'll
>>> get back to this below.
>>> 
>>>> and vendor kernel[6] refers efuse value to determine it's -T or not.
>>>> can you do similar way?
>>> 
>>> Unfortunately not at the moment, because that would require major
>>> changes to the way OPPs are handled in the upstream kernel.  Maybe
>>> we can do that at some point in the future, as part of my planned
>>> work on supporting SoC binning.
>>> 
>>> With that in place, hopefully, we could handle any ROCK 3C and ZERO
>>> 3E/3W boards that actually have the "full-fat" RK3566 variant as
>>> such, but until then, it's much safer to treat them all as having
>>> the RK3566T, and avoid the possible overclocking.
>> 
>> Just checking, and having the subsequent discussion on IRC in mind,
>> are you fine with the above-proposed approach?  Please let me know
>> if some further clarification is needed.
> 
> we have no objection. please do safer way.

Great, thanks!  It's better to be on the safe side, until we get
the full support for SoC binning in the upstream kernel.

>>>>>   / {
>>>>>       chosen {
>>>>> diff --git a/arch/arm64/boot/dts/rockchip/rk3566-rock-3c.dts 
>>>>> b/arch/arm64/boot/dts/rockchip/rk3566-rock-3c.dts
>>>>> index f2cc086e5001..9a8f4f774dbc 100644
>>>>> --- a/arch/arm64/boot/dts/rockchip/rk3566-rock-3c.dts
>>>>> +++ b/arch/arm64/boot/dts/rockchip/rk3566-rock-3c.dts
>>>>> @@ -5,7 +5,7 @@
>>>>>   #include <dt-bindings/leds/common.h>
>>>>>   #include <dt-bindings/pinctrl/rockchip.h>
>>>>>   #include <dt-bindings/soc/rockchip,vop2.h>
>>>>> -#include "rk3566.dtsi"
>>>>> +#include "rk3566t.dtsi"
>>>> 
>>>> same here.
>>>> 
>>>>>   / {
>>>>>       model = "Radxa ROCK 3C";
>>>>> diff --git a/arch/arm64/boot/dts/rockchip/rk3566t.dtsi 
>>>>> b/arch/arm64/boot/dts/rockchip/rk3566t.dtsi
>>>>> new file mode 100644
>>>>> index 000000000000..cd89bd3b125b
>>>>> --- /dev/null
>>>>> +++ b/arch/arm64/boot/dts/rockchip/rk3566t.dtsi
>>>>> @@ -0,0 +1,90 @@
>>>>> +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
>>>>> +
>>>>> +#include "rk3566-base.dtsi"
>>>>> +
>>>>> +/ {
>>>>> +    cpu0_opp_table: opp-table-0 {
>>>>> +        compatible = "operating-points-v2";
>>>>> +        opp-shared;
>>>>> +
>>>>> +        opp-408000000 {
>>>>> +            opp-hz = /bits/ 64 <408000000>;
>>>>> +            opp-microvolt = <850000 850000 1150000>;
>>>>> +            clock-latency-ns = <40000>;
>>>>> +        };
>>>>> +
>>>>> +        opp-600000000 {
>>>>> +            opp-hz = /bits/ 64 <600000000>;
>>>>> +            opp-microvolt = <850000 850000 1150000>;
>>>>> +            clock-latency-ns = <40000>;
>>>>> +        };
>>>>> +
>>>>> +        opp-816000000 {
>>>>> +            opp-hz = /bits/ 64 <816000000>;
>>>>> +            opp-microvolt = <850000 850000 1150000>;
>>>>> +            clock-latency-ns = <40000>;
>>>>> +            opp-suspend;
>>>>> +        };
>>>>> +
>>>>> +        opp-1104000000 {
>>>>> +            opp-hz = /bits/ 64 <1104000000>;
>>>>> +            opp-microvolt = <900000 900000 1150000>;
>>>>> +            clock-latency-ns = <40000>;
>>>>> +        };
>>>>> +
>>>>> +        opp-1416000000 {
>>>>> +            opp-hz = /bits/ 64 <1416000000>;
>>>>> +            opp-microvolt = <1025000 1025000 1150000>;
>>>>> +            clock-latency-ns = <40000>;
>>>>> +        };
>>>>> +    };
>>>>> +
>>>>> +    gpu_opp_table: opp-table-1 {
>>>>> +        compatible = "operating-points-v2";
>>>>> +
>>>>> +        opp-200000000 {
>>>>> +            opp-hz = /bits/ 64 <200000000>;
>>>>> +            opp-microvolt = <850000 850000 1000000>;
>>>>> +        };
>>>>> +
>>>>> +        opp-300000000 {
>>>>> +            opp-hz = /bits/ 64 <300000000>;
>>>>> +            opp-microvolt = <850000 850000 1000000>;
>>>>> +        };
>>>>> +
>>>>> +        opp-400000000 {
>>>>> +            opp-hz = /bits/ 64 <400000000>;
>>>>> +            opp-microvolt = <850000 850000 1000000>;
>>>>> +        };
>>>>> +
>>>>> +        opp-600000000 {
>>>>> +            opp-hz = /bits/ 64 <600000000>;
>>>>> +            opp-microvolt = <900000 900000 1000000>;
>>>>> +        };
>>>>> +
>>>>> +        opp-700000000 {
>>>>> +            opp-hz = /bits/ 64 <700000000>;
>>>>> +            opp-microvolt = <950000 950000 1000000>;
>>>>> +        };
>>>>> +    };
>>>>> +};
>>>>> +
>>>>> +&cpu0 {
>>>>> +    operating-points-v2 = <&cpu0_opp_table>;
>>>>> +};
>>>>> +
>>>>> +&cpu1 {
>>>>> +    operating-points-v2 = <&cpu0_opp_table>;
>>>>> +};
>>>>> +
>>>>> +&cpu2 {
>>>>> +    operating-points-v2 = <&cpu0_opp_table>;
>>>>> +};
>>>>> +
>>>>> +&cpu3 {
>>>>> +    operating-points-v2 = <&cpu0_opp_table>;
>>>>> +};
>>>>> +
>>>>> +&gpu {
>>>>> +    operating-points-v2 = <&gpu_opp_table>;
>>>>> +};