[PATCH] ath10k: move driver state check before setting throttle

[PATCH] ath10k: move driver state check before setting throttle

[Rajkumar Manoharan]

Since thermal daemon is unaware of the device state, it might
try to adjust the throttle state when the device is powered down.
So the driver caches the value and will configure it while
powering up the target. The cached value will be programed later
once the device is brought up. In such case, returning error
status is confusing and misleading the user application. Hence
moving the driver state check before sending wmi command to target.

Reported-by: Matthias Kaehlcke <mka@google.com>
Signed-off-by: Rajkumar Manoharan <rmanohar@qti.qualcomm.com>
---
 drivers/net/wireless/ath/ath10k/thermal.c | 13 ++++---------
 1 file changed, 4 insertions(+), 9 deletions(-)

diff --git a/drivers/net/wireless/ath/ath10k/thermal.c b/drivers/net/wireless/ath/ath10k/thermal.c
index 3720c86..c9475ac 100644
--- a/drivers/net/wireless/ath/ath10k/thermal.c
+++ b/drivers/net/wireless/ath/ath10k/thermal.c
@@ -50,7 +50,6 @@ ath10k_thermal_set_cur_throttle_state(struct thermal_cooling_device *cdev,
 				      unsigned long throttle_state)
 {
 	struct ath10k *ar = cdev->devdata;
-	int ret = 0;
 
 	if (throttle_state > ATH10K_THERMAL_THROTTLE_MAX) {
 		ath10k_warn(ar, "throttle state %ld is exceeding the limit %d\n",
@@ -59,16 +58,9 @@ ath10k_thermal_set_cur_throttle_state(struct thermal_cooling_device *cdev,
 	}
 	mutex_lock(&ar->conf_mutex);
 	ar->thermal.throttle_state = throttle_state;
-
-	if (ar->state != ATH10K_STATE_ON) {
-		ret = -ENETDOWN;
-		goto out;
-	}
-
 	ath10k_thermal_set_throttling(ar);
-out:
 	mutex_unlock(&ar->conf_mutex);
-	return ret;
+	return 0;
 }
 
 static struct thermal_cooling_device_ops ath10k_thermal_ops = {
@@ -190,6 +182,9 @@ void ath10k_thermal_set_throttling(struct ath10k *ar)
 
 	lockdep_assert_held(&ar->conf_mutex);
 
+	if (ar->state != ATH10K_STATE_ON)
+		return;
+
 	period = ar->thermal.quiet_period;
 	duration = (period * ar->thermal.throttle_state) / 100;
 	enabled = duration ? 1 : 0;
-- 
2.3.2


_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

[PATCH] ath10k: move driver state check before setting throttle

[Rajkumar Manoharan]

Since thermal daemon is unaware of the device state, it might
try to adjust the throttle state when the device is powered down.
So the driver caches the value and will configure it while
powering up the target. The cached value will be programed later
once the device is brought up. In such case, returning error
status is confusing and misleading the user application. Hence
moving the driver state check before sending wmi command to target.

Reported-by: Matthias Kaehlcke <mka@google.com>
Signed-off-by: Rajkumar Manoharan <rmanohar@qti.qualcomm.com>
---
 drivers/net/wireless/ath/ath10k/thermal.c | 13 ++++---------
 1 file changed, 4 insertions(+), 9 deletions(-)

diff --git a/drivers/net/wireless/ath/ath10k/thermal.c b/drivers/net/wireless/ath/ath10k/thermal.c
index 3720c86..c9475ac 100644
--- a/drivers/net/wireless/ath/ath10k/thermal.c
+++ b/drivers/net/wireless/ath/ath10k/thermal.c
@@ -50,7 +50,6 @@ ath10k_thermal_set_cur_throttle_state(struct thermal_cooling_device *cdev,
 				      unsigned long throttle_state)
 {
 	struct ath10k *ar = cdev->devdata;
-	int ret = 0;
 
 	if (throttle_state > ATH10K_THERMAL_THROTTLE_MAX) {
 		ath10k_warn(ar, "throttle state %ld is exceeding the limit %d\n",
@@ -59,16 +58,9 @@ ath10k_thermal_set_cur_throttle_state(struct thermal_cooling_device *cdev,
 	}
 	mutex_lock(&ar->conf_mutex);
 	ar->thermal.throttle_state = throttle_state;
-
-	if (ar->state != ATH10K_STATE_ON) {
-		ret = -ENETDOWN;
-		goto out;
-	}
-
 	ath10k_thermal_set_throttling(ar);
-out:
 	mutex_unlock(&ar->conf_mutex);
-	return ret;
+	return 0;
 }
 
 static struct thermal_cooling_device_ops ath10k_thermal_ops = {
@@ -190,6 +182,9 @@ void ath10k_thermal_set_throttling(struct ath10k *ar)
 
 	lockdep_assert_held(&ar->conf_mutex);
 
+	if (ar->state != ATH10K_STATE_ON)
+		return;
+
 	period = ar->thermal.quiet_period;
 	duration = (period * ar->thermal.throttle_state) / 100;
 	enabled = duration ? 1 : 0;
-- 
2.3.2

Re: [PATCH] ath10k: move driver state check before setting throttle

[Kalle Valo]

Rajkumar Manoharan <rmanohar@qti.qualcomm.com> writes:

> Since thermal daemon is unaware of the device state, it might
> try to adjust the throttle state when the device is powered down.
> So the driver caches the value and will configure it while
> powering up the target. The cached value will be programed later
> once the device is brought up. In such case, returning error
> status is confusing and misleading the user application. Hence
> moving the driver state check before sending wmi command to target.
>
> Reported-by: Matthias Kaehlcke <mka@google.com>
> Signed-off-by: Rajkumar Manoharan <rmanohar@qti.qualcomm.com>

What tree do you use as the baseline? Three-way merge doesn't work as
sha1 ids are not found:

Applying: ath10k: move driver state check before setting throttle
fatal: sha1 information is lacking or useless (drivers/net/wireless/ath/ath10k/thermal.c).
Repository lacks necessary blobs to fall back on 3-way merge.
Cannot fall back to three-way merge.
Patch failed at 0001 ath10k: move driver state check before setting throttle

-- 
Kalle Valo

_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

Re: [PATCH] ath10k: move driver state check before setting throttle

[Kalle Valo]

Rajkumar Manoharan <rmanohar@qti.qualcomm.com> writes:

> Since thermal daemon is unaware of the device state, it might
> try to adjust the throttle state when the device is powered down.
> So the driver caches the value and will configure it while
> powering up the target. The cached value will be programed later
> once the device is brought up. In such case, returning error
> status is confusing and misleading the user application. Hence
> moving the driver state check before sending wmi command to target.
>
> Reported-by: Matthias Kaehlcke <mka@google.com>
> Signed-off-by: Rajkumar Manoharan <rmanohar@qti.qualcomm.com>

What tree do you use as the baseline? Three-way merge doesn't work as
sha1 ids are not found:

Applying: ath10k: move driver state check before setting throttle
fatal: sha1 information is lacking or useless (drivers/net/wireless/ath/ath10k/thermal.c).
Repository lacks necessary blobs to fall back on 3-way merge.
Cannot fall back to three-way merge.
Patch failed at 0001 ath10k: move driver state check before setting throttle

-- 
Kalle Valo

Re: [PATCH] ath10k: move driver state check before setting throttle

[Rajkumar Manoharan]

On Fri, Mar 13, 2015 at 06:48:21PM +0200, Kalle Valo wrote:
> Rajkumar Manoharan <rmanohar@qti.qualcomm.com> writes:
> 
> > Since thermal daemon is unaware of the device state, it might
> > try to adjust the throttle state when the device is powered down.
> > So the driver caches the value and will configure it while
> > powering up the target. The cached value will be programed later
> > once the device is brought up. In such case, returning error
> > status is confusing and misleading the user application. Hence
> > moving the driver state check before sending wmi command to target.
> >
> > Reported-by: Matthias Kaehlcke <mka@google.com>
> > Signed-off-by: Rajkumar Manoharan <rmanohar@qti.qualcomm.com>
> 
> What tree do you use as the baseline? Three-way merge doesn't work as
> sha1 ids are not found:
> 
Will send rebased version.

-Rajkumar

_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

Re: [PATCH] ath10k: move driver state check before setting throttle

[Rajkumar Manoharan]

On Fri, Mar 13, 2015 at 06:48:21PM +0200, Kalle Valo wrote:
> Rajkumar Manoharan <rmanohar@qti.qualcomm.com> writes:
> 
> > Since thermal daemon is unaware of the device state, it might
> > try to adjust the throttle state when the device is powered down.
> > So the driver caches the value and will configure it while
> > powering up the target. The cached value will be programed later
> > once the device is brought up. In such case, returning error
> > status is confusing and misleading the user application. Hence
> > moving the driver state check before sending wmi command to target.
> >
> > Reported-by: Matthias Kaehlcke <mka@google.com>
> > Signed-off-by: Rajkumar Manoharan <rmanohar@qti.qualcomm.com>
> 
> What tree do you use as the baseline? Three-way merge doesn't work as
> sha1 ids are not found:
> 
Will send rebased version.

-Rajkumar