[PATCH v2 1/4] wl12xx: don't check wow param on suspend/resume

[PATCH v2 1/4] wl12xx: don't check wow param on suspend/resume

[Eliad Peller]

Since mac80211 calls suspend/resume only when wowlan triggers
exist, there is no need to check for triggers existance in the
callbacks as well.
Add a WARN_ON() to verify it.

Signed-off-by: Eliad Peller <eliad@wizery.com>
---
 drivers/net/wireless/wl12xx/main.c |   87 ++++++++++++++++++------------------
 1 files changed, 43 insertions(+), 44 deletions(-)

diff --git a/drivers/net/wireless/wl12xx/main.c b/drivers/net/wireless/wl12xx/main.c
index fb0f764..8eb52b7 100644
--- a/drivers/net/wireless/wl12xx/main.c
+++ b/drivers/net/wireless/wl12xx/main.c
@@ -1544,69 +1544,68 @@ static int wl1271_op_suspend(struct ieee80211_hw *hw,
 			    struct cfg80211_wowlan *wow)
 {
 	struct wl1271 *wl = hw->priv;
+	int ret;
+
 	wl1271_debug(DEBUG_MAC80211, "mac80211 suspend wow=%d", !!wow);
-	wl->wow_enabled = !!wow;
-	if (wl->wow_enabled) {
-		int ret;
-		ret = wl1271_configure_suspend(wl);
-		if (ret < 0) {
-			wl1271_warning("couldn't prepare device to suspend");
-			return ret;
-		}
-		/* flush any remaining work */
-		wl1271_debug(DEBUG_MAC80211, "flushing remaining works");
-		flush_delayed_work(&wl->scan_complete_work);
+	WARN_ON(!wow || !wow->any);
 
-		/*
-		 * disable and re-enable interrupts in order to flush
-		 * the threaded_irq
-		 */
-		wl1271_disable_interrupts(wl);
+	wl->wow_enabled = true;
+	ret = wl1271_configure_suspend(wl);
+	if (ret < 0) {
+		wl1271_warning("couldn't prepare device to suspend");
+		return ret;
+	}
+	/* flush any remaining work */
+	wl1271_debug(DEBUG_MAC80211, "flushing remaining works");
+	flush_delayed_work(&wl->scan_complete_work);
 
-		/*
-		 * set suspended flag to avoid triggering a new threaded_irq
-		 * work. no need for spinlock as interrupts are disabled.
-		 */
-		set_bit(WL1271_FLAG_SUSPENDED, &wl->flags);
+	/*
+	 * disable and re-enable interrupts in order to flush
+	 * the threaded_irq
+	 */
+	wl1271_disable_interrupts(wl);
+
+	/*
+	 * set suspended flag to avoid triggering a new threaded_irq
+	 * work. no need for spinlock as interrupts are disabled.
+	 */
+	set_bit(WL1271_FLAG_SUSPENDED, &wl->flags);
+
+	wl1271_enable_interrupts(wl);
+	flush_work(&wl->tx_work);
+	flush_delayed_work(&wl->pspoll_work);
+	flush_delayed_work(&wl->elp_work);
 
-		wl1271_enable_interrupts(wl);
-		flush_work(&wl->tx_work);
-		flush_delayed_work(&wl->pspoll_work);
-		flush_delayed_work(&wl->elp_work);
-	}
 	return 0;
 }
 
 static int wl1271_op_resume(struct ieee80211_hw *hw)
 {
 	struct wl1271 *wl = hw->priv;
+	unsigned long flags;
+	bool run_irq_work = false;
+
 	wl1271_debug(DEBUG_MAC80211, "mac80211 resume wow=%d",
 		     wl->wow_enabled);
+	WARN_ON(!wl->wow_enabled);
 
 	/*
 	 * re-enable irq_work enqueuing, and call irq_work directly if
 	 * there is a pending work.
 	 */
-	if (wl->wow_enabled) {
-		struct wl1271 *wl = hw->priv;
-		unsigned long flags;
-		bool run_irq_work = false;
-
-		spin_lock_irqsave(&wl->wl_lock, flags);
-		clear_bit(WL1271_FLAG_SUSPENDED, &wl->flags);
-		if (test_and_clear_bit(WL1271_FLAG_PENDING_WORK, &wl->flags))
-			run_irq_work = true;
-		spin_unlock_irqrestore(&wl->wl_lock, flags);
-
-		if (run_irq_work) {
-			wl1271_debug(DEBUG_MAC80211,
-				     "run postponed irq_work directly");
-			wl1271_irq(0, wl);
-			wl1271_enable_interrupts(wl);
-		}
+	spin_lock_irqsave(&wl->wl_lock, flags);
+	clear_bit(WL1271_FLAG_SUSPENDED, &wl->flags);
+	if (test_and_clear_bit(WL1271_FLAG_PENDING_WORK, &wl->flags))
+		run_irq_work = true;
+	spin_unlock_irqrestore(&wl->wl_lock, flags);
 
-		wl1271_configure_resume(wl);
+	if (run_irq_work) {
+		wl1271_debug(DEBUG_MAC80211,
+			     "run postponed irq_work directly");
+		wl1271_irq(0, wl);
+		wl1271_enable_interrupts(wl);
 	}
+	wl1271_configure_resume(wl);
 
 	return 0;
 }
-- 
1.7.0.4

[PATCH v2 2/4] wl12xx: clear wl->wow_enabled on resume

[Eliad Peller]

We set wl->wow_enabled on every suspend(), so we need to clear it
on every resume().
(we can't rely on setting wl->wow_enabled=false in suspend(),
as it being called only when wowlan triggers are configured)

Signed-off-by: Eliad Peller <eliad@wizery.com>
---
 drivers/net/wireless/wl12xx/main.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/drivers/net/wireless/wl12xx/main.c b/drivers/net/wireless/wl12xx/main.c
index 8eb52b7..4f20711 100644
--- a/drivers/net/wireless/wl12xx/main.c
+++ b/drivers/net/wireless/wl12xx/main.c
@@ -1606,6 +1606,7 @@ static int wl1271_op_resume(struct ieee80211_hw *hw)
 		wl1271_enable_interrupts(wl);
 	}
 	wl1271_configure_resume(wl);
+	wl->wow_enabled = false;
 
 	return 0;
 }
-- 
1.7.0.4

[PATCH v2 3/4] wl12xx: enable/disable beacon filtering on ap suspend/resume

[Eliad Peller]

Beacon filtering needs to be enabled so AP won't wake up by
by every received beacon.

Signed-off-by: Eliad Peller <eliad@wizery.com>
---
 drivers/net/wireless/wl12xx/main.c |   49 +++++++++++++++++++++++++++++------
 1 files changed, 40 insertions(+), 9 deletions(-)

diff --git a/drivers/net/wireless/wl12xx/main.c b/drivers/net/wireless/wl12xx/main.c
index 4f20711..97f565c 100644
--- a/drivers/net/wireless/wl12xx/main.c
+++ b/drivers/net/wireless/wl12xx/main.c
@@ -1467,13 +1467,10 @@ static struct notifier_block wl1271_dev_notifier = {
 	.notifier_call = wl1271_dev_notify,
 };
 
-static int wl1271_configure_suspend(struct wl1271 *wl)
+static int wl1271_configure_suspend_sta(struct wl1271 *wl)
 {
 	int ret;
 
-	if (wl->bss_type != BSS_TYPE_STA_BSS)
-		return 0;
-
 	mutex_lock(&wl->mutex);
 
 	ret = wl1271_ps_elp_wakeup(wl);
@@ -1518,11 +1515,41 @@ out:
 
 }
 
+static int wl1271_configure_suspend_ap(struct wl1271 *wl)
+{
+	int ret;
+
+	mutex_lock(&wl->mutex);
+
+	ret = wl1271_ps_elp_wakeup(wl);
+	if (ret < 0)
+		goto out_unlock;
+
+	ret = wl1271_acx_set_ap_beacon_filter(wl, true);
+
+	wl1271_ps_elp_sleep(wl);
+out_unlock:
+	mutex_unlock(&wl->mutex);
+	return ret;
+
+}
+
+static int wl1271_configure_suspend(struct wl1271 *wl)
+{
+	if (wl->bss_type == BSS_TYPE_STA_BSS)
+		return wl1271_configure_suspend_sta(wl);
+	if (wl->bss_type == BSS_TYPE_AP_BSS)
+		return wl1271_configure_suspend_ap(wl);
+	return 0;
+}
+
 static void wl1271_configure_resume(struct wl1271 *wl)
 {
 	int ret;
+	bool is_sta = wl->bss_type == BSS_TYPE_STA_BSS;
+	bool is_ap = wl->bss_type == BSS_TYPE_AP_BSS;
 
-	if (wl->bss_type != BSS_TYPE_STA_BSS)
+	if (!is_sta && !is_ap)
 		return;
 
 	mutex_lock(&wl->mutex);
@@ -1530,10 +1557,14 @@ static void wl1271_configure_resume(struct wl1271 *wl)
 	if (ret < 0)
 		goto out;
 
-	/* exit psm if it wasn't configured */
-	if (!test_bit(WL1271_FLAG_PSM_REQUESTED, &wl->flags))
-		wl1271_ps_set_mode(wl, STATION_ACTIVE_MODE,
-				   wl->basic_rate, true);
+	if (is_sta) {
+		/* exit psm if it wasn't configured */
+		if (!test_bit(WL1271_FLAG_PSM_REQUESTED, &wl->flags))
+			wl1271_ps_set_mode(wl, STATION_ACTIVE_MODE,
+					   wl->basic_rate, true);
+	} else if (is_ap) {
+		wl1271_acx_set_ap_beacon_filter(wl, false);
+	}
 
 	wl1271_ps_elp_sleep(wl);
 out:
-- 
1.7.0.4

[PATCH v2 4/4] wl12xx_sdio: enable wowlan only if enable_irq_wake() succeeded

[Eliad Peller]

Some platforms don't support the wake_irq, so disable wowlan
in this case, and avoid the "Unbalanced IRQ wake disable"
warning on disable_irq_wake().

Signed-off-by: Eliad Peller <eliad@wizery.com>
---
v1->v2:
	call disable_irq() regardless of enable_irq_wake() result

 drivers/net/wireless/wl12xx/sdio.c   |   26 +++++++++++++++-----------
 drivers/net/wireless/wl12xx/wl12xx.h |    1 +
 2 files changed, 16 insertions(+), 11 deletions(-)

diff --git a/drivers/net/wireless/wl12xx/sdio.c b/drivers/net/wireless/wl12xx/sdio.c
index 74fd7c6..22d0b8a 100644
--- a/drivers/net/wireless/wl12xx/sdio.c
+++ b/drivers/net/wireless/wl12xx/sdio.c
@@ -267,17 +267,19 @@ static int __devinit wl1271_probe(struct sdio_func *func,
 		goto out_free;
 	}
 
-	enable_irq_wake(wl->irq);
-	device_init_wakeup(wl1271_sdio_wl_to_dev(wl), 1);
+	ret = enable_irq_wake(wl->irq);
+	if (!ret) {
+		wl->irq_wake_enabled = true;
+		device_init_wakeup(wl1271_sdio_wl_to_dev(wl), 1);
 
-	disable_irq(wl->irq);
-
-	/* if sdio can keep power while host is suspended, enable wow */
-	mmcflags = sdio_get_host_pm_caps(func);
-	wl1271_debug(DEBUG_SDIO, "sdio PM caps = 0x%x", mmcflags);
+		/* if sdio can keep power while host is suspended, enable wow */
+		mmcflags = sdio_get_host_pm_caps(func);
+		wl1271_debug(DEBUG_SDIO, "sdio PM caps = 0x%x", mmcflags);
 
-	if (mmcflags & MMC_PM_KEEP_POWER)
-		hw->wiphy->wowlan.flags = WIPHY_WOWLAN_ANY;
+		if (mmcflags & MMC_PM_KEEP_POWER)
+			hw->wiphy->wowlan.flags = WIPHY_WOWLAN_ANY;
+	}
+	disable_irq(wl->irq);
 
 	ret = wl1271_init_ieee80211(wl);
 	if (ret)
@@ -311,8 +313,10 @@ static void __devexit wl1271_remove(struct sdio_func *func)
 	pm_runtime_get_noresume(&func->dev);
 
 	wl1271_unregister_hw(wl);
-	device_init_wakeup(wl1271_sdio_wl_to_dev(wl), 0);
-	disable_irq_wake(wl->irq);
+	if (wl->irq_wake_enabled) {
+		device_init_wakeup(wl1271_sdio_wl_to_dev(wl), 0);
+		disable_irq_wake(wl->irq);
+	}
 	free_irq(wl->irq, wl);
 	wl1271_free_hw(wl);
 }
diff --git a/drivers/net/wireless/wl12xx/wl12xx.h b/drivers/net/wireless/wl12xx/wl12xx.h
index 1cafb08..06785db 100644
--- a/drivers/net/wireless/wl12xx/wl12xx.h
+++ b/drivers/net/wireless/wl12xx/wl12xx.h
@@ -579,6 +579,7 @@ struct wl1271 {
 	 * (currently, only "ANY" trigger is supported)
 	 */
 	bool wow_enabled;
+	bool irq_wake_enabled;
 
 	/*
 	 * AP-mode - links indexed by HLID. The global and broadcast links
-- 
1.7.0.4

Re: [PATCH v2 1/4] wl12xx: don't check wow param on suspend/resume

[Luciano Coelho]

On Mon, 2011-06-06 at 12:21 +0300, Eliad Peller wrote: 
> Since mac80211 calls suspend/resume only when wowlan triggers
> exist, there is no need to check for triggers existance in the
> callbacks as well.
> Add a WARN_ON() to verify it.
> 
> Signed-off-by: Eliad Peller <eliad@wizery.com>
> ---

Applied the series, thanks!

-- 
Cheers,
Luca.