Re: [PATCH v2 2/9] gpio: mxc: fix wakeup_pads bit operations

[Andy Shevchenko]

On Mon, Oct 5, 2026 at 5:24 PM Peng Fan (OSS) <peng.fan@xxxxxxxxxxx> wrote:
>
> From: Peng Fan <peng.fan@xxxxxxx>
>
> gpio_set_wake_irq() can be called concurrently for different pins on
> the same port, so need to use atomic bitops when modifying wakeup_pads.

so we need

> Since wakeup_pads is u32 but assign_bit() operate on unsigned long

operates

> pointers.

This sentence doesn't sound like it's finished.

> On 64-bit platforms this causes an 8-byte read-modify-write
> on a 4-byte field, corrupting the adjacent is_pad_wakeup member.
> Change wakeup_pads to unsigned long and reorder to 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.

...

> +       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);
>

This blank line may be removed.

> -       return ret;
> +       if (ret)
> +               return ret;

...

> -       for (i = 0; i < 32; i++) {
> -               if ((port->wakeup_pads & (1 << i))) {
> -                       type = port->pad_type[i];
> -                       if (enable)
> -                               config = pad_type_map[type];
> -                       else
> -                               config = IMX_SCU_WAKEUP_OFF;
> -
> -                       if (is_imx8qm && config == IMX_SCU_WAKEUP_FALL_EDGE) {
> -                               dev_warn_once(port->dev,
> -                                             "No falling-edge support for wakeup on i.MX8QM\n");
> -                               config = IMX_SCU_WAKEUP_OFF;
> -                       }
> +       for_each_set_bit(i, &port->wakeup_pads, 32) {

This change can be done later, can it?
(I mean in a separate patch)

> +               type = port->pad_type[i];
> +               if (enable)
> +                       config = pad_type_map[type];
> +               else
> +                       config = IMX_SCU_WAKEUP_OFF;
>
> -                       ret |= mxc_gpio_generic_config(port, i, config);
> +               if (is_imx8qm && config == IMX_SCU_WAKEUP_FALL_EDGE) {
> +                       dev_warn_once(port->dev,
> +                                     "No falling-edge support for wakeup on i.MX8QM\n");
> +                       config = IMX_SCU_WAKEUP_OFF;
>                 }
> +
> +               ret |= mxc_gpio_generic_config(port, i, config);
>         }

-- 
With Best Regards,
Andy Shevchenko