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

[Santosh Shilimkar]

Colin,

On 4/3/2011 8:57 AM, Colin Cross wrote:
> 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:
>
OMAP4 interrupt controller extension doesn't use 'irq_ack' extension.
So it should be ok.

Will,
Have already tested your V3 vesrion while generating the GIC
IO_relaxed patch and it worked OK. This series seems to be same.
I shall try this on OMAP4 and let you know ?

> 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)