[PATCH v4 02/10] gpio: mxc: fix wakeup_pads bit operations

[Peng Fan]

On Thu, Oct 08, 2026 at 02:59:22PM -0500, Frank Li wrote:
>On Wed, Oct 07, 2026 at 06:44:17PM +0800, Peng Fan (OSS) wrote:
>> From: Peng Fan <peng.fan at nxp.com>
>>
>> gpio_set_wake_irq() can be called concurrently for different pins on
>> the same port, so we need to use atomic bitops when modifying
>> wakeup_pads.
>
>So Need to use atomic ..

Andy suggested: so we need
https://lore.kernel.org/all/CAHp75Ve+p6TMr7MwQw0orGdjxLgNBN8gfTw2SY+EKBBgCyYXgA@mail.gmail.com/

>
>>
>> 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 member.
>
>is_pad_wakeup is not member, that's local variable. so should be
>
>"corrupting the adjacent is_pad_wakeup local variable"
>
>> Change wakeup_pads to unsigned long and reorder to avoid the overlap.
>
>reorder can't "avoid the overlay"
>
>Change "corrupting the adjacent is_pad_wakeup" already avoid the overlap.
>
>
>>
>> And the enable/disable path unconditionally sets/clears the wakeup_pads
>> bit even when enable_irq_wake()/disable_irq_wake() fails. Only update
>> the bit on success.
>>
>> While at here, simplify the logic by consolidating into a single
>> irq_set_irq_wake() call based on the enable parameter.
>
>And use irq_set_irq_wake() simple code. this part need seperate patch.

Thanks,
Peng

>
>Frank
>
>>
>> Fixes: f60c9eac54af ("gpio: mxc: enable pad wakeup on i.MX8x platforms")
>> Assisted-by: LLM
>> Signed-off-by: Peng Fan <peng.fan at nxp.com>
>> ---
>>  drivers/gpio/gpio-mxc.c | 25 ++++++++++---------------
>>  1 file changed, 10 insertions(+), 15 deletions(-)
>>
>> diff --git a/drivers/gpio/gpio-mxc.c b/drivers/gpio/gpio-mxc.c
>> index e05f276a50e8..627fff6f1886 100644
>> --- a/drivers/gpio/gpio-mxc.c
>> +++ b/drivers/gpio/gpio-mxc.c
>> @@ -71,8 +71,8 @@ struct mxc_gpio_port {
>>  	u32 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;
>>  };
>> @@ -325,21 +325,16 @@ static int gpio_set_wake_irq(struct irq_data *d, u32 enable)
>>  	u32 gpio_idx = d->hwirq;
>>  	int ret;
>>
>> -	if (enable) {
>> -		if (port->irq_high && (gpio_idx >= 16))
>> -			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 (port->irq_high && (gpio_idx >= 16))
>> +		ret = irq_set_irq_wake(port->irq_high, enable);
>> +	else
>> +		ret = irq_set_irq_wake(port->irq, enable);
>> +	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
>>
>>
>