Re: [PATCH v7 2/4] gpio: mxc: fix both_edges bit operations

[Peng Fan <peng.fan@oss.nxp.com>]

On Tue, Oct 13, 2026 at 06:26:06AM +0000, sashiko-bot@kernel.org wrote:
>Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
>- [Medium] gpio: state corruption on error path in gpio_set_irq_type()
>--
>
>commit 3d5872f9d45fab5b313d49ded5e6a9aacb019069
>Author: Peng Fan <peng.fan@nxp.com>
>
>gpio: mxc: fix both_edges bit operations
>
>This commit converts both_edges from u32 to unsigned long and uses atomic
>bitops to prevent concurrent updates from overwriting each other.
>
>> diff --git a/drivers/gpio/gpio-mxc.c b/drivers/gpio/gpio-mxc.c
>> index e05f276a50e8d..7e17d7668980d 100644
>> --- a/drivers/gpio/gpio-mxc.c
>> +++ b/drivers/gpio/gpio-mxc.c
>
>[ ... ]
>
>> @@ -168,7 +169,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);
>> +	clear_bit(gpio_idx, &port->both_edges);
>
>[Severity: Medium]
>If a caller requests an unsupported IRQ type, the switch statement later in
>this function falls through to the default case and returns -EINVAL:

This is false alarm. __irq_set_trigger() already guard, there is no chance
to 'return -EINVAL' here. The 'return -EINVAL' is just defensive code.

Regards
Peng

>
>	switch (type) {
>	...
>	default:
>		return -EINVAL;
>	}
>
>Could clearing both_edges early here leave the software state desynchronized
>from the hardware on this error path?
>
>If the hardware was previously configured for dual-edge interrupts, test_bit()
>in the IRQ handler will now return false, failing to flip the edge and
>potentially leading to a stuck IRQ line or interrupt storm.
>
>-- 
>Sashiko AI review · https://sashiko.dev/#/patchset/20261013-gpio-mxc-cleanup-v7-0-c4d769587d3e@nxp.com?part=2
>