Re: [PATCH 04/11] pmdomain: core: Add a bus and a driver for genpd providers

Abel Vesa
2025-04-22T14:03:03Z

On 25-04-17 16:25:02, Ulf Hansson wrote:
> When we create a genpd via pm_genpd_init() we are initializing a
> corresponding struct device for it, but we don't add the device to any
> bus_type. It has not really been needed as the device is used as cookie to
> help us manage OPP tables.
>
> However, to prepare to make better use of the device let's add a new genpd
> provider bus_type and a corresponding genpd provider driver. Subsequent
> changes will make use of this.
>
> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
> ---
> drivers/pmdomain/core.c | 89 ++++++++++++++++++++++++++++++++++++++++-
> 1 file changed, 88 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/pmdomain/core.c b/drivers/pmdomain/core.c
> index 035b65563947..da51a61a974c 100644
> --- a/drivers/pmdomain/core.c
> +++ b/drivers/pmdomain/core.c
> @@ -27,6 +27,11 @@
> /* Provides a unique ID for each genpd device */
> static DEFINE_IDA(genpd_ida);
>
> +/* The parent for genpd_provider devices. */
> +static struct device genpd_provider_bus = {
> + .init_name = "genpd_provider",
> +};
> +
> #define GENPD_RETRY_MAX_MS 250 /* Approximate */
>
> #define GENPD_DEV_CALLBACK(genpd, type, callback, dev) \
> @@ -44,6 +49,14 @@ static DEFINE_IDA(genpd_ida);
> static LIST_HEAD(gpd_list);
> static DEFINE_MUTEX(gpd_list_lock);
>
> +#define to_genpd_provider_drv(d) container_of(d, struct genpd_provider_drv, drv)
> +
> +struct genpd_provider_drv {

I'd replace "provider" substring and expand drv to driver everywhere.

I think that's more in line with all other subsystems.

> + struct device_driver drv;
> + int (*probe)(struct device *dev);
> + void (*remove)(struct device *dev);
> +};
> +
> struct genpd_lock_ops {
> void (*lock)(struct generic_pm_domain *genpd);
> void (*lock_nested)(struct generic_pm_domain *genpd, int depth);
> @@ -2225,6 +2238,26 @@ static int genpd_set_default_power_state(struct generic_pm_domain *genpd)
> return 0;
> }
>
> +static int genpd_provider_bus_probe(struct device *dev)

... and then here drop the "provider" as well.

Other than that, LGTM:

Reviewed-by: Abel Vesa <abel.vesa@linaro.org>