Message-ID: <20251211132044.10f5b1ea@bootlin.com>
Date: Thu, 11 Dec 2025 13:20:44 +0100
From: Herve Codina <herve.codina@...tlin.com>
To: Matti Vaittinen <mazziesaccount@...il.com>
Cc: Geert Uytterhoeven <geert@...ux-m68k.org>, Kalle Niemi
 <kaleposti@...il.com>, Rob Herring <robh@...nel.org>,
 linux-arm-kernel@...ts.infradead.org, Andrew Lunn <andrew@...n.ch>,
 Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor Dooley
 <conor+dt@...nel.org>, Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
 "Rafael J. Wysocki" <rafael@...nel.org>, Danilo Krummrich
 <dakr@...nel.org>, Shawn Guo <shawnguo@...nel.org>, Sascha Hauer
 <s.hauer@...gutronix.de>, Pengutronix Kernel Team <kernel@...gutronix.de>,
 Fabio Estevam <festevam@...il.com>, Michael Turquette
 <mturquette@...libre.com>, Stephen Boyd <sboyd@...nel.org>, Andi Shyti
 <andi.shyti@...nel.org>, Wolfram Sang <wsa+renesas@...g-engineering.com>,
 Peter Rosin <peda@...ntia.se>, Arnd Bergmann <arnd@...db.de>, Bjorn Helgaas
 <bhelgaas@...gle.com>, Charles Keepax <ckeepax@...nsource.cirrus.com>,
 Richard Fitzgerald <rf@...nsource.cirrus.com>, David Rhodes
 <david.rhodes@...rus.com>, Linus Walleij <linus.walleij@...aro.org>, Ulf
 Hansson <ulf.hansson@...aro.org>, Mark Brown <broonie@...nel.org>, Andy
 Shevchenko <andriy.shevchenko@...ux.intel.com>, Daniel Scally
 <djrscally@...il.com>, Heikki Krogerus <heikki.krogerus@...ux.intel.com>,
 Sakari Ailus <sakari.ailus@...ux.intel.com>, Len Brown <lenb@...nel.org>,
 Davidlohr Bueso <dave@...olabs.net>, Jonathan Cameron
 <jonathan.cameron@...wei.com>, Dave Jiang <dave.jiang@...el.com>, Alison
 Schofield <alison.schofield@...el.com>, Vishal Verma
 <vishal.l.verma@...el.com>, Ira Weiny <ira.weiny@...el.com>, Dan Williams
 <dan.j.williams@...el.com>, Wolfram Sang <wsa@...nel.org>,
 devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
 imx@...ts.linux.dev, linux-clk@...r.kernel.org, linux-i2c@...r.kernel.org,
 linux-pci@...r.kernel.org, linux-sound@...r.kernel.org,
 patches@...nsource.cirrus.com, linux-gpio@...r.kernel.org,
 linux-pm@...r.kernel.org, linux-spi@...r.kernel.org,
 linux-acpi@...r.kernel.org, linux-cxl@...r.kernel.org, Allan Nielsen
 <allan.nielsen@...rochip.com>, Horatiu Vultur
 <horatiu.vultur@...rochip.com>, Steen Hegelund
 <steen.hegelund@...rochip.com>, Luca Ceresoli <luca.ceresoli@...tlin.com>,
 Thomas Petazzoni <thomas.petazzoni@...tlin.com>
Subject: Re: [PATCH v4 01/29] Revert "treewide: Fix probing of devices in DT
 overlays"

Hi Matti,

On Thu, 11 Dec 2025 10:34:46 +0200
Matti Vaittinen <mazziesaccount@...il.com> wrote:

> Hi Dee Ho peeps,
> 
> I tried to create a minimal piece of code/dts to demonstrate the issue 
> seem in the ROHM automated testing.
> 
> On 10/12/2025 14:21, Herve Codina wrote:
> > Hi Geert, Kalle, Rob,
> > 
> > On Thu, 4 Dec 2025 11:49:13 +0100
> > Geert Uytterhoeven <geert@...ux-m68k.org> wrote:  
> 
> //snip
> 
> > When a new node is added, a new device is created. Indeed, because the
> > driver is an MFD driver, it is a bus driver and handled by of_platform bus.  
> 
> We do also have an MFD device - but it is not a platform device but an 
> I2C device - thus it should be probed by the I2C bus (if I'm not 
> mistaken). So, I guess this is not bus-specific problem.
> https://elixir.bootlin.com/linux/v6.18/source/drivers/mfd/rohm-bd718x7.c#L206
> 
> 
> > My new node is considered by devlink as a node that will have a device ready
> > to work (driver attached and device probed). A link is created between this
> > node and the consumers of this node (i.e. the SPI controller). devlink is
> > waiting for this provider to be ready before allowing the its consumer to probe.
> > This node (simple pinmux description) will never lead to a device and devlink
> > will never see this "provider" ready.  
> 
> I believe Kalle did see the same "probe-not-called" -problem, even when 
> disabling the fw_devlink from the kernel commandline. (It's worth 
> mentioning that I am not sure if Kalle tried if probe was called with 
> "previously working" kernels when fw_devlink is disabled).
> 
> > Did a test with a Renesas RZ/N1D (r9a06g032) based board and built a similar
> > overlay involving I2C controller pinmux, I2C controller and an EEPROM.
> > 
> > Here, also the overlay didn't work but the issue is different.
> > 
> > The pinmux definition for pinctrl (i.e. pinctrl subnodes) are looked when
> > the pinctrl driver probes. Adding a new node later is not handled by the
> > pinctrl driver.
> > Applying the overlay leads to a simple:
> >    [   16.934168] rzn1-pinctrl 40067000.pinctrl: unable to find group for node /soc/pinctrl@...67000/pins_i2c2
> > 
> > Indeed, the 'pins_i2c2' has been added by the overlay and was not present
> > when the pinctrl probed.
> > 
> > Tried without adding a new pinmux node (pinctrl subnode) from the overlay
> > and used nodes already existing in the base DT.
> > 
> > On my Marvell Armada 3720 board, it works with or without my patches.
> > No regression detected due to my patches.
> > 
> > On my RZ/N1D board, it works also with or without my patches.
> > Here also, no regression detected.
> > 
> > Also, on my Marvell Armada 3720 board, I can plug my LAN966x PCI board.
> > The LAN966x PCI driver used an overlay to describe the LAN966x PCI board.
> > 
> > With the upstream patch not reverted, i.e. 1a50d9403fb9 ("treewide: Fix
> > probing of devices in DT overlays")" applied, devlinks created for the
> > LAN966x PCI board internal devices are incorrect and lead to crashes when
> > the LAN966x PCI driver is removed due to wrong provider/consumer dependencies.
> > 
> > When this patch is reverted and replaced by "of: dynamic: Fix overlayed
> > devices not probing because of fw_devlink", devlinks created for the LAN966x
> > PCI board internal devices are corrects and crashes are no more present on
> > removal.
> > 
> > Kalle, Geert, can you perform a test on your hardware with my patches
> > applied and moving your pinmux definition from the overlay to the base
> > device-tree?  
> 
> I got a bit lost regarding which patches to test :)

The next-20251127 tag has every patches needed for the test.
Tests you did with this kernel are perfectly valid. Many Thanks for that!

> 
> > The kernel you can use is for instance the kernel at the next-20251127 tag.
> > Needed patches for test are present in this kernel:
> >      - 76841259ac092 ("of: dynamic: Fix overlayed devices not probing because of fw_devlink")
> >      - 7d67ddc5f0148 ("Revert "treewide: Fix probing of devices in DT overlays"")
> >   
> 
> I did a minimal overlay test which can be ran on beaglebone black. I 
> assume the same can be done on any board where you have 
> (i2c/spi/xxx)-controller node with status="disabled". Doing this on BBB 
> requires recompiling the beaglebone black (base)device-tree with -@ 
> though, so that the overlay target nodes are found. I'll attach the 
> files for interested.
> 
> overlay-test.c:
> Is a 'device-driver' for device added in overlay. (simply a probe() with 
> print, extracted from the bd71847 driver).
> 
> overlay-test.dts:
> Is a minimal device-tree overlay describing the 'test device' matching 
> above overlay-test driver. When this is overlaid using next-20251121 
> (contains the 7d67ddc5f0148b3a03594a45bba5547e92640c89), probe in 
> overlay-test.c is not called. When 
> 7d67ddc5f0148b3a03594a45bba5547e92640c89 is reverted, the probe is called.
> 
> mva_overlay.c:
> Is simplified 'glue-code' for adding an overlay to running kernel by 
> feeding the compiled overlay to the bin_attribute - for example using:
> 
> dd if=/overlay-test.dtbo of=/sys/kernel/mva_overlay/overlay_add bs=4M
> 
> am335x-boneblack.dtb.dts.tmp and tps65217.dtsi:
> are (intermediate) beaglebone-black device-trees which can be recompiled 
> to a 'base device-tree' using:
> 
> dtc -O dtb -o am335x-boneblack.dtb -b 0 -@ am335x-boneblack.dtb.dts.tmp
>   - but I suggest you to use the dts from your kernel build. I provided 
> this just for the sake of the completeness.
> 
> Makefile:
> Off-tree build targets to build the above DTSes and modules. Requires 
> KERNEL_DIR and CC to be correctly set.
> 
> 
> My findings:
> The pinctrl node indeed plays a role. When the "pinctrl-0 = 
> <&i2c1_pins>;" (and fragment0) was removed from the dts, the 
> 'overlay-test' was probed with the "next-20251121".
> 
> With the pinctrl node, I see:
> [  104.098958] probe of 4802a000.i2c returned -517 (EPROBE_DEFER I 
> suppose) after 50 usecs
> - and the 'overlay-test' probe is not called.

Do you see the same trace with:
- "pinctrl-0 = <&i2c1_pins>;" in your overlay
- fragment0 removed from the overlay (i2c1_pins definition removed from
  the overlay.
- i2c1_pins node defined in your base DT.

In other word, is the issues related to adding a pinctrl sub-node (pinctrl
pins definition) in the overlay or is it something else?

Best regards,
Hervé

Powered by blists - more mailing lists