Re: [PATCH] gpio: Use IRQ trigger mask helpers

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

Pre-existing issues:
- [High] Missing spinlock serialization for shared bank registers in IRQ ch=
ip callbacks.
--

--- Patch [1]: [PATCH] gpio: Use IRQ trigger mask helpers ---
commit a6c81ea868de6b9a08df67a583f7641fb4809834
Author: Maulik Shah <[email protected]>

    gpio: Use IRQ trigger mask helpers
   =20
    Use IRQ_TYPE_LEVEL_MASK and IRQ_TYPE_EDGE_BOTH instead of open-coded
    trigger type combinations in irqchip callbacks.
   =20
    No functional change intended.

[ ... ]

> diff --git a/drivers/gpio/gpio-pxa.c b/drivers/gpio/gpio-pxa.c
> index 5d61053e0596a..2b5da973f51fe 100644
> --- a/drivers/gpio/gpio-pxa.c
> +++ b/drivers/gpio/gpio-pxa.c
> @@ -406,7 +406,7 @@ static int pxa_gpio_irq_type(struct irq_data *d, unsi=
gned int type)
>  		if (__gpio_is_occupied(pchip, gpio))
>  			return 0;
> =20
> -		type =3D IRQ_TYPE_EDGE_RISING | IRQ_TYPE_EDGE_FALLING;
> +		type =3D IRQ_TYPE_EDGE_BOTH;
>  	}
> =20
>  	gpdr =3D readl_relaxed(c->regbase + GPDR_OFFSET);
> =20
>  	if (__gpio_is_inverted(gpio))
>  		writel_relaxed(gpdr | mask,  c->regbase + GPDR_OFFSET);
>  	else
>  		writel_relaxed(gpdr & ~mask, c->regbase + GPDR_OFFSET);

[Severity: High]
This is a pre-existing issue, but since you are touching this function,
is it safe to perform these read-modify-write operations on the GPDR regist=
er=20
without a spinlock?=20

It seems this could race with other IRQs in the same bank and with=20
gpio_direction operations. For instance, pxa_gpio_irq_type() races with=20
pxa_gpio_direction_input() and pxa_gpio_direction_output() over the GPDR=20
register, as the latter use gpio_lock but the former does not.

A similar issue appears in pxa_mask_muxed_gpio():

pxa_mask_muxed_gpio() {
    ...
    b->irq_mask &=3D ~GPIO_bit(gpio);

    grer =3D readl_relaxed(base + GRER_OFFSET) & ~GPIO_bit(gpio);
    gfer =3D readl_relaxed(base + GFER_OFFSET) & ~GPIO_bit(gpio);
    writel_relaxed(grer, base + GRER_OFFSET);
    writel_relaxed(gfer, base + GFER_OFFSET);
    ...
}

and pxa_unmask_muxed_gpio():

pxa_unmask_muxed_gpio() {
    ...
    c->irq_mask |=3D GPIO_bit(gpio);
    update_edge_detect(c);
    ...
}

Can this lack of serialization lead to data corruption of the GPIO direction
configuration (GPDR) and interrupt edge detection configuration, potentially
causing improperly configured pins or permanent loss of interrupts on SMP
systems when called concurrently for different GPIOs within the same bank?

--=20
Sashiko AI review =C2=B7 https://sashiko.dev/#/patchset/20260802-b4b4-irq-t=
[email protected]?part=3D1