[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