[PATCH 0/4] wl1271 -> wl1251 port patches

[PATCH 0/4] wl1271 -> wl1251 port patches

[Grazvydas Ignotas]

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(-)

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

[Grazvydas Ignotas]

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 <notasas@gmail.com>
---
 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

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

[Grazvydas Ignotas]

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 <notasas@gmail.com>
---
 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

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

[Grazvydas Ignotas]

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 <notasas@gmail.com>
---
 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

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

[Grazvydas Ignotas]

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 <notasas@gmail.com>
---
 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

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

[Kalle Valo]

Grazvydas Ignotas <notasas@gmail.com> 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 <notasas@gmail.com>

Acked-by: Kalle Valo <kvalo@adurom.com>

-- 
Kalle Valo

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

[Kalle Valo]

Grazvydas Ignotas <notasas@gmail.com> 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 <notasas@gmail.com>

Acked-by: Kalle Valo <kvalo@adurom.com>

-- 
Kalle Valo

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

[Kalle Valo]

Grazvydas Ignotas <notasas@gmail.com> 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 <notasas@gmail.com>

Acked-by: Kalle Valo <kvalo@adurom.com>

-- 
Kalle Valo

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

[Kalle Valo]

Grazvydas Ignotas <notasas@gmail.com> 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 <notasas@gmail.com>

Acked-by: Kalle Valo <kvalo@adurom.com>

-- 
Kalle Valo

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

[Kalle Valo]

Grazvydas Ignotas <notasas@gmail.com> 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

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

[Kalle Valo]

Grazvydas Ignotas <notasas@gmail.com> 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