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

[christophe leroy]

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

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/