Re: [PATCH v2 2/2] arm64: dts: rockchip: Add rk3576 evb2 board
From: Quentin Schulz <hidden>
Date: 2026-01-07 15:46:36
Also in:
linux-devicetree, linux-rockchip, lkml
Hi Chaoyi, On 1/7/26 8:03 AM, Chaoyi Chen wrote: [...]
quoted hunk ↗ jump to hunk
diff --git a/arch/arm64/boot/dts/rockchip/rk3576-evb2-v10.dts b/arch/arm64/boot/dts/rockchip/rk3576-evb2-v10.dts new file mode 100644 index 000000000000..52788c514ec0 --- /dev/null +++ b/arch/arm64/boot/dts/rockchip/rk3576-evb2-v10.dts@@ -0,0 +1,997 @@ +// SPDX-License-Identifier: (GPL-2.0+ OR MIT) +/* + * Copyright (c) 2025 Rockchip Electronics Co., Ltd. + * + */ + +/dts-v1/; + +#include <dt-bindings/gpio/gpio.h> +#include <dt-bindings/input/input.h> +#include <dt-bindings/pinctrl/rockchip.h> +#include <dt-bindings/soc/rockchip,vop2.h> +#include "rk3576.dtsi" + +/ { + model = "Rockchip RK3576 EVB2 V10 Board"; + compatible = "rockchip,rk3576-evb2-v10", "rockchip,rk3576"; + + aliases { + ethernet0 = &gmac0; + ethernet1 = &gmac1; + }; + + chosen: chosen {
Why a label here? There are also many other instances of nodes being labelled but whose label is never used. I would understand for some if you want to have DTSOs working with this DTB, but here chosen really doesn't make much sense to me?
+ stdout-path = "serial0:1500000n8";
+ };
+
+ adc_keys: adc-keys {Are we expecting to extend this node from another DT? Why the label? Won't comment on all other labeled-but-no-phandle-use instances, please check.
+ vcc3v3_rtc_s5: regulator-vcc3v3-rtc-s5 {
+ compatible = "regulator-fixed";
+ regulator-name = "vcc3v3_rtc_s5";
+ regulator-boot-on;
+ regulator-always-on;
+ regulator-min-microvolt = <3300000>;
+ regulator-max-microvolt = <3300000>;
+ vin-supply = <&vcc_sys>;If this is for the rtc, shouldn't we declare this dependency in the RTC device node and not have it always-on?
+ };
+
+ vcc3v3_sata_pwren: vcc3v3-sata-pwren {
+ compatible = "regulator-fixed";
+ regulator-name = "vcc3v3_sata_pwren";
+ enable-active-high;
+ regulator-boot-on;
+ regulator-always-on;Why do we have this always-on? Seems like we're missing a dependency on this regulator in the SATA controller?
+ gpio = <&gpio4 RK_PC7 GPIO_ACTIVE_HIGH>;
+ pinctrl-names = "default";
+ pinctrl-0 = <&satapm_pwren>;
+ };
+
+ vcc5v0_device: regulator-vcc5v0-device {
+ compatible = "regulator-fixed";
+ regulator-name = "vcc5v0_device";
+ regulator-always-on;
+ regulator-boot-on;
+ regulator-min-microvolt = <5000000>;
+ regulator-max-microvolt = <5000000>;
+ vin-supply = <&vcc12v_dcin>;
+ };
+
+ vcc5v0_host: regulator-vcc5v0-host {
+ compatible = "regulator-fixed";
+ regulator-name = "vcc5v0_host";
+ regulator-boot-on;
+ regulator-always-on;
+ regulator-min-microvolt = <5000000>;
+ regulator-max-microvolt = <5000000>;
+ enable-active-high;
+ gpio = <&gpio0 RK_PC3 GPIO_ACTIVE_HIGH>;
+ vin-supply = <&vcc5v0_device>;
+ pinctrl-names = "default";
+ pinctrl-0 = <&usb_host_pwren>;
+ };
+I assume both of the above are related to USB operating in host or device mode? Maybe there's a way to have something more useful to the user in regulator-name (and possibly the regulator node name) so that they have an idea what this pertains to? Additionally, why is this always-on? I would assume the USB controller is capable of controlling its regulator(s)? [...]
+ vcc_ufs_s0: regulator-vcc-ufs-s0 {We also have another regulator for UFS that is mentioned in the UFS controller node but not this one, why?
+ compatible = "regulator-fixed"; + regulator-name = "vcc_ufs_s0"; + regulator-boot-on; + regulator-always-on;
Why always on? [...]
+&mdio0 {
+ rgmii_phy0: ethernet-phy@1 {
+ compatible = "ethernet-phy-id4f51.e91b";Is MDIO auto-detection broken such that you need to specify the PHY vendor and product id? Which PHY is that? Why can't you use c22 or c45 compatible? A comment would be nice.
+ reg = <0x1>;
+ pinctrl-names = "default";
+ pinctrl-0 = <&rgmii_phy0_rst>;
+ reset-assert-us = <20000>;
+ reset-deassert-us = <100000>;
+ reset-gpios = <&gpio3 RK_PD3 GPIO_ACTIVE_LOW>;
+ };
+};
+
+&mdio1 {
+ rgmii_phy1: ethernet-phy@1 {
+ compatible = "ethernet-phy-id4f51.e91b";Ditto. [...]
+&sdhci {
+ bus-width = <8>;
+ full-pwr-cycle-in-suspend;
+ max-frequency = <200000000>;Already that value in rk3576.dtsi.
+ mmc-hs400-1_8v;
+ mmc-hs400-enhanced-strobe;
+ no-sdio;
+ no-sd;
+ non-removable;
+ status = "okay";
+};
+
+&sdmmc {
+ bus-width = <4>;
+ cap-sd-highspeed;
+ disable-wp;
+ max-frequency = <200000000>;Already that value in rk3576.dtsi.
+ no-sdio;
+ no-mmc;
+ sd-uhs-sdr104;
+ vqmmc-supply = <&vccio_sd_s0>;
+ status = "okay";
+};
+
+&saradc {This is not alphabetically sorted, it should be before &sata0. [...]
+ bluetooth {
+ compatible = "brcm,bcm43438-bt";
+ clocks = <&hym8563>;
+ clock-names = "lpo";
+ device-wakeup-gpios = <&gpio1 RK_PD4 GPIO_ACTIVE_HIGH>;
+ interrupt-parent = <&gpio0>;
+ interrupts = <RK_PB1 IRQ_TYPE_LEVEL_HIGH>;
+ pinctrl-0 = <&bt_reg_on &bt_wake_host &host_wake_bt>;
+ pinctrl-names = "default";
+ shutdown-gpios = <&gpio1 RK_PC7 GPIO_ACTIVE_HIGH>;Is this GPIO only controlling Bluetooth or also WiFi? I've seen a few combo chips where there's a common GPIO that controls both WiFi and Bluetooth. Making this bluetooth-specific means we need Bluetooth on for WiFi to work, a bit unexpected and should probably be modeled another way. Cheers, Quentin