Re: [PATCH v7 17/34] pinctrl: airoha: fix edge-triggered interrupts handling

[sashiko-bot@kernel.org]

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

New issues:
- [High] Leftover manual status clear in `airoha_irq_handler` double-acknowledges edge-triggered interrupts, causing newly latched edge events to be permanently lost.

Pre-existing issues:
- [High] The irq_chip callbacks acquire a non-raw spinlock (`spinlock_t`) inside a raw spinlock critical section, causing a lockdep violation and a panic on PREEMPT_RT kernels.
--

--- Patch [17]: [PATCH v7 17/34] pinctrl: airoha: fix edge-triggered interrupts handling ---
commit 2624d620bd965f151504e589dca531b9b80d52fe
Author: Mikhail Kshevetskiy <mikhail.kshevetskiy@iopsys.eu>

pinctrl: airoha: fix edge-triggered interrupts handling

This patch aims to fix edge-triggered interrupts handling for the Airoha
EN7581 SoC. It defines an irq_ack handler, switches to using the appropriate
handle_level_irq or handle_edge_irq handler based on the interrupt type,
and assigns the handler type.

> diff --git a/drivers/pinctrl/airoha/pinctrl-airoha.c b/drivers/pinctrl/airoha/pinctrl-airoha.c
> index c886ecc2c39e9..0a865cf0c2728 100644
> --- a/drivers/pinctrl/airoha/pinctrl-airoha.c
> +++ b/drivers/pinctrl/airoha/pinctrl-airoha.c
> @@ -2613,11 +2613,28 @@ static void airoha_irq_mask(struct irq_data *data)
>  	u8 index = data->hwirq / AIROHA_REG_GPIOCTRL_NUM_PIN;
>  	u32 mask = GENMASK(2 * offset + 1, 2 * offset);
>  
> +	if (data->hwirq >= ARRAY_SIZE(gpiochip->irq_type))
> +		return;
> +
>  	regmap_clear_bits(pinctrl->regmap, gpiochip->level[index], mask);
>  	regmap_clear_bits(pinctrl->regmap, gpiochip->edge[index], mask);

[Severity: High]
This is a pre-existing issue, but does calling regmap operations here cause
a lock nesting violation on PREEMPT_RT kernels?

The core IRQ flow handlers call irq_chip callbacks like irq_mask and irq_ack
while holding the IRQ descriptor lock, which is a raw_spinlock_t. Because
the driver initializes this regmap via device_node_to_regmap, it defaults to
using a standard spinlock_t.

On PREEMPT_RT, spinlock_t becomes a sleeping rt_mutex. Acquiring a sleeping
lock while holding a raw_spinlock_t causes a kernel panic. Since this patch
adds airoha_irq_ack which also uses regmap_write, it introduces another path
for this same lockdep violation.

>  	gpiochip_disable_irq(gc, irqd_to_hwirq(data));
>  }

[ ... ]

> @@ -2658,8 +2687,7 @@ static irqreturn_t airoha_irq_handler(int irq, void *data)
>  		for_each_set_bit(irq, &status, AIROHA_PIN_BANK_SIZE) {
>  			u32 offset = irq + i * AIROHA_PIN_BANK_SIZE;
>  
> -			generic_handle_irq(irq_find_mapping(girq->domain,
> -							    offset));
> +			generic_handle_domain_irq(girq->domain, offset);
>  			regmap_write(pinctrl->regmap,
>  				     pinctrl->gpiochip.status[i], BIT(irq));

[Severity: High]
Are edge interrupts permanently lost due to this trailing regmap_write?

With this patch properly configuring handle_edge_irq for edge interrupts,
the core IRQ code calls the newly added airoha_irq_ack function before
invoking the device driver's action handler. This early ack correctly clears
the hardware status bit so new edges can be latched.

However, when the driver's action handler completes and the call to
generic_handle_domain_irq returns, this manual regmap_write unconditionally
clears the status bit a second time. If a new edge arrives and is latched 
while the ISR is executing, wouldn't this trailing write wipe out the newly
latched state in the hardware status register?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260727074234.3761170-1-mikhail.kshevetskiy@iopsys.eu?part=17