[PATCH] arm: exynos4: Add support for dt irq specifier to linux virq conversion

[Thomas Abraham]

Hi Grant,

On 31 July 2011 04:46, Grant Likely <grant.likely at secretlab.ca> 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 at linaro.org>
>> ---
>>  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 at 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 at 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 at 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 at 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.

[...]