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

2015-11-26 Thread Denis Kirjanov 
On 11/26/15, 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 
> ---
> Changes for V2:
> * Using ->ready callback for cpu cooling device registering.
>
>  drivers/cpufreq/qoriq-cpufreq.c | 27 +++
>  1 file changed, 27 insertions(+)
>
> diff --git a/drivers/cpufreq/qoriq-cpufreq.c
> b/drivers/cpufreq/qoriq-cpufreq.c
> index 4f53fa2..a39f868 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,30 @@ 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 (WARN_ON(!np))
> + return;
I doubt that WARN_ON is necessary here. Do you know the exact case
where the np is 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 +294,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
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

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

2015-11-26 Thread Hongtao Jia 


> -Original Message-
> From: Denis Kirjanov [mailto:k...@linux-powerpc.org]
> Sent: Thursday, November 26, 2015 4:32 PM
> To: Jia Hongtao-B38951
> Cc: edubez...@gmail.com; viresh.ku...@linaro.org; Wood Scott-B07421;
> devicet...@vger.kernel.org; linuxppc-dev@lists.ozlabs.org; linux-
> p...@vger.kernel.org
> Subject: Re: [PATCH V2] cpufreq: qoriq: Register cooling device based on
> device tree
> 
> On 11/26/15, Jia Hongtao <hongtao@freescale.com> 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>
> > ---
> > Changes for V2:
> > * Using ->ready callback for cpu cooling device registering.
> >
> >  drivers/cpufreq/qoriq-cpufreq.c | 27 +++
> >  1 file changed, 27 insertions(+)
> >
> > diff --git a/drivers/cpufreq/qoriq-cpufreq.c
> > b/drivers/cpufreq/qoriq-cpufreq.c index 4f53fa2..a39f868 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,30 @@ 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 (WARN_ON(!np))
> > +   return;
> I doubt that WARN_ON is necessary here. Do you know the exact case where
> the np is NULL?

This seems not necessary as you said.
If no one else has the opposite opinion I will remove it.

Thanks.

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

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

2015-11-26 Thread Viresh Kumar 
On 26-11-15, 14:27, Jia Hongtao wrote:
> +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 (WARN_ON(!np))
> + return;

Drop this and add my:

Reviewed-by: Viresh Kumar 

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

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

2015-11-26 Thread Hongtao Jia 
Thanks.
I just sent the new patch.

> -Original Message-
> From: Viresh Kumar [mailto:viresh.ku...@linaro.org]
> Sent: Thursday, November 26, 2015 5:08 PM
> To: Jia Hongtao-B38951
> Cc: edubez...@gmail.com; linux...@vger.kernel.org; linuxppc-
> d...@lists.ozlabs.org; devicet...@vger.kernel.org; Wood Scott-B07421
> Subject: Re: [PATCH V2] cpufreq: qoriq: Register cooling device based on
> device tree
> 
> On 26-11-15, 14:27, Jia Hongtao wrote:
> > +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 (WARN_ON(!np))
> > +   return;
> 
> Drop this and add my:
> 
> Reviewed-by: Viresh Kumar <viresh.ku...@linaro.org>
> 
> --
> viresh
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev