[PATCH 1/9] PM / Domains: Add dev_pm_domain_get|put() APIs

Rafael J. Wysocki rjw at rjwysocki.net
Mon Mar 16 20:01:57 PDT 2015


On Monday, March 16, 2015 10:26:46 AM Ulf Hansson wrote:
> On 14 March 2015 at 02:31, Rafael J. Wysocki <rjw at rjwysocki.net> wrote:
> > On Friday, March 13, 2015 04:43:41 PM Ulf Hansson wrote:
> >> There may be more than one device in a PM domain which then will be
> >> probed at different points in time.
> >>
> >> Depending on timing and runtime PM support, in for the device related
> >> driver/subsystem, a PM domain may be advised to power off after a
> >> successful probe sequence.
> >>
> >> A general requirement for a device within a PM domain, is that the
> >> PM domain must stay powered during the probe sequence. To cope with
> >> such requirement, let's add the dev_pm_domain_get|put() APIs.
> >>
> >> These APIs are intended to be invoked from subsystem-level code and the
> >> calls between get/put needs to be balanced.
> >>
> >> dev_pm_domain_get(), tells the PM domain that it needs to increase a
> >> usage count and to keep supplying power. dev_pm_domain_put(), does the
> >> opposite.
> >>
> >> For a PM domain to support this feature, it must implement the optional
> >> ->get|put() callbacks.
> >>
> >> Signed-off-by: Ulf Hansson <ulf.hansson at linaro.org>
> >> ---
> >>  drivers/base/power/common.c | 40 ++++++++++++++++++++++++++++++++++++++++
> >>  include/linux/pm.h          |  2 ++
> >>  include/linux/pm_domain.h   |  4 ++++
> >>  3 files changed, 46 insertions(+)
> >>
> >> diff --git a/drivers/base/power/common.c b/drivers/base/power/common.c
> >> index f32b802..99225af 100644
> >> --- a/drivers/base/power/common.c
> >> +++ b/drivers/base/power/common.c
> >> @@ -128,3 +128,43 @@ void dev_pm_domain_detach(struct device *dev, bool power_off)
> >>               dev->pm_domain->detach(dev, power_off);
> >>  }
> >>  EXPORT_SYMBOL_GPL(dev_pm_domain_detach);
> >> +
> >> +/**
> >> + * dev_pm_domain_get - Increase usage count to keep a PM domain powered.
> >> + * @domain: The PM domain to operate on.
> >> + *
> >> + * This function will not by itself increase the usage count, that's up to each
> >> + * PM domain implementation to support. Typically it should be invoked from
> >> + * subsystem level code prior drivers starts probing.
> >> + *
> >> + * Do note, it's optional to implement the ->get() callback for a PM domain.
> >> + *
> >> + * Returns 0 on successfully increased usage count or negative error code.
> >> + */
> >> +int dev_pm_domain_get(struct dev_pm_domain *domain)
> >> +{
> >> +     int ret = 0;
> >> +
> >> +     if (domain && domain->get)
> >> +             ret = domain->get(domain);
> >> +
> >> +     return ret;
> >> +}
> >> +EXPORT_SYMBOL_GPL(dev_pm_domain_get);
> >> +
> >> +/**
> >> + * dev_pm_domain_put - Decrease usage count to allow a PM domain to power off.
> >> + * @domain: The PM domain to operate on.
> >> + *
> >> + * This function will not by itself decrease the usage count, that's up to each
> >> + * PM domain implementation to support. Typically it should be invoked from
> >> + * subsystem level code after drivers has finished probing.
> >> + *
> >> + * Do note, it's optional to implement the ->put() callback for a PM domain.
> >> + */
> >> +void dev_pm_domain_put(struct dev_pm_domain *domain)
> >> +{
> >> +     if (domain && domain->put)
> >> +             domain->put(domain);
> >> +}
> >> +EXPORT_SYMBOL_GPL(dev_pm_domain_put);
> >> diff --git a/include/linux/pm.h b/include/linux/pm.h
> >> index e2f1be6..e62330b 100644
> >> --- a/include/linux/pm.h
> >> +++ b/include/linux/pm.h
> >> @@ -607,6 +607,8 @@ extern void dev_pm_put_subsys_data(struct device *dev);
> >>  struct dev_pm_domain {
> >>       struct dev_pm_ops       ops;
> >>       void (*detach)(struct device *dev, bool power_off);
> >> +     int (*get)(struct dev_pm_domain *domain);
> >> +     void (*put)(struct dev_pm_domain *domain);
> >
> > I don't like these names.  They suggest that it's always going to be about
> > reference counting which doesn't have to be the case in principle.
> 
> I am happy to change, you don't happen to have a proposal? :-)
> 
> For genpd we already have these related APIs:
> 
> pm_genpd_poweron()
> pm_genpd_name_poweron()
> pm_genpd_poweroff_unused()
> 
> Theoretically we should be able to replace these with
> dev_pm_domain_get|put() or whatever we decide to name them to.
> 
> >
> > Also what about calling these from the driver core, ie. really_probe()?
> 
> I like that!
> 
> That also implies moving the calls to dev_pm_domain_attach|detach()
> out of the buses and into the drivercore, since we need the PM domain
> to be attached before calling dev_pm_domain_get|put(). I assume that's
> what you also propose me to change, right?

Not really.  I don't want to inflict the ACPI PM domain on every bus type
that may not be prepared for using it or even may not want to use it (like
PCI or PNP).  That applies to genpd too to some extent.

So bus types need to be able to opt in for using "default" PM domains, but
at the same time if they do, the core is a better place to invoke the
callbacks.

The patch below shows how that can be done.  For the bus types using
genpd or the ACPI PM domain today ->pm_domain_init and ->pm_domain_exit
may point to the routines initializing/detaching those (which also will help
to reduce code duplication somewhat) and that guarantees that the domains
will be attached to devices before probing and the core can do the ->pre_probe
and ->post_probe things for everybody.

Rafael


---
 drivers/base/bus.c     |   12 +++++++++++-
 drivers/base/dd.c      |    9 +++++++++
 include/linux/device.h |    3 +++
 include/linux/pm.h     |    2 ++
 4 files changed, 25 insertions(+), 1 deletion(-)

Index: linux-pm/drivers/base/bus.c
===================================================================
--- linux-pm.orig/drivers/base/bus.c
+++ linux-pm/drivers/base/bus.c
@@ -509,10 +509,15 @@ int bus_add_device(struct device *dev)
 	int error = 0;
 
 	if (bus) {
+		if (bus->pm_domain_init) {
+			error = bus->pm_domain_init(dev);
+			if (error)
+				goto out_put;
+		}
 		pr_debug("bus: '%s': add device %s\n", bus->name, dev_name(dev));
 		error = device_add_attrs(bus, dev);
 		if (error)
-			goto out_put;
+			goto out_pm_domain;
 		error = device_add_groups(dev, bus->dev_groups);
 		if (error)
 			goto out_groups;
@@ -534,6 +539,9 @@ out_groups:
 	device_remove_groups(dev, bus->dev_groups);
 out_id:
 	device_remove_attrs(bus, dev);
+out_pm_domain:
+	if (bus->pm_domain_exit)
+		bus->pm_domain_exit(dev);
 out_put:
 	bus_put(dev->bus);
 	return error;
@@ -597,6 +605,8 @@ void bus_remove_device(struct device *de
 	device_remove_groups(dev, dev->bus->dev_groups);
 	if (klist_node_attached(&dev->p->knode_bus))
 		klist_del(&dev->p->knode_bus);
+	if (bus->pm_domain_exit)
+		bus->pm_domain_exit(dev);
 
 	pr_debug("bus: '%s': remove device %s\n",
 		 dev->bus->name, dev_name(dev));
Index: linux-pm/include/linux/device.h
===================================================================
--- linux-pm.orig/include/linux/device.h
+++ linux-pm/include/linux/device.h
@@ -123,6 +123,9 @@ struct bus_type {
 	int (*suspend)(struct device *dev, pm_message_t state);
 	int (*resume)(struct device *dev);
 
+	int (*pm_domain_init)(struct device *dev);
+	void (*pm_domain_exit)(struct device *dev);
+
 	const struct dev_pm_ops *pm;
 
 	const struct iommu_ops *iommu_ops;
Index: linux-pm/include/linux/pm.h
===================================================================
--- linux-pm.orig/include/linux/pm.h
+++ linux-pm/include/linux/pm.h
@@ -607,6 +607,8 @@ extern void dev_pm_put_subsys_data(struc
 struct dev_pm_domain {
 	struct dev_pm_ops	ops;
 	void (*detach)(struct device *dev, bool power_off);
+	int (*pre_probe)(struct device *dev);
+	void (*post_probe)(struct device *dev);
 };
 
 /*
Index: linux-pm/drivers/base/dd.c
===================================================================
--- linux-pm.orig/drivers/base/dd.c
+++ linux-pm/drivers/base/dd.c
@@ -298,6 +298,12 @@ static int really_probe(struct device *d
 		goto probe_failed;
 	}
 
+	if (dev->pm_domain && dev->pm_domain->pre_probe) {
+		ret = dev->pm_domain->pre_probe(dev);
+		if (ret)
+			goto probe_failed;
+	}
+
 	if (dev->bus->probe) {
 		ret = dev->bus->probe(dev);
 		if (ret)
@@ -308,6 +314,9 @@ static int really_probe(struct device *d
 			goto probe_failed;
 	}
 
+	if (dev->pm_domain && dev->pm_domain->post_probe)
+		dev->pm_domain->post_probe(dev);
+
 	driver_bound(dev);
 	ret = 1;
 	pr_debug("bus: '%s': %s: bound device %s to driver %s\n",




More information about the linux-arm-kernel mailing list