RE: [PATCH rtw-next v3 3/3] wifi: rtl8xxxu: preserve RX requests across recoverable transfer errors

[Ping-Ke Shih <pkshih@realtek.com>]

Kim Wooseok via B4 Relay <devnull+5mghybrid.khu.ac.kr@kernel.org> wrote:

[...]

> @@ -5855,9 +5856,8 @@ static int rtl8xxxu_alloc_rx_urbs(struct rtl8xxxu_priv *priv)
>  }
> 
>  static void rtl8xxxu_queue_rx_urb(struct rtl8xxxu_priv *priv,
> -                                 struct rtl8xxxu_rx_urb *rx_urb)
> +                                 struct rtl8xxxu_rx_urb *rx_urb, bool retry)

'retry' --> 'defer_schedule'?

>  {
> -       struct sk_buff *skb;
>         unsigned long flags;
> 
>         spin_lock_irqsave(&priv->rx_urb_lock, flags);
> @@ -5865,16 +5865,13 @@ static void rtl8xxxu_queue_rx_urb(struct rtl8xxxu_priv *priv,
>         if (!priv->shutdown) {
>                 list_add_tail(&rx_urb->list, &priv->rx_urb_pending_list);
>                 priv->rx_urb_pending_count++;
> -               /*
> -                * Arm the worker under rx_urb_lock so this is atomic with the
> -                * shutdown check: moving it out of the lock would let a
> -                * completion arm the work after rtl8xxxu_stop() canceled it.
> -                */
> -               if (priv->rx_urb_pending_count > RTL8XXXU_RX_URB_PENDING_WATER)
> -                       schedule_work(&priv->rx_urb_wq);
> +               /* Serialize scheduling with the shutdown check and cancellation. */
> +               if (retry)
> +                       queue_delayed_work(system_wq, &priv->rx_urb_wq,
> +                                          msecs_to_jiffies(RTL8XXXU_RX_URB_RETRY_DELAY_MS));
> +               else if (priv->rx_urb_pending_count > RTL8XXXU_RX_URB_PENDING_WATER)
> +                       mod_delayed_work(system_wq, &priv->rx_urb_wq, 0);

Not prefer using bare system_wq.
Can both use schedule_delayed_work()? 

>         } else {
> -               skb = (struct sk_buff *)rx_urb->urb.context;
> -               dev_kfree_skb_irq(skb);
>                 usb_free_urb(&rx_urb->urb);
>         }
> 

[...]

> @@ -6585,10 +6583,23 @@ static void rtl8xxxu_rx_complete(struct urb *urb)
> 
>                 skb = NULL;
>                 rx_urb->urb.context = NULL;
> -               rtl8xxxu_queue_rx_urb(priv, rx_urb);
> +               rtl8xxxu_queue_rx_urb(priv, rx_urb, false);
>         } else {
>                 dev_dbg(dev, "%s: status %i\n", __func__, urb->status);

nit: a blank line

> -               goto cleanup;
> +               switch (urb->status) {
> +               case -EPROTO:
> +               case -EILSEQ:
> +               case -ETIME:
> +               case -EOVERFLOW:
> +               case -ECOMM:
> +               case -ENOSR:
> +                       dev_kfree_skb(skb);
> +                       urb->context = NULL;
> +                       rtl8xxxu_queue_rx_urb(priv, rx_urb, true);
> +                       return;
> +               default:
> +                       goto cleanup;
> +               }
>         }
>         return;
> 

[...]