Re: Issue with commit 33c133cc7598e60976a phy: IRQ cannot be shared
Hello. On 08/15/2014 01:10 PM, christophe leroy 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. Indeed, my reasoning seems obsolete now, if ever valid at all. :-/ I couldn't yet remember other reasons that caused me to do that patch last December. Perhaps it was also connected to the "rude" behaviour of the phylib's IRQ handler, which calls disable_irq_nosync()... [...] Reading the commit log, I can't really understand the reason for the change. 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. Yes, but you can't avoid it even with valid IRQ, the way phylib is written: the state workqueue is activated once a second even in the absence of interrupts. What can also be done is getting rid of the IRQ workqueue and using threaded IRQs, BR Christophe WBR, Sergei -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: Issue with commit 33c133cc7598e60976a phy: IRQ cannot be shared
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 majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: Issue with commit 33c133cc7598e60976a phy: IRQ cannot be shared
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 majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: Issue with commit 33c133cc7598e60976a phy: IRQ cannot be shared
Hello. On 08/15/2014 01:10 PM, christophe leroy 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. Indeed, my reasoning seems obsolete now, if ever valid at all. :-/ I couldn't yet remember other reasons that caused me to do that patch last December. Perhaps it was also connected to the rude behaviour of the phylib's IRQ handler, which calls disable_irq_nosync()... [...] Reading the commit log, I can't really understand the reason for the change. 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. Yes, but you can't avoid it even with valid IRQ, the way phylib is written: the state workqueue is activated once a second even in the absence of interrupts. What can also be done is getting rid of the IRQ workqueue and using threaded IRQs, BR Christophe WBR, Sergei -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: Issue with commit 33c133cc7598e60976a phy: IRQ cannot be shared
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. 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. 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). Is it really worth it, and therefore how shall my case be handled ? PHY IRQs are not necessary for the phylib state machine. Christophe WBR, Sergei -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Issue with commit 33c133cc7598e60976a phy: IRQ cannot be shared
Hello Segei, Florian and David, 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. But since your commit, introduced in Linux 3.13, my interfaces don't work anymore as the second PHYs can't register IRQ. Reading the commit log, I can't really understand the reason for the change. Is it really worth it, and therefore how shall my case be handled ? Christophe -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Issue with commit 33c133cc7598e60976a phy: IRQ cannot be shared
Hello Segei, Florian and David, 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. But since your commit, introduced in Linux 3.13, my interfaces don't work anymore as the second PHYs can't register IRQ. Reading the commit log, I can't really understand the reason for the change. Is it really worth it, and therefore how shall my case be handled ? Christophe -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: Issue with commit 33c133cc7598e60976a phy: IRQ cannot be shared
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. 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. 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). Is it really worth it, and therefore how shall my case be handled ? PHY IRQs are not necessary for the phylib state machine. Christophe WBR, Sergei -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/