[PATCH 0/3] PM / Domains: Further consolidate and clarify genpd code

[PATCH 0/3] PM / Domains: Further consolidate and clarify genpd code

[Ulf Hansson]

This is yet another quite trivial series of patches that further intends to
consolidate and clarify some of the code for the generic PM domain (genpd).

Ulf Hansson (3):
  PM / Domains: Rename stop_ok to suspend_ok for the genpd governor
  PM / Domains: Rename pm_genpd_runtime_suspend|resume()
  PM / Domains: Remove ->save|restore_state() callbacks

 drivers/base/power/domain.c          | 131 ++++++++++++++++-------------------
 drivers/base/power/domain_governor.c |  20 +++---
 include/linux/pm_domain.h            |   6 +-
 3 files changed, 70 insertions(+), 87 deletions(-)

-- 
1.9.1

[PATCH 1/3] PM / Domains: Rename stop_ok to suspend_ok for the genpd governor

[Ulf Hansson]

The genpd governor validates the latency constraints to find out whether
it's acceptable to runtime suspend a device. Earlier this validation was
made to know whether it was okay to invoke the ->stop() callback for the
device, hence the governor used the name "stop_ok" for the related
variables.

To clarify the code around this, let's rename these variables from
"stop_ok" to "suspend_ok".

Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
---
 drivers/base/power/domain.c          |  6 +++---
 drivers/base/power/domain_governor.c | 20 ++++++++++----------
 include/linux/pm_domain.h            |  4 ++--
 3 files changed, 15 insertions(+), 15 deletions(-)

diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
index 56705b5..c62687d 100644
--- a/drivers/base/power/domain.c
+++ b/drivers/base/power/domain.c
@@ -382,7 +382,7 @@ static void genpd_power_off_work_fn(struct work_struct *work)
 static int pm_genpd_runtime_suspend(struct device *dev)
 {
 	struct generic_pm_domain *genpd;
-	bool (*stop_ok)(struct device *__dev);
+	bool (*suspend_ok)(struct device *__dev);
 	struct gpd_timing_data *td = &dev_gpd_data(dev)->td;
 	bool runtime_pm = pm_runtime_enabled(dev);
 	ktime_t time_start;
@@ -401,8 +401,8 @@ static int pm_genpd_runtime_suspend(struct device *dev)
 	 * runtime PM is disabled. Under these circumstances, we shall skip
 	 * validating/measuring the PM QoS latency.
 	 */
-	stop_ok = genpd->gov ? genpd->gov->stop_ok : NULL;
-	if (runtime_pm && stop_ok && !stop_ok(dev))
+	suspend_ok = genpd->gov ? genpd->gov->suspend_ok : NULL;
+	if (runtime_pm && suspend_ok && !suspend_ok(dev))
 		return -EBUSY;
 
 	/* Measure suspend latency. */
diff --git a/drivers/base/power/domain_governor.c b/drivers/base/power/domain_governor.c
index 00a5436..2e0fce7 100644
--- a/drivers/base/power/domain_governor.c
+++ b/drivers/base/power/domain_governor.c
@@ -37,10 +37,10 @@ static int dev_update_qos_constraint(struct device *dev, void *data)
 }
 
 /**
- * default_stop_ok - Default PM domain governor routine for stopping devices.
+ * default_suspend_ok - Default PM domain governor routine to suspend devices.
  * @dev: Device to check.
  */
-static bool default_stop_ok(struct device *dev)
+static bool default_suspend_ok(struct device *dev)
 {
 	struct gpd_timing_data *td = &dev_gpd_data(dev)->td;
 	unsigned long flags;
@@ -51,13 +51,13 @@ static bool default_stop_ok(struct device *dev)
 	spin_lock_irqsave(&dev->power.lock, flags);
 
 	if (!td->constraint_changed) {
-		bool ret = td->cached_stop_ok;
+		bool ret = td->cached_suspend_ok;
 
 		spin_unlock_irqrestore(&dev->power.lock, flags);
 		return ret;
 	}
 	td->constraint_changed = false;
-	td->cached_stop_ok = false;
+	td->cached_suspend_ok = false;
 	td->effective_constraint_ns = -1;
 	constraint_ns = __dev_pm_qos_read_value(dev);
 
@@ -83,13 +83,13 @@ static bool default_stop_ok(struct device *dev)
 			return false;
 	}
 	td->effective_constraint_ns = constraint_ns;
-	td->cached_stop_ok = constraint_ns >= 0;
+	td->cached_suspend_ok = constraint_ns >= 0;
 
 	/*
 	 * The children have been suspended already, so we don't need to take
-	 * their stop latencies into account here.
+	 * their suspend latencies into account here.
 	 */
-	return td->cached_stop_ok;
+	return td->cached_suspend_ok;
 }
 
 /**
@@ -150,7 +150,7 @@ static bool __default_power_down_ok(struct dev_pm_domain *pd,
 		 */
 		td = &to_gpd_data(pdd)->td;
 		constraint_ns = td->effective_constraint_ns;
-		/* default_stop_ok() need not be called before us. */
+		/* default_suspend_ok() need not be called before us. */
 		if (constraint_ns < 0) {
 			constraint_ns = dev_pm_qos_read_value(pdd->dev);
 			constraint_ns *= NSEC_PER_USEC;
@@ -227,7 +227,7 @@ static bool always_on_power_down_ok(struct dev_pm_domain *domain)
 }
 
 struct dev_power_governor simple_qos_governor = {
-	.stop_ok = default_stop_ok,
+	.suspend_ok = default_suspend_ok,
 	.power_down_ok = default_power_down_ok,
 };
 
@@ -236,5 +236,5 @@ struct dev_power_governor simple_qos_governor = {
  */
 struct dev_power_governor pm_domain_always_on_gov = {
 	.power_down_ok = always_on_power_down_ok,
-	.stop_ok = default_stop_ok,
+	.suspend_ok = default_suspend_ok,
 };
diff --git a/include/linux/pm_domain.h b/include/linux/pm_domain.h
index 49cd889..e913939 100644
--- a/include/linux/pm_domain.h
+++ b/include/linux/pm_domain.h
@@ -28,7 +28,7 @@ enum gpd_status {
 
 struct dev_power_governor {
 	bool (*power_down_ok)(struct dev_pm_domain *domain);
-	bool (*stop_ok)(struct device *dev);
+	bool (*suspend_ok)(struct device *dev);
 };
 
 struct gpd_dev_ops {
@@ -94,7 +94,7 @@ struct gpd_timing_data {
 	s64 resume_latency_ns;
 	s64 effective_constraint_ns;
 	bool constraint_changed;
-	bool cached_stop_ok;
+	bool cached_suspend_ok;
 };
 
 struct pm_domain_data {
-- 
1.9.1

[PATCH 2/3] PM / Domains: Rename pm_genpd_runtime_suspend|resume()

[Ulf Hansson]

Follow genpd's rule for names of static functions, by renaming
pm_genpd_runtime_suspend|resume() to genpd_runtime_suspend|resume().

Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
---
 drivers/base/power/domain.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
index c62687d..75b994a 100644
--- a/drivers/base/power/domain.c
+++ b/drivers/base/power/domain.c
@@ -372,14 +372,14 @@ static void genpd_power_off_work_fn(struct work_struct *work)
 }
 
 /**
- * pm_genpd_runtime_suspend - Suspend a device belonging to I/O PM domain.
+ * genpd_runtime_suspend - Suspend a device belonging to I/O PM domain.
  * @dev: Device to suspend.
  *
  * Carry out a runtime suspend of a device under the assumption that its
  * pm_domain field points to the domain member of an object of type
  * struct generic_pm_domain representing a PM domain consisting of I/O devices.
  */
-static int pm_genpd_runtime_suspend(struct device *dev)
+static int genpd_runtime_suspend(struct device *dev)
 {
 	struct generic_pm_domain *genpd;
 	bool (*suspend_ok)(struct device *__dev);
@@ -446,14 +446,14 @@ static int pm_genpd_runtime_suspend(struct device *dev)
 }
 
 /**
- * pm_genpd_runtime_resume - Resume a device belonging to I/O PM domain.
+ * genpd_runtime_resume - Resume a device belonging to I/O PM domain.
  * @dev: Device to resume.
  *
  * Carry out a runtime resume of a device under the assumption that its
  * pm_domain field points to the domain member of an object of type
  * struct generic_pm_domain representing a PM domain consisting of I/O devices.
  */
-static int pm_genpd_runtime_resume(struct device *dev)
+static int genpd_runtime_resume(struct device *dev)
 {
 	struct generic_pm_domain *genpd;
 	struct gpd_timing_data *td = &dev_gpd_data(dev)->td;
@@ -1498,8 +1498,8 @@ void pm_genpd_init(struct generic_pm_domain *genpd,
 	genpd->device_count = 0;
 	genpd->max_off_time_ns = -1;
 	genpd->max_off_time_changed = true;
-	genpd->domain.ops.runtime_suspend = pm_genpd_runtime_suspend;
-	genpd->domain.ops.runtime_resume = pm_genpd_runtime_resume;
+	genpd->domain.ops.runtime_suspend = genpd_runtime_suspend;
+	genpd->domain.ops.runtime_resume = genpd_runtime_resume;
 	genpd->domain.ops.prepare = pm_genpd_prepare;
 	genpd->domain.ops.suspend = pm_genpd_suspend;
 	genpd->domain.ops.suspend_late = pm_genpd_suspend_late;
-- 
1.9.1

[PATCH 3/3] PM / Domains: Remove ->save|restore_state() callbacks

[Ulf Hansson]

As a part of the ongoing consolidation of genpd, it's become questionable
whether clients actually needs to be able to assign their own set of
->save|restore_state() callbacks. Currently all users copes fine with the
default callbacks, so let's remove the configuration option and stick to
the default ones.

This enables further clarifications of the related code and let's also
rename pm_genpd_default_save|restore_state() into
__genpd_runtime_suspend|resume() to apply the rule of static functionnames
in genpd.

Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
---
 drivers/base/power/domain.c | 113 +++++++++++++++++++-------------------------
 include/linux/pm_domain.h   |   2 -
 2 files changed, 49 insertions(+), 66 deletions(-)

diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
index 75b994a..4ce4ce0 100644
--- a/drivers/base/power/domain.c
+++ b/drivers/base/power/domain.c
@@ -229,17 +229,6 @@ static int genpd_poweron(struct generic_pm_domain *genpd, unsigned int depth)
 	return ret;
 }
 
-static int genpd_save_dev(struct generic_pm_domain *genpd, struct device *dev)
-{
-	return GENPD_DEV_CALLBACK(genpd, int, save_state, dev);
-}
-
-static int genpd_restore_dev(struct generic_pm_domain *genpd,
-			struct device *dev)
-{
-	return GENPD_DEV_CALLBACK(genpd, int, restore_state, dev);
-}
-
 static int genpd_dev_pm_qos_notifier(struct notifier_block *nb,
 				     unsigned long val, void *ptr)
 {
@@ -372,6 +361,52 @@ static void genpd_power_off_work_fn(struct work_struct *work)
 }
 
 /**
+ * __genpd_runtime_suspend - walk the hierarchy of ->runtime_suspend() callbacks
+ * @dev: Device to handle.
+ */
+static int __genpd_runtime_suspend(struct device *dev)
+{
+	int (*cb)(struct device *__dev);
+
+	if (dev->type && dev->type->pm)
+		cb = dev->type->pm->runtime_suspend;
+	else if (dev->class && dev->class->pm)
+		cb = dev->class->pm->runtime_suspend;
+	else if (dev->bus && dev->bus->pm)
+		cb = dev->bus->pm->runtime_suspend;
+	else
+		cb = NULL;
+
+	if (!cb && dev->driver && dev->driver->pm)
+		cb = dev->driver->pm->runtime_suspend;
+
+	return cb ? cb(dev) : 0;
+}
+
+/**
+ * __genpd_runtime_resume - walk the hierarchy of ->runtime_resume() callbacks
+ * @dev: Device to handle.
+ */
+static int __genpd_runtime_resume(struct device *dev)
+{
+	int (*cb)(struct device *__dev);
+
+	if (dev->type && dev->type->pm)
+		cb = dev->type->pm->runtime_resume;
+	else if (dev->class && dev->class->pm)
+		cb = dev->class->pm->runtime_resume;
+	else if (dev->bus && dev->bus->pm)
+		cb = dev->bus->pm->runtime_resume;
+	else
+		cb = NULL;
+
+	if (!cb && dev->driver && dev->driver->pm)
+		cb = dev->driver->pm->runtime_resume;
+
+	return cb ? cb(dev) : 0;
+}
+
+/**
  * genpd_runtime_suspend - Suspend a device belonging to I/O PM domain.
  * @dev: Device to suspend.
  *
@@ -409,13 +444,13 @@ static int genpd_runtime_suspend(struct device *dev)
 	if (runtime_pm)
 		time_start = ktime_get();
 
-	ret = genpd_save_dev(genpd, dev);
+	ret = __genpd_runtime_suspend(dev);
 	if (ret)
 		return ret;
 
 	ret = genpd_stop_dev(genpd, dev);
 	if (ret) {
-		genpd_restore_dev(genpd, dev);
+		__genpd_runtime_resume(dev);
 		return ret;
 	}
 
@@ -491,7 +526,7 @@ static int genpd_runtime_resume(struct device *dev)
 	if (ret)
 		goto err_poweroff;
 
-	ret = genpd_restore_dev(genpd, dev);
+	ret = __genpd_runtime_resume(dev);
 	if (ret)
 		goto err_stop;
 
@@ -1427,54 +1462,6 @@ out:
 }
 EXPORT_SYMBOL_GPL(pm_genpd_remove_subdomain);
 
-/* Default device callbacks for generic PM domains. */
-
-/**
- * pm_genpd_default_save_state - Default "save device state" for PM domains.
- * @dev: Device to handle.
- */
-static int pm_genpd_default_save_state(struct device *dev)
-{
-	int (*cb)(struct device *__dev);
-
-	if (dev->type && dev->type->pm)
-		cb = dev->type->pm->runtime_suspend;
-	else if (dev->class && dev->class->pm)
-		cb = dev->class->pm->runtime_suspend;
-	else if (dev->bus && dev->bus->pm)
-		cb = dev->bus->pm->runtime_suspend;
-	else
-		cb = NULL;
-
-	if (!cb && dev->driver && dev->driver->pm)
-		cb = dev->driver->pm->runtime_suspend;
-
-	return cb ? cb(dev) : 0;
-}
-
-/**
- * pm_genpd_default_restore_state - Default PM domains "restore device state".
- * @dev: Device to handle.
- */
-static int pm_genpd_default_restore_state(struct device *dev)
-{
-	int (*cb)(struct device *__dev);
-
-	if (dev->type && dev->type->pm)
-		cb = dev->type->pm->runtime_resume;
-	else if (dev->class && dev->class->pm)
-		cb = dev->class->pm->runtime_resume;
-	else if (dev->bus && dev->bus->pm)
-		cb = dev->bus->pm->runtime_resume;
-	else
-		cb = NULL;
-
-	if (!cb && dev->driver && dev->driver->pm)
-		cb = dev->driver->pm->runtime_resume;
-
-	return cb ? cb(dev) : 0;
-}
-
 /**
  * pm_genpd_init - Initialize a generic I/O PM domain object.
  * @genpd: PM domain object to initialize.
@@ -1520,8 +1507,6 @@ void pm_genpd_init(struct generic_pm_domain *genpd,
 	genpd->domain.ops.restore_early = pm_genpd_resume_early;
 	genpd->domain.ops.restore = pm_genpd_resume;
 	genpd->domain.ops.complete = pm_genpd_complete;
-	genpd->dev_ops.save_state = pm_genpd_default_save_state;
-	genpd->dev_ops.restore_state = pm_genpd_default_restore_state;
 
 	if (genpd->flags & GENPD_FLAG_PM_CLK) {
 		genpd->dev_ops.stop = pm_clk_suspend;
diff --git a/include/linux/pm_domain.h b/include/linux/pm_domain.h
index e913939..39285c7 100644
--- a/include/linux/pm_domain.h
+++ b/include/linux/pm_domain.h
@@ -34,8 +34,6 @@ struct dev_power_governor {
 struct gpd_dev_ops {
 	int (*start)(struct device *dev);
 	int (*stop)(struct device *dev);
-	int (*save_state)(struct device *dev);
-	int (*restore_state)(struct device *dev);
 	bool (*active_wakeup)(struct device *dev);
 };
 
-- 
1.9.1

Re: [PATCH 0/3] PM / Domains: Further consolidate and clarify genpd code

[Ulf Hansson]

On 31 March 2016 at 11:21, Ulf Hansson <ulf.hansson@linaro.org> wrote:
> This is yet another quite trivial series of patches that further intends to
> consolidate and clarify some of the code for the generic PM domain (genpd).
>
> Ulf Hansson (3):
>   PM / Domains: Rename stop_ok to suspend_ok for the genpd governor
>   PM / Domains: Rename pm_genpd_runtime_suspend|resume()
>   PM / Domains: Remove ->save|restore_state() callbacks
>
>  drivers/base/power/domain.c          | 131 ++++++++++++++++-------------------
>  drivers/base/power/domain_governor.c |  20 +++---
>  include/linux/pm_domain.h            |   6 +-
>  3 files changed, 70 insertions(+), 87 deletions(-)
>
> --
> 1.9.1
>

Rafael, Kevin, do these look okay with you?

Kind regards
Uffe

Re: [PATCH 0/3] PM / Domains: Further consolidate and clarify genpd code

[Rafael J. Wysocki]

On Wednesday, April 20, 2016 07:55:09 PM Ulf Hansson wrote:
> On 31 March 2016 at 11:21, Ulf Hansson <ulf.hansson@linaro.org> wrote:
> > This is yet another quite trivial series of patches that further intends to
> > consolidate and clarify some of the code for the generic PM domain (genpd).
> >
> > Ulf Hansson (3):
> >   PM / Domains: Rename stop_ok to suspend_ok for the genpd governor
> >   PM / Domains: Rename pm_genpd_runtime_suspend|resume()
> >   PM / Domains: Remove ->save|restore_state() callbacks
> >
> >  drivers/base/power/domain.c          | 131 ++++++++++++++++-------------------
> >  drivers/base/power/domain_governor.c |  20 +++---
> >  include/linux/pm_domain.h            |   6 +-
> >  3 files changed, 70 insertions(+), 87 deletions(-)
> >
> > --
> > 1.9.1
> >
> 
> Rafael, Kevin, do these look okay with you?

I'm waiting for Kevin.

Thanks,
Rafael

Re: [PATCH 0/3] PM / Domains: Further consolidate and clarify genpd code

[Kevin Hilman]

Ulf Hansson <ulf.hansson@linaro.org> writes:

> This is yet another quite trivial series of patches that further intends to
> consolidate and clarify some of the code for the generic PM domain (genpd).
>
> Ulf Hansson (3):
>   PM / Domains: Rename stop_ok to suspend_ok for the genpd governor
>   PM / Domains: Rename pm_genpd_runtime_suspend|resume()
>   PM / Domains: Remove ->save|restore_state() callbacks

Reviewed-by: Kevin Hilman <khilman@baylibre.com>

with an enthusiastic yes!  :)

Thanks for the cleanups and clarifications.

Kevin

Re: [PATCH 0/3] PM / Domains: Further consolidate and clarify genpd code

[Rafael J. Wysocki]

On Thursday, April 21, 2016 05:19:19 PM Kevin Hilman wrote:
> Ulf Hansson <ulf.hansson@linaro.org> writes:
> 
> > This is yet another quite trivial series of patches that further intends to
> > consolidate and clarify some of the code for the generic PM domain (genpd).
> >
> > Ulf Hansson (3):
> >   PM / Domains: Rename stop_ok to suspend_ok for the genpd governor
> >   PM / Domains: Rename pm_genpd_runtime_suspend|resume()
> >   PM / Domains: Remove ->save|restore_state() callbacks
> 
> Reviewed-by: Kevin Hilman <khilman@baylibre.com>
> 
> with an enthusiastic yes!  :)
> 
> Thanks for the cleanups and clarifications.

All [1-3/3] applied, thanks!