[PATCH V3 0/5] thermal: minor cleanup/fixes

[PATCH V3 0/5] thermal: minor cleanup/fixes

[Viresh Kumar]

Hi,

This series contains minor fixes/cleanups for thermal cooling drivers.

V2->V3:
- Added Acks from Rui
- Resending as it was getting ignored until now :(

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(-)

-- 
2.7.1.410.g6faf27b

[PATCH V3 1/5] thermal: devfreq: Simplify expression

[Viresh Kumar]

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>
Acked-by: Zhang Rui <rui.zhang@intel.com>
---
 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 7743a78d4723..4793fc7b06dd 100644
--- a/drivers/thermal/devfreq_cooling.c
+++ b/drivers/thermal/devfreq_cooling.c
@@ -186,7 +186,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 */
-- 
2.7.1.410.g6faf27b

[PATCH V3 2/5] thermal: devfreq_cooling: Replace dev_warn with dev_err

[Viresh Kumar]

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>
Acked-by: Zhang Rui <rui.zhang@intel.com>
---
 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 4793fc7b06dd..e99d0e026a53 100644
--- a/drivers/thermal/devfreq_cooling.c
+++ b/drivers/thermal/devfreq_cooling.c
@@ -193,9 +193,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;
 	}
 
-- 
2.7.1.410.g6faf27b

[PATCH V3 3/5] thermal: devfreq: Check OPP for errors

[Viresh Kumar]

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>
Acked-by: Zhang Rui <rui.zhang@intel.com>
---
 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 e99d0e026a53..4bf4ad58cffd 100644
--- a/drivers/thermal/devfreq_cooling.c
+++ b/drivers/thermal/devfreq_cooling.c
@@ -189,13 +189,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;
 	}
 
-- 
2.7.1.410.g6faf27b

[PATCH V3 4/5] thermal: cpu_cooling: Replace dev_warn with dev_err

[Viresh Kumar]

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>
Acked-by: Zhang Rui <rui.zhang@intel.com>
---
 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 91048eeca28b..2f9a1c7e7b1f 100644
--- a/drivers/thermal/cpu_cooling.c
+++ b/drivers/thermal/cpu_cooling.c
@@ -399,9 +399,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;
 	}
 
@@ -693,9 +693,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;
 	}
 
-- 
2.7.1.410.g6faf27b

[PATCH V3 5/5] thermal: cpu_cooling: Check OPP for errors

[Viresh Kumar]

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>
Acked-by: Zhang Rui <rui.zhang@intel.com>
---
 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 2f9a1c7e7b1f..c2525b585487 100644
--- a/drivers/thermal/cpu_cooling.c
+++ b/drivers/thermal/cpu_cooling.c
@@ -395,13 +395,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;
 	}
 
-- 
2.7.1.410.g6faf27b

Re: [PATCH V3 0/5] thermal: minor cleanup/fixes

[Zhang Rui]

On Mon, 2017-03-13 at 15:37 +0530, Viresh Kumar wrote:
> Hi,
> 
> This series contains minor fixes/cleanups for thermal cooling
> drivers.
> 
> V2->V3:
> - Added Acks from Rui
> - Resending as it was getting ignored until now :(

I just applied V2 this morning, and has not pushed to kernel.org yet.
The patch set should be queued for next -rc.

thanks,
rui
> 
> 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(-)
>