On Tue, Jul 3, 2018 at 11:26 PM, Marek Vasut <ma...@denx.de> wrote: > On 07/03/2018 07:09 PM, Jagan Teki wrote: >> On Tue, Jul 3, 2018 at 10:14 PM, Marek Vasut <ma...@denx.de> wrote: >>> On 07/03/2018 05:22 PM, Andre Przywara wrote: >>>> Hi, >>>> >>>> On 02/07/18 22:57, Marek Vasut wrote: >>>>> On 07/02/2018 11:40 PM, Tom Rini wrote: >>>>>> On Mon, Jul 02, 2018 at 11:27:58PM +0200, Marek Vasut wrote: >>>>>>> On 07/02/2018 10:53 PM, Jagan Teki wrote: >>>>>>>> During usb shutdown or 'usb reset' all the necessary clocks >>>>>>>> on the specific controller will disable. Usually this shutdown >>>>>>>> happen during U-Boot proper handoff to Linux. >>>>>>> >>>>>>> No, 'usb reset' can be triggered by the user any time. >>>>>> >>>>>> Yes, and it's also triggered as part of the hand-off, which is the use >>>>>> case in question. >>>>> >>>>> No, that's not true. The USB controllers are torn down when starting the >>>>> OS, which is a different thing from usb reset, which brings them back up >>>>> and rescans the bus too. >>>>> >>>>>>>> There is an issue in Allwinner A64, is during OHCI1 shutdown >>>>>>>> the controller is unable to access the register space >>>>>>>> so the Linux boot hangs at this place. >>>>>>> >>>>>>> This doesn't make any sense, Linux should enable the necessary clock >>>>>>> before accessing any controller registers, unless there is a bug in >>>>>>> Linux. >>>>>> >>>>>> Should but doesn't always? So yes, there's possibly / hopefully a bug >>>>>> in the dts files. >>>>> >>>>> How did you reach that conclusion about the DTS files ? There is a bug >>>>> in Linux, but it's likely in the driver which doesn't enable the clock >>>>> before accessing the registers. >>>>> >>>>> But maybe the description here is completely confusing, since the output >>>>> down below would indicate this hang is still in U-Boot. >>>> >>>> Yes, it is. There is no bug in Linux. >>>> >>>> U-Boot trips over its own feet when bringing down the USB controllers: >>>> It shutdowns one part (EHCI or OHCI) on the register level, then turns >>>> off the clocks and reset gates. But because they are shared between >>>> controllers, this turns off the other controller as well. Then it tries >>>> to bring down the the second part (OHCI or EHCI, respectively) on the >>>> USB register level, which hangs, because the AHB clock is already off. >>>> As this just happens quite late, it looks like U-Boot already said >>>> goodbye, but it actually hasn't completely finished. >>>> So Linux is completely fine and the bug is entirely in U-Boot. >>>> My patch [1] tries to paper^Wsolve this in a different way, though it >>>> isn't perfect either. I think there is a bit more to it than I assumed >>>> yesterday, so I need to go back to the code later tonight to see what's >>>> really going on (I suspect it's about OHCI 0 and 1 sharing a clock, not >>>> about EHCI and OHCI). >>> >>> Well, please keep poking. >>> >>> Maybe a dumb idea, but what about enabling the clock at the beginning of >>> remove function to guarantee they are ON and then disabling the clock at >>> the end of the function. Would that work maybe ? ie. >>> >>> .remove() { >>> clk_enable(..); >>> readl()/writel()/... >>> clk_disable(..); >>> } >> >> I've verified clock bits before disabling, and bits are enabled as >> usual. and even verified your idea of enabling before disable[2] > > Verified ... with what result ?
Same hang, with properly disabling clock during OHCI0 shutdown. > >> => usb reset >> resetting USB... >> ohci_usb_remove: input mask = 0x10000, input usb_clk_cfg = 0x30303 >> ohci_usb_remove: usb_clk_cfg = 0x20303 >> EHCI failed to shut down host controller. << hang here >> >> >> [2] https://paste.ubuntu.com/p/V9KKxMx6Cj/ > > (no need to use pastebin to share 20 line patch, in fact it doesn't help > the ML archives at all) Sorry, copying same here. --- a/drivers/usb/host/ohci-sunxi.c +++ b/drivers/usb/host/ohci-sunxi.c @@ -128,10 +128,14 @@ static int ohci_usb_remove(struct udevice *dev) if (ret) return ret; + printf("%s: mask = 0x%x, usb_clk_cfg = 0x%x\n", __func__, + priv->usb_gate_mask, readl(&priv->ccm->usb_clk_cfg)); + setbits_le32(&priv->ccm->usb_clk_cfg, priv->usb_gate_mask); if (priv->cfg->has_reset) clrbits_le32(priv->reset0_cfg, priv->ahb_gate_mask); - clrbits_le32(&priv->ccm->usb_clk_cfg, priv->usb_gate_mask); clrbits_le32(&priv->ccm->ahb_gate0, priv->ahb_gate_mask); + clrbits_le32(&priv->ccm->usb_clk_cfg, priv->usb_gate_mask); + printf("%s: usb_clk_cfg = 0x%x\n", __func__, readl(&priv->ccm->usb_clk_cfg)); _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot