From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by smtp.subspace.kernel.org (Postfix) with ESMTP id 2267B386421 for ; Mon, 20 Apr 2026 08:18:39 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=217.140.110.172 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1776673123; cv=none; b=lB/lQRxClVwwCU/hdnKtSlpLDLa5kTVKZgY8MJV8uEo0/7FA6VdXGSXzOrF43luGjaw8tTT72S75eqJ/pNsNMhMoLqjioxwG5Yl7Bvq8Hk5gZ2EwkFmj0zSmKNfzuL3WeUO3tX78C8zjZpCeI8GXssT4xHBGbfkfKCXEImCgvSM= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1776673123; c=relaxed/simple; bh=wG+g7g76v9OMgXvc7rocVhXGNENd3FdHC4we4kBeJD0=; h=Message-ID:Date:MIME-Version:Subject:To:Cc:References:From: In-Reply-To:Content-Type; b=FMVJw4vofV/1Txo5hH3xyB9ffxiqyheTpkNLvninXM8y1ultYcKWJnmNQImqTjpnOb+BKixG5ENwpAX6oo4Dv8e6Q8MX5kf4D3pqaQaTMXsdJYLF3Gw79Vra9DJvbxkyyMyrOljx3XysKETAwAq09n4BVV1yP78PW9iVVFj85/I= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=arm.com; spf=pass smtp.mailfrom=arm.com; dkim=pass (1024-bit key) header.d=arm.com header.i=@arm.com header.b=iXQTmlm0; arc=none smtp.client-ip=217.140.110.172 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=arm.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=arm.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=arm.com header.i=@arm.com header.b="iXQTmlm0" Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 88F821516; Mon, 20 Apr 2026 01:18:33 -0700 (PDT) Received: from [10.57.32.80] (unknown [10.57.32.80]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 84DCB3F641; Mon, 20 Apr 2026 01:18:38 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=simple/simple; d=arm.com; s=foss; t=1776673119; bh=wG+g7g76v9OMgXvc7rocVhXGNENd3FdHC4we4kBeJD0=; h=Date:Subject:To:Cc:References:From:In-Reply-To:From; b=iXQTmlm0/9QjZ+BQSiS8ZMbQUOYa1Tp4voCNaQ/N+aGsSoh/7d+qyxV3d9F+8YN9C bHmDH9GZyKF86vn9lgyh+cPIWyF9mDPJEHxnAHACQ2p69NAuIW/+Q+o3hQCLI0Tr3H UJNWZ+eSNrVy2Y71OPbfaZ/C9kV5Y1JIlbhhcCc0= Message-ID: Date: Mon, 20 Apr 2026 09:18:39 +0100 Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH] reset: use a shared SRCU domain for reset controls To: Philipp Zabel Cc: linux-kernel@vger.kernel.org, Bartosz Golaszewski References: <20260417154809.1984386-1-steven.price@arm.com> From: Steven Price Content-Language: en-GB In-Reply-To: <20260417154809.1984386-1-steven.price@arm.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit 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 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)