Re: [PATCH v14 5/5] reset: Leverage revocable for accessing struct reset_controller_dev

From: Bartosz Golaszewski

Date: Thu Oct 15 2026 - 05:53:18 EST


On Sun, 11 Oct 2026 14:12:45 +0200, Tzung-Bi Shih <tzungbi@xxxxxxxxxx> said:
> Commit 78ebbff6d1a0 ("reset: handle removing supplier before consumers")
> protected the `rcdev` pointer in `struct reset_control` with SRCU so
> that the reset core can handle suppliers unbinding while consumers still
> hold reset control handles.
>
> Because `struct reset_control` handles can outlive `rcdev`, each of the
> N handles had to embed its own `srcu_struct` and call synchronize_srcu()
> on release, and reset_controller_unregister() had to call
> synchronize_srcu() N times in a loop.
>
> Leverage dynamic revocable to allocate a single reference-counted
> `struct revocable` for `rcdev` shared across all N `struct reset_control`
> handles (1-to-N). This reduces N `srcu_struct` instances per controller
> to 1, drops synchronize_srcu() from reset_control_put_internal(), and
> replaces the N synchronize_srcu() calls on unregistration with a single
> revocable_revoke().
>
> In addition, calling revocable_revoke() outside `rcdev->lock` avoids an
> ABBA lock inversion between `rcdev->lock` and SRCU, as
> reset_control_acquire() and reset_control_put_internal() acquire
> `rcdev->lock` inside the SRCU read-side critical section.
>
> Signed-off-by: Tzung-Bi Shih <tzungbi@xxxxxxxxxx>
> ---
> v14:
> - New to the series.
> ---
> drivers/reset/core.c | 93 ++++++++------------------------
> include/linux/reset-controller.h | 3 ++
> 2 files changed, 26 insertions(+), 70 deletions(-)
>
> diff --git a/drivers/reset/core.c b/drivers/reset/core.c
> index 38e189d04d09..788a235c3965 100644
> --- a/drivers/reset/core.c
> +++ b/drivers/reset/core.c
> @@ -24,8 +24,8 @@
> #include <linux/property.h>
> #include <linux/reset.h>
> #include <linux/reset-controller.h>
> +#include <linux/revocable.h>
> #include <linux/slab.h>
> -#include <linux/srcu.h>
>
> static DEFINE_MUTEX(reset_list_mutex);
> static LIST_HEAD(reset_controller_list);
> @@ -37,9 +37,7 @@ static DEFINE_IDA(reset_gpio_ida);
>
> /**
> * struct reset_control - a reset control
> - * @rcdev: a pointer to the reset controller device
> - * this reset control belongs to
> - * @srcu: protects the rcdev pointer from removal during consumer access
> + * @rev: revocable handle for accessing the reset controller device

Please call it "revocable". The name "rev" does not indicate what it is at all.
Same elsewhere if you call it the same.

Otherwise looks good to me and nicely shrinks the code! We still don't know
what the fate of that series will be though, right?

Bart