Re: [PATCH v3 1/1] arm64: dts: qcom: sc7180: Add USB related nodes

[Matthias Kaehlcke <mka@chromium.org>]

On Wed, Nov 27, 2019 at 09:01:07AM +0000, Sandeep Maheswaram (Temp) wrote:
> Hi Matthias,
> 
> Thanks for the review.
> 
> On 11/22/2019 6:21 AM, Matthias Kaehlcke wrote:
> > Hi Sandeep,
> > 
> > On Fri, Nov 15, 2019 at 10:53:41AM +0530, Sandeep Maheswaram wrote:
> > > Add nodes for DWC3 USB controller, QMP and QUSB PHYs.
> > > 
> > > Signed-off-by: Sandeep Maheswaram <sanm@codeaurora.org>
> > > Reviewed-by: Stephen Boyd <swboyd@chromium.org>
> > > ---
> > >   arch/arm64/boot/dts/qcom/sc7180-idp.dts |  25 ++++++++
> > >   arch/arm64/boot/dts/qcom/sc7180.dtsi    | 105 ++++++++++++++++++++++++++++++++
> > >   2 files changed, 130 insertions(+)
> > > 
> > > diff --git a/arch/arm64/boot/dts/qcom/sc7180.dtsi b/arch/arm64/boot/dts/qcom/sc7180.dtsi
> > > index 666e9b9..2c7dbdc 100644
> > > --- a/arch/arm64/boot/dts/qcom/sc7180.dtsi
> > > +++ b/arch/arm64/boot/dts/qcom/sc7180.dtsi
> > > 
> > > ...
> > > 
> > > +		usb_1: usb@a6f8800 {
> > > +			compatible = "qcom,sc7180-dwc3", "qcom,dwc3";
> > > +			reg = <0 0x0a6f8800 0 0x400>;
> > > +			status = "disabled";
> > > +			#address-cells = <2>;
> > > +			#size-cells = <2>;
> > > +			ranges;
> > > +			dma-ranges;
> > > +
> > > +			clocks = <&gcc GCC_CFG_NOC_USB3_PRIM_AXI_CLK>,
> > > +				 <&gcc GCC_USB30_PRIM_MASTER_CLK>,
> > > +				 <&gcc GCC_AGGRE_USB3_PRIM_AXI_CLK>,
> > > +				 <&gcc GCC_USB30_PRIM_MOCK_UTMI_CLK>,
> > > +				 <&gcc GCC_USB30_PRIM_SLEEP_CLK>;
> > > +			clock-names = "cfg_noc", "core", "iface", "mock_utmi",
> > > +				      "sleep";
> > > +
> > > +			assigned-clocks = <&gcc GCC_USB30_PRIM_MOCK_UTMI_CLK>,
> > > +					  <&gcc GCC_USB30_PRIM_MASTER_CLK>;
> > > +			assigned-clock-rates = <19200000>, <150000000>;
> > > +
> > > +			interrupts = <GIC_SPI 131 IRQ_TYPE_LEVEL_HIGH>,
> > > +				     <GIC_SPI 486 IRQ_TYPE_LEVEL_HIGH>,
> > > +				     <GIC_SPI 488 IRQ_TYPE_LEVEL_HIGH>,
> > > +				     <GIC_SPI 489 IRQ_TYPE_LEVEL_HIGH>;
> > > +			interrupt-names = "hs_phy_irq", "ss_phy_irq",
> > > +					  "dm_hs_phy_irq", "dp_hs_phy_irq";
> > > +
> > > +			power-domains = <&gcc USB30_PRIM_GDSC>;
> > > +
> > > +			resets = <&gcc GCC_USB30_PRIM_BCR>;
> > > +
> > > +			usb_1_dwc3: dwc3@a600000 {
> > > +				compatible = "snps,dwc3";
> > > +				reg = <0 0x0a600000 0 0xe000>;
> > > +				interrupts = <GIC_SPI 133 IRQ_TYPE_LEVEL_HIGH>;
> > > +				iommus = <&apps_smmu 0x540 0>;
> > > +				snps,dis_u2_susphy_quirk;
> > > +				snps,dis_enblslpm_quirk;
> > > +				phys = <&usb_1_hsphy>, <&usb_1_ssphy>;
> > > +				phy-names = "usb2-phy", "usb3-phy";
> > > +			};
> > I see the following message at boot:
> > 
> > [    4.248436] dwc3 a600000.dwc3: Failed to get clk 'ref': -2
> > 
> > Apparently the driver is operational regardless, however the binding lists
> > the clocks as required:
> > 
> >    Required properties:
> >     ...
> >     - clock-names: should contain "ref", "bus_early", "suspend"
> >     - clocks: list of phandle and clock specifier pairs corresponding to
> >               entries in the clock-names property.
> > 
> >    [Documentation/devicetree/bindings/usb/dwc3.txt]
> > 
> > The driver code also has a comment stating that the clocks should be
> > specified:
> > 
> >    /*
> >     * Clocks are optional, but new DT platforms should support all
> >     * clocks as required by the DT-binding.
> >     */
> > 
> >    [drivers/usb/dwc3/core.txt]
> We are implenting all the required clocks in glue driver
> drivers/usb/dwc3/dwc3-qcom.c.
> 
> Also there is exception for qcom,dwc3 in documentation
> 
> Documentation/devicetree/bindings/usb/dwc3.txt
> 
> 
> Exception for clocks:
> 
> clocks are optional if the parent node (i.e. glue-layer) is compatible to
> one of the following:
> "amlogic,meson-axg-dwc3"
> "amlogic,meson-gxl-dwc3"
> "cavium,octeon-7130-usb-uctl"
> "qcom,dwc3"
> "samsung,exynos5250-dwusb3"
> "samsung,exynos5433-dwusb3"
> "samsung,exynos7-dwusb3"
> "sprd,sc9860-dwc3"
> "st,stih407-dwc3"
> "ti,am437x-dwc3"
> "ti,dwc3"
> "ti,keystone-dwc3"
> "rockchip,rk3399-dwc3"
> "xlnx,zynqmp-dwc3"

ah, I missed this, thanks for the pointer!