diff mbox

[for,3.9-rc] arm: mvebu: Use local interrupt only for the timer 0

Message ID 1363792175-3136-1-git-send-email-gregory.clement@free-electrons.com (mailing list archive)
State New, archived
Headers show

Commit Message

Gregory CLEMENT March 20, 2013, 3:09 p.m. UTC
The commit 3a6f08a37 "arm: mvebu: Add support for local interrupt",
managed the 28th first interrupts as local interrupt to match the
hardware specification. Among these interrupts there are the Gigabits
Ethernet ones used by the mvneta driver. Unfortunately the state of
the percpu_irq API prevents the driver to use it.

Indeed the interrupts have to be freed when the .stop() function is
called. As the free_percpu_irq() function don't disable the interrupt
line, we have to do it on each CPU before calling this. The function
disable_percpu_irq() only disable the percpu on the current CPU and
there is no function which allows to disable a percpu irq on a given
CPU. Waiting for the extension of the percpu_irq API, this fix allows
to use again the mvneta driver.

Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
---
 arch/arm/mach-mvebu/irq-armada-370-xp.c |    8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

Comments

Thomas Petazzoni March 20, 2013, 3:37 p.m. UTC | #1
Hello,

On Wed, 20 Mar 2013 16:09:35 +0100, Gregory CLEMENT wrote:
> The commit 3a6f08a37 "arm: mvebu: Add support for local interrupt",
> managed the 28th first interrupts as local interrupt to match the
> hardware specification. Among these interrupts there are the Gigabits
> Ethernet ones used by the mvneta driver. Unfortunately the state of
> the percpu_irq API prevents the driver to use it.
> 
> Indeed the interrupts have to be freed when the .stop() function is
> called. As the free_percpu_irq() function don't disable the interrupt
> line, we have to do it on each CPU before calling this. The function
> disable_percpu_irq() only disable the percpu on the current CPU and
> there is no function which allows to disable a percpu irq on a given
> CPU. Waiting for the extension of the percpu_irq API, this fix allows
> to use again the mvneta driver.
> 
> Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com>

Tested-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
 (on Armada XP GP board)

Thanks!

Thomas
Ryan Press March 20, 2013, 4:44 p.m. UTC | #2
On Wed, Mar 20, 2013 at 8:09 AM, Gregory CLEMENT
<gregory.clement@free-electrons.com> wrote:
> The commit 3a6f08a37 "arm: mvebu: Add support for local interrupt",
> managed the 28th first interrupts as local interrupt to match the
> hardware specification. Among these interrupts there are the Gigabits
> Ethernet ones used by the mvneta driver. Unfortunately the state of
> the percpu_irq API prevents the driver to use it.
>
> Indeed the interrupts have to be freed when the .stop() function is
> called. As the free_percpu_irq() function don't disable the interrupt
> line, we have to do it on each CPU before calling this. The function
> disable_percpu_irq() only disable the percpu on the current CPU and
> there is no function which allows to disable a percpu irq on a given
> CPU. Waiting for the extension of the percpu_irq API, this fix allows
> to use again the mvneta driver.

Thanks Gregory!

I can confirm this patch works for me as well.  I still have the
problem on my Globalscale Mirabox where only a forced 10Mb link works
on tx/rx.  With 100Mb only rx works, and 1Gb neither works.

Ryan
Thomas Petazzoni March 20, 2013, 5:27 p.m. UTC | #3
Dear Ryan Press,

On Wed, 20 Mar 2013 09:44:42 -0700, Ryan Press wrote:

> I can confirm this patch works for me as well.  I still have the
> problem on my Globalscale Mirabox where only a forced 10Mb link works
> on tx/rx.  With 100Mb only rx works, and 1Gb neither works.

Did you test both network interfaces? Do you have the problem on both?
I know there are issues on the Mirabox with the network interface that
wasn't initialized by U-Boot (i.e if you tftp your kernel from
interface 0, interface 0 works fine in Linux but not interface 1, if
your tftp your kernel from interface 1, it's the opposite).

Best regards,

Thomas
Ryan Press March 20, 2013, 11:57 p.m. UTC | #4
Hi Thomas,

On Wed, Mar 20, 2013 at 10:27 AM, Thomas Petazzoni
<thomas.petazzoni@free-electrons.com> wrote:
> Dear Ryan Press,
>
> On Wed, 20 Mar 2013 09:44:42 -0700, Ryan Press wrote:
>
>> I can confirm this patch works for me as well.  I still have the
>> problem on my Globalscale Mirabox where only a forced 10Mb link works
>> on tx/rx.  With 100Mb only rx works, and 1Gb neither works.
>
> Did you test both network interfaces? Do you have the problem on both?
> I know there are issues on the Mirabox with the network interface that
> wasn't initialized by U-Boot (i.e if you tftp your kernel from
> interface 0, interface 0 works fine in Linux but not interface 1, if
> your tftp your kernel from interface 1, it's the opposite).

Ah, yes, so when I initialize the controller in U-Boot by attempting a
tftp download (and then just ctrl-c out) before booting then I get 1Gb
link.  At first I suspected the PHY registers being different, but I
dumped the registers when working vs non-working and they seemed the
same, but hard to know for sure.  Then I dumped some of the MAC
registers and I see that 0x2410 is different, it shows that
MVNETA_GMAC_RX_FLOW_CTRL_ENABLE and MVNETA_GMAC_TX_FLOW_CTRL_ENABLE
(working 0x2c3b vs non-working 0x2401) bits are set when it is
working.  0x2410 is just a status word and I don't know how to turn
the flow control on otherwise, and probably there is more than this
needed.

Thanks,
Ryan
Masami Hiramatsu March 21, 2013, 2:19 a.m. UTC | #5
(2013/03/21 0:09), Gregory CLEMENT wrote:
> The commit 3a6f08a37 "arm: mvebu: Add support for local interrupt",
> managed the 28th first interrupts as local interrupt to match the
> hardware specification. Among these interrupts there are the Gigabits
> Ethernet ones used by the mvneta driver. Unfortunately the state of
> the percpu_irq API prevents the driver to use it.
> 
> Indeed the interrupts have to be freed when the .stop() function is
> called. As the free_percpu_irq() function don't disable the interrupt
> line, we have to do it on each CPU before calling this. The function
> disable_percpu_irq() only disable the percpu on the current CPU and
> there is no function which allows to disable a percpu irq on a given
> CPU. Waiting for the extension of the percpu_irq API, this fix allows
> to use again the mvneta driver.
> 
> Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com>

Hi,

I've tested this too.

Tested-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>

Thank you!
Thomas Petazzoni March 26, 2013, 4 p.m. UTC | #6
Jason, Andrew,

On Wed, 20 Mar 2013 16:09:35 +0100, Gregory CLEMENT wrote:
> The commit 3a6f08a37 "arm: mvebu: Add support for local interrupt",
> managed the 28th first interrupts as local interrupt to match the
> hardware specification. Among these interrupts there are the Gigabits
> Ethernet ones used by the mvneta driver. Unfortunately the state of
> the percpu_irq API prevents the driver to use it.
> 
> Indeed the interrupts have to be freed when the .stop() function is
> called. As the free_percpu_irq() function don't disable the interrupt
> line, we have to do it on each CPU before calling this. The function
> disable_percpu_irq() only disable the percpu on the current CPU and
> there is no function which allows to disable a percpu irq on a given
> CPU. Waiting for the extension of the percpu_irq API, this fix allows
> to use again the mvneta driver.
> 
> Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com>

Would it be possible to get this commit pushed for 3.9-rc? Without it,
the Marvell Armada 370/XP network interfaces are completely broken. The
patch has been sent almost a week ago, and has received Tested-by from
Ryan Press, Masami Hiramatsu and myself.

Thanks,

Thomas
Jason Cooper March 26, 2013, 10:43 p.m. UTC | #7
On Tue, Mar 26, 2013 at 05:00:51PM +0100, Thomas Petazzoni wrote:
> Jason, Andrew,
> 
> On Wed, 20 Mar 2013 16:09:35 +0100, Gregory CLEMENT wrote:
> > The commit 3a6f08a37 "arm: mvebu: Add support for local interrupt",
> > managed the 28th first interrupts as local interrupt to match the
> > hardware specification. Among these interrupts there are the Gigabits
> > Ethernet ones used by the mvneta driver. Unfortunately the state of
> > the percpu_irq API prevents the driver to use it.
> > 
> > Indeed the interrupts have to be freed when the .stop() function is
> > called. As the free_percpu_irq() function don't disable the interrupt
> > line, we have to do it on each CPU before calling this. The function
> > disable_percpu_irq() only disable the percpu on the current CPU and
> > there is no function which allows to disable a percpu irq on a given
> > CPU. Waiting for the extension of the percpu_irq API, this fix allows
> > to use again the mvneta driver.
> > 
> > Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
> 
> Would it be possible to get this commit pushed for 3.9-rc? Without it,
> the Marvell Armada 370/XP network interfaces are completely broken. The
> patch has been sent almost a week ago, and has received Tested-by from
> Ryan Press, Masami Hiramatsu and myself.

It's in the queue, I just got a little behind.

thx,

Jason.
Gregory CLEMENT March 27, 2013, 6:48 a.m. UTC | #8
On 03/26/2013 11:43 PM, Jason Cooper wrote:
> On Tue, Mar 26, 2013 at 05:00:51PM +0100, Thomas Petazzoni wrote:
>> Jason, Andrew,
>>
>> On Wed, 20 Mar 2013 16:09:35 +0100, Gregory CLEMENT wrote:
>>> The commit 3a6f08a37 "arm: mvebu: Add support for local interrupt",
>>> managed the 28th first interrupts as local interrupt to match the
>>> hardware specification. Among these interrupts there are the Gigabits
>>> Ethernet ones used by the mvneta driver. Unfortunately the state of
>>> the percpu_irq API prevents the driver to use it.
>>>
>>> Indeed the interrupts have to be freed when the .stop() function is
>>> called. As the free_percpu_irq() function don't disable the interrupt
>>> line, we have to do it on each CPU before calling this. The function
>>> disable_percpu_irq() only disable the percpu on the current CPU and
>>> there is no function which allows to disable a percpu irq on a given
>>> CPU. Waiting for the extension of the percpu_irq API, this fix allows
>>> to use again the mvneta driver.
>>>
>>> Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
>>
>> Would it be possible to get this commit pushed for 3.9-rc? Without it,
>> the Marvell Armada 370/XP network interfaces are completely broken. The
>> patch has been sent almost a week ago, and has received Tested-by from
>> Ryan Press, Masami Hiramatsu and myself.
> 
> It's in the queue, I just got a little behind.
> 
> thx,
> 

Thanks Jason

> Jason.
>
Jason Cooper March 28, 2013, 4:49 p.m. UTC | #9
On Wed, Mar 20, 2013 at 04:09:35PM +0100, Gregory CLEMENT wrote:
> The commit 3a6f08a37 "arm: mvebu: Add support for local interrupt",
> managed the 28th first interrupts as local interrupt to match the
> hardware specification. Among these interrupts there are the Gigabits
> Ethernet ones used by the mvneta driver. Unfortunately the state of
> the percpu_irq API prevents the driver to use it.
> 
> Indeed the interrupts have to be freed when the .stop() function is
> called. As the free_percpu_irq() function don't disable the interrupt
> line, we have to do it on each CPU before calling this. The function
> disable_percpu_irq() only disable the percpu on the current CPU and
> there is no function which allows to disable a percpu irq on a given
> CPU. Waiting for the extension of the percpu_irq API, this fix allows
> to use again the mvneta driver.
> 
> Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
> ---
>  arch/arm/mach-mvebu/irq-armada-370-xp.c |    8 +++++---
>  1 file changed, 5 insertions(+), 3 deletions(-)

Applied to mvebu/fixes

thx,

Jason.
diff mbox

Patch

diff --git a/arch/arm/mach-mvebu/irq-armada-370-xp.c b/arch/arm/mach-mvebu/irq-armada-370-xp.c
index 274ff58..6a9195e 100644
--- a/arch/arm/mach-mvebu/irq-armada-370-xp.c
+++ b/arch/arm/mach-mvebu/irq-armada-370-xp.c
@@ -44,6 +44,8 @@ 
 
 #define ARMADA_370_XP_MAX_PER_CPU_IRQS		(28)
 
+#define ARMADA_370_XP_TIMER0_PER_CPU_IRQ	(5)
+
 #define ACTIVE_DOORBELLS			(8)
 
 static DEFINE_RAW_SPINLOCK(irq_controller_lock);
@@ -62,7 +64,7 @@  static void armada_370_xp_irq_mask(struct irq_data *d)
 #ifdef CONFIG_SMP
 	irq_hw_number_t hwirq = irqd_to_hwirq(d);
 
-	if (hwirq > ARMADA_370_XP_MAX_PER_CPU_IRQS)
+	if (hwirq != ARMADA_370_XP_TIMER0_PER_CPU_IRQ)
 		writel(hwirq, main_int_base +
 				ARMADA_370_XP_INT_CLEAR_ENABLE_OFFS);
 	else
@@ -79,7 +81,7 @@  static void armada_370_xp_irq_unmask(struct irq_data *d)
 #ifdef CONFIG_SMP
 	irq_hw_number_t hwirq = irqd_to_hwirq(d);
 
-	if (hwirq > ARMADA_370_XP_MAX_PER_CPU_IRQS)
+	if (hwirq != ARMADA_370_XP_TIMER0_PER_CPU_IRQ)
 		writel(hwirq, main_int_base +
 				ARMADA_370_XP_INT_SET_ENABLE_OFFS);
 	else
@@ -147,7 +149,7 @@  static int armada_370_xp_mpic_irq_map(struct irq_domain *h,
 	writel(hw, main_int_base + ARMADA_370_XP_INT_SET_ENABLE_OFFS);
 	irq_set_status_flags(virq, IRQ_LEVEL);
 
-	if (hw < ARMADA_370_XP_MAX_PER_CPU_IRQS) {
+	if (hw == ARMADA_370_XP_TIMER0_PER_CPU_IRQ) {
 		irq_set_percpu_devid(virq);
 		irq_set_chip_and_handler(virq, &armada_370_xp_irq_chip,
 					handle_percpu_devid_irq);