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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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@xxxxxxxxxxxxxx>
Signed-off-by: Finley Xiao <finley.xiao@xxxxxxxxxxxxxx>
Signed-off-by: Yifeng Zhao <yifeng.zhao@xxxxxxxxxxxxxx>
Signed-off-by: Elaine Zhang <zhangqing@xxxxxxxxxxxxxx>
[rebase, squash and reword commit message]
Signed-off-by: Detlev Casanova <detlev.casanova@xxxxxxxxxxxxx>
---

[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.




[Index of Archives]     [Kernel Newbies]     [Security]     [Netfilter]     [Bugtraq]     [Linux PPP]     [Linux FS]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Samba]     [Video 4 Linux]     [Linmodem]     [Device Mapper]     [Linux Kernel for ARM]

  Powered by Linux