[U-Boot] [PATCH] arm64: allwinner: a64: Disable ehci1 and ohci1 for bananapi, nanopi

[Jagan Teki]

On Tue, Jul 3, 2018 at 10:14 PM, Marek Vasut <marex at 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]

=> 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.

[2] https://paste.ubuntu.com/p/V9KKxMx6Cj/