Re: [PATCH V3] cpufreq: qoriq: Register cooling device based on device tree

[Arnd Bergmann]

On Monday 29 February 2016 15:33:37 Rafael J. Wysocki wrote:
> >
> > Note the check on !CPU_THERMAL rather than !THERMAL in my patch, that
> > part was correct. I think the line should be
> >
> > depends on !CPU_THERMAL || THERMAL
> >
> > as some other drivers do. I must have copied the line
> > from ARM_MT8173_CPUFREQ, which is a 'bool' symbol, when
> > it should have been the same as ARM_BIG_LITTLE_CPUFREQ and
> > CPUFREQ_DT.
> 
> OK, so any chance to update the patch?
> 

I've committed the update locally now, and will follow up tomorrow
with a new patch, unless it causes another build-time regression.

I'll also send the respective update for the mediatek driver, making
that a tristate option.

Arnd
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [PATCH V3] cpufreq: qoriq: Register cooling device based on device tree

[Rafael J. Wysocki]

On Mon, Feb 29, 2016 at 11:05 AM, Arnd Bergmann  wrote:
> On Friday 26 February 2016 18:41:06 Li Yang wrote:
>> >> >
>> >> > It would be perfect if this it true.  But I tried with the following
>> >> > change, it just makes QORIQ_CPUFREQ non-selectable if THERMAL=m.
>> >> >
>> >> > diff --git a/drivers/cpufreq/Kconfig b/drivers/cpufreq/Kconfig
>> >> > index dcb972a38fbc..ca05037dd565 100644
>> >> > --- a/drivers/cpufreq/Kconfig
>> >> > +++ b/drivers/cpufreq/Kconfig
>> >> > @@ -297,6 +297,7 @@ endif
>> >> >  config QORIQ_CPUFREQ
>> >> > tristate "CPU frequency scaling driver for Freescale QorIQ SoCs"
>> >> > depends on OF && COMMON_CLK && (PPC_E500MC || ARM)
>> >> > +   depends on !CPU_THERMAL || THERMAL=y
>> >> > select CLK_QORIQ
>> >> > help
>> >> >   This adds the CPUFreq driver support for Freescale QorIQ SoCs
>> >>
>
> Oops.
>
>> >> I find we can achieve your desired result with the following change 
>> >> instead:
>> >>
>> >> +   depends on (THERMAL=m && m) || THERMAL=y || THERMAL=n
>> >
>> > "depends on THERMAL || !THERMAL" should also work.
>>
>> Right.  And this is more simpler.
>
>
> Note the check on !CPU_THERMAL rather than !THERMAL in my patch, that
> part was correct. I think the line should be
>
> depends on !CPU_THERMAL || THERMAL
>
> as some other drivers do. I must have copied the line
> from ARM_MT8173_CPUFREQ, which is a 'bool' symbol, when
> it should have been the same as ARM_BIG_LITTLE_CPUFREQ and
> CPUFREQ_DT.

OK, so any chance to update the patch?

Thanks,
Rafael
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [PATCH V3] cpufreq: qoriq: Register cooling device based on device tree

[Arnd Bergmann]

On Friday 26 February 2016 18:41:06 Li Yang wrote:
> >> >
> >> > It would be perfect if this it true.  But I tried with the following
> >> > change, it just makes QORIQ_CPUFREQ non-selectable if THERMAL=m.
> >> >
> >> > diff --git a/drivers/cpufreq/Kconfig b/drivers/cpufreq/Kconfig
> >> > index dcb972a38fbc..ca05037dd565 100644
> >> > --- a/drivers/cpufreq/Kconfig
> >> > +++ b/drivers/cpufreq/Kconfig
> >> > @@ -297,6 +297,7 @@ endif
> >> >  config QORIQ_CPUFREQ
> >> > tristate "CPU frequency scaling driver for Freescale QorIQ SoCs"
> >> > depends on OF && COMMON_CLK && (PPC_E500MC || ARM)
> >> > +   depends on !CPU_THERMAL || THERMAL=y
> >> > select CLK_QORIQ
> >> > help
> >> >   This adds the CPUFreq driver support for Freescale QorIQ SoCs
> >>

Oops.

> >> I find we can achieve your desired result with the following change 
> >> instead:
> >>
> >> +   depends on (THERMAL=m && m) || THERMAL=y || THERMAL=n
> >
> > "depends on THERMAL || !THERMAL" should also work.
> 
> Right.  And this is more simpler.


Note the check on !CPU_THERMAL rather than !THERMAL in my patch, that
part was correct. I think the line should be

depends on !CPU_THERMAL || THERMAL

as some other drivers do. I must have copied the line
from ARM_MT8173_CPUFREQ, which is a 'bool' symbol, when
it should have been the same as ARM_BIG_LITTLE_CPUFREQ and
CPUFREQ_DT.

Arnd
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [PATCH V3] cpufreq: qoriq: Register cooling device based on device tree

[Li Yang]

On Fri, Feb 26, 2016 at 6:08 PM, Scott Wood  wrote:
> On Fri, 2016-02-26 at 18:04 -0600, Li Yang wrote:
>> On Fri, Feb 26, 2016 at 5:31 PM, Li Yang  wrote:
>> > On Fri, Feb 26, 2016 at 5:16 PM, Arnd Bergmann  wrote:
>> > > On Friday 26 February 2016 17:07:09 Li Yang wrote:
>> > > >
>> > > > I don't have a perfect solution either.  But I think this is still
>> > > > better than making cpufreq not usable.  The cpufreq driver will print
>> > > > out an error message if thermal is not reachable.  Maybe this can
>> > > > relief the confusion a little bit?
>> > >
>> > > With my patch, the configuration will just force the cpufreq
>> > > driver to be a loadable module as well if thermal is a module,
>> > > so the dependency can be resolved by loading the thermal module first.
>> >
>> > It would be perfect if this it true.  But I tried with the following
>> > change, it just makes QORIQ_CPUFREQ non-selectable if THERMAL=m.
>> >
>> > diff --git a/drivers/cpufreq/Kconfig b/drivers/cpufreq/Kconfig
>> > index dcb972a38fbc..ca05037dd565 100644
>> > --- a/drivers/cpufreq/Kconfig
>> > +++ b/drivers/cpufreq/Kconfig
>> > @@ -297,6 +297,7 @@ endif
>> >  config QORIQ_CPUFREQ
>> > tristate "CPU frequency scaling driver for Freescale QorIQ SoCs"
>> > depends on OF && COMMON_CLK && (PPC_E500MC || ARM)
>> > +   depends on !CPU_THERMAL || THERMAL=y
>> > select CLK_QORIQ
>> > help
>> >   This adds the CPUFreq driver support for Freescale QorIQ SoCs
>>
>>
>> I find we can achieve your desired result with the following change instead:
>>
>> +   depends on (THERMAL=m && m) || THERMAL=y || THERMAL=n
>
> "depends on THERMAL || !THERMAL" should also work.

Right.  And this is more simpler.

Regards,
Leo
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [PATCH V3] cpufreq: qoriq: Register cooling device based on device tree

[Scott Wood]

On Fri, 2016-02-26 at 18:04 -0600, Li Yang wrote:
> On Fri, Feb 26, 2016 at 5:31 PM, Li Yang  wrote:
> > On Fri, Feb 26, 2016 at 5:16 PM, Arnd Bergmann  wrote:
> > > On Friday 26 February 2016 17:07:09 Li Yang wrote:
> > > > 
> > > > I don't have a perfect solution either.  But I think this is still
> > > > better than making cpufreq not usable.  The cpufreq driver will print
> > > > out an error message if thermal is not reachable.  Maybe this can
> > > > relief the confusion a little bit?
> > > 
> > > With my patch, the configuration will just force the cpufreq
> > > driver to be a loadable module as well if thermal is a module,
> > > so the dependency can be resolved by loading the thermal module first.
> > 
> > It would be perfect if this it true.  But I tried with the following
> > change, it just makes QORIQ_CPUFREQ non-selectable if THERMAL=m.
> > 
> > diff --git a/drivers/cpufreq/Kconfig b/drivers/cpufreq/Kconfig
> > index dcb972a38fbc..ca05037dd565 100644
> > --- a/drivers/cpufreq/Kconfig
> > +++ b/drivers/cpufreq/Kconfig
> > @@ -297,6 +297,7 @@ endif
> >  config QORIQ_CPUFREQ
> > tristate "CPU frequency scaling driver for Freescale QorIQ SoCs"
> > depends on OF && COMMON_CLK && (PPC_E500MC || ARM)
> > +   depends on !CPU_THERMAL || THERMAL=y
> > select CLK_QORIQ
> > help
> >   This adds the CPUFreq driver support for Freescale QorIQ SoCs
> 
> 
> I find we can achieve your desired result with the following change instead:
> 
> +   depends on (THERMAL=m && m) || THERMAL=y || THERMAL=n

"depends on THERMAL || !THERMAL" should also work.

-Scott

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [PATCH V3] cpufreq: qoriq: Register cooling device based on device tree

[Li Yang]

On Fri, Feb 26, 2016 at 5:31 PM, Li Yang  wrote:
> On Fri, Feb 26, 2016 at 5:16 PM, Arnd Bergmann  wrote:
>> On Friday 26 February 2016 17:07:09 Li Yang wrote:
>>>
>>> I don't have a perfect solution either.  But I think this is still
>>> better than making cpufreq not usable.  The cpufreq driver will print
>>> out an error message if thermal is not reachable.  Maybe this can
>>> relief the confusion a little bit?
>>
>> With my patch, the configuration will just force the cpufreq
>> driver to be a loadable module as well if thermal is a module,
>> so the dependency can be resolved by loading the thermal module first.
>
> It would be perfect if this it true.  But I tried with the following
> change, it just makes QORIQ_CPUFREQ non-selectable if THERMAL=m.
>
> diff --git a/drivers/cpufreq/Kconfig b/drivers/cpufreq/Kconfig
> index dcb972a38fbc..ca05037dd565 100644
> --- a/drivers/cpufreq/Kconfig
> +++ b/drivers/cpufreq/Kconfig
> @@ -297,6 +297,7 @@ endif
>  config QORIQ_CPUFREQ
> tristate "CPU frequency scaling driver for Freescale QorIQ SoCs"
> depends on OF && COMMON_CLK && (PPC_E500MC || ARM)
> +   depends on !CPU_THERMAL || THERMAL=y
> select CLK_QORIQ
> help
>   This adds the CPUFreq driver support for Freescale QorIQ SoCs


I find we can achieve your desired result with the following change instead:

+   depends on (THERMAL=m && m) || THERMAL=y || THERMAL=n

Regards,
Leo
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [PATCH V3] cpufreq: qoriq: Register cooling device based on device tree

[Li Yang]

On Fri, Feb 26, 2016 at 5:16 PM, Arnd Bergmann  wrote:
> On Friday 26 February 2016 17:07:09 Li Yang wrote:
>>
>> I don't have a perfect solution either.  But I think this is still
>> better than making cpufreq not usable.  The cpufreq driver will print
>> out an error message if thermal is not reachable.  Maybe this can
>> relief the confusion a little bit?
>
> With my patch, the configuration will just force the cpufreq
> driver to be a loadable module as well if thermal is a module,
> so the dependency can be resolved by loading the thermal module first.

It would be perfect if this it true.  But I tried with the following
change, it just makes QORIQ_CPUFREQ non-selectable if THERMAL=m.

diff --git a/drivers/cpufreq/Kconfig b/drivers/cpufreq/Kconfig
index dcb972a38fbc..ca05037dd565 100644
--- a/drivers/cpufreq/Kconfig
+++ b/drivers/cpufreq/Kconfig
@@ -297,6 +297,7 @@ endif
 config QORIQ_CPUFREQ
tristate "CPU frequency scaling driver for Freescale QorIQ SoCs"
depends on OF && COMMON_CLK && (PPC_E500MC || ARM)
+   depends on !CPU_THERMAL || THERMAL=y
select CLK_QORIQ
help
  This adds the CPUFreq driver support for Freescale QorIQ SoCs

Regards,
Leo
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [PATCH V3] cpufreq: qoriq: Register cooling device based on device tree

[Arnd Bergmann]

On Friday 26 February 2016 17:07:09 Li Yang wrote:
> 
> I don't have a perfect solution either.  But I think this is still
> better than making cpufreq not usable.  The cpufreq driver will print
> out an error message if thermal is not reachable.  Maybe this can
> relief the confusion a little bit?

With my patch, the configuration will just force the cpufreq
driver to be a loadable module as well if thermal is a module,
so the dependency can be resolved by loading the thermal module first.

I think that is really the best way around the problem, and it
matches what other platforms do for the same problem.

Arnd
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [PATCH V3] cpufreq: qoriq: Register cooling device based on device tree

[Li Yang]

On Fri, Feb 26, 2016 at 2:20 PM, Arnd Bergmann  wrote:
> On Friday 26 February 2016 12:04:59 Li Yang wrote:
>> On Fri, Dec 18, 2015 at 4:32 PM, Arnd Bergmann  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 
>> >> > Reviewed-by: Viresh Kumar 
>> >>
>> >> 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.
>
> It already does this when CONFIG_THERMAL is not defined, and my
> patch doesn't change that. I'm not sure what you are asking for now.

Oh.  Actually I didn't see you just sent a patch for this.  I
accidentally get into this topic when I tried to find out why cpufreq
is not working on ARMv8 platforms.  I didn't notice that
of_cpufreq_cooling_register() is already ifdef-ed.

>
> Do you want to allow using the cpufreq driver as a built-in driver
> even when the thermal code is in a module, and then silently skip
> all thermal management as if it was turned off?

Having thermal code to be built as module and qoriq_cpufreq to be
built-in is a valid situation.  Making cpufreq not possible to be used
when thermal is a module doesn't seem to be right.

>
> That would be this patch:
>
> diff --git a/include/linux/cpu_cooling.h b/include/linux/cpu_cooling.h
> index c156f5082758..a8d9241fc1bb 100644
> --- a/include/linux/cpu_cooling.h
> +++ b/include/linux/cpu_cooling.h
> @@ -48,7 +48,7 @@ cpufreq_power_cooling_register(const struct cpumask 
> *clip_cpus,
>   * @np: a valid struct device_node to the cooling device device tree node.
>   * @clip_cpus: cpumask of cpus where the frequency constraints will happen
>   */
> -#ifdef CONFIG_THERMAL_OF
> +#if IS_REACHABLE(CONFIG_THERMAL_OF)
>  struct thermal_cooling_device *
>  of_cpufreq_cooling_register(struct device_node *np,
> const struct cpumask *clip_cpus);
>
>
> but my feeling is that this would cause more surprises when users
> find their thermal management is not active even though it was
> enabled in Kconfig and the thermal module gets loaded.

I don't have a perfect solution either.  But I think this is still
better than making cpufreq not usable.  The cpufreq driver will print
out an error message if thermal is not reachable.  Maybe this can
relief the confusion a little bit?

Regards,
Leo
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [PATCH V3] cpufreq: qoriq: Register cooling device based on device tree

[Arnd Bergmann]

On Friday 26 February 2016 12:04:59 Li Yang wrote:
> On Fri, Dec 18, 2015 at 4:32 PM, Arnd Bergmann  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 
> >> > Reviewed-by: Viresh Kumar 
> >>
> >> 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.

It already does this when CONFIG_THERMAL is not defined, and my
patch doesn't change that. I'm not sure what you are asking for now.

Do you want to allow using the cpufreq driver as a built-in driver
even when the thermal code is in a module, and then silently skip
all thermal management as if it was turned off?

That would be this patch:

diff --git a/include/linux/cpu_cooling.h b/include/linux/cpu_cooling.h
index c156f5082758..a8d9241fc1bb 100644
--- a/include/linux/cpu_cooling.h
+++ b/include/linux/cpu_cooling.h
@@ -48,7 +48,7 @@ cpufreq_power_cooling_register(const struct cpumask 
*clip_cpus,
  * @np: a valid struct device_node to the cooling device device tree node.
  * @clip_cpus: cpumask of cpus where the frequency constraints will happen
  */
-#ifdef CONFIG_THERMAL_OF
+#if IS_REACHABLE(CONFIG_THERMAL_OF)
 struct thermal_cooling_device *
 of_cpufreq_cooling_register(struct device_node *np,
const struct cpumask *clip_cpus);


but my feeling is that this would cause more surprises when users
find their thermal management is not active even though it was
enabled in Kconfig and the thermal module gets loaded.

Arnd
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [PATCH V3] cpufreq: qoriq: Register cooling device based on device tree

[Li Yang]

On Fri, Dec 18, 2015 at 4:32 PM, Arnd Bergmann  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 
>> > Reviewed-by: Viresh Kumar 
>>
>> 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
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: 答复: [PATCH V3] cpufreq: qoriq: Register cooling device based on device tree

[Arnd Bergmann]

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
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

答复: [PATCH V3] cpufreq: qoriq: Register cooling device based on device tree

[Hongtao Jia]

Sorry for the late response. I got a knee surgery to do.
See comments at the end.

> -邮件原件-
> 发件人: Arnd Bergmann [mailto:a...@arndb.de]
> 发送时间: Saturday, December 19, 2015 6:33 AM
> 收件人: Rafael J. Wysocki <r...@rjwysocki.net>
> 抄送: Jia Hongtao <hongtao@freescale.com>; edubez...@gmail.com;
> viresh.ku...@linaro.org; linux...@vger.kernel.org; linuxppc-
> d...@lists.ozlabs.org; devicet...@vger.kernel.org; Scott Wood
> <scottw...@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@freescale.com>
> > > Reviewed-by: Viresh Kumar <viresh.ku...@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.
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: 答复: [PATCH V3] cpufreq: qoriq: Register cooling device based on device tree

[Scott Wood]

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:a...@arndb.de]
>> 发送时间: Saturday, December 19, 2015 6:33 AM
>> 收件人: Rafael J. Wysocki <r...@rjwysocki.net>
>> 抄送: Jia Hongtao <hongtao@freescale.com>; edubez...@gmail.com;
>> viresh.ku...@linaro.org; linux...@vger.kernel.org; linuxppc-
>> d...@lists.ozlabs.org; devicet...@vger.kernel.org; Scott Wood
>> <scottw...@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@freescale.com>
>>>> Reviewed-by: Viresh Kumar <viresh.ku...@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

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [PATCH V3] cpufreq: qoriq: Register cooling device based on device tree

[Arnd Bergmann]

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 
> > Reviewed-by: Viresh Kumar 
> 
> 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
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [PATCH V3] cpufreq: qoriq: Register cooling device based on device tree

[Rafael J. Wysocki]

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 
> Reviewed-by: Viresh Kumar 

Applied, thanks!

Rafael

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

[PATCH V3] cpufreq: qoriq: Register cooling device based on device tree

[Jia Hongtao]

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 
Reviewed-by: Viresh Kumar 
---
Changes for V3:
* Removed unnecessary cpu node NULL check.

Changes for V2:
* Using ->ready callback for cpu cooling device registering.

 drivers/cpufreq/qoriq-cpufreq.c | 24 
 1 file changed, 24 insertions(+)

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 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -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,
 };
 
-- 
2.1.0.27.g96db324

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev