All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] reset: fix shared reset triggered_count decrement on error
@ 2017-02-15 18:15 ` Jerome Brunet
  0 siblings, 0 replies; 6+ messages in thread
From: Jerome Brunet @ 2017-02-15 18:15 UTC (permalink / raw)
  To: Philipp Zabel, Martin Blumenstingl
  Cc: Jerome Brunet, linux-kernel, linux-amlogic

For a shared reset, when the reset is successful, the triggered_count is
incremented when trying to call the reset callback, so that another device
sharing the same reset line won't trigger it again. If the reset has not
been triggered successfully, the trigger_count should be decremented.

The code does the opposite, and decrements the trigger_count on success.
As a consequence, another device sharing the reset will be able to trigger
it again.

Fixed be removing negation in from of the error code of the reset function.

Fixes: 7da33a37b48f ("reset: allow using reset_control_reset with shared reset")

Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
---
 Hi Philipp,

 I found this issue while testing your patch [0]
 It fixes a regression we have been having with usb. On meson-gxbb
 platforms, usb0 and usb1 share the same reset line. Martin had
 reports that usb0 recently got broken. In fact usb1 was able to
 trigger the reset again because the issue mentioned above.

[0]: http://lkml.kernel.org/r/20170130114116.22089-10-p.zabel@pengutronix.de

 drivers/reset/core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/reset/core.c b/drivers/reset/core.c
index 71ccf281dce3..f1e5e65388bb 100644
--- a/drivers/reset/core.c
+++ b/drivers/reset/core.c
@@ -169,7 +169,7 @@ int reset_control_reset(struct reset_control *rstc)
 	}
 
 	ret = rstc->rcdev->ops->reset(rstc->rcdev, rstc->id);
-	if (rstc->shared && !ret)
+	if (rstc->shared && ret)
 		atomic_dec(&rstc->triggered_count);
 
 	return ret;
-- 
2.7.4

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

* [PATCH] reset: fix shared reset triggered_count decrement on error
@ 2017-02-15 18:15 ` Jerome Brunet
  0 siblings, 0 replies; 6+ messages in thread
From: Jerome Brunet @ 2017-02-15 18:15 UTC (permalink / raw)
  To: linus-amlogic

For a shared reset, when the reset is successful, the triggered_count is
incremented when trying to call the reset callback, so that another device
sharing the same reset line won't trigger it again. If the reset has not
been triggered successfully, the trigger_count should be decremented.

The code does the opposite, and decrements the trigger_count on success.
As a consequence, another device sharing the reset will be able to trigger
it again.

Fixed be removing negation in from of the error code of the reset function.

Fixes: 7da33a37b48f ("reset: allow using reset_control_reset with shared reset")

Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
---
 Hi Philipp,

 I found this issue while testing your patch [0]
 It fixes a regression we have been having with usb. On meson-gxbb
 platforms, usb0 and usb1 share the same reset line. Martin had
 reports that usb0 recently got broken. In fact usb1 was able to
 trigger the reset again because the issue mentioned above.

[0]: http://lkml.kernel.org/r/20170130114116.22089-10-p.zabel at pengutronix.de

 drivers/reset/core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/reset/core.c b/drivers/reset/core.c
index 71ccf281dce3..f1e5e65388bb 100644
--- a/drivers/reset/core.c
+++ b/drivers/reset/core.c
@@ -169,7 +169,7 @@ int reset_control_reset(struct reset_control *rstc)
 	}
 
 	ret = rstc->rcdev->ops->reset(rstc->rcdev, rstc->id);
-	if (rstc->shared && !ret)
+	if (rstc->shared && ret)
 		atomic_dec(&rstc->triggered_count);
 
 	return ret;
-- 
2.7.4

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

* Re: [PATCH] reset: fix shared reset triggered_count decrement on error
  2017-02-15 18:15 ` Jerome Brunet
@ 2017-02-15 20:09   ` Martin Blumenstingl
  -1 siblings, 0 replies; 6+ messages in thread
From: Martin Blumenstingl @ 2017-02-15 20:09 UTC (permalink / raw)
  To: Jerome Brunet; +Cc: Philipp Zabel, linux-kernel, linux-amlogic

Hi Jerome,

ouch, thanks for spotting and fixing this!

On Wed, Feb 15, 2017 at 7:15 PM, Jerome Brunet <jbrunet@baylibre.com> wrote:
> For a shared reset, when the reset is successful, the triggered_count is
> incremented when trying to call the reset callback, so that another device
> sharing the same reset line won't trigger it again. If the reset has not
> been triggered successfully, the trigger_count should be decremented.
>
> The code does the opposite, and decrements the trigger_count on success.
> As a consequence, another device sharing the reset will be able to trigger
> it again.
>
> Fixed be removing negation in from of the error code of the reset function.
>
> Fixes: 7da33a37b48f ("reset: allow using reset_control_reset with shared reset")
>
> Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
Acked-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>

should this still go into 4.10?
I guess the meson8b-usb2 PHY driver is the only user of the shared
reset pulse, but if there are other users out there then these may
face strange issues as well!

> ---
>  Hi Philipp,
>
>  I found this issue while testing your patch [0]
>  It fixes a regression we have been having with usb. On meson-gxbb
>  platforms, usb0 and usb1 share the same reset line. Martin had
>  reports that usb0 recently got broken. In fact usb1 was able to
>  trigger the reset again because the issue mentioned above.
>
> [0]: http://lkml.kernel.org/r/20170130114116.22089-10-p.zabel@pengutronix.de
>
>  drivers/reset/core.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/reset/core.c b/drivers/reset/core.c
> index 71ccf281dce3..f1e5e65388bb 100644
> --- a/drivers/reset/core.c
> +++ b/drivers/reset/core.c
> @@ -169,7 +169,7 @@ int reset_control_reset(struct reset_control *rstc)
>         }
>
>         ret = rstc->rcdev->ops->reset(rstc->rcdev, rstc->id);
> -       if (rstc->shared && !ret)
> +       if (rstc->shared && ret)
>                 atomic_dec(&rstc->triggered_count);
>
>         return ret;
> --
> 2.7.4
>

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

* [PATCH] reset: fix shared reset triggered_count decrement on error
@ 2017-02-15 20:09   ` Martin Blumenstingl
  0 siblings, 0 replies; 6+ messages in thread
From: Martin Blumenstingl @ 2017-02-15 20:09 UTC (permalink / raw)
  To: linus-amlogic

Hi Jerome,

ouch, thanks for spotting and fixing this!

On Wed, Feb 15, 2017 at 7:15 PM, Jerome Brunet <jbrunet@baylibre.com> wrote:
> For a shared reset, when the reset is successful, the triggered_count is
> incremented when trying to call the reset callback, so that another device
> sharing the same reset line won't trigger it again. If the reset has not
> been triggered successfully, the trigger_count should be decremented.
>
> The code does the opposite, and decrements the trigger_count on success.
> As a consequence, another device sharing the reset will be able to trigger
> it again.
>
> Fixed be removing negation in from of the error code of the reset function.
>
> Fixes: 7da33a37b48f ("reset: allow using reset_control_reset with shared reset")
>
> Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
Acked-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>

should this still go into 4.10?
I guess the meson8b-usb2 PHY driver is the only user of the shared
reset pulse, but if there are other users out there then these may
face strange issues as well!

> ---
>  Hi Philipp,
>
>  I found this issue while testing your patch [0]
>  It fixes a regression we have been having with usb. On meson-gxbb
>  platforms, usb0 and usb1 share the same reset line. Martin had
>  reports that usb0 recently got broken. In fact usb1 was able to
>  trigger the reset again because the issue mentioned above.
>
> [0]: http://lkml.kernel.org/r/20170130114116.22089-10-p.zabel at pengutronix.de
>
>  drivers/reset/core.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/reset/core.c b/drivers/reset/core.c
> index 71ccf281dce3..f1e5e65388bb 100644
> --- a/drivers/reset/core.c
> +++ b/drivers/reset/core.c
> @@ -169,7 +169,7 @@ int reset_control_reset(struct reset_control *rstc)
>         }
>
>         ret = rstc->rcdev->ops->reset(rstc->rcdev, rstc->id);
> -       if (rstc->shared && !ret)
> +       if (rstc->shared && ret)
>                 atomic_dec(&rstc->triggered_count);
>
>         return ret;
> --
> 2.7.4
>

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

* Re: [PATCH] reset: fix shared reset triggered_count decrement on error
  2017-02-15 18:15 ` Jerome Brunet
@ 2017-02-16  9:52   ` Philipp Zabel
  -1 siblings, 0 replies; 6+ messages in thread
From: Philipp Zabel @ 2017-02-16  9:52 UTC (permalink / raw)
  To: Jerome Brunet; +Cc: Martin Blumenstingl, linux-kernel, linux-amlogic

On Wed, 2017-02-15 at 19:15 +0100, Jerome Brunet wrote:
> For a shared reset, when the reset is successful, the triggered_count is
> incremented when trying to call the reset callback, so that another device
> sharing the same reset line won't trigger it again. If the reset has not
> been triggered successfully, the trigger_count should be decremented.
> 
> The code does the opposite, and decrements the trigger_count on success.
> As a consequence, another device sharing the reset will be able to trigger
> it again.
> 
> Fixed be removing negation in from of the error code of the reset function.
> 
> Fixes: 7da33a37b48f ("reset: allow using reset_control_reset with shared reset")
> 
> Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
> ---
>  Hi Philipp,
> 
>  I found this issue while testing your patch [0]
>  It fixes a regression we have been having with usb. On meson-gxbb
>  platforms, usb0 and usb1 share the same reset line. Martin had
>  reports that usb0 recently got broken. In fact usb1 was able to
>  trigger the reset again because the issue mentioned above.

Thanks, applied with Martin's Acked-by. I'll send a pull request for
this later today.

regards
Philipp

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

* [PATCH] reset: fix shared reset triggered_count decrement on error
@ 2017-02-16  9:52   ` Philipp Zabel
  0 siblings, 0 replies; 6+ messages in thread
From: Philipp Zabel @ 2017-02-16  9:52 UTC (permalink / raw)
  To: linus-amlogic

On Wed, 2017-02-15 at 19:15 +0100, Jerome Brunet wrote:
> For a shared reset, when the reset is successful, the triggered_count is
> incremented when trying to call the reset callback, so that another device
> sharing the same reset line won't trigger it again. If the reset has not
> been triggered successfully, the trigger_count should be decremented.
> 
> The code does the opposite, and decrements the trigger_count on success.
> As a consequence, another device sharing the reset will be able to trigger
> it again.
> 
> Fixed be removing negation in from of the error code of the reset function.
> 
> Fixes: 7da33a37b48f ("reset: allow using reset_control_reset with shared reset")
> 
> Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
> ---
>  Hi Philipp,
> 
>  I found this issue while testing your patch [0]
>  It fixes a regression we have been having with usb. On meson-gxbb
>  platforms, usb0 and usb1 share the same reset line. Martin had
>  reports that usb0 recently got broken. In fact usb1 was able to
>  trigger the reset again because the issue mentioned above.

Thanks, applied with Martin's Acked-by. I'll send a pull request for
this later today.

regards
Philipp

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

end of thread, other threads:[~2017-02-16  9:52 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-02-15 18:15 [PATCH] reset: fix shared reset triggered_count decrement on error Jerome Brunet
2017-02-15 18:15 ` Jerome Brunet
2017-02-15 20:09 ` Martin Blumenstingl
2017-02-15 20:09   ` Martin Blumenstingl
2017-02-16  9:52 ` Philipp Zabel
2017-02-16  9:52   ` Philipp Zabel

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.