On Sat, Feb 04, 2017 at 03:24:56PM -0800, Eric Dumazet wrote:

> Use napi_complete_done() instead of __napi_complete() to :
> 
> 1) Get support of gro_flush_timeout if opt-in
> 2) Not rearm interrupts for busy-polling users.
> 3) use standard NAPI API.
> 4) get rid of baroque code and ease maintenance.
>
> [...]
>
> @@ -310,35 +311,17 @@ static int ep93xx_rx(struct net_device *dev, int 
> processed, int budget)
>       return processed;
>  }
>  
> -static int ep93xx_have_more_rx(struct ep93xx_priv *ep)
> -{
> -     struct ep93xx_rstat *rstat = ep->descs->rstat + ep->rx_pointer;
> -     return !!((rstat->rstat0 & RSTAT0_RFP) && (rstat->rstat1 & RSTAT1_RFP));
> -}
> -
>  static int ep93xx_poll(struct napi_struct *napi, int budget)
>  {
>       struct ep93xx_priv *ep = container_of(napi, struct ep93xx_priv, napi);
>       struct net_device *dev = ep->dev;
> -     int rx = 0;
> -
> -poll_some_more:
> -     rx = ep93xx_rx(dev, rx, budget);
> -     if (rx < budget) {
> -             int more = 0;
> +     int rx;
>  
> +     rx = ep93xx_rx(dev, budget);
> +     if (rx < budget && napi_complete_done(napi, rx)) {
>               spin_lock_irq(&ep->rx_lock);
> -             __napi_complete(napi);
>               wrl(ep, REG_INTEN, REG_INTEN_TX | REG_INTEN_RX);
> -             if (ep93xx_have_more_rx(ep)) {
> -                     wrl(ep, REG_INTEN, REG_INTEN_TX);
> -                     wrl(ep, REG_INTSTSP, REG_INTSTS_RX);
> -                     more = 1;
> -             }
>               spin_unlock_irq(&ep->rx_lock);
> -
> -             if (more && napi_reschedule(napi))
> -                     goto poll_some_more;
>       }
>  
>       if (rx) {

This code was the way it was because the ep93xx hardware is somewhat
braindead.  If I remember correctly (but it's been a while since I wrote
this code):

1. ep93xx netdev IRQs are edge-triggered, so if you re-enable IRQs
   while there was still work to be done, you will not get another IRQ.

2. Disabling an interrupt source in the interrupt mask register will
   cause its interrupt status bit to always return zero, so you cannot
   check whether an interrupt status is pending without having the
   interrupt source enabled.

(I'll admit that a comment explaining this would have been in order.)

I don't know if we really care about this hardware anymore (I don't),
but the ep93xx platform is still listed as being maintained in the
MAINTAINERS file -- adding Ryan and Hartley.

Reply via email to