From: Raja Mani <[email protected]>
Using these patch sets, WOW patterns can be controlled
and configured via iw command. Please refer iw help menu for more details.
There are some limitation in the recent "iw" command such as it doesn't
take the pattern offset where to start pattern matching in the received packets.
Such a enhancement will be done later..
These patch sets are re-based on master branch, available in git://github.com/kvalo/ath6kl.git
Raja Mani (7):
ath6kl: Add wmi functions to add/delete wow patterns
ath6kl: Add wmi functions to configure wow mode and host sleep mode
ath6kl: Introduce new variable to track wow state machine
ath6kl: Add new functions to handle wow suspend/resume operations
ath6kl: Invoke wow suspend/resume calls during PM operations
ath6kl: Perform WOW resume in RX path in case of SDIO IRQ wakeup
ath6kl: Expose ath6kl wow capabilities to cfg layer
drivers/net/wireless/ath/ath6kl/cfg80211.c | 163 ++++++++++++++++++++++++++++
drivers/net/wireless/ath/ath6kl/core.h | 13 ++-
drivers/net/wireless/ath/ath6kl/hif-ops.h | 5 +
drivers/net/wireless/ath/ath6kl/hif.h | 1 +
drivers/net/wireless/ath/ath6kl/init.c | 2 +-
drivers/net/wireless/ath/ath6kl/sdio.c | 18 +++-
drivers/net/wireless/ath/ath6kl/txrx.c | 2 +
drivers/net/wireless/ath/ath6kl/wmi.c | 141 ++++++++++++++++++++++++
drivers/net/wireless/ath/ath6kl/wmi.h | 47 ++++++++
9 files changed, 389 insertions(+), 3 deletions(-)
On Friday 28 October 2011 05:49 PM, Kalle Valo wrote:
> On 10/25/2011 01:37 PM, [email protected] wrote:
>> From: Raja Mani<[email protected]>
>>
>> Signed-off-by: Raja Mani<[email protected]>
>
> Empty commit log.
I'll add commit log here..
>
>> +static void ath6kl_wmi_relinquish_implicit_pstream_credits(struct wmi *wmi)
>> +{
>> + u16 active_tsids;
>> + u8 stream_exist;
>> + int i;
>> +
>> + /*
>> + * Relinquish credits from all implicitly created pstreams
>> + * since when we go to sleep. If user created explicit
>> + * thinstreams exists with in a fatpipe leave them intact
>> + * for the user to delete.
>> + */
>> + spin_lock_bh(&wmi->lock);
>> + stream_exist = wmi->fat_pipe_exist;
>> + spin_unlock_bh(&wmi->lock);
>> +
>> + for (i = 0; i< WMM_NUM_AC; i++) {
>> + if (stream_exist& (1<< i)) {
>> +
>> + spin_lock_bh(&wmi->lock);
>> + active_tsids = wmi->stream_exist_for_ac[i];
>> + spin_unlock_bh(&wmi->lock);
>> +
>> + /*
>> + * If there are no user created thin streams
>> + * delete the fatpipe
>> + */
>> + if (!active_tsids) {
>> + stream_exist&= ~(1<< i);
>> + /*
>> + * Indicate inactivity to driver layer for
>> + * this fatpipe (pstream)
>> + */
>> + ath6kl_indicate_tx_activity(wmi->parent_dev,
>> + i, false);
>> + }
>> + }
>> + }
>> +
>> + spin_lock_bh(&wmi->lock);
>> + wmi->fat_pipe_exist = stream_exist;
>> + spin_unlock_bh(&wmi->lock);
>
> The locking here doesn't really make sense. For example, any changes to
> wmi->fat_pipe_exist during the for loop will be overwritten. Also lock
> handling with wmi->stream_exist_for_ac[i] looks fishy.
Is it fine just removal of both locking (For "active_tsids =
wmi->stream_exist_for_ac[i]" and "wmi->fat_pipe_exist = stream_exist") ?
I am sure about the impact..Could you please give some point how above
locking can be improved ?
>
>> +int ath6kl_wmi_set_host_sleep_mode_cmd(struct wmi *wmi,
>> + struct wmi_set_host_sleep_mode_cmd *host_mode)
>
> Don't use the struct as a parameter, you could instead provide an enum
> like this:
>
> enum ath6kl_host_mode {
> ATH6KL_HOST_MODE_AWAKE,
> ATH6KL_HOST_MODE_ASLEEP,
> };
>
Okay.
>> +{
>> + struct sk_buff *skb;
>> + struct wmi_set_host_sleep_mode_cmd *cmd;
>> + int ret;
>> +
>> + if (host_mode->awake == host_mode->asleep)
>> + return -EINVAL;
>
> ...and then you can remove this test.
Fine.
>
>> +int ath6kl_wmi_set_wow_mode_cmd(struct wmi *wmi,
>> + struct wmi_set_wow_mode_cmd *wow_mode)
>
> I'm sure you can guess what I'm about to say here ;)
:) Got it..
>
> Kalle
On 10/25/2011 01:37 PM, [email protected] wrote:
> From: Raja Mani <[email protected]>
>
> Signed-off-by: Raja Mani <[email protected]>
Empty commit log.
> +int ath6kl_pm_wow_suspend(struct ath6kl *ar, struct cfg80211_wowlan *wow)
> +{
> + struct wmi_set_wow_mode_cmd wakeup_filter_cmd;
> + struct wmi_add_wow_pattern_cmd add_pattern_cmd;
> + struct wmi_del_wow_pattern_cmd del_pattern_cmd;
> + struct wmi_set_host_sleep_mode_cmd hsleep_cmd;
> + int i, ret, pos, left;
> + u8 mask[WOW_PATTERN_SIZE];
> +
> + if (WARN_ON(!wow))
> + return -EINVAL;
Unnecessary null check, wow is non-zero here.
> + for (pos = 0; pos < wow->patterns[i].pattern_len;
> + pos++) {
> + if (wow->patterns[i].mask[pos / 8] & (0x1 << (pos % 8)))
> + mask[pos] = 0xFF;
This loop needs a comment.
> + if (ar->tx_pending[ar->ctrl_ep]) {
> + left = wait_event_interruptible_timeout(ar->event_wq,
> + ar->tx_pending[ar->ctrl_ep] == 0, WMI_TIMEOUT);
> + if (left == 0) {
> + ret = -ETIMEDOUT;
> + goto wow_setup_failed;
> + } else if (left < 0) {
> + ret = left;
> + goto wow_setup_failed;
A warning message for both of these error case would be nice.
> +#define WOW_HOST_REQ_DELAY 500 /* 500 ms */
No point of duplicating the value, "/* ms */" is enough.
Kalle
On Friday 28 October 2011 06:48 PM, Kalle Valo wrote:
> On 10/25/2011 01:37 PM, [email protected] wrote:
>> From: Raja Mani<[email protected]>
>>
>> Signed-off-by: Raja Mani<[email protected]>
>
> Empty commit log.
I'll add commit message here..
>
>> --- a/drivers/net/wireless/ath/ath6kl/txrx.c
>> +++ b/drivers/net/wireless/ath/ath6kl/txrx.c
>> @@ -1081,6 +1081,8 @@ void ath6kl_rx(struct htc_target *target, struct htc_packet *packet)
>> return;
>> }
>>
>> + ath6kl_pm_check_wow_status(ar);
>
> Now we resume from two places, cfg80211 resume handler and here. Why do
> we need to do that? Why isn't cfg80211 resume handler enough?
This path hits when the target wants to wake up the host. When given WOW
pattern matches, the target will pull SDIO data line to wake up the
host. During that time , the control will reach here (sdio irq handler
-> ath6kl_rx function), not to cfg80211 resume.
>
> Kalle
From: Raja Mani <[email protected]>
Signed-off-by: Raja Mani <[email protected]>
---
drivers/net/wireless/ath/ath6kl/cfg80211.c | 121 ++++++++++++++++++++++++++++
drivers/net/wireless/ath/ath6kl/core.h | 3 +
2 files changed, 124 insertions(+), 0 deletions(-)
diff --git a/drivers/net/wireless/ath/ath6kl/cfg80211.c b/drivers/net/wireless/ath/ath6kl/cfg80211.c
index 0680d2b..aaf8d0f 100644
--- a/drivers/net/wireless/ath/ath6kl/cfg80211.c
+++ b/drivers/net/wireless/ath/ath6kl/cfg80211.c
@@ -1481,6 +1481,127 @@ static int ath6kl_flush_pmksa(struct wiphy *wiphy, struct net_device *netdev)
}
#ifdef CONFIG_PM
+
+int ath6kl_pm_wow_suspend(struct ath6kl *ar, struct cfg80211_wowlan *wow)
+{
+ struct wmi_set_wow_mode_cmd wakeup_filter_cmd;
+ struct wmi_add_wow_pattern_cmd add_pattern_cmd;
+ struct wmi_del_wow_pattern_cmd del_pattern_cmd;
+ struct wmi_set_host_sleep_mode_cmd hsleep_cmd;
+ int i, ret, pos, left;
+ u8 mask[WOW_PATTERN_SIZE];
+
+ if (WARN_ON(!wow))
+ return -EINVAL;
+
+ if (ar->wow_state != ATH6KL_WOW_STATE_NONE)
+ return -EINVAL;
+
+ ar->wow_state = ATH6KL_WOW_STATE_SUSPENDING;
+
+ /* Clear existing WOW patterns */
+ for (i = 0; i < WOW_MAX_FILTERS_PER_LIST; i++) {
+ del_pattern_cmd.filter_list_id = cpu_to_le16(WOW_LIST_ID);
+ del_pattern_cmd.filter_id = cpu_to_le16(i);
+ ath6kl_wmi_del_wow_pattern_cmd(ar->wmi, &del_pattern_cmd);
+ }
+
+ /* Configure new WOW patterns */
+ memset(&wakeup_filter_cmd, 0, sizeof(wakeup_filter_cmd));
+ wakeup_filter_cmd.enable_wow = cpu_to_le32(0x1);
+ wakeup_filter_cmd.host_req_delay = cpu_to_le32(WOW_HOST_REQ_DELAY);
+
+ if (wow->disconnect)
+ wakeup_filter_cmd.filter |=
+ cpu_to_le32(WOW_FILTER_OPTION_NWK_DISASSOC);
+
+ if (wow->magic_pkt)
+ wakeup_filter_cmd.filter |=
+ cpu_to_le32(WOW_FILTER_OPTION_MAGIC_PACKET);
+
+ if (wow->gtk_rekey_failure)
+ wakeup_filter_cmd.filter |=
+ cpu_to_le32(WOW_FILTER_OPTION_GTK_ERROR);
+
+ if (wow->eap_identity_req)
+ wakeup_filter_cmd.filter |=
+ cpu_to_le32(WOW_FILTER_OPTION_EAP_REQ);
+
+ if (wow->four_way_handshake)
+ wakeup_filter_cmd.filter |=
+ cpu_to_le32(WOW_FILTER_OPTION_8021X_4WAYHS);
+
+ for (i = 0; i < wow->n_patterns; i++) {
+ memset(&add_pattern_cmd, 0, sizeof(add_pattern_cmd));
+ add_pattern_cmd.filter_list_id = WOW_LIST_ID;
+ add_pattern_cmd.filter_offset = 0;
+ add_pattern_cmd.filter_size = wow->patterns[i].pattern_len;
+
+ memset(&mask, 0, sizeof(mask));
+ for (pos = 0; pos < wow->patterns[i].pattern_len;
+ pos++) {
+ if (wow->patterns[i].mask[pos / 8] & (0x1 << (pos % 8)))
+ mask[pos] = 0xFF;
+ }
+
+ ret = ath6kl_wmi_add_wow_pattern_cmd(ar->wmi,
+ &add_pattern_cmd,
+ wow->patterns[i].pattern, mask);
+ if (ret)
+ return ret;
+ }
+
+ if (wakeup_filter_cmd.filter || wow->n_patterns) {
+ ret = ath6kl_wmi_set_wow_mode_cmd(ar->wmi, &wakeup_filter_cmd);
+ if (ret)
+ goto wow_setup_failed;
+ }
+
+ hsleep_cmd.awake = cpu_to_le32(0);
+ hsleep_cmd.asleep = cpu_to_le32(1);
+ ret = ath6kl_wmi_set_host_sleep_mode_cmd(ar->wmi, &hsleep_cmd);
+ if (ret)
+ goto wow_setup_failed;
+
+ if (ar->tx_pending[ar->ctrl_ep]) {
+ left = wait_event_interruptible_timeout(ar->event_wq,
+ ar->tx_pending[ar->ctrl_ep] == 0, WMI_TIMEOUT);
+ if (left == 0) {
+ ret = -ETIMEDOUT;
+ goto wow_setup_failed;
+ } else if (left < 0) {
+ ret = left;
+ goto wow_setup_failed;
+ }
+ }
+
+ ar->wow_state = ATH6KL_WOW_STATE_SUSPENDED;
+ return 0;
+
+wow_setup_failed:
+ ar->wow_state = ATH6KL_WOW_STATE_NONE;
+ return ret;
+}
+
+int ath6kl_pm_wow_resume(struct ath6kl *ar)
+{
+ struct wmi_set_host_sleep_mode_cmd hsleep_cmd;
+ int ret;
+
+ if (ar->wow_state == ATH6KL_WOW_STATE_NONE)
+ return -EINVAL;
+
+ hsleep_cmd.awake = cpu_to_le32(1);
+ hsleep_cmd.asleep = cpu_to_le32(0);
+ ret = ath6kl_wmi_set_host_sleep_mode_cmd(ar->wmi, &hsleep_cmd);
+ if (ret)
+ return ret;
+
+ ar->wow_state = ATH6KL_WOW_STATE_NONE;
+
+ return 0;
+}
+
static int ar6k_cfg80211_suspend(struct wiphy *wiphy,
struct cfg80211_wowlan *wow)
{
diff --git a/drivers/net/wireless/ath/ath6kl/core.h b/drivers/net/wireless/ath/ath6kl/core.h
index 706443c..cefb233 100644
--- a/drivers/net/wireless/ath/ath6kl/core.h
+++ b/drivers/net/wireless/ath/ath6kl/core.h
@@ -386,6 +386,9 @@ enum ath6kl_wow_state {
ATH6KL_WOW_STATE_SUSPENDED
};
+#define WOW_LIST_ID 0
+#define WOW_HOST_REQ_DELAY 500 /* 500 ms */
+
/* Flag info */
#define WMI_ENABLED 0
#define WMI_READY 1
--
1.7.0.4
From: Raja Mani <[email protected]>
Signed-off-by: Raja Mani <[email protected]>
---
drivers/net/wireless/ath/ath6kl/wmi.c | 53 +++++++++++++++++++++++++++++++++
drivers/net/wireless/ath/ath6kl/wmi.h | 17 ++++++++++
2 files changed, 70 insertions(+), 0 deletions(-)
diff --git a/drivers/net/wireless/ath/ath6kl/wmi.c b/drivers/net/wireless/ath/ath6kl/wmi.c
index 7b6bfdd..f38b30e 100644
--- a/drivers/net/wireless/ath/ath6kl/wmi.c
+++ b/drivers/net/wireless/ath/ath6kl/wmi.c
@@ -2356,6 +2356,59 @@ int ath6kl_wmi_set_ip_cmd(struct wmi *wmi, struct wmi_set_ip_cmd *ip_cmd)
return ret;
}
+int ath6kl_wmi_add_wow_pattern_cmd(struct wmi *wmi,
+ struct wmi_add_wow_pattern_cmd *add_wow_cmd,
+ u8 *pattern, u8 *mask)
+{
+ struct sk_buff *skb;
+ struct wmi_add_wow_pattern_cmd *cmd;
+ u16 size;
+ u8 *filter_mask;
+ int ret;
+
+ size = sizeof(*cmd) +
+ ((2 * add_wow_cmd->filter_size) * sizeof(u8));
+
+ skb = ath6kl_wmi_get_new_buf(size);
+ if (!skb)
+ return -ENOMEM;
+
+ cmd = (struct wmi_add_wow_pattern_cmd *) skb->data;
+ cmd->filter_list_id = add_wow_cmd->filter_list_id;
+ cmd->filter_offset = add_wow_cmd->filter_offset;
+ cmd->filter_size = add_wow_cmd->filter_size;
+
+ memcpy(cmd->filter, pattern, add_wow_cmd->filter_size);
+
+ filter_mask = (u8 *) (cmd->filter + cmd->filter_size);
+ memcpy(filter_mask, mask, add_wow_cmd->filter_size);
+
+ ret = ath6kl_wmi_cmd_send(wmi, skb, WMI_ADD_WOW_PATTERN_CMDID,
+ NO_SYNC_WMIFLAG);
+
+ return ret;
+}
+
+int ath6kl_wmi_del_wow_pattern_cmd(struct wmi *wmi,
+ struct wmi_del_wow_pattern_cmd *del_wow_cmd)
+{
+ struct sk_buff *skb;
+ struct wmi_del_wow_pattern_cmd *cmd;
+ int ret;
+
+ skb = ath6kl_wmi_get_new_buf(sizeof(*cmd));
+ if (!skb)
+ return -ENOMEM;
+
+ cmd = (struct wmi_del_wow_pattern_cmd *) skb->data;
+ memcpy(cmd, del_wow_cmd, sizeof(*cmd));
+
+ ret = ath6kl_wmi_cmd_send(wmi, skb, WMI_DEL_WOW_PATTERN_CMDID,
+ NO_SYNC_WMIFLAG);
+
+ return ret;
+}
+
static int ath6kl_wmi_get_wow_list_event_rx(struct wmi *wmi, u8 * datap,
int len)
{
diff --git a/drivers/net/wireless/ath/ath6kl/wmi.h b/drivers/net/wireless/ath/ath6kl/wmi.h
index f0ca899..3a54933 100644
--- a/drivers/net/wireless/ath/ath6kl/wmi.h
+++ b/drivers/net/wireless/ath/ath6kl/wmi.h
@@ -1796,6 +1796,18 @@ struct wmi_set_ip_cmd {
__le32 ips[MAX_IP_ADDRS];
} __packed;
+struct wmi_add_wow_pattern_cmd {
+ u8 filter_list_id;
+ u8 filter_size;
+ u8 filter_offset;
+ u8 filter[1];
+} __packed;
+
+struct wmi_del_wow_pattern_cmd {
+ __le16 filter_list_id;
+ __le16 filter_id;
+} __packed;
+
/* WMI_GET_WOW_LIST_CMD reply */
struct wmi_get_wow_list_reply {
/* number of patterns in reply */
@@ -2243,6 +2255,11 @@ int ath6kl_wmi_test_cmd(struct wmi *wmi, void *buf, size_t len);
s32 ath6kl_wmi_get_rate(s8 rate_index);
int ath6kl_wmi_set_ip_cmd(struct wmi *wmi, struct wmi_set_ip_cmd *ip_cmd);
+int ath6kl_wmi_add_wow_pattern_cmd(struct wmi *wmi,
+ struct wmi_add_wow_pattern_cmd *add_wow_cmd,
+ u8 *pattern, u8 *mask);
+int ath6kl_wmi_del_wow_pattern_cmd(struct wmi *wmi,
+ struct wmi_del_wow_pattern_cmd *del_wow_cmd);
int ath6kl_wmi_set_roam_lrssi_cmd(struct wmi *wmi, u8 lrssi);
int ath6kl_wmi_force_roam_cmd(struct wmi *wmi, const u8 *bssid);
int ath6kl_wmi_set_roam_mode_cmd(struct wmi *wmi, enum wmi_roam_mode mode);
--
1.7.0.4
On Friday 28 October 2011 06:01 PM, Kalle Valo wrote:
> On 10/25/2011 01:37 PM, [email protected] wrote:
>> From: Raja Mani<[email protected]>
>>
>> Signed-off-by: Raja Mani<[email protected]>
>
> Empty commit log.
>
>> +int ath6kl_pm_wow_suspend(struct ath6kl *ar, struct cfg80211_wowlan *wow)
>> +{
>> + struct wmi_set_wow_mode_cmd wakeup_filter_cmd;
>> + struct wmi_add_wow_pattern_cmd add_pattern_cmd;
>> + struct wmi_del_wow_pattern_cmd del_pattern_cmd;
>> + struct wmi_set_host_sleep_mode_cmd hsleep_cmd;
>> + int i, ret, pos, left;
>> + u8 mask[WOW_PATTERN_SIZE];
>> +
>> + if (WARN_ON(!wow))
>> + return -EINVAL;
>
> Unnecessary null check, wow is non-zero here.
>
>> + for (pos = 0; pos< wow->patterns[i].pattern_len;
>> + pos++) {
>> + if (wow->patterns[i].mask[pos / 8]& (0x1<< (pos % 8)))
>> + mask[pos] = 0xFF;
>
> This loop needs a comment.
>
>> + if (ar->tx_pending[ar->ctrl_ep]) {
>> + left = wait_event_interruptible_timeout(ar->event_wq,
>> + ar->tx_pending[ar->ctrl_ep] == 0, WMI_TIMEOUT);
>> + if (left == 0) {
>> + ret = -ETIMEDOUT;
>> + goto wow_setup_failed;
>> + } else if (left< 0) {
>> + ret = left;
>> + goto wow_setup_failed;
>
> A warning message for both of these error case would be nice.
>
>> +#define WOW_HOST_REQ_DELAY 500 /* 500 ms */
>
> No point of duplicating the value, "/* ms */" is enough.
>
> Kalle
Will fix all your above comments in V2..
On 10/25/2011 01:37 PM, [email protected] wrote:
> From: Raja Mani <[email protected]>
>
> Signed-off-by: Raja Mani <[email protected]>
Empty commit log.
> --- a/drivers/net/wireless/ath/ath6kl/txrx.c
> +++ b/drivers/net/wireless/ath/ath6kl/txrx.c
> @@ -1081,6 +1081,8 @@ void ath6kl_rx(struct htc_target *target, struct htc_packet *packet)
> return;
> }
>
> + ath6kl_pm_check_wow_status(ar);
Now we resume from two places, cfg80211 resume handler and here. Why do
we need to do that? Why isn't cfg80211 resume handler enough?
Kalle
On 10/25/2011 01:37 PM, [email protected] wrote:
> From: Raja Mani <[email protected]>
>
> Signed-off-by: Raja Mani <[email protected]>
Empty commit log.
> +static void ath6kl_wmi_relinquish_implicit_pstream_credits(struct wmi *wmi)
> +{
> + u16 active_tsids;
> + u8 stream_exist;
> + int i;
> +
> + /*
> + * Relinquish credits from all implicitly created pstreams
> + * since when we go to sleep. If user created explicit
> + * thinstreams exists with in a fatpipe leave them intact
> + * for the user to delete.
> + */
> + spin_lock_bh(&wmi->lock);
> + stream_exist = wmi->fat_pipe_exist;
> + spin_unlock_bh(&wmi->lock);
> +
> + for (i = 0; i < WMM_NUM_AC; i++) {
> + if (stream_exist & (1 << i)) {
> +
> + spin_lock_bh(&wmi->lock);
> + active_tsids = wmi->stream_exist_for_ac[i];
> + spin_unlock_bh(&wmi->lock);
> +
> + /*
> + * If there are no user created thin streams
> + * delete the fatpipe
> + */
> + if (!active_tsids) {
> + stream_exist &= ~(1 << i);
> + /*
> + * Indicate inactivity to driver layer for
> + * this fatpipe (pstream)
> + */
> + ath6kl_indicate_tx_activity(wmi->parent_dev,
> + i, false);
> + }
> + }
> + }
> +
> + spin_lock_bh(&wmi->lock);
> + wmi->fat_pipe_exist = stream_exist;
> + spin_unlock_bh(&wmi->lock);
The locking here doesn't really make sense. For example, any changes to
wmi->fat_pipe_exist during the for loop will be overwritten. Also lock
handling with wmi->stream_exist_for_ac[i] looks fishy.
> +int ath6kl_wmi_set_host_sleep_mode_cmd(struct wmi *wmi,
> + struct wmi_set_host_sleep_mode_cmd *host_mode)
Don't use the struct as a parameter, you could instead provide an enum
like this:
enum ath6kl_host_mode {
ATH6KL_HOST_MODE_AWAKE,
ATH6KL_HOST_MODE_ASLEEP,
};
> +{
> + struct sk_buff *skb;
> + struct wmi_set_host_sleep_mode_cmd *cmd;
> + int ret;
> +
> + if (host_mode->awake == host_mode->asleep)
> + return -EINVAL;
...and then you can remove this test.
> +int ath6kl_wmi_set_wow_mode_cmd(struct wmi *wmi,
> + struct wmi_set_wow_mode_cmd *wow_mode)
I'm sure you can guess what I'm about to say here ;)
Kalle
On Friday 28 October 2011 05:41 PM, Kalle Valo wrote:
> On 10/25/2011 01:37 PM, [email protected] wrote:
>> From: Raja Mani<[email protected]>
>>
>> Signed-off-by: Raja Mani<[email protected]>
>
> Try to avoid empty commit logs. You could just say that these commands
> will be used by the following patch if nothing else.
>
>> +int ath6kl_wmi_add_wow_pattern_cmd(struct wmi *wmi,
>> + struct wmi_add_wow_pattern_cmd *add_wow_cmd,
>> + u8 *pattern, u8 *mask)
>> +{
>
> Don't use a struct as the parameter, instead add individual parameters.
> So something like this:
>
> int ath6kl_wmi_add_wow_pattern_cmd(struct wmi *wmi,
> u8 id, u8 size, 8 offset,
> u8 *pattern, u8 *mask)
>
> This is easier for the caller and you get to handle the endian in the
> callee which simplifies the code.
Agree.. I'll take care in V2.
>
>> + size = sizeof(*cmd) +
>> + ((2 * add_wow_cmd->filter_size) * sizeof(u8));
>
> This can fit into line. And sizeof(u8) really doesn't make any sense.
>
> And is this correct? The struct is defined like this:
>
> +struct wmi_add_wow_pattern_cmd {
> + u8 filter_list_id;
> + u8 filter_size;
> + u8 filter_offset;
> + u8 filter[1];
> +} __packed;
>
> So there's one extra byte for the filter and above you include also that
> byte. But if the sctruct is defined like this the extra byte is not
> included:
>
> +struct wmi_add_wow_pattern_cmd {
> + u8 filter_list_id;
> + u8 filter_size;
> + u8 filter_offset;
> + u8 filter[0];
> +} __packed;
>
Good catch.. I'll correct it.
>> +int ath6kl_wmi_del_wow_pattern_cmd(struct wmi *wmi,
>> + struct wmi_del_wow_pattern_cmd *del_wow_cmd)
>
> Same here as earlier, don't use the struct as a parameter.
Okay..
>
> Kalle
From: Raja Mani <[email protected]>
Set the list of ath6kl's WOW trigger options in wiphy->wowlan.flags
variable during wiphy registration. So that, those options can be
configured via iw.
Signed-off-by: Raja Mani <[email protected]>
---
drivers/net/wireless/ath/ath6kl/cfg80211.c | 10 ++++++++++
1 files changed, 10 insertions(+), 0 deletions(-)
diff --git a/drivers/net/wireless/ath/ath6kl/cfg80211.c b/drivers/net/wireless/ath/ath6kl/cfg80211.c
index 241b13d..da4a2ff 100644
--- a/drivers/net/wireless/ath/ath6kl/cfg80211.c
+++ b/drivers/net/wireless/ath/ath6kl/cfg80211.c
@@ -2117,6 +2117,16 @@ struct wireless_dev *ath6kl_cfg80211_init(struct device *dev)
wdev->wiphy->cipher_suites = cipher_suites;
wdev->wiphy->n_cipher_suites = ARRAY_SIZE(cipher_suites);
+ wdev->wiphy->wowlan.flags = WIPHY_WOWLAN_MAGIC_PKT |
+ WIPHY_WOWLAN_DISCONNECT |
+ WIPHY_WOWLAN_GTK_REKEY_FAILURE |
+ WIPHY_WOWLAN_SUPPORTS_GTK_REKEY |
+ WIPHY_WOWLAN_EAP_IDENTITY_REQ |
+ WIPHY_WOWLAN_4WAY_HANDSHAKE;
+ wdev->wiphy->wowlan.n_patterns = WOW_MAX_FILTERS_PER_LIST;
+ wdev->wiphy->wowlan.pattern_min_len = 1;
+ wdev->wiphy->wowlan.pattern_max_len = WOW_PATTERN_SIZE;
+
ret = wiphy_register(wdev->wiphy);
if (ret < 0) {
ath6kl_err("couldn't register wiphy device\n");
--
1.7.0.4
On Friday 28 October 2011 06:45 PM, Kalle Valo wrote:
> On 10/25/2011 01:37 PM, [email protected] wrote:
>> From: Raja Mani<[email protected]>
>>
>> Link ath6kl's wow suspend/resume functions with the callback functions
>> registered with the CFG layer for suspend and resume operation.
>
> [..]
>
>> --- a/drivers/net/wireless/ath/ath6kl/cfg80211.c
>> +++ b/drivers/net/wireless/ath/ath6kl/cfg80211.c
>> @@ -1606,6 +1606,19 @@ static int ar6k_cfg80211_suspend(struct wiphy *wiphy,
>> struct cfg80211_wowlan *wow)
>> {
>> struct ath6kl *ar = wiphy_priv(wiphy);
>> + int ret;
>> +
>> + if (wow&& ath6kl_cfg80211_ready(ar)&&
>> + test_bit(CONNECTED,&ar->flag)&&
>> + ath6kl_hif_keep_pwr_caps(ar)) {
>> +
>> + /* Flush all non control pkts in Tx path */
>> + ath6kl_tx_data_cleanup(ar);
>> +
>> + ret = ath6kl_pm_wow_suspend(ar, wow);
>> + if (ret)
>> + return ret;
>> + }
>
> This is now confusing. Some of the suspend logic is now in sdio.c and
> some here. It's better to have everything in one place, that is in
> sdio.c. We just need to add an interface so that sdio.c can request the
> correct suspend mode.
Do you want me to keep ath6kl_pm_wow_suspend() implementation in
cfg80211.c file and move only above code to ath6kl_sdio_suspend() ?
>
> Kalle
On 10/25/2011 01:37 PM, [email protected] wrote:
> Using these patch sets, WOW patterns can be controlled
> and configured via iw command. Please refer iw help menu for more details.
>
> There are some limitation in the recent "iw" command such as it doesn't
> take the pattern offset where to start pattern matching in the received packets.
> Such a enhancement will be done later..
>
> These patch sets are re-based on master branch, available in git://github.com/kvalo/ath6kl.git
These patches conflict with multi vif so you need to rebase your
patches. I also saw few sparse warnings:
drivers/net/wireless/ath/ath6kl/cfg80211.c:1512:42: warning: incorrect
type in assignment (different base types)
drivers/net/wireless/ath/ath6kl/cfg80211.c:1512:42: expected
restricted __le16 [addressable] [assigned] [usertype] host_req_delay
drivers/net/wireless/ath/ath6kl/cfg80211.c:1512:42: got restricted
__le32 [usertype] <noident>
drivers/net/wireless/ath/ath6kl/cfg80211.c:1485:5: warning: symbol
'ath6kl_pm_wow_suspend' was not declared. Should it be static?
drivers/net/wireless/ath/ath6kl/cfg80211.c:1586:5: warning: symbol
'ath6kl_pm_wow_resume' was not declared. Should it be static?
Kalle
On Friday 28 October 2011 06:51 PM, Kalle Valo wrote:
> On 10/25/2011 01:37 PM, [email protected] wrote:
>
>> + if (wow&& ath6kl_cfg80211_ready(ar)&&
>> + test_bit(CONNECTED,&ar->flag)&&
>> + ath6kl_hif_keep_pwr_caps(ar)) {
>> +
>> + /* Flush all non control pkts in Tx path */
>> + ath6kl_tx_data_cleanup(ar);
>> +
>> + ret = ath6kl_pm_wow_suspend(ar, wow);
>> + if (ret)
>> + return ret;
>> + }
>
> Forgot to mention: don't you also need to check for MMC_PM_WAKE_SDIO_IRQ
> and also set it with sdio_set_host_pm_flags()?
I tested it in x86 machine. Wow suspend/resume works even without
setting MMC_PM_WAKE_SDIO_IRQ. Should i really handle it ?
>
> Kalle
From: Raja Mani <[email protected]>
Add new variable wow_state to maintain the current WOW state machine.
Signed-off-by: Raja Mani <[email protected]>
---
drivers/net/wireless/ath/ath6kl/core.h | 8 ++++++++
drivers/net/wireless/ath/ath6kl/init.c | 2 +-
2 files changed, 9 insertions(+), 1 deletions(-)
diff --git a/drivers/net/wireless/ath/ath6kl/core.h b/drivers/net/wireless/ath/ath6kl/core.h
index 31e5c7e..706443c 100644
--- a/drivers/net/wireless/ath/ath6kl/core.h
+++ b/drivers/net/wireless/ath/ath6kl/core.h
@@ -380,6 +380,12 @@ struct ath6kl_req_key {
u8 key_len;
};
+enum ath6kl_wow_state {
+ ATH6KL_WOW_STATE_NONE,
+ ATH6KL_WOW_STATE_SUSPENDING,
+ ATH6KL_WOW_STATE_SUSPENDED
+};
+
/* Flag info */
#define WMI_ENABLED 0
#define WMI_READY 1
@@ -517,6 +523,8 @@ struct ath6kl {
u16 assoc_bss_beacon_int;
u8 assoc_bss_dtim_period;
+ enum ath6kl_wow_state wow_state;
+
#ifdef CONFIG_ATH6KL_DEBUG
struct {
struct circ_buf fwlog_buf;
diff --git a/drivers/net/wireless/ath/ath6kl/init.c b/drivers/net/wireless/ath/ath6kl/init.c
index 1700423..6781531 100644
--- a/drivers/net/wireless/ath/ath6kl/init.c
+++ b/drivers/net/wireless/ath/ath6kl/init.c
@@ -594,7 +594,7 @@ struct ath6kl *ath6kl_core_alloc(struct device *sdev)
set_bit(WLAN_ENABLED, &ar->flag);
ar->wlan_pwr_state = WLAN_POWER_STATE_ON;
-
+ ar->wow_state = ATH6KL_WOW_STATE_NONE;
spin_lock_init(&ar->lock);
ath6kl_init_control_info(ar);
--
1.7.0.4
On 10/31/2011 11:55 AM, Raja Mani wrote:
> On Friday 28 October 2011 06:48 PM, Kalle Valo wrote:
>> On 10/25/2011 01:37 PM, [email protected] wrote:
>>> --- a/drivers/net/wireless/ath/ath6kl/txrx.c
>>> +++ b/drivers/net/wireless/ath/ath6kl/txrx.c
>>> @@ -1081,6 +1081,8 @@ void ath6kl_rx(struct htc_target *target, struct htc_packet *packet)
>>> return;
>>> }
>>>
>>> + ath6kl_pm_check_wow_status(ar);
>>
>> Now we resume from two places, cfg80211 resume handler and here. Why do
>> we need to do that? Why isn't cfg80211 resume handler enough?
>
> This path hits when the target wants to wake up the host. When given WOW
> pattern matches, the target will pull SDIO data line to wake up the
> host. During that time , the control will reach here (sdio irq handler
> -> ath6kl_rx function), not to cfg80211 resume.
I don't understand this. Are you saying that host resumes but cfg80211
resume handler is not called? It doesn't make sense, every suspend call
should have an equivalent resume call. If that doesn't happen, something
is broken somewhere.
Kalle
On 10/25/2011 01:37 PM, [email protected] wrote:
> + if (wow && ath6kl_cfg80211_ready(ar) &&
> + test_bit(CONNECTED, &ar->flag) &&
> + ath6kl_hif_keep_pwr_caps(ar)) {
> +
> + /* Flush all non control pkts in Tx path */
> + ath6kl_tx_data_cleanup(ar);
> +
> + ret = ath6kl_pm_wow_suspend(ar, wow);
> + if (ret)
> + return ret;
> + }
Forgot to mention: don't you also need to check for MMC_PM_WAKE_SDIO_IRQ
and also set it with sdio_set_host_pm_flags()?
Kalle
On Friday 28 October 2011 05:28 PM, Kalle Valo wrote:
> On 10/25/2011 01:37 PM, [email protected] wrote:
>
>> Using these patch sets, WOW patterns can be controlled
>> and configured via iw command. Please refer iw help menu for more details.
>>
>> There are some limitation in the recent "iw" command such as it doesn't
>> take the pattern offset where to start pattern matching in the received packets.
>> Such a enhancement will be done later..
>>
>> These patch sets are re-based on master branch, available in git://github.com/kvalo/ath6kl.git
>
> These patches conflict with multi vif so you need to rebase your
> patches. I also saw few sparse warnings:
Thanks for your review, I'll fix below sparse warnings and rebase &
submit V2.
>
> drivers/net/wireless/ath/ath6kl/cfg80211.c:1512:42: warning: incorrect
> type in assignment (different base types)
> drivers/net/wireless/ath/ath6kl/cfg80211.c:1512:42: expected
> restricted __le16 [addressable] [assigned] [usertype] host_req_delay
> drivers/net/wireless/ath/ath6kl/cfg80211.c:1512:42: got restricted
> __le32 [usertype]<noident>
> drivers/net/wireless/ath/ath6kl/cfg80211.c:1485:5: warning: symbol
> 'ath6kl_pm_wow_suspend' was not declared. Should it be static?
> drivers/net/wireless/ath/ath6kl/cfg80211.c:1586:5: warning: symbol
> 'ath6kl_pm_wow_resume' was not declared. Should it be static?
>
> Kalle
On 10/31/2011 11:54 AM, Raja Mani wrote:
> On Friday 28 October 2011 06:45 PM, Kalle Valo wrote:
>> On 10/25/2011 01:37 PM, [email protected] wrote:
>>> From: Raja Mani<[email protected]>
>>>
>>> --- a/drivers/net/wireless/ath/ath6kl/cfg80211.c
>>> +++ b/drivers/net/wireless/ath/ath6kl/cfg80211.c
>>> @@ -1606,6 +1606,19 @@ static int ar6k_cfg80211_suspend(struct wiphy *wiphy,
>>> struct cfg80211_wowlan *wow)
>>> {
>>> struct ath6kl *ar = wiphy_priv(wiphy);
>>> + int ret;
>>> +
>>> + if (wow&& ath6kl_cfg80211_ready(ar)&&
>>> + test_bit(CONNECTED,&ar->flag)&&
>>> + ath6kl_hif_keep_pwr_caps(ar)) {
>>> +
>>> + /* Flush all non control pkts in Tx path */
>>> + ath6kl_tx_data_cleanup(ar);
>>> +
>>> + ret = ath6kl_pm_wow_suspend(ar, wow);
>>> + if (ret)
>>> + return ret;
>>> + }
>>
>> This is now confusing. Some of the suspend logic is now in sdio.c and
>> some here. It's better to have everything in one place, that is in
>> sdio.c. We just need to add an interface so that sdio.c can request the
>> correct suspend mode.
>
> Do you want me to keep ath6kl_pm_wow_suspend() implementation in
> cfg80211.c file and move only above code to ath6kl_sdio_suspend() ?
I gave this more thought and in my suspend patches earlier today I added
this function:
int ath6kl_cfg80211_suspend(struct ath6kl *ar,
enum ath6kl_cfg_suspend_mode mode)
{
int ret;
ath6kl_cfg80211_stop(ar);
switch (mode) {
case ATH6KL_CFG_SUSPEND_DEEPSLEEP:
/* save the current power mode before enabling power save */
ar->wmi->saved_pwr_mode = ar->wmi->pwr_mode;
ret = ath6kl_wmi_powermode_cmd(ar->wmi, 0, REC_POWER);
if (ret) {
ath6kl_warn("wmi powermode command failed during suspend: %d\n",
ret);
}
ar->state = ATH6KL_STATE_DEEPSLEEP;
break;
You can add a new mode for WOW and call your wow functions from that
function. And you need to make changes to ath6kl_sdio_suspend() so that
it will enable WOW in correct cases. Also I added a state variable
ar->state and hopefully we don't need a separate wow state anymore.
Kalle
On 10/25/2011 01:37 PM, [email protected] wrote:
> From: Raja Mani <[email protected]>
>
> Signed-off-by: Raja Mani <[email protected]>
Try to avoid empty commit logs. You could just say that these commands
will be used by the following patch if nothing else.
> +int ath6kl_wmi_add_wow_pattern_cmd(struct wmi *wmi,
> + struct wmi_add_wow_pattern_cmd *add_wow_cmd,
> + u8 *pattern, u8 *mask)
> +{
Don't use a struct as the parameter, instead add individual parameters.
So something like this:
int ath6kl_wmi_add_wow_pattern_cmd(struct wmi *wmi,
u8 id, u8 size, 8 offset,
u8 *pattern, u8 *mask)
This is easier for the caller and you get to handle the endian in the
callee which simplifies the code.
> + size = sizeof(*cmd) +
> + ((2 * add_wow_cmd->filter_size) * sizeof(u8));
This can fit into line. And sizeof(u8) really doesn't make any sense.
And is this correct? The struct is defined like this:
+struct wmi_add_wow_pattern_cmd {
+ u8 filter_list_id;
+ u8 filter_size;
+ u8 filter_offset;
+ u8 filter[1];
+} __packed;
So there's one extra byte for the filter and above you include also that
byte. But if the sctruct is defined like this the extra byte is not
included:
+struct wmi_add_wow_pattern_cmd {
+ u8 filter_list_id;
+ u8 filter_size;
+ u8 filter_offset;
+ u8 filter[0];
+} __packed;
> +int ath6kl_wmi_del_wow_pattern_cmd(struct wmi *wmi,
> + struct wmi_del_wow_pattern_cmd *del_wow_cmd)
Same here as earlier, don't use the struct as a parameter.
Kalle
On 10/31/2011 11:54 AM, Raja Mani wrote:
> On Friday 28 October 2011 06:51 PM, Kalle Valo wrote:
>> On 10/25/2011 01:37 PM, [email protected] wrote:
>>
>>> + if (wow&& ath6kl_cfg80211_ready(ar)&&
>>> + test_bit(CONNECTED,&ar->flag)&&
>>> + ath6kl_hif_keep_pwr_caps(ar)) {
>>> +
>>> + /* Flush all non control pkts in Tx path */
>>> + ath6kl_tx_data_cleanup(ar);
>>> +
>>> + ret = ath6kl_pm_wow_suspend(ar, wow);
>>> + if (ret)
>>> + return ret;
>>> + }
>>
>> Forgot to mention: don't you also need to check for MMC_PM_WAKE_SDIO_IRQ
>> and also set it with sdio_set_host_pm_flags()?
>
> I tested it in x86 machine. Wow suspend/resume works even without
> setting MMC_PM_WAKE_SDIO_IRQ. Should i really handle it ?
Yes, you should. Otherwise firmware is not able to wake up the host.
Also you should both check that wake sdio irq supported by the sdio
controller. Otherwise we might enable WoW even in cases when sdio
controller doesn't support it and which would result in broken wow
functionality.
Kalle
From: Raja Mani <[email protected]>
Signed-off-by: Raja Mani <[email protected]>
---
drivers/net/wireless/ath/ath6kl/cfg80211.c | 12 ++++++++++++
drivers/net/wireless/ath/ath6kl/core.h | 2 +-
drivers/net/wireless/ath/ath6kl/txrx.c | 2 ++
3 files changed, 15 insertions(+), 1 deletions(-)
diff --git a/drivers/net/wireless/ath/ath6kl/cfg80211.c b/drivers/net/wireless/ath/ath6kl/cfg80211.c
index ddef445..241b13d 100644
--- a/drivers/net/wireless/ath/ath6kl/cfg80211.c
+++ b/drivers/net/wireless/ath/ath6kl/cfg80211.c
@@ -1602,6 +1602,12 @@ int ath6kl_pm_wow_resume(struct ath6kl *ar)
return 0;
}
+void ath6kl_pm_check_wow_status(struct ath6kl *ar)
+{
+ if (ar->wow_state == ATH6KL_WOW_STATE_SUSPENDED)
+ ath6kl_pm_wow_resume(ar);
+}
+
static int ar6k_cfg80211_suspend(struct wiphy *wiphy,
struct cfg80211_wowlan *wow)
{
@@ -1636,6 +1642,12 @@ static int ar6k_cfg80211_resume(struct wiphy *wiphy)
return ath6kl_hif_resume(ar);
}
+
+#else
+
+void ath6kl_pm_check_wow_status(struct ath6kl *ar)
+{
+}
#endif
static int ath6kl_set_channel(struct wiphy *wiphy, struct net_device *dev,
diff --git a/drivers/net/wireless/ath/ath6kl/core.h b/drivers/net/wireless/ath/ath6kl/core.h
index cefb233..4684933 100644
--- a/drivers/net/wireless/ath/ath6kl/core.h
+++ b/drivers/net/wireless/ath/ath6kl/core.h
@@ -651,5 +651,5 @@ void aggr_recv_addba_req_evt(struct ath6kl *ar, u8 tid, u16 seq_no,
u8 win_sz);
void ath6kl_wakeup_event(void *dev);
void ath6kl_target_failure(struct ath6kl *ar);
-
+void ath6kl_pm_check_wow_status(struct ath6kl *ar);
#endif /* CORE_H */
diff --git a/drivers/net/wireless/ath/ath6kl/txrx.c b/drivers/net/wireless/ath/ath6kl/txrx.c
index a9dff01..704ed50 100644
--- a/drivers/net/wireless/ath/ath6kl/txrx.c
+++ b/drivers/net/wireless/ath/ath6kl/txrx.c
@@ -1081,6 +1081,8 @@ void ath6kl_rx(struct htc_target *target, struct htc_packet *packet)
return;
}
+ ath6kl_pm_check_wow_status(ar);
+
if (ept == ar->ctrl_ep) {
ath6kl_wmi_control_rx(ar->wmi, skb);
return;
--
1.7.0.4
From: Raja Mani <[email protected]>
Link ath6kl's wow suspend/resume functions with the callback functions
registered with the CFG layer for suspend and resume operation.
Signed-off-by: Raja Mani <[email protected]>
---
drivers/net/wireless/ath/ath6kl/cfg80211.c | 20 ++++++++++++++++++++
drivers/net/wireless/ath/ath6kl/hif-ops.h | 5 +++++
drivers/net/wireless/ath/ath6kl/hif.h | 1 +
drivers/net/wireless/ath/ath6kl/sdio.c | 18 +++++++++++++++++-
4 files changed, 43 insertions(+), 1 deletions(-)
diff --git a/drivers/net/wireless/ath/ath6kl/cfg80211.c b/drivers/net/wireless/ath/ath6kl/cfg80211.c
index aaf8d0f..ddef445 100644
--- a/drivers/net/wireless/ath/ath6kl/cfg80211.c
+++ b/drivers/net/wireless/ath/ath6kl/cfg80211.c
@@ -1606,6 +1606,19 @@ static int ar6k_cfg80211_suspend(struct wiphy *wiphy,
struct cfg80211_wowlan *wow)
{
struct ath6kl *ar = wiphy_priv(wiphy);
+ int ret;
+
+ if (wow && ath6kl_cfg80211_ready(ar) &&
+ test_bit(CONNECTED, &ar->flag) &&
+ ath6kl_hif_keep_pwr_caps(ar)) {
+
+ /* Flush all non control pkts in Tx path */
+ ath6kl_tx_data_cleanup(ar);
+
+ ret = ath6kl_pm_wow_suspend(ar, wow);
+ if (ret)
+ return ret;
+ }
return ath6kl_hif_suspend(ar);
}
@@ -1613,6 +1626,13 @@ static int ar6k_cfg80211_suspend(struct wiphy *wiphy,
static int ar6k_cfg80211_resume(struct wiphy *wiphy)
{
struct ath6kl *ar = wiphy_priv(wiphy);
+ int ret;
+
+ if (ar->wow_state == ATH6KL_WOW_STATE_SUSPENDED) {
+ ret = ath6kl_pm_wow_resume(ar);
+ if (ret)
+ return ret;
+ }
return ath6kl_hif_resume(ar);
}
diff --git a/drivers/net/wireless/ath/ath6kl/hif-ops.h b/drivers/net/wireless/ath/ath6kl/hif-ops.h
index 95e7303..ab1140d 100644
--- a/drivers/net/wireless/ath/ath6kl/hif-ops.h
+++ b/drivers/net/wireless/ath/ath6kl/hif-ops.h
@@ -96,4 +96,9 @@ static inline int ath6kl_hif_resume(struct ath6kl *ar)
return ar->hif_ops->resume(ar);
}
+
+static inline int ath6kl_hif_keep_pwr_caps(struct ath6kl *ar)
+{
+ return ar->hif_ops->keep_pwr_caps(ar);
+}
#endif
diff --git a/drivers/net/wireless/ath/ath6kl/hif.h b/drivers/net/wireless/ath/ath6kl/hif.h
index 93d2912..fbe3622 100644
--- a/drivers/net/wireless/ath/ath6kl/hif.h
+++ b/drivers/net/wireless/ath/ath6kl/hif.h
@@ -242,6 +242,7 @@ struct ath6kl_hif_ops {
void (*cleanup_scatter)(struct ath6kl *ar);
int (*suspend)(struct ath6kl *ar);
int (*resume)(struct ath6kl *ar);
+ bool (*keep_pwr_caps)(struct ath6kl *ar);
};
int ath6kl_hif_setup(struct ath6kl_device *dev);
diff --git a/drivers/net/wireless/ath/ath6kl/sdio.c b/drivers/net/wireless/ath/ath6kl/sdio.c
index 58e31f6..ec0e58c 100644
--- a/drivers/net/wireless/ath/ath6kl/sdio.c
+++ b/drivers/net/wireless/ath/ath6kl/sdio.c
@@ -739,7 +739,8 @@ static int ath6kl_sdio_suspend(struct ath6kl *ar)
return ret;
}
- ath6kl_deep_sleep_enable(ar);
+ if (ar->wow_state != ATH6KL_WOW_STATE_SUSPENDED)
+ ath6kl_deep_sleep_enable(ar);
return 0;
}
@@ -756,6 +757,20 @@ static int ath6kl_sdio_resume(struct ath6kl *ar)
return 0;
}
+static bool ath6kl_sdio_keep_pwr_caps(struct ath6kl *ar)
+{
+ struct ath6kl_sdio *ar_sdio = ath6kl_sdio_priv(ar);
+ struct sdio_func *func = ar_sdio->func;
+ mmc_pm_flag_t flags;
+
+ flags = sdio_get_host_pm_caps(func);
+
+ if (!(flags & MMC_PM_KEEP_POWER))
+ return false;
+
+ return true;
+}
+
static const struct ath6kl_hif_ops ath6kl_sdio_ops = {
.read_write_sync = ath6kl_sdio_read_write_sync,
.write_async = ath6kl_sdio_write_async,
@@ -768,6 +783,7 @@ static const struct ath6kl_hif_ops ath6kl_sdio_ops = {
.cleanup_scatter = ath6kl_sdio_cleanup_scatter,
.suspend = ath6kl_sdio_suspend,
.resume = ath6kl_sdio_resume,
+ .keep_pwr_caps = ath6kl_sdio_keep_pwr_caps,
};
static int ath6kl_sdio_probe(struct sdio_func *func,
--
1.7.0.4
From: Raja Mani <[email protected]>
Signed-off-by: Raja Mani <[email protected]>
---
drivers/net/wireless/ath/ath6kl/wmi.c | 88 +++++++++++++++++++++++++++++++++
drivers/net/wireless/ath/ath6kl/wmi.h | 30 +++++++++++
2 files changed, 118 insertions(+), 0 deletions(-)
diff --git a/drivers/net/wireless/ath/ath6kl/wmi.c b/drivers/net/wireless/ath/ath6kl/wmi.c
index f38b30e..fceb801 100644
--- a/drivers/net/wireless/ath/ath6kl/wmi.c
+++ b/drivers/net/wireless/ath/ath6kl/wmi.c
@@ -2356,6 +2356,94 @@ int ath6kl_wmi_set_ip_cmd(struct wmi *wmi, struct wmi_set_ip_cmd *ip_cmd)
return ret;
}
+static void ath6kl_wmi_relinquish_implicit_pstream_credits(struct wmi *wmi)
+{
+ u16 active_tsids;
+ u8 stream_exist;
+ int i;
+
+ /*
+ * Relinquish credits from all implicitly created pstreams
+ * since when we go to sleep. If user created explicit
+ * thinstreams exists with in a fatpipe leave them intact
+ * for the user to delete.
+ */
+ spin_lock_bh(&wmi->lock);
+ stream_exist = wmi->fat_pipe_exist;
+ spin_unlock_bh(&wmi->lock);
+
+ for (i = 0; i < WMM_NUM_AC; i++) {
+ if (stream_exist & (1 << i)) {
+
+ spin_lock_bh(&wmi->lock);
+ active_tsids = wmi->stream_exist_for_ac[i];
+ spin_unlock_bh(&wmi->lock);
+
+ /*
+ * If there are no user created thin streams
+ * delete the fatpipe
+ */
+ if (!active_tsids) {
+ stream_exist &= ~(1 << i);
+ /*
+ * Indicate inactivity to driver layer for
+ * this fatpipe (pstream)
+ */
+ ath6kl_indicate_tx_activity(wmi->parent_dev,
+ i, false);
+ }
+ }
+ }
+
+ spin_lock_bh(&wmi->lock);
+ wmi->fat_pipe_exist = stream_exist;
+ spin_unlock_bh(&wmi->lock);
+}
+
+int ath6kl_wmi_set_host_sleep_mode_cmd(struct wmi *wmi,
+ struct wmi_set_host_sleep_mode_cmd *host_mode)
+{
+ struct sk_buff *skb;
+ struct wmi_set_host_sleep_mode_cmd *cmd;
+ int ret;
+
+ if (host_mode->awake == host_mode->asleep)
+ return -EINVAL;
+
+ skb = ath6kl_wmi_get_new_buf(sizeof(*cmd));
+ if (!skb)
+ return -ENOMEM;
+
+ cmd = (struct wmi_set_host_sleep_mode_cmd *) skb->data;
+ memcpy(cmd, host_mode, sizeof(*cmd));
+
+ if (host_mode->asleep)
+ ath6kl_wmi_relinquish_implicit_pstream_credits(wmi);
+
+ ret = ath6kl_wmi_cmd_send(wmi, skb, WMI_SET_HOST_SLEEP_MODE_CMDID,
+ NO_SYNC_WMIFLAG);
+ return ret;
+}
+
+int ath6kl_wmi_set_wow_mode_cmd(struct wmi *wmi,
+ struct wmi_set_wow_mode_cmd *wow_mode)
+{
+ struct sk_buff *skb;
+ struct wmi_set_wow_mode_cmd *cmd;
+ int ret;
+
+ skb = ath6kl_wmi_get_new_buf(sizeof(*cmd));
+ if (!skb)
+ return -ENOMEM;
+
+ cmd = (struct wmi_set_wow_mode_cmd *) skb->data;
+ memcpy(cmd, wow_mode, sizeof(*cmd));
+
+ ret = ath6kl_wmi_cmd_send(wmi, skb, WMI_SET_WOW_MODE_CMDID,
+ NO_SYNC_WMIFLAG);
+ return ret;
+}
+
int ath6kl_wmi_add_wow_pattern_cmd(struct wmi *wmi,
struct wmi_add_wow_pattern_cmd *add_wow_cmd,
u8 *pattern, u8 *mask)
diff --git a/drivers/net/wireless/ath/ath6kl/wmi.h b/drivers/net/wireless/ath/ath6kl/wmi.h
index 3a54933..2ca3a18 100644
--- a/drivers/net/wireless/ath/ath6kl/wmi.h
+++ b/drivers/net/wireless/ath/ath6kl/wmi.h
@@ -1796,6 +1796,32 @@ struct wmi_set_ip_cmd {
__le32 ips[MAX_IP_ADDRS];
} __packed;
+enum ath6kl_wow_filters {
+ WOW_FILTER_SSID = BIT(0),
+ WOW_FILTER_OPTION_MAGIC_PACKET = BIT(2),
+ WOW_FILTER_OPTION_EAP_REQ = BIT(3),
+ WOW_FILTER_OPTION_PATTERNS = BIT(4),
+ WOW_FILTER_OPTION_OFFLOAD_ARP = BIT(5),
+ WOW_FILTER_OPTION_OFFLOAD_NS = BIT(6),
+ WOW_FILTER_OPTION_OFFLOAD_GTK = BIT(7),
+ WOW_FILTER_OPTION_8021X_4WAYHS = BIT(8),
+ WOW_FILTER_OPTION_NLO_DISCVRY = BIT(9),
+ WOW_FILTER_OPTION_NWK_DISASSOC = BIT(10),
+ WOW_FILTER_OPTION_GTK_ERROR = BIT(11),
+ WOW_FILTER_OPTION_TEST_MODE = BIT(15),
+};
+
+struct wmi_set_host_sleep_mode_cmd {
+ __le32 awake;
+ __le32 asleep;
+} __packed;
+
+struct wmi_set_wow_mode_cmd {
+ __le32 enable_wow;
+ __le32 filter;
+ __le16 host_req_delay;
+} __packed;
+
struct wmi_add_wow_pattern_cmd {
u8 filter_list_id;
u8 filter_size;
@@ -2255,6 +2281,10 @@ int ath6kl_wmi_test_cmd(struct wmi *wmi, void *buf, size_t len);
s32 ath6kl_wmi_get_rate(s8 rate_index);
int ath6kl_wmi_set_ip_cmd(struct wmi *wmi, struct wmi_set_ip_cmd *ip_cmd);
+int ath6kl_wmi_set_host_sleep_mode_cmd(struct wmi *wmi,
+ struct wmi_set_host_sleep_mode_cmd *host_mode);
+int ath6kl_wmi_set_wow_mode_cmd(struct wmi *wmi,
+ struct wmi_set_wow_mode_cmd *wow_mode);
int ath6kl_wmi_add_wow_pattern_cmd(struct wmi *wmi,
struct wmi_add_wow_pattern_cmd *add_wow_cmd,
u8 *pattern, u8 *mask);
--
1.7.0.4
On 10/31/2011 11:53 AM, Raja Mani wrote:
>>> +static void ath6kl_wmi_relinquish_implicit_pstream_credits(struct wmi *wmi)
>>> >> +{
>>> >> + u16 active_tsids;
>>> >> + u8 stream_exist;
>>> >> + int i;
>>> >> +
>>> >> + /*
>>> >> + * Relinquish credits from all implicitly created pstreams
>>> >> + * since when we go to sleep. If user created explicit
>>> >> + * thinstreams exists with in a fatpipe leave them intact
>>> >> + * for the user to delete.
>>> >> + */
>>> >> + spin_lock_bh(&wmi->lock);
>>> >> + stream_exist = wmi->fat_pipe_exist;
>>> >> + spin_unlock_bh(&wmi->lock);
>>> >> +
>>> >> + for (i = 0; i< WMM_NUM_AC; i++) {
>>> >> + if (stream_exist& (1<< i)) {
>>> >> +
>>> >> + spin_lock_bh(&wmi->lock);
>>> >> + active_tsids = wmi->stream_exist_for_ac[i];
>>> >> + spin_unlock_bh(&wmi->lock);
>>> >> +
>>> >> + /*
>>> >> + * If there are no user created thin streams
>>> >> + * delete the fatpipe
>>> >> + */
>>> >> + if (!active_tsids) {
>>> >> + stream_exist&= ~(1<< i);
>>> >> + /*
>>> >> + * Indicate inactivity to driver layer for
>>> >> + * this fatpipe (pstream)
>>> >> + */
>>> >> + ath6kl_indicate_tx_activity(wmi->parent_dev,
>>> >> + i, false);
>>> >> + }
>>> >> + }
>>> >> + }
>>> >> +
>>> >> + spin_lock_bh(&wmi->lock);
>>> >> + wmi->fat_pipe_exist = stream_exist;
>>> >> + spin_unlock_bh(&wmi->lock);
>> >
>> > The locking here doesn't really make sense. For example, any changes to
>> > wmi->fat_pipe_exist during the for loop will be overwritten. Also lock
>> > handling with wmi->stream_exist_for_ac[i] looks fishy.
>
> Is it fine just removal of both locking (For "active_tsids =
> wmi->stream_exist_for_ac[i]" and "wmi->fat_pipe_exist = stream_exist") ?
>
> I am sure about the impact..Could you please give some point how above
> locking can be improved ?
I haven't reviewed the locking in detail so it's difficult to say. For
example, you could hold the lock the entire duration of for loop. But
that might create deadlocks. Another option is take it into account the
changes happening to fat_pipe_exist while lock is not held.
Kalle
On 10/25/2011 01:37 PM, [email protected] wrote:
> From: Raja Mani <[email protected]>
>
> Link ath6kl's wow suspend/resume functions with the callback functions
> registered with the CFG layer for suspend and resume operation.
[..]
> --- a/drivers/net/wireless/ath/ath6kl/cfg80211.c
> +++ b/drivers/net/wireless/ath/ath6kl/cfg80211.c
> @@ -1606,6 +1606,19 @@ static int ar6k_cfg80211_suspend(struct wiphy *wiphy,
> struct cfg80211_wowlan *wow)
> {
> struct ath6kl *ar = wiphy_priv(wiphy);
> + int ret;
> +
> + if (wow && ath6kl_cfg80211_ready(ar) &&
> + test_bit(CONNECTED, &ar->flag) &&
> + ath6kl_hif_keep_pwr_caps(ar)) {
> +
> + /* Flush all non control pkts in Tx path */
> + ath6kl_tx_data_cleanup(ar);
> +
> + ret = ath6kl_pm_wow_suspend(ar, wow);
> + if (ret)
> + return ret;
> + }
This is now confusing. Some of the suspend logic is now in sdio.c and
some here. It's better to have everything in one place, that is in
sdio.c. We just need to add an interface so that sdio.c can request the
correct suspend mode.
Kalle