| Message ID | 1417565531-4507-4-git-send-email-stefan@agner.ch (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wednesday 03 December 2014 01:12:02 Stefan Agner wrote: > The inline function register_routable_domain_ops is only usable if > CONFIG_ARM_GIC is set. Make it depend on this configuration. This > also allows other SoC interrupt controller to provide such a > function. > > Signed-off-by: Stefan Agner <stefan@agner.ch> I don't think this is a good idea: either the interface is meant to be generic and should work with any interrupt controller, or it is specific to one irqchip and another irqchip should provide a different interface that has a nonconflicting name. I suppose you want the latter here, given that the declaration is part of the gic specific header file. Arnd
On Wed, 3 Dec 2014, Arnd Bergmann wrote: > On Wednesday 03 December 2014 01:12:02 Stefan Agner wrote: > > The inline function register_routable_domain_ops is only usable if > > CONFIG_ARM_GIC is set. Make it depend on this configuration. This > > also allows other SoC interrupt controller to provide such a > > function. > > > > Signed-off-by: Stefan Agner <stefan@agner.ch> > > I don't think this is a good idea: either the interface is meant > to be generic and should work with any interrupt controller, or > it is specific to one irqchip and another irqchip should provide > a different interface that has a nonconflicting name. > > I suppose you want the latter here, given that the declaration > is part of the gic specific header file. That routable_domain stuff should have never been invented. In hindsight we should have done the stacked irq domains back then already, but in hindsight ... If you look at the crossbar scheme what we have now: GIC domain |--------------| | |---------- private | non routable |---------- peripheral | |---------- peripheral | | |--------------| | |---------- peripheral | routable |---------- peripheral | | |---------- peripheral |--------------| | |----------------| |-------------->| crossbar magic | |----------------| which is not how the hardware looks. The hardware actually looks like this: GIC domain |--------------| | |---------- private | non routable |---------- peripheral | |---------- peripheral | | |--------------| |--------------| | | | |---------- peripheral | routable |-----------| crossbar |---------- peripheral | | | domain |---------- peripheral |--------------| |--------------| So it is completely obvious that the peripheral interrupts which need to be routed through the crossbar belong to a different domain. We really should convert crossbar to that scheme and get rid of the routable domain stuff completely. With vybrid thats not different according to the diagram in the reference manual. GIC domain |--------------| | |---------- private | non routable |---------- peripheral | |---------- peripheral | | |--------------| |--------------| | | | | | routable |-----------| IRQ router | | | | domain | | | | | |--------------| |--------------| | Shared state |---------- CPU to CPU NVIC domain | and hardware |---------- peripheral |--------------| | |---------- peripheral | | |--------------| | | | | | routable |-----------| IRQ router | | | | domain | | | | | |--------------| |--------------| | | | |---------- private | non routable |---------- peripheral | |---------- peripheral |--------------| The routable domain stuff is just an adhoc redirector which must be tied to a particular base irq chip implementation (i.e. GIC). With stacked irq domains there is no dependency on the underlying irq chip. No ifdeffery, nothing. So you can use the same router domain implementation for both the A5 and the M4 side. On the A5 side the router domain has the gic domain as parent and on the M4 side the nvic domain. The shared interrupts are allocated through the router domain which decides whether the interrupt can be assigned to a particular core or not. If it can be assigned it allocates the corresponding interrupt in the parent domain, sets up the routing and everything just works. See: http://git.kernel.org/cgit/linux/kernel/git/tip/tip.git/log/?h=irq/irqdomain-arm Thanks, tglx
On 2014-12-03 14:04, Thomas Gleixner wrote: > On Wed, 3 Dec 2014, Arnd Bergmann wrote: > >> On Wednesday 03 December 2014 01:12:02 Stefan Agner wrote: >> > The inline function register_routable_domain_ops is only usable if >> > CONFIG_ARM_GIC is set. Make it depend on this configuration. This >> > also allows other SoC interrupt controller to provide such a >> > function. >> > >> > Signed-off-by: Stefan Agner <stefan@agner.ch> >> >> I don't think this is a good idea: either the interface is meant >> to be generic and should work with any interrupt controller, or >> it is specific to one irqchip and another irqchip should provide >> a different interface that has a nonconflicting name. >> >> I suppose you want the latter here, given that the declaration >> is part of the gic specific header file. > > That routable_domain stuff should have never been invented. In > hindsight we should have done the stacked irq domains back then > already, but in hindsight ... I must admit that my first implementation even used arch_extn (through the mask/unmask stuff). Then I saw the routable domain stuff, which looked more like a fit. But when I realized that only GIC has support for that, I soon realized that this might either need a proper framework... my hackish NVIC extension probably won't be acceptable... Stack-able IRQ domain sounds like exactly what I was looking for... > > If you look at the crossbar scheme what we have now: > > GIC domain > |--------------| > | |---------- private > | non routable |---------- peripheral > | |---------- peripheral > | | > |--------------| > | |---------- peripheral > | routable |---------- peripheral > | | |---------- peripheral > |--------------| > | |----------------| > |-------------->| crossbar magic | > |----------------| > > which is not how the hardware looks. The hardware actually looks like > this: > > GIC domain > |--------------| > | |---------- private > | non routable |---------- peripheral > | |---------- peripheral > | | > |--------------| |--------------| > | | | |---------- peripheral > | routable |-----------| crossbar |---------- peripheral > | | | domain |---------- peripheral > |--------------| |--------------| > > So it is completely obvious that the peripheral interrupts which need > to be routed through the crossbar belong to a different domain. We > really should convert crossbar to that scheme and get rid of the > routable domain stuff completely. > > With vybrid thats not different according to the diagram in the > reference manual. > > GIC domain > |--------------| > | |---------- private > | non routable |---------- peripheral > | |---------- peripheral > | | > |--------------| |--------------| > | | | | > | routable |-----------| IRQ router | > | | | domain | > | | | | > |--------------| |--------------| > | Shared state |---------- CPU to CPU > NVIC domain | and hardware |---------- peripheral > |--------------| | |---------- peripheral > | | |--------------| > | | | | > | routable |-----------| IRQ router | > | | | domain | > | | | | > |--------------| |--------------| > | | > | |---------- private > | non routable |---------- peripheral > | |---------- peripheral > |--------------| > > The routable domain stuff is just an adhoc redirector which must be > tied to a particular base irq chip implementation (i.e. GIC). With > stacked irq domains there is no dependency on the underlying irq > chip. No ifdeffery, nothing. So you can use the same router domain > implementation for both the A5 and the M4 side. > > On the A5 side the router domain has the gic domain as parent and on > the M4 side the nvic domain. > > The shared interrupts are allocated through the router domain which > decides whether the interrupt can be assigned to a particular core or > not. If it can be assigned it allocates the corresponding interrupt in > the parent domain, sets up the routing and everything just works. What do you mean by the shared state in the drawing above? Currently, I check whether a interrupt is already used by the other core by reading the register (do this configuration register reflect the "shared state" in your drawing?). > > See: > http://git.kernel.org/cgit/linux/kernel/git/tip/tip.git/log/?h=irq/irqdomain-arm > Thanks for the link. So my work would involve to support domain hierarchy for NVIC (proper irq_domain_ops, introduce arm,routable-irqs property, anything else?) and then make use of the hierarchy code in my MSCM driver like for instance the mtk-sysirq driver...? What is the state of the IRQ domain hierarchy, when will it go upstream? -- Stefan
Hi Stefan, On 03/12/14 17:28, Stefan Agner wrote: > On 2014-12-03 14:04, Thomas Gleixner wrote: >> On Wed, 3 Dec 2014, Arnd Bergmann wrote: >> >>> On Wednesday 03 December 2014 01:12:02 Stefan Agner wrote: >>>> The inline function register_routable_domain_ops is only usable if >>>> CONFIG_ARM_GIC is set. Make it depend on this configuration. This >>>> also allows other SoC interrupt controller to provide such a >>>> function. >>>> >>>> Signed-off-by: Stefan Agner <stefan@agner.ch> >>> >>> I don't think this is a good idea: either the interface is meant >>> to be generic and should work with any interrupt controller, or >>> it is specific to one irqchip and another irqchip should provide >>> a different interface that has a nonconflicting name. >>> >>> I suppose you want the latter here, given that the declaration >>> is part of the gic specific header file. >> >> That routable_domain stuff should have never been invented. In >> hindsight we should have done the stacked irq domains back then >> already, but in hindsight ... > > I must admit that my first implementation even used arch_extn (through I'm glad I didn't see that! ;-) > the mask/unmask stuff). Then I saw the routable domain stuff, which > looked more like a fit. But when I realized that only GIC has support > for that, I soon realized that this might either need a proper > framework... my hackish NVIC extension probably won't be acceptable... > > Stack-able IRQ domain sounds like exactly what I was looking for... Like Thomas said, we should have had that ages ago. It solves so many problems in one go it's mind-boggling. On the down side, we end-up refactoring *a lot* of the existing code... >> >> If you look at the crossbar scheme what we have now: >> >> GIC domain >> |--------------| >> | |---------- private >> | non routable |---------- peripheral >> | |---------- peripheral >> | | >> |--------------| >> | |---------- peripheral >> | routable |---------- peripheral >> | | |---------- peripheral >> |--------------| >> | |----------------| >> |-------------->| crossbar magic | >> |----------------| >> >> which is not how the hardware looks. The hardware actually looks like >> this: >> >> GIC domain >> |--------------| >> | |---------- private >> | non routable |---------- peripheral >> | |---------- peripheral >> | | >> |--------------| |--------------| >> | | | |---------- peripheral >> | routable |-----------| crossbar |---------- peripheral >> | | | domain |---------- peripheral >> |--------------| |--------------| >> >> So it is completely obvious that the peripheral interrupts which need >> to be routed through the crossbar belong to a different domain. We >> really should convert crossbar to that scheme and get rid of the >> routable domain stuff completely. >> >> With vybrid thats not different according to the diagram in the >> reference manual. >> >> GIC domain >> |--------------| >> | |---------- private >> | non routable |---------- peripheral >> | |---------- peripheral >> | | >> |--------------| |--------------| >> | | | | >> | routable |-----------| IRQ router | >> | | | domain | >> | | | | >> |--------------| |--------------| >> | Shared state |---------- CPU to CPU >> NVIC domain | and hardware |---------- peripheral >> |--------------| | |---------- peripheral >> | | |--------------| >> | | | | >> | routable |-----------| IRQ router | >> | | | domain | >> | | | | >> |--------------| |--------------| >> | | >> | |---------- private >> | non routable |---------- peripheral >> | |---------- peripheral >> |--------------| >> >> The routable domain stuff is just an adhoc redirector which must be >> tied to a particular base irq chip implementation (i.e. GIC). With >> stacked irq domains there is no dependency on the underlying irq >> chip. No ifdeffery, nothing. So you can use the same router domain >> implementation for both the A5 and the M4 side. >> >> On the A5 side the router domain has the gic domain as parent and on >> the M4 side the nvic domain. >> >> The shared interrupts are allocated through the router domain which >> decides whether the interrupt can be assigned to a particular core or >> not. If it can be assigned it allocates the corresponding interrupt in >> the parent domain, sets up the routing and everything just works. > > What do you mean by the shared state in the drawing above? Currently, I > check whether a interrupt is already used by the other core by reading > the register (do this configuration register reflect the "shared state" > in your drawing?). I think that is basically it. It should only be the register that decides on the actual routing. BTW, how do you arbitrate between concurrent accesses to this register? Or is only the A5 allowed to change it? > >> >> See: >> http://git.kernel.org/cgit/linux/kernel/git/tip/tip.git/log/?h=irq/irqdomain-arm >> > > Thanks for the link. So my work would involve to support domain > hierarchy for NVIC (proper irq_domain_ops, introduce arm,routable-irqs > property, anything else?) and then make use of the hierarchy code in my > MSCM driver like for instance the mtk-sysirq driver...? I don't think we need the arm,routable-irq property at all, and I'm looking at refactoring the crossbar thingy to remove most of its entanglement with the GIC. All you need to know is the range of interrupts you're allowed to request through the "routable" domain. The inner-most irqchip shouldn't even know about it (after all, they are just wires coming in). It should be the duty of the outer-most irqchip (the "router") to generate the correct request to the GIC/NVIC. So all the knowledge should be at the router level. The mtk-sysrq code is indeed a good example of what you can do. > What is the state of the IRQ domain hierarchy, when will it go upstream? Scheduled for 3.19, if everything goes according to plan. Don't think we can go back anyway... ;-) Thanks, M.
On Wed, 3 Dec 2014, Marc Zyngier wrote: > On 03/12/14 17:28, Stefan Agner wrote: > > On 2014-12-03 14:04, Thomas Gleixner wrote: > >> The shared interrupts are allocated through the router domain which > >> decides whether the interrupt can be assigned to a particular core or > >> not. If it can be assigned it allocates the corresponding interrupt in > >> the parent domain, sets up the routing and everything just works. > > > > What do you mean by the shared state in the drawing above? Currently, I > > check whether a interrupt is already used by the other core by reading > > the register (do this configuration register reflect the "shared state" > > in your drawing?). > > I think that is basically it. It should only be the register that > decides on the actual routing. BTW, how do you arbitrate between > concurrent accesses to this register? Or is only the A5 allowed to > change it? What I meant with 'shared state' is basically the configuration register space. Plus depending on the mechanism you want to use for correlating the routing between the A5 and the M4 some shared memory state, locking, IPC or whatever you need for this. > >> http://git.kernel.org/cgit/linux/kernel/git/tip/tip.git/log/?h=irq/irqdomain-arm > >> > > > > Thanks for the link. So my work would involve to support domain > > hierarchy for NVIC (proper irq_domain_ops, introduce arm,routable-irqs > > property, anything else?) and then make use of the hierarchy code in my > > MSCM driver like for instance the mtk-sysirq driver...? > > I don't think we need the arm,routable-irq property at all, and I'm > looking at refactoring the crossbar thingy to remove most of its > entanglement with the GIC. > > All you need to know is the range of interrupts you're allowed to > request through the "routable" domain. The inner-most irqchip shouldn't > even know about it (after all, they are just wires coming in). It should > be the duty of the outer-most irqchip (the "router") to generate the > correct request to the GIC/NVIC. So all the knowledge should be at the > router level. > > The mtk-sysrq code is indeed a good example of what you can do. The gic-v2m MSI stuff is probably helpful as well. > > What is the state of the IRQ domain hierarchy, when will it go upstream? > > Scheduled for 3.19, if everything goes according to plan. Don't think we > can go back anyway... ;-) Indeed. That would be a major headache... Thanks, tglx
On 2014-12-03 20:04, Marc Zyngier wrote: <snip> >> What do you mean by the shared state in the drawing above? Currently, I >> check whether a interrupt is already used by the other core by reading >> the register (do this configuration register reflect the "shared state" >> in your drawing?). > > I think that is basically it. It should only be the register that > decides on the actual routing. BTW, how do you arbitrate between > concurrent accesses to this register? Or is only the A5 allowed to > change it? No arbitration so far... The whole Vybrid on M4 stuff is quite a hack right now. For instance also the concurrent access to the clock registers is not handled. Currently, I start the M4 from a booted A5 Linux. To avoid half of the clocks get turned of by the M4 clock driver, I need to specify clk_ignore_unused. Beside that, peripherals have to be enabled/disabled in a non conflicting manor in the device trees... For the interrupt router in MSCM, it would be nice if the access could be done an atomic way, which would avoid the use of a lock mechanism. But I guess this is not possible, since peripherals only support standard ldr/str...? There is the SEMA4 module which provides hardware semaphores. I'm aware of the hardware spinlock drivers (drivers/hwspinlock/), I started to implement such a driver for Vybrid. But so far a grep through the kernel does not show one usage of that framework... I guess we could add dt support for that, so we can assign the locks to individual drivers. I also plan to have a deeper look into remoteproc/rpmsg, not sure if locking of shared peripherals is part (or planned to be part) of that framework. For the clock stuff, the problem is more complex: I guess the would need some kind of master/slave definition, where we disallow the change of the shared clocks for the slave. If you are aware of patches/solutions, I'm happy to hear it... -- Stefan <snip>
On 04/12/14 13:35, Stefan Agner wrote: > On 2014-12-03 20:04, Marc Zyngier wrote: > <snip> >>> What do you mean by the shared state in the drawing above? Currently, I >>> check whether a interrupt is already used by the other core by reading >>> the register (do this configuration register reflect the "shared state" >>> in your drawing?). >> >> I think that is basically it. It should only be the register that >> decides on the actual routing. BTW, how do you arbitrate between >> concurrent accesses to this register? Or is only the A5 allowed to >> change it? > > No arbitration so far... The whole Vybrid on M4 stuff is quite a hack > right now. For instance also the concurrent access to the clock > registers is not handled. Currently, I start the M4 from a booted A5 > Linux. To avoid half of the clocks get turned of by the M4 clock driver, > I need to specify clk_ignore_unused. Beside that, peripherals have to be > enabled/disabled in a non conflicting manor in the device trees... > > For the interrupt router in MSCM, it would be nice if the access could > be done an atomic way, which would avoid the use of a lock mechanism. > But I guess this is not possible, since peripherals only support > standard ldr/str...? > > There is the SEMA4 module which provides hardware semaphores. I'm aware > of the hardware spinlock drivers (drivers/hwspinlock/), I started to > implement such a driver for Vybrid. But so far a grep through the kernel > does not show one usage of that framework... I guess we could add dt > support for that, so we can assign the locks to individual drivers. > > I also plan to have a deeper look into remoteproc/rpmsg, not sure if > locking of shared peripherals is part (or planned to be part) of that > framework. > > For the clock stuff, the problem is more complex: I guess the would need > some kind of master/slave definition, where we disallow the change of > the shared clocks for the slave. > > If you are aware of patches/solutions, I'm happy to hear it... I don't have a real solution for this, but I'd be tempted to generate the M4 DT based on the HW the A5 is not using, and only describe that. Clearly not ideal, but it gives you the control you need (don't describe the HW you don't want to see touched)... M.
On 2014-12-04 14:42, Marc Zyngier wrote: > On 04/12/14 13:35, Stefan Agner wrote: >> On 2014-12-03 20:04, Marc Zyngier wrote: >> <snip> >>>> What do you mean by the shared state in the drawing above? Currently, I >>>> check whether a interrupt is already used by the other core by reading >>>> the register (do this configuration register reflect the "shared state" >>>> in your drawing?). >>> >>> I think that is basically it. It should only be the register that >>> decides on the actual routing. BTW, how do you arbitrate between >>> concurrent accesses to this register? Or is only the A5 allowed to >>> change it? >> >> No arbitration so far... The whole Vybrid on M4 stuff is quite a hack >> right now. For instance also the concurrent access to the clock >> registers is not handled. Currently, I start the M4 from a booted A5 >> Linux. To avoid half of the clocks get turned of by the M4 clock driver, >> I need to specify clk_ignore_unused. Beside that, peripherals have to be >> enabled/disabled in a non conflicting manor in the device trees... >> >> For the interrupt router in MSCM, it would be nice if the access could >> be done an atomic way, which would avoid the use of a lock mechanism. >> But I guess this is not possible, since peripherals only support >> standard ldr/str...? >> >> There is the SEMA4 module which provides hardware semaphores. I'm aware >> of the hardware spinlock drivers (drivers/hwspinlock/), I started to >> implement such a driver for Vybrid. But so far a grep through the kernel >> does not show one usage of that framework... I guess we could add dt >> support for that, so we can assign the locks to individual drivers. >> >> I also plan to have a deeper look into remoteproc/rpmsg, not sure if >> locking of shared peripherals is part (or planned to be part) of that >> framework. >> >> For the clock stuff, the problem is more complex: I guess the would need >> some kind of master/slave definition, where we disallow the change of >> the shared clocks for the slave. >> >> If you are aware of patches/solutions, I'm happy to hear it... > > I don't have a real solution for this, but I'd be tempted to generate > the M4 DT based on the HW the A5 is not using, and only describe that. > > Clearly not ideal, but it gives you the control you need (don't describe > the HW you don't want to see touched)... > Yeah, that avoids the need for any synchronization for those peripherals. However, for some hardware (e.g. clocks and that interrupt controller) both sides need access... I guess to do it properly, I need to take care of it... -- Stefan
diff --git a/include/linux/irqchip/arm-gic.h b/include/linux/irqchip/arm-gic.h index 13eed92..3b1baf1 100644 --- a/include/linux/irqchip/arm-gic.h +++ b/include/linux/irqchip/arm-gic.h @@ -111,11 +111,14 @@ int gic_get_cpu_id(unsigned int cpu); void gic_migrate_target(unsigned int new_cpu_id); unsigned long gic_get_sgir_physaddr(void); +#ifdef CONFIG_ARM_GIC extern const struct irq_domain_ops *gic_routable_irq_domain_ops; static inline void __init register_routable_domain_ops (const struct irq_domain_ops *ops) { gic_routable_irq_domain_ops = ops; } +#endif /* CONFIG_ARM_GIC */ + #endif /* __ASSEMBLY */ #endif
The inline function register_routable_domain_ops is only usable if CONFIG_ARM_GIC is set. Make it depend on this configuration. This also allows other SoC interrupt controller to provide such a function. Signed-off-by: Stefan Agner <stefan@agner.ch> --- include/linux/irqchip/arm-gic.h | 3 +++ 1 file changed, 3 insertions(+)