From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Rafael J. Wysocki" Subject: Re: [PATCH] cpufreq-dt: register cooling device after validating cpufreq table Date: Tue, 25 Nov 2014 23:05:25 +0100 Message-ID: <1447952.pbyqNN4RJG@vostro.rjw.lan> References: <2189780.D9n5NYrXn8@vostro.rjw.lan> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7Bit Return-path: Received: from v094114.home.net.pl ([79.96.170.134]:49591 "HELO v094114.home.net.pl" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with SMTP id S1750913AbaKYVoL (ORCPT ); Tue, 25 Nov 2014 16:44:11 -0500 In-Reply-To: <2189780.D9n5NYrXn8@vostro.rjw.lan> Sender: linux-pm-owner@vger.kernel.org List-Id: linux-pm@vger.kernel.org To: Viresh Kumar Cc: linaro-kernel@lists.linaro.org, linux-pm@vger.kernel.org, l.majewski@samsung.com, edubezval@gmail.com On Tuesday, November 25, 2014 10:49:32 PM Rafael J. Wysocki wrote: > On Monday, November 24, 2014 04:29:13 PM Viresh Kumar wrote: > > of_cpufreq_cooling_register() can use frequency values from > > policy->min/max/cpuinfo.min_freq/cpuinfo.max_freq, which are available only > > after calling cpufreq_table_validate_and_show(). > > > > The right order of calling should be: cpufreq_table_validate_and_show() followed > > by of_cpufreq_cooling_register(). Fix it. > > > > Reported-by: Lukasz Majewski > > Reported-by: Eduardo Valentin > > Signed-off-by: Viresh Kumar > > > > --- > > For 3.18. > > Plus "stable" I suppose? Which ones? And what bad things are going to happen if this is not pushed for 3.18? > > --- > > drivers/cpufreq/cpufreq-dt.c | 35 +++++++++++++++++------------------ > > 1 file changed, 17 insertions(+), 18 deletions(-) > > > > diff --git a/drivers/cpufreq/cpufreq-dt.c b/drivers/cpufreq/cpufreq-dt.c > > index 8cba13d..22eb6e5 100644 > > --- a/drivers/cpufreq/cpufreq-dt.c > > +++ b/drivers/cpufreq/cpufreq-dt.c > > @@ -186,7 +186,6 @@ static int cpufreq_init(struct cpufreq_policy *policy) > > { > > struct cpufreq_dt_platform_data *pd; > > struct cpufreq_frequency_table *freq_table; > > - struct thermal_cooling_device *cdev; > > struct device_node *np; > > struct private_data *priv; > > struct device *cpu_dev; > > @@ -269,20 +268,6 @@ static int cpufreq_init(struct cpufreq_policy *policy) > > goto out_free_priv; > > } > > > > - /* > > - * For now, just loading the cooling device; > > - * thermal DT code takes care of matching them. > > - */ > > - if (of_find_property(np, "#cooling-cells", NULL)) { > > - cdev = of_cpufreq_cooling_register(np, cpu_present_mask); > > - if (IS_ERR(cdev)) > > - dev_err(cpu_dev, > > - "running cpufreq without cooling device: %ld\n", > > - PTR_ERR(cdev)); > > - else > > - priv->cdev = cdev; > > - } > > - > > priv->cpu_dev = cpu_dev; > > priv->cpu_reg = cpu_reg; > > policy->driver_data = priv; > > @@ -292,7 +277,22 @@ static int cpufreq_init(struct cpufreq_policy *policy) > > if (ret) { > > dev_err(cpu_dev, "%s: invalid frequency table: %d\n", __func__, > > ret); > > - goto out_cooling_unregister; > > + goto out_free_cpufreq_table; > > + } > > + > > + /* > > + * For now, just loading the cooling device; > > + * thermal DT code takes care of matching them. > > + */ > > + if (of_find_property(np, "#cooling-cells", NULL)) { > > + priv->cdev = of_cpufreq_cooling_register(np, cpu_present_mask); > > + if (IS_ERR(priv->cdev)) { > > + dev_err(cpu_dev, > > + "running cpufreq without cooling device: %ld\n", > > + PTR_ERR(priv->cdev)); > > + > > + priv->cdev = NULL; > > + } > > } > > > > policy->cpuinfo.transition_latency = transition_latency; > > @@ -305,8 +305,7 @@ static int cpufreq_init(struct cpufreq_policy *policy) > > > > return 0; > > > > -out_cooling_unregister: > > - cpufreq_cooling_unregister(priv->cdev); > > +out_free_cpufreq_table: > > dev_pm_opp_free_cpufreq_table(cpu_dev, &freq_table); > > out_free_priv: > > kfree(priv); > > > > -- I speak only for myself. Rafael J. Wysocki, Intel Open Source Technology Center.