From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-alma10-1.taild15c8.ts.net [100.103.45.18]) (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 5E6E63CBE91 for ; Wed, 19 Aug 2026 19:57:56 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=100.103.45.18 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1787169477; cv=none; b=NMgZs+Vd7JuIaiJKGMA8b6Ra4BTq2q0h+NAnHymEGY/x5Ohyc+zG1xSR2HhDggdXaqhKc4y5N9psNiVxCArA2vAGjh2dfbQycReFsUhKOciBiAxTkzx+1xm4Xy5NTde+bIacK9xJHsa04ru8y/nzvFGIXY5C0C0EzanA6PGDmw8= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1787169477; c=relaxed/simple; bh=jzu94EQsXPGisGHrguT4Dbp3bz6VBoudl7azIi21Kng=; h=From:To:Cc:Subject:In-Reply-To:References:Date:Message-ID: MIME-Version:Content-Type; b=Dk6Hg+JzTfFcrXLZxXM+YLbeosuv8wTbPJMpGRWupuT7MD+Mdxv3cf4S4r6vYgUQBV1K7ZCRj0vNQn+P5PwXLkjAUNXVuWSzOT1ZPjysJVyDCiBoAepbBcGEFkkJXPdoN1xjsKRIFCNB3Sb1S+m73cV0WO6RBxtGQAeZVIPlJjg= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=B/PnaBQN; arc=none smtp.client-ip=100.103.45.18 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="B/PnaBQN" Received: by smtp.kernel.org (Postfix) with ESMTPSA id 2035D1F000E9; Wed, 19 Aug 2026 19:57:54 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=kernel.org; s=k20260515; t=1787169475; bh=+OPYTPXQqsC4hS7zw8e+Dc/1g7hm/IJ2+WXTEbvt4/Y=; h=From:To:Cc:Subject:In-Reply-To:References:Date; b=B/PnaBQNnMu2ijZDpPYgNJkwqocajh+rvx78qxP0wq6l4J7CZMG0miRt3umbqzQeW HLe7agf8zXYNCglv2A9RsCbHjbHVqQF66U62EHNjN2UnsY2RqxQ74pYz3P5RKhIf2m H3UPj8prnE4OSgCSjGopobch9ZSuAkgxMbZCW9NA/URtkPxdz5LjsWIBjaBhH9fXis gVGALsCGjLBTrH+qu3mzgMxUZ9015E+O4nGaG4MNtb/AOE+EdhlEH35juKOJyzNZDD yPEImQafHIUaTyR371kLcufNl05Ju9Qft9VKPGb+ocUKn2JP/hVra/A8QNi33p3Zeu V00h4/BJwByKQ== From: Thomas Gleixner To: Radu Rendec , Ju Nan , mcoquelin.stm32@gmail.com, alexandre.torgue@foss.st.com Cc: linux-kernel@vger.kernel.org, linux-stm32@st-md-mailman.stormreply.com, linux-arm-kernel@lists.infradead.org Subject: Re: [PATCH] irqchip/stm32mp-exti: fix the unit of the hwspinlock timeout In-Reply-To: References: <20260805032139.35420-2-junan76@163.com> <0748a117106e6ae41ae5361ba053af3e96feed40.camel@rendec.net> Date: Wed, 19 Aug 2026 21:57:52 +0200 Message-ID: <87cxvdkhmn.ffs@fw13> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable On Sat, Aug 15 2026 at 10:19, Radu Rendec wrote: > On Sat, 2026-08-15 at 10:08 -0400, Radu Rendec wrote: >> On Wed, 2026-08-05 at 11:21 +0800, Ju Nan wrote: >> > HWSPNLCK_TIMEOUT is passed to hwspin_lock_timeout_in_atomic(), whose >> > timeout argument is in milliseconds, not microseconds: >> >=20 >> > =C2=A0 atomic_delay +=3D HWSPINLOCK_RETRY_DELAY_US; >> > =C2=A0 if (atomic_delay > to * 1000) >> > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 return -ETIMEDO= UT; >> >=20 >> > So stm32mp_exti_set_type() asks for a 1 second timeout where the comme= nt >> > next to the macro says it wants 1 millisecond. The semaphore is polled >> > with udelay() from a section that holds chip_data->rlock, a >> > raw_spinlock_t, so preemption stays disabled for the whole wait on eve= ry >> > configuration, PREEMPT_RT included. >> >=20 >> > The hwspinlock core documents this explicitly: >> >=20 >> > =C2=A0 If the mode is HWLOCK_IN_ATOMIC (called from an atomic context)= the >> > =C2=A0 timeout is handled with busy-waiting delays, hence shall not ex= ceed >> > =C2=A0 few msecs. >> >=20 >> > Pass the value the comment always described. The core retries every >> > HWSPINLOCK_RETRY_DELAY_US (100 us), so the semaphore is still polled t= en >> > times before giving up, which is far longer than any plausible hold ti= me >> > on the coprocessor side. A timeout is reported with pr_err() and fails >> > the trigger type configuration, so shortening it degrades gracefully. >> >=20 >> > Signed-off-by: Ju Nan >> > --- >> > =C2=A0drivers/irqchip/irq-stm32mp-exti.c | 2 +- >> > =C2=A01 file changed, 1 insertion(+), 1 deletion(-) >> >=20 >> > diff --git a/drivers/irqchip/irq-stm32mp-exti.c b/drivers/irqchip/irq-= stm32mp-exti.c >> > index a24f4f1a4..f5f0109bf 100644 >> > --- a/drivers/irqchip/irq-stm32mp-exti.c >> > +++ b/drivers/irqchip/irq-stm32mp-exti.c >> > @@ -23,7 +23,7 @@ >> > =C2=A0 >> > =C2=A0#define IRQS_PER_BANK 32 >> > =C2=A0 >> > -#define HWSPNLCK_TIMEOUT 1000 /* usec */ >> > +#define HWSPNLCK_TIMEOUT 1 /* msec */ >> > =C2=A0 >> > =C2=A0#define EXTI_EnCIDCFGR(n) (0x180 + (n) * 4) >> > =C2=A0#define EXTI_HWCFGR1 0x3f0 >>=20 >> Reviewed-by: Radu Rendec > > Oops! Hit the "send" button too soon. The patch is OK, so the r-b tag > stays. But it also needs this: > > Fixes: 5257169ade8c ("irqchip/stm32-exti: Use the hwspin_lock_timeout_in_= atomic() API") That's correct, but the real problem with that culprit commit is that it just used the existing HWSPNLCK_TIMEOUT define without looking what the units are, which in turn is a stupidity of the original code which picked the most generic naming convention for that define. That should have been: #define HWSPNLCK_TIMEOUT_US 1000 which would have made it entirely clear what the unit is without the stupid tail comment. Now this "fix" just proliferates the same stupidity instead of changing the define to: #define HWSPNLCK_TIMEOUT_MS 1 No? Ju, please send a V3 to that effect. Thanks, tglx