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

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

On Mon, Oct 12, 2026 at 12:15:28PM +0300, Andy Shevchenko wrote:
>On Sun, Oct 11, 2026 at 2:54 PM Peng Fan (OSS) <peng.fan@oss.nxp.com> wrote:
>>
>> gpio_set_wake_irq() can be called concurrently for different pins on
>> the same port, so use atomic bitops when modifying wakeup_pads.
>
>The code doesn't modify any reader part of this member. So, the above
>paragraph is incorrect, right? Otherwise it needs to be clarified how
>assign_bit() does change anything.

I will replace with below:

gpio_set_wake_irq() can be called concurrently for different pins on
the same port. The IRQ core serializes per irq descriptor, but
different pins have different descriptors, so concurrent calls for
pins on the same port run without mutual exclusion. The plain |= and
&= on wakeup_pads are non-atomic read-modify-write operations that
can lose updates when two CPUs modify different bits simultaneously.
Replace them with assign_bit() to make the modifications atomic.

Thanks
Peng

>
>> 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 fix 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.
>
>-- 
>With Best Regards,
>Andy Shevchenko
>