Re: [PATCH v5 03/13] gpio: mxc: fix wakeup_pads bit operations

[Frank Li]

On Sat, Oct 10, 2026 at 02:05:24AM +0800, Peng Fan (OSS) wrote:
> From: Peng Fan <peng.fan@xxxxxxx>
>
> gpio_set_wake_irq() can be called concurrently for different pins on
> the same port, so use atomic bitops when modifying wakeup_pads.
>
> wakeup_pads is a u32, while assign_bit() operates on unsigned long
> pointers. On 64-bit platforms, this causes an 8-byte read-modify-write
> on a 4-byte field, corrupting the adjacent is_pad_wakeup. So change
> wakeup_pads to unsigned long and reorder to avoid wasting words.
>
> And the enable/disable path unconditionally sets/clears the wakeup_pads

Nit:  And fix the enable/disable ....

Reviewed-by: Frank Li <Frank.Li@xxxxxxx>

> bit even when enable_irq_wake()/disable_irq_wake() fails. Only update
> the bit on success.
>
> Fixes: f60c9eac54af ("gpio: mxc: enable pad wakeup on i.MX8x platforms")
> Assisted-by: LLM
> Signed-off-by: Peng Fan <peng.fan@xxxxxxx>
> ---
>  drivers/gpio/gpio-mxc.c | 10 ++++++----
>  1 file changed, 6 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpio/gpio-mxc.c b/drivers/gpio/gpio-mxc.c
> index 2f83a0f076bf..29396135717a 100644
> --- a/drivers/gpio/gpio-mxc.c
> +++ b/drivers/gpio/gpio-mxc.c
> @@ -71,8 +71,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;
>  	u32 pad_type[32];
>  	const struct mxc_gpio_hwdata *hwdata;
>  };
> @@ -330,16 +330,18 @@ static int gpio_set_wake_irq(struct irq_data *d, u32 enable)
>  			ret = enable_irq_wake(port->irq_high);
>  		else
>  			ret = enable_irq_wake(port->irq);
> -		port->wakeup_pads |= BIT(gpio_idx);
>  	} else {
>  		if (port->irq_high && (gpio_idx >= 16))
>  			ret = disable_irq_wake(port->irq_high);
>  		else
>  			ret = disable_irq_wake(port->irq);
> -		port->wakeup_pads &= ~BIT(gpio_idx);
>  	}
> +	if (ret)
> +		return ret;
>
> -	return ret;
> +	assign_bit(gpio_idx, &port->wakeup_pads, enable);
> +
> +	return 0;
>  }
>
>  static int mxc_gpio_init_gc(struct mxc_gpio_port *port, int irq_base)
>
> --
> 2.51.0
>
>