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

[sashiko-bot@kernel.org]

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))) {
			...

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