[PATCH] arm64: dts: imx8mp: correct usb clocks

[Jun Li]

> -----Original Message-----
> From: Lucas Stach <l.stach at pengutronix.de>
> Sent: Wednesday, August 24, 2022 4:56 PM
> To: Jun Li <jun.li at nxp.com>; shawnguo at kernel.org; robh+dt at kernel.org;
> krzysztof.kozlowski+dt at linaro.org; s.hauer at pengutronix.de;
> kernel at pengutronix.de; festevam at gmail.com
> Cc: marex at denx.de; devicetree at vger.kernel.org; Peng Fan <peng.fan at nxp.com>;
> Markus.Niebel at ew.tq-group.com; laurent.pinchart at ideasonboard.com;
> paul.elder at ideasonboard.com; dl-linux-imx <linux-imx at nxp.com>;
> aford173 at gmail.com; linux-arm-kernel at lists.infradead.org
> Subject: Re: [PATCH] arm64: dts: imx8mp: correct usb clocks
> 
> Am Mittwoch, dem 24.08.2022 um 16:10 +0800 schrieb Li Jun:
> > After commit cf7f3f4fa9e5 ("clk: imx8mp: fix usb_root_clk parent"),
> > usb_root_clk is no longer for suspend clock so update dts accordingly
> > to use right bus clock and suspend clock.
> >
> So who is keeping IMX8MP_CLK_USB_ROOT enabled after this change? What is
> clocked by this and is it safe to disable while the USB subsystem is in working
> state? I see that things may still work, due to the shared gate with the
> suspend clock, but we should really try to model the DT after the HW. Especially
> since this is a ABI breaking change it should be right this time, so a unused
> USB_ROOT clock looks very suspicious.

The related clk driver change is [1]. I should put it in one patch set.

Before my change this shared clock gate is defined only for USB bus,
it will be off while system sleep even USB wakeup is enabled, this cause
the suspend clock is gated too, so USB remote wakeup cannot work.
In case system sleep without USB wakeup enabled, the shared gate
will be really off. Shared clock gate is matching the real HW.

You mean I should just simply define this gate only for suspend clock
instead of define 2 clock gates(then share)?

[1] https://patchwork.kernel.org/project/linux-clk/patch/1661328262-3867-2-git-send-email-jun.li@nxp.com/

Li Jun
> 
> Regards,
> Lucas
> 
> > Signed-off-by: Li Jun <jun.li at nxp.com>
> > ---
> >  arch/arm64/boot/dts/freescale/imx8mp.dtsi | 12 ++++++------
> >  1 file changed, 6 insertions(+), 6 deletions(-)
> >
> > diff --git a/arch/arm64/boot/dts/freescale/imx8mp.dtsi
> > b/arch/arm64/boot/dts/freescale/imx8mp.dtsi
> > index fe178b7d063c..2f18778a057f 100644
> > --- a/arch/arm64/boot/dts/freescale/imx8mp.dtsi
> > +++ b/arch/arm64/boot/dts/freescale/imx8mp.dtsi
> > @@ -1169,7 +1169,7 @@ usb3_0: usb at 32f10100 {
> >  			reg = <0x32f10100 0x8>,
> >  			      <0x381f0000 0x20>;
> >  			clocks = <&clk IMX8MP_CLK_HSIO_ROOT>,
> > -				 <&clk IMX8MP_CLK_USB_ROOT>;
> > +				 <&clk IMX8MP_CLK_USB_SUSP>;
> >  			clock-names = "hsio", "suspend";
> >  			interrupts = <GIC_SPI 148 IRQ_TYPE_LEVEL_HIGH>;
> >  			power-domains = <&hsio_blk_ctrl IMX8MP_HSIOBLK_PD_USB>; @@
> -1182,9
> > +1182,9 @@ usb3_0: usb at 32f10100 {
> >  			usb_dwc3_0: usb at 38100000 {
> >  				compatible = "snps,dwc3";
> >  				reg = <0x38100000 0x10000>;
> > -				clocks = <&clk IMX8MP_CLK_HSIO_AXI>,
> > +				clocks = <&clk IMX8MP_CLK_USB_ROOT>,
> >  					 <&clk IMX8MP_CLK_USB_CORE_REF>,
> > -					 <&clk IMX8MP_CLK_USB_ROOT>;
> > +					 <&clk IMX8MP_CLK_USB_SUSP>;
> >  				clock-names = "bus_early", "ref", "suspend";
> >  				interrupts = <GIC_SPI 40 IRQ_TYPE_LEVEL_HIGH>;
> >  				phys = <&usb3_phy0>, <&usb3_phy0>; @@ -1211,7 +1211,7 @@
> usb3_1:
> > usb at 32f10108 {
> >  			reg = <0x32f10108 0x8>,
> >  			      <0x382f0000 0x20>;
> >  			clocks = <&clk IMX8MP_CLK_HSIO_ROOT>,
> > -				 <&clk IMX8MP_CLK_USB_ROOT>;
> > +				 <&clk IMX8MP_CLK_USB_SUSP>;
> >  			clock-names = "hsio", "suspend";
> >  			interrupts = <GIC_SPI 149 IRQ_TYPE_LEVEL_HIGH>;
> >  			power-domains = <&hsio_blk_ctrl IMX8MP_HSIOBLK_PD_USB>; @@
> -1224,9
> > +1224,9 @@ usb3_1: usb at 32f10108 {
> >  			usb_dwc3_1: usb at 38200000 {
> >  				compatible = "snps,dwc3";
> >  				reg = <0x38200000 0x10000>;
> > -				clocks = <&clk IMX8MP_CLK_HSIO_AXI>,
> > +				clocks = <&clk IMX8MP_CLK_USB_ROOT>,
> >  					 <&clk IMX8MP_CLK_USB_CORE_REF>,
> > -					 <&clk IMX8MP_CLK_USB_ROOT>;
> > +					 <&clk IMX8MP_CLK_USB_SUSP>;
> >  				clock-names = "bus_early", "ref", "suspend";
> >  				interrupts = <GIC_SPI 41 IRQ_TYPE_LEVEL_HIGH>;
> >  				phys = <&usb3_phy1>, <&usb3_phy1>;
>