pinctrl, probe order, and CONFIG_MODULES

pinctrl, probe order, and CONFIG_MODULES

[Phil Elwell]

Hi,

A Raspberry Pi user recently asked us why I2C wasn't working in their
custom kernel build. The primary change they had made was to trim the
number of enabled drivers, make them all built-in, and to remove
CONFIG_MODULES=y. The end result of this is that the pin muxing
required for I2C to work was not being applied, leaving the interface
talking to thin air.

I eventually traced the failure back to this change:
https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/drivers/pinctrl/devicetree.c?h=next-20250829&id=d19c5e79d46efdf89306be99f3c8824cf58e35f6
It introduces a requirement for CONFIG_MODULES to be enabled in order
to get an EPROBE_DEFER in the event that the pinctrl driver has not
yet been probed. Without CONFIG_MODULES, the pinctrl requirements are
waived. Removing the IS_ENABLED(CONFIG_MODULES) clause allows the
probing of the I2C driver to be deferred, fixing the user's problem.

Is this requirement for supporting modules reasonable? If so, how is
one supposed to get the pinctrl to probe before the I2C driver?

Thanks,

Phil Elwell

Re: pinctrl, probe order, and CONFIG_MODULES

[Linus Walleij]

On Fri, Aug 29, 2025 at 12:37 PM Phil Elwell <phil@raspberrypi.com> wrote:

> I eventually traced the failure back to this change:
> https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/drivers/pinctrl/devicetree.c?h=next-20250829&id=d19c5e79d46efdf89306be99f3c8824cf58e35f6
> It introduces a requirement for CONFIG_MODULES to be enabled in order
> to get an EPROBE_DEFER in the event that the pinctrl driver has not
> yet been probed. Without CONFIG_MODULES, the pinctrl requirements are
> waived. Removing the IS_ENABLED(CONFIG_MODULES) clause allows the
> probing of the I2C driver to be deferred, fixing the user's problem.
>
> Is this requirement for supporting modules reasonable? If so, how is
> one supposed to get the pinctrl to probe before the I2C driver?

No deferred probing of pin controllers should absolutely work also when
not using modules.

Send a patch for the above and let us discuss this.

Yours,
Linus Walleij

Re: pinctrl, probe order, and CONFIG_MODULES

[Rob Herring]

On Fri, Aug 29, 2025 at 5:37 AM Phil Elwell <phil@raspberrypi.com> wrote:
>
> Hi,
>
> A Raspberry Pi user recently asked us why I2C wasn't working in their
> custom kernel build. The primary change they had made was to trim the
> number of enabled drivers, make them all built-in, and to remove
> CONFIG_MODULES=y. The end result of this is that the pin muxing
> required for I2C to work was not being applied, leaving the interface
> talking to thin air.
>
> I eventually traced the failure back to this change:
> https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/drivers/pinctrl/devicetree.c?h=next-20250829&id=d19c5e79d46efdf89306be99f3c8824cf58e35f6
> It introduces a requirement for CONFIG_MODULES to be enabled in order
> to get an EPROBE_DEFER in the event that the pinctrl driver has not
> yet been probed. Without CONFIG_MODULES, the pinctrl requirements are
> waived. Removing the IS_ENABLED(CONFIG_MODULES) clause allows the
> probing of the I2C driver to be deferred, fixing the user's problem.
>
> Is this requirement for supporting modules reasonable?

That's not the requirement. If CONFIG_MODULES=n, then we only defer
probes until late_initcall because after that point no more drivers
are going to load. If CONFIG_MODULES=y, then deferring probe is based
on a timeout which can be disabled.

> If so, how is
> one supposed to get the pinctrl to probe before the I2C driver?

That shouldn't be required.

Rob

Re: pinctrl, probe order, and CONFIG_MODULES

[Phil Elwell]

Hi Rob,

On Fri, 29 Aug 2025 at 14:25, Rob Herring <robh@kernel.org> wrote:
>
> On Fri, Aug 29, 2025 at 5:37 AM Phil Elwell <phil@raspberrypi.com> wrote:
> >
> > Hi,
> >
> > A Raspberry Pi user recently asked us why I2C wasn't working in their
> > custom kernel build. The primary change they had made was to trim the
> > number of enabled drivers, make them all built-in, and to remove
> > CONFIG_MODULES=y. The end result of this is that the pin muxing
> > required for I2C to work was not being applied, leaving the interface
> > talking to thin air.
> >
> > I eventually traced the failure back to this change:
> > https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/drivers/pinctrl/devicetree.c?h=next-20250829&id=d19c5e79d46efdf89306be99f3c8824cf58e35f6
> > It introduces a requirement for CONFIG_MODULES to be enabled in order
> > to get an EPROBE_DEFER in the event that the pinctrl driver has not
> > yet been probed. Without CONFIG_MODULES, the pinctrl requirements are
> > waived. Removing the IS_ENABLED(CONFIG_MODULES) clause allows the
> > probing of the I2C driver to be deferred, fixing the user's problem.
> >
> > Is this requirement for supporting modules reasonable?
>
> That's not the requirement. If CONFIG_MODULES=n, then we only defer
> probes until late_initcall because after that point no more drivers
> are going to load. If CONFIG_MODULES=y, then deferring probe is based
> on a timeout which can be disabled.

Thanks for replying. I'm probably missing something here, but if the
pinctrl and I2C drivers are both regular platform drivers, what is to
stop the I2C driver being probed first?

Phil

Re: pinctrl, probe order, and CONFIG_MODULES

[Rob Herring]

On Fri, Aug 29, 2025 at 8:35 AM Phil Elwell <phil@raspberrypi.com> wrote:
>
> Hi Rob,
>
> On Fri, 29 Aug 2025 at 14:25, Rob Herring <robh@kernel.org> wrote:
> >
> > On Fri, Aug 29, 2025 at 5:37 AM Phil Elwell <phil@raspberrypi.com> wrote:
> > >
> > > Hi,
> > >
> > > A Raspberry Pi user recently asked us why I2C wasn't working in their
> > > custom kernel build. The primary change they had made was to trim the
> > > number of enabled drivers, make them all built-in, and to remove
> > > CONFIG_MODULES=y. The end result of this is that the pin muxing
> > > required for I2C to work was not being applied, leaving the interface
> > > talking to thin air.
> > >
> > > I eventually traced the failure back to this change:
> > > https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/drivers/pinctrl/devicetree.c?h=next-20250829&id=d19c5e79d46efdf89306be99f3c8824cf58e35f6
> > > It introduces a requirement for CONFIG_MODULES to be enabled in order
> > > to get an EPROBE_DEFER in the event that the pinctrl driver has not
> > > yet been probed. Without CONFIG_MODULES, the pinctrl requirements are
> > > waived. Removing the IS_ENABLED(CONFIG_MODULES) clause allows the
> > > probing of the I2C driver to be deferred, fixing the user's problem.
> > >
> > > Is this requirement for supporting modules reasonable?
> >
> > That's not the requirement. If CONFIG_MODULES=n, then we only defer
> > probes until late_initcall because after that point no more drivers
> > are going to load. If CONFIG_MODULES=y, then deferring probe is based
> > on a timeout which can be disabled.
>
> Thanks for replying. I'm probably missing something here, but if the
> pinctrl and I2C drivers are both regular platform drivers, what is to
> stop the I2C driver being probed first?

Nothing, but it should defer unless you've reached late initcall or
you've set "pinctrl-use-default".

Rob

Re: pinctrl, probe order, and CONFIG_MODULES

[Phil Elwell]

On Fri, 29 Aug 2025 at 15:44, Rob Herring <robh@kernel.org> wrote:
>
> On Fri, Aug 29, 2025 at 8:35 AM Phil Elwell <phil@raspberrypi.com> wrote:
> >
> > Hi Rob,
> >
> > On Fri, 29 Aug 2025 at 14:25, Rob Herring <robh@kernel.org> wrote:
> > >
> > > On Fri, Aug 29, 2025 at 5:37 AM Phil Elwell <phil@raspberrypi.com> wrote:
> > > >
> > > > Hi,
> > > >
> > > > A Raspberry Pi user recently asked us why I2C wasn't working in their
> > > > custom kernel build. The primary change they had made was to trim the
> > > > number of enabled drivers, make them all built-in, and to remove
> > > > CONFIG_MODULES=y. The end result of this is that the pin muxing
> > > > required for I2C to work was not being applied, leaving the interface
> > > > talking to thin air.
> > > >
> > > > I eventually traced the failure back to this change:
> > > > https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/drivers/pinctrl/devicetree.c?h=next-20250829&id=d19c5e79d46efdf89306be99f3c8824cf58e35f6
> > > > It introduces a requirement for CONFIG_MODULES to be enabled in order
> > > > to get an EPROBE_DEFER in the event that the pinctrl driver has not
> > > > yet been probed. Without CONFIG_MODULES, the pinctrl requirements are
> > > > waived. Removing the IS_ENABLED(CONFIG_MODULES) clause allows the
> > > > probing of the I2C driver to be deferred, fixing the user's problem.
> > > >
> > > > Is this requirement for supporting modules reasonable?
> > >
> > > That's not the requirement. If CONFIG_MODULES=n, then we only defer
> > > probes until late_initcall because after that point no more drivers
> > > are going to load. If CONFIG_MODULES=y, then deferring probe is based
> > > on a timeout which can be disabled.
> >
> > Thanks for replying. I'm probably missing something here, but if the
> > pinctrl and I2C drivers are both regular platform drivers, what is to
> > stop the I2C driver being probed first?
>
> Nothing, but it should defer unless you've reached late initcall or
> you've set "pinctrl-use-default".

From the "next" tree:

if (!np_pctldev || of_node_is_root(np_pctldev)) {
        of_node_put(np_pctldev);
        ret = -ENODEV;
        /* keep deferring if modules are enabled */
        if (IS_ENABLED(CONFIG_MODULES) && !allow_default && ret < 0)
                ret = -EPROBE_DEFER;
        return ret;
}

Unless CONFIG_MODULES=y you get ENODEV.

Phil

Re: pinctrl, probe order, and CONFIG_MODULES

[Rob Herring]

On Fri, Aug 29, 2025 at 9:52 AM Phil Elwell <phil@raspberrypi.com> wrote:
>
> On Fri, 29 Aug 2025 at 15:44, Rob Herring <robh@kernel.org> wrote:
> >
> > On Fri, Aug 29, 2025 at 8:35 AM Phil Elwell <phil@raspberrypi.com> wrote:
> > >
> > > Hi Rob,
> > >
> > > On Fri, 29 Aug 2025 at 14:25, Rob Herring <robh@kernel.org> wrote:
> > > >
> > > > On Fri, Aug 29, 2025 at 5:37 AM Phil Elwell <phil@raspberrypi.com> wrote:
> > > > >
> > > > > Hi,
> > > > >
> > > > > A Raspberry Pi user recently asked us why I2C wasn't working in their
> > > > > custom kernel build. The primary change they had made was to trim the
> > > > > number of enabled drivers, make them all built-in, and to remove
> > > > > CONFIG_MODULES=y. The end result of this is that the pin muxing
> > > > > required for I2C to work was not being applied, leaving the interface
> > > > > talking to thin air.
> > > > >
> > > > > I eventually traced the failure back to this change:
> > > > > https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/drivers/pinctrl/devicetree.c?h=next-20250829&id=d19c5e79d46efdf89306be99f3c8824cf58e35f6
> > > > > It introduces a requirement for CONFIG_MODULES to be enabled in order
> > > > > to get an EPROBE_DEFER in the event that the pinctrl driver has not
> > > > > yet been probed. Without CONFIG_MODULES, the pinctrl requirements are
> > > > > waived. Removing the IS_ENABLED(CONFIG_MODULES) clause allows the
> > > > > probing of the I2C driver to be deferred, fixing the user's problem.
> > > > >
> > > > > Is this requirement for supporting modules reasonable?
> > > >
> > > > That's not the requirement. If CONFIG_MODULES=n, then we only defer
> > > > probes until late_initcall because after that point no more drivers
> > > > are going to load. If CONFIG_MODULES=y, then deferring probe is based
> > > > on a timeout which can be disabled.
> > >
> > > Thanks for replying. I'm probably missing something here, but if the
> > > pinctrl and I2C drivers are both regular platform drivers, what is to
> > > stop the I2C driver being probed first?
> >
> > Nothing, but it should defer unless you've reached late initcall or
> > you've set "pinctrl-use-default".
>
> From the "next" tree:
>
> if (!np_pctldev || of_node_is_root(np_pctldev)) {
>         of_node_put(np_pctldev);
>         ret = -ENODEV;
>         /* keep deferring if modules are enabled */
>         if (IS_ENABLED(CONFIG_MODULES) && !allow_default && ret < 0)
>                 ret = -EPROBE_DEFER;
>         return ret;
> }
>
> Unless CONFIG_MODULES=y you get ENODEV.

Indeed, as 'ret = driver_deferred_probe_check_state(p->dev);' is gone due to:

commit 24a026f85241a01bbcfe1b263caeeaa9a79bab40
Author: Saravana Kannan <saravanak@google.com>
Date:   Wed Jun 1 00:06:58 2022 -0700

    pinctrl: devicetree: Delete usage of driver_deferred_probe_check_state()

    Now that fw_devlink=on by default and fw_devlink supports
    "pinctrl-[0-8]" property, the execution will never get to the point
    where driver_deferred_probe_check_state() is called before the supplier
    has probed successfully or before deferred probe timeout has expired.

    So, delete the call and replace it with -ENODEV.

    Tested-by: Geert Uytterhoeven <geert+renesas@glider.be>
    Signed-off-by: Saravana Kannan <saravanak@google.com>
    Link: https://lore.kernel.org/r/20220601070707.3946847-3-saravanak@google.com
    Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>


So fw_devlink will ensure that pinctrl probes first. Is that turned
off by chance (kernel cmdline is the only way to disable).

Rob

Re: pinctrl, probe order, and CONFIG_MODULES

[Phil Elwell]

On Fri, 29 Aug 2025 at 16:01, Rob Herring <robh@kernel.org> wrote:
>
> On Fri, Aug 29, 2025 at 9:52 AM Phil Elwell <phil@raspberrypi.com> wrote:
> >
> > On Fri, 29 Aug 2025 at 15:44, Rob Herring <robh@kernel.org> wrote:
> > >
> > > On Fri, Aug 29, 2025 at 8:35 AM Phil Elwell <phil@raspberrypi.com> wrote:
> > > >
> > > > Hi Rob,
> > > >
> > > > On Fri, 29 Aug 2025 at 14:25, Rob Herring <robh@kernel.org> wrote:
> > > > >
> > > > > On Fri, Aug 29, 2025 at 5:37 AM Phil Elwell <phil@raspberrypi.com> wrote:
> > > > > >
> > > > > > Hi,
> > > > > >
> > > > > > A Raspberry Pi user recently asked us why I2C wasn't working in their
> > > > > > custom kernel build. The primary change they had made was to trim the
> > > > > > number of enabled drivers, make them all built-in, and to remove
> > > > > > CONFIG_MODULES=y. The end result of this is that the pin muxing
> > > > > > required for I2C to work was not being applied, leaving the interface
> > > > > > talking to thin air.
> > > > > >
> > > > > > I eventually traced the failure back to this change:
> > > > > > https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/drivers/pinctrl/devicetree.c?h=next-20250829&id=d19c5e79d46efdf89306be99f3c8824cf58e35f6
> > > > > > It introduces a requirement for CONFIG_MODULES to be enabled in order
> > > > > > to get an EPROBE_DEFER in the event that the pinctrl driver has not
> > > > > > yet been probed. Without CONFIG_MODULES, the pinctrl requirements are
> > > > > > waived. Removing the IS_ENABLED(CONFIG_MODULES) clause allows the
> > > > > > probing of the I2C driver to be deferred, fixing the user's problem.
> > > > > >
> > > > > > Is this requirement for supporting modules reasonable?
> > > > >
> > > > > That's not the requirement. If CONFIG_MODULES=n, then we only defer
> > > > > probes until late_initcall because after that point no more drivers
> > > > > are going to load. If CONFIG_MODULES=y, then deferring probe is based
> > > > > on a timeout which can be disabled.
> > > >
> > > > Thanks for replying. I'm probably missing something here, but if the
> > > > pinctrl and I2C drivers are both regular platform drivers, what is to
> > > > stop the I2C driver being probed first?
> > >
> > > Nothing, but it should defer unless you've reached late initcall or
> > > you've set "pinctrl-use-default".
> >
> > From the "next" tree:
> >
> > if (!np_pctldev || of_node_is_root(np_pctldev)) {
> >         of_node_put(np_pctldev);
> >         ret = -ENODEV;
> >         /* keep deferring if modules are enabled */
> >         if (IS_ENABLED(CONFIG_MODULES) && !allow_default && ret < 0)
> >                 ret = -EPROBE_DEFER;
> >         return ret;
> > }
> >
> > Unless CONFIG_MODULES=y you get ENODEV.
>
> Indeed, as 'ret = driver_deferred_probe_check_state(p->dev);' is gone due to:
>
> commit 24a026f85241a01bbcfe1b263caeeaa9a79bab40
> Author: Saravana Kannan <saravanak@google.com>
> Date:   Wed Jun 1 00:06:58 2022 -0700
>
>     pinctrl: devicetree: Delete usage of driver_deferred_probe_check_state()
>
>     Now that fw_devlink=on by default and fw_devlink supports
>     "pinctrl-[0-8]" property, the execution will never get to the point
>     where driver_deferred_probe_check_state() is called before the supplier
>     has probed successfully or before deferred probe timeout has expired.
>
>     So, delete the call and replace it with -ENODEV.
>
>     Tested-by: Geert Uytterhoeven <geert+renesas@glider.be>
>     Signed-off-by: Saravana Kannan <saravanak@google.com>
>     Link: https://lore.kernel.org/r/20220601070707.3946847-3-saravanak@google.com
>     Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
>
>
> So fw_devlink will ensure that pinctrl probes first. Is that turned
> off by chance (kernel cmdline is the only way to disable).

No, I don't think it's turned off. After a bit more digging I'm
starting to think that of_fwnode_add_links doesn't work in the pinctrl
case because the pinctrl-n properties are links to children of the
pinctrl controller, not the controller itself. Perhaps they're not
simple enough to be declared with DEFINE_SIMPLE_PROP and need a custom
parser that returns the parent instead?

Phil

Re: pinctrl, probe order, and CONFIG_MODULES

[Linus Walleij]

On Fri, Aug 29, 2025 at 5:51 PM Phil Elwell <phil@raspberrypi.com> wrote:
> On Fri, 29 Aug 2025 at 16:01, Rob Herring <robh@kernel.org> wrote:
> > On Fri, Aug 29, 2025 at 9:52 AM Phil Elwell <phil@raspberrypi.com> wrote:
> > > On Fri, 29 Aug 2025 at 15:44, Rob Herring <robh@kernel.org> wrote:
> > > >
> > > > On Fri, Aug 29, 2025 at 8:35 AM Phil Elwell <phil@raspberrypi.com> wrote:
> > > > >
> > > > > Hi Rob,
> > > > >
> > > > > On Fri, 29 Aug 2025 at 14:25, Rob Herring <robh@kernel.org> wrote:
> > > > > >
> > > > > > On Fri, Aug 29, 2025 at 5:37 AM Phil Elwell <phil@raspberrypi.com> wrote:
> > > > > > >
> > > > > > > Hi,
> > > > > > >
> > > > > > > A Raspberry Pi user recently asked us why I2C wasn't working in their
> > > > > > > custom kernel build. The primary change they had made was to trim the
> > > > > > > number of enabled drivers, make them all built-in, and to remove
> > > > > > > CONFIG_MODULES=y. The end result of this is that the pin muxing
> > > > > > > required for I2C to work was not being applied, leaving the interface
> > > > > > > talking to thin air.
> > > > > > >
> > > > > > > I eventually traced the failure back to this change:
> > > > > > > https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/drivers/pinctrl/devicetree.c?h=next-20250829&id=d19c5e79d46efdf89306be99f3c8824cf58e35f6
> > > > > > > It introduces a requirement for CONFIG_MODULES to be enabled in order
> > > > > > > to get an EPROBE_DEFER in the event that the pinctrl driver has not
> > > > > > > yet been probed. Without CONFIG_MODULES, the pinctrl requirements are
> > > > > > > waived. Removing the IS_ENABLED(CONFIG_MODULES) clause allows the
> > > > > > > probing of the I2C driver to be deferred, fixing the user's problem.
> > > > > > >
> > > > > > > Is this requirement for supporting modules reasonable?
> > > > > >
> > > > > > That's not the requirement. If CONFIG_MODULES=n, then we only defer
> > > > > > probes until late_initcall because after that point no more drivers
> > > > > > are going to load. If CONFIG_MODULES=y, then deferring probe is based
> > > > > > on a timeout which can be disabled.
> > > > >
> > > > > Thanks for replying. I'm probably missing something here, but if the
> > > > > pinctrl and I2C drivers are both regular platform drivers, what is to
> > > > > stop the I2C driver being probed first?
> > > >
> > > > Nothing, but it should defer unless you've reached late initcall or
> > > > you've set "pinctrl-use-default".
> > >
> > > From the "next" tree:
> > >
> > > if (!np_pctldev || of_node_is_root(np_pctldev)) {
> > >         of_node_put(np_pctldev);
> > >         ret = -ENODEV;
> > >         /* keep deferring if modules are enabled */
> > >         if (IS_ENABLED(CONFIG_MODULES) && !allow_default && ret < 0)
> > >                 ret = -EPROBE_DEFER;
> > >         return ret;
> > > }
> > >
> > > Unless CONFIG_MODULES=y you get ENODEV.
> >
> > Indeed, as 'ret = driver_deferred_probe_check_state(p->dev);' is gone due to:
> >
> > commit 24a026f85241a01bbcfe1b263caeeaa9a79bab40
> > Author: Saravana Kannan <saravanak@google.com>
> > Date:   Wed Jun 1 00:06:58 2022 -0700
> >
> >     pinctrl: devicetree: Delete usage of driver_deferred_probe_check_state()
> >
> >     Now that fw_devlink=on by default and fw_devlink supports
> >     "pinctrl-[0-8]" property, the execution will never get to the point
> >     where driver_deferred_probe_check_state() is called before the supplier
> >     has probed successfully or before deferred probe timeout has expired.
> >
> >     So, delete the call and replace it with -ENODEV.
> >
> >     Tested-by: Geert Uytterhoeven <geert+renesas@glider.be>
> >     Signed-off-by: Saravana Kannan <saravanak@google.com>
> >     Link: https://lore.kernel.org/r/20220601070707.3946847-3-saravanak@google.com
> >     Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> >
> >
> > So fw_devlink will ensure that pinctrl probes first. Is that turned
> > off by chance (kernel cmdline is the only way to disable).
>
> No, I don't think it's turned off. After a bit more digging I'm
> starting to think that of_fwnode_add_links doesn't work in the pinctrl
> case because the pinctrl-n properties are links to children of the
> pinctrl controller, not the controller itself. Perhaps they're not
> simple enough to be declared with DEFINE_SIMPLE_PROP and need a custom
> parser that returns the parent instead?

Have you tried setting this property in the pin controller?

$ git grep link_consumers drivers/pinctrl/
drivers/pinctrl/core.c: if (pctldev->desc->link_consumers)
drivers/pinctrl/pinctrl-stmfx.c:        pctl->pctl_desc.link_consumers = true;
drivers/pinctrl/stm32/pinctrl-stm32.c:  pctl->pctl_desc.link_consumers = true;

Maybe it should be the default.

5+ years ago I patched something like this but thought it was
too invasive:
https://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-pinctrl.git/commit/?h=consumer-link-enforce&id=73441cf773ed91bff0e7f66614d391b2514188bf

I don't know if it has something to do with it?

Yours,
Linus Walleij

Re: pinctrl, probe order, and CONFIG_MODULES

[Phil Elwell]

Hi Linus, Rob,

On Fri, 29 Aug 2025 at 23:43, Linus Walleij <linus.walleij@linaro.org> wrote:
>
> On Fri, Aug 29, 2025 at 5:51 PM Phil Elwell <phil@raspberrypi.com> wrote:
> > On Fri, 29 Aug 2025 at 16:01, Rob Herring <robh@kernel.org> wrote:
> > > On Fri, Aug 29, 2025 at 9:52 AM Phil Elwell <phil@raspberrypi.com> wrote:
> > > > On Fri, 29 Aug 2025 at 15:44, Rob Herring <robh@kernel.org> wrote:
> > > > >
> > > > > On Fri, Aug 29, 2025 at 8:35 AM Phil Elwell <phil@raspberrypi.com> wrote:
> > > > > >
> > > > > > Hi Rob,
> > > > > >
> > > > > > On Fri, 29 Aug 2025 at 14:25, Rob Herring <robh@kernel.org> wrote:
> > > > > > >
> > > > > > > On Fri, Aug 29, 2025 at 5:37 AM Phil Elwell <phil@raspberrypi.com> wrote:
> > > > > > > >
> > > > > > > > Hi,
> > > > > > > >
> > > > > > > > A Raspberry Pi user recently asked us why I2C wasn't working in their
> > > > > > > > custom kernel build. The primary change they had made was to trim the
> > > > > > > > number of enabled drivers, make them all built-in, and to remove
> > > > > > > > CONFIG_MODULES=y. The end result of this is that the pin muxing
> > > > > > > > required for I2C to work was not being applied, leaving the interface
> > > > > > > > talking to thin air.
> > > > > > > >
> > > > > > > > I eventually traced the failure back to this change:
> > > > > > > > https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/drivers/pinctrl/devicetree.c?h=next-20250829&id=d19c5e79d46efdf89306be99f3c8824cf58e35f6
> > > > > > > > It introduces a requirement for CONFIG_MODULES to be enabled in order
> > > > > > > > to get an EPROBE_DEFER in the event that the pinctrl driver has not
> > > > > > > > yet been probed. Without CONFIG_MODULES, the pinctrl requirements are
> > > > > > > > waived. Removing the IS_ENABLED(CONFIG_MODULES) clause allows the
> > > > > > > > probing of the I2C driver to be deferred, fixing the user's problem.
> > > > > > > >
> > > > > > > > Is this requirement for supporting modules reasonable?
> > > > > > >
> > > > > > > That's not the requirement. If CONFIG_MODULES=n, then we only defer
> > > > > > > probes until late_initcall because after that point no more drivers
> > > > > > > are going to load. If CONFIG_MODULES=y, then deferring probe is based
> > > > > > > on a timeout which can be disabled.
> > > > > >
> > > > > > Thanks for replying. I'm probably missing something here, but if the
> > > > > > pinctrl and I2C drivers are both regular platform drivers, what is to
> > > > > > stop the I2C driver being probed first?
> > > > >
> > > > > Nothing, but it should defer unless you've reached late initcall or
> > > > > you've set "pinctrl-use-default".
> > > >
> > > > From the "next" tree:
> > > >
> > > > if (!np_pctldev || of_node_is_root(np_pctldev)) {
> > > >         of_node_put(np_pctldev);
> > > >         ret = -ENODEV;
> > > >         /* keep deferring if modules are enabled */
> > > >         if (IS_ENABLED(CONFIG_MODULES) && !allow_default && ret < 0)
> > > >                 ret = -EPROBE_DEFER;
> > > >         return ret;
> > > > }
> > > >
> > > > Unless CONFIG_MODULES=y you get ENODEV.
> > >
> > > Indeed, as 'ret = driver_deferred_probe_check_state(p->dev);' is gone due to:
> > >
> > > commit 24a026f85241a01bbcfe1b263caeeaa9a79bab40
> > > Author: Saravana Kannan <saravanak@google.com>
> > > Date:   Wed Jun 1 00:06:58 2022 -0700
> > >
> > >     pinctrl: devicetree: Delete usage of driver_deferred_probe_check_state()
> > >
> > >     Now that fw_devlink=on by default and fw_devlink supports
> > >     "pinctrl-[0-8]" property, the execution will never get to the point
> > >     where driver_deferred_probe_check_state() is called before the supplier
> > >     has probed successfully or before deferred probe timeout has expired.
> > >
> > >     So, delete the call and replace it with -ENODEV.
> > >
> > >     Tested-by: Geert Uytterhoeven <geert+renesas@glider.be>
> > >     Signed-off-by: Saravana Kannan <saravanak@google.com>
> > >     Link: https://lore.kernel.org/r/20220601070707.3946847-3-saravanak@google.com
> > >     Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> > >
> > >
> > > So fw_devlink will ensure that pinctrl probes first. Is that turned
> > > off by chance (kernel cmdline is the only way to disable).
> >
> > No, I don't think it's turned off. After a bit more digging I'm
> > starting to think that of_fwnode_add_links doesn't work in the pinctrl
> > case because the pinctrl-n properties are links to children of the
> > pinctrl controller, not the controller itself. Perhaps they're not
> > simple enough to be declared with DEFINE_SIMPLE_PROP and need a custom
> > parser that returns the parent instead?
>
> Have you tried setting this property in the pin controller?
>
> $ git grep link_consumers drivers/pinctrl/
> drivers/pinctrl/core.c: if (pctldev->desc->link_consumers)
> drivers/pinctrl/pinctrl-stmfx.c:        pctl->pctl_desc.link_consumers = true;
> drivers/pinctrl/stm32/pinctrl-stm32.c:  pctl->pctl_desc.link_consumers = true;
>
> Maybe it should be the default.
>
> 5+ years ago I patched something like this but thought it was
> too invasive:
> https://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-pinctrl.git/commit/?h=consumer-link-enforce&id=73441cf773ed91bff0e7f66614d391b2514188bf
>
> I don't know if it has something to do with it?

Using the information you've provided, further investigation uncovered
that the conversion of the supplier/consumer relationships from fwlink
to device link was rejecting many of the dependencies because an
ancestor was marked as initialised but with no device pointer. The
ancestor in this case is the RP1 I/O chip used on Raspberry Pi 5 - a
PCIe device whose content is described in Device Tree. Ensuring that
the device pointer is initialised allows the device link feature to
work as expected. It remains to be seen if the upstream RP1 driver
will need a similar patch for the no-modules corner case.

Thanks for your help,

Phil