Re: [PATCH v8 16/35] pinctrl: airoha: fix IRQ mask/unmask code

[sashiko-bot@kernel.org]

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

New issues:
- [High] The newly added `gpiochip_enable_irq()` and `gpiochip_disable_irq()` calls will continuously trigger kernel warnings and fail to set their flags because the driver lacks `GPIOCHIP_IRQ_RESOURCE_HELPERS`.

Pre-existing issues:
- [High] Lock nesting violation in IRQ callbacks causing a 'sleeping in atomic context' bug on PREEMPT_RT.
- [High] Race condition leading to permanently lost edge-triggered interrupts.
--

--- Patch [16]: [PATCH v8 16/35] pinctrl: airoha: fix IRQ mask/unmask code ---
commit ccf67b0bce7e44b6db4c8d2e60b3cba3f820c9f4
Author: Mikhail Kshevetskiy <mikhail.kshevetskiy@iopsys.eu>

pinctrl: airoha: fix IRQ mask/unmask code

When using IRQCHIP_IMMUTABLE, airoha_irq_unmask() must manually call
gpiochip_enable_irq() and airoha_irq_mask() must call
gpiochip_disable_irq(). Without these calls, gpiolib never sets the
GPIOD_FLAG_IRQ_IS_ENABLED bit. Because this bit is missing,
gpiod_direction_output() will not realize the pin is actively used
as an interrupt.

> diff --git a/drivers/pinctrl/airoha/pinctrl-airoha.c b/drivers/pinctrl/airoha/pinctrl-airoha.c
> index 3feddec2e29e3..6561f288dbf05 100644
> --- a/drivers/pinctrl/airoha/pinctrl-airoha.c
> +++ b/drivers/pinctrl/airoha/pinctrl-airoha.c
> @@ -2575,6 +2575,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]
Will this cause continuous warning spam on every unmask operation?

The gpiochip_enable_irq() function enforces a WARN_ON check to verify
that GPIOD_FLAG_USED_AS_IRQ is set. Because airoha_gpio_irq_chip specifies
IRQCHIP_IMMUTABLE but omits GPIOCHIP_IRQ_RESOURCE_HELPERS, the
gpiochip_irq_reqres() setup is never invoked. As a result, the required flag
is missing and the flag updates bypass being set entirely.

[ ... ]

> @@ -2609,6 +2610,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);

[Severity: High]
This is a pre-existing issue, but could calling regmap_clear_bits() here
cause a lock nesting violation on PREEMPT_RT systems?

The core IRQ subsystem invokes airoha_irq_mask() with the raw
irq_desc->lock held. The regmap operations use a standard spinlock,
which becomes a sleepable rt_mutex on PREEMPT_RT, leading to a sleep in
atomic context.

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

[ ... ]

[Severity: High]
This is also a pre-existing issue, but is there a race condition in the
interrupt handler that could cause permanently lost edge-triggered interrupts?

Looking at airoha_irq_handler() in the same file:

drivers/pinctrl/airoha/pinctrl-airoha.c:airoha_irq_handler() {
	...
	generic_handle_irq(irq_find_mapping(girq->domain, offset));
	regmap_write(pinctrl->regmap, pinctrl->gpiochip.status[i], BIT(irq));
	...
}

If a new edge interrupt arrives while generic_handle_irq() is executing,
it is latched by the hardware. The subsequent Write-1-to-Clear via
regmap_write() would then inadvertently clear this newly latched edge
without servicing it. Should the status be cleared before invoking the
handler?

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