2010-08-17 19:47:20

by Grazvydas Ignotas

[permalink] [raw]
Subject: [PATCH 0/4] wl1271 -> wl1251 port patches

Hi,

I've been looking at wl1271 changelog and found several patches that
seem to be also applicable to wl1251. I'm not that sure about patches
2 and 4, but 1 and 3 should be correct (I've verified 3 with TI driver).
I've been running these on pandora for several days now without problems.

It's a shame wl12xx drivers were completely split, it would look like
they could share fair bit of code. Now wl1251 does not benefit from more
actively developed wl1271 fixes.

There are even more wl1271 patches that could be applicable to wl1251.
Kalle, could you take a look at the list below? I could try porting if
you think any of those make sense.


Adding disconnect command, it seems to be also used by TI driver:
25a7dc6d22adda590be4932ebc772ea35914880c
c7f43e451ba40e66a89d51e63bc21a57824592f2

powersave fail handling:
19ad0715d8d9acc259ef02f83df767df2cf1eafe

not masking interrupts while processing:
4aa05917051b01da037a80c3207b48aee252eed2

connection monitoring control:
6ccbb92ead9379d7de2cc25cd950d15a8d22e0c9
a9af092b524614dd3fc7b52bde7c87f8b82cd2a6

broken ps-poll handling:
90494a90bea010af47547880634e0f1c52824a7d

improved queue stopping:
06f7bc7db79fabe6b2ec16eff0f59e4acc21eb72

probably more..


Grazvydas Ignotas (4):
wl1251: add missing __packed modifier for some structs
wl1251: fix event handling mechanism
wl1251: fix beacon filter table structure
wl1251: wait for join command complete event

drivers/net/wireless/wl12xx/wl1251_acx.h | 8 +++---
drivers/net/wireless/wl12xx/wl1251_boot.c | 2 +-
drivers/net/wireless/wl12xx/wl1251_cmd.h | 6 ++--
drivers/net/wireless/wl12xx/wl1251_event.c | 29 ++++++++++++++++++++++++++++
drivers/net/wireless/wl12xx/wl1251_event.h | 1 +
drivers/net/wireless/wl12xx/wl1251_main.c | 24 ++++++++++------------
6 files changed, 49 insertions(+), 21 deletions(-)



2010-08-17 19:47:21

by Grazvydas Ignotas

[permalink] [raw]
Subject: [PATCH 1/4] wl1251: add missing __packed modifier for some structs

Several acx and cmd structures are missing __packed modifier, add it.
This was noticed while comparing them with corresponding wl1271 code.

Signed-off-by: Grazvydas Ignotas <[email protected]>
---
drivers/net/wireless/wl12xx/wl1251_acx.h | 6 +++---
drivers/net/wireless/wl12xx/wl1251_cmd.h | 6 +++---
2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/net/wireless/wl12xx/wl1251_acx.h b/drivers/net/wireless/wl12xx/wl1251_acx.h
index 842df31..9864124 100644
--- a/drivers/net/wireless/wl12xx/wl1251_acx.h
+++ b/drivers/net/wireless/wl12xx/wl1251_acx.h
@@ -37,7 +37,7 @@ struct acx_header {

/* payload length (not including headers */
u16 len;
-};
+} __packed;

struct acx_error_counter {
struct acx_header header;
@@ -471,7 +471,7 @@ struct acx_conn_monit_params {

u32 synch_fail_thold; /* number of beacons missed */
u32 bss_lose_timeout; /* number of TU's from synch fail */
-};
+} __packed;

enum {
SG_ENABLE = 0,
@@ -1056,7 +1056,7 @@ struct acx_rate_class {
u8 long_retry_limit;
u8 aflags;
u8 reserved;
-};
+} __packed;

struct acx_rate_policy {
struct acx_header header;
diff --git a/drivers/net/wireless/wl12xx/wl1251_cmd.h b/drivers/net/wireless/wl12xx/wl1251_cmd.h
index a9e4991..60d7e52 100644
--- a/drivers/net/wireless/wl12xx/wl1251_cmd.h
+++ b/drivers/net/wireless/wl12xx/wl1251_cmd.h
@@ -111,7 +111,7 @@ struct wl1251_cmd_header {
struct wl1251_command {
struct wl1251_cmd_header header;
u8 parameters[MAX_CMD_PARAMS];
-};
+} __packed;

enum {
CMD_MAILBOX_IDLE = 0,
@@ -164,7 +164,7 @@ struct cmd_read_write_memory {
of this field is the Host in WRITE command or the Wilink in READ
command. */
u8 value[MAX_READ_SIZE];
-};
+} __packed;

#define CMDMBOX_HEADER_LEN 4
#define CMDMBOX_INFO_ELEM_HEADER_LEN 4
@@ -339,7 +339,7 @@ struct wl1251_cmd_trigger_scan_to {
struct wl1251_cmd_header header;

u32 timeout;
-};
+} __packed;

/* HW encryption keys */
#define NUM_ACCESS_CATEGORIES_COPY 4
--
1.5.6.3


2010-08-18 15:12:10

by Kalle Valo

[permalink] [raw]
Subject: Re: [PATCH 1/4] wl1251: add missing __packed modifier for some structs

Grazvydas Ignotas <[email protected]> writes:

> Several acx and cmd structures are missing __packed modifier, add it.
> This was noticed while comparing them with corresponding wl1271 code.

Looks good.

> Signed-off-by: Grazvydas Ignotas <[email protected]>

Acked-by: Kalle Valo <[email protected]>

--
Kalle Valo

2010-08-29 11:15:44

by Kalle Valo

[permalink] [raw]
Subject: Re: [PATCH 0/4] wl1271 -> wl1251 port patches

Grazvydas Ignotas <[email protected]> writes:

> There are even more wl1271 patches that could be applicable to wl1251.
> Kalle, could you take a look at the list below? I could try porting if
> you think any of those make sense.
>
> Adding disconnect command, it seems to be also used by TI driver:
> 25a7dc6d22adda590be4932ebc772ea35914880c
> c7f43e451ba40e66a89d51e63bc21a57824592f2

This is good to do also in wl1251.

> powersave fail handling:
> 19ad0715d8d9acc259ef02f83df767df2cf1eafe

I don't like the fact that driver handles the retries, I prefer
mac80211 doing all that. But we can do it in the driver first and
later on add support to mac80211.

> not masking interrupts while processing:
> 4aa05917051b01da037a80c3207b48aee252eed2

Looks good to me.

> connection monitoring control:
> 6ccbb92ead9379d7de2cc25cd950d15a8d22e0c9
> a9af092b524614dd3fc7b52bde7c87f8b82cd2a6

AFAIK wl1251 doesn't support this.

> broken ps-poll handling:
> 90494a90bea010af47547880634e0f1c52824a7d

Again I would not like the driver to handle this, too much logic in
the driver for my taste. But maybe again as a first step to do this in
the driver and later on improve mac80211.

> improved queue stopping:
> 06f7bc7db79fabe6b2ec16eff0f59e4acc21eb72

This is important. Denis Carikli already sent a patch for this, but I
haven't tested it yet.

--
Kalle Valo

2010-08-18 15:13:05

by Kalle Valo

[permalink] [raw]
Subject: Re: [PATCH 2/4] wl1251: fix event handling mechanism

Grazvydas Ignotas <[email protected]> writes:

> In case both A and B events occured simultaneously, current code
> would only process A and clear both interrupts. Make it process both
> events instead.
>
> Based on wl1271 patches by Juuso Oikarinen:
> 1fd2794f36913992798184c464fe8f85753b13e0
> 13f2dc52c69bcca074cd12d4806953b2af45c386
>
> Signed-off-by: Grazvydas Ignotas <[email protected]>

Acked-by: Kalle Valo <[email protected]>

--
Kalle Valo

2010-08-18 15:15:59

by Kalle Valo

[permalink] [raw]
Subject: Re: [PATCH 3/4] wl1251: fix beacon filter table structure

Grazvydas Ignotas <[email protected]> writes:

> The beacon filter table configuration ACX structure had certain elements
> reversed, fix it to match TI driver.
>
> Based on wl1271 patch 1937e742639c03a6fe77239c3003ce9602302117 by
> Juuso Oikarinen.

Yes, the padding doesn't make any sense otherwise.

> Signed-off-by: Grazvydas Ignotas <[email protected]>

Acked-by: Kalle Valo <[email protected]>

--
Kalle Valo

2010-08-17 19:47:25

by Grazvydas Ignotas

[permalink] [raw]
Subject: [PATCH 4/4] wl1251: wait for join command complete event

Poll for join command completion instead of waiting blindly for 10
msecs. There is a timeout of 100 msecs, if the command doesn't complete
by then, we return an error code.

Based on wl1271 patch 99d84c1de8fdf5f9b09f07fdbc628857a040bf8b
by Luciano Coelho.

Signed-off-by: Grazvydas Ignotas <[email protected]>
---
drivers/net/wireless/wl12xx/wl1251_boot.c | 2 +-
drivers/net/wireless/wl12xx/wl1251_event.c | 29 ++++++++++++++++++++++++++++
drivers/net/wireless/wl12xx/wl1251_event.h | 1 +
drivers/net/wireless/wl12xx/wl1251_main.c | 8 ++----
4 files changed, 34 insertions(+), 6 deletions(-)

diff --git a/drivers/net/wireless/wl12xx/wl1251_boot.c b/drivers/net/wireless/wl12xx/wl1251_boot.c
index 65e0416..5e65f47 100644
--- a/drivers/net/wireless/wl12xx/wl1251_boot.c
+++ b/drivers/net/wireless/wl12xx/wl1251_boot.c
@@ -302,7 +302,7 @@ int wl1251_boot_run_firmware(struct wl1251 *wl)
ROAMING_TRIGGER_LOW_RSSI_EVENT_ID |
ROAMING_TRIGGER_REGAINED_RSSI_EVENT_ID |
REGAINED_BSS_EVENT_ID | BT_PTA_SENSE_EVENT_ID |
- BT_PTA_PREDICTION_EVENT_ID;
+ BT_PTA_PREDICTION_EVENT_ID | JOIN_EVENT_COMPLETE_ID;

ret = wl1251_event_unmask(wl);
if (ret < 0) {
diff --git a/drivers/net/wireless/wl12xx/wl1251_event.c b/drivers/net/wireless/wl12xx/wl1251_event.c
index 020d764..e093a1c 100644
--- a/drivers/net/wireless/wl12xx/wl1251_event.c
+++ b/drivers/net/wireless/wl12xx/wl1251_event.c
@@ -97,6 +97,35 @@ static int wl1251_event_process(struct wl1251 *wl, struct event_mailbox *mbox)
return 0;
}

+/*
+ * Poll the mailbox event field until any of the bits in the mask is set or a
+ * timeout occurs (WL1251_EVENT_TIMEOUT in msecs)
+ */
+int wl1251_event_wait(struct wl1251 *wl, u32 mask, int timeout_ms)
+{
+ u32 events_vector, event;
+ unsigned long timeout;
+
+ timeout = jiffies + msecs_to_jiffies(timeout_ms);
+
+ do {
+ if (time_after(jiffies, timeout))
+ return -ETIMEDOUT;
+
+ msleep(1);
+
+ /* read from both event fields */
+ wl1251_mem_read(wl, wl->mbox_ptr[0], &events_vector,
+ sizeof(events_vector));
+ event = events_vector & mask;
+ wl1251_mem_read(wl, wl->mbox_ptr[1], &events_vector,
+ sizeof(events_vector));
+ event |= events_vector & mask;
+ } while (!event);
+
+ return 0;
+}
+
int wl1251_event_unmask(struct wl1251 *wl)
{
int ret;
diff --git a/drivers/net/wireless/wl12xx/wl1251_event.h b/drivers/net/wireless/wl12xx/wl1251_event.h
index f48a2b6..ec45647 100644
--- a/drivers/net/wireless/wl12xx/wl1251_event.h
+++ b/drivers/net/wireless/wl12xx/wl1251_event.h
@@ -117,5 +117,6 @@ struct event_mailbox {
int wl1251_event_unmask(struct wl1251 *wl);
void wl1251_event_mbox_config(struct wl1251 *wl);
int wl1251_event_handle(struct wl1251 *wl, u8 mbox);
+int wl1251_event_wait(struct wl1251 *wl, u32 mask, int timeout_ms);

#endif
diff --git a/drivers/net/wireless/wl12xx/wl1251_main.c b/drivers/net/wireless/wl12xx/wl1251_main.c
index 51474b6..c81e95b 100644
--- a/drivers/net/wireless/wl12xx/wl1251_main.c
+++ b/drivers/net/wireless/wl12xx/wl1251_main.c
@@ -339,11 +339,9 @@ static int wl1251_join(struct wl1251 *wl, u8 bss_type, u8 channel,
if (ret < 0)
goto out;

- /*
- * FIXME: we should wait for JOIN_EVENT_COMPLETE_ID but to simplify
- * locking we just sleep instead, for now
- */
- msleep(10);
+ ret = wl1251_event_wait(wl, JOIN_EVENT_COMPLETE_ID, 100);
+ if (ret < 0)
+ wl1251_warning("join timeout");

out:
return ret;
--
1.5.6.3


2010-08-18 15:49:01

by Kalle Valo

[permalink] [raw]
Subject: Re: [PATCH 0/4] wl1271 -> wl1251 port patches

Grazvydas Ignotas <[email protected]> writes:

> Hi,

Hi Grazvydas,

> I've been looking at wl1271 changelog and found several patches that
> seem to be also applicable to wl1251. I'm not that sure about patches
> 2 and 4, but 1 and 3 should be correct (I've verified 3 with TI driver).
> I've been running these on pandora for several days now without problems.

Patches looked good, thank you for them.

> It's a shame wl12xx drivers were completely split, it would look like
> they could share fair bit of code. Now wl1251 does not benefit from more
> actively developed wl1271 fixes.

We tried that first. But there were so many subtle differences
everywhere that it became very difficult to handle and we decided to
split the driver. Of course that also has it's problems like you
mentioned. A stable firmware API would have solved this, but as usual
when dealing with hardware, it's just a pipe dream.

> There are even more wl1271 patches that could be applicable to wl1251.
> Kalle, could you take a look at the list below? I could try porting if
> you think any of those make sense.

Sorry, I can't do it right now but I should have time for it later
this week.

--
Kalle Valo

2010-08-17 19:47:24

by Grazvydas Ignotas

[permalink] [raw]
Subject: [PATCH 3/4] wl1251: fix beacon filter table structure

The beacon filter table configuration ACX structure had certain elements
reversed, fix it to match TI driver.

Based on wl1271 patch 1937e742639c03a6fe77239c3003ce9602302117 by
Juuso Oikarinen.

Signed-off-by: Grazvydas Ignotas <[email protected]>
---
drivers/net/wireless/wl12xx/wl1251_acx.h | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/net/wireless/wl12xx/wl1251_acx.h b/drivers/net/wireless/wl12xx/wl1251_acx.h
index 9864124..a8845b8 100644
--- a/drivers/net/wireless/wl12xx/wl1251_acx.h
+++ b/drivers/net/wireless/wl12xx/wl1251_acx.h
@@ -459,8 +459,8 @@ struct acx_beacon_filter_ie_table {
struct acx_header header;

u8 num_ie;
- u8 table[BEACON_FILTER_TABLE_MAX_SIZE];
u8 pad[3];
+ u8 table[BEACON_FILTER_TABLE_MAX_SIZE];
} __packed;

#define SYNCH_FAIL_DEFAULT_THRESHOLD 10 /* number of beacons */
--
1.5.6.3


2010-08-17 19:47:23

by Grazvydas Ignotas

[permalink] [raw]
Subject: [PATCH 2/4] wl1251: fix event handling mechanism

In case both A and B events occured simultaneously, current code
would only process A and clear both interrupts. Make it process both
events instead.

Based on wl1271 patches by Juuso Oikarinen:
1fd2794f36913992798184c464fe8f85753b13e0
13f2dc52c69bcca074cd12d4806953b2af45c386

Signed-off-by: Grazvydas Ignotas <[email protected]>
---
drivers/net/wireless/wl12xx/wl1251_main.c | 16 ++++++++--------
1 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/net/wireless/wl12xx/wl1251_main.c b/drivers/net/wireless/wl12xx/wl1251_main.c
index 6d31c85..51474b6 100644
--- a/drivers/net/wireless/wl12xx/wl1251_main.c
+++ b/drivers/net/wireless/wl12xx/wl1251_main.c
@@ -293,14 +293,14 @@ static void wl1251_irq_work(struct work_struct *work)
wl1251_tx_complete(wl);
}

- if (intr & (WL1251_ACX_INTR_EVENT_A |
- WL1251_ACX_INTR_EVENT_B)) {
- wl1251_debug(DEBUG_IRQ, "WL1251_ACX_INTR_EVENT (0x%x)",
- intr);
- if (intr & WL1251_ACX_INTR_EVENT_A)
- wl1251_event_handle(wl, 0);
- else
- wl1251_event_handle(wl, 1);
+ if (intr & WL1251_ACX_INTR_EVENT_A) {
+ wl1251_debug(DEBUG_IRQ, "WL1251_ACX_INTR_EVENT_A");
+ wl1251_event_handle(wl, 0);
+ }
+
+ if (intr & WL1251_ACX_INTR_EVENT_B) {
+ wl1251_debug(DEBUG_IRQ, "WL1251_ACX_INTR_EVENT_B");
+ wl1251_event_handle(wl, 1);
}

if (intr & WL1251_ACX_INTR_INIT_COMPLETE)
--
1.5.6.3


2010-08-18 15:18:39

by Kalle Valo

[permalink] [raw]
Subject: Re: [PATCH 4/4] wl1251: wait for join command complete event

Grazvydas Ignotas <[email protected]> writes:

> Poll for join command completion instead of waiting blindly for 10
> msecs. There is a timeout of 100 msecs, if the command doesn't complete
> by then, we return an error code.
>
> Based on wl1271 patch 99d84c1de8fdf5f9b09f07fdbc628857a040bf8b
> by Luciano Coelho.
>
> Signed-off-by: Grazvydas Ignotas <[email protected]>

Acked-by: Kalle Valo <[email protected]>

--
Kalle Valo