| Message ID | 1422015260-14225-1-git-send-email-l.majewski@samsung.com (mailing list archive) |
|---|---|
| State | Accepted |
| Delegated to: | Eduardo Valentin |
| Headers | show |
On 23 January 2015 at 17:44, Lukasz Majewski <l.majewski@samsung.com> wrote: > + cpus = of_find_node_by_path("/cpus"); > + if (!cpus) { > + pr_err("failed to find cpus node\n"); > + return 0; > + } > + > + np = of_get_next_child(cpus, NULL); > + if (!np) { > + pr_err("failed to find cpus child node\n"); > + of_node_put(cpus); > return 0; > + } Why making it complex? Just get device node for cpu 0 and do cpu_dev->np. -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Viresh, > On 23 January 2015 at 17:44, Lukasz Majewski <l.majewski@samsung.com> > wrote: > > + cpus = of_find_node_by_path("/cpus"); > > + if (!cpus) { > > + pr_err("failed to find cpus node\n"); > > + return 0; > > + } > > + > > + np = of_get_next_child(cpus, NULL); > > + if (!np) { > > + pr_err("failed to find cpus child node\n"); > > + of_node_put(cpus); > > return 0; > > + } > > Why making it complex? Just get device node for cpu 0 and > do cpu_dev->np. Please pay a note about following problem: Previously we got: cpu0: cpu@0 for all Exynos devices. Now, however, cpu numbering has changed (due to GIC rework). For example: Exynos4412: cpus { cpu0: cpu@A00 { ... #cooling-cells = <2>; /* min followed by max */ }; cpu@A01 { }; cpu@A02 { }; cpu@A03 { }; } Exynos 4210: cpus { cpu0: cpu@900 { #cooling-cells = <2>; /* min followed by max */ }; cpu@901 { }; }; Exynos 5250: cpus { cpu0: cpu@0 { #cooling-cells = <2>; /* min followed by max */ }; cpu@1 { }; }; As you can see different cpu@XXY nodes we have and simply calling cpu@0 won't work.
On 23 January 2015 at 19:27, Lukasz Majewski <l.majewski@samsung.com> wrote: > Please pay a note about following problem: > > Previously we got: cpu0: cpu@0 for all Exynos devices. > > Now, however, cpu numbering has changed (due to GIC rework). > For example: > > Exynos4412: > cpus { > cpu0: cpu@A00 { > ... > #cooling-cells = <2>; /* min followed by max */ > }; > > cpu@A01 { > }; > > cpu@A02 { > }; > > cpu@A03 { > }; > } > > Exynos 4210: > cpus { > cpu0: cpu@900 { > #cooling-cells = <2>; /* min followed by max */ > }; > > cpu@901 { > }; > }; > > Exynos 5250: > cpus { > cpu0: cpu@0 { > #cooling-cells = <2>; /* min followed by max */ > }; > > cpu@1 { > }; > }; > > > As you can see different cpu@XXY nodes we have and simply calling cpu@0 > won't work. I wasn't asked you to get the cpu0 node from dt but this: cpu_dev = get_cpu_dev(0); np = of_node_get(cpu_dev->of_node); Wouldn't this work? You only need to guarantee that the cooling-cells is added onto the boot CPUs node. -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Sun, 25 Jan 2015 19:31:14 +0530 Viresh Kumar <viresh.kumar@linaro.org> wrote: > On 23 January 2015 at 19:27, Lukasz Majewski <l.majewski@samsung.com> > wrote: > > Please pay a note about following problem: > > > > Previously we got: cpu0: cpu@0 for all Exynos devices. > > > > Now, however, cpu numbering has changed (due to GIC rework). > > For example: > > > > Exynos4412: > > cpus { > > cpu0: cpu@A00 { > > ... > > #cooling-cells = <2>; /* min followed by > > max */ }; > > > > cpu@A01 { > > }; > > > > cpu@A02 { > > }; > > > > cpu@A03 { > > }; > > } > > > > Exynos 4210: > > cpus { > > cpu0: cpu@900 { > > #cooling-cells = <2>; /* min followed by > > max */ }; > > > > cpu@901 { > > }; > > }; > > > > Exynos 5250: > > cpus { > > cpu0: cpu@0 { > > #cooling-cells = <2>; /* min followed by > > max */ }; > > > > cpu@1 { > > }; > > }; > > > > > > As you can see different cpu@XXY nodes we have and simply calling > > cpu@0 won't work. > > I wasn't asked you to get the cpu0 node from dt but this: > > cpu_dev = get_cpu_dev(0); > np = of_node_get(cpu_dev->of_node); Ingenious simplicity :-) > > Wouldn't this work? You only need to guarantee that the cooling-cells > is added onto the boot CPUs node. I will test it tomorrow and share results. Viresh, thanks for tip. Best regards, Lukasz Majewski
On Sun, Jan 25, 2015 at 07:31:14PM +0530, Viresh Kumar wrote: > On 23 January 2015 at 19:27, Lukasz Majewski <l.majewski@samsung.com> wrote: > > Please pay a note about following problem: > > > > Previously we got: cpu0: cpu@0 for all Exynos devices. > > > > Now, however, cpu numbering has changed (due to GIC rework). > > For example: > > > > Exynos4412: > > cpus { > > cpu0: cpu@A00 { > > ... > > #cooling-cells = <2>; /* min followed by max */ > > }; > > > > cpu@A01 { > > }; > > > > cpu@A02 { > > }; > > > > cpu@A03 { > > }; > > } > > > > Exynos 4210: > > cpus { > > cpu0: cpu@900 { > > #cooling-cells = <2>; /* min followed by max */ > > }; > > > > cpu@901 { > > }; > > }; > > > > Exynos 5250: > > cpus { > > cpu0: cpu@0 { > > #cooling-cells = <2>; /* min followed by max */ > > }; > > > > cpu@1 { > > }; > > }; > > > > > > As you can see different cpu@XXY nodes we have and simply calling cpu@0 > > won't work. > > I wasn't asked you to get the cpu0 node from dt but this: > > cpu_dev = get_cpu_dev(0); > np = of_node_get(cpu_dev->of_node); > > Wouldn't this work? You only need to guarantee that the cooling-cells is added > onto the boot CPUs node. Lukasz, I agree with Viresh here, you can simplify your code. I, somehow, missed this conversation and already applied v6 of this patch in my -fixes branch. Can you please fix this by sending a differential patch on top of this one applying Viresh's commit? Viresh, my bad, I missed your comments. Thanks Eduardo
On Sun, 25 Jan 2015 12:46:21 -0400 Eduardo Valentin <edubezval@gmail.com> wrote: > On Sun, Jan 25, 2015 at 07:31:14PM +0530, Viresh Kumar wrote: > > On 23 January 2015 at 19:27, Lukasz Majewski > > <l.majewski@samsung.com> wrote: > > > Please pay a note about following problem: > > > > > > Previously we got: cpu0: cpu@0 for all Exynos devices. > > > > > > Now, however, cpu numbering has changed (due to GIC rework). > > > For example: > > > > > > Exynos4412: > > > cpus { > > > cpu0: cpu@A00 { > > > ... > > > #cooling-cells = <2>; /* min followed by > > > max */ }; > > > > > > cpu@A01 { > > > }; > > > > > > cpu@A02 { > > > }; > > > > > > cpu@A03 { > > > }; > > > } > > > > > > Exynos 4210: > > > cpus { > > > cpu0: cpu@900 { > > > #cooling-cells = <2>; /* min followed by > > > max */ }; > > > > > > cpu@901 { > > > }; > > > }; > > > > > > Exynos 5250: > > > cpus { > > > cpu0: cpu@0 { > > > #cooling-cells = <2>; /* min followed by > > > max */ }; > > > > > > cpu@1 { > > > }; > > > }; > > > > > > > > > As you can see different cpu@XXY nodes we have and simply calling > > > cpu@0 won't work. > > > > I wasn't asked you to get the cpu0 node from dt but this: > > > > cpu_dev = get_cpu_dev(0); > > np = of_node_get(cpu_dev->of_node); > > > > Wouldn't this work? You only need to guarantee that the > > cooling-cells is added onto the boot CPUs node. > > Lukasz, > > I agree with Viresh here, you can simplify your code. > > I, somehow, missed this conversation and already applied v6 of this > patch in my -fixes branch. Can you please fix this by sending a > differential patch on top of this one applying Viresh's commit? No problem, I will check this in my office tomorrow. Thanks. > > Viresh, my bad, I missed your comments. > > Thanks > > > Eduardo
diff --git a/drivers/cpufreq/exynos-cpufreq.c b/drivers/cpufreq/exynos-cpufreq.c index f99a0b0..5e98c6b 100644 --- a/drivers/cpufreq/exynos-cpufreq.c +++ b/drivers/cpufreq/exynos-cpufreq.c @@ -18,10 +18,13 @@ #include <linux/cpufreq.h> #include <linux/platform_device.h> #include <linux/of.h> +#include <linux/cpu_cooling.h> +#include <linux/cpu.h> #include "exynos-cpufreq.h" static struct exynos_dvfs_info *exynos_info; +static struct thermal_cooling_device *cdev; static struct regulator *arm_regulator; static unsigned int locking_frequency; @@ -156,6 +159,7 @@ static struct cpufreq_driver exynos_driver = { static int exynos_cpufreq_probe(struct platform_device *pdev) { + struct device_node *cpus, *np; int ret = -EINVAL; exynos_info = kzalloc(sizeof(*exynos_info), GFP_KERNEL); @@ -198,9 +202,36 @@ static int exynos_cpufreq_probe(struct platform_device *pdev) /* Done here as we want to capture boot frequency */ locking_frequency = clk_get_rate(exynos_info->cpu_clk) / 1000; - if (!cpufreq_register_driver(&exynos_driver)) + ret = cpufreq_register_driver(&exynos_driver); + if (ret) + goto err_cpufreq_reg; + + cpus = of_find_node_by_path("/cpus"); + if (!cpus) { + pr_err("failed to find cpus node\n"); + return 0; + } + + np = of_get_next_child(cpus, NULL); + if (!np) { + pr_err("failed to find cpus child node\n"); + of_node_put(cpus); return 0; + } + + if (of_find_property(np, "#cooling-cells", NULL)) { + cdev = of_cpufreq_cooling_register(np, + cpu_present_mask); + if (IS_ERR(cdev)) + pr_err("running cpufreq without cooling device: %ld\n", + PTR_ERR(cdev)); + } + of_node_put(np); + of_node_put(cpus); + + return 0; +err_cpufreq_reg: dev_err(&pdev->dev, "failed to register cpufreq driver\n"); regulator_put(arm_regulator); err_vdd_arm:
With thermal subsystem rework it is necessary to tune current cpufreq code to use cpu frequency change as a potential cooling device. Now the cpu cooling device is registered only when proper nodes and properties are available in device tree. Lack of them, however, will not prevent cpufreq for normal operation. Signed-off-by: Lukasz Majewski <l.majewski@samsung.com> --- Changes for v6: - New patch --- drivers/cpufreq/exynos-cpufreq.c | 33 ++++++++++++++++++++++++++++++++- 1 file changed, 32 insertions(+), 1 deletion(-)