On 7.06.2023 12:56, Varadarajan Narayanan wrote: > Add USB phy and controller nodes > > Signed-off-by: Varadarajan Narayanan <quic_varada@xxxxxxxxxxx> > --- > arch/arm64/boot/dts/qcom/ipq5332.dtsi | 55 +++++++++++++++++++++++++++++++++++ > 1 file changed, 55 insertions(+) > > diff --git a/arch/arm64/boot/dts/qcom/ipq5332.dtsi b/arch/arm64/boot/dts/qcom/ipq5332.dtsi > index c2d6cc65..3183357 100644 > --- a/arch/arm64/boot/dts/qcom/ipq5332.dtsi > +++ b/arch/arm64/boot/dts/qcom/ipq5332.dtsi > @@ -383,6 +383,61 @@ > status = "disabled"; > }; > }; > + > + usb_0_m31phy: hs_m31phy@7b000 { > + compatible = "qcom,ipq5332-m31-usb-hsphy"; > + reg = <0x0007b000 0x12C>, random uppercase hex > + <0x08af8800 0x400>; > + reg-names = "m31usb_phy_base", > + "qscratch_base"; > + phy_type= "utmi"; Missing space before '=' > + > + resets = <&gcc GCC_QUSB2_0_PHY_BCR>; > + reset-names = "usb2_phy_reset"; > + > + status = "okay"; If you're only defining the node, it's enabled by default In this case, you'd probably want to disable it by default. > + }; > + > + usb2: usb2@8a00000 { > + compatible = "qcom,ipq5332-dwc3", "qcom,dwc3"; > + #address-cells = <1>; > + #size-cells = <1>; > + ranges; Please push these 3 properties to the end And add status = "disabled" below them. > + > + reg = <0x08af8800 0x100>; > + > + clocks = <&gcc GCC_USB0_MASTER_CLK>, > + <&gcc GCC_SNOC_USB_CLK>, > + <&gcc GCC_USB0_SLEEP_CLK>, > + <&gcc GCC_USB0_MOCK_UTMI_CLK>; > + Please remove this newline. > + clock-names = "core", > + "iface", > + "sleep", > + "mock_utmi"; Please align this, and all other similar lists. > + > + interrupts-extended = <&intc GIC_SPI 134 IRQ_TYPE_LEVEL_HIGH>; interrupts-extended is unnecessary with just a single interrupt source.. you can make it `interrupts` and drop the GIC reference. It would also be nice to push the interrupt properties below 'reg'. We're working on documenting and automating checking the preferred property order. > + interrupt-names = "pwr_event"; > + > + resets = <&gcc GCC_USB_BCR>; > + > + qcom,select-utmi-as-pipe-clk; > + > + usb2_0_dwc: usb@8a00000 { > + compatible = "snps,dwc3"; > + reg = <0x08a00000 0xe000>; > + clocks = <&gcc GCC_USB0_MOCK_UTMI_CLK>; > + clock-names = "ref"; > + interrupts = <GIC_SPI 64 IRQ_TYPE_LEVEL_HIGH>; > + usb-phy = <&usb_0_m31phy>; > + tx-fifo-resize; > + snps,is-utmi-l1-suspend; > + snps,hird-threshold = /bits/ 8 <0x0>; > + snps,dis_u2_susphy_quirk; > + snps,dis_u3_susphy_quirk; > + snps,ref-clock-period-ns = <21>; 1/21 is 0.0476.. that doesn't seem to correspond to the ref clk frequency? Konrad > + }; > + }; > }; > > timer {