[PATCH v3 0/6] libertas: ieee80211 powersave mode

[PATCH v3 0/6] libertas: ieee80211 powersave mode

[Andreas Kemnade]

This series makes IEEE 80211 powersave mode work again so
that power usage is dramatically reduced when the device is connected.
It does not include other power saving methods which are working
when the device is not connected (like enabling
deep sleep modus)

Tested on GTA04 which includes a
W2CBW003 chip (Marvel 8686) with sdio interface

Changes in v3:
s/wireless:libertas/libertas/
s|net/wireless/libertas|net/wireless/marvell/libertas|

Changes in v2:
improved some commit messages, order changed,
former 6/6 was too late it needs to be before
implementing the cfg80211  power saving interface
to have it git-bisectable. 
moving the former 3/6 to the end fixes
all bisect problems

[PATCH v3 1/6] libertas: fix pointer bugs for PS_MODE
[PATCH v3 2/6] libertas: check whether bus can do more than
[PATCH v3 3/6] libertas: do not confirm sleep if commands
[PATCH v3 4/6] libertas: go back to ps mode without commands
[PATCH v3 5/6] libertas: fix ps-mode related removal
[PATCH v3 6/6] libertas: add an cfg80211 interface for

[PATCH v3 1/6] libertas: fix pointer bugs for PS_MODE commands

[Andreas Kemnade]

struct cmd_ds_802_11_ps_mode
contains the command header and a pointer to it was
initialized with data points to the body which leads to
mis-interpretation of the cmd_ds_802_11_ps_mode.action member.
cmd[0] contains the header, &cmd[1] points beyond that.
cmdnode->cmdbuf is a pointer to the command buffer
This piece of code was unused since power saving was
not enabled.

Signed-off-by: Andreas Kemnade <andreas@kemnade.info>
---
changes in v3:
corrected paths
changes in v2:
improved commit message

 drivers/net/wireless/marvell/libertas/cmd.c     | 4 ++--
 drivers/net/wireless/marvell/libertas/cmdresp.c | 5 ++++-
 2 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/net/wireless/marvell/libertas/cmd.c b/drivers/net/wireless/libertas/cmd.c
index 0387a5b..40467d6 100644
--- a/drivers/net/wireless/marvell/libertas/cmd.c
+++ b/drivers/net/wireless/marvell/libertas/cmd.c
@@ -957,7 +957,7 @@ static void lbs_queue_cmd(struct lbs_private *priv,
 
 	/* Exit_PS command needs to be queued in the header always. */
 	if (le16_to_cpu(cmdnode->cmdbuf->command) == CMD_802_11_PS_MODE) {
-		struct cmd_ds_802_11_ps_mode *psm = (void *) &cmdnode->cmdbuf;
+		struct cmd_ds_802_11_ps_mode *psm = (void *)cmdnode->cmdbuf;
 
 		if (psm->action == cpu_to_le16(PS_MODE_ACTION_EXIT_PS)) {
 			if (priv->psstate != PS_STATE_FULL_POWER)
@@ -1387,7 +1387,7 @@ int lbs_execute_next_command(struct lbs_private *priv)
 				 * PS command. Ignore it if it is not Exit_PS.
 				 * otherwise send it down immediately.
 				 */
-				struct cmd_ds_802_11_ps_mode *psm = (void *)&cmd[1];
+				struct cmd_ds_802_11_ps_mode *psm = (void *)cmd;
 
 				lbs_deb_host(
 				       "EXEC_NEXT_CMD: PS cmd, action 0x%02x\n",
diff --git a/drivers/net/wireless/marvell/libertas/cmdresp.c b/drivers/net/wireless/libertas/cmdresp.c
index e5442e8..701125f 100644
--- a/drivers/net/wireless/marvell/libertas/cmdresp.c
+++ b/drivers/net/wireless/marvell/libertas/cmdresp.c
@@ -123,7 +123,10 @@ int lbs_process_command_response(struct lbs_private *priv, u8 *data, u32 len)
 	priv->cmd_timed_out = 0;
 
 	if (respcmd == CMD_RET(CMD_802_11_PS_MODE)) {
-		struct cmd_ds_802_11_ps_mode *psmode = (void *) &resp[1];
+		/* struct cmd_ds_802_11_ps_mode also contains
+		 * the header
+		 */
+		struct cmd_ds_802_11_ps_mode *psmode = (void *)resp;
 		u16 action = le16_to_cpu(psmode->action);
 
 		lbs_deb_host(
-- 
2.1.4

Re: [v3,1/6] libertas: fix pointer bugs for PS_MODE commands

[Kalle Valo]

> struct cmd_ds_802_11_ps_mode
> contains the command header and a pointer to it was
> initialized with data points to the body which leads to
> mis-interpretation of the cmd_ds_802_11_ps_mode.action member.
> cmd[0] contains the header, &cmd[1] points beyond that.
> cmdnode->cmdbuf is a pointer to the command buffer
> This piece of code was unused since power saving was
> not enabled.
> 
> Signed-off-by: Andreas Kemnade <andreas@kemnade.info>

Thanks, 6 patches applied to wireless-drivers-next.git:

0a7701b4defc libertas: fix pointer bugs for PS_MODE commands
fae4f9f78ab1 libertas: check whether bus can do more than polling
57954b94cad7 libertas: do not confirm sleep if commands are pending
fada24a54770 libertas: go back to ps mode without commands pending
0b8802dc5f59 libertas: fix ps-mode related removal problems
143e49458424 libertas: add an cfg80211 interface for powersaving

Kalle Valo

[PATCH v3 2/6] libertas: check whether bus can do more than polling

[Andreas Kemnade]

If a sdio host does not support sdio irqs, polling is used
instead. That has an impact on performance. Some functionality
should not be enabled then. This add a variable in
libertas_priv to indicate that.

Signed-off-by: Andreas Kemnade <andreas@kemnade.info>
---
changes in v3:
corrected paths
 drivers/net/wireless/marvell/libertas/dev.h     | 1 +
 drivers/net/wireless/marvell/libertas/if_sdio.c | 2 +-
 drivers/net/wireless/marvell/libertas/if_usb.c  | 1 +
 3 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/net/wireless/marvell/libertas/dev.h b/drivers/net/wireless/libertas/dev.h
index 6bd1608..edf710b 100644
--- a/drivers/net/wireless/marvell/libertas/dev.h
+++ b/drivers/net/wireless/marvell/libertas/dev.h
@@ -99,6 +99,7 @@ struct lbs_private {
 	/* Hardware access */
 	void *card;
 	bool iface_running;
+	u8 is_polling; /* host has to poll the card irq */
 	u8 fw_ready;
 	u8 surpriseremoved;
 	u8 setup_fw_on_resume;
diff --git a/drivers/net/wireless/marvell/libertas/if_sdio.c b/drivers/net/wireless/libertas/if_sdio.c
index f1f31a2..13db8b5 100644
--- a/drivers/net/wireless/marvell/libertas/if_sdio.c
+++ b/drivers/net/wireless/marvell/libertas/if_sdio.c
@@ -1271,7 +1271,7 @@ static int if_sdio_probe(struct sdio_func *func,
 	priv->reset_card = if_sdio_reset_card;
 	priv->power_save = if_sdio_power_save;
 	priv->power_restore = if_sdio_power_restore;
-
+	priv->is_polling = !(func->card->host->caps & MMC_CAP_SDIO_IRQ);
 	ret = if_sdio_power_on(card);
 	if (ret)
 		goto err_activate_card;
diff --git a/drivers/net/wireless/marvell/libertas/if_usb.c b/drivers/net/wireless/libertas/if_usb.c
index dff08a2..aba0c99 100644
--- a/drivers/net/wireless/marvell/libertas/if_usb.c
+++ b/drivers/net/wireless/marvell/libertas/if_usb.c
@@ -267,6 +267,7 @@ static int if_usb_probe(struct usb_interface *intf,
 	priv->enter_deep_sleep = NULL;
 	priv->exit_deep_sleep = NULL;
 	priv->reset_deep_sleep_wakeup = NULL;
+	priv->is_polling = false;
 #ifdef CONFIG_OLPC
 	if (machine_is_olpc())
 		priv->reset_card = if_usb_reset_olpc_card;
-- 
2.1.4

[PATCH v3 3/6] libertas: do not confirm sleep if commands are pending

[Andreas Kemnade]

If the main thread gets one PS AWAKE event and one PS SLEEP event
in one iteration over event_fifo there will never be checks for
commands to be processed, since psstate will always be
PS_STATE_SLEEP or PS_STATE_PRE_SLEEP

Signed-off-by: Andreas Kemnade <andreas@kemnade.info>
---
changes in v3: corrected paths
changes in v2: reordered: was 4/6
 drivers/net/wireless/marvell/libertas/cmdresp.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/net/wireless/marvell/libertas/cmdresp.c b/drivers/net/wireless/libertas/cmdresp.c
index 701125f..c95bf6d 100644
--- a/drivers/net/wireless/marvell/libertas/cmdresp.c
+++ b/drivers/net/wireless/marvell/libertas/cmdresp.c
@@ -257,6 +257,10 @@ int lbs_process_event(struct lbs_private *priv, u32 event)
 			       "EVENT: in FULL POWER mode, ignoring PS_SLEEP\n");
 			break;
 		}
+		if (!list_empty(&priv->cmdpendingq)) {
+			lbs_deb_cmd("EVENT: commands in queue, do not sleep\n");
+			break;
+		}
 		priv->psstate = PS_STATE_PRE_SLEEP;
 
 		lbs_ps_confirm_sleep(priv);
-- 
2.1.4

[PATCH v3 4/6] libertas: go back to ps mode without commands pending

[Andreas Kemnade]

Removes the old todo block and checks only whether ieee powersave
mode is requested. We still have to check for being connected as
this powersave mode includes logic for regularly waking up and
checking for packets which only makes sense when connected.
For not being connected, another mode is needed.

Signed-off-by: Andreas Kemnade <andreas@kemnade.info>
---
changes in v3: corrected paths
changes in v2: reordered, was 5/6
 drivers/net/wireless/marvell/libertas/cmd.c | 36 +++++-------------------------------
 1 file changed, 5 insertions(+), 31 deletions(-)

diff --git a/drivers/net/wireless/marvell/libertas/cmd.c b/drivers/net/wireless/libertas/cmd.c
index 40467d6..4ddd0e5 100644
--- a/drivers/net/wireless/marvell/libertas/cmd.c
+++ b/drivers/net/wireless/marvell/libertas/cmd.c
@@ -1428,40 +1428,14 @@ int lbs_execute_next_command(struct lbs_private *priv)
 		 * check if in power save mode, if yes, put the device back
 		 * to PS mode
 		 */
-#ifdef TODO
-		/*
-		 * This was the old code for libertas+wext. Someone that
-		 * understands this beast should re-code it in a sane way.
-		 *
-		 * I actually don't understand why this is related to WPA
-		 * and to connection status, shouldn't powering should be
-		 * independ of such things?
-		 */
 		if ((priv->psmode != LBS802_11POWERMODECAM) &&
 		    (priv->psstate == PS_STATE_FULL_POWER) &&
-		    ((priv->connect_status == LBS_CONNECTED) ||
-		    lbs_mesh_connected(priv))) {
-			if (priv->secinfo.WPAenabled ||
-			    priv->secinfo.WPA2enabled) {
-				/* check for valid WPA group keys */
-				if (priv->wpa_mcast_key.len ||
-				    priv->wpa_unicast_key.len) {
-					lbs_deb_host(
-					       "EXEC_NEXT_CMD: WPA enabled and GTK_SET"
-					       " go back to PS_SLEEP");
-					lbs_set_ps_mode(priv,
-							PS_MODE_ACTION_ENTER_PS,
-							false);
-				}
-			} else {
-				lbs_deb_host(
-				       "EXEC_NEXT_CMD: cmdpendingq empty, "
-				       "go back to PS_SLEEP");
-				lbs_set_ps_mode(priv, PS_MODE_ACTION_ENTER_PS,
-						false);
-			}
+		    (priv->connect_status == LBS_CONNECTED)) {
+			lbs_deb_host(
+				"EXEC_NEXT_CMD: cmdpendingq empty, go back to PS_SLEEP");
+			lbs_set_ps_mode(priv, PS_MODE_ACTION_ENTER_PS,
+					false);
 		}
-#endif
 	}
 
 	ret = 0;
-- 
2.1.4

[PATCH v3 5/6] libertas: fix ps-mode related removal problems

[Andreas Kemnade]

When the device is remove e.g. because of going to suspend
 mode with powersaving enabled, lbs_remove_card tries to exit
powersaving state even when already woken up. That command is
not processed properly in that situation, since the command
processing queue is already stopped, so it waits forever
for the command being processed, so disable it.

Signed-off-by: Andreas Kemnade <andreas@kemnade.info>
---
changes in v3: corrected paths
changes in v2: improved commit message, reordered: was 6/6
 drivers/net/wireless/marvell/libertas/main.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/net/wireless/marvell/libertas/main.c b/drivers/net/wireless/libertas/main.c
index 8079560..b35b8bc 100644
--- a/drivers/net/wireless/marvell/libertas/main.c
+++ b/drivers/net/wireless/marvell/libertas/main.c
@@ -1060,7 +1060,12 @@ void lbs_remove_card(struct lbs_private *priv)
 
 	if (priv->psmode == LBS802_11POWERMODEMAX_PSP) {
 		priv->psmode = LBS802_11POWERMODECAM;
-		lbs_set_ps_mode(priv, PS_MODE_ACTION_EXIT_PS, true);
+		/* no need to wakeup if already woken up,
+		 * on suspend, this exit ps command is not processed
+		 * the driver hangs
+		 */
+		if (priv->psstate != PS_STATE_FULL_POWER)
+			lbs_set_ps_mode(priv, PS_MODE_ACTION_EXIT_PS, true);
 	}
 
 	if (priv->is_deep_sleep) {
-- 
2.1.4

[PATCH v3 6/6] libertas: add an cfg80211 interface for powersaving

[Andreas Kemnade]

This patch adds an interface for handling commands like
iwconfig wlanX power on/off. Such an interface formerly existed
when the driver used wext.

While performance with sdio in polling mode without using
powersave mode is quite bad, powersaving mode is unusable,
so do not enable it under such conditions.

Signed-off-by: Andreas Kemnade <andreas@kemnade.info>
---
changes in v3: corrected paths
changes in v2: reordered, was 3/6
 drivers/net/wireless/marvell/libertas/cfg.c | 38 +++++++++++++++++++++++++++++++++++++
 1 file changed, 38 insertions(+)

diff --git a/drivers/net/wireless/marvell/libertas/cfg.c b/drivers/net/wireless/libertas/cfg.c
index 8317afd..fd18d03 100644
--- a/drivers/net/wireless/marvell/libertas/cfg.c
+++ b/drivers/net/wireless/marvell/libertas/cfg.c
@@ -2038,6 +2038,43 @@ static int lbs_leave_ibss(struct wiphy *wiphy, struct net_device *dev)
 
 
 
+int lbs_set_power_mgmt(struct wiphy *wiphy, struct net_device *dev,
+		       bool enabled, int timeout)
+{
+	struct lbs_private *priv = wiphy_priv(wiphy);
+
+	if  (!(priv->fwcapinfo & FW_CAPINFO_PS)) {
+		if (!enabled)
+			return 0;
+		else
+			return -EINVAL;
+	}
+	/* firmware does not work well with too long latency with power saving
+	 * enabled, so do not enable it if there is only polling, no
+	 * interrupts (like in some sdio hosts which can only
+	 * poll for sdio irqs)
+	 */
+	if  (priv->is_polling) {
+		if (!enabled)
+			return 0;
+		else
+			return -EINVAL;
+	}
+	if (!enabled) {
+		priv->psmode = LBS802_11POWERMODECAM;
+		if (priv->psstate != PS_STATE_FULL_POWER)
+			lbs_set_ps_mode(priv,
+					PS_MODE_ACTION_EXIT_PS,
+					true);
+		return 0;
+	}
+	if (priv->psmode != LBS802_11POWERMODECAM)
+		return 0;
+	priv->psmode = LBS802_11POWERMODEMAX_PSP;
+	if (priv->connect_status == LBS_CONNECTED)
+		lbs_set_ps_mode(priv, PS_MODE_ACTION_ENTER_PS, true);
+	return 0;
+}
 
 /*
  * Initialization
@@ -2056,6 +2093,7 @@ static struct cfg80211_ops lbs_cfg80211_ops = {
 	.change_virtual_intf = lbs_change_intf,
 	.join_ibss = lbs_join_ibss,
 	.leave_ibss = lbs_leave_ibss,
+	.set_power_mgmt = lbs_set_power_mgmt,
 };
 
 
-- 
2.1.4