[PATCH 1/3] mac80211: refactor dynamic power save check

[PATCH 1/3] mac80211: refactor dynamic power save check

[Kalle Valo]

From: Kalle Valo <kalle.valo@nokia.com>

Refactor dynamic power save checks to a function of it's own for better
readibility. No functional changes.

Signed-off-by: Kalle Valo <kalle.valo@nokia.com>
---

 net/mac80211/tx.c |   24 +++++++++++++++++++++---
 1 files changed, 21 insertions(+), 3 deletions(-)

diff --git a/net/mac80211/tx.c b/net/mac80211/tx.c
index 10a1099..03005f9 100644
--- a/net/mac80211/tx.c
+++ b/net/mac80211/tx.c
@@ -1384,6 +1384,26 @@ static int ieee80211_skb_resize(struct ieee80211_local *local,
 	return 0;
 }
 
+static bool is_dynamic_ps_enabled(struct ieee80211_local *local)
+{
+	if (!(local->hw.flags & IEEE80211_HW_PS_NULLFUNC_STACK))
+		/* driver doesn't support power save */
+		return false;
+
+	if (local->hw.conf.dynamic_ps_timeout <= 0)
+		/* dynamic power save disabled */
+		return false;
+
+	if (local->scanning)
+		/* we are scanning, don't enable power save */
+		return false;
+
+	if (!local->ps_sdata)
+		return false;
+
+	return true;
+}
+
 static void ieee80211_xmit(struct ieee80211_sub_if_data *sdata,
 			   struct sk_buff *skb)
 {
@@ -1396,9 +1416,7 @@ static void ieee80211_xmit(struct ieee80211_sub_if_data *sdata,
 
 	dev_hold(sdata->dev);
 
-	if ((local->hw.flags & IEEE80211_HW_PS_NULLFUNC_STACK) &&
-	    local->hw.conf.dynamic_ps_timeout > 0 &&
-	    !(local->scanning) && local->ps_sdata) {
+	if (is_dynamic_ps_enabled(local)) {
 		if (local->hw.conf.flags & IEEE80211_CONF_PS) {
 			ieee80211_stop_queues_by_reason(&local->hw,
 					IEEE80211_QUEUE_STOP_REASON_PS);

[PATCH 2/3] mac80211: fix dynamic power save for devices which have nullfunc support

[Kalle Valo]

From: Kalle Valo <kalle.valo@nokia.com>

In TX path it was assumed that dynamic power save works only if
IEEE80211_HW_PS_NULLFUNC_STACK is set. But is not the case, there are
devices which have nullfunc support in hardware but need mac80211
to handle dynamic power save timers, TI's wl1251 is one of them.

The fix is to not check for IEEE80211_HW_PS_NULLFUNC_STACK in
is_dynamic_ps_enabled(), instead check IEEE80211_HW_SUPPORTS_PS and
IEEE80211_HW_SUPPORTS_DYNAMIC_PS flags and act accordingly.

Tested with wl1251.

Signed-off-by: Kalle Valo <kalle.valo@nokia.com>
---

 net/mac80211/tx.c |    7 ++++++-
 1 files changed, 6 insertions(+), 1 deletions(-)

diff --git a/net/mac80211/tx.c b/net/mac80211/tx.c
index 03005f9..0e7273c 100644
--- a/net/mac80211/tx.c
+++ b/net/mac80211/tx.c
@@ -1386,10 +1386,15 @@ static int ieee80211_skb_resize(struct ieee80211_local *local,
 
 static bool is_dynamic_ps_enabled(struct ieee80211_local *local)
 {
-	if (!(local->hw.flags & IEEE80211_HW_PS_NULLFUNC_STACK))
+
+	if (!(local->hw.flags & IEEE80211_HW_SUPPORTS_PS))
 		/* driver doesn't support power save */
 		return false;
 
+	if (local->hw.flags & IEEE80211_HW_SUPPORTS_DYNAMIC_PS)
+		/* hardware does dynamic power save */
+		return false;
+
 	if (local->hw.conf.dynamic_ps_timeout <= 0)
 		/* dynamic power save disabled */
 		return false;

Re: [PATCH 2/3] mac80211: fix dynamic power save for devices which have nullfunc support

[Johannes Berg]

[-- Attachment #1: Type: text/plain, Size: 760 bytes --]

On Wed, 2009-08-12 at 17:52 +0300, Kalle Valo wrote:

>  static bool is_dynamic_ps_enabled(struct ieee80211_local *local)
>  {
> -	if (!(local->hw.flags & IEEE80211_HW_PS_NULLFUNC_STACK))
> +
> +	if (!(local->hw.flags & IEEE80211_HW_SUPPORTS_PS))
>  		/* driver doesn't support power save */
>  		return false;

Why the blank line?
 
> +	if (local->hw.flags & IEEE80211_HW_SUPPORTS_DYNAMIC_PS)
> +		/* hardware does dynamic power save */
> +		return false;

FWIW, also applies to patch 1, I think I prefer

	/* check if hardware does dynamic power save */
	if (local->hw.flags & ...)
		return false;

(wrt. comment location)

and you should probably rename the function to do_software_dynamic_ps()
or something like that?

johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

Re: [PATCH 2/3] mac80211: fix dynamic power save for devices which have nullfunc support

[Kalle Valo]

Johannes Berg <johannes@sipsolutions.net> writes:

> On Wed, 2009-08-12 at 17:52 +0300, Kalle Valo wrote:
>
>>  static bool is_dynamic_ps_enabled(struct ieee80211_local *local)
>>  {
>> -	if (!(local->hw.flags & IEEE80211_HW_PS_NULLFUNC_STACK))
>> +
>> +	if (!(local->hw.flags & IEEE80211_HW_SUPPORTS_PS))
>>  		/* driver doesn't support power save */
>>  		return false;
>
> Why the blank line?

A mistake, will remove.

>> +	if (local->hw.flags & IEEE80211_HW_SUPPORTS_DYNAMIC_PS)
>> +		/* hardware does dynamic power save */
>> +		return false;
>
> FWIW, also applies to patch 1, I think I prefer
>
> 	/* check if hardware does dynamic power save */
> 	if (local->hw.flags & ...)
> 		return false;
>
> (wrt. comment location)

I'll change that.

> and you should probably rename the function to do_software_dynamic_ps()
> or something like that?

I had problems coming up with a good name. "do" in a sense is confusing
because the function only checks the state, it doesn't have any code to
change the timer or anything like that. I'll think of this a bit more.
Suggestions are welcome.

I'll send v2 later this week. John, please drop these three patches.

-- 
Kalle Valo

[PATCH 3/3] wl1251: enable power save

[Kalle Valo]

From: Kalle Valo <kalle.valo@nokia.com>

wl1251 supports power save and it can be enabled now.

Signed-off-by: Kalle Valo <kalle.valo@nokia.com>
---

 drivers/net/wireless/wl12xx/wl1251_main.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/drivers/net/wireless/wl12xx/wl1251_main.c b/drivers/net/wireless/wl12xx/wl1251_main.c
index 7148934..72e2dc9 100644
--- a/drivers/net/wireless/wl12xx/wl1251_main.c
+++ b/drivers/net/wireless/wl12xx/wl1251_main.c
@@ -1310,7 +1310,8 @@ int wl1251_init_ieee80211(struct wl1251 *wl)
 	wl->hw->channel_change_time = 10000;
 
 	wl->hw->flags = IEEE80211_HW_SIGNAL_DBM |
-		IEEE80211_HW_NOISE_DBM;
+		IEEE80211_HW_NOISE_DBM |
+		IEEE80211_HW_SUPPORTS_PS;
 
 	wl->hw->wiphy->interface_modes = BIT(NL80211_IFTYPE_STATION);
 	wl->hw->wiphy->max_scan_ssids = 1;