[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <76b47a88-e40a-a95a-bc0b-1899a655de5a@baylibre.com>
Date: Thu, 27 Aug 2020 16:54:30 +0200
From: Neil Armstrong <narmstrong@...libre.com>
To: Amjad Ouled-Ameur <aouledameur@...libre.com>,
Felipe Balbi <balbi@...nel.org>
Cc: Dan Robertson <dan@...obertson.com>,
Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
Kevin Hilman <khilman@...libre.com>,
Jerome Brunet <jbrunet@...libre.com>,
Martin Blumenstingl <martin.blumenstingl@...glemail.com>,
Philipp Zabel <p.zabel@...gutronix.de>,
linux-usb@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
linux-amlogic@...ts.infradead.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] Revert "usb: dwc3: meson-g12a: fix shared reset control
use"
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@...libre.com>
> Reported-by: Jerome Brunet <jbrunet@...libre.com>
> ---
> 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);
>
Acked-by: Neil Armstrong <narmstrong@...libre.com>
Powered by blists - more mailing lists