Re: [PATCH v3] pinctrl: stm32: program the EXTI mux from .alloc instead of .activate

Thank you for your contribution! Sashiko AI review found 3 potential issue(=
s) to consider:

Pre-existing issues:
- [High] Missing hardware spinlock serialization when restoring `irqmux` in=
 the resume path.
- [Medium] Missing cleanup of `irqmux_map` reservation and hardware mux sta=
te when parent IRQ domain allocation fails.
- [Medium] HWSPNLCK_TIMEOUT is defined as microseconds but passed to a func=
tion expecting milliseconds.
--

--- Patch [1]: [PATCH v3] pinctrl: stm32: program the EXTI mux from .alloc =
instead of .activate ---
(Note: The format of this report has been altered to provide a free-form su=
mmary
without quoting the original patch, due to recitation restrictions.)

commit a303f103b81b7e7b2ea355f3dc199454d3911bbc
Author: Ju Nan <[email protected]>

pinctrl: stm32: program the EXTI mux from .alloc instead of .activate

This commit moves the programming of the EXTI interrupt multiplexer from the
.activate callback to the sleepable .alloc callback. This prevents a wait
context violation on PREEMPT_RT caused by a spinlock-protected regmap access
in the syscon driver. It also ensures the EXTI mux write is serialized with=
in
the existing irqmux_lock section.

Closes: https://lore.kernel.org/all/20220202174430.pf37tt6lua2op3gc@pengutr=
onix.de/
v2: https://lore.kernel.org/all/[email protected]/
v1: https://lore.kernel.org/all/[email protected]/

[Severity: Medium]
This is a pre-existing issue, but in stm32_gpio_domain_alloc(), if
irq_domain_alloc_irqs_parent() fails, the function directly returns the err=
or
without reverting the irqmux_map reservation or the hardware state.

Because the core IRQ subsystem does not invoke the .free callback for a
domain that fails mid-allocation, doesn't this result in the irqmux_map bit
remaining permanently set and the hardware EXTI mux remaining incorrectly
programmed for an abandoned allocation?

[Severity: Medium]
This is also a pre-existing issue, but when hwspin_lock_timeout_in_atomic()=
 is
called in stm32_gpio_domain_alloc(), HWSPNLCK_TIMEOUT is passed as the time=
out
argument.

Since HWSPNLCK_TIMEOUT is defined as 1000 microseconds, but the timeout
parameter expects milliseconds (using msecs_to_jiffies), doesn't this result
in a 1-second timeout instead of a 1-millisecond timeout?

Spinning for 1 second in an atomic context could potentially trigger lockup
detectors if the lock is heavily contended.

[Severity: High]
This is another pre-existing issue, but in stm32_pinctrl_restore_gpio_regs(=
),
the irqmux is reprogrammed using regmap_field_write() without holding the
hardware spinlock.

Since stm32_gpio_domain_alloc() correctly serializes access to the shared
EXTI register against the coprocessor by acquiring pctl->hwlock, doesn't the
unprotected write in the resume path risk a data race and potential hardware
register corruption if the coprocessor contends for it concurrently?

--=20
Sashiko AI review =C2=B7 https://sashiko.dev/#/patchset/20260804141402.8691=
[email protected]?part=3D1