| Message ID | 1351695517-5636-4-git-send-email-robherring2@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, Oct 31, 2012 at 09:58:35AM -0500, Rob Herring wrote: > From: Rob Herring <rob.herring@calxeda.com> > > Now that we have drivers/irqchip, move GIC irqchip to drivers/irqchip. This > is necessary to share the GIC with arm and arm64. > > Signed-off-by: Rob Herring <rob.herring@calxeda.com> > Cc: Russell King <linux@arm.linux.org.uk> > Cc: Thomas Gleixner <tglx@linutronix.de> > --- > arch/arm/common/Kconfig | 8 -------- > arch/arm/common/Makefile | 1 - > drivers/irqchip/Kconfig | 8 ++++++++ > drivers/irqchip/Makefile | 1 + > arch/arm/common/gic.c => drivers/irqchip/irq-gic.c | 0 > drivers/irqchip/irqchip.c | 10 ++++++++++ > drivers/irqchip/irqchip.h | 1 + > 7 files changed, 20 insertions(+), 9 deletions(-) > rename arch/arm/common/gic.c => drivers/irqchip/irq-gic.c (100%) What about its dependent arch/arm/include/asm/hardware/gic.h header, which I believe after patch 1 becomes just a bunch of function calls, and so no longer has any right to be in asm/hardware. Nothing should be moved out of arch/arm without its associated header file also moving with it.
On 10/31/2012 10:09 AM, Russell King - ARM Linux wrote: > On Wed, Oct 31, 2012 at 09:58:35AM -0500, Rob Herring wrote: >> From: Rob Herring <rob.herring@calxeda.com> >> >> Now that we have drivers/irqchip, move GIC irqchip to drivers/irqchip. This >> is necessary to share the GIC with arm and arm64. >> >> Signed-off-by: Rob Herring <rob.herring@calxeda.com> >> Cc: Russell King <linux@arm.linux.org.uk> >> Cc: Thomas Gleixner <tglx@linutronix.de> >> --- >> arch/arm/common/Kconfig | 8 -------- >> arch/arm/common/Makefile | 1 - >> drivers/irqchip/Kconfig | 8 ++++++++ >> drivers/irqchip/Makefile | 1 + >> arch/arm/common/gic.c => drivers/irqchip/irq-gic.c | 0 >> drivers/irqchip/irqchip.c | 10 ++++++++++ >> drivers/irqchip/irqchip.h | 1 + >> 7 files changed, 20 insertions(+), 9 deletions(-) >> rename arch/arm/common/gic.c => drivers/irqchip/irq-gic.c (100%) > > What about its dependent arch/arm/include/asm/hardware/gic.h header, > which I believe after patch 1 becomes just a bunch of function calls, > and so no longer has any right to be in asm/hardware. > > Nothing should be moved out of arch/arm without its associated header > file also moving with it. What is left is only used within arch/arm and I expect we will get rid of the remaining users. So I didn't want to encourage any additional users by moving to include/linux. gic_secondary_init and gic_cascade_irq could be function ptrs. gic_of_init can be removed once users are converted to call irqchip_init instead. That leaves gic_init which are all the non-DT converted GIC users and will take some time to convert. I am puzzled by tegra and zynq which should be DT only already. $ git grep -l 'gic_init(' -- arch/arm arch/arm/include/asm/hardware/gic.h arch/arm/mach-cns3xxx/core.c arch/arm/mach-omap2/board-omap3logic.c arch/arm/mach-omap2/omap4-common.c arch/arm/mach-realview/realview_eb.c arch/arm/mach-realview/realview_pb1176.c arch/arm/mach-realview/realview_pb11mp.c arch/arm/mach-realview/realview_pba8.c arch/arm/mach-realview/realview_pbx.c arch/arm/mach-shmobile/intc-r8a7779.c arch/arm/mach-shmobile/intc-sh73a0.c arch/arm/mach-shmobile/setup-emev2.c arch/arm/mach-tegra/irq.c arch/arm/mach-ux500/cpu.c arch/arm/mach-vexpress/ct-ca9x4.c arch/arm/mach-zynq/common.c Rob
On Wed, Oct 31, 2012 at 10:41:44AM -0500, Rob Herring wrote: > On 10/31/2012 10:09 AM, Russell King - ARM Linux wrote: > > On Wed, Oct 31, 2012 at 09:58:35AM -0500, Rob Herring wrote: > >> From: Rob Herring <rob.herring@calxeda.com> > >> > >> Now that we have drivers/irqchip, move GIC irqchip to drivers/irqchip. This > >> is necessary to share the GIC with arm and arm64. > >> > >> Signed-off-by: Rob Herring <rob.herring@calxeda.com> > >> Cc: Russell King <linux@arm.linux.org.uk> > >> Cc: Thomas Gleixner <tglx@linutronix.de> > >> --- > >> arch/arm/common/Kconfig | 8 -------- > >> arch/arm/common/Makefile | 1 - > >> drivers/irqchip/Kconfig | 8 ++++++++ > >> drivers/irqchip/Makefile | 1 + > >> arch/arm/common/gic.c => drivers/irqchip/irq-gic.c | 0 > >> drivers/irqchip/irqchip.c | 10 ++++++++++ > >> drivers/irqchip/irqchip.h | 1 + > >> 7 files changed, 20 insertions(+), 9 deletions(-) > >> rename arch/arm/common/gic.c => drivers/irqchip/irq-gic.c (100%) > > > > What about its dependent arch/arm/include/asm/hardware/gic.h header, > > which I believe after patch 1 becomes just a bunch of function calls, > > and so no longer has any right to be in asm/hardware. > > > > Nothing should be moved out of arch/arm without its associated header > > file also moving with it. > > What is left is only used within arch/arm and I expect we will get rid > of the remaining users. So I didn't want to encourage any additional > users by moving to include/linux. > > gic_secondary_init and gic_cascade_irq could be function ptrs. gic_secondary_init() can be, but I don't see how why you think it would be a good idea to turn gic_casade_irq() into a function pointer. That makes zero sense to me what so ever. As I've already pointed out, gic_cascade_irq() is only used by a minority of platforms where there are two cascaded GICs. That being Realview, where you have a SMP tile with its own GIC on top of the motherboard with a motherboard GIC. Moreover, what gic_cascade_irq() is doing is can't really be turned into something generic; it's setting up genirq for the chained GIC, which requires GIC specific data. You can't get around the fact that this function is attaching a secondary GIC to an upstream IRQ. That's already as generic as it can be. And finally the exercise of turning that into a function pointer is pure code obfuscation with zero benefit what so ever. The fact is these platforms _definitely_ have a GIC present, and as the driver needs to be built into the kernel for these platforms to function there is no point what so ever in having a direct function call into the GIC code. To think otherwise... your off your rocker IMHO. > gic_of_init can be removed once users are converted to call irqchip_init > instead. That leaves gic_init which are all the non-DT converted GIC > users and will take some time to convert. I am puzzled by tegra and zynq > which should be DT only already. Still, my point stands. After this change this header file has no business being in asm/hardware. At all. It needs to move as a result of this change.
On 10/31/2012 09:41 AM, Rob Herring wrote: > On 10/31/2012 10:09 AM, Russell King - ARM Linux wrote: >> On Wed, Oct 31, 2012 at 09:58:35AM -0500, Rob Herring wrote: >>> From: Rob Herring <rob.herring@calxeda.com> >>> >>> Now that we have drivers/irqchip, move GIC irqchip to drivers/irqchip. This >>> is necessary to share the GIC with arm and arm64. >>> >>> Signed-off-by: Rob Herring <rob.herring@calxeda.com> >>> Cc: Russell King <linux@arm.linux.org.uk> >>> Cc: Thomas Gleixner <tglx@linutronix.de> >>> --- >>> arch/arm/common/Kconfig | 8 -------- >>> arch/arm/common/Makefile | 1 - >>> drivers/irqchip/Kconfig | 8 ++++++++ >>> drivers/irqchip/Makefile | 1 + >>> arch/arm/common/gic.c => drivers/irqchip/irq-gic.c | 0 >>> drivers/irqchip/irqchip.c | 10 ++++++++++ >>> drivers/irqchip/irqchip.h | 1 + >>> 7 files changed, 20 insertions(+), 9 deletions(-) >>> rename arch/arm/common/gic.c => drivers/irqchip/irq-gic.c (100%) >> >> What about its dependent arch/arm/include/asm/hardware/gic.h header, >> which I believe after patch 1 becomes just a bunch of function calls, >> and so no longer has any right to be in asm/hardware. >> >> Nothing should be moved out of arch/arm without its associated header >> file also moving with it. > > What is left is only used within arch/arm and I expect we will get rid > of the remaining users. So I didn't want to encourage any additional > users by moving to include/linux. > > gic_secondary_init and gic_cascade_irq could be function ptrs. > gic_of_init can be removed once users are converted to call irqchip_init > instead. That leaves gic_init which are all the non-DT converted GIC > users and will take some time to convert. I am puzzled by tegra and zynq > which should be DT only already. I imagine you're talking about: /* * Check if there is a devicetree present, since the GIC will be * initialized elsewhere under DT. */ if (!of_have_populated_dt()) gic_init(0, 29, distbase, IO_ADDRESS(TEGRA_ARM_PERIF_BASE + 0x100)); This is probably legacy code from when we weren't DT only, and I imagine can simply be removed. There are probably other pieces of code we can go through and remove now.
diff --git a/arch/arm/common/Kconfig b/arch/arm/common/Kconfig index 45ceeb0..7bf52b2 100644 --- a/arch/arm/common/Kconfig +++ b/arch/arm/common/Kconfig @@ -1,11 +1,3 @@ -config ARM_GIC - bool - select IRQ_DOMAIN - select MULTI_IRQ_HANDLER - -config GIC_NON_BANKED - bool - config ARM_VIC bool select IRQ_DOMAIN diff --git a/arch/arm/common/Makefile b/arch/arm/common/Makefile index e8a4e58..4104b82 100644 --- a/arch/arm/common/Makefile +++ b/arch/arm/common/Makefile @@ -2,7 +2,6 @@ # Makefile for the linux kernel. # -obj-$(CONFIG_ARM_GIC) += gic.o obj-$(CONFIG_ARM_VIC) += vic.o obj-$(CONFIG_ICST) += icst.o obj-$(CONFIG_SA1111) += sa1111.o diff --git a/drivers/irqchip/Kconfig b/drivers/irqchip/Kconfig index 88b0929..2d7f350 100644 --- a/drivers/irqchip/Kconfig +++ b/drivers/irqchip/Kconfig @@ -1,3 +1,11 @@ config IRQCHIP def_bool y depends on OF_IRQ + +config ARM_GIC + bool + select IRQ_DOMAIN + select MULTI_IRQ_HANDLER + +config GIC_NON_BANKED + bool diff --git a/drivers/irqchip/Makefile b/drivers/irqchip/Makefile index 5148ffd..94118db 100644 --- a/drivers/irqchip/Makefile +++ b/drivers/irqchip/Makefile @@ -1,3 +1,4 @@ obj-$(CONFIG_IRQCHIP) += irqchip.o obj-$(CONFIG_ARCH_BCM2835) += irq-bcm2835.o obj-$(CONFIG_ARCH_MVEBU) += irq-armada-370-xp.o +obj-$(CONFIG_ARM_GIC) += irq-gic.o diff --git a/arch/arm/common/gic.c b/drivers/irqchip/irq-gic.c similarity index 100% rename from arch/arm/common/gic.c rename to drivers/irqchip/irq-gic.c diff --git a/drivers/irqchip/irqchip.c b/drivers/irqchip/irqchip.c index f36d423..3f37397 100644 --- a/drivers/irqchip/irqchip.c +++ b/drivers/irqchip/irqchip.c @@ -14,6 +14,16 @@ #include "irqchip.h" static const struct of_device_id irqchip_of_match[] __initconst = { +#ifdef CONFIG_ARM_GIC + { + .compatible = "arm,cortex-a15-gic", + .data = gic_of_init, + }, + { + .compatible = "arm,cortex-a9-gic", + .data = gic_of_init, + }, +#endif #ifdef CONFIG_ARCH_BCM2835 { .compatible = "brcm,bcm2835-armctrl-ic", diff --git a/drivers/irqchip/irqchip.h b/drivers/irqchip/irqchip.h index 0a0d7af..62773ab3 100644 --- a/drivers/irqchip/irqchip.h +++ b/drivers/irqchip/irqchip.h @@ -14,5 +14,6 @@ int bcm2835_irqchip_init(struct device_node *node, struct device_node *parent); int armada_370_xp_mpic_of_init(struct device_node *node, struct device_node *parent); +int gic_of_init(struct device_node *node, struct device_node *parent); #endif