PM / OPP: Add debugfs support

PATCH Superseded Viresh Kumar 2 participants 16 messages Aug 11, 2015 – Aug 12, 2015

Subsystems (6)

Driver core CPU frequency scaling framework Suspend to RAM Hibernation +2 more subsystems

lore

Share & tools

lore

Viresh Kumar

author maintainer

2015-08-11 10:34 UTC #0 lore raw

v2 cover PM / OPP: Add debugfs support

Hi Guys,

This isn't necessarily 4.3 material, but its fine if it can hit 4.3 :).

This series is targeted towards exposing opp information at user space. I wasn't really sure about adding them to sysfs, as its mostly static (fixed) data and so debugfs looked to be better place.

This series starts with 3 cleanup patches, and then moves all opp stuff to a new directory as its expanding enough now.

The last patch adds debugfs support for OPPs and works for both old and new v2 bindings.

Rebased over: pm/bleeding-edge branch.

V1->V2:
- Moved the second patch to the top as its a fix (Dan Carpenter & Stephen).
- Also updated it to fix things properly.
- rearranged opp entry in drivers/base/power/Makefile (Stephen)
- Device-directory name also includes its parent name now, to make it more unique (Stephen).
- Migrate debugfs directory properly if unregistered for the owner device (Stephen).

Viresh Kumar (6):
PM / OPP: Free resources and properly return error on failure PM / OPP: reuse of_parse_phandle()
PM / OPP: Prefix exported opp routines with dev_pm_opp_ PM / OPP: Move opp core to its own directory
PM / OPP: Move cpu specific code to opp/cpu.c
PM / OPP: Add debugfs support

 drivers/base/power/Makefile              |   2 +-
 drivers/base/power/opp/Makefile          |   3 +
 drivers/base/power/{opp.c => opp/core.c} | 376 ++++---------------------------
 drivers/base/power/opp/cpu.c             | 268 ++++++++++++++++++++++
 drivers/base/power/opp/debugfs.c         | 217 ++++++++++++++++++
 drivers/base/power/opp/opp.h             | 186 +++++++++++++++
 drivers/cpufreq/Makefile                 |   1 -
 drivers/cpufreq/cpufreq-dt.c             |  10 +-
 drivers/cpufreq/cpufreq_opp.c            | 114 ----------
 include/linux/pm_opp.h                   |  16 +-
 10 files changed, 733 insertions(+), 460 deletions(-)
 create mode 100644 drivers/base/power/opp/Makefile
 rename drivers/base/power/{opp.c => opp/core.c} (80%)
 create mode 100644 drivers/base/power/opp/cpu.c
 create mode 100644 drivers/base/power/opp/debugfs.c
 create mode 100644 drivers/base/power/opp/opp.h
 delete mode 100644 drivers/cpufreq/cpufreq_opp.c

Signature · 1 line

2.4.0

Viresh Kumar

author maintainer

2015-08-11 10:34 UTC #1 lore raw

PATCH v2 1/6 PM / OPP: Free resources and properly return error on failure

_of_init_opp_table_v2() isn't freeing up resources on some errors and the error values returned are also not correct always.

This fixes following problems:
- Return -ENOENT, if no entries are found in the table.
- Use IS_ERR() to properly check return value of _find_device_opp().
- Return error value with PTR_ERR() in above case.
- Free table if _find_device_opp() fails.

Fixes 274659029c9d ("PM / OPP: Add support to parse operating-points-v2" bindings")

Reported-by Dan Carpenter <[email protected]>

Signed-off-by Viresh Kumar <[email protected]>

---
 drivers/base/power/opp.c | 29 +++++++++++++++--------------
 1 file changed, 15 insertions(+), 14 deletions(-)

drivers/base/power/opp.c +15 ~~-14~~

index 204c6c945168..bcbd92c3b717 100644
@@ -1323,28 +1323,29 @@ static int _of_init_opp_table_v2(struct device *dev,
 		if (ret) {
 			dev_err(dev, "%s: Failed to add OPP, %d\n", __func__,
 				ret);
-			break;
+			goto free_table;
 		}
 	}
 
 	/* There should be one of more OPP defined */
-	if (WARN_ON(!count))
+	if (WARN_ON(!count)) {
+		ret = -ENOENT;
 		goto put_opp_np;
+	}
 
-	if (!ret) {
-		if (!dev_opp) {
-			dev_opp = _find_device_opp(dev);
-			if (WARN_ON(!dev_opp))
-				goto put_opp_np;
-		}
-
-		dev_opp->np = opp_np;
-		dev_opp->shared_opp = of_property_read_bool(opp_np,
-							    "opp-shared");
-	} else {
-		of_free_opp_table(dev);
+	dev_opp = _find_device_opp(dev);
+	if (WARN_ON(IS_ERR(dev_opp))) {
+		ret = PTR_ERR(dev_opp);
+		goto free_table;
 	}
 
+	dev_opp->np = opp_np;
+	dev_opp->shared_opp = of_property_read_bool(opp_np, "opp-shared");
+
+	goto put_opp_np;
+
+free_table:
+	of_free_opp_table(dev);
 put_opp_np:
 	of_node_put(opp_np);

Signature · 1 line

2.4.0

Dan Carpenter

2015-08-11 14:44 UTC #2 lore raw

Re: [PATCH V2 1/6] PM / OPP: Free resources and properly return error on failure

Quoted 25 lines · Viresh Kumar

_of_init_opp_table_v2() isn't freeing up resources on some errors and the error values returned are also not correct always.
This fixes following problems:
- Return -ENOENT, if no entries are found in the table.
- Use IS_ERR() to properly check return value of _find_device_opp().
- Return error value with PTR_ERR() in above case.
- Free table if _find_device_opp() fails.
Fixes 274659029c9d ("PM / OPP: Add support to parse operating-points-v2" bindings")
Reported-by Dan Carpenter <[email protected]>
Signed-off-by Viresh Kumar <[email protected]>
---
 drivers/base/power/opp.c | 29 +++++++++++++++--------------
 1 file changed, 15 insertions(+), 14 deletions(-)
drivers/base/power/opp.c +0 -1
index 204c6c945168..bcbd92c3b717 100644
@@ -1323,28 +1323,29 @@ static int _of_init_opp_table_v2(struct device *dev,
 		if (ret) {
 			dev_err(dev, "%s: Failed to add OPP, %d\n", __func__,
 				ret);
-			break;

drivers/base/power/opp.c static int _of_init_opp_table_v2(struct device *dev, · line 1334

Quoted 10 lines

drivers/base/power/opp.c +4 -1

+			goto free_table;
 		}
 	}
 
 	/* There should be one of more OPP defined */
-	if (WARN_ON(!count))
+	if (WARN_ON(!count)) {
+		ret = -ENOENT;
 		goto put_opp_np;
+	}

This is weird to me, because we are going backwards. What happens if we goto free_table without adding anything? I suspect it's fine, but if it's a bug then this code still has problems.

What about if we only increment count when _opp_add_static_v2() succeeds, and change it back to the original where we break, check count, then check ret. That way we don't need to know the details of free_table to see that the code is correct.

regards,
dan carpenter

Viresh Kumar

author maintainer

2015-08-11 14:59 UTC #3 lore raw

Re: [PATCH V2 1/6] PM / OPP: Free resources and properly return error on failure

Quoted 9 lines · Dan Carpenter

Quoted 9 lines

drivers/base/power/opp.c +1 -1

@@ -1323,28 +1323,29 @@ static int _of_init_opp_table_v2(struct device *dev,
 		if (ret) {
 			dev_err(dev, "%s: Failed to add OPP, %d\n", __func__,
 				ret);
-			break;
+			goto free_table;
 		}
 	}

Quoted 6 lines

drivers/base/power/opp.c static int _of_init_opp_table_v2(struct device *dev, · line 1334
Quoted 6 lines
drivers/base/power/opp.c +3 -1
 	/* There should be one of more OPP defined */
-	if (WARN_ON(!count))
+	if (WARN_ON(!count)) {
+		ret = -ENOENT;
 		goto put_opp_np;
+	}

The purpose of 'count' here is to see if the dtb contained any OPP nodes or not. i.e. if we ever entered the body of for_each_available_child_of_node() or not..

Its different than, "if we were able to add any OPPs";

Quoted 2 lines

This is weird to me, because we are going backwards. What happens if we goto free_table without adding anything?

It will WARN() today.

Quoted 2 lines

I suspect it's fine, but if
it's a bug then this code still has problems.

I don't think we have a bug here, we never added anything and so don't need to free it.

Quoted 2 lines

What about if we only increment count when _opp_add_static_v2() succeeds

That's not what we want.

Signature · 1 line

viresh

Dan Carpenter

2015-08-11 17:14 UTC #4 lore raw

Re: [PATCH V2 1/6] PM / OPP: Free resources and properly return error on failure

Quoted 4 lines · Viresh Kumar

Quoted 2 lines
This is weird to me, because we are going backwards. What happens if we goto free_table without adding anything?
It will WARN() today.

Then the current code is buggy.

Quoted 11 lines

Quoted 3 lines
I suspect it's fine, but if
it's a bug then this code still has problems.
I don't think we have a bug here, we never added anything and so don't need to free it.
Quoted 2 lines
What about if we only increment count when _opp_add_static_v2() succeeds
That's not what we want.

If the first call to _opp_add_static_v2() fails we call of_free_opp_table() and you say that triggers a WARN().

regards,
dan carpenter

Viresh Kumar

author maintainer

2015-08-12 06:43 UTC #5 lore raw

Re: [PATCH V2 1/6] PM / OPP: Free resources and properly return error on failure

Quoted 7 lines · Dan Carpenter

Quoted 4 lines · Viresh Kumar
Quoted 2 lines
This is weird to me, because we are going backwards. What happens if we goto free_table without adding anything?
It will WARN() today.
Then the current code is buggy.

Urg, it wouldn't WARN in this case. Sorry, didn't read it correctly earlier.

Quoted 6 lines

Quoted 6 lines
Quoted 2 lines
I suspect it's fine, but if
it's a bug then this code still has problems.
I don't think we have a bug here, we never added anything and so don't need to free it.

Quoted 7 lines

Quoted 4 lines
Quoted 2 lines
What about if we only increment count when _opp_add_static_v2() succeeds
That's not what we want.
If the first call to _opp_add_static_v2() fails we call of_free_opp_table() and you say that triggers a WARN().

No it doesn't.

So, coming back to the point you made about freeing table on !count, because there were no nodes present in the DT opp table, we have never tried to add any OPPs. And so there is no need to call of_free_opp_table() in that case.

Do you still think the current code is wrong ?

Signature · 1 line

viresh

Dan Carpenter

2015-08-12 08:12 UTC #6 lore raw

Re: [PATCH V2 1/6] PM / OPP: Free resources and properly return error on failure

Quoted 11 lines · Viresh Kumar

Quoted 2 lines
If the first call to _opp_add_static_v2() fails we call of_free_opp_table() and you say that triggers a WARN().
No it doesn't.
So, coming back to the point you made about freeing table on !count, because there were no nodes present in the DT opp table, we have never tried to add any OPPs. And so there is no need to call of_free_opp_table() in that case.
Do you still think the current code is wrong ?

If it doesn't WARN() then it's not buggy, but it's still ugly. We should not call of_free_opp_table() because we *tried* to add an OPP, we should only call it if we *succeeded*.

The way the code is written and from your emails I was afraid that if you tried to call _opp_add_static_v2() and it fails then it leaves artifacts lying around that need to be cleaned up by the caller. This would be the ugliest scenario. But I looked at _opp_add_static_v2() and looks fine. It cleans up properly on failure. We only need to clean up if it succeeds.

regards,
dan carpenter

Viresh Kumar

author maintainer

2015-08-12 08:23 UTC #7 lore raw

Re: [PATCH V2 1/6] PM / OPP: Free resources and properly return error on failure

Quoted 3 lines · Dan Carpenter

If it doesn't WARN() then it's not buggy, but it's still ugly. We should not call of_free_opp_table() because we *tried* to add an OPP, we should only call it if we *succeeded*.

This is done in order to write lesser code. Otherwise we need something like this:

drivers/base/power/opp.c +3 -2

index bcbd92c3b717..650e92e2f2f0 100644
@@ -1317,14 +1317,15 @@ static int _of_init_opp_table_v2(struct device *dev,
 
 	/* We have opp-list node now, iterate over it and add OPPs */
 	for_each_available_child_of_node(opp_np, np) {
-		count++;
-
 		ret = _opp_add_static_v2(dev, np);
 		if (ret) {
 			dev_err(dev, "%s: Failed to add OPP, %d\n", __func__,
 				ret);
+			if (!count)
+				goto put_opp_np;
 			goto free_table;
 		}
+		count++;
 	}
 
 	/* There should be one of more OPP defined */

To some people, this will look even more *ugly*. And so I just called free_table() on error.

Quoted 3 lines

The way the code is written and from your emails I was afraid that if you tried to call _opp_add_static_v2() and it fails then it leaves artifacts lying around that need to be cleaned up by the caller.

No. The problem is that we are trying to add OPPs in a while loop and on failure we need to free all we added earlier.

Signature · 1 line

viresh

Dan Carpenter

2015-08-12 09:03 UTC #8 lore raw

Re: [PATCH V2 1/6] PM / OPP: Free resources and properly return error on failure

Quoted 6 lines · Viresh Kumar

Quoted 3 lines · Dan Carpenter
If it doesn't WARN() then it's not buggy, but it's still ugly. We should not call of_free_opp_table() because we *tried* to add an OPP, we should only call it if we *succeeded*.
This is done in order to write lesser code.

I have been trying to discourage you from focusing so much on the writing fewer lines. That little bunny hop "goto put_opp_np;" at the end of the success path should be replaced with:

	of_node_put(opp_np);
	return 0;

It's one extra line of code, but "return 0;" is so much more clear. A lot of my focus for the past few years has been on error handling so I am perhaps more sensitive than many devs. But here is what I like:

	success_path();
	if (fail)
		fail_path();
	success_path();
	success_path();
	success_path();
	if (fail)
		fail_path();
	success_path();

	return 0;

more_fail:

	fail_path();
fail:
	fail_path();
	return ret;

The success path is a list of commands indented one tab. The fail path is intended two tabs or in reverse order at the bottom of the function indented one tab. This uses the minimum amount of if statements and indenting.

In the original code, we had:

	success_path();
	if (fail)
		fail_path();
	success_path();
	success_path();
	success_path();
	if (success) {
		success_path();
		if (fail)
			fail_path();
	} else {
		fail_path();
	}
free:
	success_and_fail_path();
	return ret;

The original code was confusing and, as a direct result, buggy. Now we have improved the code so it looks something like this:

	success_path();
	if (fail)
		fail_path();
	success_path();
	success_path();
	success_path();
	if (fail)
		fail_path();

	goto free;

more_fail:

	fail_path();
free:
	success_and_fail_path();
	return ret;

It's a lot better but it would be better yet with a return 0;. There is an earlier goto put_opp_np on the success path, but that's fine. It's not the normal success path so it's necessarily complicated.

Anyway, here is what I would suggest:

drivers/base/power/opp.c +6 -3

index 51b220e..243317c 100644
@@ -1317,19 +1317,23 @@ static int _of_init_opp_table_v2(struct device *dev,
 
 	/* We have opp-list node now, iterate over it and add OPPs */
 	for_each_available_child_of_node(opp_np, np) {
-		count++;
-
 		ret = _opp_add_static_v2(dev, np);
 		if (ret) {
 			dev_err(dev, "%s: Failed to add OPP, %d\n", __func__,
 				ret);
 			break;
 		}
+		count++;
 	}
 
 	/* There should be one of more OPP defined */
-	if (WARN_ON(!count))
+	if (WARN_ON(!count)) {
+		ret = -ENOENT;
 		goto put_opp_np;
+	}
+	if (ret)
+		goto free_table;

regards,
dan carpenter

Viresh Kumar

author maintainer

2015-08-12 10:10 UTC #9 lore raw

Re: [PATCH V2 1/6] PM / OPP: Free resources and properly return error on failure

Quoted 3 lines · Dan Carpenter

It's a lot better but it would be better yet with a return 0;. There is an earlier goto put_opp_np on the success path, but that's fine. It's not the normal success path so it's necessarily complicated.

I see where you are coming from and it makes lot of sense. Thanks for the teaching part :)

Quoted 14 lines

Anyway, here is what I would suggest:

drivers/base/power/opp.c +0 -2

index 51b220e..243317c 100644
@@ -1317,19 +1317,23 @@ static int _of_init_opp_table_v2(struct device *dev,
 
 	/* We have opp-list node now, iterate over it and add OPPs */
 	for_each_available_child_of_node(opp_np, np) {
-		count++;
-
 		ret = _opp_add_static_v2(dev, np);
 		if (ret) {

drivers/base/power/opp.c static int _of_init_opp_table_v2(struct device *dev, · line 1330

Quoted 10 lines

drivers/base/power/opp.c +2 -1

 			dev_err(dev, "%s: Failed to add OPP, %d\n", __func__,
 				ret);
 			break;
 		}
+		count++;
 	}
 
 	/* There should be one of more OPP defined */
-	if (WARN_ON(!count))
+	if (WARN_ON(!count)) {

This is wrong. _opp_add_static_v2() failed and so count was 0. And we aren't supposed to WARN() in this case. We only WARN if the user hasn't added any available nodes in the opp table.

drivers/base/power/opp.c static int _of_init_opp_table_v2(struct device *dev, · line 1335

Quoted 5 lines

drivers/base/power/opp.c +4 -0

+		ret = -ENOENT;
 		goto put_opp_np;
+	}
+	if (ret)
+		goto free_table;

Also the 'put_opp_np' thing is getting removed, check 2/6 patch of this series.

What about this:

Message-Id: <e2412c33aa6923767b394adffee9d3f7be1ee27f.1439373912.git.viresh.kumar@linaro.org>
From: Viresh Kumar <[email protected]>
Date: Tue, 11 Aug 2015 14:23:34 +0530
Subject: [PATCH] PM / OPP: Free resources and properly return error on failure

_of_init_opp_table_v2() isn't freeing up resources on some errors and the error values returned are also not correct always.

Reported-by Dan Carpenter <[email protected]>

Signed-off-by Viresh Kumar <[email protected]>

---
 drivers/base/power/opp.c | 30 ++++++++++++++++--------------
 1 file changed, 16 insertions(+), 14 deletions(-)

drivers/base/power/opp.c +16 ~~-14~~

index 204c6c945168..4d6c4576f7ae 100644
@@ -1323,28 +1323,30 @@ static int _of_init_opp_table_v2(struct device *dev,
 		if (ret) {
 			dev_err(dev, "%s: Failed to add OPP, %d\n", __func__,
 				ret);
-			break;
+			goto free_table;
 		}
 	}
 
 	/* There should be one of more OPP defined */
-	if (WARN_ON(!count))
+	if (WARN_ON(!count)) {
+		ret = -ENOENT;
 		goto put_opp_np;
+	}
 
-	if (!ret) {
-		if (!dev_opp) {
-			dev_opp = _find_device_opp(dev);
-			if (WARN_ON(!dev_opp))
-				goto put_opp_np;
-		}
-
-		dev_opp->np = opp_np;
-		dev_opp->shared_opp = of_property_read_bool(opp_np,
-							    "opp-shared");
-	} else {
-		of_free_opp_table(dev);
+	dev_opp = _find_device_opp(dev);
+	if (WARN_ON(IS_ERR(dev_opp))) {
+		ret = PTR_ERR(dev_opp);
+		goto free_table;
 	}
 
+	dev_opp->np = opp_np;
+	dev_opp->shared_opp = of_property_read_bool(opp_np, "opp-shared");
+
+	of_node_put(opp_np);
+	return 0;
+
+free_table:
+	of_free_opp_table(dev);
 put_opp_np:
 	of_node_put(opp_np);

Dan Carpenter

2015-08-12 10:52 UTC #10 lore raw

Re: [PATCH V2 1/6] PM / OPP: Free resources and properly return error on failure

Fine, fine. Much improved etc. :) Thanks.

regards,
dan carpenter

Viresh Kumar

author maintainer

2015-08-11 10:34 UTC #11 lore raw

PATCH v2 2/6 PM / OPP: reuse of_parse_phandle()

In reply to Viresh Kumar

We already have a better API to get the opp descriptor block's node from cpu-node. Lets reuse that instead of creating our own routines for the same stuff. That cleans the code a lot.

This also kills a check we had earlier (as we are using the generic API now). Earlier we used to check if the operating-points-v2 property contained multiple phandles instead of a single phandle.

Killing this check isn't an issue because, we only parse the first entry with of_parse_phandle(). So, if a user passes multiple phandles, its really his problem :)

Signed-off-by Viresh Kumar <[email protected]>

---
 drivers/base/power/opp.c | 72 ++++++++++++------------------------------------
 1 file changed, 18 insertions(+), 54 deletions(-)

drivers/base/power/opp.c +18 ~~-54~~

index bcbd92c3b717..7527de217edb 100644
@@ -1250,69 +1250,33 @@ void of_cpumask_free_opp_table(cpumask_var_t cpumask)
 }
 EXPORT_SYMBOL_GPL(of_cpumask_free_opp_table);
 
-/* Returns opp descriptor node from its phandle. Caller must do of_node_put() */
-static struct device_node *
-_of_get_opp_desc_node_from_prop(struct device *dev, const struct property *prop)
-{
-	struct device_node *opp_np;
-
-	opp_np = of_find_node_by_phandle(be32_to_cpup(prop->value));
-	if (!opp_np) {
-		dev_err(dev, "%s: Prop: %s contains invalid opp desc phandle\n",
-			__func__, prop->name);
-		return ERR_PTR(-EINVAL);
-	}
-
-	return opp_np;
-}
-
-/* Returns opp descriptor node for a device. Caller must do of_node_put() */
+/* Returns opp descriptor node for a device, caller must do of_node_put() */
 static struct device_node *_of_get_opp_desc_node(struct device *dev)
 {
-	const struct property *prop;
-
-	prop = of_find_property(dev->of_node, "operating-points-v2", NULL);
-	if (!prop)
-		return ERR_PTR(-ENODEV);
-	if (!prop->value)
-		return ERR_PTR(-ENODATA);
-
 	/*
 	 * TODO: Support for multiple OPP tables.
 	 *
 	 * There should be only ONE phandle present in "operating-points-v2"
 	 * property.
 	 */
-	if (prop->length != sizeof(__be32)) {
-		dev_err(dev, "%s: Invalid opp desc phandle\n", __func__);
-		return ERR_PTR(-EINVAL);
-	}
 
-	return _of_get_opp_desc_node_from_prop(dev, prop);
+	return of_parse_phandle(dev->of_node, "operating-points-v2", 0);
 }
 
 /* Initializes OPP tables based on new bindings */
 static int _of_init_opp_table_v2(struct device *dev,
-				 const struct property *prop)
+				 struct device_node *opp_np)
 {
-	struct device_node *opp_np, *np;
+	struct device_node *np;
 	struct device_opp *dev_opp;
 	int ret = 0, count = 0;
 
-	if (!prop->value)
-		return -ENODATA;
-
-	/* Get opp node */
-	opp_np = _of_get_opp_desc_node_from_prop(dev, prop);
-	if (IS_ERR(opp_np))
-		return PTR_ERR(opp_np);
-
 	dev_opp = _managed_opp(opp_np);
 	if (dev_opp) {
 		/* OPPs are already managed */
 		if (!_add_list_dev(dev, dev_opp))
 			ret = -ENOMEM;
-		goto put_opp_np;
+		return ret;
 	}
 
 	/* We have opp-list node now, iterate over it and add OPPs */

@@ -1328,10 +1292,8 @@ static int _of_init_opp_table_v2(struct device *dev,
 	}
 
 	/* There should be one of more OPP defined */
-	if (WARN_ON(!count)) {
-		ret = -ENOENT;
-		goto put_opp_np;
-	}
+	if (WARN_ON(!count))
+		return -ENOENT;
 
 	dev_opp = _find_device_opp(dev);
 	if (WARN_ON(IS_ERR(dev_opp))) {

@@ -1342,12 +1304,10 @@ static int _of_init_opp_table_v2(struct device *dev,
 	dev_opp->np = opp_np;
 	dev_opp->shared_opp = of_property_read_bool(opp_np, "opp-shared");
 
-	goto put_opp_np;
+	return 0;
 
 free_table:
 	of_free_opp_table(dev);
-put_opp_np:
-	of_node_put(opp_np);
 
 	return ret;
 }

@@ -1414,14 +1374,15 @@ static int _of_init_opp_table_v1(struct device *dev)
  */
 int of_init_opp_table(struct device *dev)
 {
-	const struct property *prop;
+	struct device_node *opp_np;
+	int ret;
 
 	/*
 	 * OPPs have two version of bindings now. The older one is deprecated,
 	 * try for the new binding first.
 	 */
-	prop = of_find_property(dev->of_node, "operating-points-v2", NULL);
-	if (!prop) {
+	opp_np = _of_get_opp_desc_node(dev);
+	if (!opp_np) {
 		/*
 		 * Try old-deprecated bindings for backward compatibility with
 		 * older dtbs.

@@ -1429,7 +1390,10 @@ int of_init_opp_table(struct device *dev)
 		return _of_init_opp_table_v1(dev);
 	}
 
-	return _of_init_opp_table_v2(dev, prop);
+	ret = _of_init_opp_table_v2(dev, opp_np);
+	of_node_put(opp_np);
+
+	return ret;
 }
 EXPORT_SYMBOL_GPL(of_init_opp_table);

@@ -1518,7 +1482,7 @@ int of_get_cpus_sharing_opps(struct device *cpu_dev, cpumask_var_t cpumask)
 
 	/* Get OPP descriptor node */
 	np = _of_get_opp_desc_node(cpu_dev);
-	if (IS_ERR(np)) {
+	if (!np) {
 		dev_dbg(cpu_dev, "%s: Couldn't find opp node: %ld\n", __func__,
 			PTR_ERR(np));
 		return -ENOENT;

@@ -1542,7 +1506,7 @@ int of_get_cpus_sharing_opps(struct device *cpu_dev, cpumask_var_t cpumask)
 
 		/* Get OPP descriptor node */
 		tmp_np = _of_get_opp_desc_node(tcpu_dev);
-		if (IS_ERR(tmp_np)) {
+		if (!tmp_np) {
 			dev_err(tcpu_dev, "%s: Couldn't find opp node: %ld\n",
 				__func__, PTR_ERR(tmp_np));
 			ret = PTR_ERR(tmp_np);

Signature · 1 line

2.4.0

Viresh Kumar

author maintainer

2015-08-11 10:34 UTC #12 lore raw

PATCH v2 3/6 PM / OPP: Prefix exported opp routines with dev_pm_opp_

In reply to Viresh Kumar

That's the naming convention followed in most of opp core, but few recent additions didn't follow this, fix them.

Reviewed-by Stephen Boyd <[email protected]>

Signed-off-by Viresh Kumar <[email protected]>

---
 drivers/base/power/opp.c     | 18 +++++++++---------
 drivers/cpufreq/cpufreq-dt.c | 10 +++++-----
 include/linux/pm_opp.h       | 16 ++++++++--------
 3 files changed, 22 insertions(+), 22 deletions(-)

drivers/base/power/opp.c +9 -9

index 7527de217edb..ccc636ad8e0f 100644
@@ -1230,7 +1230,7 @@ void of_free_opp_table(struct device *dev)
 }
 EXPORT_SYMBOL_GPL(of_free_opp_table);
 
-void of_cpumask_free_opp_table(cpumask_var_t cpumask)
+void dev_pm_opp_cpumask_free_table(cpumask_var_t cpumask)
 {
 	struct device *cpu_dev;
 	int cpu;

@@ -1248,7 +1248,7 @@ void of_cpumask_free_opp_table(cpumask_var_t cpumask)
 		of_free_opp_table(cpu_dev);
 	}
 }
-EXPORT_SYMBOL_GPL(of_cpumask_free_opp_table);
+EXPORT_SYMBOL_GPL(dev_pm_opp_cpumask_free_table);
 
 /* Returns opp descriptor node for a device, caller must do of_node_put() */
 static struct device_node *_of_get_opp_desc_node(struct device *dev)

@@ -1397,7 +1397,7 @@ int of_init_opp_table(struct device *dev)
 }
 EXPORT_SYMBOL_GPL(of_init_opp_table);
 
-int of_cpumask_init_opp_table(cpumask_var_t cpumask)
+int dev_pm_opp_cpumask_init_opp(cpumask_var_t cpumask)
 {
 	struct device *cpu_dev;
 	int cpu, ret = 0;

@@ -1418,17 +1418,17 @@ int of_cpumask_init_opp_table(cpumask_var_t cpumask)
 			       __func__, cpu, ret);
 
 			/* Free all other OPPs */
-			of_cpumask_free_opp_table(cpumask);
+			dev_pm_opp_cpumask_free_table(cpumask);
 			break;
 		}
 	}
 
 	return ret;
 }
-EXPORT_SYMBOL_GPL(of_cpumask_init_opp_table);
+EXPORT_SYMBOL_GPL(dev_pm_opp_cpumask_init_opp);
 
 /* Required only for V1 bindings, as v2 can manage it from DT itself */
-int set_cpus_sharing_opps(struct device *cpu_dev, cpumask_var_t cpumask)
+int dev_pm_opp_set_sharing_cpus(struct device *cpu_dev, cpumask_var_t cpumask)
 {
 	struct device_list_opp *list_dev;
 	struct device_opp *dev_opp;

@@ -1466,7 +1466,7 @@ int set_cpus_sharing_opps(struct device *cpu_dev, cpumask_var_t cpumask)
 
 	return 0;
 }
-EXPORT_SYMBOL_GPL(set_cpus_sharing_opps);
+EXPORT_SYMBOL_GPL(dev_pm_opp_set_sharing_cpus);
 
 /*
  * Works only for OPP v2 bindings.

@@ -1474,7 +1474,7 @@ EXPORT_SYMBOL_GPL(set_cpus_sharing_opps);
  * cpumask should be already set to mask of cpu_dev->id.
  * Returns -ENOENT if operating-points-v2 bindings aren't supported.
  */
-int of_get_cpus_sharing_opps(struct device *cpu_dev, cpumask_var_t cpumask)
+int dev_pm_opp_get_sharing_cpus(struct device *cpu_dev, cpumask_var_t cpumask)
 {
 	struct device_node *np, *tmp_np;
 	struct device *tcpu_dev;

@@ -1524,5 +1524,5 @@ int of_get_cpus_sharing_opps(struct device *cpu_dev, cpumask_var_t cpumask)
 	of_node_put(np);
 	return ret;
 }
-EXPORT_SYMBOL_GPL(of_get_cpus_sharing_opps);
+EXPORT_SYMBOL_GPL(dev_pm_opp_get_sharing_cpus);
 #endif

drivers/cpufreq/cpufreq-dt.c +5 -5

index c3583cdfadbd..025e76efdec3 100644
@@ -215,7 +215,7 @@ static int cpufreq_init(struct cpufreq_policy *policy)
 	}
 
 	/* Get OPP-sharing information from "operating-points-v2" bindings */
-	ret = of_get_cpus_sharing_opps(cpu_dev, policy->cpus);
+	ret = dev_pm_opp_get_sharing_cpus(cpu_dev, policy->cpus);
 	if (ret) {
 		/*
 		 * operating-points-v2 not supported, fallback to old method of

@@ -237,7 +237,7 @@ static int cpufreq_init(struct cpufreq_policy *policy)
 	 *
 	 * OPPs might be populated at runtime, don't check for error here
 	 */
-	of_cpumask_init_opp_table(policy->cpus);
+	dev_pm_opp_cpumask_init_opp(policy->cpus);
 
 	if (need_update) {
 		struct cpufreq_dt_platform_data *pd = cpufreq_get_driver_data();

@@ -249,7 +249,7 @@ static int cpufreq_init(struct cpufreq_policy *policy)
 		 * OPP tables are initialized only for policy->cpu, do it for
 		 * others as well.
 		 */
-		set_cpus_sharing_opps(cpu_dev, policy->cpus);
+		dev_pm_opp_set_sharing_cpus(cpu_dev, policy->cpus);
 
 		of_property_read_u32(np, "clock-latency", &transition_latency);
 	} else {

@@ -356,7 +356,7 @@ static int cpufreq_init(struct cpufreq_policy *policy)
 out_free_priv:
 	kfree(priv);
 out_free_opp:
-	of_cpumask_free_opp_table(policy->cpus);
+	dev_pm_opp_cpumask_free_table(policy->cpus);
 out_node_put:
 	of_node_put(np);
 out_put_reg_clk:

@@ -373,7 +373,7 @@ static int cpufreq_exit(struct cpufreq_policy *policy)
 
 	cpufreq_cooling_unregister(priv->cdev);
 	dev_pm_opp_free_cpufreq_table(priv->cpu_dev, &policy->freq_table);
-	of_cpumask_free_opp_table(policy->related_cpus);
+	dev_pm_opp_cpumask_free_table(policy->related_cpus);
 	clk_put(policy->clk);
 	if (!IS_ERR(priv->cpu_reg))
 		regulator_put(priv->cpu_reg);

include/linux/pm_opp.h +8 -8

index cab7ba55bedb..076e0309f206 100644
@@ -128,10 +128,10 @@ static inline struct srcu_notifier_head *dev_pm_opp_get_notifier(
 #if defined(CONFIG_PM_OPP) && defined(CONFIG_OF)
 int of_init_opp_table(struct device *dev);
 void of_free_opp_table(struct device *dev);
-int of_cpumask_init_opp_table(cpumask_var_t cpumask);
-void of_cpumask_free_opp_table(cpumask_var_t cpumask);
-int of_get_cpus_sharing_opps(struct device *cpu_dev, cpumask_var_t cpumask);
-int set_cpus_sharing_opps(struct device *cpu_dev, cpumask_var_t cpumask);
+int dev_pm_opp_cpumask_init_opp(cpumask_var_t cpumask);
+void dev_pm_opp_cpumask_free_table(cpumask_var_t cpumask);
+int dev_pm_opp_get_sharing_cpus(struct device *cpu_dev, cpumask_var_t cpumask);
+int dev_pm_opp_set_sharing_cpus(struct device *cpu_dev, cpumask_var_t cpumask);
 #else
 static inline int of_init_opp_table(struct device *dev)
 {

@@ -142,21 +142,21 @@ static inline void of_free_opp_table(struct device *dev)
 {
 }
 
-static inline int of_cpumask_init_opp_table(cpumask_var_t cpumask)
+static inline int dev_pm_opp_cpumask_init_opp(cpumask_var_t cpumask)
 {
 	return -ENOSYS;
 }
 
-static inline void of_cpumask_free_opp_table(cpumask_var_t cpumask)
+static inline void dev_pm_opp_cpumask_free_table(cpumask_var_t cpumask)
 {
 }
 
-static inline int of_get_cpus_sharing_opps(struct device *cpu_dev, cpumask_var_t cpumask)
+static inline int dev_pm_opp_get_sharing_cpus(struct device *cpu_dev, cpumask_var_t cpumask)
 {
 	return -ENOSYS;
 }
 
-static inline int set_cpus_sharing_opps(struct device *cpu_dev, cpumask_var_t cpumask)
+static inline int dev_pm_opp_set_sharing_cpus(struct device *cpu_dev, cpumask_var_t cpumask)
 {
 	return -ENOSYS;
 }

Signature · 1 line

2.4.0

Viresh Kumar

author maintainer

2015-08-11 10:34 UTC #13 lore raw

PATCH v2 4/6 PM / OPP: Move opp core to its own directory

In reply to Viresh Kumar

OPP code is expanding and is already present in multiple directories (cpufreq and power). Lets move it to its own directory, to manage it better.

This also moves/renames the cpufreq_opp file to cpu.c, as it will contain helpers for cpu device. Its not just about cpufreq, other frameworks can use OPPs as well.

Signed-off-by Viresh Kumar <[email protected]>

---
 drivers/base/power/Makefile                             | 2 +-
 drivers/base/power/opp/Makefile                         | 2 ++
 drivers/base/power/{opp.c => opp/core.c}                | 0
 drivers/{cpufreq/cpufreq_opp.c => base/power/opp/cpu.c} | 4 +++-
 drivers/cpufreq/Makefile                                | 1 -
 5 files changed, 6 insertions(+), 3 deletions(-)
 create mode 100644 drivers/base/power/opp/Makefile
 rename drivers/base/power/{opp.c => opp/core.c} (100%)
 rename drivers/{cpufreq/cpufreq_opp.c => base/power/opp/cpu.c} (97%)

drivers/base/power/Makefile +1 -1

index f94a6ccfe787..5998c53280f5 100644
@@ -1,7 +1,7 @@
 obj-$(CONFIG_PM)	+= sysfs.o generic_ops.o common.o qos.o runtime.o wakeirq.o
 obj-$(CONFIG_PM_SLEEP)	+= main.o wakeup.o
 obj-$(CONFIG_PM_TRACE_RTC)	+= trace.o
-obj-$(CONFIG_PM_OPP)	+= opp.o
+obj-$(CONFIG_PM_OPP)	+= opp/
 obj-$(CONFIG_PM_GENERIC_DOMAINS)	+=  domain.o domain_governor.o
 obj-$(CONFIG_HAVE_CLK)	+= clock_ops.o

drivers/base/power/opp/Makefile new file +2 -0

new file mode 100644
index 000000000000..33c1e18c41a4
@@ -0,0 +1,2 @@
+ccflags-$(CONFIG_DEBUG_DRIVER)	:= -DDEBUG
+obj-y				+= core.o cpu.o

drivers/base/power/opp/core.c renamed from drivers/base/power/opp.c +0 -0

similarity index 100%
rename from drivers/base/power/opp.c
rename to drivers/base/power/opp/core.c

drivers/base/power/opp/cpu.c renamed from drivers/cpufreq/cpufreq_opp.c +3 -1

similarity index 97%
rename from drivers/cpufreq/cpufreq_opp.c
rename to drivers/base/power/opp/cpu.c
index 0f5e6d5f6da0..0dd033016e9d 100644
@@ -1,5 +1,5 @@
 /*
- * Generic OPP helper interface for CPUFreq drivers
+ * Generic OPP helper interface for CPU device
  *
  * Copyright (C) 2009-2014 Texas Instruments Incorporated.
  *	Nishanth Menon

@@ -20,6 +20,7 @@
 #include <linux/rcupdate.h>
 #include <linux/slab.h>
 
+#ifdef CONFIG_CPU_FREQ
 /**
  * dev_pm_opp_init_cpufreq_table() - create a cpufreq table for a device
  * @dev:	device for which we do this operation

@@ -112,3 +113,4 @@ void dev_pm_opp_free_cpufreq_table(struct device *dev,
 	*table = NULL;
 }
 EXPORT_SYMBOL_GPL(dev_pm_opp_free_cpufreq_table);
+#endif	/* CONFIG_CPU_FREQ */

drivers/cpufreq/Makefile +0 -1

index 2169bf792db7..2ba5b7c0bed1 100644
@@ -1,6 +1,5 @@
 # CPUfreq core
 obj-$(CONFIG_CPU_FREQ)			+= cpufreq.o freq_table.o
-obj-$(CONFIG_PM_OPP)			+= cpufreq_opp.o
 
 # CPUfreq stats
 obj-$(CONFIG_CPU_FREQ_STAT)             += cpufreq_stats.o

Signature · 1 line

2.4.0

Viresh Kumar

author maintainer

2015-08-11 10:34 UTC #14 lore raw

PATCH v2 5/6 PM / OPP: Move cpu specific code to opp/cpu.c

In reply to Viresh Kumar

Move cpu device specific code out of generic opp library, and add it to cpu.c.

Along with that, create a core-internal opp.h header, which will be used to share structures and function prototypes within opp core.

Reviewed-by Stephen Boyd <[email protected]>

Signed-off-by Viresh Kumar <[email protected]>

---
 drivers/base/power/opp/core.c | 276 +-----------------------------------------
 drivers/base/power/opp/cpu.c  | 160 +++++++++++++++++++++++-
 drivers/base/power/opp/opp.h  | 143 ++++++++++++++++++++++
 3 files changed, 304 insertions(+), 275 deletions(-)
 create mode 100644 drivers/base/power/opp/opp.h

drivers/base/power/opp/core.c +5 ~~-271~~

index ccc636ad8e0f..29a603772992 100644
@@ -11,131 +11,14 @@
  * published by the Free Software Foundation.
  */
 
-#include <linux/cpu.h>
-#include <linux/kernel.h>
 #include <linux/errno.h>
 #include <linux/err.h>
 #include <linux/slab.h>
 #include <linux/device.h>
-#include <linux/list.h>
-#include <linux/rculist.h>
-#include <linux/rcupdate.h>
-#include <linux/pm_opp.h>
 #include <linux/of.h>
 #include <linux/export.h>
 
-/*
- * Internal data structure organization with the OPP layer library is as
- * follows:
- * dev_opp_list (root)
- *	|- device 1 (represents voltage domain 1)
- *	|	|- opp 1 (availability, freq, voltage)
- *	|	|- opp 2 ..
- *	...	...
- *	|	`- opp n ..
- *	|- device 2 (represents the next voltage domain)
- *	...
- *	`- device m (represents mth voltage domain)
- * device 1, 2.. are represented by dev_opp structure while each opp
- * is represented by the opp structure.
- */
-
-/**
- * struct dev_pm_opp - Generic OPP description structure
- * @node:	opp list node. The nodes are maintained throughout the lifetime
- *		of boot. It is expected only an optimal set of OPPs are
- *		added to the library by the SoC framework.
- *		RCU usage: opp list is traversed with RCU locks. node
- *		modification is possible realtime, hence the modifications
- *		are protected by the dev_opp_list_lock for integrity.
- *		IMPORTANT: the opp nodes should be maintained in increasing
- *		order.
- * @dynamic:	not-created from static DT entries.
- * @available:	true/false - marks if this OPP as available or not
- * @turbo:	true if turbo (boost) OPP
- * @rate:	Frequency in hertz
- * @u_volt:	Target voltage in microvolts corresponding to this OPP
- * @u_volt_min:	Minimum voltage in microvolts corresponding to this OPP
- * @u_volt_max:	Maximum voltage in microvolts corresponding to this OPP
- * @u_amp:	Maximum current drawn by the device in microamperes
- * @clock_latency_ns: Latency (in nanoseconds) of switching to this OPP's
- *		frequency from any other OPP's frequency.
- * @dev_opp:	points back to the device_opp struct this opp belongs to
- * @rcu_head:	RCU callback head used for deferred freeing
- * @np:		OPP's device node.
- *
- * This structure stores the OPP information for a given device.
- */
-struct dev_pm_opp {
-	struct list_head node;
-
-	bool available;
-	bool dynamic;
-	bool turbo;
-	unsigned long rate;
-
-	unsigned long u_volt;
-	unsigned long u_volt_min;
-	unsigned long u_volt_max;
-	unsigned long u_amp;
-	unsigned long clock_latency_ns;
-
-	struct device_opp *dev_opp;
-	struct rcu_head rcu_head;
-
-	struct device_node *np;
-};
-
-/**
- * struct device_list_opp - devices managed by 'struct device_opp'
- * @node:	list node
- * @dev:	device to which the struct object belongs
- * @rcu_head:	RCU callback head used for deferred freeing
- *
- * This is an internal data structure maintaining the list of devices that are
- * managed by 'struct device_opp'.
- */
-struct device_list_opp {
-	struct list_head node;
-	const struct device *dev;
-	struct rcu_head rcu_head;
-};
-
-/**
- * struct device_opp - Device opp structure
- * @node:	list node - contains the devices with OPPs that
- *		have been registered. Nodes once added are not modified in this
- *		list.
- *		RCU usage: nodes are not modified in the list of device_opp,
- *		however addition is possible and is secured by dev_opp_list_lock
- * @srcu_head:	notifier head to notify the OPP availability changes.
- * @rcu_head:	RCU callback head used for deferred freeing
- * @dev_list:	list of devices that share these OPPs
- * @opp_list:	list of opps
- * @np:		struct device_node pointer for opp's DT node.
- * @shared_opp: OPP is shared between multiple devices.
- *
- * This is an internal data structure maintaining the link to opps attached to
- * a device. This structure is not meant to be shared to users as it is
- * meant for book keeping and private to OPP library.
- *
- * Because the opp structures can be used from both rcu and srcu readers, we
- * need to wait for the grace period of both of them before freeing any
- * resources. And so we have used kfree_rcu() from within call_srcu() handlers.
- */
-struct device_opp {
-	struct list_head node;
-
-	struct srcu_notifier_head srcu_head;
-	struct rcu_head rcu_head;
-	struct list_head dev_list;
-	struct list_head opp_list;
-
-	struct device_node *np;
-	unsigned long clock_latency_ns_max;
-	bool shared_opp;
-	struct dev_pm_opp *suspend_opp;
-};
+#include "opp.h"
 
 /*
  * The root of the list of all devices. All device_opp structures branch off

@@ -200,7 +83,7 @@ static struct device_opp *_managed_opp(const struct device_node *np)
  * is a RCU protected pointer. This means that device_opp is valid as long
  * as we are under RCU lock.
  */
-static struct device_opp *_find_device_opp(struct device *dev)
+struct device_opp *_find_device_opp(struct device *dev)
 {
 	struct device_opp *dev_opp;

@@ -551,8 +434,8 @@ static void _remove_list_dev(struct device_list_opp *list_dev,
 		  _kfree_list_dev_rcu);
 }
 
-static struct device_list_opp *_add_list_dev(const struct device *dev,
-					     struct device_opp *dev_opp)
+struct device_list_opp *_add_list_dev(const struct device *dev,
+				      struct device_opp *dev_opp)
 {
 	struct device_list_opp *list_dev;

@@ -1230,28 +1113,8 @@ void of_free_opp_table(struct device *dev)
 }
 EXPORT_SYMBOL_GPL(of_free_opp_table);
 
-void dev_pm_opp_cpumask_free_table(cpumask_var_t cpumask)
-{
-	struct device *cpu_dev;
-	int cpu;
-
-	WARN_ON(cpumask_empty(cpumask));
-
-	for_each_cpu(cpu, cpumask) {
-		cpu_dev = get_cpu_device(cpu);
-		if (!cpu_dev) {
-			pr_err("%s: failed to get cpu%d device\n", __func__,
-			       cpu);
-			continue;
-		}
-
-		of_free_opp_table(cpu_dev);
-	}
-}
-EXPORT_SYMBOL_GPL(dev_pm_opp_cpumask_free_table);
-
 /* Returns opp descriptor node for a device, caller must do of_node_put() */
-static struct device_node *_of_get_opp_desc_node(struct device *dev)
+struct device_node *_of_get_opp_desc_node(struct device *dev)
 {
 	/*
 	 * TODO: Support for multiple OPP tables.

@@ -1396,133 +1259,4 @@ int of_init_opp_table(struct device *dev)
 	return ret;
 }
 EXPORT_SYMBOL_GPL(of_init_opp_table);
-
-int dev_pm_opp_cpumask_init_opp(cpumask_var_t cpumask)
-{
-	struct device *cpu_dev;
-	int cpu, ret = 0;
-
-	WARN_ON(cpumask_empty(cpumask));
-
-	for_each_cpu(cpu, cpumask) {
-		cpu_dev = get_cpu_device(cpu);
-		if (!cpu_dev) {
-			pr_err("%s: failed to get cpu%d device\n", __func__,
-			       cpu);
-			continue;
-		}
-
-		ret = of_init_opp_table(cpu_dev);
-		if (ret) {
-			pr_err("%s: couldn't find opp table for cpu:%d, %d\n",
-			       __func__, cpu, ret);
-
-			/* Free all other OPPs */
-			dev_pm_opp_cpumask_free_table(cpumask);
-			break;
-		}
-	}
-
-	return ret;
-}
-EXPORT_SYMBOL_GPL(dev_pm_opp_cpumask_init_opp);
-
-/* Required only for V1 bindings, as v2 can manage it from DT itself */
-int dev_pm_opp_set_sharing_cpus(struct device *cpu_dev, cpumask_var_t cpumask)
-{
-	struct device_list_opp *list_dev;
-	struct device_opp *dev_opp;
-	struct device *dev;
-	int cpu, ret = 0;
-
-	rcu_read_lock();
-
-	dev_opp = _find_device_opp(cpu_dev);
-	if (IS_ERR(dev_opp)) {
-		ret = -EINVAL;
-		goto out_rcu_read_unlock;
-	}
-
-	for_each_cpu(cpu, cpumask) {
-		if (cpu == cpu_dev->id)
-			continue;
-
-		dev = get_cpu_device(cpu);
-		if (!dev) {
-			dev_err(cpu_dev, "%s: failed to get cpu%d device\n",
-				__func__, cpu);
-			continue;
-		}
-
-		list_dev = _add_list_dev(dev, dev_opp);
-		if (!list_dev) {
-			dev_err(dev, "%s: failed to add list-dev for cpu%d device\n",
-				__func__, cpu);
-			continue;
-		}
-	}
-out_rcu_read_unlock:
-	rcu_read_unlock();
-
-	return 0;
-}
-EXPORT_SYMBOL_GPL(dev_pm_opp_set_sharing_cpus);
-
-/*
- * Works only for OPP v2 bindings.
- *
- * cpumask should be already set to mask of cpu_dev->id.
- * Returns -ENOENT if operating-points-v2 bindings aren't supported.
- */
-int dev_pm_opp_get_sharing_cpus(struct device *cpu_dev, cpumask_var_t cpumask)
-{
-	struct device_node *np, *tmp_np;
-	struct device *tcpu_dev;
-	int cpu, ret = 0;
-
-	/* Get OPP descriptor node */
-	np = _of_get_opp_desc_node(cpu_dev);
-	if (!np) {
-		dev_dbg(cpu_dev, "%s: Couldn't find opp node: %ld\n", __func__,
-			PTR_ERR(np));
-		return -ENOENT;
-	}
-
-	/* OPPs are shared ? */
-	if (!of_property_read_bool(np, "opp-shared"))
-		goto put_cpu_node;
-
-	for_each_possible_cpu(cpu) {
-		if (cpu == cpu_dev->id)
-			continue;
-
-		tcpu_dev = get_cpu_device(cpu);
-		if (!tcpu_dev) {
-			dev_err(cpu_dev, "%s: failed to get cpu%d device\n",
-				__func__, cpu);
-			ret = -ENODEV;
-			goto put_cpu_node;
-		}
-
-		/* Get OPP descriptor node */
-		tmp_np = _of_get_opp_desc_node(tcpu_dev);
-		if (!tmp_np) {
-			dev_err(tcpu_dev, "%s: Couldn't find opp node: %ld\n",
-				__func__, PTR_ERR(tmp_np));
-			ret = PTR_ERR(tmp_np);
-			goto put_cpu_node;
-		}
-
-		/* CPUs are sharing opp node */
-		if (np == tmp_np)
-			cpumask_set_cpu(cpu, cpumask);
-
-		of_node_put(tmp_np);
-	}
-
-put_cpu_node:
-	of_node_put(np);
-	return ret;
-}
-EXPORT_SYMBOL_GPL(dev_pm_opp_get_sharing_cpus);
 #endif

drivers/base/power/opp/cpu.c +156 -4

176 lines not on this page · raw message

drivers/base/power/opp/opp.h new file +143 -0

143 lines not on this page · raw message

Signature · 1 line

2.4.0

Viresh Kumar

author maintainer

2015-08-11 10:34 UTC #15 lore raw

PATCH v2 6/6 PM / OPP: Add debugfs support

In reply to Viresh Kumar

This patch adds debugfs support to OPP layer to export OPPs and their properties for all the devices.

This creates a top level directory: /sys/kernel/debug/opp and then device specific directories (based on device names) inside it. For
example: 'cpu0', 'cpu1', etc..

If multiple devices share the OPP table, then the real directory is created only for the first device. For all others, links are created to the real directory.

Inside the device specific directory, a separate directory is created for each OPP. And within that files per opp property.

Signed-off-by Viresh Kumar <[email protected]>

---
 drivers/base/power/opp/Makefile  |   1 +
 drivers/base/power/opp/core.c    |  15 +++
 drivers/base/power/opp/debugfs.c | 217 +++++++++++++++++++++++++++++++++++++++
 drivers/base/power/opp/opp.h     |  43 ++++++++
 4 files changed, 276 insertions(+)
 create mode 100644 drivers/base/power/opp/debugfs.c

drivers/base/power/opp/Makefile +1 -0

index 33c1e18c41a4..19837ef04d8e 100644
@@ -1,2 +1,3 @@
 ccflags-$(CONFIG_DEBUG_DRIVER)	:= -DDEBUG
 obj-y				+= core.o cpu.o
+obj-$(CONFIG_DEBUG_FS)		+= debugfs.o

drivers/base/power/opp/core.c +15 -0

index 29a603772992..89a07fa364d5 100644
@@ -429,6 +429,7 @@ static void _kfree_list_dev_rcu(struct rcu_head *head)
 static void _remove_list_dev(struct device_list_opp *list_dev,
 			     struct device_opp *dev_opp)
 {
+	opp_debug_unregister(list_dev, dev_opp);
 	list_del(&list_dev->node);
 	call_srcu(&dev_opp->srcu_head.srcu, &list_dev->rcu_head,
 		  _kfree_list_dev_rcu);

@@ -438,6 +439,7 @@ struct device_list_opp *_add_list_dev(const struct device *dev,
 				      struct device_opp *dev_opp)
 {
 	struct device_list_opp *list_dev;
+	int ret;
 
 	list_dev = kzalloc(sizeof(*list_dev), GFP_KERNEL);
 	if (!list_dev)

@@ -447,6 +449,12 @@ struct device_list_opp *_add_list_dev(const struct device *dev,
 	list_dev->dev = dev;
 	list_add_rcu(&list_dev->node, &dev_opp->dev_list);
 
+	/* Create debugfs entries for the dev_opp */
+	ret = opp_debug_register(list_dev, dev_opp);
+	if (ret)
+		dev_err(dev, "%s: Failed to register opp debugfs (%d)\n",
+			__func__, ret);
+
 	return list_dev;
 }

@@ -562,6 +570,7 @@ static void _opp_remove(struct device_opp *dev_opp,
 	 */
 	if (notify)
 		srcu_notifier_call_chain(&dev_opp->srcu_head, OPP_EVENT_REMOVE, opp);
+	opp_debug_remove_one(opp);
 	list_del_rcu(&opp->node);
 	call_srcu(&dev_opp->srcu_head.srcu, &opp->rcu_head, _kfree_opp_rcu);

@@ -639,6 +648,7 @@ static int _opp_add(struct device *dev, struct dev_pm_opp *new_opp,
 {
 	struct dev_pm_opp *opp;
 	struct list_head *head = &dev_opp->opp_list;
+	int ret;
 
 	/*
 	 * Insert new OPP in order of increasing frequency and discard if

@@ -669,6 +679,11 @@ static int _opp_add(struct device *dev, struct dev_pm_opp *new_opp,
 	new_opp->dev_opp = dev_opp;
 	list_add_rcu(&new_opp->node, head);
 
+	ret = opp_debug_create_one(new_opp, dev_opp);
+	if (ret)
+		dev_err(dev, "%s: Failed to register opp to debugfs (%d)\n",
+			__func__, ret);
+
 	return 0;
 }

drivers/base/power/opp/debugfs.c new file +217 -0

217 lines not on this page · raw message

drivers/base/power/opp/opp.h +43 -0

89 lines not on this page · raw message

Signature · 1 line

2.4.0