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

[Andy Shevchenko]

On Mon, Oct 12, 2026 at 12:46 PM Peng Fan <peng.fan at oss.nxp.com> wrote:
> 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 at 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.

Right, but the code does change only the writer side. There is no
reader side that was changed. So this patch does *not* add any
atomicity. You also need to check if the reader side already uses
test_bit() or something like that. Otherwise it makes no sense.

> >> 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