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 {
+ 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)
+{
+ struct genpd_provider_drv *drv = to_genpd_provider_drv(dev->driver);
+
+ return drv->probe(dev);
+}
+
+static void genpd_provider_bus_remove(struct device *dev)
+{
+ struct genpd_provider_drv *drv = to_genpd_provider_drv(dev->driver);
+
+ drv->remove(dev);
+}
+
+static const struct bus_type genpd_provider_bus_type = {
+ .name = "genpd_provider",
+ .probe = genpd_provider_bus_probe,
+ .remove = genpd_provider_bus_remove,
+};
+
static void genpd_provider_release(struct device *dev)
{
/* nothing to be done here */
@@ -2262,6 +2295,8 @@ static int genpd_alloc_data(struct generic_pm_domain *genpd)
genpd->gd = gd;
device_initialize(&genpd->dev);
genpd->dev.release = genpd_provider_release;
+ genpd->dev.bus = &genpd_provider_bus_type;
+ genpd->dev.parent = &genpd_provider_bus;
if (!genpd_is_dev_name_fw(genpd)) {
dev_set_name(&genpd->dev, "%s", genpd->name);
@@ -3355,9 +3390,61 @@ int of_genpd_parse_idle_states(struct device_node *dn,
}
EXPORT_SYMBOL_GPL(of_genpd_parse_idle_states);
+static int genpd_provider_probe(struct device *dev)
+{
+ return 0;
+}
+
+static void genpd_provider_remove(struct device *dev)
+{
+}
+
+static void genpd_provider_sync_state(struct device *dev)
+{
+}
+
+static struct genpd_provider_drv genpd_provider_drv = {
+ .drv = {
+ .name = "genpd_provider",
+ .bus = &genpd_provider_bus_type,
+ .sync_state = genpd_provider_sync_state,
+ .suppress_bind_attrs = true,
+ },
+ .probe = genpd_provider_probe,
+ .remove = genpd_provider_remove,
+};
+
static int __init genpd_bus_init(void)
{
- return bus_register(&genpd_bus_type);
+ int ret;
+
+ ret = device_register(&genpd_provider_bus);
+ if (ret) {
+ put_device(&genpd_provider_bus);
+ return ret;
+ }
+
+ ret = bus_register(&genpd_provider_bus_type);
+ if (ret)
+ goto err_dev;
+
+ ret = bus_register(&genpd_bus_type);
+ if (ret)
+ goto err_prov_bus;
+
+ ret = driver_register(&genpd_provider_drv.drv);
+ if (ret)
+ goto err_bus;
+
+ return 0;
+
+err_bus:
+ bus_unregister(&genpd_bus_type);
+err_prov_bus:
+ bus_unregister(&genpd_provider_bus_type);
+err_dev:
+ device_unregister(&genpd_provider_bus);
+ return ret;
}
core_initcall(genpd_bus_init);
--
2.43.0On 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>
On Tue, 22 Apr 2025 at 16:03, Abel Vesa <abel.vesa@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@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@linaro.org>
Thanks for reviewing!
Kind regards
Uffe
© 2016 - 2025 Red Hat, Inc.