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

[Andy Shevchenko]

On Mon, Oct 12, 2026 at 2:37 PM Peng Fan <peng.fan at oss.nxp.com> wrote:
> On Mon, Oct 12, 2026 at 01:30:04PM +0300, Andy Shevchenko wrote:
> >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.
>
> 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. The old
> |= and &= compile to separate load/modify/store instructions that can
> interleave, losing one CPU's update.
>
> assign_bit() uses an atomic RMW instruction, so concurrent writers to
> different bits in the same word are safe.
>
> The reader side (for_each_set_bit in noirq_suspend, wakeup_pads > 0)
> runs in noirq context where no concurrent writer can execute, so
> reader atomicity is not needed.

Thanks, now make sure that summary of this is in the commit message.

-- 
With Best Regards,
Andy Shevchenko