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