Re: [PATCH] rtl8187: Change rate-control feedback

[Hin-Tak Leung <htl10@users.sourceforge.net>]

--- 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;
>