On Fri, Aug 29, 2025 at 5:51 PM Phil Elwell <phil@xxxxxxxxxxxxxxx> wrote: > On 2025年8月29日 at 16:01, Rob Herring <robh@xxxxxxxxxx> wrote: > > On Fri, Aug 29, 2025 at 9:52 AM Phil Elwell <phil@xxxxxxxxxxxxxxx> wrote: > > > On 2025年8月29日 at 15:44, Rob Herring <robh@xxxxxxxxxx> wrote: > > > > > > > > On Fri, Aug 29, 2025 at 8:35 AM Phil Elwell <phil@xxxxxxxxxxxxxxx> wrote: > > > > > > > > > > Hi Rob, > > > > > > > > > > On 2025年8月29日 at 14:25, Rob Herring <robh@xxxxxxxxxx> wrote: > > > > > > > > > > > > On Fri, Aug 29, 2025 at 5:37 AM Phil Elwell <phil@xxxxxxxxxxxxxxx> 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@xxxxxxxxxx> > > 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@xxxxxxxxx> > > Signed-off-by: Saravana Kannan <saravanak@xxxxxxxxxx> > > Link: https://lore.kernel.org/r/20220601070707.3946847-3-saravanak@xxxxxxxxxx > > Signed-off-by: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx> > > > > > > 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