Re: [PATCH v10 19/36] pinctrl: airoha: fix IRQ mask/unmask code

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

Pre-existing issues:
- [High] The driver incorrectly uses `handle_simple_irq` for all interrupts, leading to interrupt storms for level-triggered interrupts. Additionally, the interrupt handler clears the interrupt status register after executing the specific IRQ handler, creating a race condition where edge-triggered interrupts can be lost. Finally, the irq_chip lacks a proper `.irq_ack` callback.
- [High] The IRQ callbacks and chained handler invoke regmap operations on a syscon regmap that uses a sleepable `spinlock_t`. On PREEMPT_RT, this causes an `rt_mutex` to be acquired in hardirq context and while holding a `raw_spinlock_t`, leading to a Wait-Type Nesting violation and a 'sleeping in atomic context' BUG.
--

--- Patch [19]: [PATCH v10 19/36] pinctrl: airoha: fix IRQ mask/unmask code ---
commit 97f2ae6176e0b5295c8894fb1446d64bc5196ae8
Author: Mikhail Kshevetskiy <[email protected]>
Subject: pinctrl: airoha: fix IRQ mask/unmask code

This commit updates the IRQ mask and unmask functions to correctly call
gpiochip_disable_irq() and gpiochip_enable_irq() when using
IRQCHIP_IMMUTABLE. This ensures gpiolib sets the GPIOD_FLAG_IRQ_IS_ENABLED
bit, allowing gpiod_direction_output() to realize when a pin is actively
used as an interrupt.

> diff --git a/drivers/pinctrl/airoha/pinctrl-airoha.c b/drivers/pinctrl/airoha/pinctrl-airoha.c
> index f3078f9b14a76..4859fcebac7de 100644
> --- a/drivers/pinctrl/airoha/pinctrl-airoha.c
> +++ b/drivers/pinctrl/airoha/pinctrl-airoha.c
> @@ -2595,6 +2595,7 @@ static void airoha_irq_unmask(struct irq_data *data)
>  	if (WARN_ON_ONCE(data->hwirq >= ARRAY_SIZE(gpiochip->irq_type)))
>  		return;
>  
> +	gpiochip_enable_irq(gc, irqd_to_hwirq(data));

[Severity: High]
This is a pre-existing issue, but does airoha_irq_unmask() safely interact
with the regmap?

The core IRQ subsystem calls airoha_irq_unmask() while holding the raw
irq_desc spinlock with interrupts disabled. However, this function invokes
regmap operations (e.g., regmap_update_bits()) on a syscon regmap that uses
a sleepable spinlock_t (which becomes an rt_mutex on PREEMPT_RT).

Can this cause a 'sleeping in atomic context' BUG and Wait-Type Nesting
violation on PREEMPT_RT kernels?

>  	switch (gpiochip->irq_type[data->hwirq]) {
>  	case IRQ_TYPE_LEVEL_LOW:
>  		val = val << 1;

[ ... ]

> @@ -2629,6 +2630,7 @@ static void airoha_irq_mask(struct irq_data *data)
>  
>  	regmap_clear_bits(pinctrl->regmap, gpiochip->level[index], mask);
>  	regmap_clear_bits(pinctrl->regmap, gpiochip->edge[index], mask);
> +	gpiochip_disable_irq(gc, irqd_to_hwirq(data));
>  }

[Severity: High]
This isn't a bug introduced by this patch, but does the IRQ configuration
in airoha_pinctrl_add_gpiochip() handle level and edge interrupts correctly?

The driver unconditionally uses handle_simple_irq for all interrupts:

drivers/pinctrl/airoha/pinctrl-airoha.c:airoha_pinctrl_add_gpiochip() {
    ...
	girq->handler = handle_simple_irq;
    ...
}

Because handle_simple_irq fails to mask the interrupt while it is being
processed, won't this lead to an infinite interrupt storm for
level-triggered interrupts?

Additionally, does airoha_irq_handler() drop edge interrupts?

drivers/pinctrl/airoha/pinctrl-airoha.c:airoha_irq_handler() {
    ...
		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));
			regmap_write(pinctrl->regmap,
				     pinctrl->gpiochip.status[i], BIT(irq));
		}
    ...
}

The interrupt status register is cleared after generic_handle_irq() returns.
If a second edge arrives during generic_handle_irq(), won't this clear
operation drop it permanently?

Also, airoha_irq_handler() acquires the same sleepable syscon regmap lock
in hardirq context when calling regmap_read() and regmap_write(). Will this
also cause a 'sleeping in atomic context' BUG on PREEMPT_RT?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=19