Re: [PATCH v2 01/16] wl1251: fix scan behaviour while not associated

2013-12-31 Thread Pali Rohár 
On Tuesday 10 December 2013 10:21:14 Pavel Machek wrote:
> Hi!
> 
> > diff --git a/drivers/net/wireless/ti/wl1251/cmd.c
> > b/drivers/net/wireless/ti/wl1251/cmd.c index
> > 6822b84..16b6479 100644
> > --- a/drivers/net/wireless/ti/wl1251/cmd.c
> > +++ b/drivers/net/wireless/ti/wl1251/cmd.c
> > @@ -410,7 +411,10 @@ int wl1251_cmd_scan(struct wl1251 *wl,
> > u8 *ssid, size_t ssid_len,
> > 
> > struct wl1251_cmd_scan *cmd;
> > int i, ret = 0;
> > 
> > -   wl1251_debug(DEBUG_CMD, "cmd scan");
> > +   wl1251_debug(DEBUG_CMD, "cmd scan channels %d ssid(%d)
> > '%s'", + n_channels, (int)ssid_len, ssid);
> > +
> > +   WARN_ON(n_channels > SCAN_MAX_NUM_OF_CHANNELS);
> 
> ssids can have \0s in them... and what is worse, they may not
> be 0 terminated AFAICT.
> 
> Potential solution is at
> http://www.spinics.net/lists/linux-wireless/msg98640.html .
> 
> Thanks,
>   Pavel

Ok. To prevent other problems in future, I removed printing ssid
and len params from debug output. I think it is not needed...

Here is updated patch:

From: =?UTF-8?q?Pali=20Roh=C3=A1r?= 
Subject: [PATCH v2 01/16] wl1251: fix scan behaviour while not associated
Date: Sun,  8 Dec 2013 10:24:59 +0100
MIME-Version: 1.0
Content-Type: text/plain;
  charset=UTF-8
Content-Transfer-Encoding: 8bit

From: David Gnedt 

With a dissacociated card I often encoutered very long scan delays.

My guess is that it has something to do with the cards DTIM handling and
another firmware bug mentioned in the TI WLAN driver, which is described as
the card may never end scanning if the channel is overloaded because it
can't send probe requests. I think the firmware somehow also tries to
receive DTIM messages when the BSSID is not set. Therefore most of the time
it waits for DTIM messages and can't do scanning work.

Anyway we can workaround this misbehaviour by setting the HIGH_PRIORITY
bit for scans in disassociated state.

Signed-off-by: David Gnedt 
Signed-off-by: Pali Rohár 
---
 drivers/net/wireless/ti/wl1251/cmd.c  |   13 -
 drivers/net/wireless/ti/wl1251/cmd.h  |5 +
 drivers/net/wireless/ti/wl1251/main.c |1 +
 3 files changed, 18 insertions(+), 1 deletion(-)

diff --git a/drivers/net/wireless/ti/wl1251/cmd.c 
b/drivers/net/wireless/ti/wl1251/cmd.c
index 6822b84..16b6479 100644
--- a/drivers/net/wireless/ti/wl1251/cmd.c
+++ b/drivers/net/wireless/ti/wl1251/cmd.c
@@ -3,6 +3,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include "wl1251.h"
 #include "reg.h"
@@ -410,7 +411,9 @@ int wl1251_cmd_scan(struct wl1251 *wl, u8 *ssid, size_t 
ssid_len,
struct wl1251_cmd_scan *cmd;
int i, ret = 0;
 
-   wl1251_debug(DEBUG_CMD, "cmd scan");
+   wl1251_debug(DEBUG_CMD, "cmd scan channels %d", n_channels);
+
+   WARN_ON(n_channels > SCAN_MAX_NUM_OF_CHANNELS);
 
cmd = kzalloc(sizeof(*cmd), GFP_KERNEL);
if (!cmd)
@@ -421,6 +425,13 @@ int wl1251_cmd_scan(struct wl1251 *wl, u8 *ssid, size_t 
ssid_len,
CFG_RX_MGMT_EN |
CFG_RX_BCN_EN);
cmd->params.scan_options = 0;
+   /*
+* Use high priority scan when not associated to prevent fw issue
+* causing never-ending scans (sometimes 20+ minutes).
+* Note: This bug may be caused by the fw's DTIM handling.
+*/
+   if (is_zero_ether_addr(wl->bssid))
+   cmd->params.scan_options |= WL1251_SCAN_OPT_PRIORITY_HIGH;
cmd->params.num_channels = n_channels;
cmd->params.num_probe_requests = n_probes;
cmd->params.tx_rate = cpu_to_le16(1 << 1); /* 2 Mbps */
diff --git a/drivers/net/wireless/ti/wl1251/cmd.h 
b/drivers/net/wireless/ti/wl1251/cmd.h
index ee4f2b3..126f273 100644
--- a/drivers/net/wireless/ti/wl1251/cmd.h
+++ b/drivers/net/wireless/ti/wl1251/cmd.h
@@ -167,6 +167,11 @@ struct cmd_read_write_memory {
 #define CMDMBOX_HEADER_LEN 4
 #define CMDMBOX_INFO_ELEM_HEADER_LEN 4
 
+#define WL1251_SCAN_OPT_PASSIVE1
+#define WL1251_SCAN_OPT_5GHZ_BAND  2
+#define WL1251_SCAN_OPT_TRIGGERD_SCAN  4
+#define WL1251_SCAN_OPT_PRIORITY_HIGH  8
+
 #define WL1251_SCAN_MIN_DURATION 3
 #define WL1251_SCAN_MAX_DURATION 6
 
diff --git a/drivers/net/wireless/ti/wl1251/main.c 
b/drivers/net/wireless/ti/wl1251/main.c
index 3291ffa..4d89ac8 100644
--- a/drivers/net/wireless/ti/wl1251/main.c
+++ b/drivers/net/wireless/ti/wl1251/main.c
@@ -930,6 +930,7 @@ static int wl1251_op_hw_scan(struct ieee80211_hw *hw,
ret = wl1251_cmd_scan(wl, ssid, ssid_len, req->channels,
  req->n_channels, WL1251_SCAN_NUM_PROBES);
if (ret

Re: [PATCH v2 01/16] wl1251: fix scan behaviour while not associated

2013-12-11 Thread Ben Hutchings 
On Tue, 2013-12-10 at 18:08 +0100, Pali Rohár wrote:
> On Tuesday 10 December 2013 16:41:04 Kalle Valo wrote:
> > Pavel Machek  writes:
> > > ssids can have \0s in them... and what is worse, they may
> > > not be 0 terminated AFAICT.
> > > 
> > > Potential solution is at
> > > http://www.spinics.net/lists/linux-wireless/msg98640.html .
> > 
> > I just use print_hex_dump_bytes() to print SSIDs.
> 
> Ok and has kernel printf modifier for size_t?

It is 'z', same as in userland.

Ben.

-- 
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v2 01/16] wl1251: fix scan behaviour while not associated

2013-12-10 Thread Ивайло Димитров 
  > Оригинално писмо 
 >От:  Pali Rohár 
 >Относно: Re: [PATCH v2 01/16] wl1251: fix scan behaviour while not associated
 >До: Kalle Valo ,
 Pavel Machek 
 >Изпратено на: Вторник, 2013, Декември 10 19:08:44 EET
 >
 >
 >On Tuesday 10 December 2013 16:41:04 Kalle Valo wrote:
 >> Pavel Machek  writes:
 >> > ssids can have \0s in them... and what is worse, they may
 >> > not be 0 terminated AFAICT.
 >> > 
 >> > Potential solution is at
 >> > http://www.spinics.net/lists/linux-wireless/msg98640.html .
 >> 
 >> I just use print_hex_dump_bytes() to print SSIDs.
 >
 >Ok and has kernel printf modifier for size_t?
 >
 >-- 
 >Pali Rohár
 >pali.ro...@gmail.com
 >

If I read lib/vsprintf.c correctly, you can use 'z'

Regards,
Ivo

PS: Please use my gmail address when cc-ing me, LKML really hates my abv dot bg 
account :)

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v2 01/16] wl1251: fix scan behaviour while not associated

2013-12-10 Thread Pali Rohár 
On Tuesday 10 December 2013 16:41:04 Kalle Valo wrote:
> Pavel Machek  writes:
> > ssids can have \0s in them... and what is worse, they may
> > not be 0 terminated AFAICT.
> > 
> > Potential solution is at
> > http://www.spinics.net/lists/linux-wireless/msg98640.html .
> 
> I just use print_hex_dump_bytes() to print SSIDs.

Ok and has kernel printf modifier for size_t?

-- 
Pali Rohár
pali.ro...@gmail.com


signature.asc
Description: This is a digitally signed message part.

Re: [PATCH v2 01/16] wl1251: fix scan behaviour while not associated

2013-12-10 Thread Kalle Valo 
Pavel Machek  writes:

> ssids can have \0s in them... and what is worse, they may not be 0
> terminated AFAICT.
>
> Potential solution is at
> http://www.spinics.net/lists/linux-wireless/msg98640.html . 

I just use print_hex_dump_bytes() to print SSIDs.

-- 
Kalle Valo
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v2 01/16] wl1251: fix scan behaviour while not associated

2013-12-10 Thread Pavel Machek 
Hi!

> diff --git a/drivers/net/wireless/ti/wl1251/cmd.c 
> b/drivers/net/wireless/ti/wl1251/cmd.c
> index 6822b84..16b6479 100644
> --- a/drivers/net/wireless/ti/wl1251/cmd.c
> +++ b/drivers/net/wireless/ti/wl1251/cmd.c
> @@ -410,7 +411,10 @@ int wl1251_cmd_scan(struct wl1251 *wl, u8 *ssid, size_t 
> ssid_len,
>   struct wl1251_cmd_scan *cmd;
>   int i, ret = 0;
>  
> - wl1251_debug(DEBUG_CMD, "cmd scan");
> + wl1251_debug(DEBUG_CMD, "cmd scan channels %d ssid(%d) '%s'",
> +  n_channels, (int)ssid_len, ssid);
> +
> + WARN_ON(n_channels > SCAN_MAX_NUM_OF_CHANNELS);
>  

ssids can have \0s in them... and what is worse, they may not be 0
terminated AFAICT.

Potential solution is at
http://www.spinics.net/lists/linux-wireless/msg98640.html . 

Thanks,
Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH v2 01/16] wl1251: fix scan behaviour while not associated

2013-12-08 Thread Pali Rohár 
From: David Gnedt 

With a dissacociated card I often encoutered very long scan delays.

My guess is that it has something to do with the cards DTIM handling and
another firmware bug mentioned in the TI WLAN driver, which is described as
the card may never end scanning if the channel is overloaded because it
can't send probe requests. I think the firmware somehow also tries to
receive DTIM messages when the BSSID is not set. Therefore most of the time
it waits for DTIM messages and can't do scanning work.

Anyway we can workaround this misbehaviour by setting the HIGH_PRIORITY
bit for scans in disassociated state.

Signed-off-by: David Gnedt 
Signed-off-by: Pali Rohár 
---
 drivers/net/wireless/ti/wl1251/cmd.c  |   13 -
 drivers/net/wireless/ti/wl1251/cmd.h  |5 +
 drivers/net/wireless/ti/wl1251/main.c |1 +
 3 files changed, 18 insertions(+), 1 deletion(-)

diff --git a/drivers/net/wireless/ti/wl1251/cmd.c 
b/drivers/net/wireless/ti/wl1251/cmd.c
index 6822b84..16b6479 100644
--- a/drivers/net/wireless/ti/wl1251/cmd.c
+++ b/drivers/net/wireless/ti/wl1251/cmd.c
@@ -3,6 +3,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include "wl1251.h"
 #include "reg.h"
@@ -410,7 +411,10 @@ int wl1251_cmd_scan(struct wl1251 *wl, u8 *ssid, size_t 
ssid_len,
struct wl1251_cmd_scan *cmd;
int i, ret = 0;
 
-   wl1251_debug(DEBUG_CMD, "cmd scan");
+   wl1251_debug(DEBUG_CMD, "cmd scan channels %d ssid(%d) '%s'",
+n_channels, (int)ssid_len, ssid);
+
+   WARN_ON(n_channels > SCAN_MAX_NUM_OF_CHANNELS);
 
cmd = kzalloc(sizeof(*cmd), GFP_KERNEL);
if (!cmd)
@@ -421,6 +425,13 @@ int wl1251_cmd_scan(struct wl1251 *wl, u8 *ssid, size_t 
ssid_len,
CFG_RX_MGMT_EN |
CFG_RX_BCN_EN);
cmd->params.scan_options = 0;
+   /*
+* Use high priority scan when not associated to prevent fw issue
+* causing never-ending scans (sometimes 20+ minutes).
+* Note: This bug may be caused by the fw's DTIM handling.
+*/
+   if (is_zero_ether_addr(wl->bssid))
+   cmd->params.scan_options |= WL1251_SCAN_OPT_PRIORITY_HIGH;
cmd->params.num_channels = n_channels;
cmd->params.num_probe_requests = n_probes;
cmd->params.tx_rate = cpu_to_le16(1 << 1); /* 2 Mbps */
diff --git a/drivers/net/wireless/ti/wl1251/cmd.h 
b/drivers/net/wireless/ti/wl1251/cmd.h
index ee4f2b3..126f273 100644
--- a/drivers/net/wireless/ti/wl1251/cmd.h
+++ b/drivers/net/wireless/ti/wl1251/cmd.h
@@ -167,6 +167,11 @@ struct cmd_read_write_memory {
 #define CMDMBOX_HEADER_LEN 4
 #define CMDMBOX_INFO_ELEM_HEADER_LEN 4
 
+#define WL1251_SCAN_OPT_PASSIVE1
+#define WL1251_SCAN_OPT_5GHZ_BAND  2
+#define WL1251_SCAN_OPT_TRIGGERD_SCAN  4
+#define WL1251_SCAN_OPT_PRIORITY_HIGH  8
+
 #define WL1251_SCAN_MIN_DURATION 3
 #define WL1251_SCAN_MAX_DURATION 6
 
diff --git a/drivers/net/wireless/ti/wl1251/main.c 
b/drivers/net/wireless/ti/wl1251/main.c
index 3291ffa..4d89ac8 100644
--- a/drivers/net/wireless/ti/wl1251/main.c
+++ b/drivers/net/wireless/ti/wl1251/main.c
@@ -930,6 +930,7 @@ static int wl1251_op_hw_scan(struct ieee80211_hw *hw,
ret = wl1251_cmd_scan(wl, ssid, ssid_len, req->channels,
  req->n_channels, WL1251_SCAN_NUM_PROBES);
if (ret < 0) {
+   wl1251_debug(DEBUG_SCAN, "scan failed %d", ret);
wl->scanning = false;
goto out_idle;
}
-- 
1.7.9.5

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/