Message ID | 20230825093531.7399-5-quic_nsekar@quicinc.com (mailing list archive) |
---|---|
State | Superseded |
Headers | show |
Series | Enable IPQ5018 USB2 support | expand |
On 25/08/2023 11:35, Nitheesh Sekar wrote: > Add USB phy and controller nodes. > > Co-developed-by: Amandeep Singh <quic_amansing@quicinc.com> > Signed-off-by: Amandeep Singh <quic_amansing@quicinc.com> > Signed-off-by: Nitheesh Sekar <quic_nsekar@quicinc.com> > --- > arch/arm64/boot/dts/qcom/ipq5018.dtsi | 54 +++++++++++++++++++++++++++ > 1 file changed, 54 insertions(+) > > diff --git a/arch/arm64/boot/dts/qcom/ipq5018.dtsi b/arch/arm64/boot/dts/qcom/ipq5018.dtsi > index 9f13d2dcdfd5..ff477923a823 100644 > --- a/arch/arm64/boot/dts/qcom/ipq5018.dtsi > +++ b/arch/arm64/boot/dts/qcom/ipq5018.dtsi > @@ -238,6 +238,60 @@ > status = "disabled"; > }; > }; > + > + usbphy0: phy@5b000 { > + compatible = "qcom,ipq5018-usb-hsphy"; > + reg = <0x0005b000 0x120>; > + > + clocks = <&gcc GCC_USB0_PHY_CFG_AHB_CLK>; > + > + resets = <&gcc GCC_QUSB2_0_PHY_BCR>; > + > + #phy-cells = <0>; > + > + status = "disabled"; > + }; > + > + usb: usb2@8a00000 { usb@ Node names should be generic. See also an explanation and list of examples (not exhaustive) in DT specification: https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation > + compatible = "qcom,ipq5018-dwc3", "qcom,dwc3"; > + reg = <0x08af8800 0x400>; > + Best regards, Krzysztof
On 25/08/2023 11:35, Nitheesh Sekar wrote: > Add USB phy and controller nodes. > > Co-developed-by: Amandeep Singh <quic_amansing@quicinc.com> > Signed-off-by: Amandeep Singh <quic_amansing@quicinc.com> > Signed-off-by: Nitheesh Sekar <quic_nsekar@quicinc.com> > --- > arch/arm64/boot/dts/qcom/ipq5018.dtsi | 54 +++++++++++++++++++++++++++ > 1 file changed, 54 insertions(+) > > diff --git a/arch/arm64/boot/dts/qcom/ipq5018.dtsi b/arch/arm64/boot/dts/qcom/ipq5018.dtsi > index 9f13d2dcdfd5..ff477923a823 100644 > --- a/arch/arm64/boot/dts/qcom/ipq5018.dtsi > +++ b/arch/arm64/boot/dts/qcom/ipq5018.dtsi > @@ -238,6 +238,60 @@ > status = "disabled"; > }; > }; > + > + usbphy0: phy@5b000 { > + compatible = "qcom,ipq5018-usb-hsphy"; > + reg = <0x0005b000 0x120>; > + > + clocks = <&gcc GCC_USB0_PHY_CFG_AHB_CLK>; > + > + resets = <&gcc GCC_QUSB2_0_PHY_BCR>; > + > + #phy-cells = <0>; > + > + status = "disabled"; > + }; > + > + usb: usb2@8a00000 { Eee, where did you put these? Don't stuff things to the end of any list. This applies to your entire code: Makefile, Kconfig, .c, .h, .dts, .dtsi and other languages and projects as well. Krzysztof
On 8/25/2023 4:08 PM, Krzysztof Kozlowski wrote: > On 25/08/2023 11:35, Nitheesh Sekar wrote: >> Add USB phy and controller nodes. >> >> Co-developed-by: Amandeep Singh <quic_amansing@quicinc.com> >> Signed-off-by: Amandeep Singh <quic_amansing@quicinc.com> >> Signed-off-by: Nitheesh Sekar <quic_nsekar@quicinc.com> >> --- >> arch/arm64/boot/dts/qcom/ipq5018.dtsi | 54 +++++++++++++++++++++++++++ >> 1 file changed, 54 insertions(+) >> >> diff --git a/arch/arm64/boot/dts/qcom/ipq5018.dtsi b/arch/arm64/boot/dts/qcom/ipq5018.dtsi >> index 9f13d2dcdfd5..ff477923a823 100644 >> --- a/arch/arm64/boot/dts/qcom/ipq5018.dtsi >> +++ b/arch/arm64/boot/dts/qcom/ipq5018.dtsi >> @@ -238,6 +238,60 @@ >> status = "disabled"; >> }; >> }; >> + >> + usbphy0: phy@5b000 { >> + compatible = "qcom,ipq5018-usb-hsphy"; >> + reg = <0x0005b000 0x120>; >> + >> + clocks = <&gcc GCC_USB0_PHY_CFG_AHB_CLK>; >> + >> + resets = <&gcc GCC_QUSB2_0_PHY_BCR>; >> + >> + #phy-cells = <0>; >> + >> + status = "disabled"; >> + }; >> + >> + usb: usb2@8a00000 { > usb@ > > Node names should be generic. See also an explanation and list of > examples (not exhaustive) in DT specification: > https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation Sure, will update. Regards, Nitheesh
On 8/25/2023 4:09 PM, Krzysztof Kozlowski wrote: > On 25/08/2023 11:35, Nitheesh Sekar wrote: >> Add USB phy and controller nodes. >> >> Co-developed-by: Amandeep Singh <quic_amansing@quicinc.com> >> Signed-off-by: Amandeep Singh <quic_amansing@quicinc.com> >> Signed-off-by: Nitheesh Sekar <quic_nsekar@quicinc.com> >> --- >> arch/arm64/boot/dts/qcom/ipq5018.dtsi | 54 +++++++++++++++++++++++++++ >> 1 file changed, 54 insertions(+) >> >> diff --git a/arch/arm64/boot/dts/qcom/ipq5018.dtsi b/arch/arm64/boot/dts/qcom/ipq5018.dtsi >> index 9f13d2dcdfd5..ff477923a823 100644 >> --- a/arch/arm64/boot/dts/qcom/ipq5018.dtsi >> +++ b/arch/arm64/boot/dts/qcom/ipq5018.dtsi >> @@ -238,6 +238,60 @@ >> status = "disabled"; >> }; >> }; >> + >> + usbphy0: phy@5b000 { >> + compatible = "qcom,ipq5018-usb-hsphy"; >> + reg = <0x0005b000 0x120>; >> + >> + clocks = <&gcc GCC_USB0_PHY_CFG_AHB_CLK>; >> + >> + resets = <&gcc GCC_QUSB2_0_PHY_BCR>; >> + >> + #phy-cells = <0>; >> + >> + status = "disabled"; >> + }; >> + >> + usb: usb2@8a00000 { > Eee, where did you put these? Don't stuff things to the end of any list. > This applies to your entire code: Makefile, Kconfig, .c, .h, .dts, .dtsi > and other languages and projects as well. Will reorder accordingly and post an update. Regards, Nitheesh
On Fri, 25 Aug 2023 at 12:37, Nitheesh Sekar <quic_nsekar@quicinc.com> wrote: > > Add USB phy and controller nodes. > > Co-developed-by: Amandeep Singh <quic_amansing@quicinc.com> > Signed-off-by: Amandeep Singh <quic_amansing@quicinc.com> > Signed-off-by: Nitheesh Sekar <quic_nsekar@quicinc.com> > --- > arch/arm64/boot/dts/qcom/ipq5018.dtsi | 54 +++++++++++++++++++++++++++ > 1 file changed, 54 insertions(+) > > diff --git a/arch/arm64/boot/dts/qcom/ipq5018.dtsi b/arch/arm64/boot/dts/qcom/ipq5018.dtsi > index 9f13d2dcdfd5..ff477923a823 100644 > --- a/arch/arm64/boot/dts/qcom/ipq5018.dtsi > +++ b/arch/arm64/boot/dts/qcom/ipq5018.dtsi > @@ -238,6 +238,60 @@ > status = "disabled"; > }; > }; > + > + usbphy0: phy@5b000 { > + compatible = "qcom,ipq5018-usb-hsphy"; > + reg = <0x0005b000 0x120>; > + > + clocks = <&gcc GCC_USB0_PHY_CFG_AHB_CLK>; > + > + resets = <&gcc GCC_QUSB2_0_PHY_BCR>; > + > + #phy-cells = <0>; > + > + status = "disabled"; > + }; > + > + usb: usb2@8a00000 { > + compatible = "qcom,ipq5018-dwc3", "qcom,dwc3"; > + reg = <0x08af8800 0x400>; > + > + interrupts = <GIC_SPI 62 IRQ_TYPE_LEVEL_HIGH>; > + interrupt-names = "hs_phy_irq"; > + > + clocks = <&gcc GCC_USB0_MASTER_CLK>, > + <&gcc GCC_SYS_NOC_USB0_AXI_CLK>, > + <&gcc GCC_USB0_SLEEP_CLK>, > + <&gcc GCC_USB0_MOCK_UTMI_CLK>; > + clock-names = "core", > + "iface", > + "sleep", > + "mock_utmi"; > + > + resets = <&gcc GCC_USB0_BCR>; > + > + qcom,select-utmi-as-pipe-clk; > + #address-cells = <1>; > + #size-cells = <1>; > + ranges; > + > + status = "disabled"; > + > + usb2_0_dwc: usb@8a00000 { Could you please remove the `2_0' part of the label? > + compatible = "snps,dwc3"; > + reg = <0x08a00000 0xe000>; > + clocks = <&gcc GCC_USB0_MOCK_UTMI_CLK>; > + clock-names = "ref"; > + interrupts = <GIC_SPI 140 IRQ_TYPE_LEVEL_HIGH>; > + phy-names = "usb2-phy"; > + phys = <&usbphy0>; > + tx-fifo-resize; > + snps,is-utmi-l1-suspend; > + snps,hird-threshold = /bits/ 8 <0x0>; > + snps,dis_u2_susphy_quirk; > + snps,dis_u3_susphy_quirk; > + }; > + }; > }; > > timer { > -- > The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, > a Linux Foundation Collaborative Project >
On 8/31/2023 1:45 AM, Dmitry Baryshkov wrote: > On Fri, 25 Aug 2023 at 12:37, Nitheesh Sekar <quic_nsekar@quicinc.com> wrote: >> Add USB phy and controller nodes. >> >> Co-developed-by: Amandeep Singh <quic_amansing@quicinc.com> >> Signed-off-by: Amandeep Singh <quic_amansing@quicinc.com> >> Signed-off-by: Nitheesh Sekar <quic_nsekar@quicinc.com> >> --- >> arch/arm64/boot/dts/qcom/ipq5018.dtsi | 54 +++++++++++++++++++++++++++ >> 1 file changed, 54 insertions(+) >> >> diff --git a/arch/arm64/boot/dts/qcom/ipq5018.dtsi b/arch/arm64/boot/dts/qcom/ipq5018.dtsi >> index 9f13d2dcdfd5..ff477923a823 100644 >> --- a/arch/arm64/boot/dts/qcom/ipq5018.dtsi >> +++ b/arch/arm64/boot/dts/qcom/ipq5018.dtsi >> @@ -238,6 +238,60 @@ >> status = "disabled"; >> }; >> }; >> + >> + usbphy0: phy@5b000 { >> + compatible = "qcom,ipq5018-usb-hsphy"; >> + reg = <0x0005b000 0x120>; >> + >> + clocks = <&gcc GCC_USB0_PHY_CFG_AHB_CLK>; >> + >> + resets = <&gcc GCC_QUSB2_0_PHY_BCR>; >> + >> + #phy-cells = <0>; >> + >> + status = "disabled"; >> + }; >> + >> + usb: usb2@8a00000 { >> + compatible = "qcom,ipq5018-dwc3", "qcom,dwc3"; >> + reg = <0x08af8800 0x400>; >> + >> + interrupts = <GIC_SPI 62 IRQ_TYPE_LEVEL_HIGH>; >> + interrupt-names = "hs_phy_irq"; >> + >> + clocks = <&gcc GCC_USB0_MASTER_CLK>, >> + <&gcc GCC_SYS_NOC_USB0_AXI_CLK>, >> + <&gcc GCC_USB0_SLEEP_CLK>, >> + <&gcc GCC_USB0_MOCK_UTMI_CLK>; >> + clock-names = "core", >> + "iface", >> + "sleep", >> + "mock_utmi"; >> + >> + resets = <&gcc GCC_USB0_BCR>; >> + >> + qcom,select-utmi-as-pipe-clk; >> + #address-cells = <1>; >> + #size-cells = <1>; >> + ranges; >> + >> + status = "disabled"; >> + >> + usb2_0_dwc: usb@8a00000 { > Could you please remove the `2_0' part of the label? Sure. Regards, Nitheesh
diff --git a/arch/arm64/boot/dts/qcom/ipq5018.dtsi b/arch/arm64/boot/dts/qcom/ipq5018.dtsi index 9f13d2dcdfd5..ff477923a823 100644 --- a/arch/arm64/boot/dts/qcom/ipq5018.dtsi +++ b/arch/arm64/boot/dts/qcom/ipq5018.dtsi @@ -238,6 +238,60 @@ status = "disabled"; }; }; + + usbphy0: phy@5b000 { + compatible = "qcom,ipq5018-usb-hsphy"; + reg = <0x0005b000 0x120>; + + clocks = <&gcc GCC_USB0_PHY_CFG_AHB_CLK>; + + resets = <&gcc GCC_QUSB2_0_PHY_BCR>; + + #phy-cells = <0>; + + status = "disabled"; + }; + + usb: usb2@8a00000 { + compatible = "qcom,ipq5018-dwc3", "qcom,dwc3"; + reg = <0x08af8800 0x400>; + + interrupts = <GIC_SPI 62 IRQ_TYPE_LEVEL_HIGH>; + interrupt-names = "hs_phy_irq"; + + clocks = <&gcc GCC_USB0_MASTER_CLK>, + <&gcc GCC_SYS_NOC_USB0_AXI_CLK>, + <&gcc GCC_USB0_SLEEP_CLK>, + <&gcc GCC_USB0_MOCK_UTMI_CLK>; + clock-names = "core", + "iface", + "sleep", + "mock_utmi"; + + resets = <&gcc GCC_USB0_BCR>; + + qcom,select-utmi-as-pipe-clk; + #address-cells = <1>; + #size-cells = <1>; + ranges; + + status = "disabled"; + + usb2_0_dwc: usb@8a00000 { + compatible = "snps,dwc3"; + reg = <0x08a00000 0xe000>; + clocks = <&gcc GCC_USB0_MOCK_UTMI_CLK>; + clock-names = "ref"; + interrupts = <GIC_SPI 140 IRQ_TYPE_LEVEL_HIGH>; + phy-names = "usb2-phy"; + phys = <&usbphy0>; + tx-fifo-resize; + snps,is-utmi-l1-suspend; + snps,hird-threshold = /bits/ 8 <0x0>; + snps,dis_u2_susphy_quirk; + snps,dis_u3_susphy_quirk; + }; + }; }; timer {