[PATCH 6/6] ARM: gic: use handle_fasteoi_irq for SPIs

[Colin Cross]

On Fri, Apr 1, 2011 at 1:31 PM, Colin Cross <ccross at google.com> wrote:
> On Fri, Apr 1, 2011 at 7:50 AM, Will Deacon <will.deacon at arm.com> wrote:
>> Currently, the gic uses handle_level_irq for handling SPIs (Shared
>> Peripheral Interrupts), requiring active interrupts to be masked at
>> the distributor level during IRQ handling.
>>
>> On a virtualised system, only the CPU interfaces are virtualised in
>> hardware. Accesses to the distributor must be trapped by the
>> hypervisor, adding latency to the critical interrupt path in Linux.
>>
>> This patch modifies the GIC code to use handle_fasteoi_irq for handling
>> interrupts, which only requires us to signal EOI to the CPU interface
>> when handling is complete. Cascaded IRQ handling is also updated to use
>> the chained IRQ enter/exit functions to honour the flow control of the
>> parent chip.
>>
>> Note that commit 846afbd1 ("GIC: Dont disable INT in ack callback")
>> broke cascading interrupts by forgetting to add IRQ masking. This is
>> no longer an issue because the unmask call is now unnecessary.
>>
>> Tested on Versatile Express and Realview EB (1176 w/ cascaded GICs).
>>
>> Cc: Abhijeet Dharmapurikar <adharmap at codeaurora.org>
>> Cc: Russell King - ARM Linux <linux at arm.linux.org.uk>
>> Acked-by: Catalin Marinas <catalin.marinas at arm.com>
>> Signed-off-by: Will Deacon <will.deacon at arm.com>
>> ---
>>  arch/arm/common/gic.c |   24 +++++++++---------------
>>  1 files changed, 9 insertions(+), 15 deletions(-)
>>
>> diff --git a/arch/arm/common/gic.c b/arch/arm/common/gic.c
>> index f70ec7d..f8719ec 100644
>> --- a/arch/arm/common/gic.c
>> +++ b/arch/arm/common/gic.c
>> @@ -84,15 +84,6 @@ static inline unsigned int gic_irq(struct irq_data *d)
>>  /*
>>  * Routines to acknowledge, disable and enable interrupts
>>  */
>> -static void gic_ack_irq(struct irq_data *d)
>> -{
>> -       spin_lock(&irq_controller_lock);
>> -       if (gic_arch_extn.irq_ack)
>> -               gic_arch_extn.irq_ack(d);
>> -       writel(gic_irq(d), gic_cpu_base(d) + GIC_CPU_EOI);
>> -       spin_unlock(&irq_controller_lock);
>> -}
>> -
>>  static void gic_mask_irq(struct irq_data *d)
>>  {
>>        u32 mask = 1 << (d->irq % 32);
>> @@ -115,6 +106,11 @@ static void gic_unmask_irq(struct irq_data *d)
>>        spin_unlock(&irq_controller_lock);
>>  }
>>
>> +static void gic_eoi_irq(struct irq_data *d)
>> +{
>> +       writel(gic_irq(d), gic_cpu_base(d) + GIC_CPU_EOI);
>> +}
>> +
>>  static int gic_set_type(struct irq_data *d, unsigned int type)
>>  {
>>        void __iomem *base = gic_dist_base(d);
>> @@ -218,8 +214,7 @@ static void gic_handle_cascade_irq(unsigned int irq, struct irq_desc *desc)
>>        unsigned int cascade_irq, gic_irq;
>>        unsigned long status;
>>
>> -       /* primary controller ack'ing */
>> -       chip->irq_ack(&desc->irq_data);
>> +       chained_irq_enter(chip, desc);
>>
>>        spin_lock(&irq_controller_lock);
>>        status = readl(chip_data->cpu_base + GIC_CPU_INTACK);
>> @@ -236,15 +231,14 @@ static void gic_handle_cascade_irq(unsigned int irq, struct irq_desc *desc)
>>                generic_handle_irq(cascade_irq);
>>
>>  out:
>> -       /* primary controller unmasking */
>> -       chip->irq_unmask(&desc->irq_data);
>> +       chained_irq_exit(chip, desc);
>>  }
>>
>>  static struct irq_chip gic_chip = {
>>        .name                   = "GIC",
>> -       .irq_ack                = gic_ack_irq,
>>        .irq_mask               = gic_mask_irq,
>>        .irq_unmask             = gic_unmask_irq,
>> +       .irq_eoi                = gic_eoi_irq,
>>        .irq_set_type           = gic_set_type,
>>        .irq_retrigger          = gic_retrigger,
>>  #ifdef CONFIG_SMP
>> @@ -319,7 +313,7 @@ static void __init gic_dist_init(struct gic_chip_data *gic,
>>         * Setup the Linux IRQ subsystem.
>>         */
>>        for (i = irq_start; i < irq_limit; i++) {
>> -               irq_set_chip_and_handler(i, &gic_chip, handle_level_irq);
>> +               irq_set_chip_and_handler(i, &gic_chip, handle_fasteoi_irq);
>>                irq_set_chip_data(i, gic);
>>                set_irq_flags(i, IRQF_VALID | IRQF_PROBE);
>>        }
>> --
>> 1.7.0.4
>>
>>
>
> Tested-by: Colin Cross <ccross at android.com>
> Works on tegra after fixing the problems with the tegra gpio patch.
>

In further testing I found one bug.  d7ed36a added gic_arch_extn,
which needs to be used in gic_eoi.   arch/arm/mach-tegra/irq.c will
need to be fixed to replace tegra_ack with tegra_eoi, and any other
platform that uses gic_arch_extn also needs to be checked (omap4?).
This patch fixes gic.c:

diff --git a/arch/arm/common/gic.c b/arch/arm/common/gic.c
index 6ecd5c7..8e46dac 100644
--- a/arch/arm/common/gic.c
+++ b/arch/arm/common/gic.c
@@ -118,7 +118,11 @@ static void gic_unmask_irq(struct irq_data *d)

 static void gic_eoi_irq(struct irq_data *d)
 {
+       spin_lock(&irq_controller_lock);
+       if (gic_arch_extn.irq_eoi)
+               gic_arch_extn.irq_eoi(d);
        writel(gic_irq(d), gic_cpu_base(d) + GIC_CPU_EOI);
+       spin_unlock(&irq_controller_lock);
 }

 static int gic_set_type(struct irq_data *d, unsigned int type)