Hi Rob,

On Mon, 14 Jul 2025 12:44:22 -0500
Rob Herring [off-list ref] wrote:

On Fri, Jul 4, 2025 at 3:57 AM Herve Codina [off-list ref] wrote:

quoted

Hi Rob,

On Thu, 3 Jul 2025 09:33:02 +0200
Herve Codina [off-list ref] wrote:
 

quoted

Hi Rob,

On Fri, 27 Jun 2025 10:52:00 -0500
Rob Herring [off-list ref] wrote:
 

quoted

On Fri, Jun 13, 2025 at 03:47:45PM +0200, Herve Codina wrote:  

quoted

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?
 

quoted

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.  

The problem is how does a bus driver know if there is a specific MFD
driver or not? It doesn't. The MFD driver could be a module and loaded
any time later. We'd really need some sort of unbind the generic
driver and re-bind to a more specific driver when and if that driver
appears. We could perhaps have a list of devices with a driver because
in theory that should be a short list as the (broken) promise of
simple-mfd is the child nodes have no dependency on the parent node
which implies the parent doesn't have a driver. The specific
compatible is there in case that assumption turns out wrong.

Hum, I see.

In my use case, I don't use MFD drivers but only simple-bus compatible.
I think your point is also relevant with simple-bus. Indeed how does a
parent bus driver know if there is a specific bus driver that handles
the child simple-bus compatible one in case of 'simple-bus' used as
fallback.

Related to your proposal related to the "list of devices with a driver",
what do you mean? I don't see how to set this kind of list. Can you give
me some pointers?

If I understood the discussion, the issue seems that 'simple-bus' can't
populate unconditionally children at his probe. The possible recursion
in creating devices done by of_platform_populate() should be kept and
'simple-bus' should rely on that.

The other solution that fixes my use case is to use an other compatible
string. Would you accept a new compatible string: "simple-platform-bus"?

In simple-pm-bus.c, this compatible would populate children at probe.
In fact, it will act the same way as 'simple-pm-bus' without looking at
clocks nor handling pm_runtime.

Best regards,
Hervé