2013-09-03 14:34:20

by Eliad Peller

[permalink] [raw]
Subject: [PATCH 01/12] wlcore: ROC on AP channel before auth reply

From: Arik Nemtsov <[email protected]>

Start a ROC on the AP channel beforing sending the authentication reply
to a connecting STA. This ROC is held up to 1 second via a timer. If the
station is authorized and added by mac80211, the ROC is extended until
the station is fully authorized.
We make sure not to ROC twice when several stations are connecting in
parallel and to only release the ROC when both the pending-reply timer
and the STA-state callbacks do not require it.

Signed-off-by: Arik Nemtsov <[email protected]>
Signed-off-by: Eliad Peller <[email protected]>
---
drivers/net/wireless/ti/wlcore/main.c | 101 +++++++++++++++++++++++++-----
drivers/net/wireless/ti/wlcore/tx.c | 25 ++++++--
drivers/net/wireless/ti/wlcore/tx.h | 3 +
drivers/net/wireless/ti/wlcore/wlcore.h | 2 +
drivers/net/wireless/ti/wlcore/wlcore_i.h | 9 +++
5 files changed, 120 insertions(+), 20 deletions(-)

diff --git a/drivers/net/wireless/ti/wlcore/main.c b/drivers/net/wireless/ti/wlcore/main.c
index 38995f9..b64b465 100644
--- a/drivers/net/wireless/ti/wlcore/main.c
+++ b/drivers/net/wireless/ti/wlcore/main.c
@@ -2008,6 +2008,47 @@ out:
mutex_unlock(&wl->mutex);
}

+static void wlcore_pending_auth_complete_work(struct work_struct *work)
+{
+ struct delayed_work *dwork;
+ struct wl1271 *wl;
+ struct wl12xx_vif *wlvif;
+ unsigned long time_spare;
+ int ret;
+
+ dwork = container_of(work, struct delayed_work, work);
+ wlvif = container_of(dwork, struct wl12xx_vif,
+ pending_auth_complete_work);
+ wl = wlvif->wl;
+
+ mutex_lock(&wl->mutex);
+
+ if (unlikely(wl->state != WLCORE_STATE_ON))
+ goto out;
+
+ /*
+ * Make sure a second really passed since the last auth reply. Maybe
+ * a second auth reply arrived while we were stuck on the mutex.
+ * Check for a little less than the timeout to protect from scheduler
+ * irregularities.
+ */
+ time_spare = jiffies +
+ msecs_to_jiffies(WLCORE_PEND_AUTH_ROC_TIMEOUT - 50);
+ if (!time_after(time_spare, wlvif->pending_auth_reply_time))
+ goto out;
+
+ ret = wl1271_ps_elp_wakeup(wl);
+ if (ret < 0)
+ goto out;
+
+ /* cancel the ROC if active */
+ wlcore_update_inconn_sta(wl, wlvif, NULL, false);
+
+ wl1271_ps_elp_sleep(wl);
+out:
+ mutex_unlock(&wl->mutex);
+}
+
static int wl12xx_allocate_rate_policy(struct wl1271 *wl, u8 *idx)
{
u8 policy = find_first_zero_bit(wl->rate_policies_map,
@@ -2159,6 +2200,8 @@ static int wl12xx_init_vif_data(struct wl1271 *wl, struct ieee80211_vif *vif)
wlcore_channel_switch_work);
INIT_DELAYED_WORK(&wlvif->connection_loss_work,
wlcore_connection_loss_work);
+ INIT_DELAYED_WORK(&wlvif->pending_auth_complete_work,
+ wlcore_pending_auth_complete_work);
INIT_LIST_HEAD(&wlvif->list);

setup_timer(&wlvif->rx_streaming_timer, wl1271_rx_streaming_timer,
@@ -2590,6 +2633,7 @@ unlock:
cancel_work_sync(&wlvif->rx_streaming_disable_work);
cancel_delayed_work_sync(&wlvif->connection_loss_work);
cancel_delayed_work_sync(&wlvif->channel_switch_work);
+ cancel_delayed_work_sync(&wlvif->pending_auth_complete_work);

mutex_lock(&wl->mutex);
}
@@ -3969,6 +4013,13 @@ static void wl1271_bss_info_changed_ap(struct wl1271 *wl,
}
} else {
if (test_bit(WLVIF_FLAG_AP_STARTED, &wlvif->flags)) {
+ /*
+ * AP might be in ROC in case we have just
+ * sent auth reply. handle it.
+ */
+ if (test_bit(wlvif->role_id, wl->roc_map))
+ wl12xx_croc(wl, wlvif->role_id);
+
ret = wl12xx_cmd_role_stop_ap(wl, wlvif);
if (ret < 0)
goto out;
@@ -4656,29 +4707,49 @@ static void wlcore_roc_if_possible(struct wl1271 *wl,
wl12xx_roc(wl, wlvif, wlvif->role_id, wlvif->band, wlvif->channel);
}

-static void wlcore_update_inconn_sta(struct wl1271 *wl,
- struct wl12xx_vif *wlvif,
- struct wl1271_station *wl_sta,
- bool in_connection)
+/*
+ * when wl_sta is NULL, we treat this call as if coming from a
+ * pending auth reply.
+ * wl->mutex must be taken and the FW must be awake when the call
+ * takes place.
+ */
+void wlcore_update_inconn_sta(struct wl1271 *wl, struct wl12xx_vif *wlvif,
+ struct wl1271_station *wl_sta, bool in_conn)
{
- if (in_connection) {
- if (WARN_ON(wl_sta->in_connection))
+ if (in_conn) {
+ if (WARN_ON(wl_sta && wl_sta->in_connection))
return;
- wl_sta->in_connection = true;
- if (!wlvif->inconn_count++)
+
+ if (!wlvif->ap_pending_auth_reply &&
+ !wlvif->inconn_count)
wlcore_roc_if_possible(wl, wlvif);
+
+ if (wl_sta) {
+ wl_sta->in_connection = true;
+ wlvif->inconn_count++;
+ } else {
+ wlvif->ap_pending_auth_reply = true;
+ }
} else {
- if (!wl_sta->in_connection)
+ if (wl_sta && !wl_sta->in_connection)
+ return;
+
+ if (WARN_ON(!wl_sta && !wlvif->ap_pending_auth_reply))
return;

- wl_sta->in_connection = false;
- wlvif->inconn_count--;
- if (WARN_ON(wlvif->inconn_count < 0))
+ if (WARN_ON(wl_sta && !wlvif->inconn_count))
return;

- if (!wlvif->inconn_count)
- if (test_bit(wlvif->role_id, wl->roc_map))
- wl12xx_croc(wl, wlvif->role_id);
+ if (wl_sta) {
+ wl_sta->in_connection = false;
+ wlvif->inconn_count--;
+ } else {
+ wlvif->ap_pending_auth_reply = false;
+ }
+
+ if (!wlvif->inconn_count && !wlvif->ap_pending_auth_reply &&
+ test_bit(wlvif->role_id, wl->roc_map))
+ wl12xx_croc(wl, wlvif->role_id);
}
}

diff --git a/drivers/net/wireless/ti/wlcore/tx.c b/drivers/net/wireless/ti/wlcore/tx.c
index 7e93fe6..03249da 100644
--- a/drivers/net/wireless/ti/wlcore/tx.c
+++ b/drivers/net/wireless/ti/wlcore/tx.c
@@ -86,19 +86,34 @@ void wl1271_free_tx_id(struct wl1271 *wl, int id)
EXPORT_SYMBOL(wl1271_free_tx_id);

static void wl1271_tx_ap_update_inconnection_sta(struct wl1271 *wl,
+ struct wl12xx_vif *wlvif,
struct sk_buff *skb)
{
struct ieee80211_hdr *hdr;

+ hdr = (struct ieee80211_hdr *)(skb->data +
+ sizeof(struct wl1271_tx_hw_descr));
+ if (!ieee80211_is_auth(hdr->frame_control))
+ return;
+
/*
* add the station to the known list before transmitting the
* authentication response. this way it won't get de-authed by FW
* when transmitting too soon.
*/
- hdr = (struct ieee80211_hdr *)(skb->data +
- sizeof(struct wl1271_tx_hw_descr));
- if (ieee80211_is_auth(hdr->frame_control))
- wl1271_acx_set_inconnection_sta(wl, hdr->addr1);
+ wl1271_acx_set_inconnection_sta(wl, hdr->addr1);
+
+ /*
+ * ROC for 1 second on the AP channel for completing the connection.
+ * Note the ROC will be continued by the update_sta_state callbacks
+ * once the station reaches the associated state.
+ */
+ wlcore_update_inconn_sta(wl, wlvif, NULL, true);
+ wlvif->pending_auth_reply_time = jiffies;
+ cancel_delayed_work(&wlvif->pending_auth_complete_work);
+ ieee80211_queue_delayed_work(wl->hw,
+ &wlvif->pending_auth_complete_work,
+ msecs_to_jiffies(WLCORE_PEND_AUTH_ROC_TIMEOUT));
}

static void wl1271_tx_regulate_link(struct wl1271 *wl,
@@ -404,7 +419,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 && wlvif->bss_type == BSS_TYPE_AP_BSS) {
- wl1271_tx_ap_update_inconnection_sta(wl, skb);
+ wl1271_tx_ap_update_inconnection_sta(wl, wlvif, skb);
wl1271_tx_regulate_link(wl, wlvif, hlid);
}

diff --git a/drivers/net/wireless/ti/wlcore/tx.h b/drivers/net/wireless/ti/wlcore/tx.h
index 55aa4ac..35489c3 100644
--- a/drivers/net/wireless/ti/wlcore/tx.h
+++ b/drivers/net/wireless/ti/wlcore/tx.h
@@ -56,6 +56,9 @@
/* Used for management frames and dummy packets */
#define WL1271_TID_MGMT 7

+/* stop a ROC for pending authentication reply after this time (ms) */
+#define WLCORE_PEND_AUTH_ROC_TIMEOUT 1000
+
struct wl127x_tx_mem {
/*
* Number of extra memory blocks to allocate for this packet
diff --git a/drivers/net/wireless/ti/wlcore/wlcore.h b/drivers/net/wireless/ti/wlcore/wlcore.h
index 0034979..54ce5d5 100644
--- a/drivers/net/wireless/ti/wlcore/wlcore.h
+++ b/drivers/net/wireless/ti/wlcore/wlcore.h
@@ -481,6 +481,8 @@ int wlcore_set_key(struct wl1271 *wl, enum set_key_cmd cmd,
struct ieee80211_sta *sta,
struct ieee80211_key_conf *key_conf);
void wlcore_regdomain_config(struct wl1271 *wl);
+void wlcore_update_inconn_sta(struct wl1271 *wl, struct wl12xx_vif *wlvif,
+ struct wl1271_station *wl_sta, bool in_conn);

static inline void
wlcore_set_ht_cap(struct wl1271 *wl, enum ieee80211_band band,
diff --git a/drivers/net/wireless/ti/wlcore/wlcore_i.h b/drivers/net/wireless/ti/wlcore/wlcore_i.h
index e5e1464..14fd111 100644
--- a/drivers/net/wireless/ti/wlcore/wlcore_i.h
+++ b/drivers/net/wireless/ti/wlcore/wlcore_i.h
@@ -456,6 +456,15 @@ struct wl12xx_vif {
*/
int hw_queue_base;

+ /* do we have a pending auth reply? (and ROC) */
+ bool ap_pending_auth_reply;
+
+ /* time when we sent the pending auth reply */
+ unsigned long pending_auth_reply_time;
+
+ /* work for canceling ROC after pending auth reply */
+ struct delayed_work pending_auth_complete_work;
+
/*
* This struct must be last!
* data that has to be saved acrossed reconfigs (e.g. recovery)
--
1.8.3.rc1.35.g9b79519



2013-09-10 14:51:16

by Eliad Peller

[permalink] [raw]
Subject: Re: [PATCH 09/12] wlcore: fix regulatory domain bit translation

On Tue, Sep 10, 2013 at 10:39 AM, Luca Coelho <[email protected]> wrote:
> On Tue, 2013-09-03 at 17:34 +0300, Eliad Peller wrote:
>> From: Ido Reis <[email protected]>
>>
>> This is a fix for channels 52,56,60,64 bit translation.
>>
>> Reported-by: Yaniv Machani <[email protected]>
>> Signed-off-by: Ido Reis <[email protected]>
>> Signed-off-by: Victor Goldenshtein <[email protected]>
>> Signed-off-by: Eliad Peller <[email protected]>
>> ---
>> drivers/net/wireless/ti/wlcore/cmd.c | 4 +++-
>> 1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/net/wireless/ti/wlcore/cmd.c b/drivers/net/wireless/ti/wlcore/cmd.c
>> index e3ae425..1cb3296 100644
>> --- a/drivers/net/wireless/ti/wlcore/cmd.c
>> +++ b/drivers/net/wireless/ti/wlcore/cmd.c
>> @@ -1613,8 +1613,10 @@ static int wlcore_get_reg_conf_ch_idx(enum ieee80211_band band, u16 ch)
>> case IEEE80211_BAND_5GHZ:
>> if (ch >= 8 && ch <= 16)
>> idx = ((ch-8)/4 + 18);
>> - else if (ch >= 34 && ch <= 64)
>> + else if (ch >= 34 && ch <= 48)
>> idx = ((ch-34)/2 + 3 + 18);
>> + else if (ch >= 52 && ch <= 64)
>> + idx = ((ch-52)/4 + 11 + 18);
>> else if (ch >= 100 && ch <= 140)
>> idx = ((ch-100)/4 + 15 + 18);
>> else if (ch >= 149 && ch <= 165)
>
> Hmmm... I don't have a clue what is going on here. I don't know how I
> let this function pass as is originally, shame on me. :)
>
:)

> The change probably makes things work better, since someone apparently
> saw a bug in real life and reported it, but can anyone explain what is
> going on during this translation? Aren't we losing data here? Eg.
> channels 8, 9, 10 and 11 all use the same bit in the firmware command
> bitmask?
>
the 8-16 indeed looks weird, as the driver indeed declares supports
for channels 7,8,9,11...
the other conditions are a simple attempt to reduce the gap between
the channels (e.g. no channels between 64 and 100) in order to get a
minimized bitmap.
since the gap between the defined channels in the 5ghz band is usually
2/4 (36->38 vs. 60->64) the other calculations seems fine (i didn't
dig into them as well, though).

i'll try getting some more information from the fw guys regarding the
8-16 range.

Eliad.

2013-09-10 19:16:30

by Luca Coelho

[permalink] [raw]
Subject: Re: [PATCH 02/12] wlcore: add new plt power-mode: CHIP_AWAKE

On Tue, 2013-09-10 at 15:46 +0200, Eliad Peller wrote:
> On Tue, Sep 10, 2013 at 9:56 AM, Luca Coelho <[email protected]> wrote:
> > On Tue, 2013-09-10 at 08:47 +0200, Arik Nemtsov wrote:
> >> On Tue, Sep 10, 2013 at 9:33 AM, Luca Coelho <[email protected]> wrote:
> >> > On Tue, 2013-09-03 at 17:33 +0300, Eliad Peller wrote:
> >> >> From: Yair Shapira <[email protected]>
> >> >>
> >> >> Under this mode the chip is powered on including sdio
> >> >> but no FW is downloaded and run, interrupts are not enabled, etc...
> >> >>
> >> >> This mode is intended to allow RTTT to bridge sdio as a transport
> >> >> to the chip.
> >> >>
> >> >> Driver only provides sdio access using the dev_mem debugfs file.
> >> >>
> >> >> Some fixes done to the code that ensures that PLT mode and normal
> >> >> driver power mode (ifconfig/add_interface) are mutually excluded.
> >> >>
> >> >> Signed-off-by: Yair Shapira <[email protected]>
> >> >> Signed-off-by: Eliad Peller <[email protected]>
> >> >> ---
> >> >
> >> > I had some comments to this patch internally while I was still at TI.
> >> > Namely, I asked why do we need a new way of doing this if this is
> >> > already possible via debugsfs (using the gpio_power file)?
> >>
> >> Are you commenting on the correct patch? Seems this is just a patch to
> >> prevent "ifconfig up" during PLT mode..
> >
> > Yes, I'm commenting on the right patch. It allows the chip power
> > (namely the WLAN_EN GPIO pin) to be set directly, without loading the
> > firmware and doing other initialization stuff. This can already be
> > controlled via the gpio_power debugfs file. I've used that a bunch of
> > times, including with the RTTT tool.
> >
> > Okay, this patch has a few more protections (eg. not allowing an
> > interface to be added while the chip is powered in this way), but this
> > could also be added on top of the existing implementation.
> >
> i guess this just a bit "cleaner" (similar to the way PLT is used).
> the gpio_power debugfs only toggles the gpio power, without changing
> the driver mode, so adding similar protections will be more
> complicated.

Maybe, but the truth is that you don't need to add these protections,
unless you really want to shoot your own foot and start mangling with
the interfaces. These tools are usually used in very controlled
environments (like RF R&D and production testing).

Still, this is okay, but it's just bad to see things being done in two
different ways. I guess whoever requested this "new feature" didn't
know about the gpio toggling debugfs file.

--
Luca.


2013-09-10 07:47:07

by Luca Coelho

[permalink] [raw]
Subject: Re: [PATCH 11/12] wl18xx: print new RDL versions during boot

On Tue, 2013-09-03 at 17:34 +0300, Eliad Peller wrote:
> From: Victor Goldenshtein <[email protected]>
>
> Extract and print info for the new RDL 5, 6, 7 and 8.
> Replace const struct with function which translates
> the RDL number to string.
>
> Signed-off-by: Victor Goldenshtein <[email protected]>
> Signed-off-by: Barak Bercovitz <[email protected]>
> Signed-off-by: Eliad Peller <[email protected]>
> ---

Why convert the array with a function? The array looks much cleaner to
me.

[...[
> diff --git a/drivers/net/wireless/ti/wl18xx/main.c b/drivers/net/wireless/ti/wl18xx/main.c
> index b47eb62..aef0c91 100644
> --- a/drivers/net/wireless/ti/wl18xx/main.c
> +++ b/drivers/net/wireless/ti/wl18xx/main.c
> @@ -1228,16 +1228,46 @@ static u32 wl18xx_ap_get_mimo_wide_rate_mask(struct wl1271 *wl,
> }
> }
>
> +static const char *wl18xx_rdl_name(enum wl18xx_rdl_num rdl_num)
> +{
> + switch (rdl_num) {
> + case RDL_1_HP:
> + return "183xH";
> + case RDL_2_SP:
> + return "183x or 180x";
> + case RDL_3_HP:
> + return "187xH";
> + case RDL_4_SP:
> + return "187x";
> + case RDL_5_SP:
> + return "RDL11 - Not Supported";
> + case RDL_6_SP:
> + return "180xD";
> + case RDL_7_SP:
> + return "RDL13 - Not Supported (1893Q)";
> + case RDL_8_SP:
> + return "18xxQ";
> + default:
> + return "UNTRIMMED";

This may become misleading if we get more RDLs versions in the future.
And the untrimmed case is probably reporting 0? Or something predefined,
hopefully, otherwise how can we know that we wouldn't randomly get a
valid value?

Also, in the unsupported cases, it would probably be better to bail out?

--
Luca.


2013-09-10 19:26:27

by Luca Coelho

[permalink] [raw]
Subject: Re: [PATCH 06/12] wlcore: send EAPOL frames with voice priority

On Tue, 2013-09-10 at 16:15 +0200, Eliad Peller wrote:
> On Tue, Sep 10, 2013 at 12:15 PM, Luca Coelho <[email protected]> wrote:
> > On Tue, 2013-09-10 at 11:11 +0200, Arik Nemtsov wrote:
> >> On Tue, Sep 10, 2013 at 9:47 AM, Luca Coelho <[email protected]> wrote:
> >> > On Tue, 2013-09-03 at 17:34 +0300, Eliad Peller wrote:
> >> >> From: Igal Chernobelsky <[email protected]>
> >> >>
> >> >> Send EAPOL frames with voice priority by setting TX_HW_ATTR_EAPOL_FRAME
> >> >> new bit in tx attribute. Sending EAPOL with voice priority fixes
> >> >> re-key timeout during heavy traffic issue.
> >> >>
> >> >> Signed-off-by: Igal Chernobelsky <[email protected]>
> >> >> Signed-off-by: Eliad Peller <[email protected]>
> >> >> ---
> >> >
> >> > This seems to be the same problem that Ben had and debugged by himself
> >> > [1]. Fixing this in hostapd/wpa_supplicant seems more appropriate?
> >> >
> >> > This patch seems to take an advantage of some sort of hack in the
> >> > firmware that will change the priority by itself when the
> >> > TX_HW_ATTR_EAPOL_GRAME bit is set. If we have a good reason to use this
> >> > patch, we need to take care of the firmware version as well. This
> >> > probably doesn't work with the latest published firmware.
> >> >
> >> > [1] http://mid.gmane.org/[email protected]
> >>
> >> I believe he only fixed one mode (AP - hostapd) but not the other (GO
> >> - wpa_supplicant). Anyway that's what we tested at the time.
> >
> > Okay, I didn't dig much into it.
> >
> >
> >> Anyway marking it as this level doesn't do any harm. You're right
> >> about the FW version though - it should be upstreamed first.
> >
> > Fair enough. If the firmware implements this, no reason why not use it.
> > But I'll wait for the new firmware before applying this.
> >
> this change is backward compatible, so there's no issue applying it as is.
> however, if you prefer waiting for a newer fw that's fine too.
> i'll add it to the waiting list :)

Okay, indeed it's probably backwards compatible because that bit was not
used before. But I still prefer if you put this in the waiting list,
because otherwise it seems the issue is solved (if you look at the git
log) when it is not really fixed until a new firmware version is
available.

--
Luca.


2013-09-10 07:38:54

by Luca Coelho

[permalink] [raw]
Subject: Re: [PATCH 09/12] wlcore: fix regulatory domain bit translation

On Tue, 2013-09-03 at 17:34 +0300, Eliad Peller wrote:
> From: Ido Reis <[email protected]>
>
> This is a fix for channels 52,56,60,64 bit translation.
>
> Reported-by: Yaniv Machani <[email protected]>
> Signed-off-by: Ido Reis <[email protected]>
> Signed-off-by: Victor Goldenshtein <[email protected]>
> Signed-off-by: Eliad Peller <[email protected]>
> ---
> drivers/net/wireless/ti/wlcore/cmd.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/wireless/ti/wlcore/cmd.c b/drivers/net/wireless/ti/wlcore/cmd.c
> index e3ae425..1cb3296 100644
> --- a/drivers/net/wireless/ti/wlcore/cmd.c
> +++ b/drivers/net/wireless/ti/wlcore/cmd.c
> @@ -1613,8 +1613,10 @@ static int wlcore_get_reg_conf_ch_idx(enum ieee80211_band band, u16 ch)
> case IEEE80211_BAND_5GHZ:
> if (ch >= 8 && ch <= 16)
> idx = ((ch-8)/4 + 18);
> - else if (ch >= 34 && ch <= 64)
> + else if (ch >= 34 && ch <= 48)
> idx = ((ch-34)/2 + 3 + 18);
> + else if (ch >= 52 && ch <= 64)
> + idx = ((ch-52)/4 + 11 + 18);
> else if (ch >= 100 && ch <= 140)
> idx = ((ch-100)/4 + 15 + 18);
> else if (ch >= 149 && ch <= 165)

Hmmm... I don't have a clue what is going on here. I don't know how I
let this function pass as is originally, shame on me. :)

The change probably makes things work better, since someone apparently
saw a bug in real life and reported it, but can anyone explain what is
going on during this translation? Aren't we losing data here? Eg.
channels 8, 9, 10 and 11 all use the same bit in the firmware command
bitmask?

--
Luca.


2013-09-10 09:11:41

by Arik Nemtsov

[permalink] [raw]
Subject: Re: [PATCH 06/12] wlcore: send EAPOL frames with voice priority

On Tue, Sep 10, 2013 at 9:47 AM, Luca Coelho <[email protected]> wrote:
> On Tue, 2013-09-03 at 17:34 +0300, Eliad Peller wrote:
>> From: Igal Chernobelsky <[email protected]>
>>
>> Send EAPOL frames with voice priority by setting TX_HW_ATTR_EAPOL_FRAME
>> new bit in tx attribute. Sending EAPOL with voice priority fixes
>> re-key timeout during heavy traffic issue.
>>
>> Signed-off-by: Igal Chernobelsky <[email protected]>
>> Signed-off-by: Eliad Peller <[email protected]>
>> ---
>
> This seems to be the same problem that Ben had and debugged by himself
> [1]. Fixing this in hostapd/wpa_supplicant seems more appropriate?
>
> This patch seems to take an advantage of some sort of hack in the
> firmware that will change the priority by itself when the
> TX_HW_ATTR_EAPOL_GRAME bit is set. If we have a good reason to use this
> patch, we need to take care of the firmware version as well. This
> probably doesn't work with the latest published firmware.
>
> [1] http://mid.gmane.org/[email protected]

I believe he only fixed one mode (AP - hostapd) but not the other (GO
- wpa_supplicant). Anyway that's what we tested at the time.

Anyway marking it as this level doesn't do any harm. You're right
about the FW version though - it should be upstreamed first.

Arik

2013-09-11 08:03:57

by Eliad Peller

[permalink] [raw]
Subject: Re: [PATCH 03/12] wlcore: disable elp sleep while in plt mode

On Tue, Sep 10, 2013 at 10:24 PM, Luca Coelho <[email protected]> wrote:
> On Tue, 2013-09-10 at 16:04 +0200, Eliad Peller wrote:
>> On Tue, Sep 10, 2013 at 9:34 AM, Luca Coelho <[email protected]> wrote:
>> > On Tue, 2013-09-03 at 17:33 +0300, Eliad Peller wrote:
>> >> From: Yair Shapira <[email protected]>
>> >>
>> >> We now disable elp sleep during plt mode to allow normal operation of
>> >> plt tools such as calibrator.
>> >>
>> >> Having elp_sleep enabled during plt mode is actually not required and
>> >> in fact it disrupt plt operations such as rx statistics etc.
>> >>
>> >> Signed-off-by: Yair Shapira <[email protected]>
>> >> Signed-off-by: Eliad Peller <[email protected]>
>> >> ---
>> >
>> > I also had a comment internally about this one. Why do we need this?
>> > AFAICT this would never happen in real life, since the firmware is not
>> > even loaded at this point. Is there any real life situation where we
>> > try to go into ELP while in PLT mode?
>> >
>> i'm not familiar with the whole plt process, but i guess some flows
>> might indeed end up in elp (e.g. via wl1271_cmd_interrogate(), in
>> order to read statistics)
>
> Well, I don't think this would ever happen. The PLT stuff is a
> completely different firmware and, really, if it tries to go into ELP
> mode it is a bug.
>
i can see the calibrator calls various WL1271_TM_CMD_TEST commands, so
i guess they are likely to end up with ps_elp_sleep() calls (in
wl1271_tm_cmd_test()).

Eliad.

2013-09-10 06:46:51

by Luca Coelho

[permalink] [raw]
Subject: Re: [PATCH 06/12] wlcore: send EAPOL frames with voice priority

On Tue, 2013-09-03 at 17:34 +0300, Eliad Peller wrote:
> From: Igal Chernobelsky <[email protected]>
>
> Send EAPOL frames with voice priority by setting TX_HW_ATTR_EAPOL_FRAME
> new bit in tx attribute. Sending EAPOL with voice priority fixes
> re-key timeout during heavy traffic issue.
>
> Signed-off-by: Igal Chernobelsky <[email protected]>
> Signed-off-by: Eliad Peller <[email protected]>
> ---

This seems to be the same problem that Ben had and debugged by himself
[1]. Fixing this in hostapd/wpa_supplicant seems more appropriate?

This patch seems to take an advantage of some sort of hack in the
firmware that will change the priority by itself when the
TX_HW_ATTR_EAPOL_GRAME bit is set. If we have a good reason to use this
patch, we need to take care of the firmware version as well. This
probably doesn't work with the latest published firmware.

[1] http://mid.gmane.org/[email protected]

--
Cheers,
Luca.


2013-09-10 06:34:34

by Luca Coelho

[permalink] [raw]
Subject: Re: [PATCH 03/12] wlcore: disable elp sleep while in plt mode

On Tue, 2013-09-03 at 17:33 +0300, Eliad Peller wrote:
> From: Yair Shapira <[email protected]>
>
> We now disable elp sleep during plt mode to allow normal operation of
> plt tools such as calibrator.
>
> Having elp_sleep enabled during plt mode is actually not required and
> in fact it disrupt plt operations such as rx statistics etc.
>
> Signed-off-by: Yair Shapira <[email protected]>
> Signed-off-by: Eliad Peller <[email protected]>
> ---

I also had a comment internally about this one. Why do we need this?
AFAICT this would never happen in real life, since the firmware is not
even loaded at this point. Is there any real life situation where we
try to go into ELP while in PLT mode?

--
Luca.


2013-09-10 07:50:06

by Luca Coelho

[permalink] [raw]
Subject: Re: [PATCH 12/12] wlcore: always register dummy hardirq

On Tue, 2013-09-03 at 17:34 +0300, Eliad Peller wrote:
> From: Arik Nemtsov <[email protected]>
>
> This keeps the kernel happy when using edge-irqs and requesting a
> threaded irq.
>
> Signed-off-by: Arik Nemtsov <[email protected]>
> Signed-off-by: Eliad Peller <[email protected]>
> ---

Ouch, I think this is not going to fit at all with the DT patches I
sent.

BTW, someone should probably take over those patches, rework according
to the reviews and submit them again, as I don't think I'll have the
time to rework them anytime soon... :(

--
Luca.


2013-09-11 07:19:42

by Eliad Peller

[permalink] [raw]
Subject: Re: [PATCH 07/12] wlcore: cleanup scan debug prints

On Tue, Sep 10, 2013 at 10:19 AM, Luca Coelho <[email protected]> wrote:
> On Tue, 2013-09-03 at 17:34 +0300, Eliad Peller wrote:
>> From: Victor Goldenshtein <[email protected]>
>>
>> Remove scan debug dumps which are rarely used.
>> Make scan debug prints more clear and short.
>>
>> Signed-off-by: Victor Goldenshtein <[email protected]>
>> Signed-off-by: Eliad Peller <[email protected]>
>> ---
> [...]
>> @@ -222,6 +212,18 @@ wlcore_scan_get_channels(struct wl1271 *wl,
>> *n_pactive_ch);
>> }
>>
>> + wl1271_debug(DEBUG_SCAN, "freq %04d, ch. %03d, flags 0x%02X, power %02d, min/max_dwell %02d/%02d%s%s",
>
> Left-padding with zeros is quite ugly with the decimals here.
>
i agree :)

i'll take care of your comments and resubmit.

thanks,
Eliad.

2013-09-11 07:48:48

by Eliad Peller

[permalink] [raw]
Subject: Re: [PATCH 12/12] wlcore: always register dummy hardirq

On Tue, Sep 10, 2013 at 10:50 AM, Luca Coelho <[email protected]> wrote:
> On Tue, 2013-09-03 at 17:34 +0300, Eliad Peller wrote:
>> From: Arik Nemtsov <[email protected]>
>>
>> This keeps the kernel happy when using edge-irqs and requesting a
>> threaded irq.
>>
>> Signed-off-by: Arik Nemtsov <[email protected]>
>> Signed-off-by: Eliad Peller <[email protected]>
>> ---
>
> Ouch, I think this is not going to fit at all with the DT patches I
> sent.
>
this is quite minor patch (only making sure we always define hardirq
when threaded_irq is used. i don't think it should conflict (at least
logically) with DT patches...

Eliad.

2013-09-10 14:11:45

by Eliad Peller

[permalink] [raw]
Subject: Re: [PATCH 05/12] wl18xx: increase the number of allowed BA sessions

On Tue, Sep 10, 2013 at 9:39 AM, Luca Coelho <[email protected]> wrote:
> On Tue, 2013-09-03 at 17:34 +0300, Eliad Peller wrote:
>> From: Victor Goldenshtein <[email protected]>
>>
>> The current fw (actually starting from fw 8.5.0.0.58)
>> supports 8 rx BA sessions.
>>
>> Signed-off-by: Victor Goldenshtein <[email protected]>
>> Signed-off-by: Eliad Peller <[email protected]>
>> ---
>
> The latest firmware version published at linux-firmware.git is
> 8.5.0.0.55 (wl18xx-fw-2.bin), so this patch cannot be taken into the
> official tree yet.
>
> Please publish a newer version of the firmware (with appropriate
> increase in the filename version), since I cannot do that myself
> anymore.
>
you're right.
i'll check with fw guys what fw should be upstreamed.
please drop this patch for now, and let's continue only with the other
patches (that don't require a new fw, afaict).

thanks,
Eliad.

2013-09-03 14:34:35

by Eliad Peller

[permalink] [raw]
Subject: [PATCH 11/12] wl18xx: print new RDL versions during boot

From: Victor Goldenshtein <[email protected]>

Extract and print info for the new RDL 5, 6, 7 and 8.
Replace const struct with function which translates
the RDL number to string.

Signed-off-by: Victor Goldenshtein <[email protected]>
Signed-off-by: Barak Bercovitz <[email protected]>
Signed-off-by: Eliad Peller <[email protected]>
---
drivers/net/wireless/ti/wl18xx/main.c | 40 +++++++++++++++++++++++++++++------
drivers/net/wireless/ti/wl18xx/reg.h | 20 +++++++++---------
2 files changed, 44 insertions(+), 16 deletions(-)

diff --git a/drivers/net/wireless/ti/wl18xx/main.c b/drivers/net/wireless/ti/wl18xx/main.c
index b47eb62..aef0c91 100644
--- a/drivers/net/wireless/ti/wl18xx/main.c
+++ b/drivers/net/wireless/ti/wl18xx/main.c
@@ -1228,16 +1228,46 @@ static u32 wl18xx_ap_get_mimo_wide_rate_mask(struct wl1271 *wl,
}
}

+static const char *wl18xx_rdl_name(enum wl18xx_rdl_num rdl_num)
+{
+ switch (rdl_num) {
+ case RDL_1_HP:
+ return "183xH";
+ case RDL_2_SP:
+ return "183x or 180x";
+ case RDL_3_HP:
+ return "187xH";
+ case RDL_4_SP:
+ return "187x";
+ case RDL_5_SP:
+ return "RDL11 - Not Supported";
+ case RDL_6_SP:
+ return "180xD";
+ case RDL_7_SP:
+ return "RDL13 - Not Supported (1893Q)";
+ case RDL_8_SP:
+ return "18xxQ";
+ default:
+ return "UNTRIMMED";
+ }
+}
+
static int wl18xx_get_pg_ver(struct wl1271 *wl, s8 *ver)
{
u32 fuse;
- s8 rom = 0, metal = 0, pg_ver = 0, rdl_ver = 0;
+ s8 rom = 0, metal = 0, pg_ver = 0, rdl_ver = 0, package_type = 0;
int ret;

ret = wlcore_set_partition(wl, &wl->ptable[PART_TOP_PRCM_ELP_SOC]);
if (ret < 0)
goto out;

+ ret = wlcore_read32(wl, WL18XX_REG_FUSE_DATA_2_3, &fuse);
+ if (ret < 0)
+ goto out;
+
+ package_type = (fuse >> WL18XX_PACKAGE_TYPE_OFFSET) & 1;
+
ret = wlcore_read32(wl, WL18XX_REG_FUSE_DATA_1_3, &fuse);
if (ret < 0)
goto out;
@@ -1245,7 +1275,7 @@ static int wl18xx_get_pg_ver(struct wl1271 *wl, s8 *ver)
pg_ver = (fuse & WL18XX_PG_VER_MASK) >> WL18XX_PG_VER_OFFSET;
rom = (fuse & WL18XX_ROM_VER_MASK) >> WL18XX_ROM_VER_OFFSET;

- if (rom <= 0xE)
+ if ((rom <= 0xE) && (package_type == WL18XX_PACKAGE_TYPE_WSP))
metal = (fuse & WL18XX_METAL_VER_MASK) >>
WL18XX_METAL_VER_OFFSET;
else
@@ -1257,11 +1287,9 @@ static int wl18xx_get_pg_ver(struct wl1271 *wl, s8 *ver)
goto out;

rdl_ver = (fuse & WL18XX_RDL_VER_MASK) >> WL18XX_RDL_VER_OFFSET;
- if (rdl_ver > RDL_MAX)
- rdl_ver = RDL_NONE;

- wl1271_info("wl18xx HW: RDL %d, %s, PG %x.%x (ROM %x)",
- rdl_ver, rdl_names[rdl_ver], pg_ver, metal, rom);
+ wl1271_info("wl18xx HW: %s, PG %d.%d (ROM 0x%x)",
+ wl18xx_rdl_name(rdl_ver), pg_ver, metal, rom);

if (ver)
*ver = pg_ver;
diff --git a/drivers/net/wireless/ti/wl18xx/reg.h b/drivers/net/wireless/ti/wl18xx/reg.h
index 88de3f2..a433a75 100644
--- a/drivers/net/wireless/ti/wl18xx/reg.h
+++ b/drivers/net/wireless/ti/wl18xx/reg.h
@@ -147,13 +147,16 @@
#define WL18XX_REG_FUSE_DATA_1_3 0xA0260C
#define WL18XX_PG_VER_MASK 0x70
#define WL18XX_PG_VER_OFFSET 4
-#define WL18XX_ROM_VER_MASK 0x3
-#define WL18XX_ROM_VER_OFFSET 0
+#define WL18XX_ROM_VER_MASK 0x3e00
+#define WL18XX_ROM_VER_OFFSET 9
#define WL18XX_METAL_VER_MASK 0xC
#define WL18XX_METAL_VER_OFFSET 2
#define WL18XX_NEW_METAL_VER_MASK 0x180
#define WL18XX_NEW_METAL_VER_OFFSET 7

+#define WL18XX_PACKAGE_TYPE_OFFSET 13
+#define WL18XX_PACKAGE_TYPE_WSP 0
+
#define WL18XX_REG_FUSE_DATA_2_3 0xA02614
#define WL18XX_RDL_VER_MASK 0x1f00
#define WL18XX_RDL_VER_OFFSET 8
@@ -214,24 +217,21 @@ enum {
NUM_BOARD_TYPES,
};

-enum {
+enum wl18xx_rdl_num {
RDL_NONE = 0,
RDL_1_HP = 1,
RDL_2_SP = 2,
RDL_3_HP = 3,
RDL_4_SP = 4,
+ RDL_5_SP = 0x11,
+ RDL_6_SP = 0x12,
+ RDL_7_SP = 0x13,
+ RDL_8_SP = 0x14,

_RDL_LAST,
RDL_MAX = _RDL_LAST - 1,
};

-static const char * const rdl_names[] = {
- [RDL_NONE] = "",
- [RDL_1_HP] = "1853 SISO",
- [RDL_2_SP] = "1857 MIMO",
- [RDL_3_HP] = "1893 SISO",
- [RDL_4_SP] = "1897 MIMO",
-};

/* FPGA_SPARE_1 register - used to change the PHY ATPG clock at boot time */
#define WL18XX_PHY_FPGA_SPARE_1 0x8093CA40
--
1.8.3.rc1.35.g9b79519


2013-09-03 14:34:23

by Eliad Peller

[permalink] [raw]
Subject: [PATCH 03/12] wlcore: disable elp sleep while in plt mode

From: Yair Shapira <[email protected]>

We now disable elp sleep during plt mode to allow normal operation of
plt tools such as calibrator.

Having elp_sleep enabled during plt mode is actually not required and
in fact it disrupt plt operations such as rx statistics etc.

Signed-off-by: Yair Shapira <[email protected]>
Signed-off-by: Eliad Peller <[email protected]>
---
drivers/net/wireless/ti/wlcore/ps.c | 4 ++++
1 file changed, 4 insertions(+)

diff --git a/drivers/net/wireless/ti/wlcore/ps.c b/drivers/net/wireless/ti/wlcore/ps.c
index 98066d4..26bfc36 100644
--- a/drivers/net/wireless/ti/wlcore/ps.c
+++ b/drivers/net/wireless/ti/wlcore/ps.c
@@ -83,6 +83,10 @@ void wl1271_ps_elp_sleep(struct wl1271 *wl)
struct wl12xx_vif *wlvif;
u32 timeout;

+ /* We do not enter elp sleep in PLT mode */
+ if (wl->plt)
+ return;
+
if (wl->sleep_auth != WL1271_PSM_ELP)
return;

--
1.8.3.rc1.35.g9b79519


2013-09-11 08:10:37

by Eliad Peller

[permalink] [raw]
Subject: Re: [PATCH 11/12] wl18xx: print new RDL versions during boot

On Tue, Sep 10, 2013 at 10:47 AM, Luca Coelho <[email protected]> wrote:
> On Tue, 2013-09-03 at 17:34 +0300, Eliad Peller wrote:
>> From: Victor Goldenshtein <[email protected]>
>>
>> Extract and print info for the new RDL 5, 6, 7 and 8.
>> Replace const struct with function which translates
>> the RDL number to string.
>>
>> Signed-off-by: Victor Goldenshtein <[email protected]>
>> Signed-off-by: Barak Bercovitz <[email protected]>
>> Signed-off-by: Eliad Peller <[email protected]>
>> ---
>
> Why convert the array with a function? The array looks much cleaner to
> me.
>
the version numbers are not successive (4 -> 0x11), so i think it
makes sense, but i guess we can revert back to array if you prefer it
that way.

> [...[
>> diff --git a/drivers/net/wireless/ti/wl18xx/main.c b/drivers/net/wireless/ti/wl18xx/main.c
>> index b47eb62..aef0c91 100644
>> --- a/drivers/net/wireless/ti/wl18xx/main.c
>> +++ b/drivers/net/wireless/ti/wl18xx/main.c
>> @@ -1228,16 +1228,46 @@ static u32 wl18xx_ap_get_mimo_wide_rate_mask(struct wl1271 *wl,
>> }
>> }
>>
>> +static const char *wl18xx_rdl_name(enum wl18xx_rdl_num rdl_num)
>> +{
>> + switch (rdl_num) {
>> + case RDL_1_HP:
>> + return "183xH";
>> + case RDL_2_SP:
>> + return "183x or 180x";
>> + case RDL_3_HP:
>> + return "187xH";
>> + case RDL_4_SP:
>> + return "187x";
>> + case RDL_5_SP:
>> + return "RDL11 - Not Supported";
>> + case RDL_6_SP:
>> + return "180xD";
>> + case RDL_7_SP:
>> + return "RDL13 - Not Supported (1893Q)";
>> + case RDL_8_SP:
>> + return "18xxQ";
>> + default:
>> + return "UNTRIMMED";
>
> This may become misleading if we get more RDLs versions in the future.
> And the untrimmed case is probably reporting 0? Or something predefined,
> hopefully, otherwise how can we know that we wouldn't randomly get a
> valid value?
>
you're right here.
untrimmed devices always use the 0 value.
i'll split it to "untrimmed" and "unknown".

> Also, in the unsupported cases, it would probably be better to bail out?
i'm not sure what "not supported" actually mean here (it might still work...).
i'd prefer continue using these values only for debugging (there
shouldn't really be such unsupported devices out there, afaik)

Eliad.

2013-09-10 06:48:14

by Arik Nemtsov

[permalink] [raw]
Subject: Re: [PATCH 02/12] wlcore: add new plt power-mode: CHIP_AWAKE

On Tue, Sep 10, 2013 at 9:33 AM, Luca Coelho <[email protected]> wrote:
> On Tue, 2013-09-03 at 17:33 +0300, Eliad Peller wrote:
>> From: Yair Shapira <[email protected]>
>>
>> Under this mode the chip is powered on including sdio
>> but no FW is downloaded and run, interrupts are not enabled, etc...
>>
>> This mode is intended to allow RTTT to bridge sdio as a transport
>> to the chip.
>>
>> Driver only provides sdio access using the dev_mem debugfs file.
>>
>> Some fixes done to the code that ensures that PLT mode and normal
>> driver power mode (ifconfig/add_interface) are mutually excluded.
>>
>> Signed-off-by: Yair Shapira <[email protected]>
>> Signed-off-by: Eliad Peller <[email protected]>
>> ---
>
> I had some comments to this patch internally while I was still at TI.
> Namely, I asked why do we need a new way of doing this if this is
> already possible via debugsfs (using the gpio_power file)?

Are you commenting on the correct patch? Seems this is just a patch to
prevent "ifconfig up" during PLT mode..

2013-09-10 19:23:54

by Luca Coelho

[permalink] [raw]
Subject: Re: [PATCH 03/12] wlcore: disable elp sleep while in plt mode

On Tue, 2013-09-10 at 16:04 +0200, Eliad Peller wrote:
> On Tue, Sep 10, 2013 at 9:34 AM, Luca Coelho <[email protected]> wrote:
> > On Tue, 2013-09-03 at 17:33 +0300, Eliad Peller wrote:
> >> From: Yair Shapira <[email protected]>
> >>
> >> We now disable elp sleep during plt mode to allow normal operation of
> >> plt tools such as calibrator.
> >>
> >> Having elp_sleep enabled during plt mode is actually not required and
> >> in fact it disrupt plt operations such as rx statistics etc.
> >>
> >> Signed-off-by: Yair Shapira <[email protected]>
> >> Signed-off-by: Eliad Peller <[email protected]>
> >> ---
> >
> > I also had a comment internally about this one. Why do we need this?
> > AFAICT this would never happen in real life, since the firmware is not
> > even loaded at this point. Is there any real life situation where we
> > try to go into ELP while in PLT mode?
> >
> i'm not familiar with the whole plt process, but i guess some flows
> might indeed end up in elp (e.g. via wl1271_cmd_interrogate(), in
> order to read statistics)

Well, I don't think this would ever happen. The PLT stuff is a
completely different firmware and, really, if it tries to go into ELP
mode it is a bug.

In any case, there probably is a bug somewhere if this was implemented
at all. And applying it won't really hurt. I'm just nagging because it
would be nice to know what caused this to become necessary.

--
Luca.


2013-09-10 14:15:22

by Eliad Peller

[permalink] [raw]
Subject: Re: [PATCH 06/12] wlcore: send EAPOL frames with voice priority

On Tue, Sep 10, 2013 at 12:15 PM, Luca Coelho <[email protected]> wrote:
> On Tue, 2013-09-10 at 11:11 +0200, Arik Nemtsov wrote:
>> On Tue, Sep 10, 2013 at 9:47 AM, Luca Coelho <[email protected]> wrote:
>> > On Tue, 2013-09-03 at 17:34 +0300, Eliad Peller wrote:
>> >> From: Igal Chernobelsky <[email protected]>
>> >>
>> >> Send EAPOL frames with voice priority by setting TX_HW_ATTR_EAPOL_FRAME
>> >> new bit in tx attribute. Sending EAPOL with voice priority fixes
>> >> re-key timeout during heavy traffic issue.
>> >>
>> >> Signed-off-by: Igal Chernobelsky <[email protected]>
>> >> Signed-off-by: Eliad Peller <[email protected]>
>> >> ---
>> >
>> > This seems to be the same problem that Ben had and debugged by himself
>> > [1]. Fixing this in hostapd/wpa_supplicant seems more appropriate?
>> >
>> > This patch seems to take an advantage of some sort of hack in the
>> > firmware that will change the priority by itself when the
>> > TX_HW_ATTR_EAPOL_GRAME bit is set. If we have a good reason to use this
>> > patch, we need to take care of the firmware version as well. This
>> > probably doesn't work with the latest published firmware.
>> >
>> > [1] http://mid.gmane.org/[email protected]
>>
>> I believe he only fixed one mode (AP - hostapd) but not the other (GO
>> - wpa_supplicant). Anyway that's what we tested at the time.
>
> Okay, I didn't dig much into it.
>
>
>> Anyway marking it as this level doesn't do any harm. You're right
>> about the FW version though - it should be upstreamed first.
>
> Fair enough. If the firmware implements this, no reason why not use it.
> But I'll wait for the new firmware before applying this.
>
this change is backward compatible, so there's no issue applying it as is.
however, if you prefer waiting for a newer fw that's fine too.
i'll add it to the waiting list :)

Eliad.

2013-09-10 14:04:58

by Eliad Peller

[permalink] [raw]
Subject: Re: [PATCH 03/12] wlcore: disable elp sleep while in plt mode

On Tue, Sep 10, 2013 at 9:34 AM, Luca Coelho <[email protected]> wrote:
> On Tue, 2013-09-03 at 17:33 +0300, Eliad Peller wrote:
>> From: Yair Shapira <[email protected]>
>>
>> We now disable elp sleep during plt mode to allow normal operation of
>> plt tools such as calibrator.
>>
>> Having elp_sleep enabled during plt mode is actually not required and
>> in fact it disrupt plt operations such as rx statistics etc.
>>
>> Signed-off-by: Yair Shapira <[email protected]>
>> Signed-off-by: Eliad Peller <[email protected]>
>> ---
>
> I also had a comment internally about this one. Why do we need this?
> AFAICT this would never happen in real life, since the firmware is not
> even loaded at this point. Is there any real life situation where we
> try to go into ELP while in PLT mode?
>
i'm not familiar with the whole plt process, but i guess some flows
might indeed end up in elp (e.g. via wl1271_cmd_interrogate(), in
order to read statistics)

Eliad.

2013-09-03 14:34:32

by Eliad Peller

[permalink] [raw]
Subject: [PATCH 09/12] wlcore: fix regulatory domain bit translation

From: Ido Reis <[email protected]>

This is a fix for channels 52,56,60,64 bit translation.

Reported-by: Yaniv Machani <[email protected]>
Signed-off-by: Ido Reis <[email protected]>
Signed-off-by: Victor Goldenshtein <[email protected]>
Signed-off-by: Eliad Peller <[email protected]>
---
drivers/net/wireless/ti/wlcore/cmd.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/net/wireless/ti/wlcore/cmd.c b/drivers/net/wireless/ti/wlcore/cmd.c
index e3ae425..1cb3296 100644
--- a/drivers/net/wireless/ti/wlcore/cmd.c
+++ b/drivers/net/wireless/ti/wlcore/cmd.c
@@ -1613,8 +1613,10 @@ static int wlcore_get_reg_conf_ch_idx(enum ieee80211_band band, u16 ch)
case IEEE80211_BAND_5GHZ:
if (ch >= 8 && ch <= 16)
idx = ((ch-8)/4 + 18);
- else if (ch >= 34 && ch <= 64)
+ else if (ch >= 34 && ch <= 48)
idx = ((ch-34)/2 + 3 + 18);
+ else if (ch >= 52 && ch <= 64)
+ idx = ((ch-52)/4 + 11 + 18);
else if (ch >= 100 && ch <= 140)
idx = ((ch-100)/4 + 15 + 18);
else if (ch >= 149 && ch <= 165)
--
1.8.3.rc1.35.g9b79519


2013-09-03 14:34:30

by Eliad Peller

[permalink] [raw]
Subject: [PATCH 08/12] wlcore: fix unsafe dereference of the wlvif

From: Victor Goldenshtein <[email protected]>

wlvif could be passed as NULL from the wlcore_tx_work_locked()
to the wl1271_prepare_tx_frame() and to wl1271_skb_queue_head()
functions. This may lead to a Kernel panic, fix this by
validating that wlvif != NULL.

Signed-off-by: Victor Goldenshtein <[email protected]>
Signed-off-by: Eliad Peller <[email protected]>
---
drivers/net/wireless/ti/wlcore/tx.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/wireless/ti/wlcore/tx.c b/drivers/net/wireless/ti/wlcore/tx.c
index 3c02023..aa11a17 100644
--- a/drivers/net/wireless/ti/wlcore/tx.c
+++ b/drivers/net/wireless/ti/wlcore/tx.c
@@ -405,7 +405,7 @@ static int wl1271_prepare_tx_frame(struct wl1271 *wl, struct wl12xx_vif *wlvif,
is_wep = (cipher == WLAN_CIPHER_SUITE_WEP40) ||
(cipher == WLAN_CIPHER_SUITE_WEP104);

- if (WARN_ON(is_wep && wlvif->default_key != idx)) {
+ if (WARN_ON(is_wep && wlvif && wlvif->default_key != idx)) {
ret = wl1271_set_default_wep_key(wl, wlvif, idx);
if (ret < 0)
return ret;
--
1.8.3.rc1.35.g9b79519


2013-09-30 16:53:45

by Luca Coelho

[permalink] [raw]
Subject: Re: [PATCH 12/12] wlcore: always register dummy hardirq

On Wed, 2013-09-11 at 09:48 +0200, Eliad Peller wrote:
> On Tue, Sep 10, 2013 at 10:50 AM, Luca Coelho <[email protected]> wrote:
> > On Tue, 2013-09-03 at 17:34 +0300, Eliad Peller wrote:
> >> From: Arik Nemtsov <[email protected]>
> >>
> >> This keeps the kernel happy when using edge-irqs and requesting a
> >> threaded irq.
> >>
> >> Signed-off-by: Arik Nemtsov <[email protected]>
> >> Signed-off-by: Eliad Peller <[email protected]>
> >> ---
> >
> > Ouch, I think this is not going to fit at all with the DT patches I
> > sent.
> >
> this is quite minor patch (only making sure we always define hardirq
> when threaded_irq is used. i don't think it should conflict (at least
> logically) with DT patches...

It will. Just take a look at my DT patch series. Anyway, I'll apply
this for now and whoever (if ever) take over my DT series, will have to
make sure it still works. :P

--
Cheers,
Luca.


2013-09-03 14:34:37

by Eliad Peller

[permalink] [raw]
Subject: [PATCH 12/12] wlcore: always register dummy hardirq

From: Arik Nemtsov <[email protected]>

This keeps the kernel happy when using edge-irqs and requesting a
threaded irq.

Signed-off-by: Arik Nemtsov <[email protected]>
Signed-off-by: Eliad Peller <[email protected]>
---
drivers/net/wireless/ti/wlcore/main.c | 14 +++++++++++---
1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/drivers/net/wireless/ti/wlcore/main.c b/drivers/net/wireless/ti/wlcore/main.c
index d952593..a537483 100644
--- a/drivers/net/wireless/ti/wlcore/main.c
+++ b/drivers/net/wireless/ti/wlcore/main.c
@@ -5997,6 +5997,11 @@ static const struct wiphy_wowlan_support wlcore_wowlan_support = {
};
#endif

+static irqreturn_t wlcore_hardirq(int irq, void *cookie)
+{
+ return IRQ_WAKE_THREAD;
+}
+
static void wlcore_nvs_cb(const struct firmware *fw, void *context)
{
struct wl1271 *wl = context;
@@ -6005,6 +6010,7 @@ static void wlcore_nvs_cb(const struct firmware *fw, void *context)
struct wl12xx_platform_data *pdata = pdev_data->pdata;
unsigned long irqflags;
int ret;
+ irq_handler_t hardirq_fn = NULL;

if (fw) {
wl->nvs = kmemdup(fw->data, fw->size, GFP_KERNEL);
@@ -6033,12 +6039,14 @@ static void wlcore_nvs_cb(const struct firmware *fw, void *context)
wl->platform_quirks = pdata->platform_quirks;
wl->if_ops = pdev_data->if_ops;

- if (wl->platform_quirks & WL12XX_PLATFORM_QUIRK_EDGE_IRQ)
+ if (wl->platform_quirks & WL12XX_PLATFORM_QUIRK_EDGE_IRQ) {
irqflags = IRQF_TRIGGER_RISING;
- else
+ hardirq_fn = wlcore_hardirq;
+ } else {
irqflags = IRQF_TRIGGER_HIGH | IRQF_ONESHOT;
+ }

- ret = request_threaded_irq(wl->irq, NULL, wlcore_irq,
+ ret = request_threaded_irq(wl->irq, hardirq_fn, wlcore_irq,
irqflags, pdev->name, wl);
if (ret < 0) {
wl1271_error("request_irq() failed: %d", ret);
--
1.8.3.rc1.35.g9b79519


2013-09-03 14:34:20

by Eliad Peller

[permalink] [raw]
Subject: [PATCH 02/12] wlcore: add new plt power-mode: CHIP_AWAKE

From: Yair Shapira <[email protected]>

Under this mode the chip is powered on including sdio
but no FW is downloaded and run, interrupts are not enabled, etc...

This mode is intended to allow RTTT to bridge sdio as a transport
to the chip.

Driver only provides sdio access using the dev_mem debugfs file.

Some fixes done to the code that ensures that PLT mode and normal
driver power mode (ifconfig/add_interface) are mutually excluded.

Signed-off-by: Yair Shapira <[email protected]>
Signed-off-by: Eliad Peller <[email protected]>
---
drivers/net/wireless/ti/wlcore/main.c | 16 ++++++++++++----
drivers/net/wireless/ti/wlcore/testmode.c | 13 +++++++++++--
drivers/net/wireless/ti/wlcore/wlcore_i.h | 1 +
3 files changed, 24 insertions(+), 6 deletions(-)

diff --git a/drivers/net/wireless/ti/wlcore/main.c b/drivers/net/wireless/ti/wlcore/main.c
index b64b465..611e81d 100644
--- a/drivers/net/wireless/ti/wlcore/main.c
+++ b/drivers/net/wireless/ti/wlcore/main.c
@@ -1062,7 +1062,8 @@ int wl1271_plt_start(struct wl1271 *wl, const enum plt_mode plt_mode)
static const char* const PLT_MODE[] = {
"PLT_OFF",
"PLT_ON",
- "PLT_FEM_DETECT"
+ "PLT_FEM_DETECT",
+ "PLT_CHIP_AWAKE"
};

int ret;
@@ -1088,9 +1089,11 @@ int wl1271_plt_start(struct wl1271 *wl, const enum plt_mode plt_mode)
if (ret < 0)
goto power_off;

- ret = wl->ops->plt_init(wl);
- if (ret < 0)
- goto power_off;
+ if (plt_mode != PLT_CHIP_AWAKE) {
+ ret = wl->ops->plt_init(wl);
+ if (ret < 0)
+ goto power_off;
+ }

wl->state = WLCORE_STATE_ON;
wl1271_notice("firmware booted in PLT mode %s (%s)",
@@ -2419,6 +2422,11 @@ static int wl1271_op_add_interface(struct ieee80211_hw *hw,
int ret = 0;
u8 role_type;

+ if (wl->plt) {
+ wl1271_error("Adding Interface not allowed while in PLT mode");
+ return -EBUSY;
+ }
+
vif->driver_flags |= IEEE80211_VIF_BEACON_FILTER |
IEEE80211_VIF_SUPPORTS_CQM_RSSI;

diff --git a/drivers/net/wireless/ti/wlcore/testmode.c b/drivers/net/wireless/ti/wlcore/testmode.c
index 527590f..a3b7d95 100644
--- a/drivers/net/wireless/ti/wlcore/testmode.c
+++ b/drivers/net/wireless/ti/wlcore/testmode.c
@@ -297,7 +297,8 @@ static int wl1271_tm_cmd_set_plt_mode(struct wl1271 *wl, struct nlattr *tb[])
ret = wl1271_plt_stop(wl);
break;
case PLT_ON:
- ret = wl1271_plt_start(wl, PLT_ON);
+ case PLT_CHIP_AWAKE:
+ ret = wl1271_plt_start(wl, val);
break;
case PLT_FEM_DETECT:
ret = wl1271_tm_detect_fem(wl, tb);
@@ -361,6 +362,7 @@ int wl1271_tm_cmd(struct ieee80211_hw *hw, struct ieee80211_vif *vif,
{
struct wl1271 *wl = hw->priv;
struct nlattr *tb[WL1271_TM_ATTR_MAX + 1];
+ u32 nla_cmd;
int err;

err = nla_parse(tb, WL1271_TM_ATTR_MAX, data, len, wl1271_tm_policy);
@@ -370,7 +372,14 @@ int wl1271_tm_cmd(struct ieee80211_hw *hw, struct ieee80211_vif *vif,
if (!tb[WL1271_TM_ATTR_CMD_ID])
return -EINVAL;

- switch (nla_get_u32(tb[WL1271_TM_ATTR_CMD_ID])) {
+ nla_cmd = nla_get_u32(tb[WL1271_TM_ATTR_CMD_ID]);
+
+ /* Only SET_PLT_MODE is allowed in case of mode PLT_CHIP_AWAKE */
+ if (wl->plt_mode == PLT_CHIP_AWAKE &&
+ nla_cmd != WL1271_TM_CMD_SET_PLT_MODE)
+ return -EOPNOTSUPP;
+
+ switch (nla_cmd) {
case WL1271_TM_CMD_TEST:
return wl1271_tm_cmd_test(wl, tb);
case WL1271_TM_CMD_INTERROGATE:
diff --git a/drivers/net/wireless/ti/wlcore/wlcore_i.h b/drivers/net/wireless/ti/wlcore/wlcore_i.h
index 14fd111..3f4f08b 100644
--- a/drivers/net/wireless/ti/wlcore/wlcore_i.h
+++ b/drivers/net/wireless/ti/wlcore/wlcore_i.h
@@ -307,6 +307,7 @@ enum plt_mode {
PLT_OFF = 0,
PLT_ON = 1,
PLT_FEM_DETECT = 2,
+ PLT_CHIP_AWAKE = 3
};

struct wl12xx_rx_filter_field {
--
1.8.3.rc1.35.g9b79519


2013-09-10 06:56:14

by Luca Coelho

[permalink] [raw]
Subject: Re: [PATCH 02/12] wlcore: add new plt power-mode: CHIP_AWAKE

On Tue, 2013-09-10 at 08:47 +0200, Arik Nemtsov wrote:
> On Tue, Sep 10, 2013 at 9:33 AM, Luca Coelho <[email protected]> wrote:
> > On Tue, 2013-09-03 at 17:33 +0300, Eliad Peller wrote:
> >> From: Yair Shapira <[email protected]>
> >>
> >> Under this mode the chip is powered on including sdio
> >> but no FW is downloaded and run, interrupts are not enabled, etc...
> >>
> >> This mode is intended to allow RTTT to bridge sdio as a transport
> >> to the chip.
> >>
> >> Driver only provides sdio access using the dev_mem debugfs file.
> >>
> >> Some fixes done to the code that ensures that PLT mode and normal
> >> driver power mode (ifconfig/add_interface) are mutually excluded.
> >>
> >> Signed-off-by: Yair Shapira <[email protected]>
> >> Signed-off-by: Eliad Peller <[email protected]>
> >> ---
> >
> > I had some comments to this patch internally while I was still at TI.
> > Namely, I asked why do we need a new way of doing this if this is
> > already possible via debugsfs (using the gpio_power file)?
>
> Are you commenting on the correct patch? Seems this is just a patch to
> prevent "ifconfig up" during PLT mode..

Yes, I'm commenting on the right patch. It allows the chip power
(namely the WLAN_EN GPIO pin) to be set directly, without loading the
firmware and doing other initialization stuff. This can already be
controlled via the gpio_power debugfs file. I've used that a bunch of
times, including with the RTTT tool.

Okay, this patch has a few more protections (eg. not allowing an
interface to be added while the chip is powered in this way), but this
could also be added on top of the existing implementation.

--
Luca.


2013-09-10 13:46:53

by Eliad Peller

[permalink] [raw]
Subject: Re: [PATCH 02/12] wlcore: add new plt power-mode: CHIP_AWAKE

On Tue, Sep 10, 2013 at 9:56 AM, Luca Coelho <[email protected]> wrote:
> On Tue, 2013-09-10 at 08:47 +0200, Arik Nemtsov wrote:
>> On Tue, Sep 10, 2013 at 9:33 AM, Luca Coelho <[email protected]> wrote:
>> > On Tue, 2013-09-03 at 17:33 +0300, Eliad Peller wrote:
>> >> From: Yair Shapira <[email protected]>
>> >>
>> >> Under this mode the chip is powered on including sdio
>> >> but no FW is downloaded and run, interrupts are not enabled, etc...
>> >>
>> >> This mode is intended to allow RTTT to bridge sdio as a transport
>> >> to the chip.
>> >>
>> >> Driver only provides sdio access using the dev_mem debugfs file.
>> >>
>> >> Some fixes done to the code that ensures that PLT mode and normal
>> >> driver power mode (ifconfig/add_interface) are mutually excluded.
>> >>
>> >> Signed-off-by: Yair Shapira <[email protected]>
>> >> Signed-off-by: Eliad Peller <[email protected]>
>> >> ---
>> >
>> > I had some comments to this patch internally while I was still at TI.
>> > Namely, I asked why do we need a new way of doing this if this is
>> > already possible via debugsfs (using the gpio_power file)?
>>
>> Are you commenting on the correct patch? Seems this is just a patch to
>> prevent "ifconfig up" during PLT mode..
>
> Yes, I'm commenting on the right patch. It allows the chip power
> (namely the WLAN_EN GPIO pin) to be set directly, without loading the
> firmware and doing other initialization stuff. This can already be
> controlled via the gpio_power debugfs file. I've used that a bunch of
> times, including with the RTTT tool.
>
> Okay, this patch has a few more protections (eg. not allowing an
> interface to be added while the chip is powered in this way), but this
> could also be added on top of the existing implementation.
>
i guess this just a bit "cleaner" (similar to the way PLT is used).
the gpio_power debugfs only toggles the gpio power, without changing
the driver mode, so adding similar protections will be more
complicated.

Eliad.

2013-09-10 15:31:47

by Ben Greear

[permalink] [raw]
Subject: Re: [PATCH 06/12] wlcore: send EAPOL frames with voice priority

On 09/10/2013 07:15 AM, Eliad Peller wrote:
> On Tue, Sep 10, 2013 at 12:15 PM, Luca Coelho <[email protected]> wrote:
>> On Tue, 2013-09-10 at 11:11 +0200, Arik Nemtsov wrote:
>>> On Tue, Sep 10, 2013 at 9:47 AM, Luca Coelho <[email protected]> wrote:
>>>> On Tue, 2013-09-03 at 17:34 +0300, Eliad Peller wrote:
>>>>> From: Igal Chernobelsky <[email protected]>
>>>>>
>>>>> Send EAPOL frames with voice priority by setting TX_HW_ATTR_EAPOL_FRAME
>>>>> new bit in tx attribute. Sending EAPOL with voice priority fixes
>>>>> re-key timeout during heavy traffic issue.
>>>>>
>>>>> Signed-off-by: Igal Chernobelsky <[email protected]>
>>>>> Signed-off-by: Eliad Peller <[email protected]>
>>>>> ---
>>>>
>>>> This seems to be the same problem that Ben had and debugged by himself
>>>> [1]. Fixing this in hostapd/wpa_supplicant seems more appropriate?

I just posted hostapd/supplicant patches yesterday that fix it for both
AP mode and for station mode (at least in my configuration using ath9k
and 3.11 kernel).

Thanks,
Ben

--
Ben Greear <[email protected]>
Candela Technologies Inc http://www.candelatech.com


2013-09-03 14:34:27

by Eliad Peller

[permalink] [raw]
Subject: [PATCH 05/12] wl18xx: increase the number of allowed BA sessions

From: Victor Goldenshtein <[email protected]>

The current fw (actually starting from fw 8.5.0.0.58)
supports 8 rx BA sessions.

Signed-off-by: Victor Goldenshtein <[email protected]>
Signed-off-by: Eliad Peller <[email protected]>
---
drivers/net/wireless/ti/wl18xx/wl18xx.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/wireless/ti/wl18xx/wl18xx.h b/drivers/net/wireless/ti/wl18xx/wl18xx.h
index 9204e07..f700c4de 100644
--- a/drivers/net/wireless/ti/wl18xx/wl18xx.h
+++ b/drivers/net/wireless/ti/wl18xx/wl18xx.h
@@ -40,7 +40,7 @@

#define WL18XX_NUM_MAC_ADDRESSES 3

-#define WL18XX_RX_BA_MAX_SESSIONS 5
+#define WL18XX_RX_BA_MAX_SESSIONS 8

struct wl18xx_priv {
/* buffer for sending commands to FW */
--
1.8.3.rc1.35.g9b79519


2013-09-10 06:38:50

by Luca Coelho

[permalink] [raw]
Subject: Re: [PATCH 05/12] wl18xx: increase the number of allowed BA sessions

On Tue, 2013-09-03 at 17:34 +0300, Eliad Peller wrote:
> From: Victor Goldenshtein <[email protected]>
>
> The current fw (actually starting from fw 8.5.0.0.58)
> supports 8 rx BA sessions.
>
> Signed-off-by: Victor Goldenshtein <[email protected]>
> Signed-off-by: Eliad Peller <[email protected]>
> ---

The latest firmware version published at linux-firmware.git is
8.5.0.0.55 (wl18xx-fw-2.bin), so this patch cannot be taken into the
official tree yet.

Please publish a newer version of the firmware (with appropriate
increase in the filename version), since I cannot do that myself
anymore.

--
Luca.


2013-09-03 14:34:27

by Eliad Peller

[permalink] [raw]
Subject: [PATCH 06/12] wlcore: send EAPOL frames with voice priority

From: Igal Chernobelsky <[email protected]>

Send EAPOL frames with voice priority by setting TX_HW_ATTR_EAPOL_FRAME
new bit in tx attribute. Sending EAPOL with voice priority fixes
re-key timeout during heavy traffic issue.

Signed-off-by: Igal Chernobelsky <[email protected]>
Signed-off-by: Eliad Peller <[email protected]>
---
drivers/net/wireless/ti/wlcore/tx.c | 4 ++++
drivers/net/wireless/ti/wlcore/tx.h | 1 +
2 files changed, 5 insertions(+)

diff --git a/drivers/net/wireless/ti/wlcore/tx.c b/drivers/net/wireless/ti/wlcore/tx.c
index 03249da..3c02023 100644
--- a/drivers/net/wireless/ti/wlcore/tx.c
+++ b/drivers/net/wireless/ti/wlcore/tx.c
@@ -357,6 +357,10 @@ static void wl1271_tx_fill_hdr(struct wl1271 *wl, struct wl12xx_vif *wlvif,
ieee80211_has_protected(frame_control))
tx_attr |= TX_HW_ATTR_HOST_ENCRYPT;

+ /* send EAPOL frames as voice */
+ if (control->control.flags & IEEE80211_TX_CTRL_PORT_CTRL_PROTO)
+ tx_attr |= TX_HW_ATTR_EAPOL_FRAME;
+
desc->tx_attr = cpu_to_le16(tx_attr);

wlcore_hw_set_tx_desc_csum(wl, desc, skb);
diff --git a/drivers/net/wireless/ti/wlcore/tx.h b/drivers/net/wireless/ti/wlcore/tx.h
index 35489c3..79cb3ff 100644
--- a/drivers/net/wireless/ti/wlcore/tx.h
+++ b/drivers/net/wireless/ti/wlcore/tx.h
@@ -37,6 +37,7 @@
#define TX_HW_ATTR_TX_CMPLT_REQ BIT(12)
#define TX_HW_ATTR_TX_DUMMY_REQ BIT(13)
#define TX_HW_ATTR_HOST_ENCRYPT BIT(14)
+#define TX_HW_ATTR_EAPOL_FRAME BIT(15)

#define TX_HW_ATTR_OFST_SAVE_RETRIES 0
#define TX_HW_ATTR_OFST_HEADER_PAD 1
--
1.8.3.rc1.35.g9b79519


2013-09-10 06:33:08

by Luca Coelho

[permalink] [raw]
Subject: Re: [PATCH 02/12] wlcore: add new plt power-mode: CHIP_AWAKE

On Tue, 2013-09-03 at 17:33 +0300, Eliad Peller wrote:
> From: Yair Shapira <[email protected]>
>
> Under this mode the chip is powered on including sdio
> but no FW is downloaded and run, interrupts are not enabled, etc...
>
> This mode is intended to allow RTTT to bridge sdio as a transport
> to the chip.
>
> Driver only provides sdio access using the dev_mem debugfs file.
>
> Some fixes done to the code that ensures that PLT mode and normal
> driver power mode (ifconfig/add_interface) are mutually excluded.
>
> Signed-off-by: Yair Shapira <[email protected]>
> Signed-off-by: Eliad Peller <[email protected]>
> ---

I had some comments to this patch internally while I was still at TI.
Namely, I asked why do we need a new way of doing this if this is
already possible via debugsfs (using the gpio_power file)?

--
Cheers,
Luca.


2013-09-03 14:34:29

by Eliad Peller

[permalink] [raw]
Subject: [PATCH 07/12] wlcore: cleanup scan debug prints

From: Victor Goldenshtein <[email protected]>

Remove scan debug dumps which are rarely used.
Make scan debug prints more clear and short.

Signed-off-by: Victor Goldenshtein <[email protected]>
Signed-off-by: Eliad Peller <[email protected]>
---
drivers/net/wireless/ti/wlcore/cmd.c | 6 +++---
drivers/net/wireless/ti/wlcore/scan.c | 28 ++++++++++++++--------------
2 files changed, 17 insertions(+), 17 deletions(-)

diff --git a/drivers/net/wireless/ti/wlcore/cmd.c b/drivers/net/wireless/ti/wlcore/cmd.c
index c9e0607..e3ae425 100644
--- a/drivers/net/wireless/ti/wlcore/cmd.c
+++ b/drivers/net/wireless/ti/wlcore/cmd.c
@@ -1126,6 +1126,8 @@ int wl12xx_cmd_build_probe_req(struct wl1271 *wl, struct wl12xx_vif *wlvif,
u16 template_id_2_4 = wl->scan_templ_id_2_4;
u16 template_id_5 = wl->scan_templ_id_5;

+ wl1271_debug(DEBUG_SCAN, "build probe request band %d", band);
+
skb = ieee80211_probereq_get(wl->hw, vif, ssid, ssid_len,
ie_len);
if (!skb) {
@@ -1135,8 +1137,6 @@ int wl12xx_cmd_build_probe_req(struct wl1271 *wl, struct wl12xx_vif *wlvif,
if (ie_len)
memcpy(skb_put(skb, ie_len), ie, ie_len);

- wl1271_dump(DEBUG_SCAN, "PROBE REQ: ", skb->data, skb->len);
-
if (sched_scan &&
(wl->quirks & WLCORE_QUIRK_DUAL_PROBE_TMPL)) {
template_id_2_4 = wl->sched_scan_templ_id_2_4;
@@ -1172,7 +1172,7 @@ struct sk_buff *wl1271_cmd_build_ap_probe_req(struct wl1271 *wl,
if (!skb)
goto out;

- wl1271_dump(DEBUG_SCAN, "AP PROBE REQ: ", skb->data, skb->len);
+ wl1271_debug(DEBUG_SCAN, "set ap probe request template");

rate = wl1271_tx_min_rate_get(wl, wlvif->bitrate_masks[wlvif->band]);
if (wlvif->band == IEEE80211_BAND_2GHZ)
diff --git a/drivers/net/wireless/ti/wlcore/scan.c b/drivers/net/wireless/ti/wlcore/scan.c
index f407101..bbe0bc4 100644
--- a/drivers/net/wireless/ti/wlcore/scan.c
+++ b/drivers/net/wireless/ti/wlcore/scan.c
@@ -174,17 +174,7 @@ wlcore_scan_get_channels(struct wl1271 *wl,
/* if radar is set, we ignore the passive flag */
(radar ||
!!(flags & IEEE80211_CHAN_PASSIVE_SCAN) == passive)) {
- wl1271_debug(DEBUG_SCAN, "band %d, center_freq %d ",
- req_channels[i]->band,
- req_channels[i]->center_freq);
- wl1271_debug(DEBUG_SCAN, "hw_value %d, flags %X",
- req_channels[i]->hw_value,
- req_channels[i]->flags);
- wl1271_debug(DEBUG_SCAN, "max_power %d",
- req_channels[i]->max_power);
- wl1271_debug(DEBUG_SCAN, "min_dwell_time %d max dwell time %d",
- min_dwell_time_active,
- max_dwell_time_active);
+

if (flags & IEEE80211_CHAN_RADAR) {
channels[j].flags |= SCAN_CHANNEL_FLAGS_DFS;
@@ -222,6 +212,18 @@ wlcore_scan_get_channels(struct wl1271 *wl,
*n_pactive_ch);
}

+ wl1271_debug(DEBUG_SCAN, "freq %04d, ch. %03d, flags 0x%02X, power %02d, min/max_dwell %02d/%02d%s%s",
+ req_channels[i]->center_freq,
+ req_channels[i]->hw_value,
+ req_channels[i]->flags,
+ req_channels[i]->max_power,
+ min_dwell_time_active,
+ max_dwell_time_active,
+ flags & IEEE80211_CHAN_RADAR ?
+ ", DFS" : "",
+ flags & IEEE80211_CHAN_PASSIVE_SCAN ?
+ ", PASSIVE" : "");
+
j++;
}
}
@@ -364,7 +366,7 @@ wlcore_scan_sched_scan_ssid_list(struct wl1271 *wl,
struct cfg80211_ssid *ssids = req->ssids;
int ret = 0, type, i, j, n_match_ssids = 0;

- wl1271_debug(DEBUG_CMD, "cmd sched scan ssid list");
+ wl1271_debug((DEBUG_CMD | DEBUG_SCAN), "cmd sched scan ssid list");

/* count the match sets that contain SSIDs */
for (i = 0; i < req->n_match_sets; i++)
@@ -442,8 +444,6 @@ wlcore_scan_sched_scan_ssid_list(struct wl1271 *wl,
}
}

- wl1271_dump(DEBUG_SCAN, "SSID_LIST: ", cmd, sizeof(*cmd));
-
ret = wl1271_cmd_send(wl, CMD_CONNECTION_SCAN_SSID_CFG, cmd,
sizeof(*cmd), 0);
if (ret < 0) {
--
1.8.3.rc1.35.g9b79519


2013-09-10 19:40:47

by Luca Coelho

[permalink] [raw]
Subject: Re: [PATCH 09/12] wlcore: fix regulatory domain bit translation

On Tue, 2013-09-10 at 16:51 +0200, Eliad Peller wrote:
> On Tue, Sep 10, 2013 at 10:39 AM, Luca Coelho <[email protected]> wrote:
> > On Tue, 2013-09-03 at 17:34 +0300, Eliad Peller wrote:
> >> From: Ido Reis <[email protected]>
> >>
> >> This is a fix for channels 52,56,60,64 bit translation.
> >>
> >> Reported-by: Yaniv Machani <[email protected]>
> >> Signed-off-by: Ido Reis <[email protected]>
> >> Signed-off-by: Victor Goldenshtein <[email protected]>
> >> Signed-off-by: Eliad Peller <[email protected]>
> >> ---
> >> drivers/net/wireless/ti/wlcore/cmd.c | 4 +++-
> >> 1 file changed, 3 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/net/wireless/ti/wlcore/cmd.c b/drivers/net/wireless/ti/wlcore/cmd.c
> >> index e3ae425..1cb3296 100644
> >> --- a/drivers/net/wireless/ti/wlcore/cmd.c
> >> +++ b/drivers/net/wireless/ti/wlcore/cmd.c
> >> @@ -1613,8 +1613,10 @@ static int wlcore_get_reg_conf_ch_idx(enum ieee80211_band band, u16 ch)
> >> case IEEE80211_BAND_5GHZ:
> >> if (ch >= 8 && ch <= 16)
> >> idx = ((ch-8)/4 + 18);
> >> - else if (ch >= 34 && ch <= 64)
> >> + else if (ch >= 34 && ch <= 48)
> >> idx = ((ch-34)/2 + 3 + 18);
> >> + else if (ch >= 52 && ch <= 64)
> >> + idx = ((ch-52)/4 + 11 + 18);
> >> else if (ch >= 100 && ch <= 140)
> >> idx = ((ch-100)/4 + 15 + 18);
> >> else if (ch >= 149 && ch <= 165)
> >
> > Hmmm... I don't have a clue what is going on here. I don't know how I
> > let this function pass as is originally, shame on me. :)
> >
> :)
>
> > The change probably makes things work better, since someone apparently
> > saw a bug in real life and reported it, but can anyone explain what is
> > going on during this translation? Aren't we losing data here? Eg.
> > channels 8, 9, 10 and 11 all use the same bit in the firmware command
> > bitmask?
> >
> the 8-16 indeed looks weird, as the driver indeed declares supports
> for channels 7,8,9,11...

Yes, weird, because channel 7 would return an error even. And 9 and 11
would use 8...


> the other conditions are a simple attempt to reduce the gap between
> the channels (e.g. no channels between 64 and 100) in order to get a
> minimized bitmap.

Right, I should have looked into the channel list we advertise.


> since the gap between the defined channels in the 5ghz band is usually
> 2/4 (36->38 vs. 60->64) the other calculations seems fine (i didn't
> dig into them as well, though).

Yes, now that you mentioned it looks clearer, but still, this function
sucks big time. If it takes more than a couple of minutes to understand
what is going on (in such a simple thing as this is), it should be
rewritten. Or at _least_ better documented.


> i'll try getting some more information from the fw guys regarding the
> 8-16 range.

Great! Thanks.

--
Luca.


2013-09-10 09:14:45

by Luca Coelho

[permalink] [raw]
Subject: Re: [PATCH 06/12] wlcore: send EAPOL frames with voice priority

On Tue, 2013-09-10 at 11:11 +0200, Arik Nemtsov wrote:
> On Tue, Sep 10, 2013 at 9:47 AM, Luca Coelho <[email protected]> wrote:
> > On Tue, 2013-09-03 at 17:34 +0300, Eliad Peller wrote:
> >> From: Igal Chernobelsky <[email protected]>
> >>
> >> Send EAPOL frames with voice priority by setting TX_HW_ATTR_EAPOL_FRAME
> >> new bit in tx attribute. Sending EAPOL with voice priority fixes
> >> re-key timeout during heavy traffic issue.
> >>
> >> Signed-off-by: Igal Chernobelsky <[email protected]>
> >> Signed-off-by: Eliad Peller <[email protected]>
> >> ---
> >
> > This seems to be the same problem that Ben had and debugged by himself
> > [1]. Fixing this in hostapd/wpa_supplicant seems more appropriate?
> >
> > This patch seems to take an advantage of some sort of hack in the
> > firmware that will change the priority by itself when the
> > TX_HW_ATTR_EAPOL_GRAME bit is set. If we have a good reason to use this
> > patch, we need to take care of the firmware version as well. This
> > probably doesn't work with the latest published firmware.
> >
> > [1] http://mid.gmane.org/[email protected]
>
> I believe he only fixed one mode (AP - hostapd) but not the other (GO
> - wpa_supplicant). Anyway that's what we tested at the time.

Okay, I didn't dig much into it.


> Anyway marking it as this level doesn't do any harm. You're right
> about the FW version though - it should be upstreamed first.

Fair enough. If the firmware implements this, no reason why not use it.
But I'll wait for the new firmware before applying this.

--
Luca.


2013-09-03 14:34:34

by Eliad Peller

[permalink] [raw]
Subject: [PATCH 10/12] wl18xx: fix boot process in high temperature environment

From: Victor Goldenshtein <[email protected]>

In addition to existing WCS PLL configuration add and enable
also the coex PLL during init phase. This fixes boot failures
due to silicon latchup in high temperature environment (>85c).

Signed-off-by: Victor Goldenshtein <[email protected]>
Signed-off-by: Nadim Zubidat <[email protected]>
Signed-off-by: Eliad Peller <[email protected]>
---
drivers/net/wireless/ti/wl18xx/main.c | 53 +++++++++++++++++++++++++++++++++++
drivers/net/wireless/ti/wl18xx/reg.h | 13 +++++++++
2 files changed, 66 insertions(+)

diff --git a/drivers/net/wireless/ti/wl18xx/main.c b/drivers/net/wireless/ti/wl18xx/main.c
index 7aa0eb8..b47eb62 100644
--- a/drivers/net/wireless/ti/wl18xx/main.c
+++ b/drivers/net/wireless/ti/wl18xx/main.c
@@ -623,6 +623,18 @@ static const int wl18xx_rtable[REG_TABLE_LEN] = {
[REG_RAW_FW_STATUS_ADDR] = WL18XX_FW_STATUS_ADDR,
};

+static const struct wl18xx_clk_cfg wl18xx_clk_table_coex[NUM_CLOCK_CONFIGS] = {
+ [CLOCK_CONFIG_16_2_M] = { 8, 121, 0, 0, false },
+ [CLOCK_CONFIG_16_368_M] = { 8, 120, 0, 0, false },
+ [CLOCK_CONFIG_16_8_M] = { 8, 117, 0, 0, false },
+ [CLOCK_CONFIG_19_2_M] = { 10, 128, 0, 0, false },
+ [CLOCK_CONFIG_26_M] = { 11, 104, 0, 0, false },
+ [CLOCK_CONFIG_32_736_M] = { 8, 120, 0, 0, false },
+ [CLOCK_CONFIG_33_6_M] = { 8, 117, 0, 0, false },
+ [CLOCK_CONFIG_38_468_M] = { 10, 128, 0, 0, false },
+ [CLOCK_CONFIG_52_M] = { 11, 104, 0, 0, false },
+};
+
static const struct wl18xx_clk_cfg wl18xx_clk_table[NUM_CLOCK_CONFIGS] = {
[CLOCK_CONFIG_16_2_M] = { 7, 104, 801, 4, true },
[CLOCK_CONFIG_16_368_M] = { 9, 132, 3751, 4, true },
@@ -704,6 +716,23 @@ static int wl18xx_set_clk(struct wl1271 *wl)
wl18xx_clk_table[clk_freq].p, wl18xx_clk_table[clk_freq].q,
wl18xx_clk_table[clk_freq].swallow ? "swallow" : "spit");

+ /* coex PLL configuration */
+ ret = wl18xx_top_reg_write(wl, PLLSH_COEX_PLL_N,
+ wl18xx_clk_table_coex[clk_freq].n);
+ if (ret < 0)
+ goto out;
+
+ ret = wl18xx_top_reg_write(wl, PLLSH_COEX_PLL_M,
+ wl18xx_clk_table_coex[clk_freq].m);
+ if (ret < 0)
+ goto out;
+
+ /* bypass the swallowing logic */
+ ret = wl18xx_top_reg_write(wl, PLLSH_COEX_PLL_SWALLOW_EN,
+ PLLSH_COEX_PLL_SWALLOW_EN_VAL1);
+ if (ret < 0)
+ goto out;
+
ret = wl18xx_top_reg_write(wl, PLLSH_WCS_PLL_N,
wl18xx_clk_table[clk_freq].n);
if (ret < 0)
@@ -745,6 +774,30 @@ static int wl18xx_set_clk(struct wl1271 *wl)
PLLSH_WCS_PLL_SWALLOW_EN_VAL2);
}

+ /* choose WCS PLL */
+ ret = wl18xx_top_reg_write(wl, PLLSH_WL_PLL_SEL,
+ PLLSH_WL_PLL_SEL_WCS_PLL);
+ if (ret < 0)
+ goto out;
+
+ /* enable both PLLs */
+ ret = wl18xx_top_reg_write(wl, PLLSH_WL_PLL_EN, PLLSH_WL_PLL_EN_VAL1);
+ if (ret < 0)
+ goto out;
+
+ udelay(1000);
+
+ /* disable coex PLL */
+ ret = wl18xx_top_reg_write(wl, PLLSH_WL_PLL_EN, PLLSH_WL_PLL_EN_VAL2);
+ if (ret < 0)
+ goto out;
+
+ /* reset the swallowing logic */
+ ret = wl18xx_top_reg_write(wl, PLLSH_COEX_PLL_SWALLOW_EN,
+ PLLSH_COEX_PLL_SWALLOW_EN_VAL2);
+ if (ret < 0)
+ goto out;
+
out:
return ret;
}
diff --git a/drivers/net/wireless/ti/wl18xx/reg.h b/drivers/net/wireless/ti/wl18xx/reg.h
index 05dd8ba..88de3f2 100644
--- a/drivers/net/wireless/ti/wl18xx/reg.h
+++ b/drivers/net/wireless/ti/wl18xx/reg.h
@@ -114,6 +114,11 @@
#define PLATFORM_DETECTION 0xA0E3E0
#define OCS_EN 0xA02080
#define PRIMARY_CLK_DETECT 0xA020A6
+#define PLLSH_COEX_PLL_N 0xA02384
+#define PLLSH_COEX_PLL_M 0xA02382
+#define PLLSH_COEX_PLL_SWALLOW_EN 0xA0238E
+#define PLLSH_WL_PLL_SEL 0xA02398
+
#define PLLSH_WCS_PLL_N 0xA02362
#define PLLSH_WCS_PLL_M 0xA02360
#define PLLSH_WCS_PLL_Q_FACTOR_CFG_1 0xA02364
@@ -128,9 +133,17 @@
#define PLLSH_WCS_PLL_P_FACTOR_CFG_1_MASK 0xFFFF
#define PLLSH_WCS_PLL_P_FACTOR_CFG_2_MASK 0x000F

+#define PLLSH_WL_PLL_EN_VAL1 0x7
+#define PLLSH_WL_PLL_EN_VAL2 0x2
+#define PLLSH_COEX_PLL_SWALLOW_EN_VAL1 0x2
+#define PLLSH_COEX_PLL_SWALLOW_EN_VAL2 0x11
+
#define PLLSH_WCS_PLL_SWALLOW_EN_VAL1 0x1
#define PLLSH_WCS_PLL_SWALLOW_EN_VAL2 0x12

+#define PLLSH_WL_PLL_SEL_WCS_PLL 0x0
+#define PLLSH_WL_PLL_SEL_COEX_PLL 0x1
+
#define WL18XX_REG_FUSE_DATA_1_3 0xA0260C
#define WL18XX_PG_VER_MASK 0x70
#define WL18XX_PG_VER_OFFSET 4
--
1.8.3.rc1.35.g9b79519


2013-09-03 14:34:23

by Eliad Peller

[permalink] [raw]
Subject: [PATCH 04/12] wlcore: re-enable idle handling

From: Arik Nemtsov <[email protected]>

We need some stuff done on idle change, most notably we have to stop
sched-scanning. Take care of this by reintroducing idle handling.

Signed-off-by: Arik Nemtsov <[email protected]>
Signed-off-by: Eliad Peller <[email protected]>
---
drivers/net/wireless/ti/wlcore/main.c | 22 ++++++++++++++++++++++
drivers/net/wireless/ti/wlcore/wlcore_i.h | 1 +
2 files changed, 23 insertions(+)

diff --git a/drivers/net/wireless/ti/wlcore/main.c b/drivers/net/wireless/ti/wlcore/main.c
index 611e81d..d952593 100644
--- a/drivers/net/wireless/ti/wlcore/main.c
+++ b/drivers/net/wireless/ti/wlcore/main.c
@@ -2927,6 +2927,25 @@ static void wl1271_set_band_rate(struct wl1271 *wl, struct wl12xx_vif *wlvif)
wlvif->rate_set = wlvif->basic_rate_set;
}

+static void wl1271_sta_handle_idle(struct wl1271 *wl, struct wl12xx_vif *wlvif,
+ bool idle)
+{
+ bool cur_idle = !test_bit(WLVIF_FLAG_ACTIVE, &wlvif->flags);
+
+ if (idle == cur_idle)
+ return;
+
+ if (idle) {
+ clear_bit(WLVIF_FLAG_ACTIVE, &wlvif->flags);
+ } else {
+ /* The current firmware only supports sched_scan in idle */
+ if (wl->sched_vif == wlvif)
+ wl->ops->sched_scan_stop(wl, wlvif);
+
+ set_bit(WLVIF_FLAG_ACTIVE, &wlvif->flags);
+ }
+}
+
static int wl12xx_config_vif(struct wl1271 *wl, struct wl12xx_vif *wlvif,
struct ieee80211_conf *conf, u32 changed)
{
@@ -4179,6 +4198,9 @@ static void wl1271_bss_info_changed_sta(struct wl1271 *wl,
do_join = true;
}

+ if (changed & BSS_CHANGED_IDLE && !is_ibss)
+ wl1271_sta_handle_idle(wl, wlvif, bss_conf->idle);
+
if (changed & BSS_CHANGED_CQM) {
bool enable = false;
if (bss_conf->cqm_rssi_thold)
diff --git a/drivers/net/wireless/ti/wlcore/wlcore_i.h b/drivers/net/wireless/ti/wlcore/wlcore_i.h
index 3f4f08b..2a50e08 100644
--- a/drivers/net/wireless/ti/wlcore/wlcore_i.h
+++ b/drivers/net/wireless/ti/wlcore/wlcore_i.h
@@ -255,6 +255,7 @@ enum wl12xx_vif_flags {
WLVIF_FLAG_CS_PROGRESS,
WLVIF_FLAG_AP_PROBE_RESP_SET,
WLVIF_FLAG_IN_USE,
+ WLVIF_FLAG_ACTIVE,
};

struct wl12xx_vif;
--
1.8.3.rc1.35.g9b79519


2013-09-10 07:19:19

by Luca Coelho

[permalink] [raw]
Subject: Re: [PATCH 07/12] wlcore: cleanup scan debug prints

On Tue, 2013-09-03 at 17:34 +0300, Eliad Peller wrote:
> From: Victor Goldenshtein <[email protected]>
>
> Remove scan debug dumps which are rarely used.
> Make scan debug prints more clear and short.
>
> Signed-off-by: Victor Goldenshtein <[email protected]>
> Signed-off-by: Eliad Peller <[email protected]>
> ---

This is fine, these dumps were used when the firmware scan APIs were
changing all the time and we were having lots of problems following it.
It can be removed now to reduce excessive output.

Some nitpicks below.

[...]
> diff --git a/drivers/net/wireless/ti/wlcore/scan.c b/drivers/net/wireless/ti/wlcore/scan.c
> index f407101..bbe0bc4 100644
> --- a/drivers/net/wireless/ti/wlcore/scan.c
> +++ b/drivers/net/wireless/ti/wlcore/scan.c
> @@ -174,17 +174,7 @@ wlcore_scan_get_channels(struct wl1271 *wl,
> /* if radar is set, we ignore the passive flag */
> (radar ||
> !!(flags & IEEE80211_CHAN_PASSIVE_SCAN) == passive)) {
> - wl1271_debug(DEBUG_SCAN, "band %d, center_freq %d ",
> - req_channels[i]->band,
> - req_channels[i]->center_freq);
> - wl1271_debug(DEBUG_SCAN, "hw_value %d, flags %X",
> - req_channels[i]->hw_value,
> - req_channels[i]->flags);
> - wl1271_debug(DEBUG_SCAN, "max_power %d",
> - req_channels[i]->max_power);
> - wl1271_debug(DEBUG_SCAN, "min_dwell_time %d max dwell time %d",
> - min_dwell_time_active,
> - max_dwell_time_active);
> +
>

Unnecessary extra line here.

[...]
> @@ -222,6 +212,18 @@ wlcore_scan_get_channels(struct wl1271 *wl,
> *n_pactive_ch);
> }
>
> + wl1271_debug(DEBUG_SCAN, "freq %04d, ch. %03d, flags 0x%02X, power %02d, min/max_dwell %02d/%02d%s%s",

Left-padding with zeros is quite ugly with the decimals here.

--
Luca.