All of lore.kernel.org
 help / color / mirror / Atom feed
* [RFC] ath9k: Fix ath9k prevents CPU to enter C3 states
@ 2011-02-09 14:33 Mohammed Shafi Shajakhan
  2011-02-09 14:42 ` John W. Linville
                   ` (2 more replies)
  0 siblings, 3 replies; 17+ messages in thread
From: Mohammed Shafi Shajakhan @ 2011-02-09 14:33 UTC (permalink / raw)
  To: linville; +Cc: linux-wireless, lrodriguez, Mohammed Shafi Shajakhan

From: Mohammed Shafi Shajakhan <mshajakhan@atheros.com>

The DMA latency issue is observed only in Intel pinetrail platforms but
in the driver we had a default PM-QOS value of 55. This caused
unnecessary power consumption and battery drain in other platforms.
Address this issue by disabling PM-QOS by default by setting it's value
as '0' and making code changes appropriately.This addresses the bug:
https://bugzilla.kernel.org/show_bug.cgi?id=27532
	If the user sees some DMA latency issue he can still use the pmqos as a
module parameter to trade power for throughput as below:
sudo modprobe ath9k pmqos=55

Signed-off-by: Mohammed Shafi Shajakhan <mshajakhan@atheros.com>
---
 drivers/net/wireless/ath/ath9k/ath9k.h |    4 +++-
 drivers/net/wireless/ath/ath9k/init.c  |    9 +++++----
 drivers/net/wireless/ath/ath9k/main.c  |    9 +++++----
 3 files changed, 13 insertions(+), 9 deletions(-)

diff --git a/drivers/net/wireless/ath/ath9k/ath9k.h b/drivers/net/wireless/ath/ath9k/ath9k.h
index 9272278..55fcd01 100644
--- a/drivers/net/wireless/ath/ath9k/ath9k.h
+++ b/drivers/net/wireless/ath/ath9k/ath9k.h
@@ -57,7 +57,9 @@ struct ath_node;
 
 #define A_MAX(a, b) ((a) > (b) ? (a) : (b))
 
-#define ATH9K_PM_QOS_DEFAULT_VALUE	55
+/* By default PM QOS is disabled */
+#define ATH9K_PM_QOS_DEFAULT_VALUE	0
+
 
 #define TSF_TO_TU(_h,_l) \
 	((((u32)(_h)) << 22) | (((u32)(_l)) >> 10))
diff --git a/drivers/net/wireless/ath/ath9k/init.c b/drivers/net/wireless/ath/ath9k/init.c
index e5c1eea..658ea02 100644
--- a/drivers/net/wireless/ath/ath9k/init.c
+++ b/drivers/net/wireless/ath/ath9k/init.c
@@ -43,7 +43,7 @@ MODULE_PARM_DESC(btcoex_enable, "Enable wifi-BT coexistence");
 
 int ath9k_pm_qos_value = ATH9K_PM_QOS_DEFAULT_VALUE;
 module_param_named(pmqos, ath9k_pm_qos_value, int, S_IRUSR | S_IRGRP | S_IROTH);
-MODULE_PARM_DESC(pmqos, "User specified PM-QOS value");
+MODULE_PARM_DESC(pmqos, "User specified PM-QOS value, by default disabled");
 
 bool is_ath9k_unloaded;
 /* We use the hw_value as an index into our private channel structure */
@@ -759,8 +759,8 @@ int ath9k_init_device(u16 devid, struct ath_softc *sc, u16 subsysid,
 
 	ath_init_leds(sc);
 	ath_start_rfkill_poll(sc);
-
-	pm_qos_add_request(&sc->pm_qos_req, PM_QOS_CPU_DMA_LATENCY,
+	if (ath9k_pm_qos_value)
+		pm_qos_add_request(&sc->pm_qos_req, PM_QOS_CPU_DMA_LATENCY,
 			   PM_QOS_DEFAULT_VALUE);
 
 	return 0;
@@ -819,7 +819,8 @@ void ath9k_deinit_device(struct ath_softc *sc)
 	ath9k_ps_restore(sc);
 
 	ieee80211_unregister_hw(hw);
-	pm_qos_remove_request(&sc->pm_qos_req);
+	if (ath9k_pm_qos_value)
+		pm_qos_remove_request(&sc->pm_qos_req);
 	ath_rx_cleanup(sc);
 	ath_tx_cleanup(sc);
 	ath9k_deinit_softc(sc);
diff --git a/drivers/net/wireless/ath/ath9k/main.c b/drivers/net/wireless/ath/ath9k/main.c
index 4ed43b2..3855092 100644
--- a/drivers/net/wireless/ath/ath9k/main.c
+++ b/drivers/net/wireless/ath/ath9k/main.c
@@ -1119,9 +1119,10 @@ static int ath9k_start(struct ieee80211_hw *hw)
 
 	/* User has the option to provide pm-qos value as a module
 	 * parameter rather than using the default value of
-	 * 'ATH9K_PM_QOS_DEFAULT_VALUE'.
+	 * 'ATH9K_PM_QOS_DEFAULT_VALUE' which disables PM QOS.
 	 */
-	pm_qos_update_request(&sc->pm_qos_req, ath9k_pm_qos_value);
+	if (ath9k_pm_qos_value)
+		pm_qos_update_request(&sc->pm_qos_req, ath9k_pm_qos_value);
 
 	if (ah->caps.pcie_lcr_extsync_en && common->bus_ops->extn_synch_en)
 		common->bus_ops->extn_synch_en(common);
@@ -1266,8 +1267,8 @@ static void ath9k_stop(struct ieee80211_hw *hw)
 	ath_radio_disable(sc, hw);
 
 	sc->sc_flags |= SC_OP_INVALID;
-
-	pm_qos_update_request(&sc->pm_qos_req, PM_QOS_DEFAULT_VALUE);
+	if (ath9k_pm_qos_value)
+		pm_qos_update_request(&sc->pm_qos_req, PM_QOS_DEFAULT_VALUE);
 
 	mutex_unlock(&sc->mutex);
 
-- 
1.7.0.4


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* Re: [RFC] ath9k: Fix ath9k prevents CPU to enter C3 states
  2011-02-09 14:33 [RFC] ath9k: Fix ath9k prevents CPU to enter C3 states Mohammed Shafi Shajakhan
@ 2011-02-09 14:42 ` John W. Linville
  2011-02-09 14:50   ` Mohammed Shafi
  2011-02-09 16:48 ` Richard Schütz
  2011-02-09 19:58 ` Luis R. Rodriguez
  2 siblings, 1 reply; 17+ messages in thread
From: John W. Linville @ 2011-02-09 14:42 UTC (permalink / raw)
  To: Mohammed Shafi Shajakhan; +Cc: linux-wireless, lrodriguez

On Wed, Feb 09, 2011 at 08:03:27PM +0530, Mohammed Shafi Shajakhan wrote:
> From: Mohammed Shafi Shajakhan <mshajakhan@atheros.com>
> 
> The DMA latency issue is observed only in Intel pinetrail platforms but
> in the driver we had a default PM-QOS value of 55. This caused
> unnecessary power consumption and battery drain in other platforms.
> Address this issue by disabling PM-QOS by default by setting it's value
> as '0' and making code changes appropriately.This addresses the bug:
> https://bugzilla.kernel.org/show_bug.cgi?id=27532
> 	If the user sees some DMA latency issue he can still use the pmqos as a
> module parameter to trade power for throughput as below:
> sudo modprobe ath9k pmqos=55
> 
> Signed-off-by: Mohammed Shafi Shajakhan <mshajakhan@atheros.com>

Seems a bit unclear to me -- if you have ath9k_pm_qos_value then why
use ATH9K_PM_QOS_DEFAULT_VALUE at all?

John
-- 
John W. Linville		Someday the world will need a hero, and you
linville@tuxdriver.com			might be all we have.  Be ready.

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [RFC] ath9k: Fix ath9k prevents CPU to enter C3 states
  2011-02-09 14:42 ` John W. Linville
@ 2011-02-09 14:50   ` Mohammed Shafi
  0 siblings, 0 replies; 17+ messages in thread
From: Mohammed Shafi @ 2011-02-09 14:50 UTC (permalink / raw)
  To: John W. Linville; +Cc: Mohammed Shajakhan, linux-wireless, Luis Rodriguez

On Wednesday 09 February 2011 08:12 PM, John W. Linville wrote:
> On Wed, Feb 09, 2011 at 08:03:27PM +0530, Mohammed Shafi Shajakhan wrote:
>    
>> From: Mohammed Shafi Shajakhan<mshajakhan@atheros.com>
>>
>> The DMA latency issue is observed only in Intel pinetrail platforms but
>> in the driver we had a default PM-QOS value of 55. This caused
>> unnecessary power consumption and battery drain in other platforms.
>> Address this issue by disabling PM-QOS by default by setting it's value
>> as '0' and making code changes appropriately.This addresses the bug:
>> https://bugzilla.kernel.org/show_bug.cgi?id=27532
>> 	If the user sees some DMA latency issue he can still use the pmqos as a
>> module parameter to trade power for throughput as below:
>> sudo modprobe ath9k pmqos=55
>>
>> Signed-off-by: Mohammed Shafi Shajakhan<mshajakhan@atheros.com>
>>      
> Seems a bit unclear to me -- if you have ath9k_pm_qos_value then why
> use ATH9K_PM_QOS_DEFAULT_VALUE at all?
>    
Yes thanks, I think that macro is not needed at all with the current 
approach
> John
>    

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [RFC] ath9k: Fix ath9k prevents CPU to enter C3 states
  2011-02-09 14:33 [RFC] ath9k: Fix ath9k prevents CPU to enter C3 states Mohammed Shafi Shajakhan
  2011-02-09 14:42 ` John W. Linville
@ 2011-02-09 16:48 ` Richard Schütz
  2011-02-10 14:19   ` Mohammed Shafi
  2011-02-09 19:58 ` Luis R. Rodriguez
  2 siblings, 1 reply; 17+ messages in thread
From: Richard Schütz @ 2011-02-09 16:48 UTC (permalink / raw)
  To: Mohammed Shafi Shajakhan; +Cc: linville, linux-wireless, lrodriguez

> From: Mohammed Shafi Shajakhan<mshajakhan@atheros.com>
>
> The DMA latency issue is observed only in Intel pinetrail platforms but
> in the driver we had a default PM-QOS value of 55. This caused
> unnecessary power consumption and battery drain in other platforms.
> Address this issue by disabling PM-QOS by default by setting it's value
> as '0' and making code changes appropriately.This addresses the bug:
> https://bugzilla.kernel.org/show_bug.cgi?id=27532
> 	If the user sees some DMA latency issue he can still use the pmqos as a
> module parameter to trade power for throughput as below:
> sudo modprobe ath9k pmqos=55
>
> Signed-off-by: Mohammed Shafi Shajakhan<mshajakhan@atheros.com>
Acked-By: Richard Schütz <r.schtz@t-online.de>

Looks good to me with the exception of ATH9K_PM_QOS_DEFAULT_VALUE. But 
John already mentioned that.

-- 
Regards,
Richard Schütz

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [RFC] ath9k: Fix ath9k prevents CPU to enter C3 states
  2011-02-09 14:33 [RFC] ath9k: Fix ath9k prevents CPU to enter C3 states Mohammed Shafi Shajakhan
  2011-02-09 14:42 ` John W. Linville
  2011-02-09 16:48 ` Richard Schütz
@ 2011-02-09 19:58 ` Luis R. Rodriguez
  2011-02-10 14:18     ` Mohammed Shafi
  2 siblings, 1 reply; 17+ messages in thread
From: Luis R. Rodriguez @ 2011-02-09 19:58 UTC (permalink / raw)
  To: Mohammed Shajakhan
  Cc: linville, linux-wireless, Luis Rodriguez, linux-kernel, netdev,
	Tim Gardner, Mark Gross

On Wed, Feb 09, 2011 at 06:33:27AM -0800, Mohammed Shajakhan wrote:
> From: Mohammed Shafi Shajakhan <mshajakhan@atheros.com>
> 
> The DMA latency issue is observed only in Intel pinetrail platforms but
> in the driver we had a default PM-QOS value of 55. This caused
> unnecessary power consumption and battery drain in other platforms.
> Address this issue by disabling PM-QOS by default by setting it's value
> as '0' and making code changes appropriately.This addresses the bug:
> https://bugzilla.kernel.org/show_bug.cgi?id=27532
> 	If the user sees some DMA latency issue he can still use the pmqos as a
> module parameter to trade power for throughput as below:
> sudo modprobe ath9k pmqos=55

How many times are we going to write some work around for some
platform unpatched BIOS bug ?

mcgrof@tux ~/wireless-testing (git::master)$ git grep pm_qos_update_request drivers/
drivers/net/e1000e/netdev.c:                    pm_qos_update_request(&adapter->netdev->pm_qos_req, 55);
drivers/net/e1000e/netdev.c:                    pm_qos_update_request(&adapter->netdev->pm_qos_req,
drivers/net/wireless/ath/ath9k/main.c:  pm_qos_update_request(&sc->pm_qos_req, ath9k_pm_qos_value);
drivers/net/wireless/ath/ath9k/main.c:  pm_qos_update_request(&sc->pm_qos_req, PM_QOS_DEFAULT_VALUE);
drivers/net/wireless/ipw2x00/ipw2100.c: pm_qos_update_request(&ipw2100_pm_qos_req, 175);
drivers/net/wireless/ipw2x00/ipw2100.c: pm_qos_update_request(&ipw2100_pm_qos_req, PM_QOS_DEFAULT_VALUE);

Can't we punt this crap to userspace?

http://www.kernel.org/pub/linux/kernel/people/mcgrof/scripts/cpudmalatency.c

Can distributions just autodetect this issue and call something like this?

  Luis

> Signed-off-by: Mohammed Shafi Shajakhan <mshajakhan@atheros.com>
> ---
>  drivers/net/wireless/ath/ath9k/ath9k.h |    4 +++-
>  drivers/net/wireless/ath/ath9k/init.c  |    9 +++++----
>  drivers/net/wireless/ath/ath9k/main.c  |    9 +++++----
>  3 files changed, 13 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/net/wireless/ath/ath9k/ath9k.h b/drivers/net/wireless/ath/ath9k/ath9k.h
> index 9272278..55fcd01 100644
> --- a/drivers/net/wireless/ath/ath9k/ath9k.h
> +++ b/drivers/net/wireless/ath/ath9k/ath9k.h
> @@ -57,7 +57,9 @@ struct ath_node;
>  
>  #define A_MAX(a, b) ((a) > (b) ? (a) : (b))
>  
> -#define ATH9K_PM_QOS_DEFAULT_VALUE	55
> +/* By default PM QOS is disabled */
> +#define ATH9K_PM_QOS_DEFAULT_VALUE	0
> +
>  
>  #define TSF_TO_TU(_h,_l) \
>  	((((u32)(_h)) << 22) | (((u32)(_l)) >> 10))
> diff --git a/drivers/net/wireless/ath/ath9k/init.c b/drivers/net/wireless/ath/ath9k/init.c
> index e5c1eea..658ea02 100644
> --- a/drivers/net/wireless/ath/ath9k/init.c
> +++ b/drivers/net/wireless/ath/ath9k/init.c
> @@ -43,7 +43,7 @@ MODULE_PARM_DESC(btcoex_enable, "Enable wifi-BT coexistence");
>  
>  int ath9k_pm_qos_value = ATH9K_PM_QOS_DEFAULT_VALUE;
>  module_param_named(pmqos, ath9k_pm_qos_value, int, S_IRUSR | S_IRGRP | S_IROTH);
> -MODULE_PARM_DESC(pmqos, "User specified PM-QOS value");
> +MODULE_PARM_DESC(pmqos, "User specified PM-QOS value, by default disabled");
>  
>  bool is_ath9k_unloaded;
>  /* We use the hw_value as an index into our private channel structure */
> @@ -759,8 +759,8 @@ int ath9k_init_device(u16 devid, struct ath_softc *sc, u16 subsysid,
>  
>  	ath_init_leds(sc);
>  	ath_start_rfkill_poll(sc);
> -
> -	pm_qos_add_request(&sc->pm_qos_req, PM_QOS_CPU_DMA_LATENCY,
> +	if (ath9k_pm_qos_value)
> +		pm_qos_add_request(&sc->pm_qos_req, PM_QOS_CPU_DMA_LATENCY,
>  			   PM_QOS_DEFAULT_VALUE);
>  
>  	return 0;
> @@ -819,7 +819,8 @@ void ath9k_deinit_device(struct ath_softc *sc)
>  	ath9k_ps_restore(sc);
>  
>  	ieee80211_unregister_hw(hw);
> -	pm_qos_remove_request(&sc->pm_qos_req);
> +	if (ath9k_pm_qos_value)
> +		pm_qos_remove_request(&sc->pm_qos_req);
>  	ath_rx_cleanup(sc);
>  	ath_tx_cleanup(sc);
>  	ath9k_deinit_softc(sc);
> diff --git a/drivers/net/wireless/ath/ath9k/main.c b/drivers/net/wireless/ath/ath9k/main.c
> index 4ed43b2..3855092 100644
> --- a/drivers/net/wireless/ath/ath9k/main.c
> +++ b/drivers/net/wireless/ath/ath9k/main.c
> @@ -1119,9 +1119,10 @@ static int ath9k_start(struct ieee80211_hw *hw)
>  
>  	/* User has the option to provide pm-qos value as a module
>  	 * parameter rather than using the default value of
> -	 * 'ATH9K_PM_QOS_DEFAULT_VALUE'.
> +	 * 'ATH9K_PM_QOS_DEFAULT_VALUE' which disables PM QOS.
>  	 */
> -	pm_qos_update_request(&sc->pm_qos_req, ath9k_pm_qos_value);
> +	if (ath9k_pm_qos_value)
> +		pm_qos_update_request(&sc->pm_qos_req, ath9k_pm_qos_value);
>  
>  	if (ah->caps.pcie_lcr_extsync_en && common->bus_ops->extn_synch_en)
>  		common->bus_ops->extn_synch_en(common);
> @@ -1266,8 +1267,8 @@ static void ath9k_stop(struct ieee80211_hw *hw)
>  	ath_radio_disable(sc, hw);
>  
>  	sc->sc_flags |= SC_OP_INVALID;
> -
> -	pm_qos_update_request(&sc->pm_qos_req, PM_QOS_DEFAULT_VALUE);
> +	if (ath9k_pm_qos_value)
> +		pm_qos_update_request(&sc->pm_qos_req, PM_QOS_DEFAULT_VALUE);
>  
>  	mutex_unlock(&sc->mutex);
>  
> -- 
> 1.7.0.4
> 

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [RFC] ath9k: Fix ath9k prevents CPU to enter C3 states
  2011-02-09 19:58 ` Luis R. Rodriguez
@ 2011-02-10 14:18     ` Mohammed Shafi
  0 siblings, 0 replies; 17+ messages in thread
From: Mohammed Shafi @ 2011-02-10 14:18 UTC (permalink / raw)
  To: Luis Rodriguez
  Cc: Mohammed Shajakhan, linville, linux-wireless, linux-kernel,
	netdev, Tim Gardner, Mark Gross, Senthilkumar Balasubramanian

On Thursday 10 February 2011 01:28 AM, Luis Rodriguez wrote:
> On Wed, Feb 09, 2011 at 06:33:27AM -0800, Mohammed Shajakhan wrote:
>    
>> From: Mohammed Shafi Shajakhan<mshajakhan@atheros.com>
>>
>> The DMA latency issue is observed only in Intel pinetrail platforms but
>> in the driver we had a default PM-QOS value of 55. This caused
>> unnecessary power consumption and battery drain in other platforms.
>> Address this issue by disabling PM-QOS by default by setting it's value
>> as '0' and making code changes appropriately.This addresses the bug:
>> https://bugzilla.kernel.org/show_bug.cgi?id=27532
>> 	If the user sees some DMA latency issue he can still use the pmqos as a
>> module parameter to trade power for throughput as below:
>> sudo modprobe ath9k pmqos=55
>>      
> How many times are we going to write some work around for some
> platform unpatched BIOS bug ?
>
> mcgrof@tux ~/wireless-testing (git::master)$ git grep pm_qos_update_request drivers/
> drivers/net/e1000e/netdev.c:                    pm_qos_update_request(&adapter->netdev->pm_qos_req, 55);
> drivers/net/e1000e/netdev.c:                    pm_qos_update_request(&adapter->netdev->pm_qos_req,
> drivers/net/wireless/ath/ath9k/main.c:  pm_qos_update_request(&sc->pm_qos_req, ath9k_pm_qos_value);
> drivers/net/wireless/ath/ath9k/main.c:  pm_qos_update_request(&sc->pm_qos_req, PM_QOS_DEFAULT_VALUE);
> drivers/net/wireless/ipw2x00/ipw2100.c: pm_qos_update_request(&ipw2100_pm_qos_req, 175);
> drivers/net/wireless/ipw2x00/ipw2100.c: pm_qos_update_request(&ipw2100_pm_qos_req, PM_QOS_DEFAULT_VALUE);
>
> Can't we punt this crap to userspace?
>    
Luis thanks for the script and sorry I missed it in the bugzilla. Luis I 
agree these workarounds are better placed to be in user place and the 
only thing is those have issues must be aware of the script.
I am ccing Senthil to have his views.

thanks,
shafi
> http://www.kernel.org/pub/linux/kernel/people/mcgrof/scripts/cpudmalatency.c
>
> Can distributions just autodetect this issue and call something like this?
>
>    Luis
>
>    
>> Signed-off-by: Mohammed Shafi Shajakhan<mshajakhan@atheros.com>
>> ---
>>   drivers/net/wireless/ath/ath9k/ath9k.h |    4 +++-
>>   drivers/net/wireless/ath/ath9k/init.c  |    9 +++++----
>>   drivers/net/wireless/ath/ath9k/main.c  |    9 +++++----
>>   3 files changed, 13 insertions(+), 9 deletions(-)
>>
>> diff --git a/drivers/net/wireless/ath/ath9k/ath9k.h b/drivers/net/wireless/ath/ath9k/ath9k.h
>> index 9272278..55fcd01 100644
>> --- a/drivers/net/wireless/ath/ath9k/ath9k.h
>> +++ b/drivers/net/wireless/ath/ath9k/ath9k.h
>> @@ -57,7 +57,9 @@ struct ath_node;
>>
>>   #define A_MAX(a, b) ((a)>  (b) ? (a) : (b))
>>
>> -#define ATH9K_PM_QOS_DEFAULT_VALUE	55
>> +/* By default PM QOS is disabled */
>> +#define ATH9K_PM_QOS_DEFAULT_VALUE	0
>> +
>>
>>   #define TSF_TO_TU(_h,_l) \
>>   	((((u32)(_h))<<  22) | (((u32)(_l))>>  10))
>> diff --git a/drivers/net/wireless/ath/ath9k/init.c b/drivers/net/wireless/ath/ath9k/init.c
>> index e5c1eea..658ea02 100644
>> --- a/drivers/net/wireless/ath/ath9k/init.c
>> +++ b/drivers/net/wireless/ath/ath9k/init.c
>> @@ -43,7 +43,7 @@ MODULE_PARM_DESC(btcoex_enable, "Enable wifi-BT coexistence");
>>
>>   int ath9k_pm_qos_value = ATH9K_PM_QOS_DEFAULT_VALUE;
>>   module_param_named(pmqos, ath9k_pm_qos_value, int, S_IRUSR | S_IRGRP | S_IROTH);
>> -MODULE_PARM_DESC(pmqos, "User specified PM-QOS value");
>> +MODULE_PARM_DESC(pmqos, "User specified PM-QOS value, by default disabled");
>>
>>   bool is_ath9k_unloaded;
>>   /* We use the hw_value as an index into our private channel structure */
>> @@ -759,8 +759,8 @@ int ath9k_init_device(u16 devid, struct ath_softc *sc, u16 subsysid,
>>
>>   	ath_init_leds(sc);
>>   	ath_start_rfkill_poll(sc);
>> -
>> -	pm_qos_add_request(&sc->pm_qos_req, PM_QOS_CPU_DMA_LATENCY,
>> +	if (ath9k_pm_qos_value)
>> +		pm_qos_add_request(&sc->pm_qos_req, PM_QOS_CPU_DMA_LATENCY,
>>   			   PM_QOS_DEFAULT_VALUE);
>>
>>   	return 0;
>> @@ -819,7 +819,8 @@ void ath9k_deinit_device(struct ath_softc *sc)
>>   	ath9k_ps_restore(sc);
>>
>>   	ieee80211_unregister_hw(hw);
>> -	pm_qos_remove_request(&sc->pm_qos_req);
>> +	if (ath9k_pm_qos_value)
>> +		pm_qos_remove_request(&sc->pm_qos_req);
>>   	ath_rx_cleanup(sc);
>>   	ath_tx_cleanup(sc);
>>   	ath9k_deinit_softc(sc);
>> diff --git a/drivers/net/wireless/ath/ath9k/main.c b/drivers/net/wireless/ath/ath9k/main.c
>> index 4ed43b2..3855092 100644
>> --- a/drivers/net/wireless/ath/ath9k/main.c
>> +++ b/drivers/net/wireless/ath/ath9k/main.c
>> @@ -1119,9 +1119,10 @@ static int ath9k_start(struct ieee80211_hw *hw)
>>
>>   	/* User has the option to provide pm-qos value as a module
>>   	 * parameter rather than using the default value of
>> -	 * 'ATH9K_PM_QOS_DEFAULT_VALUE'.
>> +	 * 'ATH9K_PM_QOS_DEFAULT_VALUE' which disables PM QOS.
>>   	 */
>> -	pm_qos_update_request(&sc->pm_qos_req, ath9k_pm_qos_value);
>> +	if (ath9k_pm_qos_value)
>> +		pm_qos_update_request(&sc->pm_qos_req, ath9k_pm_qos_value);
>>
>>   	if (ah->caps.pcie_lcr_extsync_en&&  common->bus_ops->extn_synch_en)
>>   		common->bus_ops->extn_synch_en(common);
>> @@ -1266,8 +1267,8 @@ static void ath9k_stop(struct ieee80211_hw *hw)
>>   	ath_radio_disable(sc, hw);
>>
>>   	sc->sc_flags |= SC_OP_INVALID;
>> -
>> -	pm_qos_update_request(&sc->pm_qos_req, PM_QOS_DEFAULT_VALUE);
>> +	if (ath9k_pm_qos_value)
>> +		pm_qos_update_request(&sc->pm_qos_req, PM_QOS_DEFAULT_VALUE);
>>
>>   	mutex_unlock(&sc->mutex);
>>
>> -- 
>> 1.7.0.4
>>
>>      
> .
>
>    

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [RFC] ath9k: Fix ath9k prevents CPU to enter C3 states
@ 2011-02-10 14:18     ` Mohammed Shafi
  0 siblings, 0 replies; 17+ messages in thread
From: Mohammed Shafi @ 2011-02-10 14:18 UTC (permalink / raw)
  To: Luis Rodriguez
  Cc: Mohammed Shajakhan, linville, linux-wireless, linux-kernel,
	netdev, Tim Gardner, Mark Gross, Senthilkumar Balasubramanian

On Thursday 10 February 2011 01:28 AM, Luis Rodriguez wrote:
> On Wed, Feb 09, 2011 at 06:33:27AM -0800, Mohammed Shajakhan wrote:
>    
>> From: Mohammed Shafi Shajakhan<mshajakhan@atheros.com>
>>
>> The DMA latency issue is observed only in Intel pinetrail platforms but
>> in the driver we had a default PM-QOS value of 55. This caused
>> unnecessary power consumption and battery drain in other platforms.
>> Address this issue by disabling PM-QOS by default by setting it's value
>> as '0' and making code changes appropriately.This addresses the bug:
>> https://bugzilla.kernel.org/show_bug.cgi?id=27532
>> 	If the user sees some DMA latency issue he can still use the pmqos as a
>> module parameter to trade power for throughput as below:
>> sudo modprobe ath9k pmqos=55
>>      
> How many times are we going to write some work around for some
> platform unpatched BIOS bug ?
>
> mcgrof@tux ~/wireless-testing (git::master)$ git grep pm_qos_update_request drivers/
> drivers/net/e1000e/netdev.c:                    pm_qos_update_request(&adapter->netdev->pm_qos_req, 55);
> drivers/net/e1000e/netdev.c:                    pm_qos_update_request(&adapter->netdev->pm_qos_req,
> drivers/net/wireless/ath/ath9k/main.c:  pm_qos_update_request(&sc->pm_qos_req, ath9k_pm_qos_value);
> drivers/net/wireless/ath/ath9k/main.c:  pm_qos_update_request(&sc->pm_qos_req, PM_QOS_DEFAULT_VALUE);
> drivers/net/wireless/ipw2x00/ipw2100.c: pm_qos_update_request(&ipw2100_pm_qos_req, 175);
> drivers/net/wireless/ipw2x00/ipw2100.c: pm_qos_update_request(&ipw2100_pm_qos_req, PM_QOS_DEFAULT_VALUE);
>
> Can't we punt this crap to userspace?
>    
Luis thanks for the script and sorry I missed it in the bugzilla. Luis I 
agree these workarounds are better placed to be in user place and the 
only thing is those have issues must be aware of the script.
I am ccing Senthil to have his views.

thanks,
shafi
> http://www.kernel.org/pub/linux/kernel/people/mcgrof/scripts/cpudmalatency.c
>
> Can distributions just autodetect this issue and call something like this?
>
>    Luis
>
>    
>> Signed-off-by: Mohammed Shafi Shajakhan<mshajakhan@atheros.com>
>> ---
>>   drivers/net/wireless/ath/ath9k/ath9k.h |    4 +++-
>>   drivers/net/wireless/ath/ath9k/init.c  |    9 +++++----
>>   drivers/net/wireless/ath/ath9k/main.c  |    9 +++++----
>>   3 files changed, 13 insertions(+), 9 deletions(-)
>>
>> diff --git a/drivers/net/wireless/ath/ath9k/ath9k.h b/drivers/net/wireless/ath/ath9k/ath9k.h
>> index 9272278..55fcd01 100644
>> --- a/drivers/net/wireless/ath/ath9k/ath9k.h
>> +++ b/drivers/net/wireless/ath/ath9k/ath9k.h
>> @@ -57,7 +57,9 @@ struct ath_node;
>>
>>   #define A_MAX(a, b) ((a)>  (b) ? (a) : (b))
>>
>> -#define ATH9K_PM_QOS_DEFAULT_VALUE	55
>> +/* By default PM QOS is disabled */
>> +#define ATH9K_PM_QOS_DEFAULT_VALUE	0
>> +
>>
>>   #define TSF_TO_TU(_h,_l) \
>>   	((((u32)(_h))<<  22) | (((u32)(_l))>>  10))
>> diff --git a/drivers/net/wireless/ath/ath9k/init.c b/drivers/net/wireless/ath/ath9k/init.c
>> index e5c1eea..658ea02 100644
>> --- a/drivers/net/wireless/ath/ath9k/init.c
>> +++ b/drivers/net/wireless/ath/ath9k/init.c
>> @@ -43,7 +43,7 @@ MODULE_PARM_DESC(btcoex_enable, "Enable wifi-BT coexistence");
>>
>>   int ath9k_pm_qos_value = ATH9K_PM_QOS_DEFAULT_VALUE;
>>   module_param_named(pmqos, ath9k_pm_qos_value, int, S_IRUSR | S_IRGRP | S_IROTH);
>> -MODULE_PARM_DESC(pmqos, "User specified PM-QOS value");
>> +MODULE_PARM_DESC(pmqos, "User specified PM-QOS value, by default disabled");
>>
>>   bool is_ath9k_unloaded;
>>   /* We use the hw_value as an index into our private channel structure */
>> @@ -759,8 +759,8 @@ int ath9k_init_device(u16 devid, struct ath_softc *sc, u16 subsysid,
>>
>>   	ath_init_leds(sc);
>>   	ath_start_rfkill_poll(sc);
>> -
>> -	pm_qos_add_request(&sc->pm_qos_req, PM_QOS_CPU_DMA_LATENCY,
>> +	if (ath9k_pm_qos_value)
>> +		pm_qos_add_request(&sc->pm_qos_req, PM_QOS_CPU_DMA_LATENCY,
>>   			   PM_QOS_DEFAULT_VALUE);
>>
>>   	return 0;
>> @@ -819,7 +819,8 @@ void ath9k_deinit_device(struct ath_softc *sc)
>>   	ath9k_ps_restore(sc);
>>
>>   	ieee80211_unregister_hw(hw);
>> -	pm_qos_remove_request(&sc->pm_qos_req);
>> +	if (ath9k_pm_qos_value)
>> +		pm_qos_remove_request(&sc->pm_qos_req);
>>   	ath_rx_cleanup(sc);
>>   	ath_tx_cleanup(sc);
>>   	ath9k_deinit_softc(sc);
>> diff --git a/drivers/net/wireless/ath/ath9k/main.c b/drivers/net/wireless/ath/ath9k/main.c
>> index 4ed43b2..3855092 100644
>> --- a/drivers/net/wireless/ath/ath9k/main.c
>> +++ b/drivers/net/wireless/ath/ath9k/main.c
>> @@ -1119,9 +1119,10 @@ static int ath9k_start(struct ieee80211_hw *hw)
>>
>>   	/* User has the option to provide pm-qos value as a module
>>   	 * parameter rather than using the default value of
>> -	 * 'ATH9K_PM_QOS_DEFAULT_VALUE'.
>> +	 * 'ATH9K_PM_QOS_DEFAULT_VALUE' which disables PM QOS.
>>   	 */
>> -	pm_qos_update_request(&sc->pm_qos_req, ath9k_pm_qos_value);
>> +	if (ath9k_pm_qos_value)
>> +		pm_qos_update_request(&sc->pm_qos_req, ath9k_pm_qos_value);
>>
>>   	if (ah->caps.pcie_lcr_extsync_en&&  common->bus_ops->extn_synch_en)
>>   		common->bus_ops->extn_synch_en(common);
>> @@ -1266,8 +1267,8 @@ static void ath9k_stop(struct ieee80211_hw *hw)
>>   	ath_radio_disable(sc, hw);
>>
>>   	sc->sc_flags |= SC_OP_INVALID;
>> -
>> -	pm_qos_update_request(&sc->pm_qos_req, PM_QOS_DEFAULT_VALUE);
>> +	if (ath9k_pm_qos_value)
>> +		pm_qos_update_request(&sc->pm_qos_req, PM_QOS_DEFAULT_VALUE);
>>
>>   	mutex_unlock(&sc->mutex);
>>
>> -- 
>> 1.7.0.4
>>
>>      
> .
>
>    

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [RFC] ath9k: Fix ath9k prevents CPU to enter C3 states
  2011-02-09 16:48 ` Richard Schütz
@ 2011-02-10 14:19   ` Mohammed Shafi
  2011-02-10 17:41     ` Luis R. Rodriguez
  0 siblings, 1 reply; 17+ messages in thread
From: Mohammed Shafi @ 2011-02-10 14:19 UTC (permalink / raw)
  To: Richard Schütz
  Cc: Mohammed Shajakhan, linville, linux-wireless, Luis Rodriguez

On Wednesday 09 February 2011 10:18 PM, Richard Schütz wrote:
>> From: Mohammed Shafi Shajakhan<mshajakhan@atheros.com>
>>
>> The DMA latency issue is observed only in Intel pinetrail platforms but
>> in the driver we had a default PM-QOS value of 55. This caused
>> unnecessary power consumption and battery drain in other platforms.
>> Address this issue by disabling PM-QOS by default by setting it's value
>> as '0' and making code changes appropriately.This addresses the bug:
>> https://bugzilla.kernel.org/show_bug.cgi?id=27532
>> 	If the user sees some DMA latency issue he can still use the pmqos as a
>> module parameter to trade power for throughput as below:
>> sudo modprobe ath9k pmqos=55
>>
>> Signed-off-by: Mohammed Shafi Shajakhan<mshajakhan@atheros.com>
>>      
> Acked-By: Richard Schütz<r.schtz@t-online.de>
>    
Thanks, but we got to consider user space approach which Luis had mentioned.
> Looks good to me with the exception of ATH9K_PM_QOS_DEFAULT_VALUE. But
> John already mentioned that.
>
>    

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [RFC] ath9k: Fix ath9k prevents CPU to enter C3 states
  2011-02-10 14:19   ` Mohammed Shafi
@ 2011-02-10 17:41     ` Luis R. Rodriguez
  2011-02-10 17:44       ` Richard Schütz
  0 siblings, 1 reply; 17+ messages in thread
From: Luis R. Rodriguez @ 2011-02-10 17:41 UTC (permalink / raw)
  To: Mohammed Shajakhan
  Cc: Richard Schütz, linville, linux-wireless, Luis Rodriguez

On Thu, Feb 10, 2011 at 06:19:10AM -0800, Mohammed Shajakhan wrote:
> On Wednesday 09 February 2011 10:18 PM, Richard Schütz wrote:
> >> From: Mohammed Shafi Shajakhan<mshajakhan@atheros.com>
> >>
> >> The DMA latency issue is observed only in Intel pinetrail platforms but
> >> in the driver we had a default PM-QOS value of 55. This caused
> >> unnecessary power consumption and battery drain in other platforms.
> >> Address this issue by disabling PM-QOS by default by setting it's value
> >> as '0' and making code changes appropriately.This addresses the bug:
> >> https://bugzilla.kernel.org/show_bug.cgi?id=27532
> >> 	If the user sees some DMA latency issue he can still use the pmqos as a
> >> module parameter to trade power for throughput as below:
> >> sudo modprobe ath9k pmqos=55
> >>
> >> Signed-off-by: Mohammed Shafi Shajakhan<mshajakhan@atheros.com>
> >>      
> > Acked-By: Richard Schütz<r.schtz@t-online.de>
> >    
> Thanks, but we got to consider user space approach which Luis had mentioned.

Richard, can you test the userspace app as a replacement? Or did your board not
exerpience the issue?

  Luis

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [RFC] ath9k: Fix ath9k prevents CPU to enter C3 states
  2011-02-10 17:41     ` Luis R. Rodriguez
@ 2011-02-10 17:44       ` Richard Schütz
  2011-02-11 13:31         ` Richard Schütz
  0 siblings, 1 reply; 17+ messages in thread
From: Richard Schütz @ 2011-02-10 17:44 UTC (permalink / raw)
  To: Luis R. Rodriguez
  Cc: Mohammed Shajakhan, linville, linux-wireless, Luis Rodriguez

> Richard, can you test the userspace app as a replacement? Or did your board not
> exerpience the issue?

I was unaffected. Therefore I was unhappy with losing my C-states for no 
good reason.

-- 
Regards,
Richard Schütz

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [RFC] ath9k: Fix ath9k prevents CPU to enter C3 states
  2011-02-10 17:44       ` Richard Schütz
@ 2011-02-11 13:31         ` Richard Schütz
  0 siblings, 0 replies; 17+ messages in thread
From: Richard Schütz @ 2011-02-11 13:31 UTC (permalink / raw)
  To: Luis R. Rodriguez
  Cc: Mohammed Shajakhan, linville, linux-wireless, Luis Rodriguez

>> Richard, can you test the userspace app as a replacement? Or did your board not
>> exerpience the issue?
>
> I was unaffected. Therefore I was unhappy with losing my C-states for no good reason.
>

Nevertheless I tested the userspace approach now. It disables the C4-state on my machine,
so it should also help those who really have problems with the performance of their
wireless connection.

-- 
Regards,
Richard Schütz

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [RFC] ath9k: Fix ath9k prevents CPU to enter C3 states
  2011-03-04 13:37       ` John W. Linville
@ 2011-03-04 14:26         ` Mohammed Shafi
  0 siblings, 0 replies; 17+ messages in thread
From: Mohammed Shafi @ 2011-03-04 14:26 UTC (permalink / raw)
  To: John W. Linville
  Cc: Mohammed Shajakhan, linux-wireless, Luis Rodriguez, stable

On Friday 04 March 2011 07:07 PM, John W. Linville wrote:
> On Fri, Mar 04, 2011 at 11:36:37AM +0530, Mohammed Shafi wrote:
>    
>> On Wednesday 16 February 2011 10:35 AM, Mohammed Shafi wrote:
>>      
>>> On Tuesday 15 February 2011 09:42 PM, John W. Linville wrote:
>>>        
>>>> Despite the [RFC], I am applying this.  I should have reverted all
>>>> of this three weeks ago...
>>>>          
>> John can you please push this to stable kernel ?
>>      
> Stable folks, this is the commit in Linus's tree;
>    
Thanks a lot John.
> commit 0f5cd45960173ba5b36727decbb4a241cbd35ef9
> Author: Mohammed Shafi Shajakhan<mshajakhan@atheros.com>
> Date:   Tue Feb 15 21:29:32 2011 +0530
>
>      ath9k: Fix ath9k prevents CPU to enter C3 states
>
>      The DMA latency issue is observed only in Intel pinetrail platforms
>      but in the driver we had a default PM-QOS value of 55. This caused
>      unnecessary power consumption and battery drain in other platforms.
>
>      Remove the pm-qos thing in the driver code and address the throughput
>      issue in Intel pinetrail platfroms in user space using any one of
>      the scripts in below links:
>
>      http://www.kernel.org/pub/linux/kernel/people/mcgrof/scripts/cpudmalatency.c
>      http://johannes.sipsolutions.net/files/netlatency.c.txt
>
>      More details can be found in the following bugzilla link:
>
>      https://bugzilla.kernel.org/show_bug.cgi?id=27532
>
>      This reverts the following commits:
>
>          98c316e348bedffa730e6f1e4baeb8a3c3e0f28b
>          4dc3530df7c0428b41c00399a7ee8c929406d181
>          10598c124ecabbbfd7522f74de19b8f7d52a1bee
>
>      Signed-off-by: Mohammed Shafi Shajakhan<mshajakhan@atheros.com>
>      Signed-off-by: John W. Linville<linville@tuxdriver.com>
>
>    
>>> Thanks John.
>>>        
>>>> John
>>>>
>>>> On Tue, Feb 15, 2011 at 09:29:32PM +0530, Mohammed Shafi
>>>> Shajakhan wrote:
>>>>          
>>>>> From: Mohammed Shafi Shajakhan<mshajakhan@atheros.com>
>>>>>
>>>>> The DMA latency issue is observed only in Intel pinetrail platforms but
>>>>> in the driver we had a default PM-QOS value of 55. This caused
>>>>> unnecessary power consumption and battery drain in other platforms.
>>>>>     Remove the pm-qos thing in the driver code and address the
>>>>> throughput issue in
>>>>> Intel pinetrail platfroms in user space using any one of the
>>>>> scripts in below links:
>>>>> http://www.kernel.org/pub/linux/kernel/people/mcgrof/scripts/cpudmalatency.c
>>>>>
>>>>> http://johannes.sipsolutions.net/files/netlatency.c.txt
>>>>> More details can be found in the following bugzilla link:
>>>>> https://bugzilla.kernel.org/show_bug.cgi?id=27532
>>>>>
>>>>> Signed-off-by: Mohammed Shafi Shajakhan<mshajakhan@atheros.com>
>>>>> ---
>>>>>   drivers/net/wireless/ath/ath9k/ath9k.h |    5 -----
>>>>>   drivers/net/wireless/ath/ath9k/init.c  |    7 -------
>>>>>   drivers/net/wireless/ath/ath9k/main.c  |    8 --------
>>>>>   3 files changed, 0 insertions(+), 20 deletions(-)
>>>>>
>>>>> diff --git a/drivers/net/wireless/ath/ath9k/ath9k.h
>>>>> b/drivers/net/wireless/ath/ath9k/ath9k.h
>>>>> index ba436cd..0052f64 100644
>>>>> --- a/drivers/net/wireless/ath/ath9k/ath9k.h
>>>>> +++ b/drivers/net/wireless/ath/ath9k/ath9k.h
>>>>> @@ -21,7 +21,6 @@
>>>>>   #include<linux/device.h>
>>>>>   #include<linux/leds.h>
>>>>>   #include<linux/completion.h>
>>>>> -#include<linux/pm_qos_params.h>
>>>>>
>>>>>   #include "debug.h"
>>>>>   #include "common.h"
>>>>> @@ -57,8 +56,6 @@ struct ath_node;
>>>>>
>>>>>   #define A_MAX(a, b) ((a)>   (b) ? (a) : (b))
>>>>>
>>>>> -#define ATH9K_PM_QOS_DEFAULT_VALUE    55
>>>>> -
>>>>>   #define TSF_TO_TU(_h,_l) \
>>>>>       ((((u32)(_h))<<   22) | (((u32)(_l))>>   10))
>>>>>
>>>>> @@ -650,7 +647,6 @@ struct ath_softc {
>>>>>
>>>>>       struct ath_ant_comb ant_comb;
>>>>>
>>>>> -    struct pm_qos_request_list pm_qos_req;
>>>>>   };
>>>>>
>>>>>   void ath9k_tasklet(unsigned long data);
>>>>> @@ -665,7 +661,6 @@ static inline void
>>>>> ath_read_cachesize(struct ath_common *common, int *csz)
>>>>>   extern struct ieee80211_ops ath9k_ops;
>>>>>   extern int ath9k_modparam_nohwcrypt;
>>>>>   extern int led_blink;
>>>>> -extern int ath9k_pm_qos_value;
>>>>>   extern bool is_ath9k_unloaded;
>>>>>
>>>>>   irqreturn_t ath_isr(int irq, void *dev);
>>>>> diff --git a/drivers/net/wireless/ath/ath9k/init.c
>>>>> b/drivers/net/wireless/ath/ath9k/init.c
>>>>> index e5c1eea..8fed4e4 100644
>>>>> --- a/drivers/net/wireless/ath/ath9k/init.c
>>>>> +++ b/drivers/net/wireless/ath/ath9k/init.c
>>>>> @@ -41,9 +41,6 @@ static int ath9k_btcoex_enable;
>>>>>   module_param_named(btcoex_enable, ath9k_btcoex_enable, int, 0444);
>>>>>   MODULE_PARM_DESC(btcoex_enable, "Enable wifi-BT coexistence");
>>>>>
>>>>> -int ath9k_pm_qos_value = ATH9K_PM_QOS_DEFAULT_VALUE;
>>>>> -module_param_named(pmqos, ath9k_pm_qos_value, int, S_IRUSR |
>>>>> S_IRGRP | S_IROTH);
>>>>> -MODULE_PARM_DESC(pmqos, "User specified PM-QOS value");
>>>>>
>>>>>   bool is_ath9k_unloaded;
>>>>>   /* We use the hw_value as an index into our private channel
>>>>> structure */
>>>>> @@ -760,9 +757,6 @@ int ath9k_init_device(u16 devid, struct
>>>>> ath_softc *sc, u16 subsysid,
>>>>>       ath_init_leds(sc);
>>>>>       ath_start_rfkill_poll(sc);
>>>>>
>>>>> -    pm_qos_add_request(&sc->pm_qos_req, PM_QOS_CPU_DMA_LATENCY,
>>>>> -               PM_QOS_DEFAULT_VALUE);
>>>>> -
>>>>>       return 0;
>>>>>
>>>>>   error_world:
>>>>> @@ -819,7 +813,6 @@ void ath9k_deinit_device(struct ath_softc *sc)
>>>>>       ath9k_ps_restore(sc);
>>>>>
>>>>>       ieee80211_unregister_hw(hw);
>>>>> -    pm_qos_remove_request(&sc->pm_qos_req);
>>>>>       ath_rx_cleanup(sc);
>>>>>       ath_tx_cleanup(sc);
>>>>>       ath9k_deinit_softc(sc);
>>>>> diff --git a/drivers/net/wireless/ath/ath9k/main.c
>>>>> b/drivers/net/wireless/ath/ath9k/main.c
>>>>> index 4f568b8..1d2c7c3 100644
>>>>> --- a/drivers/net/wireless/ath/ath9k/main.c
>>>>> +++ b/drivers/net/wireless/ath/ath9k/main.c
>>>>> @@ -1117,12 +1117,6 @@ static int ath9k_start(struct ieee80211_hw *hw)
>>>>>               ath9k_btcoex_timer_resume(sc);
>>>>>       }
>>>>>
>>>>> -    /* User has the option to provide pm-qos value as a module
>>>>> -     * parameter rather than using the default value of
>>>>> -     * 'ATH9K_PM_QOS_DEFAULT_VALUE'.
>>>>> -     */
>>>>> -    pm_qos_update_request(&sc->pm_qos_req, ath9k_pm_qos_value);
>>>>> -
>>>>>       if (ah->caps.pcie_lcr_extsync_en&&
>>>>> common->bus_ops->extn_synch_en)
>>>>>           common->bus_ops->extn_synch_en(common);
>>>>>
>>>>> @@ -1267,8 +1261,6 @@ static void ath9k_stop(struct ieee80211_hw *hw)
>>>>>
>>>>>       sc->sc_flags |= SC_OP_INVALID;
>>>>>
>>>>> -    pm_qos_update_request(&sc->pm_qos_req, PM_QOS_DEFAULT_VALUE);
>>>>> -
>>>>>       mutex_unlock(&sc->mutex);
>>>>>
>>>>>       ath_dbg(common, ATH_DBG_CONFIG, "Driver halt\n");
>>>>> -- 
>>>>> 1.7.0.4
>>>>>
>>>>>
>>>>>            
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>
>>      
>    

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [RFC] ath9k: Fix ath9k prevents CPU to enter C3 states
  2011-03-04  6:06     ` Mohammed Shafi
@ 2011-03-04 13:37       ` John W. Linville
  2011-03-04 14:26         ` Mohammed Shafi
  0 siblings, 1 reply; 17+ messages in thread
From: John W. Linville @ 2011-03-04 13:37 UTC (permalink / raw)
  To: Mohammed Shafi; +Cc: Mohammed Shajakhan, linux-wireless, Luis Rodriguez, stable

On Fri, Mar 04, 2011 at 11:36:37AM +0530, Mohammed Shafi wrote:
> On Wednesday 16 February 2011 10:35 AM, Mohammed Shafi wrote:
> >On Tuesday 15 February 2011 09:42 PM, John W. Linville wrote:
> >>Despite the [RFC], I am applying this.  I should have reverted all
> >>of this three weeks ago...
> John can you please push this to stable kernel ?

Stable folks, this is the commit in Linus's tree;

commit 0f5cd45960173ba5b36727decbb4a241cbd35ef9
Author: Mohammed Shafi Shajakhan <mshajakhan@atheros.com>
Date:   Tue Feb 15 21:29:32 2011 +0530

    ath9k: Fix ath9k prevents CPU to enter C3 states
    
    The DMA latency issue is observed only in Intel pinetrail platforms
    but in the driver we had a default PM-QOS value of 55. This caused
    unnecessary power consumption and battery drain in other platforms.
    
    Remove the pm-qos thing in the driver code and address the throughput
    issue in Intel pinetrail platfroms in user space using any one of
    the scripts in below links:
    
    http://www.kernel.org/pub/linux/kernel/people/mcgrof/scripts/cpudmalatency.c
    http://johannes.sipsolutions.net/files/netlatency.c.txt
    
    More details can be found in the following bugzilla link:
    
    https://bugzilla.kernel.org/show_bug.cgi?id=27532
    
    This reverts the following commits:
    
        98c316e348bedffa730e6f1e4baeb8a3c3e0f28b
        4dc3530df7c0428b41c00399a7ee8c929406d181
        10598c124ecabbbfd7522f74de19b8f7d52a1bee
    
    Signed-off-by: Mohammed Shafi Shajakhan <mshajakhan@atheros.com>
    Signed-off-by: John W. Linville <linville@tuxdriver.com>

> >Thanks John.
> >>John
> >>
> >>On Tue, Feb 15, 2011 at 09:29:32PM +0530, Mohammed Shafi
> >>Shajakhan wrote:
> >>>From: Mohammed Shafi Shajakhan<mshajakhan@atheros.com>
> >>>
> >>>The DMA latency issue is observed only in Intel pinetrail platforms but
> >>>in the driver we had a default PM-QOS value of 55. This caused
> >>>unnecessary power consumption and battery drain in other platforms.
> >>>    Remove the pm-qos thing in the driver code and address the
> >>>throughput issue in
> >>>Intel pinetrail platfroms in user space using any one of the
> >>>scripts in below links:
> >>>http://www.kernel.org/pub/linux/kernel/people/mcgrof/scripts/cpudmalatency.c
> >>>
> >>>http://johannes.sipsolutions.net/files/netlatency.c.txt
> >>>More details can be found in the following bugzilla link:
> >>>https://bugzilla.kernel.org/show_bug.cgi?id=27532
> >>>
> >>>Signed-off-by: Mohammed Shafi Shajakhan<mshajakhan@atheros.com>
> >>>---
> >>>  drivers/net/wireless/ath/ath9k/ath9k.h |    5 -----
> >>>  drivers/net/wireless/ath/ath9k/init.c  |    7 -------
> >>>  drivers/net/wireless/ath/ath9k/main.c  |    8 --------
> >>>  3 files changed, 0 insertions(+), 20 deletions(-)
> >>>
> >>>diff --git a/drivers/net/wireless/ath/ath9k/ath9k.h
> >>>b/drivers/net/wireless/ath/ath9k/ath9k.h
> >>>index ba436cd..0052f64 100644
> >>>--- a/drivers/net/wireless/ath/ath9k/ath9k.h
> >>>+++ b/drivers/net/wireless/ath/ath9k/ath9k.h
> >>>@@ -21,7 +21,6 @@
> >>>  #include<linux/device.h>
> >>>  #include<linux/leds.h>
> >>>  #include<linux/completion.h>
> >>>-#include<linux/pm_qos_params.h>
> >>>
> >>>  #include "debug.h"
> >>>  #include "common.h"
> >>>@@ -57,8 +56,6 @@ struct ath_node;
> >>>
> >>>  #define A_MAX(a, b) ((a)>  (b) ? (a) : (b))
> >>>
> >>>-#define ATH9K_PM_QOS_DEFAULT_VALUE    55
> >>>-
> >>>  #define TSF_TO_TU(_h,_l) \
> >>>      ((((u32)(_h))<<  22) | (((u32)(_l))>>  10))
> >>>
> >>>@@ -650,7 +647,6 @@ struct ath_softc {
> >>>
> >>>      struct ath_ant_comb ant_comb;
> >>>
> >>>-    struct pm_qos_request_list pm_qos_req;
> >>>  };
> >>>
> >>>  void ath9k_tasklet(unsigned long data);
> >>>@@ -665,7 +661,6 @@ static inline void
> >>>ath_read_cachesize(struct ath_common *common, int *csz)
> >>>  extern struct ieee80211_ops ath9k_ops;
> >>>  extern int ath9k_modparam_nohwcrypt;
> >>>  extern int led_blink;
> >>>-extern int ath9k_pm_qos_value;
> >>>  extern bool is_ath9k_unloaded;
> >>>
> >>>  irqreturn_t ath_isr(int irq, void *dev);
> >>>diff --git a/drivers/net/wireless/ath/ath9k/init.c
> >>>b/drivers/net/wireless/ath/ath9k/init.c
> >>>index e5c1eea..8fed4e4 100644
> >>>--- a/drivers/net/wireless/ath/ath9k/init.c
> >>>+++ b/drivers/net/wireless/ath/ath9k/init.c
> >>>@@ -41,9 +41,6 @@ static int ath9k_btcoex_enable;
> >>>  module_param_named(btcoex_enable, ath9k_btcoex_enable, int, 0444);
> >>>  MODULE_PARM_DESC(btcoex_enable, "Enable wifi-BT coexistence");
> >>>
> >>>-int ath9k_pm_qos_value = ATH9K_PM_QOS_DEFAULT_VALUE;
> >>>-module_param_named(pmqos, ath9k_pm_qos_value, int, S_IRUSR |
> >>>S_IRGRP | S_IROTH);
> >>>-MODULE_PARM_DESC(pmqos, "User specified PM-QOS value");
> >>>
> >>>  bool is_ath9k_unloaded;
> >>>  /* We use the hw_value as an index into our private channel
> >>>structure */
> >>>@@ -760,9 +757,6 @@ int ath9k_init_device(u16 devid, struct
> >>>ath_softc *sc, u16 subsysid,
> >>>      ath_init_leds(sc);
> >>>      ath_start_rfkill_poll(sc);
> >>>
> >>>-    pm_qos_add_request(&sc->pm_qos_req, PM_QOS_CPU_DMA_LATENCY,
> >>>-               PM_QOS_DEFAULT_VALUE);
> >>>-
> >>>      return 0;
> >>>
> >>>  error_world:
> >>>@@ -819,7 +813,6 @@ void ath9k_deinit_device(struct ath_softc *sc)
> >>>      ath9k_ps_restore(sc);
> >>>
> >>>      ieee80211_unregister_hw(hw);
> >>>-    pm_qos_remove_request(&sc->pm_qos_req);
> >>>      ath_rx_cleanup(sc);
> >>>      ath_tx_cleanup(sc);
> >>>      ath9k_deinit_softc(sc);
> >>>diff --git a/drivers/net/wireless/ath/ath9k/main.c
> >>>b/drivers/net/wireless/ath/ath9k/main.c
> >>>index 4f568b8..1d2c7c3 100644
> >>>--- a/drivers/net/wireless/ath/ath9k/main.c
> >>>+++ b/drivers/net/wireless/ath/ath9k/main.c
> >>>@@ -1117,12 +1117,6 @@ static int ath9k_start(struct ieee80211_hw *hw)
> >>>              ath9k_btcoex_timer_resume(sc);
> >>>      }
> >>>
> >>>-    /* User has the option to provide pm-qos value as a module
> >>>-     * parameter rather than using the default value of
> >>>-     * 'ATH9K_PM_QOS_DEFAULT_VALUE'.
> >>>-     */
> >>>-    pm_qos_update_request(&sc->pm_qos_req, ath9k_pm_qos_value);
> >>>-
> >>>      if (ah->caps.pcie_lcr_extsync_en&&
> >>>common->bus_ops->extn_synch_en)
> >>>          common->bus_ops->extn_synch_en(common);
> >>>
> >>>@@ -1267,8 +1261,6 @@ static void ath9k_stop(struct ieee80211_hw *hw)
> >>>
> >>>      sc->sc_flags |= SC_OP_INVALID;
> >>>
> >>>-    pm_qos_update_request(&sc->pm_qos_req, PM_QOS_DEFAULT_VALUE);
> >>>-
> >>>      mutex_unlock(&sc->mutex);
> >>>
> >>>      ath_dbg(common, ATH_DBG_CONFIG, "Driver halt\n");
> >>>-- 
> >>>1.7.0.4
> >>>
> >>>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

-- 
John W. Linville		Someday the world will need a hero, and you
linville@tuxdriver.com			might be all we have.  Be ready.

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [RFC] ath9k: Fix ath9k prevents CPU to enter C3 states
  2011-02-16  5:05   ` Mohammed Shafi
@ 2011-03-04  6:06     ` Mohammed Shafi
  2011-03-04 13:37       ` John W. Linville
  0 siblings, 1 reply; 17+ messages in thread
From: Mohammed Shafi @ 2011-03-04  6:06 UTC (permalink / raw)
  To: John W. Linville; +Cc: Mohammed Shajakhan, linux-wireless, Luis Rodriguez

On Wednesday 16 February 2011 10:35 AM, Mohammed Shafi wrote:
> On Tuesday 15 February 2011 09:42 PM, John W. Linville wrote:
>> Despite the [RFC], I am applying this.  I should have reverted all
>> of this three weeks ago...
John can you please push this to stable kernel ?
> Thanks John.
>> John
>>
>> On Tue, Feb 15, 2011 at 09:29:32PM +0530, Mohammed Shafi Shajakhan 
>> wrote:
>>> From: Mohammed Shafi Shajakhan<mshajakhan@atheros.com>
>>>
>>> The DMA latency issue is observed only in Intel pinetrail platforms but
>>> in the driver we had a default PM-QOS value of 55. This caused
>>> unnecessary power consumption and battery drain in other platforms.
>>>     Remove the pm-qos thing in the driver code and address the 
>>> throughput issue in
>>> Intel pinetrail platfroms in user space using any one of the scripts 
>>> in below links:
>>> http://www.kernel.org/pub/linux/kernel/people/mcgrof/scripts/cpudmalatency.c 
>>>
>>> http://johannes.sipsolutions.net/files/netlatency.c.txt
>>> More details can be found in the following bugzilla link:
>>> https://bugzilla.kernel.org/show_bug.cgi?id=27532
>>>
>>> Signed-off-by: Mohammed Shafi Shajakhan<mshajakhan@atheros.com>
>>> ---
>>>   drivers/net/wireless/ath/ath9k/ath9k.h |    5 -----
>>>   drivers/net/wireless/ath/ath9k/init.c  |    7 -------
>>>   drivers/net/wireless/ath/ath9k/main.c  |    8 --------
>>>   3 files changed, 0 insertions(+), 20 deletions(-)
>>>
>>> diff --git a/drivers/net/wireless/ath/ath9k/ath9k.h 
>>> b/drivers/net/wireless/ath/ath9k/ath9k.h
>>> index ba436cd..0052f64 100644
>>> --- a/drivers/net/wireless/ath/ath9k/ath9k.h
>>> +++ b/drivers/net/wireless/ath/ath9k/ath9k.h
>>> @@ -21,7 +21,6 @@
>>>   #include<linux/device.h>
>>>   #include<linux/leds.h>
>>>   #include<linux/completion.h>
>>> -#include<linux/pm_qos_params.h>
>>>
>>>   #include "debug.h"
>>>   #include "common.h"
>>> @@ -57,8 +56,6 @@ struct ath_node;
>>>
>>>   #define A_MAX(a, b) ((a)>  (b) ? (a) : (b))
>>>
>>> -#define ATH9K_PM_QOS_DEFAULT_VALUE    55
>>> -
>>>   #define TSF_TO_TU(_h,_l) \
>>>       ((((u32)(_h))<<  22) | (((u32)(_l))>>  10))
>>>
>>> @@ -650,7 +647,6 @@ struct ath_softc {
>>>
>>>       struct ath_ant_comb ant_comb;
>>>
>>> -    struct pm_qos_request_list pm_qos_req;
>>>   };
>>>
>>>   void ath9k_tasklet(unsigned long data);
>>> @@ -665,7 +661,6 @@ static inline void ath_read_cachesize(struct 
>>> ath_common *common, int *csz)
>>>   extern struct ieee80211_ops ath9k_ops;
>>>   extern int ath9k_modparam_nohwcrypt;
>>>   extern int led_blink;
>>> -extern int ath9k_pm_qos_value;
>>>   extern bool is_ath9k_unloaded;
>>>
>>>   irqreturn_t ath_isr(int irq, void *dev);
>>> diff --git a/drivers/net/wireless/ath/ath9k/init.c 
>>> b/drivers/net/wireless/ath/ath9k/init.c
>>> index e5c1eea..8fed4e4 100644
>>> --- a/drivers/net/wireless/ath/ath9k/init.c
>>> +++ b/drivers/net/wireless/ath/ath9k/init.c
>>> @@ -41,9 +41,6 @@ static int ath9k_btcoex_enable;
>>>   module_param_named(btcoex_enable, ath9k_btcoex_enable, int, 0444);
>>>   MODULE_PARM_DESC(btcoex_enable, "Enable wifi-BT coexistence");
>>>
>>> -int ath9k_pm_qos_value = ATH9K_PM_QOS_DEFAULT_VALUE;
>>> -module_param_named(pmqos, ath9k_pm_qos_value, int, S_IRUSR | 
>>> S_IRGRP | S_IROTH);
>>> -MODULE_PARM_DESC(pmqos, "User specified PM-QOS value");
>>>
>>>   bool is_ath9k_unloaded;
>>>   /* We use the hw_value as an index into our private channel 
>>> structure */
>>> @@ -760,9 +757,6 @@ int ath9k_init_device(u16 devid, struct 
>>> ath_softc *sc, u16 subsysid,
>>>       ath_init_leds(sc);
>>>       ath_start_rfkill_poll(sc);
>>>
>>> -    pm_qos_add_request(&sc->pm_qos_req, PM_QOS_CPU_DMA_LATENCY,
>>> -               PM_QOS_DEFAULT_VALUE);
>>> -
>>>       return 0;
>>>
>>>   error_world:
>>> @@ -819,7 +813,6 @@ void ath9k_deinit_device(struct ath_softc *sc)
>>>       ath9k_ps_restore(sc);
>>>
>>>       ieee80211_unregister_hw(hw);
>>> -    pm_qos_remove_request(&sc->pm_qos_req);
>>>       ath_rx_cleanup(sc);
>>>       ath_tx_cleanup(sc);
>>>       ath9k_deinit_softc(sc);
>>> diff --git a/drivers/net/wireless/ath/ath9k/main.c 
>>> b/drivers/net/wireless/ath/ath9k/main.c
>>> index 4f568b8..1d2c7c3 100644
>>> --- a/drivers/net/wireless/ath/ath9k/main.c
>>> +++ b/drivers/net/wireless/ath/ath9k/main.c
>>> @@ -1117,12 +1117,6 @@ static int ath9k_start(struct ieee80211_hw *hw)
>>>               ath9k_btcoex_timer_resume(sc);
>>>       }
>>>
>>> -    /* User has the option to provide pm-qos value as a module
>>> -     * parameter rather than using the default value of
>>> -     * 'ATH9K_PM_QOS_DEFAULT_VALUE'.
>>> -     */
>>> -    pm_qos_update_request(&sc->pm_qos_req, ath9k_pm_qos_value);
>>> -
>>>       if (ah->caps.pcie_lcr_extsync_en&&  
>>> common->bus_ops->extn_synch_en)
>>>           common->bus_ops->extn_synch_en(common);
>>>
>>> @@ -1267,8 +1261,6 @@ static void ath9k_stop(struct ieee80211_hw *hw)
>>>
>>>       sc->sc_flags |= SC_OP_INVALID;
>>>
>>> -    pm_qos_update_request(&sc->pm_qos_req, PM_QOS_DEFAULT_VALUE);
>>> -
>>>       mutex_unlock(&sc->mutex);
>>>
>>>       ath_dbg(common, ATH_DBG_CONFIG, "Driver halt\n");
>>> -- 
>>> 1.7.0.4
>>>
>>>

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [RFC] ath9k: Fix ath9k prevents CPU to enter C3 states
  2011-02-15 16:12 ` John W. Linville
@ 2011-02-16  5:05   ` Mohammed Shafi
  2011-03-04  6:06     ` Mohammed Shafi
  0 siblings, 1 reply; 17+ messages in thread
From: Mohammed Shafi @ 2011-02-16  5:05 UTC (permalink / raw)
  To: John W. Linville; +Cc: Mohammed Shajakhan, linux-wireless, Luis Rodriguez

On Tuesday 15 February 2011 09:42 PM, John W. Linville wrote:
> Despite the [RFC], I am applying this.  I should have reverted all
> of this three weeks ago...
>    
Thanks John.
> John
>
> On Tue, Feb 15, 2011 at 09:29:32PM +0530, Mohammed Shafi Shajakhan wrote:
>    
>> From: Mohammed Shafi Shajakhan<mshajakhan@atheros.com>
>>
>> The DMA latency issue is observed only in Intel pinetrail platforms but
>> in the driver we had a default PM-QOS value of 55. This caused
>> unnecessary power consumption and battery drain in other platforms.
>> 	Remove the pm-qos thing in the driver code and address the throughput issue in
>> Intel pinetrail platfroms in user space using any one of the scripts in below links:
>> http://www.kernel.org/pub/linux/kernel/people/mcgrof/scripts/cpudmalatency.c
>> http://johannes.sipsolutions.net/files/netlatency.c.txt
>> More details can be found in the following bugzilla link:
>> https://bugzilla.kernel.org/show_bug.cgi?id=27532
>>
>> Signed-off-by: Mohammed Shafi Shajakhan<mshajakhan@atheros.com>
>> ---
>>   drivers/net/wireless/ath/ath9k/ath9k.h |    5 -----
>>   drivers/net/wireless/ath/ath9k/init.c  |    7 -------
>>   drivers/net/wireless/ath/ath9k/main.c  |    8 --------
>>   3 files changed, 0 insertions(+), 20 deletions(-)
>>
>> diff --git a/drivers/net/wireless/ath/ath9k/ath9k.h b/drivers/net/wireless/ath/ath9k/ath9k.h
>> index ba436cd..0052f64 100644
>> --- a/drivers/net/wireless/ath/ath9k/ath9k.h
>> +++ b/drivers/net/wireless/ath/ath9k/ath9k.h
>> @@ -21,7 +21,6 @@
>>   #include<linux/device.h>
>>   #include<linux/leds.h>
>>   #include<linux/completion.h>
>> -#include<linux/pm_qos_params.h>
>>
>>   #include "debug.h"
>>   #include "common.h"
>> @@ -57,8 +56,6 @@ struct ath_node;
>>
>>   #define A_MAX(a, b) ((a)>  (b) ? (a) : (b))
>>
>> -#define ATH9K_PM_QOS_DEFAULT_VALUE	55
>> -
>>   #define TSF_TO_TU(_h,_l) \
>>   	((((u32)(_h))<<  22) | (((u32)(_l))>>  10))
>>
>> @@ -650,7 +647,6 @@ struct ath_softc {
>>
>>   	struct ath_ant_comb ant_comb;
>>
>> -	struct pm_qos_request_list pm_qos_req;
>>   };
>>
>>   void ath9k_tasklet(unsigned long data);
>> @@ -665,7 +661,6 @@ static inline void ath_read_cachesize(struct ath_common *common, int *csz)
>>   extern struct ieee80211_ops ath9k_ops;
>>   extern int ath9k_modparam_nohwcrypt;
>>   extern int led_blink;
>> -extern int ath9k_pm_qos_value;
>>   extern bool is_ath9k_unloaded;
>>
>>   irqreturn_t ath_isr(int irq, void *dev);
>> diff --git a/drivers/net/wireless/ath/ath9k/init.c b/drivers/net/wireless/ath/ath9k/init.c
>> index e5c1eea..8fed4e4 100644
>> --- a/drivers/net/wireless/ath/ath9k/init.c
>> +++ b/drivers/net/wireless/ath/ath9k/init.c
>> @@ -41,9 +41,6 @@ static int ath9k_btcoex_enable;
>>   module_param_named(btcoex_enable, ath9k_btcoex_enable, int, 0444);
>>   MODULE_PARM_DESC(btcoex_enable, "Enable wifi-BT coexistence");
>>
>> -int ath9k_pm_qos_value = ATH9K_PM_QOS_DEFAULT_VALUE;
>> -module_param_named(pmqos, ath9k_pm_qos_value, int, S_IRUSR | S_IRGRP | S_IROTH);
>> -MODULE_PARM_DESC(pmqos, "User specified PM-QOS value");
>>
>>   bool is_ath9k_unloaded;
>>   /* We use the hw_value as an index into our private channel structure */
>> @@ -760,9 +757,6 @@ int ath9k_init_device(u16 devid, struct ath_softc *sc, u16 subsysid,
>>   	ath_init_leds(sc);
>>   	ath_start_rfkill_poll(sc);
>>
>> -	pm_qos_add_request(&sc->pm_qos_req, PM_QOS_CPU_DMA_LATENCY,
>> -			   PM_QOS_DEFAULT_VALUE);
>> -
>>   	return 0;
>>
>>   error_world:
>> @@ -819,7 +813,6 @@ void ath9k_deinit_device(struct ath_softc *sc)
>>   	ath9k_ps_restore(sc);
>>
>>   	ieee80211_unregister_hw(hw);
>> -	pm_qos_remove_request(&sc->pm_qos_req);
>>   	ath_rx_cleanup(sc);
>>   	ath_tx_cleanup(sc);
>>   	ath9k_deinit_softc(sc);
>> diff --git a/drivers/net/wireless/ath/ath9k/main.c b/drivers/net/wireless/ath/ath9k/main.c
>> index 4f568b8..1d2c7c3 100644
>> --- a/drivers/net/wireless/ath/ath9k/main.c
>> +++ b/drivers/net/wireless/ath/ath9k/main.c
>> @@ -1117,12 +1117,6 @@ static int ath9k_start(struct ieee80211_hw *hw)
>>   			ath9k_btcoex_timer_resume(sc);
>>   	}
>>
>> -	/* User has the option to provide pm-qos value as a module
>> -	 * parameter rather than using the default value of
>> -	 * 'ATH9K_PM_QOS_DEFAULT_VALUE'.
>> -	 */
>> -	pm_qos_update_request(&sc->pm_qos_req, ath9k_pm_qos_value);
>> -
>>   	if (ah->caps.pcie_lcr_extsync_en&&  common->bus_ops->extn_synch_en)
>>   		common->bus_ops->extn_synch_en(common);
>>
>> @@ -1267,8 +1261,6 @@ static void ath9k_stop(struct ieee80211_hw *hw)
>>
>>   	sc->sc_flags |= SC_OP_INVALID;
>>
>> -	pm_qos_update_request(&sc->pm_qos_req, PM_QOS_DEFAULT_VALUE);
>> -
>>   	mutex_unlock(&sc->mutex);
>>
>>   	ath_dbg(common, ATH_DBG_CONFIG, "Driver halt\n");
>> -- 
>> 1.7.0.4
>>
>>
>>      
>    

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [RFC] ath9k: Fix ath9k prevents CPU to enter C3 states
  2011-02-15 15:59 Mohammed Shafi Shajakhan
@ 2011-02-15 16:12 ` John W. Linville
  2011-02-16  5:05   ` Mohammed Shafi
  0 siblings, 1 reply; 17+ messages in thread
From: John W. Linville @ 2011-02-15 16:12 UTC (permalink / raw)
  To: Mohammed Shafi Shajakhan; +Cc: linux-wireless, lrodriguez

Despite the [RFC], I am applying this.  I should have reverted all
of this three weeks ago...

John

On Tue, Feb 15, 2011 at 09:29:32PM +0530, Mohammed Shafi Shajakhan wrote:
> From: Mohammed Shafi Shajakhan <mshajakhan@atheros.com>
> 
> The DMA latency issue is observed only in Intel pinetrail platforms but
> in the driver we had a default PM-QOS value of 55. This caused
> unnecessary power consumption and battery drain in other platforms.
> 	Remove the pm-qos thing in the driver code and address the throughput issue in
> Intel pinetrail platfroms in user space using any one of the scripts in below links:
> http://www.kernel.org/pub/linux/kernel/people/mcgrof/scripts/cpudmalatency.c
> http://johannes.sipsolutions.net/files/netlatency.c.txt
> More details can be found in the following bugzilla link:
> https://bugzilla.kernel.org/show_bug.cgi?id=27532
> 
> Signed-off-by: Mohammed Shafi Shajakhan <mshajakhan@atheros.com>
> ---
>  drivers/net/wireless/ath/ath9k/ath9k.h |    5 -----
>  drivers/net/wireless/ath/ath9k/init.c  |    7 -------
>  drivers/net/wireless/ath/ath9k/main.c  |    8 --------
>  3 files changed, 0 insertions(+), 20 deletions(-)
> 
> diff --git a/drivers/net/wireless/ath/ath9k/ath9k.h b/drivers/net/wireless/ath/ath9k/ath9k.h
> index ba436cd..0052f64 100644
> --- a/drivers/net/wireless/ath/ath9k/ath9k.h
> +++ b/drivers/net/wireless/ath/ath9k/ath9k.h
> @@ -21,7 +21,6 @@
>  #include <linux/device.h>
>  #include <linux/leds.h>
>  #include <linux/completion.h>
> -#include <linux/pm_qos_params.h>
>  
>  #include "debug.h"
>  #include "common.h"
> @@ -57,8 +56,6 @@ struct ath_node;
>  
>  #define A_MAX(a, b) ((a) > (b) ? (a) : (b))
>  
> -#define ATH9K_PM_QOS_DEFAULT_VALUE	55
> -
>  #define TSF_TO_TU(_h,_l) \
>  	((((u32)(_h)) << 22) | (((u32)(_l)) >> 10))
>  
> @@ -650,7 +647,6 @@ struct ath_softc {
>  
>  	struct ath_ant_comb ant_comb;
>  
> -	struct pm_qos_request_list pm_qos_req;
>  };
>  
>  void ath9k_tasklet(unsigned long data);
> @@ -665,7 +661,6 @@ static inline void ath_read_cachesize(struct ath_common *common, int *csz)
>  extern struct ieee80211_ops ath9k_ops;
>  extern int ath9k_modparam_nohwcrypt;
>  extern int led_blink;
> -extern int ath9k_pm_qos_value;
>  extern bool is_ath9k_unloaded;
>  
>  irqreturn_t ath_isr(int irq, void *dev);
> diff --git a/drivers/net/wireless/ath/ath9k/init.c b/drivers/net/wireless/ath/ath9k/init.c
> index e5c1eea..8fed4e4 100644
> --- a/drivers/net/wireless/ath/ath9k/init.c
> +++ b/drivers/net/wireless/ath/ath9k/init.c
> @@ -41,9 +41,6 @@ static int ath9k_btcoex_enable;
>  module_param_named(btcoex_enable, ath9k_btcoex_enable, int, 0444);
>  MODULE_PARM_DESC(btcoex_enable, "Enable wifi-BT coexistence");
>  
> -int ath9k_pm_qos_value = ATH9K_PM_QOS_DEFAULT_VALUE;
> -module_param_named(pmqos, ath9k_pm_qos_value, int, S_IRUSR | S_IRGRP | S_IROTH);
> -MODULE_PARM_DESC(pmqos, "User specified PM-QOS value");
>  
>  bool is_ath9k_unloaded;
>  /* We use the hw_value as an index into our private channel structure */
> @@ -760,9 +757,6 @@ int ath9k_init_device(u16 devid, struct ath_softc *sc, u16 subsysid,
>  	ath_init_leds(sc);
>  	ath_start_rfkill_poll(sc);
>  
> -	pm_qos_add_request(&sc->pm_qos_req, PM_QOS_CPU_DMA_LATENCY,
> -			   PM_QOS_DEFAULT_VALUE);
> -
>  	return 0;
>  
>  error_world:
> @@ -819,7 +813,6 @@ void ath9k_deinit_device(struct ath_softc *sc)
>  	ath9k_ps_restore(sc);
>  
>  	ieee80211_unregister_hw(hw);
> -	pm_qos_remove_request(&sc->pm_qos_req);
>  	ath_rx_cleanup(sc);
>  	ath_tx_cleanup(sc);
>  	ath9k_deinit_softc(sc);
> diff --git a/drivers/net/wireless/ath/ath9k/main.c b/drivers/net/wireless/ath/ath9k/main.c
> index 4f568b8..1d2c7c3 100644
> --- a/drivers/net/wireless/ath/ath9k/main.c
> +++ b/drivers/net/wireless/ath/ath9k/main.c
> @@ -1117,12 +1117,6 @@ static int ath9k_start(struct ieee80211_hw *hw)
>  			ath9k_btcoex_timer_resume(sc);
>  	}
>  
> -	/* User has the option to provide pm-qos value as a module
> -	 * parameter rather than using the default value of
> -	 * 'ATH9K_PM_QOS_DEFAULT_VALUE'.
> -	 */
> -	pm_qos_update_request(&sc->pm_qos_req, ath9k_pm_qos_value);
> -
>  	if (ah->caps.pcie_lcr_extsync_en && common->bus_ops->extn_synch_en)
>  		common->bus_ops->extn_synch_en(common);
>  
> @@ -1267,8 +1261,6 @@ static void ath9k_stop(struct ieee80211_hw *hw)
>  
>  	sc->sc_flags |= SC_OP_INVALID;
>  
> -	pm_qos_update_request(&sc->pm_qos_req, PM_QOS_DEFAULT_VALUE);
> -
>  	mutex_unlock(&sc->mutex);
>  
>  	ath_dbg(common, ATH_DBG_CONFIG, "Driver halt\n");
> -- 
> 1.7.0.4
> 
> 

-- 
John W. Linville		Someday the world will need a hero, and you
linville@tuxdriver.com			might be all we have.  Be ready.

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [RFC] ath9k: Fix ath9k prevents CPU to enter C3 states
@ 2011-02-15 15:59 Mohammed Shafi Shajakhan
  2011-02-15 16:12 ` John W. Linville
  0 siblings, 1 reply; 17+ messages in thread
From: Mohammed Shafi Shajakhan @ 2011-02-15 15:59 UTC (permalink / raw)
  To: linville; +Cc: linux-wireless, lrodriguez, Mohammed Shafi Shajakhan

From: Mohammed Shafi Shajakhan <mshajakhan@atheros.com>

The DMA latency issue is observed only in Intel pinetrail platforms but
in the driver we had a default PM-QOS value of 55. This caused
unnecessary power consumption and battery drain in other platforms.
	Remove the pm-qos thing in the driver code and address the throughput issue in
Intel pinetrail platfroms in user space using any one of the scripts in below links:
http://www.kernel.org/pub/linux/kernel/people/mcgrof/scripts/cpudmalatency.c
http://johannes.sipsolutions.net/files/netlatency.c.txt
More details can be found in the following bugzilla link:
https://bugzilla.kernel.org/show_bug.cgi?id=27532

Signed-off-by: Mohammed Shafi Shajakhan <mshajakhan@atheros.com>
---
 drivers/net/wireless/ath/ath9k/ath9k.h |    5 -----
 drivers/net/wireless/ath/ath9k/init.c  |    7 -------
 drivers/net/wireless/ath/ath9k/main.c  |    8 --------
 3 files changed, 0 insertions(+), 20 deletions(-)

diff --git a/drivers/net/wireless/ath/ath9k/ath9k.h b/drivers/net/wireless/ath/ath9k/ath9k.h
index ba436cd..0052f64 100644
--- a/drivers/net/wireless/ath/ath9k/ath9k.h
+++ b/drivers/net/wireless/ath/ath9k/ath9k.h
@@ -21,7 +21,6 @@
 #include <linux/device.h>
 #include <linux/leds.h>
 #include <linux/completion.h>
-#include <linux/pm_qos_params.h>
 
 #include "debug.h"
 #include "common.h"
@@ -57,8 +56,6 @@ struct ath_node;
 
 #define A_MAX(a, b) ((a) > (b) ? (a) : (b))
 
-#define ATH9K_PM_QOS_DEFAULT_VALUE	55
-
 #define TSF_TO_TU(_h,_l) \
 	((((u32)(_h)) << 22) | (((u32)(_l)) >> 10))
 
@@ -650,7 +647,6 @@ struct ath_softc {
 
 	struct ath_ant_comb ant_comb;
 
-	struct pm_qos_request_list pm_qos_req;
 };
 
 void ath9k_tasklet(unsigned long data);
@@ -665,7 +661,6 @@ static inline void ath_read_cachesize(struct ath_common *common, int *csz)
 extern struct ieee80211_ops ath9k_ops;
 extern int ath9k_modparam_nohwcrypt;
 extern int led_blink;
-extern int ath9k_pm_qos_value;
 extern bool is_ath9k_unloaded;
 
 irqreturn_t ath_isr(int irq, void *dev);
diff --git a/drivers/net/wireless/ath/ath9k/init.c b/drivers/net/wireless/ath/ath9k/init.c
index e5c1eea..8fed4e4 100644
--- a/drivers/net/wireless/ath/ath9k/init.c
+++ b/drivers/net/wireless/ath/ath9k/init.c
@@ -41,9 +41,6 @@ static int ath9k_btcoex_enable;
 module_param_named(btcoex_enable, ath9k_btcoex_enable, int, 0444);
 MODULE_PARM_DESC(btcoex_enable, "Enable wifi-BT coexistence");
 
-int ath9k_pm_qos_value = ATH9K_PM_QOS_DEFAULT_VALUE;
-module_param_named(pmqos, ath9k_pm_qos_value, int, S_IRUSR | S_IRGRP | S_IROTH);
-MODULE_PARM_DESC(pmqos, "User specified PM-QOS value");
 
 bool is_ath9k_unloaded;
 /* We use the hw_value as an index into our private channel structure */
@@ -760,9 +757,6 @@ int ath9k_init_device(u16 devid, struct ath_softc *sc, u16 subsysid,
 	ath_init_leds(sc);
 	ath_start_rfkill_poll(sc);
 
-	pm_qos_add_request(&sc->pm_qos_req, PM_QOS_CPU_DMA_LATENCY,
-			   PM_QOS_DEFAULT_VALUE);
-
 	return 0;
 
 error_world:
@@ -819,7 +813,6 @@ void ath9k_deinit_device(struct ath_softc *sc)
 	ath9k_ps_restore(sc);
 
 	ieee80211_unregister_hw(hw);
-	pm_qos_remove_request(&sc->pm_qos_req);
 	ath_rx_cleanup(sc);
 	ath_tx_cleanup(sc);
 	ath9k_deinit_softc(sc);
diff --git a/drivers/net/wireless/ath/ath9k/main.c b/drivers/net/wireless/ath/ath9k/main.c
index 4f568b8..1d2c7c3 100644
--- a/drivers/net/wireless/ath/ath9k/main.c
+++ b/drivers/net/wireless/ath/ath9k/main.c
@@ -1117,12 +1117,6 @@ static int ath9k_start(struct ieee80211_hw *hw)
 			ath9k_btcoex_timer_resume(sc);
 	}
 
-	/* User has the option to provide pm-qos value as a module
-	 * parameter rather than using the default value of
-	 * 'ATH9K_PM_QOS_DEFAULT_VALUE'.
-	 */
-	pm_qos_update_request(&sc->pm_qos_req, ath9k_pm_qos_value);
-
 	if (ah->caps.pcie_lcr_extsync_en && common->bus_ops->extn_synch_en)
 		common->bus_ops->extn_synch_en(common);
 
@@ -1267,8 +1261,6 @@ static void ath9k_stop(struct ieee80211_hw *hw)
 
 	sc->sc_flags |= SC_OP_INVALID;
 
-	pm_qos_update_request(&sc->pm_qos_req, PM_QOS_DEFAULT_VALUE);
-
 	mutex_unlock(&sc->mutex);
 
 	ath_dbg(common, ATH_DBG_CONFIG, "Driver halt\n");
-- 
1.7.0.4


^ permalink raw reply related	[flat|nested] 17+ messages in thread

end of thread, other threads:[~2011-03-04 14:26 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-02-09 14:33 [RFC] ath9k: Fix ath9k prevents CPU to enter C3 states Mohammed Shafi Shajakhan
2011-02-09 14:42 ` John W. Linville
2011-02-09 14:50   ` Mohammed Shafi
2011-02-09 16:48 ` Richard Schütz
2011-02-10 14:19   ` Mohammed Shafi
2011-02-10 17:41     ` Luis R. Rodriguez
2011-02-10 17:44       ` Richard Schütz
2011-02-11 13:31         ` Richard Schütz
2011-02-09 19:58 ` Luis R. Rodriguez
2011-02-10 14:18   ` Mohammed Shafi
2011-02-10 14:18     ` Mohammed Shafi
2011-02-15 15:59 Mohammed Shafi Shajakhan
2011-02-15 16:12 ` John W. Linville
2011-02-16  5:05   ` Mohammed Shafi
2011-03-04  6:06     ` Mohammed Shafi
2011-03-04 13:37       ` John W. Linville
2011-03-04 14:26         ` Mohammed Shafi

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.