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

[shengjiu . wang]

From: Shengjiu Wang <shengjiu.wang@xxxxxxx>

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@xxxxxxxxxx/
Assisted-by: Anthropic:claude-opus-4-8
Signed-off-by: Shengjiu Wang <shengjiu.wang@xxxxxxx>
---
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