Re: [PATCH] Revert "usb: dwc3: meson-g12a: fix shared reset control use"

From: Neil Armstrong
Date: Wed Sep 02 2020 - 02:57:56 EST


Hi Felipe, Greg,

Is it possible to apply this ? USB is broken on 5.8 stable and 5.9-rc on multiple GXL/GXM boards.

Thanks,
Neil

On 27/08/2020 16:48, Amjad Ouled-Ameur wrote:
> This reverts commit 7a410953d1fb4dbe91ffcfdee9cbbf889d19b0d7.
>
> This commit breaks USB on meson-gxl-s905x-libretech-cc. Reverting
> the change solves the issue.
>
> In fact, according to the reset framework code, consumers must not use
> reset_control_(de)assert() on shared reset lines when reset_control_reset
> has been used, and vice-versa.
>
> Moreover, with this commit, usb is not guaranted to be reset since the
> reset is likely to be initially deasserted.
>
> Reverting the commit will bring back the suspend warning mentioned in the
> commit description. Nevertheless, a warning is much less critical than
> breaking dwc3-meson-g12a USB completely. We will address the warning
> issue in another way as a 2nd step.
>
> Signed-off-by: Amjad Ouled-Ameur <aouledameur@xxxxxxxxxxxx>
> Reported-by: Jerome Brunet <jbrunet@xxxxxxxxxxxx>
> ---
> drivers/usb/dwc3/dwc3-meson-g12a.c | 15 ++++++---------
> 1 file changed, 6 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/usb/dwc3/dwc3-meson-g12a.c b/drivers/usb/dwc3/dwc3-meson-g12a.c
> index 88b75b5a039c..1f7f4d88ed9d 100644
> --- a/drivers/usb/dwc3/dwc3-meson-g12a.c
> +++ b/drivers/usb/dwc3/dwc3-meson-g12a.c
> @@ -737,13 +737,13 @@ static int dwc3_meson_g12a_probe(struct platform_device *pdev)
> goto err_disable_clks;
> }
>
> - ret = reset_control_deassert(priv->reset);
> + ret = reset_control_reset(priv->reset);
> if (ret)
> - goto err_assert_reset;
> + goto err_disable_clks;
>
> ret = dwc3_meson_g12a_get_phys(priv);
> if (ret)
> - goto err_assert_reset;
> + goto err_disable_clks;
>
> ret = priv->drvdata->setup_regmaps(priv, base);
> if (ret)
> @@ -752,7 +752,7 @@ static int dwc3_meson_g12a_probe(struct platform_device *pdev)
> if (priv->vbus) {
> ret = regulator_enable(priv->vbus);
> if (ret)
> - goto err_assert_reset;
> + goto err_disable_clks;
> }
>
> /* Get dr_mode */
> @@ -765,13 +765,13 @@ static int dwc3_meson_g12a_probe(struct platform_device *pdev)
>
> ret = priv->drvdata->usb_init(priv);
> if (ret)
> - goto err_assert_reset;
> + goto err_disable_clks;
>
> /* Init PHYs */
> for (i = 0 ; i < PHY_COUNT ; ++i) {
> ret = phy_init(priv->phys[i]);
> if (ret)
> - goto err_assert_reset;
> + goto err_disable_clks;
> }
>
> /* Set PHY Power */
> @@ -809,9 +809,6 @@ static int dwc3_meson_g12a_probe(struct platform_device *pdev)
> for (i = 0 ; i < PHY_COUNT ; ++i)
> phy_exit(priv->phys[i]);
>
> -err_assert_reset:
> - reset_control_assert(priv->reset);
> -
> err_disable_clks:
> clk_bulk_disable_unprepare(priv->drvdata->num_clks,
> priv->drvdata->clks);
>