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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAPDyKFqftAzxhoSo=A7Eu_0ueK6_BgMs+-mCZy=jQdm9aXQMog@mail.gmail.com>
Date: Wed, 23 Apr 2025 09:34:34 +0200
From: Ulf Hansson <ulf.hansson@...aro.org>
To: Abel Vesa <abel.vesa@...aro.org>
Cc: Saravana Kannan <saravanak@...gle.com>, Stephen Boyd <sboyd@...nel.org>, linux-pm@...r.kernel.org, 
	"Rafael J . Wysocki" <rafael@...nel.org>, Greg Kroah-Hartman <gregkh@...uxfoundation.org>, 
	Michael Grzeschik <m.grzeschik@...gutronix.de>, Bjorn Andersson <andersson@...nel.org>, 
	Devarsh Thakkar <devarsht@...v0571a.ent.ti.com>, Peng Fan <peng.fan@....nxp.com>, 
	Tomi Valkeinen <tomi.valkeinen@...asonboard.com>, Johan Hovold <johan@...nel.org>, 
	Maulik Shah <maulik.shah@....qualcomm.com>, linux-arm-kernel@...ts.infradead.org, 
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH 04/11] pmdomain: core: Add a bus and a driver for genpd providers

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

Thanks for reviewing!

Kind regards
Uffe