linux-kernel - Re: [PATCH 09/10] arm64: dts: rockchip: Add rk3576 SoC base DT

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: <e794a247b52dd2fc10b470ed7df4d463@manjaro.org>
Date: Sat, 17 Aug 2024 05:11:30 +0200
From: Dragan Simic <dsimic@...jaro.org>
To: Detlev Casanova <detlev.casanova@...labora.com>
Cc: linux-kernel@...r.kernel.org, Rob Herring <robh@...nel.org>, Krzysztof
 Kozlowski <krzk+dt@...nel.org>, Conor Dooley <conor+dt@...nel.org>, Heiko
 Stuebner <heiko@...ech.de>, Andi Shyti <andi.shyti@...nel.org>, Jonathan
 Cameron <jic23@...nel.org>, Lars-Peter Clausen <lars@...afoo.de>, Lee Jones
 <lee@...nel.org>, Greg Kroah-Hartman <gregkh@...uxfoundation.org>, Jiri
 Slaby <jirislaby@...nel.org>, Daniel Lezcano <daniel.lezcano@...aro.org>,
 Thomas Gleixner <tglx@...utronix.de>, Chris Morgan
 <macromorgan@...mail.com>, Jonas Karlman <jonas@...boo.se>, Tim Lunn
 <tim@...thertop.org>, Muhammed Efe Cetin <efectn@...tonmail.com>, Andy Yan
 <andyshrk@....com>, Jagan Teki <jagan@...eble.ai>, Sebastian Reichel
 <sebastian.reichel@...labora.com>, Shresth Prasad
 <shresthprasad7@...il.com>, Ondrej Jirman <megi@....cz>, Weizhao Ouyang
 <weizhao.ouyang@....com>, Alexey Charkov <alchark@...il.com>, Jimmy Hon
 <honyuenkwun@...il.com>, Finley Xiao <finley.xiao@...k-chips.com>, Yifeng
 Zhao <yifeng.zhao@...k-chips.com>, Elaine Zhang <zhangqing@...k-chips.com>,
 Liang Chen <cl@...k-chips.com>, devicetree@...r.kernel.org,
 linux-arm-kernel@...ts.infradead.org, linux-rockchip@...ts.infradead.org,
 linux-i2c@...r.kernel.org, linux-iio@...r.kernel.org,
 linux-serial@...r.kernel.org, kernel@...labora.com
Subject: Re: [PATCH 09/10] arm64: dts: rockchip: Add rk3576 SoC base DT

Hello Detlev,

Please see a few comments below.

On 2024-08-02 23:45, Detlev Casanova wrote:
> This device tree contains all devices necessary for booting from 
> network
> or SD Card.
> 
> It supports CPU, CRU, PM domains, dma, interrupts, timers, UART and
> SDHCI (everything necessary to boot Linux on this system on chip) as
> well as Ethernet, I2C, SPI and OTP.
> 
> Also add the necessary DT bindings for the SoC.
> 
> Signed-off-by: Liang Chen <cl@...k-chips.com>
> Signed-off-by: Finley Xiao <finley.xiao@...k-chips.com>
> Signed-off-by: Yifeng Zhao <yifeng.zhao@...k-chips.com>
> Signed-off-by: Elaine Zhang <zhangqing@...k-chips.com>
> [rebase, squash and reword commit message]
> Signed-off-by: Detlev Casanova <detlev.casanova@...labora.com>
> ---

[snip]

> diff --git a/arch/arm64/boot/dts/rockchip/rk3576.dtsi
> b/arch/arm64/boot/dts/rockchip/rk3576.dtsi
> new file mode 100644
> index 0000000000000..00c4d2a153ced
> --- /dev/null
> +++ b/arch/arm64/boot/dts/rockchip/rk3576.dtsi
> @@ -0,0 +1,1635 @@
> +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
> +/*
> + * Copyright (c) 2023 Rockchip Electronics Co., Ltd.
> + */
> +
> +#include <dt-bindings/clock/rockchip,rk3576-cru.h>
> +#include <dt-bindings/reset/rockchip,rk3576-cru.h>
> +#include <dt-bindings/interrupt-controller/arm-gic.h>
> +#include <dt-bindings/interrupt-controller/irq.h>
> +#include <dt-bindings/phy/phy.h>
> +#include <dt-bindings/power/rk3576-power.h>
> +#include <dt-bindings/pinctrl/rockchip.h>
> +#include <dt-bindings/soc/rockchip,boot-mode.h>
> +
> +/ {
> +	compatible = "rockchip,rk3576";
> +
> +	interrupt-parent = <&gic>;
> +	#address-cells = <2>;
> +	#size-cells = <2>;
> +
> +	aliases {
> +		ethernet0 = &gmac0;
> +		ethernet1 = &gmac1;

Please remove ethernetX aliases from the SoC dtsi.  The consensus
is that those aliases need to be defined at the board level instead.

See the commit 5d90cb1edcf7 (arm64: dts: rockchip: Remove ethernet0
alias from the SoC dtsi for RK3399, 2023-12-12), for example, for
more details.

> +		gpio0 = &gpio0;
> +		gpio1 = &gpio1;
> +		gpio2 = &gpio2;
> +		gpio3 = &gpio3;
> +		gpio4 = &gpio4;
> +		i2c0 = &i2c0;
> +		i2c1 = &i2c1;
> +		i2c2 = &i2c2;
> +		i2c3 = &i2c3;
> +		i2c4 = &i2c4;
> +		i2c5 = &i2c5;
> +		i2c6 = &i2c6;
> +		i2c7 = &i2c7;
> +		i2c8 = &i2c8;
> +		i2c9 = &i2c9;
> +		serial0 = &uart0;
> +		serial1 = &uart1;
> +		serial2 = &uart2;
> +		serial3 = &uart3;
> +		serial4 = &uart4;
> +		serial5 = &uart5;
> +		serial6 = &uart6;
> +		serial7 = &uart7;
> +		serial8 = &uart8;
> +		serial9 = &uart9;
> +		serial10 = &uart10;
> +		serial11 = &uart11;
> +		spi0 = &spi0;
> +		spi1 = &spi1;
> +		spi2 = &spi2;
> +		spi3 = &spi3;
> +		spi4 = &spi4;
> +	};
> +
> +	xin32k: clock-32k {

Please use "xin32k: clock-xin32k { ... }" instead, because that follows
the recently established revised pattern for clock names.  We should 
have
come consistency in the new SoC dtsi additions.

> +		compatible = "fixed-clock";
> +		#clock-cells = <0>;
> +		clock-frequency = <32768>;
> +		clock-output-names = "xin32k";
> +	};
> +
> +	xin24m: clock-24m {
> +		compatible = "fixed-clock";
> +		#clock-cells = <0>;
> +		clock-frequency = <24000000>;
> +		clock-output-names = "xin24m";
> +	};

Please use "xin24m: clock-xin24m { ... }" instead, for the same reasons
as already described above.

> +	spll: clock-702m {
> +		compatible = "fixed-clock";
> +		#clock-cells = <0>;
> +		clock-frequency = <702000000>;
> +		clock-output-names = "spll";
> +	};

Perhaps using "spll: clock-spll { ... }" instead would also be a good
idea, because it would improve the overall consistency.