* [PATCH] rtl8187: Change rate-control feedback
@ 2011-02-27 20:44 Larry Finger
2011-02-28 2:21 ` Hin-Tak Leung
0 siblings, 1 reply; 3+ messages in thread
From: Larry Finger @ 2011-02-27 20:44 UTC (permalink / raw)
To: John W Linville; +Cc: Herton Ronaldo Krzesinski, Hin-Tak Leung, linux-wireless
The driver for the RTL8187L chips returns IEEE80211_TX_STAT_ACK for all
packets, even if the maximum number of retries was exhausted. In addition
it fails to setup max_rates in the ieee80211_hw struct, This behavior
may be responsible for the problems noted in Bug 14168. As the bug is very
old, testers have not been found, and I do not have the case where the
indicated signal is less than -70 dBm.
Signed-off-by: Larry Finger <Larry.Finger@lwfinger.net>
Cc: Stable <stable@kernel.org>
---
Index: wireless-testing/drivers/net/wireless/rtl818x/rtl8187/dev.c
===================================================================
--- wireless-testing.orig/drivers/net/wireless/rtl818x/rtl8187/dev.c
+++ wireless-testing/drivers/net/wireless/rtl818x/rtl8187/dev.c
@@ -869,23 +869,29 @@ static void rtl8187_work(struct work_str
/* The RTL8187 returns the retry count through register 0xFFFA. In
* addition, it appears to be a cumulative retry count, not the
* value for the current TX packet. When multiple TX entries are
- * queued, the retry count will be valid for the last one in the queue.
- * The "error" should not matter for purposes of rate setting. */
+ * waiting in the queue, the retry count will be the total for all.
+ * The "error" may matter for purposes of rate setting, but there is
+ * no other choice with this hardware.
+ */
struct rtl8187_priv *priv = container_of(work, struct rtl8187_priv,
work.work);
struct ieee80211_tx_info *info;
struct ieee80211_hw *dev = priv->dev;
static u16 retry;
u16 tmp;
+ u16 avg_retry;
mutex_lock(&priv->conf_mutex);
tmp = rtl818x_ioread16(priv, (__le16 *)0xFFFA);
+ avg_retry = (tmp - retry) / skb_queue_len(&priv->b_tx_status.queue);
while (skb_queue_len(&priv->b_tx_status.queue) > 0) {
struct sk_buff *old_skb;
old_skb = skb_dequeue(&priv->b_tx_status.queue);
info = IEEE80211_SKB_CB(old_skb);
- info->status.rates[0].count = tmp - retry + 1;
+ info->status.rates[0].count = avg_retry + 1;
+ if (info->status.rates[0].count > RETRY_COUNT)
+ info->flags &= ~IEEE80211_TX_STAT_ACK;
ieee80211_tx_status_irqsafe(dev, old_skb);
}
retry = tmp;
@@ -931,8 +937,8 @@ static int rtl8187_start(struct ieee8021
rtl818x_iowrite32(priv, &priv->map->TX_CONF,
RTL818X_TX_CONF_HW_SEQNUM |
RTL818X_TX_CONF_DISREQQSIZE |
- (7 << 8 /* short retry limit */) |
- (7 << 0 /* long retry limit */) |
+ (RETRY_COUNT << 8 /* short retry limit */) |
+ (RETRY_COUNT << 0 /* long retry limit */) |
(7 << 21 /* MAX TX DMA */));
rtl8187_init_urbs(dev);
rtl8187b_init_status_urb(dev);
@@ -1376,6 +1382,9 @@ static int __devinit rtl8187_probe(struc
dev->flags = IEEE80211_HW_HOST_BROADCAST_PS_BUFFERING |
IEEE80211_HW_SIGNAL_DBM |
IEEE80211_HW_RX_INCLUDES_FCS;
+ /* Initialize rate-control variables */
+ dev->max_rates = 1;
+ dev->max_rate_tries = RETRY_COUNT;
eeprom.data = dev;
eeprom.register_read = rtl8187_eeprom_register_read;
Index: wireless-testing/drivers/net/wireless/rtl818x/rtl8187/rtl8187.h
===================================================================
--- wireless-testing.orig/drivers/net/wireless/rtl818x/rtl8187/rtl8187.h
+++ wireless-testing/drivers/net/wireless/rtl818x/rtl8187/rtl8187.h
@@ -35,6 +35,8 @@
#define RFKILL_MASK_8187_89_97 0x2
#define RFKILL_MASK_8198 0x4
+#define RETRY_COUNT 7
+
struct rtl8187_rx_info {
struct urb *urb;
struct ieee80211_hw *dev;
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] rtl8187: Change rate-control feedback
2011-02-27 20:44 [PATCH] rtl8187: Change rate-control feedback Larry Finger
@ 2011-02-28 2:21 ` Hin-Tak Leung
2011-02-28 3:41 ` Larry Finger
0 siblings, 1 reply; 3+ messages in thread
From: Hin-Tak Leung @ 2011-02-28 2:21 UTC (permalink / raw)
To: John W Linville, Larry Finger; +Cc: Herton Ronaldo Krzesinski, linux-wireless
--- On Sun, 27/2/11, Larry Finger <Larry.Finger@lwfinger.net> wrote:
> The driver for the RTL8187L chips
> returns IEEE80211_TX_STAT_ACK for all
> packets, even if the maximum number of retries was
> exhausted. In addition
> it fails to setup max_rates in the ieee80211_hw struct,
> This behavior
> may be responsible for the problems noted in Bug 14168. As
> the bug is very
> old, testers have not been found, and I do not have the
> case where the
> indicated signal is less than -70 dBm.
Possibility of division by zero? The while loop dequeues - does this code always get invoked with a non-empty queue? Also it worrys me a little if tmp < retry?
> + avg_retry = (tmp - retry) /
> skb_queue_len(&priv->b_tx_status.queue);
> while
> (skb_queue_len(&priv->b_tx_status.queue) > 0) {
>
> Signed-off-by: Larry Finger <Larry.Finger@lwfinger.net>
> Cc: Stable <stable@kernel.org>
> ---
>
> Index:
> wireless-testing/drivers/net/wireless/rtl818x/rtl8187/dev.c
> ===================================================================
> ---
> wireless-testing.orig/drivers/net/wireless/rtl818x/rtl8187/dev.c
> +++
> wireless-testing/drivers/net/wireless/rtl818x/rtl8187/dev.c
> @@ -869,23 +869,29 @@ static void rtl8187_work(struct
> work_str
> /* The RTL8187 returns the retry count
> through register 0xFFFA. In
> * addition, it appears to be
> a cumulative retry count, not the
> * value for the current TX
> packet. When multiple TX entries are
> - * queued, the retry count
> will be valid for the last one in the queue.
> - * The "error" should not
> matter for purposes of rate setting. */
> + * waiting in the queue, the
> retry count will be the total for all.
> + * The "error" may matter for
> purposes of rate setting, but there is
> + * no other choice with this
> hardware.
> + */
> struct rtl8187_priv *priv =
> container_of(work, struct rtl8187_priv,
>
> work.work);
> struct ieee80211_tx_info *info;
> struct ieee80211_hw *dev =
> priv->dev;
> static u16 retry;
> u16 tmp;
> + u16 avg_retry;
>
> mutex_lock(&priv->conf_mutex);
> tmp = rtl818x_ioread16(priv, (__le16
> *)0xFFFA);
> + avg_retry = (tmp - retry) /
> skb_queue_len(&priv->b_tx_status.queue);
> while
> (skb_queue_len(&priv->b_tx_status.queue) > 0) {
> struct sk_buff
> *old_skb;
>
> old_skb =
> skb_dequeue(&priv->b_tx_status.queue);
> info =
> IEEE80211_SKB_CB(old_skb);
> -
> info->status.rates[0].count = tmp - retry + 1;
> +
> info->status.rates[0].count = avg_retry + 1;
> + if
> (info->status.rates[0].count > RETRY_COUNT)
> +
> info->flags &= ~IEEE80211_TX_STAT_ACK;
>
> ieee80211_tx_status_irqsafe(dev, old_skb);
> }
> retry = tmp;
> @@ -931,8 +937,8 @@ static int rtl8187_start(struct
> ieee8021
>
> rtl818x_iowrite32(priv, &priv->map->TX_CONF,
>
> RTL818X_TX_CONF_HW_SEQNUM |
>
> RTL818X_TX_CONF_DISREQQSIZE |
> -
> (7 << 8 /* short retry
> limit */) |
> -
> (7 << 0 /* long retry
> limit */) |
> +
> (RETRY_COUNT << 8 /*
> short retry limit */) |
> +
> (RETRY_COUNT << 0 /*
> long retry limit */) |
>
> (7 << 21 /* MAX TX DMA
> */));
>
> rtl8187_init_urbs(dev);
>
> rtl8187b_init_status_urb(dev);
> @@ -1376,6 +1382,9 @@ static int __devinit
> rtl8187_probe(struc
> dev->flags =
> IEEE80211_HW_HOST_BROADCAST_PS_BUFFERING |
>
> IEEE80211_HW_SIGNAL_DBM |
>
> IEEE80211_HW_RX_INCLUDES_FCS;
> + /* Initialize rate-control variables
> */
> + dev->max_rates = 1;
> + dev->max_rate_tries = RETRY_COUNT;
>
> eeprom.data = dev;
> eeprom.register_read =
> rtl8187_eeprom_register_read;
> Index:
> wireless-testing/drivers/net/wireless/rtl818x/rtl8187/rtl8187.h
> ===================================================================
> ---
> wireless-testing.orig/drivers/net/wireless/rtl818x/rtl8187/rtl8187.h
> +++
> wireless-testing/drivers/net/wireless/rtl818x/rtl8187/rtl8187.h
> @@ -35,6 +35,8 @@
> #define RFKILL_MASK_8187_89_97 0x2
> #define RFKILL_MASK_8198 0x4
>
> +#define RETRY_COUNT
> 7
> +
> struct rtl8187_rx_info {
> struct urb *urb;
> struct ieee80211_hw *dev;
>
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] rtl8187: Change rate-control feedback
2011-02-28 2:21 ` Hin-Tak Leung
@ 2011-02-28 3:41 ` Larry Finger
0 siblings, 0 replies; 3+ messages in thread
From: Larry Finger @ 2011-02-28 3:41 UTC (permalink / raw)
To: htl10; +Cc: John W Linville, Herton Ronaldo Krzesinski, linux-wireless
On 02/27/2011 08:21 PM, Hin-Tak Leung wrote:
>
> --- On Sun, 27/2/11, Larry Finger<Larry.Finger@lwfinger.net> wrote:
>
>> The driver for the RTL8187L chips
>> returns IEEE80211_TX_STAT_ACK for all
>> packets, even if the maximum number of retries was
>> exhausted. In addition
>> it fails to setup max_rates in the ieee80211_hw struct,
>> This behavior
>> may be responsible for the problems noted in Bug 14168. As
>> the bug is very
>> old, testers have not been found, and I do not have the
>> case where the
>> indicated signal is less than -70 dBm.
>
>
> Possibility of division by zero? The while loop dequeues - does this code always get invoked with a non-empty queue? Also it worrys me a little if tmp< retry?
This routine is not schedules until a packet has been transmitted, thus the
queue length will always be at least one, but I should protect against a divide
by zero anyway.
The hardware keeps track of the retries. I suppose there could be a possibility
of the register wrapping back to zero. I'll throw in a test there as well.
Thanks,
Larry
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2011-02-28 3:41 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-02-27 20:44 [PATCH] rtl8187: Change rate-control feedback Larry Finger
2011-02-28 2:21 ` Hin-Tak Leung
2011-02-28 3:41 ` Larry Finger
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox