[PATCH 0/3] enable reset_control_reset for shared reset lines

[PATCH 0/3] enable reset_control_reset for shared reset lines

[Martin Blumenstingl]

This patch allows using reset_control_reset on a shared reset line.
There are some restrictions though (all of these only apply to shared
resets):
- reset_control_reset cannot be used after reset_control_assert
- reset_control_assert cannot be used after reset_control_reset
- only the first call to reset_control_reset will be passed to the
  hardware. The only exception is when all consumers of the
  reset_control are gone (reset_control_put)

The idea came up when discussing the reset logic in the
phy-meson8b-usb2 PHY driver, see [0] and [1]

An example where this is needed is the Amlogic Meson8b and GXBB SoCs
where two USB PHYs share one reset line. The reset controller only
supports reset pulses (and thus only implements .reset).
The only workaround so far is to make the reset optional in the
driver and only add the reset to one of the two PHYs, to mimic the
behavior implemented by patch 1.

I also added the Meson USB2 PHY patch to this series to show that this
new functionality can be used with only minimal changes. These patches
can simply be dropped as they depend on the phy-meson8b-usb2 driver
patch, and the dts patch has a runtime-dependency on patch 1 and 2
(otherwise the phy-meson8b-usb2 driver would try to request the reset
line exclusively, preventing the other PHY from initializing).


[0] http://lists.infradead.org/pipermail/linux-amlogic/2016-September/001233.html
[1] http://lists.infradead.org/pipermail/linux-amlogic/2016-September/001197.html


Martin Blumenstingl (3):
  reset: allow using reset_control_reset with shared reset
  phy: meson8b-usb2: request a shared reset line
  ARM64: dts: meson-gxbb: add the USB reset also to the second USB PHY

 arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi |  1 +
 drivers/phy/phy-meson8b-usb2.c              |  3 +--
 drivers/reset/core.c                        | 31 +++++++++++++++++++++++------
 3 files changed, 27 insertions(+), 8 deletions(-)

-- 
2.10.0

[PATCH 1/3] reset: allow using reset_control_reset with shared reset

[Martin Blumenstingl]

Some SoCs (for example Amlogic GXBB) implement a reset controller which
only supports a reset pulse (triggered via reset_control_reset). At the
same time multiple devices (in case of the Amlogic GXBB SoC both USB
PHYs) are sharing the same reset line.

This patch allows using reset_control_reset also for shared resets.
There are limitations though:
reset_control_reset can only be used if reset_control_assert was not
used yet.
reset_control_assert can only be used if reset_control_reset was not
used yet.
For shared resets the reset is only triggered once for the lifetime of
the reset_control instance (the reset can be triggered again if all
consumers of that specific reset_control are gone, as the reset
framework will free the reset_control instance in that case).

Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
---
 drivers/reset/core.c | 31 +++++++++++++++++++++++++------
 1 file changed, 25 insertions(+), 6 deletions(-)

diff --git a/drivers/reset/core.c b/drivers/reset/core.c
index 395dc9c..1565348 100644
--- a/drivers/reset/core.c
+++ b/drivers/reset/core.c
@@ -32,6 +32,7 @@ static LIST_HEAD(reset_controller_list);
  * @refcnt: Number of gets of this reset_control
  * @shared: Is this a shared (1), or an exclusive (0) reset_control?
  * @deassert_cnt: Number of times this reset line has been deasserted
+ * triggered_count: Number of times this reset line has been reset
  */
 struct reset_control {
 	struct reset_controller_dev *rcdev;
@@ -40,6 +41,7 @@ struct reset_control {
 	unsigned int refcnt;
 	int shared;
 	atomic_t deassert_count;
+	atomic_t triggered_count;
 };
 
 /**
@@ -134,17 +136,28 @@ EXPORT_SYMBOL_GPL(devm_reset_controller_register);
  * reset_control_reset - reset the controlled device
  * @rstc: reset controller
  *
- * Calling this on a shared reset controller is an error.
+ * On a shared reset line the actual reset pulse is only triggered once.
  */
 int reset_control_reset(struct reset_control *rstc)
 {
-	if (WARN_ON(rstc->shared))
-		return -EINVAL;
+	int ret;
 
-	if (rstc->rcdev->ops->reset)
-		return rstc->rcdev->ops->reset(rstc->rcdev, rstc->id);
+	if (!rstc->rcdev->ops->reset)
+		return -ENOTSUPP;
 
-	return -ENOTSUPP;
+	if (rstc->shared) {
+		if (WARN_ON(atomic_read(&rstc->deassert_count) != 0))
+			return -EINVAL;
+
+		if (atomic_read(&rstc->triggered_count) != 0)
+			return 0;
+	}
+
+	ret = rstc->rcdev->ops->reset(rstc->rcdev, rstc->id);
+	if (rstc->shared && !ret)
+		atomic_inc(&rstc->triggered_count);
+
+	return ret;
 }
 EXPORT_SYMBOL_GPL(reset_control_reset);
 
@@ -165,6 +178,9 @@ int reset_control_assert(struct reset_control *rstc)
 		return -ENOTSUPP;
 
 	if (rstc->shared) {
+		if (WARN_ON(atomic_read(&rstc->triggered_count) != 0))
+			return -EINVAL;
+
 		if (WARN_ON(atomic_read(&rstc->deassert_count) == 0))
 			return -EINVAL;
 
@@ -188,6 +204,9 @@ int reset_control_deassert(struct reset_control *rstc)
 		return -ENOTSUPP;
 
 	if (rstc->shared) {
+		if (WARN_ON(atomic_read(&rstc->triggered_count) != 0))
+			return -EINVAL;
+
 		if (atomic_inc_return(&rstc->deassert_count) != 1)
 			return 0;
 	}
-- 
2.10.0

[PATCH 1/3] reset: allow using reset_control_reset with shared reset

[Philipp Zabel]

Hi Martin,

thank you for the patch. I have a few comments below.

Am Samstag, den 01.10.2016, 17:21 +0200 schrieb Martin Blumenstingl:
> Some SoCs (for example Amlogic GXBB) implement a reset controller which
> only supports a reset pulse (triggered via reset_control_reset). At the
> same time multiple devices (in case of the Amlogic GXBB SoC both USB
> PHYs) are sharing the same reset line.
> 
> This patch allows using reset_control_reset also for shared resets.
> There are limitations though:
> reset_control_reset can only be used if reset_control_assert was not
> used yet.
> reset_control_assert can only be used if reset_control_reset was not
> used yet.
> For shared resets the reset is only triggered once for the lifetime of
> the reset_control instance (the reset can be triggered again if all
> consumers of that specific reset_control are gone, as the reset
> framework will free the reset_control instance in that case).
> 
> Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
> ---
>  drivers/reset/core.c | 31 +++++++++++++++++++++++++------
>  1 file changed, 25 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/reset/core.c b/drivers/reset/core.c
> index 395dc9c..1565348 100644
> --- a/drivers/reset/core.c
> +++ b/drivers/reset/core.c
> @@ -32,6 +32,7 @@ static LIST_HEAD(reset_controller_list);
>   * @refcnt: Number of gets of this reset_control
>   * @shared: Is this a shared (1), or an exclusive (0) reset_control?
>   * @deassert_cnt: Number of times this reset line has been deasserted
> + * triggered_count: Number of times this reset line has been reset

Missing @

To avoid confusion, should we mention here that triggered_count will
always be <= 1?

>   */
>  struct reset_control {
>  	struct reset_controller_dev *rcdev;
> @@ -40,6 +41,7 @@ struct reset_control {
>  	unsigned int refcnt;
>  	int shared;
>  	atomic_t deassert_count;
> +	atomic_t triggered_count;
>  };
>  
>  /**
> @@ -134,17 +136,28 @@ EXPORT_SYMBOL_GPL(devm_reset_controller_register);
>   * reset_control_reset - reset the controlled device
>   * @rstc: reset controller
>   *
> - * Calling this on a shared reset controller is an error.
> + * On a shared reset line the actual reset pulse is only triggered once.

... for the lifetime of the reset_control instance, for all but the
first caller this is a no-op.

Add a note that it is invalid for different consumers to mix this and
reset_control_(de)assert on the same shared reset line, also to the
reset_control_(de)assert kerneldoc comments.

>   */
>  int reset_control_reset(struct reset_control *rstc)
>  {
> -	if (WARN_ON(rstc->shared))
> -		return -EINVAL;
> +	int ret;
>  
> -	if (rstc->rcdev->ops->reset)
> -		return rstc->rcdev->ops->reset(rstc->rcdev, rstc->id);
> +	if (!rstc->rcdev->ops->reset)
> +		return -ENOTSUPP;
>  
> -	return -ENOTSUPP;
> +	if (rstc->shared) {
> +		if (WARN_ON(atomic_read(&rstc->deassert_count) != 0))
> +			return -EINVAL;

I'm not worried about races between reset and (de)assert for
deassert_count because that's invalid use, but

> +		if (atomic_read(&rstc->triggered_count) != 0)
> +			return 0;

two simultaneously called _resets probably shouldn't race for
triggered_count, so I think this should be

		if (atomic_inc_return(&rstc->triggered_count) != 0)
			return 0;

instead.

> +	}
> +
> +	ret = rstc->rcdev->ops->reset(rstc->rcdev, rstc->id);
> +	if (rstc->shared && !ret)
> +		atomic_inc(&rstc->triggered_count);

With this dropped, that would mean instead of only incrementing
triggered_count if the reset callback returned without error, we'd also
increment in the error case.
I'm not sure if this could be a problem as I wouldn't expect the reset
callback to fail once and then work later, but maybe this could be
handled by instead replacing it with

	if (rstc->shared && ret)
		atomic_dec(&rstc->triggered_count)

which would again introduce a race, but only in the error case.

> +
> +	return ret;
>  }
>  EXPORT_SYMBOL_GPL(reset_control_reset);
>  
> @@ -165,6 +178,9 @@ int reset_control_assert(struct reset_control *rstc)
>  		return -ENOTSUPP;
>  
>  	if (rstc->shared) {
> +		if (WARN_ON(atomic_read(&rstc->triggered_count) != 0))
> +			return -EINVAL;
> +
>  		if (WARN_ON(atomic_read(&rstc->deassert_count) == 0))
>  			return -EINVAL;
>  
> @@ -188,6 +204,9 @@ int reset_control_deassert(struct reset_control *rstc)
>  		return -ENOTSUPP;
>  
>  	if (rstc->shared) {
> +		if (WARN_ON(atomic_read(&rstc->triggered_count) != 0))
> +			return -EINVAL;
> +
>  		if (atomic_inc_return(&rstc->deassert_count) != 1)
>  			return 0;
>  	}

regards
Philipp

[PATCH 2/3] phy: meson8b-usb2: request a shared reset line

[Martin Blumenstingl]

Both PHYs are sharing one reset line. With recent improvements to the
reset framework we can now also use reset_control_reset with shared
resets.
This allows us to drop some workarounds where the reset was only
specified for one PHY but not the other, to make sure that the reset it
only executed once (as the reset framework was not able to use
reset_control_reset with shared reset lines).

Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
---
 drivers/phy/phy-meson8b-usb2.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/phy/phy-meson8b-usb2.c b/drivers/phy/phy-meson8b-usb2.c
index 73bf632..f1ee96a 100644
--- a/drivers/phy/phy-meson8b-usb2.c
+++ b/drivers/phy/phy-meson8b-usb2.c
@@ -237,8 +237,7 @@ static int phy_meson8b_usb2_probe(struct platform_device *pdev)
 	if (IS_ERR(priv->clk_usb))
 		return PTR_ERR(priv->clk_usb);
 
-	priv->reset = devm_reset_control_get_optional_exclusive(&pdev->dev,
-								NULL);
+	priv->reset = devm_reset_control_get_optional_shared(&pdev->dev, NULL);
 	if (PTR_ERR(priv->reset) == -EPROBE_DEFER)
 		return PTR_ERR(priv->reset);
 
-- 
2.10.0

[PATCH 3/3] ARM64: dts: meson-gxbb: add the USB reset also to the second USB PHY

[Martin Blumenstingl]

When the USB PHY driver was introduced the reset framework did not
have support for triggering a reset pulse for shared resets. On GXBB
however there is only one reset line for both PHYs (meaning we have a
shared reset line). With the latest changes to the reset framework and
the corresponding updates to the phy-meson8b-usb2 driver we can now pass
the reset to the second PHY as well.

Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
---
 arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi b/arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi
index cfa3902..2e89ec4 100644
--- a/arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi
+++ b/arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi
@@ -165,6 +165,7 @@
 			compatible = "amlogic,meson-gxbb-usb2-phy";
 			#phy-cells = <0>;
 			reg = <0x0 0xc0000020 0x0 0x20>;
+			resets = <&reset RESET_USB_OTG>;
 			clocks = <&clkc CLKID_USB>, <&clkc CLKID_USB1>;
 			clock-names = "usb_general", "usb";
 			status = "disabled";
-- 
2.10.0

[PATCH 0/3] enable reset_control_reset for shared reset lines

[Martin Blumenstingl]

This patch allows using reset_control_reset on a shared reset line.
There are some restrictions though (all of these only apply to shared
resets):
- reset_control_reset cannot be used after reset_control_assert
- reset_control_assert cannot be used after reset_control_reset
- only the first call to reset_control_reset will be passed to the
  hardware. The only exception is when all consumers of the
  reset_control are gone (reset_control_put)

The idea came up when discussing the reset logic in the
phy-meson8b-usb2 PHY driver, see [0] and [1]

An example where this is needed is the Amlogic Meson8b and GXBB SoCs
where two USB PHYs share one reset line. The reset controller only
supports reset pulses (and thus only implements .reset).
The only workaround so far is to make the reset optional in the
driver and only add the reset to one of the two PHYs, to mimic the
behavior implemented by patch 1.

I also added the Meson USB2 PHY patch to this series to show that this
new functionality can be used with only minimal changes. These patches
can simply be dropped as they depend on the phy-meson8b-usb2 driver
patch, and the dts patch has a runtime-dependency on patch 1 and 2
(otherwise the phy-meson8b-usb2 driver would try to request the reset
line exclusively, preventing the other PHY from initializing).


Changes since RFC (thanks to Philipp Zabel for reviewing the RFC):
- refined kernel documentation of triggered_count and fixed missing
  struct member "@" prefix
- refined reset_control_reset kernel documentation
- added note that reset_control_(de)assert and reset_control_reset
  can not be used together for shared resets) in the kernel documentation
- fixed a potential race condition in reset_control_reset (the new
  logic still has a potential race condition, but only in the error case,
  which is pretty unlikely to happen in my opinion)
- rebased to apply after reset/core.c changes ("reset: warn on invalid
  input to reset_control_reset/assert/deassert/status")


[0] http://lists.infradead.org/pipermail/linux-amlogic/2016-September/001233.html
[1] http://lists.infradead.org/pipermail/linux-amlogic/2016-September/001197.html

Martin Blumenstingl (3):
  reset: allow using reset_control_reset with shared reset
  phy: meson8b-usb2: request a shared reset line
  ARM64: dts: meson-gxbb: add the USB reset also to the second USB PHY

 arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi |  1 +
 drivers/phy/phy-meson8b-usb2.c              |  3 +-
 drivers/reset/core.c                        | 43 +++++++++++++++++++++++++----
 3 files changed, 39 insertions(+), 8 deletions(-)

-- 
2.10.2

[PATCH 1/3] reset: allow using reset_control_reset with shared reset

[Martin Blumenstingl]

Some SoCs (for example Amlogic GXBB) implement a reset controller which
only supports a reset pulse (triggered via reset_control_reset). At the
same time multiple devices (in case of the Amlogic GXBB SoC both USB
PHYs) are sharing the same reset line.

This patch allows using reset_control_reset also for shared resets.
There are limitations though:
reset_control_reset can only be used if reset_control_assert was not
used yet.
reset_control_assert can only be used if reset_control_reset was not
used yet.
For shared resets the reset is only triggered once for the lifetime of
the reset_control instance (the reset can be triggered again if all
consumers of that specific reset_control are gone, as the reset
framework will free the reset_control instance in that case).

Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
---
 drivers/reset/core.c | 43 +++++++++++++++++++++++++++++++++++++------
 1 file changed, 37 insertions(+), 6 deletions(-)

diff --git a/drivers/reset/core.c b/drivers/reset/core.c
index b8ae1db..10368ed 100644
--- a/drivers/reset/core.c
+++ b/drivers/reset/core.c
@@ -32,6 +32,9 @@ static LIST_HEAD(reset_controller_list);
  * @refcnt: Number of gets of this reset_control
  * @shared: Is this a shared (1), or an exclusive (0) reset_control?
  * @deassert_cnt: Number of times this reset line has been deasserted
+ * @triggered_count: Number of times this reset line has been reset. Currently
+ *                   only used for shared resets, which means that the value
+ *                   will be either 0 or 1.
  */
 struct reset_control {
 	struct reset_controller_dev *rcdev;
@@ -40,6 +43,7 @@ struct reset_control {
 	unsigned int refcnt;
 	int shared;
 	atomic_t deassert_count;
+	atomic_t triggered_count;
 };
 
 /**
@@ -134,18 +138,35 @@ EXPORT_SYMBOL_GPL(devm_reset_controller_register);
  * reset_control_reset - reset the controlled device
  * @rstc: reset controller
  *
- * Calling this on a shared reset controller is an error.
+ * On a shared reset line the actual reset pulse is only triggered once for the
+ * lifetime of the reset_control instance: for all but the first caller this is
+ * a no-op.
+ * Consumers must not use reset_control_(de)assert on shared reset lines when
+ * reset_control_reset has been used.
  */
 int reset_control_reset(struct reset_control *rstc)
 {
-	if (WARN_ON(IS_ERR_OR_NULL(rstc)) ||
-	    WARN_ON(rstc->shared))
+	int ret;
+
+	if (WARN_ON(IS_ERR_OR_NULL(rstc)))
 		return -EINVAL;
 
-	if (rstc->rcdev->ops->reset)
-		return rstc->rcdev->ops->reset(rstc->rcdev, rstc->id);
+	if (!rstc->rcdev->ops->reset)
+		return -ENOTSUPP;
 
-	return -ENOTSUPP;
+	if (rstc->shared) {
+		if (WARN_ON(atomic_read(&rstc->deassert_count) != 0))
+			return -EINVAL;
+
+		if (atomic_inc_return(&rstc->triggered_count) != 1)
+			return 0;
+	}
+
+	ret = rstc->rcdev->ops->reset(rstc->rcdev, rstc->id);
+	if (rstc->shared && !ret)
+		atomic_dec(&rstc->triggered_count);
+
+	return ret;
 }
 EXPORT_SYMBOL_GPL(reset_control_reset);
 
@@ -159,6 +180,8 @@ EXPORT_SYMBOL_GPL(reset_control_reset);
  *
  * For shared reset controls a driver cannot expect the hw's registers and
  * internal state to be reset, but must be prepared for this to happen.
+ * Consumers must not use reset_control_reset on shared reset lines when
+ * reset_control_(de)assert has been used.
  */
 int reset_control_assert(struct reset_control *rstc)
 {
@@ -169,6 +192,9 @@ int reset_control_assert(struct reset_control *rstc)
 		return -ENOTSUPP;
 
 	if (rstc->shared) {
+		if (WARN_ON(atomic_read(&rstc->triggered_count) != 0))
+			return -EINVAL;
+
 		if (WARN_ON(atomic_read(&rstc->deassert_count) == 0))
 			return -EINVAL;
 
@@ -185,6 +211,8 @@ EXPORT_SYMBOL_GPL(reset_control_assert);
  * @rstc: reset controller
  *
  * After calling this function, the reset is guaranteed to be deasserted.
+ * Consumers must not use reset_control_reset on shared reset lines when
+ * reset_control_(de)assert has been used.
  */
 int reset_control_deassert(struct reset_control *rstc)
 {
@@ -195,6 +223,9 @@ int reset_control_deassert(struct reset_control *rstc)
 		return -ENOTSUPP;
 
 	if (rstc->shared) {
+		if (WARN_ON(atomic_read(&rstc->triggered_count) != 0))
+			return -EINVAL;
+
 		if (atomic_inc_return(&rstc->deassert_count) != 1)
 			return 0;
 	}
-- 
2.10.2

[PATCH 1/3] reset: allow using reset_control_reset with shared reset

[Philipp Zabel]

Hi Martin,

Am Samstag, den 12.11.2016, 14:13 +0100 schrieb Martin Blumenstingl:
> Some SoCs (for example Amlogic GXBB) implement a reset controller which
> only supports a reset pulse (triggered via reset_control_reset). At the
> same time multiple devices (in case of the Amlogic GXBB SoC both USB
> PHYs) are sharing the same reset line.
> 
> This patch allows using reset_control_reset also for shared resets.
> There are limitations though:
> reset_control_reset can only be used if reset_control_assert was not
> used yet.
> reset_control_assert can only be used if reset_control_reset was not
> used yet.
> For shared resets the reset is only triggered once for the lifetime of
> the reset_control instance (the reset can be triggered again if all
> consumers of that specific reset_control are gone, as the reset
> framework will free the reset_control instance in that case).
> 
> Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>

Looks good to me, I've applied it to the reset/next branch.

thanks
Philipp

> ---
>  drivers/reset/core.c | 43 +++++++++++++++++++++++++++++++++++++------
>  1 file changed, 37 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/reset/core.c b/drivers/reset/core.c
> index b8ae1db..10368ed 100644
> --- a/drivers/reset/core.c
> +++ b/drivers/reset/core.c
> @@ -32,6 +32,9 @@ static LIST_HEAD(reset_controller_list);
>   * @refcnt: Number of gets of this reset_control
>   * @shared: Is this a shared (1), or an exclusive (0) reset_control?
>   * @deassert_cnt: Number of times this reset line has been deasserted
> + * @triggered_count: Number of times this reset line has been reset. Currently
> + *                   only used for shared resets, which means that the value
> + *                   will be either 0 or 1.
>   */
>  struct reset_control {
>  	struct reset_controller_dev *rcdev;
> @@ -40,6 +43,7 @@ struct reset_control {
>  	unsigned int refcnt;
>  	int shared;
>  	atomic_t deassert_count;
> +	atomic_t triggered_count;
>  };
>  
>  /**
> @@ -134,18 +138,35 @@ EXPORT_SYMBOL_GPL(devm_reset_controller_register);
>   * reset_control_reset - reset the controlled device
>   * @rstc: reset controller
>   *
> - * Calling this on a shared reset controller is an error.
> + * On a shared reset line the actual reset pulse is only triggered once for the
> + * lifetime of the reset_control instance: for all but the first caller this is
> + * a no-op.
> + * Consumers must not use reset_control_(de)assert on shared reset lines when
> + * reset_control_reset has been used.
>   */
>  int reset_control_reset(struct reset_control *rstc)
>  {
> -	if (WARN_ON(IS_ERR_OR_NULL(rstc)) ||
> -	    WARN_ON(rstc->shared))
> +	int ret;
> +
> +	if (WARN_ON(IS_ERR_OR_NULL(rstc)))
>  		return -EINVAL;
>  
> -	if (rstc->rcdev->ops->reset)
> -		return rstc->rcdev->ops->reset(rstc->rcdev, rstc->id);
> +	if (!rstc->rcdev->ops->reset)
> +		return -ENOTSUPP;
>  
> -	return -ENOTSUPP;
> +	if (rstc->shared) {
> +		if (WARN_ON(atomic_read(&rstc->deassert_count) != 0))
> +			return -EINVAL;
> +
> +		if (atomic_inc_return(&rstc->triggered_count) != 1)
> +			return 0;
> +	}
> +
> +	ret = rstc->rcdev->ops->reset(rstc->rcdev, rstc->id);
> +	if (rstc->shared && !ret)
> +		atomic_dec(&rstc->triggered_count);
> +
> +	return ret;
>  }
>  EXPORT_SYMBOL_GPL(reset_control_reset);
>  
> @@ -159,6 +180,8 @@ EXPORT_SYMBOL_GPL(reset_control_reset);
>   *
>   * For shared reset controls a driver cannot expect the hw's registers and
>   * internal state to be reset, but must be prepared for this to happen.
> + * Consumers must not use reset_control_reset on shared reset lines when
> + * reset_control_(de)assert has been used.
>   */
>  int reset_control_assert(struct reset_control *rstc)
>  {
> @@ -169,6 +192,9 @@ int reset_control_assert(struct reset_control *rstc)
>  		return -ENOTSUPP;
>  
>  	if (rstc->shared) {
> +		if (WARN_ON(atomic_read(&rstc->triggered_count) != 0))
> +			return -EINVAL;
> +
>  		if (WARN_ON(atomic_read(&rstc->deassert_count) == 0))
>  			return -EINVAL;
>  
> @@ -185,6 +211,8 @@ EXPORT_SYMBOL_GPL(reset_control_assert);
>   * @rstc: reset controller
>   *
>   * After calling this function, the reset is guaranteed to be deasserted.
> + * Consumers must not use reset_control_reset on shared reset lines when
> + * reset_control_(de)assert has been used.
>   */
>  int reset_control_deassert(struct reset_control *rstc)
>  {
> @@ -195,6 +223,9 @@ int reset_control_deassert(struct reset_control *rstc)
>  		return -ENOTSUPP;
>  
>  	if (rstc->shared) {
> +		if (WARN_ON(atomic_read(&rstc->triggered_count) != 0))
> +			return -EINVAL;
> +
>  		if (atomic_inc_return(&rstc->deassert_count) != 1)
>  			return 0;
>  	}

[PATCH 2/3] phy: meson8b-usb2: request a shared reset line

[Martin Blumenstingl]

Both PHYs are sharing one reset line. With recent improvements to the
reset framework we can now also use reset_control_reset with shared
resets.
This allows us to drop some workarounds where the reset was only
specified for one PHY but not the other, to make sure that the reset it
only executed once (as the reset framework was not able to use
reset_control_reset with shared reset lines).

Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
---
 drivers/phy/phy-meson8b-usb2.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/phy/phy-meson8b-usb2.c b/drivers/phy/phy-meson8b-usb2.c
index 73bf632..f1ee96a 100644
--- a/drivers/phy/phy-meson8b-usb2.c
+++ b/drivers/phy/phy-meson8b-usb2.c
@@ -237,8 +237,7 @@ static int phy_meson8b_usb2_probe(struct platform_device *pdev)
 	if (IS_ERR(priv->clk_usb))
 		return PTR_ERR(priv->clk_usb);
 
-	priv->reset = devm_reset_control_get_optional_exclusive(&pdev->dev,
-								NULL);
+	priv->reset = devm_reset_control_get_optional_shared(&pdev->dev, NULL);
 	if (PTR_ERR(priv->reset) == -EPROBE_DEFER)
 		return PTR_ERR(priv->reset);
 
-- 
2.10.2

[PATCH 2/3] phy: meson8b-usb2: request a shared reset line

[Kishon Vijay Abraham I]

On Saturday 12 November 2016 06:43 PM, Martin Blumenstingl wrote:
> Both PHYs are sharing one reset line. With recent improvements to the
> reset framework we can now also use reset_control_reset with shared
> resets.
> This allows us to drop some workarounds where the reset was only
> specified for one PHY but not the other, to make sure that the reset it
> only executed once (as the reset framework was not able to use
> reset_control_reset with shared reset lines).
> 
> Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>

merged $patch to -next.

Thanks
Kishon
> ---
>  drivers/phy/phy-meson8b-usb2.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/drivers/phy/phy-meson8b-usb2.c b/drivers/phy/phy-meson8b-usb2.c
> index 73bf632..f1ee96a 100644
> --- a/drivers/phy/phy-meson8b-usb2.c
> +++ b/drivers/phy/phy-meson8b-usb2.c
> @@ -237,8 +237,7 @@ static int phy_meson8b_usb2_probe(struct platform_device *pdev)
>  	if (IS_ERR(priv->clk_usb))
>  		return PTR_ERR(priv->clk_usb);
>  
> -	priv->reset = devm_reset_control_get_optional_exclusive(&pdev->dev,
> -								NULL);
> +	priv->reset = devm_reset_control_get_optional_shared(&pdev->dev, NULL);
>  	if (PTR_ERR(priv->reset) == -EPROBE_DEFER)
>  		return PTR_ERR(priv->reset);
>  
> 

[PATCH 3/3] ARM64: dts: meson-gxbb: add the USB reset also to the second USB PHY

[Martin Blumenstingl]

When the USB PHY driver was introduced the reset framework did not
have support for triggering a reset pulse for shared resets. On GXBB
however there is only one reset line for both PHYs (meaning we have a
shared reset line). With the latest changes to the reset framework and
the corresponding updates to the phy-meson8b-usb2 driver we can now pass
the reset to the second PHY as well.

Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
---
 arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi b/arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi
index 719bf50..a548663 100644
--- a/arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi
+++ b/arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi
@@ -87,6 +87,7 @@
 			compatible = "amlogic,meson-gxbb-usb2-phy";
 			#phy-cells = <0>;
 			reg = <0x0 0xc0000020 0x0 0x20>;
+			resets = <&reset RESET_USB_OTG>;
 			clocks = <&clkc CLKID_USB>, <&clkc CLKID_USB1>;
 			clock-names = "usb_general", "usb";
 			status = "disabled";
-- 
2.10.2

[PATCH 3/3] ARM64: dts: meson-gxbb: add the USB reset also to the second USB PHY

[Kevin Hilman]

Martin Blumenstingl <martin.blumenstingl@googlemail.com> writes:

> When the USB PHY driver was introduced the reset framework did not
> have support for triggering a reset pulse for shared resets. On GXBB
> however there is only one reset line for both PHYs (meaning we have a
> shared reset line). With the latest changes to the reset framework and
> the corresponding updates to the phy-meson8b-usb2 driver we can now pass
> the reset to the second PHY as well.
>
> Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>

Applied.

Thanks,

Kevin

[PATCH 3/3] ARM64: dts: meson-gxbb: add the USB reset also to the second USB PHY

[Martin Blumenstingl]

Hi Kevin,

On Wed, Nov 16, 2016 at 10:35 PM, Kevin Hilman <khilman@baylibre.com> wrote:
> Martin Blumenstingl <martin.blumenstingl@googlemail.com> writes:
>
>> When the USB PHY driver was introduced the reset framework did not
>> have support for triggering a reset pulse for shared resets. On GXBB
>> however there is only one reset line for both PHYs (meaning we have a
>> shared reset line). With the latest changes to the reset framework and
>> the corresponding updates to the phy-meson8b-usb2 driver we can now pass
>> the reset to the second PHY as well.
>>
>> Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
>
> Applied.
Unfortunately I think I put crucial information only in the
cover-letter's description:
"the dts patch has a runtime-dependency on patch 1 and 2"

So please feel free to keep or drop the patch as it is. In case you
decide drop it I will re-send it for 4.11 (after all the 4.10 stuff is
done).

[PATCH 3/3] ARM64: dts: meson-gxbb: add the USB reset also to the second USB PHY

[Kevin Hilman]

Martin Blumenstingl <martin.blumenstingl@googlemail.com> writes:

> Hi Kevin,
>
> On Wed, Nov 16, 2016 at 10:35 PM, Kevin Hilman <khilman@baylibre.com> wrote:
>> Martin Blumenstingl <martin.blumenstingl@googlemail.com> writes:
>>
>>> When the USB PHY driver was introduced the reset framework did not
>>> have support for triggering a reset pulse for shared resets. On GXBB
>>> however there is only one reset line for both PHYs (meaning we have a
>>> shared reset line). With the latest changes to the reset framework and
>>> the corresponding updates to the phy-meson8b-usb2 driver we can now pass
>>> the reset to the second PHY as well.
>>>
>>> Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
>>
>> Applied.
> Unfortunately I think I put crucial information only in the
> cover-letter's description:
> "the dts patch has a runtime-dependency on patch 1 and 2"

I saw that, but also see that both of those have been queued, so should
land in v4.10 also.

> So please feel free to keep or drop the patch as it is. In case you
> decide drop it I will re-send it for 4.11 (after all the 4.10 stuff is
> done).

IMO, it's fine to keep it.  That means there may be some versions of
linux-next that have the problem where the reset will get asserted
twice, but since that is affecting very few people (probably only you),
I think it's OK, since it will be fine once v4.10-rc1 is released.

If you don't want that, let me know and I'll drop it for now.

Kevin

[PATCH 3/3] ARM64: dts: meson-gxbb: add the USB reset also to the second USB PHY

[Martin Blumenstingl]

On Mon, Nov 21, 2016 at 9:15 PM, Kevin Hilman <khilman@baylibre.com> wrote:
> Martin Blumenstingl <martin.blumenstingl@googlemail.com> writes:
>
>> Hi Kevin,
>>
>> On Wed, Nov 16, 2016 at 10:35 PM, Kevin Hilman <khilman@baylibre.com> wrote:
>>> Martin Blumenstingl <martin.blumenstingl@googlemail.com> writes:
>>>
>>>> When the USB PHY driver was introduced the reset framework did not
>>>> have support for triggering a reset pulse for shared resets. On GXBB
>>>> however there is only one reset line for both PHYs (meaning we have a
>>>> shared reset line). With the latest changes to the reset framework and
>>>> the corresponding updates to the phy-meson8b-usb2 driver we can now pass
>>>> the reset to the second PHY as well.
>>>>
>>>> Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
>>>
>>> Applied.
>> Unfortunately I think I put crucial information only in the
>> cover-letter's description:
>> "the dts patch has a runtime-dependency on patch 1 and 2"
>
> I saw that, but also see that both of those have been queued, so should
> land in v4.10 also.
>
>> So please feel free to keep or drop the patch as it is. In case you
>> decide drop it I will re-send it for 4.11 (after all the 4.10 stuff is
>> done).
>
> IMO, it's fine to keep it.  That means there may be some versions of
> linux-next that have the problem where the reset will get asserted
> twice, but since that is affecting very few people (probably only you),
> I think it's OK, since it will be fine once v4.10-rc1 is released.
fine with me, just wanted to let you know (so you're aware of it in
case someone runs into an issue with this)

> If you don't want that, let me know and I'll drop it for now.
let's keep it - this will mean that more users will test it :)


Martin