[PATCH v4 0/2] pinctrl: stm32: fix hwspinlock timeout and EXTI mux locking

[PATCH v4 0/2] pinctrl: stm32: fix hwspinlock timeout and EXTI mux locking

[Ju Nan]

This series fixes two problems around the hwspinlock-protected EXTI mux
and pin configuration accesses in the STM32 pinctrl driver.

Patch 1 fixes the timeout passed to hwspin_lock_timeout_in_atomic().
The value was written as 1000 microseconds, but the argument is in
milliseconds, so the driver could busy-wait for up to one second with
interrupts disabled instead of the intended one millisecond. It is a
standalone fix and can be applied on its own.

Patch 2 moves the EXTI mux programming from the irq domain .activate
callback to .alloc. .activate runs under the raw desc->lock, while the
mux is written through a syscon regmap protected by a spinlock_t, which
lockdep reports as an invalid wait context and which is a real bug on
PREEMPT_RT. This was originally reported by Uwe Kleine-König in 2022.
.alloc runs in sleepable context and already reserves the mux line, so
the write now happens in the same irqmux_lock section as the
reservation. While at it, the reservation is released when the
hwspinlock cannot be taken.

---
Changes in v4:

- make the two patches a series to resolve the dependency issue
- reproduce the bug fixed by patch 2 on a modified version of
  STM32MP157-DK board with kernel version 7.1.13
- check the concurrent-requester case raised by Antonio in v3: the
  second bank is rejected in .alloc under irqmux_lock (details in
  patch 2)

v3: https://lore.kernel.org/all/20260804141402.86911-2-junan76@163.com/
v2: https://lore.kernel.org/all/20260804032227.35017-3-junan76@163.com/
v1: https://lore.kernel.org/all/20260803061718.43210-1-junan76@163.com/

Ju Nan (2):
  pinctrl: stm32: Fix the unit of the hwspinlock timeout
  pinctrl: stm32: program the EXTI mux from .alloc instead of .activate

 drivers/pinctrl/stm32/pinctrl-stm32.c | 60 ++++++++++++---------------
 1 file changed, 26 insertions(+), 34 deletions(-)

-- 
2.56.0

[PATCH v4 1/2] pinctrl: stm32: Fix the unit of the hwspinlock timeout

[Ju Nan]

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;

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: 290a9f937e5a ("pinctrl: stm32: use the hwspin_lock_timeout_in_atomic() API")
Signed-off-by: Ju Nan <junan76@163.com>
---
 drivers/pinctrl/stm32/pinctrl-stm32.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/pinctrl/stm32/pinctrl-stm32.c b/drivers/pinctrl/stm32/pinctrl-stm32.c
index 6a99708a5a23..e71651d4fad4 100644
--- a/drivers/pinctrl/stm32/pinctrl-stm32.c
+++ b/drivers/pinctrl/stm32/pinctrl-stm32.c
@@ -87,7 +87,7 @@
 #define gpio_range_to_bank(chip) \
 		container_of(chip, struct stm32_gpio_bank, range)
 
-#define HWSPNLCK_TIMEOUT	1000 /* usec */
+#define HWSPNLCK_TIMEOUT_MS	1
 
 static const char * const stm32_gpio_functions[] = {
 	"gpio", "af0", "af1",
@@ -609,7 +609,7 @@ static int stm32_gpio_domain_activate(struct irq_domain *d,
 
 	if (pctl->hwlock) {
 		ret = hwspin_lock_timeout_in_atomic(pctl->hwlock,
-						    HWSPNLCK_TIMEOUT);
+						    HWSPNLCK_TIMEOUT_MS);
 		if (ret) {
 			dev_err(pctl->dev, "Can't get hwspinlock\n");
 			return ret;
@@ -946,7 +946,7 @@ static int stm32_pmx_set_mode(struct stm32_gpio_bank *bank,
 
 	if (pctl->hwlock) {
 		err = hwspin_lock_timeout_in_atomic(pctl->hwlock,
-						    HWSPNLCK_TIMEOUT);
+						    HWSPNLCK_TIMEOUT_MS);
 		if (err) {
 			dev_err(pctl->dev, "Can't get hwspinlock\n");
 			goto unlock;
@@ -1089,7 +1089,7 @@ static int stm32_pconf_set_driving(struct stm32_gpio_bank *bank,
 
 	if (pctl->hwlock) {
 		err = hwspin_lock_timeout_in_atomic(pctl->hwlock,
-						    HWSPNLCK_TIMEOUT);
+						    HWSPNLCK_TIMEOUT_MS);
 		if (err) {
 			dev_err(pctl->dev, "Can't get hwspinlock\n");
 			goto unlock;
@@ -1135,7 +1135,7 @@ static int stm32_pconf_set_speed(struct stm32_gpio_bank *bank,
 
 	if (pctl->hwlock) {
 		err = hwspin_lock_timeout_in_atomic(pctl->hwlock,
-						    HWSPNLCK_TIMEOUT);
+						    HWSPNLCK_TIMEOUT_MS);
 		if (err) {
 			dev_err(pctl->dev, "Can't get hwspinlock\n");
 			goto unlock;
@@ -1181,7 +1181,7 @@ static int stm32_pconf_set_bias(struct stm32_gpio_bank *bank,
 
 	if (pctl->hwlock) {
 		err = hwspin_lock_timeout_in_atomic(pctl->hwlock,
-						    HWSPNLCK_TIMEOUT);
+						    HWSPNLCK_TIMEOUT_MS);
 		if (err) {
 			dev_err(pctl->dev, "Can't get hwspinlock\n");
 			goto unlock;
@@ -1242,7 +1242,7 @@ static int stm32_pconf_set_advcfgr(struct stm32_gpio_bank *bank, int offset, u32
 	spin_lock_irqsave(&bank->lock, flags);
 
 	if (pctl->hwlock) {
-		err = hwspin_lock_timeout_in_atomic(pctl->hwlock, HWSPNLCK_TIMEOUT);
+		err = hwspin_lock_timeout_in_atomic(pctl->hwlock, HWSPNLCK_TIMEOUT_MS);
 		if (err) {
 			dev_err(pctl->dev, "Can't get hwspinlock\n");
 			goto unlock;
@@ -1320,7 +1320,7 @@ stm32_pconf_set_skew_delay(struct stm32_gpio_bank *bank, int offset, u32 delay,
 	spin_lock_irqsave(&bank->lock, flags);
 
 	if (pctl->hwlock) {
-		err = hwspin_lock_timeout_in_atomic(pctl->hwlock, HWSPNLCK_TIMEOUT);
+		err = hwspin_lock_timeout_in_atomic(pctl->hwlock, HWSPNLCK_TIMEOUT_MS);
 		if (err) {
 			dev_err(pctl->dev, "Can't get hwspinlock\n");
 			goto unlock;
-- 
2.56.0

Re: [PATCH v4 1/2] pinctrl: stm32: Fix the unit of the hwspinlock timeout

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] pinctrl: stm32: same-CPU starvation with HWLOCK_IN_ATOMIC on PREEMPT_RT
--

commit 635cf6ace8c829d5e8fd3222bc79311f9ac07fc2
Author: Ju Nan <junan76@163.com>

pinctrl: stm32: Fix the unit of the hwspinlock timeout

This commit updates the hardware spinlock timeout parameter passed to
hwspin_lock_timeout_in_atomic(). It modifies the timeout constant name and
changes its value from microseconds to milliseconds across the stm32 pinctrl
driver to match the expected API units.

> diff --git a/drivers/pinctrl/stm32/pinctrl-stm32.c b/drivers/pinctrl/stm32/pinctrl-stm32.c
> --- a/drivers/pinctrl/stm32/pinctrl-stm32.c
> +++ b/drivers/pinctrl/stm32/pinctrl-stm32.c
[ ... ]
> @@ -946,7 +946,7 @@ static int stm32_pmx_set_mode(struct stm32_gpio_bank *bank,
>  
>  	if (pctl->hwlock) {
>  		err = hwspin_lock_timeout_in_atomic(pctl->hwlock,
> -						    HWSPNLCK_TIMEOUT);
> +						    HWSPNLCK_TIMEOUT_MS);

[Severity: High]
Does calling hwspin_lock_timeout_in_atomic() here cause a same-CPU starvation
issue on PREEMPT_RT?

Because hwspin_lock_timeout_in_atomic() uses the HWLOCK_IN_ATOMIC mode,
it implements the timeout by busy-waiting via udelay() and assumes preemption
is disabled by the caller.

However, on PREEMPT_RT, spin_lock_irqsave(&bank->lock) acquires an rt_mutex
which leaves preemption enabled.

If thread A acquires pctl->hwlock and is preempted, and thread B on the
same CPU subsequently acquires a different bank's lock and attempts to acquire
this shared pctl->hwlock, thread B will busy-wait.

Since thread B loops in a busy-wait for the entire 1ms timeout, thread A is
starved and cannot run to release the hardware lock, resulting in a guaranteed
spurious -ETIMEDOUT failure for thread B.

>  		if (err) {
>  			dev_err(pctl->dev, "Can't get hwspinlock\n");
>  			goto unlock;
[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261014160917.139241-2-junan76@163.com?part=1

[PATCH v4 2/2] pinctrl: stm32: program the EXTI mux from .alloc instead of .activate

[Ju Nan]

stm32_gpio_domain_activate() writes the EXTI interrupt multiplexer
through a regmap obtained from the generic syscon driver. The irq core
calls .activate from __setup_irq() with the raw desc->lock held, so this
happens in a raw atomic section. regmap-mmio sets fast_io, and syscon
does not ask for a raw spinlock, so the regmap is protected by a
spinlock_t. On PREEMPT_RT that is a sleeping lock, which must not be
taken there. lockdep reports it as soon as a GPIO interrupt is
requested:

 [ BUG: Invalid wait context ]
 7.1.13 #7 Not tainted
 -----------------------------
 kworker/u8:1/29 is trying to lock:
 (syscon:117:(&syscon_config)->lock){....}-{3:3}, at: regmap_lock_spinlock
 other info that might help us debug this:
 context-{5:5}
 5 locks held by kworker/u8:1/29:
  #0: ((wq_completion)async){+.+.}-{0:0}, at: process_one_work
  #1: ((work_completion)(&entry->work)){+.+.}-{0:0}, at: process_one_work
  #2: (&dev->mutex){....}-{4:4}, at: __device_attach_async_helper
  #3: (&desc->request_mutex){+.+.}-{4:4}, at: __setup_irq
  #4: (&irq_desc_lock_class){-...}-{2:2}, at: __setup_irq

The driver was already aware of running in atomic context there: it uses
the _in_atomic() hwspinlock primitives around the very same access. The
syscon lock is the one lock in that section it does not control.

Program the mux from .alloc instead, which runs in a sleepable context.
That callback already owns this resource: it reserves the mux line in
pctl->irqmux_map under irqmux_lock, so writing the value it just claimed
is a natural fit, and the value only depends on the bank.

Nothing requires the mux to be reprogrammed at interrupt startup time:
the domain has no .deactivate, .free() only releases the irqmux_map bit
without touching the registers.

Keep the access inside the existing irqmux_lock section rather than
after it. hwspin_lock_timeout_in_atomic() deliberately skips the local
lock that the other hwspinlock modes take, so its caller is responsible
for disabling preemption and for excluding other local contexts; every
other hwspinlock user in this driver relies on bank->lock for that.
irqmux_lock plays the same role here, and it also turns the mux
reservation and the mux write into a single critical section. Nesting
the regmap spinlock_t inside irqmux_lock, itself a spinlock_t, is what
the wait-context checker expects.

Note that this bounds the semaphore hold time on !PREEMPT_RT only. On
PREEMPT_RT spinlock_t disables neither interrupts nor preemption, so the
hardware semaphore can be held across a preemptible section and a
coprocessor contending for it may poll for an unbounded time. That is
not introduced here: bank->lock is a spinlock_t too, so every existing
hwspinlock user in this driver behaves the same way on PREEMPT_RT.
Removing that exposure requires the EXTI mux regmap to be raw-spinlock
based, which the syscon core does not currently offer, and is left for a
separate change.

While moving the code, release the mux reservation when the hwspinlock
cannot be taken, which the .activate callback had no way of doing.

Reported-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
Closes: https://lore.kernel.org/all/20220202174430.pf37tt6lua2op3gc@pengutronix.de/
Signed-off-by: Ju Nan <junan76@163.com>
---
The issue mentioned by Antonio in v3 cannot happen: irqmux_lock is held
while irqmux_map is checked, so PB5 cannot write the mux once PA5 has
claimed the line. I tested it on the same STM32 board:

1. add a fake device in dts:

&i2c5{

hdmi-transmitter@39 {
	compatible = "sil,sii9022";
...
	interrupts = <14 IRQ_TYPE_EDGE_FALLING>;
	interrupt-parent = <&gpioa>;
...
};

fake-hdmi-transmitter@40 {
	compatible = "sil,sii9022";
...
	interrupts = <14 IRQ_TYPE_EDGE_FALLING>;
	interrupt-parent = <&gpiob>;
...
};
};

2. hack the sii902x driver so that the second and later instances only
   call request_irq() and return 0 from .probe.

3. dmesg result:

# dmesg | grep -i error
[    2.107147] stm32_rtc 5c004000.rtc: probe with driver stm32_rtc failed with error -110
[    5.439201] GPT: Use GNU Parted to correct GPT errors.
[    9.333158] sii902x 0-0040: error -EINVAL: request_irq(0) 0x0 sii902x_interrupt [sii902x] 0-0040
# dmesg | grep -i "irq line"
[    9.324176] stm32mp157-pinctrl soc:pinctrl@50002000: irq line 14 already requested.
# uname -a
Linux fsmp1a 7.1.13 #7 SMP PREEMPT Wed Oct 14 19:26:21 CST 2026 armv7l GNU/Linux

The second instance (0-0040) fails to map its interrupt: .alloc refuses
the line with "irq line 14 already requested", so the mux is never
written for GPIOB. client->irq is then left at 0, and the hacked probe
fails with "-EINVAL: request_irq(0)".

---
 drivers/pinctrl/stm32/pinctrl-stm32.c | 46 +++++++++++----------------
 1 file changed, 19 insertions(+), 27 deletions(-)

diff --git a/drivers/pinctrl/stm32/pinctrl-stm32.c b/drivers/pinctrl/stm32/pinctrl-stm32.c
index e71651d4fad4..b5f9091bd990 100644
--- a/drivers/pinctrl/stm32/pinctrl-stm32.c
+++ b/drivers/pinctrl/stm32/pinctrl-stm32.c
@@ -600,30 +600,6 @@ static int stm32_gpio_domain_translate(struct irq_domain *d,
 	return 0;
 }
 
-static int stm32_gpio_domain_activate(struct irq_domain *d,
-				      struct irq_data *irq_data, bool reserve)
-{
-	struct stm32_gpio_bank *bank = d->host_data;
-	struct stm32_pinctrl *pctl = dev_get_drvdata(bank->gpio_chip.parent);
-	int ret = 0;
-
-	if (pctl->hwlock) {
-		ret = hwspin_lock_timeout_in_atomic(pctl->hwlock,
-						    HWSPNLCK_TIMEOUT_MS);
-		if (ret) {
-			dev_err(pctl->dev, "Can't get hwspinlock\n");
-			return ret;
-		}
-	}
-
-	regmap_field_write(pctl->irqmux[irq_data->hwirq], bank->bank_ioport_nr);
-
-	if (pctl->hwlock)
-		hwspin_unlock_in_atomic(pctl->hwlock);
-
-	return ret;
-}
-
 static int stm32_gpio_domain_alloc(struct irq_domain *d,
 				   unsigned int virq,
 				   unsigned int nr_irqs, void *data)
@@ -645,10 +621,27 @@ static int stm32_gpio_domain_alloc(struct irq_domain *d,
 	if (pctl->irqmux_map & BIT(hwirq)) {
 		dev_err(pctl->dev, "irq line %ld already requested.\n", hwirq);
 		ret = -EBUSY;
-	} else {
-		pctl->irqmux_map |= BIT(hwirq);
+		goto unlock;
+	}
+
+	pctl->irqmux_map |= BIT(hwirq);
+
+	if (pctl->hwlock) {
+		ret = hwspin_lock_timeout_in_atomic(pctl->hwlock,
+						    HWSPNLCK_TIMEOUT_MS);
+		if (ret) {
+			dev_err(pctl->dev, "Can't get hwspinlock\n");
+			pctl->irqmux_map &= ~BIT(hwirq);
+			goto unlock;
+		}
 	}
 
+	regmap_field_write(pctl->irqmux[hwirq], bank->bank_ioport_nr);
+
+	if (pctl->hwlock)
+		hwspin_unlock_in_atomic(pctl->hwlock);
+
+unlock:
 	spin_unlock_irqrestore(&pctl->irqmux_lock, flags);
 	if (ret)
 		return ret;
@@ -683,7 +676,6 @@ static const struct irq_domain_ops stm32_gpio_domain_ops = {
 	.translate	= stm32_gpio_domain_translate,
 	.alloc		= stm32_gpio_domain_alloc,
 	.free		= stm32_gpio_domain_free,
-	.activate	= stm32_gpio_domain_activate,
 };
 
 /* Pinctrl functions */
-- 
2.56.0

Re: [PATCH v4 0/2] pinctrl: stm32: fix hwspinlock timeout and EXTI mux locking

[Linus Walleij]

Hi Ju,

On Wed, Oct 14, 2026 at 6:10 PM Ju Nan <junan76@163.com> wrote:

> This series fixes two problems around the hwspinlock-protected EXTI mux
> and pin configuration accesses in the STM32 pinctrl driver.

Patches applied for v7.4.

There are still complaints from Sashiko, but Sashiko always
complains so I'd say at this point we are close to the merge
window so it is better to follow up with separate fixes
if there are more issues.

As maintainer I kind of have to decide whether to wait for
the perfect patch or if perfect is the enemy of good and
just merge the patch so the baseline looks better, so I
decided for the latter here.

Yours,
Linus Walleij

Re: [PATCH v4 0/2] pinctrl: stm32: fix hwspinlock timeout and EXTI mux locking

[Ju Nan]

On 10/16/26 9:19 PM, Linus Walleij wrote:
> Hi Ju,
> 
> On Wed, Oct 14, 2026 at 6:10 PM Ju Nan <junan76@163.com> wrote:
> 
>> This series fixes two problems around the hwspinlock-protected EXTI mux
>> and pin configuration accesses in the STM32 pinctrl driver.
> 
> Patches applied for v7.4.
> 
> There are still complaints from Sashiko, but Sashiko always
> complains so I'd say at this point we are close to the merge
> window so it is better to follow up with separate fixes
> if there are more issues.
> 

Thank you for applying the series.

Many thanks also to Antonio and Sebastian for taking the time to
review the patches; your feedback helped improve the series a lot.

> As maintainer I kind of have to decide whether to wait for
> the perfect patch or if perfect is the enemy of good and
> just merge the patch so the baseline looks better, so I
> decided for the latter here.
> 
> Yours,
> Linus Walleij

I've looked through the remaining Sashiko comments. I'll keep an
eye on it and send fixes if anything comes up.

Yours,
Ju Nan