2011-06-20 11:42:31

by Jussi Kivilinna

[permalink] [raw]
Subject: [PATCH 1/6] zd1211rw: fix invalid signal values from device

Driver uses IEEE80211_HW_SIGNAL_UNSPEC and so signal values reported to
mac80211 should be in range 0..100. Sometimes device return out of range
values. These out of range values can then trigger warning in
cfg80211_inform_bss_frame.

This patch adds checks to enforce range returned from driver to
mac80211 be in 0..100 range.

Signed-off-by: Jussi Kivilinna <[email protected]>
---
drivers/net/wireless/zd1211rw/zd_def.h | 6 ++++++
drivers/net/wireless/zd1211rw/zd_mac.c | 20 ++++++++++++++++++--
2 files changed, 24 insertions(+), 2 deletions(-)

diff --git a/drivers/net/wireless/zd1211rw/zd_def.h b/drivers/net/wireless/zd1211rw/zd_def.h
index 5463ca9..9a1b013 100644
--- a/drivers/net/wireless/zd1211rw/zd_def.h
+++ b/drivers/net/wireless/zd1211rw/zd_def.h
@@ -37,9 +37,15 @@ typedef u16 __nocast zd_addr_t;
if (net_ratelimit()) \
dev_printk_f(KERN_DEBUG, dev, fmt, ## args); \
} while (0)
+# define dev_dbg_f_cond(dev, cond, fmt, args...) ({ \
+ bool __cond = !!(cond); \
+ if (unlikely(__cond)) \
+ dev_printk_f(KERN_DEBUG, dev, fmt, ## args); \
+})
#else
# define dev_dbg_f(dev, fmt, args...) do { (void)(dev); } while (0)
# define dev_dbg_f_limit(dev, fmt, args...) do { (void)(dev); } while (0)
+# define dev_dbg_f_cond(dev, cond, fmt, args...) do { (void)(dev); } while (0)
#endif /* DEBUG */

#ifdef DEBUG
diff --git a/drivers/net/wireless/zd1211rw/zd_mac.c b/drivers/net/wireless/zd1211rw/zd_mac.c
index 5037c8b..5de28bf 100644
--- a/drivers/net/wireless/zd1211rw/zd_mac.c
+++ b/drivers/net/wireless/zd1211rw/zd_mac.c
@@ -160,6 +160,22 @@ static int zd_reg2alpha2(u8 regdomain, char *alpha2)
return 1;
}

+static int zd_check_signal(struct ieee80211_hw *hw, int signal)
+{
+ struct zd_mac *mac = zd_hw_mac(hw);
+
+ dev_dbg_f_cond(zd_mac_dev(mac), signal < 0 || signal > 100,
+ "%s: signal value from device not in range 0..100, "
+ "but %d.\n", __func__, signal);
+
+ if (signal < 0)
+ signal = 0;
+ else if (signal > 100)
+ signal = 100;
+
+ return signal;
+}
+
int zd_mac_preinit_hw(struct ieee80211_hw *hw)
{
int r;
@@ -461,7 +477,7 @@ static void zd_mac_tx_status(struct ieee80211_hw *hw, struct sk_buff *skb,
if (i<IEEE80211_TX_MAX_RATES)
info->status.rates[i].idx = -1; /* terminate */

- info->status.ack_signal = ackssi;
+ info->status.ack_signal = zd_check_signal(hw, ackssi);
ieee80211_tx_status_irqsafe(hw, skb);
}

@@ -982,7 +998,7 @@ int zd_mac_rx(struct ieee80211_hw *hw, const u8 *buffer, unsigned int length)

stats.freq = zd_channels[_zd_chip_get_channel(&mac->chip) - 1].center_freq;
stats.band = IEEE80211_BAND_2GHZ;
- stats.signal = status->signal_strength;
+ stats.signal = zd_check_signal(hw, status->signal_strength);

rate = zd_rx_rate(buffer, status);




2011-06-20 11:42:40

by Jussi Kivilinna

[permalink] [raw]
Subject: [PATCH 3/6] zd1211rw: only update HW beacon if new beacon differs from currect

Update HW beacon only when needed. This appears to make device work in AP-mode
(dtim_period=1) somewhat more stable.

Signed-off-by: Jussi Kivilinna <[email protected]>
---
drivers/net/wireless/zd1211rw/zd_mac.c | 76 +++++++++++++++++++++++++++-----
drivers/net/wireless/zd1211rw/zd_mac.h | 1
2 files changed, 64 insertions(+), 13 deletions(-)

diff --git a/drivers/net/wireless/zd1211rw/zd_mac.c b/drivers/net/wireless/zd1211rw/zd_mac.c
index 5de28bf..0828605 100644
--- a/drivers/net/wireless/zd1211rw/zd_mac.c
+++ b/drivers/net/wireless/zd1211rw/zd_mac.c
@@ -403,10 +403,8 @@ int zd_restore_settings(struct zd_mac *mac)
mac->type == NL80211_IFTYPE_AP) {
if (mac->vif != NULL) {
beacon = ieee80211_beacon_get(mac->hw, mac->vif);
- if (beacon) {
+ if (beacon)
zd_mac_config_beacon(mac->hw, beacon);
- kfree_skb(beacon);
- }
}

zd_set_beacon_interval(&mac->chip, beacon_interval,
@@ -680,6 +678,32 @@ static void cs_set_control(struct zd_mac *mac, struct zd_ctrlset *cs,
/* FIXME: Management frame? */
}

+static bool zd_mac_match_cur_beacon(struct zd_mac *mac, struct sk_buff *beacon)
+{
+ if (!mac->beacon.cur_beacon)
+ return false;
+
+ if (mac->beacon.cur_beacon->len != beacon->len)
+ return false;
+
+ return !memcmp(beacon->data, mac->beacon.cur_beacon->data, beacon->len);
+}
+
+static void zd_mac_free_cur_beacon_locked(struct zd_mac *mac)
+{
+ ZD_ASSERT(mutex_is_locked(&mac->chip.mutex));
+
+ kfree_skb(mac->beacon.cur_beacon);
+ mac->beacon.cur_beacon = NULL;
+}
+
+static void zd_mac_free_cur_beacon(struct zd_mac *mac)
+{
+ mutex_lock(&mac->chip.mutex);
+ zd_mac_free_cur_beacon_locked(mac);
+ mutex_unlock(&mac->chip.mutex);
+}
+
static int zd_mac_config_beacon(struct ieee80211_hw *hw, struct sk_buff *beacon)
{
struct zd_mac *mac = zd_hw_mac(hw);
@@ -690,13 +714,21 @@ static int zd_mac_config_beacon(struct ieee80211_hw *hw, struct sk_buff *beacon)
unsigned long end_jiffies, message_jiffies;
struct zd_ioreq32 *ioreqs;

+ mutex_lock(&mac->chip.mutex);
+
+ /* Check if hw already has this beacon. */
+ if (zd_mac_match_cur_beacon(mac, beacon)) {
+ r = 0;
+ goto out_nofree;
+ }
+
/* Alloc memory for full beacon write at once. */
num_cmds = 1 + zd_chip_is_zd1211b(&mac->chip) + full_len;
ioreqs = kmalloc(num_cmds * sizeof(struct zd_ioreq32), GFP_KERNEL);
- if (!ioreqs)
- return -ENOMEM;
-
- mutex_lock(&mac->chip.mutex);
+ if (!ioreqs) {
+ r = -ENOMEM;
+ goto out_nofree;
+ }

r = zd_iowrite32_locked(&mac->chip, 0, CR_BCN_FIFO_SEMAPHORE);
if (r < 0)
@@ -773,9 +805,19 @@ release_sema:
if (r < 0 || ret < 0) {
if (r >= 0)
r = ret;
+
+ /* We don't know if beacon was written successfully or not,
+ * so clear current. */
+ zd_mac_free_cur_beacon_locked(mac);
+
goto out;
}

+ /* Beacon has now been written successfully, update current. */
+ zd_mac_free_cur_beacon_locked(mac);
+ mac->beacon.cur_beacon = beacon;
+ beacon = NULL;
+
/* 802.11b/g 2.4G CCK 1Mb
* 802.11a, not yet implemented, uses different values (see GPL vendor
* driver)
@@ -783,11 +825,17 @@ release_sema:
r = zd_iowrite32_locked(&mac->chip, 0x00000400 | (full_len << 19),
CR_BCN_PLCP_CFG);
out:
- mutex_unlock(&mac->chip.mutex);
kfree(ioreqs);
+out_nofree:
+ kfree_skb(beacon);
+ mutex_unlock(&mac->chip.mutex);
+
return r;

reset_device:
+ zd_mac_free_cur_beacon_locked(mac);
+ kfree_skb(beacon);
+
mutex_unlock(&mac->chip.mutex);
kfree(ioreqs);

@@ -1073,6 +1121,8 @@ static void zd_op_remove_interface(struct ieee80211_hw *hw,
mac->vif = NULL;
zd_set_beacon_interval(&mac->chip, 0, 0, NL80211_IFTYPE_UNSPECIFIED);
zd_write_mac_addr(&mac->chip, NULL);
+
+ zd_mac_free_cur_beacon(mac);
}

static int zd_op_config(struct ieee80211_hw *hw, u32 changed)
@@ -1110,10 +1160,8 @@ static void zd_beacon_done(struct zd_mac *mac)
* Fetch next beacon so that tim_count is updated.
*/
beacon = ieee80211_beacon_get(mac->hw, mac->vif);
- if (beacon) {
+ if (beacon)
zd_mac_config_beacon(mac->hw, beacon);
- kfree_skb(beacon);
- }

spin_lock_irq(&mac->lock);
mac->beacon.last_update = jiffies;
@@ -1240,7 +1288,6 @@ static void zd_op_bss_info_changed(struct ieee80211_hw *hw,
zd_chip_disable_hwint(&mac->chip);
zd_mac_config_beacon(hw, beacon);
zd_chip_enable_hwint(&mac->chip);
- kfree_skb(beacon);
}
}

@@ -1390,8 +1437,9 @@ static void beacon_watchdog_handler(struct work_struct *work)

beacon = ieee80211_beacon_get(mac->hw, mac->vif);
if (beacon) {
+ zd_mac_free_cur_beacon(mac);
+
zd_mac_config_beacon(mac->hw, beacon);
- kfree_skb(beacon);
}

zd_set_beacon_interval(&mac->chip, interval, period, mac->type);
@@ -1426,6 +1474,8 @@ static void beacon_disable(struct zd_mac *mac)
{
dev_dbg_f(zd_mac_dev(mac), "\n");
cancel_delayed_work_sync(&mac->beacon.watchdog_work);
+
+ zd_mac_free_cur_beacon(mac);
}

#define LINK_LED_WORK_DELAY HZ
diff --git a/drivers/net/wireless/zd1211rw/zd_mac.h b/drivers/net/wireless/zd1211rw/zd_mac.h
index f8c93c3..c01eca8 100644
--- a/drivers/net/wireless/zd1211rw/zd_mac.h
+++ b/drivers/net/wireless/zd1211rw/zd_mac.h
@@ -165,6 +165,7 @@ struct housekeeping {

struct beacon {
struct delayed_work watchdog_work;
+ struct sk_buff *cur_beacon;
unsigned long last_update;
u16 interval;
u8 period;


2011-06-20 11:42:50

by Jussi Kivilinna

[permalink] [raw]
Subject: [PATCH 5/6] zd1211rw: don't let zd_mac_config_beacon() run too long from beacon interrupt handler

zd_mac_config_beacon() has only limited time to set up beacon when called from
beacon interrupt handler/worker. So do not let it retry acquiring beacon fifo
semaphore in interrupt handler. Beacon fifo semaphore should not be locked by
firmware anyway at this time, it's only locked when device is using/txing
beacon.

Signed-off-by: Jussi Kivilinna <[email protected]>
---
drivers/net/wireless/zd1211rw/zd_mac.c | 19 ++++++++++++-------
1 files changed, 12 insertions(+), 7 deletions(-)

diff --git a/drivers/net/wireless/zd1211rw/zd_mac.c b/drivers/net/wireless/zd1211rw/zd_mac.c
index 0828605..b67c52d 100644
--- a/drivers/net/wireless/zd1211rw/zd_mac.c
+++ b/drivers/net/wireless/zd1211rw/zd_mac.c
@@ -143,7 +143,7 @@ static void beacon_enable(struct zd_mac *mac);
static void beacon_disable(struct zd_mac *mac);
static void set_rts_cts(struct zd_mac *mac, unsigned int short_preamble);
static int zd_mac_config_beacon(struct ieee80211_hw *hw,
- struct sk_buff *beacon);
+ struct sk_buff *beacon, bool in_intr);

static int zd_reg2alpha2(u8 regdomain, char *alpha2)
{
@@ -404,7 +404,7 @@ int zd_restore_settings(struct zd_mac *mac)
if (mac->vif != NULL) {
beacon = ieee80211_beacon_get(mac->hw, mac->vif);
if (beacon)
- zd_mac_config_beacon(mac->hw, beacon);
+ zd_mac_config_beacon(mac->hw, beacon, false);
}

zd_set_beacon_interval(&mac->chip, beacon_interval,
@@ -704,7 +704,8 @@ static void zd_mac_free_cur_beacon(struct zd_mac *mac)
mutex_unlock(&mac->chip.mutex);
}

-static int zd_mac_config_beacon(struct ieee80211_hw *hw, struct sk_buff *beacon)
+static int zd_mac_config_beacon(struct ieee80211_hw *hw, struct sk_buff *beacon,
+ bool in_intr)
{
struct zd_mac *mac = zd_hw_mac(hw);
int r, ret, num_cmds, req_pos = 0;
@@ -736,6 +737,10 @@ static int zd_mac_config_beacon(struct ieee80211_hw *hw, struct sk_buff *beacon)
r = zd_ioread32_locked(&mac->chip, &tmp, CR_BCN_FIFO_SEMAPHORE);
if (r < 0)
goto release_sema;
+ if (in_intr && tmp & 0x2) {
+ r = -EBUSY;
+ goto release_sema;
+ }

end_jiffies = jiffies + HZ / 2; /*~500ms*/
message_jiffies = jiffies + HZ / 10; /*~100ms*/
@@ -790,7 +795,7 @@ release_sema:
end_jiffies = jiffies + HZ / 2; /*~500ms*/
ret = zd_iowrite32_locked(&mac->chip, 1, CR_BCN_FIFO_SEMAPHORE);
while (ret < 0) {
- if (time_is_before_eq_jiffies(end_jiffies)) {
+ if (in_intr || time_is_before_eq_jiffies(end_jiffies)) {
ret = -ETIMEDOUT;
break;
}
@@ -1161,7 +1166,7 @@ static void zd_beacon_done(struct zd_mac *mac)
*/
beacon = ieee80211_beacon_get(mac->hw, mac->vif);
if (beacon)
- zd_mac_config_beacon(mac->hw, beacon);
+ zd_mac_config_beacon(mac->hw, beacon, true);

spin_lock_irq(&mac->lock);
mac->beacon.last_update = jiffies;
@@ -1286,7 +1291,7 @@ static void zd_op_bss_info_changed(struct ieee80211_hw *hw,

if (beacon) {
zd_chip_disable_hwint(&mac->chip);
- zd_mac_config_beacon(hw, beacon);
+ zd_mac_config_beacon(hw, beacon, false);
zd_chip_enable_hwint(&mac->chip);
}
}
@@ -1439,7 +1444,7 @@ static void beacon_watchdog_handler(struct work_struct *work)
if (beacon) {
zd_mac_free_cur_beacon(mac);

- zd_mac_config_beacon(mac->hw, beacon);
+ zd_mac_config_beacon(mac->hw, beacon, false);
}

zd_set_beacon_interval(&mac->chip, interval, period, mac->type);


2011-06-20 11:42:36

by Jussi Kivilinna

[permalink] [raw]
Subject: [PATCH 2/6] zd1211rw: make 'handle_rx_packet: invalid, small RX packet' message debug-only

This message is should be debug-only as it tells almost nothing useful. It also
happens very often and just floods logs.

Signed-off-by: Jussi Kivilinna <[email protected]>
---
drivers/net/wireless/zd1211rw/zd_usb.c | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/wireless/zd1211rw/zd_usb.c b/drivers/net/wireless/zd1211rw/zd_usb.c
index 631194d..621c2cc 100644
--- a/drivers/net/wireless/zd1211rw/zd_usb.c
+++ b/drivers/net/wireless/zd1211rw/zd_usb.c
@@ -579,8 +579,8 @@ static void handle_rx_packet(struct zd_usb *usb, const u8 *buffer,

if (length < sizeof(struct rx_length_info)) {
/* It's not a complete packet anyhow. */
- printk("%s: invalid, small RX packet : %d\n",
- __func__, length);
+ dev_dbg_f(zd_usb_dev(usb), "invalid, small RX packet : %d\n",
+ length);
return;
}
length_info = (struct rx_length_info *)


2011-06-20 11:42:55

by Jussi Kivilinna

[permalink] [raw]
Subject: [PATCH 6/6] zd1211rw: detect stalled beacon interrupt faster

As USB_INT_ID_RETRY_FAILED can override USB_INT_ID_REGS, beacon interrupt
(CR_INTERRUPT) might be lost. Problem is that when device trigger CR_INTERRUPT
it disables HW interrupt. Now if USB_INT_ID_REGS with CR_INTERRUPT gets lost,
beacon interrupt stays disabled until beacon watchdog notices the stall. This
happen very often on heavy TX. Improve watchdog to trigger earlier, after three
missing beacon interrupts.

Signed-off-by: Jussi Kivilinna <[email protected]>
---
drivers/net/wireless/zd1211rw/zd_mac.c | 3 ++-
1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/drivers/net/wireless/zd1211rw/zd_mac.c b/drivers/net/wireless/zd1211rw/zd_mac.c
index b67c52d..cabfae1 100644
--- a/drivers/net/wireless/zd1211rw/zd_mac.c
+++ b/drivers/net/wireless/zd1211rw/zd_mac.c
@@ -1429,7 +1429,8 @@ static void beacon_watchdog_handler(struct work_struct *work)
spin_lock_irq(&mac->lock);
interval = mac->beacon.interval;
period = mac->beacon.period;
- timeout = mac->beacon.last_update + msecs_to_jiffies(interval) + HZ;
+ timeout = mac->beacon.last_update +
+ msecs_to_jiffies(interval * 1024 / 1000) * 3;
spin_unlock_irq(&mac->lock);

if (interval > 0 && time_is_before_jiffies(timeout)) {


2011-06-20 11:42:46

by Jussi Kivilinna

[permalink] [raw]
Subject: [PATCH 4/6] zd1211rw: handle lost read-reg interrupts

Device losses read-reg interrupts. By looking at usbmon it appears that
USB_INT_ID_RETRY_FAILED can override USB_INT_ID_REGS. This causes read
command to timeout, usually under heavy TX.

Fix by retrying read registers again if USB_INT_ID_RETRY_FAILED is received
while waiting for USB_INT_ID_REGS.

However USB_INT_ID_REGS is not always lost but is received after
USB_INT_ID_RETRY_FAILED and is usually received by the retried read
command. USB_INT_ID_REGS of the retry is then left unhandled and might
be received by next read command. Handle this by ignoring previous
USB_INT_ID_REGS that doesn't match current read command request.

Signed-off-by: Jussi Kivilinna <[email protected]>
---
drivers/net/wireless/zd1211rw/zd_usb.c | 125 +++++++++++++++++++++++++++-----
drivers/net/wireless/zd1211rw/zd_usb.h | 5 +
2 files changed, 109 insertions(+), 21 deletions(-)

diff --git a/drivers/net/wireless/zd1211rw/zd_usb.c b/drivers/net/wireless/zd1211rw/zd_usb.c
index 621c2cc..cf0d69d 100644
--- a/drivers/net/wireless/zd1211rw/zd_usb.c
+++ b/drivers/net/wireless/zd1211rw/zd_usb.c
@@ -111,6 +111,9 @@ MODULE_DEVICE_TABLE(usb, usb_ids);
#define FW_ZD1211_PREFIX "zd1211/zd1211_"
#define FW_ZD1211B_PREFIX "zd1211/zd1211b_"

+static bool check_read_regs(struct zd_usb *usb, struct usb_req_read_regs *req,
+ unsigned int count);
+
/* USB device initialization */
static void int_urb_complete(struct urb *urb);

@@ -365,6 +368,20 @@ exit:

#define urb_dev(urb) (&(urb)->dev->dev)

+static inline void handle_regs_int_override(struct urb *urb)
+{
+ struct zd_usb *usb = urb->context;
+ struct zd_usb_interrupt *intr = &usb->intr;
+
+ spin_lock(&intr->lock);
+ if (atomic_read(&intr->read_regs_enabled)) {
+ atomic_set(&intr->read_regs_enabled, 0);
+ intr->read_regs_int_overridden = 1;
+ complete(&intr->read_regs.completion);
+ }
+ spin_unlock(&intr->lock);
+}
+
static inline void handle_regs_int(struct urb *urb)
{
struct zd_usb *usb = urb->context;
@@ -383,25 +400,45 @@ static inline void handle_regs_int(struct urb *urb)
USB_MAX_EP_INT_BUFFER);
spin_unlock(&mac->lock);
schedule_work(&mac->process_intr);
- } else if (intr->read_regs_enabled) {
- intr->read_regs.length = len = urb->actual_length;
-
+ } else if (atomic_read(&intr->read_regs_enabled)) {
+ len = urb->actual_length;
+ intr->read_regs.length = urb->actual_length;
if (len > sizeof(intr->read_regs.buffer))
len = sizeof(intr->read_regs.buffer);
+
memcpy(intr->read_regs.buffer, urb->transfer_buffer, len);
- intr->read_regs_enabled = 0;
+
+ /* Sometimes USB_INT_ID_REGS is not overridden, but comes after
+ * USB_INT_ID_RETRY_FAILED. Read-reg retry then gets this
+ * delayed USB_INT_ID_REGS, but leaves USB_INT_ID_REGS of
+ * retry unhandled. Next read-reg command then might catch
+ * this wrong USB_INT_ID_REGS. Fix by ignoring wrong reads.
+ */
+ if (!check_read_regs(usb, intr->read_regs.req,
+ intr->read_regs.req_count))
+ goto out;
+
+ atomic_set(&intr->read_regs_enabled, 0);
+ intr->read_regs_int_overridden = 0;
complete(&intr->read_regs.completion);
+
goto out;
}

out:
spin_unlock(&intr->lock);
+
+ /* CR_INTERRUPT might override read_reg too. */
+ if (int_num == CR_INTERRUPT && atomic_read(&intr->read_regs_enabled))
+ handle_regs_int_override(urb);
}

static void int_urb_complete(struct urb *urb)
{
int r;
struct usb_int_header *hdr;
+ struct zd_usb *usb;
+ struct zd_usb_interrupt *intr;

switch (urb->status) {
case 0:
@@ -430,6 +467,14 @@ static void int_urb_complete(struct urb *urb)
goto resubmit;
}

+ /* USB_INT_ID_RETRY_FAILED triggered by tx-urb submit can override
+ * pending USB_INT_ID_REGS causing read command timeout.
+ */
+ usb = urb->context;
+ intr = &usb->intr;
+ if (hdr->id != USB_INT_ID_REGS && atomic_read(&intr->read_regs_enabled))
+ handle_regs_int_override(urb);
+
switch (hdr->id) {
case USB_INT_ID_REGS:
handle_regs_int(urb);
@@ -1129,6 +1174,7 @@ static inline void init_usb_interrupt(struct zd_usb *usb)
spin_lock_init(&intr->lock);
intr->interval = int_urb_interval(zd_usb_to_usbdev(usb));
init_completion(&intr->read_regs.completion);
+ atomic_set(&intr->read_regs_enabled, 0);
intr->read_regs.cr_int_addr = cpu_to_le16((u16)CR_INTERRUPT);
}

@@ -1563,12 +1609,16 @@ static int usb_int_regs_length(unsigned int count)
return sizeof(struct usb_int_regs) + count * sizeof(struct reg_data);
}

-static void prepare_read_regs_int(struct zd_usb *usb)
+static void prepare_read_regs_int(struct zd_usb *usb,
+ struct usb_req_read_regs *req,
+ unsigned int count)
{
struct zd_usb_interrupt *intr = &usb->intr;

spin_lock_irq(&intr->lock);
- intr->read_regs_enabled = 1;
+ atomic_set(&intr->read_regs_enabled, 1);
+ intr->read_regs.req = req;
+ intr->read_regs.req_count = count;
INIT_COMPLETION(intr->read_regs.completion);
spin_unlock_irq(&intr->lock);
}
@@ -1578,22 +1628,18 @@ static void disable_read_regs_int(struct zd_usb *usb)
struct zd_usb_interrupt *intr = &usb->intr;

spin_lock_irq(&intr->lock);
- intr->read_regs_enabled = 0;
+ atomic_set(&intr->read_regs_enabled, 0);
spin_unlock_irq(&intr->lock);
}

-static int get_results(struct zd_usb *usb, u16 *values,
- struct usb_req_read_regs *req, unsigned int count)
+static bool check_read_regs(struct zd_usb *usb, struct usb_req_read_regs *req,
+ unsigned int count)
{
- int r;
int i;
struct zd_usb_interrupt *intr = &usb->intr;
struct read_regs_int *rr = &intr->read_regs;
struct usb_int_regs *regs = (struct usb_int_regs *)rr->buffer;

- spin_lock_irq(&intr->lock);
-
- r = -EIO;
/* The created block size seems to be larger than expected.
* However results appear to be correct.
*/
@@ -1601,13 +1647,14 @@ static int get_results(struct zd_usb *usb, u16 *values,
dev_dbg_f(zd_usb_dev(usb),
"error: actual length %d less than expected %d\n",
rr->length, usb_int_regs_length(count));
- goto error_unlock;
+ return false;
}
+
if (rr->length > sizeof(rr->buffer)) {
dev_dbg_f(zd_usb_dev(usb),
"error: actual length %d exceeds buffer size %zu\n",
rr->length, sizeof(rr->buffer));
- goto error_unlock;
+ return false;
}

for (i = 0; i < count; i++) {
@@ -1617,8 +1664,39 @@ static int get_results(struct zd_usb *usb, u16 *values,
"rd[%d] addr %#06hx expected %#06hx\n", i,
le16_to_cpu(rd->addr),
le16_to_cpu(req->addr[i]));
- goto error_unlock;
+ return false;
}
+ }
+
+ return true;
+}
+
+static int get_results(struct zd_usb *usb, u16 *values,
+ struct usb_req_read_regs *req, unsigned int count,
+ bool *retry)
+{
+ int r;
+ int i;
+ struct zd_usb_interrupt *intr = &usb->intr;
+ struct read_regs_int *rr = &intr->read_regs;
+ struct usb_int_regs *regs = (struct usb_int_regs *)rr->buffer;
+
+ spin_lock_irq(&intr->lock);
+
+ r = -EIO;
+
+ /* Read failed because firmware bug? */
+ *retry = !!intr->read_regs_int_overridden;
+ if (*retry)
+ goto error_unlock;
+
+ if (!check_read_regs(usb, req, count)) {
+ dev_dbg_f(zd_usb_dev(usb), "error: invalid read regs\n");
+ goto error_unlock;
+ }
+
+ for (i = 0; i < count; i++) {
+ struct reg_data *rd = &regs->regs[i];
values[i] = le16_to_cpu(rd->value);
}

@@ -1631,11 +1709,11 @@ error_unlock:
int zd_usb_ioread16v(struct zd_usb *usb, u16 *values,
const zd_addr_t *addresses, unsigned int count)
{
- int r;
- int i, req_len, actual_req_len;
+ int r, i, req_len, actual_req_len, try_count = 0;
struct usb_device *udev;
struct usb_req_read_regs *req = NULL;
unsigned long timeout;
+ bool retry = false;

if (count < 1) {
dev_dbg_f(zd_usb_dev(usb), "error: count is zero\n");
@@ -1671,8 +1749,10 @@ int zd_usb_ioread16v(struct zd_usb *usb, u16 *values,
for (i = 0; i < count; i++)
req->addr[i] = cpu_to_le16((u16)addresses[i]);

+retry_read:
+ try_count++;
udev = zd_usb_to_usbdev(usb);
- prepare_read_regs_int(usb);
+ prepare_read_regs_int(usb, req, count);
r = zd_ep_regs_out_msg(udev, req, req_len, &actual_req_len, 50 /*ms*/);
if (r) {
dev_dbg_f(zd_usb_dev(usb),
@@ -1696,7 +1776,12 @@ int zd_usb_ioread16v(struct zd_usb *usb, u16 *values,
goto error;
}

- r = get_results(usb, values, req, count);
+ r = get_results(usb, values, req, count, &retry);
+ if (retry && try_count < 20) {
+ dev_dbg_f(zd_usb_dev(usb), "read retry, tries so far: %d\n",
+ try_count);
+ goto retry_read;
+ }
error:
return r;
}
diff --git a/drivers/net/wireless/zd1211rw/zd_usb.h b/drivers/net/wireless/zd1211rw/zd_usb.h
index bf94284..99193b4 100644
--- a/drivers/net/wireless/zd1211rw/zd_usb.h
+++ b/drivers/net/wireless/zd1211rw/zd_usb.h
@@ -144,6 +144,8 @@ struct usb_int_retry_fail {

struct read_regs_int {
struct completion completion;
+ struct usb_req_read_regs *req;
+ unsigned int req_count;
/* Stores the USB int structure and contains the USB address of the
* first requested register before request.
*/
@@ -169,7 +171,8 @@ struct zd_usb_interrupt {
void *buffer;
dma_addr_t buffer_dma;
int interval;
- u8 read_regs_enabled:1;
+ atomic_t read_regs_enabled;
+ u8 read_regs_int_overridden:1;
};

static inline struct usb_int_regs *get_read_regs(struct zd_usb_interrupt *intr)


2011-07-02 12:39:57

by Walter Goldens

[permalink] [raw]
Subject: Re: [PATCH 3/6] zd1211rw: only update HW beacon if new beacon differs from currect



--- On Mon, 6/20/11, Jussi Kivilinna <[email protected]> wrote:

> This appears to make device work in AP-mode
> (dtim_period=1) somewhat more stable.
>
> Signed-off-by: Jussi Kivilinna <[email protected]>

Jussi,

I ran a hostapd AP with a zd1211b device, and while the stability is relatively OK, throughput appears to be below-par. The most I was able to get was about 3.5 megabits. Do you experience any throughput issues yourself and what is the average speed you see when you transfer files?

Walter

2011-07-04 14:37:24

by Jussi Kivilinna

[permalink] [raw]
Subject: Re: [PATCH 3/6] zd1211rw: only update HW beacon if new beacon differs from currect

Quoting Walter Goldens <[email protected]>:

>
>
> --- On Mon, 6/20/11, Jussi Kivilinna <[email protected]> wrote:
>
>> This appears to make device work in AP-mode
>> (dtim_period=1) somewhat more stable.
>>
>> Signed-off-by: Jussi Kivilinna <[email protected]>
>
> Jussi,
>
> I ran a hostapd AP with a zd1211b device, and while the stability is
> relatively OK, throughput appears to be below-par. The most I was
> able to get was about 3.5 megabits. Do you experience any throughput
> issues yourself and what is the average speed you see when you
> transfer files?

I have throughput issue when doing transfers in both directions at
same time. I get 17-19 Mbps when transfering in one direction only.
However if I start download and upload at sametime, zd1211rw AP can
only do tx at about 3.5 Mbps when other end is not zd1211rw too. With
zd1211rw at both ends tx average 8-10 Mbps. Same problem exists when
using zd1211rw device on b43 AP.

-Jussi

>
> Walter
>