* [RFC 1/9] cfg80211: add WoW support
2011-03-01 20:36 [RFC 0/9] add WoW support Eliad Peller
@ 2011-03-01 20:36 ` Eliad Peller
2011-03-08 14:09 ` Johannes Berg
2011-03-01 20:36 ` [RFC 2/9] mac80211: add WoW param to suspend/resume functions Eliad Peller
` (10 subsequent siblings)
11 siblings, 1 reply; 21+ messages in thread
From: Eliad Peller @ 2011-03-01 20:36 UTC (permalink / raw)
To: Johannes Berg; +Cc: linux-wireless
From: Luis R. Rodriguez <lrodriguez@atheros.com>
Add initial support for wake-on-wireless.
Let the user enable specific WoW triggers, and pass them
while calling suspend/resume.
Signed-off-by: Luis R. Rodriguez <lrodriguez@atheros.com>
Signed-off-by: Eliad Peller <eliad@wizery.com>
---
include/linux/nl80211.h | 42 +++++++++++++++++++++++++
include/net/cfg80211.h | 31 +++++++++++++++++--
net/wireless/core.h | 3 ++
net/wireless/nl80211.c | 78 +++++++++++++++++++++++++++++++++++++++++++++++
net/wireless/sysfs.c | 4 +-
5 files changed, 153 insertions(+), 5 deletions(-)
diff --git a/include/linux/nl80211.h b/include/linux/nl80211.h
index 821ffb9..44f75f0 100644
--- a/include/linux/nl80211.h
+++ b/include/linux/nl80211.h
@@ -410,6 +410,18 @@
* notification. This event is used to indicate that an unprotected
* disassociation frame was dropped when MFP is in use.
*
+ * @NL80211_CMD_GET_WOW: get Wake-on-Wireless-LAN (WoW) settings.
+ * @NL80211_CMD_SET_WOW: set Wake-on-Wireless-LAN (Wow) settings. Wake on
+ * wireless makes use of standard Wake-on-LAN (WoL) frames, you receive
+ * a WoW frame when your AP sends you a regular WOL frame. The difference
+ * is you need to be associated to an AP in order to receive WoW frames,
+ * so additional triggers are available for a wakeup.
+ * A driver capable of WoW should initialize the wiphy with its supported
+ * WoW triggers. Upon suspend cfg80211 will inform the driver of the user
+ * enabled triggers. By default no WoW triggers are enabled.
+ * For more information see:
+ * http://wireless.kernel.org/en/users/Documentation/WoW
+ *
* @NL80211_CMD_MAX: highest used command number
* @__NL80211_CMD_AFTER_LAST: internal use
*/
@@ -522,6 +534,9 @@ enum nl80211_commands {
NL80211_CMD_UNPROT_DEAUTHENTICATE,
NL80211_CMD_UNPROT_DISASSOCIATE,
+ NL80211_CMD_GET_WOW,
+ NL80211_CMD_SET_WOW,
+
/* add new commands above here */
/* used to define NL80211_CMD_MAX below */
@@ -541,6 +556,8 @@ enum nl80211_commands {
#define NL80211_CMD_DEAUTHENTICATE NL80211_CMD_DEAUTHENTICATE
#define NL80211_CMD_DISASSOCIATE NL80211_CMD_DISASSOCIATE
#define NL80211_CMD_REG_BEACON_HINT NL80211_CMD_REG_BEACON_HINT
+#define NL80211_CMD_GET_WOW NL80211_CMD_GET_WOW
+#define NL80211_CMD_SET_WOW NL80211_CMD_SET_WOW
/* source-level API compatibility */
#define NL80211_CMD_GET_MESH_PARAMS NL80211_CMD_GET_MESH_CONFIG
@@ -887,6 +904,12 @@ enum nl80211_commands {
* @NL80211_ATTR_MESH_CONFIG: Mesh configuration parameters, a nested attribute
* containing attributes from &enum nl80211_meshconf_params.
*
+ * @NL80211_ATTR_WOW_TRIGGERS_SUPPORTED: the supported WoW triggers
+ * @NL80211_ATTR_WOW_TRIGGERS_ENABLED: used by %NL80211_CMD_SET_WOW to
+ * indicate which WoW triggers should be enabled. This is also
+ * used by %NL80211_CMD_GET_WOW to get the currently enabled WoW
+ * triggers.
+ *
* @NL80211_ATTR_MAX: highest attribute number currently defined
* @__NL80211_ATTR_AFTER_LAST: internal use
*/
@@ -1074,6 +1097,9 @@ enum nl80211_attrs {
NL80211_ATTR_WIPHY_ANTENNA_AVAIL_TX,
NL80211_ATTR_WIPHY_ANTENNA_AVAIL_RX,
+ NL80211_ATTR_WOW_TRIGGERS_SUPPORTED,
+ NL80211_ATTR_WOW_TRIGGERS_ENABLED,
+
/* add attributes here, update the policy in nl80211.c */
__NL80211_ATTR_AFTER_LAST,
@@ -1999,4 +2025,20 @@ enum nl80211_tx_power_setting {
NL80211_TX_POWER_FIXED,
};
+/**
+ * enum nl80211_wow_triggers - Wake-on-Wireless-LAN triggers
+ *
+ * @NL80211_WOW_TRIGGER_MAGIC_PACKET: wake up when a magic packet is received.
+ * @NL80211_WOW_TRIGGER_BMISS: wake up on a beacon loss.
+ * @NL80211_WOW_TRIGGER_ANYTHING: wake up by any irq. in this mode there is no
+ * need for any special hw support but staying up while the host is being
+ * suspended. however, any hw capability might be used in order to avoid
+ * unnecessary wake-ups (e.g. beacon-filtering, arp-filtering, etc.)
+ */
+enum nl80211_wow_triggers {
+ NL80211_WOW_TRIGGER_MAGIC_PACKET = 1 << 0,
+ NL80211_WOW_TRIGGER_BMISS = 1 << 1,
+ NL80211_WOW_TRIGGER_ANYTHING = 1 << 2,
+};
+
#endif /* __LINUX_NL80211_H */
diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
index 679a049..533eb18 100644
--- a/include/net/cfg80211.h
+++ b/include/net/cfg80211.h
@@ -1043,6 +1043,20 @@ struct cfg80211_pmksa {
u8 *pmkid;
};
+/*
+ * struct cfg80211_wow - Wake on Wireless-LAN support info
+ *
+ * This structure defines the enabled WoW triggers for the device.
+ * For now we only carry the enabled triggers but this is expected to grow
+ * once we add support for user patterns.
+ *
+ * @enabled_triggers: bitmap of enabled triggers (none by default).
+ * The flags for this bitmask are %NL80211_WOW_TRIGGER_*.
+ */
+struct cfg80211_wow {
+ u32 enabled_triggers;
+};
+
/**
* struct cfg80211_ops - backend description for wireless configuration
*
@@ -1056,8 +1070,13 @@ struct cfg80211_pmksa {
* wireless extensions but this is subject to reevaluation as soon as this
* code is used more widely and we have a first user without wext.
*
- * @suspend: wiphy device needs to be suspended
+ * @suspend: wiphy device needs to be suspended.
+ * @wow will contain the enabled Wake-on-Wireless triggers that should
+ * be configured to the device.
* @resume: wiphy device needs to be resumed
+ * @wow will contain the enabled Wake-on-Wireless triggers that were
+ * configured to the device (it does not tell what was the actual
+ * wake-up reason).
*
* @add_virtual_intf: create a new virtual interface with the given name,
* must set the struct wireless_dev's iftype. Beware: You must create
@@ -1196,8 +1215,8 @@ struct cfg80211_pmksa {
* @get_antenna: Get current antenna configuration from device (tx_ant, rx_ant).
*/
struct cfg80211_ops {
- int (*suspend)(struct wiphy *wiphy);
- int (*resume)(struct wiphy *wiphy);
+ int (*suspend)(struct wiphy *wiphy, struct cfg80211_wow *wow);
+ int (*resume)(struct wiphy *wiphy, struct cfg80211_wow *wow);
struct net_device * (*add_virtual_intf)(struct wiphy *wiphy,
char *name,
@@ -1494,6 +1513,10 @@ struct ieee80211_txrx_stypes {
*
* @max_remain_on_channel_duration: Maximum time a remain-on-channel operation
* may request, if implemented.
+ * @supported_wow_triggers: bitmap of supported Wake-on-Wireless triggers.
+ * The flags for this bitmask are %NL80211_WOW_TRIGGER_*. The driver
+ * should set the capabilities before registering the wiphy. WoW triggers
+ * which should be used are passed to the driver upon suspend.
*/
struct wiphy {
/* assign these fields before you register the wiphy */
@@ -1538,6 +1561,8 @@ struct wiphy {
u32 available_antennas_tx;
u32 available_antennas_rx;
+ u32 supported_wow_triggers;
+
/* If multiple wiphys are registered and you're handed e.g.
* a regular netdev with assigned ieee80211_ptr, you won't
* know whether it points to a wiphy your driver has registered
diff --git a/net/wireless/core.h b/net/wireless/core.h
index 26a0a08..aa63760 100644
--- a/net/wireless/core.h
+++ b/net/wireless/core.h
@@ -70,6 +70,9 @@ struct cfg80211_registered_device {
struct work_struct conn_work;
struct work_struct event_work;
+ /* enabled WoW triggers (user configurable)*/
+ struct cfg80211_wow wow;
+
/* must be last because of the way we do wiphy_priv(),
* and it should at least be aligned to NETDEV_ALIGN */
struct wiphy wiphy __attribute__((__aligned__(NETDEV_ALIGN)));
diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index 864ddfb..eea7aae 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -172,6 +172,9 @@ static const struct nla_policy nl80211_policy[NL80211_ATTR_MAX+1] = {
[NL80211_ATTR_MCAST_RATE] = { .type = NLA_U32 },
[NL80211_ATTR_OFFCHANNEL_TX_OK] = { .type = NLA_FLAG },
[NL80211_ATTR_KEY_DEFAULT_TYPES] = { .type = NLA_NESTED },
+
+ [NL80211_ATTR_WOW_TRIGGERS_SUPPORTED] = { .type = NLA_U32 },
+ [NL80211_ATTR_WOW_TRIGGERS_ENABLED] = { .type = NLA_U32 },
};
/* policy for the key attributes */
@@ -4762,6 +4765,64 @@ static int nl80211_leave_mesh(struct sk_buff *skb, struct genl_info *info)
return cfg80211_leave_mesh(rdev, dev);
}
+static int nl80211_get_wow(struct sk_buff *skb, struct genl_info *info)
+{
+ struct cfg80211_registered_device *rdev = info->user_ptr[0];
+ int err;
+ void *hdr;
+ struct sk_buff *msg;
+
+ msg = nlmsg_new(NLMSG_DEFAULT_SIZE, GFP_KERNEL);
+ if (!msg)
+ return -ENOMEM;
+
+ hdr = nl80211hdr_put(msg, info->snd_pid, info->snd_seq, 0,
+ NL80211_CMD_GET_WOW);
+ if (!hdr) {
+ err = -ENOBUFS;
+ goto free_msg;
+ }
+
+ NLA_PUT_U32(msg, NL80211_ATTR_WOW_TRIGGERS_SUPPORTED,
+ rdev->wiphy.supported_wow_triggers);
+ NLA_PUT_U32(msg, NL80211_ATTR_WOW_TRIGGERS_ENABLED,
+ rdev->wow.enabled_triggers);
+
+ genlmsg_end(msg, hdr);
+ return genlmsg_reply(msg, info);
+
+nla_put_failure:
+ err = -ENOBUFS;
+free_msg:
+ nlmsg_free(msg);
+ return err;
+}
+
+static int nl80211_set_wow(struct sk_buff *skb, struct genl_info *info)
+{
+ struct cfg80211_registered_device *rdev = info->user_ptr[0];
+ u32 requested_trig, supported_trig;
+
+ if (!info->attrs[NL80211_ATTR_WOW_TRIGGERS_ENABLED])
+ return -EINVAL;
+
+ requested_trig =
+ nla_get_u32(info->attrs[NL80211_ATTR_WOW_TRIGGERS_ENABLED]);
+ supported_trig = rdev->wiphy.supported_wow_triggers;
+
+ /* check for unsupported triggers */
+ if ((requested_trig & supported_trig) != requested_trig)
+ return -EOPNOTSUPP;
+
+ /*
+ * Apply changes.
+ * This information gets passed to the drivers during suspend.
+ */
+ rdev->wow.enabled_triggers = requested_trig;
+
+ return 0;
+}
+
#define NL80211_FLAG_NEED_WIPHY 0x01
#define NL80211_FLAG_NEED_NETDEV 0x02
#define NL80211_FLAG_NEED_RTNL 0x04
@@ -5260,6 +5321,23 @@ static struct genl_ops nl80211_ops[] = {
.internal_flags = NL80211_FLAG_NEED_NETDEV_UP |
NL80211_FLAG_NEED_RTNL,
},
+ {
+ .cmd = NL80211_CMD_GET_WOW,
+ .doit = nl80211_get_wow,
+ .policy = nl80211_policy,
+ /* can be retrieved by unprivileged users */
+ .internal_flags = NL80211_FLAG_NEED_WIPHY |
+ NL80211_FLAG_NEED_RTNL,
+ },
+ {
+ .cmd = NL80211_CMD_SET_WOW,
+ .doit = nl80211_set_wow,
+ .policy = nl80211_policy,
+ .flags = GENL_ADMIN_PERM,
+ .internal_flags = NL80211_FLAG_NEED_WIPHY |
+ NL80211_FLAG_NEED_RTNL,
+ },
+
};
static struct genl_multicast_group nl80211_mlme_mcgrp = {
diff --git a/net/wireless/sysfs.c b/net/wireless/sysfs.c
index 4294fa2..d2fb411 100644
--- a/net/wireless/sysfs.c
+++ b/net/wireless/sysfs.c
@@ -93,7 +93,7 @@ static int wiphy_suspend(struct device *dev, pm_message_t state)
if (rdev->ops->suspend) {
rtnl_lock();
- ret = rdev->ops->suspend(&rdev->wiphy);
+ ret = rdev->ops->suspend(&rdev->wiphy, &rdev->wow);
rtnl_unlock();
}
@@ -112,7 +112,7 @@ static int wiphy_resume(struct device *dev)
if (rdev->ops->resume) {
rtnl_lock();
- ret = rdev->ops->resume(&rdev->wiphy);
+ ret = rdev->ops->resume(&rdev->wiphy, &rdev->wow);
rtnl_unlock();
}
--
1.7.0.4
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [RFC 1/9] cfg80211: add WoW support
2011-03-01 20:36 ` [RFC 1/9] cfg80211: " Eliad Peller
@ 2011-03-08 14:09 ` Johannes Berg
0 siblings, 0 replies; 21+ messages in thread
From: Johannes Berg @ 2011-03-08 14:09 UTC (permalink / raw)
To: Eliad Peller; +Cc: linux-wireless
On Tue, 2011-03-01 at 22:36 +0200, Eliad Peller wrote:
> + * @NL80211_CMD_GET_WOW: get Wake-on-Wireless-LAN (WoW) settings.
> + * @NL80211_CMD_SET_WOW: set Wake-on-Wireless-LAN (Wow) settings. Wake on
> + * wireless makes use of standard Wake-on-LAN (WoL) frames, you receive
> + * a WoW frame when your AP sends you a regular WOL frame. The difference
> + * is you need to be associated to an AP in order to receive WoW frames,
> + * so additional triggers are available for a wakeup.
> + * A driver capable of WoW should initialize the wiphy with its supported
> + * WoW triggers. Upon suspend cfg80211 will inform the driver of the user
> + * enabled triggers. By default no WoW triggers are enabled.
> + * For more information see:
> + * http://wireless.kernel.org/en/users/Documentation/WoW
I wonder if we should restrict setting some WOW triggers to when we're
associated?
> + * @NL80211_ATTR_WOW_TRIGGERS_SUPPORTED: the supported WoW triggers
> + * @NL80211_ATTR_WOW_TRIGGERS_ENABLED: used by %NL80211_CMD_SET_WOW to
> + * indicate which WoW triggers should be enabled. This is also
> + * used by %NL80211_CMD_GET_WOW to get the currently enabled WoW
> + * triggers.
These are u32 now (I think?), but I think they should be nested
attributes to allow nesting more data like pattern matching etc. without
using more global attributes.
> +/**
> + * enum nl80211_wow_triggers - Wake-on-Wireless-LAN triggers
> + *
> + * @NL80211_WOW_TRIGGER_MAGIC_PACKET: wake up when a magic packet is received.
> + * @NL80211_WOW_TRIGGER_BMISS: wake up on a beacon loss.
> + * @NL80211_WOW_TRIGGER_ANYTHING: wake up by any irq. in this mode there is no
> + * need for any special hw support but staying up while the host is being
> + * suspended. however, any hw capability might be used in order to avoid
> + * unnecessary wake-ups (e.g. beacon-filtering, arp-filtering, etc.)
> + */
> +enum nl80211_wow_triggers {
> + NL80211_WOW_TRIGGER_MAGIC_PACKET = 1 << 0,
> + NL80211_WOW_TRIGGER_BMISS = 1 << 1,
> + NL80211_WOW_TRIGGER_ANYTHING = 1 << 2,
> +};
They can be nla flags in the nested struct then.
> - * @suspend: wiphy device needs to be suspended
> + * @suspend: wiphy device needs to be suspended.
> + * @wow will contain the enabled Wake-on-Wireless triggers that should
> + * be configured to the device.
> * @resume: wiphy device needs to be resumed
> + * @wow will contain the enabled Wake-on-Wireless triggers that were
> + * configured to the device (it does not tell what was the actual
> + * wake-up reason).
Why does resume need to get the triggers? That seems pointless? If the
driver really needs them it can just store them somewhere? Also, I think
we should somehow allow the driver to give us information about why it
woke up, if it was actually the cause for the system wakeup. That could
be valuable input for a suspend daemon, I think?
> +static int nl80211_set_wow(struct sk_buff *skb, struct genl_info *info)
> +{
> + struct cfg80211_registered_device *rdev = info->user_ptr[0];
> + u32 requested_trig, supported_trig;
> +
> + if (!info->attrs[NL80211_ATTR_WOW_TRIGGERS_ENABLED])
> + return -EINVAL;
> +
> + requested_trig =
> + nla_get_u32(info->attrs[NL80211_ATTR_WOW_TRIGGERS_ENABLED]);
> + supported_trig = rdev->wiphy.supported_wow_triggers;
> +
> + /* check for unsupported triggers */
> + if ((requested_trig & supported_trig) != requested_trig)
> + return -EOPNOTSUPP;
> +
> + /*
> + * Apply changes.
> + * This information gets passed to the drivers during suspend.
> + */
> + rdev->wow.enabled_triggers = requested_trig;
I think it might be worthwhile to allow drivers to validate the settings
here? Otherwise, we have to have absolutely perfect advertising. Magic
packet for instance can also require match patterns in some cases, as
far as I've seen. Alternatively, how about we just leave all of this
complexity out for a first draft -- wl12xx won't support it anyway.
> @@ -93,7 +93,7 @@ static int wiphy_suspend(struct device *dev, pm_message_t state)
>
> if (rdev->ops->suspend) {
> rtnl_lock();
> - ret = rdev->ops->suspend(&rdev->wiphy);
> + ret = rdev->ops->suspend(&rdev->wiphy, &rdev->wow);
> rtnl_unlock();
> }
>
> @@ -112,7 +112,7 @@ static int wiphy_resume(struct device *dev)
>
> if (rdev->ops->resume) {
> rtnl_lock();
> - ret = rdev->ops->resume(&rdev->wiphy);
> + ret = rdev->ops->resume(&rdev->wiphy, &rdev->wow);
Should we pass NULL if no triggers were configured? That makes for an
easy check in the driver. Alternatively, there's no reason to be passing
the wow struct at all -- we could just store it in the wiphy instead of
here and the driver will have access.
One thing I'm a bit worried about is this with multiple interfaces, and
interface dependent triggers. iwlagn, for example, will only support a
single connection (as far as I know right now), and wake up when the
connection is lost. This wakeup event might not even be configurable. Do
we therefore need something like "wow profiles" that contain a set of
wakeup triggers etc?
johannes
^ permalink raw reply [flat|nested] 21+ messages in thread
* [RFC 2/9] mac80211: add WoW param to suspend/resume functions
2011-03-01 20:36 [RFC 0/9] add WoW support Eliad Peller
2011-03-01 20:36 ` [RFC 1/9] cfg80211: " Eliad Peller
@ 2011-03-01 20:36 ` Eliad Peller
2011-03-08 14:10 ` Johannes Berg
2011-03-01 20:36 ` [RFC 3/9] mac80211: add WoW param to .start/.stop callbacks Eliad Peller
` (9 subsequent siblings)
11 siblings, 1 reply; 21+ messages in thread
From: Eliad Peller @ 2011-03-01 20:36 UTC (permalink / raw)
To: Johannes Berg; +Cc: linux-wireless
Upon suspend/resume cfg80211 passes additional WoW param,
which contains the enabled Wake-On-Wireless triggers.
Add this param to the related functions (ieee80211_suspend,
ieee80211_resume, ieee80211_reconfig, ieee80211_stop_driver),
and pass NULL when calling these functions by ourselves
(e.g. on restart work)
Signed-off-by: Luis R. Rodriguez <lrodriguez@atheros.com>
Signed-off-by: Eliad Peller <eliad@wizery.com>
---
net/mac80211/cfg.c | 8 ++++----
net/mac80211/debugfs.c | 4 ++--
net/mac80211/ieee80211_i.h | 20 +++++++++++++-------
net/mac80211/iface.c | 2 +-
net/mac80211/main.c | 2 +-
net/mac80211/pm.c | 5 +++--
net/mac80211/util.c | 6 ++++--
7 files changed, 28 insertions(+), 19 deletions(-)
diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c
index 140503d..d3476ad 100644
--- a/net/mac80211/cfg.c
+++ b/net/mac80211/cfg.c
@@ -1258,14 +1258,14 @@ static int ieee80211_set_channel(struct wiphy *wiphy,
}
#ifdef CONFIG_PM
-static int ieee80211_suspend(struct wiphy *wiphy)
+static int ieee80211_suspend(struct wiphy *wiphy, struct cfg80211_wow *wow)
{
- return __ieee80211_suspend(wiphy_priv(wiphy));
+ return __ieee80211_suspend(wiphy_priv(wiphy), wow);
}
-static int ieee80211_resume(struct wiphy *wiphy)
+static int ieee80211_resume(struct wiphy *wiphy, struct cfg80211_wow *wow)
{
- return __ieee80211_resume(wiphy_priv(wiphy));
+ return __ieee80211_resume(wiphy_priv(wiphy), wow);
}
#else
#define ieee80211_suspend NULL
diff --git a/net/mac80211/debugfs.c b/net/mac80211/debugfs.c
index 51f0d78..bf0dd8c 100644
--- a/net/mac80211/debugfs.c
+++ b/net/mac80211/debugfs.c
@@ -130,8 +130,8 @@ static ssize_t reset_write(struct file *file, const char __user *user_buf,
struct ieee80211_local *local = file->private_data;
rtnl_lock();
- __ieee80211_suspend(&local->hw);
- __ieee80211_resume(&local->hw);
+ __ieee80211_suspend(&local->hw, NULL);
+ __ieee80211_resume(&local->hw, NULL);
rtnl_unlock();
return count;
diff --git a/net/mac80211/ieee80211_i.h b/net/mac80211/ieee80211_i.h
index 0a570a1..aeced4a 100644
--- a/net/mac80211/ieee80211_i.h
+++ b/net/mac80211/ieee80211_i.h
@@ -1241,13 +1241,17 @@ void ieee80211_process_measurement_req(struct ieee80211_sub_if_data *sdata,
size_t len);
/* Suspend/resume and hw reconfiguration */
-int ieee80211_reconfig(struct ieee80211_local *local);
-void ieee80211_stop_device(struct ieee80211_local *local);
+int ieee80211_reconfig(struct ieee80211_local *local,
+ struct cfg80211_wow *wow);
+void ieee80211_stop_device(struct ieee80211_local *local,
+ struct cfg80211_wow *wow);
#ifdef CONFIG_PM
-int __ieee80211_suspend(struct ieee80211_hw *hw);
+int __ieee80211_suspend(struct ieee80211_hw *hw,
+ struct cfg80211_wow *wow);
-static inline int __ieee80211_resume(struct ieee80211_hw *hw)
+static inline int __ieee80211_resume(struct ieee80211_hw *hw,
+ struct cfg80211_wow *wow)
{
struct ieee80211_local *local = hw_to_local(hw);
@@ -1255,15 +1259,17 @@ static inline int __ieee80211_resume(struct ieee80211_hw *hw)
"%s: resume with hardware scan still in progress\n",
wiphy_name(hw->wiphy));
- return ieee80211_reconfig(hw_to_local(hw));
+ return ieee80211_reconfig(hw_to_local(hw), wow);
}
#else
-static inline int __ieee80211_suspend(struct ieee80211_hw *hw)
+static inline int __ieee80211_suspend(struct ieee80211_hw *hw,
+ struct cfg80211_wow *wow)
{
return 0;
}
-static inline int __ieee80211_resume(struct ieee80211_hw *hw)
+static inline int __ieee80211_resume(struct ieee80211_hw *hw,
+ struct cfg80211_wow *wow)
{
return 0;
}
diff --git a/net/mac80211/iface.c b/net/mac80211/iface.c
index 5a4e19b..a0e3779 100644
--- a/net/mac80211/iface.c
+++ b/net/mac80211/iface.c
@@ -537,7 +537,7 @@ static void ieee80211_do_stop(struct ieee80211_sub_if_data *sdata,
if (local->ops->napi_poll)
napi_disable(&local->napi);
ieee80211_clear_tx_pending(local);
- ieee80211_stop_device(local);
+ ieee80211_stop_device(local, NULL);
/* no reconfiguring after stop! */
hw_reconf_flags = 0;
diff --git a/net/mac80211/main.c b/net/mac80211/main.c
index 2543e48..7c597bb 100644
--- a/net/mac80211/main.c
+++ b/net/mac80211/main.c
@@ -370,7 +370,7 @@ static void ieee80211_restart_work(struct work_struct *work)
rtnl_lock();
ieee80211_scan_cancel(local);
- ieee80211_reconfig(local);
+ ieee80211_reconfig(local, NULL);
rtnl_unlock();
}
diff --git a/net/mac80211/pm.c b/net/mac80211/pm.c
index e373551..6d135ea 100644
--- a/net/mac80211/pm.c
+++ b/net/mac80211/pm.c
@@ -6,7 +6,8 @@
#include "driver-ops.h"
#include "led.h"
-int __ieee80211_suspend(struct ieee80211_hw *hw)
+int __ieee80211_suspend(struct ieee80211_hw *hw,
+ struct cfg80211_wow *wow)
{
struct ieee80211_local *local = hw_to_local(hw);
struct ieee80211_sub_if_data *sdata;
@@ -96,7 +97,7 @@ int __ieee80211_suspend(struct ieee80211_hw *hw)
/* stop hardware - this must stop RX */
if (local->open_count)
- ieee80211_stop_device(local);
+ ieee80211_stop_device(local, wow);
local->suspended = true;
/* need suspended to be visible before quiescing is false */
diff --git a/net/mac80211/util.c b/net/mac80211/util.c
index 556647a..2058965 100644
--- a/net/mac80211/util.c
+++ b/net/mac80211/util.c
@@ -1107,7 +1107,8 @@ u32 ieee80211_sta_get_rates(struct ieee80211_local *local,
return supp_rates;
}
-void ieee80211_stop_device(struct ieee80211_local *local)
+void ieee80211_stop_device(struct ieee80211_local *local,
+ struct cfg80211_wow *wow)
{
ieee80211_led_radio(local, false);
ieee80211_mod_tpt_led_trig(local, 0, IEEE80211_TPT_LEDTRIG_FL_RADIO);
@@ -1118,7 +1119,8 @@ void ieee80211_stop_device(struct ieee80211_local *local)
drv_stop(local);
}
-int ieee80211_reconfig(struct ieee80211_local *local)
+int ieee80211_reconfig(struct ieee80211_local *local,
+ struct cfg80211_wow *wow)
{
struct ieee80211_hw *hw = &local->hw;
struct ieee80211_sub_if_data *sdata;
--
1.7.0.4
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [RFC 2/9] mac80211: add WoW param to suspend/resume functions
2011-03-01 20:36 ` [RFC 2/9] mac80211: add WoW param to suspend/resume functions Eliad Peller
@ 2011-03-08 14:10 ` Johannes Berg
0 siblings, 0 replies; 21+ messages in thread
From: Johannes Berg @ 2011-03-08 14:10 UTC (permalink / raw)
To: Eliad Peller; +Cc: linux-wireless
On Tue, 2011-03-01 at 22:36 +0200, Eliad Peller wrote:
> Upon suspend/resume cfg80211 passes additional WoW param,
> which contains the enabled Wake-On-Wireless triggers.
>
> Add this param to the related functions (ieee80211_suspend,
> ieee80211_resume, ieee80211_reconfig, ieee80211_stop_driver),
> and pass NULL when calling these functions by ourselves
> (e.g. on restart work)
Same comment/question here for passing to suspend and passing NULL --
but it's just pass-through from cfg80211.
johannes
^ permalink raw reply [flat|nested] 21+ messages in thread
* [RFC 3/9] mac80211: add WoW param to .start/.stop callbacks
2011-03-01 20:36 [RFC 0/9] add WoW support Eliad Peller
2011-03-01 20:36 ` [RFC 1/9] cfg80211: " Eliad Peller
2011-03-01 20:36 ` [RFC 2/9] mac80211: add WoW param to suspend/resume functions Eliad Peller
@ 2011-03-01 20:36 ` Eliad Peller
2011-03-08 14:12 ` Johannes Berg
2011-03-01 20:36 ` [RFC 4/9] mac80211: don't remove/add interfaces when WoW is enabled Eliad Peller
` (8 subsequent siblings)
11 siblings, 1 reply; 21+ messages in thread
From: Eliad Peller @ 2011-03-01 20:36 UTC (permalink / raw)
To: Johannes Berg; +Cc: linux-wireless
Add a param which contains the enabled Wake-On-Wireless triggers,
so lower drivers prepare themselves to suspend/resume
(e.g. configure triggers to fw, mask irqs, etc.)
NOTE: this patch requires changing all the mac80211-based drivers.
currently, as this is a preliminary version, it contains only patch
for wl12xx (on which the developmont was done).
Signed-off-by: Luis R. Rodriguez <lrodriguez@atheros.com>
Signed-off-by: Eliad Peller <eliad@wizery.com>
---
drivers/net/wireless/wl12xx/main.c | 6 +++-
include/net/mac80211.h | 19 ++++++++++----
net/mac80211/driver-ops.h | 14 ++++++----
net/mac80211/driver-trace.h | 47 +++++++++++++++++++++++++++++++----
net/mac80211/iface.c | 6 ++--
net/mac80211/util.c | 4 +-
6 files changed, 71 insertions(+), 25 deletions(-)
diff --git a/drivers/net/wireless/wl12xx/main.c b/drivers/net/wireless/wl12xx/main.c
index 95aa19a..3a7d788 100644
--- a/drivers/net/wireless/wl12xx/main.c
+++ b/drivers/net/wireless/wl12xx/main.c
@@ -1081,7 +1081,8 @@ static struct notifier_block wl1271_dev_notifier = {
.notifier_call = wl1271_dev_notify,
};
-static int wl1271_op_start(struct ieee80211_hw *hw)
+static int wl1271_op_start(struct ieee80211_hw *hw,
+ struct cfg80211_wow *wow)
{
wl1271_debug(DEBUG_MAC80211, "mac80211 start");
@@ -1102,7 +1103,8 @@ static int wl1271_op_start(struct ieee80211_hw *hw)
return 0;
}
-static void wl1271_op_stop(struct ieee80211_hw *hw)
+static void wl1271_op_stop(struct ieee80211_hw *hw,
+ struct cfg80211_wow *wow)
{
wl1271_debug(DEBUG_MAC80211, "mac80211 stop");
}
diff --git a/include/net/mac80211.h b/include/net/mac80211.h
index 8fcd169..86d5781 100644
--- a/include/net/mac80211.h
+++ b/include/net/mac80211.h
@@ -1588,8 +1588,12 @@ enum ieee80211_ampdu_mlme_action {
* Must be implemented and atomic.
*
* @start: Called before the first netdevice attached to the hardware
- * is enabled. This should turn on the hardware and must turn on
- * frame reception (for possibly enabled monitor interfaces.)
+ * is enabled, or when waking-on-wireless.
+ * In case of no enabled WoW triggers - this should turn on the
+ * hardware and must turn on frame reception (for possibly
+ * enabled monitor interfaces).
+ * In case of enabled WoW triggers - this might remove the WoW
+ * triggers configuration from the hardware (if needed).
* Returns negative error codes, these may be seen in userspace,
* or zero.
* When the device is started it should not have a MAC address
@@ -1598,8 +1602,11 @@ enum ieee80211_ampdu_mlme_action {
* Must be implemented and can sleep.
*
* @stop: Called after last netdevice attached to the hardware
- * is disabled. This should turn off the hardware (at least
- * it must turn off frame reception.)
+ * is disabled, or during suspend while WoW triggers are configured.
+ * In case of no enabled WoW triggers - this should turn off the
+ * hardware (at least it must turn off frame reception).
+ * In case of enabled WoW triggers - this should configure the
+ * hardware with the WoW triggers.
* May be called right after add_interface if that rejects
* an interface. If you added any work onto the mac80211 workqueue
* you should ensure to cancel it on this callback.
@@ -1801,8 +1808,8 @@ enum ieee80211_ampdu_mlme_action {
*/
struct ieee80211_ops {
int (*tx)(struct ieee80211_hw *hw, struct sk_buff *skb);
- int (*start)(struct ieee80211_hw *hw);
- void (*stop)(struct ieee80211_hw *hw);
+ int (*start)(struct ieee80211_hw *hw, struct cfg80211_wow *wow);
+ void (*stop)(struct ieee80211_hw *hw, struct cfg80211_wow *wow);
int (*add_interface)(struct ieee80211_hw *hw,
struct ieee80211_vif *vif);
int (*change_interface)(struct ieee80211_hw *hw,
diff --git a/net/mac80211/driver-ops.h b/net/mac80211/driver-ops.h
index 78af32d..ca93e82 100644
--- a/net/mac80211/driver-ops.h
+++ b/net/mac80211/driver-ops.h
@@ -10,26 +10,28 @@ static inline int drv_tx(struct ieee80211_local *local, struct sk_buff *skb)
return local->ops->tx(&local->hw, skb);
}
-static inline int drv_start(struct ieee80211_local *local)
+static inline int drv_start(struct ieee80211_local *local,
+ struct cfg80211_wow *wow)
{
int ret;
might_sleep();
- trace_drv_start(local);
+ trace_drv_start(local, wow);
local->started = true;
smp_mb();
- ret = local->ops->start(&local->hw);
+ ret = local->ops->start(&local->hw, wow);
trace_drv_return_int(local, ret);
return ret;
}
-static inline void drv_stop(struct ieee80211_local *local)
+static inline void drv_stop(struct ieee80211_local *local,
+ struct cfg80211_wow *wow)
{
might_sleep();
- trace_drv_stop(local);
- local->ops->stop(&local->hw);
+ trace_drv_stop(local, wow);
+ local->ops->stop(&local->hw, wow);
trace_drv_return_void(local);
/* sync away all work on the tasklet before clearing started */
diff --git a/net/mac80211/driver-trace.h b/net/mac80211/driver-trace.h
index e5cce19..cbd4b45 100644
--- a/net/mac80211/driver-trace.h
+++ b/net/mac80211/driver-trace.h
@@ -88,14 +88,49 @@ TRACE_EVENT(drv_return_u64,
TP_printk(LOCAL_PR_FMT " - %llu", LOCAL_PR_ARG, __entry->ret)
);
-DEFINE_EVENT(local_only_evt, drv_start,
- TP_PROTO(struct ieee80211_local *local),
- TP_ARGS(local)
+TRACE_EVENT(drv_start,
+ TP_PROTO(struct ieee80211_local *local,
+ struct cfg80211_wow *wow),
+
+
+ TP_ARGS(local, wow),
+
+ TP_STRUCT__entry(
+ LOCAL_ENTRY
+ __field(bool, resume)
+ ),
+
+ TP_fast_assign(
+ LOCAL_ASSIGN;
+ __entry->resume = !!wow;
+ ),
+
+ TP_printk(
+ LOCAL_PR_FMT "%s",
+ LOCAL_PR_ARG, __entry->resume ? "resume" : ""
+ )
);
-DEFINE_EVENT(local_only_evt, drv_stop,
- TP_PROTO(struct ieee80211_local *local),
- TP_ARGS(local)
+TRACE_EVENT(drv_stop,
+ TP_PROTO(struct ieee80211_local *local,
+ struct cfg80211_wow *wow),
+
+ TP_ARGS(local, wow),
+
+ TP_STRUCT__entry(
+ LOCAL_ENTRY
+ __field(bool, suspend)
+ ),
+
+ TP_fast_assign(
+ LOCAL_ASSIGN;
+ __entry->suspend = !!wow;
+ ),
+
+ TP_printk(
+ LOCAL_PR_FMT "%s",
+ LOCAL_PR_ARG, __entry->suspend ? "suspend" : ""
+ )
);
TRACE_EVENT(drv_add_interface,
diff --git a/net/mac80211/iface.c b/net/mac80211/iface.c
index a0e3779..a9b25b1 100644
--- a/net/mac80211/iface.c
+++ b/net/mac80211/iface.c
@@ -212,7 +212,7 @@ static int ieee80211_do_open(struct net_device *dev, bool coming_up)
}
if (local->open_count == 0) {
- res = drv_start(local);
+ res = drv_start(local, NULL);
if (res)
goto err_del_bss;
if (local->ops->napi_poll)
@@ -236,7 +236,7 @@ static int ieee80211_do_open(struct net_device *dev, bool coming_up)
if (!is_valid_ether_addr(dev->dev_addr)) {
if (!local->open_count)
- drv_stop(local);
+ drv_stop(local, NULL);
return -EADDRNOTAVAIL;
}
}
@@ -348,7 +348,7 @@ static int ieee80211_do_open(struct net_device *dev, bool coming_up)
drv_remove_interface(local, &sdata->vif);
err_stop:
if (!local->open_count)
- drv_stop(local);
+ drv_stop(local, NULL);
err_del_bss:
sdata->bss = NULL;
if (sdata->vif.type == NL80211_IFTYPE_AP_VLAN)
diff --git a/net/mac80211/util.c b/net/mac80211/util.c
index 2058965..627175a 100644
--- a/net/mac80211/util.c
+++ b/net/mac80211/util.c
@@ -1116,7 +1116,7 @@ void ieee80211_stop_device(struct ieee80211_local *local,
cancel_work_sync(&local->reconfig_filter);
flush_workqueue(local->workqueue);
- drv_stop(local);
+ drv_stop(local, wow);
}
int ieee80211_reconfig(struct ieee80211_local *local,
@@ -1138,7 +1138,7 @@ int ieee80211_reconfig(struct ieee80211_local *local,
* the device may at times not work immediately. Propagate
* the error.
*/
- res = drv_start(local);
+ res = drv_start(local, wow);
if (res) {
WARN(local->suspended, "Hardware became unavailable "
"upon resume. This could be a software issue "
--
1.7.0.4
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [RFC 3/9] mac80211: add WoW param to .start/.stop callbacks
2011-03-01 20:36 ` [RFC 3/9] mac80211: add WoW param to .start/.stop callbacks Eliad Peller
@ 2011-03-08 14:12 ` Johannes Berg
0 siblings, 0 replies; 21+ messages in thread
From: Johannes Berg @ 2011-03-08 14:12 UTC (permalink / raw)
To: Eliad Peller; +Cc: linux-wireless
On Tue, 2011-03-01 at 22:36 +0200, Eliad Peller wrote:
> Add a param which contains the enabled Wake-On-Wireless triggers,
> so lower drivers prepare themselves to suspend/resume
> (e.g. configure triggers to fw, mask irqs, etc.)
I'm not sure I like using the start/stop API for suspend in this way.
Basically, I think what happens with this is that we're saying to the
driver "stop(but really don't because of WoW)". That seems a bit strange
to me, wouldn't you agree?
I'm inclined to not use the "deconfigure everything" code in mac80211 in
the case that wow triggers were requested, and instead call suspend() in
the driver with the WoW triggers. Upon resume, also don't reconfigure,
but call resume() instead?
johannes
^ permalink raw reply [flat|nested] 21+ messages in thread
* [RFC 4/9] mac80211: don't remove/add interfaces when WoW is enabled
2011-03-01 20:36 [RFC 0/9] add WoW support Eliad Peller
` (2 preceding siblings ...)
2011-03-01 20:36 ` [RFC 3/9] mac80211: add WoW param to .start/.stop callbacks Eliad Peller
@ 2011-03-01 20:36 ` Eliad Peller
2011-03-08 14:14 ` Johannes Berg
2011-03-01 20:36 ` [RFC 5/9] wl12xx_sdio: set interrupt as wake_up interrupt Eliad Peller
` (7 subsequent siblings)
11 siblings, 1 reply; 21+ messages in thread
From: Eliad Peller @ 2011-03-01 20:36 UTC (permalink / raw)
To: Johannes Berg; +Cc: linux-wireless
When WoW triggers are enabled, we shouldn't remove the interfaces
on suspend, as we need them in order to get the triggers.
Consequently, we shouldn't add them back when resuming.
TODO:
what settings should we configure when going to WoW (ps,
rx_filtering, etc.)?
whose responsibility is it - the mac80211 or the lower driver?
(the lower driver may implement some features that are not known
to the mac80211)
should they be configurable by the user?
Signed-off-by: Eliad Peller <eliad@wizery.com>
---
net/mac80211/pm.c | 63 +++++++++++++++++++++++++++---------------------
net/mac80211/util.c | 66 +++++++++++++++++++++++++++++++--------------------
2 files changed, 76 insertions(+), 53 deletions(-)
diff --git a/net/mac80211/pm.c b/net/mac80211/pm.c
index 6d135ea..f687606 100644
--- a/net/mac80211/pm.c
+++ b/net/mac80211/pm.c
@@ -6,36 +6,11 @@
#include "driver-ops.h"
#include "led.h"
-int __ieee80211_suspend(struct ieee80211_hw *hw,
- struct cfg80211_wow *wow)
+static void ieee80211_clear_interfaces(struct ieee80211_local *local)
{
- struct ieee80211_local *local = hw_to_local(hw);
struct ieee80211_sub_if_data *sdata;
struct sta_info *sta;
-
- ieee80211_scan_cancel(local);
-
- ieee80211_stop_queues_by_reason(hw,
- IEEE80211_QUEUE_STOP_REASON_SUSPEND);
-
- /* flush out all packets */
- synchronize_net();
-
- local->quiescing = true;
- /* make quiescing visible to timers everywhere */
- mb();
-
- flush_workqueue(local->workqueue);
-
- /* Don't try to run timers while suspended. */
- del_timer_sync(&local->sta_cleanup);
-
- /*
- * Note that this particular timer doesn't need to be
- * restarted at resume.
- */
- cancel_work_sync(&local->dynamic_ps_enable_work);
- del_timer_sync(&local->dynamic_ps_timer);
+ struct ieee80211_hw *hw = &local->hw;
/* disable keys */
list_for_each_entry(sdata, &local->interfaces, list)
@@ -94,6 +69,40 @@ int __ieee80211_suspend(struct ieee80211_hw *hw,
drv_remove_interface(local, &sdata->vif);
}
+}
+int __ieee80211_suspend(struct ieee80211_hw *hw,
+ struct cfg80211_wow *wow)
+{
+ struct ieee80211_local *local = hw_to_local(hw);
+ bool wow_enabled = wow && wow->enabled_triggers;
+
+ ieee80211_scan_cancel(local);
+
+ ieee80211_stop_queues_by_reason(hw,
+ IEEE80211_QUEUE_STOP_REASON_SUSPEND);
+
+ /* flush out all packets */
+ synchronize_net();
+
+ local->quiescing = true;
+ /* make quiescing visible to timers everywhere */
+ mb();
+
+ flush_workqueue(local->workqueue);
+
+ /* Don't try to run timers while suspended. */
+ del_timer_sync(&local->sta_cleanup);
+
+ /*
+ * Note that this particular timer doesn't need to be
+ * restarted at resume.
+ */
+ cancel_work_sync(&local->dynamic_ps_enable_work);
+ del_timer_sync(&local->dynamic_ps_timer);
+
+ /* in case of wow, we want to leave the interfaces up */
+ if (!wow_enabled)
+ ieee80211_clear_interfaces(local);
/* stop hardware - this must stop RX */
if (local->open_count)
diff --git a/net/mac80211/util.c b/net/mac80211/util.c
index 627175a..d640ffd 100644
--- a/net/mac80211/util.c
+++ b/net/mac80211/util.c
@@ -1119,38 +1119,13 @@ void ieee80211_stop_device(struct ieee80211_local *local,
drv_stop(local, wow);
}
-int ieee80211_reconfig(struct ieee80211_local *local,
- struct cfg80211_wow *wow)
+static void ieee80211_reconfig_interface(struct ieee80211_local *local)
{
struct ieee80211_hw *hw = &local->hw;
struct ieee80211_sub_if_data *sdata;
struct sta_info *sta;
int res;
- if (local->suspended)
- local->resuming = true;
-
- /* restart hardware */
- if (local->open_count) {
- /*
- * Upon resume hardware can sometimes be goofy due to
- * various platform / driver / bus issues, so restarting
- * the device may at times not work immediately. Propagate
- * the error.
- */
- res = drv_start(local, wow);
- if (res) {
- WARN(local->suspended, "Hardware became unavailable "
- "upon resume. This could be a software issue "
- "prior to suspend or a hardware issue.\n");
- return res;
- }
-
- ieee80211_led_radio(local, true);
- ieee80211_mod_tpt_led_trig(local,
- IEEE80211_TPT_LEDTRIG_FL_RADIO, 0);
- }
-
/* add interfaces */
list_for_each_entry(sdata, &local->interfaces, list) {
if (sdata->vif.type != NL80211_IFTYPE_AP_VLAN &&
@@ -1259,7 +1234,46 @@ int ieee80211_reconfig(struct ieee80211_local *local,
list_for_each_entry(sdata, &local->interfaces, list)
if (ieee80211_sdata_running(sdata))
ieee80211_enable_keys(sdata);
+}
+
+int ieee80211_reconfig(struct ieee80211_local *local,
+ struct cfg80211_wow *wow)
+{
+ struct ieee80211_hw *hw = &local->hw;
+ struct ieee80211_sub_if_data *sdata;
+ struct sta_info *sta;
+ bool wow_enabled = wow && wow->enabled_triggers;
+ int res;
+
+ if (local->suspended)
+ local->resuming = true;
+
+ /* restart hardware */
+ if (local->open_count) {
+ /*
+ * Upon resume hardware can sometimes be goofy due to
+ * various platform / driver / bus issues, so restarting
+ * the device may at times not work immediately. Propagate
+ * the error.
+ */
+ res = drv_start(local, wow);
+ if (res) {
+ WARN(local->suspended, "Hardware became unavailable "
+ "upon resume. This could be a software issue "
+ "prior to suspend or a hardware issue.\n");
+ return res;
+ }
+
+ ieee80211_led_radio(local, true);
+ ieee80211_mod_tpt_led_trig(local,
+ IEEE80211_TPT_LEDTRIG_FL_RADIO, 0);
+ }
+
+ /* reconfig interface only if they were removed */
+ if (!wow_enabled)
+ ieee80211_reconfig_interface(local);
+ /* should we do it anyway? */
ieee80211_wake_queues_by_reason(hw,
IEEE80211_QUEUE_STOP_REASON_SUSPEND);
--
1.7.0.4
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [RFC 4/9] mac80211: don't remove/add interfaces when WoW is enabled
2011-03-01 20:36 ` [RFC 4/9] mac80211: don't remove/add interfaces when WoW is enabled Eliad Peller
@ 2011-03-08 14:14 ` Johannes Berg
0 siblings, 0 replies; 21+ messages in thread
From: Johannes Berg @ 2011-03-08 14:14 UTC (permalink / raw)
To: Eliad Peller; +Cc: linux-wireless
On Tue, 2011-03-01 at 22:36 +0200, Eliad Peller wrote:
> When WoW triggers are enabled, we shouldn't remove the interfaces
> on suspend, as we need them in order to get the triggers.
> Consequently, we shouldn't add them back when resuming.
>
> TODO:
> what settings should we configure when going to WoW (ps,
> rx_filtering, etc.)?
> whose responsibility is it - the mac80211 or the lower driver?
> (the lower driver may implement some features that are not known
> to the mac80211)
> should they be configurable by the user?
Yeah this goes with what I just said... but why do it in mac80211?
FWIW, iwlwifi will also require a pretty much complete reconfig when
going to suspend, and also when resuming, and will also drop aggregation
sessions while asleep (for example) so more tricks will be needed.
johannes
^ permalink raw reply [flat|nested] 21+ messages in thread
* [RFC 5/9] wl12xx_sdio: set interrupt as wake_up interrupt
2011-03-01 20:36 [RFC 0/9] add WoW support Eliad Peller
` (3 preceding siblings ...)
2011-03-01 20:36 ` [RFC 4/9] mac80211: don't remove/add interfaces when WoW is enabled Eliad Peller
@ 2011-03-01 20:36 ` Eliad Peller
2011-03-01 20:36 ` [RFC 6/9] wl12xx_sdio: set MMC_PM_KEEP_POWER flag on suspend Eliad Peller
` (6 subsequent siblings)
11 siblings, 0 replies; 21+ messages in thread
From: Eliad Peller @ 2011-03-01 20:36 UTC (permalink / raw)
To: Johannes Berg; +Cc: linux-wireless
set the sdio interrupt as wake_up interrupt, so we will be able
to wake up the suspended system (Wake-On-Wireless)
Signed-off-by: Eliad Peller <eliad@wizery.com>
---
drivers/net/wireless/wl12xx/sdio.c | 2 ++
1 files changed, 2 insertions(+), 0 deletions(-)
diff --git a/drivers/net/wireless/wl12xx/sdio.c b/drivers/net/wireless/wl12xx/sdio.c
index c3792cb..eec98a0 100644
--- a/drivers/net/wireless/wl12xx/sdio.c
+++ b/drivers/net/wireless/wl12xx/sdio.c
@@ -265,6 +265,7 @@ static int __devinit wl1271_probe(struct sdio_func *func,
}
set_irq_type(wl->irq, IRQ_TYPE_EDGE_RISING);
+ set_irq_wake(wl->irq, 1);
disable_irq(wl->irq);
@@ -303,6 +304,7 @@ static void __devexit wl1271_remove(struct sdio_func *func)
pm_runtime_get_noresume(&func->dev);
wl1271_unregister_hw(wl);
+ set_irq_wake(wl->irq, 0);
free_irq(wl->irq, wl);
wl1271_free_hw(wl);
}
--
1.7.0.4
^ permalink raw reply related [flat|nested] 21+ messages in thread* [RFC 6/9] wl12xx_sdio: set MMC_PM_KEEP_POWER flag on suspend
2011-03-01 20:36 [RFC 0/9] add WoW support Eliad Peller
` (4 preceding siblings ...)
2011-03-01 20:36 ` [RFC 5/9] wl12xx_sdio: set interrupt as wake_up interrupt Eliad Peller
@ 2011-03-01 20:36 ` Eliad Peller
2011-03-01 20:36 ` [RFC 7/9] wl12xx: save wl->wow_enabled " Eliad Peller
` (5 subsequent siblings)
11 siblings, 0 replies; 21+ messages in thread
From: Eliad Peller @ 2011-03-01 20:36 UTC (permalink / raw)
To: Johannes Berg; +Cc: linux-wireless
if a wow trigger was configured, set the MMC_PM_KEEP_POWER flag
on suspend, so our power will be kept while the system is suspended.
We needed to set this flag on each suspend attempt (when we want
to keep power)
Signed-off-by: Eliad Peller <eliad@wizery.com>
---
drivers/net/wireless/wl12xx/sdio.c | 30 ++++++++++++++++++++++++++++++
1 files changed, 30 insertions(+), 0 deletions(-)
diff --git a/drivers/net/wireless/wl12xx/sdio.c b/drivers/net/wireless/wl12xx/sdio.c
index eec98a0..6083c3d 100644
--- a/drivers/net/wireless/wl12xx/sdio.c
+++ b/drivers/net/wireless/wl12xx/sdio.c
@@ -313,11 +313,41 @@ static int wl1271_suspend(struct device *dev)
{
/* Tell MMC/SDIO core it's OK to power down the card
* (if it isn't already), but not to remove it completely */
+ struct sdio_func *func = dev_to_sdio_func(dev);
+ struct wl1271 *wl = sdio_get_drvdata(func);
+ mmc_pm_flag_t sdio_flags;
+ int ret = 0;
+
+ /*
+ * we need to look into wl to tell which suspend method to use.
+ * we will have full power, ps mode, elp, and power off
+ */
+ wl1271_info("%s: wow_enabled: %d", __func__, wl->wow_enabled);
+ if (wl->wow_enabled) {
+ sdio_flags = sdio_get_host_pm_caps(func);
+
+ wl1271_info("suspend PM flags = 0x%x", sdio_flags);
+
+ if (!(sdio_flags & MMC_PM_KEEP_POWER)) {
+ wl1271_error("can't keep power while host "
+ "is suspended");
+ goto power_off;
+ }
+
+ /* keep power while host suspended */
+ ret = sdio_set_host_pm_flags(func, MMC_PM_KEEP_POWER);
+ if (ret) {
+ wl1271_error("error while trying to keep power");
+ goto power_off;
+ }
+ }
+power_off:
return 0;
}
static int wl1271_resume(struct device *dev)
{
+ wl1271_info("wl1271 resume");
return 0;
}
--
1.7.0.4
^ permalink raw reply related [flat|nested] 21+ messages in thread* [RFC 7/9] wl12xx: save wl->wow_enabled on suspend
2011-03-01 20:36 [RFC 0/9] add WoW support Eliad Peller
` (5 preceding siblings ...)
2011-03-01 20:36 ` [RFC 6/9] wl12xx_sdio: set MMC_PM_KEEP_POWER flag on suspend Eliad Peller
@ 2011-03-01 20:36 ` Eliad Peller
2011-03-01 20:36 ` [RFC 8/9] wl12xx: prevent scheduling while suspending (WoW enabled) Eliad Peller
` (4 subsequent siblings)
11 siblings, 0 replies; 21+ messages in thread
From: Eliad Peller @ 2011-03-01 20:36 UTC (permalink / raw)
To: Johannes Berg; +Cc: linux-wireless
save the WoW parameters we got in the .stop callback, so we will be
able to consider them later (while suspending wl12xx_sdio/spi).
Signed-off-by: Eliad Peller <eliad@wizery.com>
---
drivers/net/wireless/wl12xx/main.c | 6 ++++--
drivers/net/wireless/wl12xx/wl12xx.h | 3 +++
2 files changed, 7 insertions(+), 2 deletions(-)
diff --git a/drivers/net/wireless/wl12xx/main.c b/drivers/net/wireless/wl12xx/main.c
index 3a7d788..a46022f 100644
--- a/drivers/net/wireless/wl12xx/main.c
+++ b/drivers/net/wireless/wl12xx/main.c
@@ -1084,7 +1084,7 @@ static struct notifier_block wl1271_dev_notifier = {
static int wl1271_op_start(struct ieee80211_hw *hw,
struct cfg80211_wow *wow)
{
- wl1271_debug(DEBUG_MAC80211, "mac80211 start");
+ wl1271_debug(DEBUG_MAC80211, "mac80211 start resume=%d", !!wow);
/*
* We have to delay the booting of the hardware because
@@ -1106,7 +1106,9 @@ static int wl1271_op_start(struct ieee80211_hw *hw,
static void wl1271_op_stop(struct ieee80211_hw *hw,
struct cfg80211_wow *wow)
{
- wl1271_debug(DEBUG_MAC80211, "mac80211 stop");
+ struct wl1271 *wl = hw->priv;
+ wl1271_debug(DEBUG_MAC80211, "mac80211 stop suspend=%d", !!wow);
+ wl->wow_enabled = !!(wow && wow->enabled_triggers);
}
static int wl1271_op_add_interface(struct ieee80211_hw *hw,
diff --git a/drivers/net/wireless/wl12xx/wl12xx.h b/drivers/net/wireless/wl12xx/wl12xx.h
index 338acc9..dd21818 100644
--- a/drivers/net/wireless/wl12xx/wl12xx.h
+++ b/drivers/net/wireless/wl12xx/wl12xx.h
@@ -521,6 +521,9 @@ struct wl1271 {
bool ba_support;
u8 ba_rx_bitmap;
+ /* tell sdio to stay awake (TODO: move to other place) */
+ bool wow_enabled;
+
/*
* AP-mode - links indexed by HLID. The global and broadcast links
* are always active.
--
1.7.0.4
^ permalink raw reply related [flat|nested] 21+ messages in thread* [RFC 8/9] wl12xx: prevent scheduling while suspending (WoW enabled)
2011-03-01 20:36 [RFC 0/9] add WoW support Eliad Peller
` (6 preceding siblings ...)
2011-03-01 20:36 ` [RFC 7/9] wl12xx: save wl->wow_enabled " Eliad Peller
@ 2011-03-01 20:36 ` Eliad Peller
2011-03-01 20:36 ` [RFC 9/9] wl12xx_sdio: declare support for NL80211_WOW_TRIGGER_ANYTHING trigger Eliad Peller
` (3 subsequent siblings)
11 siblings, 0 replies; 21+ messages in thread
From: Eliad Peller @ 2011-03-01 20:36 UTC (permalink / raw)
To: Johannes Berg; +Cc: linux-wireless
When WoW is enabled, the interface will stay up and the chip will
be powered on, so we have to flush/cancel any remaining work, and
prevent the irq handler from scheduling a new work until the system
is resumed.
Add 2 new flags:
* WL1271_FLAG_SUSPENDED - the system is (about to be) suspended.
* WL1271_FLAG_PENDING_WORK - there is a pending irq work which
should be scheduled when the system is being resumed.
In order to wake-up the system while getting an irq, we initialize
the device as wakeup device, and calling pm_wakeup_event() upon
getting the interrupt (while the system is about to be suspended)
Signed-off-by: Eliad Peller <eliad@wizery.com>
---
drivers/net/wireless/wl12xx/main.c | 33 +++++++++++++++++++++++++++++++++
drivers/net/wireless/wl12xx/sdio.c | 31 ++++++++++++++++++++++++++++---
drivers/net/wireless/wl12xx/wl12xx.h | 4 +++-
3 files changed, 64 insertions(+), 4 deletions(-)
diff --git a/drivers/net/wireless/wl12xx/main.c b/drivers/net/wireless/wl12xx/main.c
index a46022f..6de0567 100644
--- a/drivers/net/wireless/wl12xx/main.c
+++ b/drivers/net/wireless/wl12xx/main.c
@@ -1086,6 +1086,24 @@ static int wl1271_op_start(struct ieee80211_hw *hw,
{
wl1271_debug(DEBUG_MAC80211, "mac80211 start resume=%d", !!wow);
+ /* on resume we have to enable irq_work enqueuing */
+ if (wow) {
+ struct wl1271 *wl = hw->priv;
+ unsigned long flags;
+
+ wl1271_debug(DEBUG_MAC80211, "enabled wow triggers: 0x%x",
+ wow->enabled_triggers);
+ 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)) {
+ ieee80211_queue_work(wl->hw, &wl->irq_work);
+ set_bit(WL1271_FLAG_IRQ_PENDING, &wl->flags);
+ wl1271_debug(DEBUG_MAC80211,
+ "enqueuing 'missing' irq_work");
+ }
+ spin_unlock_irqrestore(&wl->wl_lock, flags);
+ }
+
/*
* We have to delay the booting of the hardware because
* we need to know the local MAC address before downloading and
@@ -1109,6 +1127,21 @@ static void wl1271_op_stop(struct ieee80211_hw *hw,
struct wl1271 *wl = hw->priv;
wl1271_debug(DEBUG_MAC80211, "mac80211 stop suspend=%d", !!wow);
wl->wow_enabled = !!(wow && wow->enabled_triggers);
+ if (wl->wow_enabled) {
+ unsigned long flags;
+
+ spin_lock_irqsave(&wl->wl_lock, flags);
+ set_bit(WL1271_FLAG_SUSPENDED, &wl->flags);
+ spin_unlock_irqrestore(&wl->wl_lock, flags);
+
+ /* we don't want any remaining work */
+ wl1271_info("flushing remaining works\n");
+ flush_delayed_work(&wl->scan_complete_work);
+ flush_work(&wl->irq_work);
+ flush_work(&wl->tx_work);
+ flush_delayed_work(&wl->pspoll_work);
+ flush_delayed_work(&wl->elp_work);
+ }
}
static int wl1271_op_add_interface(struct ieee80211_hw *hw,
diff --git a/drivers/net/wireless/wl12xx/sdio.c b/drivers/net/wireless/wl12xx/sdio.c
index 6083c3d..f5d4b21 100644
--- a/drivers/net/wireless/wl12xx/sdio.c
+++ b/drivers/net/wireless/wl12xx/sdio.c
@@ -75,9 +75,16 @@ static irqreturn_t wl1271_irq(int irq, void *cookie)
wl->elp_compl = NULL;
}
- if (!test_and_set_bit(WL1271_FLAG_IRQ_RUNNING, &wl->flags))
- ieee80211_queue_work(wl->hw, &wl->irq_work);
- set_bit(WL1271_FLAG_IRQ_PENDING, &wl->flags);
+ if (test_bit(WL1271_FLAG_SUSPENDED, &wl->flags)) {
+ /* we shouldn't enqueue a work right now. mark it as pending */
+ set_bit(WL1271_FLAG_PENDING_WORK, &wl->flags);
+ wl1271_debug(DEBUG_IRQ, "not enqueuing work");
+ pm_wakeup_event(wl1271_sdio_wl_to_dev(wl), 0);
+ } else {
+ if (!test_and_set_bit(WL1271_FLAG_IRQ_RUNNING, &wl->flags))
+ ieee80211_queue_work(wl->hw, &wl->irq_work);
+ set_bit(WL1271_FLAG_IRQ_PENDING, &wl->flags);
+ }
spin_unlock_irqrestore(&wl->wl_lock, flags);
return IRQ_HANDLED;
@@ -266,6 +273,8 @@ static int __devinit wl1271_probe(struct sdio_func *func,
set_irq_type(wl->irq, IRQ_TYPE_EDGE_RISING);
set_irq_wake(wl->irq, 1);
+ device_init_wakeup(wl1271_sdio_wl_to_dev(wl), 1);
+
disable_irq(wl->irq);
@@ -304,6 +313,7 @@ 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);
set_irq_wake(wl->irq, 0);
free_irq(wl->irq, wl);
wl1271_free_hw(wl);
@@ -316,9 +326,24 @@ static int wl1271_suspend(struct device *dev)
struct sdio_func *func = dev_to_sdio_func(dev);
struct wl1271 *wl = sdio_get_drvdata(func);
mmc_pm_flag_t sdio_flags;
+ unsigned long flags;
+ bool abort;
int ret = 0;
/*
+ * if there is a pending irq work, we should abort the suspension.
+ * (irq might come between mac80211 suspension and our suspension)
+ * TODO: maybe remove it, since system will wake up anyway?
+ */
+ spin_lock_irqsave(&wl->wl_lock, flags);
+ abort = !!test_bit(WL1271_FLAG_PENDING_WORK, &wl->flags);
+ spin_unlock_irqrestore(&wl->wl_lock, flags);
+ if (abort) {
+ wl1271_info("pending irq work - aborting suspend");
+ return -EBUSY;
+ }
+
+ /*
* we need to look into wl to tell which suspend method to use.
* we will have full power, ps mode, elp, and power off
*/
diff --git a/drivers/net/wireless/wl12xx/wl12xx.h b/drivers/net/wireless/wl12xx/wl12xx.h
index dd21818..2dc0b31 100644
--- a/drivers/net/wireless/wl12xx/wl12xx.h
+++ b/drivers/net/wireless/wl12xx/wl12xx.h
@@ -327,7 +327,9 @@ enum wl12xx_flags {
WL1271_FLAG_PSPOLL_FAILURE,
WL1271_FLAG_STA_STATE_SENT,
WL1271_FLAG_FW_TX_BUSY,
- WL1271_FLAG_AP_STARTED
+ WL1271_FLAG_AP_STARTED,
+ WL1271_FLAG_SUSPENDED,
+ WL1271_FLAG_PENDING_WORK,
};
struct wl1271_link {
--
1.7.0.4
^ permalink raw reply related [flat|nested] 21+ messages in thread* [RFC 9/9] wl12xx_sdio: declare support for NL80211_WOW_TRIGGER_ANYTHING trigger
2011-03-01 20:36 [RFC 0/9] add WoW support Eliad Peller
` (7 preceding siblings ...)
2011-03-01 20:36 ` [RFC 8/9] wl12xx: prevent scheduling while suspending (WoW enabled) Eliad Peller
@ 2011-03-01 20:36 ` Eliad Peller
2011-03-22 14:46 ` [RFC 0/9] add WoW support Johannes Berg
` (2 subsequent siblings)
11 siblings, 0 replies; 21+ messages in thread
From: Eliad Peller @ 2011-03-01 20:36 UTC (permalink / raw)
To: Johannes Berg; +Cc: linux-wireless
Finally, declare support for NL80211_WOW_TRIGGER_ANYTHING.
Since this feature requires the ability to stay awake while the host
is suspended, the support should be declared per-interface (sdio
will support it if it supports the MMC_PM_KEEP_POWER capability)
Signed-off-by: Eliad Peller <eliad@wizery.com>
---
drivers/net/wireless/wl12xx/sdio.c | 9 +++++++++
1 files changed, 9 insertions(+), 0 deletions(-)
diff --git a/drivers/net/wireless/wl12xx/sdio.c b/drivers/net/wireless/wl12xx/sdio.c
index f5d4b21..0a5b05d 100644
--- a/drivers/net/wireless/wl12xx/sdio.c
+++ b/drivers/net/wireless/wl12xx/sdio.c
@@ -237,6 +237,7 @@ static int __devinit wl1271_probe(struct sdio_func *func,
struct ieee80211_hw *hw;
const struct wl12xx_platform_data *wlan_data;
struct wl1271 *wl;
+ mmc_pm_flag_t flags;
int ret;
/* We are only able to handle the wlan function */
@@ -278,6 +279,14 @@ static int __devinit wl1271_probe(struct sdio_func *func,
disable_irq(wl->irq);
+ /* if sdio can keep power while host is suspended, enable wow */
+ flags = sdio_get_host_pm_caps(func);
+ wl1271_debug(DEBUG_SDIO, "sdio PM caps = 0x%x", flags);
+
+ if (flags & MMC_PM_KEEP_POWER)
+ hw->wiphy->supported_wow_triggers =
+ NL80211_WOW_TRIGGER_ANYTHING;
+
ret = wl1271_init_ieee80211(wl);
if (ret)
goto out_irq;
--
1.7.0.4
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [RFC 0/9] add WoW support
2011-03-01 20:36 [RFC 0/9] add WoW support Eliad Peller
` (8 preceding siblings ...)
2011-03-01 20:36 ` [RFC 9/9] wl12xx_sdio: declare support for NL80211_WOW_TRIGGER_ANYTHING trigger Eliad Peller
@ 2011-03-22 14:46 ` Johannes Berg
2011-03-22 15:13 ` Johannes Berg
2011-03-22 15:20 ` Johannes Berg
11 siblings, 0 replies; 21+ messages in thread
From: Johannes Berg @ 2011-03-22 14:46 UTC (permalink / raw)
To: Eliad Peller; +Cc: linux-wireless
Let's back up a bit on this and think about the design some more, since
this touches a whole bunch of different things.
First, I think there at least two WoWLAN models:
1) The "traditional" model:
In this model, the device handles many things, there are different
wakeup triggers, for example:
* magic packet
* packet pattern
* connection lost
* GKT rekeying (although some devices may support this in firmware and
then don't wake up), or rekeying failure
* rfkill (?)
* random other things
These are only available while connected to a (typically exactly one)
AP. Also, importantly, some of these require explicit configuration.
Additionally, some devices may support "network detection", a form of
scanning and waking up the host if certain conditions are met.
However, with that, we also get closer to
2) The "wake whenever there's something to do" model:
I don't have a good name for this, really, but it's what the TI chip
does right now -- it just wakes the host when there's something to do
for it, whatever that might be. It's closer to Android's wake-lock model
in a way I guess., the "network detection" would also be possible to
implement with this in the TI chip since it has such functionality
(which also doesn't depend on going to suspend, unlike ours which
basically cannot be interleaved with other things).
Whichever model is used, however, there will be spurious wakeups. I hope
there's some way of identifying which device woke up the system and then
go to sleep again. I think the only reasonable use case for WoWlan is
with some kind of "suspend manager" that evaluates this information and
puts the system to sleep again as appropriate.
This manager component might also connect to a network after "network
detection" while suspended found something, and then suspend the system
again with new triggers.
johannes
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [RFC 0/9] add WoW support
2011-03-01 20:36 [RFC 0/9] add WoW support Eliad Peller
` (9 preceding siblings ...)
2011-03-22 14:46 ` [RFC 0/9] add WoW support Johannes Berg
@ 2011-03-22 15:13 ` Johannes Berg
2011-03-22 15:40 ` Ohad Ben-Cohen
2011-03-23 9:40 ` Eliad Peller
2011-03-22 15:20 ` Johannes Berg
11 siblings, 2 replies; 21+ messages in thread
From: Johannes Berg @ 2011-03-22 15:13 UTC (permalink / raw)
To: Eliad Peller; +Cc: linux-wireless
In the second email, I'll gather some thoughts about userspace APIs.
Regardless of how suspend works, the device may need special
configuration (e.g. wakeup patterns [1], rekeying information or
similar), in our case it even needs special firmware uploaded. Suspend
might also only be possible in certain configurations, like being
associated with exactly one AP, and not while operating as an AP, for
example.
Clearly, there are race conditions here. Say magic packet wakeup is
configured, but we get disconnected by the AP just before suspend. There
will not be a way to honour that trigger now, so I would argue that we
should fail the suspend in that case. The only other alternative is
silently not failing, but then there will be no wakeup trigger at all,
and had the disconnect happened earlier network detection might've been
configured. If it fails, the userspace suspend agent I was talking about
can reconfigure.
However, in this case the userspace agent would also have to completely
deconfigure wowlan triggers before being able to suspend "normally".
Maybe even this distinction has to be under userspace control to allow
it to operate in multiple models.
Back to the unsupported modes though, should those fail suspend as well?
Currently, we simply deconfigure everything during suspend. But once we
have a model of "the connection stays up during suspend", should we
still silently shut down AP mode interfaces and keep the others during
suspend? That seems inconsistent.
Again, the solution might be a suspend agent that will configure the
system in a way that suspend is possible. But how can it tell that
suspend will be possible? Between the different possible models, there
can be a lot of variety. We can expose those, but what will userspace do
with the information? Do we require that it configures the device in a
way that it can suspend, or do we do that in the drivers?
As far as the actual API is concerned, I don't really see any big issues
with it, since we simply set and retrieve triggers. There will be a need
for getting some more information after wakeup, particularly when the
device supports GTK rekeying while asleep.
The bigger questions I have are around the semantic issues I've
outlined. How do we want to treat those?
johannes
[1] incidentally, this might be supported in the "continue operating"
model, by programming filters into the device, which might not be
exposed as host runtime APIs.
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [RFC 0/9] add WoW support
2011-03-22 15:13 ` Johannes Berg
@ 2011-03-22 15:40 ` Ohad Ben-Cohen
2011-03-23 9:40 ` Eliad Peller
1 sibling, 0 replies; 21+ messages in thread
From: Ohad Ben-Cohen @ 2011-03-22 15:40 UTC (permalink / raw)
To: Johannes Berg; +Cc: Eliad Peller, linux-wireless
On Tue, Mar 22, 2011 at 5:13 PM, Johannes Berg
<johannes@sipsolutions.net> wrote:
> In the second email, I'll gather some thoughts about userspace APIs.
>
> Regardless of how suspend works, the device may need special
> configuration (e.g. wakeup patterns [1], rekeying information or
> similar), in our case it even needs special firmware uploaded. Suspend
> might also only be possible in certain configurations, like being
> associated with exactly one AP, and not while operating as an AP, for
> example.
With TI's 12xx, we actually do want to let the host suspend itself
even when acting as an AP, because the 12xx firmware itself is
responsible for sending beacons, and responding to probe requests,
without involving the host.
So in case there's no traffic going on, the host can stay suspended
even if we have stations connected.
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [RFC 0/9] add WoW support
2011-03-22 15:13 ` Johannes Berg
2011-03-22 15:40 ` Ohad Ben-Cohen
@ 2011-03-23 9:40 ` Eliad Peller
2011-03-23 9:51 ` Johannes Berg
1 sibling, 1 reply; 21+ messages in thread
From: Eliad Peller @ 2011-03-23 9:40 UTC (permalink / raw)
To: Johannes Berg; +Cc: linux-wireless
On Tue, Mar 22, 2011 at 5:13 PM, Johannes Berg
<johannes@sipsolutions.net> wrote:
> In the second email, I'll gather some thoughts about userspace APIs.
>
> Regardless of how suspend works, the device may need special
> configuration (e.g. wakeup patterns [1], rekeying information or
> similar), in our case it even needs special firmware uploaded. Suspend
> might also only be possible in certain configurations, like being
> associated with exactly one AP, and not while operating as an AP, for
> example.
>
as Ohad noted, we do need to be able to suspend while operating as AP
(which is possible as the beaconing is done in the fw), at least with
our model.
i can also think of use cases in which it will be useful to have a
suspended AP also in standard wowlan - like letting the ap go to sleep
while there are no clients, and waking it up on a directed probe
request.
> Back to the unsupported modes though, should those fail suspend as well?
> Currently, we simply deconfigure everything during suspend. But once we
> have a model of "the connection stays up during suspend", should we
> still silently shut down AP mode interfaces and keep the others during
> suspend? That seems inconsistent.
>
> Again, the solution might be a suspend agent that will configure the
> system in a way that suspend is possible. But how can it tell that
> suspend will be possible? Between the different possible models, there
> can be a lot of variety. We can expose those, but what will userspace do
> with the information? Do we require that it configures the device in a
> way that it can suspend, or do we do that in the drivers?
maybe we can add each trigger an additional parameter which describes
whether this
trigger is optional (when applicable) or mandatory (which will abort
the suspend if it can't
be configured)?
Eliad.
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [RFC 0/9] add WoW support
2011-03-23 9:40 ` Eliad Peller
@ 2011-03-23 9:51 ` Johannes Berg
0 siblings, 0 replies; 21+ messages in thread
From: Johannes Berg @ 2011-03-23 9:51 UTC (permalink / raw)
To: Eliad Peller; +Cc: linux-wireless
On Wed, 2011-03-23 at 11:40 +0200, Eliad Peller wrote:
> > Regardless of how suspend works, the device may need special
> > configuration (e.g. wakeup patterns [1], rekeying information or
> > similar), in our case it even needs special firmware uploaded. Suspend
> > might also only be possible in certain configurations, like being
> > associated with exactly one AP, and not while operating as an AP, for
> > example.
> >
> as Ohad noted, we do need to be able to suspend while operating as AP
> (which is possible as the beaconing is done in the fw), at least with
> our model.
> i can also think of use cases in which it will be useful to have a
> suspended AP also in standard wowlan - like letting the ap go to sleep
> while there are no clients, and waking it up on a directed probe
> request.
Yeah, I was more using that as an example -- our device can't do it at
this point afaik (but I'm not entirely sure).
> > Again, the solution might be a suspend agent that will configure the
> > system in a way that suspend is possible. But how can it tell that
> > suspend will be possible? Between the different possible models, there
> > can be a lot of variety. We can expose those, but what will userspace do
> > with the information? Do we require that it configures the device in a
> > way that it can suspend, or do we do that in the drivers?
>
> maybe we can add each trigger an additional parameter which describes
> whether this
> trigger is optional (when applicable) or mandatory (which will abort
> the suspend if it can't be configured)?
Yeah that's a good idea, that way userspace can tell what it needs and
wants more specifically than a global flag would be. It will know of
course that it can never configure magic packet when the device doesn't
support that, but when it is supported and the connection drops in the
meantime ...
johannes
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [RFC 0/9] add WoW support
2011-03-01 20:36 [RFC 0/9] add WoW support Eliad Peller
` (10 preceding siblings ...)
2011-03-22 15:13 ` Johannes Berg
@ 2011-03-22 15:20 ` Johannes Berg
2011-03-23 9:46 ` Eliad Peller
11 siblings, 1 reply; 21+ messages in thread
From: Johannes Berg @ 2011-03-22 15:20 UTC (permalink / raw)
To: Eliad Peller; +Cc: linux-wireless
In this third email, some thoughts about the internal APIs.
For the model 2) that I described, I think the current APIs as
implemented in this patchset are not effective. Remember that in this
model, there's no change to how the chip operates, it just continues
operating as before, while the host is suspended, and has no extra
wakeup conditions, just the regular interrupt.
Things are more complex in the more traditional model, mostly depending
on how we answer the questions I posed in my second email.
However, ultimately I think we should move away from the start/stop
model we've imposed right now. The model makes sense for the case where
the device is shut down, but I think it doesn't make sense at all for
the case where the device stays up, in whatever form.
Just looking at patch 4/9 in Eliad's series that I'm replying to, I
think that it's clear that there's no sense in trying to keep the
mac80211 logic for the wowlan suspend case.
I think what we should do instead is this:
1) Add suspend/resume ops instead of going through stop/start and all
the deconfiguration / reconfiguration (of course keep all the
previous logic for drivers that don't implement suspend.)
2) Implement more iterators / accessors to allow drivers to get access
to more information. Some of that might not be necessary since at
resume in some cases the device might just go and request a restart,
but in other cases this might be necessary to simplify/avoid book-
keeping in drivers.
Adding such APIs would allow drivers to be more in control of what
happens at suspend, which given the complexity of how this might work,
with configuring the driver etc.
Thoughts?
johannes
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [RFC 0/9] add WoW support
2011-03-22 15:20 ` Johannes Berg
@ 2011-03-23 9:46 ` Eliad Peller
0 siblings, 0 replies; 21+ messages in thread
From: Eliad Peller @ 2011-03-23 9:46 UTC (permalink / raw)
To: Johannes Berg; +Cc: linux-wireless
On Tue, Mar 22, 2011 at 5:20 PM, Johannes Berg
<johannes@sipsolutions.net> wrote:
> I think what we should do instead is this:
> 1) Add suspend/resume ops instead of going through stop/start and all
> the deconfiguration / reconfiguration (of course keep all the
> previous logic for drivers that don't implement suspend.)
>
> 2) Implement more iterators / accessors to allow drivers to get access
> to more information. Some of that might not be necessary since at
> resume in some cases the device might just go and request a restart,
> but in other cases this might be necessary to simplify/avoid book-
> keeping in drivers.
>
> Adding such APIs would allow drivers to be more in control of what
> happens at suspend, which given the complexity of how this might work,
> with configuring the driver etc.
>
> Thoughts?
>
i agree.
new suspend/resume ops sounds like the right thing to do.
i'm not sure why i didn't add them in the first place. :)
Eliad.
^ permalink raw reply [flat|nested] 21+ messages in thread