Sergei Shtylyov <[email protected]> writes: > Hello. > > Kevin Hilman wrote: > >>>Add support for Texas Instuments Common Platform Interrupt Controller >>>(cp_intc). > >>>It's being added as a part of DA830/OMAP-L137 support, however as cp_intc is >>>not really specific to this architecture (or even ARM specific), I thought >>>that >>>the best place for this code will be in arch/arm/common/... > >> Are there other known, or upcoming users this block outside the > > TI Puma 5 ARM SoC, used in the Avalanche family of the broadband > processors (it even seems that this one can have 2 instances of > cp_intc); support for it has never hit mainline though. And even as I > suspected their Titan MIPS SoC also makes use of it; looks like it's > used in the Avalanches too.
I don't think any of those platforms are headed for mailine either. Which begs the question whether or not arch/arm/common is the right place for this. It seems to me the only mainline kernels using this will be DaVinci. >> da830/omap-L1x family parts? If not, I would lean towards keeping it >> in arch/arm/mach-davinci and abstracting it out later if/when >> necessary since if it really is generic, and not ARM specific, >> arch/arm/common is no less appropriate than arch/arm/mach-davinci. > > Come on, i8259 is generic, and yet every arch has its own driver of > this PIC. ;-) > I'm afraid that Linux jst doesn't have a proper place for the > shared PIC drivers yet... Then, my vote would still be to keep it in mach-davinci, unless Russell is ok with putting it in arch/arm/common. >>>Signed-off-by: Steve Chen <[email protected]> >>>Signed-off-by: Mark Greer <[email protected]> >>>Signed-off-by: Sergei Shtylyov <[email protected]> > >> Mostly, it looks pretty good. Some comments below... > >>>Index: linux-2.6/arch/arm/common/cp_intc.c >>>=================================================================== >>>--- /dev/null >>>+++ linux-2.6/arch/arm/common/cp_intc.c >>>@@ -0,0 +1,159 @@ >>>+/* >>>+ * TI Common Platform Interrupt Controller (cp_intc) driver >>>+ * >>>+ * Author: Steve Chen <[email protected]> >>>+ * Copyright (C) 2008-2009, MontaVista Software, Inc. <[email protected]> >>>+ * >>>+ * This file is licensed under the terms of the GNU General Public License >>>+ * version 2. This program is licensed "as is" without any warranty of any >>>+ * kind, whether express or implied. >>>+ */ >>>+ >>>+#include <linux/init.h> >>>+#include <linux/sched.h> >>>+#include <linux/interrupt.h> >>>+#include <linux/kernel.h> >>>+#include <linux/irq.h> >>>+#include <linux/io.h> >>>+#include <asm/hardware/cp_intc.h> >>>+ >>>+static void __iomem *cp_intc_base; >>>+ > >> Is there only ever going to be a single CP_INTC on any given device? >> If not, this approach will fall down. > > Sigh, I know... and it looks like TI has actually come up with a > SoC that has 2 cp_intc's (I only don't understand why -- even a single > one can support hell of a lot of interrupts). > >> Instead of this global, maybe adding a struct with the base and >> num_irqs that is allocated during cp_intc_init() ? > > With pointers stored in an array? Can do in principle, though I'm > not eager to add another indirection level without a dire need. IMHO, if there is more than one CP_INTC then there is a dire need. Otherwise, this code simply will not work. Multiple INTCs could be managed like the multiple IRQ banks in OMAP. Kevin _______________________________________________ Davinci-linux-open-source mailing list [email protected] http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
