* [wireless-next PATCH v2 1/4] mac80211: Support forcing station to disable 11n.
From: greearb @ 2011-10-28 22:21 UTC (permalink / raw)
To: linux-wireless; +Cc: Ben Greear
From: Ben Greear <greearb@candelatech.com>
This allows a user to configure a wifi station interface
to disable 802.11n, even if the AP and NIC supports it.
Other interface types may be supported in the future.
Signed-off-by: Ben Greear <greearb@candelatech.com>
---
* Renamed NL80211_ATTR_DISABLE_11N to NL80211_ATTR_DISABLE_HT
* Fix patched description w/regard to 'netlink bits'.
* Didn't add checks for capability bits or nor moved the disable_11n
flag to u.mgd, pending answers to earlier questions.
:100644 100644 8049bf7... 8a317c9... M include/linux/nl80211.h
:100644 100644 92cf1c2... 1331a5b... M include/net/cfg80211.h
:100644 100644 e253afa... 8221a3a... M net/mac80211/cfg.c
:100644 100644 4c3d1f5... 72c6726... M net/mac80211/ieee80211_i.h
:100644 100644 0e5d8da... 393b480... M net/mac80211/mlme.c
:100644 100644 48260c2... db24d25... M net/wireless/nl80211.c
include/linux/nl80211.h | 4 ++++
include/net/cfg80211.h | 4 +++-
net/mac80211/cfg.c | 3 +++
net/mac80211/ieee80211_i.h | 2 ++
net/mac80211/mlme.c | 3 +++
net/wireless/nl80211.c | 7 +++++++
6 files changed, 22 insertions(+), 1 deletions(-)
diff --git a/include/linux/nl80211.h b/include/linux/nl80211.h
index 8049bf7..8a317c9 100644
--- a/include/linux/nl80211.h
+++ b/include/linux/nl80211.h
@@ -1109,6 +1109,9 @@ enum nl80211_commands {
* %NL80211_CMD_TDLS_MGMT. Otherwise %NL80211_CMD_TDLS_OPER should be
* used for asking the driver to perform a TDLS operation.
*
+ * @NL80211_ATTR_DISABLE_HT: Force /n capable interfaces to instead
+ * function as /a/b/g interfaces.
+ *
* @NL80211_ATTR_MAX: highest attribute number currently defined
* @__NL80211_ATTR_AFTER_LAST: internal use
*/
@@ -1337,6 +1340,7 @@ enum nl80211_attrs {
NL80211_ATTR_TDLS_SUPPORT,
NL80211_ATTR_TDLS_EXTERNAL_SETUP,
+ NL80211_ATTR_DISABLE_HT,
/* add attributes here, update the policy in nl80211.c */
__NL80211_ATTR_AFTER_LAST,
diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
index 92cf1c2..1331a5b 100644
--- a/include/net/cfg80211.h
+++ b/include/net/cfg80211.h
@@ -254,9 +254,11 @@ struct ieee80211_supported_band {
/**
* struct vif_params - describes virtual interface parameters
* @use_4addr: use 4-address frames
+ * @disable_11n: Don't use 11n features (HT, etc)
*/
struct vif_params {
- int use_4addr;
+ int use_4addr;
+ int disable_11n;
};
/**
diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c
index e253afa..8221a3a 100644
--- a/net/mac80211/cfg.c
+++ b/net/mac80211/cfg.c
@@ -57,6 +57,9 @@ static int ieee80211_change_iface(struct wiphy *wiphy,
struct ieee80211_sub_if_data *sdata = IEEE80211_DEV_TO_SUB_IF(dev);
int ret;
+ if (params->disable_11n != -1)
+ sdata->cfg_disable_11n = params->disable_11n;
+
ret = ieee80211_if_change_type(sdata, type);
if (ret)
return ret;
diff --git a/net/mac80211/ieee80211_i.h b/net/mac80211/ieee80211_i.h
index 4c3d1f5..72c6726 100644
--- a/net/mac80211/ieee80211_i.h
+++ b/net/mac80211/ieee80211_i.h
@@ -595,6 +595,8 @@ struct ieee80211_sub_if_data {
/* to detect idle changes */
bool old_idle;
+ bool cfg_disable_11n; /* configured to disable 11n? */
+
/* Fragment table for host-based reassembly */
struct ieee80211_fragment_entry fragments[IEEE80211_FRAGMENT_MAX];
unsigned int fragment_next;
diff --git a/net/mac80211/mlme.c b/net/mac80211/mlme.c
index 0e5d8da..393b480 100644
--- a/net/mac80211/mlme.c
+++ b/net/mac80211/mlme.c
@@ -2597,6 +2597,9 @@ int ieee80211_mgd_assoc(struct ieee80211_sub_if_data *sdata,
ifmgd->flags |= IEEE80211_STA_DISABLE_11N;
+ if (sdata->cfg_disable_11n)
+ ifmgd->flags |= IEEE80211_STA_DISABLE_11N;
+
if (req->ie && req->ie_len) {
memcpy(wk->ie, req->ie, req->ie_len);
wk->ie_len = req->ie_len;
diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index 48260c2..db24d25 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -1641,6 +1641,13 @@ static int nl80211_set_interface(struct sk_buff *skb, struct genl_info *info)
change = true;
}
+ if (info->attrs[NL80211_ATTR_DISABLE_HT]) {
+ params.disable_11n = !!nla_get_u8(info->attrs[NL80211_ATTR_DISABLE_HT]);
+ change = true;
+ } else {
+ params.disable_11n = -1;
+ }
+
if (change)
err = cfg80211_change_iface(rdev, dev, ntype, flags, ¶ms);
else
--
1.7.3.4
^ permalink raw reply related
* Compat-wireless release for 2011-10-28 is baked
From: Compat-wireless cronjob account @ 2011-10-28 19:02 UTC (permalink / raw)
To: linux-wireless
compat-wireless code metrics
814862 - Total upstream lines of code being pulled
2431 - backport code changes
2113 - backport code additions
318 - backport code deletions
8588 - backport from compat module
11019 - total backport code
1.3523 - % of code consists of backport work
^ permalink raw reply
* Re: [wireless-next PATCH 1/5] mac80211: Support forcing station to disable 11n.
From: Ben Greear @ 2011-10-28 18:55 UTC (permalink / raw)
To: Johannes Berg; +Cc: linux-wireless
In-Reply-To: <1319789318.3914.10.camel@jlt3.sipsolutions.net>
On 10/28/2011 01:08 AM, Johannes Berg wrote:
> On Thu, 2011-10-27 at 22:11 -0700, greearb@candelatech.com wrote:
>> +++ b/net/wireless/nl80211.c
>> @@ -1641,6 +1641,13 @@ static int nl80211_set_interface(struct sk_buff *skb, struct genl_info *info)
>> change = true;
>> }
>>
>> + if (info->attrs[NL80211_ATTR_DISABLE_11N]) {
>> + params.disable_11n = !!nla_get_u8(info->attrs[NL80211_ATTR_DISABLE_11N]);
>> + change = true;
>> + } else {
>> + params.disable_11n = -1;
>> + }
>
> This should be a parameter to connect() and assoc(), not a generic
> netdev parameter, since it applies to the connection.
>
> Also, it would be good to have a capability check for it etc. since a
> lot of fullmac drivers will likely never implement this.
The existing code always sets the IEEE80211_STA_DISABLE_11N flag in u.mgd if
WEP or TKIP is configured, without any capability checks, and my patch
sets that flag in the same location.
So, maybe it is OK as is?
If not, I will add a new capability bit and just enable
it in ath9k (and let others enable it in their drivers as they wish).
Thanks,
Ben
--
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc http://www.candelatech.com
^ permalink raw reply
* Re: [RFC v2 13/12] cfg80211/mac80211: allow management TX to not wait for ACK
From: Arend van Spriel @ 2011-10-28 17:22 UTC (permalink / raw)
To: Helmut Schaa; +Cc: Johannes Berg, linux-wireless@vger.kernel.org
In-Reply-To: <CAGXE3d9NiHgmE_ABc_red7MeNnhUGG+fOgbL19B9U3e9Fx9Vxw@mail.gmail.com>
On 10/28/2011 11:28 AM, Helmut Schaa wrote:
> On Fri, Oct 28, 2011 at 11:10 AM, Arend Van Spriel <arend@broadcom.com> wrote:
>>>> I would expect this to affect the duration field in the
>>>> 802.11 header to indicate the shorter use of the medium.
>>>> Going through your patch I am not sure this is done.
>>>
>>> Hmmm. You're right, but that's something we already need to fix in many
>>> cases -- as the duration field is overwritten by most hardware these
>>> days, we haven't always maintained it very well.
>>
>> If the station still does the ACK we should not touch the duration field
>> in this case (see my reply to Helmut).
>
> The reason is the following: if you're running a mac80211 AP with multiple
> bssids, hostapd will answer to broadcast probe requests sent by STAs with
> one probe response per bssid (unicast of course). And since some clients
> stay only for a short amount of time on the scan-channel (a few ms) under
> some circumstances the probe responses aren't sent out yet before the
> scanning STA leaves the channel and thus retried till the maximum retry
> limit is reached. And these retries consume quite a lot of airtime. And to
> avoid this we just don't want to retry the probe responses in that case.
>
> Helmut
>
Hi Helmut,
That makes perfect sense. Indeed not doing retries will obviously save
airtime. Thanks for taking time to explain the scenario.
Gr. AvS
^ permalink raw reply
* [PATCH] b43: Remove unneeded message
From: Larry Finger @ 2011-10-28 17:17 UTC (permalink / raw)
To: John W Linville, zajec5, Michael Buesch; +Cc: b43-dev, linux-wireless
The driver can spam the logs with "RX: Packet dropped" messages. These drops
originate from 1. a correpted PLCP, 2. decryption errors, and 3. packet
size underruns. Condition #3 logs a separate message, thus no dropped message
is needed.
Signed-off-by: Larry Finger <Larry.Finger@lwfinger.net>
---
Index: wireless-testing-new/drivers/net/wireless/b43/xmit.c
===================================================================
--- wireless-testing-new.orig/drivers/net/wireless/b43/xmit.c
+++ wireless-testing-new/drivers/net/wireless/b43/xmit.c
@@ -831,7 +831,6 @@ void b43_rx(struct b43_wldev *dev, struc
#endif
return;
drop:
- b43dbg(dev->wl, "RX: Packet dropped\n");
dev_kfree_skb_any(skb);
}
^ permalink raw reply
* Re: [wireless-next PATCH 3/5] wifi: Allow overriding some HT information.
From: Ben Greear @ 2011-10-28 16:33 UTC (permalink / raw)
To: Johannes Berg; +Cc: linux-wireless
In-Reply-To: <1319789573.3914.15.camel@jlt3.sipsolutions.net>
On 10/28/2011 01:12 AM, Johannes Berg wrote:
> On Thu, 2011-10-27 at 22:11 -0700, greearb@candelatech.com wrote:
>> From: Ben Greear<greearb@candelatech.com>
>>
>> * Allow configuring the mcs (/n) rates available.
>> * Allow configuration of MAX-A-MSDU
>> * Allow configuration of A-MPDU factor& density.
>>
>> Users can only remove existing rates. The MSDU and MPDU
>> values can be set to any value allowed by the 802.11n
>> specification.
>
> That can't work -- the device might not support it.
The device would always support removing rates, right?
As for the MSDU and MPDU stuff, I would need to add capabilities
flags and then enable each driver as they are tested?
> I also don't really like the way you pass in some binary "mask" when
> it's not really a binary masking operation.
I found the mask to work very well. It's easy enough to
deal with in user-space, and easily allows us to add override features
(the additional bits to support the MPDU stuff once the
HT rates logic was in is a very small bit of code).
Otherwise, I'll need to add new netlink commands for each new
feature. That is going to bloat hostapd as well as the
kernel.
>
>> struct vif_params {
>> int use_4addr;
>> int disable_11n;
>> int disable_ht40;
>> + struct ieee80211_ht_cap *ht_capa;
>> + struct ieee80211_ht_cap *ht_capa_mask;
>
> Same comments as before again -- this is per connection right?
I meant it to be per interface. I'm using lots of virtual stations,
and want some to act like /abg radios, and others to be ht-20 only,
and others to be capable of only up to mcs7, etc.
>> @@ -114,6 +115,19 @@ static void ieee80211_add_ht_ie(struct sk_buff *skb, const u8 *ht_info_ie,
>> if (ht_info_ie[1]< sizeof(struct ieee80211_ht_info))
>> return;
>>
>> + memcpy(&ht_cap,&sband->ht_cap, sizeof(ht_cap));
>> + /*
>> + * This is for an association attempt, and we must
>> + * advert at least the first 8 rates, even if we
>> + * will later force the rate control to a lower rate.
>> + */
>> + ieee80211_apply_htcap_overrides(sdata,&ht_cap, 8);
>
> Yuck, why, why hard-code 8, etc.
I can make a define that involves MCS7.
Thanks,
Ben
--
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc http://www.candelatech.com
^ permalink raw reply
* Re: [wireless-next PATCH 2/5] wifi: Support disabling ht40.
From: Ben Greear @ 2011-10-28 16:25 UTC (permalink / raw)
To: Johannes Berg; +Cc: linux-wireless
In-Reply-To: <1319789370.3914.11.camel@jlt3.sipsolutions.net>
On 10/28/2011 01:09 AM, Johannes Berg wrote:
> On Thu, 2011-10-27 at 22:11 -0700, greearb@candelatech.com wrote:
>
>> struct vif_params {
>> int use_4addr;
>> int disable_11n;
>> + int disable_ht40;
>> };
>
> All the comments on patch 1 apply -- per connection parameter,
> capability flag etc.
>
> I do wonder if it would be worthwhile to make it a u32 "connection
> flags" or so instead of defining them separately.
I don't care either way. If you want a single variable to hold
the flags that is easy enough. We'd need to pass in a mask to
allow user-space to change only a subset of flags at a time
though.
Thanks,
Ben
>
> johannes
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc http://www.candelatech.com
^ permalink raw reply
* Re: [wireless-next PATCH 1/5] mac80211: Support forcing station to disable 11n.
From: Ben Greear @ 2011-10-28 16:24 UTC (permalink / raw)
To: Johannes Berg; +Cc: linux-wireless
In-Reply-To: <1319789318.3914.10.camel@jlt3.sipsolutions.net>
On 10/28/2011 01:08 AM, Johannes Berg wrote:
> On Thu, 2011-10-27 at 22:11 -0700, greearb@candelatech.com wrote:
>
>> The additional netlink bits is to allow this patch to work on 3.0
>> and should not be included in the final patch.
>
> What additional bit?
Sorry, that description is bad. It was from when I wrote the patch
against the 3.0.6 kernel and I had to add a bunch of netlink
crap to make the header file sync up with top-of-tree hostapd.
>> + * @NL80211_ATTR_DISABLE_80211N: Force /n capable stations to instead
>> + * function as /a/b/g stations.
>
> IMHO this should be called DISABLE_HT -- "11n" will not exist for much
> longer.
Ok, I can change that.
>
>> +++ b/net/mac80211/cfg.c
>> @@ -57,6 +57,9 @@ static int ieee80211_change_iface(struct wiphy *wiphy,
>> struct ieee80211_sub_if_data *sdata = IEEE80211_DEV_TO_SUB_IF(dev);
>> int ret;
>>
>> + if (params->disable_11n != -1)
>> + sdata->cfg_disable_11n = params->disable_11n;
>
> This doesn't seem right -- why change the iface for it? It's a per
> connection parameter.
I wanted it to be an interface parameter, or at least I think
that is what I want.
>> +++ b/net/mac80211/ieee80211_i.h
>> @@ -595,6 +595,8 @@ struct ieee80211_sub_if_data {
>> /* to detect idle changes */
>> bool old_idle;
>>
>> + bool cfg_disable_11n; /* configured to disable 11n? */
>
> That should be in the u.mgd part of the struct.
I would like eventually to support this same feature for AP
interfaces, and probably other types. Would it still be in
the u.mgd struct in that case?
>> +++ b/net/wireless/nl80211.c
>> @@ -1641,6 +1641,13 @@ static int nl80211_set_interface(struct sk_buff *skb, struct genl_info *info)
>> change = true;
>> }
>>
>> + if (info->attrs[NL80211_ATTR_DISABLE_11N]) {
>> + params.disable_11n = !!nla_get_u8(info->attrs[NL80211_ATTR_DISABLE_11N]);
>> + change = true;
>> + } else {
>> + params.disable_11n = -1;
>> + }
>
> This should be a parameter to connect() and assoc(), not a generic
> netdev parameter, since it applies to the connection.
>
> Also, it would be good to have a capability check for it etc. since a
> lot of fullmac drivers will likely never implement this.
I'll see if I can make it thus.
Thanks,
Ben
--
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc http://www.candelatech.com
^ permalink raw reply
* Re: [wireless-next PATCH 4/5] wifi: Warn if cannot add station debugfs entries.
From: Ben Greear @ 2011-10-28 16:13 UTC (permalink / raw)
To: Johannes Berg; +Cc: linux-wireless
In-Reply-To: <1319789588.3914.16.camel@jlt3.sipsolutions.net>
On 10/28/2011 01:13 AM, Johannes Berg wrote:
> On Thu, 2011-10-27 at 22:11 -0700, greearb@candelatech.com wrote:
>> From: Ben Greear<greearb@candelatech.com>
>>
>> Signed-off-by: Ben Greear<greearb@candelatech.com>
>> ---
>> :100644 100644 c5f3417... 1ceec86... M net/mac80211/debugfs_sta.c
>> net/mac80211/debugfs_sta.c | 10 ++++++++--
>> 1 files changed, 8 insertions(+), 2 deletions(-)
>>
>> diff --git a/net/mac80211/debugfs_sta.c b/net/mac80211/debugfs_sta.c
>> index c5f3417..1ceec86 100644
>> --- a/net/mac80211/debugfs_sta.c
>> +++ b/net/mac80211/debugfs_sta.c
>> @@ -337,8 +337,11 @@ void ieee80211_sta_debugfs_add(struct sta_info *sta)
>>
>> sta->debugfs.add_has_run = true;
>>
>> - if (!stations_dir)
>> + if (!stations_dir) {
>> + printk(KERN_DEBUG "%s: sta_debugfs_add: stations_dir is NULL\n",
>> + sta->sdata->name);
>> return;
>> + }
>
> I honestly don't see any point in this.
Ok, I wasn't sure if this was wanted, so please just
drop it. I did see a case where the debugfs files didn't
exist, and was trying to figure out why..but never reproduced
the problem after this printk was in, so not sure if this
was the issue or not.
Thanks,
Ben
>
> johannes
--
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc http://www.candelatech.com
^ permalink raw reply
* Re: [RFC v2 13/12] cfg80211/mac80211: allow management TX to not wait for ACK
From: Kalle Valo @ 2011-10-28 13:34 UTC (permalink / raw)
To: Helmut Schaa
Cc: Arend Van Spriel, Johannes Berg, linux-wireless@vger.kernel.org
In-Reply-To: <CAGXE3d9NiHgmE_ABc_red7MeNnhUGG+fOgbL19B9U3e9Fx9Vxw@mail.gmail.com>
Helmut Schaa <helmut.schaa@googlemail.com> writes:
> The reason is the following: if you're running a mac80211 AP with multiple
> bssids, hostapd will answer to broadcast probe requests sent by STAs with
> one probe response per bssid (unicast of course). And since some clients
> stay only for a short amount of time on the scan-channel (a few ms) under
> some circumstances the probe responses aren't sent out yet before the
> scanning STA leaves the channel and thus retried till the maximum retry
> limit is reached. And these retries consume quite a lot of airtime. And to
> avoid this we just don't want to retry the probe responses in that case.
This is all good. But doesn't it also mean that this change increases
probability that the client doesn't receive the probe response, for
example due to interference where retransmission would help, and hence
the AP isn't included in the client's scan results?
--
Kalle Valo
^ permalink raw reply
* [PATCH] ath6kl: change name of sdio driver to ath6kl
From: Kalle Valo @ 2011-10-28 13:23 UTC (permalink / raw)
To: kvalo; +Cc: linux-wireless
Currently the name of the driver in struct sdio_driver is "ath6kl_sdio",
this is for example what uevent advertises. This is wrong as the module
is named as ath6kl.ko. Change it to "ath6kl" so that the names match.
Signed-off-by: Kalle Valo <kvalo@qca.qualcomm.com>
---
drivers/net/wireless/ath/ath6kl/sdio.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/drivers/net/wireless/ath/ath6kl/sdio.c b/drivers/net/wireless/ath/ath6kl/sdio.c
index 682c47c..56f83c4 100644
--- a/drivers/net/wireless/ath/ath6kl/sdio.c
+++ b/drivers/net/wireless/ath/ath6kl/sdio.c
@@ -914,7 +914,7 @@ static const struct sdio_device_id ath6kl_sdio_devices[] = {
MODULE_DEVICE_TABLE(sdio, ath6kl_sdio_devices);
static struct sdio_driver ath6kl_sdio_driver = {
- .name = "ath6kl_sdio",
+ .name = "ath6kl",
.id_table = ath6kl_sdio_devices,
.probe = ath6kl_sdio_probe,
.remove = ath6kl_sdio_remove,
^ permalink raw reply related
* Re: [PATCH 5/7] ath6kl: Invoke wow suspend/resume calls during PM operations
From: Kalle Valo @ 2011-10-28 13:21 UTC (permalink / raw)
To: rmani; +Cc: linux-wireless
In-Reply-To: <1319539047-8756-6-git-send-email-rmani@qca.qualcomm.com>
On 10/25/2011 01:37 PM, rmani@qca.qualcomm.com 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
^ permalink raw reply
* Re: [PATCH 6/7] ath6kl: Perform WOW resume in RX path in case of SDIO IRQ wakeup
From: Kalle Valo @ 2011-10-28 13:18 UTC (permalink / raw)
To: rmani; +Cc: linux-wireless
In-Reply-To: <1319539047-8756-7-git-send-email-rmani@qca.qualcomm.com>
On 10/25/2011 01:37 PM, rmani@qca.qualcomm.com wrote:
> From: Raja Mani <rmani@qca.qualcomm.com>
>
> Signed-off-by: Raja Mani <rmani@qca.qualcomm.com>
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
^ permalink raw reply
* Re: [PATCH 5/7] ath6kl: Invoke wow suspend/resume calls during PM operations
From: Kalle Valo @ 2011-10-28 13:15 UTC (permalink / raw)
To: rmani; +Cc: linux-wireless
In-Reply-To: <1319539047-8756-6-git-send-email-rmani@qca.qualcomm.com>
On 10/25/2011 01:37 PM, rmani@qca.qualcomm.com wrote:
> From: Raja Mani <rmani@qca.qualcomm.com>
>
> 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
^ permalink raw reply
* Re: [PATCH 4/7] ath6kl: Add new functions to handle wow suspend/resume operations
From: Kalle Valo @ 2011-10-28 12:31 UTC (permalink / raw)
To: rmani; +Cc: linux-wireless
In-Reply-To: <1319539047-8756-5-git-send-email-rmani@qca.qualcomm.com>
On 10/25/2011 01:37 PM, rmani@qca.qualcomm.com wrote:
> From: Raja Mani <rmani@qca.qualcomm.com>
>
> Signed-off-by: Raja Mani <rmani@qca.qualcomm.com>
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
^ permalink raw reply
* Re: [PATCH 2/7] ath6kl: Add wmi functions to configure wow mode and host sleep mode
From: Kalle Valo @ 2011-10-28 12:19 UTC (permalink / raw)
To: rmani; +Cc: linux-wireless
In-Reply-To: <1319539047-8756-3-git-send-email-rmani@qca.qualcomm.com>
On 10/25/2011 01:37 PM, rmani@qca.qualcomm.com wrote:
> From: Raja Mani <rmani@qca.qualcomm.com>
>
> Signed-off-by: Raja Mani <rmani@qca.qualcomm.com>
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
^ permalink raw reply
* Re: [PATCH 1/7] ath6kl: Add wmi functions to add/delete wow patterns
From: Kalle Valo @ 2011-10-28 12:11 UTC (permalink / raw)
To: rmani; +Cc: linux-wireless
In-Reply-To: <1319539047-8756-2-git-send-email-rmani@qca.qualcomm.com>
On 10/25/2011 01:37 PM, rmani@qca.qualcomm.com wrote:
> From: Raja Mani <rmani@qca.qualcomm.com>
>
> Signed-off-by: Raja Mani <rmani@qca.qualcomm.com>
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
^ permalink raw reply
* Re: [PATCH 0/7] ath6kl: Add WOW support
From: Kalle Valo @ 2011-10-28 11:58 UTC (permalink / raw)
To: rmani; +Cc: linux-wireless
In-Reply-To: <1319539047-8756-1-git-send-email-rmani@qca.qualcomm.com>
On 10/25/2011 01:37 PM, rmani@qca.qualcomm.com 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
^ permalink raw reply
* Re: [PATCH] mac80211: disable powersave for broken APs
From: Johannes Berg @ 2011-10-28 11:55 UTC (permalink / raw)
To: Bill C Riemers; +Cc: John Linville, linux-wireless
In-Reply-To: <4EAA979D.7090108@redhat.com>
On Fri, 2011-10-28 at 07:53 -0400, Bill C Riemers wrote:
> I'm not sure how patching this patch will stop the abort in
> iwl-core.c. Unless of course the aid value set here ends up as what
> is tested in iwl-core.c.
That's what happens, yes.
> BTW. Is it 2007 or 0x2007. The ieee80211_softmac.c code is using
> 0x2007. While it could just be coincidence, I would expect these two
> should be the same constant...
ieee80211_softmac.c is wrong then -- but it doesn't exist in upstream
kernels any more anyway so I'm not sure where you're seeing it? The
range is 1-2007, not 0x2007.
> I won't be able to test the code as it will take about 1-2 hours for a
> kernel build, and I am checking out of my hotel in about 30 minutes.
> However, my colleague mentioned he was seeing the same problem on his
> home network. So I will ask him if my previous build fixed his
> problem. If so it probably really is the exact same problem, and I
> can ask him to test this new patch.
Alright. I'm pretty confident it'd fix it since this is where the AID
value is obtained from the AP and passed down to iwlwifi.
johannes
^ permalink raw reply
* Re: [PATCH] mac80211: disable powersave for broken APs
From: Bill C Riemers @ 2011-10-28 11:53 UTC (permalink / raw)
To: Johannes Berg; +Cc: John Linville, linux-wireless
In-Reply-To: <1319795987.8931.7.camel@jlt3.sipsolutions.net>
I'm not sure how patching this patch will stop the abort in iwl-core.c. Unless of course the aid value set here ends up as what is tested in iwl-core.c. BTW. Is it 2007 or 0x2007. The ieee80211_softmac.c code is using 0x2007. While it could just be coincidence, I would expect these two should be the same constant...
I won't be able to test the code as it will take about 1-2 hours for a kernel build, and I am checking out of my hotel in about 30 minutes. However, my colleague mentioned he was seeing the same problem on his home network. So I will ask him if my previous build fixed his problem. If so it probably really is the exact same problem, and I can ask him to test this new patch.
Bill
On 10/28/2011 05:59 AM, Johannes Berg wrote:
> From: Johannes Berg<johannes.berg@intel.com>
>
> Only AID values 1-2007 are valid, but some APs have been
> found to send random bogus values, in the reported case an
> AP that was sending the AID field value 0xffff, an AID of
> 0x3fff (16383).
>
> There isn't much we can do but disable powersave since
> there's no way it can work properly in this case.
>
> Cc: stable@vger.kernel.org
> Reported-by: Bill C Riemers<briemers@redhat.com>
> Signed-off-by: Johannes Berg<johannes.berg@intel.com>
> ---
> Bill, please test this patch and see if you still see the
> warnings, or you can revert your iwlwifi changes and test
> then, it should work.
>
> John, please wait for Bill to test before you apply, just
> to confirm it fixes the issue.
>
> net/mac80211/ieee80211_i.h | 1 +
> net/mac80211/mlme.c | 18 ++++++++++++++++--
> 2 files changed, 17 insertions(+), 2 deletions(-)
>
> --- a/net/mac80211/ieee80211_i.h 2011-10-28 11:03:50.000000000 +0200
> +++ b/net/mac80211/ieee80211_i.h 2011-10-28 11:51:48.000000000 +0200
> @@ -393,6 +393,7 @@ struct ieee80211_if_managed {
>
> unsigned long timers_running; /* used for quiesce/restart */
> bool powersave; /* powersave requested for this iface */
> + bool broken_ap; /* AP is broken -- turn off powersave */
> enum ieee80211_smps_mode req_smps, /* requested smps mode */
> ap_smps, /* smps mode AP thinks we're in */
> driver_smps_mode; /* smps mode request */
> --- a/net/mac80211/mlme.c 2011-10-12 17:11:31.000000000 +0200
> +++ b/net/mac80211/mlme.c 2011-10-28 11:57:50.000000000 +0200
> @@ -637,6 +637,9 @@ static bool ieee80211_powersave_allowed(
> if (!mgd->powersave)
> return false;
>
> + if (mgd->broken_ap)
> + return false;
> +
> if (!mgd->associated)
> return false;
>
> @@ -1489,10 +1492,21 @@ static bool ieee80211_assoc_success(stru
> capab_info = le16_to_cpu(mgmt->u.assoc_resp.capab_info);
>
> if ((aid& (BIT(15) | BIT(14))) != (BIT(15) | BIT(14)))
> - printk(KERN_DEBUG "%s: invalid aid value %d; bits 15:14 not "
> - "set\n", sdata->name, aid);
> + printk(KERN_DEBUG
> + "%s: invalid AID value 0x%x; bits 15:14 not set\n",
> + sdata->name, aid);
> aid&= ~(BIT(15) | BIT(14));
>
> + ifmgd->broken_ap = false;
> +
> + if (aid == 0 || aid> IEEE80211_MAX_AID) {
> + printk(KERN_DEBUG
> + "%s: invalid AID value %d (out of range), turn off PS\n",
> + sdata->name, aid);
> + aid = 0;
> + ifmgd->broken_ap = true;
> + }
> +
> pos = mgmt->u.assoc_resp.variable;
> ieee802_11_parse_elems(pos, len - (pos - (u8 *) mgmt),&elems);
>
>
>
^ permalink raw reply
* [PATCH v2] orinoco: release BSS structures returned by cfg80211_inform_bss()
From: David Kilroy @ 2011-10-28 11:47 UTC (permalink / raw)
To: linux-wireless, linville, johannes; +Cc: David Kilroy
In-Reply-To: <1319796588.8931.9.camel@jlt3.sipsolutions.net>
The pointer returned by cfg80211_inform_bss is a referenced
struct. The orinoco driver does not need to keep the struct, so
we just release it.
Signed-off-by: David Kilroy <kilroyd@googlemail.com>
---
v2: Avoid unnecessary NULL checks
---
drivers/net/wireless/orinoco/scan.c | 16 ++++++++++------
1 files changed, 10 insertions(+), 6 deletions(-)
diff --git a/drivers/net/wireless/orinoco/scan.c b/drivers/net/wireless/orinoco/scan.c
index e99ca1c..96e39ed 100644
--- a/drivers/net/wireless/orinoco/scan.c
+++ b/drivers/net/wireless/orinoco/scan.c
@@ -76,6 +76,7 @@ static void orinoco_add_hostscan_result(struct orinoco_private *priv,
{
struct wiphy *wiphy = priv_to_wiphy(priv);
struct ieee80211_channel *channel;
+ struct cfg80211_bss *cbss;
u8 *ie;
u8 ie_buf[46];
u64 timestamp;
@@ -121,9 +122,10 @@ static void orinoco_add_hostscan_result(struct orinoco_private *priv,
beacon_interval = le16_to_cpu(bss->a.beacon_interv);
signal = SIGNAL_TO_MBM(le16_to_cpu(bss->a.level));
- cfg80211_inform_bss(wiphy, channel, bss->a.bssid, timestamp,
- capability, beacon_interval, ie_buf, ie_len,
- signal, GFP_KERNEL);
+ cbss = cfg80211_inform_bss(wiphy, channel, bss->a.bssid, timestamp,
+ capability, beacon_interval, ie_buf, ie_len,
+ signal, GFP_KERNEL);
+ cfg80211_put_bss(cbss);
}
void orinoco_add_extscan_result(struct orinoco_private *priv,
@@ -132,6 +134,7 @@ void orinoco_add_extscan_result(struct orinoco_private *priv,
{
struct wiphy *wiphy = priv_to_wiphy(priv);
struct ieee80211_channel *channel;
+ struct cfg80211_bss *cbss;
const u8 *ie;
u64 timestamp;
s32 signal;
@@ -152,9 +155,10 @@ void orinoco_add_extscan_result(struct orinoco_private *priv,
ie = bss->data;
signal = SIGNAL_TO_MBM(bss->level);
- cfg80211_inform_bss(wiphy, channel, bss->bssid, timestamp,
- capability, beacon_interval, ie, ie_len,
- signal, GFP_KERNEL);
+ cbss = cfg80211_inform_bss(wiphy, channel, bss->bssid, timestamp,
+ capability, beacon_interval, ie, ie_len,
+ signal, GFP_KERNEL);
+ cfg80211_put_bss(cbss);
}
void orinoco_add_hostscan_results(struct orinoco_private *priv,
--
1.7.4.1
^ permalink raw reply related
* Re: [PATCH 01/10] ath6kl: don't use cfg80211_scan_request after cfg80211_scan_done()
From: Kalle Valo @ 2011-10-28 11:22 UTC (permalink / raw)
To: Kalle Valo; +Cc: linux-wireless
In-Reply-To: <20111027154746.23519.39680.stgit@localhost6.localdomain6>
On 10/27/2011 06:47 PM, Kalle Valo wrote:
> Use of cfg80211_scan_request is not valid after calling cfg80211_scan_done()
> but ath6kl_cfg80211_scan_complete_event() was doing exactly that. Change
> the function to call cfg80211_scan_done() last.
>
> This was found during code review, I didn't see any visible problems
> due to this bug.
All 10 applied.
Kalle
^ permalink raw reply
* Re: [PATCH] Fix missing copy of frame contents to local buffer
From: Kalle Valo @ 2011-10-28 11:18 UTC (permalink / raw)
To: Aarthi Thiruvengadam; +Cc: linux-wireless, Aarthi Thiruvengadam
In-Reply-To: <1319733356-31320-1-git-send-email-aarthi.thiruvengadam@qca.qualcomm.com>
On 10/27/2011 07:35 PM, Aarthi Thiruvengadam wrote:
> The wpa_supplicant was receiving incorrect frame contents in the
> callback function that indicates the status of the frame transmitted.
> This patch fixes a missing copy of the frame contents to a local
> buffer. The local buffer keeps track of the last sent management frame.
Thanks, applied. But I changes the title to "ath6kl: fix missing copy of
action frame contents".
Kalle
^ permalink raw reply
* Re: [PATCH] ath6kl: Report unique remain-on-channel cookie values
From: Kalle Valo @ 2011-10-28 11:13 UTC (permalink / raw)
To: Jouni Malinen; +Cc: linux-wireless
In-Reply-To: <20111027130013.GA32096@jouni.qca.qualcomm.com>
On 10/27/2011 04:00 PM, Jouni Malinen wrote:
> Even though only a single concurrent remain-on-channel operation is
> supported, there may be two pending remain-on-channel events (one to
> indicate end of a canceled operation and another to indicate start of a
> new operation). User space won't be able to distinguish these events
> unless unique cookies are used.
>
> The previous behavior resulted in wpa_supplicant getting quite
> confused about the driver's offchannel state in various sequences
> and this made the P2P state machine behave incorrectly. Use of
> more than a single remain-on-channel cookie value fixes this.
Thanks, applied.
Kalle
^ permalink raw reply
* Re: [PATCH] orinoco: release BSS structures returned by cfg80211_inform_bss()
From: Johannes Berg @ 2011-10-28 10:09 UTC (permalink / raw)
To: David Kilroy; +Cc: linux-wireless, linville
In-Reply-To: <1319796003-404-1-git-send-email-kilroyd@googlemail.com>
On Fri, 2011-10-28 at 11:00 +0100, David Kilroy wrote:
> The pointer returned by cfg80211_inform_bss is a referenced
> struct. The orinoco driver does not need to keep the struct, so
> we just release it.
Thanks!
> + cbss = cfg80211_inform_bss(wiphy, channel, bss->a.bssid, timestamp,
> + capability, beacon_interval, ie_buf, ie_len,
> + signal, GFP_KERNEL);
> + if (cbss)
> + cfg80211_put_bss(cbss);
Obviously doesn't hurt, but cfg80211_put_bss(NULL) is acceptable, if
you'd prefer the code that way.
johannes
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox