Re: [PATCH 5/5] ARM: gic: add OF based initialization
On 9/19/2011 11:53 PM, Rob Herring wrote: On 09/19/2011 04:14 PM, Grant Likely wrote: On Mon, Sep 19, 2011 at 7:48 AM, Rob Herringrobherri...@gmail.com wrote: On 09/19/2011 07:09 AM, Cousson, Benoit wrote: On 9/18/2011 11:23 PM, Rob Herring wrote: I was headed down the path of implementing the 2nd option above, but had a dilemma. What would be the numbering base for PPIs in this case? Should it be 0 in the DT as proposed for SPIs or does it stay at 16? Both SGI and PPI are internal to the CortexA9 MP core, and referring to the CortexA9 MP core TRM [1], you can see that the PPI# - ID# mapping is already documented: - Private timer, PPI(2) Each Cortex-A9 processor has its own private timers that can generate interrupts, using ID29. - Watchdog timers, PPI(3) Each Cortex-A9 processor has its own watchdog timers that can generate interrupts, using ID30. So in that case, it can makes sense to use the ID. But it is interesting to note that the PPI is identified with a 0 based index number. It's even worse than I thought: we could use 13 (ID16 == PPI0), 29 or 2 for the timer interrupt. The first would match 0 based SPI convention. The last 2 would both match the documentation. We could never use 2 as this will for sure be different and the GIC code will have no way to know how to do the translation to ID. The only sane choice is using the ID as you say. But you can't have it both ways. It does not make sense to use the ID for some interrupts and a different scheme for others. Hmmm, it seems to me that some orthogonal issues are getting conflated. Specifically, the binding vs. what the GIC driver using internally. For my own understanding, let me see if I can summarize and clarify the problem. Each GIC IRQ is represented in 5 different ways: 1) the hardware documentation (PPI[0-15] or SPI[988] input pin) 2) The DT binding to represent the connection. 3) The Interrupt ID as specified by the GIC architecture reference[1] (SGI:[0-15], PPI:[16-31], SPI:[32-1019], special:[1020-1023]) 4) The internal HWIRQ representation used by the GIC driver 5) The Linux VIRQ number that #4 maps to. [1] http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.ihi0048b/BCGBFHCH.html Some thoughts: - Generally the DT binding (#2) should reflect the HW view of the system (#1) since that is the number most likely to be represented in hardware manuals. The interrupt ID is an internal detail of the GIC, and isn't really exposed in the block diagram of the hardware. - Presumably it is preferable for the GIC to directly use the Interrupt ID (#3) as the HWIRQ number (#4) because it is the most efficient from an interrupt handling perspective, and indeed this is currently what the GIC driver does. - Translation between the DT binding (#2) and the Interrupt ID / HWIRQ (#3/#4) is trivial, and easily managed by the GIC's irq_domain. - Though not necessarily as trivial, the mapping between Linux VIRQ and HWIRQ is not fixed, and when migrating to DT it should be assumed to be assigned at runtime. Perhaps not so important for a core IRQ controller like the GIC (as opposed to an i2c irq expander), but assuming an fixed offset still should be avoided. We may still force a SPI0-VIRQ32 on the root GIC as an optimization, but it is not necessary and the driver still needs to support remapping for a secondary GIC. The irq base is dynamic in my series, but is typically still GIC ID = VIRQ for a primary GIC for now. A platform can adjust this with irq_alloc_descs if necessary (but recommended not to of course). So, for the GIC DT binding, I'm inclined to agree with Benoit that the binding should reflect the hardware connections, not the values used by software for decoding IRQs. Also, I see absolutely no need to use separate nodes for each GIC interrupt space. The DT interrupt specifier number space can more than handle the features of the GIC in a clear and concise manor. So, here's my counter proposal for a GIC bindings (using Rob's text as the starting point): * ARM Generic Interrupt Controller ARM SMP cores are often associated with a GIC, providing per processor interrupts (PPI), shared processor interrupts (SPI) and software generated interrupts (SGI). Primary GIC is attached directly to the CPU and typically has PPIs and SGIs. Secondary GICs are cascaded into the upward interrupt controller and do not have PPIs or SGIs. Main node required properties: - compatible : should be one of: arm,cortex-a9-gic arm,arm11mp-gic - interrupt-controller : Identifies the node as an interrupt controller - #interrupt-cells : Specifies the number of cells needed to encode an interrupt source. The type shall be au32 and the value shall be 3. The 1st cell is the interrupt type; 0 for SPI interrupts, 1 for PPI interrupts. The 2nd cell contains the interrupt number for the interrupt type. SPI interrupts are in the range [0-987]. PPI interrupts are in the range [0-15]. The 3rd cell is the flags,
Re: [PATCH 5/5] ARM: gic: add OF based initialization
On 9/18/2011 8:15 AM, Grant Likely wrote:
On Thu, Sep 15, 2011 at 12:07:25PM +0200, Cousson, Benoit wrote:
Hi Rob,
On 9/15/2011 9:55 AM, Thomas Abraham wrote:
Hi Rob,
On 14 September 2011 22:01, Rob Herringrobherri...@gmail.com wrote:
From: Rob Herringrob.herr...@calxeda.com
This adds gic initialization using device tree data. The initialization
functions are intended to be called by a generic OF interrupt
controller parsing function once the right pieces are in place.
PPIs are handled using 3rd cell of interrupts properties to specify the cpu
mask the PPI is assigned to.
Signed-off-by: Rob Herringrob.herr...@calxeda.com
---
Documentation/devicetree/bindings/arm/gic.txt | 53
arch/arm/common/gic.c | 55 +++--
arch/arm/include/asm/hardware/gic.h | 10 +
3 files changed, 114 insertions(+), 4 deletions(-)
create mode 100644 Documentation/devicetree/bindings/arm/gic.txt
[...]
diff --git a/arch/arm/common/gic.c b/arch/arm/common/gic.c
index d1ccc72..14de380 100644
--- a/arch/arm/common/gic.c
+++ b/arch/arm/common/gic.c
[...]
+void __init gic_of_init(struct device_node *node, struct device_node *parent)
+{
+ void __iomem *cpu_base;
+ void __iomem *dist_base;
+ int irq;
+ struct irq_domain *domain =gic_data[gic_cnt].domain;
+
+ if (WARN_ON(!node))
+ return;
+
+ dist_base = of_iomap(node, 0);
+ WARN(!dist_base, unable to map gic dist registers\n);
+
+ cpu_base = of_iomap(node, 1);
+ WARN(!cpu_base, unable to map gic cpu registers\n);
+
+ domain-nr_irq = gic_irq_count(dist_base);
+ domain-irq_base = irq_alloc_descs(-1, 0, domain-nr_irq,
numa_node_id());
For exynos4, all the interrupts originating from GIC are statically
mapped to start from 32 in the linux virq space (GIC SPI interrupts
start from 64). In the above code, since irq_base would be 0 for
exynos4, the interrupt mapping is not working correctly. In your
previous version of the patch, you have given a option to the platform
code to choose the offset. Could that option be added to this series
also. Or a provision to use platform specific translate function
instead of the irq_domain_simple translator.
I have another concern on a similar topic.
On OMAP4 the SoC interrupts external to the MPU (SPI) have an offset
of 32. Only the internal PPI are between 0 and 31.
For the moment we add 32 to every SoC interrupts in the irq.h
define, but I'm assuming that this offset calculation should be done
thanks to a dedicated irq domain for the SPI.
The real HW physical number start at 0, and thus this is that value
that should be in the irq binding of the device.
Yes.
So ideally we should have a irq domain for the PPI starting at 0 and
another one for the SPI starting at 32. Or 32 and 64 for the exynos4
case, but it looks like the PPI/SPI offset is always 32.
Part of the purpose behind irq_domains is to have a translator
callback that can take care of complex mappings, such as mapping each
of the GIC irq ranges onto the Linux irq space. Plus, by being based
on the DT irq specifiers and dynamically assigning the linux numbers,
the actual mapping that the kernel chooses to use shouldn't actually
have any relevance. So whether or not the driver uses an offset is 32
becomes an implementation detail.
I do agree, my point was not about the driver usage but about how the
device node should populate its irq entry. The +32 offset is due to the
internal implementation of the GIC. That should not be exposed outside
the MPUSS.
Here are the first IRQs from the OMAP4430 public TRM.
MA_IRQ_0 L2_CACHE_IRQ CORTEXA9 L2 cache controller interrupt
MA_IRQ_1 CTI_IRQ_0 CORTEXA9 Cross-trigger module 0 (CTI0) interrupt
MA_IRQ_2 CTI_IRQ_1 CORTEXA9 Cross-trigger module 1 (CTI1) interrupt
MA_IRQ_3 Reserved Reserved Reserved
MA_IRQ_4 ELM_IRQ ELM Error location process completion
MA_IRQ_5 Reserved Reserved Reserved
MA_IRQ_6 Reserved Reserved Reserved
MA_IRQ_7 sys_nirq1 External External interrupt 1 (active low)
MA_IRQ_8 Reserved Reserved Reserved
MA_IRQ_9 L3_DBG_IRQ L3 L3 interconnect debug error
MA_IRQ_10 L3_APP_IRQ L3 L3 interconnect application error
MA_IRQ_11 PRCM_MPU_IRQ PRCM PRCM interrupt
...
It is a 0 based index, and thus this is the value I'm expecting to enter
in the irq attribute of the DT node.
Regards,
Benoit
___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH 5/5] ARM: gic: add OF based initialization
On Sun, Sep 18, 2011 at 7:21 AM, Grant Likely grant.lik...@secretlab.ca wrote: On Fri, Sep 16, 2011 at 05:09:39PM +0100, Dave Martin wrote: For now, we express the mapping by putting an interrupt-map in the core-tile DT, but this feels inelegant as well as wasteful -- expressing + 32 using a table which is about 1K in size and duplicates that information 43 times. Using a dedicated irq domain or a fake interrupt controller node to encapsulate the motherboard interrupts feels like a cleaner approach, but for now I'm not clear on the best way to do it. An irq nexus node would indeed be something to investigate for your particular case. Look for examples of interrupt-map. It is most often used for handling IRQ swizzling on PCI busses. That's what I currently have -- this is logically correct, and it works; it just feels like a sledgehammer for cracking this particular nut, because we don't really have 43 independent interrupt mappings and types. We have one offset and one type which is applied to all the interrupts, and this situation is expected to be the same for all variations of this board, except that offset may change. I suspect this situation applies to many platforms -- the number of interrupts may in general be much larger than the number of independent mappings. Another way of looking at the problem is that it's difficult to come up with a one-size-fits-all description of interrupt mappings which is also efficient. Requiring a single set of mapping semantics requires the mappings to be both rather overspecified for most cases, and flattened into a large, structureless table which may become pretty large and unwieldy. If there were 100+ interrupts instead of 43, we'd really have to be generating the device tree using a script in order for it to be maintainable (which is not necessarily a problem, but can be a warning sign) An alternative approach is to introduce a special interrupt controller node which serves as the interrupt-parent for the child domain and maps the interrupts with flexible semantics defined by the node's bindings; or different kinds of interrupt-map/interrupt-map-mask properties could be defined. Bindings could be added as needed to support different cases -- though really we should only expect to have a small number at most. When the interrupt mapping is significantly complex, using interrupt-map will probably be the best approach anyway. This is just a view for discussion. For now, I'll keep the interrupt-map for VE. since although it feels inefficient it is at least obviously correct. Cheers ---Dave ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH 5/5] ARM: gic: add OF based initialization
On 9/18/2011 11:23 PM, Rob Herring wrote: On 09/15/2011 11:43 AM, Rob Herring wrote: On 09/15/2011 08:52 AM, Cousson, Benoit wrote: On 9/15/2011 3:11 PM, Rob Herring wrote: On 09/15/2011 05:07 AM, Cousson, Benoit wrote: [...] I have another concern on a similar topic. On OMAP4 the SoC interrupts external to the MPU (SPI) have an offset of 32. Only the internal PPI are between 0 and 31. For the moment we add 32 to every SoC interrupts in the irq.h define, Those defines will not be used in the DT case. So the question is whether to add 32 or not in the DT. Since we have just a single node and a linear mapping of PPIs and SPIs, the only choice is to have SPIs start at 32. And from the h/w definition, SPIs always start at 32, so it's in agreement. This is a agreement inside the MPUSS, but not outside. Both Tegra and OMAP4 must add an offset to the HW irq number to deal with that today. but I'm assuming that this offset calculation should be done thanks to a dedicated irq domain for the SPI. The real HW physical number start at 0, and thus this is that value that should be in the irq binding of the device. So ideally we should have a irq domain for the PPI starting at 0 and another one for the SPI starting at 32. Or 32 and 64 for the exynos4 case, but it looks like the PPI/SPI offset is always 32. That offset of SPIs is always there. If you have a GIC as a secondary controller, It will have 32 reserved interrupts and the register layout is exactly the same as a cpu's GIC. Yep, but that's the GIC view and not the SoC one. My concern is to have to tweak the HW number provided by the HW spec in order to add that offset. If you look at SoC level, the MPUSS is just an IP that can be potentially replaced by other one that will not have a GIC. In that case you will not change the IRQ mapping at SoC level. For example if you replace the Dual-cortexA9 by a single CortexA8, then all the interrupts will have to be shifted by 32 just because the MPU subsystem is different. Is that a realistic case? That would be a new chip and new device tree. You could argue that the whole peripheral subsystem DT could be reused and the numbering needs to be the same. However, there's one thing that would prevent that. The number of interrupt cells is defined by the controller binding. So you have to change the peripheral nodes anyway. It's good that OMAP is trying to standardize the peripheral layout, but in my experience that's not something you can rely on. At some point the interrupt numbering is going to differ from the h/w documentation. If it's not in the DT, then it will be in linux. Right now its just offset of 32, but if irqdescs get assigned on demand as PPC is doing, then there will be no relationship to the documentation. Since that offset is dependent of the GIC internals and is not exposed outside the MPUSS, it should not be visible by the SoC IPs. And the HW spec is exposing exactly that. Since the idea of splitting PPIs for each core out to a flattened linux irq map has been abandoned, I see no reason to have more than 1 domain with a simple linear translation. Ultimately, domains will do dynamic irqdesc allocation and the translation within the gic will be completely dynamic. I think the only reason to do that is to separate internal MPU interrupts with the external ones that should not have a clue about the GIC. I see 2 options (besides leaving it as is): - Revert back to my previous binding where PPIs are a sub-node and a different interrupt parent. - Use the current binding, but allow SPIs to start at 0. We can still distinguish PPIs and SPIs by the cpu mask cell. A cpu mask of 0 is a SPI. If there was ever a reason to have a cpu mask for an SPI, you would not be able to with this scheme. Either way you will still have the above issue with the cell size changing. I was headed down the path of implementing the 2nd option above, but had a dilemma. What would be the numbering base for PPIs in this case? Should it be 0 in the DT as proposed for SPIs or does it stay at 16? Both SGI and PPI are internal to the CortexA9 MP core, and referring to the CortexA9 MP core TRM [1], you can see that the PPI# - ID# mapping is already documented: - Private timer, PPI(2) Each Cortex-A9 processor has its own private timers that can generate interrupts, using ID29. - Watchdog timers, PPI(3) Each Cortex-A9 processor has its own watchdog timers that can generate interrupts, using ID30. So in that case, it can makes sense to use the ID. But it is interesting to note that the PPI is identified with a 0 based index number. Numbering PPIs at 0 will just cause confusion as will numbering differently from SPIs. There is absolutely no mention of SPI0 or SPIx numbering in the GIC spec. Probably because it is the generic GIC spec that focus on internals stuff only, it not an integration spec that will show how the SPIs are connected to the
Re: [PATCH 5/5] ARM: gic: add OF based initialization
Hi Grant,
On 18 September 2011 11:40, Grant Likely grant.lik...@secretlab.ca wrote:
On Fri, Sep 16, 2011 at 03:04:11PM +0530, Thomas Abraham wrote:
Hi Rob,
On 15 September 2011 18:24, Rob Herring robherri...@gmail.com wrote:
On 09/15/2011 02:55 AM, Thomas Abraham wrote:
+void __init gic_of_init(struct device_node *node, struct device_node
*parent)
+{
+ void __iomem *cpu_base;
+ void __iomem *dist_base;
+ int irq;
+ struct irq_domain *domain = gic_data[gic_cnt].domain;
+
+ if (WARN_ON(!node))
+ return;
+
+ dist_base = of_iomap(node, 0);
+ WARN(!dist_base, unable to map gic dist registers\n);
+
+ cpu_base = of_iomap(node, 1);
+ WARN(!cpu_base, unable to map gic cpu registers\n);
+
+ domain-nr_irq = gic_irq_count(dist_base);
+ domain-irq_base = irq_alloc_descs(-1, 0, domain-nr_irq,
numa_node_id());
For exynos4, all the interrupts originating from GIC are statically
mapped to start from 32 in the linux virq space (GIC SPI interrupts
start from 64). In the above code, since irq_base would be 0 for
exynos4, the interrupt mapping is not working correctly. In your
previous version of the patch, you have given a option to the platform
code to choose the offset. Could that option be added to this series
also. Or a provision to use platform specific translate function
instead of the irq_domain_simple translator.
So I guess you have the A9 external nIRQ hooked up to another
controller? Why can't the 0-31 interrupts get mapped to after the gic
interrupts? Ultimately we want h/w irq numbers completely decoupled from
linux irq numbers. So you will want to put that controller in devicetree
and have an DT init function for it as well.
There are chained interrupt handlers mapped in between linux irq
number 0 to 31. So the offset for GIC interrupts was set to 32 (SGI[0]
= 32). The interrupt chaining for the interrupts mapped between 0 to
31 seems unnecessary though. I will try removing them and check.
Please note; when using the DT, the linux virq number should be
dynamically assigned and therefore will not matter. Historically
Exynos may have started from irq 32, but it doesn't really have any
relevance when all IRQ references are via DT irq specifiers.
Ok. But the exynos4 modules that have dt support depend on other
modules that are yet to get dt support. Those modules still use static
linux interrupt numbers and so until all the modules get dt support,
interrupt number specified in dt would have to follow the static
numbering that the rest of the system uses.
So to use Rob's patches for exynos4 dt, the interrupts mapped in the
range 1 to 31 will have to be reworked or moved to some other irq
numbers.
Plus, for dynamically allocated irq_descs, I really want to make sure
that irq 0 never gets assigned. We're not supposed to be using it,
and that becomes an easy rule to enforce when interrupt numbers are no
longer assigned with #defines.
g.
Thanks,
Thomas.
___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH 5/5] ARM: gic: add OF based initialization
On 9/19/2011 2:07 PM, Dave Martin wrote: On Sun, Sep 18, 2011 at 7:21 AM, Grant Likelygrant.lik...@secretlab.ca wrote: On Fri, Sep 16, 2011 at 05:09:39PM +0100, Dave Martin wrote: For now, we express the mapping by putting an interrupt-map in the core-tile DT, but this feels inelegant as well as wasteful -- expressing + 32 using a table which is about 1K in size and duplicates that information 43 times. Using a dedicated irq domain or a fake interrupt controller node to encapsulate the motherboard interrupts feels like a cleaner approach, but for now I'm not clear on the best way to do it. An irq nexus node would indeed be something to investigate for your particular case. Look for examples of interrupt-map. It is most often used for handling IRQ swizzling on PCI busses. That's what I currently have -- this is logically correct, and it works; it just feels like a sledgehammer for cracking this particular nut, because we don't really have 43 independent interrupt mappings and types. We have one offset and one type which is applied to all the interrupts, and this situation is expected to be the same for all variations of this board, except that offset may change. I suspect this situation applies to many platforms -- the number of interrupts may in general be much larger than the number of independent mappings. Another way of looking at the problem is that it's difficult to come up with a one-size-fits-all description of interrupt mappings which is also efficient. Requiring a single set of mapping semantics requires the mappings to be both rather overspecified for most cases, and flattened into a large, structureless table which may become pretty large and unwieldy. If there were 100+ interrupts instead of 43, we'd really have to be generating the device tree using a script in order for it to be maintainable (which is not necessarily a problem, but can be a warning sign) Yep, this is exactly the issue I was facing when I tried to map the 128 interrupts lines of an OMAP4 to the GIC. Adding 128 entries to an interrupt map just to handle a simple linear translation with a constant value of 32 is clearly overkill. An alternative approach is to introduce a special interrupt controller node which serves as the interrupt-parent for the child domain and maps the interrupts with flexible semantics defined by the node's bindings; or different kinds of interrupt-map/interrupt-map-mask properties could be defined. Bindings could be added as needed to support different cases -- though really we should only expect to have a small number at most. When the interrupt mapping is significantly complex, using interrupt-map will probably be the best approach anyway. Maybe we can just extend or add a new type of interrupt nexus to handle simple translation. The actual one was done for complex PCI mapping but with a small number of lines to handle. In our case, the mapping is simple, but the number of lines is huge. Regards, Benoit ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH 5/5] ARM: gic: add OF based initialization
On Mon, Sep 19, 2011 at 11:47:18AM +0200, Cousson, Benoit wrote: Since the cpumask is not relevant for the SPI, maybe having two interrupt controllers will be more relevant. Or maybe 3, since there is some SGIs as well. I don't think anyone uses SGIs outside of the common SMP code. Therefore they're handled entirely separately for the root GIC. (If there's two GICs - some platforms do have two - then the SGIs on the non-root GIC are unused.) ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH 5/5] ARM: gic: add OF based initialization
On 09/19/2011 07:09 AM, Cousson, Benoit wrote: On 9/18/2011 11:23 PM, Rob Herring wrote: On 09/15/2011 11:43 AM, Rob Herring wrote: On 09/15/2011 08:52 AM, Cousson, Benoit wrote: On 9/15/2011 3:11 PM, Rob Herring wrote: On 09/15/2011 05:07 AM, Cousson, Benoit wrote: [...] I have another concern on a similar topic. On OMAP4 the SoC interrupts external to the MPU (SPI) have an offset of 32. Only the internal PPI are between 0 and 31. For the moment we add 32 to every SoC interrupts in the irq.h define, Those defines will not be used in the DT case. So the question is whether to add 32 or not in the DT. Since we have just a single node and a linear mapping of PPIs and SPIs, the only choice is to have SPIs start at 32. And from the h/w definition, SPIs always start at 32, so it's in agreement. This is a agreement inside the MPUSS, but not outside. Both Tegra and OMAP4 must add an offset to the HW irq number to deal with that today. but I'm assuming that this offset calculation should be done thanks to a dedicated irq domain for the SPI. The real HW physical number start at 0, and thus this is that value that should be in the irq binding of the device. So ideally we should have a irq domain for the PPI starting at 0 and another one for the SPI starting at 32. Or 32 and 64 for the exynos4 case, but it looks like the PPI/SPI offset is always 32. That offset of SPIs is always there. If you have a GIC as a secondary controller, It will have 32 reserved interrupts and the register layout is exactly the same as a cpu's GIC. Yep, but that's the GIC view and not the SoC one. My concern is to have to tweak the HW number provided by the HW spec in order to add that offset. If you look at SoC level, the MPUSS is just an IP that can be potentially replaced by other one that will not have a GIC. In that case you will not change the IRQ mapping at SoC level. For example if you replace the Dual-cortexA9 by a single CortexA8, then all the interrupts will have to be shifted by 32 just because the MPU subsystem is different. Is that a realistic case? That would be a new chip and new device tree. You could argue that the whole peripheral subsystem DT could be reused and the numbering needs to be the same. However, there's one thing that would prevent that. The number of interrupt cells is defined by the controller binding. So you have to change the peripheral nodes anyway. It's good that OMAP is trying to standardize the peripheral layout, but in my experience that's not something you can rely on. At some point the interrupt numbering is going to differ from the h/w documentation. If it's not in the DT, then it will be in linux. Right now its just offset of 32, but if irqdescs get assigned on demand as PPC is doing, then there will be no relationship to the documentation. Since that offset is dependent of the GIC internals and is not exposed outside the MPUSS, it should not be visible by the SoC IPs. And the HW spec is exposing exactly that. Since the idea of splitting PPIs for each core out to a flattened linux irq map has been abandoned, I see no reason to have more than 1 domain with a simple linear translation. Ultimately, domains will do dynamic irqdesc allocation and the translation within the gic will be completely dynamic. I think the only reason to do that is to separate internal MPU interrupts with the external ones that should not have a clue about the GIC. I see 2 options (besides leaving it as is): - Revert back to my previous binding where PPIs are a sub-node and a different interrupt parent. - Use the current binding, but allow SPIs to start at 0. We can still distinguish PPIs and SPIs by the cpu mask cell. A cpu mask of 0 is a SPI. If there was ever a reason to have a cpu mask for an SPI, you would not be able to with this scheme. Either way you will still have the above issue with the cell size changing. I was headed down the path of implementing the 2nd option above, but had a dilemma. What would be the numbering base for PPIs in this case? Should it be 0 in the DT as proposed for SPIs or does it stay at 16? Both SGI and PPI are internal to the CortexA9 MP core, and referring to the CortexA9 MP core TRM [1], you can see that the PPI# - ID# mapping is already documented: - Private timer, PPI(2) Each Cortex-A9 processor has its own private timers that can generate interrupts, using ID29. - Watchdog timers, PPI(3) Each Cortex-A9 processor has its own watchdog timers that can generate interrupts, using ID30. So in that case, it can makes sense to use the ID. But it is interesting to note that the PPI is identified with a 0 based index number. It's even worse than I thought: we could use 13 (ID16 == PPI0), 29 or 2 for the timer interrupt. The first would match 0 based SPI convention. The last 2 would both match the documentation. We could never use 2 as this will for sure be
Re: [PATCH 5/5] ARM: gic: add OF based initialization
On 9/19/2011 3:48 PM, Rob Herring wrote: On 09/19/2011 07:09 AM, Cousson, Benoit wrote: On 9/18/2011 11:23 PM, Rob Herring wrote: On 09/15/2011 11:43 AM, Rob Herring wrote: On 09/15/2011 08:52 AM, Cousson, Benoit wrote: On 9/15/2011 3:11 PM, Rob Herring wrote: On 09/15/2011 05:07 AM, Cousson, Benoit wrote: [...] I have another concern on a similar topic. On OMAP4 the SoC interrupts external to the MPU (SPI) have an offset of 32. Only the internal PPI are between 0 and 31. For the moment we add 32 to every SoC interrupts in the irq.h define, Those defines will not be used in the DT case. So the question is whether to add 32 or not in the DT. Since we have just a single node and a linear mapping of PPIs and SPIs, the only choice is to have SPIs start at 32. And from the h/w definition, SPIs always start at 32, so it's in agreement. This is a agreement inside the MPUSS, but not outside. Both Tegra and OMAP4 must add an offset to the HW irq number to deal with that today. but I'm assuming that this offset calculation should be done thanks to a dedicated irq domain for the SPI. The real HW physical number start at 0, and thus this is that value that should be in the irq binding of the device. So ideally we should have a irq domain for the PPI starting at 0 and another one for the SPI starting at 32. Or 32 and 64 for the exynos4 case, but it looks like the PPI/SPI offset is always 32. That offset of SPIs is always there. If you have a GIC as a secondary controller, It will have 32 reserved interrupts and the register layout is exactly the same as a cpu's GIC. Yep, but that's the GIC view and not the SoC one. My concern is to have to tweak the HW number provided by the HW spec in order to add that offset. If you look at SoC level, the MPUSS is just an IP that can be potentially replaced by other one that will not have a GIC. In that case you will not change the IRQ mapping at SoC level. For example if you replace the Dual-cortexA9 by a single CortexA8, then all the interrupts will have to be shifted by 32 just because the MPU subsystem is different. Is that a realistic case? That would be a new chip and new device tree. You could argue that the whole peripheral subsystem DT could be reused and the numbering needs to be the same. However, there's one thing that would prevent that. The number of interrupt cells is defined by the controller binding. So you have to change the peripheral nodes anyway. It's good that OMAP is trying to standardize the peripheral layout, but in my experience that's not something you can rely on. At some point the interrupt numbering is going to differ from the h/w documentation. If it's not in the DT, then it will be in linux. Right now its just offset of 32, but if irqdescs get assigned on demand as PPC is doing, then there will be no relationship to the documentation. Since that offset is dependent of the GIC internals and is not exposed outside the MPUSS, it should not be visible by the SoC IPs. And the HW spec is exposing exactly that. Since the idea of splitting PPIs for each core out to a flattened linux irq map has been abandoned, I see no reason to have more than 1 domain with a simple linear translation. Ultimately, domains will do dynamic irqdesc allocation and the translation within the gic will be completely dynamic. I think the only reason to do that is to separate internal MPU interrupts with the external ones that should not have a clue about the GIC. I see 2 options (besides leaving it as is): - Revert back to my previous binding where PPIs are a sub-node and a different interrupt parent. - Use the current binding, but allow SPIs to start at 0. We can still distinguish PPIs and SPIs by the cpu mask cell. A cpu mask of 0 is a SPI. If there was ever a reason to have a cpu mask for an SPI, you would not be able to with this scheme. Either way you will still have the above issue with the cell size changing. I was headed down the path of implementing the 2nd option above, but had a dilemma. What would be the numbering base for PPIs in this case? Should it be 0 in the DT as proposed for SPIs or does it stay at 16? Both SGI and PPI are internal to the CortexA9 MP core, and referring to the CortexA9 MP core TRM [1], you can see that the PPI# - ID# mapping is already documented: - Private timer, PPI(2) Each Cortex-A9 processor has its own private timers that can generate interrupts, using ID29. - Watchdog timers, PPI(3) Each Cortex-A9 processor has its own watchdog timers that can generate interrupts, using ID30. So in that case, it can makes sense to use the ID. But it is interesting to note that the PPI is identified with a 0 based index number. It's even worse than I thought: we could use 13 (ID16 == PPI0), 29 or 2 for the timer interrupt. The first would match 0 based SPI convention. I didn't even noticed that mess :-( Maybe that PPI number is not that relevant in that case. It looks like there are using the last 4 lines
Re: [PATCH 5/5] ARM: gic: add OF based initialization
On Mon, Sep 19, 2011 at 02:09:46PM +0200, Cousson, Benoit wrote: Every CortexA9 based SoC have to add the 32 offset to the SoC level interrupt number line. The ID numbering scheme is relevant only inside the GIC, but at SoC level only the IRQ lines that entered the MP core are relevant. That ID is a pure internal GIC encoding. As far as SPIs go, I think what should be done is that the DT should refer to a SPI phandle plus the SPI number, and be done with just that. The point as to whether SoCs internally use SPIs themselves is a complete distraction - if they're using SPIs internally then we _also_ need some way for the on-SoC peripherals to refer to them too. What the GIC exports are 16 PPIs per CPU, 16 SGIs and N SPIs. That's what we should be modelling for the GIC, not something else. So peripherals connect to an SPI numbered N where N = 0. How we want SPIs to map to Linux IRQ numbers is the issue, and as things stand at present, we want SPI0 to map to IRQ32 on all platforms where the GIC is the root, to avoid any unnecessary complexity (because the hardware tells us that SPI0 gives us ID32 in the interrupt acknowledge register.) Doing anything else requires computation or a lookup table, and we shouldn't be doing that kind of thing unless there's a real reason to do so (there isn't, especially with sparse irq support.) As far as PPIs go, support for that is still being worked on, and most of that at present does not go through genirq stuff (and it isn't relevant to use the standard genirq interfaces for PPIs _anyway_.) SGIs don't use genirq in any way, and are used for SMP IPIs. That's completely separate from the way IRQs are used - they're not connected to devices at all. (They're provided as an inter-processor communication method.) So forget SGIs. They may apparantly occupy IRQ IDs 0-15, but reality is they leave those IDs unused, IRQs 0-15 are not requestable, which is a definite *good* thing. ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH 5/5] ARM: gic: add OF based initialization
On Mon, Sep 19, 2011 at 7:33 AM, Russell King - ARM Linux li...@arm.linux.org.uk wrote: On Mon, Sep 19, 2011 at 11:47:18AM +0200, Cousson, Benoit wrote: Since the cpumask is not relevant for the SPI, maybe having two interrupt controllers will be more relevant. Or maybe 3, since there is some SGIs as well. I don't think anyone uses SGIs outside of the common SMP code. Therefore they're handled entirely separately for the root GIC. (If there's two GICs - some platforms do have two - then the SGIs on the non-root GIC are unused.) ...and since SGIs aren't external HW inputs to the GIC, there really isn't any need to reflect them in the GIC DT binding. g. ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH 5/5] ARM: gic: add OF based initialization
On Sun, Sep 18, 2011 at 04:23:40PM -0500, Rob Herring wrote: On 09/15/2011 11:43 AM, Rob Herring wrote: I see 2 options (besides leaving it as is): - Revert back to my previous binding where PPIs are a sub-node and a different interrupt parent. - Use the current binding, but allow SPIs to start at 0. We can still distinguish PPIs and SPIs by the cpu mask cell. A cpu mask of 0 is a SPI. If there was ever a reason to have a cpu mask for an SPI, you would not be able to with this scheme. Either way you will still have the above issue with the cell size changing. I was headed down the path of implementing the 2nd option above, but had a dilemma. What would be the numbering base for PPIs in this case? Should it be 0 in the DT as proposed for SPIs or does it stay at 16? Numbering PPIs at 0 will just cause confusion as will numbering differently from SPIs. There is absolutely no mention of SPI0 or SPIx numbering in the GIC spec. All interrupt number references refer to the absolute interrupt ID, not a relative number based on the type. Hi Rob, See here[1] and [2] (figures 3.14 and 3.16). In both cases, there is clearly a reference to PPI numbering from 0-15 and SPI numbering from 0-987 (as inputs to the distributor block). [1] http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.ddi0416b/Bhacbfdb.html [2] http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.ddi0416b/Cihebcbg.html g. ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH 5/5] ARM: gic: add OF based initialization
On Mon, Sep 19, 2011 at 3:53 PM, Rob Herring robherri...@gmail.com wrote: On 09/19/2011 04:14 PM, Grant Likely wrote: * ARM Generic Interrupt Controller ARM SMP cores are often associated with a GIC, providing per processor interrupts (PPI), shared processor interrupts (SPI) and software generated interrupts (SGI). Primary GIC is attached directly to the CPU and typically has PPIs and SGIs. Secondary GICs are cascaded into the upward interrupt controller and do not have PPIs or SGIs. Main node required properties: - compatible : should be one of: arm,cortex-a9-gic arm,arm11mp-gic - interrupt-controller : Identifies the node as an interrupt controller - #interrupt-cells : Specifies the number of cells needed to encode an interrupt source. The type shall be a u32 and the value shall be 3. The 1st cell is the interrupt type; 0 for SPI interrupts, 1 for PPI interrupts. The 2nd cell contains the interrupt number for the interrupt type. SPI interrupts are in the range [0-987]. PPI interrupts are in the range [0-15]. The 3rd cell is the flags, encoded as follows: bits[3:0] trigger type and level flags. 1 = low-to-high edge triggered 2 = high-to-low edge triggered 4 = active high level-sensitive 8 = active low level-sensitive bits[15:8] PPI interrupt cpu mask. Each bit corresponds to each of the 8 possible cpus attached to the GIC. A bit set to '1' indicated the interrupt is wired to that CPU. Only valid for PPI interrupts. How about a cpu mask of 0 means SPI and non-zero means PPI? Then we can drop the first cell. Cells are cheap, and it is better to be explicit. It is certainly easier to extend in the future too if the type cell is used. g. ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH 5/5] ARM: gic: add OF based initialization
On Mon, Sep 19, 2011 at 04:53:39PM -0500, Rob Herring wrote: On 09/19/2011 04:14 PM, Grant Likely wrote: (Alternately, if there is no need for a CPU mask because PPI interrupts will never be wired to more than one CPU, then it would be better to encode the CPU number into the second cell with the SPI number). You meant PPI number, right?^^^ Yes, I meant PPI number. I keep transposing the two; I don't know why. The common case at least on the A9 is a PPI is routed to all cores. QC is different though. This was discussed previously. Basically, anything is possible here, so the mask is needed for sure. Okay. g. ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH 5/5] ARM: gic: add OF based initialization
On Fri, Sep 16, 2011 at 03:04:11PM +0530, Thomas Abraham wrote:
Hi Rob,
On 15 September 2011 18:24, Rob Herring robherri...@gmail.com wrote:
On 09/15/2011 02:55 AM, Thomas Abraham wrote:
+void __init gic_of_init(struct device_node *node, struct device_node
*parent)
+{
+ void __iomem *cpu_base;
+ void __iomem *dist_base;
+ int irq;
+ struct irq_domain *domain = gic_data[gic_cnt].domain;
+
+ if (WARN_ON(!node))
+ return;
+
+ dist_base = of_iomap(node, 0);
+ WARN(!dist_base, unable to map gic dist registers\n);
+
+ cpu_base = of_iomap(node, 1);
+ WARN(!cpu_base, unable to map gic cpu registers\n);
+
+ domain-nr_irq = gic_irq_count(dist_base);
+ domain-irq_base = irq_alloc_descs(-1, 0, domain-nr_irq,
numa_node_id());
For exynos4, all the interrupts originating from GIC are statically
mapped to start from 32 in the linux virq space (GIC SPI interrupts
start from 64). In the above code, since irq_base would be 0 for
exynos4, the interrupt mapping is not working correctly. In your
previous version of the patch, you have given a option to the platform
code to choose the offset. Could that option be added to this series
also. Or a provision to use platform specific translate function
instead of the irq_domain_simple translator.
So I guess you have the A9 external nIRQ hooked up to another
controller? Why can't the 0-31 interrupts get mapped to after the gic
interrupts? Ultimately we want h/w irq numbers completely decoupled from
linux irq numbers. So you will want to put that controller in devicetree
and have an DT init function for it as well.
There are chained interrupt handlers mapped in between linux irq
number 0 to 31. So the offset for GIC interrupts was set to 32 (SGI[0]
= 32). The interrupt chaining for the interrupts mapped between 0 to
31 seems unnecessary though. I will try removing them and check.
Please note; when using the DT, the linux virq number should be
dynamically assigned and therefore will not matter. Historically
Exynos may have started from irq 32, but it doesn't really have any
relevance when all IRQ references are via DT irq specifiers.
Plus, for dynamically allocated irq_descs, I really want to make sure
that irq 0 never gets assigned. We're not supposed to be using it,
and that becomes an easy rule to enforce when interrupt numbers are no
longer assigned with #defines.
g.
___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH 5/5] ARM: gic: add OF based initialization
On Fri, Sep 16, 2011 at 05:09:39PM +0100, Dave Martin wrote: For now, we express the mapping by putting an interrupt-map in the core-tile DT, but this feels inelegant as well as wasteful -- expressing + 32 using a table which is about 1K in size and duplicates that information 43 times. Using a dedicated irq domain or a fake interrupt controller node to encapsulate the motherboard interrupts feels like a cleaner approach, but for now I'm not clear on the best way to do it. An irq nexus node would indeed be something to investigate for your particular case. Look for examples of interrupt-map. It is most often used for handling IRQ swizzling on PCI busses. g. ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH 5/5] ARM: gic: add OF based initialization
On Thu, Sep 15, 2011 at 12:07:25PM +0200, Cousson, Benoit wrote:
Hi Rob,
On 9/15/2011 9:55 AM, Thomas Abraham wrote:
Hi Rob,
On 14 September 2011 22:01, Rob Herringrobherri...@gmail.com wrote:
From: Rob Herringrob.herr...@calxeda.com
This adds gic initialization using device tree data. The initialization
functions are intended to be called by a generic OF interrupt
controller parsing function once the right pieces are in place.
PPIs are handled using 3rd cell of interrupts properties to specify the cpu
mask the PPI is assigned to.
Signed-off-by: Rob Herringrob.herr...@calxeda.com
---
Documentation/devicetree/bindings/arm/gic.txt | 53
arch/arm/common/gic.c | 55
+++--
arch/arm/include/asm/hardware/gic.h | 10 +
3 files changed, 114 insertions(+), 4 deletions(-)
create mode 100644 Documentation/devicetree/bindings/arm/gic.txt
[...]
diff --git a/arch/arm/common/gic.c b/arch/arm/common/gic.c
index d1ccc72..14de380 100644
--- a/arch/arm/common/gic.c
+++ b/arch/arm/common/gic.c
[...]
+void __init gic_of_init(struct device_node *node, struct device_node
*parent)
+{
+ void __iomem *cpu_base;
+ void __iomem *dist_base;
+ int irq;
+ struct irq_domain *domain =gic_data[gic_cnt].domain;
+
+ if (WARN_ON(!node))
+ return;
+
+ dist_base = of_iomap(node, 0);
+ WARN(!dist_base, unable to map gic dist registers\n);
+
+ cpu_base = of_iomap(node, 1);
+ WARN(!cpu_base, unable to map gic cpu registers\n);
+
+ domain-nr_irq = gic_irq_count(dist_base);
+ domain-irq_base = irq_alloc_descs(-1, 0, domain-nr_irq,
numa_node_id());
For exynos4, all the interrupts originating from GIC are statically
mapped to start from 32 in the linux virq space (GIC SPI interrupts
start from 64). In the above code, since irq_base would be 0 for
exynos4, the interrupt mapping is not working correctly. In your
previous version of the patch, you have given a option to the platform
code to choose the offset. Could that option be added to this series
also. Or a provision to use platform specific translate function
instead of the irq_domain_simple translator.
I have another concern on a similar topic.
On OMAP4 the SoC interrupts external to the MPU (SPI) have an offset
of 32. Only the internal PPI are between 0 and 31.
For the moment we add 32 to every SoC interrupts in the irq.h
define, but I'm assuming that this offset calculation should be done
thanks to a dedicated irq domain for the SPI.
The real HW physical number start at 0, and thus this is that value
that should be in the irq binding of the device.
Yes.
So ideally we should have a irq domain for the PPI starting at 0 and
another one for the SPI starting at 32. Or 32 and 64 for the exynos4
case, but it looks like the PPI/SPI offset is always 32.
Part of the purpose behind irq_domains is to have a translator
callback that can take care of complex mappings, such as mapping each
of the GIC irq ranges onto the Linux irq space. Plus, by being based
on the DT irq specifiers and dynamically assigning the linux numbers,
the actual mapping that the kernel chooses to use shouldn't actually
have any relevance. So whether or not the driver uses an offset is 32
becomes an implementation detail.
___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH 5/5] ARM: gic: add OF based initialization
On Wed, Sep 14, 2011 at 11:31:40AM -0500, Rob Herring wrote: From: Rob Herring rob.herr...@calxeda.com This adds gic initialization using device tree data. The initialization functions are intended to be called by a generic OF interrupt controller parsing function once the right pieces are in place. PPIs are handled using 3rd cell of interrupts properties to specify the cpu mask the PPI is assigned to. Signed-off-by: Rob Herring rob.herr...@calxeda.com --- Documentation/devicetree/bindings/arm/gic.txt | 53 arch/arm/common/gic.c | 55 +++-- arch/arm/include/asm/hardware/gic.h | 10 + 3 files changed, 114 insertions(+), 4 deletions(-) create mode 100644 Documentation/devicetree/bindings/arm/gic.txt diff --git a/Documentation/devicetree/bindings/arm/gic.txt b/Documentation/devicetree/bindings/arm/gic.txt new file mode 100644 index 000..6c513de --- /dev/null +++ b/Documentation/devicetree/bindings/arm/gic.txt @@ -0,0 +1,53 @@ +* ARM Generic Interrupt Controller + +ARM SMP cores are often associated with a GIC, providing per processor +interrupts (PPI), shared processor interrupts (SPI) and software +generated interrupts (SGI). + +Primary GIC is attached directly to the CPU and typically has PPIs and SGIs. +Secondary GICs are cascaded into the upward interrupt controller and do not +have PPIs or SGIs. + +Main node required properties: + +- compatible : should be one of: + arm,cortex-a9-gic + arm,arm11mp-gic +- interrupt-controller : Identifies the node as an interrupt controller +- #interrupt-cells : Specifies the number of cells needed to encode an + interrupt source. The type shall be a u32 and the value shall be 3. + + The 1st cell is the interrupt number. 0-15 are reserved for SGIs. 16-31 are + for PPIs. + + The 2nd cell is the level-sense information, encoded as follows: +1 = low-to-high edge triggered +2 = high-to-low edge triggered +4 = active high level-sensitive +8 = active low level-sensitive + + Only values of 1 and 4 are valid for GIC 1.0 spec. + + The 3rd cell contains the mask of the cpu number for the interrupt source. + The cpu mask is only valid for PPIs and shall be 0 for SPIs. This value shall + be 0 for PPIs. THe binding looks good, but I really think the first cell should be the mask; followed by the irq number in the 2nd and the flags in the 3rd. That would better match with existing practice. + +- reg : Specifies base physical address(s) and size of the GIC registers. The + first 2 values are the GIC distributor register base and size. The 2nd 2 ... first region is the GIC distributor... ... The 2nd region is the GIC cpu ... I've only looked at the code changes superficially, but it looks good to me. g. ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH 5/5] ARM: gic: add OF based initialization
On 09/15/2011 11:43 AM, Rob Herring wrote:
Benoit,
On 09/15/2011 08:52 AM, Cousson, Benoit wrote:
On 9/15/2011 3:11 PM, Rob Herring wrote:
Benoit,
On 09/15/2011 05:07 AM, Cousson, Benoit wrote:
Hi Rob,
On 9/15/2011 9:55 AM, Thomas Abraham wrote:
Hi Rob,
On 14 September 2011 22:01, Rob Herringrobherri...@gmail.com wrote:
From: Rob Herringrob.herr...@calxeda.com
This adds gic initialization using device tree data. The
initialization
functions are intended to be called by a generic OF interrupt
controller parsing function once the right pieces are in place.
PPIs are handled using 3rd cell of interrupts properties to specify
the cpu
mask the PPI is assigned to.
Signed-off-by: Rob Herringrob.herr...@calxeda.com
---
Documentation/devicetree/bindings/arm/gic.txt | 53
arch/arm/common/gic.c | 55
+++--
arch/arm/include/asm/hardware/gic.h | 10 +
3 files changed, 114 insertions(+), 4 deletions(-)
create mode 100644 Documentation/devicetree/bindings/arm/gic.txt
[...]
diff --git a/arch/arm/common/gic.c b/arch/arm/common/gic.c
index d1ccc72..14de380 100644
--- a/arch/arm/common/gic.c
+++ b/arch/arm/common/gic.c
[...]
+void __init gic_of_init(struct device_node *node, struct device_node
*parent)
+{
+ void __iomem *cpu_base;
+ void __iomem *dist_base;
+ int irq;
+ struct irq_domain *domain =gic_data[gic_cnt].domain;
+
+ if (WARN_ON(!node))
+ return;
+
+ dist_base = of_iomap(node, 0);
+ WARN(!dist_base, unable to map gic dist registers\n);
+
+ cpu_base = of_iomap(node, 1);
+ WARN(!cpu_base, unable to map gic cpu registers\n);
+
+ domain-nr_irq = gic_irq_count(dist_base);
+ domain-irq_base = irq_alloc_descs(-1, 0, domain-nr_irq,
numa_node_id());
For exynos4, all the interrupts originating from GIC are statically
mapped to start from 32 in the linux virq space (GIC SPI interrupts
start from 64). In the above code, since irq_base would be 0 for
exynos4, the interrupt mapping is not working correctly. In your
previous version of the patch, you have given a option to the platform
code to choose the offset. Could that option be added to this series
also. Or a provision to use platform specific translate function
instead of the irq_domain_simple translator.
I have another concern on a similar topic.
On OMAP4 the SoC interrupts external to the MPU (SPI) have an offset of
32. Only the internal PPI are between 0 and 31.
For the moment we add 32 to every SoC interrupts in the irq.h define,
Those defines will not be used in the DT case. So the question is
whether to add 32 or not in the DT. Since we have just a single node and
a linear mapping of PPIs and SPIs, the only choice is to have SPIs start
at 32. And from the h/w definition, SPIs always start at 32, so it's in
agreement.
This is a agreement inside the MPUSS, but not outside.
Both Tegra and OMAP4 must add an offset to the HW irq number to deal
with that today.
but I'm assuming that this offset calculation should be done thanks to a
dedicated irq domain for the SPI.
The real HW physical number start at 0, and thus this is that value that
should be in the irq binding of the device.
So ideally we should have a irq domain for the PPI starting at 0 and
another one for the SPI starting at 32. Or 32 and 64 for the exynos4
case, but it looks like the PPI/SPI offset is always 32.
That offset of SPIs is always there. If you have a GIC as a secondary
controller, It will have 32 reserved interrupts and the register layout
is exactly the same as a cpu's GIC.
Yep, but that's the GIC view and not the SoC one. My concern is to have
to tweak the HW number provided by the HW spec in order to add that offset.
If you look at SoC level, the MPUSS is just an IP that can be
potentially replaced by other one that will not have a GIC. In that case
you will not change the IRQ mapping at SoC level.
For example if you replace the Dual-cortexA9 by a single CortexA8, then
all the interrupts will have to be shifted by 32 just because the MPU
subsystem is different.
Is that a realistic case? That would be a new chip and new device tree.
You could argue that the whole peripheral subsystem DT could be reused
and the numbering needs to be the same. However, there's one thing that
would prevent that. The number of interrupt cells is defined by the
controller binding. So you have to change the peripheral nodes anyway.
It's good that OMAP is trying to standardize the peripheral layout, but
in my experience that's not something you can rely on.
At some point the interrupt numbering is going to differ from the h/w
documentation. If it's not in the DT, then it will be in linux. Right
now its just offset of 32, but if irqdescs get assigned on demand as PPC
is doing, then there will be no relationship to
Re: [PATCH 5/5] ARM: gic: add OF based initialization
On Wed, Sep 14, 2011 at 01:51:42PM -0500, Rob Herring wrote: On 09/14/2011 01:34 PM, Marc Zyngier wrote: Hi Rob, On 14/09/11 18:57, Rob Herring wrote: Marc, On 09/14/2011 12:46 PM, Marc Zyngier wrote: On 14/09/11 17:31, Rob Herring wrote: From: Rob Herring rob.herr...@calxeda.com This adds gic initialization using device tree data. The initialization functions are intended to be called by a generic OF interrupt controller parsing function once the right pieces are in place. PPIs are handled using 3rd cell of interrupts properties to specify the cpu mask the PPI is assigned to. Signed-off-by: Rob Herring rob.herr...@calxeda.com --- Documentation/devicetree/bindings/arm/gic.txt | 53 arch/arm/common/gic.c | 55 +++-- arch/arm/include/asm/hardware/gic.h | 10 + 3 files changed, 114 insertions(+), 4 deletions(-) create mode 100644 Documentation/devicetree/bindings/arm/gic.txt diff --git a/Documentation/devicetree/bindings/arm/gic.txt b/Documentation/devicetree/bindings/arm/gic.txt new file mode 100644 index 000..6c513de --- /dev/null +++ b/Documentation/devicetree/bindings/arm/gic.txt @@ -0,0 +1,53 @@ +* ARM Generic Interrupt Controller + +ARM SMP cores are often associated with a GIC, providing per processor +interrupts (PPI), shared processor interrupts (SPI) and software +generated interrupts (SGI). + +Primary GIC is attached directly to the CPU and typically has PPIs and SGIs. +Secondary GICs are cascaded into the upward interrupt controller and do not +have PPIs or SGIs. + +Main node required properties: + +- compatible : should be one of: +arm,cortex-a9-gic +arm,arm11mp-gic +- interrupt-controller : Identifies the node as an interrupt controller +- #interrupt-cells : Specifies the number of cells needed to encode an + interrupt source. The type shall be a u32 and the value shall be 3. + + The 1st cell is the interrupt number. 0-15 are reserved for SGIs. 16-31 are + for PPIs. + + The 2nd cell is the level-sense information, encoded as follows: +1 = low-to-high edge triggered +2 = high-to-low edge triggered +4 = active high level-sensitive +8 = active low level-sensitive + + Only values of 1 and 4 are valid for GIC 1.0 spec. + + The 3rd cell contains the mask of the cpu number for the interrupt source. + The cpu mask is only valid for PPIs and shall be 0 for SPIs. This value shall + be 0 for PPIs. ^ Typo here ? The way I understand it, it should read For PPIs, this value shall be the mask of the possible CPU numbers for the interrupt source (or something to similar effect...). Cut and paste error. This sentence goes in the previous paragraph. What I meant is the 2nd cell should contain 0 for PPIs as you cannot set the edge/level on PPIs (that is always true, right?). I probably should also add 0 in the list of values. Ah, right. It makes sense indeed. You're correct about PPIs polarity, this is defined by the hardware and cannot be configured. But it may be interesting to have the DT to reflect the way the hardware is actually configured (on the Cortex-A9, some PPIs are configured active-low, and others are rising-edge). So we should allow specifying what it is as the OS may need to know that. If it is a difference between level edge, then the OS absolutely needs to know about it. ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH 5/5] ARM: gic: add OF based initialization
Hi Rob,
On 15 September 2011 18:24, Rob Herring robherri...@gmail.com wrote:
On 09/15/2011 02:55 AM, Thomas Abraham wrote:
Hi Rob,
On 14 September 2011 22:01, Rob Herring robherri...@gmail.com wrote:
From: Rob Herring rob.herr...@calxeda.com
This adds gic initialization using device tree data. The initialization
functions are intended to be called by a generic OF interrupt
controller parsing function once the right pieces are in place.
PPIs are handled using 3rd cell of interrupts properties to specify the cpu
mask the PPI is assigned to.
Signed-off-by: Rob Herring rob.herr...@calxeda.com
---
Documentation/devicetree/bindings/arm/gic.txt | 53
arch/arm/common/gic.c | 55
+++--
arch/arm/include/asm/hardware/gic.h | 10 +
3 files changed, 114 insertions(+), 4 deletions(-)
create mode 100644 Documentation/devicetree/bindings/arm/gic.txt
[...]
diff --git a/arch/arm/common/gic.c b/arch/arm/common/gic.c
index d1ccc72..14de380 100644
--- a/arch/arm/common/gic.c
+++ b/arch/arm/common/gic.c
[...]
+void __init gic_of_init(struct device_node *node, struct device_node
*parent)
+{
+ void __iomem *cpu_base;
+ void __iomem *dist_base;
+ int irq;
+ struct irq_domain *domain = gic_data[gic_cnt].domain;
+
+ if (WARN_ON(!node))
+ return;
+
+ dist_base = of_iomap(node, 0);
+ WARN(!dist_base, unable to map gic dist registers\n);
+
+ cpu_base = of_iomap(node, 1);
+ WARN(!cpu_base, unable to map gic cpu registers\n);
+
+ domain-nr_irq = gic_irq_count(dist_base);
+ domain-irq_base = irq_alloc_descs(-1, 0, domain-nr_irq,
numa_node_id());
For exynos4, all the interrupts originating from GIC are statically
mapped to start from 32 in the linux virq space (GIC SPI interrupts
start from 64). In the above code, since irq_base would be 0 for
exynos4, the interrupt mapping is not working correctly. In your
previous version of the patch, you have given a option to the platform
code to choose the offset. Could that option be added to this series
also. Or a provision to use platform specific translate function
instead of the irq_domain_simple translator.
So I guess you have the A9 external nIRQ hooked up to another
controller? Why can't the 0-31 interrupts get mapped to after the gic
interrupts? Ultimately we want h/w irq numbers completely decoupled from
linux irq numbers. So you will want to put that controller in devicetree
and have an DT init function for it as well.
There are chained interrupt handlers mapped in between linux irq
number 0 to 31. So the offset for GIC interrupts was set to 32 (SGI[0]
= 32). The interrupt chaining for the interrupts mapped between 0 to
31 seems unnecessary though. I will try removing them and check.
In anycase, there's a simple solution. You just need a call to
irq_alloc_descs to reserve the first 32 interrupts before calling
of_irq_init.
Rob
Thanks for your comments.
Regards,
Thomas.
___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH 5/5] ARM: gic: add OF based initialization
On Thu, Sep 15, 2011 at 12:07:25PM +0200, Cousson, Benoit wrote:
Hi Rob,
On 9/15/2011 9:55 AM, Thomas Abraham wrote:
Hi Rob,
On 14 September 2011 22:01, Rob Herringrobherri...@gmail.com wrote:
From: Rob Herringrob.herr...@calxeda.com
This adds gic initialization using device tree data. The initialization
functions are intended to be called by a generic OF interrupt
controller parsing function once the right pieces are in place.
PPIs are handled using 3rd cell of interrupts properties to specify the cpu
mask the PPI is assigned to.
Signed-off-by: Rob Herringrob.herr...@calxeda.com
---
Documentation/devicetree/bindings/arm/gic.txt | 53
arch/arm/common/gic.c | 55
+++--
arch/arm/include/asm/hardware/gic.h | 10 +
3 files changed, 114 insertions(+), 4 deletions(-)
create mode 100644 Documentation/devicetree/bindings/arm/gic.txt
[...]
diff --git a/arch/arm/common/gic.c b/arch/arm/common/gic.c
index d1ccc72..14de380 100644
--- a/arch/arm/common/gic.c
+++ b/arch/arm/common/gic.c
[...]
+void __init gic_of_init(struct device_node *node, struct device_node
*parent)
+{
+ void __iomem *cpu_base;
+ void __iomem *dist_base;
+ int irq;
+ struct irq_domain *domain =gic_data[gic_cnt].domain;
+
+ if (WARN_ON(!node))
+ return;
+
+ dist_base = of_iomap(node, 0);
+ WARN(!dist_base, unable to map gic dist registers\n);
+
+ cpu_base = of_iomap(node, 1);
+ WARN(!cpu_base, unable to map gic cpu registers\n);
+
+ domain-nr_irq = gic_irq_count(dist_base);
+ domain-irq_base = irq_alloc_descs(-1, 0, domain-nr_irq,
numa_node_id());
For exynos4, all the interrupts originating from GIC are statically
mapped to start from 32 in the linux virq space (GIC SPI interrupts
start from 64). In the above code, since irq_base would be 0 for
exynos4, the interrupt mapping is not working correctly. In your
previous version of the patch, you have given a option to the platform
code to choose the offset. Could that option be added to this series
also. Or a provision to use platform specific translate function
instead of the irq_domain_simple translator.
I have another concern on a similar topic.
On OMAP4 the SoC interrupts external to the MPU (SPI) have an offset
of 32. Only the internal PPI are between 0 and 31.
On a similar theme, on Versatile express the motherboard and core-tiles
are independent things, and the GIC lives on the core-tile (the motherboard
has no interrupt controller of its owh).
This means that the mapping of motherboard peripheral interrupts onto
GIC inputs is dependent on the coretile.
Since the DT is supposed to be descrbing the hardware in a componentised
way, the motherboard description should be independent of the core-tile
description except for describing the interface between the two.
For now, we express the mapping by putting an interrupt-map in the
core-tile DT, but this feels inelegant as well as wasteful -- expressing
+ 32 using a table which is about 1K in size and duplicates that
information 43 times.
Using a dedicated irq domain or a fake interrupt controller node to
encapsulate the motherboard interrupts feels like a cleaner approach,
but for now I'm not clear on the best way to do it.
Cheers
---Dave
___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH 5/5] ARM: gic: add OF based initialization
Hi Rob,
On 9/15/2011 9:55 AM, Thomas Abraham wrote:
Hi Rob,
On 14 September 2011 22:01, Rob Herringrobherri...@gmail.com wrote:
From: Rob Herringrob.herr...@calxeda.com
This adds gic initialization using device tree data. The initialization
functions are intended to be called by a generic OF interrupt
controller parsing function once the right pieces are in place.
PPIs are handled using 3rd cell of interrupts properties to specify the cpu
mask the PPI is assigned to.
Signed-off-by: Rob Herringrob.herr...@calxeda.com
---
Documentation/devicetree/bindings/arm/gic.txt | 53
arch/arm/common/gic.c | 55 +++--
arch/arm/include/asm/hardware/gic.h | 10 +
3 files changed, 114 insertions(+), 4 deletions(-)
create mode 100644 Documentation/devicetree/bindings/arm/gic.txt
[...]
diff --git a/arch/arm/common/gic.c b/arch/arm/common/gic.c
index d1ccc72..14de380 100644
--- a/arch/arm/common/gic.c
+++ b/arch/arm/common/gic.c
[...]
+void __init gic_of_init(struct device_node *node, struct device_node *parent)
+{
+ void __iomem *cpu_base;
+ void __iomem *dist_base;
+ int irq;
+ struct irq_domain *domain =gic_data[gic_cnt].domain;
+
+ if (WARN_ON(!node))
+ return;
+
+ dist_base = of_iomap(node, 0);
+ WARN(!dist_base, unable to map gic dist registers\n);
+
+ cpu_base = of_iomap(node, 1);
+ WARN(!cpu_base, unable to map gic cpu registers\n);
+
+ domain-nr_irq = gic_irq_count(dist_base);
+ domain-irq_base = irq_alloc_descs(-1, 0, domain-nr_irq,
numa_node_id());
For exynos4, all the interrupts originating from GIC are statically
mapped to start from 32 in the linux virq space (GIC SPI interrupts
start from 64). In the above code, since irq_base would be 0 for
exynos4, the interrupt mapping is not working correctly. In your
previous version of the patch, you have given a option to the platform
code to choose the offset. Could that option be added to this series
also. Or a provision to use platform specific translate function
instead of the irq_domain_simple translator.
I have another concern on a similar topic.
On OMAP4 the SoC interrupts external to the MPU (SPI) have an offset of
32. Only the internal PPI are between 0 and 31.
For the moment we add 32 to every SoC interrupts in the irq.h define,
but I'm assuming that this offset calculation should be done thanks to a
dedicated irq domain for the SPI.
The real HW physical number start at 0, and thus this is that value that
should be in the irq binding of the device.
So ideally we should have a irq domain for the PPI starting at 0 and
another one for the SPI starting at 32. Or 32 and 64 for the exynos4
case, but it looks like the PPI/SPI offset is always 32.
Regards,
Benoit
___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH 5/5] ARM: gic: add OF based initialization
On Thu, Sep 15, 2011 at 12:07:25PM +0200, Cousson, Benoit wrote: On OMAP4 the SoC interrupts external to the MPU (SPI) have an offset of 32. Only the internal PPI are between 0 and 31. SGIs are 0 to 15, PPIs are 16 to 31, and SPIs are 32+ - that's the numbering given to us by the GIC. The real HW physical number start at 0, and thus this is that value that should be in the irq binding of the device. That depends whether you're counting SPI number or whether you're counting IRQ number in the GIC interfaces. SPI0 will be reported to us from the GIC as 32, not 0, so to start numbering from 0 (which is already frowned upon for many reasons) we'd have to subtract 32 after checking that the IRQ is not a SGI nor PPI in the assembly code instead. ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH 5/5] ARM: gic: add OF based initialization
On Wednesday 14 September 2011, Rob Herring wrote: From: Rob Herring rob.herr...@calxeda.com This adds gic initialization using device tree data. The initialization functions are intended to be called by a generic OF interrupt controller parsing function once the right pieces are in place. PPIs are handled using 3rd cell of interrupts properties to specify the cpu mask the PPI is assigned to. Signed-off-by: Rob Herring rob.herr...@calxeda.com Acked-by: Arnd Bergmann a...@arndb.de ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH 5/5] ARM: gic: add OF based initialization
On 9/15/2011 12:29 PM, Russell King - ARM Linux wrote: On Thu, Sep 15, 2011 at 12:07:25PM +0200, Cousson, Benoit wrote: On OMAP4 the SoC interrupts external to the MPU (SPI) have an offset of 32. Only the internal PPI are between 0 and 31. SGIs are 0 to 15, PPIs are 16 to 31, and SPIs are 32+ - that's the numbering given to us by the GIC. The real HW physical number start at 0, and thus this is that value that should be in the irq binding of the device. That depends whether you're counting SPI number or whether you're counting IRQ number in the GIC interfaces. SPI0 will be reported to us from the GIC as 32, not 0, so to start numbering from 0 (which is already frowned upon for many reasons) we'd have to subtract 32 after checking that the IRQ is not a SGI nor PPI in the assembly code instead. The HW specs is obviously counting the IRQ number at the GIC interface. That offset is not known outside the MPUSS. Please have a look at the OMAP4430 TRM p4761 (NDA vM version). FWIW, the same numbering scheme is used on tegra2. My proposal is just to handle the addition within the irq_domain_to_irq, so I'm not sure to understand your concern. Regards, Benoit ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH 5/5] ARM: gic: add OF based initialization
On Thu, Sep 15, 2011 at 02:28:06PM +0200, Cousson, Benoit wrote: The HW specs is obviously counting the IRQ number at the GIC interface. That offset is not known outside the MPUSS. Please have a look at the OMAP4430 TRM p4761 (NDA vM version). As far as I know, I have no access to that. I've certainly never been pointed to any documentation on OMAP4. ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH 5/5] ARM: gic: add OF based initialization
On 09/15/2011 02:55 AM, Thomas Abraham wrote:
Hi Rob,
On 14 September 2011 22:01, Rob Herring robherri...@gmail.com wrote:
From: Rob Herring rob.herr...@calxeda.com
This adds gic initialization using device tree data. The initialization
functions are intended to be called by a generic OF interrupt
controller parsing function once the right pieces are in place.
PPIs are handled using 3rd cell of interrupts properties to specify the cpu
mask the PPI is assigned to.
Signed-off-by: Rob Herring rob.herr...@calxeda.com
---
Documentation/devicetree/bindings/arm/gic.txt | 53
arch/arm/common/gic.c | 55
+++--
arch/arm/include/asm/hardware/gic.h | 10 +
3 files changed, 114 insertions(+), 4 deletions(-)
create mode 100644 Documentation/devicetree/bindings/arm/gic.txt
[...]
diff --git a/arch/arm/common/gic.c b/arch/arm/common/gic.c
index d1ccc72..14de380 100644
--- a/arch/arm/common/gic.c
+++ b/arch/arm/common/gic.c
[...]
+void __init gic_of_init(struct device_node *node, struct device_node
*parent)
+{
+ void __iomem *cpu_base;
+ void __iomem *dist_base;
+ int irq;
+ struct irq_domain *domain = gic_data[gic_cnt].domain;
+
+ if (WARN_ON(!node))
+ return;
+
+ dist_base = of_iomap(node, 0);
+ WARN(!dist_base, unable to map gic dist registers\n);
+
+ cpu_base = of_iomap(node, 1);
+ WARN(!cpu_base, unable to map gic cpu registers\n);
+
+ domain-nr_irq = gic_irq_count(dist_base);
+ domain-irq_base = irq_alloc_descs(-1, 0, domain-nr_irq,
numa_node_id());
For exynos4, all the interrupts originating from GIC are statically
mapped to start from 32 in the linux virq space (GIC SPI interrupts
start from 64). In the above code, since irq_base would be 0 for
exynos4, the interrupt mapping is not working correctly. In your
previous version of the patch, you have given a option to the platform
code to choose the offset. Could that option be added to this series
also. Or a provision to use platform specific translate function
instead of the irq_domain_simple translator.
So I guess you have the A9 external nIRQ hooked up to another
controller? Why can't the 0-31 interrupts get mapped to after the gic
interrupts? Ultimately we want h/w irq numbers completely decoupled from
linux irq numbers. So you will want to put that controller in devicetree
and have an DT init function for it as well.
In anycase, there's a simple solution. You just need a call to
irq_alloc_descs to reserve the first 32 interrupts before calling
of_irq_init.
Rob
___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH 5/5] ARM: gic: add OF based initialization
On 9/15/2011 2:51 PM, Russell King - ARM Linux wrote: On Thu, Sep 15, 2011 at 02:28:06PM +0200, Cousson, Benoit wrote: The HW specs is obviously counting the IRQ number at the GIC interface. That offset is not known outside the MPUSS. Please have a look at the OMAP4430 TRM p4761 (NDA vM version). As far as I know, I have no access to that. I've certainly never been pointed to any documentation on OMAP4. That's easy to fix since the same information is in the public TRM (p3386 in that case). And here is the link: http://focus.ti.com/pdfs/wtbu/OMAP4430_ES2.x_PUBLIC_TRM_vX.zip Do not hesitate to ask if you never further information on OMAP SoC. Regards, Benoit ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH 5/5] ARM: gic: add OF based initialization
Benoit,
On 09/15/2011 05:07 AM, Cousson, Benoit wrote:
Hi Rob,
On 9/15/2011 9:55 AM, Thomas Abraham wrote:
Hi Rob,
On 14 September 2011 22:01, Rob Herringrobherri...@gmail.com wrote:
From: Rob Herringrob.herr...@calxeda.com
This adds gic initialization using device tree data. The initialization
functions are intended to be called by a generic OF interrupt
controller parsing function once the right pieces are in place.
PPIs are handled using 3rd cell of interrupts properties to specify
the cpu
mask the PPI is assigned to.
Signed-off-by: Rob Herringrob.herr...@calxeda.com
---
Documentation/devicetree/bindings/arm/gic.txt | 53
arch/arm/common/gic.c | 55
+++--
arch/arm/include/asm/hardware/gic.h | 10 +
3 files changed, 114 insertions(+), 4 deletions(-)
create mode 100644 Documentation/devicetree/bindings/arm/gic.txt
[...]
diff --git a/arch/arm/common/gic.c b/arch/arm/common/gic.c
index d1ccc72..14de380 100644
--- a/arch/arm/common/gic.c
+++ b/arch/arm/common/gic.c
[...]
+void __init gic_of_init(struct device_node *node, struct device_node
*parent)
+{
+ void __iomem *cpu_base;
+ void __iomem *dist_base;
+ int irq;
+ struct irq_domain *domain =gic_data[gic_cnt].domain;
+
+ if (WARN_ON(!node))
+ return;
+
+ dist_base = of_iomap(node, 0);
+ WARN(!dist_base, unable to map gic dist registers\n);
+
+ cpu_base = of_iomap(node, 1);
+ WARN(!cpu_base, unable to map gic cpu registers\n);
+
+ domain-nr_irq = gic_irq_count(dist_base);
+ domain-irq_base = irq_alloc_descs(-1, 0, domain-nr_irq,
numa_node_id());
For exynos4, all the interrupts originating from GIC are statically
mapped to start from 32 in the linux virq space (GIC SPI interrupts
start from 64). In the above code, since irq_base would be 0 for
exynos4, the interrupt mapping is not working correctly. In your
previous version of the patch, you have given a option to the platform
code to choose the offset. Could that option be added to this series
also. Or a provision to use platform specific translate function
instead of the irq_domain_simple translator.
I have another concern on a similar topic.
On OMAP4 the SoC interrupts external to the MPU (SPI) have an offset of
32. Only the internal PPI are between 0 and 31.
For the moment we add 32 to every SoC interrupts in the irq.h define,
Those defines will not be used in the DT case. So the question is
whether to add 32 or not in the DT. Since we have just a single node and
a linear mapping of PPIs and SPIs, the only choice is to have SPIs start
at 32. And from the h/w definition, SPIs always start at 32, so it's in
agreement.
but I'm assuming that this offset calculation should be done thanks to a
dedicated irq domain for the SPI.
The real HW physical number start at 0, and thus this is that value that
should be in the irq binding of the device.
So ideally we should have a irq domain for the PPI starting at 0 and
another one for the SPI starting at 32. Or 32 and 64 for the exynos4
case, but it looks like the PPI/SPI offset is always 32.
That offset of SPIs is always there. If you have a GIC as a secondary
controller, It will have 32 reserved interrupts and the register layout
is exactly the same as a cpu's GIC.
Since the idea of splitting PPIs for each core out to a flattened linux
irq map has been abandoned, I see no reason to have more than 1 domain
with a simple linear translation. Ultimately, domains will do dynamic
irqdesc allocation and the translation within the gic will be completely
dynamic.
The exynos4 case appears to be another controller hooked up in parallel
to the GIC. The GIC itself is exactly the same as other platforms AFAICT.
Rob
___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH 5/5] ARM: gic: add OF based initialization
On 9/15/2011 3:11 PM, Rob Herring wrote:
Benoit,
On 09/15/2011 05:07 AM, Cousson, Benoit wrote:
Hi Rob,
On 9/15/2011 9:55 AM, Thomas Abraham wrote:
Hi Rob,
On 14 September 2011 22:01, Rob Herringrobherri...@gmail.com wrote:
From: Rob Herringrob.herr...@calxeda.com
This adds gic initialization using device tree data. The initialization
functions are intended to be called by a generic OF interrupt
controller parsing function once the right pieces are in place.
PPIs are handled using 3rd cell of interrupts properties to specify
the cpu
mask the PPI is assigned to.
Signed-off-by: Rob Herringrob.herr...@calxeda.com
---
Documentation/devicetree/bindings/arm/gic.txt | 53
arch/arm/common/gic.c | 55
+++--
arch/arm/include/asm/hardware/gic.h | 10 +
3 files changed, 114 insertions(+), 4 deletions(-)
create mode 100644 Documentation/devicetree/bindings/arm/gic.txt
[...]
diff --git a/arch/arm/common/gic.c b/arch/arm/common/gic.c
index d1ccc72..14de380 100644
--- a/arch/arm/common/gic.c
+++ b/arch/arm/common/gic.c
[...]
+void __init gic_of_init(struct device_node *node, struct device_node
*parent)
+{
+ void __iomem *cpu_base;
+ void __iomem *dist_base;
+ int irq;
+ struct irq_domain *domain =gic_data[gic_cnt].domain;
+
+ if (WARN_ON(!node))
+ return;
+
+ dist_base = of_iomap(node, 0);
+ WARN(!dist_base, unable to map gic dist registers\n);
+
+ cpu_base = of_iomap(node, 1);
+ WARN(!cpu_base, unable to map gic cpu registers\n);
+
+ domain-nr_irq = gic_irq_count(dist_base);
+ domain-irq_base = irq_alloc_descs(-1, 0, domain-nr_irq,
numa_node_id());
For exynos4, all the interrupts originating from GIC are statically
mapped to start from 32 in the linux virq space (GIC SPI interrupts
start from 64). In the above code, since irq_base would be 0 for
exynos4, the interrupt mapping is not working correctly. In your
previous version of the patch, you have given a option to the platform
code to choose the offset. Could that option be added to this series
also. Or a provision to use platform specific translate function
instead of the irq_domain_simple translator.
I have another concern on a similar topic.
On OMAP4 the SoC interrupts external to the MPU (SPI) have an offset of
32. Only the internal PPI are between 0 and 31.
For the moment we add 32 to every SoC interrupts in the irq.h define,
Those defines will not be used in the DT case. So the question is
whether to add 32 or not in the DT. Since we have just a single node and
a linear mapping of PPIs and SPIs, the only choice is to have SPIs start
at 32. And from the h/w definition, SPIs always start at 32, so it's in
agreement.
This is a agreement inside the MPUSS, but not outside.
Both Tegra and OMAP4 must add an offset to the HW irq number to deal
with that today.
but I'm assuming that this offset calculation should be done thanks to a
dedicated irq domain for the SPI.
The real HW physical number start at 0, and thus this is that value that
should be in the irq binding of the device.
So ideally we should have a irq domain for the PPI starting at 0 and
another one for the SPI starting at 32. Or 32 and 64 for the exynos4
case, but it looks like the PPI/SPI offset is always 32.
That offset of SPIs is always there. If you have a GIC as a secondary
controller, It will have 32 reserved interrupts and the register layout
is exactly the same as a cpu's GIC.
Yep, but that's the GIC view and not the SoC one. My concern is to have
to tweak the HW number provided by the HW spec in order to add that offset.
If you look at SoC level, the MPUSS is just an IP that can be
potentially replaced by other one that will not have a GIC. In that case
you will not change the IRQ mapping at SoC level.
For example if you replace the Dual-cortexA9 by a single CortexA8, then
all the interrupts will have to be shifted by 32 just because the MPU
subsystem is different.
Since that offset is dependent of the GIC internals and is not exposed
outside the MPUSS, it should not be visible by the SoC IPs. And the HW
spec is exposing exactly that.
Since the idea of splitting PPIs for each core out to a flattened linux
irq map has been abandoned, I see no reason to have more than 1 domain
with a simple linear translation. Ultimately, domains will do dynamic
irqdesc allocation and the translation within the gic will be completely
dynamic.
I think the only reason to do that is to separate internal MPU
interrupts with the external ones that should not have a clue about the GIC.
Regards,
Benoit
___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH 5/5] ARM: gic: add OF based initialization
Benoit,
On 09/15/2011 08:52 AM, Cousson, Benoit wrote:
On 9/15/2011 3:11 PM, Rob Herring wrote:
Benoit,
On 09/15/2011 05:07 AM, Cousson, Benoit wrote:
Hi Rob,
On 9/15/2011 9:55 AM, Thomas Abraham wrote:
Hi Rob,
On 14 September 2011 22:01, Rob Herringrobherri...@gmail.com wrote:
From: Rob Herringrob.herr...@calxeda.com
This adds gic initialization using device tree data. The
initialization
functions are intended to be called by a generic OF interrupt
controller parsing function once the right pieces are in place.
PPIs are handled using 3rd cell of interrupts properties to specify
the cpu
mask the PPI is assigned to.
Signed-off-by: Rob Herringrob.herr...@calxeda.com
---
Documentation/devicetree/bindings/arm/gic.txt | 53
arch/arm/common/gic.c | 55
+++--
arch/arm/include/asm/hardware/gic.h | 10 +
3 files changed, 114 insertions(+), 4 deletions(-)
create mode 100644 Documentation/devicetree/bindings/arm/gic.txt
[...]
diff --git a/arch/arm/common/gic.c b/arch/arm/common/gic.c
index d1ccc72..14de380 100644
--- a/arch/arm/common/gic.c
+++ b/arch/arm/common/gic.c
[...]
+void __init gic_of_init(struct device_node *node, struct device_node
*parent)
+{
+ void __iomem *cpu_base;
+ void __iomem *dist_base;
+ int irq;
+ struct irq_domain *domain =gic_data[gic_cnt].domain;
+
+ if (WARN_ON(!node))
+ return;
+
+ dist_base = of_iomap(node, 0);
+ WARN(!dist_base, unable to map gic dist registers\n);
+
+ cpu_base = of_iomap(node, 1);
+ WARN(!cpu_base, unable to map gic cpu registers\n);
+
+ domain-nr_irq = gic_irq_count(dist_base);
+ domain-irq_base = irq_alloc_descs(-1, 0, domain-nr_irq,
numa_node_id());
For exynos4, all the interrupts originating from GIC are statically
mapped to start from 32 in the linux virq space (GIC SPI interrupts
start from 64). In the above code, since irq_base would be 0 for
exynos4, the interrupt mapping is not working correctly. In your
previous version of the patch, you have given a option to the platform
code to choose the offset. Could that option be added to this series
also. Or a provision to use platform specific translate function
instead of the irq_domain_simple translator.
I have another concern on a similar topic.
On OMAP4 the SoC interrupts external to the MPU (SPI) have an offset of
32. Only the internal PPI are between 0 and 31.
For the moment we add 32 to every SoC interrupts in the irq.h define,
Those defines will not be used in the DT case. So the question is
whether to add 32 or not in the DT. Since we have just a single node and
a linear mapping of PPIs and SPIs, the only choice is to have SPIs start
at 32. And from the h/w definition, SPIs always start at 32, so it's in
agreement.
This is a agreement inside the MPUSS, but not outside.
Both Tegra and OMAP4 must add an offset to the HW irq number to deal
with that today.
but I'm assuming that this offset calculation should be done thanks to a
dedicated irq domain for the SPI.
The real HW physical number start at 0, and thus this is that value that
should be in the irq binding of the device.
So ideally we should have a irq domain for the PPI starting at 0 and
another one for the SPI starting at 32. Or 32 and 64 for the exynos4
case, but it looks like the PPI/SPI offset is always 32.
That offset of SPIs is always there. If you have a GIC as a secondary
controller, It will have 32 reserved interrupts and the register layout
is exactly the same as a cpu's GIC.
Yep, but that's the GIC view and not the SoC one. My concern is to have
to tweak the HW number provided by the HW spec in order to add that offset.
If you look at SoC level, the MPUSS is just an IP that can be
potentially replaced by other one that will not have a GIC. In that case
you will not change the IRQ mapping at SoC level.
For example if you replace the Dual-cortexA9 by a single CortexA8, then
all the interrupts will have to be shifted by 32 just because the MPU
subsystem is different.
Is that a realistic case? That would be a new chip and new device tree.
You could argue that the whole peripheral subsystem DT could be reused
and the numbering needs to be the same. However, there's one thing that
would prevent that. The number of interrupt cells is defined by the
controller binding. So you have to change the peripheral nodes anyway.
It's good that OMAP is trying to standardize the peripheral layout, but
in my experience that's not something you can rely on.
At some point the interrupt numbering is going to differ from the h/w
documentation. If it's not in the DT, then it will be in linux. Right
now its just offset of 32, but if irqdescs get assigned on demand as PPC
is doing, then there will be no relationship to the documentation.
Since that offset is dependent of the
Re: [PATCH 5/5] ARM: gic: add OF based initialization
Marc, On 09/14/2011 12:46 PM, Marc Zyngier wrote: On 14/09/11 17:31, Rob Herring wrote: From: Rob Herring rob.herr...@calxeda.com This adds gic initialization using device tree data. The initialization functions are intended to be called by a generic OF interrupt controller parsing function once the right pieces are in place. PPIs are handled using 3rd cell of interrupts properties to specify the cpu mask the PPI is assigned to. Signed-off-by: Rob Herring rob.herr...@calxeda.com --- Documentation/devicetree/bindings/arm/gic.txt | 53 arch/arm/common/gic.c | 55 +++-- arch/arm/include/asm/hardware/gic.h | 10 + 3 files changed, 114 insertions(+), 4 deletions(-) create mode 100644 Documentation/devicetree/bindings/arm/gic.txt diff --git a/Documentation/devicetree/bindings/arm/gic.txt b/Documentation/devicetree/bindings/arm/gic.txt new file mode 100644 index 000..6c513de --- /dev/null +++ b/Documentation/devicetree/bindings/arm/gic.txt @@ -0,0 +1,53 @@ +* ARM Generic Interrupt Controller + +ARM SMP cores are often associated with a GIC, providing per processor +interrupts (PPI), shared processor interrupts (SPI) and software +generated interrupts (SGI). + +Primary GIC is attached directly to the CPU and typically has PPIs and SGIs. +Secondary GICs are cascaded into the upward interrupt controller and do not +have PPIs or SGIs. + +Main node required properties: + +- compatible : should be one of: +arm,cortex-a9-gic +arm,arm11mp-gic +- interrupt-controller : Identifies the node as an interrupt controller +- #interrupt-cells : Specifies the number of cells needed to encode an + interrupt source. The type shall be a u32 and the value shall be 3. + + The 1st cell is the interrupt number. 0-15 are reserved for SGIs. 16-31 are + for PPIs. + + The 2nd cell is the level-sense information, encoded as follows: +1 = low-to-high edge triggered +2 = high-to-low edge triggered +4 = active high level-sensitive +8 = active low level-sensitive + + Only values of 1 and 4 are valid for GIC 1.0 spec. + + The 3rd cell contains the mask of the cpu number for the interrupt source. + The cpu mask is only valid for PPIs and shall be 0 for SPIs. This value shall + be 0 for PPIs. ^ Typo here ? The way I understand it, it should read For PPIs, this value shall be the mask of the possible CPU numbers for the interrupt source (or something to similar effect...). Cut and paste error. This sentence goes in the previous paragraph. What I meant is the 2nd cell should contain 0 for PPIs as you cannot set the edge/level on PPIs (that is always true, right?). I probably should also add 0 in the list of values. I take it you are otherwise fine with this binding? Rob ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH 5/5] ARM: gic: add OF based initialization
On 14/09/11 17:31, Rob Herring wrote: From: Rob Herring rob.herr...@calxeda.com This adds gic initialization using device tree data. The initialization functions are intended to be called by a generic OF interrupt controller parsing function once the right pieces are in place. PPIs are handled using 3rd cell of interrupts properties to specify the cpu mask the PPI is assigned to. Signed-off-by: Rob Herring rob.herr...@calxeda.com --- Documentation/devicetree/bindings/arm/gic.txt | 53 arch/arm/common/gic.c | 55 +++-- arch/arm/include/asm/hardware/gic.h | 10 + 3 files changed, 114 insertions(+), 4 deletions(-) create mode 100644 Documentation/devicetree/bindings/arm/gic.txt diff --git a/Documentation/devicetree/bindings/arm/gic.txt b/Documentation/devicetree/bindings/arm/gic.txt new file mode 100644 index 000..6c513de --- /dev/null +++ b/Documentation/devicetree/bindings/arm/gic.txt @@ -0,0 +1,53 @@ +* ARM Generic Interrupt Controller + +ARM SMP cores are often associated with a GIC, providing per processor +interrupts (PPI), shared processor interrupts (SPI) and software +generated interrupts (SGI). + +Primary GIC is attached directly to the CPU and typically has PPIs and SGIs. +Secondary GICs are cascaded into the upward interrupt controller and do not +have PPIs or SGIs. + +Main node required properties: + +- compatible : should be one of: + arm,cortex-a9-gic + arm,arm11mp-gic +- interrupt-controller : Identifies the node as an interrupt controller +- #interrupt-cells : Specifies the number of cells needed to encode an + interrupt source. The type shall be a u32 and the value shall be 3. + + The 1st cell is the interrupt number. 0-15 are reserved for SGIs. 16-31 are + for PPIs. + + The 2nd cell is the level-sense information, encoded as follows: +1 = low-to-high edge triggered +2 = high-to-low edge triggered +4 = active high level-sensitive +8 = active low level-sensitive + + Only values of 1 and 4 are valid for GIC 1.0 spec. + + The 3rd cell contains the mask of the cpu number for the interrupt source. + The cpu mask is only valid for PPIs and shall be 0 for SPIs. This value shall + be 0 for PPIs. ^ Typo here ? The way I understand it, it should read For PPIs, this value shall be the mask of the possible CPU numbers for the interrupt source (or something to similar effect...). M. -- Jazz is not dead. It just smells funny... ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH 5/5] ARM: gic: add OF based initialization
Hi Rob,
On 14/09/11 18:57, Rob Herring wrote:
Marc,
On 09/14/2011 12:46 PM, Marc Zyngier wrote:
On 14/09/11 17:31, Rob Herring wrote:
From: Rob Herring rob.herr...@calxeda.com
This adds gic initialization using device tree data. The initialization
functions are intended to be called by a generic OF interrupt
controller parsing function once the right pieces are in place.
PPIs are handled using 3rd cell of interrupts properties to specify the cpu
mask the PPI is assigned to.
Signed-off-by: Rob Herring rob.herr...@calxeda.com
---
Documentation/devicetree/bindings/arm/gic.txt | 53
arch/arm/common/gic.c | 55
+++--
arch/arm/include/asm/hardware/gic.h | 10 +
3 files changed, 114 insertions(+), 4 deletions(-)
create mode 100644 Documentation/devicetree/bindings/arm/gic.txt
diff --git a/Documentation/devicetree/bindings/arm/gic.txt
b/Documentation/devicetree/bindings/arm/gic.txt
new file mode 100644
index 000..6c513de
--- /dev/null
+++ b/Documentation/devicetree/bindings/arm/gic.txt
@@ -0,0 +1,53 @@
+* ARM Generic Interrupt Controller
+
+ARM SMP cores are often associated with a GIC, providing per processor
+interrupts (PPI), shared processor interrupts (SPI) and software
+generated interrupts (SGI).
+
+Primary GIC is attached directly to the CPU and typically has PPIs and
SGIs.
+Secondary GICs are cascaded into the upward interrupt controller and do not
+have PPIs or SGIs.
+
+Main node required properties:
+
+- compatible : should be one of:
+ arm,cortex-a9-gic
+ arm,arm11mp-gic
+- interrupt-controller : Identifies the node as an interrupt controller
+- #interrupt-cells : Specifies the number of cells needed to encode an
+ interrupt source. The type shall be a u32 and the value shall be 3.
+
+ The 1st cell is the interrupt number. 0-15 are reserved for SGIs. 16-31
are
+ for PPIs.
+
+ The 2nd cell is the level-sense information, encoded as follows:
+1 = low-to-high edge triggered
+2 = high-to-low edge triggered
+4 = active high level-sensitive
+8 = active low level-sensitive
+
+ Only values of 1 and 4 are valid for GIC 1.0 spec.
+
+ The 3rd cell contains the mask of the cpu number for the interrupt
source.
+ The cpu mask is only valid for PPIs and shall be 0 for SPIs. This value
shall
+ be 0 for PPIs.
^
Typo here ? The way I understand it, it should read For PPIs, this
value shall be the mask of the possible CPU numbers for the interrupt
source (or something to similar effect...).
Cut and paste error. This sentence goes in the previous paragraph. What
I meant is the 2nd cell should contain 0 for PPIs as you cannot set the
edge/level on PPIs (that is always true, right?). I probably should also
add 0 in the list of values.
Ah, right. It makes sense indeed. You're correct about PPIs polarity,
this is defined by the hardware and cannot be configured. But it may be
interesting to have the DT to reflect the way the hardware is actually
configured (on the Cortex-A9, some PPIs are configured active-low, and
others are rising-edge).
I take it you are otherwise fine with this binding?
I very much like the fact that it (or at least that's the way I
understand it...) allows for a very compact expression of peripherals
connected to PPIs.
What I'd like to write is the following:
twd@1f000600 {
compatible = arm,11mpcore-twd;
reg = 0x1f000600 0x100;
interrupt-parent = intc;
interrupt = 29 0 0xf;
}
where 0xf would indicate that the TWD is connected to all four cores. Is
that correct?
Thanks,
M.
--
Jazz is not dead. It just smells funny...
___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH 5/5] ARM: gic: add OF based initialization
On 09/14/2011 01:34 PM, Marc Zyngier wrote:
Hi Rob,
On 14/09/11 18:57, Rob Herring wrote:
Marc,
On 09/14/2011 12:46 PM, Marc Zyngier wrote:
On 14/09/11 17:31, Rob Herring wrote:
From: Rob Herring rob.herr...@calxeda.com
This adds gic initialization using device tree data. The initialization
functions are intended to be called by a generic OF interrupt
controller parsing function once the right pieces are in place.
PPIs are handled using 3rd cell of interrupts properties to specify the cpu
mask the PPI is assigned to.
Signed-off-by: Rob Herring rob.herr...@calxeda.com
---
Documentation/devicetree/bindings/arm/gic.txt | 53
arch/arm/common/gic.c | 55
+++--
arch/arm/include/asm/hardware/gic.h | 10 +
3 files changed, 114 insertions(+), 4 deletions(-)
create mode 100644 Documentation/devicetree/bindings/arm/gic.txt
diff --git a/Documentation/devicetree/bindings/arm/gic.txt
b/Documentation/devicetree/bindings/arm/gic.txt
new file mode 100644
index 000..6c513de
--- /dev/null
+++ b/Documentation/devicetree/bindings/arm/gic.txt
@@ -0,0 +1,53 @@
+* ARM Generic Interrupt Controller
+
+ARM SMP cores are often associated with a GIC, providing per processor
+interrupts (PPI), shared processor interrupts (SPI) and software
+generated interrupts (SGI).
+
+Primary GIC is attached directly to the CPU and typically has PPIs and
SGIs.
+Secondary GICs are cascaded into the upward interrupt controller and do
not
+have PPIs or SGIs.
+
+Main node required properties:
+
+- compatible : should be one of:
+ arm,cortex-a9-gic
+ arm,arm11mp-gic
+- interrupt-controller : Identifies the node as an interrupt controller
+- #interrupt-cells : Specifies the number of cells needed to encode an
+ interrupt source. The type shall be a u32 and the value shall be 3.
+
+ The 1st cell is the interrupt number. 0-15 are reserved for SGIs. 16-31
are
+ for PPIs.
+
+ The 2nd cell is the level-sense information, encoded as follows:
+1 = low-to-high edge triggered
+2 = high-to-low edge triggered
+4 = active high level-sensitive
+8 = active low level-sensitive
+
+ Only values of 1 and 4 are valid for GIC 1.0 spec.
+
+ The 3rd cell contains the mask of the cpu number for the interrupt
source.
+ The cpu mask is only valid for PPIs and shall be 0 for SPIs. This value
shall
+ be 0 for PPIs.
^
Typo here ? The way I understand it, it should read For PPIs, this
value shall be the mask of the possible CPU numbers for the interrupt
source (or something to similar effect...).
Cut and paste error. This sentence goes in the previous paragraph. What
I meant is the 2nd cell should contain 0 for PPIs as you cannot set the
edge/level on PPIs (that is always true, right?). I probably should also
add 0 in the list of values.
Ah, right. It makes sense indeed. You're correct about PPIs polarity,
this is defined by the hardware and cannot be configured. But it may be
interesting to have the DT to reflect the way the hardware is actually
configured (on the Cortex-A9, some PPIs are configured active-low, and
others are rising-edge).
So we should allow specifying what it is as the OS may need to know that.
I take it you are otherwise fine with this binding?
I very much like the fact that it (or at least that's the way I
understand it...) allows for a very compact expression of peripherals
connected to PPIs.
What I'd like to write is the following:
twd@1f000600 {
compatible = arm,11mpcore-twd;
reg = 0x1f000600 0x100;
interrupt-parent = intc;
interrupt = 29 0 0xf;
}
where 0xf would indicate that the TWD is connected to all four cores. Is
that correct?
Yes, that's exactly why I did a mask and is what I have for twd on
highbank. Also, I specified SPIs as 0 specifically so no one gets the
idea to use the mask to set the affinity.
Rob
___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss
