driver/core: cpu: initialize opp table

PATCH In review Viresh Kumar 4 participants 19 messages May 27, 2014 – Jun 6, 2014

Subsystems (10)

Arm (32-bit) Arm sub-architectures Arm i.MX and Layerscape Arm Samsung Exynos +6 more subsystems

lore

Share & tools

lore

Viresh Kumar

author maintainer

2014-05-27 11:52 UTC #0 lore raw

v4 cover driver/core: cpu: initialize opp table

Drivers expecting CPU's OPPs from device tree initialize OPP table themselves by calling of_init_opp_table() and there is nothing driver specific in that. They all do it in the same redundant way.

It would be better if we can get rid of redundancy by initializing CPU OPPs from CPU core code for all CPUs (that have a "operating-points" property defined in their node).

This patchset callsl of_init_opp_table() directly from register_cpu() right after CPU device is registered. Later patches updates existing cpufreq drivers which were calling of_init_opp_table() directly.

V3->V4:
- Remove of_init_cpu_opp_table() and use of_init_opp_table() instead after adding more print messages to it.
- Drop a rather unrelated patch which was getting rid of dependency on thermal for cpufreq-cpu0. Will be sent separately.

Cc Greg Kroah-Hartman <[email protected]>

Cc Amit Daniel Kachhap <[email protected]>

Cc Kukjin Kim <[email protected]>

Cc Shawn Guo <[email protected]>

Cc Sudeep Holla <[email protected]>

Viresh Kumar (8):
opp: of_init_opp_table(): return -ENOSYS when feature isn't
implemented
opp: call of_node_{get|put}() from of_init_opp_table()
opp: Enhance debug messages in of_init_opp_table() driver/core: cpu: initialize opp table
cpufreq: arm_big_little: don't initialize opp table
cpufreq: imx6q: don't initialize opp table
cpufreq: cpufreq-cpu0: don't initialize opp table
cpufreq: exynos5440: don't initialize opp table

 arch/arm/mach-imx/mach-imx6q.c       | 36 ++++++++----------------------------
 drivers/base/cpu.c                   | 11 +++++++----
 drivers/base/power/opp.c             | 11 ++++++++++-
 drivers/cpufreq/arm_big_little.c     | 12 +++++++-----
 drivers/cpufreq/arm_big_little_dt.c  | 18 ------------------
 drivers/cpufreq/cpufreq-cpu0.c       |  6 ------
 drivers/cpufreq/exynos5440-cpufreq.c |  6 ------
 drivers/cpufreq/imx6q-cpufreq.c      | 20 +-------------------
 include/linux/pm_opp.h               |  2 +-
 9 files changed, 34 insertions(+), 88 deletions(-)

Signature · 1 line

2.0.0.rc2

Viresh Kumar

author maintainer

2014-05-27 11:52 UTC #1 lore raw

PATCH v4 1/8 opp: of_init_opp_table(): return -ENOSYS when feature isn't implemented

When none of CONFIG_PM_OPP or CONFIG_OF is enabled we use the dummy implementation of of_init_opp_table() routine, which returns -EINVAL currently. -EINVAL can confuse the callers a bit as it can have other meanings for the actual implementation of this routine.

It is more appropriate to return -ENOSYS instead to avoid confusion at caller.

Suggested-and-reviewed-by Sudeep Holla <[email protected]>

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

---
 include/linux/pm_opp.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

include/linux/pm_opp.h +1 -1

index 0330217..6668150 100644
@@ -112,7 +112,7 @@ int of_init_opp_table(struct device *dev);
 #else
 static inline int of_init_opp_table(struct device *dev)
 {
-	return -EINVAL;
+	return -ENOSYS;
 }
 #endif

Signature · 1 line

2.0.0.rc2

Nishanth Menon

maintainer Acked-by on v4 1/8

2014-05-29 22:20 UTC #2 lore raw

Re: [PATCH V4 1/8] opp: of_init_opp_table(): return -ENOSYS when feature isn't implemented

minor (Rafael usually fixes it up, but still):
$subject: PM / OPP:

Quoted 9 lines · Viresh Kumar

When none of CONFIG_PM_OPP or CONFIG_OF is enabled we use the dummy implementation of of_init_opp_table() routine, which returns -EINVAL currently. -EINVAL can confuse the callers a bit as it can have other meanings for the actual implementation of this routine.
It is more appropriate to return -ENOSYS instead to avoid confusion at caller.
Suggested-and-reviewed-by Sudeep Holla <[email protected]>
Signed-off-by Viresh Kumar <[email protected]>

otherwise,

Acked-by Nishanth Menon <[email protected]>

Quoted 18 lines

---
 include/linux/pm_opp.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

include/linux/pm_opp.h +1 -1

index 0330217..6668150 100644
@@ -112,7 +112,7 @@ int of_init_opp_table(struct device *dev);
 #else
 static inline int of_init_opp_table(struct device *dev)
 {
-	return -EINVAL;
+	return -ENOSYS;
 }
 #endif

Signature · 2 lines

Regards,
Nishanth Menon

Viresh Kumar

author maintainer

2014-05-27 11:50 UTC #3 lore raw

PATCH v4 2/8 opp: call of_node_{get|put}() from of_init_opp_table()

In reply to Viresh Kumar

All callers of of_init_opp_table() are required to take reference of dev->of_node, by initiating calls to of_node_{get|put}(), before and after calling of_init_opp_table().

Its better to call these from within of_init_opp_table(), no fun adding redundant code to every caller.

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

---
 drivers/base/power/opp.c | 4 ++++
 1 file changed, 4 insertions(+)

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

index faae9cf..2b615b9 100644
@@ -622,6 +622,9 @@ int of_init_opp_table(struct device *dev)
 	const __be32 *val;
 	int nr;
 
+	if (!of_node_get(dev->of_node))
+		return -ENODEV;
+
 	prop = of_find_property(dev->of_node, "operating-points", NULL);
 	if (!prop)
 		return -ENODEV;

@@ -649,6 +652,7 @@ int of_init_opp_table(struct device *dev)
 		nr -= 2;
 	}
 
+	of_node_put(dev->of_node);
 	return 0;
 }
 EXPORT_SYMBOL_GPL(of_init_opp_table);

Signature · 1 line

2.0.0.rc2

Nishanth Menon

maintainer Acked-by on v4 2/8

2014-05-29 22:32 UTC #4 lore raw

Re: [PATCH V4 2/8] opp: call of_node_{get|put}() from of_init_opp_table()

Quoted 28 lines · Viresh Kumar

All callers of of_init_opp_table() are required to take reference of dev->of_node, by initiating calls to of_node_{get|put}(), before and after calling of_init_opp_table().
Its better to call these from within of_init_opp_table(), no fun adding redundant code to every caller.
Signed-off-by Viresh Kumar <[email protected]>
---
 drivers/base/power/opp.c | 4 ++++
 1 file changed, 4 insertions(+)
drivers/base/power/opp.c +3 -0
index faae9cf..2b615b9 100644
@@ -622,6 +622,9 @@ int of_init_opp_table(struct device *dev)
 	const __be32 *val;
 	int nr;
 
+	if (!of_node_get(dev->of_node))
+		return -ENODEV;
+
 	prop = of_find_property(dev->of_node, "operating-points", NULL);
 	if (!prop)
 		return -ENODEV;

drivers/base/power/opp.c int of_init_opp_table(struct device *dev) · line 658

Quoted 7 lines

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

@@ -649,6 +652,7 @@ int of_init_opp_table(struct device *dev)
 		nr -= 2;
 	}
 
+	of_node_put(dev->of_node);
 	return 0;
 }
 EXPORT_SYMBOL_GPL(of_init_opp_table);

With the same $subject comment as patch #1:

Acked-by Nishanth Menon <[email protected]>

Signature · 2 lines

Regards,
Nishanth Menon

Sachin Kamat

2014-05-30 06:33 UTC #5 lore raw

Re: [PATCH V4 2/8] opp: call of_node_{get|put}() from of_init_opp_table()

In reply to Viresh Kumar

Hi Viresh,

Quoted 17 lines · Viresh Kumar

All callers of of_init_opp_table() are required to take reference of dev->of_node, by initiating calls to of_node_{get|put}(), before and after calling of_init_opp_table().
Its better to call these from within of_init_opp_table(), no fun adding redundant code to every caller.
Signed-off-by Viresh Kumar <[email protected]>
---
 drivers/base/power/opp.c | 4 ++++
 1 file changed, 4 insertions(+)
drivers/base/power/opp.c +0 -0
index faae9cf..2b615b9 100644

drivers/base/power/opp.c int of_init_opp_table(struct device *dev) · line 630

Quoted 9 lines

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

@@ -622,6 +622,9 @@ int of_init_opp_table(struct device *dev)
        const __be32 *val;
        int nr;
 
+       if (!of_node_get(dev->of_node))
+               return -ENODEV;
+
        prop = of_find_property(dev->of_node, "operating-points", NULL);
        if (!prop)
                return -ENODEV;

What about of_node_put before returning here and other places in this function?

Signature · 2 lines

With warm regards,
Sachin

Rafael J. Wysocki

2014-05-30 12:05 UTC #6 lore raw

Re: [PATCH V4 2/8] opp: call of_node_{get|put}() from of_init_opp_table()

Quoted 23 lines · Sachin Kamat

Hi Viresh,
Quoted 20 lines · Viresh Kumar
All callers of of_init_opp_table() are required to take reference of dev->of_node, by initiating calls to of_node_{get|put}(), before and after calling of_init_opp_table().
Its better to call these from within of_init_opp_table(), no fun adding redundant code to every caller.
Signed-off-by Viresh Kumar <[email protected]>
---
 drivers/base/power/opp.c | 4 ++++
 1 file changed, 4 insertions(+)
drivers/base/power/opp.c +0 -0
index faae9cf..2b615b9 100644
@@ -622,6 +622,9 @@ int of_init_opp_table(struct device *dev)
        const __be32 *val;
        int nr;
 

Quoted 8 lines

drivers/base/power/opp.c int of_init_opp_table(struct device *dev) · line 630
Quoted 6 lines
drivers/base/power/opp.c +3 -0
+       if (!of_node_get(dev->of_node))
+               return -ENODEV;
+
        prop = of_find_property(dev->of_node, "operating-points", NULL);
        if (!prop)
                return -ENODEV;
What about of_node_put before returning here and other places in this function?

Yeah, that needs to be fixed.

The rest of the series looks good to me, so Viresh, please fix this one ASAP and send an update (of this particular patch only).

Thanks!

Signature · 2 lines

I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

Viresh Kumar

author maintainer

2014-06-02 06:59 UTC #7 lore raw

PATCH v5 2/8 opp: call of_node_{get|put}() from of_init_opp_table()

In reply to Viresh Kumar

All callers of of_init_opp_table() are required to take reference of dev->of_node, by initiating calls to of_node_{get|put}(), before and after calling of_init_opp_table().

Its better to call these from within of_init_opp_table(), no fun adding redundant code to every caller.

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

---
V4->V5: do of_node_put() on errors as well

 drivers/base/power/opp.c | 23 +++++++++++++++++------
 1 file changed, 17 insertions(+), 6 deletions(-)

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

index 89ced95..28696af 100644
@@ -643,13 +643,21 @@ int of_init_opp_table(struct device *dev)
 {
 	const struct property *prop;
 	const __be32 *val;
-	int nr;
+	int nr, ret = 0;
 
-	prop = of_find_property(dev->of_node, "operating-points", NULL);
-	if (!prop)
+	if (!of_node_get(dev->of_node))
 		return -ENODEV;
-	if (!prop->value)
-		return -ENODATA;
+
+	prop = of_find_property(dev->of_node, "operating-points", NULL);
+	if (!prop) {
+		ret = -ENODEV;
+		goto out;
+	}
+
+	if (!prop->value) {
+		ret = -ENODATA;
+		goto out;
+	}
 
 	/*
 	 * Each OPP is a set of tuples consisting of frequency and

@@ -658,7 +666,8 @@ int of_init_opp_table(struct device *dev)
 	nr = prop->length / sizeof(u32);
 	if (nr % 2) {
 		dev_err(dev, "%s: Invalid OPP list\n", __func__);
-		return -EINVAL;
+		ret = -EINVAL;
+		goto out;
 	}
 
 	val = prop->value;

@@ -672,6 +681,8 @@ int of_init_opp_table(struct device *dev)
 		nr -= 2;
 	}
 
+out:
+	of_node_put(dev->of_node);
 	return 0;
 }
 EXPORT_SYMBOL_GPL(of_init_opp_table);

Signature · 1 line

2.0.0.rc2

Viresh Kumar

author maintainer

2014-05-27 11:50 UTC #8 lore raw

PATCH v4 3/8 opp: Enhance debug messages in of_init_opp_table()

In reply to Viresh Kumar

Enhance print messages for debugging purposes. Add a dev_err() whenever we fail to initialize OPP table due to some error in the table present in dts and add a dev_dbg() for success case.

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

---
 drivers/base/power/opp.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

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

index 2b615b9..ed5d580 100644
@@ -628,8 +628,11 @@ int of_init_opp_table(struct device *dev)
 	prop = of_find_property(dev->of_node, "operating-points", NULL);
 	if (!prop)
 		return -ENODEV;
-	if (!prop->value)
+	if (!prop->value) {
+		dev_err(dev, "%s: failed to init OPP: prop->value is NULL\n",
+			__func__);
 		return -ENODATA;
+	}
 
 	/*
 	 * Each OPP is a set of tuples consisting of frequency and

@@ -653,6 +656,8 @@ int of_init_opp_table(struct device *dev)
 	}
 
 	of_node_put(dev->of_node);
+
+	dev_dbg(dev, "%s: successfully created OPP table\n", __func__);
 	return 0;
 }
 EXPORT_SYMBOL_GPL(of_init_opp_table);

Signature · 1 line

2.0.0.rc2

Nishanth Menon

maintainer Acked-by on v4 3/8

2014-05-29 22:33 UTC #9 lore raw

Re: [PATCH V4 3/8] opp: Enhance debug messages in of_init_opp_table()

Quoted 28 lines · Viresh Kumar

Enhance print messages for debugging purposes. Add a dev_err() whenever we fail to initialize OPP table due to some error in the table present in dts and add a dev_dbg() for success case.
Signed-off-by Viresh Kumar <[email protected]>
---
 drivers/base/power/opp.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)
drivers/base/power/opp.c +4 -1
index 2b615b9..ed5d580 100644
@@ -628,8 +628,11 @@ int of_init_opp_table(struct device *dev)
 	prop = of_find_property(dev->of_node, "operating-points", NULL);
 	if (!prop)
 		return -ENODEV;
-	if (!prop->value)
+	if (!prop->value) {
+		dev_err(dev, "%s: failed to init OPP: prop->value is NULL\n",
+			__func__);
 		return -ENODATA;
+	}
 
 	/*
 	 * Each OPP is a set of tuples consisting of frequency and

drivers/base/power/opp.c int of_init_opp_table(struct device *dev) · line 663

Quoted 8 lines

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

@@ -653,6 +656,8 @@ int of_init_opp_table(struct device *dev)
 	}
 
 	of_node_put(dev->of_node);
+
+	dev_dbg(dev, "%s: successfully created OPP table\n", __func__);
 	return 0;
 }
 EXPORT_SYMBOL_GPL(of_init_opp_table);

With the same $subject comment as patch #1:

Acked-by Nishanth Menon <[email protected]>

Signature · 2 lines

Regards,
Nishanth Menon

Viresh Kumar

author maintainer

2014-06-02 07:00 UTC #10 lore raw

PATCH v5 3/8 opp: Enhance debug messages in of_init_opp_table()

In reply to Viresh Kumar

Enhance print messages for debugging purposes. Add a dev_err() whenever we fail to initialize OPP table due to some error in the table present in dts and add a dev_dbg() for success case.

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

---
V4->V5: Fix rebase conflict as 2/8 has changed a bit

 drivers/base/power/opp.c | 4 ++++
 1 file changed, 4 insertions(+)

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

index 28696af..ba20510 100644
@@ -655,6 +655,8 @@ int of_init_opp_table(struct device *dev)
 	}
 
 	if (!prop->value) {
+		dev_err(dev, "%s: failed to init OPP: prop->value is NULL\n",
+			__func__);
 		ret = -ENODATA;
 		goto out;
 	}

@@ -681,6 +683,8 @@ int of_init_opp_table(struct device *dev)
 		nr -= 2;
 	}
 
+	dev_dbg(dev, "%s: successfully created OPP table\n", __func__);
+
 out:
 	of_node_put(dev->of_node);
 	return 0;

Signature · 1 line

2.0.0.rc2

Viresh Kumar

author maintainer

2014-05-27 11:50 UTC #11 lore raw

PATCH v4 4/8 driver/core: cpu: initialize opp table

In reply to Viresh Kumar

Drivers expecting CPU's OPPs from device tree initialize OPP table themselves by calling of_init_opp_table() and there is nothing driver specific in that. They all do it in the same redundant way.

It would be better if we can get rid of redundancy by initializing CPU OPPs from CPU core code for all CPUs (that have a "operating-points" property defined in their node).

This patch calls of_init_opp_table() right after CPU device is registered in register_cpu(). of_init_opp_table() also has a dummy implementation which simply returns -ENOSYS when CONFIG_OPP or CONFIG_OF isn't supported by some platform.

Cc Greg Kroah-Hartman <[email protected]>

Cc Amit Daniel Kachhap <[email protected]>

Cc Kukjin Kim <[email protected]>

Cc Shawn Guo <[email protected]>

Cc Sudeep Holla <[email protected]>

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

---
 drivers/base/cpu.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

drivers/base/cpu.c +7 -4

index 006b1bc..790183f 100644
@@ -16,6 +16,7 @@
 #include <linux/acpi.h>
 #include <linux/of.h>
 #include <linux/cpufeature.h>
+#include <linux/pm_opp.h>
 
 #include "base.h"

@@ -349,10 +350,12 @@ int register_cpu(struct cpu *cpu, int num)
 	if (cpu->hotpluggable)
 		cpu->dev.groups = hotplugable_cpu_attr_groups;
 	error = device_register(&cpu->dev);
-	if (!error)
-		per_cpu(cpu_sys_devices, num) = &cpu->dev;
-	if (!error)
-		register_cpu_under_node(num, cpu_to_node(num));
+	if (error)
+		return error;
+
+	per_cpu(cpu_sys_devices, num) = &cpu->dev;
+	register_cpu_under_node(num, cpu_to_node(num));
+	of_init_opp_table(&cpu->dev);
 
 	return error;
 }

Signature · 1 line

2.0.0.rc2

Nishanth Menon

maintainer

2014-05-29 22:42 UTC #12 lore raw

Quoted 37 lines · Viresh Kumar

Drivers expecting CPU's OPPs from device tree initialize OPP table themselves by calling of_init_opp_table() and there is nothing driver specific in that. They all do it in the same redundant way.
It would be better if we can get rid of redundancy by initializing CPU OPPs from CPU core code for all CPUs (that have a "operating-points" property defined in their node).
This patch calls of_init_opp_table() right after CPU device is registered in register_cpu(). of_init_opp_table() also has a dummy implementation which simply returns -ENOSYS when CONFIG_OPP or CONFIG_OF isn't supported by some platform.
Cc Greg Kroah-Hartman <[email protected]>
Cc Amit Daniel Kachhap <[email protected]>
Cc Kukjin Kim <[email protected]>
Cc Shawn Guo <[email protected]>
Cc Sudeep Holla <[email protected]>
Signed-off-by Viresh Kumar <[email protected]>
---
 drivers/base/cpu.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)
drivers/base/cpu.c +1 -0
index 006b1bc..790183f 100644
@@ -16,6 +16,7 @@
 #include <linux/acpi.h>
 #include <linux/of.h>
 #include <linux/cpufeature.h>
+#include <linux/pm_opp.h>
 
 #include "base.h"
 
@@ -349,10 +350,12 @@ int register_cpu(struct cpu *cpu, int num)
 	if (cpu->hotpluggable)
 		cpu->dev.groups = hotplugable_cpu_attr_groups;

drivers/base/cpu.c int register_cpu(struct cpu *cpu, int num) · line 357

Quoted 10 lines

drivers/base/cpu.c +5 -4

 	error = device_register(&cpu->dev);
-	if (!error)
-		per_cpu(cpu_sys_devices, num) = &cpu->dev;
-	if (!error)
-		register_cpu_under_node(num, cpu_to_node(num));
+	if (error)
+		return error;
+
+	per_cpu(cpu_sys_devices, num) = &cpu->dev;
+	register_cpu_under_node(num, cpu_to_node(num));

The change so far is a improvement in error handling -> which, personally I find nice, but not necessarily related to the $subject or covered in commit message. I suggest splitting that specific change out as a patch of it's own.

drivers/base/cpu.c int register_cpu(struct cpu *cpu, int num) · line 358

Quoted 1 line

drivers/base/cpu.c +1 -0
+	of_init_opp_table(&cpu->dev);

tricky... :) I finally did catchup on previous discussions
https://patchwork.kernel.org/patch/4199601/
https://patchwork.kernel.org/patch/4199741/
https://patchwork.kernel.org/patch/4215901/
https://patchwork.kernel.org/patch/4220431/

So, will let Rafael comment better here.

Quoted 4 lines

drivers/base/cpu.c +0 -0
 	return error;
 }
 

Signature · 2 lines

Regards,
Nishanth Menon

Viresh Kumar

author maintainer

2014-06-02 06:37 UTC #13 lore raw

Quoted 4 lines · Nishanth Menon

The change so far is a improvement in error handling -> which, personally I find nice, but not necessarily related to the $subject or covered in commit message. I suggest splitting that specific change out as a patch of it's own.

I would be required to do this here:

if (!error)

    of_init_opp_table(**);

And so did this change as part of this patch :), probably rafael doesn't feel a need of this and so has asked me to do a resend of a single patch (reported by Sachin) ..

So, will leave it as is unless he wants me to send it again :)

For this and other patches, thanks for your review.

Rafael J. Wysocki

2014-06-06 21:24 UTC #14 lore raw

In reply to Viresh Kumar

Quoted 38 lines · Viresh Kumar

Drivers expecting CPU's OPPs from device tree initialize OPP table themselves by calling of_init_opp_table() and there is nothing driver specific in that. They all do it in the same redundant way.
It would be better if we can get rid of redundancy by initializing CPU OPPs from CPU core code for all CPUs (that have a "operating-points" property defined in their node).
This patch calls of_init_opp_table() right after CPU device is registered in register_cpu(). of_init_opp_table() also has a dummy implementation which simply returns -ENOSYS when CONFIG_OPP or CONFIG_OF isn't supported by some platform.
Cc Greg Kroah-Hartman <[email protected]>
Cc Amit Daniel Kachhap <[email protected]>
Cc Kukjin Kim <[email protected]>
Cc Shawn Guo <[email protected]>
Cc Sudeep Holla <[email protected]>
Signed-off-by Viresh Kumar <[email protected]>
---
 drivers/base/cpu.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)
drivers/base/cpu.c +1 -0
index 006b1bc..790183f 100644
@@ -16,6 +16,7 @@
 #include <linux/acpi.h>
 #include <linux/of.h>
 #include <linux/cpufeature.h>
+#include <linux/pm_opp.h>
 
 #include "base.h"
 
@@ -349,10 +350,12 @@ int register_cpu(struct cpu *cpu, int num)
 	if (cpu->hotpluggable)
 		cpu->dev.groups = hotplugable_cpu_attr_groups;
 	error = device_register(&cpu->dev);

drivers/base/cpu.c int register_cpu(struct cpu *cpu, int num) · line 358

Quoted 10 lines

drivers/base/cpu.c +6 -4

-	if (!error)
-		per_cpu(cpu_sys_devices, num) = &cpu->dev;
-	if (!error)
-		register_cpu_under_node(num, cpu_to_node(num));
+	if (error)
+		return error;
+
+	per_cpu(cpu_sys_devices, num) = &cpu->dev;
+	register_cpu_under_node(num, cpu_to_node(num));
+	of_init_opp_table(&cpu->dev);

Having thought a bit more about that I don't really like this.

In the ACPI land we have a processor driver that binds to CPU devices and carries out similar initialization. I kind of prefer that to littering core code with platform-specific stuff.

Rafael

Viresh Kumar

author maintainer

2014-05-27 11:53 UTC #15 lore raw

PATCH v4 5/8 cpufreq: arm_big_little: don't initialize opp table

In reply to Viresh Kumar

CPU OPP tables are already initialized by CPU core and we don't need to reinitialize them from arm_big_little_dt driver.

As the arm_big_little_dt driver doesn't have a .init_opp_table() callback anymore, make this callback optional.

Cc Sudeep Holla <[email protected]>

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

---
 drivers/cpufreq/arm_big_little.c    | 12 +++++++-----
 drivers/cpufreq/arm_big_little_dt.c | 18 ------------------
 2 files changed, 7 insertions(+), 23 deletions(-)

drivers/cpufreq/arm_big_little.c +7 -5

index 1f4d4e3..561261e 100644
@@ -325,11 +325,13 @@ static int _get_cluster_clk_and_freq_table(struct device *cpu_dev)
 	if (freq_table[cluster])
 		return 0;
 
-	ret = arm_bL_ops->init_opp_table(cpu_dev);
-	if (ret) {
-		dev_err(cpu_dev, "%s: init_opp_table failed, cpu: %d, err: %d\n",
+	if (arm_bL_ops->init_opp_table) {
+		ret = arm_bL_ops->init_opp_table(cpu_dev);
+		if (ret) {
+			dev_err(cpu_dev, "%s: init_opp_table failed, cpu: %d, err: %d\n",
 				__func__, cpu_dev->id, ret);
-		goto out;
+			goto out;
+		}
 	}
 
 	ret = dev_pm_opp_init_cpufreq_table(cpu_dev, &freq_table[cluster]);

@@ -542,7 +544,7 @@ int bL_cpufreq_register(struct cpufreq_arm_bL_ops *ops)
 		return -EBUSY;
 	}
 
-	if (!ops || !strlen(ops->name) || !ops->init_opp_table) {
+	if (!ops || !strlen(ops->name)) {
 		pr_err("%s: Invalid arm_bL_ops, exiting\n", __func__);
 		return -ENODEV;
 	}

drivers/cpufreq/arm_big_little_dt.c +0 ~~-18~~

index 8d9d591..502182d 100644
@@ -43,23 +43,6 @@ static struct device_node *get_cpu_node_with_valid_op(int cpu)
 	return np;
 }
 
-static int dt_init_opp_table(struct device *cpu_dev)
-{
-	struct device_node *np;
-	int ret;
-
-	np = of_node_get(cpu_dev->of_node);
-	if (!np) {
-		pr_err("failed to find cpu%d node\n", cpu_dev->id);
-		return -ENOENT;
-	}
-
-	ret = of_init_opp_table(cpu_dev);
-	of_node_put(np);
-
-	return ret;
-}
-
 static int dt_get_transition_latency(struct device *cpu_dev)
 {
 	struct device_node *np;

@@ -81,7 +64,6 @@ static int dt_get_transition_latency(struct device *cpu_dev)
 static struct cpufreq_arm_bL_ops dt_bL_ops = {
 	.name	= "dt-bl",
 	.get_transition_latency = dt_get_transition_latency,
-	.init_opp_table = dt_init_opp_table,
 };
 
 static int generic_bL_probe(struct platform_device *pdev)

Signature · 1 line

2.0.0.rc2

Viresh Kumar

author maintainer

2014-05-27 11:50 UTC #16 lore raw

PATCH v4 6/8 cpufreq: imx6q: don't initialize opp table

In reply to Viresh Kumar

CPU OPP tables are already initialized by CPU core and we don't need to reinitialize them from imx6q specific code.

Acked-by Shawn Guo <[email protected]>

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

---
 arch/arm/mach-imx/mach-imx6q.c  | 36 ++++++++----------------------------
 drivers/cpufreq/imx6q-cpufreq.c | 20 +-------------------
 2 files changed, 9 insertions(+), 47 deletions(-)

arch/arm/mach-imx/mach-imx6q.c +8 ~~-28~~

index e60456d..03819e7 100644
@@ -290,12 +290,18 @@ static void __init imx6q_init_machine(void)
 #define OCOTP_CFG3_SPEED_996MHZ		0x2
 #define OCOTP_CFG3_SPEED_852MHZ		0x1
 
-static void __init imx6q_opp_check_speed_grading(struct device *cpu_dev)
+static void __init imx6q_opp_check_speed_grading(void)
 {
+	struct device *cpu_dev = get_cpu_device(0);
 	struct device_node *np;
 	void __iomem *base;
 	u32 val;
 
+	if (!cpu_dev) {
+		pr_warn("failed to get cpu0 device\n");
+		return;
+	}
+
 	np = of_find_compatible_node(NULL, NULL, "fsl,imx6q-ocotp");
 	if (!np) {
 		pr_warn("failed to find ocotp node\n");

@@ -336,32 +342,6 @@ put_node:
 	of_node_put(np);
 }
 
-static void __init imx6q_opp_init(void)
-{
-	struct device_node *np;
-	struct device *cpu_dev = get_cpu_device(0);
-
-	if (!cpu_dev) {
-		pr_warn("failed to get cpu0 device\n");
-		return;
-	}
-	np = of_node_get(cpu_dev->of_node);
-	if (!np) {
-		pr_warn("failed to find cpu0 node\n");
-		return;
-	}
-
-	if (of_init_opp_table(cpu_dev)) {
-		pr_warn("failed to init OPP table\n");
-		goto put_node;
-	}
-
-	imx6q_opp_check_speed_grading(cpu_dev);
-
-put_node:
-	of_node_put(np);
-}
-
 static struct platform_device imx6q_cpufreq_pdev = {
 	.name = "imx6q-cpufreq",
 };

@@ -376,7 +356,7 @@ static void __init imx6q_init_late(void)
 		imx6q_cpuidle_init();
 
 	if (IS_ENABLED(CONFIG_ARM_IMX6Q_CPUFREQ)) {
-		imx6q_opp_init();
+		imx6q_opp_check_speed_grading();
 		platform_device_register(&imx6q_cpufreq_pdev);
 	}
 }

drivers/cpufreq/imx6q-cpufreq.c +1 ~~-19~~

index af366c2..b72e94f 100644
@@ -190,26 +190,8 @@ static int imx6q_cpufreq_probe(struct platform_device *pdev)
 		goto put_reg;
 	}
 
-	/*
-	 * We expect an OPP table supplied by platform.
-	 * Just, incase the platform did not supply the OPP
-	 * table, it will try to get it.
-	 */
 	num = dev_pm_opp_get_opp_count(cpu_dev);
-	if (num < 0) {
-		ret = of_init_opp_table(cpu_dev);
-		if (ret < 0) {
-			dev_err(cpu_dev, "failed to init OPP table: %d\n", ret);
-			goto put_reg;
-		}
-
-		num = dev_pm_opp_get_opp_count(cpu_dev);
-		if (num < 0) {
-			ret = num;
-			dev_err(cpu_dev, "no OPP table is found: %d\n", ret);
-			goto put_reg;
-		}
-	}
+	WARN_ON(num < 0);
 
 	ret = dev_pm_opp_init_cpufreq_table(cpu_dev, &freq_table);
 	if (ret) {

Signature · 1 line

2.0.0.rc2

Viresh Kumar

author maintainer

2014-05-27 11:53 UTC #17 lore raw

PATCH v4 7/8 cpufreq: cpufreq-cpu0: don't initialize opp table

In reply to Viresh Kumar

CPU OPP tables are already initialized by CPU core and we don't need to reinitialize them from cpufreq-cpu0 driver.

Acked-by Shawn Guo <[email protected]>

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

---
 drivers/cpufreq/cpufreq-cpu0.c | 6 ------
 1 file changed, 6 deletions(-)

drivers/cpufreq/cpufreq-cpu0.c +0 -6

index 1bf6bba..4301c7c 100644
@@ -152,12 +152,6 @@ static int cpu0_cpufreq_probe(struct platform_device *pdev)
 		goto out_put_node;
 	}
 
-	ret = of_init_opp_table(cpu_dev);
-	if (ret) {
-		pr_err("failed to init OPP table: %d\n", ret);
-		goto out_put_node;
-	}
-
 	ret = dev_pm_opp_init_cpufreq_table(cpu_dev, &freq_table);
 	if (ret) {
 		pr_err("failed to init cpufreq table: %d\n", ret);

Signature · 1 line

2.0.0.rc2

Viresh Kumar

author maintainer

2014-05-27 11:53 UTC #18 lore raw

PATCH v4 8/8 cpufreq: exynos5440: don't initialize opp table

In reply to Viresh Kumar

CPU OPP tables are already initialized by CPU core and we don't need to reinitialize them from exynos5440's driver.

Cc Amit Daniel Kachhap <[email protected]>

Cc Kukjin Kim <[email protected]>

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

---
 drivers/cpufreq/exynos5440-cpufreq.c | 6 ------
 1 file changed, 6 deletions(-)

drivers/cpufreq/exynos5440-cpufreq.c +0 -6

index f33f25b..72a4206 100644
@@ -360,12 +360,6 @@ static int exynos_cpufreq_probe(struct platform_device *pdev)
 		goto err_put_node;
 	}
 
-	ret = of_init_opp_table(dvfs_info->dev);
-	if (ret) {
-		dev_err(dvfs_info->dev, "failed to init OPP table: %d\n", ret);
-		goto err_put_node;
-	}
-
 	ret = dev_pm_opp_init_cpufreq_table(dvfs_info->dev,
 					    &dvfs_info->freq_table);
 	if (ret) {

Signature · 1 line

2.0.0.rc2