Re: [PATCH v7 2/2] bus: simple-pm-bus: Populate child nodes at probe

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The patch resolves fw_devlink corruption for transparent buses matched via `of_match_table`, but fails to apply this fix to transparent buses bound via `driver_override`, leaving devlinks broken in that path.
--

commit 428668c33cfe17b55dfc398a9e43567a082e1c3e
Author: Herve Codina <herve.codina@bootlin.com>

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

This commit modifies the simple-pm-bus driver to populate child nodes
during probe for transparent buses matched via of_match_table. This prevents
wrong or missing devlinks when fw_devlink assumes a bound driver will
populate its child firmware nodes.

> diff --git a/drivers/bus/simple-pm-bus.c b/drivers/bus/simple-pm-bus.c
> index 4b2a793dfbd90..d969bf19a4e95 100644
> --- a/drivers/bus/simple-pm-bus.c
> +++ b/drivers/bus/simple-pm-bus.c
> @@ -42,14 +42,15 @@ static int simple_pm_bus_probe(struct platform_device *pdev)
>  	match = of_match_device(dev->driver->of_match_table, dev);
>  	/*
>  	 * These are transparent bus devices (not simple-pm-bus matches) that
[ ... ]
>  	if (match && match->data) {
>  		if (of_property_match_string(np, "compatible", match->compatible) == 0)
> -			return 0;
> +			goto populate;
>  		else
>  			return -ENODEV;
>  	}

Does this leave fw_devlink corrupted when the driver is bound using
driver_override?

Looking at simple_pm_bus_probe(), there is an early exit for driver_override
just before this block:

	if (device_has_driver_override(&pdev->dev))
		return 0;

Since it still returns directly instead of jumping to the populate label,
transparent buses bound via the driver_override sysfs attribute will skip
populating their child nodes. This seems to trigger the same wrong
supplier assignments from fw_devlink that this patch intended to fix.

> @@ -78,7 +80,7 @@ static void simple_pm_bus_remove(struct platform_device *pdev)
>  {
>  	const void *data = of_device_get_match_data(&pdev->dev);
>  
> -	if (device_has_driver_override(&pdev->dev) || data)
> +	if (device_has_driver_override(&pdev->dev))
>  		return;
>  
>  	dev_dbg(&pdev->dev, "%s\n", __func__);
> @@ -86,7 +88,8 @@ static void simple_pm_bus_remove(struct platform_device *pdev)
>  	if (pdev->dev.of_node)
>  		of_platform_depopulate(&pdev->dev);

If the driver_override path in probe() is updated to populate children,
should this remove() function also be updated to depopulate them?

Additionally, does checking device_has_driver_override() here risk unbalanced
cleanup? If userspace modifies the driver_override sysfs attribute after
probe() but before remove(), this check could evaluate differently during
removal. Since of_platform_depopulate() safely ignores unpopulated buses,
would it be better to call it unconditionally?

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