Re: [PATCH v10 18/36] pinctrl: airoha: add missed IRQ resource helpers

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

New issues:
- [High] The immutable irqchip lacks required calls to `gpiochip_enable_irq()` and `gpiochip_disable_irq()`, breaking pin direction locking.

Pre-existing issues:
- [High] The custom IRQ handler acknowledges interrupts after executing generic_handle_irq(), which can cause lost edge interrupts.
--

--- Patch [18]: [PATCH v10 18/36] pinctrl: airoha: add missed IRQ resource helpers ---
commit 8b10735b561d072460bc6ee2cdf58e36ca203f47
Author: Mikhail Kshevetskiy <[email protected]>

pinctrl: airoha: add missed IRQ resource helpers

Without hooking .irq_request_resources, gpiolib cannot set
GPIOD_FLAG_USED_AS_IRQ. This breaks pin direction locking and can allow
userspace or another driver to reconfigure an active IRQ pin as an output.

> diff --git a/drivers/pinctrl/airoha/pinctrl-airoha.c b/drivers/pinctrl/airoha/pinctrl-airoha.c
> index 9b880fd20a1a0..f3078f9b14a76 100644
> --- a/drivers/pinctrl/airoha/pinctrl-airoha.c
> +++ b/drivers/pinctrl/airoha/pinctrl-airoha.c
> @@ -2689,6 +2689,7 @@ static const struct irq_chip airoha_gpio_irq_chip = {
>  	.irq_mask_ack = airoha_irq_mask,
>  	.irq_set_type = airoha_irq_type,
>  	.flags = IRQCHIP_SET_TYPE_MASKED | IRQCHIP_IMMUTABLE,
> +	GPIOCHIP_IRQ_RESOURCE_HELPERS,

[Severity: High]
Does adding GPIOCHIP_IRQ_RESOURCE_HELPERS to an immutable irq_chip require
adding calls to gpiochip_enable_irq() and gpiochip_disable_irq() to the
unmask and mask callbacks?

When .irq_request_resources runs, gpiolib locks the IRQ and sets
GPIOD_FLAG_IRQ_IS_ENABLED. Because airoha_irq_mask() doesn't call
gpiochip_disable_irq(), this flag is never cleared. Any attempt to set the
pin to output while masked will fail with -EIO in gpiod_direction_output().

Should airoha_irq_unmask() call gpiochip_enable_irq(gc, data->hwirq) and
airoha_irq_mask() call gpiochip_disable_irq(gc, data->hwirq) to properly
notify gpiolib of the IRQ state?

>  };
>  
>  static int airoha_pinctrl_add_gpiochip(struct airoha_pinctrl *pinctrl,

[Severity: High]
This is a pre-existing issue, but can acknowledging the interrupt after
executing generic_handle_irq() cause lost edge interrupts?

Looking at airoha_irq_handler() in the same file:

			generic_handle_irq(irq_find_mapping(girq->domain,
							    offset));
			regmap_write(pinctrl->regmap,
				     pinctrl->gpiochip.status[i], BIT(irq));

If a new edge arrives and latches in the hardware register while the action
executes in generic_handle_irq(), writing BIT(irq) to the W1C status
register afterwards will clear the newly latched edge without it ever being
handled.

Is it possible to clear the interrupt status bit before calling
generic_handle_irq() to avoid missing edge interrupts?

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