Re: [PATCH v8 18/35] 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] Redundant manual clearing of the interrupt status in the chained handler drops newly latched edge interrupts.

Pre-existing issues:
- [High] Acquiring a `spinlock_t`-protected regmap from within an `irq_chip` callback while a `raw_spinlock_t` is held.
--

--- Patch [18]: [PATCH v8 18/35] pinctrl: airoha: fix edge-triggered interrupts handling ---
commit 33dfa7f80fbe6a5d6d2b4b58cb7fc7442d5bbe21
Author: Mikhail Kshevetskiy <mikhail.kshevetskiy@iopsys.eu>

pinctrl: airoha: fix edge-triggered interrupts handling

This patch adds an irq_ack callback and sets the correct IRQ handlers for
edge and level triggered interrupts.

> diff --git a/drivers/pinctrl/airoha/pinctrl-airoha.c b/drivers/pinctrl/airoha/pinctrl-airoha.c
> index f11ec77f6e338..42714b8beb3cc 100644
> --- a/drivers/pinctrl/airoha/pinctrl-airoha.c
> +++ b/drivers/pinctrl/airoha/pinctrl-airoha.c

[ ... ]

> @@ -2608,11 +2608,28 @@ static void airoha_irq_mask(struct irq_data *data)

[ ... ]

> +static void airoha_irq_ack(struct irq_data *data)
> +{
> +	struct gpio_chip *gc = irq_data_get_irq_chip_data(data);
> +	struct airoha_pinctrl *pinctrl = gpiochip_get_data(gc);
> +	struct airoha_pinctrl_gpiochip *gpiochip = &pinctrl->gpiochip;
> +	u8 offset = data->hwirq % AIROHA_PIN_BANK_SIZE;
> +	u8 index = data->hwirq / AIROHA_PIN_BANK_SIZE;
> +
> +	if (data->hwirq >= ARRAY_SIZE(gpiochip->irq_type))
> +		return;
> +
> +	regmap_write(pinctrl->regmap, gpiochip->status[index], BIT(offset));

[Severity: High]
This is a pre-existing issue, but does calling regmap_write() here cause a
lock nesting violation?

The core IRQ subsystem calls airoha_irq_ack() while holding the IRQ
descriptor's raw spinlock. If this syscon regmap internally acquires a
standard spinlock, it will cause a sleeping in atomic context lockdep splat
or a kernel panic on PREEMPT_RT kernels, since standard spinlocks map to a
sleepable rt_mutex.

> +}
> +

[ ... ]

> @@ -2653,8 +2682,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]
Can this redundant status clear drop newly latched edge interrupts?

Since the core IRQ subsystem now correctly acknowledges edge interrupts early
via the newly added airoha_irq_ack() callback, if a new edge interrupt
arrives while the device driver's airoha_irq_handler() is running, the
hardware latches the new status.

Clearing the status bit again here after generic_handle_domain_irq() returns
erases the newly latched interrupt without it ever being serviced.

>  		}

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