Hi Rob,

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

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.

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

Best regards,
Hervé