From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from gloria.sntech.de (gloria.sntech.de [185.11.138.130]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id C6C682BE655 for ; Thu, 21 May 2026 21:15:50 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=185.11.138.130 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1779398157; cv=none; b=l84h8RCugRa9vU1qjWEhekNAZTulzKM6GecdEWqt6KH6soSl459tAGFr/5bNsAnypOPxYG3mco0Pn5sP2c7jwoqkV9hehLqDZPW/j4szKdtTIAlbcTpdBYz0iXsaxZwA/Y4OTub5i6V+y+Ohj5B44DXPYRNNlLjm7vXkVndOBc4= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1779398157; c=relaxed/simple; bh=zevUtlXPVMrMisWD94g9+ZgRPEpLfk/aosXwJylX8tA=; h=From:To:Cc:Subject:Date:Message-ID:In-Reply-To:References: MIME-Version:Content-Type; b=VRIu617hNSrEb1I8gjtCbcW03HdmnBTIvYX8K8EXzmYkm/IDznGey9Lzm5RxqRAasbLOB1QMc/l8Aiceb2CIkiSKA2L8Fix73C/AIjV82N7m83ckW4qeCkOEp1zqyMZRkaAZZ8+OVv6WhXsnbyuc8iXXr1LSr0UjySVp+HWBuBI= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=quarantine dis=none) header.from=sntech.de; spf=pass smtp.mailfrom=sntech.de; dkim=pass (2048-bit key) header.d=sntech.de header.i=@sntech.de header.b=0WkXu40A; arc=none smtp.client-ip=185.11.138.130 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=quarantine dis=none) header.from=sntech.de Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=sntech.de Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=sntech.de header.i=@sntech.de header.b="0WkXu40A" DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=sntech.de; s=gloria202408; h=Content-Type:Content-Transfer-Encoding:MIME-Version: References:In-Reply-To:Message-ID:Date:Subject:Cc:To:From:Reply-To; bh=UmbpbrBmKWqszodkbLA97llCbkrFLNX+52JJvlRhvCs=; b=0WkXu40AhUpvJCFOXc+MhFqQwC X4KCYji6ci5rWG2O4M6f5NfT1dtXke7KrY6rZTeyw3BWjUAJ80J4vWkq8229echZ/Us5/LsRRaigQ cgs3Y1qDyLuL4MZe7F3pCG1uHgKFvPTalmKhSq2+/2d8AH3FYLhWoy1t8FJhPV8ILUU2ryiXfHr2k ZuODac9cm2nMTTkckJKYGY2e2vkJQAl+RwB7d/o/GvPdSGjEPXGieG0fXq6jIbN4My4tZnsn1bLHx sAU1HbxkW32htTO9pO5AxoDl5LTj+8VLoVZoChk57zvU+tbZ9EBJEMUTu7XMdRSFftwsYUG+WuSCT 6u4fKa2Q==; From: Heiko Stuebner To: Philipp Zabel , Steven Price Cc: linux-kernel@vger.kernel.org, Bartosz Golaszewski Subject: Re: [PATCH] reset: use a shared SRCU domain for reset controls Date: Thu, 21 May 2026 23:15:39 +0200 Message-ID: <4719175.XrmoMso0CX@phil> In-Reply-To: References: <20260417154809.1984386-1-steven.price@arm.com> <9d06e549-ad7e-4089-99f1-88eb4b95a329@arm.com> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset="utf-8" Hi, Am Donnerstag, 14. Mai 2026, 11:07:29 Mitteleurop=C3=A4ische Sommerzeit sch= rieb Steven Price: > Gentle ping (see below) >=20 > On 24/04/2026 10:04, Steven Price wrote: > > On 23/04/2026 15:15, Philipp Zabel wrote: > >> Hi Steven, Heiko, > >> > >> On Do, 2026-04-23 at 13:45 +0100, Steven Price wrote: > >>> +Heiko for the Rockchip questions. > >>> > >>> On 23/04/2026 11:27, Philipp Zabel wrote: > >>>> Hi Steven, > >>>> > >>>> On Fr, 2026-04-17 at 16:48 +0100, Steven Price wrote: > >>>>> Commit 78ebbff6d1a0 ("reset: handle removing supplier before consum= ers") > >>>>> 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 re= set > >>>>> control for a CPU core and then drops it again before SMP bring-up = has > >>>>> finished. > >>>> > >>>> Can the reset_control_put() call be dropped from pmu_set_power_domai= n() > >>>> to fix the problem? > >>> > >>> I'm not that familiar with the code, so I'm not sure. > >>> > >>> Just dropping that call causes a WARN_ON() bringing the secondary CPUs > >>> on (because the call to rockchip_get_core_reset() expects to have > >>> exclusive access to the reset). > >> > >> Yes, caused by re-requesting the same reset again. I was thinking of > >> storing the controls in an array in rockchip_smp_prepare_cpus() and > >> reusing them in pmu_set_power_domain(), see the patch below. > >> > >>> Switching to a shared reset then his a > >>> WARN_ON() in reset_control_assert because deassert_count =3D=3D 0. I = could > >>> keep digging blindly but I'm not really sure how this code is meant t= o work. > >> > >> Shared reset is not the right mechanism here, that would be for > >> multiple drivers/devices sharing the same reset line. > >=20 > > That makes sense - I hadn't really looked into whether this was a single > > reset for all CPUs or somehow one shared between them all. Thinking > > about it more I can't see how a shared reset would have worked ;) > >=20 > >>> Hopefully Heiko might be able to shed some more light on this? > >>> > >>>> Putting the reset control should mean that the driver doesn't care > >>>> about the state of the reset line anymore, but the platsmp code very > >>>> much expects the reset line to stay deasserted after enabling a CPU. > >>>> Acquiring reset controls in rockchip_smp_prepare_cpus() once and nev= er > >>>> giving them up via reset_control_put() seems like a correct fix, > >>>> regardless of whether this patch is applied or not. > >>>> > >>>> It looks like the meson platsmp suffers from the same issue. > >>> > >>> This is why I did the fix in the reset code - > >> > >> And I'm grateful, because that made sashiko.dev point out an ABBA > >> deadlock opportunity in the existing code. > >=20 > > Cool. > >=20 > >>> how many other platforms > >>> might have similar issues? But obviously if these platforms are buggy > >>> then they should be fixed. > >> > >> Agreed. > >> > >>> My interest is keeping the devboard working so I can keep testing > >>> Panfrost on it. > >> > >> Does this patch work? > >=20 > > Indeed it does - thanks. Feel free to add my T-b when posting: > >=20 > > Tested-by: Steven Price >=20 > Can we pick the below patch up please? Let me know if there's anything I > can do to move this along. I completely missed this series, until now and until I fell onto that issue while testing on the rk3288. I tested the patch and send it out to the lists for a loop [0], (and just realized after the fact I forgot Bartosz, sorry) before I plan to apply it in the next das for the current rc kernels. Heiko [0] http://lore.kernel.org/all/20260521210915.2331176-1-heiko@sntech.de > >> ----------8<---------- > >> From: Philipp Zabel > >> Subject: [PATCH] ARM: rockchip: keep reset control around > >> > >> Do not put the reset control, retain exclusive control over it. > >> After turning on a CPU, the corresponding reset line must stay > >> deasserted. > >> > >> This also avoids calling reset_control_put() before workqueues > >> are operational. > >> > >> Signed-off-by: Philipp Zabel > >> --- > >> arch/arm/mach-rockchip/platsmp.c | 15 +++++++++------ > >> 1 file changed, 9 insertions(+), 6 deletions(-) > >> > >> diff --git a/arch/arm/mach-rockchip/platsmp.c b/arch/arm/mach-rockchip= /platsmp.c > >> index f432d22bfed8..c28816fffce8 100644 > >> --- a/arch/arm/mach-rockchip/platsmp.c > >> +++ b/arch/arm/mach-rockchip/platsmp.c > >> @@ -34,6 +34,7 @@ static int ncores; > >> =20 > >> static struct regmap *pmu; > >> static int has_pmu =3D true; > >> +static struct reset_control *cpu_rstc[4]; > >> =20 > >> static int pmu_power_domain_is_on(int pd) > >> { > >> @@ -64,9 +65,11 @@ static struct reset_control *rockchip_get_core_rese= t(int cpu) > >> static int pmu_set_power_domain(int pd, bool on) > >> { > >> u32 val =3D (on) ? 0 : BIT(pd); > >> - struct reset_control *rstc =3D rockchip_get_core_reset(pd); > >> + struct reset_control *rstc; > >> int ret; > >> =20 > >> + rstc =3D pd < ARRAY_SIZE(cpu_rstc) ? cpu_rstc[pd] : ERR_PTR(-EINVAL); > >> + > >> if (IS_ERR(rstc) && read_cpuid_part() !=3D ARM_CPU_PART_CORTEX_A9) { > >> pr_err("%s: could not get reset control for core %d\n", > >> __func__, pd); > >> @@ -100,11 +103,8 @@ static int pmu_set_power_domain(int pd, bool on) > >> } > >> } > >> =20 > >> - if (!IS_ERR(rstc)) { > >> - if (on) > >> - reset_control_deassert(rstc); > >> - reset_control_put(rstc); > >> - } > >> + if (!IS_ERR(rstc) && on) > >> + reset_control_deassert(rstc); > >> =20 > >> return 0; > >> } > >> @@ -312,6 +312,9 @@ static void __init rockchip_smp_prepare_cpus(unsig= ned int max_cpus) > >> ncores =3D ((l2ctlr >> 24) & 0x3) + 1; > >> } > >> =20 > >> + for (i =3D 0; i < ncores; i++) > >> + cpu_rstc[i] =3D rockchip_get_core_reset(i); > >> + > >> /* Make sure that all cores except the first are really off */ > >> for (i =3D 1; i < ncores; i++) > >> pmu_set_power_domain(0 + i, false); > >=20 >=20 >=20