All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v4 0/3] usb: meson: fix shared reset control use
@ 2021-12-05 21:58 ` Amjad Ouled-Ameur
  0 siblings, 0 replies; 16+ messages in thread
From: Amjad Ouled-Ameur @ 2021-12-05 21:58 UTC (permalink / raw)
  To: khilman
  Cc: Amjad Ouled-Ameur, p.zabel, balbi, jbrunet, linux-amlogic,
	linux-kernel, linux-usb

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


^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH v4 0/3] usb: meson: fix shared reset control use
@ 2021-12-05 21:58 ` Amjad Ouled-Ameur
  0 siblings, 0 replies; 16+ messages in thread
From: Amjad Ouled-Ameur @ 2021-12-05 21:58 UTC (permalink / raw)
  To: khilman
  Cc: Amjad Ouled-Ameur, p.zabel, balbi, jbrunet, linux-amlogic,
	linux-kernel, linux-usb

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


_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH v4 1/3] phy: amlogic: phy-meson-gxl-usb2: fix shared reset controller use
  2021-12-05 21:58 ` Amjad Ouled-Ameur
@ 2021-12-05 21:58   ` Amjad Ouled-Ameur
  -1 siblings, 0 replies; 16+ messages in thread
From: Amjad Ouled-Ameur @ 2021-12-05 21:58 UTC (permalink / raw)
  To: khilman
  Cc: Amjad Ouled-Ameur, p.zabel, balbi, jbrunet, linux-amlogic,
	linux-kernel, linux-usb

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 <aouledameur@baylibre.com>
Reported-by: Jerome Brunet <jbrunet@baylibre.com>
---
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


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v4 1/3] phy: amlogic: phy-meson-gxl-usb2: fix shared reset controller use
@ 2021-12-05 21:58   ` Amjad Ouled-Ameur
  0 siblings, 0 replies; 16+ messages in thread
From: Amjad Ouled-Ameur @ 2021-12-05 21:58 UTC (permalink / raw)
  To: khilman
  Cc: Amjad Ouled-Ameur, p.zabel, balbi, jbrunet, linux-amlogic,
	linux-kernel, linux-usb

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 <aouledameur@baylibre.com>
Reported-by: Jerome Brunet <jbrunet@baylibre.com>
---
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


_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v4 2/3] phy: amlogic: meson8b-usb2: Use dev_err_probe()
  2021-12-05 21:58 ` Amjad Ouled-Ameur
@ 2021-12-05 21:58   ` Amjad Ouled-Ameur
  -1 siblings, 0 replies; 16+ messages in thread
From: Amjad Ouled-Ameur @ 2021-12-05 21:58 UTC (permalink / raw)
  To: khilman
  Cc: Amjad Ouled-Ameur, p.zabel, balbi, jbrunet, linux-amlogic,
	linux-kernel, linux-usb, martin.blumenstingl

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

Signed-off-by: Amjad Ouled-Ameur <aouledameur@baylibre.com>
Reported-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
---
 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


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v4 2/3] phy: amlogic: meson8b-usb2: Use dev_err_probe()
@ 2021-12-05 21:58   ` Amjad Ouled-Ameur
  0 siblings, 0 replies; 16+ messages in thread
From: Amjad Ouled-Ameur @ 2021-12-05 21:58 UTC (permalink / raw)
  To: khilman
  Cc: Amjad Ouled-Ameur, p.zabel, balbi, jbrunet, linux-amlogic,
	linux-kernel, linux-usb, martin.blumenstingl

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

Signed-off-by: Amjad Ouled-Ameur <aouledameur@baylibre.com>
Reported-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
---
 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


_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v4 3/3] phy: amlogic: meson8b-usb2: fix shared reset control use
  2021-12-05 21:58 ` Amjad Ouled-Ameur
@ 2021-12-05 21:58   ` Amjad Ouled-Ameur
  -1 siblings, 0 replies; 16+ messages in thread
From: Amjad Ouled-Ameur @ 2021-12-05 21:58 UTC (permalink / raw)
  To: khilman
  Cc: Amjad Ouled-Ameur, p.zabel, balbi, jbrunet, linux-amlogic,
	linux-kernel, linux-usb

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 <aouledameur@baylibre.com>
Reported-by: Jerome Brunet <jbrunet@baylibre.com>
---
 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


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v4 3/3] phy: amlogic: meson8b-usb2: fix shared reset control use
@ 2021-12-05 21:58   ` Amjad Ouled-Ameur
  0 siblings, 0 replies; 16+ messages in thread
From: Amjad Ouled-Ameur @ 2021-12-05 21:58 UTC (permalink / raw)
  To: khilman
  Cc: Amjad Ouled-Ameur, p.zabel, balbi, jbrunet, linux-amlogic,
	linux-kernel, linux-usb

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 <aouledameur@baylibre.com>
Reported-by: Jerome Brunet <jbrunet@baylibre.com>
---
 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


_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [PATCH v4 2/3] phy: amlogic: meson8b-usb2: Use dev_err_probe()
  2021-12-05 21:58   ` Amjad Ouled-Ameur
@ 2021-12-06 21:14     ` Martin Blumenstingl
  -1 siblings, 0 replies; 16+ messages in thread
From: Martin Blumenstingl @ 2021-12-06 21:14 UTC (permalink / raw)
  To: Amjad Ouled-Ameur
  Cc: khilman, p.zabel, balbi, jbrunet, linux-amlogic, linux-kernel, linux-usb

On Sun, Dec 5, 2021 at 10:58 PM Amjad Ouled-Ameur
<aouledameur@baylibre.com> wrote:
>
> Use the existing dev_err_probe() helper instead of open-coding the same
> operation.
>
> Signed-off-by: Amjad Ouled-Ameur <aouledameur@baylibre.com>
> Reported-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
Reviewed-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>

Thanks for taking care of this!

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v4 2/3] phy: amlogic: meson8b-usb2: Use dev_err_probe()
@ 2021-12-06 21:14     ` Martin Blumenstingl
  0 siblings, 0 replies; 16+ messages in thread
From: Martin Blumenstingl @ 2021-12-06 21:14 UTC (permalink / raw)
  To: Amjad Ouled-Ameur
  Cc: khilman, p.zabel, balbi, jbrunet, linux-amlogic, linux-kernel, linux-usb

On Sun, Dec 5, 2021 at 10:58 PM Amjad Ouled-Ameur
<aouledameur@baylibre.com> wrote:
>
> Use the existing dev_err_probe() helper instead of open-coding the same
> operation.
>
> Signed-off-by: Amjad Ouled-Ameur <aouledameur@baylibre.com>
> Reported-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
Reviewed-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>

Thanks for taking care of this!

_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v4 3/3] phy: amlogic: meson8b-usb2: fix shared reset control use
  2021-12-05 21:58   ` Amjad Ouled-Ameur
@ 2021-12-06 21:14     ` Martin Blumenstingl
  -1 siblings, 0 replies; 16+ messages in thread
From: Martin Blumenstingl @ 2021-12-06 21:14 UTC (permalink / raw)
  To: Amjad Ouled-Ameur
  Cc: khilman, p.zabel, balbi, jbrunet, linux-amlogic, linux-kernel, linux-usb

On Sun, Dec 5, 2021 at 10:59 PM Amjad Ouled-Ameur
<aouledameur@baylibre.com> 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 <aouledameur@baylibre.com>
> Reported-by: Jerome Brunet <jbrunet@baylibre.com>
Reviewed-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v4 3/3] phy: amlogic: meson8b-usb2: fix shared reset control use
@ 2021-12-06 21:14     ` Martin Blumenstingl
  0 siblings, 0 replies; 16+ messages in thread
From: Martin Blumenstingl @ 2021-12-06 21:14 UTC (permalink / raw)
  To: Amjad Ouled-Ameur
  Cc: khilman, p.zabel, balbi, jbrunet, linux-amlogic, linux-kernel, linux-usb

On Sun, Dec 5, 2021 at 10:59 PM Amjad Ouled-Ameur
<aouledameur@baylibre.com> 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 <aouledameur@baylibre.com>
> Reported-by: Jerome Brunet <jbrunet@baylibre.com>
Reviewed-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>

_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v4 1/3] phy: amlogic: phy-meson-gxl-usb2: fix shared reset controller use
  2021-12-05 21:58   ` Amjad Ouled-Ameur
@ 2021-12-06 21:19     ` Martin Blumenstingl
  -1 siblings, 0 replies; 16+ messages in thread
From: Martin Blumenstingl @ 2021-12-06 21:19 UTC (permalink / raw)
  To: Amjad Ouled-Ameur, p.zabel
  Cc: khilman, balbi, jbrunet, linux-amlogic, linux-kernel, linux-usb

Hi Amjad,

On Sun, Dec 5, 2021 at 10:59 PM Amjad Ouled-Ameur
<aouledameur@baylibre.com> 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 <aouledameur@baylibre.com>
> Reported-by: Jerome Brunet <jbrunet@baylibre.com>
> ---
> 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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v4 1/3] phy: amlogic: phy-meson-gxl-usb2: fix shared reset controller use
@ 2021-12-06 21:19     ` Martin Blumenstingl
  0 siblings, 0 replies; 16+ messages in thread
From: Martin Blumenstingl @ 2021-12-06 21:19 UTC (permalink / raw)
  To: Amjad Ouled-Ameur, p.zabel
  Cc: khilman, balbi, jbrunet, linux-amlogic, linux-kernel, linux-usb

Hi Amjad,

On Sun, Dec 5, 2021 at 10:59 PM Amjad Ouled-Ameur
<aouledameur@baylibre.com> 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 <aouledameur@baylibre.com>
> Reported-by: Jerome Brunet <jbrunet@baylibre.com>
> ---
> 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

_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v4 1/3] phy: amlogic: phy-meson-gxl-usb2: fix shared reset controller use
  2021-12-06 21:19     ` Martin Blumenstingl
@ 2021-12-07 11:20       ` Amjad Ouled-Ameur
  -1 siblings, 0 replies; 16+ messages in thread
From: Amjad Ouled-Ameur @ 2021-12-07 11:20 UTC (permalink / raw)
  To: Martin Blumenstingl, p.zabel
  Cc: khilman, balbi, jbrunet, linux-amlogic, linux-kernel, linux-usb

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
> <aouledameur@baylibre.com> 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 <aouledameur@baylibre.com>
>> Reported-by: Jerome Brunet <jbrunet@baylibre.com>
>> ---
>> 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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v4 1/3] phy: amlogic: phy-meson-gxl-usb2: fix shared reset controller use
@ 2021-12-07 11:20       ` Amjad Ouled-Ameur
  0 siblings, 0 replies; 16+ messages in thread
From: Amjad Ouled-Ameur @ 2021-12-07 11:20 UTC (permalink / raw)
  To: Martin Blumenstingl, p.zabel
  Cc: khilman, balbi, jbrunet, linux-amlogic, linux-kernel, linux-usb

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
> <aouledameur@baylibre.com> 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 <aouledameur@baylibre.com>
>> Reported-by: Jerome Brunet <jbrunet@baylibre.com>
>> ---
>> 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

_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2021-12-07 11:21 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-12-05 21:58 [PATCH v4 0/3] usb: meson: fix shared reset control use Amjad Ouled-Ameur
2021-12-05 21:58 ` Amjad Ouled-Ameur
2021-12-05 21:58 ` [PATCH v4 1/3] phy: amlogic: phy-meson-gxl-usb2: fix shared reset controller use Amjad Ouled-Ameur
2021-12-05 21:58   ` Amjad Ouled-Ameur
2021-12-06 21:19   ` Martin Blumenstingl
2021-12-06 21:19     ` Martin Blumenstingl
2021-12-07 11:20     ` Amjad Ouled-Ameur
2021-12-07 11:20       ` Amjad Ouled-Ameur
2021-12-05 21:58 ` [PATCH v4 2/3] phy: amlogic: meson8b-usb2: Use dev_err_probe() Amjad Ouled-Ameur
2021-12-05 21:58   ` Amjad Ouled-Ameur
2021-12-06 21:14   ` Martin Blumenstingl
2021-12-06 21:14     ` Martin Blumenstingl
2021-12-05 21:58 ` [PATCH v4 3/3] phy: amlogic: meson8b-usb2: fix shared reset control use Amjad Ouled-Ameur
2021-12-05 21:58   ` Amjad Ouled-Ameur
2021-12-06 21:14   ` Martin Blumenstingl
2021-12-06 21:14     ` Martin Blumenstingl

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.