Re: [PATCH 3/4] usb: chipidea: imx: call enable_clk after all resources are requested

[Johannes Zink <j.zink@pengutronix.de>]

On Wed, 2022-07-06 at 16:21 +0200, Marco Felsch wrote:
> Currently we enable the clock immediately after requesting it and
> leave
> it on the whole time. Afterwards if the phy request is failing we
> leave
> the usb-controller <-> usb-phy connection in a partly initialized
> state.
> At least on i.MX8M SoCs this can cause strange system hangs during
> boot.
> 
> The (more) correct way would be to have the whole power-domain
> framework
> and the blk-ctrl driver support within barebox. So we can leave the
> power-domain in a known good state for linux. Since this is not the
> case
> we can move the clk_enable() call so it gets called after we know
> that
> all ressources are available. So the probability to leave the system
> in
> a partly initialized state is lesser.
> 
> Drop the !IS_ERR() since NULL and errors are handled in clk_enable()
> as
> well.
> 
> Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
> ---
>  drivers/usb/imx/chipidea-imx.c | 10 ++++++++--
>  1 file changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/usb/imx/chipidea-imx.c
> b/drivers/usb/imx/chipidea-imx.c
> index bf8b6f1eb8..f71cf80b7d 100644
> --- a/drivers/usb/imx/chipidea-imx.c
> +++ b/drivers/usb/imx/chipidea-imx.c
> @@ -260,8 +260,6 @@ static int imx_chipidea_probe(struct device_d
> *dev)
>          * devices which have only one.
>          */
>         ci->clk = clk_get(dev, NULL);
> -       if (!IS_ERR(ci->clk))
> -               clk_enable(ci->clk);
>  
>         /* Device trees are using both "phys" and "fsl,usbphy". 
> Prefer the
>          * more modern former one but fall back to the old one.
> @@ -302,6 +300,14 @@ static int imx_chipidea_probe(struct device_d
> *dev)
>         ci->data.drvdata = ci;
>         ci->data.usbphy = ci->usbphy;
>  
> +       /*
> +        * Enable the clock after we ensured that all resources are
> available.
> +        * This is crucial since the phy can be missing which and so
> the
> +        * usb-controller <-> usb-phy communication is only partly
> initialized.
> +        * This can trigger strange system hangs at least on i.MX8M
> SoCs.
> +        */
> +       clk_enable(ci->clk);
> +
>         if ((ci->flags & MXC_EHCI_PORTSC_MASK) == MXC_EHCI_MODE_HSIC)
>                 imx_chipidea_port_init(ci);
>  

Tested-by: Johannes Zink <j.zink@pengutronix.de> # i.MX7