Re: [PATCH v2 7/8] arm64: dts: rockchip: add usb2 nodes to rk3568 device tree

From: Michael Riesch
Date: Thu Oct 14 2021 - 06:02:56 EST


Hi Peter,

One cosmetic issue/question:

> [...]
> @@ -738,6 +792,50 @@ saradc: saradc@fe720000 {
> status = "disabled";
> };
>
> + usb2phy0: usb2-phy@fe8a0000 {
> + compatible = "rockchip,rk3568-usb2phy";
> + reg = <0x0 0xfe8a0000 0x0 0x10000>;
> + clocks = <&pmucru CLK_USBPHY0_REF>;
> + clock-names = "phyclk";
> + clock-output-names = "clk_usbphy0_480m";
> + interrupts = <GIC_SPI 135 IRQ_TYPE_LEVEL_HIGH>;
> + rockchip,usbgrf = <&usb2phy0_grf>;
> + #clock-cells = <0>;
> + status = "disabled";
> +
> + u2phy0_host: host-port {
> + #phy-cells = <0>;
> + status = "disabled";
> + };
> +
> + u2phy0_otg: otg-port {
> + #phy-cells = <0>;
> + status = "disabled";
> + };

Would it make sense to name those usb2phy0_{host,otg}? This would clean
up the sorting in the dts files a bit. Otherwise, u2phy... and
usb2phy... would have to be separated by e.g. uart nodes.

> + };
> +
> + usb2phy1: usb2-phy@fe8b0000 {
> + compatible = "rockchip,rk3568-usb2phy";
> + reg = <0x0 0xfe8b0000 0x0 0x10000>;
> + clocks = <&pmucru CLK_USBPHY1_REF>;
> + clock-names = "phyclk";
> + clock-output-names = "clk_usbphy1_480m";
> + interrupts = <GIC_SPI 136 IRQ_TYPE_LEVEL_HIGH>;
> + rockchip,usbgrf = <&usb2phy1_grf>;
> + #clock-cells = <0>;
> + status = "disabled";
> +
> + u2phy1_host: host-port {
> + #phy-cells = <0>;
> + status = "disabled";
> + };
> +
> + u2phy1_otg: otg-port {
> + #phy-cells = <0>;
> + status = "disabled";
> + };

Same here, of course.

> + };
> +
> [...]

Best regards,
Michael