| Message ID | 1351462748-5224-3-git-send-email-thomas.petazzoni@free-electrons.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Sun, Oct 28, 2012 at 11:19:06PM +0100, Thomas Petazzoni wrote: > Register the irq controller driver in the main > drivers/irqchip/irqchip.c file, and make sure that the initialization > function of the driver sets handle_arch_irq() appropriately. This > requires a bit of movement in the driver since the > bcm2835_handle_irq() must move before the armctrl_of_init() function > to avoid a forward declaration. > > On the arch/arm side, use irqchip_init() as the ->init_irq() callback, > and remove the definition of ->handle_irq() since this is now done by > the irq controller driver. > > Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com> > Acked-by: Stephen Warren <swarren@wwwdotorg.org> > Reviewed-by: Rob Herring <rob.herring@calxeda.com> > --- [..] > diff --git a/drivers/irqchip/irq-bcm2835.c b/drivers/irqchip/irq-bcm2835.c > index dc670cc..62d1dad 100644 > --- a/drivers/irqchip/irq-bcm2835.c > +++ b/drivers/irqchip/irq-bcm2835.c [..] > @@ -199,8 +197,8 @@ static void armctrl_handle_shortcut(int bank, struct pt_regs *regs, > handle_IRQ(irq_linear_revmap(intc.domain, irq), regs); > } > > -asmlinkage void __exception_irq_entry bcm2835_handle_irq( > - struct pt_regs *regs) > +static asmlinkage void __exception_irq_entry > +bcm2835_handle_irq(struct pt_regs *regs) > { > u32 stat, irq; > > diff --git a/drivers/irqchip/irqchip.c b/drivers/irqchip/irqchip.c > index 410f99f..e2496e4 100644 > --- a/drivers/irqchip/irqchip.c > +++ b/drivers/irqchip/irqchip.c > @@ -14,6 +14,12 @@ > #include "irqchip.h" > > static const struct of_device_id irqchip_of_match[] __initconst = { > +#ifdef CONFIG_ARCH_BCM2835 > + { > + .compatible = "brcm,bcm2835-armctrl-ic", > + .data = bcm2835_irqchip_init, > + }, > +#endif > {}, > }; > > diff --git a/drivers/irqchip/irqchip.h b/drivers/irqchip/irqchip.h > index 1e7a5c2..1075537 100644 > --- a/drivers/irqchip/irqchip.h > +++ b/drivers/irqchip/irqchip.h > @@ -11,4 +11,6 @@ > #ifndef _IRQCHIP_H > #define _IRQCHIP_H > > +int bcm2835_irqchip_init(struct device_node *node, struct device_node *parent); > + > #endif Could it make sense here to kill the irqchip.h private export, and instead rely on the linker to stitch together the builtin irqchip's of_device_id tables? Something like: drivers/irqchip/irq-bcm2835.c: static bcm2835_irqchip_init(struct device_node *node, struct device_node *parent) { /*...*/ } static const struct of_device_id bcm2835_match[] __initconst = { { .compatible = "brcm,bcm2835-armctrl-ic", .data = bcm2835_irqchip_init, }, {}, }; DECLARE_IRQCHIP(bcm2835, bcm2835_match); where include/linux/irqchip.h: #define DECLARE_IRQCHIP(name,ids) \ static of_device_id * __irqchip_##name##_matches __used \ __attribute__((__section__(".init.irqchip"))) = ids drivers/irqchip/irqchip.c: void irqchip_init(void) { extern of_device_id *__irqchip_matches_start[], *__irqchip_matches_end[]; struct of_device_id **matchesp; matchesp = __irqchip_matches_start; while (matchesp < __irqchip_matches_end) of_irq_init(*matchesp); } And a suitable entry in vmlinux.lds.h. (Hopefully?) This would eliminate the need for the 'private' irqchip.h declarations and the centrally maintained of_device_id table, which sounds like a win for maintainability. Thoughts? Thanks, Josh
On Tue, Nov 06, 2012 at 10:56:39AM -0600, Josh Cartwright wrote: > On Sun, Oct 28, 2012 at 11:19:06PM +0100, Thomas Petazzoni wrote: > > Register the irq controller driver in the main > > drivers/irqchip/irqchip.c file, and make sure that the initialization > > function of the driver sets handle_arch_irq() appropriately. This > > requires a bit of movement in the driver since the > > bcm2835_handle_irq() must move before the armctrl_of_init() function > > to avoid a forward declaration. > > > > On the arch/arm side, use irqchip_init() as the ->init_irq() callback, > > and remove the definition of ->handle_irq() since this is now done by > > the irq controller driver. > > > > Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com> > > Acked-by: Stephen Warren <swarren@wwwdotorg.org> > > Reviewed-by: Rob Herring <rob.herring@calxeda.com> > > --- > [..] > > diff --git a/drivers/irqchip/irq-bcm2835.c b/drivers/irqchip/irq-bcm2835.c > > index dc670cc..62d1dad 100644 > > --- a/drivers/irqchip/irq-bcm2835.c > > +++ b/drivers/irqchip/irq-bcm2835.c > [..] > > @@ -199,8 +197,8 @@ static void armctrl_handle_shortcut(int bank, struct pt_regs *regs, > > handle_IRQ(irq_linear_revmap(intc.domain, irq), regs); > > } > > > > -asmlinkage void __exception_irq_entry bcm2835_handle_irq( > > - struct pt_regs *regs) > > +static asmlinkage void __exception_irq_entry > > +bcm2835_handle_irq(struct pt_regs *regs) > > { > > u32 stat, irq; > > > > diff --git a/drivers/irqchip/irqchip.c b/drivers/irqchip/irqchip.c > > index 410f99f..e2496e4 100644 > > --- a/drivers/irqchip/irqchip.c > > +++ b/drivers/irqchip/irqchip.c > > @@ -14,6 +14,12 @@ > > #include "irqchip.h" > > > > static const struct of_device_id irqchip_of_match[] __initconst = { > > +#ifdef CONFIG_ARCH_BCM2835 > > + { > > + .compatible = "brcm,bcm2835-armctrl-ic", > > + .data = bcm2835_irqchip_init, > > + }, > > +#endif > > {}, > > }; > > > > diff --git a/drivers/irqchip/irqchip.h b/drivers/irqchip/irqchip.h > > index 1e7a5c2..1075537 100644 > > --- a/drivers/irqchip/irqchip.h > > +++ b/drivers/irqchip/irqchip.h > > @@ -11,4 +11,6 @@ > > #ifndef _IRQCHIP_H > > #define _IRQCHIP_H > > > > +int bcm2835_irqchip_init(struct device_node *node, struct device_node *parent); > > + > > #endif > > Could it make sense here to kill the irqchip.h private export, and > instead rely on the linker to stitch together the builtin irqchip's > of_device_id tables? > > Something like: > > drivers/irqchip/irq-bcm2835.c: > > static bcm2835_irqchip_init(struct device_node *node, struct device_node *parent) > { > /*...*/ > } > > static const struct of_device_id bcm2835_match[] __initconst = { > { .compatible = "brcm,bcm2835-armctrl-ic", .data = bcm2835_irqchip_init, }, > {}, > }; > DECLARE_IRQCHIP(bcm2835, bcm2835_match); > > where include/linux/irqchip.h: > > #define DECLARE_IRQCHIP(name,ids) \ > static of_device_id * __irqchip_##name##_matches __used \ > __attribute__((__section__(".init.irqchip"))) = ids > > drivers/irqchip/irqchip.c: > > void irqchip_init(void) > { > extern of_device_id *__irqchip_matches_start[], *__irqchip_matches_end[]; > struct of_device_id **matchesp; > > matchesp = __irqchip_matches_start; > while (matchesp < __irqchip_matches_end) > of_irq_init(*matchesp); Hmm...more thinking leads me to believe that calling of_irq_init() multiple times with an incomplete set of irqchip descriptions isn't going to work. Nevertheless, the idea could be extended such that a single of_device_id table is generated (instead of an array of pointers to incomplete an incomplete table). Josh
Josh, On Tue, 6 Nov 2012 10:56:39 -0600, Josh Cartwright wrote: > Could it make sense here to kill the irqchip.h private export, and > instead rely on the linker to stitch together the builtin irqchip's > of_device_id tables? Yes, using yet another ELF section is another option to do that. I just wasn't sure it was worth the effort for now, but if it is seen as necessary, I can certainly go ahead and implement this. I am at ELCE at the moment, so quite busy with talks, discussions... and drinks. I'll get back to this when I get back from ELCE. Thanks for your suggestion, Thomas
diff --git a/arch/arm/mach-bcm2835/bcm2835.c b/arch/arm/mach-bcm2835/bcm2835.c index f6fea49..ab1bccd 100644 --- a/arch/arm/mach-bcm2835/bcm2835.c +++ b/arch/arm/mach-bcm2835/bcm2835.c @@ -13,10 +13,10 @@ */ #include <linux/init.h> -#include <linux/irqchip/bcm2835.h> #include <linux/of_platform.h> #include <linux/bcm2835_timer.h> #include <linux/clk/bcm2835.h> +#include <linux/irqchip.h> #include <asm/mach/arch.h> #include <asm/mach/map.h> @@ -56,8 +56,7 @@ static const char * const bcm2835_compat[] = { DT_MACHINE_START(BCM2835, "BCM2835") .map_io = bcm2835_map_io, - .init_irq = bcm2835_init_irq, - .handle_irq = bcm2835_handle_irq, + .init_irq = irqchip_init, .init_machine = bcm2835_init, .timer = &bcm2835_timer, .dt_compat = bcm2835_compat diff --git a/drivers/irqchip/irq-bcm2835.c b/drivers/irqchip/irq-bcm2835.c index dc670cc..62d1dad 100644 --- a/drivers/irqchip/irq-bcm2835.c +++ b/drivers/irqchip/irq-bcm2835.c @@ -49,9 +49,11 @@ #include <linux/of_address.h> #include <linux/of_irq.h> #include <linux/irqdomain.h> -#include <linux/irqchip/bcm2835.h> #include <asm/exception.h> +#include <asm/mach/irq.h> + +#include "irqchip.h" /* Put the bank and irq (32 bits) into the hwirq */ #define MAKE_HWIRQ(b, n) ((b << 5) | (n)) @@ -135,8 +137,10 @@ static struct irq_domain_ops armctrl_ops = { .xlate = armctrl_xlate }; -static int __init armctrl_of_init(struct device_node *node, - struct device_node *parent) +static asmlinkage void bcm2835_handle_irq(struct pt_regs *regs); + +int __init bcm2835_irqchip_init(struct device_node *node, + struct device_node *parent) { void __iomem *base; int irq, b, i; @@ -164,16 +168,10 @@ static int __init armctrl_of_init(struct device_node *node, set_irq_flags(irq, IRQF_VALID | IRQF_PROBE); } } - return 0; -} -static struct of_device_id irq_of_match[] __initconst = { - { .compatible = "brcm,bcm2835-armctrl-ic", .data = armctrl_of_init } -}; + handle_arch_irq = bcm2835_handle_irq; -void __init bcm2835_init_irq(void) -{ - of_irq_init(irq_of_match); + return 0; } /* @@ -199,8 +197,8 @@ static void armctrl_handle_shortcut(int bank, struct pt_regs *regs, handle_IRQ(irq_linear_revmap(intc.domain, irq), regs); } -asmlinkage void __exception_irq_entry bcm2835_handle_irq( - struct pt_regs *regs) +static asmlinkage void __exception_irq_entry +bcm2835_handle_irq(struct pt_regs *regs) { u32 stat, irq; diff --git a/drivers/irqchip/irqchip.c b/drivers/irqchip/irqchip.c index 410f99f..e2496e4 100644 --- a/drivers/irqchip/irqchip.c +++ b/drivers/irqchip/irqchip.c @@ -14,6 +14,12 @@ #include "irqchip.h" static const struct of_device_id irqchip_of_match[] __initconst = { +#ifdef CONFIG_ARCH_BCM2835 + { + .compatible = "brcm,bcm2835-armctrl-ic", + .data = bcm2835_irqchip_init, + }, +#endif {}, }; diff --git a/drivers/irqchip/irqchip.h b/drivers/irqchip/irqchip.h index 1e7a5c2..1075537 100644 --- a/drivers/irqchip/irqchip.h +++ b/drivers/irqchip/irqchip.h @@ -11,4 +11,6 @@ #ifndef _IRQCHIP_H #define _IRQCHIP_H +int bcm2835_irqchip_init(struct device_node *node, struct device_node *parent); + #endif diff --git a/include/linux/irqchip/bcm2835.h b/include/linux/irqchip/bcm2835.h deleted file mode 100644 index 48a859b..0000000 --- a/include/linux/irqchip/bcm2835.h +++ /dev/null @@ -1,29 +0,0 @@ -/* - * Copyright (C) 2010 Broadcom - * - * This program is free software; you can redistribute it and/or modify - * it under the terms of the GNU General Public License as published by - * the Free Software Foundation; either version 2 of the License, or - * (at your option) any later version. - * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - * GNU General Public License for more details. - * - * You should have received a copy of the GNU General Public License - * along with this program; if not, write to the Free Software - * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA - */ - -#ifndef __LINUX_IRQCHIP_BCM2835_H_ -#define __LINUX_IRQCHIP_BCM2835_H_ - -#include <asm/exception.h> - -extern void bcm2835_init_irq(void); - -extern asmlinkage void __exception_irq_entry bcm2835_handle_irq( - struct pt_regs *regs); - -#endif