[PATCH v6 07/13] gpio: mxc: convert pad wakeup compatible checks to hwdata flags
Linus Walleij
linusw at kernel.org
Thu Oct 29 06:06:26 PDT 2026
Hi Peng,
On Fri, Oct 16, 2026 at 3:39 PM Peng Fan <peng.fan at oss.nxp.com> wrote:
> 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 at 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 at 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.
The current patch is NOT a cleaup because you add this:
+ if (!mxc_gpio_has_pad_wakeup(port))
+ return false;
If you want you can drop this and related parts and make a pure cleanup
patch, that is fine.
If you want the above semantic change, make change 1 above, fix the
pinctrl driver.
Yours,
Linus Walleij
More information about the linux-arm-kernel
mailing list