2021-12-12 20:19:55

by Amjad Ouled-Ameur

[permalink] [raw]
Subject: [PATCH v5 0/3] usb: meson: fix shared reset control use

This patchset fixes a usb suspend warning seen on the libretech-cc by
using reset_control_rearm() call of the reset framework API.
This call allows a reset consummer to release the reset line even when
just triggered so that it may be triggered again by other reset
consummers.

reset_control_(de)assert() calls are called, in some meson usb drivers,
on a shared reset line when reset_control_reset has been used. This is not
allowed by the reset framework.

Finally the meson usb drivers are updated to use this new call, which
solves the suspend issue addressed by the previous reverted
commit 7a410953d1fb ("usb: dwc3: meson-g12a: fix shared reset control
use").

changes since v4:
- call reset_control_rearm() after clk_prepare_enable() fails

Amjad Ouled-Ameur (3):
phy: amlogic: phy-meson-gxl-usb2: fix shared reset controller use
phy: amlogic: meson8b-usb2: Use dev_err_probe()
phy: amlogic: meson8b-usb2: fix shared reset control use

drivers/phy/amlogic/phy-meson-gxl-usb2.c | 5 ++++-
drivers/phy/amlogic/phy-meson8b-usb2.c | 9 +++++++--
2 files changed, 11 insertions(+), 3 deletions(-)

--
2.25.1



2021-12-12 20:19:58

by Amjad Ouled-Ameur

[permalink] [raw]
Subject: [PATCH v5 2/3] phy: amlogic: meson8b-usb2: Use dev_err_probe()

Use the existing dev_err_probe() helper instead of open-coding the same
operation.

Signed-off-by: Amjad Ouled-Ameur <[email protected]>
Reported-by: Martin Blumenstingl <[email protected]>
---
drivers/phy/amlogic/phy-meson8b-usb2.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/phy/amlogic/phy-meson8b-usb2.c b/drivers/phy/amlogic/phy-meson8b-usb2.c
index cf10bed40528..77e7e9b1428c 100644
--- a/drivers/phy/amlogic/phy-meson8b-usb2.c
+++ b/drivers/phy/amlogic/phy-meson8b-usb2.c
@@ -265,8 +265,9 @@ static int phy_meson8b_usb2_probe(struct platform_device *pdev)
return PTR_ERR(priv->clk_usb);

priv->reset = devm_reset_control_get_optional_shared(&pdev->dev, NULL);
- if (PTR_ERR(priv->reset) == -EPROBE_DEFER)
- return PTR_ERR(priv->reset);
+ if (IS_ERR(priv->reset))
+ return dev_err_probe(&pdev->dev, PTR_ERR(priv->reset),
+ "Failed to get the reset line");

priv->dr_mode = of_usb_get_dr_mode_by_phy(pdev->dev.of_node, -1);
if (priv->dr_mode == USB_DR_MODE_UNKNOWN) {
--
2.25.1


2021-12-12 20:20:01

by Amjad Ouled-Ameur

[permalink] [raw]
Subject: [PATCH v5 1/3] phy: amlogic: phy-meson-gxl-usb2: fix shared reset controller use

Use reset_control_rearm() call if an error occurs in case
phy_meson_gxl_usb2_init() fails after reset() has been called ; or in case
phy_meson_gxl_usb2_exit() is called i.e the resource is no longer used
and the reset line may be triggered again by other devices.

reset_control_rearm() keeps use of triggered_count sane in the reset
framework. Therefore, use of reset_control_reset() on shared reset line
should be balanced with reset_control_rearm().

Signed-off-by: Amjad Ouled-Ameur <[email protected]>
Reported-by: Jerome Brunet <[email protected]>
---
changes since v4:
- Call reset_control_rearm() after clk_prepare_enable() fails

drivers/phy/amlogic/phy-meson-gxl-usb2.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/phy/amlogic/phy-meson-gxl-usb2.c b/drivers/phy/amlogic/phy-meson-gxl-usb2.c
index 2b3c0d730f20..db17c3448bfe 100644
--- a/drivers/phy/amlogic/phy-meson-gxl-usb2.c
+++ b/drivers/phy/amlogic/phy-meson-gxl-usb2.c
@@ -114,8 +114,10 @@ static int phy_meson_gxl_usb2_init(struct phy *phy)
return ret;

ret = clk_prepare_enable(priv->clk);
- if (ret)
+ if (ret) {
+ reset_control_rearm(priv->reset);
return ret;
+ }

return 0;
}
@@ -125,6 +127,7 @@ static int phy_meson_gxl_usb2_exit(struct phy *phy)
struct phy_meson_gxl_usb2_priv *priv = phy_get_drvdata(phy);

clk_disable_unprepare(priv->clk);
+ reset_control_rearm(priv->reset);

return 0;
}
--
2.25.1


2021-12-12 20:20:04

by Amjad Ouled-Ameur

[permalink] [raw]
Subject: [PATCH v5 3/3] phy: amlogic: meson8b-usb2: fix shared reset control use

Use reset_control_rearm() call if an error occurs in case
phy_meson8b_usb2_power_on() fails after reset() has been called, or in
case phy_meson8b_usb2_power_off() is called i.e the resource is no longer
used and the reset line may be triggered again by other devices.

reset_control_rearm() keeps use of triggered_count sane in the reset
framework, use of reset_control_reset() on shared reset line should
be balanced with reset_control_rearm().

Signed-off-by: Amjad Ouled-Ameur <[email protected]>
Reported-by: Jerome Brunet <[email protected]>
---
drivers/phy/amlogic/phy-meson8b-usb2.c | 4 ++++
1 file changed, 4 insertions(+)

diff --git a/drivers/phy/amlogic/phy-meson8b-usb2.c b/drivers/phy/amlogic/phy-meson8b-usb2.c
index 77e7e9b1428c..dd96763911b8 100644
--- a/drivers/phy/amlogic/phy-meson8b-usb2.c
+++ b/drivers/phy/amlogic/phy-meson8b-usb2.c
@@ -154,6 +154,7 @@ static int phy_meson8b_usb2_power_on(struct phy *phy)
ret = clk_prepare_enable(priv->clk_usb_general);
if (ret) {
dev_err(&phy->dev, "Failed to enable USB general clock\n");
+ reset_control_rearm(priv->reset);
return ret;
}

@@ -161,6 +162,7 @@ static int phy_meson8b_usb2_power_on(struct phy *phy)
if (ret) {
dev_err(&phy->dev, "Failed to enable USB DDR clock\n");
clk_disable_unprepare(priv->clk_usb_general);
+ reset_control_rearm(priv->reset);
return ret;
}

@@ -199,6 +201,7 @@ static int phy_meson8b_usb2_power_on(struct phy *phy)
dev_warn(&phy->dev, "USB ID detect failed!\n");
clk_disable_unprepare(priv->clk_usb);
clk_disable_unprepare(priv->clk_usb_general);
+ reset_control_rearm(priv->reset);
return -EINVAL;
}
}
@@ -218,6 +221,7 @@ static int phy_meson8b_usb2_power_off(struct phy *phy)

clk_disable_unprepare(priv->clk_usb);
clk_disable_unprepare(priv->clk_usb_general);
+ reset_control_rearm(priv->reset);

/* power off the PHY by putting it into reset mode */
regmap_update_bits(priv->regmap, REG_CTRL, REG_CTRL_POWER_ON_RESET,
--
2.25.1


2021-12-12 20:45:05

by Martin Blumenstingl

[permalink] [raw]
Subject: Re: [PATCH v5 2/3] phy: amlogic: meson8b-usb2: Use dev_err_probe()

Hi Amjad,

On Sun, Dec 12, 2021 at 9:18 PM Amjad Ouled-Ameur
<[email protected]> wrote:
>
> Use the existing dev_err_probe() helper instead of open-coding the same
> operation.
>
> Signed-off-by: Amjad Ouled-Ameur <[email protected]>
> Reported-by: Martin Blumenstingl <[email protected]>
Reviewed-by: Martin Blumenstingl <[email protected]>

For future patches: if you send another version of the patchset then
please keep any Reviewed-by/Acked-by/etc. for patches in the series
which have not changed.

2021-12-12 20:45:26

by Martin Blumenstingl

[permalink] [raw]
Subject: Re: [PATCH v5 1/3] phy: amlogic: phy-meson-gxl-usb2: fix shared reset controller use

On Sun, Dec 12, 2021 at 9:20 PM Amjad Ouled-Ameur
<[email protected]> wrote:
>
> Use reset_control_rearm() call if an error occurs in case
> phy_meson_gxl_usb2_init() fails after reset() has been called ; or in case
> phy_meson_gxl_usb2_exit() is called i.e the resource is no longer used
> and the reset line may be triggered again by other devices.
>
> reset_control_rearm() keeps use of triggered_count sane in the reset
> framework. Therefore, use of reset_control_reset() on shared reset line
> should be balanced with reset_control_rearm().
>
> Signed-off-by: Amjad Ouled-Ameur <[email protected]>
> Reported-by: Jerome Brunet <[email protected]>
Reviewed-by: Martin Blumenstingl <[email protected]>

2021-12-12 20:45:46

by Martin Blumenstingl

[permalink] [raw]
Subject: Re: [PATCH v5 3/3] phy: amlogic: meson8b-usb2: fix shared reset control use

On Sun, Dec 12, 2021 at 9:20 PM Amjad Ouled-Ameur
<[email protected]> wrote:
>
> Use reset_control_rearm() call if an error occurs in case
> phy_meson8b_usb2_power_on() fails after reset() has been called, or in
> case phy_meson8b_usb2_power_off() is called i.e the resource is no longer
> used and the reset line may be triggered again by other devices.
>
> reset_control_rearm() keeps use of triggered_count sane in the reset
> framework, use of reset_control_reset() on shared reset line should
> be balanced with reset_control_rearm().
>
> Signed-off-by: Amjad Ouled-Ameur <[email protected]>
> Reported-by: Jerome Brunet <[email protected]>
Reviewed-by: Martin Blumenstingl <[email protected]>

2021-12-13 09:16:21

by Philipp Zabel

[permalink] [raw]
Subject: Re: [PATCH v5 1/3] phy: amlogic: phy-meson-gxl-usb2: fix shared reset controller use

On Sun, 2021-12-12 at 21:18 +0100, Amjad Ouled-Ameur wrote:
> Use reset_control_rearm() call if an error occurs in case
> phy_meson_gxl_usb2_init() fails after reset() has been called ; or in case
> phy_meson_gxl_usb2_exit() is called i.e the resource is no longer used
> and the reset line may be triggered again by other devices.
>
> reset_control_rearm() keeps use of triggered_count sane in the reset
> framework. Therefore, use of reset_control_reset() on shared reset line
> should be balanced with reset_control_rearm().
>
> Signed-off-by: Amjad Ouled-Ameur <[email protected]>
> Reported-by: Jerome Brunet <[email protected]>

Reviewed-by: Philipp Zabel <[email protected]>

regards
Philipp

2021-12-13 13:30:58

by Neil Armstrong

[permalink] [raw]
Subject: Re: [PATCH v5 0/3] usb: meson: fix shared reset control use

Hi Amljad,

On 12/12/2021 21:18, Amjad Ouled-Ameur wrote:
> This patchset fixes a usb suspend warning seen on the libretech-cc by
> using reset_control_rearm() call of the reset framework API.
> This call allows a reset consummer to release the reset line even when
> just triggered so that it may be triggered again by other reset
> consummers.
>
> reset_control_(de)assert() calls are called, in some meson usb drivers,
> on a shared reset line when reset_control_reset has been used. This is not
> allowed by the reset framework.
>
> Finally the meson usb drivers are updated to use this new call, which
> solves the suspend issue addressed by the previous reverted
> commit 7a410953d1fb ("usb: dwc3: meson-g12a: fix shared reset control
> use").
>
> changes since v4:
> - call reset_control_rearm() after clk_prepare_enable() fails
>
> Amjad Ouled-Ameur (3):
> phy: amlogic: phy-meson-gxl-usb2: fix shared reset controller use
> phy: amlogic: meson8b-usb2: Use dev_err_probe()
> phy: amlogic: meson8b-usb2: fix shared reset control use
>
> drivers/phy/amlogic/phy-meson-gxl-usb2.c | 5 ++++-
> drivers/phy/amlogic/phy-meson8b-usb2.c | 9 +++++++--
> 2 files changed, 11 insertions(+), 3 deletions(-)
>

For whole serie:

Acked-by: Neil Armstrong <[email protected]>

Thanks for fixing that !

Neil

2022-01-06 19:06:16

by Kevin Hilman

[permalink] [raw]
Subject: Re: [PATCH v5 0/3] usb: meson: fix shared reset control use

Hi Amjad,

Amjad Ouled-Ameur <[email protected]> writes:

> This patchset fixes a usb suspend warning seen on the libretech-cc by
> using reset_control_rearm() call of the reset framework API.
> This call allows a reset consummer to release the reset line even when
> just triggered so that it may be triggered again by other reset
> consummers.
>
> reset_control_(de)assert() calls are called, in some meson usb drivers,
> on a shared reset line when reset_control_reset has been used. This is not
> allowed by the reset framework.
>
> Finally the meson usb drivers are updated to use this new call, which
> solves the suspend issue addressed by the previous reverted
> commit 7a410953d1fb ("usb: dwc3: meson-g12a: fix shared reset control
> use").
>
> changes since v4:
> - call reset_control_rearm() after clk_prepare_enable() fails
>
> Amjad Ouled-Ameur (3):
> phy: amlogic: phy-meson-gxl-usb2: fix shared reset controller use
> phy: amlogic: meson8b-usb2: Use dev_err_probe()
> phy: amlogic: meson8b-usb2: fix shared reset control use
>
> drivers/phy/amlogic/phy-meson-gxl-usb2.c | 5 ++++-
> drivers/phy/amlogic/phy-meson8b-usb2.c | 9 +++++++--
> 2 files changed, 11 insertions(+), 3 deletions(-)

The cover letter prefix is "usb: meson" but all the patches are for
drivers/phy.

Could you collect the reviewed-by etc tags and resend to the PHY
maintainers and linux-phy list?

Thanks,

Kevin