| Message ID | 20130312045628.19701.58216.sendpatchset@w520 (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tuesday 12 March 2013, Magnus Damm wrote: > From: Magnus Damm <damm@opensource.se> > > Add IRQC interrupt controller support to r8a73a4 by > hooking up two IRQC instances to handle 58 external > IRQ signals. There IRQC controllers are tied to SPIs > of the GIC. On r8a73a4 exact IRQ pin routing is handled > by the PFC which is excluded from this patch. > > Both platform devices and DT devices are added in this > patch. The platform device versions are used to provide > a static interrupt map configuration for board code > written in C. > > Signed-off-by: Magnus Damm <damm@opensource.se> What is the status of the IRQC DT support? I remember that I wasn't happy with a prior version, but I did not follow up on some of the questions that came up, sorry about that. Did the patches end up getting merged anyway, or should we resume the discussion about those patches? I understand that lack of INTC bindings would make new SoC support particularly hard, and I don't want to be responsible for holding you up here. Arnd
Hi Arnd, Thanks for your feedback, please see below for my reply. On Tue, Mar 12, 2013 at 9:31 PM, Arnd Bergmann <arnd@arndb.de> wrote: > On Tuesday 12 March 2013, Magnus Damm wrote: >> From: Magnus Damm <damm@opensource.se> >> >> Add IRQC interrupt controller support to r8a73a4 by >> hooking up two IRQC instances to handle 58 external >> IRQ signals. There IRQC controllers are tied to SPIs >> of the GIC. On r8a73a4 exact IRQ pin routing is handled >> by the PFC which is excluded from this patch. >> >> Both platform devices and DT devices are added in this >> patch. The platform device versions are used to provide >> a static interrupt map configuration for board code >> written in C. >> >> Signed-off-by: Magnus Damm <damm@opensource.se> > > What is the status of the IRQC DT support? I remember that I wasn't > happy with a prior version, but I did not follow up on some of the > questions that came up, sorry about that. Uhm, perhaps I misunderstand, but I wonder if you refer to INTC instead of IRQC? This particular driver is for the IRQC hardware block. It is not compatible with INTC. A while ago I posted an incremental DT support patch for IRQC - "[PATCH] irqchip: irqc: Add DT support", please see https://lkml.org/lkml/2013/3/6/50 > Did the patches end up getting merged anyway, or should we resume the > discussion about those patches? I understand that lack of INTC bindings > would make new SoC support particularly hard, and I don't want to > be responsible for holding you up here. Thanks. I don't think the INTC patches went anywhere. To zoom out a bit let me list different interrupt controllers: A) INTC (drivers/sh/intc) [no DT yet] B) GIC (drivers/irqchip/irq-gic.c) [DT] C) INTC External IRQ Pin (drivers/irqchip/irq-renesas-intc-irqpin.c) [DT] D) IRQC (drivers/irqchip/irq-renesas-irqc.c) [DT] Simple use cases are: - Legacy SH SoCs or ARM SoCs with Cortex-A8 or older make use of A). - More recent ARM SoCs with Cortex-A9 or newer use B) and C) or B) and D). On top of this we now and then have GPIO controllers that have built-in interrupt controllers. Hope this helps, / magnus
On Thursday 14 March 2013, Magnus Damm wrote: > Hi Arnd, > > Thanks for your feedback, please see below for my reply. > > On Tue, Mar 12, 2013 at 9:31 PM, Arnd Bergmann <arnd@arndb.de> wrote: > > On Tuesday 12 March 2013, Magnus Damm wrote: > >> From: Magnus Damm <damm@opensource.se> > >> > >> Add IRQC interrupt controller support to r8a73a4 by > >> hooking up two IRQC instances to handle 58 external > >> IRQ signals. There IRQC controllers are tied to SPIs > >> of the GIC. On r8a73a4 exact IRQ pin routing is handled > >> by the PFC which is excluded from this patch. > >> > >> Both platform devices and DT devices are added in this > >> patch. The platform device versions are used to provide > >> a static interrupt map configuration for board code > >> written in C. > >> > >> Signed-off-by: Magnus Damm <damm@opensource.se> > > > > What is the status of the IRQC DT support? I remember that I wasn't > > happy with a prior version, but I did not follow up on some of the > > questions that came up, sorry about that. > > Uhm, perhaps I misunderstand, but I wonder if you refer to INTC instead of IRQC? Yes, you are right. > This particular driver is for the IRQC hardware block. It is not > compatible with INTC. A while ago I posted an incremental DT support > patch for IRQC - "[PATCH] irqchip: irqc: Add DT support", please see > https://lkml.org/lkml/2013/3/6/50 Ok, I had missed that, but the driver certainly looks good to me. > > Did the patches end up getting merged anyway, or should we resume the > > discussion about those patches? I understand that lack of INTC bindings > > would make new SoC support particularly hard, and I don't want to > > be responsible for holding you up here. > > Thanks. I don't think the INTC patches went anywhere. > > To zoom out a bit let me list different interrupt controllers: > > A) INTC (drivers/sh/intc) [no DT yet] > B) GIC (drivers/irqchip/irq-gic.c) [DT] > C) INTC External IRQ Pin (drivers/irqchip/irq-renesas-intc-irqpin.c) [DT] > D) IRQC (drivers/irqchip/irq-renesas-irqc.c) [DT] > > Simple use cases are: > - Legacy SH SoCs or ARM SoCs with Cortex-A8 or older make use of A). > - More recent ARM SoCs with Cortex-A9 or newer use B) and C) or B) and D). Ok. And I guess the EMMA EV2 uses only GIC but not IRQC or INTC, right? > On top of this we now and then have GPIO controllers that have > built-in interrupt controllers. Yes, obviously. Coming back to INTC, are you planning to use the same binding for A and C? Which of them the binding you posted earlier for? When I looked at the existing code, I had the impression that doing a binding for just the SH-Mobile SoCs that have an ARM core in them (including those that also have an SH core) would be much easier than doing a binding that also covers the older SH SoCs, since those are much less uniform. Arnd
On Thu, Mar 14, 2013 at 10:43 PM, Arnd Bergmann <arnd@arndb.de> wrote: > On Thursday 14 March 2013, Magnus Damm wrote: >> Hi Arnd, >> >> Thanks for your feedback, please see below for my reply. >> >> On Tue, Mar 12, 2013 at 9:31 PM, Arnd Bergmann <arnd@arndb.de> wrote: >> > On Tuesday 12 March 2013, Magnus Damm wrote: >> >> From: Magnus Damm <damm@opensource.se> >> >> >> >> Add IRQC interrupt controller support to r8a73a4 by >> >> hooking up two IRQC instances to handle 58 external >> >> IRQ signals. There IRQC controllers are tied to SPIs >> >> of the GIC. On r8a73a4 exact IRQ pin routing is handled >> >> by the PFC which is excluded from this patch. >> >> >> >> Both platform devices and DT devices are added in this >> >> patch. The platform device versions are used to provide >> >> a static interrupt map configuration for board code >> >> written in C. >> >> >> >> Signed-off-by: Magnus Damm <damm@opensource.se> >> > >> > What is the status of the IRQC DT support? I remember that I wasn't >> > happy with a prior version, but I did not follow up on some of the >> > questions that came up, sorry about that. >> >> Uhm, perhaps I misunderstand, but I wonder if you refer to INTC instead of IRQC? > > Yes, you are right. I don't blame you for the misunderstanding - as you see there are a couple of interrupt controllers just from Renesas SoC division, and you probably get to see similar situations with every SoC vendor. Quite a few different interrupt controllers to keep track of! =) >> This particular driver is for the IRQC hardware block. It is not >> compatible with INTC. A while ago I posted an incremental DT support >> patch for IRQC - "[PATCH] irqchip: irqc: Add DT support", please see >> https://lkml.org/lkml/2013/3/6/50 > > Ok, I had missed that, but the driver certainly looks good to me. Thanks. >> > Did the patches end up getting merged anyway, or should we resume the >> > discussion about those patches? I understand that lack of INTC bindings >> > would make new SoC support particularly hard, and I don't want to >> > be responsible for holding you up here. >> >> Thanks. I don't think the INTC patches went anywhere. >> >> To zoom out a bit let me list different interrupt controllers: >> >> A) INTC (drivers/sh/intc) [no DT yet] >> B) GIC (drivers/irqchip/irq-gic.c) [DT] >> C) INTC External IRQ Pin (drivers/irqchip/irq-renesas-intc-irqpin.c) [DT] >> D) IRQC (drivers/irqchip/irq-renesas-irqc.c) [DT] >> >> Simple use cases are: >> - Legacy SH SoCs or ARM SoCs with Cortex-A8 or older make use of A). >> - More recent ARM SoCs with Cortex-A9 or newer use B) and C) or B) and D). > > Ok. And I guess the EMMA EV2 uses only GIC but not IRQC or INTC, right? Yes, correct. GIC and per-GPIO interrupts provided by gpio-em.c. >> On top of this we now and then have GPIO controllers that have >> built-in interrupt controllers. > > Yes, obviously. What may not be so obvious is the GPIO interface provided by the PFC code that is now located in drivers/pinctrl/sh-pfc/. There used to be a function-only GPIOs in the PFC code but we're now moving over to PINCTRL thanks to great effort by Laurent. So besides the old function-only GPIOs provided by PFC there are also regular GPIOs exported by the PFC code. Those regular GPIOs exported by the PFC do not have any built-in interrupt controller - instead the external IRQs handled by C) and D) above are used. Other GPIO blocks like gpio-em.c and gpio-rcar.c have built-in interrupt controllers and can do per-GPIO interrupts. > Coming back to INTC, are you planning to use the same binding for A and C? I was not planning on that, no. To be honest, at this point I am not sure which way forward is best for A). > Which of them the binding you posted earlier for? Regarding A), I wrote some local DT prototype patches for INTC a year or two ago, showing how things could be done and how we can start using DT. Then I handed the job over to other people. However, from there my advice of incremental development was ignored and instead more complete bindings were developed directly without much review. So I wouldn't say that I posted the bindings myself. Right now I'm very hands-off in that area. As for C), those DT bindings were done by me. They are however not compatible with A). The hardware is different. C) is basically a special case subset of A). > When I looked at the existing code, I had the impression that doing a > binding for just the SH-Mobile SoCs that have an ARM core in them > (including those that also have an SH core) would be much easier than > doing a binding that also covers the older SH SoCs, since those are > much less uniform. I agree that in some cases it may make sense to split the SH bits from ARM, but I wonder how much we would win for the INTC. Right now I'm considering converting r8a7740 to use B) and C) (if possible), and if so then the only ARM SoC using A) for main interrupt controller left is sh7372. For sh7372 we could simply try to use C) for board-level interrupts (and board level DT) but keep the SoC portion in C with A) until the SoC is being phased out. Or we could have a simple compatible "renesas,intc-sh7372" with tables in C using irq domain to support DT, but that would be exactly as my first incremental development task that wasn't followed... What would you do? Thanks for your help, / magnus
On Friday 15 March 2013, Magnus Damm wrote: > On Thu, Mar 14, 2013 at 10:43 PM, Arnd Bergmann <arnd@arndb.de> wrote: > > On Thursday 14 March 2013, Magnus Damm wrote: > > Coming back to INTC, are you planning to use the same binding for A and C? > > I was not planning on that, no. To be honest, at this point I am not > sure which way forward is best for A). > > > Which of them the binding you posted earlier for? > > Regarding A), I wrote some local DT prototype patches for INTC a year > or two ago, showing how things could be done and how we can start > using DT. Then I handed the job over to other people. However, from > there my advice of incremental development was ignored and instead > more complete bindings were developed directly without much review. So > I wouldn't say that I posted the bindings myself. Right now I'm very > hands-off in that area. Ok. > As for C), those DT bindings were done by me. They are however not > compatible with A). The hardware is different. C) is basically a > special case subset of A). Makes sense. > > When I looked at the existing code, I had the impression that doing a > > binding for just the SH-Mobile SoCs that have an ARM core in them > > (including those that also have an SH core) would be much easier than > > doing a binding that also covers the older SH SoCs, since those are > > much less uniform. > > I agree that in some cases it may make sense to split the SH bits from > ARM, but I wonder how much we would win for the INTC. > > Right now I'm considering converting r8a7740 to use B) and C) (if > possible), and if so then the only ARM SoC using A) for main interrupt > controller left is sh7372. > > For sh7372 we could simply try to use C) for board-level interrupts > (and board level DT) but keep the SoC portion in C with A) until the > SoC is being phased out. Or we could have a simple compatible > "renesas,intc-sh7372" with tables in C using irq domain to support DT, > but that would be exactly as my first incremental development task > that wasn't followed... > > What would you do? Both of htese approaches sound fine. If there is only a single odd one out, it makes sense to have a hardwired implementation for that one. I rejected that approach originally because I wanted something more generic, and it seems you have done exactly that with the irq-renesas-intc-irqpin driver. Arnd
--- 0002/arch/arm/boot/dts/r8a73a4.dtsi +++ work/arch/arm/boot/dts/r8a73a4.dtsi 2013-03-12 00:42:47.000000000 +0900 @@ -51,4 +51,36 @@ <1 11 0xf08>, <1 10 0xf08>; }; + + irqc0: interrupt-controller@e61c0000 { + compatible = "renesas,irqc"; + #interrupt-cells = <2>; + interrupt-controller; + reg = <0xe61c0000 0x200>; + interrupt-parent = <&gic>; + interrupts = <0 0 4>, <0 1 4>, <0 2 4>, <0 3 4>, + <0 4 4>, <0 5 4>, <0 6 4>, <0 7 4>, + <0 8 4>, <0 9 4>, <0 10 4>, <0 11 4>, + <0 12 4>, <0 13 4>, <0 14 4>, <0 15 4>, + <0 16 4>, <0 17 4>, <0 18 4>, <0 19 4>, + <0 20 4>, <0 21 4>, <0 22 4>, <0 23 4>, + <0 24 4>, <0 25 4>, <0 26 4>, <0 27 4>, + <0 28 4>, <0 29 4>, <0 30 4>, <0 31 4>; + }; + + irqc1: interrupt-controller@e61c0200 { + compatible = "renesas,irqc"; + #interrupt-cells = <2>; + interrupt-controller; + reg = <0xe61c0200 0x200>; + interrupt-parent = <&gic>; + interrupts = <0 32 4>, <0 33 4>, <0 34 4>, <0 35 4>, + <0 36 4>, <0 37 4>, <0 38 4>, <0 39 4>, + <0 40 4>, <0 41 4>, <0 42 4>, <0 43 4>, + <0 44 4>, <0 45 4>, <0 46 4>, <0 47 4>, + <0 48 4>, <0 49 4>, <0 50 4>, <0 51 4>, + <0 52 4>, <0 53 4>, <0 54 4>, <0 55 4>, + <0 56 4>, <0 57 4>; + }; + }; --- 0003/arch/arm/mach-shmobile/Kconfig +++ work/arch/arm/mach-shmobile/Kconfig 2013-03-12 00:42:47.000000000 +0900 @@ -24,6 +24,7 @@ config ARCH_R8A73A4 select CPU_V7 select ARM_ARCH_TIMER select SH_CLK_CPG + select RENESAS_IRQC config ARCH_R8A7740 bool "R-Mobile A1 (R8A77400)" --- 0002/arch/arm/mach-shmobile/setup-r8a73a4.c +++ work/arch/arm/mach-shmobile/setup-r8a73a4.c 2013-03-12 00:43:25.000000000 +0900 @@ -22,6 +22,7 @@ #include <linux/of_platform.h> #include <linux/irq.h> #include <linux/serial_sci.h> +#include <linux/platform_data/irq-renesas-irqc.h> #include <mach/common.h> #include <mach/irqs.h> #include <mach/r8a73a4.h> @@ -135,6 +136,100 @@ static struct platform_device scif5_devi }, }; +static struct renesas_irqc_config irqc0_platform_data = { + .irq_base = irq_pin(0), /* IRQ0 -> IRQ31 */ +}; + +static struct resource irqc0_resources[] = { + DEFINE_RES_MEM(0xe61c0000, 0x200), /* IRQC Event Detector Block_0 */ + DEFINE_RES_IRQ(gic_spi(0)), /* IRQ0 */ + DEFINE_RES_IRQ(gic_spi(1)), /* IRQ1 */ + DEFINE_RES_IRQ(gic_spi(2)), /* IRQ2 */ + DEFINE_RES_IRQ(gic_spi(3)), /* IRQ3 */ + DEFINE_RES_IRQ(gic_spi(4)), /* IRQ4 */ + DEFINE_RES_IRQ(gic_spi(5)), /* IRQ5 */ + DEFINE_RES_IRQ(gic_spi(6)), /* IRQ6 */ + DEFINE_RES_IRQ(gic_spi(7)), /* IRQ7 */ + DEFINE_RES_IRQ(gic_spi(8)), /* IRQ8 */ + DEFINE_RES_IRQ(gic_spi(9)), /* IRQ9 */ + DEFINE_RES_IRQ(gic_spi(10)), /* IRQ10 */ + DEFINE_RES_IRQ(gic_spi(11)), /* IRQ11 */ + DEFINE_RES_IRQ(gic_spi(12)), /* IRQ12 */ + DEFINE_RES_IRQ(gic_spi(13)), /* IRQ13 */ + DEFINE_RES_IRQ(gic_spi(14)), /* IRQ14 */ + DEFINE_RES_IRQ(gic_spi(15)), /* IRQ15 */ + DEFINE_RES_IRQ(gic_spi(16)), /* IRQ16 */ + DEFINE_RES_IRQ(gic_spi(17)), /* IRQ17 */ + DEFINE_RES_IRQ(gic_spi(18)), /* IRQ18 */ + DEFINE_RES_IRQ(gic_spi(19)), /* IRQ19 */ + DEFINE_RES_IRQ(gic_spi(20)), /* IRQ20 */ + DEFINE_RES_IRQ(gic_spi(21)), /* IRQ21 */ + DEFINE_RES_IRQ(gic_spi(22)), /* IRQ22 */ + DEFINE_RES_IRQ(gic_spi(23)), /* IRQ23 */ + DEFINE_RES_IRQ(gic_spi(24)), /* IRQ24 */ + DEFINE_RES_IRQ(gic_spi(25)), /* IRQ25 */ + DEFINE_RES_IRQ(gic_spi(26)), /* IRQ26 */ + DEFINE_RES_IRQ(gic_spi(27)), /* IRQ27 */ + DEFINE_RES_IRQ(gic_spi(28)), /* IRQ28 */ + DEFINE_RES_IRQ(gic_spi(29)), /* IRQ29 */ + DEFINE_RES_IRQ(gic_spi(30)), /* IRQ30 */ + DEFINE_RES_IRQ(gic_spi(31)), /* IRQ31 */ +}; + +static struct platform_device irqc0_device = { + .name = "renesas_irqc", + .id = 0, + .resource = irqc0_resources, + .num_resources = ARRAY_SIZE(irqc0_resources), + .dev = { + .platform_data = &irqc0_platform_data, + }, +}; + +static struct renesas_irqc_config irqc1_platform_data = { + .irq_base = irq_pin(32), /* IRQ32 -> IRQ57 */ +}; + +static struct resource irqc1_resources[] = { + DEFINE_RES_MEM(0xe61c0200, 0x200), /* IRQC Event Detector Block_1 */ + DEFINE_RES_IRQ(gic_spi(32)), /* IRQ32 */ + DEFINE_RES_IRQ(gic_spi(33)), /* IRQ33 */ + DEFINE_RES_IRQ(gic_spi(34)), /* IRQ34 */ + DEFINE_RES_IRQ(gic_spi(35)), /* IRQ35 */ + DEFINE_RES_IRQ(gic_spi(36)), /* IRQ36 */ + DEFINE_RES_IRQ(gic_spi(37)), /* IRQ37 */ + DEFINE_RES_IRQ(gic_spi(38)), /* IRQ38 */ + DEFINE_RES_IRQ(gic_spi(39)), /* IRQ39 */ + DEFINE_RES_IRQ(gic_spi(40)), /* IRQ40 */ + DEFINE_RES_IRQ(gic_spi(41)), /* IRQ41 */ + DEFINE_RES_IRQ(gic_spi(42)), /* IRQ42 */ + DEFINE_RES_IRQ(gic_spi(43)), /* IRQ43 */ + DEFINE_RES_IRQ(gic_spi(44)), /* IRQ44 */ + DEFINE_RES_IRQ(gic_spi(45)), /* IRQ45 */ + DEFINE_RES_IRQ(gic_spi(46)), /* IRQ46 */ + DEFINE_RES_IRQ(gic_spi(47)), /* IRQ47 */ + DEFINE_RES_IRQ(gic_spi(48)), /* IRQ48 */ + DEFINE_RES_IRQ(gic_spi(49)), /* IRQ49 */ + DEFINE_RES_IRQ(gic_spi(50)), /* IRQ50 */ + DEFINE_RES_IRQ(gic_spi(51)), /* IRQ51 */ + DEFINE_RES_IRQ(gic_spi(52)), /* IRQ52 */ + DEFINE_RES_IRQ(gic_spi(53)), /* IRQ53 */ + DEFINE_RES_IRQ(gic_spi(54)), /* IRQ54 */ + DEFINE_RES_IRQ(gic_spi(55)), /* IRQ55 */ + DEFINE_RES_IRQ(gic_spi(56)), /* IRQ56 */ + DEFINE_RES_IRQ(gic_spi(57)), /* IRQ57 */ +}; + +static struct platform_device irqc1_device = { + .name = "renesas_irqc", + .id = 1, + .resource = irqc1_resources, + .num_resources = ARRAY_SIZE(irqc1_resources), + .dev = { + .platform_data = &irqc1_platform_data, + }, +}; + static struct platform_device *r8a73a4_devices[] __initdata = { &scif0_device, &scif1_device, @@ -142,6 +237,8 @@ static struct platform_device *r8a73a4_d &scif3_device, &scif4_device, &scif5_device, + &irqc0_device, + &irqc1_device, }; void __init r8a73a4_add_standard_devices(void)