Re: [PATCH] pinctrl: amd: Clear interrupt enable on spurious IRQ and skip unhandled EOI

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] pinctrl: amd: missing EOI in do_amd_gpio_irq_handler() on spurious IRQ
--

commit df92227f2b1cea5ec26fd7063440df3f2e4656a4
Author: Munique Feitoza <muniquefeitoz4@gmail.com>

pinctrl: amd: Clear interrupt enable on spurious IRQ and skip unhandled EOI

This commit clears BIT(INTERRUPT_ENABLE_OFF) alongside BIT(INTERRUPT_MASK_OFF)
when disabling a spurious GPIO IRQ in do_amd_gpio_irq_handler(), and returns
early before writing EOI_MASK to WAKE_INT_MASTER_REG when ret is false.

> diff --git a/drivers/pinctrl/pinctrl-amd.c b/drivers/pinctrl/pinctrl-amd.c
> index 9fa752f552d43..42844094916a7 100644
> --- a/drivers/pinctrl/pinctrl-amd.c
> +++ b/drivers/pinctrl/pinctrl-amd.c
> @@ -671,7 +671,8 @@ static bool do_amd_gpio_irq_handler(int irq, void *dev_id)
>  			raw_spin_lock_irqsave(&gpio_dev->lock, flags);
>  			regval = readl(amd_gpio_pin_reg(gpio_dev, irqnr + i));
>  			if (!gpiochip_line_is_irq(gc, irqnr + i)) {
> -				regval &= ~BIT(INTERRUPT_MASK_OFF);
> +				regval &= ~(BIT(INTERRUPT_MASK_OFF) |
> +					    BIT(INTERRUPT_ENABLE_OFF));
>  				dev_dbg(&gpio_dev->pdev->dev,
>  					"Disabling spurious GPIO IRQ %d\n",
>  					irqnr + i);
> @@ -722,7 +723,8 @@ static bool do_amd_gpio_irq_handler(int irq, void *dev_id)
>  			raw_spin_lock_irqsave(&gpio_dev->lock, flags);
>  			regval = readl(amd_gpio_pin_reg(gpio_dev, pin));
>  			if (!gpiochip_line_is_irq(gc, pin)) {
> -				regval &= ~BIT(INTERRUPT_MASK_OFF);
> +				regval &= ~(BIT(INTERRUPT_MASK_OFF) |
> +					    BIT(INTERRUPT_ENABLE_OFF));
>  				dev_dbg(&gpio_dev->pdev->dev,
>  					"Disabling spurious GPIO IRQ %d\n",
>  					pin);
> @@ -735,7 +737,7 @@ static bool do_amd_gpio_irq_handler(int irq, void *dev_id)
>  	}
>  
>  	/* did not cause wake on resume context for shared IRQ */
> -	if (irq < 0)
> +	if (irq < 0 || !ret)
>  		return false;

[Severity: High]
Does skipping the EOI write when !ret is true leave the AMD GPIO controller
with an unacknowledged interrupt after a spurious GPIO IRQ?

When an unmasked pin raises an interrupt while gpiochip_line_is_irq() is false
(for instance, at boot before a client driver such as i2c-hid probes as in
commit d21b8adbd475 ("pinctrl: amd: Disable and mask interrupts on probe"), or
after suspend/resume when firmware leaves an unused pin unmasked as in commit
0cf9e48ff22e ("pinctrl: amd: Disable and mask interrupts on resume")),
do_amd_gpio_irq_handler() clears and disables the pin interrupt in the
!gpiochip_line_is_irq() branch on the main or RGPIO bank without setting
ret = true:

drivers/pinctrl/pinctrl-amd.c:do_amd_gpio_irq_handler() {
	...
			if (!gpiochip_line_is_irq(gc, irqnr + i)) {
				regval &= ~(BIT(INTERRUPT_MASK_OFF) |
					    BIT(INTERRUPT_ENABLE_OFF));
				dev_dbg(&gpio_dev->pdev->dev,
					"Disabling spurious GPIO IRQ %d\n",
					irqnr + i);
			} else {
				ret = true;
			}
			writel(regval, amd_gpio_pin_reg(gpio_dev, irqnr + i));
	...
}

If no other registered GPIO IRQ is active during the same invocation, ret
remains false even though the AMD GPIO hardware asserted the interrupt line.

Returning early at if (irq < 0 || !ret) skips writing EOI_MASK to
WAKE_INT_MASTER_REG:

	/* did not cause wake on resume context for shared IRQ */
	if (irq < 0 || !ret)
		return false;

	/* Signal EOI to the GPIO unit */
	raw_spin_lock_irqsave(&gpio_dev->lock, flags);
	regval = readl(gpio_dev->base + WAKE_INT_MASTER_REG);
	regval |= EOI_MASK;
	writel(regval, gpio_dev->base + WAKE_INT_MASTER_REG);
	raw_spin_unlock_irqrestore(&gpio_dev->lock, flags);

Because the flow handlers used by amd_gpio_irqchip (handle_edge_irq,
handle_level_irq, and handle_simple_irq) do not call .irq_eoi,
do_amd_gpio_irq_handler() is the only place that signals EOI to the GPIO
unit. Wouldn't skipping EOI_MASK after servicing a spurious GPIO interrupt
prevent the controller from delivering subsequent valid GPIO interrupts?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261030195025.5153-1-muniquefeitoz4@gmail.com?part=1