Re: [PATCH v7 3/4] gpio: mxc: fix wakeup_pads bit operations

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

On Tue, Oct 13, 2026 at 06:35:07AM +0000, sashiko-bot@kernel.org wrote:
>Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
>- [Medium] gpio: mxc: missing for_each_set_bit hunk and sign-extension bug
>--
>
>commit 163a0b84d8efa779538111b96ea2ce2543fbd9da
>Author: Peng Fan <peng.fan@nxp.com>
>
>    gpio: mxc: fix wakeup_pads bit operations
>    
>    gpio_set_wake_irq() can be called concurrently for different pins on
>    the same port. The writer-vs-writer race is the concern: two concurrent
>    gpio_set_wake_irq() calls for different pins on the same port both do
>    a non-atomic read-modify-write on the same wakeup_pads word.
>
>> diff --git a/drivers/gpio/gpio-mxc.c b/drivers/gpio/gpio-mxc.c
>> index 7e17d7668980d..e3a80f17d0412 100644
>> --- a/drivers/gpio/gpio-mxc.c
>> +++ b/drivers/gpio/gpio-mxc.c
>> @@ -72,8 +72,8 @@ struct mxc_gpio_port {
>>  	unsigned long both_edges;
>>  	struct mxc_gpio_reg_saved gpio_saved_reg;
>>  	bool power_off;
>> -	u32 wakeup_pads;
>>  	bool is_pad_wakeup;
>> +	unsigned long wakeup_pads;
>
>[Severity: Medium]
>The commit message states "The reader side (for_each_set_bit in
>noirq_suspend, wakeup_pads > 0) runs in noirq context", but it appears
>the hunk that changes this loop is missing from the patch.
>
>Because wakeup_pads is now an unsigned long, the existing loop in
>drivers/gpio/gpio-mxc.c:mxc_gpio_set_pad_wakeup() has a sign-extension issue:
>
>	for (i = 0; i < 32; i++) {
>		if ((port->wakeup_pads & (1 << i))) {

The code will not incur error. for_each_set_bit() patch will be in
the following up patchset after the fixes patchset.

Regards
Peng

>			...
>
>On 64-bit platforms, when i == 31, the 32-bit signed expression (1 << 31)
>evaluates to -2147483648. When bitwise ANDed with the newly widened
>unsigned long, it sign-extends to 0xFFFFFFFF80000000UL, which inadvertently
>masks against bits 32-63 as well.
>
>Could the missing for_each_set_bit hunk be added to resolve this?
>
>>  	u32 pad_type[32];
>>  	const struct mxc_gpio_hwdata *hwdata;
>>  };
>
>-- 
>Sashiko AI review · https://sashiko.dev/#/patchset/20261013-gpio-mxc-cleanup-v7-0-c4d769587d3e@nxp.com?part=3
>