[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <da32bf7d522b0b55dc574526c81cae77@manjaro.org>
Date: Sat, 12 Oct 2024 22:07:53 +0200
From: Dragan Simic <dsimic@...jaro.org>
To: Diederik de Haas <didi.debian@...ow.org>
Cc: linux-rockchip@...ts.infradead.org, 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, TL Lim <tllim@...e64.org>, Marek Kraus
<gamiee@...e64.org>, Tom Cubie <tom@...xa.com>, FUKAUMI Naoki
<naoki@...xa.com>, Nicolas Frattaroli <frattaroli.nicolas@...il.com>, Jonas
Karlman <jonas@...boo.se>
Subject: Re: [PATCH 3/3] arm64: dts: rockchip: Add new SoC dtsi for the
RK3566T variant
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@...e64.org>
>> Cc: Marek Kraus <gamiee@...e64.org>
>> Cc: Tom Cubie <tom@...xa.com>
>> Cc: FUKAUMI Naoki <naoki@...xa.com>
>> Helped-by: Nicolas Frattaroli <frattaroli.nicolas@...il.com>
>> Helped-by: Jonas Karlman <jonas@...boo.se>
>> Signed-off-by: Dragan Simic <dsimic@...jaro.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/
Powered by blists - more mailing lists