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

christophe leroy
2014-08-15T09:10:20Z

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.