* [PATCH] Revert "usb: dwc3: meson-g12a: fix shared reset control use"
@ 2020-08-27 14:48 Amjad Ouled-Ameur
2020-08-27 14:54 ` Neil Armstrong
` (2 more replies)
0 siblings, 3 replies; 5+ messages in thread
From: Amjad Ouled-Ameur @ 2020-08-27 14:48 UTC (permalink / raw)
To: Felipe Balbi
Cc: Dan Robertson, Neil Armstrong, Martin Blumenstingl,
Greg Kroah-Hartman, linux-usb, linux-kernel, Amjad Ouled-Ameur,
Philipp Zabel, Kevin Hilman, linux-amlogic, linux-arm-kernel,
Jerome Brunet
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@baylibre.com>
Reported-by: Jerome Brunet <jbrunet@baylibre.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);
--
2.17.1
_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] Revert "usb: dwc3: meson-g12a: fix shared reset control use"
2020-08-27 14:48 [PATCH] Revert "usb: dwc3: meson-g12a: fix shared reset control use" Amjad Ouled-Ameur
@ 2020-08-27 14:54 ` Neil Armstrong
2020-08-27 15:03 ` Philipp Zabel
2020-09-02 6:57 ` Neil Armstrong
2 siblings, 0 replies; 5+ messages in thread
From: Neil Armstrong @ 2020-08-27 14:54 UTC (permalink / raw)
To: Amjad Ouled-Ameur, Felipe Balbi
Cc: Dan Robertson, Martin Blumenstingl, Greg Kroah-Hartman,
linux-usb, linux-kernel, Philipp Zabel, Kevin Hilman,
linux-amlogic, linux-arm-kernel, Jerome Brunet
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@baylibre.com>
> Reported-by: Jerome Brunet <jbrunet@baylibre.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@baylibre.com>
_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] Revert "usb: dwc3: meson-g12a: fix shared reset control use"
2020-08-27 14:48 [PATCH] Revert "usb: dwc3: meson-g12a: fix shared reset control use" Amjad Ouled-Ameur
2020-08-27 14:54 ` Neil Armstrong
@ 2020-08-27 15:03 ` Philipp Zabel
2020-09-02 6:57 ` Neil Armstrong
2 siblings, 0 replies; 5+ messages in thread
From: Philipp Zabel @ 2020-08-27 15:03 UTC (permalink / raw)
To: Amjad Ouled-Ameur, Felipe Balbi
Cc: Dan Robertson, Neil Armstrong, Martin Blumenstingl,
Greg Kroah-Hartman, linux-usb, linux-kernel, Kevin Hilman,
linux-amlogic, linux-arm-kernel, Jerome Brunet
On Thu, 2020-08-27 at 16:48 +0200, 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@baylibre.com>
> Reported-by: Jerome Brunet <jbrunet@baylibre.com>
Acked-by: Philipp Zabel <p.zabel@pengutronix.de>
regards
Philipp
_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] Revert "usb: dwc3: meson-g12a: fix shared reset control use"
2020-08-27 14:48 [PATCH] Revert "usb: dwc3: meson-g12a: fix shared reset control use" Amjad Ouled-Ameur
2020-08-27 14:54 ` Neil Armstrong
2020-08-27 15:03 ` Philipp Zabel
@ 2020-09-02 6:57 ` Neil Armstrong
2020-09-04 14:33 ` Greg Kroah-Hartman
2 siblings, 1 reply; 5+ messages in thread
From: Neil Armstrong @ 2020-09-02 6:57 UTC (permalink / raw)
To: Felipe Balbi, Greg Kroah-Hartman
Cc: Dan Robertson, Martin Blumenstingl, Kevin Hilman, linux-usb,
linux-kernel, Amjad Ouled-Ameur, Philipp Zabel, linux-amlogic,
linux-arm-kernel, Jerome Brunet
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@baylibre.com>
> Reported-by: Jerome Brunet <jbrunet@baylibre.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);
>
_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] Revert "usb: dwc3: meson-g12a: fix shared reset control use"
2020-09-02 6:57 ` Neil Armstrong
@ 2020-09-04 14:33 ` Greg Kroah-Hartman
0 siblings, 0 replies; 5+ messages in thread
From: Greg Kroah-Hartman @ 2020-09-04 14:33 UTC (permalink / raw)
To: Neil Armstrong
Cc: Felipe Balbi, Dan Robertson, Martin Blumenstingl, Kevin Hilman,
linux-usb, linux-kernel, Amjad Ouled-Ameur, Philipp Zabel,
linux-amlogic, linux-arm-kernel, Jerome Brunet
On Wed, Sep 02, 2020 at 08:57:42AM +0200, Neil Armstrong wrote:
> 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.
Yes, will queue it up now, thanks.
greg k-h
_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2020-09-04 14:33 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-08-27 14:48 [PATCH] Revert "usb: dwc3: meson-g12a: fix shared reset control use" Amjad Ouled-Ameur
2020-08-27 14:54 ` Neil Armstrong
2020-08-27 15:03 ` Philipp Zabel
2020-09-02 6:57 ` Neil Armstrong
2020-09-04 14:33 ` Greg Kroah-Hartman
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®