| Message ID | 1464690722-27996-1-git-send-email-lukasz.luba@arm.com (mailing list archive) |
|---|---|
| State | Accepted, archived |
| Delegated to: | Zhang Rui |
| Headers | show |
On Tue, May 31, 2016 at 11:32:02AM +0100, Lukasz Luba wrote: > The freq_table array is not populated before calling > thermal_of_cooling_register. The code which populates the freq table was > introduced in commit f6859014. > This should be done before registering new thermal cooling device. > The log shows effects of this wrong decision. > [ 2.172614] cpu cpu1: Failed to get voltage for frequency 1984518656000: -34 > [ 2.220863] cpu cpu0: Failed to get voltage for frequency 1984524416000: -34 > > Signed-off-by: Lukasz Luba <lukasz.luba@arm.com> Yes, the cooling device should only be registered after all its fields have been populated. Acked-by: Javi Merino <javi.merino@arm.com> > --- > drivers/thermal/cpu_cooling.c | 16 ++++++++-------- > 1 file changed, 8 insertions(+), 8 deletions(-) > > diff --git a/drivers/thermal/cpu_cooling.c b/drivers/thermal/cpu_cooling.c > index 6ceac4f..5b4b47e 100644 > --- a/drivers/thermal/cpu_cooling.c > +++ b/drivers/thermal/cpu_cooling.c > @@ -857,14 +857,6 @@ __cpufreq_cooling_register(struct device_node *np, > goto free_power_table; > } > > - snprintf(dev_name, sizeof(dev_name), "thermal-cpufreq-%d", > - cpufreq_dev->id); > - > - cool_dev = thermal_of_cooling_device_register(np, dev_name, cpufreq_dev, > - &cpufreq_cooling_ops); > - if (IS_ERR(cool_dev)) > - goto remove_idr; > - > /* Fill freq-table in descending order of frequencies */ > for (i = 0, freq = -1; i <= cpufreq_dev->max_level; i++) { > freq = find_next_max(table, freq); > @@ -877,6 +869,14 @@ __cpufreq_cooling_register(struct device_node *np, > pr_debug("%s: freq:%u KHz\n", __func__, freq); > } > > + snprintf(dev_name, sizeof(dev_name), "thermal-cpufreq-%d", > + cpufreq_dev->id); > + > + cool_dev = thermal_of_cooling_device_register(np, dev_name, cpufreq_dev, > + &cpufreq_cooling_ops); > + if (IS_ERR(cool_dev)) > + goto remove_idr; > + > cpufreq_dev->clipped_freq = cpufreq_dev->freq_table[0]; > cpufreq_dev->cool_dev = cool_dev; > > -- > 1.9.1 > -- 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 31-05-16, 11:32, Lukasz Luba wrote: > The freq_table array is not populated before calling > thermal_of_cooling_register. The code which populates the freq table was > introduced in commit f6859014. > This should be done before registering new thermal cooling device. > The log shows effects of this wrong decision. > [ 2.172614] cpu cpu1: Failed to get voltage for frequency 1984518656000: -34 > [ 2.220863] cpu cpu0: Failed to get voltage for frequency 1984524416000: -34 > You should have added this as well: Cc: 4.19+ <stable@vger.kernel.org> # 4.19+ Fixes: f6859014c7e7 ("thermal: cpu_cooling: Store frequencies in descending order") > Signed-off-by: Lukasz Luba <lukasz.luba@arm.com> > --- > drivers/thermal/cpu_cooling.c | 16 ++++++++-------- > 1 file changed, 8 insertions(+), 8 deletions(-) Acked-by: Viresh Kumar <viresh.kumar@linaro.org>
On 01/06/16 12:10, Viresh Kumar wrote: > On 31-05-16, 11:32, Lukasz Luba wrote: >> The freq_table array is not populated before calling >> thermal_of_cooling_register. The code which populates the freq table was >> introduced in commit f6859014. >> This should be done before registering new thermal cooling device. >> The log shows effects of this wrong decision. >> [ 2.172614] cpu cpu1: Failed to get voltage for frequency 1984518656000: -34 >> [ 2.220863] cpu cpu0: Failed to get voltage for frequency 1984524416000: -34 >> > > You should have added this as well: > > Cc: 4.19+ <stable@vger.kernel.org> # 4.19+ You mean 3.19+ Cc: <stable@vger.kernel.org> # 3.19+ > Fixes: f6859014c7e7 ("thermal: cpu_cooling: Store frequencies in descending order") > >> Signed-off-by: Lukasz Luba <lukasz.luba@arm.com> >> --- >> drivers/thermal/cpu_cooling.c | 16 ++++++++-------- >> 1 file changed, 8 insertions(+), 8 deletions(-) > > Acked-by: Viresh Kumar <viresh.kumar@linaro.org> > Eduardo can you pick these tags when you merge it? Thanks, Lukasz -- 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 01-06-16, 14:31, Lukasz Luba wrote: > > > On 01/06/16 12:10, Viresh Kumar wrote: > >On 31-05-16, 11:32, Lukasz Luba wrote: > >>The freq_table array is not populated before calling > >>thermal_of_cooling_register. The code which populates the freq table was > >>introduced in commit f6859014. > >>This should be done before registering new thermal cooling device. > >>The log shows effects of this wrong decision. > >>[ 2.172614] cpu cpu1: Failed to get voltage for frequency 1984518656000: -34 > >>[ 2.220863] cpu cpu0: Failed to get voltage for frequency 1984524416000: -34 > >> > > > >You should have added this as well: > > > >Cc: 4.19+ <stable@vger.kernel.org> # 4.19+ > > You mean 3.19+ > Cc: <stable@vger.kernel.org> # 3.19+ Urg. Thanks for fixing this :)
On Wed, 2016-06-01 at 14:31 +0100, Lukasz Luba wrote: > > On 01/06/16 12:10, Viresh Kumar wrote: > > On 31-05-16, 11:32, Lukasz Luba wrote: > >> The freq_table array is not populated before calling > >> thermal_of_cooling_register. The code which populates the freq table was > >> introduced in commit f6859014. > >> This should be done before registering new thermal cooling device. > >> The log shows effects of this wrong decision. > >> [ 2.172614] cpu cpu1: Failed to get voltage for frequency 1984518656000: -34 > >> [ 2.220863] cpu cpu0: Failed to get voltage for frequency 1984524416000: -34 > >> > > > > You should have added this as well: > > > > Cc: 4.19+ <stable@vger.kernel.org> # 4.19+ > > You mean 3.19+ > Cc: <stable@vger.kernel.org> # 3.19+ > > > Fixes: f6859014c7e7 ("thermal: cpu_cooling: Store frequencies in descending order") > > > >> Signed-off-by: Lukasz Luba <lukasz.luba@arm.com> > >> --- > >> drivers/thermal/cpu_cooling.c | 16 ++++++++-------- > >> 1 file changed, 8 insertions(+), 8 deletions(-) > > > > Acked-by: Viresh Kumar <viresh.kumar@linaro.org> > > > Eduardo can you pick these tags when you merge it? Patch queued for -rc2. :) thanks, rui
diff --git a/drivers/thermal/cpu_cooling.c b/drivers/thermal/cpu_cooling.c index 6ceac4f..5b4b47e 100644 --- a/drivers/thermal/cpu_cooling.c +++ b/drivers/thermal/cpu_cooling.c @@ -857,14 +857,6 @@ __cpufreq_cooling_register(struct device_node *np, goto free_power_table; } - snprintf(dev_name, sizeof(dev_name), "thermal-cpufreq-%d", - cpufreq_dev->id); - - cool_dev = thermal_of_cooling_device_register(np, dev_name, cpufreq_dev, - &cpufreq_cooling_ops); - if (IS_ERR(cool_dev)) - goto remove_idr; - /* Fill freq-table in descending order of frequencies */ for (i = 0, freq = -1; i <= cpufreq_dev->max_level; i++) { freq = find_next_max(table, freq); @@ -877,6 +869,14 @@ __cpufreq_cooling_register(struct device_node *np, pr_debug("%s: freq:%u KHz\n", __func__, freq); } + snprintf(dev_name, sizeof(dev_name), "thermal-cpufreq-%d", + cpufreq_dev->id); + + cool_dev = thermal_of_cooling_device_register(np, dev_name, cpufreq_dev, + &cpufreq_cooling_ops); + if (IS_ERR(cool_dev)) + goto remove_idr; + cpufreq_dev->clipped_freq = cpufreq_dev->freq_table[0]; cpufreq_dev->cool_dev = cool_dev;
The freq_table array is not populated before calling thermal_of_cooling_register. The code which populates the freq table was introduced in commit f6859014. This should be done before registering new thermal cooling device. The log shows effects of this wrong decision. [ 2.172614] cpu cpu1: Failed to get voltage for frequency 1984518656000: -34 [ 2.220863] cpu cpu0: Failed to get voltage for frequency 1984524416000: -34 Signed-off-by: Lukasz Luba <lukasz.luba@arm.com> --- drivers/thermal/cpu_cooling.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-)