[PATCH v4 0/2] reset: Serialize shared resets whose ops may sleep

[PATCH v4 0/2] reset: Serialize shared resets whose ops may sleep

[shengjiu.wang]

From: Shengjiu Wang <shengjiu.wang@nxp.com>

For a shared reset only the first caller runs the hardware op; the others
bump the refcount and return. When the op can sleep, a concurrent caller can
return before the first caller's op has finished and then touch hardware
still in reset.

Serialize the shared refcount update and the hardware op under a new
per-control mutex, but only for providers that opt in via a new
reset_controller_dev::can_sleep flag. Providers that leave it clear keep the
lockless path, so atomic callers are unaffected. reset-gpio is the first
provider to set the flag.

Changes in v4:
- Move the can_sleep/mutex description to the top of the core commit message.
- Also serialize reset_control_assert() under shared_lock.
- Split the reset-gpio can_sleep assignment into its own patch (2/2).
- Fix a kerneldoc typo; drop the Assisted-by trailer.
- Drop the Fixes: tag, present as a correctness improvement.

Changes in v3:
- Drop the reset_control_reset_atomic()/reset_control_deassert_atomic()
  variants and their reset.h declarations/stubs; gate the mutex on
  rcdev->can_sleep instead, so non-sleeping controllers keep the lockless
  path and need no new API.
- Use a dedicated rstc->shared_lock taken inside the SRCU read-side section
  instead of reusing rstc->lock before SRCU.
- Set can_sleep in reset-gpio; no other provider is converted.
- Drop the lockdep_assert_not_held() additions and the reset.rst / kerneldoc
  text about sleeping-vs-atomic variants.
  
Changes in v2:
- Serialize only the shared may-sleep path under rstc->lock (taken before
  SRCU to match reset_control_acquire()'s order); exclusive path stays
  lockless, and reset_control_assert() is left unchanged.
- Add lockless reset_control_reset_atomic()/reset_control_deassert_atomic()
  for atomic callers; gate the mutex on can_sleep so no shared consumer is
  regressed and so it works on all configs.
- Factor the body into __reset_control_*()/__reset_control_*_locked()
  helpers, and lockdep_assert_not_held() at each lock site to catch a
  controller op re-entering the API on the same control.
- Declare the new variants and stubs in reset.h, and document the sleeping
  vs atomic variants in the kerneldoc and reset.rst.

Shengjiu Wang (2):
  reset: core: Serialize concurrent deassert/reset of shared resets
  reset: gpio: Set can_sleep for the shared reset serialization

 drivers/reset/core.c             | 205 ++++++++++++++++++++-----------
 drivers/reset/reset-gpio.c       |   1 +
 include/linux/reset-controller.h |   4 +
 3 files changed, 139 insertions(+), 71 deletions(-)

-- 
2.34.1

[PATCH v4 1/2] reset: core: Serialize concurrent deassert/reset of shared resets

[shengjiu.wang]

From: Shengjiu Wang <shengjiu.wang@nxp.com>

Add a reset_controller_dev::can_sleep flag that a provider sets when its
reset ops may sleep. When it is set, serialize the shared refcount update
and the hardware op in reset_control_reset(), reset_control_deassert() and
reset_control_assert() under a new per-control mutex, taken inside the SRCU
read-side critical section. Providers that leave the flag clear keep the
existing lockless path, so callers in atomic context are unaffected.

reset_control_deassert() and reset_control_reset() are documented to
guarantee that the reset line has been deasserted, respectively that a
reset pulse has been issued, by the time they return.

For a shared reset only the first caller runs the hardware op; the other
callers just bump the refcount and return. When the op can sleep this
breaks the guarantee: a second caller can observe the bumped refcount and
return before the first caller's op has finished, and then touch hardware
that is still in reset. A reset_control_assert() racing a concurrent
reset_control_deassert() can likewise interleave the two sleeping ops and
leave the line asserted while the refcount says it is deasserted.

Suggested-by: Philipp Zabel <p.zabel@pengutronix.de>
Signed-off-by: Shengjiu Wang <shengjiu.wang@nxp.com>
Link: https://lore.kernel.org/all/aszDZlJ0KahNwgZO@analog.com/
---
 drivers/reset/core.c             | 205 ++++++++++++++++++++-----------
 include/linux/reset-controller.h |   4 +
 2 files changed, 138 insertions(+), 71 deletions(-)

diff --git a/drivers/reset/core.c b/drivers/reset/core.c
index 38e189d04d09..7eb4575413d6 100644
--- a/drivers/reset/core.c
+++ b/drivers/reset/core.c
@@ -52,6 +52,9 @@ static DEFINE_IDA(reset_gpio_ida);
  *                   only used for shared resets, which means that the value
  *                   will be either 0 or 1.
  * @lock: serializes the internals of reset_control_acquire()
+ * @shared_lock: serializes the shared refcount update and the hardware op for
+ *               controllers whose ops may sleep (rcdev->can_sleep); taken only
+ *               inside the SRCU read-side critical section
  */
 struct reset_control {
 	struct reset_controller_dev __rcu *rcdev;
@@ -65,6 +68,8 @@ struct reset_control {
 	atomic_t deassert_count;
 	atomic_t triggered_count;
 	struct mutex lock;
+	/* serializes the shared refcount update with its hardware op */
+	struct mutex shared_lock;
 };
 
 /**
@@ -355,6 +360,32 @@ static inline bool reset_control_is_array(struct reset_control *rstc)
 	return rstc->array;
 }
 
+static int __reset_control_reset(struct reset_control *rstc,
+				 struct reset_controller_dev *rcdev)
+{
+	int ret;
+
+	if (!rcdev->ops->reset)
+		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;
+	} else {
+		if (!rstc->acquired)
+			return -EPERM;
+	}
+
+	ret = rcdev->ops->reset(rcdev, rstc->id);
+	if (rstc->shared && ret)
+		atomic_dec(&rstc->triggered_count);
+
+	return ret;
+}
+
 /**
  * reset_control_reset - reset the controlled device
  * @rstc: reset controller
@@ -362,6 +393,10 @@ static inline bool reset_control_is_array(struct reset_control *rstc)
  * 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.
+ *
+ * After calling this function, the reset line is guaranteed to have seen a
+ * reset pulse at any point in the past, not necessarily during the function
+ * call.
  * Consumers must not use reset_control_(de)assert on shared reset lines when
  * reset_control_reset has been used.
  *
@@ -371,7 +406,6 @@ static inline bool reset_control_is_array(struct reset_control *rstc)
 int reset_control_reset(struct reset_control *rstc)
 {
 	struct reset_controller_dev *rcdev;
-	int ret;
 
 	if (!rstc)
 		return 0;
@@ -388,25 +422,18 @@ int reset_control_reset(struct reset_control *rstc)
 	if (!rcdev)
 		return -ENODEV;
 
-	if (!rcdev->ops->reset)
-		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;
-	} else {
-		if (!rstc->acquired)
-			return -EPERM;
+	/*
+	 * For a shared reset only the first caller runs the hardware op while
+	 * concurrent callers bump the refcount and return. If the op may sleep
+	 * (rcdev->can_sleep), serialize the refcount update and the op so a
+	 * concurrent caller cannot return before the op has finished.
+	 */
+	if (rstc->shared && rcdev->can_sleep) {
+		guard(mutex)(&rstc->shared_lock);
+		return __reset_control_reset(rstc, rcdev);
 	}
 
-	ret = rcdev->ops->reset(rcdev, rstc->id);
-	if (rstc->shared && ret)
-		atomic_dec(&rstc->triggered_count);
-
-	return ret;
+	return __reset_control_reset(rstc, rcdev);
 }
 EXPORT_SYMBOL_GPL(reset_control_reset);
 
@@ -474,6 +501,44 @@ int reset_control_rearm(struct reset_control *rstc)
 }
 EXPORT_SYMBOL_GPL(reset_control_rearm);
 
+static int __reset_control_assert(struct reset_control *rstc,
+				  struct reset_controller_dev *rcdev)
+{
+	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;
+
+		if (atomic_dec_return(&rstc->deassert_count) != 0)
+			return 0;
+
+		/*
+		 * Shared reset controls allow the reset line to be in any state
+		 * after this call, so doing nothing is a valid option.
+		 */
+		if (!rcdev->ops->assert)
+			return 0;
+	} else {
+		/*
+		 * If the reset controller does not implement .assert(), there
+		 * is no way to guarantee that the reset line is asserted after
+		 * this call.
+		 */
+		if (!rcdev->ops->assert)
+			return -ENOTSUPP;
+
+		if (!rstc->acquired) {
+			WARN(1, "reset %s (ID: %u) is not acquired\n",
+			     rcdev_name(rcdev), rstc->id);
+			return -EPERM;
+		}
+	}
+
+	return rcdev->ops->assert(rcdev, rstc->id);
+}
+
 /**
  * reset_control_assert - asserts the reset line
  * @rstc: reset controller
@@ -509,39 +574,18 @@ int reset_control_assert(struct reset_control *rstc)
 	if (!rcdev)
 		return -ENODEV;
 
-	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;
-
-		if (atomic_dec_return(&rstc->deassert_count) != 0)
-			return 0;
-
-		/*
-		 * Shared reset controls allow the reset line to be in any state
-		 * after this call, so doing nothing is a valid option.
-		 */
-		if (!rcdev->ops->assert)
-			return 0;
-	} else {
-		/*
-		 * If the reset controller does not implement .assert(), there
-		 * is no way to guarantee that the reset line is asserted after
-		 * this call.
-		 */
-		if (!rcdev->ops->assert)
-			return -ENOTSUPP;
-
-		if (!rstc->acquired) {
-			WARN(1, "reset %s (ID: %u) is not acquired\n",
-			     rcdev_name(rcdev), rstc->id);
-			return -EPERM;
-		}
+	/*
+	 * For a shared reset the last caller runs the hardware op while the
+	 * other callers just drop the refcount and return. If the op may sleep
+	 * (rcdev->can_sleep), serialize the refcount update and the op against
+	 * a concurrent reset_control_deassert() so the two cannot interleave.
+	 */
+	if (rstc->shared && rcdev->can_sleep) {
+		guard(mutex)(&rstc->shared_lock);
+		return __reset_control_assert(rstc, rcdev);
 	}
 
-	return rcdev->ops->assert(rcdev, rstc->id);
+	return __reset_control_assert(rstc, rcdev);
 }
 EXPORT_SYMBOL_GPL(reset_control_assert);
 
@@ -575,6 +619,36 @@ int reset_control_bulk_assert(int num_rstcs,
 }
 EXPORT_SYMBOL_GPL(reset_control_bulk_assert);
 
+static int __reset_control_deassert(struct reset_control *rstc,
+				    struct reset_controller_dev *rcdev)
+{
+	if (rstc->shared) {
+		if (WARN_ON(atomic_read(&rstc->triggered_count) != 0))
+			return -EINVAL;
+
+		if (atomic_inc_return(&rstc->deassert_count) != 1)
+			return 0;
+	} else {
+		if (!rstc->acquired) {
+			WARN(1, "reset %s (ID: %u) is not acquired\n",
+			     rcdev_name(rcdev), rstc->id);
+			return -EPERM;
+		}
+	}
+
+	/*
+	 * If the reset controller does not implement .deassert(), we assume
+	 * that it handles self-deasserting reset lines via .reset(). In that
+	 * case, the reset lines are deasserted by default. If that is not the
+	 * case, the reset controller driver should implement .deassert() and
+	 * return -ENOTSUPP.
+	 */
+	if (!rcdev->ops->deassert)
+		return 0;
+
+	return rcdev->ops->deassert(rcdev, rstc->id);
+}
+
 /**
  * reset_control_deassert - deasserts the reset line
  * @rstc: reset controller
@@ -605,31 +679,18 @@ int reset_control_deassert(struct reset_control *rstc)
 	if (!rcdev)
 		return -ENODEV;
 
-	if (rstc->shared) {
-		if (WARN_ON(atomic_read(&rstc->triggered_count) != 0))
-			return -EINVAL;
-
-		if (atomic_inc_return(&rstc->deassert_count) != 1)
-			return 0;
-	} else {
-		if (!rstc->acquired) {
-			WARN(1, "reset %s (ID: %u) is not acquired\n",
-			     rcdev_name(rcdev), rstc->id);
-			return -EPERM;
-		}
-	}
-
 	/*
-	 * If the reset controller does not implement .deassert(), we assume
-	 * that it handles self-deasserting reset lines via .reset(). In that
-	 * case, the reset lines are deasserted by default. If that is not the
-	 * case, the reset controller driver should implement .deassert() and
-	 * return -ENOTSUPP.
+	 * For a shared reset only the first caller runs the hardware op while
+	 * concurrent callers bump the refcount and return. If the op may sleep
+	 * (rcdev->can_sleep), serialize the refcount update and the op so a
+	 * concurrent caller cannot return before the op has finished.
 	 */
-	if (!rcdev->ops->deassert)
-		return 0;
+	if (rstc->shared && rcdev->can_sleep) {
+		guard(mutex)(&rstc->shared_lock);
+		return __reset_control_deassert(rstc, rcdev);
+	}
 
-	return rcdev->ops->deassert(rcdev, rstc->id);
+	return __reset_control_deassert(rstc, rcdev);
 }
 EXPORT_SYMBOL_GPL(reset_control_deassert);
 
@@ -879,6 +940,7 @@ __reset_control_get_internal(struct reset_controller_dev *rcdev,
 	rstc->id = index;
 	kref_init(&rstc->refcnt);
 	mutex_init(&rstc->lock);
+	mutex_init(&rstc->shared_lock);
 	rstc->acquired = acquired;
 	rstc->shared = shared;
 	get_device(rcdev->dev);
@@ -901,6 +963,7 @@ static void __reset_control_release(struct kref *kref)
 	}
 
 	mutex_destroy(&rstc->lock);
+	mutex_destroy(&rstc->shared_lock);
 }
 
 static void reset_control_put_internal(struct reset_control *rstc)
diff --git a/include/linux/reset-controller.h b/include/linux/reset-controller.h
index 52a5a4e81f18..587fb57c2c52 100644
--- a/include/linux/reset-controller.h
+++ b/include/linux/reset-controller.h
@@ -47,6 +47,9 @@ struct of_phandle_args;
  *                id as given to the reset control ops, defaults to
  *                :c:func:`fwnode_reset_simple_xlate`
  * @nr_resets: number of reset controls in this reset controller device
+ * @can_sleep: set by the provider if its reset ops may sleep; makes the core
+ *             serialize the shared reset, assert and deassert paths under a
+ *             mutex
  * @lock: protects the reset control list from concurrent access
  */
 struct reset_controller_dev {
@@ -64,6 +67,7 @@ struct reset_controller_dev {
 	int (*fwnode_xlate)(struct reset_controller_dev *rcdev,
 			    const struct fwnode_reference_args *reset_spec);
 	unsigned int nr_resets;
+	bool can_sleep;
 	struct mutex lock;
 };
 
-- 
2.34.1

Re: [PATCH v4 1/2] reset: core: Serialize concurrent deassert/reset of shared resets

[Alvin Šipraga]

On Fri, Oct 23, 2026 at 03:16:08PM +0800, shengjiu.wang@oss.nxp.com wrote:
> From: Shengjiu Wang <shengjiu.wang@nxp.com>
> 
> Add a reset_controller_dev::can_sleep flag that a provider sets when its
> reset ops may sleep. When it is set, serialize the shared refcount update
> and the hardware op in reset_control_reset(), reset_control_deassert() and
> reset_control_assert() under a new per-control mutex, taken inside the SRCU
> read-side critical section. Providers that leave the flag clear keep the
> existing lockless path, so callers in atomic context are unaffected.
> 
> reset_control_deassert() and reset_control_reset() are documented to
> guarantee that the reset line has been deasserted, respectively that a
> reset pulse has been issued, by the time they return.
> 
> For a shared reset only the first caller runs the hardware op; the other
> callers just bump the refcount and return. When the op can sleep this
> breaks the guarantee: a second caller can observe the bumped refcount and
> return before the first caller's op has finished, and then touch hardware
> that is still in reset. A reset_control_assert() racing a concurrent
> reset_control_deassert() can likewise interleave the two sleeping ops and
> leave the line asserted while the refcount says it is deasserted.
> 
> Suggested-by: Philipp Zabel <p.zabel@pengutronix.de>
> Signed-off-by: Shengjiu Wang <shengjiu.wang@nxp.com>
> Link: https://lore.kernel.org/all/aszDZlJ0KahNwgZO@analog.com/

Reviewed-by: Alvin Šipraga <alvin.sipraga@analog.com>

> ---
>  drivers/reset/core.c             | 205 ++++++++++++++++++++-----------
>  include/linux/reset-controller.h |   4 +
>  2 files changed, 138 insertions(+), 71 deletions(-)

[PATCH v4 2/2] reset: gpio: Set can_sleep for the shared reset serialization

[shengjiu.wang]

From: Shengjiu Wang <shengjiu.wang@nxp.com>

reset-gpio drives the line with gpiod_set_value_cansleep(), so its reset
ops may sleep. Set reset_controller_dev::can_sleep so the core serializes
the shared refcount update and the hardware op under its per-control mutex,
preventing a concurrent consumer from returning before the op has finished.

Signed-off-by: Shengjiu Wang <shengjiu.wang@nxp.com>
---
 drivers/reset/reset-gpio.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/reset/reset-gpio.c b/drivers/reset/reset-gpio.c
index bf95296a512e..b4a198792107 100644
--- a/drivers/reset/reset-gpio.c
+++ b/drivers/reset/reset-gpio.c
@@ -67,6 +67,7 @@ static int reset_gpio_probe(struct auxiliary_device *adev,
 				     "Could not get reset gpios\n");
 
 	priv->rc.ops = &reset_gpio_ops;
+	priv->rc.can_sleep = true;
 	priv->rc.owner = THIS_MODULE;
 	priv->rc.dev = dev;
 
-- 
2.34.1

Re: [PATCH v4 2/2] reset: gpio: Set can_sleep for the shared reset serialization

[Alvin Šipraga]

On Fri, Oct 23, 2026 at 03:16:09PM +0800, shengjiu.wang@oss.nxp.com wrote:
> From: Shengjiu Wang <shengjiu.wang@nxp.com>
> 
> reset-gpio drives the line with gpiod_set_value_cansleep(), so its reset
> ops may sleep. Set reset_controller_dev::can_sleep so the core serializes
> the shared refcount update and the hardware op under its per-control mutex,
> preventing a concurrent consumer from returning before the op has finished.
> 
> Signed-off-by: Shengjiu Wang <shengjiu.wang@nxp.com>

Reviewed-by: Alvin Šipraga <alvin.sipraga@analog.com>

> ---
>  drivers/reset/reset-gpio.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/reset/reset-gpio.c b/drivers/reset/reset-gpio.c
> index bf95296a512e..b4a198792107 100644
> --- a/drivers/reset/reset-gpio.c
> +++ b/drivers/reset/reset-gpio.c
> @@ -67,6 +67,7 @@ static int reset_gpio_probe(struct auxiliary_device *adev,
>  				     "Could not get reset gpios\n");
>  
>  	priv->rc.ops = &reset_gpio_ops;
> +	priv->rc.can_sleep = true;
>  	priv->rc.owner = THIS_MODULE;
>  	priv->rc.dev = dev;
>  
> -- 
> 2.34.1
>