| Message ID | 1300290864-20678-1-git-send-email-daniel@gaisler.com |
|---|---|
| State | Rejected |
| Delegated to: | David Miller |
| Headers | show |
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. -- To unsubscribe from this list: send the line "unsubscribe sparclinux" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 -- To unsubscribe from this list: send the line "unsubscribe sparclinux" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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. -- To unsubscribe from this list: send the line "unsubscribe sparclinux" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 -- To unsubscribe from this list: send the line "unsubscribe sparclinux" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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. -- To unsubscribe from this list: send the line "unsubscribe sparclinux" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 -- To unsubscribe from this list: send the line "unsubscribe sparclinux" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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. -- To unsubscribe from this list: send the line "unsubscribe sparclinux" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 -- To unsubscribe from this list: send the line "unsubscribe sparclinux" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 -- To unsubscribe from this list: send the line "unsubscribe sparclinux" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 :-) -- To unsubscribe from this list: send the line "unsubscribe sparclinux" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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:
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(-)