Re: [PATCH v4 01/29] Revert "treewide: Fix probing of devices in DT overlays"

Rob Herring
2025-11-27T01:54:47Z

On Tue, Nov 25, 2025 at 12:43 AM Matti Vaittinen
<mazziesaccount@gmail.com> wrote:
>
> On 24/11/2025 19:01, Rob Herring wrote:
> > On Mon, Nov 24, 2025 at 10:44 AM Kalle Niemi <kaleposti@gmail.com> wrote:
> >>
> >>
> >> On 11/24/25 16:53, Rob Herring wrote:
> >>> On Mon, Nov 24, 2025 at 8:48 AM Kalle Niemi <kaleposti@gmail.com> wrote:
> >>>> On 10/15/25 10:13, Herve Codina wrote:
> >>>>> From: Saravana Kannan <saravanak@google.com>
> >>>>>
> >>>>> This reverts commit 1a50d9403fb90cbe4dea0ec9fd0351d2ecbd8924.
> >>>>>
> >>>>> While the commit fixed fw_devlink overlay handling for one case, it
> >>>>> broke it for another case. So revert it and redo the fix in a separate
> >>>>> patch.
> >>>>>
> >>>>> Fixes: 1a50d9403fb9 ("treewide: Fix probing of devices in DT overlays")
> >>>>> Reported-by: Herve Codina <herve.codina@bootlin.com>
> >>>>> Closes: https://lore.kernel.org/lkml/CAMuHMdXEnSD4rRJ-o90x4OprUacN_rJgyo8x6=9F9rZ+-KzjOg@mail.gmail.com/
> >>>>> Closes: https://lore.kernel.org/all/20240221095137.616d2aaa@bootlin.com/
> >>>>> Closes: https://lore.kernel.org/lkml/20240312151835.29ef62a0@bootlin.com/
> >>>>> Signed-off-by: Saravana Kannan <saravanak@google.com>
> >>>>> Link: https://lore.kernel.org/lkml/20240411235623.1260061-2-saravanak@google.com/
> >>>>> Signed-off-by: Herve Codina <herve.codina@bootlin.com>
> >>>>> Acked-by: Mark Brown <broonie@kernel.org>
> >>>>> ---
> >>>>> drivers/bus/imx-weim.c | 6 ------
> >>>>> drivers/i2c/i2c-core-of.c | 5 -----
> >>>>> drivers/of/dynamic.c | 1 -
> >>>>> drivers/of/platform.c | 5 -----
> >>>>> drivers/spi/spi.c | 5 -----
> >>>>> 5 files changed, 22 deletions(-)
> >>>>>
> >>>>> diff --git a/drivers/bus/imx-weim.c b/drivers/bus/imx-weim.c
> >>>>> index 83d623d97f5f..87070155b057 100644
> >>>>> --- a/drivers/bus/imx-weim.c
> >>>>> +++ b/drivers/bus/imx-weim.c
> >>>>> @@ -327,12 +327,6 @@ static int of_weim_notify(struct notifier_block *nb, unsigned long action,
> >>>>> "Failed to setup timing for '%pOF'\n", rd->dn);
> >>>>>
> >>>>> if (!of_node_check_flag(rd->dn, OF_POPULATED)) {
> >>>>> - /*
> >>>>> - * Clear the flag before adding the device so that
> >>>>> - * fw_devlink doesn't skip adding consumers to this
> >>>>> - * device.
> >>>>> - */
> >>>>> - rd->dn->fwnode.flags &= ~FWNODE_FLAG_NOT_DEVICE;
> >>>>> if (!of_platform_device_create(rd->dn, NULL, &pdev->dev)) {
> >>>>> dev_err(&pdev->dev,
> >>>>> "Failed to create child device '%pOF'\n",
> >>>>> diff --git a/drivers/i2c/i2c-core-of.c b/drivers/i2c/i2c-core-of.c
> >>>>> index eb7fb202355f..30b48a428c0b 100644
> >>>>> --- a/drivers/i2c/i2c-core-of.c
> >>>>> +++ b/drivers/i2c/i2c-core-of.c
> >>>>> @@ -176,11 +176,6 @@ static int of_i2c_notify(struct notifier_block *nb, unsigned long action,
> >>>>> return NOTIFY_OK;
> >>>>> }
> >>>>>
> >>>>> - /*
> >>>>> - * Clear the flag before adding the device so that fw_devlink
> >>>>> - * doesn't skip adding consumers to this device.
> >>>>> - */
> >>>>> - rd->dn->fwnode.flags &= ~FWNODE_FLAG_NOT_DEVICE;
> >>>>> client = of_i2c_register_device(adap, rd->dn);
> >>>>> if (IS_ERR(client)) {
> >>>>> dev_err(&adap->dev, "failed to create client for '%pOF'\n",
> >>>>> diff --git a/drivers/of/dynamic.c b/drivers/of/dynamic.c
> >>>>> index 2eaaddcb0ec4..b5be7484fb36 100644
> >>>>> --- a/drivers/of/dynamic.c
> >>>>> +++ b/drivers/of/dynamic.c
> >>>>> @@ -225,7 +225,6 @@ static void __of_attach_node(struct device_node *np)
> >>>>> np->sibling = np->parent->child;
> >>>>> np->parent->child = np;
> >>>>> of_node_clear_flag(np, OF_DETACHED);
> >>>>> - np->fwnode.flags |= FWNODE_FLAG_NOT_DEVICE;
> >>>>>
> >>>>> raw_spin_unlock_irqrestore(&devtree_lock, flags);
> >>>>>
> >>>>> diff --git a/drivers/of/platform.c b/drivers/of/platform.c
> >>>>> index f77cb19973a5..ef9445ba168b 100644
> >>>>> --- a/drivers/of/platform.c
> >>>>> +++ b/drivers/of/platform.c
> >>>>> @@ -739,11 +739,6 @@ static int of_platform_notify(struct notifier_block *nb,
> >>>>> if (of_node_check_flag(rd->dn, OF_POPULATED))
> >>>>> return NOTIFY_OK;
> >>>>>
> >>>>> - /*
> >>>>> - * Clear the flag before adding the device so that fw_devlink
> >>>>> - * doesn't skip adding consumers to this device.
> >>>>> - */
> >>>>> - rd->dn->fwnode.flags &= ~FWNODE_FLAG_NOT_DEVICE;
> >>>>> /* pdev_parent may be NULL when no bus platform device */
> >>>>> pdev_parent = of_find_device_by_node(parent);
> >>>>> pdev = of_platform_device_create(rd->dn, NULL,
> >>>>> diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
> >>>>> index 2e0647a06890..b22944a207c9 100644
> >>>>> --- a/drivers/spi/spi.c
> >>>>> +++ b/drivers/spi/spi.c
> >>>>> @@ -4791,11 +4791,6 @@ static int of_spi_notify(struct notifier_block *nb, unsigned long action,
> >>>>> return NOTIFY_OK;
> >>>>> }
> >>>>>
> >>>>> - /*
> >>>>> - * Clear the flag before adding the device so that fw_devlink
> >>>>> - * doesn't skip adding consumers to this device.
> >>>>> - */
> >>>>> - rd->dn->fwnode.flags &= ~FWNODE_FLAG_NOT_DEVICE;
> >>>>> spi = of_register_spi_device(ctlr, rd->dn);
> >>>>> put_device(&ctlr->dev);
> >>>>>
> >>>> Sorry, some of you will receive this message now for second time. First
> >>>> message was sent to older series of patches.
> >>>> -
> >>>>
> >>>> Hello,
> >>>>
> >>>> Test system testing drivers for ROHM ICs bisected this commit to cause
> >>>> BD71847 drivers probe to not be called.
> >>> This driver (and overlay support) is in linux-next or something out of
> >>> tree on top of linux-next?
> >>>
> >>> Rob
> >>
> >> Yes the driver is in mainline linux: /drivers/mfd/rohm-bd718x7.c
> >
> > I don't see any support to apply overlays in that driver.
>
> Ah. Sorry for the confusion peeps. I asked Kalle to report this without
> proper consideration. 100% my bad.
>
> While the bd718x7 drive indeed is mainline (and tested), the actual
> 'glue-code' doing the overlay is part of the downstream test
> infrastructure. So yes, this is not a bug in upstream kernel - this
> falls in the category of an upstream change causing downstream things to
> break. So, feel free to say: "Go fix your code" :)
>
> Now that this is sorted, if someone is still interested in helping us to
> get our upstream drivers tested - the downstream piece is just taking
> the compiled device-tree overlay at runtime (via bin-attribute file),
> and applying it using the of_overlay_fdt_apply(). The approach is
> working for our testing purposes when the device is added to I2C/SPI
> node which is already enabled. However, in case where we have the I2C
> disabled, and enable it in the same overlay where we add the new device
> - then the new device does not get probed.
>
> I would be really grateful if someone had a pointer for us.

Seems to be fw_devlink related. I suppose if you turn it off it works?
There's info about the dependencies in sysfs or maybe debugfs. I don't
remember the details, but that should help to tell you why things
aren't probing.

I've dropped the changes for 6.18 for now. No one really seems to be
in need of them yet AFAICT.

Rob