[U-Boot] [PATCH] sunxi: H3/H5/A64: OHCI: prevent turning off shared gates

[Jagan Teki]

On Tue, Jul 3, 2018 at 5:14 AM, Andre Przywara <andre.przywara at arm.com> wrote:
> The USB host controllers on the H3, H5 and A64 have the oddity of
> sharing some clock and reset gates, so both the OHCI and EHCI bits have
> to be enabled to make only one of them working. We take care of this, and
> initialisation works fine (due to setting already set bits).
> However on shutdown we turn the clocks and reset gates off already when
> deregistering one controller, so the other one is no longer functional.
> In the result U-Boot complains just before launching the kernel and
> then hangs.
> Fix this by not turning off the clocks and resets on the OHCI side, so
> that the EHCI controller has a chance to properly shut down.
> This still isn't perfect, but at least prevents the hang.
>
> Signed-off-by: Andre Przywara <andre.przywara at arm.com>
> ---
> Hi,
>
> commit b62cdbddedc3 ("sunxi: clock: Fix EHCI and OHCI clocks on A64")
> introduced the proper reset and clock gates for the A64. While the patch
> itself is correct, it broke Linux as soon as one actually enables USB0 in
> the DT (in the moment we keep this "disabled" in U-Boot's DT version).
>
> I understand that this patch here is somewhat of a hack, but the proper
> ref-counting is not easy to implement between the separate EHCI and OHCI
> drivers. Those two files are doomed anyway, since driver model clocks
> and reset drivers are already on the horizon:
> https://github.com/apritzel/u-boot/commits/sunxi-dm-WIP
> So lets fix this up for now so that we can use the Linux kernel DTs with
> U-Boot itself.
>
> Cheers,
> Andre.
>
>  drivers/usb/host/ohci-sunxi.c | 16 ++++++++++++----
>  1 file changed, 12 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/usb/host/ohci-sunxi.c b/drivers/usb/host/ohci-sunxi.c
> index 0ddbdbe460..42ffb6cbcb 100644
> --- a/drivers/usb/host/ohci-sunxi.c
> +++ b/drivers/usb/host/ohci-sunxi.c
> @@ -128,10 +128,18 @@ static int ohci_usb_remove(struct udevice *dev)
>         if (ret)
>                 return ret;
>
> -       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);
> +       /*
> +        * For those SoCs that share the clock and reset gates with the EHCI
> +        * controller, we should not turn them off here, to prevent the
> +        * other one hanging (when the EHCI driver tries to shut itself down).

I've tried this "not to disable usb_clk_cfg" before sending "arm64:
allwinner: a64: Disable ehci1 and ohci1 for bananapi, nanopi" patch.
But, I thought improper shutdown may effect other issue may be with
Linux.

Initially I found an issue of disabling OHCI1 gate clock during OHCI0
shutdown. ohci driver is trying to clear BIT(16) for OHCI0, but it
also clears BIT(17), which is for OHCI1. By adding proper shutdown
support for musb and other things I came to a situation where all
shutdown happen properly. But ofcourse I still see hang.

=> usb reset
resetting USB...

PHY0: mask = 0x101, usb_clk_cfg = 0x30202
sunxi_musb_exit: ahb_gate0  = 0x33004540, reset0_cfg = 0x33004540
EHCI failed to shut down host controller.
ehci_usb_remove: ahb_gate0  = 0x32004540, reset0_cfg = 0x32004540
ohci_usb_remove: ahb_gate0  = 0x22004540, reset0_cfg = 0x22004540
ohci_usb_remove: mask = 0x10000, usb_clk_cfg = 0x20202

PHY1: mask = 0x202, usb_clk_cfg = 0x0
ehci_usb_remove: ahb_gate0  = 0x20004540, reset0_cfg = 0x20004540
<< hang  >>

Do you think this is still an issue with clock? considering proper
shutdown sequence happened above.