Re: [PATCH v6 07/13] gpio: mxc: convert pad wakeup compatible checks to hwdata flags

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

Hi Linus,
On Fri, Oct 16, 2026 at 11:13:28AM +0200, Linus Walleij wrote:
>On Wed, Oct 14, 2026 at 10:30 AM Peng Fan <peng.fan@oss.nxp.com> wrote:
>> On Wed, Oct 14, 2026 at 01:10:48AM +0200, Linus Walleij wrote:
>
>> >Hi Peng,
>> >
>> >On Sun, Oct 11, 2026 at 1:51 PM Peng Fan (OSS) <peng.fan@oss.nxp.com> wrote:
>> >
>> >>  static bool mxc_gpio_generic_config(struct mxc_gpio_port *port,
>> >>                 unsigned int offset, unsigned long conf)
>> >>  {
>> >> -       struct device_node *np = port->dev->of_node;
>> >> +       int ret;
>> >> +
>> >> +       if (!mxc_gpio_has_pad_wakeup(port))
>> >> +               return false;
>> >
>> >Why is the entire call to mxc_gpio_generic_config()
>> >bailing out here if the port does not have pad wakeup?
>> >
>> >> -       if (of_device_is_compatible(np, "fsl,imx8dxl-gpio") ||
>> >> -           of_device_is_compatible(np, "fsl,imx8qxp-gpio") ||
>> >> -           of_device_is_compatible(np, "fsl,imx8qm-gpio"))
>> >> -               return (gpiochip_generic_config(&port->gen_gc.gc,
>> >> -                                               offset, conf) == 0);
>> >> +       ret = gpiochip_generic_config(&port->gen_gc.gc, offset, conf);
>> >> +       if (ret)
>> >> +               return false;
>> >
>> >Then no generic config can happen.
>> >
>> >This looks weird, isn't it just some of the configs that
>> >don't work?
>>
>> The early return is correct and matches the original behavior - only
>> i.MX8 SoCs with SCU have pad wakeup hardware.
>>
>> gpiochip_generic_config() here calls into the pinctrl backend's
>> pin_config_set(), which on i.MX8 programs the SCU firmware with the
>> PAD wakeup source type.  Non-i.MX8 SoCs don't have an SCU pinctrl backend.
>>
>> The function name is misleading - its only caller is mxc_gpio_set_pad_wakeup().
>
>This is just a big hack and I want it removed.
>
>Implicit in this reasoning is: the only config we support on
>the pinctrl back-end is wakeups so let's gate the entire function
>call if we don't support wakeup.
>
>This is a *generic* back-end like the function says:
>gpiochip_generic_config() so the GPIO driver can't make
>assumptions about what the pinctrl backend can and
>cannot do. In a new SoC using the same driver the
>pinctrl back-end could be something different.
>
>So this is breaking modularization and encapsulation.
>
>It is better to let the pin controller return -ENOTSUPP
>or whatever is appropriate instead of gating the
>call like this,

Sure. To get rid of the hack in gpio-mxc.c, we need update pinctrl imx driver
and gpio-mxc driver both, which will create cross-dependency. So I need
to take two steps:

1. Update pinctrl-imx.c to return -ENOTSUPP for non-i.MX8 SoC
2. get rid of the hack in gpio-mxc.c

The current patch is mainly code style change. Could the above 2 steps based
on current patch/patchset? Then we no need to carry more changes in one
patchset.

Thanks
Peng

>
>Yours,
>Linus Walleij
>
>