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

[Abel Vesa <abel.vesa@linaro.org>]

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>