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 gregkh@linuxfoundation.org Cc: Amit Daniel Kachhap amit.daniel@samsung.com Cc: Kukjin Kim kgene.kim@samsung.com Cc: Shawn Guo shawn.guo@linaro.org Cc: Sudeep Holla sudeep.holla@arm.com
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(-)
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 sudeep.holla@arm.com Signed-off-by: Viresh Kumar viresh.kumar@linaro.org --- include/linux/pm_opp.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/linux/pm_opp.h b/include/linux/pm_opp.h index 0330217..6668150 100644 --- a/include/linux/pm_opp.h +++ b/include/linux/pm_opp.h @@ -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
minor (Rafael usually fixes it up, but still): $subject: PM / OPP:
On 05/27/2014 06:50 AM, Viresh Kumar wrote:
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 sudeep.holla@arm.com Signed-off-by: Viresh Kumar viresh.kumar@linaro.org
otherwise,
Acked-by: Nishanth Menon nm@ti.com
include/linux/pm_opp.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/linux/pm_opp.h b/include/linux/pm_opp.h index 0330217..6668150 100644 --- a/include/linux/pm_opp.h +++ b/include/linux/pm_opp.h @@ -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
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 viresh.kumar@linaro.org --- drivers/base/power/opp.c | 4 ++++ 1 file changed, 4 insertions(+)
diff --git a/drivers/base/power/opp.c b/drivers/base/power/opp.c index faae9cf..2b615b9 100644 --- a/drivers/base/power/opp.c +++ b/drivers/base/power/opp.c @@ -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);
On 05/27/2014 06:50 AM, Viresh Kumar wrote:
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 viresh.kumar@linaro.org
drivers/base/power/opp.c | 4 ++++ 1 file changed, 4 insertions(+)
diff --git a/drivers/base/power/opp.c b/drivers/base/power/opp.c index faae9cf..2b615b9 100644 --- a/drivers/base/power/opp.c +++ b/drivers/base/power/opp.c @@ -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);
With the same $subject comment as patch #1: Acked-by: Nishanth Menon nm@ti.com
Hi Viresh,
On 27 May 2014 17:20, Viresh Kumar viresh.kumar@linaro.org wrote:
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 viresh.kumar@linaro.org
drivers/base/power/opp.c | 4 ++++ 1 file changed, 4 insertions(+)
diff --git a/drivers/base/power/opp.c b/drivers/base/power/opp.c index faae9cf..2b615b9 100644 --- a/drivers/base/power/opp.c +++ b/drivers/base/power/opp.c @@ -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?
On Friday, May 30, 2014 12:03:08 PM Sachin Kamat wrote:
Hi Viresh,
On 27 May 2014 17:20, Viresh Kumar viresh.kumar@linaro.org wrote:
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 viresh.kumar@linaro.org
drivers/base/power/opp.c | 4 ++++ 1 file changed, 4 insertions(+)
diff --git a/drivers/base/power/opp.c b/drivers/base/power/opp.c index faae9cf..2b615b9 100644 --- a/drivers/base/power/opp.c +++ b/drivers/base/power/opp.c @@ -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?
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!
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 viresh.kumar@linaro.org --- V4->V5: do of_node_put() on errors as well
drivers/base/power/opp.c | 23 +++++++++++++++++------ 1 file changed, 17 insertions(+), 6 deletions(-)
diff --git a/drivers/base/power/opp.c b/drivers/base/power/opp.c index 89ced95..28696af 100644 --- a/drivers/base/power/opp.c +++ b/drivers/base/power/opp.c @@ -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);
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 viresh.kumar@linaro.org --- drivers/base/power/opp.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/drivers/base/power/opp.c b/drivers/base/power/opp.c index 2b615b9..ed5d580 100644 --- a/drivers/base/power/opp.c +++ b/drivers/base/power/opp.c @@ -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);
On 05/27/2014 06:50 AM, Viresh Kumar wrote:
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 viresh.kumar@linaro.org
drivers/base/power/opp.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/drivers/base/power/opp.c b/drivers/base/power/opp.c index 2b615b9..ed5d580 100644 --- a/drivers/base/power/opp.c +++ b/drivers/base/power/opp.c @@ -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__);- }
/* * 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);
With the same $subject comment as patch #1: Acked-by: Nishanth Menon nm@ti.com
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 viresh.kumar@linaro.org --- V4->V5: Fix rebase conflict as 2/8 has changed a bit
drivers/base/power/opp.c | 4 ++++ 1 file changed, 4 insertions(+)
diff --git a/drivers/base/power/opp.c b/drivers/base/power/opp.c index 28696af..ba20510 100644 --- a/drivers/base/power/opp.c +++ b/drivers/base/power/opp.c @@ -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;
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 gregkh@linuxfoundation.org Cc: Amit Daniel Kachhap amit.daniel@samsung.com Cc: Kukjin Kim kgene.kim@samsung.com Cc: Shawn Guo shawn.guo@linaro.org Cc: Sudeep Holla sudeep.holla@arm.com Signed-off-by: Viresh Kumar viresh.kumar@linaro.org --- drivers/base/cpu.c | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-)
diff --git a/drivers/base/cpu.c b/drivers/base/cpu.c index 006b1bc..790183f 100644 --- a/drivers/base/cpu.c +++ b/drivers/base/cpu.c @@ -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; }
On 05/27/2014 06:50 AM, Viresh Kumar wrote:
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 gregkh@linuxfoundation.org Cc: Amit Daniel Kachhap amit.daniel@samsung.com Cc: Kukjin Kim kgene.kim@samsung.com Cc: Shawn Guo shawn.guo@linaro.org Cc: Sudeep Holla sudeep.holla@arm.com Signed-off-by: Viresh Kumar viresh.kumar@linaro.org
drivers/base/cpu.c | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-)
diff --git a/drivers/base/cpu.c b/drivers/base/cpu.c index 006b1bc..790183f 100644 --- a/drivers/base/cpu.c +++ b/drivers/base/cpu.c @@ -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));
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.
- 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.
return error; }
On 30 May 2014 04:11, Nishanth Menon nm@ti.com wrote:
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.
On Tuesday, May 27, 2014 05:20:53 PM Viresh Kumar wrote:
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 gregkh@linuxfoundation.org Cc: Amit Daniel Kachhap amit.daniel@samsung.com Cc: Kukjin Kim kgene.kim@samsung.com Cc: Shawn Guo shawn.guo@linaro.org Cc: Sudeep Holla sudeep.holla@arm.com Signed-off-by: Viresh Kumar viresh.kumar@linaro.org
drivers/base/cpu.c | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-)
diff --git a/drivers/base/cpu.c b/drivers/base/cpu.c index 006b1bc..790183f 100644 --- a/drivers/base/cpu.c +++ b/drivers/base/cpu.c @@ -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);
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
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 sudeep.holla@arm.com Signed-off-by: Viresh Kumar viresh.kumar@linaro.org --- drivers/cpufreq/arm_big_little.c | 12 +++++++----- drivers/cpufreq/arm_big_little_dt.c | 18 ------------------ 2 files changed, 7 insertions(+), 23 deletions(-)
diff --git a/drivers/cpufreq/arm_big_little.c b/drivers/cpufreq/arm_big_little.c index 1f4d4e3..561261e 100644 --- a/drivers/cpufreq/arm_big_little.c +++ b/drivers/cpufreq/arm_big_little.c @@ -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; } diff --git a/drivers/cpufreq/arm_big_little_dt.c b/drivers/cpufreq/arm_big_little_dt.c index 8d9d591..502182d 100644 --- a/drivers/cpufreq/arm_big_little_dt.c +++ b/drivers/cpufreq/arm_big_little_dt.c @@ -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)
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 shawn.guo@linaro.org Signed-off-by: Viresh Kumar viresh.kumar@linaro.org --- arch/arm/mach-imx/mach-imx6q.c | 36 ++++++++---------------------------- drivers/cpufreq/imx6q-cpufreq.c | 20 +------------------- 2 files changed, 9 insertions(+), 47 deletions(-)
diff --git a/arch/arm/mach-imx/mach-imx6q.c b/arch/arm/mach-imx/mach-imx6q.c index e60456d..03819e7 100644 --- a/arch/arm/mach-imx/mach-imx6q.c +++ b/arch/arm/mach-imx/mach-imx6q.c @@ -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); } } diff --git a/drivers/cpufreq/imx6q-cpufreq.c b/drivers/cpufreq/imx6q-cpufreq.c index af366c2..b72e94f 100644 --- a/drivers/cpufreq/imx6q-cpufreq.c +++ b/drivers/cpufreq/imx6q-cpufreq.c @@ -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) {
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 shawn.guo@linaro.org Signed-off-by: Viresh Kumar viresh.kumar@linaro.org --- drivers/cpufreq/cpufreq-cpu0.c | 6 ------ 1 file changed, 6 deletions(-)
diff --git a/drivers/cpufreq/cpufreq-cpu0.c b/drivers/cpufreq/cpufreq-cpu0.c index 1bf6bba..4301c7c 100644 --- a/drivers/cpufreq/cpufreq-cpu0.c +++ b/drivers/cpufreq/cpufreq-cpu0.c @@ -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);
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 amit.daniel@samsung.com Cc: Kukjin Kim kgene.kim@samsung.com Signed-off-by: Viresh Kumar viresh.kumar@linaro.org --- drivers/cpufreq/exynos5440-cpufreq.c | 6 ------ 1 file changed, 6 deletions(-)
diff --git a/drivers/cpufreq/exynos5440-cpufreq.c b/drivers/cpufreq/exynos5440-cpufreq.c index f33f25b..72a4206 100644 --- a/drivers/cpufreq/exynos5440-cpufreq.c +++ b/drivers/cpufreq/exynos5440-cpufreq.c @@ -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) {
linaro-kernel@lists.linaro.org
-
Nishanth Menon -
Rafael J. Wysocki -
Sachin Kamat -
Viresh Kumar