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.
Let's better ask TI about that.
Which begs the question whether or not arch/arm/common is the right
place for this.
No question for me. I don't want to put it back into mach-davinci/ just
because the upstream support of its other users doesn't exist.
It seems to me the only mainline kernels using this will be DaVinci.
DA830 is not DaVinci. :-P
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.
My vote is against arch/arm/mach-davinci in any case.
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() ?
Wait, I'm only using base... why allocate anything if I can just have an
array?
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.
I'm not seeing any dire need in dynamic allocation and I'm going avoid it
at all costs due to reasons of efficiency.
Otherwise, this code simply will not work. Multiple INTCs could be
managed like the multiple IRQ banks in OMAP.
Ah, it uses a different approach -- this one is worth considering...
Kevin
WBR, Sergei
_______________________________________________
Davinci-linux-open-source mailing list
[email protected]
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
