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

[Laurent Pinchart <laurent.pinchart@ideasonboard.com>]

Hi Geert,

Thank you for the patch.

On Fri, Oct 16, 2026 at 05:55:44PM +0200, Geert Uytterhoeven wrote:
> 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].

But not in the other functions calling genpd_get_from_provider() and
propagating the error up. Have you validated that no existing call
stacks will break ?

> Fix this for all current and future callers by returning -EPROBE_DEFER
> instead, and drop the no longer needed remapping code.
> 
> [1] of_clk_get_hw_from_clkspec(), __fwnode_reset_control_get()),
> [2] Commit 18027d6f392ee8d8 ("PM: domains: Make of_genpd_add_subdomain()
>     return -EPROBE_DEFER").
> 
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> ---
> v4:
>   - New.
> ---
>  drivers/pmdomain/core.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/pmdomain/core.c b/drivers/pmdomain/core.c
> index 964e44c2f78f1d55..88c886814a5b9da3 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)

Expanding the context:

>		return ERR_PTR(-EINVAL); 
>					
>	mutex_lock(&of_genpd_mutex);
>	
>	/* Check if we have such a provider in our array */
>	list_for_each_entry(provider, &of_genpd_providers, link) {
>		if (provider->node == genpdspec->np)
>			genpd = provider->xlate(genpdspec, provider->data);
>		if (!IS_ERR(genpd))
>			break;
>	}
>
>	mutex_unlock(&of_genpd_mutex);
>	
>	return genpd;

The for loop seems to imply that we can have multiple providers for the
same node. In case a provider fails, the function overrides the error
code from -EPROBE_DEFER to whatever the first xlate() returns. That may
cause issues for your use case.

If the of_genpd_providers list doesn't expect multiple providers per
device node, I would change the above into

	list_for_each_entry(provider, &of_genpd_providers, link) {
		if (provider->node == genpdspec->np) {
			genpd = provider->xlate(genpdspec, provider->data);
			break;
		}
	}

to make it explicit.

> @@ -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;
>  }
>  EXPORT_SYMBOL_GPL(of_genpd_add_subdomain);
>  

-- 
Regards,

Laurent Pinchart