Re: [PATCH] reset: use a shared SRCU domain for reset controls

[Steven Price <steven.price@arm.com>]

On 17/04/2026 16:48, Steven Price wrote:
> Commit 78ebbff6d1a0 ("reset: handle removing supplier before consumers")
> added a dynamically initialized srcu_struct to every reset_control and
> cleaned it up again when the handle was dropped.
> 
> That breaks early boot users which acquire and release reset handles
> before workqueues are online. On rk3288 this shows up during
> rockchip_smp_prepare_cpus(), where pmu_set_power_domain() gets a reset
> control for a CPU core and then drops it again before SMP bring-up has
> finished. cleanup_srcu_struct() then tries to flush delayed SRCU work
> and hits the WARN_ON(!wq_online) path, which can leave the machine
> hanging before the serial console appears.
> 
> Keep the supplier-removal protection, but move it to a single shared
> static SRCU domain for the reset core. That preserves the rcdev lifetime
> protection needed for supplier unregister without requiring per-handle
> init_srcu_struct()/cleanup_srcu_struct() on normal get/put paths.
> 
> Fixes: 78ebbff6d1a0 ("reset: handle removing supplier before consumers")

Whoops, I forgot by SoB:

Signed-off-by: Steven Price <steven.price@arm.com>

Note I'm not very familiar with this region of code, so let me know if
I'm fixing this in the wrong way. This is a regression for the RK3288
Firefly board I'm using so I'm keen to get some form of fix in.

Thanks,
Steve

> ---
>  drivers/reset/core.c | 48 ++++++++++++++++++--------------------------
>  1 file changed, 20 insertions(+), 28 deletions(-)
> 
> diff --git a/drivers/reset/core.c b/drivers/reset/core.c
> index 38e189d04d09..ab4989084824 100644
> --- a/drivers/reset/core.c
> +++ b/drivers/reset/core.c
> @@ -29,6 +29,11 @@
>  
>  static DEFINE_MUTEX(reset_list_mutex);
>  static LIST_HEAD(reset_controller_list);
> +/*
> + * Use one shared SRCU domain so reset handles don't need per-instance
> + * init/cleanup on early boot paths.
> + */
> +DEFINE_STATIC_SRCU(reset_control_srcu);
>  
>  /* Protects reset_gpio_lookup_list */
>  static DEFINE_MUTEX(reset_gpio_lookup_mutex);
> @@ -39,7 +44,6 @@ 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
>   * @list: list entry for the rcdev's reset controller list
>   * @id: ID of the reset controller in the reset
>   *      controller device
> @@ -55,7 +59,6 @@ static DEFINE_IDA(reset_gpio_ida);
>   */
>  struct reset_control {
>  	struct reset_controller_dev __rcu *rcdev;
> -	struct srcu_struct srcu;
>  	struct list_head list;
>  	unsigned int id;
>  	struct kref refcnt;
> @@ -188,7 +191,7 @@ void reset_controller_unregister(struct reset_controller_dev *rcdev)
>  		 */
>  		list_for_each_entry_safe(rstc, pos, &rcdev->reset_control_head, list) {
>  			rcu_assign_pointer(rstc->rcdev, NULL);
> -			synchronize_srcu(&rstc->srcu);
> +			synchronize_srcu(&reset_control_srcu);
>  			reset_controller_remove(rcdev, rstc);
>  		}
>  	}
> @@ -382,9 +385,9 @@ int reset_control_reset(struct reset_control *rstc)
>  	if (reset_control_is_array(rstc))
>  		return reset_control_array_reset(rstc_to_array(rstc));
>  
> -	guard(srcu)(&rstc->srcu);
> +	guard(srcu)(&reset_control_srcu);
>  
> -	rcdev = srcu_dereference(rstc->rcdev, &rstc->srcu);
> +	rcdev = srcu_dereference(rstc->rcdev, &reset_control_srcu);
>  	if (!rcdev)
>  		return -ENODEV;
>  
> @@ -503,9 +506,9 @@ int reset_control_assert(struct reset_control *rstc)
>  	if (reset_control_is_array(rstc))
>  		return reset_control_array_assert(rstc_to_array(rstc));
>  
> -	guard(srcu)(&rstc->srcu);
> +	guard(srcu)(&reset_control_srcu);
>  
> -	rcdev = srcu_dereference(rstc->rcdev, &rstc->srcu);
> +	rcdev = srcu_dereference(rstc->rcdev, &reset_control_srcu);
>  	if (!rcdev)
>  		return -ENODEV;
>  
> @@ -599,9 +602,9 @@ int reset_control_deassert(struct reset_control *rstc)
>  	if (reset_control_is_array(rstc))
>  		return reset_control_array_deassert(rstc_to_array(rstc));
>  
> -	guard(srcu)(&rstc->srcu);
> +	guard(srcu)(&reset_control_srcu);
>  
> -	rcdev = srcu_dereference(rstc->rcdev, &rstc->srcu);
> +	rcdev = srcu_dereference(rstc->rcdev, &reset_control_srcu);
>  	if (!rcdev)
>  		return -ENODEV;
>  
> @@ -679,9 +682,9 @@ int reset_control_status(struct reset_control *rstc)
>  	if (WARN_ON(IS_ERR(rstc)) || reset_control_is_array(rstc))
>  		return -EINVAL;
>  
> -	guard(srcu)(&rstc->srcu);
> +	guard(srcu)(&reset_control_srcu);
>  
> -	rcdev = srcu_dereference(rstc->rcdev, &rstc->srcu);
> +	rcdev = srcu_dereference(rstc->rcdev, &reset_control_srcu);
>  	if (!rcdev)
>  		return -ENODEV;
>  
> @@ -731,9 +734,9 @@ int reset_control_acquire(struct reset_control *rstc)
>  	if (rstc->acquired)
>  		return 0;
>  
> -	guard(srcu)(&rstc->srcu);
> +	guard(srcu)(&reset_control_srcu);
>  
> -	rcdev = srcu_dereference(rstc->rcdev, &rstc->srcu);
> +	rcdev = srcu_dereference(rstc->rcdev, &reset_control_srcu);
>  	if (!rcdev)
>  		return -ENODEV;
>  
> @@ -831,7 +834,6 @@ __reset_control_get_internal(struct reset_controller_dev *rcdev,
>  	bool shared = flags & RESET_CONTROL_FLAGS_BIT_SHARED;
>  	bool acquired = flags & RESET_CONTROL_FLAGS_BIT_ACQUIRED;
>  	struct reset_control *rstc;
> -	int ret;
>  
>  	lockdep_assert_held(&rcdev->lock);
>  
> @@ -862,14 +864,7 @@ __reset_control_get_internal(struct reset_controller_dev *rcdev,
>  	if (!rstc)
>  		return ERR_PTR(-ENOMEM);
>  
> -	ret = init_srcu_struct(&rstc->srcu);
> -	if (ret) {
> -		kfree(rstc);
> -		return ERR_PTR(ret);
> -	}
> -
>  	if (!try_module_get(rcdev->owner)) {
> -		cleanup_srcu_struct(&rstc->srcu);
>  		kfree(rstc);
>  		return ERR_PTR(-ENODEV);
>  	}
> @@ -892,7 +887,7 @@ static void __reset_control_release(struct kref *kref)
>  						  refcnt);
>  	struct reset_controller_dev *rcdev;
>  
> -	lockdep_assert_held(&rstc->srcu);
> +	lockdep_assert_held(&reset_control_srcu);
>  
>  	rcdev = rcu_replace_pointer(rstc->rcdev, NULL, true);
>  	if (rcdev) {
> @@ -911,8 +906,8 @@ static void reset_control_put_internal(struct reset_control *rstc)
>  	if (IS_ERR_OR_NULL(rstc))
>  		return;
>  
> -	scoped_guard(srcu, &rstc->srcu) {
> -		rcdev = srcu_dereference(rstc->rcdev, &rstc->srcu);
> +	scoped_guard(srcu, &reset_control_srcu) {
> +		rcdev = srcu_dereference(rstc->rcdev, &reset_control_srcu);
>  		if (!rcdev)
>  			/* Already released. */
>  			return;
> @@ -921,11 +916,8 @@ static void reset_control_put_internal(struct reset_control *rstc)
>  		ret = kref_put(&rstc->refcnt, __reset_control_release);
>  	}
>  
> -	if (ret) {
> -		synchronize_srcu(&rstc->srcu);
> -		cleanup_srcu_struct(&rstc->srcu);
> +	if (ret)
>  		kfree(rstc);
> -	}
>  }
>  
>  static void reset_gpio_aux_device_release(struct device *dev)