Re: [PATCH v4 10/10] gpio: mxc: use BIT() macro for single-bit operations

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] gpio: mxc: race condition in gpio_set_irq_type updating port->both_edges
--

commit 71b368728fc9ada85f870f46f968a6583a740b0c
Author: Peng Fan <peng.fan@nxp.com>

gpio: mxc: use BIT() macro for single-bit operations

This commit replaces open-coded shifts with the BIT() macro to avoid potential
signed-shift issues and improve consistency. No functional changes are
intended.

> diff --git a/drivers/gpio/gpio-mxc.c b/drivers/gpio/gpio-mxc.c
> index 39de616cd434f..ad50b602c4a16 100644
> --- a/drivers/gpio/gpio-mxc.c
> +++ b/drivers/gpio/gpio-mxc.c
> @@ -209,7 +209,7 @@ static int gpio_set_irq_type(struct irq_data *d, u32 type)
>  	int edge;
>  	void __iomem *reg = port->base;
>  
> -	port->both_edges &= ~(1 << gpio_idx);
> +	port->both_edges &= ~BIT(gpio_idx);

[Severity: Medium]
Could this unprotected read-modify-write on the shared port bitmask lead to a
race condition?

Concurrent updates to the shared port->both_edges bitmask could cause one CPU
to overwrite and lose the IRQ edge configuration of another pin, potentially
leading to missed interrupts or stuck IRQ lines.

This can happen during parallel device probing or via userspace GPIO
configuration (e.g., libgpiod/sysfs) when two CPU cores concurrently call
gpio_set_irq_type() for different GPIO pins on the same port.

The IRQ subsystem serializes irq_set_type() on a per-pin basis (via
desc->lock), not per-port.

While the driver correctly takes the port-wide lock later in this function via
scoped_guard(gpio_generic_lock_irqsave, &port->gen_gc) for its MMIO register
updates, it performs read-modify-write operations on the shared
port->both_edges software mask completely outside of this critical section.

[ ... ]

> @@ -229,7 +229,7 @@ static int gpio_set_irq_type(struct irq_data *d, u32 type)
>  				edge = GPIO_INT_HIGH_LEV;
>  				pr_debug("mxc: set GPIO %d to high trigger\n", gpio_idx);
>  			}
> -			port->both_edges |= 1 << gpio_idx;
> +			port->both_edges |= BIT(gpio_idx);

[Severity: Medium]
This appears to be a second unprotected modification in the IRQ_TYPE_EDGE_BOTH
path.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261007-gpio-mxc-cleanup-v4-0-c9f5a8b50df9@nxp.com?part=10