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

[shengjiu . wang]

From: Shengjiu Wang <shengjiu.wang@xxxxxxx>

reset_control_deassert() is documented to guarantee that the reset is
deasserted after the call returns. For a shared reset control only the
first caller runs the hardware .deassert(); all later callers just
increment deassert_count and return immediately:

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

When two consumers of the same shared reset (for example two codecs that
share one reset GPIO) call reset_control_deassert() concurrently, the
second caller can return before the first caller's .deassert() - which
may sleep, e.g. for a GPIO-expander based controller - has actually
released the line. The second consumer then proceeds while the device is
still in reset, contradicting the documented guarantee. The assert and
reset paths have the same problem: only the first shared caller performs
the hardware operation while concurrent callers return early.

Serialize the shared refcount update and the hardware .assert()/
.deassert()/.reset() under the existing per-control rstc->lock so that a
second concurrent caller waits for the first caller's hardware operation
to complete. The lock is taken before the SRCU read section, matching the
ordering already used by reset_control_acquire(). These paths do not take
rcdev->lock, so no new lock-ordering inversion is introduced.

Fixes: 0b52297f2288 ("reset: Add support for shared reset controls")
Link: https://lore.kernel.org/all/aszDZlJ0KahNwgZO@xxxxxxxxxx/
Signed-off-by: Shengjiu Wang <shengjiu.wang@xxxxxxx>
---
 drivers/reset/core.c | 21 ++++++++++++++++++++-
 1 file changed, 20 insertions(+), 1 deletion(-)

diff --git a/drivers/reset/core.c b/drivers/reset/core.c
index 38e189d04d09..7536b61cc5df 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
+ *        assert/deassert/reset of shared reset controls
  */
 struct reset_control {
 	struct reset_controller_dev __rcu *rcdev;
@@ -382,6 +383,12 @@ int reset_control_reset(struct reset_control *rstc)
 	if (reset_control_is_array(rstc))
 		return reset_control_array_reset(rstc_to_array(rstc));
 
+	/*
+	 * Hold the lock across the shared refcount update and the hardware
+	 * reset so that a concurrent caller does not continue before this
+	 * caller's not-yet-finished reset pulse has been issued.
+	 */
+	guard(mutex)(&rstc->lock);
 	guard(srcu)(&rstc->srcu);
 
 	rcdev = srcu_dereference(rstc->rcdev, &rstc->srcu);
@@ -503,6 +510,12 @@ int reset_control_assert(struct reset_control *rstc)
 	if (reset_control_is_array(rstc))
 		return reset_control_array_assert(rstc_to_array(rstc));
 
+	/*
+	 * Hold the lock across the shared refcount update and the hardware
+	 * assert so that a concurrent caller does not observe the line as
+	 * released while this caller is still driving it.
+	 */
+	guard(mutex)(&rstc->lock);
 	guard(srcu)(&rstc->srcu);
 
 	rcdev = srcu_dereference(rstc->rcdev, &rstc->srcu);
@@ -599,6 +612,12 @@ int reset_control_deassert(struct reset_control *rstc)
 	if (reset_control_is_array(rstc))
 		return reset_control_array_deassert(rstc_to_array(rstc));
 
+	/*
+	 * Hold the lock across the shared refcount update and the hardware
+	 * deassert so that a concurrent caller does not continue while the
+	 * line is still asserted by this caller's not-yet-finished deassert.
+	 */
+	guard(mutex)(&rstc->lock);
 	guard(srcu)(&rstc->srcu);
 
 	rcdev = srcu_dereference(rstc->rcdev, &rstc->srcu);
-- 
2.34.1