lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-Id: <D4U30AUOH6UR.1QPH47KN5EWE4@cknow.org>
Date: Sat, 12 Oct 2024 21:41:09 +0200
From: "Diederik de Haas" <didi.debian@...ow.org>
To: "Dragan Simic" <dsimic@...jaro.org>,
 <linux-rockchip@...ts.infradead.org>
Cc: <heiko@...ech.de>, <linux-arm-kernel@...ts.infradead.org>,
 <linux-kernel@...r.kernel.org>, <devicetree@...r.kernel.org>,
 <robh@...nel.org>, <krzk+dt@...nel.org>, <conor+dt@...nel.org>
Subject: Re: [PATCH 2/3] arm64: dts: rockchip: Prepare RK356x SoC dtsi files
 for per-variant OPPs

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@...jaro.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@...568_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@...00000 {
>  		compatible = "rockchip,rk3568-dwc-ahci", "snps,dwc-ahci";
>  		reg = <0 0xfc000000 0 0x1000>;
> @@ -269,12 +357,24 @@ combphy0: phy@...20000 {
>  	};
>  };
>  
> -&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@...60000 {
>  		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@...ts.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-rockchip


Download attachment "signature.asc" of type "application/pgp-signature" (229 bytes)

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ