Two patches for two current issues in rtl8187:
- First it should implement conf_tx callback, instead of hardcoding some tx
queue values.
- The second one should make rate control work for 8187B sending correct
feedback about transmitted packet status, using one of the Bulk In endpoints
that provides this info. Unfortunately after recently rate control api rewrite
in my testing here I don't get anymore greater than 1M rate with pid, before I
would still get until 9M (and after some time falling to 1M, then going back
up in steps, and so on) on same environment, don't know if it's expected to
the new rate control api be more strict/have different behaviour, or if
something additional must be done in rtl8187 (the link/signal quality may be
too low and may be new rate control changes are more sensitive).
Please test and check if there are any additional issues.
Realtek 8187B has a receive command queue to feedback beacon interrupt
and transmitted packet status. Use it to feedback mac80211 about status
of transmitted packets. Unfortunately in the course of testing I found
that the sequence number reported by hardware includes entire sequence
control in a 12 bit only field, so a workaround is done to check only
lowest bits.
Signed-off-by: Herton Ronaldo Krzesinski <[email protected]>
---
drivers/net/wireless/rtl8187.h | 5 ++
drivers/net/wireless/rtl8187_dev.c | 127 +++++++++++++++++++++++++++++++++++-
2 files changed, 130 insertions(+), 2 deletions(-)
diff --git a/drivers/net/wireless/rtl8187.h b/drivers/net/wireless/rtl8187.h
index f09872e..c385407 100644
--- a/drivers/net/wireless/rtl8187.h
+++ b/drivers/net/wireless/rtl8187.h
@@ -113,6 +113,11 @@ struct rtl8187_priv {
u8 noise;
u8 slot_time;
u8 aifsn[4];
+ struct {
+ __le64 buf;
+ struct urb *urb;
+ struct sk_buff_head queue;
+ } b_tx_status;
};
void rtl8187_write_phy(struct ieee80211_hw *dev, u8 addr, u32 data);
diff --git a/drivers/net/wireless/rtl8187_dev.c b/drivers/net/wireless/rtl8187_dev.c
index 537ad64..71eade1 100644
--- a/drivers/net/wireless/rtl8187_dev.c
+++ b/drivers/net/wireless/rtl8187_dev.c
@@ -167,8 +167,27 @@ static void rtl8187_tx_cb(struct urb *urb)
skb_pull(skb, priv->is_rtl8187b ? sizeof(struct rtl8187b_tx_hdr) :
sizeof(struct rtl8187_tx_hdr));
ieee80211_tx_info_clear_status(info);
- info->flags |= IEEE80211_TX_STAT_ACK;
- ieee80211_tx_status_irqsafe(hw, skb);
+
+ if (!urb->status &&
+ !(info->flags & IEEE80211_TX_CTL_NO_ACK) &&
+ priv->is_rtl8187b) {
+ skb_queue_tail(&priv->b_tx_status.queue, skb);
+
+ /* queue is "full", discard last items */
+ while (skb_queue_len(&priv->b_tx_status.queue) > 5) {
+ struct sk_buff *old_skb;
+
+ dev_dbg(&priv->udev->dev,
+ "transmit status queue full\n");
+
+ old_skb = skb_dequeue(&priv->b_tx_status.queue);
+ ieee80211_tx_status_irqsafe(hw, old_skb);
+ }
+ } else {
+ if (!(info->flags & IEEE80211_TX_CTL_NO_ACK) && !urb->status)
+ info->flags |= IEEE80211_TX_STAT_ACK;
+ ieee80211_tx_status_irqsafe(hw, skb);
+ }
}
static int rtl8187_tx(struct ieee80211_hw *dev, struct sk_buff *skb)
@@ -394,6 +413,105 @@ static int rtl8187_init_urbs(struct ieee80211_hw *dev)
return 0;
}
+static void rtl8187b_status_cb(struct urb *urb)
+{
+ struct ieee80211_hw *hw = (struct ieee80211_hw *)urb->context;
+ struct rtl8187_priv *priv = hw->priv;
+ u64 val;
+ unsigned int cmd_type;
+
+ if (unlikely(urb->status)) {
+ usb_free_urb(urb);
+ return;
+ }
+
+ /*
+ * Read from status buffer:
+ *
+ * bits [30:31] = cmd type:
+ * - 0 indicates tx beacon interrupt
+ * - 1 indicates tx close descriptor
+ *
+ * In the case of tx beacon interrupt:
+ * [0:9] = Last Beacon CW
+ * [10:29] = reserved
+ * [30:31] = 00b
+ * [32:63] = Last Beacon TSF
+ *
+ * If it's tx close descriptor:
+ * [0:7] = Packet Retry Count
+ * [8:14] = RTS Retry Count
+ * [15] = TOK
+ * [16:27] = Sequence No
+ * [28] = LS
+ * [29] = FS
+ * [30:31] = 01b
+ * [32:47] = unused (reserved?)
+ * [48:63] = MAC Used Time
+ */
+ val = le64_to_cpu(priv->b_tx_status.buf);
+
+ cmd_type = (val >> 30) & 0x3;
+ if (cmd_type == 1) {
+ unsigned int pkt_rc, seq_no;
+ bool tok;
+ struct sk_buff *skb;
+ struct ieee80211_hdr *ieee80211hdr;
+ unsigned long flags;
+
+ pkt_rc = val & 0xFF;
+ tok = val & (1 << 15);
+ seq_no = (val >> 16) & 0xFFF;
+
+ spin_lock_irqsave(&priv->b_tx_status.queue.lock, flags);
+ skb_queue_reverse_walk(&priv->b_tx_status.queue, skb) {
+ ieee80211hdr = (struct ieee80211_hdr *)skb->data;
+
+ /*
+ * Instead of returning just the 12 bits of sequence
+ * number, hardware is returning entire sequence
+ * control, overflowing after some time. As a
+ * workaround, just consider the lower bits, and expect
+ * it's unlikely we wrongly ack some sent data
+ */
+ if ((le16_to_cpu(ieee80211hdr->seq_ctrl) & 0xFFF) == seq_no)
+ break;
+ }
+ if (skb != (struct sk_buff *) &priv->b_tx_status.queue) {
+ struct ieee80211_tx_info *info = IEEE80211_SKB_CB(skb);
+
+ __skb_unlink(skb, &priv->b_tx_status.queue);
+ if (tok)
+ info->flags |= IEEE80211_TX_STAT_ACK;
+ info->status.rates[0].count = pkt_rc + 1;
+
+ ieee80211_tx_status_irqsafe(hw, skb);
+ }
+ spin_unlock_irqrestore(&priv->b_tx_status.queue.lock, flags);
+ }
+
+ usb_submit_urb(urb, GFP_ATOMIC);
+}
+
+static int rtl8187b_init_status_urb(struct ieee80211_hw *dev)
+{
+ struct rtl8187_priv *priv = dev->priv;
+ struct urb *entry;
+
+ entry = usb_alloc_urb(0, GFP_KERNEL);
+ if (!entry)
+ return -ENOMEM;
+ priv->b_tx_status.urb = entry;
+
+ usb_fill_bulk_urb(entry, priv->udev, usb_rcvbulkpipe(priv->udev, 9),
+ &priv->b_tx_status.buf, sizeof(priv->b_tx_status.buf),
+ rtl8187b_status_cb, dev);
+
+ usb_submit_urb(entry, GFP_KERNEL);
+
+ return 0;
+}
+
static int rtl8187_cmd_reset(struct ieee80211_hw *dev)
{
struct rtl8187_priv *priv = dev->priv;
@@ -746,6 +864,7 @@ static int rtl8187_start(struct ieee80211_hw *dev)
(7 << 0 /* long retry limit */) |
(7 << 21 /* MAX TX DMA */));
rtl8187_init_urbs(dev);
+ rtl8187b_init_status_urb(dev);
mutex_unlock(&priv->conf_mutex);
return 0;
}
@@ -822,6 +941,9 @@ static void rtl8187_stop(struct ieee80211_hw *dev)
usb_kill_urb(info->urb);
kfree_skb(skb);
}
+ while ((skb = skb_dequeue(&priv->b_tx_status.queue)))
+ dev_kfree_skb_any(skb);
+ usb_kill_urb(priv->b_tx_status.urb);
mutex_unlock(&priv->conf_mutex);
}
@@ -1310,6 +1432,7 @@ static int __devinit rtl8187_probe(struct usb_interface *intf,
goto err_free_dev;
}
mutex_init(&priv->conf_mutex);
+ skb_queue_head_init(&priv->b_tx_status.queue);
printk(KERN_INFO "%s: hwaddr %s, %s V%d + %s\n",
wiphy_name(dev->wiphy), print_mac(mac, dev->wiphy->perm_addr),
--
1.6.0.2
Add conf_tx callback and use it to configure tx queues of 8187L/8187B.
Signed-off-by: Herton Ronaldo Krzesinski <[email protected]>
---
drivers/net/wireless/rtl8187.h | 2 +
drivers/net/wireless/rtl8187_dev.c | 75 +++++++++++++++++++++++++++++---
drivers/net/wireless/rtl8187_rtl8225.c | 6 ---
3 files changed, 70 insertions(+), 13 deletions(-)
diff --git a/drivers/net/wireless/rtl8187.h b/drivers/net/wireless/rtl8187.h
index 33725d0..f09872e 100644
--- a/drivers/net/wireless/rtl8187.h
+++ b/drivers/net/wireless/rtl8187.h
@@ -111,6 +111,8 @@ struct rtl8187_priv {
u8 signal;
u8 quality;
u8 noise;
+ u8 slot_time;
+ u8 aifsn[4];
};
void rtl8187_write_phy(struct ieee80211_hw *dev, u8 addr, u32 data);
diff --git a/drivers/net/wireless/rtl8187_dev.c b/drivers/net/wireless/rtl8187_dev.c
index 38e10c7..537ad64 100644
--- a/drivers/net/wireless/rtl8187_dev.c
+++ b/drivers/net/wireless/rtl8187_dev.c
@@ -703,6 +703,13 @@ static int rtl8187b_init_hw(struct ieee80211_hw *dev)
rtl818x_iowrite16_idx(priv, (__le16 *)0xFFEC, 0x0800, 1);
+ priv->slot_time = 0x9;
+ priv->aifsn[0] = 2; /* AIFSN[AC_VO] */
+ priv->aifsn[1] = 2; /* AIFSN[AC_VI] */
+ priv->aifsn[2] = 7; /* AIFSN[AC_BK] */
+ priv->aifsn[3] = 3; /* AIFSN[AC_BE] */
+ rtl818x_iowrite8(priv, &priv->map->ACM_CONTROL, 0);
+
return 0;
}
@@ -911,24 +918,38 @@ static int rtl8187_config_interface(struct ieee80211_hw *dev,
return 0;
}
+/*
+ * With 8187B, AC_*_PARAM clashes with FEMR definition in struct rtl818x_csr for
+ * example. Thus we have to use raw values for AC_*_PARAM register addresses.
+ */
+static __le32 *rtl8187b_ac_addr[4] = {
+ (__le32 *) 0xFFF0, /* AC_VO */
+ (__le32 *) 0xFFF4, /* AC_VI */
+ (__le32 *) 0xFFFC, /* AC_BK */
+ (__le32 *) 0xFFF8, /* AC_BE */
+};
+
+#define SIFS_TIME 0xa
+
static void rtl8187_conf_erp(struct rtl8187_priv *priv, bool use_short_slot,
bool use_short_preamble)
{
if (priv->is_rtl8187b) {
- u8 difs, eifs, slot_time;
+ u8 difs, eifs;
u16 ack_timeout;
+ int queue;
if (use_short_slot) {
- slot_time = 0x9;
+ priv->slot_time = 0x9;
difs = 0x1c;
eifs = 0x53;
} else {
- slot_time = 0x14;
+ priv->slot_time = 0x14;
difs = 0x32;
eifs = 0x5b;
}
- rtl818x_iowrite8(priv, &priv->map->SIFS, 0xa);
- rtl818x_iowrite8(priv, &priv->map->SLOT, slot_time);
+ rtl818x_iowrite8(priv, &priv->map->SIFS, SIFS_TIME);
+ rtl818x_iowrite8(priv, &priv->map->SLOT, priv->slot_time);
rtl818x_iowrite8(priv, &priv->map->DIFS, difs);
/*
@@ -949,18 +970,21 @@ static void rtl8187_conf_erp(struct rtl8187_priv *priv, bool use_short_slot,
ack_timeout += 144;
rtl818x_iowrite8(priv, &priv->map->CARRIER_SENSE_COUNTER,
DIV_ROUND_UP(ack_timeout, 4));
+
+ for (queue = 0; queue < 4; queue++)
+ rtl818x_iowrite8(priv, (u8 *) rtl8187b_ac_addr[queue],
+ priv->aifsn[queue] * priv->slot_time +
+ SIFS_TIME);
} else {
rtl818x_iowrite8(priv, &priv->map->SIFS, 0x22);
if (use_short_slot) {
rtl818x_iowrite8(priv, &priv->map->SLOT, 0x9);
rtl818x_iowrite8(priv, &priv->map->DIFS, 0x14);
rtl818x_iowrite8(priv, &priv->map->EIFS, 91 - 0x14);
- rtl818x_iowrite8(priv, &priv->map->CW_VAL, 0x73);
} else {
rtl818x_iowrite8(priv, &priv->map->SLOT, 0x14);
rtl818x_iowrite8(priv, &priv->map->DIFS, 0x24);
rtl818x_iowrite8(priv, &priv->map->EIFS, 91 - 0x24);
- rtl818x_iowrite8(priv, &priv->map->CW_VAL, 0xa5);
}
}
}
@@ -1009,6 +1033,42 @@ static void rtl8187_configure_filter(struct ieee80211_hw *dev,
rtl818x_iowrite32_async(priv, &priv->map->RX_CONF, priv->rx_conf);
}
+static int rtl8187_conf_tx(struct ieee80211_hw *dev, u16 queue,
+ const struct ieee80211_tx_queue_params *params)
+{
+ struct rtl8187_priv *priv = dev->priv;
+ u8 cw_min, cw_max;
+
+ if (queue > 3)
+ return -EINVAL;
+
+ cw_min = fls(params->cw_min);
+ cw_max = fls(params->cw_max);
+
+ if (priv->is_rtl8187b) {
+ priv->aifsn[queue] = params->aifs;
+
+ /*
+ * This is the structure of AC_*_PARAM registers in 8187B:
+ * - TXOP limit field, bit offset = 16
+ * - ECWmax, bit offset = 12
+ * - ECWmin, bit offset = 8
+ * - AIFS, bit offset = 0
+ */
+ rtl818x_iowrite32(priv, rtl8187b_ac_addr[queue],
+ (params->txop << 16) | (cw_max << 12) |
+ (cw_min << 8) | (params->aifs *
+ priv->slot_time + SIFS_TIME));
+ } else {
+ if (queue != 0)
+ return -EINVAL;
+
+ rtl818x_iowrite8(priv, &priv->map->CW_VAL,
+ cw_min | (cw_max << 4));
+ }
+ return 0;
+}
+
static const struct ieee80211_ops rtl8187_ops = {
.tx = rtl8187_tx,
.start = rtl8187_start,
@@ -1019,6 +1079,7 @@ static const struct ieee80211_ops rtl8187_ops = {
.config_interface = rtl8187_config_interface,
.bss_info_changed = rtl8187_bss_info_changed,
.configure_filter = rtl8187_configure_filter,
+ .conf_tx = rtl8187_conf_tx
};
static void rtl8187_eeprom_register_read(struct eeprom_93cx6 *eeprom)
diff --git a/drivers/net/wireless/rtl8187_rtl8225.c b/drivers/net/wireless/rtl8187_rtl8225.c
index b999f87..ec0045d 100644
--- a/drivers/net/wireless/rtl8187_rtl8225.c
+++ b/drivers/net/wireless/rtl8187_rtl8225.c
@@ -885,12 +885,6 @@ static void rtl8225z2_b_rf_init(struct ieee80211_hw *dev)
for (i = 0; i < ARRAY_SIZE(rtl8225z2_ofdm); i++)
rtl8225_write_phy_ofdm(dev, i, rtl8225z2_ofdm[i]);
- rtl818x_iowrite32(priv, (__le32 *)0xFFF0, (7 << 12) | (3 << 8) | 28);
- rtl818x_iowrite32(priv, (__le32 *)0xFFF4, (7 << 12) | (3 << 8) | 28);
- rtl818x_iowrite32(priv, (__le32 *)0xFFF8, (7 << 12) | (3 << 8) | 28);
- rtl818x_iowrite32(priv, (__le32 *)0xFFFC, (7 << 12) | (3 << 8) | 28);
- rtl818x_iowrite8(priv, &priv->map->ACM_CONTROL, 0);
-
rtl8225_write_phy_ofdm(dev, 0x97, 0x46); msleep(1);
rtl8225_write_phy_ofdm(dev, 0xa4, 0xb6); msleep(1);
rtl8225_write_phy_ofdm(dev, 0x85, 0xfc); msleep(1);
--
1.6.0.2
> if (condition)
> info->flags |= IEEE80211_TX_STAT_ACK;
> else
> info->status.excessive_retries = 1;
> ---------------
>
> But the else-clause is missing in the latest patch (of two), which also seems to be where both you and Larry are having rate stuck at 1Mb/s for.
> - and the line below
> "info->status.rates[0].count = pkt_rc +1 ;" doesn't compile
> under 2.6.27, so I changed the rates[0].count to the retry_count,
> and simultaneously adding the else-clause back.
>
> My question is this: is the removal of the else-clause unintensional,
> could it result in your 1Mb/s rate?
The excessive_retries status thing has been removed and mac80211 ought
to always use STAT_ACK now.
johannes
On Friday 31 October 2008 21:32:14 Larry Finger wrote:
> Herton Ronaldo Krzesinski wrote:
> > Two patches for two current issues in rtl8187:
> > - First it should implement conf_tx callback, instead of hardcoding some
> > tx queue values.
> > - The second one should make rate control work for 8187B sending correct
> > feedback about transmitted packet status, using one of the Bulk In
> > endpoints that provides this info. Unfortunately after recently rate
> > control api rewrite in my testing here I don't get anymore greater than
> > 1M rate with pid, before I would still get until 9M (and after some time
> > falling to 1M, then going back up in steps, and so on) on same
> > environment, don't know if it's expected to the new rate control api be
> > more strict/have different behaviour, or if something additional must be
> > done in rtl8187 (the link/signal quality may be too low and may be new
> > rate control changes are more sensitive).
> >
> > Please test and check if there are any additional issues.
>
> With these patches installed, my RTL8187B device was stuck at 1 Mbs using
> the 'pid' algorithm. Without them, the rate rapidly increases to 54 Mbs.
> I'm roughly 2 m from my AP, and get an indicated signal of -22 dBm and a
> link quality of 98/100. My kernel is from wireless-testing
> (v2.6.28-rc2-4159-g6c11cd2) with the rtl8187 power control and timing
> patches submitted earlier. There are no changes to any of the mac80211
> components.
I'll look why this behaviour of being stuck at 1M, I saw this too, but posted
the patch to confirm and if meanwhile may be someone sees something missing
or wrong with current version or in new rate api. While testing when
wireless-testing was still at 2.6.27, it didn't have this behaviour. Then I
updated to latest wireless-testing and had to fix the patch to build for new
rate control api, that I guess is the cause of being stuck at 1M, I have to
investigate more what is happening now. The first patch is another issue and
shouldn't affect rate control.
>
> Larry
--
[]'s
Herton
On Friday 31 October 2008 22:07:45 Hin-Tak Leung wrote:
> --- On Fri, 31/10/08, Larry Finger <[email protected]> wrote:
> > With these patches installed, my RTL8187B device was stuck
> > at 1 Mbs using the
> > 'pid' algorithm. Without them, the rate rapidly
> > increases to 54 Mbs. I'm roughly
> > 2 m from my AP, and get an indicated signal of -22 dBm and
> > a link quality of
> > 98/100. My kernel is from wireless-testing
> > (v2.6.28-rc2-4159-g6c11cd2) with the
> > rtl8187 power control and timing patches submitted earlier.
> > There are no changes
> > to any of the mac80211 components.
>
> This behavior was mentioned in Herton's e-mail, and said to be likely due
> to recent rewrite of the rate-control api?
>
> I am still using the early incarnation of the rate-control patch with
> 2.6.26.7-86.fc9.x86_64 and 2.6.27.4-69.fc10.x86_64 (by rolling back
> wireless-testing to 2.6.27.x to match), and my rate does go up and down
> (between 11M to 36M just now) and generally feels more responsive.
> Apologies I haven't looked at the newly posted rate-control patch yet - is
> there any significant code difference?
Yes, cleanups, minor optimization and fixes, like freeing skbs in status queue
on interface stop, btw this was the patch for wireless-testing from some days
ago while still in 2.6.27 (different from previous version I sent privately with
printks/debug):
diff --git a/drivers/net/wireless/rtl8187.h b/drivers/net/wireless/rtl8187.h
index f09872e..c385407 100644
--- a/drivers/net/wireless/rtl8187.h
+++ b/drivers/net/wireless/rtl8187.h
@@ -113,6 +113,11 @@ struct rtl8187_priv {
u8 noise;
u8 slot_time;
u8 aifsn[4];
+ struct {
+ __le64 buf;
+ struct urb *urb;
+ struct sk_buff_head queue;
+ } b_tx_status;
};
void rtl8187_write_phy(struct ieee80211_hw *dev, u8 addr, u32 data);
diff --git a/drivers/net/wireless/rtl8187_dev.c b/drivers/net/wireless/rtl8187_dev.c
index 537ad64..4d692d4 100644
--- a/drivers/net/wireless/rtl8187_dev.c
+++ b/drivers/net/wireless/rtl8187_dev.c
@@ -167,8 +167,34 @@ static void rtl8187_tx_cb(struct urb *urb)
skb_pull(skb, priv->is_rtl8187b ? sizeof(struct rtl8187b_tx_hdr) :
sizeof(struct rtl8187_tx_hdr));
ieee80211_tx_info_clear_status(info);
- info->flags |= IEEE80211_TX_STAT_ACK;
- ieee80211_tx_status_irqsafe(hw, skb);
+
+ if (!urb->status &&
+ !(info->flags & IEEE80211_TX_CTL_NO_ACK) &&
+ priv->is_rtl8187b) {
+ skb_queue_tail(&priv->b_tx_status.queue, skb);
+
+ /* queue is "full", discard last items */
+ while (skb_queue_len(&priv->b_tx_status.queue) > 5) {
+ struct sk_buff *old_skb;
+ struct ieee80211_tx_info *old_info;
+
+ dev_dbg(&priv->udev->dev,
+ "transmit status queue full\n");
+
+ old_skb = skb_dequeue(&priv->b_tx_status.queue);
+ old_info = IEEE80211_SKB_CB(old_skb);
+ old_info->status.excessive_retries = 1;
+ ieee80211_tx_status_irqsafe(hw, old_skb);
+ }
+ } else {
+ if (!(info->flags & IEEE80211_TX_CTL_NO_ACK)) {
+ if (!urb->status)
+ info->flags |= IEEE80211_TX_STAT_ACK;
+ else
+ info->status.excessive_retries = 1;
+ }
+ ieee80211_tx_status_irqsafe(hw, skb);
+ }
}
static int rtl8187_tx(struct ieee80211_hw *dev, struct sk_buff *skb)
@@ -394,6 +420,107 @@ static int rtl8187_init_urbs(struct ieee80211_hw *dev)
return 0;
}
+static void rtl8187b_status_cb(struct urb *urb)
+{
+ struct ieee80211_hw *hw = (struct ieee80211_hw *)urb->context;
+ struct rtl8187_priv *priv = hw->priv;
+ u64 val;
+ unsigned int cmd_type;
+
+ if (unlikely(urb->status)) {
+ usb_free_urb(urb);
+ return;
+ }
+
+ /*
+ * Read from status buffer:
+ *
+ * bits [30:31] = cmd type:
+ * - 0 indicates tx beacon interrupt
+ * - 1 indicates tx close descriptor
+ *
+ * In the case of tx beacon interrupt:
+ * [0:9] = Last Beacon CW
+ * [10:29] = reserved
+ * [30:31] = 00b
+ * [32:63] = Last Beacon TSF
+ *
+ * If it's tx close descriptor:
+ * [0:7] = Packet Retry Count
+ * [8:14] = RTS Retry Count
+ * [15] = TOK
+ * [16:27] = Sequence No
+ * [28] = LS
+ * [29] = FS
+ * [30:31] = 01b
+ * [32:47] = unused (reserved?)
+ * [48:63] = MAC Used Time
+ */
+ val = le64_to_cpu(priv->b_tx_status.buf);
+
+ cmd_type = (val >> 30) & 0x3;
+ if (cmd_type == 1) {
+ unsigned int pkt_rc, seq_no;
+ bool tok;
+ struct sk_buff *skb;
+ struct ieee80211_hdr *ieee80211hdr;
+ unsigned long flags;
+
+ pkt_rc = val & 0xFF;
+ tok = val & (1 << 15);
+ seq_no = (val >> 16) & 0xFFF;
+
+ spin_lock_irqsave(&priv->b_tx_status.queue.lock, flags);
+ skb_queue_reverse_walk(&priv->b_tx_status.queue, skb) {
+ ieee80211hdr = (struct ieee80211_hdr *)skb->data;
+
+ /*
+ * Instead of returning just the 12 bits of sequence
+ * number, hardware is returning entire sequence
+ * control, overflowing after some time. As a
+ * workaround, just consider the lower bits, and expect
+ * it's unlikely we wrongly ack some sent data
+ */
+ if ((le16_to_cpu(ieee80211hdr->seq_ctrl) & 0xFFF) == seq_no)
+ break;
+ }
+ if (skb != (struct sk_buff *) &priv->b_tx_status.queue) {
+ struct ieee80211_tx_info *info = IEEE80211_SKB_CB(skb);
+
+ __skb_unlink(skb, &priv->b_tx_status.queue);
+ if (tok)
+ info->flags |= IEEE80211_TX_STAT_ACK;
+ else
+ info->status.excessive_retries = 1;
+ info->status.retry_count = pkt_rc;
+
+ ieee80211_tx_status_irqsafe(hw, skb);
+ }
+ spin_unlock_irqrestore(&priv->b_tx_status.queue.lock, flags);
+ }
+
+ usb_submit_urb(urb, GFP_ATOMIC);
+}
+
+static int rtl8187b_init_status_urb(struct ieee80211_hw *dev)
+{
+ struct rtl8187_priv *priv = dev->priv;
+ struct urb *entry;
+
+ entry = usb_alloc_urb(0, GFP_KERNEL);
+ if (!entry)
+ return -ENOMEM;
+ priv->b_tx_status.urb = entry;
+
+ usb_fill_bulk_urb(entry, priv->udev, usb_rcvbulkpipe(priv->udev, 9),
+ &priv->b_tx_status.buf, sizeof(priv->b_tx_status.buf),
+ rtl8187b_status_cb, dev);
+
+ usb_submit_urb(entry, GFP_KERNEL);
+
+ return 0;
+}
+
static int rtl8187_cmd_reset(struct ieee80211_hw *dev)
{
struct rtl8187_priv *priv = dev->priv;
@@ -746,6 +873,7 @@ static int rtl8187_start(struct ieee80211_hw *dev)
(7 << 0 /* long retry limit */) |
(7 << 21 /* MAX TX DMA */));
rtl8187_init_urbs(dev);
+ rtl8187b_init_status_urb(dev);
mutex_unlock(&priv->conf_mutex);
return 0;
}
@@ -822,6 +950,9 @@ static void rtl8187_stop(struct ieee80211_hw *dev)
usb_kill_urb(info->urb);
kfree_skb(skb);
}
+ while ((skb = skb_dequeue(&priv->b_tx_status.queue)))
+ dev_kfree_skb_any(skb);
+ usb_kill_urb(priv->b_tx_status.urb);
mutex_unlock(&priv->conf_mutex);
}
@@ -1310,6 +1441,7 @@ static int __devinit rtl8187_probe(struct usb_interface *intf,
goto err_free_dev;
}
mutex_init(&priv->conf_mutex);
+ skb_queue_head_init(&priv->b_tx_status.queue);
printk(KERN_INFO "%s: hwaddr %s, %s V%d + %s\n",
wiphy_name(dev->wiphy), print_mac(mac, dev->wiphy->perm_addr),
--
1.6.0.2
>
> Hin-Tak
>
> Hin-Tak
--
[]'s
Herton
--- On Fri, 31/10/08, Larry Finger <[email protected]> wrote:
> With these patches installed, my RTL8187B device was stuck
> at 1 Mbs using the
> 'pid' algorithm. Without them, the rate rapidly
> increases to 54 Mbs. I'm roughly
> 2 m from my AP, and get an indicated signal of -22 dBm and
> a link quality of
> 98/100. My kernel is from wireless-testing
> (v2.6.28-rc2-4159-g6c11cd2) with the
> rtl8187 power control and timing patches submitted earlier.
> There are no changes
> to any of the mac80211 components.
This behavior was mentioned in Herton's e-mail, and said to be likely due to recent rewrite of the rate-control api?
I am still using the early incarnation of the rate-control patch with 2.6.26.7-86.fc9.x86_64 and 2.6.27.4-69.fc10.x86_64 (by rolling back wireless-testing to 2.6.27.x to match), and my rate does go up and down (between 11M to 36M just now) and generally feels more responsive. Apologies I haven't looked at the newly posted rate-control patch yet - is there any significant code difference?
Hin-Tak
Hin-Tak
> --- On Fri, 31/10/08, Larry Finger
> <[email protected]> wrote:
>
> > With these patches installed, my RTL8187B device was
> stuck
> > at 1 Mbs using the
> > 'pid' algorithm.
I back-ported the 2nd rate-change patch
"feedback transmitted packets using tx close descriptor for 8187B"
on its own to 2.6.27 and its behavior is similiar to the earlier rate-control patch but without "tranmit queue full" dmesg flood; and my trans rate goes up and down between 11Mb to 48Mb. So it looks like the rate limitation you are experiencing is either due to the conf_tx patch, the rest of 2.6.27<->2.6.28 wireless changes, or this little snipplet which I had to change for the back port, since the rates[0].* seems to be newly introduced in 2.6.28/wireless-testing.
----------------------------------------------------
--- a/drivers/net/wireless/rtl8187_dev.c
+++ b/drivers/net/wireless/rtl8187_dev.c
@@ -494,7 +494,9 @@ static void rtl8187b_status_cb(struct urb *urb)
__skb_unlink(skb, &priv->b_tx_status.queue);
if (tok)
info->flags |= IEEE80211_TX_STAT_ACK;
- info->status.rates[0].count = pkt_rc + 1;
+ else
+ info->status.excessive_retries = 1;
+ info->status.retry_count = pkt_rc;
ieee80211_tx_status_irqsafe(hw, skb);
}
-----------------------------------------------------
I don't know what's the significance of this (and the "+1" above), but I made this change based on the first rate-change patch :-).
So you can add <test-by> me for the 2nd patch of these two; the conf_tx callback patch seems to depend on some fairly recently wireless-testing/2.6.28.x changes?
Hin-Tak
Herton Ronaldo Krzesinski wrote:
> Two patches for two current issues in rtl8187:
> - First it should implement conf_tx callback, instead of hardcoding some tx
> queue values.
> - The second one should make rate control work for 8187B sending correct
> feedback about transmitted packet status, using one of the Bulk In endpoints
> that provides this info. Unfortunately after recently rate control api rewrite
> in my testing here I don't get anymore greater than 1M rate with pid, before I
> would still get until 9M (and after some time falling to 1M, then going back
> up in steps, and so on) on same environment, don't know if it's expected to
> the new rate control api be more strict/have different behaviour, or if
> something additional must be done in rtl8187 (the link/signal quality may be
> too low and may be new rate control changes are more sensitive).
>
> Please test and check if there are any additional issues.
With these patches installed, my RTL8187B device was stuck at 1 Mbs using the
'pid' algorithm. Without them, the rate rapidly increases to 54 Mbs. I'm roughly
2 m from my AP, and get an indicated signal of -22 dBm and a link quality of
98/100. My kernel is from wireless-testing (v2.6.28-rc2-4159-g6c11cd2) with the
rtl8187 power control and timing patches submitted earlier. There are no changes
to any of the mac80211 components.
Larry
--- On Sun, 2/11/08, Herton Ronaldo Krzesinski <[email protected]> wrote:
<snipped>
> Yes, cleanups, minor optimization and fixes, like freeing
> skbs in status queue
> on interface stop, btw this was the patch for
> wireless-testing from some days
> ago while still in 2.6.27 (different from previous version
> I sent privately with
> printks/debug):
<snipped>
__skb_unlink(skb, &priv->b_tx_status.queue);
> + if (tok)
> + info->flags |= IEEE80211_TX_STAT_ACK;
> + else
> + info->status.excessive_retries = 1;
> + info->status.retry_count = pkt_rc;
> +
> + ieee80211_tx_status_irqsafe(hw, skb);
<snipped>
okay, I looked through my hard disc and it seems that I have 3 versions of the rate control patch - the first one sent privately, I think, the 2nd one as part of a patch series of 2 with conf-tx, and a 3rd one above
which is supposedly between the two, posted to wireless-testing (which apologies, I don't subscribe). There is a very crucial difference:
I noticed that whenever STAT_ACK is always an alternative to
setting excessive_retries:
---------------
if (condition)
info->flags |= IEEE80211_TX_STAT_ACK;
else
info->status.excessive_retries = 1;
---------------
But the else-clause is missing in the latest patch (of two), which also seems to be where both you and Larry are having rate stuck at 1Mb/s for.
- and the line below
"info->status.rates[0].count = pkt_rc +1 ;" doesn't compile
under 2.6.27, so I changed the rates[0].count to the retry_count,
and simultaneously adding the else-clause back.
My question is this: is the removal of the else-clause unintensional,
could it result in your 1Mb/s rate?
Hin-Tak
Hi,
I am just experiencing a problem I read about but I thought I didn't have.
I have always been able to suspend/resume without unloading the rtl8187 module ever since it started to work reasonably well. It seems that there is some automatic mechanism for unloading any usb network driver which does not have its own suspend/resume code. But it doesn't work in 2.6.27.
(I filed this as https://bugzilla.redhat.com/show_bug.cgi?id=469461, and there is a reference to an upstream bug report).
Anyway, I think this is the likely breakage?
--------------
commit 78d9a487ee961c356e1a934d9a92eca38ffb3a70
Author: Alan Stern <[email protected]>
Date: Mon Jun 23 16:00:40 2008 -0400
USB: Force unbinding of drivers lacking reset_resume or other methods
This patch (as1024) takes care of a FIXME issue: Drivers that don't
have the necessary suspend, resume, reset_resume, pre_reset, or
post_reset methods will be unbound and their interface reprobed when
one of the unsupported events occurs.
This is made slightly more difficult by the fact that bind operations
won't work during a system sleep transition. So instead the code has
to defer the operation until the transition ends.
Signed-off-by: Alan Stern <[email protected]>
Signed-off-by: Greg Kroah-Hartman <[email protected]>
-----------------
Any thought on this?
Hin-Tak