| Message ID | 1448529671-48216-1-git-send-email-hongtao.jia@freescale.com (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
On Thursday, November 26, 2015 05:21:11 PM Jia Hongtao wrote: > Register the qoriq cpufreq driver as a cooling device, based on the > thermal device tree framework. When temperature crosses the passive trip > point cpufreq is used to throttle CPUs. > > Signed-off-by: Jia Hongtao <hongtao.jia@freescale.com> > Reviewed-by: Viresh Kumar <viresh.kumar@linaro.org> Applied, thanks! Rafael
On Tuesday 15 December 2015 00:58:26 Rafael J. Wysocki wrote: > On Thursday, November 26, 2015 05:21:11 PM Jia Hongtao wrote: > > Register the qoriq cpufreq driver as a cooling device, based on the > > thermal device tree framework. When temperature crosses the passive trip > > point cpufreq is used to throttle CPUs. > > > > Signed-off-by: Jia Hongtao <hongtao.jia@freescale.com> > > Reviewed-by: Viresh Kumar <viresh.kumar@linaro.org> > > Applied, thanks! > I got a randconfig build error today: drivers/built-in.o: In function `qoriq_cpufreq_ready': debugfs.c:(.text+0x1f4688): undefined reference to `of_cpufreq_cooling_register' CONFIG_OF=y CONFIG_QORIQ_CPUFREQ=y CONFIG_THERMAL=m CONFIG_THERMAL_OF=y I think you need a 'depends on THERMAL' to prevent the driver from being built-in when THERMAL=m. Arnd
Sorry for the late response. I got a knee surgery to do. See comments at the end. > -----邮件原件----- > 发件人: Arnd Bergmann [mailto:arnd@arndb.de] > 发送时间: Saturday, December 19, 2015 6:33 AM > 收件人: Rafael J. Wysocki <rjw@rjwysocki.net> > 抄送: Jia Hongtao <hongtao.jia@freescale.com>; edubezval@gmail.com; > viresh.kumar@linaro.org; linux-pm@vger.kernel.org; linuxppc- > dev@lists.ozlabs.org; devicetree@vger.kernel.org; Scott Wood > <scottwood@freescale.com> > 主题: Re: [PATCH V3] cpufreq: qoriq: Register cooling device based on device > tree > > On Tuesday 15 December 2015 00:58:26 Rafael J. Wysocki wrote: > > On Thursday, November 26, 2015 05:21:11 PM Jia Hongtao wrote: > > > Register the qoriq cpufreq driver as a cooling device, based on the > > > thermal device tree framework. When temperature crosses the passive > > > trip point cpufreq is used to throttle CPUs. > > > > > > Signed-off-by: Jia Hongtao <hongtao.jia@freescale.com> > > > Reviewed-by: Viresh Kumar <viresh.kumar@linaro.org> > > > > Applied, thanks! > > > > I got a randconfig build error today: > > drivers/built-in.o: In function `qoriq_cpufreq_ready': > debugfs.c:(.text+0x1f4688): undefined reference to > `of_cpufreq_cooling_register' > > CONFIG_OF=y > CONFIG_QORIQ_CPUFREQ=y > CONFIG_THERMAL=m > CONFIG_THERMAL_OF=y > > I think you need a 'depends on THERMAL' to prevent the driver from being > built-in when THERMAL=m. > > Arnd Correct. I need to add following lines to the Kconfig file: depends on !CPU_THERMAL || THERMAL=y Hi Rafael, Should I send a new patch include this fix or send a fix patch? Thanks. -Hongtao.
On 01/11/2016 08:54 AM, Hongtao Jia wrote: > Sorry for the late response. I got a knee surgery to do. > See comments at the end. > >> -----邮件原件----- >> 发件人: Arnd Bergmann [mailto:arnd@arndb.de] >> 发送时间: Saturday, December 19, 2015 6:33 AM >> 收件人: Rafael J. Wysocki <rjw@rjwysocki.net> >> 抄送: Jia Hongtao <hongtao.jia@freescale.com>; edubezval@gmail.com; >> viresh.kumar@linaro.org; linux-pm@vger.kernel.org; linuxppc- >> dev@lists.ozlabs.org; devicetree@vger.kernel.org; Scott Wood >> <scottwood@freescale.com> >> 主题: Re: [PATCH V3] cpufreq: qoriq: Register cooling device based on device >> tree >> >> On Tuesday 15 December 2015 00:58:26 Rafael J. Wysocki wrote: >>> On Thursday, November 26, 2015 05:21:11 PM Jia Hongtao wrote: >>>> Register the qoriq cpufreq driver as a cooling device, based on the >>>> thermal device tree framework. When temperature crosses the passive >>>> trip point cpufreq is used to throttle CPUs. >>>> >>>> Signed-off-by: Jia Hongtao <hongtao.jia@freescale.com> >>>> Reviewed-by: Viresh Kumar <viresh.kumar@linaro.org> >>> >>> Applied, thanks! >>> >> >> I got a randconfig build error today: >> >> drivers/built-in.o: In function `qoriq_cpufreq_ready': >> debugfs.c:(.text+0x1f4688): undefined reference to >> `of_cpufreq_cooling_register' >> >> CONFIG_OF=y >> CONFIG_QORIQ_CPUFREQ=y >> CONFIG_THERMAL=m >> CONFIG_THERMAL_OF=y >> >> I think you need a 'depends on THERMAL' to prevent the driver from being >> built-in when THERMAL=m. >> >> Arnd > > Correct. I need to add following lines to the Kconfig file: > depends on !CPU_THERMAL || THERMAL=y > > Hi Rafael, > Should I send a new patch include this fix or send a fix patch? Why THERMAL=y and not just THERMAL, which would allow building this driver as a module? -Scott
On Monday 11 January 2016 17:34:52 Scott Wood wrote: > >> > >> I think you need a 'depends on THERMAL' to prevent the driver from being > >> built-in when THERMAL=m. > >> > >> Arnd > > > > Correct. I need to add following lines to the Kconfig file: > > depends on !CPU_THERMAL || THERMAL=y > > > > Hi Rafael, > > Should I send a new patch include this fix or send a fix patch? > > Why THERMAL=y and not just THERMAL, which would allow building this > driver as a module? Right, that would be better, and it is what all other drivers do. For some reason, some drivers depend on !CPU_THERMAL and others depend on !THERMAL_OF here, and I think the result is the same, but we are a bit inconsistent here. CPU_THERMAL cannot be set if THERMAL_OF is disabled, and the header file only uses the 'extern' declaration if both are set. Arnd
On Fri, Dec 18, 2015 at 4:32 PM, Arnd Bergmann <arnd@arndb.de> wrote: > On Tuesday 15 December 2015 00:58:26 Rafael J. Wysocki wrote: >> On Thursday, November 26, 2015 05:21:11 PM Jia Hongtao wrote: >> > Register the qoriq cpufreq driver as a cooling device, based on the >> > thermal device tree framework. When temperature crosses the passive trip >> > point cpufreq is used to throttle CPUs. >> > >> > Signed-off-by: Jia Hongtao <hongtao.jia@freescale.com> >> > Reviewed-by: Viresh Kumar <viresh.kumar@linaro.org> >> >> Applied, thanks! >> > > I got a randconfig build error today: > > drivers/built-in.o: In function `qoriq_cpufreq_ready': > debugfs.c:(.text+0x1f4688): undefined reference to `of_cpufreq_cooling_register' > > CONFIG_OF=y > CONFIG_QORIQ_CPUFREQ=y > CONFIG_THERMAL=m > CONFIG_THERMAL_OF=y > > I think you need a 'depends on THERMAL' to prevent the driver from > being built-in when THERMAL=m. Maybe this is not the best approach. The cpufreq feature itself should be working independently without thermal framework. I think we should make the qoriq_cpufreq_ready() defined as null function if THERMAL is not defined. Regards, Leo
diff --git a/drivers/cpufreq/qoriq-cpufreq.c b/drivers/cpufreq/qoriq-cpufreq.c index 4f53fa2..cb6a62c 100644 --- a/drivers/cpufreq/qoriq-cpufreq.c +++ b/drivers/cpufreq/qoriq-cpufreq.c @@ -12,6 +12,7 @@ #include <linux/clk.h> #include <linux/cpufreq.h> +#include <linux/cpu_cooling.h> #include <linux/errno.h> #include <linux/init.h> #include <linux/kernel.h> @@ -33,6 +34,7 @@ struct cpu_data { struct clk **pclk; struct cpufreq_frequency_table *table; + struct thermal_cooling_device *cdev; }; /* @@ -260,6 +262,27 @@ static int qoriq_cpufreq_target(struct cpufreq_policy *policy, return clk_set_parent(policy->clk, parent); } + +static void qoriq_cpufreq_ready(struct cpufreq_policy *policy) +{ + struct cpu_data *cpud = policy->driver_data; + struct device_node *np = of_get_cpu_node(policy->cpu, NULL); + + if (of_find_property(np, "#cooling-cells", NULL)) { + cpud->cdev = of_cpufreq_cooling_register(np, + policy->related_cpus); + + if (IS_ERR(cpud->cdev)) { + pr_err("Failed to register cooling device cpu%d: %ld\n", + policy->cpu, PTR_ERR(cpud->cdev)); + + cpud->cdev = NULL; + } + } + + of_node_put(np); +} + static struct cpufreq_driver qoriq_cpufreq_driver = { .name = "qoriq_cpufreq", .flags = CPUFREQ_CONST_LOOPS, @@ -268,6 +291,7 @@ static struct cpufreq_driver qoriq_cpufreq_driver = { .verify = cpufreq_generic_frequency_table_verify, .target_index = qoriq_cpufreq_target, .get = cpufreq_generic_get, + .ready = qoriq_cpufreq_ready, .attr = cpufreq_generic_attr, };