Message-ID: <20250704105725.50cb72b9@bootlin.com>
Date: Fri, 4 Jul 2025 10:57:25 +0200
From: Herve Codina <herve.codina@...tlin.com>
To: Rob Herring <robh@...nel.org>
Cc: Andrew Lunn <andrew@...n.ch>, 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>, Derek
 Kiernan <derek.kiernan@....com>, Dragan Cvetic <dragan.cvetic@....com>,
 Arnd Bergmann <arnd@...db.de>, Saravana Kannan <saravanak@...gle.com>,
 Bjorn Helgaas <bhelgaas@...gle.com>, Mark Brown <broonie@...nel.org>, Len
 Brown <lenb@...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>, Wolfram Sang <wsa@...nel.org>, Geert
 Uytterhoeven <geert+renesas@...der.be>, Davidlohr Bueso
 <dave@...olabs.net>, 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>,
 linux-kernel@...r.kernel.org, imx@...ts.linux.dev,
 linux-arm-kernel@...ts.infradead.org, linux-clk@...r.kernel.org,
 linux-i2c@...r.kernel.org, devicetree@...r.kernel.org,
 linux-pci@...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 v3 05/28] bus: simple-pm-bus: Populate child nodes at
 probe

Hi Rob,

On Thu, 3 Jul 2025 09:33:02 +0200
Herve Codina <herve.codina@...tlin.com> wrote:

> Hi Rob,
> 
> On Fri, 27 Jun 2025 10:52:00 -0500
> Rob Herring <robh@...nel.org> wrote:
> 
> > On Fri, Jun 13, 2025 at 03:47:45PM +0200, Herve Codina wrote:  
> > > The simple-pm-bus driver handles several simple busses. When it is used
> > > with busses other than a compatible "simple-pm-bus", it doesn't populate
> > > its child devices during its probe.
> > > 
> > > This confuses fw_devlink and results in wrong or missing devlinks.
> > > 
> > > Once a driver is bound to a device and the probe() has been called,
> > > device_links_driver_bound() is called.
> > > 
> > > This function performs operation based on the following assumption:
> > >     If a child firmware node of the bound device is not added as a
> > >     device, it will never be added.
> > > 
> > > Among operations done on fw_devlinks of those "never be added" devices,
> > > device_links_driver_bound() changes their supplier.
> > > 
> > > With devices attached to a simple-bus compatible device, this change
> > > leads to wrong devlinks where supplier of devices points to the device
> > > parent (i.e. simple-bus compatible device) instead of the device itself
> > > (i.e. simple-bus child).
> > > 
> > > When the device attached to the simple-bus is removed, because devlinks
> > > are not correct, its consumers are not removed first.
> > > 
> > > In order to have correct devlinks created, make the simple-pm-bus driver
> > > compliant with the devlink assumption and create its child devices
> > > during its probe.    
> > 
> > IIRC, skipping child nodes was because there were problems with 
> > letting the driver handle 'simple-bus'. How does this avoid that now?  
> 
> I don't know about the specific issues related to those problems. Do you
> have some pointers about them?
> 
> > 
> > The root of_platform_populate() that created the simple-bus device that 
> > gets us to the probe here will continue descending into child nodes. 
> > Meanwhile, the probe here is also descending into those same child 
> > nodes. Best case, that's just redundant. Worst case, won't you still 
> > have the same problem if the first of_platform_populate() creates the 
> > devices first?
> >   
> 
> Maybe we could simply avoid of_platform_populate() to be recursive when a
> device populate by of_platform_populate() is one of devices handled by
> the simple-bus driver and let the simple-bus driver do the job.
> 
> of_platform_populate will handle the first level. It will populate children
> of the node given to of_platform_populate() and the children of those
> children will be populate by the simple-bus driver.
> 
> I could try a modification in that way. Do you think it could be a correct
> solution?
> 

I have started to look at this solution and it's going to be more complex
than than I thought.

Many MFD drivers uses a compatible of this kind (the same exist for bus
driver with "simple-bus"):
  compatible = "foo,bar", "simple-mfd";

Usually the last compatible string ("simple-mfd" here) is a last fallback
and the first string is the more specific one.

In the problematic case, "foo,bar" has a specific driver and the driver
performs some operations at probe() but doesn't call of_platform_populate()
and relies on the core to do the device creations (recursively) based on
the "simple,mfd" string present in the compatible property.

Some other calls of_platform_populate() in they probe (which I think is
correct) and in that case, the child device creation can be done at two
location: specific driver probe() and core.

You pointed out that the core could create devices before the specific
driver is probed. In that case, some of existing drivers calling
of_platform_populate() are going to have issues.

I can try to modify existing MFD and bus drivers (compatible fallback to
simple-mfd, simple-bus, ...) in order to have them call of_platform_populate()
in they probe() and after all problematic drivers are converted, the
recursive creation of devices done in the core could be removed.

Before starting in that way, can you confirm that this is the right
direction?

Best regards,
Hervé

Powered by blists - more mailing lists