[PATCH 1/2] mac80211: add suspend/resume callbacks

[PATCH 1/2] mac80211: add suspend/resume callbacks

[Bob Copeland]

This patch introduces suspend and resume callbacks to mac80211, allowing
mac80211 to quiesce its state (bringing down interfaces, removing keys, etc)
in preparation for suspend.  The driver is responsible for calling
ieee80211_notify_mac with the appropriate option from its suspend/resume
hook.
---
 include/net/mac80211.h |   41 +++++++++++++++++
 net/mac80211/Makefile  |    2 +
 net/mac80211/pm.c      |  116 ++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 159 insertions(+), 0 deletions(-)
 create mode 100644 net/mac80211/pm.c

diff --git a/include/net/mac80211.h b/include/net/mac80211.h
index 6a1d4ea..933f82f 100644
--- a/include/net/mac80211.h
+++ b/include/net/mac80211.h
@@ -1873,6 +1873,47 @@ void ieee80211_stop_tx_ba_cb_irqsafe(struct ieee80211_hw *hw, const u8 *ra,
 struct ieee80211_sta *ieee80211_find_sta(struct ieee80211_hw *hw,
 					 const u8 *addr);
 
+#ifdef CONFIG_PM
+void __ieee80211_suspend(struct ieee80211_hw *);
+void __ieee80211_resume(struct ieee80211_hw *);
+#endif
+/**
+ * ieee80211_suspend - prepare for platform suspend
+ *
+ * Call this function from the low level driver from the bus suspend
+ * function to prepare for suspend to ram or disk.  The function will 
+ * quiesce active interfaces and save state for resume.
+ *
+ * This function must not be called with any locks held as it may 
+ * call back into the driver.
+ *
+ * @hw: pointer as obtained from ieee80211_alloc_hw()
+ */
+static inline void ieee80211_suspend(struct ieee80211_hw *hw)
+{
+#ifdef CONFIG_PM
+	__ieee80211_suspend(hw);
+#endif
+}
+
+/**
+ * ieee80211_resume - resume operation from sleep
+ *
+ * Call this function from the low level driver from the bus resume
+ * function to resume from ram or disk.  This will reactivate interfaces 
+ * and reprogram the hardware to continue normal operation.
+ *
+ * This function must not be called with any locks held as it may 
+ * call back into the driver.
+ *
+ * @hw: pointer as obtained from ieee80211_alloc_hw()
+ */
+static inline void ieee80211_resume(struct ieee80211_hw *hw)
+{
+#ifdef CONFIG_PM
+	__ieee80211_resume(hw);
+#endif
+}
 
 /* Rate control API */
 
diff --git a/net/mac80211/Makefile b/net/mac80211/Makefile
index 31cfd1f..0ecbc93 100644
--- a/net/mac80211/Makefile
+++ b/net/mac80211/Makefile
@@ -37,6 +37,8 @@ mac80211-$(CONFIG_MAC80211_MESH) += \
 	mesh_plink.o \
 	mesh_hwmp.o
 
+mac80211-$(CONFIG_PM) += pm.o
+
 # objects for PID algorithm
 rc80211_pid-y := rc80211_pid_algo.o
 rc80211_pid-$(CONFIG_MAC80211_DEBUGFS) += rc80211_pid_debugfs.o
diff --git a/net/mac80211/pm.c b/net/mac80211/pm.c
new file mode 100644
index 0000000..e3e537f
--- /dev/null
+++ b/net/mac80211/pm.c
@@ -0,0 +1,116 @@
+#include <net/mac80211.h>
+#include <net/rtnetlink.h>
+
+#include "ieee80211_i.h"
+#include "led.h"
+
+void __ieee80211_suspend(struct ieee80211_hw *hw)
+{
+	struct ieee80211_local *local = hw_to_local(hw);
+	struct ieee80211_sub_if_data *sdata;
+	struct ieee80211_if_init_conf conf;
+	struct sta_info *sta;
+
+	printk(KERN_DEBUG "mac80211: suspending\n");
+
+	rtnl_lock();
+
+	flush_workqueue(local->hw.workqueue);
+
+	/* disable aggregation and notify that STAs are going away */
+	list_for_each_entry(sta, &local->sta_list, list) {
+		sdata = sta->sdata;
+		ieee80211_sta_tear_down_BA_sessions(sta->sdata, sta->sta.addr);
+
+		if (local->ops->sta_notify) {
+			if (sdata->vif.type == NL80211_IFTYPE_AP_VLAN)
+				sdata = container_of(sdata->bss,
+					     struct ieee80211_sub_if_data,
+					     u.ap);
+
+			local->ops->sta_notify(hw, &sdata->vif, 
+				STA_NOTIFY_REMOVE, &sta->sta);
+		}
+	}
+
+	/* remove all interfaces */
+	list_for_each_entry(sdata, &local->interfaces, list) {
+		ieee80211_disable_keys(sdata);
+
+		conf.vif = &sdata->vif;
+		conf.type = sdata->vif.type;
+		conf.mac_addr = sdata->dev->dev_addr;
+		local->ops->remove_interface(hw, &conf);
+	}
+
+	/* stop hardware */
+	if (local->open_count) {
+		ieee80211_led_radio(local, false);
+		local->ops->stop(hw);
+	}
+
+	rtnl_unlock();
+}
+EXPORT_SYMBOL(__ieee80211_suspend);
+
+void __ieee80211_resume(struct ieee80211_hw *hw)
+{
+	struct ieee80211_local *local = hw_to_local(hw);
+	struct ieee80211_sub_if_data *sdata;
+	struct ieee80211_if_init_conf conf;
+	struct sta_info *sta;
+	int res;
+
+	printk(KERN_DEBUG "mac80211: resuming\n");
+	rtnl_lock();
+
+	/* restart hardware */
+	if (local->open_count) {
+		res = local->ops->start(hw);
+
+		ieee80211_led_radio(local, hw->conf.radio_enabled);
+	}
+
+	/* add interfaces */
+	list_for_each_entry(sdata, &local->interfaces, list) {
+		
+		conf.vif = &sdata->vif;
+		conf.type = sdata->vif.type;
+		conf.mac_addr = sdata->dev->dev_addr;
+		res = local->ops->add_interface(hw, &conf);
+
+		ieee80211_enable_keys(sdata);
+
+	}
+
+	/* notify STAs are back */
+	list_for_each_entry(sta, &local->sta_list, list) {
+
+		if (local->ops->sta_notify) {
+			if (sdata->vif.type == NL80211_IFTYPE_AP_VLAN)
+				sdata = container_of(sdata->bss,
+					     struct ieee80211_sub_if_data,
+					     u.ap);
+
+			local->ops->sta_notify(hw, &sdata->vif, 
+				STA_NOTIFY_ADD, &sta->sta);
+		}
+	}
+	rtnl_unlock();
+
+	/* setup RTS and frag thresholds */
+	if (local->ops->set_rts_threshold)
+		local->ops->set_rts_threshold(hw, local->rts_threshold);
+
+	if (local->ops->set_frag_threshold)
+		local->ops->set_frag_threshold(hw, 
+			local->fragmentation_threshold);
+
+	/* reconfigure hardware */
+	ieee80211_hw_config(local, ~0);
+
+	netif_addr_lock_bh(local->mdev);
+	ieee80211_configure_filter(local);
+	netif_addr_unlock_bh(local->mdev);
+}
+EXPORT_SYMBOL(__ieee80211_resume);
-- 
1.5.4.2.182.gb3092

Re: [PATCH 1/2] mac80211: add suspend/resume callbacks

[Johannes Berg]

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

On Sat, 2008-11-22 at 23:41 -0500, Bob Copeland wrote:
> This patch introduces suspend and resume callbacks to mac80211, allowing
> mac80211 to quiesce its state (bringing down interfaces, removing keys, etc)
> in preparation for suspend.  The driver is responsible for calling
> ieee80211_notify_mac with the appropriate option from its suspend/resume
> hook.

Thanks! Looks pretty good already, a few comments:

> +void __ieee80211_suspend(struct ieee80211_hw *hw)
> +{
> +	struct ieee80211_local *local = hw_to_local(hw);
> +	struct ieee80211_sub_if_data *sdata;
> +	struct ieee80211_if_init_conf conf;
> +	struct sta_info *sta;
> +
> +	printk(KERN_DEBUG "mac80211: suspending\n");
> +
> +	rtnl_lock();
> +
> +	flush_workqueue(local->hw.workqueue);
> +
> +	/* disable aggregation and notify that STAs are going away */

I think this should run through interfaces twice, in the first run to
remove keys, then remove stas and then in the second run remove the
interfaces, because otherwise you may be having keys left when the sta
for which the key was has been removed.

> +	/* remove all interfaces */
> +	list_for_each_entry(sdata, &local->interfaces, list) {
> +		ieee80211_disable_keys(sdata);

This needs to take care that the interface is actually up and not a
monitor/ap-vlan interface (those we don't tell the driver about)

> +	/* add interfaces */
> +	list_for_each_entry(sdata, &local->interfaces, list) {
> +		
> +		conf.vif = &sdata->vif;
> +		conf.type = sdata->vif.type;
> +		conf.mac_addr = sdata->dev->dev_addr;
> +		res = local->ops->add_interface(hw, &conf);
> +
> +		ieee80211_enable_keys(sdata);

Similarly here, make sure the interface is actually up and re-enable the
keys only after adding the stas back.

> +	/* setup RTS and frag thresholds */
> +	if (local->ops->set_rts_threshold)
> +		local->ops->set_rts_threshold(hw, local->rts_threshold);
> +
> +	if (local->ops->set_frag_threshold)
> +		local->ops->set_frag_threshold(hw, 
> +			local->fragmentation_threshold);

Heh, I didn't even think of those.

What were you thinking of when you said something is missing? I can't
think of anything right now. And what warnings do you get?

Resume definitely needs to defer to schedule_work(), I think, but that's
a trivial change.

johannes

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

Re: [PATCH 1/2] mac80211: add suspend/resume callbacks

[Bob Copeland]

On Sun, Nov 23, 2008 at 10:19:13AM +0100, Johannes Berg wrote:

Great, thanks for the feedback!

> > +	/* disable aggregation and notify that STAs are going away */
> 
> I think this should run through interfaces twice, in the first run to
> remove keys, then remove stas and then in the second run remove the
> interfaces, because otherwise you may be having keys left when the sta
> for which the key was has been removed.

Ok, that makes sense.

> > +	/* setup RTS and frag thresholds */
> > +	if (local->ops->set_rts_threshold)
> > +		local->ops->set_rts_threshold(hw, local->rts_threshold);
> > +
> > +	if (local->ops->set_frag_threshold)
> > +		local->ops->set_frag_threshold(hw, 
> > +			local->fragmentation_threshold);
> 
> Heh, I didn't even think of those.

They were in your original email on the website.  I'm not making up too 
much, yet :) 

> What were you thinking of when you said something is missing? I can't
> think of anything right now. And what warnings do you get?

conf_tx was the last callback from your email that I hadn't yet worked in.

Predictably, I got the netif_running() warning on ieee80211_enable_keys,
so hopefully the reordering and checking first will take care of that.

> Resume definitely needs to defer to schedule_work(), I think, but that's
> a trivial change.

Yeah that shouldn't be a problem.
 
-- 
Bob Copeland %% www.bobcopeland.com

Re: [PATCH 1/2] mac80211: add suspend/resume callbacks

[Bob Copeland]

On Sun, 23 Nov 2008 10:19:13 +0100, Johannes Berg wrote
> I think this should run through interfaces twice, in the first run to
> remove keys, then remove stas and then in the second run remove the
> interfaces, because otherwise you may be having keys left when the sta
> for which the key was has been removed.

Is this what you had in mind? (now on top of your cfg80211 patch, btw
it called ->suspend() in the resume hook)

I tested this with ath5k, it works in the sense that device suspends and
resumes without any hangs or errors.

However, netif_running(sdata->dev) seems to randomly be false for wlan0 on
the resume side so the interface doesn't always come up.  I haven't had
time to track that down but it's not consistent.

>From ec2f9fb1db1f98caf28e90f9838a42f0508a3a80 Mon Sep 17 00:00:00 2001
From: Bob Copeland <me@bobcopeland.com>
Date: Mon, 27 Oct 2008 09:17:46 -0400
Subject: [PATCH] mac80211: add suspend/resume callbacks

This patch introduces suspend and resume callbacks to mac80211,
allowing mac80211 to quiesce its state (bringing down interfaces,
removing keys, etc) in preparation for suspend.  cfg80211 will call
the suspend hook before the device suspend, and resume hook after
the device resume.

Signed-off-by: Bob Copeland <me@bobcopeland.com>
---
 net/mac80211/Makefile |    2 +
 net/mac80211/cfg.c    |   18 +++++++
 net/mac80211/pm.c     |  125 +++++++++++++++++++++++++++++++++++++++++++++++++
 net/mac80211/pm.h     |    9 ++++
 4 files changed, 154 insertions(+), 0 deletions(-)
 create mode 100644 net/mac80211/pm.c
 create mode 100644 net/mac80211/pm.h

diff --git a/net/mac80211/Makefile b/net/mac80211/Makefile
index 31cfd1f..0ecbc93 100644
--- a/net/mac80211/Makefile
+++ b/net/mac80211/Makefile
@@ -37,6 +37,8 @@ mac80211-$(CONFIG_MAC80211_MESH) += \
 	mesh_plink.o \
 	mesh_hwmp.o
 
+mac80211-$(CONFIG_PM) += pm.o
+
 # objects for PID algorithm
 rc80211_pid-y := rc80211_pid_algo.o
 rc80211_pid-$(CONFIG_MAC80211_DEBUGFS) += rc80211_pid_debugfs.o
diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c
index 7a7a6c1..5607045 100644
--- a/net/mac80211/cfg.c
+++ b/net/mac80211/cfg.c
@@ -16,6 +16,7 @@
 #include "cfg.h"
 #include "rate.h"
 #include "mesh.h"
+#include "pm.h"
 
 static bool nl80211_type_check(enum nl80211_iftype type)
 {
@@ -1107,6 +1108,21 @@ static int ieee80211_set_channel(struct wiphy *wiphy,
 	return ieee80211_hw_config(local, IEEE80211_CONF_CHANGE_CHANNEL);
 }
 
+#ifdef CONFIG_PM
+static int ieee80211_suspend(struct wiphy *wiphy)
+{
+	return __ieee80211_suspend(wiphy_priv(wiphy));
+}
+
+static int ieee80211_resume(struct wiphy *wiphy)
+{
+	return __ieee80211_resume(wiphy_priv(wiphy));
+}
+#else
+#define ieee80211_suspend NULL
+#define ieee80211_resume NULL
+#endif
+
 struct cfg80211_ops mac80211_config_ops = {
 	.add_virtual_intf = ieee80211_add_iface,
 	.del_virtual_intf = ieee80211_del_iface,
@@ -1135,4 +1151,6 @@ struct cfg80211_ops mac80211_config_ops = {
 	.change_bss = ieee80211_change_bss,
 	.set_txq_params = ieee80211_set_txq_params,
 	.set_channel = ieee80211_set_channel,
+	.suspend = ieee80211_suspend,
+	.resume = ieee80211_resume,
 };
diff --git a/net/mac80211/pm.c b/net/mac80211/pm.c
new file mode 100644
index 0000000..2fc96f9
--- /dev/null
+++ b/net/mac80211/pm.c
@@ -0,0 +1,125 @@
+#include <net/mac80211.h>
+#include <net/rtnetlink.h>
+
+#include "ieee80211_i.h"
+#include "pm.h"
+#include "led.h"
+
+int __ieee80211_suspend(struct ieee80211_hw *hw)
+{
+	struct ieee80211_local *local = hw_to_local(hw);
+	struct ieee80211_sub_if_data *sdata;
+	struct ieee80211_if_init_conf conf;
+	struct sta_info *sta;
+
+	printk(KERN_DEBUG "mac80211: suspending\n");
+
+	flush_workqueue(local->hw.workqueue);
+
+	/* disable keys */
+	list_for_each_entry(sdata, &local->interfaces, list)
+		ieee80211_disable_keys(sdata);
+
+	/* remove STAs */
+	list_for_each_entry(sta, &local->sta_list, list) {
+
+		/* disable aggregation */
+		ieee80211_sta_tear_down_BA_sessions(sdata, sta->sta.addr);
+
+		if (local->ops->sta_notify) {
+			if (sdata->vif.type == NL80211_IFTYPE_AP_VLAN)
+				sdata = container_of(sdata->bss,
+					     struct ieee80211_sub_if_data,
+					     u.ap);
+
+			local->ops->sta_notify(hw, &sdata->vif,
+				STA_NOTIFY_REMOVE, &sta->sta);
+		}
+	}
+
+	/* remove all interfaces */
+	list_for_each_entry(sdata, &local->interfaces, list) {
+
+		if (sdata->vif.type == NL80211_IFTYPE_AP_VLAN &&
+		    sdata->vif.type == NL80211_IFTYPE_MONITOR &&
+		    netif_running(sdata->dev)) {
+			conf.vif = &sdata->vif;
+			conf.type = sdata->vif.type;
+			conf.mac_addr = sdata->dev->dev_addr;
+			local->ops->remove_interface(hw, &conf);
+		}
+	}
+
+	/* stop hardware */
+	if (local->open_count) {
+		ieee80211_led_radio(local, false);
+		local->ops->stop(hw);
+	}
+	return 0;
+}
+
+int __ieee80211_resume(struct ieee80211_hw *hw)
+{
+	struct ieee80211_local *local = hw_to_local(hw);
+	struct ieee80211_sub_if_data *sdata;
+	struct ieee80211_if_init_conf conf;
+	struct sta_info *sta;
+	int res;
+
+	printk(KERN_DEBUG "mac80211: resuming\n");
+
+	/* restart hardware */
+	if (local->open_count) {
+		res = local->ops->start(hw);
+
+		ieee80211_led_radio(local, hw->conf.radio_enabled);
+	}
+
+	/* add interfaces */
+	list_for_each_entry(sdata, &local->interfaces, list) {
+
+		if (sdata->vif.type != NL80211_IFTYPE_AP_VLAN &&
+		    sdata->vif.type != NL80211_IFTYPE_MONITOR &&
+		    netif_running(sdata->dev)) {
+			conf.vif = &sdata->vif;
+			conf.type = sdata->vif.type;
+			conf.mac_addr = sdata->dev->dev_addr;
+			res = local->ops->add_interface(hw, &conf);
+		}
+	}
+
+	/* add STAs back */
+	list_for_each_entry(sta, &local->sta_list, list) {
+
+		if (local->ops->sta_notify) {
+			if (sdata->vif.type == NL80211_IFTYPE_AP_VLAN)
+				sdata = container_of(sdata->bss,
+					     struct ieee80211_sub_if_data,
+					     u.ap);
+
+			local->ops->sta_notify(hw, &sdata->vif,
+				STA_NOTIFY_ADD, &sta->sta);
+		}
+	}
+
+	/* add back keys */
+	list_for_each_entry(sdata, &local->interfaces, list)
+		if (netif_running(sdata->dev))
+			ieee80211_enable_keys(sdata);
+
+	/* setup RTS and frag thresholds */
+	if (local->ops->set_rts_threshold)
+		local->ops->set_rts_threshold(hw, local->rts_threshold);
+
+	if (local->ops->set_frag_threshold)
+		local->ops->set_frag_threshold(hw,
+			local->fragmentation_threshold);
+
+	/* reconfigure hardware */
+	ieee80211_hw_config(local, ~0);
+
+	netif_addr_lock_bh(local->mdev);
+	ieee80211_configure_filter(local);
+	netif_addr_unlock_bh(local->mdev);
+	return 0;
+}
diff --git a/net/mac80211/pm.h b/net/mac80211/pm.h
new file mode 100644
index 0000000..3d5ff02
--- /dev/null
+++ b/net/mac80211/pm.h
@@ -0,0 +1,9 @@
+#ifndef __MAC80211_PM_H
+#define __MAC80211_PM_H
+
+#ifdef CONFIG_PM
+int __ieee80211_suspend(struct ieee80211_hw *hw);
+int __ieee80211_resume(struct ieee80211_hw *hw);
+#endif
+
+#endif
-- 
1.6.0.4

-- 
Bob Copeland %% www.bobcopeland.com

Re: [PATCH 1/2] mac80211: add suspend/resume callbacks

[Johannes Berg]

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

On Tue, 2008-12-09 at 10:40 -0500, Bob Copeland wrote:
> On Sun, 23 Nov 2008 10:19:13 +0100, Johannes Berg wrote
> > I think this should run through interfaces twice, in the first run to
> > remove keys, then remove stas and then in the second run remove the
> > interfaces, because otherwise you may be having keys left when the sta
> > for which the key was has been removed.
> 
> Is this what you had in mind? (now on top of your cfg80211 patch, btw
> it called ->suspend() in the resume hook)

Can you adopt that patch too?

> I tested this with ath5k, it works in the sense that device suspends and
> resumes without any hangs or errors.
> 
> However, netif_running(sdata->dev) seems to randomly be false for wlan0 on
> the resume side so the interface doesn't always come up.  I haven't had
> time to track that down but it's not consistent.

I think it's just a bug below?

> +int __ieee80211_suspend(struct ieee80211_hw *hw)
> +{
> +	struct ieee80211_local *local = hw_to_local(hw);
> +	struct ieee80211_sub_if_data *sdata;
> +	struct ieee80211_if_init_conf conf;
> +	struct sta_info *sta;
> +
> +	printk(KERN_DEBUG "mac80211: suspending\n");
> +
> +	flush_workqueue(local->hw.workqueue);
> +
> +	/* disable keys */
> +	list_for_each_entry(sdata, &local->interfaces, list)
> +		ieee80211_disable_keys(sdata);
> +
> +	/* remove STAs */
> +	list_for_each_entry(sta, &local->sta_list, list) {
> +
> +		/* disable aggregation */
> +		ieee80211_sta_tear_down_BA_sessions(sdata, sta->sta.addr);
> +
> +		if (local->ops->sta_notify) {
> +			if (sdata->vif.type == NL80211_IFTYPE_AP_VLAN)
> +				sdata = container_of(sdata->bss,
> +					     struct ieee80211_sub_if_data,
> +					     u.ap);
> +
> +			local->ops->sta_notify(hw, &sdata->vif,
> +				STA_NOTIFY_REMOVE, &sta->sta);
> +		}
> +	}

That looks pretty good.

> +	/* remove all interfaces */
> +	list_for_each_entry(sdata, &local->interfaces, list) {
> +
> +		if (sdata->vif.type == NL80211_IFTYPE_AP_VLAN &&
> +		    sdata->vif.type == NL80211_IFTYPE_MONITOR &&
> +		    netif_running(sdata->dev)) {

But shouldn't that be != _AP_VLAN && != _MONITOR && running?

> +	/* add interfaces */
> +	list_for_each_entry(sdata, &local->interfaces, list) {
> +
> +		if (sdata->vif.type != NL80211_IFTYPE_AP_VLAN &&
> +		    sdata->vif.type != NL80211_IFTYPE_MONITOR &&
> +		    netif_running(sdata->dev)) {

Like here

> diff --git a/net/mac80211/pm.h b/net/mac80211/pm.h
> new file mode 100644
> index 0000000..3d5ff02
> --- /dev/null
> +++ b/net/mac80211/pm.h
> @@ -0,0 +1,9 @@
> +#ifndef __MAC80211_PM_H
> +#define __MAC80211_PM_H
> +
> +#ifdef CONFIG_PM
> +int __ieee80211_suspend(struct ieee80211_hw *hw);
> +int __ieee80211_resume(struct ieee80211_hw *hw);
> +#endif

I'd just add those to ieee80211_i.h, and I don't think you really need
to protect them with #ifdef CONFIG_PM

johannes

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

Re: [PATCH 1/2] mac80211: add suspend/resume callbacks

[Bob Copeland]

On Tue, 09 Dec 2008 16:59:06 +0100, Johannes Berg wrote
> > Is this what you had in mind? (now on top of your cfg80211 patch, btw
> > it called ->suspend() in the resume hook)
> 
> Can you adopt that patch too?

Yeah, I already have it in my tree, I just didn't repost it because my 
webmail sucks.

> > +	/* remove all interfaces */
> > +	list_for_each_entry(sdata, &local->interfaces, list) {
> > +
> > +		if (sdata->vif.type == NL80211_IFTYPE_AP_VLAN &&
> > +		    sdata->vif.type == NL80211_IFTYPE_MONITOR &&
> > +		    netif_running(sdata->dev)) {
> 
> But shouldn't that be != _AP_VLAN && != _MONITOR && running?

Oops, of course it should; that if() started life as continue.  
But that would just mean a missed call to remove_interface()... 
in the driver, which for ath5k just clears the mac, so I don't 
know why it would affect the running state of the interface.

> I'd just add those to ieee80211_i.h, and I don't think you really need
> to protect them with #ifdef CONFIG_PM

Will do, thanks for the review.

-- 
Bob Copeland %% www.bobcopeland.com

Re: [PATCH 1/2] mac80211: add suspend/resume callbacks

[Johannes Berg]

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

On Tue, 2008-12-09 at 11:29 -0500, Bob Copeland wrote:

> > > +		if (sdata->vif.type == NL80211_IFTYPE_AP_VLAN &&
> > > +		    sdata->vif.type == NL80211_IFTYPE_MONITOR &&
> > > +		    netif_running(sdata->dev)) {
> > 
> > But shouldn't that be != _AP_VLAN && != _MONITOR && running?
> 
> Oops, of course it should; that if() started life as continue.  
> But that would just mean a missed call to remove_interface()... 
> in the driver, which for ath5k just clears the mac, so I don't 
> know why it would affect the running state of the interface.

Ok, no, it shouldn't affect the running state.

johannes

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