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 v3:
- Remove unnecessary reset_control_rearm() after reset_control_reset()
failure.
- Use dev_err_probe().
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 | 1 +
drivers/phy/amlogic/phy-meson8b-usb2.c | 9 +++++++--
2 files changed, 8 insertions(+), 2 deletions(-)
--
2.25.1
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
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 v3:
- Remove unnecessary reset_control_rearm() after reset_control_reset()
failure.
drivers/phy/amlogic/phy-meson-gxl-usb2.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/phy/amlogic/phy-meson-gxl-usb2.c b/drivers/phy/amlogic/phy-meson-gxl-usb2.c
index 2b3c0d730f20..d2f56075dc57 100644
--- a/drivers/phy/amlogic/phy-meson-gxl-usb2.c
+++ b/drivers/phy/amlogic/phy-meson-gxl-usb2.c
@@ -125,6 +125,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
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
On Sun, Dec 5, 2021 at 10:58 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]>
Thanks for taking care of this!
On Sun, Dec 5, 2021 at 10:59 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]>
Hi Amjad,
On Sun, Dec 5, 2021 at 10:59 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]>
> ---
> changes since v3:
> - Remove unnecessary reset_control_rearm() after reset_control_reset()
> failure.
I double-checked your patch in v3 and Philipp was right:
reset_control_rearm() should not be right after reset_control_reset().
However, I think reset_control_rearm() is still needed
phy_meson_gxl_usb2_init() whenever clk_prepare_enable() fails.
So my suggestion is to add reset_control_rearm() in
phy_meson_gxl_usb2_init() if clk_prepare_enable() fails so we are
resetting the ref-count for the reset line (just like
phy_meson_gxl_usb2_exit() does).
The difference compared to the previous version is that the
reset_control_rearm() call needs to be placed a few lines down.
Thank you!
Martin
Hi Martin,
Thank you for the thorough review.
On 06/12/2021 22:19, Martin Blumenstingl wrote:
> Hi Amjad,
>
> On Sun, Dec 5, 2021 at 10:59 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]>
>> ---
>> changes since v3:
>> - Remove unnecessary reset_control_rearm() after reset_control_reset()
>> failure.
> I double-checked your patch in v3 and Philipp was right:
> reset_control_rearm() should not be right after reset_control_reset().
> However, I think reset_control_rearm() is still needed
> phy_meson_gxl_usb2_init() whenever clk_prepare_enable() fails.
Well seen, reset_control_rearm() should actually be called after
clk_prepare_enable() fails. I will wait for any other potential reviews
before sending next version with this change you suggested.
Thank you Martin.
Amjad
> So my suggestion is to add reset_control_rearm() in
> phy_meson_gxl_usb2_init() if clk_prepare_enable() fails so we are
> resetting the ref-count for the reset line (just like
> phy_meson_gxl_usb2_exit() does).
> The difference compared to the previous version is that the
> reset_control_rearm() call needs to be placed a few lines down.
>
>
> Thank you!
> Martin