[PATCH 3/3] arm64: dts: rockchip: Add rk3588s-orangepi-cm5-base device tree

[Jimmy Hon]

A few nitpicks below

[ snip ]
> +
> +#include "rk3588s-orangepi-cm5.dtsi"
> +
> +/ {
> +       model = "Xunlong Orange Pi CM5 Base";
> +       compatible = "xunlong,orangepi-cm5-base", "xunlong,orangepi-cm5", "rockchip,rk3588s";
> +
> +       aliases {
> +               ethernet0 = &gmac1;
> +               mmc0 = &sdhci;
Since sdhci is enabled in the SoM.dtsi, this alias should probably go
there instead.

> +               mmc1 = &sdmmc;
> +       };
> +

[ snip ]

> +
> +       vbus_5v0: vbus-5v0 {
> +               compatible = "regulator-fixed";
> +               regulator-name = "vbus_5v0";
> +               regulator-min-microvolt = <5000000>;
> +               regulator-max-microvolt = <5000000>;
> +               enable-active-high;
> +               gpio = <&gpio0 RK_PD3 GPIO_ACTIVE_HIGH>;
> +               vin-supply = <&vcc5v0_sys>;
> +               pinctrl-names = "default";
> +               pinctrl-0 = <&vbus_5v0_en_pin>;
The property names in these regulators are not as organized as the
regulators for the CPU/NPU.

> +       };
> +
> +       vcc_3v3: regulator-vcc-3v3 {
> +               compatible = "regulator-fixed";
> +               regulator-name = "vcc_3v3";
> +               regulator-min-microvolt = <3300000>;
> +               regulator-max-microvolt = <3300000>;
> +               startup-delay-us = <50000>;
> +               enable-active-high;
> +               gpio = <&gpio4 RK_PA3 GPIO_ACTIVE_HIGH>;
> +               vin-supply = <&vcc5v0_sys>;
> +               pinctrl-names = "default";
> +               pinctrl-0 = <&vcc_3v3_en_pin>;
> +       };
> +
> +       vcc5v0_sys: regulator-vcc-5v0 {
> +               compatible = "regulator-fixed";
> +               regulator-name = "vcc5v0_sys";
> +               regulator-always-on;
> +               regulator-boot-on;
> +               regulator-min-microvolt = <5000000>;
> +               regulator-max-microvolt = <5000000>;
> +       };
> +};

[ snip ]

> +
> +&gmac1 {
> +       clock_in_out = "output";
> +       phy-handle = <&rgmii_phy>;
> +       phy-mode = "rgmii-id";
> +       phy-supply = <&vcc_3v3>;
> +       pinctrl-names = "default";
> +       pinctrl-0 = <&gmac1_miim
> +                    &gmac1_rx_bus2
> +                    &gmac1_tx_bus2
> +                    &gmac1_rgmii_clk
> +                    &gmac1_rgmii_bus>;
> +       tx_delay = <0x42>;
When using "rgmii-id", tx_delay will be ignored. Does the ethernet
work without this property?

See the comment by Jonas in another review.
https://lore.kernel.org/linux-rockchip/da752790-da17-4d26-b9b2-8240b38b3276@kwiboo.se/

> +       status = "okay";
> +};
> +
> +&gpu {
> +       mali-supply = <&vdd_gpu_s0>;
> +       status = "okay";
> +};
This is a feature in the SoC itself, so it's not board specific and
can be put into the SoM.dtsi.

[ snip ]

> +
> +&pd_gpu {
> +       domain-supply = <&vdd_gpu_s0>;
> +};
Same comment regarding moving to the SoM.dtsi

> --
> Regards,
>
> Laurent Pinchart
>
Jimmy Hon