[PATCH V10 3/7] PM / Domains: Catch missing genpd_set_performance_state() in masters

From: Viresh Kumar
Date: Tue Sep 19 2017 - 18:34:00 EST


This patch catches the cases where dev_get_performance_state() callback
is implemented for a genpd, but none of its masters or their masters
(and so on) have implemented genpd_set_performance_state() callback.

The internal performance state routines don't return 0 anymore for
success, rather they return count of the domains whose performance state
is updated and the top level routine checks for that.

A zero value there would indicate that the genpd_set_performance_state()
callbacks are missing in the master hierarchy of the device.

This adds very little burden on the API and can be pretty useful.

Tested-by: Rajendra Nayak <rnayak@xxxxxxxxxxxxxx>
Signed-off-by: Viresh Kumar <viresh.kumar@xxxxxxxxxx>

---
Note that similar checks aren't present in other APIs in genpd, like
genpd_power_on/off(). Maybe we should add some there as well?
---
drivers/base/power/domain.c | 42 +++++++++++++++++++++++++++++-------------
1 file changed, 29 insertions(+), 13 deletions(-)

diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
index 6d05c91cf44f..7e00b817abc7 100644
--- a/drivers/base/power/domain.c
+++ b/drivers/base/power/domain.c
@@ -264,13 +264,13 @@ EXPORT_SYMBOL_GPL(dev_pm_genpd_has_performance_state);
static int _genpd_reeval_performance_state(struct generic_pm_domain *genpd,
int state, int depth);

-/* Returns -ve errors or 0 on success */
+/* Returns -ve errors or number of domains whose performance is set */
static int _genpd_set_performance_state(struct generic_pm_domain *genpd,
int state, int depth)
{
struct generic_pm_domain *master;
struct gpd_link *link;
- int prev = genpd->performance_state, ret;
+ int prev = genpd->performance_state, ret, count = 0;

/* Propagate to masters of genpd */
list_for_each_entry(link, &genpd->slave_links, slave_node) {
@@ -280,19 +280,23 @@ static int _genpd_set_performance_state(struct generic_pm_domain *genpd,

link->performance_state = state;
ret = _genpd_reeval_performance_state(master, state, depth + 1);
- if (ret)
+ if (ret < 0)
link->performance_state = prev;

genpd_unlock(master);

- if (ret)
+ if (ret < 0)
goto err;
+
+ count += ret;
}

if (genpd->genpd_set_performance_state) {
ret = genpd->genpd_set_performance_state(genpd, state);
if (ret)
goto err;
+
+ count++;
}

/*
@@ -300,7 +304,7 @@ static int _genpd_set_performance_state(struct generic_pm_domain *genpd,
* with those.
*/
genpd->performance_state = state;
- return 0;
+ return count;

err:
/* Encountered an error, lets rollback */
@@ -310,7 +314,7 @@ static int _genpd_set_performance_state(struct generic_pm_domain *genpd,

genpd_lock_nested(master, depth + 1);
link->performance_state = prev;
- if (_genpd_reeval_performance_state(master, prev, depth + 1)) {
+ if (_genpd_reeval_performance_state(master, prev, depth + 1) < 0) {
pr_err("%s: Failed to roll back to %d performance state\n",
master->name, prev);
}
@@ -345,7 +349,7 @@ static int _genpd_set_performance_state(struct generic_pm_domain *genpd,
* - The locks are always taken in bottom->up order, i.e. subdomain first,
* followed by its masters.
*
- * Returns -ve errors or 0 on success.
+ * Returns -ve errors or number of domains whose performance is set.
*/
static int _genpd_reeval_performance_state(struct generic_pm_domain *genpd,
int state, int depth)
@@ -354,9 +358,14 @@ static int _genpd_reeval_performance_state(struct generic_pm_domain *genpd,
struct pm_domain_data *pdd;
struct gpd_link *link;

- /* New requested state is same as Max requested state */
- if (state == genpd->performance_state)
- return 0;
+ if (state == genpd->performance_state) {
+ /*
+ * New requested state is same as Max requested state, return 1
+ * to distinguish from the case where none of the masters have
+ * set their genpd_set_performance_state() callback.
+ */
+ return 1;
+ }

/* New requested state is higher than Max requested state */
if (state > genpd->performance_state)
@@ -444,7 +453,7 @@ int dev_pm_genpd_update_performance_state(struct device *dev,
}

ret = _genpd_reeval_performance_state(genpd, state, 0);
- if (!ret) {
+ if (ret > 0) {
/*
* Since we are passing "state" to
* _genpd_reeval_performance_state() as well, we don't need to
@@ -453,6 +462,13 @@ int dev_pm_genpd_update_performance_state(struct device *dev,
* state of master domain is updated.
*/
__genpd_dev_update_performance_state(dev, state);
+ ret = 0;
+ } else if (ret < 0) {
+ dev_err(dev, "Couldn't update performance state (%d)\n", ret);
+ } else {
+ WARN(1, "%s: None of %s and its masters have provided genpd_set_performance_state()\n",
+ __func__, genpd->name);
+ ret = -ENODEV;
}

unlock:
@@ -471,7 +487,7 @@ static int _genpd_on_update_performance_state(struct generic_pm_domain *genpd,
return 0;

ret = _genpd_set_performance_state(genpd, prev, depth);
- if (ret) {
+ if (ret < 0) {
pr_err("%s: Failed to restore performance state to %d (%d)\n",
genpd->name, prev, ret);
} else {
@@ -490,7 +506,7 @@ static void _genpd_off_update_performance_state(struct generic_pm_domain *genpd,
return;

ret = _genpd_set_performance_state(genpd, 0, depth);
- if (ret) {
+ if (ret < 0) {
pr_err("%s: Failed to set performance state to 0 (%d)\n",
genpd->name, ret);
} else {
--
2.7.4