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

2014-08-15 Thread Sergei Shtylyov

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

2014-08-15 Thread 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 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

2014-08-15 Thread 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 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

2014-08-15 Thread Sergei Shtylyov

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

2014-08-14 Thread Sergei Shtylyov

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

2014-08-14 Thread leroy christophe

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

2014-08-14 Thread leroy christophe

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

2014-08-14 Thread Sergei Shtylyov

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/