Hi,
This series contains minor fixes/cleanups for thermal cooling drivers.
V1->V2: - s/dev_warn/dev_err (Rafael) - Two new patches to make similar (^^) change at other places
Viresh Kumar (5): thermal: devfreq: Simplify expression thermal: devfreq_cooling: Replace dev_warn with dev_err thermal: devfreq: Check OPP for errors thermal: cpu_cooling: Replace dev_warn with dev_err thermal: cpu_cooling: Check OPP for errors
drivers/thermal/cpu_cooling.c | 19 +++++++++++++------ drivers/thermal/devfreq_cooling.c | 14 ++++++++++---- 2 files changed, 23 insertions(+), 10 deletions(-)
There is no need to check for IS_ERR() as we are looking for a very particular error value here. Drop the first check.
Reported-by: Dan Carpenter dan.carpenter@oracle.com Signed-off-by: Viresh Kumar viresh.kumar@linaro.org --- drivers/thermal/devfreq_cooling.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/thermal/devfreq_cooling.c b/drivers/thermal/devfreq_cooling.c index ba7a5cd994dc..cf71550b9d00 100644 --- a/drivers/thermal/devfreq_cooling.c +++ b/drivers/thermal/devfreq_cooling.c @@ -222,7 +222,7 @@ get_static_power(struct devfreq_cooling_device *dfc, unsigned long freq) return 0;
opp = dev_pm_opp_find_freq_exact(dev, freq, true); - if (IS_ERR(opp) && (PTR_ERR(opp) == -ERANGE)) + if (PTR_ERR(opp) == -ERANGE) opp = dev_pm_opp_find_freq_exact(dev, freq, false);
voltage = dev_pm_opp_get_voltage(opp) / 1000; /* mV */
There isn't much the user can do on seeing this warning, as the hardware is actually okay. dev_err suits much better here.
Signed-off-by: Viresh Kumar viresh.kumar@linaro.org --- drivers/thermal/devfreq_cooling.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/thermal/devfreq_cooling.c b/drivers/thermal/devfreq_cooling.c index cf71550b9d00..218ccc30c7ad 100644 --- a/drivers/thermal/devfreq_cooling.c +++ b/drivers/thermal/devfreq_cooling.c @@ -229,9 +229,9 @@ get_static_power(struct devfreq_cooling_device *dfc, unsigned long freq) dev_pm_opp_put(opp);
if (voltage == 0) { - dev_warn_ratelimited(dev, - "Failed to get voltage for frequency %lu: %ld\n", - freq, IS_ERR(opp) ? PTR_ERR(opp) : 0); + dev_err_ratelimited(dev, + "Failed to get voltage for frequency %lu: %ld\n", + freq, IS_ERR(opp) ? PTR_ERR(opp) : 0); return 0; }
It is possible for dev_pm_opp_find_freq_exact() to return errors. It was all fine earlier as dev_pm_opp_get_voltage() had a check within it to check for invalid OPPs, but dev_pm_opp_put() doesn't have any similar checks and the callers need to make sure OPP is valid before calling them.
Also update the later dev_warn_ratelimited() to not print the error message as the OPP is guaranteed to be valid now.
Reported-by: Dan Carpenter dan.carpenter@oracle.com Signed-off-by: Viresh Kumar viresh.kumar@linaro.org --- drivers/thermal/devfreq_cooling.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/drivers/thermal/devfreq_cooling.c b/drivers/thermal/devfreq_cooling.c index 218ccc30c7ad..74d4846c66cd 100644 --- a/drivers/thermal/devfreq_cooling.c +++ b/drivers/thermal/devfreq_cooling.c @@ -225,13 +225,19 @@ get_static_power(struct devfreq_cooling_device *dfc, unsigned long freq) if (PTR_ERR(opp) == -ERANGE) opp = dev_pm_opp_find_freq_exact(dev, freq, false);
+ if (IS_ERR(opp)) { + dev_err_ratelimited(dev, "Failed to find OPP for frequency %lu: %ld\n", + freq, PTR_ERR(opp)); + return 0; + } + voltage = dev_pm_opp_get_voltage(opp) / 1000; /* mV */ dev_pm_opp_put(opp);
if (voltage == 0) { dev_err_ratelimited(dev, - "Failed to get voltage for frequency %lu: %ld\n", - freq, IS_ERR(opp) ? PTR_ERR(opp) : 0); + "Failed to get voltage for frequency %lu\n", + freq); return 0; }
There isn't much the user can do on seeing these warnings, as the hardware is actually okay. dev_err suits much better here.
Signed-off-by: Viresh Kumar viresh.kumar@linaro.org --- drivers/thermal/cpu_cooling.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/thermal/cpu_cooling.c b/drivers/thermal/cpu_cooling.c index 85fdbf762fa0..caaaeb3ac0f0 100644 --- a/drivers/thermal/cpu_cooling.c +++ b/drivers/thermal/cpu_cooling.c @@ -435,9 +435,9 @@ static int get_static_power(struct cpufreq_cooling_device *cpufreq_device, dev_pm_opp_put(opp);
if (voltage == 0) { - dev_warn_ratelimited(cpufreq_device->cpu_dev, - "Failed to get voltage for frequency %lu: %ld\n", - freq_hz, IS_ERR(opp) ? PTR_ERR(opp) : 0); + dev_err_ratelimited(cpufreq_device->cpu_dev, + "Failed to get voltage for frequency %lu: %ld\n", + freq_hz, IS_ERR(opp) ? PTR_ERR(opp) : 0); return -EINVAL; }
@@ -721,9 +721,9 @@ static int cpufreq_power2state(struct thermal_cooling_device *cdev,
*state = cpufreq_cooling_get_level(cpu, target_freq); if (*state == THERMAL_CSTATE_INVALID) { - dev_warn_ratelimited(&cdev->device, - "Failed to convert %dKHz for cpu %d into a cdev state\n", - target_freq, cpu); + dev_err_ratelimited(&cdev->device, + "Failed to convert %dKHz for cpu %d into a cdev state\n", + target_freq, cpu); return -EINVAL; }
It is possible for dev_pm_opp_find_freq_exact() to return errors. It was all fine earlier as dev_pm_opp_get_voltage() had a check within it to check for invalid OPPs, but dev_pm_opp_put() doesn't have any similar checks and the callers need to make sure OPP is valid before calling them.
Also update the later dev_warn_ratelimited() to not print the error message as the OPP is guaranteed to be valid now.
Reported-by: Dan Carpenter dan.carpenter@oracle.com Signed-off-by: Viresh Kumar viresh.kumar@linaro.org --- drivers/thermal/cpu_cooling.c | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-)
diff --git a/drivers/thermal/cpu_cooling.c b/drivers/thermal/cpu_cooling.c index caaaeb3ac0f0..30d736289b87 100644 --- a/drivers/thermal/cpu_cooling.c +++ b/drivers/thermal/cpu_cooling.c @@ -431,13 +431,20 @@ static int get_static_power(struct cpufreq_cooling_device *cpufreq_device,
opp = dev_pm_opp_find_freq_exact(cpufreq_device->cpu_dev, freq_hz, true); + if (IS_ERR(opp)) { + dev_warn_ratelimited(cpufreq_device->cpu_dev, + "Failed to find OPP for frequency %lu: %ld\n", + freq_hz, PTR_ERR(opp)); + return -EINVAL; + } + voltage = dev_pm_opp_get_voltage(opp); dev_pm_opp_put(opp);
if (voltage == 0) { dev_err_ratelimited(cpufreq_device->cpu_dev, - "Failed to get voltage for frequency %lu: %ld\n", - freq_hz, IS_ERR(opp) ? PTR_ERR(opp) : 0); + "Failed to get voltage for frequency %lu\n", + freq_hz); return -EINVAL; }
On Mon, 2017-02-20 at 15:59 +0530, Viresh Kumar wrote:
On 07-02-17, 09:39, Viresh Kumar wrote:
Hi,
This series contains minor fixes/cleanups for thermal cooling drivers.
Acked-by: Zhang Rui rui.zhang@intel.com for the whole patch series.
As this patch set depends on commit 8a31d9d94297 ("PM / OPP: Update OPP users to put reference"), which goes Rafael' tree, Rafael, can you please take this patch set as well?
thanks, rui
On 21-02-17, 14:30, Zhang Rui wrote:
On Mon, 2017-02-20 at 15:59 +0530, Viresh Kumar wrote:
On 07-02-17, 09:39, Viresh Kumar wrote:
Hi,
This series contains minor fixes/cleanups for thermal cooling drivers.
Acked-by: Zhang Rui rui.zhang@intel.com for the whole patch series.
As this patch set depends on commit 8a31d9d94297 ("PM / OPP: Update OPP users to put reference"), which goes Rafael' tree, Rafael, can you please take this patch set as well?
@Rafael: Can you please apply these as well ?
On 24-02-17, 14:19, Viresh Kumar wrote:
On 21-02-17, 14:30, Zhang Rui wrote:
On Mon, 2017-02-20 at 15:59 +0530, Viresh Kumar wrote:
On 07-02-17, 09:39, Viresh Kumar wrote:
Hi,
This series contains minor fixes/cleanups for thermal cooling drivers.
Acked-by: Zhang Rui rui.zhang@intel.com for the whole patch series.
As this patch set depends on commit 8a31d9d94297 ("PM / OPP: Update OPP users to put reference"), which goes Rafael' tree, Rafael, can you please take this patch set as well?
@Rafael: Can you please apply these as well ?
Ping !!
linaro-kernel@lists.linaro.org
-
Viresh Kumar -
Zhang Rui