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.
Please test and check if there are issues.
Hin-Tak Leung wrote:
>
> Seems that in my situation (my access point is a slow machine), the behavior of the rate control mechanism depends on the *type* of work load - I was restoring a firefox session with a lot of windows, and it is stuck at 1Mb all the time. But once all the windows are stored, doing a ping I can indeed have the rate going up (and eventually coming down) to 24-36Mb.
Are those default pings? It may work differently with those short packets thaqn
it does with the longer ones found with browsing.
Larry
On Tuesday 04 November 2008 21:11:31 Felix Fietkau wrote:
> Larry Finger wrote:
> > Herton Ronaldo Krzesinski wrote:
> >> Add conf_tx callback and use it to configure tx queues of 8187L/8187B.
> >>
> >> Signed-off-by: Herton Ronaldo Krzesinski <[email protected]>
> >
> > Tested-by: Larry Finger <[email protected]>
> >
> > This patch is OK. With 1 and 2, the rate-control mechanism works the way
> > it did before.
>
> Has anybody tested this thing with the minstrel rate control yet?
> At some point I would like to make minstrel the default RC,
> since it has shown much better performance on drivers like b43 or ath5k.
Tested minstrel here, seems to be working fine, that is, it's changing rate
above 1M after some seconds of traffic and sticking to a stable rate unlike
pid where it seems to go in cicles here (from 1M to 9M and going back to 1M
and so on, unlinke minstrel that sticks to 9M, same environment in both
cases). Looks a better rate control in practice, didn't make any further
verifications.
>
> - Felix
--
[]'s
Herton
Larry Finger wrote:
> Herton Ronaldo Krzesinski wrote:
>> Add conf_tx callback and use it to configure tx queues of 8187L/8187B.
>>
>> Signed-off-by: Herton Ronaldo Krzesinski <[email protected]>
>
> Tested-by: Larry Finger <[email protected]>
>
> This patch is OK. With 1 and 2, the rate-control mechanism works the way it did
> before.
Has anybody tested this thing with the minstrel rate control yet?
At some point I would like to make minstrel the default RC,
since it has shown much better performance on drivers like b43 or ath5k.
- Felix
On Tue, 2008-11-04 at 14:31 -0800, Larry Finger wrote:
> Herton Ronaldo Krzesinski wrote:
> > 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]>
>
> Tested-by: Larry Finger <[email protected]>
>
>
> Quilt shows the following warning:
>
> Warning: trailing whitespace in lines 438,468 of drivers/net/wireless/rtl8187_dev.c
>
> scripts/checkpatch shows the following:
>
> WARNING: line over 80 characters
> #138: FILE: drivers/net/wireless/rtl8187_dev.c:475:
> + if ((le16_to_cpu(ieee80211hdr->seq_ctrl) & 0xFFF) == seq_no)
Besides, that line looks wrong? the lowest 4 bits are teh fragment
number.
johannes
The actual throughput transferring a 700MB file via sftp between my client and the AP doesn't go much over 50KB/s (according to sftp's progress meter). It used to be over 400KB/s under the same situation. The rate shown by
iwconfig is consistently 1Mb/s while manually setting it to 36/48Mb doesn't
change the throughput. (at the moment it is looking like it will take 4
hours to transfer, instead of the usual 20+ minutes).
how can I see what's wrong? (I have another few hours while it finishes, and of course, I can always try again...).
Herton Ronaldo Krzesinski wrote:
> 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]>
Tested-by: Larry Finger <[email protected]>
Quilt shows the following warning:
Warning: trailing whitespace in lines 438,468 of drivers/net/wireless/rtl8187_dev.c
scripts/checkpatch shows the following:
WARNING: line over 80 characters
#138: FILE: drivers/net/wireless/rtl8187_dev.c:475:
+ if ((le16_to_cpu(ieee80211hdr->seq_ctrl) & 0xFFF) == seq_no)
total: 0 errors, 1 warnings, 173 lines checked
Larry
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]>
---
v2: fix rate stuck at 1M reported by Larry Finger, Hin-Tak and also seen by me.
btw, I noted now with a ralink device that rt2500usb (rt2x00) has same
issue currently.
Hin-Tak: didn't add your tested-by yet as this patch has v2 fix comparing to
others, please test it again so I can add your tested-by
drivers/net/wireless/rtl8187.h | 5 ++
drivers/net/wireless/rtl8187_dev.c | 131 ++++++++++++++++++++++++++++++++++-
2 files changed, 132 insertions(+), 4 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 a84bff9..7758779 100644
--- a/drivers/net/wireless/rtl8187_dev.c
+++ b/drivers/net/wireless/rtl8187_dev.c
@@ -165,8 +165,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)
@@ -208,7 +227,7 @@ static int rtl8187_tx(struct ieee80211_hw *dev, struct sk_buff *skb)
hdr->flags = cpu_to_le32(flags);
hdr->len = 0;
hdr->rts_duration = rts_dur;
- hdr->retry = cpu_to_le32((info->control.rates[0].count - 1) << 8);
+ hdr->retry = cpu_to_le32(info->control.rates[0].count << 8);
buf = hdr;
ep = 2;
@@ -226,7 +245,7 @@ static int rtl8187_tx(struct ieee80211_hw *dev, struct sk_buff *skb)
memset(hdr, 0, sizeof(*hdr));
hdr->flags = cpu_to_le32(flags);
hdr->rts_duration = rts_dur;
- hdr->retry = cpu_to_le32((info->control.rates[0].count - 1) << 8);
+ hdr->retry = cpu_to_le32(info->control.rates[0].count << 8);
hdr->tx_duration =
ieee80211_generic_frame_duration(dev, priv->vif,
skb->len, txrate);
@@ -392,6 +411,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;
+
+ 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;
@@ -744,6 +862,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;
}
@@ -820,6 +939,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);
}
@@ -1307,6 +1429,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
Herton Ronaldo Krzesinski wrote:
> Add conf_tx callback and use it to configure tx queues of 8187L/8187B.
>
> Signed-off-by: Herton Ronaldo Krzesinski <[email protected]>
Tested-by: Larry Finger <[email protected]>
This patch is OK. With 1 and 2, the rate-control mechanism works the way it did
before.
Larry
--- On Thu, 6/11/08, Larry Finger <[email protected]> wrote:
> On my system is started at 1 Mbs and rapidly moved to
> 54Mbs. It went so quickly
> that by the time I started a ping, switched terminals, and
> issued an ivconfig,
> it was already at 54 Mbs.
Seems that in my situation (my access point is a slow machine), the behavior of the rate control mechanism depends on the *type* of work load - I was restoring a firefox session with a lot of windows, and it is stuck at 1Mb all the time. But once all the windows are stored, doing a ping I can indeed have the rate going up (and eventually coming down) to 24-36Mb.
Hin-Tak
On Wed, Nov 05, 2008 at 03:16:42PM -0800, Hin-Tak Leung wrote:
> --- On Wed, 5/11/08, Luis R. Rodriguez <[email protected]> wrote:
> > Umm I have an FC10 box and I see a mac80211.ko. But of
> > course, I have an
> > early release of FC10, its on
> > 2.6.27-0.244.rc2.git1.fc10.x86_64.
> >
> > Did the new ones really put it built in?
>
> Yes,
> $ grep 'MAC80211=' /boot/config-2.6.2*fc10*
> /boot/config-2.6.27.4-69.fc10.x86_64:CONFIG_MAC80211=y
> /boot/config-2.6.27.4-73.fc10.x86_64:CONFIG_MAC80211=y
>
> I looked through koji and found it was removed between
> 0.345.rc7.fc10
> 0.339.rc7.fc10
> but the log entry between does not say why.
>
> I filed this as https://bugzilla.redhat.com/show_bug.cgi?id=470143 ,
Thanks, I've put this on the wiki:
http://wireless.kernel.org/en/users/Download#DoIneedthisonFedora.3F
> if John wants to grab it before somebody else assign it his way :-).
Luis
--- On Thu, 6/11/08, Larry Finger <[email protected]> wrote:
> Hin-Tak Leung wrote:
> >
> > Seems that in my situation (my access point is a slow
> machine), the behavior of the rate control mechanism depends
> on the *type* of work load - I was restoring a firefox
> session with a lot of windows, and it is stuck at 1Mb all
> the time. But once all the windows are stored, doing a ping
> I can indeed have the rate going up (and eventually coming
> down) to 24-36Mb.
>
> Are those default pings? It may work differently with those
> short packets thaqn
> it does with the longer ones found with browsing.
Yes, just default "ping 192.168.0.1" without any options. I wonder if
pid is clever enough to work out that my upstream bandwidth is only 2Mb/s (web browsing obviously goes outside) and not letting it go over 1Mb/s.
Also wondering about latency: in the absence of browser traffic, I can ping the AP/routing-machine with 0.07s round-trip time, but it increases to almost 2s when restoring a firefox session. I tried to see if pinging - which is between the client and the AP - changes the rate, compared to firefox which depends on limited upstream bandwidth. It didn't, and given how poor ping works during such time, it isn't surprising.
Hin-Tak
On Wednesday 05 November 2008 09:29:12 Johannes Berg wrote:
> On Tue, 2008-11-04 at 14:31 -0800, Larry Finger wrote:
> > Herton Ronaldo Krzesinski wrote:
> > > 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]>
> >
> > Tested-by: Larry Finger <[email protected]>
> >
> >
> > Quilt shows the following warning:
> >
> > Warning: trailing whitespace in lines 438,468 of
> > drivers/net/wireless/rtl8187_dev.c
> >
> > scripts/checkpatch shows the following:
> >
> > WARNING: line over 80 characters
> > #138: FILE: drivers/net/wireless/rtl8187_dev.c:475:
> > + if ((le16_to_cpu(ieee80211hdr->seq_ctrl) & 0xFFF)
> > == seq_no)
Ops, I forgot to run checkpatch.pl here, will run and fix it when I submit
next patch.
>
> Besides, that line looks wrong? the lowest 4 bits are teh fragment
> number.
It's intended, because the hardware instead of reporting just the sequence
number in its 12 bits is reporting fragment number + sequence number, this is
a workaround.
>
> johannes
--
[]'s
Herton
On Thu, Nov 06, 2008 at 06:14:57AM -0800, John W. Linville wrote:
> On Wed, Nov 05, 2008 at 11:16:42PM +0000, Hin-Tak Leung wrote:
> > --- On Wed, 5/11/08, Luis R. Rodriguez <[email protected]> wrote:
> > > Umm I have an FC10 box and I see a mac80211.ko. But of
> > > course, I have an
> > > early release of FC10, its on
> > > 2.6.27-0.244.rc2.git1.fc10.x86_64.
> > >
> > > Did the new ones really put it built in?
> >
> > Yes,
> > $ grep 'MAC80211=' /boot/config-2.6.2*fc10*
> > /boot/config-2.6.27.4-69.fc10.x86_64:CONFIG_MAC80211=y
> > /boot/config-2.6.27.4-73.fc10.x86_64:CONFIG_MAC80211=y
> >
> > I looked through koji and found it was removed between
> > 0.345.rc7.fc10
> > 0.339.rc7.fc10
> > but the log entry between does not say why.
> >
> > I filed this as https://bugzilla.redhat.com/show_bug.cgi?id=470143 ,
> > if John wants to grab it before somebody else assign it his way :-).
>
> That decision was made against my advice -- I don't know that I can
> change it.
But you did, yay!
Luis
On Tuesday 04 November 2008 18:27:12 Hin-Tak Leung wrote:
> --- On Tue, 4/11/08, Herton Ronaldo Krzesinski <[email protected]>
wrote:
> > 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.
> >
> > ---
> > v2: fix rate stuck at 1M reported by Larry Finger, Hin-Tak
> > and also seen by me.
> > btw, I noted now with a ralink device that rt2500usb
> > (rt2x00) has same
> > issue currently.
> >
> >
> > - hdr->retry =
> > cpu_to_le32((info->control.rates[0].count - 1) <<
> > 8);
> > + hdr->retry =
> > cpu_to_le32(info->control.rates[0].count << 8);
> >
> > - hdr->retry =
> > cpu_to_le32((info->control.rates[0].count - 1) <<
> > 8);
> > + hdr->retry =
> > cpu_to_le32(info->control.rates[0].count << 8);
>
> These are the only lines which changed between the two versions of the
> patch - if removing the two -1's fixes the rate stuck (and these two lines
> are newly introduced), shouldn't this be a separate commit if it fixes a
> problem introduced by an earlier commit?
No, there was other change as well:
info->status.rates[0].count = pkt_rc - 1;
to
info->status.rates[0].count = pkt_rc;
that's in fact what fixes the rate control problem with pid (from what I
understand from the code, if rates[0].count is not zero (that will be always
the case here) pid would register the tx with errors, and would explain the
rate stuck at 1M).
The other changes is because now with new rate control api, it changed
semantics but is passing a different retry_count than before now, while
testing I found for example, connecting to an ap using wep, it is passing in
tx packet in same conditions retry_count=6 now instead of 7 as before rate
control api, so I considered a bug, unless this is desired (also depending on
code path the retry count in tx packet could be set to zero now, is this
right?); if new behaviour of retry_count in this case is right (and my change
is wrong) please someone explain to me what is the semantic now behind the
new rate control api (looking at the rate control api change why there is
some differences between drivers?, as some drivers simply just replaced
retry_count with rates[0].count usage while others changed
to "rates[0].count - 1".)
>
> I did *not* (and do not) have a rate-stuck problem, but I am running a
> slightly mod version of the patch against 2.6.27, which doesn't have
> "rates[x].count" yet, with the retry_count struct member which seems to
> have since been removed.
Please try and check the patches with latest wireless-testing.
--
[]'s
Herton
Add conf_tx callback and use it to configure tx queues of 8187L/8187B.
Signed-off-by: Herton Ronaldo Krzesinski <[email protected]>
---
v2: just rediff against latest wireless-testing from latest posted version.
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 d49f2a7..a84bff9 100644
--- a/drivers/net/wireless/rtl8187_dev.c
+++ b/drivers/net/wireless/rtl8187_dev.c
@@ -701,6 +701,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;
}
@@ -908,24 +915,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);
/*
@@ -946,18 +967,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);
}
}
}
@@ -1006,6 +1030,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,
@@ -1016,6 +1076,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 69030be..4e75e8e 100644
--- a/drivers/net/wireless/rtl8187_rtl8225.c
+++ b/drivers/net/wireless/rtl8187_rtl8225.c
@@ -878,12 +878,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);
rtl8225_write_phy_ofdm(dev, 0xa4, 0xb6);
rtl8225_write_phy_ofdm(dev, 0x85, 0xfc);
--
1.6.0.2
--- On Tue, 4/11/08, Herton Ronaldo Krzesinski <[email protected]> wrote:
> 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.
> ---
> v2: fix rate stuck at 1M reported by Larry Finger, Hin-Tak
> and also seen by me.
> btw, I noted now with a ralink device that rt2500usb
> (rt2x00) has same
> issue currently.
> - hdr->retry =
> cpu_to_le32((info->control.rates[0].count - 1) <<
> 8);
> + hdr->retry =
> cpu_to_le32(info->control.rates[0].count << 8);
> - hdr->retry =
> cpu_to_le32((info->control.rates[0].count - 1) <<
> 8);
> + hdr->retry =
> cpu_to_le32(info->control.rates[0].count << 8);
These are the only lines which changed between the two versions of the patch - if removing the two -1's fixes the rate stuck (and these two lines are newly introduced), shouldn't this be a separate commit if it fixes a problem introduced by an earlier commit?
I did *not* (and do not) have a rate-stuck problem, but I am running a slightly mod version of the patch against 2.6.27, which doesn't have "rates[x].count" yet, with the retry_count struct member which seems to have since been removed.
On Wed, 2008-11-05 at 13:38 -0200, Herton Ronaldo Krzesinski wrote:
> > > WARNING: line over 80 characters
> > > #138: FILE: drivers/net/wireless/rtl8187_dev.c:475:
> > > + if ((le16_to_cpu(ieee80211hdr->seq_ctrl) & 0xFFF)
> > > == seq_no)
>
> Ops, I forgot to run checkpatch.pl here, will run and fix it when I submit
> next patch.
I wouldn't worry about lines > 80 chars too much, breaking this down
wouldn't make more readable but less so, in my opinion. I tend to ignore
that rule where it makes the code unreadable :)
> > Besides, that line looks wrong? the lowest 4 bits are teh fragment
> > number.
>
> It's intended, because the hardware instead of reporting just the sequence
> number in its 12 bits is reporting fragment number + sequence number, this is
> a workaround.
Ah. A comment might be appropriate that "seq_no" doesn't actually
contain the sequence number?
johannes
On Wednesday 05 November 2008 13:40:22 Johannes Berg wrote:
> On Wed, 2008-11-05 at 13:38 -0200, Herton Ronaldo Krzesinski wrote:
> > > > WARNING: line over 80 characters
> > > > #138: FILE: drivers/net/wireless/rtl8187_dev.c:475:
> > > > + if ((le16_to_cpu(ieee80211hdr->seq_ctrl) &
> > > > 0xFFF) == seq_no)
> >
> > Ops, I forgot to run checkpatch.pl here, will run and fix it when I
> > submit next patch.
>
> I wouldn't worry about lines > 80 chars too much, breaking this down
> wouldn't make more readable but less so, in my opinion. I tend to ignore
> that rule where it makes the code unreadable :)
>
> > > Besides, that line looks wrong? the lowest 4 bits are teh fragment
> > > number.
> >
> > It's intended, because the hardware instead of reporting just the
> > sequence number in its 12 bits is reporting fragment number + sequence
> > number, this is a workaround.
>
> Ah. A comment might be appropriate that "seq_no" doesn't actually
> contain the sequence number?
Yes, I'll improve comment there, will change by this:
/*
* While testing, it was discovered that the seq_no doesn't
* actually contains the sequence number. Instead
* of returning just the 12 bits of sequence number,
* hardware is returning entire sequence control (fragment
* number plus sequence number) in a 12 bit only field
* overflowing after some time. As a workaround,
* just consider the lower bits, and expect it's unlikely
* we wrongly ack some sent data
*/
>
> johannes
--
[]'s
Herton