Hi Grant,
On 31 July 2011 04:46, Grant Likely <grant.likely@xxxxxxxxxxxx> wrote:
> On Wed, Jul 27, 2011 at 11:54:28PM +0530, Thomas Abraham wrote:
>> Exynos4 includes two interrupt controllers - External GIC and External
>> Interrupt Combiner. External GIC can handle 16 software generated
>> interrupts (SGI), 16 Private Peripheral Interrupts (PPI) and 128
>> Shared Peripheral Interrupts (SPI). External Interrupt Combiner manages
>> 32 groups of 8 interrupts each and feeds 32 interrupts as SPI interrupts
>> to the External GIC controller.
>>
>> This patch supports conversion of device tree interrupt specifier to
>> linux virq domain for both the interrupt controllers. The concept of
>> this patch is derived from Grant's 'simple' irq converter.
>>
>> This patch is based on Grant's following patchset
>> [PATCH v3 0/2] Simple irq_domain implementation
>>
>> Signed-off-by: Thomas Abraham <thomas.abraham@xxxxxxxxxx>
>> ---
>> Documentation/devicetree/bindings/arm/samsung.txt | 72 +++++++++++++
>> arch/arm/mach-exynos4/Makefile | 1 +
>> arch/arm/mach-exynos4/include/mach/irqs.h | 3 +
>> arch/arm/mach-exynos4/irqdomain.c | 117 +++++++++++++++++++++
>> arch/arm/mach-exynos4/mach-exynos4-dt.c | 8 +-
>> 5 files changed, 196 insertions(+), 5 deletions(-)
>> create mode 100644 arch/arm/mach-exynos4/irqdomain.c
>>
>> diff --git a/Documentation/devicetree/bindings/arm/samsung.txt b/Documentation/devicetree/bindings/arm/samsung.txt
[...]
>> +
>> +External GIC properties:
>> + - compatible: should be "samsung,exynos4-ext-gic".
>> + - interrupt-cells: should be <2>. The meaning of the cells are
>> + * First Cell: Interrupt Number.
>> + * Second Cell: Type of Interrupt (0-SPI, 1-SGI, 2-PPI).
>
> Type should probably be the first cell. That's how this has been
> handled in the past for other hardware. Also, the gic binding is
> going to be shared for a bunch of ARM hardware, not just Exynos. Can
> you split this patch into two parts; one for gic and one for the
> exynos combiner?
Ok. I will split this patch and change the order of the cells.
>
>> + - reg: The GIC includes a Distributor Interface and CPU Interface and
>> + hence requires two base addresses. The property format is
>> + <Distributor-Base Distributor-Size>, <CPU-Base CPU Size>
>> +
>> + Example:
>> +
>> + EXT_GIC:interrupt-controller@10490000 {
>> + compatible = "samsung,exynos4-ext-gic";
>> + #interrupt-cells = <2>;
>> + interrupt-controller;
>> + reg = <0x10490000 0x1000>, <0x10480000 0x100>;
>> + };
>> +
>> + Devices using External GIC as the interrupt parent should specify two
>> + cells for the interrupts property as shown below.
>> +
>> + watchdog@10060000 {
>> + compatible = "samsung,s3c2410-wdt";
>> + reg = <0x10060000 0x400>;
>> + interrupt-parent = <&EXT_GIC>;
>> + interrupts = <43 0>;
>> + };
>> +
>> +External Interrupt Combiner properties:
>> + - compatible: should be "samsung,exynos4-ext-combiner".
>> + - interrupt-cells: should be <2>. The meaning of the cells are
>> + * First Cell: Combiner Group Number.
>> + * Second Cell: Interrupt within the group.
>
> I was under the impression that the irq groupings are programmable,
> and that the combiner irq inputs are a flat numbering layout. Is that
> correct? If so, then #interrupt-cells should probably be 1, and the
> grouping should probably be configuration properties on the combiner
> node.
Each interrupt group in a combiner has fixed set of interrupts and is
not programmable. Each irq input to a interrupt group combiner has a
fixed number. So there is no programmable irq numbers supported by the
irq-combiner.
>
>> + - reg: Base address and size of interrupt combiner registers.
>> +
>> + Example:
>> +
>> + EXT_COMBINER:interrupt-controller@10440000 {
>> + compatible = "samsung,exynos4-ext-combiner";
>> + #interrupt-cells = <2>;
>> + interrupt-controller;
>> + reg = <0x10440000 0x200>;
>> + };
>> +
>> + Devices using External Interrupt Combiner as the interrupt parent should
>> + specify two cells for the interrupts property as shown below.
>> +
>> + watchdog@10060000 {
>> + compatible = "samsung,s3c2410-wdt";
>> + reg = <0x10060000 0x400>;
>> + interrupt-parent = <&EXT_COMBINER>;
>> + interrupts = <4 2>;
>> + };
[...]
>> +
>> +/*
>> + * The interrupt specifier for External GIC controller uses to two cells in
>> + * the device tree source file. The second cell denotes the type of the
>> + * interrupt (SPI/SGI/PPI). The following macros are used to represent
>> + * these different types of interrupt.
>> + */
>> +#define EXT_GIC_SPI 0
>> +#define EXT_GIC_SGI 1
>> +#define EXT_GIC_PPI 2
>
> Nit: I prefer enums over preprocessor macros, but not a big deal.
>
Ok. I will add a enum for the type of GIC interrupt.
Thanks for your comments on the patch.
Regards,
Thomas.
[...]
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
