[PATCH V2] reset: core: Serialize concurrent deassert/reset of shared resets

[PATCH V2] reset: core: Serialize concurrent deassert/reset of shared resets

[shengjiu.wang]

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

reset_control_deassert() and reset_control_reset() guarantee that the
reset line is deasserted / a reset pulse has been issued once they return.
For a shared reset only the first caller runs the hardware op; others just
bump the refcount and return early, so a concurrent caller can return
before the first caller's (possibly sleeping) op has finished.

Serialize the shared refcount update and the hardware op under the
existing per-control rstc->lock, taken before the SRCU section to match
reset_control_acquire()'s mutex->srcu order. The lock is taken only on the
shared path and only for may-sleep callers (reset_control_reset() and
reset_control_deassert()); the exclusive path stays lockless. Callers in
atomic context must use the new lockless reset_control_reset_atomic() and
reset_control_deassert_atomic() variants, which skip the mutex and so
carry no completion guarantee.

The op is now called with rstc->lock held on the shared path, so it must
not re-enter reset_control_reset()/deassert()/acquire() on the same
control; a lockdep_assert_not_held() at each lock site catches that.

reset_control_assert() is unchanged: a shared assert makes no guarantee
about the line state, so there is nothing to serialize.

Fixes: 0b52297f2288 ("reset: Add support for shared reset controls")
Link: https://lore.kernel.org/all/aszDZlJ0KahNwgZO@analog.com/
Assisted-by: Anthropic:claude-opus-4-8
Signed-off-by: Shengjiu Wang <shengjiu.wang@nxp.com>
---
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.

 Documentation/driver-api/reset.rst |  10 ++
 drivers/reset/core.c               | 204 ++++++++++++++++++++++-------
 include/linux/reset.h              |  12 ++
 3 files changed, 177 insertions(+), 49 deletions(-)

diff --git a/Documentation/driver-api/reset.rst b/Documentation/driver-api/reset.rst
index 932699c57d1e..0fbc1ba92dd8 100644
--- a/Documentation/driver-api/reset.rst
+++ b/Documentation/driver-api/reset.rst
@@ -185,13 +185,23 @@ Given the reset control, consumers can call reset_control_assert() and
 reset_control_deassert(), trigger a reset pulse using reset_control_reset(), or
 query the reset line status using reset_control_status().
 
+For shared reset controls, reset_control_reset() and reset_control_deassert()
+may sleep and must be called from process context. Consumers that need to
+operate a reset line from atomic context can use reset_control_reset_atomic()
+or reset_control_deassert_atomic(); these never sleep, but do not serialize
+concurrent shared callers and so do not provide the completion guarantee of
+their sleeping counterparts. The underlying reset controller's ops must not
+sleep when the atomic variants are used.
+
 .. kernel-doc:: include/linux/reset.h
    :internal:
 
 .. kernel-doc:: drivers/reset/core.c
    :functions: reset_control_reset
+               reset_control_reset_atomic
                reset_control_assert
                reset_control_deassert
+               reset_control_deassert_atomic
                reset_control_status
                reset_control_acquire
                reset_control_release
diff --git a/drivers/reset/core.c b/drivers/reset/core.c
index 38e189d04d09..d477935e37ea 100644
--- a/drivers/reset/core.c
+++ b/drivers/reset/core.c
@@ -51,7 +51,8 @@ static DEFINE_IDA(reset_gpio_ida);
  * @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.
- * @lock: serializes the internals of reset_control_acquire()
+ * @lock: serializes the internals of reset_control_acquire() and the
+ *        shared deassert/reset refcount update and hardware op
  */
 struct reset_control {
 	struct reset_controller_dev __rcu *rcdev;
@@ -240,12 +241,16 @@ rstc_to_array(struct reset_control *rstc) {
 	return container_of(rstc, struct reset_control_array, base);
 }
 
-static int reset_control_array_reset(struct reset_control_array *resets)
+static int __reset_control_reset(struct reset_control *rstc, bool can_sleep);
+static int __reset_control_deassert(struct reset_control *rstc, bool can_sleep);
+
+static int reset_control_array_reset(struct reset_control_array *resets,
+				     bool can_sleep)
 {
 	int ret, i;
 
 	for (i = 0; i < resets->num_rstcs; i++) {
-		ret = reset_control_reset(resets->rstc[i]);
+		ret = __reset_control_reset(resets->rstc[i], can_sleep);
 		if (ret)
 			return ret;
 	}
@@ -304,12 +309,13 @@ static int reset_control_array_assert(struct reset_control_array *resets)
 	return ret;
 }
 
-static int reset_control_array_deassert(struct reset_control_array *resets)
+static int reset_control_array_deassert(struct reset_control_array *resets,
+					bool can_sleep)
 {
 	int ret, i;
 
 	for (i = 0; i < resets->num_rstcs; i++) {
-		ret = reset_control_deassert(resets->rstc[i]);
+		ret = __reset_control_deassert(resets->rstc[i], can_sleep);
 		if (ret)
 			goto err;
 	}
@@ -355,33 +361,11 @@ static inline bool reset_control_is_array(struct reset_control *rstc)
 	return rstc->array;
 }
 
-/**
- * reset_control_reset - reset the controlled device
- * @rstc: reset controller
- *
- * 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.
- *
- * If rstc is NULL it is an optional reset and the function will just
- * return 0.
- */
-int reset_control_reset(struct reset_control *rstc)
+static int __reset_control_reset_locked(struct reset_control *rstc)
 {
 	struct reset_controller_dev *rcdev;
 	int ret;
 
-	if (!rstc)
-		return 0;
-
-	if (WARN_ON(IS_ERR(rstc)))
-		return -EINVAL;
-
-	if (reset_control_is_array(rstc))
-		return reset_control_array_reset(rstc_to_array(rstc));
-
 	guard(srcu)(&rstc->srcu);
 
 	rcdev = srcu_dereference(rstc->rcdev, &rstc->srcu);
@@ -408,8 +392,83 @@ int reset_control_reset(struct reset_control *rstc)
 
 	return ret;
 }
+
+static int __reset_control_reset(struct reset_control *rstc, bool can_sleep)
+{
+	if (!rstc)
+		return 0;
+
+	if (WARN_ON(IS_ERR(rstc)))
+		return -EINVAL;
+
+	if (reset_control_is_array(rstc))
+		return reset_control_array_reset(rstc_to_array(rstc), can_sleep);
+
+	/*
+	 * Serialize only the shared may-sleep path: holding rstc->lock across
+	 * the refcount update and the controller .reset() op stops a
+	 * concurrent caller from returning before this caller's not-yet-
+	 * finished reset pulse has been issued. can_sleep is false only for
+	 * reset_control_reset_atomic() callers, which cannot take the mutex
+	 * and so get no completion guarantee. The lock is taken before the
+	 * SRCU section (in __reset_control_reset_locked()) to match the
+	 * mutex->srcu order used by reset_control_acquire(). rstc->shared is
+	 * set at creation and is safe to read without the lock.
+	 */
+	if (!(can_sleep && rstc->shared))
+		return __reset_control_reset_locked(rstc);
+
+	might_sleep();
+	lockdep_assert_not_held(&rstc->lock);
+	guard(mutex)(&rstc->lock);
+	return __reset_control_reset_locked(rstc);
+}
+
+/**
+ * reset_control_reset - reset the controlled device
+ * @rstc: reset controller
+ *
+ * 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 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.
+ *
+ * This function may sleep when called on a shared reset control; use
+ * reset_control_reset_atomic() if the caller cannot sleep.
+ *
+ * If rstc is NULL it is an optional reset and the function will just
+ * return 0.
+ */
+int reset_control_reset(struct reset_control *rstc)
+{
+	return __reset_control_reset(rstc, true);
+}
 EXPORT_SYMBOL_GPL(reset_control_reset);
 
+/**
+ * reset_control_reset_atomic - reset the controlled device, usable in atomic
+ *                              context
+ * @rstc: reset controller
+ *
+ * Same as reset_control_reset() but does not sleep and does not serialize
+ * concurrent callers on a shared reset line. Because no lock is taken, a
+ * concurrent reset_control_reset()/_atomic() may still be in progress when
+ * this returns; the completion-ordering guarantee of reset_control_reset()
+ * does not apply. The underlying reset controller's .reset() op must not
+ * sleep.
+ *
+ * If rstc is NULL it is an optional reset and the function will just
+ * return 0.
+ */
+int reset_control_reset_atomic(struct reset_control *rstc)
+{
+	return __reset_control_reset(rstc, false);
+}
+EXPORT_SYMBOL_GPL(reset_control_reset_atomic);
+
 /**
  * reset_control_bulk_reset - reset the controlled devices in order
  * @num_rstcs: number of entries in rstcs array
@@ -575,30 +634,10 @@ int reset_control_bulk_assert(int num_rstcs,
 }
 EXPORT_SYMBOL_GPL(reset_control_bulk_assert);
 
-/**
- * reset_control_deassert - deasserts the reset line
- * @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.
- *
- * If rstc is NULL it is an optional reset and the function will just
- * return 0.
- */
-int reset_control_deassert(struct reset_control *rstc)
+static int __reset_control_deassert_locked(struct reset_control *rstc)
 {
 	struct reset_controller_dev *rcdev;
 
-	if (!rstc)
-		return 0;
-
-	if (WARN_ON(IS_ERR(rstc)))
-		return -EINVAL;
-
-	if (reset_control_is_array(rstc))
-		return reset_control_array_deassert(rstc_to_array(rstc));
-
 	guard(srcu)(&rstc->srcu);
 
 	rcdev = srcu_dereference(rstc->rcdev, &rstc->srcu);
@@ -631,8 +670,74 @@ int reset_control_deassert(struct reset_control *rstc)
 
 	return rcdev->ops->deassert(rcdev, rstc->id);
 }
+
+static int __reset_control_deassert(struct reset_control *rstc, bool can_sleep)
+{
+	if (!rstc)
+		return 0;
+
+	if (WARN_ON(IS_ERR(rstc)))
+		return -EINVAL;
+
+	if (reset_control_is_array(rstc))
+		return reset_control_array_deassert(rstc_to_array(rstc), can_sleep);
+
+	/*
+	 * See __reset_control_reset(): serialize only the shared may-sleep
+	 * path. can_sleep is false only for reset_control_deassert_atomic()
+	 * callers, which cannot take the mutex and get no completion
+	 * guarantee. The lock is taken before the SRCU section (in
+	 * __reset_control_deassert_locked()) to match the mutex->srcu order
+	 * used by reset_control_acquire().
+	 */
+	if (!(can_sleep && rstc->shared))
+		return __reset_control_deassert_locked(rstc);
+
+	might_sleep();
+	lockdep_assert_not_held(&rstc->lock);
+	guard(mutex)(&rstc->lock);
+	return __reset_control_deassert_locked(rstc);
+}
+
+/**
+ * reset_control_deassert - deasserts the reset line
+ * @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.
+ *
+ * This function may sleep when called on a shared reset control; use
+ * reset_control_deassert_atomic() if the caller cannot sleep.
+ *
+ * If rstc is NULL it is an optional reset and the function will just
+ * return 0.
+ */
+int reset_control_deassert(struct reset_control *rstc)
+{
+	return __reset_control_deassert(rstc, true);
+}
 EXPORT_SYMBOL_GPL(reset_control_deassert);
 
+/**
+ * reset_control_deassert_atomic - deasserts the reset line, usable in atomic
+ *                                 context
+ * @rstc: reset controller
+ *
+ * Same as reset_control_deassert() but does not sleep and does not serialize
+ * concurrent callers on a shared reset line, so the deassert-completion
+ * guarantee of reset_control_deassert() does not apply. The underlying reset
+ * controller's .deassert() op must not sleep.
+ *
+ * If rstc is NULL it is an optional reset and the function will just
+ * return 0.
+ */
+int reset_control_deassert_atomic(struct reset_control *rstc)
+{
+	return __reset_control_deassert(rstc, false);
+}
+EXPORT_SYMBOL_GPL(reset_control_deassert_atomic);
+
 /**
  * reset_control_bulk_deassert - deasserts the reset lines in reverse order
  * @num_rstcs: number of entries in rstcs array
@@ -726,6 +831,7 @@ int reset_control_acquire(struct reset_control *rstc)
 	if (reset_control_is_array(rstc))
 		return reset_control_array_acquire(rstc_to_array(rstc));
 
+	lockdep_assert_not_held(&rstc->lock);
 	guard(mutex)(&rstc->lock);
 
 	if (rstc->acquired)
diff --git a/include/linux/reset.h b/include/linux/reset.h
index 9c391cf0c822..ab697193dbe9 100644
--- a/include/linux/reset.h
+++ b/include/linux/reset.h
@@ -73,9 +73,11 @@ enum reset_control_flags {
 #ifdef CONFIG_RESET_CONTROLLER
 
 int reset_control_reset(struct reset_control *rstc);
+int reset_control_reset_atomic(struct reset_control *rstc);
 int reset_control_rearm(struct reset_control *rstc);
 int reset_control_assert(struct reset_control *rstc);
 int reset_control_deassert(struct reset_control *rstc);
+int reset_control_deassert_atomic(struct reset_control *rstc);
 int reset_control_status(struct reset_control *rstc);
 int reset_control_acquire(struct reset_control *rstc);
 void reset_control_release(struct reset_control *rstc);
@@ -117,6 +119,11 @@ static inline int reset_control_reset(struct reset_control *rstc)
 	return 0;
 }
 
+static inline int reset_control_reset_atomic(struct reset_control *rstc)
+{
+	return 0;
+}
+
 static inline int reset_control_rearm(struct reset_control *rstc)
 {
 	return 0;
@@ -132,6 +139,11 @@ static inline int reset_control_deassert(struct reset_control *rstc)
 	return 0;
 }
 
+static inline int reset_control_deassert_atomic(struct reset_control *rstc)
+{
+	return 0;
+}
+
 static inline int reset_control_status(struct reset_control *rstc)
 {
 	return 0;
-- 
2.34.1

Re: [PATCH V2] reset: core: Serialize concurrent deassert/reset of shared resets

[Shengjiu Wang]

Hi Philipp

On Mon, Oct 19, 2026 at 1:52 PM <shengjiu.wang@oss.nxp.com> wrote:
>
> From: Shengjiu Wang <shengjiu.wang@nxp.com>
>
> reset_control_deassert() and reset_control_reset() guarantee that the
> reset line is deasserted / a reset pulse has been issued once they return.
> For a shared reset only the first caller runs the hardware op; others just
> bump the refcount and return early, so a concurrent caller can return
> before the first caller's (possibly sleeping) op has finished.
>
> Serialize the shared refcount update and the hardware op under the
> existing per-control rstc->lock, taken before the SRCU section to match
> reset_control_acquire()'s mutex->srcu order. The lock is taken only on the
> shared path and only for may-sleep callers (reset_control_reset() and
> reset_control_deassert()); the exclusive path stays lockless. Callers in
> atomic context must use the new lockless reset_control_reset_atomic() and
> reset_control_deassert_atomic() variants, which skip the mutex and so
> carry no completion guarantee.
>
> The op is now called with rstc->lock held on the shared path, so it must
> not re-enter reset_control_reset()/deassert()/acquire() on the same
> control; a lockdep_assert_not_held() at each lock site catches that.
>
> reset_control_assert() is unchanged: a shared assert makes no guarantee
> about the line state, so there is nothing to serialize.
>
> Fixes: 0b52297f2288 ("reset: Add support for shared reset controls")
> Link: https://lore.kernel.org/all/aszDZlJ0KahNwgZO@analog.com/
> Assisted-by: Anthropic:claude-opus-4-8
> Signed-off-by: Shengjiu Wang <shengjiu.wang@nxp.com>
> ---
> 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.
>
>  Documentation/driver-api/reset.rst |  10 ++
>  drivers/reset/core.c               | 204 ++++++++++++++++++++++-------
>  include/linux/reset.h              |  12 ++
>  3 files changed, 177 insertions(+), 49 deletions(-)
>
> diff --git a/Documentation/driver-api/reset.rst b/Documentation/driver-api/reset.rst
> index 932699c57d1e..0fbc1ba92dd8 100644
> --- a/Documentation/driver-api/reset.rst
> +++ b/Documentation/driver-api/reset.rst
> @@ -185,13 +185,23 @@ Given the reset control, consumers can call reset_control_assert() and
>  reset_control_deassert(), trigger a reset pulse using reset_control_reset(), or
>  query the reset line status using reset_control_status().
>
> +For shared reset controls, reset_control_reset() and reset_control_deassert()
> +may sleep and must be called from process context. Consumers that need to
> +operate a reset line from atomic context can use reset_control_reset_atomic()
> +or reset_control_deassert_atomic(); these never sleep, but do not serialize
> +concurrent shared callers and so do not provide the completion guarantee of
> +their sleeping counterparts. The underlying reset controller's ops must not
> +sleep when the atomic variants are used.

On second thought, this change lets reset_control_deassert() sleep, so
any driver
that currently calls it from atomic context would now block. The new
_atomic() variants
let those callers opt out, but I can't tell how many drivers would
need updating, and any
one we miss is a silent risk.

Do you have another idea to fix the concurrent deassert() problem on
shared resets
without this risk?  Or should we go back to the original proposal?
https://lore.kernel.org/all/aszDZlJ0KahNwgZO@analog.com/T/#m66615477929b38f788d6d22a2c0bb9ca62173e89

Best regards
Shengjiu Wang

> +
>  .. kernel-doc:: include/linux/reset.h
>     :internal:
>
>  .. kernel-doc:: drivers/reset/core.c
>     :functions: reset_control_reset
> +               reset_control_reset_atomic
>                 reset_control_assert
>                 reset_control_deassert
> +               reset_control_deassert_atomic
>                 reset_control_status
>                 reset_control_acquire
>                 reset_control_release
> diff --git a/drivers/reset/core.c b/drivers/reset/core.c
> index 38e189d04d09..d477935e37ea 100644
> --- a/drivers/reset/core.c
> +++ b/drivers/reset/core.c
> @@ -51,7 +51,8 @@ static DEFINE_IDA(reset_gpio_ida);
>   * @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.
> - * @lock: serializes the internals of reset_control_acquire()
> + * @lock: serializes the internals of reset_control_acquire() and the
> + *        shared deassert/reset refcount update and hardware op
>   */
>  struct reset_control {
>         struct reset_controller_dev __rcu *rcdev;
> @@ -240,12 +241,16 @@ rstc_to_array(struct reset_control *rstc) {
>         return container_of(rstc, struct reset_control_array, base);
>  }
>
> -static int reset_control_array_reset(struct reset_control_array *resets)
> +static int __reset_control_reset(struct reset_control *rstc, bool can_sleep);
> +static int __reset_control_deassert(struct reset_control *rstc, bool can_sleep);
> +
> +static int reset_control_array_reset(struct reset_control_array *resets,
> +                                    bool can_sleep)
>  {
>         int ret, i;
>
>         for (i = 0; i < resets->num_rstcs; i++) {
> -               ret = reset_control_reset(resets->rstc[i]);
> +               ret = __reset_control_reset(resets->rstc[i], can_sleep);
>                 if (ret)
>                         return ret;
>         }
> @@ -304,12 +309,13 @@ static int reset_control_array_assert(struct reset_control_array *resets)
>         return ret;
>  }
>
> -static int reset_control_array_deassert(struct reset_control_array *resets)
> +static int reset_control_array_deassert(struct reset_control_array *resets,
> +                                       bool can_sleep)
>  {
>         int ret, i;
>
>         for (i = 0; i < resets->num_rstcs; i++) {
> -               ret = reset_control_deassert(resets->rstc[i]);
> +               ret = __reset_control_deassert(resets->rstc[i], can_sleep);
>                 if (ret)
>                         goto err;
>         }
> @@ -355,33 +361,11 @@ static inline bool reset_control_is_array(struct reset_control *rstc)
>         return rstc->array;
>  }
>
> -/**
> - * reset_control_reset - reset the controlled device
> - * @rstc: reset controller
> - *
> - * 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.
> - *
> - * If rstc is NULL it is an optional reset and the function will just
> - * return 0.
> - */
> -int reset_control_reset(struct reset_control *rstc)
> +static int __reset_control_reset_locked(struct reset_control *rstc)
>  {
>         struct reset_controller_dev *rcdev;
>         int ret;
>
> -       if (!rstc)
> -               return 0;
> -
> -       if (WARN_ON(IS_ERR(rstc)))
> -               return -EINVAL;
> -
> -       if (reset_control_is_array(rstc))
> -               return reset_control_array_reset(rstc_to_array(rstc));
> -
>         guard(srcu)(&rstc->srcu);
>
>         rcdev = srcu_dereference(rstc->rcdev, &rstc->srcu);
> @@ -408,8 +392,83 @@ int reset_control_reset(struct reset_control *rstc)
>
>         return ret;
>  }
> +
> +static int __reset_control_reset(struct reset_control *rstc, bool can_sleep)
> +{
> +       if (!rstc)
> +               return 0;
> +
> +       if (WARN_ON(IS_ERR(rstc)))
> +               return -EINVAL;
> +
> +       if (reset_control_is_array(rstc))
> +               return reset_control_array_reset(rstc_to_array(rstc), can_sleep);
> +
> +       /*
> +        * Serialize only the shared may-sleep path: holding rstc->lock across
> +        * the refcount update and the controller .reset() op stops a
> +        * concurrent caller from returning before this caller's not-yet-
> +        * finished reset pulse has been issued. can_sleep is false only for
> +        * reset_control_reset_atomic() callers, which cannot take the mutex
> +        * and so get no completion guarantee. The lock is taken before the
> +        * SRCU section (in __reset_control_reset_locked()) to match the
> +        * mutex->srcu order used by reset_control_acquire(). rstc->shared is
> +        * set at creation and is safe to read without the lock.
> +        */
> +       if (!(can_sleep && rstc->shared))
> +               return __reset_control_reset_locked(rstc);
> +
> +       might_sleep();
> +       lockdep_assert_not_held(&rstc->lock);
> +       guard(mutex)(&rstc->lock);
> +       return __reset_control_reset_locked(rstc);
> +}
> +
> +/**
> + * reset_control_reset - reset the controlled device
> + * @rstc: reset controller
> + *
> + * 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 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.
> + *
> + * This function may sleep when called on a shared reset control; use
> + * reset_control_reset_atomic() if the caller cannot sleep.
> + *
> + * If rstc is NULL it is an optional reset and the function will just
> + * return 0.
> + */
> +int reset_control_reset(struct reset_control *rstc)
> +{
> +       return __reset_control_reset(rstc, true);
> +}
>  EXPORT_SYMBOL_GPL(reset_control_reset);
>
> +/**
> + * reset_control_reset_atomic - reset the controlled device, usable in atomic
> + *                              context
> + * @rstc: reset controller
> + *
> + * Same as reset_control_reset() but does not sleep and does not serialize
> + * concurrent callers on a shared reset line. Because no lock is taken, a
> + * concurrent reset_control_reset()/_atomic() may still be in progress when
> + * this returns; the completion-ordering guarantee of reset_control_reset()
> + * does not apply. The underlying reset controller's .reset() op must not
> + * sleep.
> + *
> + * If rstc is NULL it is an optional reset and the function will just
> + * return 0.
> + */
> +int reset_control_reset_atomic(struct reset_control *rstc)
> +{
> +       return __reset_control_reset(rstc, false);
> +}
> +EXPORT_SYMBOL_GPL(reset_control_reset_atomic);
> +
>  /**
>   * reset_control_bulk_reset - reset the controlled devices in order
>   * @num_rstcs: number of entries in rstcs array
> @@ -575,30 +634,10 @@ int reset_control_bulk_assert(int num_rstcs,
>  }
>  EXPORT_SYMBOL_GPL(reset_control_bulk_assert);
>
> -/**
> - * reset_control_deassert - deasserts the reset line
> - * @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.
> - *
> - * If rstc is NULL it is an optional reset and the function will just
> - * return 0.
> - */
> -int reset_control_deassert(struct reset_control *rstc)
> +static int __reset_control_deassert_locked(struct reset_control *rstc)
>  {
>         struct reset_controller_dev *rcdev;
>
> -       if (!rstc)
> -               return 0;
> -
> -       if (WARN_ON(IS_ERR(rstc)))
> -               return -EINVAL;
> -
> -       if (reset_control_is_array(rstc))
> -               return reset_control_array_deassert(rstc_to_array(rstc));
> -
>         guard(srcu)(&rstc->srcu);
>
>         rcdev = srcu_dereference(rstc->rcdev, &rstc->srcu);
> @@ -631,8 +670,74 @@ int reset_control_deassert(struct reset_control *rstc)
>
>         return rcdev->ops->deassert(rcdev, rstc->id);
>  }
> +
> +static int __reset_control_deassert(struct reset_control *rstc, bool can_sleep)
> +{
> +       if (!rstc)
> +               return 0;
> +
> +       if (WARN_ON(IS_ERR(rstc)))
> +               return -EINVAL;
> +
> +       if (reset_control_is_array(rstc))
> +               return reset_control_array_deassert(rstc_to_array(rstc), can_sleep);
> +
> +       /*
> +        * See __reset_control_reset(): serialize only the shared may-sleep
> +        * path. can_sleep is false only for reset_control_deassert_atomic()
> +        * callers, which cannot take the mutex and get no completion
> +        * guarantee. The lock is taken before the SRCU section (in
> +        * __reset_control_deassert_locked()) to match the mutex->srcu order
> +        * used by reset_control_acquire().
> +        */
> +       if (!(can_sleep && rstc->shared))
> +               return __reset_control_deassert_locked(rstc);
> +
> +       might_sleep();
> +       lockdep_assert_not_held(&rstc->lock);
> +       guard(mutex)(&rstc->lock);
> +       return __reset_control_deassert_locked(rstc);
> +}
> +
> +/**
> + * reset_control_deassert - deasserts the reset line
> + * @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.
> + *
> + * This function may sleep when called on a shared reset control; use
> + * reset_control_deassert_atomic() if the caller cannot sleep.
> + *
> + * If rstc is NULL it is an optional reset and the function will just
> + * return 0.
> + */
> +int reset_control_deassert(struct reset_control *rstc)
> +{
> +       return __reset_control_deassert(rstc, true);
> +}
>  EXPORT_SYMBOL_GPL(reset_control_deassert);
>
> +/**
> + * reset_control_deassert_atomic - deasserts the reset line, usable in atomic
> + *                                 context
> + * @rstc: reset controller
> + *
> + * Same as reset_control_deassert() but does not sleep and does not serialize
> + * concurrent callers on a shared reset line, so the deassert-completion
> + * guarantee of reset_control_deassert() does not apply. The underlying reset
> + * controller's .deassert() op must not sleep.
> + *
> + * If rstc is NULL it is an optional reset and the function will just
> + * return 0.
> + */
> +int reset_control_deassert_atomic(struct reset_control *rstc)
> +{
> +       return __reset_control_deassert(rstc, false);
> +}
> +EXPORT_SYMBOL_GPL(reset_control_deassert_atomic);
> +
>  /**
>   * reset_control_bulk_deassert - deasserts the reset lines in reverse order
>   * @num_rstcs: number of entries in rstcs array
> @@ -726,6 +831,7 @@ int reset_control_acquire(struct reset_control *rstc)
>         if (reset_control_is_array(rstc))
>                 return reset_control_array_acquire(rstc_to_array(rstc));
>
> +       lockdep_assert_not_held(&rstc->lock);
>         guard(mutex)(&rstc->lock);
>
>         if (rstc->acquired)
> diff --git a/include/linux/reset.h b/include/linux/reset.h
> index 9c391cf0c822..ab697193dbe9 100644
> --- a/include/linux/reset.h
> +++ b/include/linux/reset.h
> @@ -73,9 +73,11 @@ enum reset_control_flags {
>  #ifdef CONFIG_RESET_CONTROLLER
>
>  int reset_control_reset(struct reset_control *rstc);
> +int reset_control_reset_atomic(struct reset_control *rstc);
>  int reset_control_rearm(struct reset_control *rstc);
>  int reset_control_assert(struct reset_control *rstc);
>  int reset_control_deassert(struct reset_control *rstc);
> +int reset_control_deassert_atomic(struct reset_control *rstc);
>  int reset_control_status(struct reset_control *rstc);
>  int reset_control_acquire(struct reset_control *rstc);
>  void reset_control_release(struct reset_control *rstc);
> @@ -117,6 +119,11 @@ static inline int reset_control_reset(struct reset_control *rstc)
>         return 0;
>  }
>
> +static inline int reset_control_reset_atomic(struct reset_control *rstc)
> +{
> +       return 0;
> +}
> +
>  static inline int reset_control_rearm(struct reset_control *rstc)
>  {
>         return 0;
> @@ -132,6 +139,11 @@ static inline int reset_control_deassert(struct reset_control *rstc)
>         return 0;
>  }
>
> +static inline int reset_control_deassert_atomic(struct reset_control *rstc)
> +{
> +       return 0;
> +}
> +
>  static inline int reset_control_status(struct reset_control *rstc)
>  {
>         return 0;
> --
> 2.34.1
>
>

Re: [PATCH V2] reset: core: Serialize concurrent deassert/reset of shared resets

[Alvin Šipraga]

On Tue, Oct 20, 2026 at 03:17:40PM +0800, Shengjiu Wang wrote:
> Hi Philipp
> 
> On Mon, Oct 19, 2026 at 1:52 PM <shengjiu.wang@oss.nxp.com> wrote:
> >
> > From: Shengjiu Wang <shengjiu.wang@nxp.com>
> >
> > reset_control_deassert() and reset_control_reset() guarantee that the
> > reset line is deasserted / a reset pulse has been issued once they return.
> > For a shared reset only the first caller runs the hardware op; others just
> > bump the refcount and return early, so a concurrent caller can return
> > before the first caller's (possibly sleeping) op has finished.
> >
> > Serialize the shared refcount update and the hardware op under the
> > existing per-control rstc->lock, taken before the SRCU section to match
> > reset_control_acquire()'s mutex->srcu order. The lock is taken only on the
> > shared path and only for may-sleep callers (reset_control_reset() and
> > reset_control_deassert()); the exclusive path stays lockless. Callers in
> > atomic context must use the new lockless reset_control_reset_atomic() and
> > reset_control_deassert_atomic() variants, which skip the mutex and so
> > carry no completion guarantee.
> >
> > The op is now called with rstc->lock held on the shared path, so it must
> > not re-enter reset_control_reset()/deassert()/acquire() on the same
> > control; a lockdep_assert_not_held() at each lock site catches that.
> >
> > reset_control_assert() is unchanged: a shared assert makes no guarantee
> > about the line state, so there is nothing to serialize.
> >
> > Fixes: 0b52297f2288 ("reset: Add support for shared reset controls")
> > Link: https://lore.kernel.org/all/aszDZlJ0KahNwgZO@analog.com/
> > Assisted-by: Anthropic:claude-opus-4-8
> > Signed-off-by: Shengjiu Wang <shengjiu.wang@nxp.com>
> > ---
> > 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.
> >
> >  Documentation/driver-api/reset.rst |  10 ++
> >  drivers/reset/core.c               | 204 ++++++++++++++++++++++-------
> >  include/linux/reset.h              |  12 ++
> >  3 files changed, 177 insertions(+), 49 deletions(-)
> >
> > diff --git a/Documentation/driver-api/reset.rst b/Documentation/driver-api/reset.rst
> > index 932699c57d1e..0fbc1ba92dd8 100644
> > --- a/Documentation/driver-api/reset.rst
> > +++ b/Documentation/driver-api/reset.rst
> > @@ -185,13 +185,23 @@ Given the reset control, consumers can call reset_control_assert() and
> >  reset_control_deassert(), trigger a reset pulse using reset_control_reset(), or
> >  query the reset line status using reset_control_status().
> >
> > +For shared reset controls, reset_control_reset() and reset_control_deassert()
> > +may sleep and must be called from process context. Consumers that need to
> > +operate a reset line from atomic context can use reset_control_reset_atomic()
> > +or reset_control_deassert_atomic(); these never sleep, but do not serialize
> > +concurrent shared callers and so do not provide the completion guarantee of
> > +their sleeping counterparts. The underlying reset controller's ops must not
> > +sleep when the atomic variants are used.
> 
> On second thought, this change lets reset_control_deassert() sleep, so
> any driver
> that currently calls it from atomic context would now block. The new
> _atomic() variants
> let those callers opt out, but I can't tell how many drivers would
> need updating, and any
> one we miss is a silent risk.
> 
> Do you have another idea to fix the concurrent deassert() problem on
> shared resets
> without this risk?  Or should we go back to the original proposal?
> https://lore.kernel.org/all/aszDZlJ0KahNwgZO@analog.com/T/#m66615477929b38f788d6d22a2c0bb9ca62173e89

The reset core could borrow a trick from the regmap library, where it
uses either a mutex or spinlock depending on whether the underlying
reset controller's ops can sleep or not. Check the struct
regmap_bus::fast_io member and how it's used in __regmap_init().

It would also allow the problem to be fixed for atomic users. Right now
you still have the race even if the underlying reset controller's ops
don't sleep and the _atomic() API is being used. But if you know that
the reset controller ops don't sleep, you can serialize shared reset
control operations using a spinlock.

With the above design you can leave the consumers alone and drop the
_atomic() variant of the API functions. (I'll also point out that adding
sleepy/atomic-awareness to the core would open the door for consumers to
report their requirements, e.g. that the reset control musn't sleep, and
for the core to validate this with the properties of the consumer.)

But... it requires an audit of all reset controller providers in the
tree, to see if they are atomic-safe or not, and report some kind of
.can_sleep property to the core (like regmap's .fast_io). I have a
feeling this is still more straightforward and less error-prone than
auditing all the consumers.

The other downside is that it's not a simple patch which can be
backported, which might be what you are looking for.

Any thoughts? I can also help out if the idea seems good.

Kind regards,
Alvin

Re: [PATCH V2] reset: core: Serialize concurrent deassert/reset of shared resets

[Philipp Zabel]

On Di, 2026-10-20 at 10:29 +0200, Alvin Šipraga wrote:
> On Tue, Oct 20, 2026 at 03:17:40PM +0800, Shengjiu Wang wrote:
> > Hi Philipp
> > 
> > On Mon, Oct 19, 2026 at 1:52 PM <shengjiu.wang@oss.nxp.com> wrote:
> > > 
> > > From: Shengjiu Wang <shengjiu.wang@nxp.com>
> > > 
> > > reset_control_deassert() and reset_control_reset() guarantee that the
> > > reset line is deasserted / a reset pulse has been issued once they return.
> > > For a shared reset only the first caller runs the hardware op; others just
> > > bump the refcount and return early, so a concurrent caller can return
> > > before the first caller's (possibly sleeping) op has finished.
> > > 
> > > Serialize the shared refcount update and the hardware op under the
> > > existing per-control rstc->lock, taken before the SRCU section to match
> > > reset_control_acquire()'s mutex->srcu order. The lock is taken only on the
> > > shared path and only for may-sleep callers (reset_control_reset() and
> > > reset_control_deassert()); the exclusive path stays lockless. Callers in
> > > atomic context must use the new lockless reset_control_reset_atomic() and
> > > reset_control_deassert_atomic() variants, which skip the mutex and so
> > > carry no completion guarantee.
> > > 
> > > The op is now called with rstc->lock held on the shared path, so it must
> > > not re-enter reset_control_reset()/deassert()/acquire() on the same
> > > control; a lockdep_assert_not_held() at each lock site catches that.
> > > 
> > > reset_control_assert() is unchanged: a shared assert makes no guarantee
> > > about the line state, so there is nothing to serialize.
> > > 
> > > Fixes: 0b52297f2288 ("reset: Add support for shared reset controls")
> > > Link: https://lore.kernel.org/all/aszDZlJ0KahNwgZO@analog.com/
> > > Assisted-by: Anthropic:claude-opus-4-8
> > > Signed-off-by: Shengjiu Wang <shengjiu.wang@nxp.com>
> > > ---
> > > 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.
> > > 
> > >  Documentation/driver-api/reset.rst |  10 ++
> > >  drivers/reset/core.c               | 204 ++++++++++++++++++++++-------
> > >  include/linux/reset.h              |  12 ++
> > >  3 files changed, 177 insertions(+), 49 deletions(-)
> > > 
> > > diff --git a/Documentation/driver-api/reset.rst b/Documentation/driver-api/reset.rst
> > > index 932699c57d1e..0fbc1ba92dd8 100644
> > > --- a/Documentation/driver-api/reset.rst
> > > +++ b/Documentation/driver-api/reset.rst
> > > @@ -185,13 +185,23 @@ Given the reset control, consumers can call reset_control_assert() and
> > >  reset_control_deassert(), trigger a reset pulse using reset_control_reset(), or
> > >  query the reset line status using reset_control_status().
> > > 
> > > +For shared reset controls, reset_control_reset() and reset_control_deassert()
> > > +may sleep and must be called from process context. Consumers that need to
> > > +operate a reset line from atomic context can use reset_control_reset_atomic()
> > > +or reset_control_deassert_atomic(); these never sleep, but do not serialize
> > > +concurrent shared callers and so do not provide the completion guarantee of
> > > +their sleeping counterparts. The underlying reset controller's ops must not
> > > +sleep when the atomic variants are used.
> > 
> > On second thought, this change lets reset_control_deassert() sleep, so
> > any driver
> > that currently calls it from atomic context would now block. The new
> > _atomic() variants
> > let those callers opt out, but I can't tell how many drivers would
> > need updating, and any
> > one we miss is a silent risk.
> > 
> > Do you have another idea to fix the concurrent deassert() problem on
> > shared resets
> > without this risk?  Or should we go back to the original proposal?
> > https://lore.kernel.org/all/aszDZlJ0KahNwgZO@analog.com/T/#m66615477929b38f788d6d22a2c0bb9ca62173e89
> 
> The reset core could borrow a trick from the regmap library, where it
> uses either a mutex or spinlock depending on whether the underlying
> reset controller's ops can sleep or not. Check the struct
> regmap_bus::fast_io member and how it's used in __regmap_init().
> 
> It would also allow the problem to be fixed for atomic users. Right now
> you still have the race even if the underlying reset controller's ops
> don't sleep and the _atomic() API is being used. But if you know that
> the reset controller ops don't sleep, you can serialize shared reset
> control operations using a spinlock.

Yes, that sounds like it could work.

> With the above design you can leave the consumers alone and drop the
> _atomic() variant of the API functions. (I'll also point out that adding
> sleepy/atomic-awareness to the core would open the door for consumers to
> report their requirements, e.g. that the reset control musn't sleep, and
> for the core to validate this with the properties of the consumer.)

Adding _atomic() variant at the very least would require correctly
setting can_sleep/fast_io for all reset controller implementations and
making sure that the sleeping controllers throw an error in the
_atomic() API.

> But... it requires an audit of all reset controller providers in the
> tree, to see if they are atomic-safe or not, and report some kind of
> .can_sleep property to the core (like regmap's .fast_io). I have a
> feeling this is still more straightforward and less error-prone than
> auditing all the consumers.

Seconded. I'd really like to move to an API that visibly separates
exclusive and shared resets, as well as sleeping / non-sleeping API,
but I'm afraid that would be a lot of churn.

> The other downside is that it's not a simple patch which can be
> backported, which might be what you are looking for.
> 
> Any thoughts? I can also help out if the idea seems good.

How about adding the can_sleep property, setting it for reset-gpio, and
adding the mutex first, and only then adding the other reset
controllers and the spinlock. That could allow to backport a relatively
constrained subset of the change.

regards
Philipp

Re: [PATCH V2] reset: core: Serialize concurrent deassert/reset of shared resets

[Alvin Šipraga]

On Tue, Oct 20, 2026 at 10:56:14AM +0200, Philipp Zabel wrote:
> On Di, 2026-10-20 at 10:29 +0200, Alvin Šipraga wrote:
> > On Tue, Oct 20, 2026 at 03:17:40PM +0800, Shengjiu Wang wrote:
> > > Hi Philipp
> > > 
> > > On Mon, Oct 19, 2026 at 1:52 PM <shengjiu.wang@oss.nxp.com> wrote:
> > > > 
> > > > From: Shengjiu Wang <shengjiu.wang@nxp.com>
> > > > 
> > > > reset_control_deassert() and reset_control_reset() guarantee that the
> > > > reset line is deasserted / a reset pulse has been issued once they return.
> > > > For a shared reset only the first caller runs the hardware op; others just
> > > > bump the refcount and return early, so a concurrent caller can return
> > > > before the first caller's (possibly sleeping) op has finished.
> > > > 
> > > > Serialize the shared refcount update and the hardware op under the
> > > > existing per-control rstc->lock, taken before the SRCU section to match
> > > > reset_control_acquire()'s mutex->srcu order. The lock is taken only on the
> > > > shared path and only for may-sleep callers (reset_control_reset() and
> > > > reset_control_deassert()); the exclusive path stays lockless. Callers in
> > > > atomic context must use the new lockless reset_control_reset_atomic() and
> > > > reset_control_deassert_atomic() variants, which skip the mutex and so
> > > > carry no completion guarantee.
> > > > 
> > > > The op is now called with rstc->lock held on the shared path, so it must
> > > > not re-enter reset_control_reset()/deassert()/acquire() on the same
> > > > control; a lockdep_assert_not_held() at each lock site catches that.
> > > > 
> > > > reset_control_assert() is unchanged: a shared assert makes no guarantee
> > > > about the line state, so there is nothing to serialize.
> > > > 
> > > > Fixes: 0b52297f2288 ("reset: Add support for shared reset controls")
> > > > Link: https://lore.kernel.org/all/aszDZlJ0KahNwgZO@analog.com/
> > > > Assisted-by: Anthropic:claude-opus-4-8
> > > > Signed-off-by: Shengjiu Wang <shengjiu.wang@nxp.com>
> > > > ---
> > > > 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.
> > > > 
> > > >  Documentation/driver-api/reset.rst |  10 ++
> > > >  drivers/reset/core.c               | 204 ++++++++++++++++++++++-------
> > > >  include/linux/reset.h              |  12 ++
> > > >  3 files changed, 177 insertions(+), 49 deletions(-)
> > > > 
> > > > diff --git a/Documentation/driver-api/reset.rst b/Documentation/driver-api/reset.rst
> > > > index 932699c57d1e..0fbc1ba92dd8 100644
> > > > --- a/Documentation/driver-api/reset.rst
> > > > +++ b/Documentation/driver-api/reset.rst
> > > > @@ -185,13 +185,23 @@ Given the reset control, consumers can call reset_control_assert() and
> > > >  reset_control_deassert(), trigger a reset pulse using reset_control_reset(), or
> > > >  query the reset line status using reset_control_status().
> > > > 
> > > > +For shared reset controls, reset_control_reset() and reset_control_deassert()
> > > > +may sleep and must be called from process context. Consumers that need to
> > > > +operate a reset line from atomic context can use reset_control_reset_atomic()
> > > > +or reset_control_deassert_atomic(); these never sleep, but do not serialize
> > > > +concurrent shared callers and so do not provide the completion guarantee of
> > > > +their sleeping counterparts. The underlying reset controller's ops must not
> > > > +sleep when the atomic variants are used.
> > > 
> > > On second thought, this change lets reset_control_deassert() sleep, so
> > > any driver
> > > that currently calls it from atomic context would now block. The new
> > > _atomic() variants
> > > let those callers opt out, but I can't tell how many drivers would
> > > need updating, and any
> > > one we miss is a silent risk.
> > > 
> > > Do you have another idea to fix the concurrent deassert() problem on
> > > shared resets
> > > without this risk?  Or should we go back to the original proposal?
> > > https://lore.kernel.org/all/aszDZlJ0KahNwgZO@analog.com/T/#m66615477929b38f788d6d22a2c0bb9ca62173e89
> > 
> > The reset core could borrow a trick from the regmap library, where it
> > uses either a mutex or spinlock depending on whether the underlying
> > reset controller's ops can sleep or not. Check the struct
> > regmap_bus::fast_io member and how it's used in __regmap_init().
> > 
> > It would also allow the problem to be fixed for atomic users. Right now
> > you still have the race even if the underlying reset controller's ops
> > don't sleep and the _atomic() API is being used. But if you know that
> > the reset controller ops don't sleep, you can serialize shared reset
> > control operations using a spinlock.
> 
> Yes, that sounds like it could work.
> 
> > With the above design you can leave the consumers alone and drop the
> > _atomic() variant of the API functions. (I'll also point out that adding
> > sleepy/atomic-awareness to the core would open the door for consumers to
> > report their requirements, e.g. that the reset control musn't sleep, and
> > for the core to validate this with the properties of the consumer.)
> 
> Adding _atomic() variant at the very least would require correctly
> setting can_sleep/fast_io for all reset controller implementations and
> making sure that the sleeping controllers throw an error in the
> _atomic() API.
> 
> > But... it requires an audit of all reset controller providers in the
> > tree, to see if they are atomic-safe or not, and report some kind of
> > .can_sleep property to the core (like regmap's .fast_io). I have a
> > feeling this is still more straightforward and less error-prone than
> > auditing all the consumers.
> 
> Seconded. I'd really like to move to an API that visibly separates
> exclusive and shared resets, as well as sleeping / non-sleeping API,
> but I'm afraid that would be a lot of churn.
> 
> > The other downside is that it's not a simple patch which can be
> > backported, which might be what you are looking for.
> > 
> > Any thoughts? I can also help out if the idea seems good.
> 
> How about adding the can_sleep property, setting it for reset-gpio, and
> adding the mutex first, and only then adding the other reset
> controllers and the spinlock. That could allow to backport a relatively
> constrained subset of the change.

Yeah that makes sense. Shengjiu, if you also agree, I can pick up the
second phase (tree-wide audit + spinlock variant) if you don't have the
time. Or Philipp, do you think it both phases should be part of the same
overall series?

Kind regards,
Alvin

Re: [PATCH V2] reset: core: Serialize concurrent deassert/reset of shared resets

[Shengjiu Wang]

On Tue, Oct 20, 2026 at 6:03 PM Alvin Šipraga <alvin.sipraga@analog.com> wrote:
>
> On Tue, Oct 20, 2026 at 10:56:14AM +0200, Philipp Zabel wrote:
> > On Di, 2026-10-20 at 10:29 +0200, Alvin Šipraga wrote:
> > > On Tue, Oct 20, 2026 at 03:17:40PM +0800, Shengjiu Wang wrote:
> > > > Hi Philipp
> > > >
> > > > On Mon, Oct 19, 2026 at 1:52 PM <shengjiu.wang@oss.nxp.com> wrote:
> > > > >
> > > > > From: Shengjiu Wang <shengjiu.wang@nxp.com>
> > > > >
> > > > > reset_control_deassert() and reset_control_reset() guarantee that the
> > > > > reset line is deasserted / a reset pulse has been issued once they return.
> > > > > For a shared reset only the first caller runs the hardware op; others just
> > > > > bump the refcount and return early, so a concurrent caller can return
> > > > > before the first caller's (possibly sleeping) op has finished.
> > > > >
> > > > > Serialize the shared refcount update and the hardware op under the
> > > > > existing per-control rstc->lock, taken before the SRCU section to match
> > > > > reset_control_acquire()'s mutex->srcu order. The lock is taken only on the
> > > > > shared path and only for may-sleep callers (reset_control_reset() and
> > > > > reset_control_deassert()); the exclusive path stays lockless. Callers in
> > > > > atomic context must use the new lockless reset_control_reset_atomic() and
> > > > > reset_control_deassert_atomic() variants, which skip the mutex and so
> > > > > carry no completion guarantee.
> > > > >
> > > > > The op is now called with rstc->lock held on the shared path, so it must
> > > > > not re-enter reset_control_reset()/deassert()/acquire() on the same
> > > > > control; a lockdep_assert_not_held() at each lock site catches that.
> > > > >
> > > > > reset_control_assert() is unchanged: a shared assert makes no guarantee
> > > > > about the line state, so there is nothing to serialize.
> > > > >
> > > > > Fixes: 0b52297f2288 ("reset: Add support for shared reset controls")
> > > > > Link: https://lore.kernel.org/all/aszDZlJ0KahNwgZO@analog.com/
> > > > > Assisted-by: Anthropic:claude-opus-4-8
> > > > > Signed-off-by: Shengjiu Wang <shengjiu.wang@nxp.com>
> > > > > ---
> > > > > 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.
> > > > >
> > > > >  Documentation/driver-api/reset.rst |  10 ++
> > > > >  drivers/reset/core.c               | 204 ++++++++++++++++++++++-------
> > > > >  include/linux/reset.h              |  12 ++
> > > > >  3 files changed, 177 insertions(+), 49 deletions(-)
> > > > >
> > > > > diff --git a/Documentation/driver-api/reset.rst b/Documentation/driver-api/reset.rst
> > > > > index 932699c57d1e..0fbc1ba92dd8 100644
> > > > > --- a/Documentation/driver-api/reset.rst
> > > > > +++ b/Documentation/driver-api/reset.rst
> > > > > @@ -185,13 +185,23 @@ Given the reset control, consumers can call reset_control_assert() and
> > > > >  reset_control_deassert(), trigger a reset pulse using reset_control_reset(), or
> > > > >  query the reset line status using reset_control_status().
> > > > >
> > > > > +For shared reset controls, reset_control_reset() and reset_control_deassert()
> > > > > +may sleep and must be called from process context. Consumers that need to
> > > > > +operate a reset line from atomic context can use reset_control_reset_atomic()
> > > > > +or reset_control_deassert_atomic(); these never sleep, but do not serialize
> > > > > +concurrent shared callers and so do not provide the completion guarantee of
> > > > > +their sleeping counterparts. The underlying reset controller's ops must not
> > > > > +sleep when the atomic variants are used.
> > > >
> > > > On second thought, this change lets reset_control_deassert() sleep, so
> > > > any driver
> > > > that currently calls it from atomic context would now block. The new
> > > > _atomic() variants
> > > > let those callers opt out, but I can't tell how many drivers would
> > > > need updating, and any
> > > > one we miss is a silent risk.
> > > >
> > > > Do you have another idea to fix the concurrent deassert() problem on
> > > > shared resets
> > > > without this risk?  Or should we go back to the original proposal?
> > > > https://lore.kernel.org/all/aszDZlJ0KahNwgZO@analog.com/T/#m66615477929b38f788d6d22a2c0bb9ca62173e89
> > >
> > > The reset core could borrow a trick from the regmap library, where it
> > > uses either a mutex or spinlock depending on whether the underlying
> > > reset controller's ops can sleep or not. Check the struct
> > > regmap_bus::fast_io member and how it's used in __regmap_init().
> > >
> > > It would also allow the problem to be fixed for atomic users. Right now
> > > you still have the race even if the underlying reset controller's ops
> > > don't sleep and the _atomic() API is being used. But if you know that
> > > the reset controller ops don't sleep, you can serialize shared reset
> > > control operations using a spinlock.
> >
> > Yes, that sounds like it could work.
> >
> > > With the above design you can leave the consumers alone and drop the
> > > _atomic() variant of the API functions. (I'll also point out that adding
> > > sleepy/atomic-awareness to the core would open the door for consumers to
> > > report their requirements, e.g. that the reset control musn't sleep, and
> > > for the core to validate this with the properties of the consumer.)
> >
> > Adding _atomic() variant at the very least would require correctly
> > setting can_sleep/fast_io for all reset controller implementations and
> > making sure that the sleeping controllers throw an error in the
> > _atomic() API.
> >
> > > But... it requires an audit of all reset controller providers in the
> > > tree, to see if they are atomic-safe or not, and report some kind of
> > > .can_sleep property to the core (like regmap's .fast_io). I have a
> > > feeling this is still more straightforward and less error-prone than
> > > auditing all the consumers.
> >
> > Seconded. I'd really like to move to an API that visibly separates
> > exclusive and shared resets, as well as sleeping / non-sleeping API,
> > but I'm afraid that would be a lot of churn.
> >
> > > The other downside is that it's not a simple patch which can be
> > > backported, which might be what you are looking for.
> > >
> > > Any thoughts? I can also help out if the idea seems good.
> >
> > How about adding the can_sleep property, setting it for reset-gpio, and
> > adding the mutex first, and only then adding the other reset
> > controllers and the spinlock. That could allow to backport a relatively
> > constrained subset of the change.
>
> Yeah that makes sense. Shengjiu, if you also agree, I can pick up the
> second phase (tree-wide audit + spinlock variant) if you don't have the
> time. Or Philipp, do you think it both phases should be part of the same
> overall series?
>

Agreed on the phased approach. I will send the patch for phase 1.

Alvin, please go ahead and pick up phase 2 (the tree-wide audit +
spinlock variant) - I'm happy for you to own it.

Best regards
Shengjiu Wang

Re: [PATCH V2] reset: core: Serialize concurrent deassert/reset of shared resets

[Alvin Šipraga]

On Tue, Oct 20, 2026 at 06:23:29PM +0800, Shengjiu Wang wrote:
> On Tue, Oct 20, 2026 at 6:03 PM Alvin Šipraga <alvin.sipraga@analog.com> wrote:
> >
> > On Tue, Oct 20, 2026 at 10:56:14AM +0200, Philipp Zabel wrote:
> > > On Di, 2026-10-20 at 10:29 +0200, Alvin Šipraga wrote:
> > > > On Tue, Oct 20, 2026 at 03:17:40PM +0800, Shengjiu Wang wrote:
> > > > > Hi Philipp
> > > > >
> > > > > On Mon, Oct 19, 2026 at 1:52 PM <shengjiu.wang@oss.nxp.com> wrote:
> > > > > >
> > > > > > From: Shengjiu Wang <shengjiu.wang@nxp.com>
> > > > > >
> > > > > > reset_control_deassert() and reset_control_reset() guarantee that the
> > > > > > reset line is deasserted / a reset pulse has been issued once they return.
> > > > > > For a shared reset only the first caller runs the hardware op; others just
> > > > > > bump the refcount and return early, so a concurrent caller can return
> > > > > > before the first caller's (possibly sleeping) op has finished.
> > > > > >
> > > > > > Serialize the shared refcount update and the hardware op under the
> > > > > > existing per-control rstc->lock, taken before the SRCU section to match
> > > > > > reset_control_acquire()'s mutex->srcu order. The lock is taken only on the
> > > > > > shared path and only for may-sleep callers (reset_control_reset() and
> > > > > > reset_control_deassert()); the exclusive path stays lockless. Callers in
> > > > > > atomic context must use the new lockless reset_control_reset_atomic() and
> > > > > > reset_control_deassert_atomic() variants, which skip the mutex and so
> > > > > > carry no completion guarantee.
> > > > > >
> > > > > > The op is now called with rstc->lock held on the shared path, so it must
> > > > > > not re-enter reset_control_reset()/deassert()/acquire() on the same
> > > > > > control; a lockdep_assert_not_held() at each lock site catches that.
> > > > > >
> > > > > > reset_control_assert() is unchanged: a shared assert makes no guarantee
> > > > > > about the line state, so there is nothing to serialize.
> > > > > >
> > > > > > Fixes: 0b52297f2288 ("reset: Add support for shared reset controls")
> > > > > > Link: https://urldefense.com/v3/__https://lore.kernel.org/all/aszDZlJ0KahNwgZO@analog.com/__;!!A3Ni8CS0y2Y!-DPstHB0ai7dP0DW897uht_CO6I0TiFaRb7LtPgEydRyEl3Hliigq6BxOLE3llSaFx-aTBDOtenncE_4XlyaU7qCNQ$ 
> > > > > > Assisted-by: Anthropic:claude-opus-4-8
> > > > > > Signed-off-by: Shengjiu Wang <shengjiu.wang@nxp.com>
> > > > > > ---
> > > > > > 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.
> > > > > >
> > > > > >  Documentation/driver-api/reset.rst |  10 ++
> > > > > >  drivers/reset/core.c               | 204 ++++++++++++++++++++++-------
> > > > > >  include/linux/reset.h              |  12 ++
> > > > > >  3 files changed, 177 insertions(+), 49 deletions(-)
> > > > > >
> > > > > > diff --git a/Documentation/driver-api/reset.rst b/Documentation/driver-api/reset.rst
> > > > > > index 932699c57d1e..0fbc1ba92dd8 100644
> > > > > > --- a/Documentation/driver-api/reset.rst
> > > > > > +++ b/Documentation/driver-api/reset.rst
> > > > > > @@ -185,13 +185,23 @@ Given the reset control, consumers can call reset_control_assert() and
> > > > > >  reset_control_deassert(), trigger a reset pulse using reset_control_reset(), or
> > > > > >  query the reset line status using reset_control_status().
> > > > > >
> > > > > > +For shared reset controls, reset_control_reset() and reset_control_deassert()
> > > > > > +may sleep and must be called from process context. Consumers that need to
> > > > > > +operate a reset line from atomic context can use reset_control_reset_atomic()
> > > > > > +or reset_control_deassert_atomic(); these never sleep, but do not serialize
> > > > > > +concurrent shared callers and so do not provide the completion guarantee of
> > > > > > +their sleeping counterparts. The underlying reset controller's ops must not
> > > > > > +sleep when the atomic variants are used.
> > > > >
> > > > > On second thought, this change lets reset_control_deassert() sleep, so
> > > > > any driver
> > > > > that currently calls it from atomic context would now block. The new
> > > > > _atomic() variants
> > > > > let those callers opt out, but I can't tell how many drivers would
> > > > > need updating, and any
> > > > > one we miss is a silent risk.
> > > > >
> > > > > Do you have another idea to fix the concurrent deassert() problem on
> > > > > shared resets
> > > > > without this risk?  Or should we go back to the original proposal?
> > > > > https://urldefense.com/v3/__https://lore.kernel.org/all/aszDZlJ0KahNwgZO@analog.com/T/*m66615477929b38f788d6d22a2c0bb9ca62173e89__;Iw!!A3Ni8CS0y2Y!-DPstHB0ai7dP0DW897uht_CO6I0TiFaRb7LtPgEydRyEl3Hliigq6BxOLE3llSaFx-aTBDOtenncE_4XlyfNK_xgw$ 
> > > >
> > > > The reset core could borrow a trick from the regmap library, where it
> > > > uses either a mutex or spinlock depending on whether the underlying
> > > > reset controller's ops can sleep or not. Check the struct
> > > > regmap_bus::fast_io member and how it's used in __regmap_init().
> > > >
> > > > It would also allow the problem to be fixed for atomic users. Right now
> > > > you still have the race even if the underlying reset controller's ops
> > > > don't sleep and the _atomic() API is being used. But if you know that
> > > > the reset controller ops don't sleep, you can serialize shared reset
> > > > control operations using a spinlock.
> > >
> > > Yes, that sounds like it could work.
> > >
> > > > With the above design you can leave the consumers alone and drop the
> > > > _atomic() variant of the API functions. (I'll also point out that adding
> > > > sleepy/atomic-awareness to the core would open the door for consumers to
> > > > report their requirements, e.g. that the reset control musn't sleep, and
> > > > for the core to validate this with the properties of the consumer.)
> > >
> > > Adding _atomic() variant at the very least would require correctly
> > > setting can_sleep/fast_io for all reset controller implementations and
> > > making sure that the sleeping controllers throw an error in the
> > > _atomic() API.
> > >
> > > > But... it requires an audit of all reset controller providers in the
> > > > tree, to see if they are atomic-safe or not, and report some kind of
> > > > .can_sleep property to the core (like regmap's .fast_io). I have a
> > > > feeling this is still more straightforward and less error-prone than
> > > > auditing all the consumers.
> > >
> > > Seconded. I'd really like to move to an API that visibly separates
> > > exclusive and shared resets, as well as sleeping / non-sleeping API,
> > > but I'm afraid that would be a lot of churn.
> > >
> > > > The other downside is that it's not a simple patch which can be
> > > > backported, which might be what you are looking for.
> > > >
> > > > Any thoughts? I can also help out if the idea seems good.
> > >
> > > How about adding the can_sleep property, setting it for reset-gpio, and
> > > adding the mutex first, and only then adding the other reset
> > > controllers and the spinlock. That could allow to backport a relatively
> > > constrained subset of the change.
> >
> > Yeah that makes sense. Shengjiu, if you also agree, I can pick up the
> > second phase (tree-wide audit + spinlock variant) if you don't have the
> > time. Or Philipp, do you think it both phases should be part of the same
> > overall series?
> >
> 
> Agreed on the phased approach. I will send the patch for phase 1.
> 
> Alvin, please go ahead and pick up phase 2 (the tree-wide audit +
> spinlock variant) - I'm happy for you to own it.

Sure thing! I'll wait for your patch(es) and build on top from there.

Cheers,
Alvin

Re: [PATCH V2] reset: core: Serialize concurrent deassert/reset of shared resets

[Philipp Zabel]

On Di, 2026-10-20 at 15:17 +0800, Shengjiu Wang wrote:
> Hi Philipp
> 
> On Mon, Oct 19, 2026 at 1:52 PM <shengjiu.wang@oss.nxp.com> wrote:
> > 
> > From: Shengjiu Wang <shengjiu.wang@nxp.com>
> > 
> > reset_control_deassert() and reset_control_reset() guarantee that the
> > reset line is deasserted / a reset pulse has been issued once they return.
> > For a shared reset only the first caller runs the hardware op; others just
> > bump the refcount and return early, so a concurrent caller can return
> > before the first caller's (possibly sleeping) op has finished.
> > 
> > Serialize the shared refcount update and the hardware op under the
> > existing per-control rstc->lock, taken before the SRCU section to match
> > reset_control_acquire()'s mutex->srcu order. The lock is taken only on the
> > shared path and only for may-sleep callers (reset_control_reset() and
> > reset_control_deassert()); the exclusive path stays lockless. Callers in
> > atomic context must use the new lockless reset_control_reset_atomic() and
> > reset_control_deassert_atomic() variants, which skip the mutex and so
> > carry no completion guarantee.
> > 
> > The op is now called with rstc->lock held on the shared path, so it must
> > not re-enter reset_control_reset()/deassert()/acquire() on the same
> > control; a lockdep_assert_not_held() at each lock site catches that.
> > 
> > reset_control_assert() is unchanged: a shared assert makes no guarantee
> > about the line state, so there is nothing to serialize.
> > 
> > Fixes: 0b52297f2288 ("reset: Add support for shared reset controls")
> > Link: https://lore.kernel.org/all/aszDZlJ0KahNwgZO@analog.com/
> > Assisted-by: Anthropic:claude-opus-4-8
> > Signed-off-by: Shengjiu Wang <shengjiu.wang@nxp.com>
> > ---
> > 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.
> > 
> >  Documentation/driver-api/reset.rst |  10 ++
> >  drivers/reset/core.c               | 204 ++++++++++++++++++++++-------
> >  include/linux/reset.h              |  12 ++
> >  3 files changed, 177 insertions(+), 49 deletions(-)
> > 
> > diff --git a/Documentation/driver-api/reset.rst b/Documentation/driver-api/reset.rst
> > index 932699c57d1e..0fbc1ba92dd8 100644
> > --- a/Documentation/driver-api/reset.rst
> > +++ b/Documentation/driver-api/reset.rst
> > @@ -185,13 +185,23 @@ Given the reset control, consumers can call reset_control_assert() and
> >  reset_control_deassert(), trigger a reset pulse using reset_control_reset(), or
> >  query the reset line status using reset_control_status().
> > 
> > +For shared reset controls, reset_control_reset() and reset_control_deassert()
> > +may sleep and must be called from process context. Consumers that need to
> > +operate a reset line from atomic context can use reset_control_reset_atomic()
> > +or reset_control_deassert_atomic(); these never sleep, but do not serialize
> > +concurrent shared callers and so do not provide the completion guarantee of
> > +their sleeping counterparts. The underlying reset controller's ops must not
> > +sleep when the atomic variants are used.
> 
> On second thought, this change lets reset_control_deassert() sleep, so
> any driver
> that currently calls it from atomic context would now block. The new
> _atomic() variants
> let those callers opt out, but I can't tell how many drivers would
> need updating, and any
> one we miss is a silent risk.
>
> Do you have another idea to fix the concurrent deassert() problem on
> shared resets
> without this risk?

Maybe add a bool can_sleep field to struct reset_controller_dev, set it
for the reset-gpio driver (which uses gpiod_set_value_cansleep), and
only take a mutex if can_sleep is set? That would have to be a new
lock, taken only inside the SRCU critical section.

regards
Philipp