Re: Issue with commit 33c133cc7598e60976a phy: IRQ cannot be shared

[christophe leroy <christophe.leroy@c-s.fr>]

Le 14/08/2014 13:03, Sergei Shtylyov a écrit :
> Hello.
>
> On 8/14/2014 10:31 AM, leroy christophe wrote:
>
>> I have an hardware with two ethernet interfaces, and with the two 
>> PHYs inside
>> the same component INTEL LXT973 which has only one interrupt.
>> I also have another hardware with two ethernet interfaces and two 
>> independant
>> PHYs. But the two PHYs are wired to the same interrupt.
>> This is working perfectly up to Linux 3.12.
>
>    Hm, I'm surprised it works. Are you sure you're getting interrupts 
> from both PHYs? Because if both Ethernet controllers are active 
> simultaneously, only the first registered PHY IRQ handler should get 
> all the interrupts.
Yes it works. Why should only the first one get the interrupts ? 
handle_irq_event_percpu() (from kernel/irq/handle.c, extract below) 
calls all handlers regardless of whether they answer IRQ_NONE or 
IRQ_HANDLED. The break applies to the switch(), not to the while(). So 
all handlers are called.

irqreturn_t
handle_irq_event_percpu(struct irq_desc *desc, struct irqaction *action)
{
     irqreturn_t retval = IRQ_NONE;
     unsigned int flags = 0, irq = desc->irq_data.irq;

     do {
[...]
         switch (res) {
         case IRQ_WAKE_THREAD:
[...]
         case IRQ_HANDLED:
             flags |= action->flags;
             break;

         default:
             break;
         }

         retval |= res;
         action = action->next;
     } while (action);

>
>> But since your commit, introduced in Linux 3.13, my interfaces don't 
>> work
>> anymore as the second PHYs can't register IRQ.
>
>    Strange too, the phylib should use polling in case request_irq() 
> fails.
Well, you are right, I didn't check closely enough, was assuming they 
didn't register due to the messages saying interrupt mismatch.
>
>> Reading the commit log, I can't really understand the reason for the 
>> change.
>
>    The shared IRQ handler should check for IRQ from its device and 
> return IRQ_NONE if there's no IRQ active; phy_interrupt() doesn't do 
> that (this is pushed to the workqueue).
Well, as seen above, this has no impact on whether other handlers are 
called or not.
>
>> Is it really worth it, and therefore how shall my case be handled ?
>
>    PHY IRQs are not necessary for the phylib state machine.
However, polling is less efficient than IRQs. It wastes CPU and link 
loss detection is slower.
>
>> Christophe
>
> WBR, Sergei
>
BR
Christophe

---
Ce courrier électronique ne contient aucun virus ou logiciel malveillant parce que la protection avast! Antivirus est active.
http://www.avast.com