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

[Ulf Hansson]

On Tue, 22 Apr 2025 at 16:03, Abel Vesa <abel.vesa at linaro.org> wrote:
>
> 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 at 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.

I understand your point, but it's not that straight-forward to find a
proper name this time.

We already have another bus_type for genpd consumer devices (virtual
devices created when attaching a device to one of its multiple PM
domains). That bus is already named "genpd".

>
> > +     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.

For the reason I pointed out above, I decided to use "provider" in the
bus/driver's functions names to have a clear difference from the
"consumer" genpd bus.

I am worried that if we don't use "provider" we will mix up things
with the existing genpd bus. Maybe there is a better option?

>
> Other than that, LGTM:
>
> Reviewed-by: Abel Vesa <abel.vesa at linaro.org>

Thanks for reviewing!

Kind regards
Uffe