* [patch 2.6.35 00/25] WiMAX pull request
@ 2010-05-14 21:44 Inaky Perez-Gonzalez
2010-05-14 21:45 ` [patch 2.6.35 01/25] wimax/i2400m: fix incorrect return -ESHUTDOWN when there is no Tx buffer available Inaky Perez-Gonzalez
` (25 more replies)
0 siblings, 26 replies; 27+ messages in thread
From: Inaky Perez-Gonzalez @ 2010-05-14 21:44 UTC (permalink / raw)
To: netdev, wimax
From: Inaky Perez-Gonzalez <inaky.perez-gonzalez@intel.com>
There are no new features driver wise -- this is all a set of bug
fixes, mostly small ones, some bigger:
- TX FIFO handling had a few more corner cases Q&A found
fix incorrect return -ESHUTDOWN when there is no Tx buffer available
fix the race condition for accessing TX queue
add the error recovery mechanism on TX path
Reset the TX FIFO indices when allocating the TX FIFO in tx_setup()
increase the maximum number of payloads per message to 60 [v1]
limit the message size upto 16KiB [v1]
modify i2400m_tx_fifo_push() to check for head room space in the TX FIFO [v1]
fix system freeze caused by an infinite loop [v1]
reserve additional space in the TX queue's buffer while allocating space for a new message header
- RX chain
fix incorrect handling of type 2 and 3 RX messages
fix race condition while accessing rx_roq by using kref count
- SDIO reset handling: work out quirks
- Small rearrangements to reduce forward declarations
NOTE: the WiMAX tree in
git://git.kernel.org/pub/scm/linux/kernel/git/inaky/wimax.git has been
rebased, so if you were tracking it, you will need to do a hard
pull. Sorry for that.
The reason this has been done is that wimax.git/master will now
stable, no cherry picking and always chasing net-next-2.6 /
net-2.6. The old wimax/master tree was thus unfeasible for upstream
pull as it was quite contaminated with extraenous commits.
When net-2.6 opens, wimax/2.6.X opens chasing net-2.6, later it will
chase linux-allstable-2.6/linux-2.6.X.y if need be when the kernel
release happens.
The following changes since commit 2b0b05ddc04b6d45e71cd36405df512075786f1e:
Marcel Holtmann (1):
Bluetooth: Fix issues where sk_sleep() helper is needed now
are available in the git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/inaky/wimax.git master
Patches follow for reviewing convenience.
Cindy H Kao (6):
wimax/i2400m: fix the race condition for accessing TX queue
wimax/i2400m: correct the error path handlers in dev_start()
wimax/i2400m: fix for missed reset events if triggered by dev_reset_handle()
wimax/i2400m: add the error recovery mechanism on TX path
wimax/i2400m: Correct the error path handlers order in i2400m_post_reset()
wimax/i2400m: Reset the TX FIFO indices when allocating the TX FIFO in tx_setup()
Dan Carpenter (2):
wimax: checking ERR_PTR vs null
wimax: wimax_msg_alloc() returns ERR_PTR not null
Inaky Perez-Gonzalez (2):
wimax/i2400m: driver defaults to firmware v1.5 for i5x50 devices
wimax/i2400m: driver defaults to firmware v1.5 for i6x60 devices
Prasanna S Panchamukhi (1):
wimax/i2400m: Move module params to other file so they can be static
Prasanna S. Panchamukhi (13):
wimax/i2400m: move I2400M_MAX_MTU enum from netdev.c to i2400m.h
wimax/i2400m: fix insufficient size of Tx buffer for 12 payload of 1400 MTU.
wimax/i2400m: increase the maximum number of payloads per message to 60 [v1]
wimax/i2400m: limit the message size upto 16KiB [v1]
wimax/i2400m: fix BUILD_BUG_ON() to use the maximum message size constant [v1]
wimax/i2400m: modify i2400m_tx_fifo_push() to check for head room space in the TX FIFO [v1]
wimax/i2400m: fix system freeze caused by an infinite loop [v1]
wimax/i2400m: increase tx queue length from 5 to 20 [v1]
wimax i2400m: fix race condition while accessing rx_roq by using kref count
wimax/i2400m: fix incorrect handling of type 2 and 3 RX messages
wimax/i2400m: reserve additional space in the TX queue's buffer while allocating space for a new message header
wimax/i2400m: SDIO specific TX queue's minimum buffer room for new message
wimax/i2400m: USB specific TX queue's minimum buffer room required for new message
Prasanna S.Panchamukhi (1):
wimax/i2400m: fix incorrect return -ESHUTDOWN when there is no Tx buffer available
drivers/net/wimax/i2400m/control.c | 15 +++
drivers/net/wimax/i2400m/driver.c | 165 +++++++++++++++++++++++++-------
drivers/net/wimax/i2400m/i2400m-sdio.h | 5 +-
drivers/net/wimax/i2400m/i2400m.h | 82 ++++++++++++++--
drivers/net/wimax/i2400m/netdev.c | 12 +--
drivers/net/wimax/i2400m/rx.c | 109 ++++++++++++++-------
drivers/net/wimax/i2400m/sdio-tx.c | 35 ++++++--
drivers/net/wimax/i2400m/sdio.c | 7 ++
drivers/net/wimax/i2400m/tx.c | 153 ++++++++++++++++++++++++++----
drivers/net/wimax/i2400m/usb.c | 12 ++-
net/wimax/stack.c | 2 +-
11 files changed, 480 insertions(+), 117 deletions(-)
^ permalink raw reply [flat|nested] 27+ messages in thread
* [patch 2.6.35 01/25] wimax/i2400m: fix incorrect return -ESHUTDOWN when there is no Tx buffer available
2010-05-14 21:44 [patch 2.6.35 00/25] WiMAX pull request Inaky Perez-Gonzalez
@ 2010-05-14 21:45 ` Inaky Perez-Gonzalez
2010-05-14 21:45 ` [patch 2.6.35 02/25] wimax/i2400m: move I2400M_MAX_MTU enum from netdev.c to i2400m.h Inaky Perez-Gonzalez
` (24 subsequent siblings)
25 siblings, 0 replies; 27+ messages in thread
From: Inaky Perez-Gonzalez @ 2010-05-14 21:45 UTC (permalink / raw)
To: netdev, wimax; +Cc: Prasanna S.Panchamukhi
From: Prasanna S.Panchamukhi <prasannax.s.panchamukhi@intel.com>
i2400m_tx() routine was returning -ESHUTDOWN even when there was no Tx buffer
available. This patch fixes the i2400m_tx() to return -ESHUTDOWN only when
the device is down(i2400m->tx_buf is NULL) and also to return -ENOSPC
when there is no Tx buffer. Error seen in the kernel log.
kernel: i2400m_sdio mmc0:0001:1: can't send message 0x5606: -108
kernel: i2400m_sdio mmc0:0001:1: Failed to issue 'Enter power save'command: -108
Signed-off-by: Prasanna S.Panchamukhi <prasannax.s.panchamukhi@intel.com>
---
drivers/net/wimax/i2400m/tx.c | 6 ++++--
1 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/net/wimax/i2400m/tx.c b/drivers/net/wimax/i2400m/tx.c
index 6db909e..fab27e4 100644
--- a/drivers/net/wimax/i2400m/tx.c
+++ b/drivers/net/wimax/i2400m/tx.c
@@ -643,9 +643,11 @@ int i2400m_tx(struct i2400m *i2400m, const void *buf, size_t buf_len,
* current one is out of payload slots or we have a singleton,
* close it and start a new one */
spin_lock_irqsave(&i2400m->tx_lock, flags);
- result = -ESHUTDOWN;
- if (i2400m->tx_buf == NULL)
+ /* If tx_buf is NULL, device is shutdown */
+ if (i2400m->tx_buf == NULL) {
+ result = -ESHUTDOWN;
goto error_tx_new;
+ }
try_new:
if (unlikely(i2400m->tx_msg == NULL))
i2400m_tx_new(i2400m);
--
1.6.6.1
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [patch 2.6.35 02/25] wimax/i2400m: move I2400M_MAX_MTU enum from netdev.c to i2400m.h
2010-05-14 21:44 [patch 2.6.35 00/25] WiMAX pull request Inaky Perez-Gonzalez
2010-05-14 21:45 ` [patch 2.6.35 01/25] wimax/i2400m: fix incorrect return -ESHUTDOWN when there is no Tx buffer available Inaky Perez-Gonzalez
@ 2010-05-14 21:45 ` Inaky Perez-Gonzalez
2010-05-14 21:45 ` [patch 2.6.35 03/25] wimax/i2400m: fix insufficient size of Tx buffer for 12 payload of 1400 MTU Inaky Perez-Gonzalez
` (23 subsequent siblings)
25 siblings, 0 replies; 27+ messages in thread
From: Inaky Perez-Gonzalez @ 2010-05-14 21:45 UTC (permalink / raw)
To: netdev, wimax; +Cc: Prasanna S. Panchamukhi
From: Prasanna S. Panchamukhi <prasannax.s.panchamukhi@intel.com>
This patch moves I2400M_MAX_MTU enum defined in netdev.c to i2400m.h.
Follow up changes will make use of this value in other location,
thus requiring it to be moved to a global header file i2400m.h.
Signed-off-by: Prasanna S. Panchamukhi <prasannax.s.panchamukhi@intel.com>
Signed-off-by: Inaky Perez-Gonzalez <inaky@linux.intel.com>
---
drivers/net/wimax/i2400m/i2400m.h | 10 ++++++++++
drivers/net/wimax/i2400m/netdev.c | 6 ------
2 files changed, 10 insertions(+), 6 deletions(-)
diff --git a/drivers/net/wimax/i2400m/i2400m.h b/drivers/net/wimax/i2400m/i2400m.h
index 820b128..da218b9 100644
--- a/drivers/net/wimax/i2400m/i2400m.h
+++ b/drivers/net/wimax/i2400m/i2400m.h
@@ -160,6 +160,16 @@
#include <linux/wimax/i2400m.h>
#include <asm/byteorder.h>
+enum {
+/* netdev interface */
+ /*
+ * Out of NWG spec (R1_v1.2.2), 3.3.3 ASN Bearer Plane MTU Size
+ *
+ * The MTU is 1400 or less
+ */
+ I2400M_MAX_MTU = 1400,
+};
+
/* Misc constants */
enum {
/* Size of the Boot Mode Command buffer */
diff --git a/drivers/net/wimax/i2400m/netdev.c b/drivers/net/wimax/i2400m/netdev.c
index b811c2f..fc3754a 100644
--- a/drivers/net/wimax/i2400m/netdev.c
+++ b/drivers/net/wimax/i2400m/netdev.c
@@ -84,12 +84,6 @@
enum {
/* netdev interface */
- /*
- * Out of NWG spec (R1_v1.2.2), 3.3.3 ASN Bearer Plane MTU Size
- *
- * The MTU is 1400 or less
- */
- I2400M_MAX_MTU = 1400,
/* 20 secs? yep, this is the maximum timeout that the device
* might take to get out of IDLE / negotiate it with the base
* station. We add 1sec for good measure. */
--
1.6.6.1
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [patch 2.6.35 03/25] wimax/i2400m: fix insufficient size of Tx buffer for 12 payload of 1400 MTU.
2010-05-14 21:44 [patch 2.6.35 00/25] WiMAX pull request Inaky Perez-Gonzalez
2010-05-14 21:45 ` [patch 2.6.35 01/25] wimax/i2400m: fix incorrect return -ESHUTDOWN when there is no Tx buffer available Inaky Perez-Gonzalez
2010-05-14 21:45 ` [patch 2.6.35 02/25] wimax/i2400m: move I2400M_MAX_MTU enum from netdev.c to i2400m.h Inaky Perez-Gonzalez
@ 2010-05-14 21:45 ` Inaky Perez-Gonzalez
2010-05-14 21:45 ` [patch 2.6.35 04/25] wimax/i2400m: fix the race condition for accessing TX queue Inaky Perez-Gonzalez
` (22 subsequent siblings)
25 siblings, 0 replies; 27+ messages in thread
From: Inaky Perez-Gonzalez @ 2010-05-14 21:45 UTC (permalink / raw)
To: netdev, wimax; +Cc: Prasanna S. Panchamukhi
From: Prasanna S. Panchamukhi <prasannax.s.panchamukhi@intel.com>
This patch increases the Tx buffer size so as to accommodate 12 payloads
of 1408 (1400 MTU 16 bytes aligned). Currently Tx buffer is 32 KiB which
is insufficient to accommodate 12 payloads of 1408 size.
This patch
- increases I2400M_TX_BUF_SIZE from 32KiB to 64KiB
- Adds a BUILD_BUG_ON if the calculated buffer size based
on the given MTU exceeds the I2400M_TX_BUF_SIZE.
Below is how we calculate the size of the Tx buffer.
Payload + 4 bytes prefix for each payload (1400 MTU 16 bytes boundary aligned)
= (1408 + sizeof(struct i2400m_pl_data_hdr)) * I2400M_TX_PLD_MAX
Adding 16 byte message header = + sizeof(struct i2400m_msg_hdr)
Aligning to 256 byte boundary
Total Tx buffer = (((((1408 + sizeof(struct i2400m_pl_data_hdr))
* I2400M_TX_PLD_MAX )+ sizeof(struct i2400m_msg_hdr))
/ 256) + 1) * 256 * 2
Signed-off-by: Prasanna S. Panchamukhi <prasannax.s.panchamukhi@intel.com>
Signed-off-by: Inaky Perez-Gonzalez <inaky@linux.intel.com>
---
drivers/net/wimax/i2400m/tx.c | 19 ++++++++++++++++++-
1 files changed, 18 insertions(+), 1 deletions(-)
diff --git a/drivers/net/wimax/i2400m/tx.c b/drivers/net/wimax/i2400m/tx.c
index fab27e4..8561c07 100644
--- a/drivers/net/wimax/i2400m/tx.c
+++ b/drivers/net/wimax/i2400m/tx.c
@@ -258,8 +258,10 @@ enum {
* Doc says maximum transaction is 16KiB. If we had 16KiB en
* route and 16KiB being queued, it boils down to needing
* 32KiB.
+ * 32KiB is insufficient for 1400 MTU, hence increasing
+ * tx buffer size to 64KiB.
*/
- I2400M_TX_BUF_SIZE = 32768,
+ I2400M_TX_BUF_SIZE = 65536,
/**
* Message header and payload descriptors have to be 16
* aligned (16 + 4 * N = 16 * M). If we take that average sent
@@ -274,6 +276,19 @@ enum {
I2400M_TX_PLD_SIZE = sizeof(struct i2400m_msg_hdr)
+ I2400M_TX_PLD_MAX * sizeof(struct i2400m_pld),
I2400M_TX_SKIP = 0x80000000,
+ /*
+ * 16 byte aligned MAX_MTU + 4 byte payload prefix.
+ */
+ I2400M_MAX_MTU_ALIGN = 16,
+ I2400M_TX_PDU_SIZE = I2400M_MAX_MTU % I2400M_MAX_MTU_ALIGN
+ + I2400M_MAX_MTU + sizeof(struct i2400m_pl_data_hdr),
+ /*
+ * 256 byte aligned toal size of 12 PDUs including msg header,
+ */
+ I2400M_TX_PDU_ALIGN = 256,
+ I2400M_TX_PDU_TOTAL_SIZE = ((I2400M_TX_PDU_SIZE * I2400M_TX_PLD_MAX
+ + sizeof(struct i2400m_msg_hdr))/I2400M_TX_PDU_ALIGN + 1)
+ * I2400M_TX_PDU_ALIGN * 2,
};
#define TAIL_FULL ((void *)~(unsigned long)NULL)
@@ -874,6 +889,8 @@ int i2400m_tx_setup(struct i2400m *i2400m)
INIT_WORK(&i2400m->wake_tx_ws, i2400m_wake_tx_work);
i2400m->tx_sequence = 0;
+ /* Warn if the calculated buffer size exceeds I2400M_TX_BUF_SIZE. */
+ BUILD_BUG_ON(I2400M_TX_PDU_TOTAL_SIZE > I2400M_TX_BUF_SIZE);
i2400m->tx_buf = kmalloc(I2400M_TX_BUF_SIZE, GFP_KERNEL);
if (i2400m->tx_buf == NULL)
result = -ENOMEM;
--
1.6.6.1
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [patch 2.6.35 04/25] wimax/i2400m: fix the race condition for accessing TX queue
2010-05-14 21:44 [patch 2.6.35 00/25] WiMAX pull request Inaky Perez-Gonzalez
` (2 preceding siblings ...)
2010-05-14 21:45 ` [patch 2.6.35 03/25] wimax/i2400m: fix insufficient size of Tx buffer for 12 payload of 1400 MTU Inaky Perez-Gonzalez
@ 2010-05-14 21:45 ` Inaky Perez-Gonzalez
2010-05-14 21:45 ` [patch 2.6.35 05/25] wimax/i2400m: correct the error path handlers in dev_start() Inaky Perez-Gonzalez
` (21 subsequent siblings)
25 siblings, 0 replies; 27+ messages in thread
From: Inaky Perez-Gonzalez @ 2010-05-14 21:45 UTC (permalink / raw)
To: netdev, wimax; +Cc: Cindy H Kao
From: Cindy H Kao <cindy.h.kao@intel.com>
The race condition happens when the TX queue is accessed by
the TX work while the same TX queue is being destroyed because
a bus reset is triggered either by debugfs entry or simply
by failing waking up the device from WiMAX IDLE mode.
This fix is to prevent the TX queue from being accessed by
multiple threads
Signed-off-by: Cindy H Kao <cindy.h.kao@intel.com>
---
drivers/net/wimax/i2400m/i2400m-sdio.h | 5 ++++-
drivers/net/wimax/i2400m/sdio-tx.c | 31 ++++++++++++++++++++++++-------
2 files changed, 28 insertions(+), 8 deletions(-)
diff --git a/drivers/net/wimax/i2400m/i2400m-sdio.h b/drivers/net/wimax/i2400m/i2400m-sdio.h
index b9c4bed..360d4fb 100644
--- a/drivers/net/wimax/i2400m/i2400m-sdio.h
+++ b/drivers/net/wimax/i2400m/i2400m-sdio.h
@@ -99,7 +99,10 @@ enum {
*
* @tx_workqueue: workqeueue used for data TX; we don't use the
* system's workqueue as that might cause deadlocks with code in
- * the bus-generic driver.
+ * the bus-generic driver. The read/write operation to the queue
+ * is protected with spinlock (tx_lock in struct i2400m) to avoid
+ * the queue being destroyed in the middle of a the queue read/write
+ * operation.
*
* @debugfs_dentry: dentry for the SDIO specific debugfs files
*
diff --git a/drivers/net/wimax/i2400m/sdio-tx.c b/drivers/net/wimax/i2400m/sdio-tx.c
index de66d06..412b6a8 100644
--- a/drivers/net/wimax/i2400m/sdio-tx.c
+++ b/drivers/net/wimax/i2400m/sdio-tx.c
@@ -114,13 +114,17 @@ void i2400ms_bus_tx_kick(struct i2400m *i2400m)
{
struct i2400ms *i2400ms = container_of(i2400m, struct i2400ms, i2400m);
struct device *dev = &i2400ms->func->dev;
+ unsigned long flags;
d_fnstart(3, dev, "(i2400m %p) = void\n", i2400m);
/* schedule tx work, this is because tx may block, therefore
* it has to run in a thread context.
*/
- queue_work(i2400ms->tx_workqueue, &i2400ms->tx_worker);
+ spin_lock_irqsave(&i2400m->tx_lock, flags);
+ if (i2400ms->tx_workqueue != NULL)
+ queue_work(i2400ms->tx_workqueue, &i2400ms->tx_worker);
+ spin_unlock_irqrestore(&i2400m->tx_lock, flags);
d_fnend(3, dev, "(i2400m %p) = void\n", i2400m);
}
@@ -130,27 +134,40 @@ int i2400ms_tx_setup(struct i2400ms *i2400ms)
int result;
struct device *dev = &i2400ms->func->dev;
struct i2400m *i2400m = &i2400ms->i2400m;
+ struct workqueue_struct *tx_workqueue;
+ unsigned long flags;
d_fnstart(5, dev, "(i2400ms %p)\n", i2400ms);
INIT_WORK(&i2400ms->tx_worker, i2400ms_tx_submit);
snprintf(i2400ms->tx_wq_name, sizeof(i2400ms->tx_wq_name),
"%s-tx", i2400m->wimax_dev.name);
- i2400ms->tx_workqueue =
+ tx_workqueue =
create_singlethread_workqueue(i2400ms->tx_wq_name);
- if (NULL == i2400ms->tx_workqueue) {
+ if (tx_workqueue == NULL) {
dev_err(dev, "TX: failed to create workqueue\n");
result = -ENOMEM;
} else
result = 0;
+ spin_lock_irqsave(&i2400m->tx_lock, flags);
+ i2400ms->tx_workqueue = tx_workqueue;
+ spin_unlock_irqrestore(&i2400m->tx_lock, flags);
d_fnend(5, dev, "(i2400ms %p) = %d\n", i2400ms, result);
return result;
}
void i2400ms_tx_release(struct i2400ms *i2400ms)
{
- if (i2400ms->tx_workqueue) {
- destroy_workqueue(i2400ms->tx_workqueue);
- i2400ms->tx_workqueue = NULL;
- }
+ struct i2400m *i2400m = &i2400ms->i2400m;
+ struct workqueue_struct *tx_workqueue;
+ unsigned long flags;
+
+ tx_workqueue = i2400ms->tx_workqueue;
+
+ spin_lock_irqsave(&i2400m->tx_lock, flags);
+ i2400ms->tx_workqueue = NULL;
+ spin_unlock_irqrestore(&i2400m->tx_lock, flags);
+
+ if (tx_workqueue)
+ destroy_workqueue(tx_workqueue);
}
--
1.6.6.1
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [patch 2.6.35 05/25] wimax/i2400m: correct the error path handlers in dev_start()
2010-05-14 21:44 [patch 2.6.35 00/25] WiMAX pull request Inaky Perez-Gonzalez
` (3 preceding siblings ...)
2010-05-14 21:45 ` [patch 2.6.35 04/25] wimax/i2400m: fix the race condition for accessing TX queue Inaky Perez-Gonzalez
@ 2010-05-14 21:45 ` Inaky Perez-Gonzalez
2010-05-14 21:45 ` [patch 2.6.35 06/25] wimax/i2400m: fix for missed reset events if triggered by dev_reset_handle() Inaky Perez-Gonzalez
` (20 subsequent siblings)
25 siblings, 0 replies; 27+ messages in thread
From: Inaky Perez-Gonzalez @ 2010-05-14 21:45 UTC (permalink / raw)
To: netdev, wimax; +Cc: Cindy H Kao
From: Cindy H Kao <cindy.h.kao@intel.com>
This fix is to correct order of the handlers in the error path
of dev_start(). When i2400m_firmware_check fails, all the works done
before it should be released or cleared.
Signed-off-by: Cindy H Kao <cindy.h.kao@intel.com>
---
drivers/net/wimax/i2400m/driver.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/drivers/net/wimax/i2400m/driver.c b/drivers/net/wimax/i2400m/driver.c
index 94dc83c..3a6c8dd 100644
--- a/drivers/net/wimax/i2400m/driver.c
+++ b/drivers/net/wimax/i2400m/driver.c
@@ -403,10 +403,10 @@ retry:
error_dev_initialize:
error_check_mac_addr:
+error_fw_check:
i2400m->ready = 0;
wmb(); /* see i2400m->ready's documentation */
flush_workqueue(i2400m->work_queue);
-error_fw_check:
if (i2400m->bus_dev_stop)
i2400m->bus_dev_stop(i2400m);
error_bus_dev_start:
--
1.6.6.1
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [patch 2.6.35 06/25] wimax/i2400m: fix for missed reset events if triggered by dev_reset_handle()
2010-05-14 21:44 [patch 2.6.35 00/25] WiMAX pull request Inaky Perez-Gonzalez
` (4 preceding siblings ...)
2010-05-14 21:45 ` [patch 2.6.35 05/25] wimax/i2400m: correct the error path handlers in dev_start() Inaky Perez-Gonzalez
@ 2010-05-14 21:45 ` Inaky Perez-Gonzalez
2010-05-14 21:45 ` [patch 2.6.35 07/25] wimax/i2400m: add the error recovery mechanism on TX path Inaky Perez-Gonzalez
` (19 subsequent siblings)
25 siblings, 0 replies; 27+ messages in thread
From: Inaky Perez-Gonzalez @ 2010-05-14 21:45 UTC (permalink / raw)
To: netdev, wimax; +Cc: Cindy H Kao
From: Cindy H Kao <cindy.h.kao@intel.com>
The problem is only seen on SDIO interface since on USB, a bus reset would
really re-probe the driver, but on SDIO interface, a bus reset will not
re-enumerate the SDIO bus, so no driver re-probe is happening. Therefore,
on SDIO interface, the reset event should be still detected and handled by
dev_reset_handle().
Problem description:
Whenever a reboot barker is received during operational mode (i2400m->boot_mode == 0),
dev_reset_handle() is invoked to handle that function reset event.
dev_reset_handle() then sets the flag i2400m->boot_mode to 1 indicating the device is
back to bootmode before proceeding to dev_stop() and dev_start().
If dev_start() returns failure, a bus reset is triggered by dev_reset_handle().
The flag i2400m->boot_mode then remains 1 when the second reboot barker arrives.
However the interrupt service routine i2400ms_rx() instead of invoking dev_reset_handle()
to handle that reset event, it filters out that boot event to bootmode because it sees
the flag i2400m->boot_mode equal to 1.
The fix:
Maintain the flag i2400m->boot_mode within dev_reset_handle() and set the flag
i2400m->boot_mode to 1 when entering dev_reset_handle(). It remains 1
until the dev_reset_handle() issues a bus reset. ie: the bus reset is
taking place just like it happens for the first time during operational mode.
To denote the actual device state and the state we expect, a flag i2400m->alive
is introduced in addition to the existing flag i2400m->updown.
It's maintained with the same way for i2400m->updown but instead of reflecting
the actual state like i2400m->updown does, i2400m->alive maintains the state
we expect. i2400m->alive is set 1 just like whenever i2400m->updown is set 1.
Yet i2400m->alive remains 1 since we expect the device to be up all the time
until the driver is removed. See the doc for @alive in i2400m.h.
An enumeration I2400M_BUS_RESET_RETRIES is added to define the maximum number of
bus resets that a device reboot can retry.
A counter i2400m->bus_reset_retries is added to track how many bus resets
have been retried in one device reboot. If I2400M_BUS_RESET_RETRIES bus resets
were retried in this boot, we give up any further retrying so the device would enter
low power state. The counter i2400m->bus_reset_retries is incremented whenever
dev_reset_handle() is issuing a bus reset and is cleared to 0 when dev_start() is
successfully done, ie: a successful reboot.
Signed-off-by: Cindy H Kao <cindy.h.kao@intel.com>
---
drivers/net/wimax/i2400m/driver.c | 68 ++++++++++++++++++++++++++++---------
drivers/net/wimax/i2400m/i2400m.h | 34 ++++++++++++++++++
2 files changed, 86 insertions(+), 16 deletions(-)
diff --git a/drivers/net/wimax/i2400m/driver.c b/drivers/net/wimax/i2400m/driver.c
index 3a6c8dd..1674dba 100644
--- a/drivers/net/wimax/i2400m/driver.c
+++ b/drivers/net/wimax/i2400m/driver.c
@@ -436,7 +436,8 @@ int i2400m_dev_start(struct i2400m *i2400m, enum i2400m_bri bm_flags)
result = __i2400m_dev_start(i2400m, bm_flags);
if (result >= 0) {
i2400m->updown = 1;
- wmb(); /* see i2400m->updown's documentation */
+ i2400m->alive = 1;
+ wmb();/* see i2400m->updown and i2400m->alive's doc */
}
}
mutex_unlock(&i2400m->init_mutex);
@@ -497,7 +498,8 @@ void i2400m_dev_stop(struct i2400m *i2400m)
if (i2400m->updown) {
__i2400m_dev_stop(i2400m);
i2400m->updown = 0;
- wmb(); /* see i2400m->updown's documentation */
+ i2400m->alive = 0;
+ wmb(); /* see i2400m->updown and i2400m->alive's doc */
}
mutex_unlock(&i2400m->init_mutex);
}
@@ -669,6 +671,9 @@ void __i2400m_dev_reset_handle(struct work_struct *ws)
d_fnstart(3, dev, "(ws %p i2400m %p reason %s)\n", ws, i2400m, reason);
+ i2400m->boot_mode = 1;
+ wmb(); /* Make sure i2400m_msg_to_dev() sees boot_mode */
+
result = 0;
if (mutex_trylock(&i2400m->init_mutex) == 0) {
/* We are still in i2400m_dev_start() [let it fail] or
@@ -679,32 +684,62 @@ void __i2400m_dev_reset_handle(struct work_struct *ws)
complete(&i2400m->msg_completion);
goto out;
}
- if (i2400m->updown == 0) {
- dev_info(dev, "%s: device is down, doing nothing\n", reason);
- goto out_unlock;
- }
+
dev_err(dev, "%s: reinitializing driver\n", reason);
- __i2400m_dev_stop(i2400m);
- result = __i2400m_dev_start(i2400m,
- I2400M_BRI_SOFT | I2400M_BRI_MAC_REINIT);
- if (result < 0) {
+ rmb();
+ if (i2400m->updown) {
+ __i2400m_dev_stop(i2400m);
i2400m->updown = 0;
wmb(); /* see i2400m->updown's documentation */
- dev_err(dev, "%s: cannot start the device: %d\n",
- reason, result);
- result = -EUCLEAN;
}
-out_unlock:
+
+ if (i2400m->alive) {
+ result = __i2400m_dev_start(i2400m,
+ I2400M_BRI_SOFT | I2400M_BRI_MAC_REINIT);
+ if (result < 0) {
+ dev_err(dev, "%s: cannot start the device: %d\n",
+ reason, result);
+ result = -EUCLEAN;
+ if (atomic_read(&i2400m->bus_reset_retries)
+ >= I2400M_BUS_RESET_RETRIES) {
+ result = -ENODEV;
+ dev_err(dev, "tried too many times to "
+ "reset the device, giving up\n");
+ }
+ }
+ }
+
if (i2400m->reset_ctx) {
ctx->result = result;
complete(&ctx->completion);
}
mutex_unlock(&i2400m->init_mutex);
if (result == -EUCLEAN) {
+ /*
+ * We come here because the reset during operational mode
+ * wasn't successully done and need to proceed to a bus
+ * reset. For the dev_reset_handle() to be able to handle
+ * the reset event later properly, we restore boot_mode back
+ * to the state before previous reset. ie: just like we are
+ * issuing the bus reset for the first time
+ */
+ i2400m->boot_mode = 0;
+ wmb();
+
+ atomic_inc(&i2400m->bus_reset_retries);
/* ops, need to clean up [w/ init_mutex not held] */
result = i2400m_reset(i2400m, I2400M_RT_BUS);
if (result >= 0)
result = -ENODEV;
+ } else {
+ rmb();
+ if (i2400m->alive) {
+ /* great, we expect the device state up and
+ * dev_start() actually brings the device state up */
+ i2400m->updown = 1;
+ wmb();
+ atomic_set(&i2400m->bus_reset_retries, 0);
+ }
}
out:
i2400m_put(i2400m);
@@ -729,8 +764,6 @@ out:
*/
int i2400m_dev_reset_handle(struct i2400m *i2400m, const char *reason)
{
- i2400m->boot_mode = 1;
- wmb(); /* Make sure i2400m_msg_to_dev() sees boot_mode */
return i2400m_schedule_work(i2400m, __i2400m_dev_reset_handle,
GFP_ATOMIC, &reason, sizeof(reason));
}
@@ -803,6 +836,9 @@ void i2400m_init(struct i2400m *i2400m)
mutex_init(&i2400m->init_mutex);
/* wake_tx_ws is initialized in i2400m_tx_setup() */
+ atomic_set(&i2400m->bus_reset_retries, 0);
+
+ i2400m->alive = 0;
}
EXPORT_SYMBOL_GPL(i2400m_init);
diff --git a/drivers/net/wimax/i2400m/i2400m.h b/drivers/net/wimax/i2400m/i2400m.h
index da218b9..ad8e6a3 100644
--- a/drivers/net/wimax/i2400m/i2400m.h
+++ b/drivers/net/wimax/i2400m/i2400m.h
@@ -177,6 +177,11 @@ enum {
I2400M_BM_ACK_BUF_SIZE = 256,
};
+enum {
+ /* Maximum number of bus reset can be retried */
+ I2400M_BUS_RESET_RETRIES = 3,
+};
+
/**
* struct i2400m_poke_table - Hardware poke table for the Intel 2400m
*
@@ -517,6 +522,29 @@ struct i2400m_barker_db;
* same.
*
* @pm_notifier: used to register for PM events
+ *
+ * @bus_reset_retries: counter for the number of bus resets attempted for
+ * this boot. It's not for tracking the number of bus resets during
+ * the whole driver life cycle (from insmod to rmmod) but for the
+ * number of dev_start() executed until dev_start() returns a success
+ * (ie: a good boot means a dev_stop() followed by a successful
+ * dev_start()). dev_reset_handler() increments this counter whenever
+ * it is triggering a bus reset. It checks this counter to decide if a
+ * subsequent bus reset should be retried. dev_reset_handler() retries
+ * the bus reset until dev_start() succeeds or the counter reaches
+ * I2400M_BUS_RESET_RETRIES. The counter is cleared to 0 in
+ * dev_reset_handle() when dev_start() returns a success,
+ * ie: a successul boot is completed.
+ *
+ * @alive: flag to denote if the device *should* be alive. This flag is
+ * everything like @updown (see doc for @updown) except reflecting
+ * the device state *we expect* rather than the actual state as denoted
+ * by @updown. It is set 1 whenever @updown is set 1 in dev_start().
+ * Then the device is expected to be alive all the time
+ * (i2400m->alive remains 1) until the driver is removed. Therefore
+ * all the device reboot events detected can be still handled properly
+ * by either dev_reset_handle() or .pre_reset/.post_reset as long as
+ * the driver presents. It is set 0 along with @updown in dev_stop().
*/
struct i2400m {
struct wimax_dev wimax_dev; /* FIRST! See doc */
@@ -591,6 +619,12 @@ struct i2400m {
struct i2400m_barker_db *barker;
struct notifier_block pm_notifier;
+
+ /* counting bus reset retries in this boot */
+ atomic_t bus_reset_retries;
+
+ /* if the device is expected to be alive */
+ unsigned alive;
};
--
1.6.6.1
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [patch 2.6.35 07/25] wimax/i2400m: add the error recovery mechanism on TX path
2010-05-14 21:44 [patch 2.6.35 00/25] WiMAX pull request Inaky Perez-Gonzalez
` (5 preceding siblings ...)
2010-05-14 21:45 ` [patch 2.6.35 06/25] wimax/i2400m: fix for missed reset events if triggered by dev_reset_handle() Inaky Perez-Gonzalez
@ 2010-05-14 21:45 ` Inaky Perez-Gonzalez
2010-05-14 21:45 ` [patch 2.6.35 08/25] wimax/i2400m: Correct the error path handlers order in i2400m_post_reset() Inaky Perez-Gonzalez
` (18 subsequent siblings)
25 siblings, 0 replies; 27+ messages in thread
From: Inaky Perez-Gonzalez @ 2010-05-14 21:45 UTC (permalink / raw)
To: netdev, wimax; +Cc: Cindy H Kao
From: Cindy H Kao <cindy.h.kao@intel.com>
This patch adds an error recovery mechanism on TX path.
The intention is to bring back the device to some known state
whenever TX sees -110 (-ETIMEOUT) on copying the data to the HW FIFO.
The TX failure could mean a device bus stuck or function stuck, so
the current error recovery implementation is to trigger a bus reset
and expect this can bring back the device.
Since the TX work is done in a thread context, there may be a queue of TX works
already that all hit the -ETIMEOUT error condition because the device has
somewhat stuck already. We don't want any consecutive bus resets simply because
multiple TX works in the queue all hit the same device erratum, the flag
"error_recovery" is introduced to denote if we are ready for taking any
error recovery. See @error_recovery doc in i2400m.h.
Signed-off-by: Cindy H Kao <cindy.h.kao@intel.com>
---
drivers/net/wimax/i2400m/driver.c | 74 ++++++++++++++++++++++++++++++++++++
drivers/net/wimax/i2400m/i2400m.h | 14 +++++++
drivers/net/wimax/i2400m/sdio-tx.c | 4 ++
3 files changed, 92 insertions(+), 0 deletions(-)
diff --git a/drivers/net/wimax/i2400m/driver.c b/drivers/net/wimax/i2400m/driver.c
index 1674dba..d83fe84 100644
--- a/drivers/net/wimax/i2400m/driver.c
+++ b/drivers/net/wimax/i2400m/driver.c
@@ -395,6 +395,16 @@ retry:
result = i2400m_dev_initialize(i2400m);
if (result < 0)
goto error_dev_initialize;
+
+ /* We don't want any additional unwanted error recovery triggered
+ * from any other context so if anything went wrong before we come
+ * here, let's keep i2400m->error_recovery untouched and leave it to
+ * dev_reset_handle(). See dev_reset_handle(). */
+
+ atomic_dec(&i2400m->error_recovery);
+ /* Every thing works so far, ok, now we are ready to
+ * take error recovery if it's required. */
+
/* At this point, reports will come for the device and set it
* to the right state if it is different than UNINITIALIZED */
d_fnend(3, dev, "(net_dev %p [i2400m %p]) = %d\n",
@@ -770,6 +780,66 @@ int i2400m_dev_reset_handle(struct i2400m *i2400m, const char *reason)
EXPORT_SYMBOL_GPL(i2400m_dev_reset_handle);
+ /*
+ * The actual work of error recovery.
+ *
+ * The current implementation of error recovery is to trigger a bus reset.
+ */
+static
+void __i2400m_error_recovery(struct work_struct *ws)
+{
+ struct i2400m_work *iw = container_of(ws, struct i2400m_work, ws);
+ struct i2400m *i2400m = iw->i2400m;
+
+ i2400m_reset(i2400m, I2400M_RT_BUS);
+
+ i2400m_put(i2400m);
+ kfree(iw);
+ return;
+}
+
+/*
+ * Schedule a work struct for error recovery.
+ *
+ * The intention of error recovery is to bring back the device to some
+ * known state whenever TX sees -110 (-ETIMEOUT) on copying the data to
+ * the device. The TX failure could mean a device bus stuck, so the current
+ * error recovery implementation is to trigger a bus reset to the device
+ * and hopefully it can bring back the device.
+ *
+ * The actual work of error recovery has to be in a thread context because
+ * it is kicked off in the TX thread (i2400ms->tx_workqueue) which is to be
+ * destroyed by the error recovery mechanism (currently a bus reset).
+ *
+ * Also, there may be already a queue of TX works that all hit
+ * the -ETIMEOUT error condition because the device is stuck already.
+ * Since bus reset is used as the error recovery mechanism and we don't
+ * want consecutive bus resets simply because the multiple TX works
+ * in the queue all hit the same device erratum, the flag "error_recovery"
+ * is introduced for preventing unwanted consecutive bus resets.
+ *
+ * Error recovery shall only be invoked again if previous one was completed.
+ * The flag error_recovery is set when error recovery mechanism is scheduled,
+ * and is checked when we need to schedule another error recovery. If it is
+ * in place already, then we shouldn't schedule another one.
+ */
+void i2400m_error_recovery(struct i2400m *i2400m)
+{
+ struct device *dev = i2400m_dev(i2400m);
+
+ if (atomic_add_return(1, &i2400m->error_recovery) == 1) {
+ if (i2400m_schedule_work(i2400m, __i2400m_error_recovery,
+ GFP_ATOMIC, NULL, 0) < 0) {
+ dev_err(dev, "run out of memory for "
+ "scheduling an error recovery ?\n");
+ atomic_dec(&i2400m->error_recovery);
+ }
+ } else
+ atomic_dec(&i2400m->error_recovery);
+ return;
+}
+EXPORT_SYMBOL_GPL(i2400m_error_recovery);
+
/*
* Alloc the command and ack buffers for boot mode
*
@@ -839,6 +909,10 @@ void i2400m_init(struct i2400m *i2400m)
atomic_set(&i2400m->bus_reset_retries, 0);
i2400m->alive = 0;
+
+ /* initialize error_recovery to 1 for denoting we
+ * are not yet ready to take any error recovery */
+ atomic_set(&i2400m->error_recovery, 1);
}
EXPORT_SYMBOL_GPL(i2400m_init);
diff --git a/drivers/net/wimax/i2400m/i2400m.h b/drivers/net/wimax/i2400m/i2400m.h
index ad8e6a3..7a9c2c5 100644
--- a/drivers/net/wimax/i2400m/i2400m.h
+++ b/drivers/net/wimax/i2400m/i2400m.h
@@ -545,6 +545,15 @@ struct i2400m_barker_db;
* all the device reboot events detected can be still handled properly
* by either dev_reset_handle() or .pre_reset/.post_reset as long as
* the driver presents. It is set 0 along with @updown in dev_stop().
+ *
+ * @error_recovery: flag to denote if we are ready to take an error recovery.
+ * 0 for ready to take an error recovery; 1 for not ready. It is
+ * initialized to 1 while probe() since we don't tend to take any error
+ * recovery during probe(). It is decremented by 1 whenever dev_start()
+ * succeeds to indicate we are ready to take error recovery from now on.
+ * It is checked every time we wanna schedule an error recovery. If an
+ * error recovery is already in place (error_recovery was set 1), we
+ * should not schedule another one until the last one is done.
*/
struct i2400m {
struct wimax_dev wimax_dev; /* FIRST! See doc */
@@ -625,6 +634,10 @@ struct i2400m {
/* if the device is expected to be alive */
unsigned alive;
+
+ /* 0 if we are ready for error recovery; 1 if not ready */
+ atomic_t error_recovery;
+
};
@@ -847,6 +860,7 @@ void i2400m_put(struct i2400m *i2400m)
extern int i2400m_dev_reset_handle(struct i2400m *, const char *);
extern int i2400m_pre_reset(struct i2400m *);
extern int i2400m_post_reset(struct i2400m *);
+extern void i2400m_error_recovery(struct i2400m *);
/*
* _setup()/_release() are called by the probe/disconnect functions of
diff --git a/drivers/net/wimax/i2400m/sdio-tx.c b/drivers/net/wimax/i2400m/sdio-tx.c
index 412b6a8..b53cd1c 100644
--- a/drivers/net/wimax/i2400m/sdio-tx.c
+++ b/drivers/net/wimax/i2400m/sdio-tx.c
@@ -98,6 +98,10 @@ void i2400ms_tx_submit(struct work_struct *ws)
tx_msg_size, result);
}
+ if (result == -ETIMEDOUT) {
+ i2400m_error_recovery(i2400m);
+ break;
+ }
d_printf(2, dev, "TX: %zub submitted\n", tx_msg_size);
}
--
1.6.6.1
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [patch 2.6.35 08/25] wimax/i2400m: Correct the error path handlers order in i2400m_post_reset()
2010-05-14 21:44 [patch 2.6.35 00/25] WiMAX pull request Inaky Perez-Gonzalez
` (6 preceding siblings ...)
2010-05-14 21:45 ` [patch 2.6.35 07/25] wimax/i2400m: add the error recovery mechanism on TX path Inaky Perez-Gonzalez
@ 2010-05-14 21:45 ` Inaky Perez-Gonzalez
2010-05-14 21:45 ` [patch 2.6.35 09/25] wimax/i2400m: Reset the TX FIFO indices when allocating the TX FIFO in tx_setup() Inaky Perez-Gonzalez
` (17 subsequent siblings)
25 siblings, 0 replies; 27+ messages in thread
From: Inaky Perez-Gonzalez @ 2010-05-14 21:45 UTC (permalink / raw)
To: netdev, wimax; +Cc: Cindy H Kao
From: Cindy H Kao <cindy.h.kao@intel.com>
When bus_setup fails in i2400m_post_reset(), it falls to the error path handler
"error_bus_setup:" which includes unlock the mutext. However, we didn't ever
try to the obtain the lock when running bus_setup.
The patch is to fix the misplaced error path handler "error_bus_setup:".
Signed-off-by: Cindy H Kao <cindy.h.kao@intel.com>
---
drivers/net/wimax/i2400m/driver.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/drivers/net/wimax/i2400m/driver.c b/drivers/net/wimax/i2400m/driver.c
index d83fe84..39cf96a 100644
--- a/drivers/net/wimax/i2400m/driver.c
+++ b/drivers/net/wimax/i2400m/driver.c
@@ -629,12 +629,12 @@ int i2400m_post_reset(struct i2400m *i2400m)
error_dev_start:
if (i2400m->bus_release)
i2400m->bus_release(i2400m);
-error_bus_setup:
/* even if the device was up, it could not be recovered, so we
* mark it as down. */
i2400m->updown = 0;
wmb(); /* see i2400m->updown's documentation */
mutex_unlock(&i2400m->init_mutex);
+error_bus_setup:
d_fnend(3, dev, "(i2400m %p) = %d\n", i2400m, result);
return result;
}
--
1.6.6.1
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [patch 2.6.35 09/25] wimax/i2400m: Reset the TX FIFO indices when allocating the TX FIFO in tx_setup()
2010-05-14 21:44 [patch 2.6.35 00/25] WiMAX pull request Inaky Perez-Gonzalez
` (7 preceding siblings ...)
2010-05-14 21:45 ` [patch 2.6.35 08/25] wimax/i2400m: Correct the error path handlers order in i2400m_post_reset() Inaky Perez-Gonzalez
@ 2010-05-14 21:45 ` Inaky Perez-Gonzalez
2010-05-14 21:45 ` [patch 2.6.35 10/25] wimax/i2400m: increase the maximum number of payloads per message to 60 [v1] Inaky Perez-Gonzalez
` (16 subsequent siblings)
25 siblings, 0 replies; 27+ messages in thread
From: Inaky Perez-Gonzalez @ 2010-05-14 21:45 UTC (permalink / raw)
To: netdev, wimax; +Cc: Cindy H Kao
From: Cindy H Kao <cindy.h.kao@intel.com>
This patch makes sure whenever tx_setup() is invoked during driver
initialization or device reset where TX FIFO is released and re-allocated,
the indices tx_in, tx_out, tx_msg_size, tx_sequence, tx_msg are properly
initialized.
When a device reset happens and the TX FIFO is released/re-allocated,
a new block of memory may be allocated for the TX FIFO, therefore tx_msg
should be cleared so that no any TX threads (tx_worker, tx) would access
to the out-of-date addresses.
Also, the TX threads use tx_in and tx_out to decide where to put the new
host-to-device messages and from where to copy them to the device HW FIFO,
these indices have to be cleared so after the TX FIFO is re-allocated during
the reset, the indices both refer to the head of the FIFO, ie. a new start.
The same rational applies to tx_msg_size and tx_sequence.
To protect the indices from being accessed by multiple threads simultaneously,
the lock tx_lock has to be obtained before the initializations and released
afterwards.
Signed-off-by: Cindy H Kao <cindy.h.kao@intel.com>
---
drivers/net/wimax/i2400m/tx.c | 29 +++++++++++++++++++++--------
1 files changed, 21 insertions(+), 8 deletions(-)
diff --git a/drivers/net/wimax/i2400m/tx.c b/drivers/net/wimax/i2400m/tx.c
index 8561c07..21909e5 100644
--- a/drivers/net/wimax/i2400m/tx.c
+++ b/drivers/net/wimax/i2400m/tx.c
@@ -877,27 +877,40 @@ EXPORT_SYMBOL_GPL(i2400m_tx_msg_sent);
* i2400m_tx_setup - Initialize the TX queue and infrastructure
*
* Make sure we reset the TX sequence to zero, as when this function
- * is called, the firmware has been just restarted.
+ * is called, the firmware has been just restarted. Same rational
+ * for tx_in, tx_out, tx_msg_size and tx_msg. We reset them since
+ * the memory for TX queue is reallocated.
*/
int i2400m_tx_setup(struct i2400m *i2400m)
{
- int result;
+ int result = 0;
+ void *tx_buf;
+ unsigned long flags;
/* Do this here only once -- can't do on
* i2400m_hard_start_xmit() as we'll cause race conditions if
* the WS was scheduled on another CPU */
INIT_WORK(&i2400m->wake_tx_ws, i2400m_wake_tx_work);
- i2400m->tx_sequence = 0;
+ tx_buf = kmalloc(I2400M_TX_BUF_SIZE, GFP_ATOMIC);
+ if (tx_buf == NULL) {
+ result = -ENOMEM;
+ goto error_kmalloc;
+ }
+
/* Warn if the calculated buffer size exceeds I2400M_TX_BUF_SIZE. */
BUILD_BUG_ON(I2400M_TX_PDU_TOTAL_SIZE > I2400M_TX_BUF_SIZE);
- i2400m->tx_buf = kmalloc(I2400M_TX_BUF_SIZE, GFP_KERNEL);
- if (i2400m->tx_buf == NULL)
- result = -ENOMEM;
- else
- result = 0;
+ spin_lock_irqsave(&i2400m->tx_lock, flags);
+ i2400m->tx_sequence = 0;
+ i2400m->tx_in = 0;
+ i2400m->tx_out = 0;
+ i2400m->tx_msg_size = 0;
+ i2400m->tx_msg = NULL;
+ i2400m->tx_buf = tx_buf;
+ spin_unlock_irqrestore(&i2400m->tx_lock, flags);
/* Huh? the bus layer has to define this... */
BUG_ON(i2400m->bus_tx_block_size == 0);
+error_kmalloc:
return result;
}
--
1.6.6.1
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [patch 2.6.35 10/25] wimax/i2400m: increase the maximum number of payloads per message to 60 [v1]
2010-05-14 21:44 [patch 2.6.35 00/25] WiMAX pull request Inaky Perez-Gonzalez
` (8 preceding siblings ...)
2010-05-14 21:45 ` [patch 2.6.35 09/25] wimax/i2400m: Reset the TX FIFO indices when allocating the TX FIFO in tx_setup() Inaky Perez-Gonzalez
@ 2010-05-14 21:45 ` Inaky Perez-Gonzalez
2010-05-14 21:45 ` [patch 2.6.35 11/25] wimax/i2400m: limit the message size upto 16KiB [v1] Inaky Perez-Gonzalez
` (15 subsequent siblings)
25 siblings, 0 replies; 27+ messages in thread
From: Inaky Perez-Gonzalez @ 2010-05-14 21:45 UTC (permalink / raw)
To: netdev, wimax; +Cc: Prasanna S. Panchamukhi
From: Prasanna S. Panchamukhi <prasannax.s.panchamukhi@intel.com>
According to Intel Wimax i3200, i5x50 and i6x50 device specification
documents, the maximum number of payloads per message can be up to 60.
Increasing the number of payloads to 60 per message helps to
accommodate smaller payloads in a single transaction. This patch
increases the maximum number of payloads from 12 to 60 per message.
Signed-off-by: Prasanna S. Panchamukhi <prasannax.s.panchamukhi@intel.com>
---
drivers/net/wimax/i2400m/tx.c | 8 +++++++-
1 files changed, 7 insertions(+), 1 deletions(-)
diff --git a/drivers/net/wimax/i2400m/tx.c b/drivers/net/wimax/i2400m/tx.c
index 21909e5..b10c3b7 100644
--- a/drivers/net/wimax/i2400m/tx.c
+++ b/drivers/net/wimax/i2400m/tx.c
@@ -272,7 +272,13 @@ enum {
* at the end there are less, we pad up to the nearest
* multiple of 16.
*/
- I2400M_TX_PLD_MAX = 12,
+ /*
+ * According to Intel Wimax i3200, i5x50 and i6x50 specification
+ * documents, the maximum number of payloads per message can be
+ * up to 60. Increasing the number of payloads to 60 per message
+ * helps to accommodate smaller payloads in a single transaction.
+ */
+ I2400M_TX_PLD_MAX = 60,
I2400M_TX_PLD_SIZE = sizeof(struct i2400m_msg_hdr)
+ I2400M_TX_PLD_MAX * sizeof(struct i2400m_pld),
I2400M_TX_SKIP = 0x80000000,
--
1.6.6.1
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [patch 2.6.35 11/25] wimax/i2400m: limit the message size upto 16KiB [v1]
2010-05-14 21:44 [patch 2.6.35 00/25] WiMAX pull request Inaky Perez-Gonzalez
` (9 preceding siblings ...)
2010-05-14 21:45 ` [patch 2.6.35 10/25] wimax/i2400m: increase the maximum number of payloads per message to 60 [v1] Inaky Perez-Gonzalez
@ 2010-05-14 21:45 ` Inaky Perez-Gonzalez
2010-05-14 21:45 ` [patch 2.6.35 12/25] wimax/i2400m: fix BUILD_BUG_ON() to use the maximum message size constant [v1] Inaky Perez-Gonzalez
` (14 subsequent siblings)
25 siblings, 0 replies; 27+ messages in thread
From: Inaky Perez-Gonzalez @ 2010-05-14 21:45 UTC (permalink / raw)
To: netdev, wimax; +Cc: Prasanna S. Panchamukhi
From: Prasanna S. Panchamukhi <prasannax.s.panchamukhi@intel.com>
According to Intel Wimax i3200, i5x50 and i6x50 specification
documents, the maximum size of each TX message can be upto 16KiB.
This patch modifies the i2400m_tx() routine to check that the
message size does not exceed the 16KiB limit.
Please refer the documentation in the code for details.
Signed-off-by: Prasanna S. Panchamukhi <prasannax.s.panchamukhi@intel.com>
---
drivers/net/wimax/i2400m/tx.c | 13 ++++++++++++-
1 files changed, 12 insertions(+), 1 deletions(-)
diff --git a/drivers/net/wimax/i2400m/tx.c b/drivers/net/wimax/i2400m/tx.c
index b10c3b7..a5002c8 100644
--- a/drivers/net/wimax/i2400m/tx.c
+++ b/drivers/net/wimax/i2400m/tx.c
@@ -283,6 +283,11 @@ enum {
+ I2400M_TX_PLD_MAX * sizeof(struct i2400m_pld),
I2400M_TX_SKIP = 0x80000000,
/*
+ * According to Intel Wimax i3200, i5x50 and i6x50 specification
+ * documents, the maximum size of each message can be up to 16KiB.
+ */
+ I2400M_TX_MSG_SIZE = 16384,
+ /*
* 16 byte aligned MAX_MTU + 4 byte payload prefix.
*/
I2400M_MAX_MTU_ALIGN = 16,
@@ -682,7 +687,13 @@ try_new:
}
if (i2400m->tx_msg == NULL)
goto error_tx_new;
- if (i2400m->tx_msg->size + padded_len > I2400M_TX_BUF_SIZE / 2) {
+ /*
+ * Check if this skb will fit in the TX queue's current active
+ * TX message. The total message size must not exceed the maximum
+ * size of each message I2400M_TX_MSG_SIZE. If it exceeds,
+ * close the current message and push this skb into the new message.
+ */
+ if (i2400m->tx_msg->size + padded_len > I2400M_TX_MSG_SIZE) {
d_printf(2, dev, "TX: message too big, going new\n");
i2400m_tx_close(i2400m);
i2400m_tx_new(i2400m);
--
1.6.6.1
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [patch 2.6.35 12/25] wimax/i2400m: fix BUILD_BUG_ON() to use the maximum message size constant [v1]
2010-05-14 21:44 [patch 2.6.35 00/25] WiMAX pull request Inaky Perez-Gonzalez
` (10 preceding siblings ...)
2010-05-14 21:45 ` [patch 2.6.35 11/25] wimax/i2400m: limit the message size upto 16KiB [v1] Inaky Perez-Gonzalez
@ 2010-05-14 21:45 ` Inaky Perez-Gonzalez
2010-05-14 21:45 ` [patch 2.6.35 13/25] wimax/i2400m: modify i2400m_tx_fifo_push() to check for head room space in the TX FIFO [v1] Inaky Perez-Gonzalez
` (13 subsequent siblings)
25 siblings, 0 replies; 27+ messages in thread
From: Inaky Perez-Gonzalez @ 2010-05-14 21:45 UTC (permalink / raw)
To: netdev, wimax; +Cc: Prasanna S. Panchamukhi
From: Prasanna S. Panchamukhi <prasannax.s.panchamukhi@intel.com>
The older method of computing the maximum PDU size relied
on a method that doesn't work when we prop the maximum
number of payloads up to the physical limit, and thus we kill
the whole computation and just verify that the constants are
congruent.
Signed-off-by: Prasanna S. Panchamukhi <prasannax.s.panchamukhi@intel.com>
---
drivers/net/wimax/i2400m/tx.c | 20 +++++---------------
1 files changed, 5 insertions(+), 15 deletions(-)
diff --git a/drivers/net/wimax/i2400m/tx.c b/drivers/net/wimax/i2400m/tx.c
index a5002c8..1725f2b 100644
--- a/drivers/net/wimax/i2400m/tx.c
+++ b/drivers/net/wimax/i2400m/tx.c
@@ -287,19 +287,6 @@ enum {
* documents, the maximum size of each message can be up to 16KiB.
*/
I2400M_TX_MSG_SIZE = 16384,
- /*
- * 16 byte aligned MAX_MTU + 4 byte payload prefix.
- */
- I2400M_MAX_MTU_ALIGN = 16,
- I2400M_TX_PDU_SIZE = I2400M_MAX_MTU % I2400M_MAX_MTU_ALIGN
- + I2400M_MAX_MTU + sizeof(struct i2400m_pl_data_hdr),
- /*
- * 256 byte aligned toal size of 12 PDUs including msg header,
- */
- I2400M_TX_PDU_ALIGN = 256,
- I2400M_TX_PDU_TOTAL_SIZE = ((I2400M_TX_PDU_SIZE * I2400M_TX_PLD_MAX
- + sizeof(struct i2400m_msg_hdr))/I2400M_TX_PDU_ALIGN + 1)
- * I2400M_TX_PDU_ALIGN * 2,
};
#define TAIL_FULL ((void *)~(unsigned long)NULL)
@@ -915,8 +902,11 @@ int i2400m_tx_setup(struct i2400m *i2400m)
goto error_kmalloc;
}
- /* Warn if the calculated buffer size exceeds I2400M_TX_BUF_SIZE. */
- BUILD_BUG_ON(I2400M_TX_PDU_TOTAL_SIZE > I2400M_TX_BUF_SIZE);
+ /*
+ * Fail the build if we can't fit at least two maximum size messages
+ * on the TX FIFO [one being delivered while one is constructed].
+ */
+ BUILD_BUG_ON(2 * I2400M_TX_MSG_SIZE > I2400M_TX_BUF_SIZE);
spin_lock_irqsave(&i2400m->tx_lock, flags);
i2400m->tx_sequence = 0;
i2400m->tx_in = 0;
--
1.6.6.1
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [patch 2.6.35 13/25] wimax/i2400m: modify i2400m_tx_fifo_push() to check for head room space in the TX FIFO [v1]
2010-05-14 21:44 [patch 2.6.35 00/25] WiMAX pull request Inaky Perez-Gonzalez
` (11 preceding siblings ...)
2010-05-14 21:45 ` [patch 2.6.35 12/25] wimax/i2400m: fix BUILD_BUG_ON() to use the maximum message size constant [v1] Inaky Perez-Gonzalez
@ 2010-05-14 21:45 ` Inaky Perez-Gonzalez
2010-05-14 21:45 ` [patch 2.6.35 14/25] wimax/i2400m: fix system freeze caused by an infinite loop [v1] Inaky Perez-Gonzalez
` (12 subsequent siblings)
25 siblings, 0 replies; 27+ messages in thread
From: Inaky Perez-Gonzalez @ 2010-05-14 21:45 UTC (permalink / raw)
To: netdev, wimax; +Cc: Prasanna S. Panchamukhi
From: Prasanna S. Panchamukhi <prasannax.s.panchamukhi@intel.com>
This fixes i2400m_tx_fifo_push(); the check for having enough
space in the TX FIFO's tail was obscure and broken in certain
corner cases. The new check works in all cases and is way
clearer. Please refer the documentation in the code for details.
Signed-off-by: Prasanna S. Panchamukhi <prasannax.s.panchamukhi@intel.com>
---
drivers/net/wimax/i2400m/tx.c | 16 ++++++++++++++--
1 files changed, 14 insertions(+), 2 deletions(-)
diff --git a/drivers/net/wimax/i2400m/tx.c b/drivers/net/wimax/i2400m/tx.c
index 1725f2b..101550a 100644
--- a/drivers/net/wimax/i2400m/tx.c
+++ b/drivers/net/wimax/i2400m/tx.c
@@ -396,8 +396,20 @@ void *i2400m_tx_fifo_push(struct i2400m *i2400m, size_t size, size_t padding)
/* Is there space at the tail? */
tail_room = __i2400m_tx_tail_room(i2400m);
if (tail_room < needed_size) {
- if (i2400m->tx_out % I2400M_TX_BUF_SIZE
- < i2400m->tx_in % I2400M_TX_BUF_SIZE) {
+ /*
+ * If the tail room space is not enough to push the message
+ * in the TX FIFO, then there are two possibilities:
+ * 1. There is enough head room space to accommodate
+ * this message in the TX FIFO.
+ * 2. There is not enough space in the head room and
+ * in tail room of the TX FIFO to accommodate the message.
+ * In the case (1), return TAIL_FULL so that the caller
+ * can figure out, if the caller wants to push the message
+ * into the head room space.
+ * In the case (2), return NULL, indicating that the TX FIFO
+ * cannot accommodate the message.
+ */
+ if (room - tail_room >= needed_size) {
d_printf(2, dev, "fifo push %zu/%zu: tail full\n",
size, padding);
return TAIL_FULL; /* There might be head space */
--
1.6.6.1
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [patch 2.6.35 14/25] wimax/i2400m: fix system freeze caused by an infinite loop [v1]
2010-05-14 21:44 [patch 2.6.35 00/25] WiMAX pull request Inaky Perez-Gonzalez
` (12 preceding siblings ...)
2010-05-14 21:45 ` [patch 2.6.35 13/25] wimax/i2400m: modify i2400m_tx_fifo_push() to check for head room space in the TX FIFO [v1] Inaky Perez-Gonzalez
@ 2010-05-14 21:45 ` Inaky Perez-Gonzalez
2010-05-14 21:45 ` [patch 2.6.35 15/25] wimax/i2400m: increase tx queue length from 5 to 20 [v1] Inaky Perez-Gonzalez
` (11 subsequent siblings)
25 siblings, 0 replies; 27+ messages in thread
From: Inaky Perez-Gonzalez @ 2010-05-14 21:45 UTC (permalink / raw)
To: netdev, wimax; +Cc: Prasanna S. Panchamukhi
From: Prasanna S. Panchamukhi <prasannax.s.panchamukhi@intel.com>
This patch fixes an infinite loop caused by i2400m_tx_fifo_push() due
to a corner case where there is no tail space in the TX FIFO.
Please refer the documentation in the code for details.
Signed-off-by: Prasanna S. Panchamukhi <prasannax.s.panchamukhi@intel.com>
---
drivers/net/wimax/i2400m/tx.c | 65 +++++++++++++++++++++++++++++++++++++---
1 files changed, 60 insertions(+), 5 deletions(-)
diff --git a/drivers/net/wimax/i2400m/tx.c b/drivers/net/wimax/i2400m/tx.c
index 101550a..609f1ca 100644
--- a/drivers/net/wimax/i2400m/tx.c
+++ b/drivers/net/wimax/i2400m/tx.c
@@ -341,6 +341,14 @@ size_t __i2400m_tx_tail_room(struct i2400m *i2400m)
* @padding: ensure that there is at least this many bytes of free
* contiguous space in the fifo. This is needed because later on
* we might need to add padding.
+ * @try_head: specify either to allocate head room or tail room space
+ * in the TX FIFO. This boolean is required to avoids a system hang
+ * due to an infinite loop caused by i2400m_tx_fifo_push().
+ * The caller must always try to allocate tail room space first by
+ * calling this routine with try_head = 0. In case if there
+ * is not enough tail room space but there is enough head room space,
+ * (i2400m_tx_fifo_push() returns TAIL_FULL) try to allocate head
+ * room space, by calling this routine again with try_head = 1.
*
* Returns:
*
@@ -372,6 +380,48 @@ size_t __i2400m_tx_tail_room(struct i2400m *i2400m)
* fail and return TAIL_FULL and let the caller figure out if we wants to
* skip the tail room and try to allocate from the head.
*
+ * There is a corner case, wherein i2400m_tx_new() can get into
+ * an infinite loop calling i2400m_tx_fifo_push().
+ * In certain situations, tx_in would have reached on the top of TX FIFO
+ * and i2400m_tx_tail_room() returns 0, as described below:
+ *
+ * N ___________ tail room is zero
+ * |<- IN ->|
+ * | |
+ * | |
+ * | |
+ * | data |
+ * |<- OUT ->|
+ * | |
+ * | |
+ * | head room |
+ * 0 -----------
+ * During such a time, where tail room is zero in the TX FIFO and if there
+ * is a request to add a payload to TX FIFO, which calls:
+ * i2400m_tx()
+ * ->calls i2400m_tx_close()
+ * ->calls i2400m_tx_skip_tail()
+ * goto try_new;
+ * ->calls i2400m_tx_new()
+ * |----> [try_head:]
+ * infinite loop | ->calls i2400m_tx_fifo_push()
+ * | if (tail_room < needed)
+ * | if (head_room => needed)
+ * | return TAIL_FULL;
+ * |<---- goto try_head;
+ *
+ * i2400m_tx() calls i2400m_tx_close() to close the message, since there
+ * is no tail room to accommodate the payload and calls
+ * i2400m_tx_skip_tail() to skip the tail space. Now i2400m_tx() calls
+ * i2400m_tx_new() to allocate space for new message header calling
+ * i2400m_tx_fifo_push() that returns TAIL_FULL, since there is no tail space
+ * to accommodate the message header, but there is enough head space.
+ * The i2400m_tx_new() keeps re-retrying by calling i2400m_tx_fifo_push()
+ * ending up in a loop causing system freeze.
+ *
+ * This corner case is avoided by using a try_head boolean,
+ * as an argument to i2400m_tx_fifo_push().
+ *
* Note:
*
* Assumes i2400m->tx_lock is taken, and we use that as a barrier
@@ -380,7 +430,8 @@ size_t __i2400m_tx_tail_room(struct i2400m *i2400m)
* pop data off the queue
*/
static
-void *i2400m_tx_fifo_push(struct i2400m *i2400m, size_t size, size_t padding)
+void *i2400m_tx_fifo_push(struct i2400m *i2400m, size_t size,
+ size_t padding, bool try_head)
{
struct device *dev = i2400m_dev(i2400m);
size_t room, tail_room, needed_size;
@@ -395,7 +446,7 @@ void *i2400m_tx_fifo_push(struct i2400m *i2400m, size_t size, size_t padding)
}
/* Is there space at the tail? */
tail_room = __i2400m_tx_tail_room(i2400m);
- if (tail_room < needed_size) {
+ if (!try_head && tail_room < needed_size) {
/*
* If the tail room space is not enough to push the message
* in the TX FIFO, then there are two possibilities:
@@ -510,14 +561,16 @@ void i2400m_tx_new(struct i2400m *i2400m)
{
struct device *dev = i2400m_dev(i2400m);
struct i2400m_msg_hdr *tx_msg;
+ bool try_head = 0;
BUG_ON(i2400m->tx_msg != NULL);
try_head:
- tx_msg = i2400m_tx_fifo_push(i2400m, I2400M_TX_PLD_SIZE, 0);
+ tx_msg = i2400m_tx_fifo_push(i2400m, I2400M_TX_PLD_SIZE, 0, try_head);
if (tx_msg == NULL)
goto out;
else if (tx_msg == TAIL_FULL) {
i2400m_tx_skip_tail(i2400m);
d_printf(2, dev, "new TX message: tail full, trying head\n");
+ try_head = 1;
goto try_head;
}
memset(tx_msg, 0, I2400M_TX_PLD_SIZE);
@@ -591,7 +644,7 @@ void i2400m_tx_close(struct i2400m *i2400m)
aligned_size = ALIGN(tx_msg_moved->size, i2400m->bus_tx_block_size);
padding = aligned_size - tx_msg_moved->size;
if (padding > 0) {
- pad_buf = i2400m_tx_fifo_push(i2400m, padding, 0);
+ pad_buf = i2400m_tx_fifo_push(i2400m, padding, 0, 0);
if (unlikely(WARN_ON(pad_buf == NULL
|| pad_buf == TAIL_FULL))) {
/* This should not happen -- append should verify
@@ -657,6 +710,7 @@ int i2400m_tx(struct i2400m *i2400m, const void *buf, size_t buf_len,
unsigned long flags;
size_t padded_len;
void *ptr;
+ bool try_head = 0;
unsigned is_singleton = pl_type == I2400M_PT_RESET_WARM
|| pl_type == I2400M_PT_RESET_COLD;
@@ -702,11 +756,12 @@ try_new:
/* So we have a current message header; now append space for
* the message -- if there is not enough, try the head */
ptr = i2400m_tx_fifo_push(i2400m, padded_len,
- i2400m->bus_tx_block_size);
+ i2400m->bus_tx_block_size, try_head);
if (ptr == TAIL_FULL) { /* Tail is full, try head */
d_printf(2, dev, "pl append: tail full\n");
i2400m_tx_close(i2400m);
i2400m_tx_skip_tail(i2400m);
+ try_head = 1;
goto try_new;
} else if (ptr == NULL) { /* All full */
result = -ENOSPC;
--
1.6.6.1
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [patch 2.6.35 15/25] wimax/i2400m: increase tx queue length from 5 to 20 [v1]
2010-05-14 21:44 [patch 2.6.35 00/25] WiMAX pull request Inaky Perez-Gonzalez
` (13 preceding siblings ...)
2010-05-14 21:45 ` [patch 2.6.35 14/25] wimax/i2400m: fix system freeze caused by an infinite loop [v1] Inaky Perez-Gonzalez
@ 2010-05-14 21:45 ` Inaky Perez-Gonzalez
2010-05-14 21:45 ` [patch 2.6.35 16/25] wimax i2400m: fix race condition while accessing rx_roq by using kref count Inaky Perez-Gonzalez
` (10 subsequent siblings)
25 siblings, 0 replies; 27+ messages in thread
From: Inaky Perez-Gonzalez @ 2010-05-14 21:45 UTC (permalink / raw)
To: netdev, wimax; +Cc: Prasanna S. Panchamukhi
From: Prasanna S. Panchamukhi <prasannax.s.panchamukhi@intel.com>
This patch increases the tx_queue_len to 20 so as to
minimize the jitter in the throughput.
Signed-off-by: Prasanna S. Panchamukhi <prasannax.s.panchamukhi@intel.com>
---
drivers/net/wimax/i2400m/netdev.c | 6 +++++-
1 files changed, 5 insertions(+), 1 deletions(-)
diff --git a/drivers/net/wimax/i2400m/netdev.c b/drivers/net/wimax/i2400m/netdev.c
index fc3754a..7d7b5ef 100644
--- a/drivers/net/wimax/i2400m/netdev.c
+++ b/drivers/net/wimax/i2400m/netdev.c
@@ -88,7 +88,11 @@ enum {
* might take to get out of IDLE / negotiate it with the base
* station. We add 1sec for good measure. */
I2400M_TX_TIMEOUT = 21 * HZ,
- I2400M_TX_QLEN = 5,
+ /*
+ * Experimentation has determined that, 20 to be a good value
+ * for minimizing the jitter in the throughput.
+ */
+ I2400M_TX_QLEN = 20,
};
--
1.6.6.1
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [patch 2.6.35 16/25] wimax i2400m: fix race condition while accessing rx_roq by using kref count
2010-05-14 21:44 [patch 2.6.35 00/25] WiMAX pull request Inaky Perez-Gonzalez
` (14 preceding siblings ...)
2010-05-14 21:45 ` [patch 2.6.35 15/25] wimax/i2400m: increase tx queue length from 5 to 20 [v1] Inaky Perez-Gonzalez
@ 2010-05-14 21:45 ` Inaky Perez-Gonzalez
2010-05-14 21:45 ` [patch 2.6.35 17/25] wimax/i2400m: fix incorrect handling of type 2 and 3 RX messages Inaky Perez-Gonzalez
` (9 subsequent siblings)
25 siblings, 0 replies; 27+ messages in thread
From: Inaky Perez-Gonzalez @ 2010-05-14 21:45 UTC (permalink / raw)
To: netdev, wimax; +Cc: Prasanna S. Panchamukhi
From: Prasanna S. Panchamukhi <prasannax.s.panchamukhi@intel.com>
This patch fixes the race condition when one thread tries to destroy
the memory allocated for rx_roq, while another thread still happen
to access rx_roq.
Such a race condition occurs when i2400m-sdio kernel module gets
unloaded, destroying the memory allocated for rx_roq while rx_roq
is accessed by i2400m_rx_edata(), as explained below:
$thread1 $thread2
$ void i2400m_rx_edata() $
$Access rx_roq[] $
$roq = &i2400m->rx_roq[ro_cin] $
$ i2400m_roq_[reset/queue/update_ws] $
$ $ void i2400m_rx_release();
$ $kfree(rx->roq);
$ $rx->roq = NULL;
$Oops! rx_roq is NULL
This patch fixes the race condition using refcount approach.
Signed-off-by: Prasanna S. Panchamukhi <prasannax.s.panchamukhi@intel.com>
---
drivers/net/wimax/i2400m/i2400m.h | 12 +++++++--
drivers/net/wimax/i2400m/rx.c | 44 ++++++++++++++++++++++++++++++++----
2 files changed, 48 insertions(+), 8 deletions(-)
diff --git a/drivers/net/wimax/i2400m/i2400m.h b/drivers/net/wimax/i2400m/i2400m.h
index 7a9c2c5..b8c7dbf 100644
--- a/drivers/net/wimax/i2400m/i2400m.h
+++ b/drivers/net/wimax/i2400m/i2400m.h
@@ -412,7 +412,7 @@ struct i2400m_barker_db;
*
* @tx_size_max: biggest TX message sent.
*
- * @rx_lock: spinlock to protect RX members
+ * @rx_lock: spinlock to protect RX members and rx_roq_refcount.
*
* @rx_pl_num: total number of payloads received
*
@@ -436,6 +436,10 @@ struct i2400m_barker_db;
* delivered. Then the driver can release them to the host. See
* drivers/net/i2400m/rx.c for details.
*
+ * @rx_roq_refcount: refcount rx_roq. This refcounts any access to
+ * rx_roq thus preventing rx_roq being destroyed when rx_roq
+ * is being accessed. rx_roq_refcount is protected by rx_lock.
+ *
* @rx_reports: reports received from the device that couldn't be
* processed because the driver wasn't still ready; when ready,
* they are pulled from here and chewed.
@@ -597,10 +601,12 @@ struct i2400m {
tx_num, tx_size_acc, tx_size_min, tx_size_max;
/* RX stuff */
- spinlock_t rx_lock; /* protect RX state */
+ /* protect RX state and rx_roq_refcount */
+ spinlock_t rx_lock;
unsigned rx_pl_num, rx_pl_max, rx_pl_min,
rx_num, rx_size_acc, rx_size_min, rx_size_max;
- struct i2400m_roq *rx_roq; /* not under rx_lock! */
+ struct i2400m_roq *rx_roq; /* access is refcounted */
+ struct kref rx_roq_refcount; /* refcount access to rx_roq */
u8 src_mac_addr[ETH_HLEN];
struct list_head rx_reports; /* under rx_lock! */
struct work_struct rx_report_ws;
diff --git a/drivers/net/wimax/i2400m/rx.c b/drivers/net/wimax/i2400m/rx.c
index fa2e11e..71b697f 100644
--- a/drivers/net/wimax/i2400m/rx.c
+++ b/drivers/net/wimax/i2400m/rx.c
@@ -917,6 +917,25 @@ void i2400m_roq_queue_update_ws(struct i2400m *i2400m, struct i2400m_roq *roq,
/*
+ * This routine destroys the memory allocated for rx_roq, when no
+ * other thread is accessing it. Access to rx_roq is refcounted by
+ * rx_roq_refcount, hence memory allocated must be destroyed when
+ * rx_roq_refcount becomes zero. This routine gets executed when
+ * rx_roq_refcount becomes zero.
+ */
+void i2400m_rx_roq_destroy(struct kref *ref)
+{
+ unsigned itr;
+ struct i2400m *i2400m
+ = container_of(ref, struct i2400m, rx_roq_refcount);
+ for (itr = 0; itr < I2400M_RO_CIN + 1; itr++)
+ __skb_queue_purge(&i2400m->rx_roq[itr].queue);
+ kfree(i2400m->rx_roq[0].log);
+ kfree(i2400m->rx_roq);
+ i2400m->rx_roq = NULL;
+}
+
+/*
* Receive and send up an extended data packet
*
* @i2400m: device descriptor
@@ -969,6 +988,7 @@ void i2400m_rx_edata(struct i2400m *i2400m, struct sk_buff *skb_rx,
unsigned ro_needed, ro_type, ro_cin, ro_sn;
struct i2400m_roq *roq;
struct i2400m_roq_data *roq_data;
+ unsigned long flags;
BUILD_BUG_ON(ETH_HLEN > sizeof(*hdr));
@@ -1007,7 +1027,16 @@ void i2400m_rx_edata(struct i2400m *i2400m, struct sk_buff *skb_rx,
ro_cin = (reorder >> I2400M_RO_CIN_SHIFT) & I2400M_RO_CIN;
ro_sn = (reorder >> I2400M_RO_SN_SHIFT) & I2400M_RO_SN;
+ spin_lock_irqsave(&i2400m->rx_lock, flags);
roq = &i2400m->rx_roq[ro_cin];
+ if (roq == NULL) {
+ kfree_skb(skb); /* rx_roq is already destroyed */
+ spin_unlock_irqrestore(&i2400m->rx_lock, flags);
+ goto error;
+ }
+ kref_get(&i2400m->rx_roq_refcount);
+ spin_unlock_irqrestore(&i2400m->rx_lock, flags);
+
roq_data = (struct i2400m_roq_data *) &skb->cb;
roq_data->sn = ro_sn;
roq_data->cs = cs;
@@ -1034,6 +1063,10 @@ void i2400m_rx_edata(struct i2400m *i2400m, struct sk_buff *skb_rx,
default:
dev_err(dev, "HW BUG? unknown reorder type %u\n", ro_type);
}
+
+ spin_lock_irqsave(&i2400m->rx_lock, flags);
+ kref_put(&i2400m->rx_roq_refcount, i2400m_rx_roq_destroy);
+ spin_unlock_irqrestore(&i2400m->rx_lock, flags);
}
else
i2400m_net_erx(i2400m, skb, cs);
@@ -1344,6 +1377,7 @@ int i2400m_rx_setup(struct i2400m *i2400m)
__i2400m_roq_init(&i2400m->rx_roq[itr]);
i2400m->rx_roq[itr].log = &rd[itr];
}
+ kref_init(&i2400m->rx_roq_refcount);
}
return 0;
@@ -1357,12 +1391,12 @@ error_roq_alloc:
/* Tear down the RX queue and infrastructure */
void i2400m_rx_release(struct i2400m *i2400m)
{
+ unsigned long flags;
+
if (i2400m->rx_reorder) {
- unsigned itr;
- for(itr = 0; itr < I2400M_RO_CIN + 1; itr++)
- __skb_queue_purge(&i2400m->rx_roq[itr].queue);
- kfree(i2400m->rx_roq[0].log);
- kfree(i2400m->rx_roq);
+ spin_lock_irqsave(&i2400m->rx_lock, flags);
+ kref_put(&i2400m->rx_roq_refcount, i2400m_rx_roq_destroy);
+ spin_unlock_irqrestore(&i2400m->rx_lock, flags);
}
/* at this point, nothing can be received... */
i2400m_report_hook_flush(i2400m);
--
1.6.6.1
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [patch 2.6.35 17/25] wimax/i2400m: fix incorrect handling of type 2 and 3 RX messages
2010-05-14 21:44 [patch 2.6.35 00/25] WiMAX pull request Inaky Perez-Gonzalez
` (15 preceding siblings ...)
2010-05-14 21:45 ` [patch 2.6.35 16/25] wimax i2400m: fix race condition while accessing rx_roq by using kref count Inaky Perez-Gonzalez
@ 2010-05-14 21:45 ` Inaky Perez-Gonzalez
2010-05-14 21:45 ` [patch 2.6.35 18/25] wimax/i2400m: reserve additional space in the TX queue's buffer while allocating space for a new message header Inaky Perez-Gonzalez
` (8 subsequent siblings)
25 siblings, 0 replies; 27+ messages in thread
From: Inaky Perez-Gonzalez @ 2010-05-14 21:45 UTC (permalink / raw)
To: netdev, wimax; +Cc: Prasanna S. Panchamukhi
From: Prasanna S. Panchamukhi <prasannax.s.panchamukhi@intel.com>
According to Intel Wimax i3200, i5x50 and i6x60 device specification documents,
the host driver must not reset the device if the normalized sequence numbers
are greater than 1023 for type 2 and type 3 RX messages.
This patch removes the code that incorrectly used to reset the device.
Signed-off-by: Prasanna S. Panchamukhi <prasannax.s.panchamukhi@intel.com>
---
drivers/net/wimax/i2400m/rx.c | 51 +++++++++++++++++++++--------------------
1 files changed, 26 insertions(+), 25 deletions(-)
diff --git a/drivers/net/wimax/i2400m/rx.c b/drivers/net/wimax/i2400m/rx.c
index 71b697f..66f968a 100644
--- a/drivers/net/wimax/i2400m/rx.c
+++ b/drivers/net/wimax/i2400m/rx.c
@@ -743,12 +743,12 @@ unsigned __i2400m_roq_update_ws(struct i2400m *i2400m, struct i2400m_roq *roq,
unsigned new_nws, nsn_itr;
new_nws = __i2400m_roq_nsn(roq, sn);
- if (unlikely(new_nws >= 1024) && d_test(1)) {
- dev_err(dev, "SW BUG? __update_ws new_nws %u (sn %u ws %u)\n",
- new_nws, sn, roq->ws);
- WARN_ON(1);
- i2400m_roq_log_dump(i2400m, roq);
- }
+ /*
+ * For type 2(update_window_start) rx messages, there is no
+ * need to check if the normalized sequence number is greater 1023.
+ * Simply insert and deliver all packets to the host up to the
+ * window start.
+ */
skb_queue_walk_safe(&roq->queue, skb_itr, tmp_itr) {
roq_data_itr = (struct i2400m_roq_data *) &skb_itr->cb;
nsn_itr = __i2400m_roq_nsn(roq, roq_data_itr->sn);
@@ -890,26 +890,27 @@ void i2400m_roq_queue_update_ws(struct i2400m *i2400m, struct i2400m_roq *roq,
i2400m, roq, skb, sn);
len = skb_queue_len(&roq->queue);
nsn = __i2400m_roq_nsn(roq, sn);
+ /*
+ * For type 3(queue_update_window_start) rx messages, there is no
+ * need to check if the normalized sequence number is greater 1023.
+ * Simply insert and deliver all packets to the host up to the
+ * window start.
+ */
old_ws = roq->ws;
- if (unlikely(nsn >= 1024)) {
- dev_err(dev, "SW BUG? queue_update_ws nsn %u (sn %u ws %u)\n",
- nsn, sn, roq->ws);
- i2400m_roq_log_dump(i2400m, roq);
- i2400m_reset(i2400m, I2400M_RT_WARM);
- } else {
- /* if the queue is empty, don't bother as we'd queue
- * it and inmediately unqueue it -- just deliver it */
- if (len == 0) {
- struct i2400m_roq_data *roq_data;
- roq_data = (struct i2400m_roq_data *) &skb->cb;
- i2400m_net_erx(i2400m, skb, roq_data->cs);
- }
- else
- __i2400m_roq_queue(i2400m, roq, skb, sn, nsn);
- __i2400m_roq_update_ws(i2400m, roq, sn + 1);
- i2400m_roq_log_add(i2400m, roq, I2400M_RO_TYPE_PACKET_WS,
- old_ws, len, sn, nsn, roq->ws);
- }
+ /* If the queue is empty, don't bother as we'd queue
+ * it and immediately unqueue it -- just deliver it.
+ */
+ if (len == 0) {
+ struct i2400m_roq_data *roq_data;
+ roq_data = (struct i2400m_roq_data *) &skb->cb;
+ i2400m_net_erx(i2400m, skb, roq_data->cs);
+ } else
+ __i2400m_roq_queue(i2400m, roq, skb, sn, nsn);
+
+ __i2400m_roq_update_ws(i2400m, roq, sn + 1);
+ i2400m_roq_log_add(i2400m, roq, I2400M_RO_TYPE_PACKET_WS,
+ old_ws, len, sn, nsn, roq->ws);
+
d_fnend(2, dev, "(i2400m %p roq %p skb %p sn %u) = void\n",
i2400m, roq, skb, sn);
return;
--
1.6.6.1
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [patch 2.6.35 18/25] wimax/i2400m: reserve additional space in the TX queue's buffer while allocating space for a new message header
2010-05-14 21:44 [patch 2.6.35 00/25] WiMAX pull request Inaky Perez-Gonzalez
` (16 preceding siblings ...)
2010-05-14 21:45 ` [patch 2.6.35 17/25] wimax/i2400m: fix incorrect handling of type 2 and 3 RX messages Inaky Perez-Gonzalez
@ 2010-05-14 21:45 ` Inaky Perez-Gonzalez
2010-05-14 21:45 ` [patch 2.6.35 19/25] wimax/i2400m: SDIO specific TX queue's minimum buffer room for new message Inaky Perez-Gonzalez
` (7 subsequent siblings)
25 siblings, 0 replies; 27+ messages in thread
From: Inaky Perez-Gonzalez @ 2010-05-14 21:45 UTC (permalink / raw)
To: netdev, wimax; +Cc: Prasanna S. Panchamukhi
From: Prasanna S. Panchamukhi <prasannax.s.panchamukhi@intel.com>
Increase the possibilities of including at least one payload by reserving
some additional space in the TX queue while allocating TX queue's space
for new message header. Please refer the documentation in the code for details.
Signed-off-by: Prasanna S. Panchamukhi <prasannax.s.panchamukhi@intel.com>
---
drivers/net/wimax/i2400m/i2400m.h | 6 ++++++
drivers/net/wimax/i2400m/tx.c | 11 ++++++++++-
2 files changed, 16 insertions(+), 1 deletions(-)
diff --git a/drivers/net/wimax/i2400m/i2400m.h b/drivers/net/wimax/i2400m/i2400m.h
index b8c7dbf..1babc55 100644
--- a/drivers/net/wimax/i2400m/i2400m.h
+++ b/drivers/net/wimax/i2400m/i2400m.h
@@ -242,6 +242,11 @@ struct i2400m_barker_db;
* so we have a tx_blk_size variable that the bus layer sets to
* tell the engine how much of that we need.
*
+ * @bus_tx_room_min: [fill] Minimum room required while allocating
+ * TX queue's buffer space for message header. SDIO requires
+ * 224 bytes and USB 16 bytes. Refer bus specific driver code
+ * for details.
+ *
* @bus_pl_size_max: [fill] Maximum payload size.
*
* @bus_setup: [optional fill] Function called by the bus-generic code
@@ -573,6 +578,7 @@ struct i2400m {
wait_queue_head_t state_wq; /* Woken up when on state updates */
size_t bus_tx_block_size;
+ size_t bus_tx_room_min;
size_t bus_pl_size_max;
unsigned bus_bm_retries;
diff --git a/drivers/net/wimax/i2400m/tx.c b/drivers/net/wimax/i2400m/tx.c
index 609f1ca..3f819ef 100644
--- a/drivers/net/wimax/i2400m/tx.c
+++ b/drivers/net/wimax/i2400m/tx.c
@@ -563,8 +563,17 @@ void i2400m_tx_new(struct i2400m *i2400m)
struct i2400m_msg_hdr *tx_msg;
bool try_head = 0;
BUG_ON(i2400m->tx_msg != NULL);
+ /*
+ * In certain situations, TX queue might have enough space to
+ * accommodate the new message header I2400M_TX_PLD_SIZE, but
+ * might not have enough space to accommodate the payloads.
+ * Adding bus_tx_room_min padding while allocating a new TX message
+ * increases the possibilities of including at least one payload of the
+ * size <= bus_tx_room_min.
+ */
try_head:
- tx_msg = i2400m_tx_fifo_push(i2400m, I2400M_TX_PLD_SIZE, 0, try_head);
+ tx_msg = i2400m_tx_fifo_push(i2400m, I2400M_TX_PLD_SIZE,
+ i2400m->bus_tx_room_min, try_head);
if (tx_msg == NULL)
goto out;
else if (tx_msg == TAIL_FULL) {
--
1.6.6.1
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [patch 2.6.35 19/25] wimax/i2400m: SDIO specific TX queue's minimum buffer room for new message
2010-05-14 21:44 [patch 2.6.35 00/25] WiMAX pull request Inaky Perez-Gonzalez
` (17 preceding siblings ...)
2010-05-14 21:45 ` [patch 2.6.35 18/25] wimax/i2400m: reserve additional space in the TX queue's buffer while allocating space for a new message header Inaky Perez-Gonzalez
@ 2010-05-14 21:45 ` Inaky Perez-Gonzalez
2010-05-14 21:45 ` [patch 2.6.35 20/25] wimax/i2400m: USB specific TX queue's minimum buffer room required " Inaky Perez-Gonzalez
` (6 subsequent siblings)
25 siblings, 0 replies; 27+ messages in thread
From: Inaky Perez-Gonzalez @ 2010-05-14 21:45 UTC (permalink / raw)
To: netdev, wimax; +Cc: Prasanna S. Panchamukhi
From: Prasanna S. Panchamukhi <prasannax.s.panchamukhi@intel.com>
This patch specifies the TX queue's minimum buffer room required to
accommodate one smallest SDIO payload.
Please refer the documentation in the code.
Signed-off-by: Prasanna S. Panchamukhi <prasannax.s.panchamukhi@intel.com>
---
drivers/net/wimax/i2400m/sdio.c | 7 +++++++
1 files changed, 7 insertions(+), 0 deletions(-)
diff --git a/drivers/net/wimax/i2400m/sdio.c b/drivers/net/wimax/i2400m/sdio.c
index 7632f80..9bfc26e 100644
--- a/drivers/net/wimax/i2400m/sdio.c
+++ b/drivers/net/wimax/i2400m/sdio.c
@@ -483,6 +483,13 @@ int i2400ms_probe(struct sdio_func *func,
sdio_set_drvdata(func, i2400ms);
i2400m->bus_tx_block_size = I2400MS_BLK_SIZE;
+ /*
+ * Room required in the TX queue for SDIO message to accommodate
+ * a smallest payload while allocating header space is 224 bytes,
+ * which is the smallest message size(the block size 256 bytes)
+ * minus the smallest message header size(32 bytes).
+ */
+ i2400m->bus_tx_room_min = I2400MS_BLK_SIZE - I2400M_PL_ALIGN * 2;
i2400m->bus_pl_size_max = I2400MS_PL_SIZE_MAX;
i2400m->bus_setup = i2400ms_bus_setup;
i2400m->bus_dev_start = i2400ms_bus_dev_start;
--
1.6.6.1
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [patch 2.6.35 20/25] wimax/i2400m: USB specific TX queue's minimum buffer room required for new message
2010-05-14 21:44 [patch 2.6.35 00/25] WiMAX pull request Inaky Perez-Gonzalez
` (18 preceding siblings ...)
2010-05-14 21:45 ` [patch 2.6.35 19/25] wimax/i2400m: SDIO specific TX queue's minimum buffer room for new message Inaky Perez-Gonzalez
@ 2010-05-14 21:45 ` Inaky Perez-Gonzalez
2010-05-14 21:45 ` [patch 2.6.35 21/25] wimax: checking ERR_PTR vs null Inaky Perez-Gonzalez
` (5 subsequent siblings)
25 siblings, 0 replies; 27+ messages in thread
From: Inaky Perez-Gonzalez @ 2010-05-14 21:45 UTC (permalink / raw)
To: netdev, wimax; +Cc: Prasanna S. Panchamukhi
From: Prasanna S. Panchamukhi <prasannax.s.panchamukhi@intel.com>
This patch specifies the TX queue's buffer room required by the
USB bus driver while allocating header space for a new message.
Please refer the documentation in the code.
Signed-off-by: Prasanna S. Panchamukhi <prasannax.s.panchamukhi@intel.com>
---
drivers/net/wimax/i2400m/usb.c | 7 +++++++
1 files changed, 7 insertions(+), 0 deletions(-)
diff --git a/drivers/net/wimax/i2400m/usb.c b/drivers/net/wimax/i2400m/usb.c
index d8c4d64..6fd8cf5 100644
--- a/drivers/net/wimax/i2400m/usb.c
+++ b/drivers/net/wimax/i2400m/usb.c
@@ -467,6 +467,13 @@ int i2400mu_probe(struct usb_interface *iface,
usb_set_intfdata(iface, i2400mu);
i2400m->bus_tx_block_size = I2400MU_BLK_SIZE;
+ /*
+ * Room required in the Tx queue for USB message to accommodate
+ * a smallest payload while allocating header space is 16 bytes.
+ * Adding this room for the new tx message increases the
+ * possibilities of including any payload with size <= 16 bytes.
+ */
+ i2400m->bus_tx_room_min = I2400MU_BLK_SIZE;
i2400m->bus_pl_size_max = I2400MU_PL_SIZE_MAX;
i2400m->bus_setup = NULL;
i2400m->bus_dev_start = i2400mu_bus_dev_start;
--
1.6.6.1
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [patch 2.6.35 21/25] wimax: checking ERR_PTR vs null
2010-05-14 21:44 [patch 2.6.35 00/25] WiMAX pull request Inaky Perez-Gonzalez
` (19 preceding siblings ...)
2010-05-14 21:45 ` [patch 2.6.35 20/25] wimax/i2400m: USB specific TX queue's minimum buffer room required " Inaky Perez-Gonzalez
@ 2010-05-14 21:45 ` Inaky Perez-Gonzalez
2010-05-14 21:45 ` [patch 2.6.35 22/25] wimax: wimax_msg_alloc() returns ERR_PTR not null Inaky Perez-Gonzalez
` (4 subsequent siblings)
25 siblings, 0 replies; 27+ messages in thread
From: Inaky Perez-Gonzalez @ 2010-05-14 21:45 UTC (permalink / raw)
To: netdev, wimax; +Cc: Dan Carpenter
From: Dan Carpenter <error27@gmail.com>
stch_skb is allocated with wimax_gnl_re_state_change_alloc(). That
function returns ERR_PTRs on failure and doesn't return NULL.
Signed-off-by: Dan Carpenter <error27@gmail.com>
---
net/wimax/stack.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/net/wimax/stack.c b/net/wimax/stack.c
index 1ed65db..62b1a66 100644
--- a/net/wimax/stack.c
+++ b/net/wimax/stack.c
@@ -315,7 +315,7 @@ void __wimax_state_change(struct wimax_dev *wimax_dev, enum wimax_st new_state)
BUG();
}
__wimax_state_set(wimax_dev, new_state);
- if (stch_skb)
+ if (!IS_ERR(stch_skb))
wimax_gnl_re_state_change_send(wimax_dev, stch_skb, header);
out:
d_fnend(3, dev, "(wimax_dev %p new_state %u [old %u]) = void\n",
--
1.6.6.1
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [patch 2.6.35 22/25] wimax: wimax_msg_alloc() returns ERR_PTR not null
2010-05-14 21:44 [patch 2.6.35 00/25] WiMAX pull request Inaky Perez-Gonzalez
` (20 preceding siblings ...)
2010-05-14 21:45 ` [patch 2.6.35 21/25] wimax: checking ERR_PTR vs null Inaky Perez-Gonzalez
@ 2010-05-14 21:45 ` Inaky Perez-Gonzalez
2010-05-14 21:45 ` [patch 2.6.35 23/25] wimax/i2400m: Move module params to other file so they can be static Inaky Perez-Gonzalez
` (3 subsequent siblings)
25 siblings, 0 replies; 27+ messages in thread
From: Inaky Perez-Gonzalez @ 2010-05-14 21:45 UTC (permalink / raw)
To: netdev, wimax; +Cc: Dan Carpenter
From: Dan Carpenter <error27@gmail.com>
wimax_msg_alloc() returns an ERR_PTR and not null. I changed it to test
for ERR_PTR instead of null. I also added a check in front of the
kfree() because kfree() can handle null but not ERR_PTR.
Signed-off-by: Dan Carpenter <error27@gmail.com>
---
drivers/net/wimax/i2400m/rx.c | 9 ++++-----
1 files changed, 4 insertions(+), 5 deletions(-)
diff --git a/drivers/net/wimax/i2400m/rx.c b/drivers/net/wimax/i2400m/rx.c
index 66f968a..0004c68 100644
--- a/drivers/net/wimax/i2400m/rx.c
+++ b/drivers/net/wimax/i2400m/rx.c
@@ -300,17 +300,16 @@ void i2400m_rx_ctl_ack(struct i2400m *i2400m,
d_printf(1, dev, "Huh? waiter for command reply cancelled\n");
goto error_waiter_cancelled;
}
- if (ack_skb == NULL) {
+ if (IS_ERR(ack_skb))
dev_err(dev, "CMD/GET/SET ack: cannot allocate SKB\n");
- i2400m->ack_skb = ERR_PTR(-ENOMEM);
- } else
- i2400m->ack_skb = ack_skb;
+ i2400m->ack_skb = ack_skb;
spin_unlock_irqrestore(&i2400m->rx_lock, flags);
complete(&i2400m->msg_completion);
return;
error_waiter_cancelled:
- kfree_skb(ack_skb);
+ if (!IS_ERR(ack_skb))
+ kfree_skb(ack_skb);
error_no_waiter:
spin_unlock_irqrestore(&i2400m->rx_lock, flags);
return;
--
1.6.6.1
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [patch 2.6.35 23/25] wimax/i2400m: Move module params to other file so they can be static
2010-05-14 21:44 [patch 2.6.35 00/25] WiMAX pull request Inaky Perez-Gonzalez
` (21 preceding siblings ...)
2010-05-14 21:45 ` [patch 2.6.35 22/25] wimax: wimax_msg_alloc() returns ERR_PTR not null Inaky Perez-Gonzalez
@ 2010-05-14 21:45 ` Inaky Perez-Gonzalez
2010-05-14 21:45 ` [patch 2.6.35 24/25] wimax/i2400m: driver defaults to firmware v1.5 for i5x50 devices Inaky Perez-Gonzalez
` (2 subsequent siblings)
25 siblings, 0 replies; 27+ messages in thread
From: Inaky Perez-Gonzalez @ 2010-05-14 21:45 UTC (permalink / raw)
To: netdev, wimax; +Cc: Prasanna S Panchamukhi
From: Prasanna S Panchamukhi <prasannax.s.panchamukhi@intel.com>
This patch moves the module parameters to the file where they
can be avoided to be global and allow them to be static.
The module param : idle_mode_disabled and power_save_disabled
are moved from driver.c to control.c. Also these module parameters
are declared to be static as they are not required to be global anymore.
The module param : rx_reorder_disabled is moved from driver.c file to
rx.c file. Also this parameter is declated as static as it is not
required to be global anymore.
Signed-off-by: Prasanna S Panchamukhi<prasannax.s.panchamukhi@intel.com>
---
drivers/net/wimax/i2400m/control.c | 15 +++++++++++++++
drivers/net/wimax/i2400m/driver.c | 19 -------------------
drivers/net/wimax/i2400m/i2400m.h | 6 ------
drivers/net/wimax/i2400m/rx.c | 5 +++++
4 files changed, 20 insertions(+), 25 deletions(-)
diff --git a/drivers/net/wimax/i2400m/control.c b/drivers/net/wimax/i2400m/control.c
index 6180772..0c1aa88 100644
--- a/drivers/net/wimax/i2400m/control.c
+++ b/drivers/net/wimax/i2400m/control.c
@@ -83,6 +83,21 @@
#define D_SUBMODULE control
#include "debug-levels.h"
+static int i2400m_idle_mode_disabled;/* 0 (idle mode enabled) by default */
+module_param_named(idle_mode_disabled, i2400m_idle_mode_disabled, int, 0644);
+MODULE_PARM_DESC(idle_mode_disabled,
+ "If true, the device will not enable idle mode negotiation "
+ "with the base station (when connected) to save power.");
+
+/* 0 (power saving enabled) by default */
+static int i2400m_power_save_disabled;
+module_param_named(power_save_disabled, i2400m_power_save_disabled, int, 0644);
+MODULE_PARM_DESC(power_save_disabled,
+ "If true, the driver will not tell the device to enter "
+ "power saving mode when it reports it is ready for it. "
+ "False by default (so the device is told to do power "
+ "saving).");
+
int i2400m_passive_mode; /* 0 (passive mode disabled) by default */
module_param_named(passive_mode, i2400m_passive_mode, int, 0644);
MODULE_PARM_DESC(passive_mode,
diff --git a/drivers/net/wimax/i2400m/driver.c b/drivers/net/wimax/i2400m/driver.c
index 39cf96a..66bdb5d 100644
--- a/drivers/net/wimax/i2400m/driver.c
+++ b/drivers/net/wimax/i2400m/driver.c
@@ -75,25 +75,6 @@
#include "debug-levels.h"
-int i2400m_idle_mode_disabled; /* 0 (idle mode enabled) by default */
-module_param_named(idle_mode_disabled, i2400m_idle_mode_disabled, int, 0644);
-MODULE_PARM_DESC(idle_mode_disabled,
- "If true, the device will not enable idle mode negotiation "
- "with the base station (when connected) to save power.");
-
-int i2400m_rx_reorder_disabled; /* 0 (rx reorder enabled) by default */
-module_param_named(rx_reorder_disabled, i2400m_rx_reorder_disabled, int, 0644);
-MODULE_PARM_DESC(rx_reorder_disabled,
- "If true, RX reordering will be disabled.");
-
-int i2400m_power_save_disabled; /* 0 (power saving enabled) by default */
-module_param_named(power_save_disabled, i2400m_power_save_disabled, int, 0644);
-MODULE_PARM_DESC(power_save_disabled,
- "If true, the driver will not tell the device to enter "
- "power saving mode when it reports it is ready for it. "
- "False by default (so the device is told to do power "
- "saving).");
-
static char i2400m_debug_params[128];
module_param_string(debug, i2400m_debug_params, sizeof(i2400m_debug_params),
0644);
diff --git a/drivers/net/wimax/i2400m/i2400m.h b/drivers/net/wimax/i2400m/i2400m.h
index 1babc55..fa74777 100644
--- a/drivers/net/wimax/i2400m/i2400m.h
+++ b/drivers/net/wimax/i2400m/i2400m.h
@@ -885,7 +885,6 @@ extern int i2400m_rx(struct i2400m *, struct sk_buff *);
extern struct i2400m_msg_hdr *i2400m_tx_msg_get(struct i2400m *, size_t *);
extern void i2400m_tx_msg_sent(struct i2400m *);
-extern int i2400m_power_save_disabled;
/*
* Utility functions
@@ -992,10 +991,5 @@ extern int i2400m_barker_db_init(const char *);
extern void i2400m_barker_db_exit(void);
-/* Module parameters */
-
-extern int i2400m_idle_mode_disabled;
-extern int i2400m_rx_reorder_disabled;
-
#endif /* #ifndef __I2400M_H__ */
diff --git a/drivers/net/wimax/i2400m/rx.c b/drivers/net/wimax/i2400m/rx.c
index 0004c68..c835ae8 100644
--- a/drivers/net/wimax/i2400m/rx.c
+++ b/drivers/net/wimax/i2400m/rx.c
@@ -155,6 +155,11 @@
#define D_SUBMODULE rx
#include "debug-levels.h"
+static int i2400m_rx_reorder_disabled; /* 0 (rx reorder enabled) by default */
+module_param_named(rx_reorder_disabled, i2400m_rx_reorder_disabled, int, 0644);
+MODULE_PARM_DESC(rx_reorder_disabled,
+ "If true, RX reordering will be disabled.");
+
struct i2400m_report_hook_args {
struct sk_buff *skb_rx;
const struct i2400m_l3l4_hdr *l3l4_hdr;
--
1.6.6.1
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [patch 2.6.35 24/25] wimax/i2400m: driver defaults to firmware v1.5 for i5x50 devices
2010-05-14 21:44 [patch 2.6.35 00/25] WiMAX pull request Inaky Perez-Gonzalez
` (22 preceding siblings ...)
2010-05-14 21:45 ` [patch 2.6.35 23/25] wimax/i2400m: Move module params to other file so they can be static Inaky Perez-Gonzalez
@ 2010-05-14 21:45 ` Inaky Perez-Gonzalez
2010-05-14 21:45 ` [patch 2.6.35 25/25] wimax/i2400m: driver defaults to firmware v1.5 for i6x60 devices Inaky Perez-Gonzalez
2010-05-16 6:24 ` [patch 2.6.35 00/25] WiMAX pull request David Miller
25 siblings, 0 replies; 27+ messages in thread
From: Inaky Perez-Gonzalez @ 2010-05-14 21:45 UTC (permalink / raw)
To: netdev, wimax
Updates the i2400m driver to default to firmware versions v1.5 for the
Intel Wireless WiMAX Connection 5150 and 5350 devices.
Firmware available in linux-firmware.
Signed-off-by: Inaky Perez-Gonzalez <inaky@linux.intel.com>
---
drivers/net/wimax/i2400m/usb.c | 4 +++-
1 files changed, 3 insertions(+), 1 deletions(-)
diff --git a/drivers/net/wimax/i2400m/usb.c b/drivers/net/wimax/i2400m/usb.c
index 6fd8cf5..f456807 100644
--- a/drivers/net/wimax/i2400m/usb.c
+++ b/drivers/net/wimax/i2400m/usb.c
@@ -82,6 +82,8 @@ MODULE_PARM_DESC(debug,
/* Our firmware file name */
static const char *i2400mu_bus_fw_names_5x50[] = {
+#define I2400MU_FW_FILE_NAME_v1_5 "i2400m-fw-usb-1.5.sbcf"
+ I2400MU_FW_FILE_NAME_v1_5,
#define I2400MU_FW_FILE_NAME_v1_4 "i2400m-fw-usb-1.4.sbcf"
I2400MU_FW_FILE_NAME_v1_4,
NULL,
@@ -785,4 +787,4 @@ MODULE_AUTHOR("Intel Corporation <linux-wimax@intel.com>");
MODULE_DESCRIPTION("Driver for USB based Intel Wireless WiMAX Connection 2400M "
"(5x50 & 6050)");
MODULE_LICENSE("GPL");
-MODULE_FIRMWARE(I2400MU_FW_FILE_NAME_v1_4);
+MODULE_FIRMWARE(I2400MU_FW_FILE_NAME_v1_5);
--
1.6.6.1
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [patch 2.6.35 25/25] wimax/i2400m: driver defaults to firmware v1.5 for i6x60 devices
2010-05-14 21:44 [patch 2.6.35 00/25] WiMAX pull request Inaky Perez-Gonzalez
` (23 preceding siblings ...)
2010-05-14 21:45 ` [patch 2.6.35 24/25] wimax/i2400m: driver defaults to firmware v1.5 for i5x50 devices Inaky Perez-Gonzalez
@ 2010-05-14 21:45 ` Inaky Perez-Gonzalez
2010-05-16 6:24 ` [patch 2.6.35 00/25] WiMAX pull request David Miller
25 siblings, 0 replies; 27+ messages in thread
From: Inaky Perez-Gonzalez @ 2010-05-14 21:45 UTC (permalink / raw)
To: netdev, wimax
Firmware is available in the linux-firmware package.
Signed-off-by: Inaky Perez-Gonzalez <inaky@linux.intel.com>
---
drivers/net/wimax/i2400m/usb.c | 1 +
1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/drivers/net/wimax/i2400m/usb.c b/drivers/net/wimax/i2400m/usb.c
index f456807..16341ff 100644
--- a/drivers/net/wimax/i2400m/usb.c
+++ b/drivers/net/wimax/i2400m/usb.c
@@ -788,3 +788,4 @@ MODULE_DESCRIPTION("Driver for USB based Intel Wireless WiMAX Connection 2400M "
"(5x50 & 6050)");
MODULE_LICENSE("GPL");
MODULE_FIRMWARE(I2400MU_FW_FILE_NAME_v1_5);
+MODULE_FIRMWARE(I6050U_FW_FILE_NAME_v1_5);
--
1.6.6.1
^ permalink raw reply related [flat|nested] 27+ messages in thread
* Re: [patch 2.6.35 00/25] WiMAX pull request
2010-05-14 21:44 [patch 2.6.35 00/25] WiMAX pull request Inaky Perez-Gonzalez
` (24 preceding siblings ...)
2010-05-14 21:45 ` [patch 2.6.35 25/25] wimax/i2400m: driver defaults to firmware v1.5 for i6x60 devices Inaky Perez-Gonzalez
@ 2010-05-16 6:24 ` David Miller
25 siblings, 0 replies; 27+ messages in thread
From: David Miller @ 2010-05-16 6:24 UTC (permalink / raw)
To: inaky; +Cc: netdev, wimax, inaky.perez-gonzalez
From: Inaky Perez-Gonzalez <inaky@linux.intel.com>
Date: Fri, 14 May 2010 14:44:59 -0700
> The following changes since commit 2b0b05ddc04b6d45e71cd36405df512075786f1e:
> Marcel Holtmann (1):
> Bluetooth: Fix issues where sk_sleep() helper is needed now
>
> are available in the git repository at:
>
> git://git.kernel.org/pub/scm/linux/kernel/git/inaky/wimax.git master
>
> Patches follow for reviewing convenience.
Pulled, thanks a lot.
^ permalink raw reply [flat|nested] 27+ messages in thread
end of thread, other threads:[~2010-05-16 6:24 UTC | newest]
Thread overview: 27+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-05-14 21:44 [patch 2.6.35 00/25] WiMAX pull request Inaky Perez-Gonzalez
2010-05-14 21:45 ` [patch 2.6.35 01/25] wimax/i2400m: fix incorrect return -ESHUTDOWN when there is no Tx buffer available Inaky Perez-Gonzalez
2010-05-14 21:45 ` [patch 2.6.35 02/25] wimax/i2400m: move I2400M_MAX_MTU enum from netdev.c to i2400m.h Inaky Perez-Gonzalez
2010-05-14 21:45 ` [patch 2.6.35 03/25] wimax/i2400m: fix insufficient size of Tx buffer for 12 payload of 1400 MTU Inaky Perez-Gonzalez
2010-05-14 21:45 ` [patch 2.6.35 04/25] wimax/i2400m: fix the race condition for accessing TX queue Inaky Perez-Gonzalez
2010-05-14 21:45 ` [patch 2.6.35 05/25] wimax/i2400m: correct the error path handlers in dev_start() Inaky Perez-Gonzalez
2010-05-14 21:45 ` [patch 2.6.35 06/25] wimax/i2400m: fix for missed reset events if triggered by dev_reset_handle() Inaky Perez-Gonzalez
2010-05-14 21:45 ` [patch 2.6.35 07/25] wimax/i2400m: add the error recovery mechanism on TX path Inaky Perez-Gonzalez
2010-05-14 21:45 ` [patch 2.6.35 08/25] wimax/i2400m: Correct the error path handlers order in i2400m_post_reset() Inaky Perez-Gonzalez
2010-05-14 21:45 ` [patch 2.6.35 09/25] wimax/i2400m: Reset the TX FIFO indices when allocating the TX FIFO in tx_setup() Inaky Perez-Gonzalez
2010-05-14 21:45 ` [patch 2.6.35 10/25] wimax/i2400m: increase the maximum number of payloads per message to 60 [v1] Inaky Perez-Gonzalez
2010-05-14 21:45 ` [patch 2.6.35 11/25] wimax/i2400m: limit the message size upto 16KiB [v1] Inaky Perez-Gonzalez
2010-05-14 21:45 ` [patch 2.6.35 12/25] wimax/i2400m: fix BUILD_BUG_ON() to use the maximum message size constant [v1] Inaky Perez-Gonzalez
2010-05-14 21:45 ` [patch 2.6.35 13/25] wimax/i2400m: modify i2400m_tx_fifo_push() to check for head room space in the TX FIFO [v1] Inaky Perez-Gonzalez
2010-05-14 21:45 ` [patch 2.6.35 14/25] wimax/i2400m: fix system freeze caused by an infinite loop [v1] Inaky Perez-Gonzalez
2010-05-14 21:45 ` [patch 2.6.35 15/25] wimax/i2400m: increase tx queue length from 5 to 20 [v1] Inaky Perez-Gonzalez
2010-05-14 21:45 ` [patch 2.6.35 16/25] wimax i2400m: fix race condition while accessing rx_roq by using kref count Inaky Perez-Gonzalez
2010-05-14 21:45 ` [patch 2.6.35 17/25] wimax/i2400m: fix incorrect handling of type 2 and 3 RX messages Inaky Perez-Gonzalez
2010-05-14 21:45 ` [patch 2.6.35 18/25] wimax/i2400m: reserve additional space in the TX queue's buffer while allocating space for a new message header Inaky Perez-Gonzalez
2010-05-14 21:45 ` [patch 2.6.35 19/25] wimax/i2400m: SDIO specific TX queue's minimum buffer room for new message Inaky Perez-Gonzalez
2010-05-14 21:45 ` [patch 2.6.35 20/25] wimax/i2400m: USB specific TX queue's minimum buffer room required " Inaky Perez-Gonzalez
2010-05-14 21:45 ` [patch 2.6.35 21/25] wimax: checking ERR_PTR vs null Inaky Perez-Gonzalez
2010-05-14 21:45 ` [patch 2.6.35 22/25] wimax: wimax_msg_alloc() returns ERR_PTR not null Inaky Perez-Gonzalez
2010-05-14 21:45 ` [patch 2.6.35 23/25] wimax/i2400m: Move module params to other file so they can be static Inaky Perez-Gonzalez
2010-05-14 21:45 ` [patch 2.6.35 24/25] wimax/i2400m: driver defaults to firmware v1.5 for i5x50 devices Inaky Perez-Gonzalez
2010-05-14 21:45 ` [patch 2.6.35 25/25] wimax/i2400m: driver defaults to firmware v1.5 for i6x60 devices Inaky Perez-Gonzalez
2010-05-16 6:24 ` [patch 2.6.35 00/25] WiMAX pull request David Miller
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).