| Message ID | 23b259b72c8f6faad99f09c37ac8b7b6b027cea1.1689065318.git.quic_varada@quicinc.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Enable IPQ5332 USB2 | expand |
On 11/07/2023 10:51, Varadarajan Narayanan wrote: > Add USB phy and controller nodes. > > Signed-off-by: Varadarajan Narayanan <quic_varada@quicinc.com> > --- > v1: > Rename phy node I don't see any improvements. > Change compatible from m31,ipq5332-usb-hsphy -> qcom,ipq5332-usb-hsphy > Remove 'qscratch' from phy node > Fix alignment and upper-case hex no.s > Add clock definition for the phy > Remove snps,ref-clock-period-ns as it is not used. dwc3_ref_clk_period() > in dwc3/core.c takes the frequency from ref clock and calculates fladj > as appropriate. > --- > arch/arm64/boot/dts/qcom/ipq5332.dtsi | 54 +++++++++++++++++++++++++++++++++++ > 1 file changed, 54 insertions(+) > > diff --git a/arch/arm64/boot/dts/qcom/ipq5332.dtsi b/arch/arm64/boot/dts/qcom/ipq5332.dtsi > index 8bfc2db..c945ff6 100644 > --- a/arch/arm64/boot/dts/qcom/ipq5332.dtsi > +++ b/arch/arm64/boot/dts/qcom/ipq5332.dtsi > @@ -405,6 +405,60 @@ > status = "disabled"; > }; > }; > + > + usbphy0: ipq5332-hsphy@7b000 { 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 "phy" > + compatible = "qcom,ipq5332-usb-hsphy"; > + reg = <0x0007b000 0x12c>; > + > + clocks = <&gcc GCC_USB0_PHY_CFG_AHB_CLK>; > + clock-names = "cfg_ahb"; > + > + resets = <&gcc GCC_QUSB2_0_PHY_BCR>; > + > + status = "disabled"; > + }; > + > + usb2: usb2@8a00000 { It does not look like you tested the DTS against bindings. Please run `make dtbs_check` (see Documentation/devicetree/bindings/writing-schema.rst or https://www.linaro.org/blog/tips-and-tricks-for-validating-devicetree-sources-with-the-devicetree-schema/ for instructions). > + compatible = "qcom,ipq5332-dwc3", "qcom,dwc3"; > + No need for blank line. > + reg = <0x08af8800 0x400>; > + > + interrupts = <GIC_SPI 62 IRQ_ Best regards, Krzysztof
On Tue, Jul 11, 2023 at 11:01:03AM +0200, Krzysztof Kozlowski wrote: > On 11/07/2023 10:51, Varadarajan Narayanan wrote: > > Add USB phy and controller nodes. > > > > Signed-off-by: Varadarajan Narayanan <quic_varada@quicinc.com> > > --- > > v1: > > Rename phy node > > I don't see any improvements. Will fix and post a new patch > > Change compatible from m31,ipq5332-usb-hsphy -> qcom,ipq5332-usb-hsphy > > Remove 'qscratch' from phy node > > Fix alignment and upper-case hex no.s > > Add clock definition for the phy > > Remove snps,ref-clock-period-ns as it is not used. dwc3_ref_clk_period() > > in dwc3/core.c takes the frequency from ref clock and calculates fladj > > as appropriate. > > --- > > arch/arm64/boot/dts/qcom/ipq5332.dtsi | 54 +++++++++++++++++++++++++++++++++++ > > 1 file changed, 54 insertions(+) > > > > diff --git a/arch/arm64/boot/dts/qcom/ipq5332.dtsi b/arch/arm64/boot/dts/qcom/ipq5332.dtsi > > index 8bfc2db..c945ff6 100644 > > --- a/arch/arm64/boot/dts/qcom/ipq5332.dtsi > > +++ b/arch/arm64/boot/dts/qcom/ipq5332.dtsi > > @@ -405,6 +405,60 @@ > > status = "disabled"; > > }; > > }; > > + > > + usbphy0: ipq5332-hsphy@7b000 { > > 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 > > "phy" Will fix and post a new patch > > + compatible = "qcom,ipq5332-usb-hsphy"; > > + reg = <0x0007b000 0x12c>; > > + > > + clocks = <&gcc GCC_USB0_PHY_CFG_AHB_CLK>; > > + clock-names = "cfg_ahb"; > > + > > + resets = <&gcc GCC_QUSB2_0_PHY_BCR>; > > + > > + status = "disabled"; > > + }; > > + > > + usb2: usb2@8a00000 { > > It does not look like you tested the DTS against bindings. Please run > `make dtbs_check` (see > Documentation/devicetree/bindings/writing-schema.rst or > https://www.linaro.org/blog/tips-and-tricks-for-validating-devicetree-sources-with-the-devicetree-schema/ > for instructions). 'make dtbs_check' passed. The '2' in 'usb2' is to indicate USB v2. There is one more USB v3 controller in this SoC. Hence, to differentiate between the two used 'usb2'. Hope that is ok. > > + compatible = "qcom,ipq5332-dwc3", "qcom,dwc3"; > > + > > No need for blank line. Will remove. Thanks Varada > > + reg = <0x08af8800 0x400>; > > + > > + interrupts = <GIC_SPI 62 IRQ_ > > > Best regards, > Krzysztof >
On 12/07/2023 13:28, Varadarajan Narayanan wrote: >>> + >>> + usb2: usb2@8a00000 { >> >> It does not look like you tested the DTS against bindings. Please run >> `make dtbs_check` (see >> Documentation/devicetree/bindings/writing-schema.rst or >> https://www.linaro.org/blog/tips-and-tricks-for-validating-devicetree-sources-with-the-devicetree-schema/ >> for instructions). > > 'make dtbs_check' passed. The '2' in 'usb2' is to indicate USB v2. > There is one more USB v3 controller in this SoC. Hence, to > differentiate between the two used 'usb2'. > > Hope that is ok. Nope, unfortunately it is not. Best regards, Krzysztof
diff --git a/arch/arm64/boot/dts/qcom/ipq5332.dtsi b/arch/arm64/boot/dts/qcom/ipq5332.dtsi index 8bfc2db..c945ff6 100644 --- a/arch/arm64/boot/dts/qcom/ipq5332.dtsi +++ b/arch/arm64/boot/dts/qcom/ipq5332.dtsi @@ -405,6 +405,60 @@ status = "disabled"; }; }; + + usbphy0: ipq5332-hsphy@7b000 { + compatible = "qcom,ipq5332-usb-hsphy"; + reg = <0x0007b000 0x12c>; + + clocks = <&gcc GCC_USB0_PHY_CFG_AHB_CLK>; + clock-names = "cfg_ahb"; + + resets = <&gcc GCC_QUSB2_0_PHY_BCR>; + + status = "disabled"; + }; + + usb2: usb2@8a00000 { + compatible = "qcom,ipq5332-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_SNOC_USB_CLK>, + <&gcc GCC_USB0_SLEEP_CLK>, + <&gcc GCC_USB0_MOCK_UTMI_CLK>; + clock-names = "core", + "iface", + "sleep", + "mock_utmi"; + + resets = <&gcc GCC_USB_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 64 IRQ_TYPE_LEVEL_HIGH>; + usb-phy = <&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 {
Add USB phy and controller nodes. Signed-off-by: Varadarajan Narayanan <quic_varada@quicinc.com> --- v1: Rename phy node Change compatible from m31,ipq5332-usb-hsphy -> qcom,ipq5332-usb-hsphy Remove 'qscratch' from phy node Fix alignment and upper-case hex no.s Add clock definition for the phy Remove snps,ref-clock-period-ns as it is not used. dwc3_ref_clk_period() in dwc3/core.c takes the frequency from ref clock and calculates fladj as appropriate. --- arch/arm64/boot/dts/qcom/ipq5332.dtsi | 54 +++++++++++++++++++++++++++++++++++ 1 file changed, 54 insertions(+)