Re: [PATCH v9 4/4] bus: simple-pm-bus: Populate child nodes at probe

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential
issues to consider:
- [High] bus: simple-pm-bus: unpopulated grandchildren in
  simple_pm_bus_populate()
- [Medium] bus: simple-pm-bus: auxdata lookup table dropped in
  simple_pm_bus_probe()
- [Low] bus: simple-pm-bus: missing Fixes trailer for commit
  98e96cf80045
--

commit 7ba99d9ea04b1e1a29eebc1e1525489b300fe107
Author: Herve Codina <herve.codina@bootlin.com>

bus: simple-pm-bus: Populate child nodes at probe

This commit updates simple_pm_bus_probe() to populate child devices for
transparent bus matches (such as simple-bus, simple-mfd, isa, and
arm,amba-bus) during probe, and skips runtime PM calls in
simple_pm_bus_depopulate() when runtime PM is not enabled.

[Severity: Low]
In the commit message, this patch fixes the probe behavior for bus-only
devices where returning 0 without populating children caused
device_links_driver_bound() to create wrong fw_devlinks. Should the commit
message include a Fixes tag for 98e96cf80045 ("drivers: bus: simple-pm-bus:
Add support for probing simple bus only devices")?

[Severity: High]
In drivers/bus/simple-pm-bus.c, simple_pm_bus_probe() at line 104 now calls
simple_pm_bus_populate(), which at line 64 calls of_platform_populate() with
NULL for the matches table instead of calling of_platform_default_populate().
Does this leave grandchild devices unpopulated when a child node has a
device-specific compatible string followed by a fallback bus or MFD
compatible string (such as compatible = "vendor,syscon", "syscon",
"simple-mfd")?

When an outer of_platform_default_populate() creates an ONLY_BUS device
after simple_pm_bus_driver is registered, of_platform_device_create_pdata()
synchronously runs simple_pm_bus_probe() before the outer
of_platform_bus_create() reaches its child loop. Inside
simple_pm_bus_populate(), of_platform_populate(np, NULL, lookup, dev)
creates the child platform_device and sets OF_POPULATED on the child node.
During device_add(), simple_pm_bus_probe() rejects that child with -ENODEV
at line 100 because the generic bus compatible is not first, and
of_platform_bus_create() returns 0 at drivers/of/platform.c line 371 without
recursing into the child's subnodes because matches is NULL. When control
returns to the outer of_platform_default_populate() loop,
of_platform_device_create_pdata() sees OF_POPULATED already set on the child
and returns NULL, so the outer loop also returns 0 without populating the
grandchildren. Calling of_platform_default_populate(np, lookup, dev) in
simple_pm_bus_populate() would pass the default match table and avoid
skipping those grandchildren.

[Severity: Medium]
In drivers/bus/simple-pm-bus.c, simple_pm_bus_probe() reads lookup from
dev_get_platdata(dev) at line 74 and passes it to simple_pm_bus_populate()
at line 104. Can this drop the caller's of_dev_auxdata lookup table for
devices beneath a simple-bus node?

When a caller such as sysc_probe() in drivers/bus/ti-sysc.c (line 3243)
calls of_platform_populate() with a match table that includes simple-bus and
a non-NULL auxdata lookup table, of_platform_bus_create() only attaches
platform_data to the simple-bus device itself if the auxdata table has an
explicit entry matching the simple-bus node. Otherwise dev_get_platdata(dev)
on the simple-bus device is NULL, and simple_pm_bus_probe() synchronously
populates all child devices of the simple-bus with lookup = NULL (setting
OF_POPULATED on each child) before the caller's of_platform_bus_create()
loop can recurse with the caller's auxdata table. As a result, child devices
under that simple-bus lose their auxdata bus_id and platform_data overrides.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261109140916.90625-1-herve.codina@bootlin.com?part=4