Re: [PATCH v4 5/8] pmdomain: Make genpd_get_from_provider() return -EPROBE_DEFER

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] pmdomain: unnecessary probe deferral on invalid domain index
--

commit c105920d803c0cf9396fb90d8428fdc4d83e8655
Author: Geert Uytterhoeven <geert+renesas@glider.be>

pmdomain: Make genpd_get_from_provider() return -EPROBE_DEFER

Unlike provider-based look-up functions in other subsystems[1],
genpd_get_from_provider() returns -ENOENT instead of -EPROBE_DEFER when
the provider is not found, thus not supporting probe deferral.
Currently this is worked around in of_genpd_add_subdomain() by remapping
-ENOENT[2].

Fix this for all current and future callers by returning -EPROBE_DEFER
instead, and drop the no longer needed remapping code.

> diff --git a/drivers/pmdomain/core.c b/drivers/pmdomain/core.c
> index 964e44c2f78f1..88c886814a5b9 100644
> --- a/drivers/pmdomain/core.c
> +++ b/drivers/pmdomain/core.c
> @@ -3021,7 +3021,7 @@ EXPORT_SYMBOL_GPL(of_genpd_del_provider);
>  static struct generic_pm_domain *genpd_get_from_provider(
>  					const struct of_phandle_args *genpdspec)
>  {
> -	struct generic_pm_domain *genpd = ERR_PTR(-ENOENT);
> +	struct generic_pm_domain *genpd = ERR_PTR(-EPROBE_DEFER);
>  	struct of_genpd_provider *provider;
>  
>  	if (!genpdspec)

[ ... ]

> @@ -3276,7 +3276,7 @@ int of_genpd_add_subdomain(const struct of_phandle_args *parent_spec,
>  out:
>  	mutex_unlock(&gpd_list_lock);
>  
> -	return ret == -ENOENT ? -EPROBE_DEFER : ret;
> +	return ret;
>  }

[Severity: Low]
Does this change leave an unnecessary probe deferral in __genpd_dev_pm_attach()?

If a device specifies a valid PM domain provider but an invalid domain index,
genpd_get_from_provider() returns the specific error code from the provider's
xlate callback, such as -ENOENT.

However, __genpd_dev_pm_attach() still unconditionally routes all errors
through driver_deferred_probe_check_state():

drivers/pmdomain/core.c:__genpd_dev_pm_attach() {
	...
	pd = genpd_get_from_provider(&pd_args);
	of_node_put(pd_args.np);
	if (IS_ERR(pd)) {
		mutex_unlock(&gpd_list_lock);
		dev_dbg(dev, "%s() failed to find PM domain: %ld\n",
			__func__, PTR_ERR(pd));
		return driver_deferred_probe_check_state(base_dev);
	}
	...
}

Could this unnecessarily delay boot and error reporting for devices with
permanently invalid PM domain IDs by forcing a wait until late_initcall?

Should __genpd_dev_pm_attach() be updated to only call
driver_deferred_probe_check_state() when PTR_ERR(pd) is -EPROBE_DEFER?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1792163045.git.geert+renesas@glider.be?part=5