* Re: [PATCH 21/29] wl12xx: add band field to wlvif
From: Victor Goldenshtein @ 2011-10-11 9:48 UTC (permalink / raw)
To: Eliad Peller, Luciano Coelho; +Cc: linux-wireless
In-Reply-To: <CAB3XZEf+YgQMdsduXb1eL5r_E+A-Y0EdyP-4map4ZqmQbLTweA@mail.gmail.com>
On 10/10/2011 22:47, Eliad Peller wrote:
> On Mon, Oct 10, 2011 at 10:38 PM, Luciano Coelho<coelho@ti.com> wrote:
>> On Mon, 2011-10-10 at 10:13 +0200, Eliad Peller wrote:
> (otoh, wl12xx_roc is usually get called along with start_role_dev. i
> have some pending patch that couples them together and cleans the code
> a bit. i'll send it soon).
>
> Eliad.
> --
> 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
Guys,
Just fyi we will use the roc/croc also in DFS implementation.
Thanks,
Victor.
^ permalink raw reply
* [PATCH] wl12xx: couple role_start_dev with roc
From: Eliad Peller @ 2011-10-11 9:55 UTC (permalink / raw)
To: Luciano Coelho; +Cc: linux-wireless
Device role is always started along with ROC.
Couple them together by introducing new wl12xx_start_dev
and wl12xx_stop_dev functions.
By using these functions, we solve a bug that occured during
channel switch - we started the dev role on one channel, and
ROCed on a different one.
Signed-off-by: Eliad Peller <eliad@wizery.com>
---
drivers/net/wireless/wl12xx/cmd.c | 53 ++++++++++++++++++++++++++++++++++-
drivers/net/wireless/wl12xx/cmd.h | 4 +-
drivers/net/wireless/wl12xx/main.c | 40 +++++++--------------------
drivers/net/wireless/wl12xx/scan.c | 3 +-
drivers/net/wireless/wl12xx/tx.c | 6 +---
5 files changed, 65 insertions(+), 41 deletions(-)
diff --git a/drivers/net/wireless/wl12xx/cmd.c b/drivers/net/wireless/wl12xx/cmd.c
index 65bf952..8ea295e 100644
--- a/drivers/net/wireless/wl12xx/cmd.c
+++ b/drivers/net/wireless/wl12xx/cmd.c
@@ -467,7 +467,8 @@ static int wl12xx_get_new_session_id(struct wl1271 *wl,
return wlvif->session_counter;
}
-int wl12xx_cmd_role_start_dev(struct wl1271 *wl, struct wl12xx_vif *wlvif)
+static int wl12xx_cmd_role_start_dev(struct wl1271 *wl,
+ struct wl12xx_vif *wlvif)
{
struct wl12xx_cmd_role_start *cmd;
int ret;
@@ -515,7 +516,8 @@ out:
return ret;
}
-int wl12xx_cmd_role_stop_dev(struct wl1271 *wl, struct wl12xx_vif *wlvif)
+static int wl12xx_cmd_role_stop_dev(struct wl1271 *wl,
+ struct wl12xx_vif *wlvif)
{
struct wl12xx_cmd_role_stop *cmd;
int ret;
@@ -1775,3 +1777,50 @@ out_free:
out:
return ret;
}
+
+/* start dev role and roc on its channel */
+int wl12xx_start_dev(struct wl1271 *wl, struct wl12xx_vif *wlvif)
+{
+ int ret;
+
+ if (WARN_ON(!(wlvif->bss_type == BSS_TYPE_STA_BSS ||
+ wlvif->bss_type == BSS_TYPE_IBSS)))
+ return -EINVAL;
+
+ ret = wl12xx_cmd_role_start_dev(wl, wlvif);
+ if (ret < 0)
+ goto out;
+
+ ret = wl12xx_roc(wl, wlvif, wlvif->dev_role_id);
+ if (ret < 0)
+ goto out_stop;
+
+ return 0;
+
+out_stop:
+ wl12xx_cmd_role_stop_dev(wl, wlvif);
+out:
+ return ret;
+}
+
+/* croc dev hlid, and stop the role */
+int wl12xx_stop_dev(struct wl1271 *wl, struct wl12xx_vif *wlvif)
+{
+ int ret;
+
+ if (WARN_ON(!(wlvif->bss_type == BSS_TYPE_STA_BSS ||
+ wlvif->bss_type == BSS_TYPE_IBSS)))
+ return -EINVAL;
+
+ if (test_bit(wlvif->dev_role_id, wl->roc_map)) {
+ ret = wl12xx_croc(wl, wlvif->dev_role_id);
+ if (ret < 0)
+ goto out;
+ }
+
+ ret = wl12xx_cmd_role_stop_dev(wl, wlvif);
+ if (ret < 0)
+ goto out;
+out:
+ return ret;
+}
diff --git a/drivers/net/wireless/wl12xx/cmd.h b/drivers/net/wireless/wl12xx/cmd.h
index 968d5bd..3f7d0b9 100644
--- a/drivers/net/wireless/wl12xx/cmd.h
+++ b/drivers/net/wireless/wl12xx/cmd.h
@@ -39,13 +39,13 @@ int wl1271_cmd_ext_radio_parms(struct wl1271 *wl);
int wl12xx_cmd_role_enable(struct wl1271 *wl, u8 *addr, u8 role_type,
u8 *role_id);
int wl12xx_cmd_role_disable(struct wl1271 *wl, u8 *role_id);
-int wl12xx_cmd_role_start_dev(struct wl1271 *wl, struct wl12xx_vif *wlvif);
-int wl12xx_cmd_role_stop_dev(struct wl1271 *wl, struct wl12xx_vif *wlvif);
int wl12xx_cmd_role_start_sta(struct wl1271 *wl, struct wl12xx_vif *wlvif);
int wl12xx_cmd_role_stop_sta(struct wl1271 *wl, struct wl12xx_vif *wlvif);
int wl12xx_cmd_role_start_ap(struct wl1271 *wl, struct wl12xx_vif *wlvif);
int wl12xx_cmd_role_stop_ap(struct wl1271 *wl, struct wl12xx_vif *wlvif);
int wl12xx_cmd_role_start_ibss(struct wl1271 *wl, struct wl12xx_vif *wlvif);
+int wl12xx_start_dev(struct wl1271 *wl, struct wl12xx_vif *wlvif);
+int wl12xx_stop_dev(struct wl1271 *wl, struct wl12xx_vif *wlvif);
int wl1271_cmd_test(struct wl1271 *wl, void *buf, size_t buf_len, u8 answer);
int wl1271_cmd_interrogate(struct wl1271 *wl, u16 id, void *buf, size_t len);
int wl1271_cmd_configure(struct wl1271 *wl, u16 id, void *buf, size_t len);
diff --git a/drivers/net/wireless/wl12xx/main.c b/drivers/net/wireless/wl12xx/main.c
index f29d18d..2037208 100644
--- a/drivers/net/wireless/wl12xx/main.c
+++ b/drivers/net/wireless/wl12xx/main.c
@@ -2441,11 +2441,7 @@ static int wl1271_sta_handle_idle(struct wl1271 *wl, struct wl12xx_vif *wlvif,
if (idle) {
/* no need to croc if we weren't busy (e.g. during boot) */
if (wl12xx_is_roc(wl)) {
- ret = wl12xx_croc(wl, wlvif->dev_role_id);
- if (ret < 0)
- goto out;
-
- ret = wl12xx_cmd_role_stop_dev(wl, wlvif);
+ ret = wl12xx_stop_dev(wl, wlvif);
if (ret < 0)
goto out;
}
@@ -2467,11 +2463,7 @@ static int wl1271_sta_handle_idle(struct wl1271 *wl, struct wl12xx_vif *wlvif,
ieee80211_sched_scan_stopped(wl->hw);
}
- ret = wl12xx_cmd_role_start_dev(wl, wlvif);
- if (ret < 0)
- goto out;
-
- ret = wl12xx_roc(wl, wlvif, wlvif->dev_role_id);
+ ret = wl12xx_start_dev(wl, wlvif);
if (ret < 0)
goto out;
clear_bit(WL1271_FLAG_IDLE, &wl->flags);
@@ -2537,16 +2529,13 @@ static int wl12xx_config_vif(struct wl1271 *wl, struct wl12xx_vif *wlvif,
*/
if (wl12xx_is_roc(wl) &&
!(conf->flags & IEEE80211_CONF_IDLE)) {
- ret = wl12xx_croc(wl,
- wlvif->dev_role_id);
+ ret = wl12xx_stop_dev(wl, wlvif);
if (ret < 0)
return ret;
- ret = wl12xx_roc(wl, wlvif,
- wlvif->dev_role_id);
+ ret = wl12xx_start_dev(wl, wlvif);
if (ret < 0)
- wl1271_warning("roc failed %d",
- ret);
+ return ret;
}
}
}
@@ -3099,8 +3088,7 @@ static int wl1271_op_hw_scan(struct ieee80211_hw *hw,
ret = -EBUSY;
goto out_sleep;
}
- wl12xx_croc(wl, wlvif->dev_role_id);
- wl12xx_cmd_role_stop_dev(wl, wlvif);
+ wl12xx_stop_dev(wl, wlvif);
}
ret = wl1271_scan(hw->priv, vif, ssid, len, req);
@@ -3611,8 +3599,7 @@ static void wl1271_bss_info_changed_sta(struct wl1271 *wl,
if (test_and_clear_bit(WLVIF_FLAG_IBSS_JOINED,
&wlvif->flags)) {
wl1271_unjoin(wl, wlvif);
- wl12xx_cmd_role_start_dev(wl, wlvif);
- wl12xx_roc(wl, wlvif, wlvif->dev_role_id);
+ wl12xx_start_dev(wl, wlvif);
}
}
}
@@ -3788,11 +3775,8 @@ sta_not_found:
}
wl1271_unjoin(wl, wlvif);
- if (!(conf_flags & IEEE80211_CONF_IDLE)) {
- wl12xx_cmd_role_start_dev(wl, wlvif);
- wl12xx_roc(wl, wlvif,
- wlvif->dev_role_id);
- }
+ if (!(conf_flags & IEEE80211_CONF_IDLE))
+ wl12xx_start_dev(wl, wlvif);
}
}
}
@@ -3871,11 +3855,7 @@ sta_not_found:
* STA role). TODO: make it better.
*/
if (wlvif->dev_role_id != WL12XX_INVALID_ROLE_ID) {
- ret = wl12xx_croc(wl, wlvif->dev_role_id);
- if (ret < 0)
- goto out;
-
- ret = wl12xx_cmd_role_stop_dev(wl, wlvif);
+ ret = wl12xx_stop_dev(wl, wlvif);
if (ret < 0)
goto out;
}
diff --git a/drivers/net/wireless/wl12xx/scan.c b/drivers/net/wireless/wl12xx/scan.c
index 2711438..96d804f 100644
--- a/drivers/net/wireless/wl12xx/scan.c
+++ b/drivers/net/wireless/wl12xx/scan.c
@@ -76,8 +76,7 @@ void wl1271_scan_complete_work(struct work_struct *work)
(is_ibss && !test_bit(WLVIF_FLAG_IBSS_JOINED, &wlvif->flags))) &&
!test_bit(wlvif->dev_role_id, wl->roc_map)) {
/* restore remain on channel */
- wl12xx_cmd_role_start_dev(wl, wlvif);
- wl12xx_roc(wl, wlvif, wlvif->dev_role_id);
+ wl12xx_start_dev(wl, wlvif);
}
wl1271_ps_elp_sleep(wl);
diff --git a/drivers/net/wireless/wl12xx/tx.c b/drivers/net/wireless/wl12xx/tx.c
index 05968c6..02d606f 100644
--- a/drivers/net/wireless/wl12xx/tx.c
+++ b/drivers/net/wireless/wl12xx/tx.c
@@ -98,11 +98,7 @@ static int wl1271_tx_update_filters(struct wl1271 *wl,
goto out;
wl1271_debug(DEBUG_CMD, "starting device role for roaming");
- ret = wl12xx_cmd_role_start_dev(wl, wlvif);
- if (ret < 0)
- goto out;
-
- ret = wl12xx_roc(wl, wlvif, wlvif->dev_role_id);
+ ret = wl12xx_start_dev(wl, wlvif);
if (ret < 0)
goto out;
out:
--
1.7.6.401.g6a319
^ permalink raw reply related
* [PATCH v2 08/29] wl12xx: configure sleep_policy according to active roles
From: Eliad Peller @ 2011-10-11 9:49 UTC (permalink / raw)
To: Luciano Coelho; +Cc: linux-wireless
If there is an active AP role, stay always on.
Otherwise, allow chip to enter elp.
(Note that this is a global configuration, so if the
device is already configured according to our policy,
we don't have to configure it again)
Signed-off-by: Eliad Peller <eliad@wizery.com>
---
v2: use ap_count/sta_count instead of iterating
drivers/net/wireless/wl12xx/init.c | 29 ++++++++++++++++++-----------
drivers/net/wireless/wl12xx/main.c | 10 ++++++++++
drivers/net/wireless/wl12xx/wl12xx.h | 2 ++
3 files changed, 30 insertions(+), 11 deletions(-)
diff --git a/drivers/net/wireless/wl12xx/init.c b/drivers/net/wireless/wl12xx/init.c
index 80e89e3..4af7e2f 100644
--- a/drivers/net/wireless/wl12xx/init.c
+++ b/drivers/net/wireless/wl12xx/init.c
@@ -343,11 +343,6 @@ static int wl1271_sta_hw_init(struct wl1271 *wl, struct wl12xx_vif *wlvif)
if (ret < 0)
return ret;
- /* Configure for ELP power saving */
- ret = wl1271_acx_sleep_auth(wl, WL1271_PSM_ELP);
- if (ret < 0)
- return ret;
-
ret = wl1271_acx_sta_rate_policies(wl, wlvif);
if (ret < 0)
return ret;
@@ -382,11 +377,6 @@ static int wl1271_ap_hw_init(struct wl1271 *wl, struct wl12xx_vif *wlvif)
{
int ret;
- /* Configure for power always on */
- ret = wl1271_acx_sleep_auth(wl, WL1271_PSM_CAM);
- if (ret < 0)
- return ret;
-
ret = wl1271_init_ap_rates(wl, wlvif);
if (ret < 0)
return ret;
@@ -577,9 +567,26 @@ int wl1271_init_vif_specific(struct wl1271 *wl, struct ieee80211_vif *vif)
struct conf_tx_ac_category *conf_ac;
struct conf_tx_tid *conf_tid;
bool is_ap = (wlvif->bss_type == BSS_TYPE_AP_BSS);
-
int ret, i;
+ /*
+ * consider all existing roles before configuring psm.
+ * TODO: reconfigure on interface removal.
+ */
+ if (!wl->ap_count) {
+ if (is_ap) {
+ /* Configure for power always on */
+ ret = wl1271_acx_sleep_auth(wl, WL1271_PSM_CAM);
+ if (ret < 0)
+ return ret;
+ } else if (!wl->sta_count) {
+ /* Configure for ELP power saving */
+ ret = wl1271_acx_sleep_auth(wl, WL1271_PSM_ELP);
+ if (ret < 0)
+ return ret;
+ }
+ }
+
/* Mode specific init */
if (is_ap) {
ret = wl1271_ap_hw_init(wl, wlvif);
diff --git a/drivers/net/wireless/wl12xx/main.c b/drivers/net/wireless/wl12xx/main.c
index 0623f5d..b52deac 100644
--- a/drivers/net/wireless/wl12xx/main.c
+++ b/drivers/net/wireless/wl12xx/main.c
@@ -2117,6 +2117,11 @@ static int wl1271_op_add_interface(struct ieee80211_hw *hw,
wl->vif = vif;
list_add(&wlvif->list, &wl->wlvif_list);
set_bit(WL1271_FLAG_IF_INITIALIZED, &wl->flags);
+
+ if (wlvif->bss_type == BSS_TYPE_AP_BSS)
+ wl->ap_count++;
+ else
+ wl->sta_count++;
out:
mutex_unlock(&wl->mutex);
@@ -2188,6 +2193,11 @@ deinit:
wlvif->role_id = WL12XX_INVALID_ROLE_ID;
wlvif->dev_role_id = WL12XX_INVALID_ROLE_ID;
+ if (wlvif->bss_type == BSS_TYPE_AP_BSS)
+ wl->ap_count--;
+ else
+ wl->sta_count--;
+
mutex_unlock(&wl->mutex);
cancel_delayed_work_sync(&wlvif->pspoll_work);
diff --git a/drivers/net/wireless/wl12xx/wl12xx.h b/drivers/net/wireless/wl12xx/wl12xx.h
index 55561c5..fd78f8c 100644
--- a/drivers/net/wireless/wl12xx/wl12xx.h
+++ b/drivers/net/wireless/wl12xx/wl12xx.h
@@ -395,6 +395,8 @@ struct wl1271 {
unsigned long roc_map[BITS_TO_LONGS(WL12XX_MAX_ROLES)];
struct list_head wlvif_list;
+ u8 sta_count;
+ u8 ap_count;
struct wl1271_acx_mem_map *target_mem_map;
--
1.7.6.401.g6a319
^ permalink raw reply related
* [PATCH] wl12xx: reconfigure rate policies on set_bitrate_mask
From: Eliad Peller @ 2011-10-11 9:57 UTC (permalink / raw)
To: Luciano Coelho; +Cc: linux-wireless
The rate policies are configured only after association,
resulting in auth req being sent in wrong rates.
Reconfigure rate policies on bitrate mask change.
Signed-off-by: Eliad Peller <eliad@wizery.com>
---
drivers/net/wireless/wl12xx/main.c | 23 +++++++++++++++++++++--
1 files changed, 21 insertions(+), 2 deletions(-)
diff --git a/drivers/net/wireless/wl12xx/main.c b/drivers/net/wireless/wl12xx/main.c
index 2037208..d2b13ca 100644
--- a/drivers/net/wireless/wl12xx/main.c
+++ b/drivers/net/wireless/wl12xx/main.c
@@ -4330,7 +4330,7 @@ static int wl12xx_set_bitrate_mask(struct ieee80211_hw *hw,
{
struct wl12xx_vif *wlvif = wl12xx_vif_to_data(vif);
struct wl1271 *wl = hw->priv;
- int i;
+ int i, ret = 0;
wl1271_debug(DEBUG_MAC80211, "mac80211 set_bitrate_mask 0x%x 0x%x",
mask->control[NL80211_BAND_2GHZ].legacy,
@@ -4343,9 +4343,28 @@ static int wl12xx_set_bitrate_mask(struct ieee80211_hw *hw,
wl1271_tx_enabled_rates_get(wl,
mask->control[i].legacy,
i);
+
+ if (unlikely(wl->state == WL1271_STATE_OFF))
+ goto out;
+
+ if (wlvif->bss_type == BSS_TYPE_STA_BSS &&
+ !test_bit(WLVIF_FLAG_STA_ASSOCIATED, &wlvif->flags)) {
+
+ ret = wl1271_ps_elp_wakeup(wl);
+ if (ret < 0)
+ goto out;
+
+ wl1271_set_band_rate(wl, wlvif);
+ wlvif->basic_rate =
+ wl1271_tx_min_rate_get(wl, wlvif->basic_rate_set);
+ ret = wl1271_acx_sta_rate_policies(wl, wlvif);
+
+ wl1271_ps_elp_sleep(wl);
+ }
+out:
mutex_unlock(&wl->mutex);
- return 0;
+ return ret;
}
static void wl12xx_op_channel_switch(struct ieee80211_hw *hw,
--
1.7.6.401.g6a319
^ permalink raw reply related
* Re: [PATCH v2 08/29] wl12xx: configure sleep_policy according to active roles
From: Arik Nemtsov @ 2011-10-11 10:16 UTC (permalink / raw)
To: Eliad Peller; +Cc: Luciano Coelho, linux-wireless
In-Reply-To: <1318326555-13905-1-git-send-email-eliad@wizery.com>
On Tue, Oct 11, 2011 at 11:49, Eliad Peller <eliad@wizery.com> wrote:
> If there is an active AP role, stay always on.
> Otherwise, allow chip to enter elp.
>
> (Note that this is a global configuration, so if the
> device is already configured according to our policy,
> we don't have to configure it again)
>
> Signed-off-by: Eliad Peller <eliad@wizery.com>
> ---
> v2: use ap_count/sta_count instead of iterating
> + if (!wl->ap_count) {
> + if (is_ap) {
> + /* Configure for power always on */
> + ret = wl1271_acx_sleep_auth(wl, WL1271_PSM_CAM);
> + if (ret < 0)
> + return ret;
> + } else if (!wl->sta_count) {
> + /* Configure for ELP power saving */
> + ret = wl1271_acx_sleep_auth(wl, WL1271_PSM_ELP);
> + if (ret < 0)
> + return ret;
> + }
> + }
Probably need to add a symmetric part on remove_interface(). If ap was
shutdown we can return to normal ELP.
btw, elp_work() still gets called even in AP-mode. I'm guessing it has
no effect, but if you want to prevent the SDIO transaction from taking
place (like you said on IRC), you can check for it :)
(Just nitpicking here)
^ permalink raw reply
* Re: [PATCH v2 08/29] wl12xx: configure sleep_policy according to active roles
From: Eliad Peller @ 2011-10-11 10:46 UTC (permalink / raw)
To: Arik Nemtsov; +Cc: Luciano Coelho, linux-wireless
In-Reply-To: <CA+XVXfdM7=G1YdWVYpzfKajRYu+sAJcfKSFkbtgneUPTPabQ=w@mail.gmail.com>
On Tue, Oct 11, 2011 at 12:16 PM, Arik Nemtsov <anamtsov@gmail.com> wrote:
> On Tue, Oct 11, 2011 at 11:49, Eliad Peller <eliad@wizery.com> wrote:
>> If there is an active AP role, stay always on.
>> Otherwise, allow chip to enter elp.
>>
>> (Note that this is a global configuration, so if the
>> device is already configured according to our policy,
>> we don't have to configure it again)
>>
>> Signed-off-by: Eliad Peller <eliad@wizery.com>
>> ---
>> v2: use ap_count/sta_count instead of iterating
>> + if (!wl->ap_count) {
>> + if (is_ap) {
>> + /* Configure for power always on */
>> + ret = wl1271_acx_sleep_auth(wl, WL1271_PSM_CAM);
>> + if (ret < 0)
>> + return ret;
>> + } else if (!wl->sta_count) {
>> + /* Configure for ELP power saving */
>> + ret = wl1271_acx_sleep_auth(wl, WL1271_PSM_ELP);
>> + if (ret < 0)
>> + return ret;
>> + }
>> + }
>
> Probably need to add a symmetric part on remove_interface(). If ap was
> shutdown we can return to normal ELP.
>
right. that's why i added:
+ /*
+ * consider all existing roles before configuring psm.
+ * TODO: reconfigure on interface removal.
+ */
> btw, elp_work() still gets called even in AP-mode. I'm guessing it has
> no effect, but if you want to prevent the SDIO transaction from taking
> place (like you said on IRC), you can check for it :)
> (Just nitpicking here)
we are not in idle-on while beaconing, so elp_work should return
before configuring the chip.
Eliad.
^ permalink raw reply
* [PATCH] iw: fix HT PHY BSS Membership selector value encoding
From: Christian Lamparter @ 2011-10-11 11:15 UTC (permalink / raw)
To: linux-wireless; +Cc: Johannes Berg
IEEE 802.11n 7.3.2.2 extended the supported rate IE to
support a special encoding for HT rate support.
iw needs to be updated in order to recognize
the magic value and parse it accordingly.
e.g.:
> Extended supported rates: 63.5*
now becomes:
> Extended supported rates: HT*
Signed-off-by: Christian Lamparter <chunkeey@googlemail.com>
---
diff --git a/scan.c b/scan.c
index d083591..2220f26 100644
--- a/scan.c
+++ b/scan.c
@@ -158,6 +158,8 @@ static void print_ssid(const uint8_t type, uint8_t len, const uint8_t *data)
printf("\n");
}
+#define BSS_MEMBERSHIP_SELECTOR_HT_PHY 127
+
static void print_supprates(const uint8_t type, uint8_t len, const uint8_t *data)
{
int i;
@@ -166,7 +168,13 @@ static void print_supprates(const uint8_t type, uint8_t len, const uint8_t *data
for (i = 0; i < len; i++) {
int r = data[i] & 0x7f;
- printf("%d.%d%s ", r/2, 5*(r&1), data[i] & 0x80 ? "*":"");
+
+ if (r == BSS_MEMBERSHIP_SELECTOR_HT_PHY)
+ printf("HT");
+ else
+ printf("%d.%d", r/2, 5*(r&1));
+
+ printf("%s ", data[i] & 0x80 ? "*" : "");
}
printf("\n");
}
^ permalink raw reply related
* Re: [PATCH 1/1] ath6kl: Add WSC IE on the associate message
From: Kalle Valo @ 2011-10-11 11:24 UTC (permalink / raw)
To: Kevin Fang; +Cc: linux-wireless
In-Reply-To: <1317948679-10299-1-git-send-email-kevin.fang@qca.qualcomm.com>
On 10/07/2011 03:51 AM, Kevin Fang wrote:
> For some WPS test items, such as item "5.1.14"
> STAUT must include the WSC IE in the 802.11 Association Request frame.
> Therefore, add the corresponding IE in association message.
Applied, thanks!
Kalle
^ permalink raw reply
* Re: [PATCH] ath6kl: fix null skb dereference in ath6kl_rx()
From: Kalle Valo @ 2011-10-11 11:23 UTC (permalink / raw)
To: Kalle Valo; +Cc: linux-wireless
In-Reply-To: <20111006113232.24700.12168.stgit@localhost6.localdomain6>
On 10/06/2011 02:32 PM, Kalle Valo wrote:
> smatch found that skb might be null in some cases in ath6kl_rx():
>
> ath6kl/txrx.c +1252 ath6kl_rx(222) error: potential null derefence 'skb'.
>
> This will happen when ath6kl is in AP mode and two clients send traffic
> to each other.
Applied to ath6kl.git.
Kalle
^ permalink raw reply
* [PATCH] mac80211: Populate radiotap header with MCS info for tx'ed frames
From: Helmut Schaa @ 2011-10-11 11:28 UTC (permalink / raw)
To: linux-wireless; +Cc: linville, johannes, Helmut Schaa
mac80211 already filled in the MCS rate info for rx'ed frames but tx'ed
frames that are sent to a monitor interface during the status callback
lack this information.
Add the radiotap fields for MCS info to ieee80211_tx_status_rtap_hdr
and populate them when sending tx'ed frames to the monitors.
One minor flaw is that the radiotap header now includes both, the rate
field and the mcs field. For HT frames the rate field will be zero and
for legacy frames the mcs fields will be zero but still this could
be improved.
Signed-off-by: Helmut Schaa <helmut.schaa@googlemail.com>
---
Wireshark will display the MCS rate correctly but will show the zero'd rate
field as well with 0Mbps. For legacy frames the MCS field won't be shown
since IEEE80211_RADIOTAP_MCS_HAVE_MCS isn't set. Only the MCS flags will be
shown (all zero of course).
I still think it is justified to use it like this as otherwise we would have
to build the radiotap header in a more generic fashion and we won't be able to
easily catch build bugs like
BUILD_BUG_ON(IEEE80211_TX_STATUS_HEADROOM !=
sizeof(struct ieee80211_tx_status_rtap_hdr));
include/net/mac80211.h | 2 +-
net/mac80211/ieee80211_i.h | 3 +++
net/mac80211/status.c | 26 +++++++++++++++++++++-----
3 files changed, 25 insertions(+), 6 deletions(-)
diff --git a/include/net/mac80211.h b/include/net/mac80211.h
index cd108df..a500c7b 100644
--- a/include/net/mac80211.h
+++ b/include/net/mac80211.h
@@ -2520,7 +2520,7 @@ static inline int ieee80211_sta_ps_transition_ni(struct ieee80211_sta *sta,
* The TX headroom reserved by mac80211 for its own tx_status functions.
* This is enough for the radiotap header.
*/
-#define IEEE80211_TX_STATUS_HEADROOM 13
+#define IEEE80211_TX_STATUS_HEADROOM 16
/**
* ieee80211_sta_set_buffered - inform mac80211 about driver-buffered frames
diff --git a/net/mac80211/ieee80211_i.h b/net/mac80211/ieee80211_i.h
index 9fa5f8a..41dc565 100644
--- a/net/mac80211/ieee80211_i.h
+++ b/net/mac80211/ieee80211_i.h
@@ -1187,6 +1187,9 @@ struct ieee80211_tx_status_rtap_hdr {
u8 padding_for_rate;
__le16 tx_flags;
u8 data_retries;
+ u8 mcs_known;
+ u8 mcs_flags;
+ u8 mcs;
} __packed;
diff --git a/net/mac80211/status.c b/net/mac80211/status.c
index 864a9c3..34319fe 100644
--- a/net/mac80211/status.c
+++ b/net/mac80211/status.c
@@ -475,7 +475,8 @@ void ieee80211_tx_status(struct ieee80211_hw *hw, struct sk_buff *skb)
rthdr->hdr.it_present =
cpu_to_le32((1 << IEEE80211_RADIOTAP_TX_FLAGS) |
(1 << IEEE80211_RADIOTAP_DATA_RETRIES) |
- (1 << IEEE80211_RADIOTAP_RATE));
+ (1 << IEEE80211_RADIOTAP_RATE) |
+ (1 << IEEE80211_RADIOTAP_MCS));
if (!(info->flags & IEEE80211_TX_STAT_ACK) &&
!is_multicast_ether_addr(hdr->addr1))
@@ -491,10 +492,25 @@ void ieee80211_tx_status(struct ieee80211_hw *hw, struct sk_buff *skb)
rthdr->tx_flags |= cpu_to_le16(IEEE80211_RADIOTAP_F_TX_CTS);
else if (info->status.rates[0].flags & IEEE80211_TX_RC_USE_RTS_CTS)
rthdr->tx_flags |= cpu_to_le16(IEEE80211_RADIOTAP_F_TX_RTS);
- if (info->status.rates[0].idx >= 0 &&
- !(info->status.rates[0].flags & IEEE80211_TX_RC_MCS))
- rthdr->rate = sband->bitrates[
- info->status.rates[0].idx].bitrate / 5;
+ if (info->status.rates[0].idx >= 0) {
+ if (!(info->status.rates[0].flags & IEEE80211_TX_RC_MCS)) {
+ /* legacy rate */
+ rthdr->rate = sband->bitrates[
+ info->status.rates[0].idx].bitrate / 5;
+ } else {
+ /* HT rate */
+ rthdr->mcs_known = IEEE80211_RADIOTAP_MCS_HAVE_MCS |
+ IEEE80211_RADIOTAP_MCS_HAVE_GI |
+ IEEE80211_RADIOTAP_MCS_HAVE_BW;
+ if (info->status.rates[0].flags & IEEE80211_TX_RC_SHORT_GI)
+ rthdr->mcs_flags |= IEEE80211_RADIOTAP_MCS_SGI;
+ if (info->status.rates[0].flags & IEEE80211_TX_RC_40_MHZ_WIDTH)
+ rthdr->mcs_flags |= IEEE80211_RADIOTAP_MCS_BW_40;
+ if (info->status.rates[0].flags & IEEE80211_TX_RC_GREEN_FIELD)
+ rthdr->mcs_flags |= IEEE80211_RADIOTAP_MCS_FMT_GF;
+ rthdr->mcs = info->status.rates[0].idx;
+ }
+ }
/* for now report the total retry_count */
rthdr->data_retries = retry_count;
--
1.7.3.4
^ permalink raw reply related
* Re: [PATCH] mac80211: Populate radiotap header with MCS info for tx'ed frames
From: Johannes Berg @ 2011-10-11 11:35 UTC (permalink / raw)
To: Helmut Schaa; +Cc: linux-wireless, linville
In-Reply-To: <1318332535-8815-1-git-send-email-helmut.schaa@googlemail.com>
On Tue, 2011-10-11 at 13:28 +0200, Helmut Schaa wrote:
> mac80211 already filled in the MCS rate info for rx'ed frames but tx'ed
> frames that are sent to a monitor interface during the status callback
> lack this information.
>
> Add the radiotap fields for MCS info to ieee80211_tx_status_rtap_hdr
> and populate them when sending tx'ed frames to the monitors.
>
> One minor flaw is that the radiotap header now includes both, the rate
> field and the mcs field. For HT frames the rate field will be zero and
> for legacy frames the mcs fields will be zero but still this could
> be improved.
> Wireshark will display the MCS rate correctly but will show the zero'd rate
> field as well with 0Mbps. For legacy frames the MCS field won't be shown
> since IEEE80211_RADIOTAP_MCS_HAVE_MCS isn't set. Only the MCS flags will be
> shown (all zero of course).
>
> I still think it is justified to use it like this as otherwise we would have
> to build the radiotap header in a more generic fashion and we won't be able to
> easily catch build bugs like
>
> BUILD_BUG_ON(IEEE80211_TX_STATUS_HEADROOM !=
> sizeof(struct ieee80211_tx_status_rtap_hdr));
Hmm. I don't like it much, but it's a bug right now too. Too bad the
rate field doesn't just fit into the padding :-)
Actually though, the MCS field is three bytes. So using something like
this would allow us to save some space as well:
struct ieee80211_tx_status_rtap_hdr {
struct ieee80211_radiotap_header hdr;
union {
struct {
u8 rate;
u8 padding_for_rate;
__le16 tx_flags;
u8 data_retries;
} non_mcs;
struct {
__le16 tx_flags;
u8 data_retries;
u8 mcs_known, mcs_flags, mcs;
} mcs;
}
} __packed;
johannes
^ permalink raw reply
* Re: [PATCH 2/5] ath6kl: Add debugfs file for target roam table
From: Kalle Valo @ 2011-10-11 11:39 UTC (permalink / raw)
To: Jouni Malinen; +Cc: linux-wireless
In-Reply-To: <1318243411-16110-3-git-send-email-jouni@qca.qualcomm.com>
On 10/10/2011 01:43 PM, Jouni Malinen wrote:
> The new roam_table debugfs file can be used to display the current
> roam table from the target.
One comment:
> +static int ath6kl_wmi_roam_tbl_event_rx(struct wmi *wmi, u8 *datap, int len)
> +{
> +#ifdef CONFIG_ATH6KL_DEBUG
> + struct ath6kl *ar = wmi->parent_dev;
> + struct wmi_target_roam_tbl *tbl;
> + u16 num_entries;
> +
> + if (len < sizeof(*tbl))
> + return -EINVAL;
> +
> + tbl = (struct wmi_target_roam_tbl *) datap;
> + num_entries = le16_to_cpu(tbl->num_entries);
> + if (sizeof(*tbl) + num_entries * sizeof(struct wmi_bss_roam_info) > len)
> + return -EINVAL;
> +
> + if (ar->debug.roam_tbl == NULL ||
> + ar->debug.roam_tbl_len < (unsigned int) len) {
> + kfree(ar->debug.roam_tbl);
> + ar->debug.roam_tbl = kmalloc(len, GFP_ATOMIC);
> + if (ar->debug.roam_tbl == NULL)
> + return -ENOMEM;
> + }
> +
> + memcpy(ar->debug.roam_tbl, datap, len);
> + ar->debug.roam_tbl_len = len;
> +
> + if (test_bit(ROAM_TBL_PEND, &ar->flag)) {
> + clear_bit(ROAM_TBL_PEND, &ar->flag);
> + wake_up(&ar->event_wq);
> + }
> +#endif /* CONFIG_ATH6KL_DEBUG */
> +
> + return 0;
> +}
I would prefer to have the part inside ifdef in debug.c, for example
like ath6kl_debug_fwlog_event() is implemented. That way we can get rid
of the ifdef inside code and related functinality would be in the same file.
Kalle
^ permalink raw reply
* Re: [PATCH] mac80211: Populate radiotap header with MCS info for tx'ed frames
From: Helmut Schaa @ 2011-10-11 11:48 UTC (permalink / raw)
To: Johannes Berg; +Cc: linux-wireless, linville
In-Reply-To: <1318332927.3965.9.camel@jlt3.sipsolutions.net>
On Tue, Oct 11, 2011 at 1:35 PM, Johannes Berg
<johannes@sipsolutions.net> wrote:
> So using something like
> this would allow us to save some space as well:
>
> struct ieee80211_tx_status_rtap_hdr {
> struct ieee80211_radiotap_header hdr;
> union {
> struct {
> u8 rate;
> u8 padding_for_rate;
> __le16 tx_flags;
> u8 data_retries;
> } non_mcs;
> struct {
> __le16 tx_flags;
> u8 data_retries;
> u8 mcs_known, mcs_flags, mcs;
> } mcs;
> }
> } __packed;
I thought about the same but this means we have to fill in
different fields (non_mcs.tx_flags vs mcs.tx_flags) for the MCS
vs legacy path and set the radiotap header len differently in
both cases.
Or should we just get rid of the whole struct and fill in the
rtap header dynamically like it is done in the rx path and
drop the build-bug-on thing?
Helmut
^ permalink raw reply
* Re: [PATCH] mac80211: Populate radiotap header with MCS info for tx'ed frames
From: Johannes Berg @ 2011-10-11 11:51 UTC (permalink / raw)
To: Helmut Schaa; +Cc: linux-wireless, linville
In-Reply-To: <CAGXE3d8cuBg28PfMYQcAJ96oTEqUg6rG8ExUr+Sezq51eM2J_Q@mail.gmail.com>
On Tue, 2011-10-11 at 13:48 +0200, Helmut Schaa wrote:
> On Tue, Oct 11, 2011 at 1:35 PM, Johannes Berg
> <johannes@sipsolutions.net> wrote:
> > So using something like
> > this would allow us to save some space as well:
> >
> > struct ieee80211_tx_status_rtap_hdr {
> > struct ieee80211_radiotap_header hdr;
> > union {
> > struct {
> > u8 rate;
> > u8 padding_for_rate;
> > __le16 tx_flags;
> > u8 data_retries;
> > } non_mcs;
> > struct {
> > __le16 tx_flags;
> > u8 data_retries;
> > u8 mcs_known, mcs_flags, mcs;
> > } mcs;
> > }
> > } __packed;
>
> I thought about the same but this means we have to fill in
> different fields (non_mcs.tx_flags vs mcs.tx_flags) for the MCS
> vs legacy path and set the radiotap header len differently in
> both cases.
Good point.
> Or should we just get rid of the whole struct and fill in the
> rtap header dynamically like it is done in the rx path and
> drop the build-bug-on thing?
I wouldn't mind that either, but we'll want to have a WARN_ON_ONCE()
somewhere if we run out of space.
johannes
^ permalink raw reply
* [PATCH] wl12xx: handle injected packets
From: Eliad Peller @ 2011-10-11 11:52 UTC (permalink / raw)
To: Luciano Coelho; +Cc: linux-wireless
Injected packets are sent with no vif, causing the wl12xx
to NULL-dereference in multiple places.
Furthermore, injected packets are currently not sent at all,
as system_hlid doesn't belong to any specific role, so
wl1271_skb_dequeue() never return its packets.
Handle both these problems.
Signed-off-by: Eliad Peller <eliad@wizery.com>
---
thanks Johannes for providing a testing app
drivers/net/wireless/wl12xx/main.c | 7 +++++--
drivers/net/wireless/wl12xx/tx.c | 27 +++++++++++++--------------
2 files changed, 18 insertions(+), 16 deletions(-)
diff --git a/drivers/net/wireless/wl12xx/main.c b/drivers/net/wireless/wl12xx/main.c
index 692a275..fc652c4 100644
--- a/drivers/net/wireless/wl12xx/main.c
+++ b/drivers/net/wireless/wl12xx/main.c
@@ -1517,11 +1517,14 @@ static void wl1271_op_tx(struct ieee80211_hw *hw, struct sk_buff *skb)
struct wl1271 *wl = hw->priv;
struct ieee80211_tx_info *info = IEEE80211_SKB_CB(skb);
struct ieee80211_vif *vif = info->control.vif;
- struct wl12xx_vif *wlvif = wl12xx_vif_to_data(vif);
+ struct wl12xx_vif *wlvif = NULL;
unsigned long flags;
int q, mapping;
u8 hlid;
+ if (vif)
+ wlvif = wl12xx_vif_to_data(vif);
+
mapping = skb_get_queue_mapping(skb);
q = wl1271_tx_get_queue(mapping);
@@ -1531,7 +1534,7 @@ static void wl1271_op_tx(struct ieee80211_hw *hw, struct sk_buff *skb)
/* queue the packet */
if (hlid == WL12XX_INVALID_LINK_ID ||
- !test_bit(hlid, wlvif->links_map)) {
+ (wlvif && !test_bit(hlid, wlvif->links_map))) {
wl1271_debug(DEBUG_TX, "DROP skb hlid %d q %d", hlid, q);
dev_kfree_skb(skb);
goto out;
diff --git a/drivers/net/wireless/wl12xx/tx.c b/drivers/net/wireless/wl12xx/tx.c
index 02d606f..33b800d 100644
--- a/drivers/net/wireless/wl12xx/tx.c
+++ b/drivers/net/wireless/wl12xx/tx.c
@@ -180,7 +180,7 @@ u8 wl12xx_tx_get_hlid(struct wl1271 *wl, struct wl12xx_vif *wlvif,
{
struct ieee80211_hdr *hdr = (struct ieee80211_hdr *)skb->data;
- if (wl12xx_is_dummy_packet(wl, skb))
+ if (!wlvif || wl12xx_is_dummy_packet(wl, skb))
return wl->system_hlid;
if (wlvif->bss_type == BSS_TYPE_AP_BSS)
@@ -259,7 +259,8 @@ static int wl1271_tx_allocate(struct wl1271 *wl, struct wl12xx_vif *wlvif,
ac = wl1271_tx_get_queue(skb_get_queue_mapping(skb));
wl->tx_allocated_pkts[ac]++;
- if (!is_dummy && wlvif->bss_type == BSS_TYPE_AP_BSS &&
+ if (!is_dummy && wlvif &&
+ wlvif->bss_type == BSS_TYPE_AP_BSS &&
test_bit(hlid, wlvif->ap.sta_hlid_map))
wl->links[hlid].allocated_pkts++;
@@ -302,7 +303,7 @@ static void wl1271_tx_fill_hdr(struct wl1271 *wl, struct wl12xx_vif *wlvif,
desc->start_time = cpu_to_le32(hosttime - wl->time_offset);
is_dummy = wl12xx_is_dummy_packet(wl, skb);
- if (is_dummy || wlvif->bss_type != BSS_TYPE_AP_BSS)
+ if (is_dummy || !wlvif || wlvif->bss_type != BSS_TYPE_AP_BSS)
desc->life_time = cpu_to_le16(TX_HW_MGMT_PKT_LIFETIME_TU);
else
desc->life_time = cpu_to_le16(TX_HW_AP_MODE_PKT_LIFETIME_TU);
@@ -321,14 +322,14 @@ static void wl1271_tx_fill_hdr(struct wl1271 *wl, struct wl12xx_vif *wlvif,
TX_HW_ATTR_SESSION_COUNTER;
tx_attr |= TX_HW_ATTR_TX_DUMMY_REQ;
- } else {
+ } else if (wlvif) {
/* configure the tx attributes */
tx_attr = wlvif->session_counter <<
TX_HW_ATTR_OFST_SESSION_COUNTER;
}
desc->hlid = hlid;
- if (is_dummy)
+ if (is_dummy || !wlvif)
rate_idx = 0;
else if (wlvif->bss_type != BSS_TYPE_AP_BSS) {
/* if the packets are destined for AP (have a STA entry)
@@ -433,7 +434,7 @@ static int wl1271_prepare_tx_frame(struct wl1271 *wl, struct wl12xx_vif *wlvif,
wl1271_tx_fill_hdr(wl, wlvif, skb, extra, info, hlid);
- if (!is_dummy && wlvif->bss_type == BSS_TYPE_AP_BSS) {
+ if (!is_dummy && wlvif && wlvif->bss_type == BSS_TYPE_AP_BSS) {
wl1271_tx_ap_update_inconnection_sta(wl, skb);
wl1271_tx_regulate_link(wl, wlvif, hlid);
}
@@ -610,6 +611,9 @@ static struct sk_buff *wl1271_skb_dequeue(struct wl1271 *wl)
}
}
+ if (!skb)
+ skb = wl12xx_lnk_skb_dequeue(wl, &wl->links[wl->system_hlid]);
+
if (!skb &&
test_and_clear_bit(WL1271_FLAG_DUMMY_PACKET_PENDING, &wl->flags)) {
int q;
@@ -703,19 +707,14 @@ void wl1271_tx_work_locked(struct wl1271 *wl)
return;
while ((skb = wl1271_skb_dequeue(wl))) {
+ struct ieee80211_tx_info *info = IEEE80211_SKB_CB(skb);
bool has_data = false;
wlvif = NULL;
- if (!wl12xx_is_dummy_packet(wl, skb)) {
- struct ieee80211_tx_info *info;
- struct ieee80211_vif *vif;
+ if (!wl12xx_is_dummy_packet(wl, skb) && info->control.vif)
+ wlvif = wl12xx_vif_to_data(info->control.vif);
- info = IEEE80211_SKB_CB(skb);
- vif = info->control.vif;
- wlvif = wl12xx_vif_to_data(vif);
- }
has_data = wlvif && wl1271_tx_is_data_present(skb);
-
ret = wl1271_prepare_tx_frame(wl, wlvif, skb, buf_offset);
if (ret == -EAGAIN) {
/*
--
1.7.6.401.g6a319
^ permalink raw reply related
* Re: [PATCH] mac80211: Populate radiotap header with MCS info for tx'ed frames
From: Helmut Schaa @ 2011-10-11 11:52 UTC (permalink / raw)
To: Johannes Berg; +Cc: linux-wireless, linville
In-Reply-To: <1318333875.3965.10.camel@jlt3.sipsolutions.net>
On Tue, Oct 11, 2011 at 1:51 PM, Johannes Berg
<johannes@sipsolutions.net> wrote:
>> Or should we just get rid of the whole struct and fill in the
>> rtap header dynamically like it is done in the rx path and
>> drop the build-bug-on thing?
>
> I wouldn't mind that either, but we'll want to have a WARN_ON_ONCE()
> somewhere if we run out of space.
Ok, let's try it that way. John please drop this one, I'll do a v2.
Helmut
^ permalink raw reply
* Re: [PATCH 4/5] ath6kl: Add debugfs control for keepalive and disconnection timeout
From: Kalle Valo @ 2011-10-11 11:55 UTC (permalink / raw)
To: Jouni Malinen; +Cc: linux-wireless
In-Reply-To: <1318243411-16110-5-git-send-email-jouni@qca.qualcomm.com>
On 10/10/2011 01:43 PM, Jouni Malinen wrote:
> The new debugfs files keepalive and disconnect_timeout can be used to
> fetch the current values and to change the values for keepalive and
> disconnect event timeout (both in seconds).
[...]
> --- a/drivers/net/wireless/ath/ath6kl/wmi.c
> +++ b/drivers/net/wireless/ath/ath6kl/wmi.c
> @@ -1940,6 +1940,10 @@ int ath6kl_wmi_disctimeout_cmd(struct wmi *wmi, u8 timeout)
>
> ret = ath6kl_wmi_cmd_send(wmi, skb, WMI_SET_DISC_TIMEOUT_CMDID,
> NO_SYNC_WMIFLAG);
> +#ifdef CONFIG_ATH6KL_DEBUG
> + if (ret == 0)
> + wmi->parent_dev->debug.disc_timeout = timeout;
> +#endif /* CONFIG_ATH6KL_DEBUG */
> return ret;
> }
>
> @@ -2524,6 +2528,10 @@ int ath6kl_wmi_set_keepalive_cmd(struct wmi *wmi, u8 keep_alive_intvl)
>
> ret = ath6kl_wmi_cmd_send(wmi, skb, WMI_SET_KEEPALIVE_CMDID,
> NO_SYNC_WMIFLAG);
> +#ifdef CONFIG_ATH6KL_DEBUG
> + if (ret == 0)
> + wmi->parent_dev->debug.keepalive = keep_alive_intvl;
> +#endif /* CONFIG_ATH6KL_DEBUG */
Similar comment for these two as well. It would better to have the code
in debug.c (or .h) and add functions for providing this information.
Kalle
^ permalink raw reply
* Re: [PATCH 17/34] iwlagn: fix a race in the unmapping of the TFDs
From: Stanislaw Gruszka @ 2011-10-11 12:22 UTC (permalink / raw)
To: Wey-Yi Guy; +Cc: linville, linux-wireless, Emmanuel Grumbach
In-Reply-To: <1318256839-31837-18-git-send-email-wey-yi.w.guy@intel.com>
On Mon, Oct 10, 2011 at 07:27:02AM -0700, Wey-Yi Guy wrote:
> While inspecting the code, I saw that iwl_tx_queue_unmap modifies
> the read pointer of the Tx queue without taking any locks. This means
> that it can race with the reclaim flow. This can possibly lead to
> a DMA warning complaining that we unmap the same buffer twice.
>
> This is more a W/A than a fix since it is really weird to take
> sta_lock inside iwl_tx_queue_unmap, but it can help until we revamp
> the locking model in the transport layer.
>
> Signed-off-by: Emmanuel Grumbach <emmanuel.grumbach@intel.com>
> Signed-off-by: Wey-Yi Guy <wey-yi.w.guy@intel.com>
> ---
> drivers/net/wireless/iwlwifi/iwl-trans-pcie.c | 3 +++
> 1 files changed, 3 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/net/wireless/iwlwifi/iwl-trans-pcie.c b/drivers/net/wireless/iwlwifi/iwl-trans-pcie.c
> index 60067c7..f69aecb 100644
> --- a/drivers/net/wireless/iwlwifi/iwl-trans-pcie.c
> +++ b/drivers/net/wireless/iwlwifi/iwl-trans-pcie.c
> @@ -406,6 +406,7 @@ static void iwl_tx_queue_unmap(struct iwl_trans *trans, int txq_id)
> struct iwl_tx_queue *txq = &trans_pcie->txq[txq_id];
> struct iwl_queue *q = &txq->q;
> enum dma_data_direction dma_dir;
> + unsigned long flags;
>
> if (!q->n_bd)
> return;
> @@ -418,12 +419,14 @@ static void iwl_tx_queue_unmap(struct iwl_trans *trans, int txq_id)
> else
> dma_dir = DMA_TO_DEVICE;
>
> + spin_lock_irqsave(&trans->shrd->sta_lock, flags);
sta_lock does not look like right lock to use.
Stanislaw
^ permalink raw reply
* RE: [PATCH 17/34] iwlagn: fix a race in the unmapping of the TFDs
From: Grumbach, Emmanuel @ 2011-10-11 12:26 UTC (permalink / raw)
To: Stanislaw Gruszka, Guy, Wey-Yi W
Cc: linville@tuxdriver.com, linux-wireless@vger.kernel.org
In-Reply-To: <20111011122216.GA2218@redhat.com>
>
> On Mon, Oct 10, 2011 at 07:27:02AM -0700, Wey-Yi Guy wrote:
> > While inspecting the code, I saw that iwl_tx_queue_unmap modifies
> > the read pointer of the Tx queue without taking any locks. This means
> > that it can race with the reclaim flow. This can possibly lead to
> > a DMA warning complaining that we unmap the same buffer twice.
> >
> > This is more a W/A than a fix since it is really weird to take
> > sta_lock inside iwl_tx_queue_unmap, but it can help until we revamp
> > the locking model in the transport layer.
> >
> > Signed-off-by: Emmanuel Grumbach <emmanuel.grumbach@intel.com>
> > Signed-off-by: Wey-Yi Guy <wey-yi.w.guy@intel.com>
> > ---
> > drivers/net/wireless/iwlwifi/iwl-trans-pcie.c | 3 +++
> > 1 files changed, 3 insertions(+), 0 deletions(-)
> >
> > diff --git a/drivers/net/wireless/iwlwifi/iwl-trans-pcie.c
> b/drivers/net/wireless/iwlwifi/iwl-trans-pcie.c
> > index 60067c7..f69aecb 100644
> > --- a/drivers/net/wireless/iwlwifi/iwl-trans-pcie.c
> > +++ b/drivers/net/wireless/iwlwifi/iwl-trans-pcie.c
> > @@ -406,6 +406,7 @@ static void iwl_tx_queue_unmap(struct iwl_trans
> *trans, int txq_id)
> > struct iwl_tx_queue *txq = &trans_pcie->txq[txq_id];
> > struct iwl_queue *q = &txq->q;
> > enum dma_data_direction dma_dir;
> > + unsigned long flags;
> >
> > if (!q->n_bd)
> > return;
> > @@ -418,12 +419,14 @@ static void iwl_tx_queue_unmap(struct iwl_trans
> *trans, int txq_id)
> > else
> > dma_dir = DMA_TO_DEVICE;
> >
> > + spin_lock_irqsave(&trans->shrd->sta_lock, flags);
> sta_lock does not look like right lock to use.
>
True. And I wrote in the commit message that this is a W/A until we revamp the locking model.
---------------------------------------------------------------------
Intel Israel (74) Limited
This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.
^ permalink raw reply
* Re: [PATCH 17/34] iwlagn: fix a race in the unmapping of the TFDs
From: Stanislaw Gruszka @ 2011-10-11 12:36 UTC (permalink / raw)
To: Grumbach, Emmanuel
Cc: Guy, Wey-Yi W, linville@tuxdriver.com,
linux-wireless@vger.kernel.org
In-Reply-To: <4825B8A2C4E264489E57869F0DCFB2234457A944D0@hasmsx502.ger.corp.intel.com>
On Tue, Oct 11, 2011 at 02:26:30PM +0200, Grumbach, Emmanuel wrote:
> >
> > On Mon, Oct 10, 2011 at 07:27:02AM -0700, Wey-Yi Guy wrote:
> > > While inspecting the code, I saw that iwl_tx_queue_unmap modifies
> > > the read pointer of the Tx queue without taking any locks. This means
> > > that it can race with the reclaim flow. This can possibly lead to
> > > a DMA warning complaining that we unmap the same buffer twice.
> > >
> > > This is more a W/A than a fix since it is really weird to take
> > > sta_lock inside iwl_tx_queue_unmap, but it can help until we revamp
> > > the locking model in the transport layer.
> > >
> > > Signed-off-by: Emmanuel Grumbach <emmanuel.grumbach@intel.com>
> > > Signed-off-by: Wey-Yi Guy <wey-yi.w.guy@intel.com>
> > > ---
> > > drivers/net/wireless/iwlwifi/iwl-trans-pcie.c | 3 +++
> > > 1 files changed, 3 insertions(+), 0 deletions(-)
> > >
> > > diff --git a/drivers/net/wireless/iwlwifi/iwl-trans-pcie.c
> > b/drivers/net/wireless/iwlwifi/iwl-trans-pcie.c
> > > index 60067c7..f69aecb 100644
> > > --- a/drivers/net/wireless/iwlwifi/iwl-trans-pcie.c
> > > +++ b/drivers/net/wireless/iwlwifi/iwl-trans-pcie.c
> > > @@ -406,6 +406,7 @@ static void iwl_tx_queue_unmap(struct iwl_trans
> > *trans, int txq_id)
> > > struct iwl_tx_queue *txq = &trans_pcie->txq[txq_id];
> > > struct iwl_queue *q = &txq->q;
> > > enum dma_data_direction dma_dir;
> > > + unsigned long flags;
> > >
> > > if (!q->n_bd)
> > > return;
> > > @@ -418,12 +419,14 @@ static void iwl_tx_queue_unmap(struct iwl_trans
> > *trans, int txq_id)
> > > else
> > > dma_dir = DMA_TO_DEVICE;
> > >
> > > + spin_lock_irqsave(&trans->shrd->sta_lock, flags);
> > sta_lock does not look like right lock to use.
> >
> True. And I wrote in the commit message that this is a W/A until we revamp the locking model.
Why not simply take hcmd_lock?
^ permalink raw reply
* RE: [PATCH 17/34] iwlagn: fix a race in the unmapping of the TFDs
From: Grumbach, Emmanuel @ 2011-10-11 12:38 UTC (permalink / raw)
To: Stanislaw Gruszka
Cc: Guy, Wey-Yi W, linville@tuxdriver.com,
linux-wireless@vger.kernel.org
In-Reply-To: <20111011123630.GB2218@redhat.com>
> > > > While inspecting the code, I saw that iwl_tx_queue_unmap modifies
> > > > the read pointer of the Tx queue without taking any locks. This means
> > > > that it can race with the reclaim flow. This can possibly lead to
> > > > a DMA warning complaining that we unmap the same buffer twice.
> > > >
> > > > This is more a W/A than a fix since it is really weird to take
> > > > sta_lock inside iwl_tx_queue_unmap, but it can help until we revamp
> > > > the locking model in the transport layer.
> > > >
> > > > Signed-off-by: Emmanuel Grumbach <emmanuel.grumbach@intel.com>
> > > > Signed-off-by: Wey-Yi Guy <wey-yi.w.guy@intel.com>
> > > > ---
> > > > drivers/net/wireless/iwlwifi/iwl-trans-pcie.c | 3 +++
> > > > 1 files changed, 3 insertions(+), 0 deletions(-)
> > > >
> > > > diff --git a/drivers/net/wireless/iwlwifi/iwl-trans-pcie.c
> > > b/drivers/net/wireless/iwlwifi/iwl-trans-pcie.c
> > > > index 60067c7..f69aecb 100644
> > > > --- a/drivers/net/wireless/iwlwifi/iwl-trans-pcie.c
> > > > +++ b/drivers/net/wireless/iwlwifi/iwl-trans-pcie.c
> > > > @@ -406,6 +406,7 @@ static void iwl_tx_queue_unmap(struct iwl_trans
> > > *trans, int txq_id)
> > > > struct iwl_tx_queue *txq = &trans_pcie->txq[txq_id];
> > > > struct iwl_queue *q = &txq->q;
> > > > enum dma_data_direction dma_dir;
> > > > + unsigned long flags;
> > > >
> > > > if (!q->n_bd)
> > > > return;
> > > > @@ -418,12 +419,14 @@ static void iwl_tx_queue_unmap(struct iwl_trans
> > > *trans, int txq_id)
> > > > else
> > > > dma_dir = DMA_TO_DEVICE;
> > > >
> > > > + spin_lock_irqsave(&trans->shrd->sta_lock, flags);
> > > sta_lock does not look like right lock to use.
> > >
> > True. And I wrote in the commit message that this is a W/A until we
> revamp the locking model.
>
> Why not simply take hcmd_lock?
Hcmd_lock is related to the host command queue only, it won't work for Tx queues.
---------------------------------------------------------------------
Intel Israel (74) Limited
This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.
^ permalink raw reply
* Re: [PATCH 00/29] make the driver ready for multi-vif support
From: Luciano Coelho @ 2011-10-11 12:52 UTC (permalink / raw)
To: Eliad Peller; +Cc: linux-wireless
In-Reply-To: <1318234397-21081-1-git-send-email-eliad@wizery.com>
On Mon, 2011-10-10 at 10:12 +0200, Eliad Peller wrote:
> This patchset finishes adding multiple vif support to the
> wl12xx driver.
>
> The driver still allows only a single vif, as the current
> fw only supports a single vif.
>
> This patchset doesn't break compatibility with the current fw.
Applied, thanks! With the following small changes:
* used v2 of 08/29;
* used '(foo - bar) & 0xff' instead of '(foo - bar + 256) % 256'.
--
Cheers,
Luca.
^ permalink raw reply
* Re: [PATCH 17/34] iwlagn: fix a race in the unmapping of the TFDs
From: Stanislaw Gruszka @ 2011-10-11 12:58 UTC (permalink / raw)
To: Grumbach, Emmanuel
Cc: Guy, Wey-Yi W, linville@tuxdriver.com,
linux-wireless@vger.kernel.org
In-Reply-To: <4825B8A2C4E264489E57869F0DCFB2234457A944F2@hasmsx502.ger.corp.intel.com>
On Tue, Oct 11, 2011 at 02:38:52PM +0200, Grumbach, Emmanuel wrote:
> > Why not simply take hcmd_lock?
>
> Hcmd_lock is related to the host command queue only, it won't work for Tx queues.
So you only fix reces between
iwl_trans_pcie_tx()/iwl_tx_queue_reclaim() and iwl_tx_queue_unmap().
Races between
iwl_enqueue_hcmd()/iwl_cmd_queue_reclaim() and iwl_tx_queue_unmap()
are unresolved.
Stanislaw
^ permalink raw reply
* Re: [PATCH 0/8] wl12xx: forward ported Balbi's bus driver refactor
From: Luciano Coelho @ 2011-10-11 13:05 UTC (permalink / raw)
To: linux-wireless
In-Reply-To: <1317928259-26437-1-git-send-email-coelho@ti.com>
On Thu, 2011-10-06 at 22:10 +0300, Luciano Coelho wrote:
> Hi,
>
> Felipe sent this series of patches a long time ago with very good
> suggestion on how to avoid the duplicate code we have in the sdio and
> spi drivers.
>
> With this, we create a platform device that is handled by a platform
> driver. The bus-specific module, creates a platform device and the
> core module implements the driver that supports both platform devices
> (namely "wl12xx-sdio" and "wl12xx-spi").
>
> I didn't apply this earlier because I had concerns about the change in
> the platform data, but now I'm convinced it's not a real problem. It
> may affect compat-wireless, but it's easy to solve.
>
> I have now forward-ported the patches, fixed some bugs, removed some
> style changes and moved some other things around. For the changes I
> made, see the commit message of each patch. Some changes are not in
> the commit message, but are explained in the patch emails, after the
> Signed-off-by area.
>
> I'm keeping Felipe as the author, since most of the work is his. I
> have just tested and fixed it up.
>
> Cheers,
> Luca.
>
> Felipe Balbi (8):
> wl12xx: add an sdio glue struct to keep wl and device side-by-side
> wl12xx: add an spi glue struct to keep wl and device side-by-side
> wl12xx: add a platform device to the sdio module
> wl12xx: add a platform device to the spi module
> wl12xx: add platform driver to the core module
> wl12xx: move common init code from bus modules to main
> wl12xx: mark some symbols static
> wl12xx: drop unneeded plat_dev
Applied the whole series.
--
Cheers,
Luca.
^ permalink raw reply
* Re: [PATCH] wl12xx: move debugging definitions to a separate file
From: Luciano Coelho @ 2011-10-11 13:20 UTC (permalink / raw)
To: linux-wireless
In-Reply-To: <1318085047.9690.32.camel@cumari>
On Sat, 2011-10-08 at 17:44 +0300, Luciano Coelho wrote:
> On Fri, 2011-10-07 at 11:13 +0300, Luciano Coelho wrote:
> > Separate the debugging macros and other definitions to a new debug.h
> > file. This is be needed because the sdio and spi modules don't need
> > to depend on the wl12xx module anymore, but still need to include
> > wl12xx.h. Currently they do depend on it, because of the debugging
> > global that wl12xx exports. A future patch will remove this
> > dependency.
> >
> > Signed-off-by: Luciano Coelho <coelho@ti.com>
> > ---
>
> These defines should be in debug.h as well:
>
> #define DRIVER_NAME "wl1271"
> #define DRIVER_PREFIX DRIVER_NAME ": "
>
> I'll move them too and rename wl1271 to wl12xx when I apply.
Applied.
--
Cheers,
Luca.
^ 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