1. Patchew
  2. linux
  3. View series

[PATCH] irqchip/stm32mp-exti: fix the unit of the hwspinlock timeout

Ju Nan posted 1 patch 1 month, 4 weeks ago
There is a newer version of this series
drivers/irqchip/irq-stm32mp-exti.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
[PATCH] irqchip/stm32mp-exti: fix the unit of the hwspinlock timeout
Posted by Ju Nan 1 month, 4 weeks ago 
HWSPNLCK_TIMEOUT is passed to hwspin_lock_timeout_in_atomic(), whose
timeout argument is in milliseconds, not microseconds:

  atomic_delay += HWSPINLOCK_RETRY_DELAY_US;
  if (atomic_delay > to * 1000)
          return -ETIMEDOUT;

So stm32mp_exti_set_type() asks for a 1 second timeout where the comment
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 every
configuration, PREEMPT_RT included.

The hwspinlock core documents this explicitly:

  If the mode is HWLOCK_IN_ATOMIC (called from an atomic context) the
  timeout is handled with busy-waiting delays, hence shall not exceed
  few msecs.

Pass the value the comment always described. The core retries every
HWSPINLOCK_RETRY_DELAY_US (100 us), so the semaphore is still polled ten
times before giving up, which is far longer than any plausible hold time
on the coprocessor side. A timeout is reported with pr_err() and fails
the trigger type configuration, so shortening it degrades gracefully.

Signed-off-by: Ju Nan <junan76@163.com>
---
 drivers/irqchip/irq-stm32mp-exti.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

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 @@
 
 #define IRQS_PER_BANK			32
 
-#define HWSPNLCK_TIMEOUT		1000 /* usec */
+#define HWSPNLCK_TIMEOUT		1 /* msec */
 
 #define EXTI_EnCIDCFGR(n)		(0x180 + (n) * 4)
 #define EXTI_HWCFGR1			0x3f0
-- 
2.55.0
Re: [PATCH] irqchip/stm32mp-exti: fix the unit of the hwspinlock timeout
Posted by Radu Rendec 1 month, 2 weeks ago 
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:
> 
>   atomic_delay += HWSPINLOCK_RETRY_DELAY_US;
>   if (atomic_delay > to * 1000)
>           return -ETIMEDOUT;
> 
> So stm32mp_exti_set_type() asks for a 1 second timeout where the comment
> 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 every
> configuration, PREEMPT_RT included.
> 
> The hwspinlock core documents this explicitly:
> 
>   If the mode is HWLOCK_IN_ATOMIC (called from an atomic context) the
>   timeout is handled with busy-waiting delays, hence shall not exceed
>   few msecs.
> 
> Pass the value the comment always described. The core retries every
> HWSPINLOCK_RETRY_DELAY_US (100 us), so the semaphore is still polled ten
> times before giving up, which is far longer than any plausible hold time
> on the coprocessor side. A timeout is reported with pr_err() and fails
> the trigger type configuration, so shortening it degrades gracefully.
> 
> Signed-off-by: Ju Nan <junan76@163.com>
> ---
>  drivers/irqchip/irq-stm32mp-exti.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> 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 @@
>  
>  #define IRQS_PER_BANK			32
>  
> -#define HWSPNLCK_TIMEOUT		1000 /* usec */
> +#define HWSPNLCK_TIMEOUT		1 /* msec */
>  
>  #define EXTI_EnCIDCFGR(n)		(0x180 + (n) * 4)
>  #define EXTI_HWCFGR1			0x3f0

Reviewed-by: Radu Rendec <radu@rendec.net>
Re: [PATCH] irqchip/stm32mp-exti: fix the unit of the hwspinlock timeout
Posted by Radu Rendec 1 month, 2 weeks ago 
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:
> > 
> >   atomic_delay += HWSPINLOCK_RETRY_DELAY_US;
> >   if (atomic_delay > to * 1000)
> >           return -ETIMEDOUT;
> > 
> > So stm32mp_exti_set_type() asks for a 1 second timeout where the comment
> > 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 every
> > configuration, PREEMPT_RT included.
> > 
> > The hwspinlock core documents this explicitly:
> > 
> >   If the mode is HWLOCK_IN_ATOMIC (called from an atomic context) the
> >   timeout is handled with busy-waiting delays, hence shall not exceed
> >   few msecs.
> > 
> > Pass the value the comment always described. The core retries every
> > HWSPINLOCK_RETRY_DELAY_US (100 us), so the semaphore is still polled ten
> > times before giving up, which is far longer than any plausible hold time
> > on the coprocessor side. A timeout is reported with pr_err() and fails
> > the trigger type configuration, so shortening it degrades gracefully.
> > 
> > Signed-off-by: Ju Nan <junan76@163.com>
> > ---
> >  drivers/irqchip/irq-stm32mp-exti.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > 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 @@
> >  
> >  #define IRQS_PER_BANK			32
> >  
> > -#define HWSPNLCK_TIMEOUT		1000 /* usec */
> > +#define HWSPNLCK_TIMEOUT		1 /* msec */
> >  
> >  #define EXTI_EnCIDCFGR(n)		(0x180 + (n) * 4)
> >  #define EXTI_HWCFGR1			0x3f0
> 
> Reviewed-by: Radu Rendec <radu@rendec.net>

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")
Re: [PATCH] irqchip/stm32mp-exti: fix the unit of the hwspinlock timeout
Posted by Thomas Gleixner 1 month, 1 week ago 
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:
>> > 
>> >   atomic_delay += HWSPINLOCK_RETRY_DELAY_US;
>> >   if (atomic_delay > to * 1000)
>> >           return -ETIMEDOUT;
>> > 
>> > So stm32mp_exti_set_type() asks for a 1 second timeout where the comment
>> > 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 every
>> > configuration, PREEMPT_RT included.
>> > 
>> > The hwspinlock core documents this explicitly:
>> > 
>> >   If the mode is HWLOCK_IN_ATOMIC (called from an atomic context) the
>> >   timeout is handled with busy-waiting delays, hence shall not exceed
>> >   few msecs.
>> > 
>> > Pass the value the comment always described. The core retries every
>> > HWSPINLOCK_RETRY_DELAY_US (100 us), so the semaphore is still polled ten
>> > times before giving up, which is far longer than any plausible hold time
>> > on the coprocessor side. A timeout is reported with pr_err() and fails
>> > the trigger type configuration, so shortening it degrades gracefully.
>> > 
>> > Signed-off-by: Ju Nan <junan76@163.com>
>> > ---
>> >  drivers/irqchip/irq-stm32mp-exti.c | 2 +-
>> >  1 file changed, 1 insertion(+), 1 deletion(-)
>> > 
>> > 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 @@
>> >  
>> >  #define IRQS_PER_BANK			32
>> >  
>> > -#define HWSPNLCK_TIMEOUT		1000 /* usec */
>> > +#define HWSPNLCK_TIMEOUT		1 /* msec */
>> >  
>> >  #define EXTI_EnCIDCFGR(n)		(0x180 + (n) * 4)
>> >  #define EXTI_HWCFGR1			0x3f0
>> 
>> Reviewed-by: Radu Rendec <radu@rendec.net>
>
> 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
[PATCH v3] irqchip/stm32mp-exti: fix the unit of the hwspinlock timeout
Posted by Ju Nan 1 month, 1 week ago 
HWSPNLCK_TIMEOUT is passed to hwspin_lock_timeout_in_atomic(), whose
timeout argument is in milliseconds, not microseconds:

  atomic_delay += HWSPINLOCK_RETRY_DELAY_US;
  if (atomic_delay > to * 1000)
          return -ETIMEDOUT;

So stm32mp_exti_set_type() asks for a 1 second timeout where the comment
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 every
configuration, PREEMPT_RT included.

The hwspinlock core documents this explicitly:

  If the mode is HWLOCK_IN_ATOMIC (called from an atomic context) the
  timeout is handled with busy-waiting delays, hence shall not exceed
  few msecs.

Fixes: 5257169ade8c ("irqchip/stm32-exti: Use the hwspin_lock_timeout_in_atomic() API")
Reviewed-by: Radu Rendec <radu@rendec.net>
Signed-off-by: Ju Nan <junan76@163.com>
---
changelog:

v3: redefine the timeout macro with time unit on it
v2: add "Fixes" tag suggested by Radu Rendec
v1: https://lore.kernel.org/all/20260805032139.35420-2-junan76@163.com/
---
 drivers/irqchip/irq-stm32mp-exti.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/irqchip/irq-stm32mp-exti.c b/drivers/irqchip/irq-stm32mp-exti.c
index a24f4f1a4..ce4e03ee1 100644
--- a/drivers/irqchip/irq-stm32mp-exti.c
+++ b/drivers/irqchip/irq-stm32mp-exti.c
@@ -23,7 +23,7 @@
 
 #define IRQS_PER_BANK			32
 
-#define HWSPNLCK_TIMEOUT		1000 /* usec */
+#define HWSPNLCK_TIMEOUT_MS		1
 
 #define EXTI_EnCIDCFGR(n)		(0x180 + (n) * 4)
 #define EXTI_HWCFGR1			0x3f0
@@ -377,7 +377,7 @@ static int stm32mp_exti_set_type(struct irq_data *d, unsigned int type)
 	raw_spin_lock(&chip_data->rlock);
 
 	if (hwlock) {
-		err = hwspin_lock_timeout_in_atomic(hwlock, HWSPNLCK_TIMEOUT);
+		err = hwspin_lock_timeout_in_atomic(hwlock, HWSPNLCK_TIMEOUT_MS);
 		if (err) {
 			pr_err("%s can't get hwspinlock (%d)\n", __func__, err);
 			goto unlock;
-- 
2.55.0
Re: [Linux-stm32] [PATCH v3] irqchip/stm32mp-exti: fix the unit of the hwspinlock timeout
Posted by Antonio Borneo 1 month, 1 week ago 
On Fri, 2026-08-21 at 10:47 +0800, Ju Nan wrote:
> HWSPNLCK_TIMEOUT is passed to hwspin_lock_timeout_in_atomic(), whose
> timeout argument is in milliseconds, not microseconds:
> 
>   atomic_delay += HWSPINLOCK_RETRY_DELAY_US;
>   if (atomic_delay > to * 1000)
>           return -ETIMEDOUT;
> 
> So stm32mp_exti_set_type() asks for a 1 second timeout where the comment
> 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 every
> configuration, PREEMPT_RT included.
> 
> The hwspinlock core documents this explicitly:
> 
>   If the mode is HWLOCK_IN_ATOMIC (called from an atomic context) the
>   timeout is handled with busy-waiting delays, hence shall not exceed
>   few msecs.
> 
> Fixes: 5257169ade8c ("irqchip/stm32-exti: Use the hwspin_lock_timeout_in_atomic() API")
> Reviewed-by: Radu Rendec <radu@rendec.net>
> Signed-off-by: Ju Nan <junan76@163.com>
> ---
> changelog:
> 
> v3: redefine the timeout macro with time unit on it
> v2: add "Fixes" tag suggested by Radu Rendec
> v1: https://lore.kernel.org/all/20260805032139.35420-2-junan76@163.com/
> ---
>  drivers/irqchip/irq-stm32mp-exti.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/irqchip/irq-stm32mp-exti.c b/drivers/irqchip/irq-stm32mp-exti.c
> index a24f4f1a4..ce4e03ee1 100644
> --- a/drivers/irqchip/irq-stm32mp-exti.c
> +++ b/drivers/irqchip/irq-stm32mp-exti.c
> @@ -23,7 +23,7 @@
>  
>  #define IRQS_PER_BANK                  32
>  
> -#define HWSPNLCK_TIMEOUT               1000 /* usec */
> +#define HWSPNLCK_TIMEOUT_MS            1
>  
>  #define EXTI_EnCIDCFGR(n)              (0x180 + (n) * 4)
>  #define EXTI_HWCFGR1                   0x3f0
> @@ -377,7 +377,7 @@ static int stm32mp_exti_set_type(struct irq_data *d, unsigned int type)
>         raw_spin_lock(&chip_data->rlock);
>  
>         if (hwlock) {
> -               err = hwspin_lock_timeout_in_atomic(hwlock, HWSPNLCK_TIMEOUT);
> +               err = hwspin_lock_timeout_in_atomic(hwlock, HWSPNLCK_TIMEOUT_MS);
>                 if (err) {
>                         pr_err("%s can't get hwspinlock (%d)\n", __func__, err);
>                         goto unlock;

Thanks!

Reviewed-by: Antonio Borneo <antonio.borneo@foss.st.com>
[tip: irq/urgent] irqchip/stm32mp-exti: Fix the unit of the hwspinlock timeout
Posted by tip-bot2 for Ju Nan 3 weeks, 6 days ago 
The following commit has been merged into the irq/urgent branch of tip:

Commit-ID:     d31fbbade43f880b7e59e2b3a72722fe2725d93f
Gitweb:        https://git.kernel.org/tip/d31fbbade43f880b7e59e2b3a72722fe2725d93f
Author:        Ju Nan <junan76@163.com>
AuthorDate:    Fri, 21 Aug 2026 10:47:57 +08:00
Committer:     Thomas Gleixner <tglx@kernel.org>
CommitterDate: Fri, 04 Sep 2026 16:19:04 +02:00

irqchip/stm32mp-exti: Fix the unit of the hwspinlock timeout

HWSPNLCK_TIMEOUT is passed to hwspin_lock_timeout_in_atomic(), whose
timeout argument is in milliseconds, not microseconds:

  atomic_delay += HWSPINLOCK_RETRY_DELAY_US;
  if (atomic_delay > to * 1000)
          return -ETIMEDOUT;

So stm32mp_exti_set_type() asks for a 1 second timeout where the comment
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 every
configuration, PREEMPT_RT included.

The hwspinlock core documents this explicitly:

  If the mode is HWLOCK_IN_ATOMIC (called from an atomic context) the
  timeout is handled with busy-waiting delays, hence shall not exceed
  few msecs.

Fixes: 5257169ade8c ("irqchip/stm32-exti: Use the hwspin_lock_timeout_in_atomic() API")
Signed-off-by: Ju Nan <junan76@163.com>
Signed-off-by: Thomas Gleixner <tglx@kernel.org>
Reviewed-by: Radu Rendec <radu@rendec.net>
Reviewed-by: Antonio Borneo <antonio.borneo@foss.st.com>
Cc: stable@vger.kernel.org
Link: https://patch.msgid.link/20260821024756.24927-2-junan76@163.com
---
 drivers/irqchip/irq-stm32mp-exti.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/irqchip/irq-stm32mp-exti.c b/drivers/irqchip/irq-stm32mp-exti.c
index bf3a2de..a19e91f 100644
--- a/drivers/irqchip/irq-stm32mp-exti.c
+++ b/drivers/irqchip/irq-stm32mp-exti.c
@@ -22,7 +22,7 @@
 
 #define IRQS_PER_BANK			32
 
-#define HWSPNLCK_TIMEOUT		1000 /* usec */
+#define HWSPNLCK_TIMEOUT_MS		1
 
 #define EXTI_EnCIDCFGR(n)		(0x180 + (n) * 4)
 #define EXTI_HWCFGR1			0x3f0
@@ -376,7 +376,7 @@ static int stm32mp_exti_set_type(struct irq_data *d, unsigned int type)
 	raw_spin_lock(&chip_data->rlock);
 
 	if (hwlock) {
-		err = hwspin_lock_timeout_in_atomic(hwlock, HWSPNLCK_TIMEOUT);
+		err = hwspin_lock_timeout_in_atomic(hwlock, HWSPNLCK_TIMEOUT_MS);
 		if (err) {
 			pr_err("%s can't get hwspinlock (%d)\n", __func__, err);
 			goto unlock;
[PATCH v2] irqchip/stm32mp-exti: fix the unit of the hwspinlock timeout
Posted by Ju Nan 1 month, 2 weeks ago 
HWSPNLCK_TIMEOUT is passed to hwspin_lock_timeout_in_atomic(), whose
timeout argument is in milliseconds, not microseconds:

  atomic_delay += HWSPINLOCK_RETRY_DELAY_US;
  if (atomic_delay > to * 1000)
          return -ETIMEDOUT;

So stm32mp_exti_set_type() asks for a 1 second timeout where the comment
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 every
configuration, PREEMPT_RT included.

The hwspinlock core documents this explicitly:

  If the mode is HWLOCK_IN_ATOMIC (called from an atomic context) the
  timeout is handled with busy-waiting delays, hence shall not exceed
  few msecs.

Pass the value the comment always described. The core retries every
HWSPINLOCK_RETRY_DELAY_US (100 us), so the semaphore is still polled ten
times before giving up, which is far longer than any plausible hold time
on the coprocessor side. A timeout is reported with pr_err() and fails
the trigger type configuration, so shortening it degrades gracefully.

Fixes: 5257169ade8c ("irqchip/stm32-exti: Use the hwspin_lock_timeout_in_atomic() API")
Reviewed-by: Radu Rendec <radu@rendec.net>
Signed-off-by: Ju Nan <junan76@163.com>
---
changelog:

v2: Add Fixes tag suggested by Radu Rendec
v1: https://lore.kernel.org/all/20260805032139.35420-2-junan76@163.com/
---
 drivers/irqchip/irq-stm32mp-exti.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

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 @@
 
 #define IRQS_PER_BANK			32
 
-#define HWSPNLCK_TIMEOUT		1000 /* usec */
+#define HWSPNLCK_TIMEOUT		1 /* msec */
 
 #define EXTI_EnCIDCFGR(n)		(0x180 + (n) * 4)
 #define EXTI_HWCFGR1			0x3f0
-- 
2.55.0

Patchew 2.1-devel-ea86937

© 2016 - 2026 Red Hat, Inc.