[PATCH] sparc32,leon: need VIRQ:IRQ 1:1, don't mask/ack IRQ Controller

[PATCH] sparc32,leon: need VIRQ:IRQ 1:1, don't mask/ack IRQ Controller

[Daniel Hellstrom]

The AMBA Plug&Play information supports only 1 IRQ per AMBA device,
a device node in linux is an AMBA core which consists of 3 devices
(AHB Master, AHB Slave and APB Slave in any combination). The IRQ
information is really the first IRQ of a device, the Plug&Play does
not tell how many IRQs are actually present on a device so the
interrupt property of a device can impossibly tell how many IRQs
each device has, it must be up to the driver to probe the hardware
in a device-specific way simply know how many IRQs are present.

Using Virtual IRQs does not work for such devices, because VIRQ+1
is not REAL_IRQ+1. Thus, the LEON architecture needs real IRQs or
map virtual IRQs 1:1 to real IRQs.

This patch changes the VIRQ allocation to try the real IRQ if not
already taken.

LEON IRQ Controller is edge triggered, not masking or acking is
needed in the normal case, handle_simple_irq handler can be used
for that.

Signed-off-by: Daniel Hellstrom <daniel@gaisler.com>
---
 arch/sparc/kernel/irq_32.c      |   11 ++++++++---
 arch/sparc/kernel/leon_kernel.c |    2 +-
 2 files changed, 9 insertions(+), 4 deletions(-)

diff --git a/arch/sparc/kernel/irq_32.c b/arch/sparc/kernel/irq_32.c
index 9ce6b97..46a573a 100644
--- a/arch/sparc/kernel/irq_32.c
+++ b/arch/sparc/kernel/irq_32.c
@@ -135,9 +135,14 @@ unsigned int irq_alloc(unsigned int real_irq, unsigned int pil)
 			return i;
 	}
 
-	for (i = 1; i < NR_IRQS; i++) {
-		if (!irq_table[i].irq)
-			break;
+	/* Try 1:1 map between Virtual IRQ and real IRQ */
+	if (real_irq < NR_IRQS && irq_table[real_irq].irq = 0) {
+		i = real_irq;
+	} else {
+		for (i = 1; i < NR_IRQS; i++) {
+			if (!irq_table[i].irq)
+				break;
+		}
 	}
 
 	if (i >= NR_IRQS) {
diff --git a/arch/sparc/kernel/leon_kernel.c b/arch/sparc/kernel/leon_kernel.c
index feaba14..0c1ae2b 100644
--- a/arch/sparc/kernel/leon_kernel.c
+++ b/arch/sparc/kernel/leon_kernel.c
@@ -136,7 +136,7 @@ static unsigned int leon_build_device_irq(struct platform_device *op,
 		goto out;
 
 	set_irq_chip_and_handler_name(irq, &leon_irq,
-	                              handle_level_irq, "LEON");
+				      handle_simple_irq, "LEON");
 	set_irq_chip_data(irq, (void *)mask);
 
 out:
-- 
1.5.4

Re: [PATCH] sparc32,leon: need VIRQ:IRQ 1:1, don't mask/ack IRQ

[David Miller]

From: Daniel Hellstrom <daniel@gaisler.com>
Date: Wed, 16 Mar 2011 16:54:24 +0100

> The AMBA Plug&Play information supports only 1 IRQ per AMBA device,
> a device node in linux is an AMBA core which consists of 3 devices
> (AHB Master, AHB Slave and APB Slave in any combination). The IRQ
> information is really the first IRQ of a device, the Plug&Play does
> not tell how many IRQs are actually present on a device so the
> interrupt property of a device can impossibly tell how many IRQs
> each device has, it must be up to the driver to probe the hardware
> in a device-specific way simply know how many IRQs are present.
> 
> Using Virtual IRQs does not work for such devices, because VIRQ+1
> is not REAL_IRQ+1. Thus, the LEON architecture needs real IRQs or
> map virtual IRQs 1:1 to real IRQs.
> 
> This patch changes the VIRQ allocation to try the real IRQ if not
> already taken.
> 
> LEON IRQ Controller is edge triggered, not masking or acking is
> needed in the normal case, handle_simple_irq handler can be used
> for that.
> 
> Signed-off-by: Daniel Hellstrom <daniel@gaisler.com>

As I stated in another email I am absolutely and completely against
this change.

Just because you only store one interrupt in the firmware device node,
it does not mean that you cannot store multiple interrupts in the
internal device node structure we build.  And then have the drivers
use that.

Re: [PATCH] sparc32,leon: need VIRQ:IRQ 1:1, don't mask/ack IRQ Controller

[Daniel Hellstrom]

David Miller wrote:

>From: Daniel Hellstrom <daniel@gaisler.com>
>Date: Wed, 16 Mar 2011 16:54:24 +0100
>
>  
>
>>The AMBA Plug&Play information supports only 1 IRQ per AMBA device,
>>a device node in linux is an AMBA core which consists of 3 devices
>>(AHB Master, AHB Slave and APB Slave in any combination). The IRQ
>>information is really the first IRQ of a device, the Plug&Play does
>>not tell how many IRQs are actually present on a device so the
>>interrupt property of a device can impossibly tell how many IRQs
>>each device has, it must be up to the driver to probe the hardware
>>in a device-specific way simply know how many IRQs are present.
>>
>>Using Virtual IRQs does not work for such devices, because VIRQ+1
>>is not REAL_IRQ+1. Thus, the LEON architecture needs real IRQs or
>>map virtual IRQs 1:1 to real IRQs.
>>
>>This patch changes the VIRQ allocation to try the real IRQ if not
>>already taken.
>>
>>LEON IRQ Controller is edge triggered, not masking or acking is
>>needed in the normal case, handle_simple_irq handler can be used
>>for that.
>>
>>Signed-off-by: Daniel Hellstrom <daniel@gaisler.com>
>>    
>>
>
>As I stated in another email I am absolutely and completely against
>this change.
>
>Just because you only store one interrupt in the firmware device node,
>it does not mean that you cannot store multiple interrupts in the
>internal device node structure we build.  And then have the drivers
>use that.
>  
>
Yes, I agree with you that this is the best solution. However, the 
hardware simply does not support that. That would require the bootloader 
to have one driver per core and that the bootloader must be modified 
each time a user of LEON or gaisler adds a new core.

I have made another patch that creates all 0..15 IRQs on startup instead 
in leon_kernel.c. It will ensure linearity within that range, and 
patching of irq_32.c is not needed. Perhaps that is more acceptable?

Regards,
Daniel

Re: [PATCH] sparc32,leon: need VIRQ:IRQ 1:1, don't mask/ack IRQ

[David Miller]

From: Daniel Hellstrom <daniel@gaisler.com>
Date: Wed, 30 Mar 2011 12:00:53 +0200

> Yes, I agree with you that this is the best solution. However, the
> hardware simply does not support that. That would require the
> bootloader to have one driver per core and that the bootloader must be
> modified each time a user of LEON or gaisler adds a new core.
> 
> I have made another patch that creates all 0..15 IRQs on startup
> instead in leon_kernel.c. It will ensure linearity within that range,
> and patching of irq_32.c is not needed. Perhaps that is more
> acceptable?

I am not asking you to modify the boot loader or whatever creates
the firmware device tree on LEON.

I'm telling you to modify the kernel to create device node objects
which have an array of multiple IRQ entries when such devices are
found during the import of the device tree.

Re: [PATCH] sparc32,leon: need VIRQ:IRQ 1:1, don't mask/ack IRQ Controller

[Daniel Hellstrom]

David Miller wrote:

>From: Daniel Hellstrom <daniel@gaisler.com>
>Date: Wed, 30 Mar 2011 12:00:53 +0200
>
>  
>
>>Yes, I agree with you that this is the best solution. However, the
>>hardware simply does not support that. That would require the
>>bootloader to have one driver per core and that the bootloader must be
>>modified each time a user of LEON or gaisler adds a new core.
>>
>>I have made another patch that creates all 0..15 IRQs on startup
>>instead in leon_kernel.c. It will ensure linearity within that range,
>>and patching of irq_32.c is not needed. Perhaps that is more
>>acceptable?
>>    
>>
>
>I am not asking you to modify the boot loader or whatever creates
>the firmware device tree on LEON.
>
>I'm telling you to modify the kernel to create device node objects
>which have an array of multiple IRQ entries when such devices are
>found during the import of the device tree.
>  
>
I see, however that is almost as bad because that would also require one 
driver per core but in the LEON IRQ layer of Linux.

If I always create an array of say 8 IRQs starting from the first IRQ, 
for example for IRQ5: {5,6,7,8,9,10,11,12}, that would be a solution for 
the drivers as well. 8 IRQs should be enough per core. That would 
however require that I change the scan_one_device() (of_device_32.c) and 
add an extra call to sparc_irq_config structure, fixup_device_irqs() or 
prepare_device_irqs() or something like that. This will result in 
devices with an archdata.irqs[] containing unused IRQs that are not 
really available. Do you think this is an acceptable solution?

Daniel

Re: [PATCH] sparc32,leon: need VIRQ:IRQ 1:1, don't mask/ack IRQ

[David Miller]

From: Daniel Hellstrom <daniel@gaisler.com>
Date: Wed, 30 Mar 2011 14:41:45 +0200

> I see, however that is almost as bad because that would also require
> one driver per core but in the LEON IRQ layer of Linux.

This is absolutely unavoidable since the firmware device tree doesn't
pass along the proper set of IRQs.  You have to fixup your tree,
because it is broken.

Re: [PATCH] sparc32,leon: need VIRQ:IRQ 1:1, don't mask/ack IRQ Controller

[Daniel Hellstrom]

David Miller wrote:

>From: Daniel Hellstrom <daniel@gaisler.com>
>Date: Wed, 30 Mar 2011 14:41:45 +0200
>
>  
>
>>I see, however that is almost as bad because that would also require
>>one driver per core but in the LEON IRQ layer of Linux.
>>    
>>
>
>This is absolutely unavoidable since the firmware device tree doesn't
>pass along the proper set of IRQs.  You have to fixup your tree,
>because it is broken.
>  
>
This is sad, as we have designed all LEON architecture to be Plug&Play 
as far as possible, and it will be software that limits it in the end. I 
think it is quite uncommon (if not unique) to do plug&play on the most 
low-level stuff such as timers and IRQ Controller etc., I believe that 
part of the idea with the OpenBoot tree is to provide that low-level 
plug&play to Linux and LEON that has true PnP in hardware can not do 
that fully. I'm starting to think virtual IRQ is a bad thing.. :)

I will give it another thought tomorrow before doing anything. A part of 
me thinks that the bootloader should be modified to report the correct 
information, however that requires changes to u-boot as well.

Daniel

Re: [PATCH] sparc32,leon: need VIRQ:IRQ 1:1, don't mask/ack IRQ

[David Miller]

From: Daniel Hellstrom <daniel@gaisler.com>
Date: Thu, 31 Mar 2011 17:13:35 +0200

> This is sad, as we have designed all LEON architecture to be Plug&Play
> as far as possible, and it will be software that limits it in the
> end.

Never is this the case.

For any future device you can provide properly formed firmware
device tree nodes, with all IRQ entries included.

Stop fighting this, you made a mistake in the past, and the set of
broken device nodes that need fixing will not expand if you start
creating them properly starting now.

You made a mistake, add handlers to cope with that mistake, and
take this opportunity to fix things so that future LEON device
nodes found in firmware are properly formed and contain multiple
IRQ entries when necessary.

Re: [PATCH] sparc32,leon: need VIRQ:IRQ 1:1, don't mask/ack IRQ Controller

[Daniel Hellstrom]

David Miller wrote:

>From: Daniel Hellstrom <daniel@gaisler.com>
>Date: Thu, 31 Mar 2011 17:13:35 +0200
>
>  
>
>>This is sad, as we have designed all LEON architecture to be Plug&Play
>>as far as possible, and it will be software that limits it in the
>>end.
>>    
>>
>
>Never is this the case.
>
>For any future device you can provide properly formed firmware
>device tree nodes, with all IRQ entries included.
>  
>
That is the problem, some cores need tweeking in bootloader or 
SPARC/LEON low IRQ layers. One can think of it as the LEON has a 
read-only OpenBoot device tree in hardware, but only provides the first IRQ.

>Stop fighting this, you made a mistake in the past, and the set of
>broken device nodes that need fixing will not expand if you start
>creating them properly starting now.
>
>You made a mistake, add handlers to cope with that mistake, and
>take this opportunity to fix things so that future LEON device
>nodes found in firmware are properly formed and contain multiple
>IRQ entries when necessary.
>  
>
You are correct. The hardware should be redesigned to contain the full 
information, and in the meantime we must fix this in software. I will 
try fixing this in the bootloader at first and see how things goes.

Thanks,
Daniel

Re: [PATCH] sparc32,leon: need VIRQ:IRQ 1:1, don't mask/ack IRQ Controller

[Daniel Hellstrom]

Daniel Hellstrom wrote:

> David Miller wrote:
>
>> From: Daniel Hellstrom <daniel@gaisler.com>
>> Date: Thu, 31 Mar 2011 17:13:35 +0200
>>
>>  
>>
>>> This is sad, as we have designed all LEON architecture to be Plug&Play
>>> as far as possible, and it will be software that limits it in the
>>> end.
>>>   
>>
>>
>> Never is this the case.
>>
>> For any future device you can provide properly formed firmware
>> device tree nodes, with all IRQ entries included.
>>  
>>
> That is the problem, some cores need tweeking in bootloader or 
> SPARC/LEON low IRQ layers. One can think of it as the LEON has a 
> read-only OpenBoot device tree in hardware, but only provides the 
> first IRQ.
>
>> Stop fighting this, you made a mistake in the past, and the set of
>> broken device nodes that need fixing will not expand if you start
>> creating them properly starting now.
>>
>> You made a mistake, add handlers to cope with that mistake, and
>> take this opportunity to fix things so that future LEON device
>> nodes found in firmware are properly formed and contain multiple
>> IRQ entries when necessary.
>>  
>>
> You are correct. The hardware should be redesigned to contain the full 
> information, and in the meantime we must fix this in software. I will 
> try fixing this in the bootloader at first and see how things goes.


I have updated my genirq patches by relying on the bootloader to create 
correct IRQ numbers. And I have added a PCI layer to LEON which also 
depends on the GENIRQ layer.

Dave, how do you prefer that I continue my work, should I wait for Sam 
to complete his work, or should I post the patches even though you can't 
apply them?

Daniel

Re: [PATCH] sparc32,leon: need VIRQ:IRQ 1:1, don't mask/ack IRQ

[David Miller]

From: Daniel Hellstrom <daniel@gaisler.com>
Date: Mon, 11 Apr 2011 17:30:21 +0200

> Dave, how do you prefer that I continue my work, should I wait for Sam
> to complete his work, or should I post the patches even though you
> can't apply them?

You and Sam will need to coordinate somehow, and only once the whole
set is ready should I get involved :-)