Messages in this thread | | | From | Jun Li <> | Subject | RE: commit 3497b9a5 (usb: dwc3: add power down scale setting) breaks imx8mp | Date | Fri, 6 Jan 2023 13:35:23 +0000 |
| |
> -----Original Message----- > From: Rasmus Villemoes <rasmus.villemoes@prevas.dk> > Sent: Friday, January 6, 2023 7:55 PM > To: Greg Kroah-Hartman <gregkh@linuxfoundation.org>; Jun Li > <jun.li@nxp.com> > Cc: LKML <linux-kernel@vger.kernel.org> > Subject: commit 3497b9a5 (usb: dwc3: add power down scale setting) breaks > imx8mp > > We have an imx8mp board with a lan7801 usb ethernet chip hardwired on the > PCB, which is used as the host port for a Microchip KSZ9567 switch. > > While trying to update the kernel to 6.1.y, I found something quite > weird: When the switch was being probed for the second time (the first ends > with a standard -EPROBE_DEFER), the board would spontaneously reset. > > Now when I disable the switch driver in .config just to see how far I could > otherwise get, the lan7801 device didn't appear until about 47 seconds after > boot. Bisecting unambiguously points at 3497b9a5, and digging in, it's pretty > obvious why that is bogus at least for imx8mp. > > The .dtsi file lists IMX8MP_CLK_USB_ROOT as the "suspend" clk, and > clk_get_rate() of that returns 500000000 ; divided by 16000 that's 31250, > which certainly doesn't fit in the 13-bit field GCTL_PWRDNSCALE. > But I assume the .dtsi file is wrong, because imx8mq.dtsi has > 74bd5951dd3 (arm64: dts: imx8mq: correct usb controller clocks), and it seems > likely from the commit log of 3497b9a5 that it was at least tested on imx8mq. > > Now I have no idea if the right clock for imx8mp is also some 32kHz clk, > but it would certainly make sense; unlike what the reference manual claims, > it seems that the reset value of the GCTL register is 0x00112004, amounting > to a pwrdwnscale value of 0x00100000>>19 == 2 == 32kHz/16kHz, and that could > explain why things worked just fine without 3497b9a5. > > Li Jun, please either revert 3497b9a5 or figure out if imx8mp.dtsi is broken > and needs a fix similar to 74bd5951dd3.
iMX8MP suspend clock(with name IMX8MP_CLK_USB_ROOT) was 32K when 3497b9a5 was merged, a later iMX8MP clock driver patch change the clock to be root clock gate(actually this is a shared clock gate for both suspend and root clock), so break the iMX8MP USB as you are seeing, a fix patch set already addressed this issue, please check and apply below 3 patches:
https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/include/dt-bindings/clock/imx8mp-clock.h?id=5c1f7f1090947d494c30042123e0ec846f696336 https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/drivers/clk/imx/clk-imx8mp.c?id=ed1f4ccfe947a3e1018a3bd7325134574c7ff9b3 https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/arch/arm64/boot/dts/freescale/imx8mp.dtsi?id=8a1ed98fe0f2e7669f0409de0f46f317b275f8be
Li Jun > > Rasmus
| |