On 15/05/17 20:31, Lennert Buytenhek wrote:
> 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.
I no longer have any ep93xx hardware to test with, and I never looked at
the Ethernet, so don't know the details. I think there are still a
handful of users. Adding Alexander who sent an ADC support series this
week, who might be able to test this?
~Ryan
