[PATCH 0/6] Use chained handler entry/exit functions in platform code

[PATCH 0/6] Use chained handler entry/exit functions in platform code

[Will Deacon]

Hello,

Commit 10a8c383 ("ARM: irq: introduce entry and exit functions for
chained handlers") added chained handler entry/exit functions in
preparation for moving the GIC to fasteoi flow control.

This series of patches updates the relevant platforms to use the new
functions so that when the GIC changes its flow control type, breakage
will not occur.

I've included the GIC patch as the final patch (6) in the series so you
can test the change easily enough on your favourite platform.

All Acked-bys/Tested-bys greatly appreciated!

Cheers,

Will


Will Deacon (6):
  ARM: omap: update GPIO chained IRQ handler to use entry/exit
    functions
  ARM: tegra: update GPIO chained IRQ handler to use entry/exit
    functions
  ARM: s5pv310: update IRQ combiner to use chained entry/exit functions
  ARM: msm: update GPIO chained IRQ handler to use entry/exit functions
  ARM: nmk: update GPIO chained IRQ handler to entry/exit functions
  ARM: gic: use handle_fasteoi_irq for SPIs

 arch/arm/common/gic.c                |   24 +++++++++---------------
 arch/arm/mach-exynos4/irq-combiner.c |    6 ++----
 arch/arm/mach-msm/gpio-v2.c          |    6 +++++-
 arch/arm/mach-tegra/gpio.c           |    7 ++++---
 arch/arm/plat-nomadik/gpio.c         |   10 ++--------
 arch/arm/plat-omap/gpio.c            |    7 ++++---
 6 files changed, 26 insertions(+), 34 deletions(-)

[PATCH 1/6] ARM: omap: update GPIO chained IRQ handler to use entry/exit functions

[Will Deacon]

This patch updates the OMAP gpio chained IRQ handler to use the chained
IRQ enter/exit functions in order to function correctly on primary
controllers with different methods of flow control.

Cc: Colin Cross <ccross@google.com>
Cc: Tony Lindgren <tony@atomide.com>
Cc: Santosh Shilimkar <santosh.shilimkar@ti.com>
Signed-off-by: Will Deacon <will.deacon@arm.com>
---
 arch/arm/plat-omap/gpio.c |    7 ++++---
 1 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/arch/arm/plat-omap/gpio.c b/arch/arm/plat-omap/gpio.c
index d2adcdd..8f5296a 100644
--- a/arch/arm/plat-omap/gpio.c
+++ b/arch/arm/plat-omap/gpio.c
@@ -1137,8 +1137,9 @@ static void gpio_irq_handler(unsigned int irq, struct irq_desc *desc)
 	struct gpio_bank *bank;
 	u32 retrigger = 0;
 	int unmasked = 0;
+	struct irq_chip *chip = get_irq_desc_chip(desc);
 
-	desc->irq_data.chip->irq_ack(&desc->irq_data);
+	chained_irq_enter(chip, desc);
 
 	bank = irq_get_handler_data(irq);
 #ifdef CONFIG_ARCH_OMAP1
@@ -1195,7 +1196,7 @@ static void gpio_irq_handler(unsigned int irq, struct irq_desc *desc)
 		configured, we could unmask GPIO bank interrupt immediately */
 		if (!level_mask && !unmasked) {
 			unmasked = 1;
-			desc->irq_data.chip->irq_unmask(&desc->irq_data);
+			chained_irq_exit(chip, desc);
 		}
 
 		isr |= retrigger;
@@ -1231,7 +1232,7 @@ static void gpio_irq_handler(unsigned int irq, struct irq_desc *desc)
 	interrupt */
 exit:
 	if (!unmasked)
-		desc->irq_data.chip->irq_unmask(&desc->irq_data);
+		chained_irq_exit(chip, desc);
 }
 
 static void gpio_irq_shutdown(struct irq_data *d)
-- 
1.7.0.4

[PATCH 2/6] ARM: tegra: update GPIO chained IRQ handler to use entry/exit functions

[Will Deacon]

This patch updates the Tegra gpio chained IRQ handler to use the chained
IRQ enter/exit functions in order to function correctly on primary
controllers with different methods of flow control.

This is required for the GIC to move to fasteoi interrupt handling.

Cc: Colin Cross <ccross@android.com>
Signed-off-by: Will Deacon <will.deacon@arm.com>
---
 arch/arm/mach-tegra/gpio.c |    7 ++++---
 1 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/arch/arm/mach-tegra/gpio.c b/arch/arm/mach-tegra/gpio.c
index 76a3f65..08c43dd 100644
--- a/arch/arm/mach-tegra/gpio.c
+++ b/arch/arm/mach-tegra/gpio.c
@@ -221,8 +221,9 @@ static void tegra_gpio_irq_handler(unsigned int irq, struct irq_desc *desc)
 	int port;
 	int pin;
 	int unmasked = 0;
+	struct irq_chip chip = get_irq_desc_chip(desc);
 
-	desc->irq_data.chip->irq_ack(&desc->irq_data);
+	chained_irq_enter(chip, desc);
 
 	bank = irq_get_handler_data(irq);
 
@@ -241,7 +242,7 @@ static void tegra_gpio_irq_handler(unsigned int irq, struct irq_desc *desc)
 			 */
 			if (lvl & (0x100 << pin)) {
 				unmasked = 1;
-				desc->irq_data.chip->irq_unmask(&desc->irq_data);
+				chained_irq_exit(chip, desc);
 			}
 
 			generic_handle_irq(gpio_to_irq(gpio + pin));
@@ -249,7 +250,7 @@ static void tegra_gpio_irq_handler(unsigned int irq, struct irq_desc *desc)
 	}
 
 	if (!unmasked)
-		desc->irq_data.chip->irq_unmask(&desc->irq_data);
+		chained_irq_exit(chip, desc);
 
 }
 
-- 
1.7.0.4

[PATCH 2/6] ARM: tegra: update GPIO chained IRQ handler to use entry/exit functions

[Colin Cross]

On Fri, Apr 1, 2011 at 7:50 AM, Will Deacon <will.deacon@arm.com> wrote:
> This patch updates the Tegra gpio chained IRQ handler to use the chained
> IRQ enter/exit functions in order to function correctly on primary
> controllers with different methods of flow control.
>
> This is required for the GIC to move to fasteoi interrupt handling.
>
> Cc: Colin Cross <ccross@android.com>
> Signed-off-by: Will Deacon <will.deacon@arm.com>
> ---
> ?arch/arm/mach-tegra/gpio.c | ? ?7 ++++---
> ?1 files changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/arch/arm/mach-tegra/gpio.c b/arch/arm/mach-tegra/gpio.c
> index 76a3f65..08c43dd 100644
> --- a/arch/arm/mach-tegra/gpio.c
> +++ b/arch/arm/mach-tegra/gpio.c
> @@ -221,8 +221,9 @@ static void tegra_gpio_irq_handler(unsigned int irq, struct irq_desc *desc)
> ? ? ? ?int port;
> ? ? ? ?int pin;
> ? ? ? ?int unmasked = 0;
> + ? ? ? struct irq_chip chip = get_irq_desc_chip(desc);
Should be a pointer, and 35e857c changed get_irq_desc_chip to irq_desc_get_chip

> - ? ? ? desc->irq_data.chip->irq_ack(&desc->irq_data);
> + ? ? ? chained_irq_enter(chip, desc);
asm/mach/irq.h needs to be included to use this function

> ? ? ? ?bank = irq_get_handler_data(irq);
>
> @@ -241,7 +242,7 @@ static void tegra_gpio_irq_handler(unsigned int irq, struct irq_desc *desc)
> ? ? ? ? ? ? ? ? ? ? ? ? */
> ? ? ? ? ? ? ? ? ? ? ? ?if (lvl & (0x100 << pin)) {
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?unmasked = 1;
> - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? desc->irq_data.chip->irq_unmask(&desc->irq_data);
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? chained_irq_exit(chip, desc);
> ? ? ? ? ? ? ? ? ? ? ? ?}
>
> ? ? ? ? ? ? ? ? ? ? ? ?generic_handle_irq(gpio_to_irq(gpio + pin));
> @@ -249,7 +250,7 @@ static void tegra_gpio_irq_handler(unsigned int irq, struct irq_desc *desc)
> ? ? ? ?}
>
> ? ? ? ?if (!unmasked)
> - ? ? ? ? ? ? ? desc->irq_data.chip->irq_unmask(&desc->irq_data);
> + ? ? ? ? ? ? ? chained_irq_exit(chip, desc);
>
> ?}
>
> --
> 1.7.0.4
>
>

[PATCH 2/6] ARM: tegra: update GPIO chained IRQ handler to use entry/exit functions

[Will Deacon]

Hi Colin,

On Fri, 2011-04-01 at 21:29 +0100, Colin Cross wrote:
> > diff --git a/arch/arm/mach-tegra/gpio.c b/arch/arm/mach-tegra/gpio.c
> > index 76a3f65..08c43dd 100644
> > --- a/arch/arm/mach-tegra/gpio.c
> > +++ b/arch/arm/mach-tegra/gpio.c
> > @@ -221,8 +221,9 @@ static void tegra_gpio_irq_handler(unsigned int irq, struct irq_desc *desc)
> >        int port;
> >        int pin;
> >        int unmasked = 0;
> > +       struct irq_chip chip = get_irq_desc_chip(desc);
> Should be a pointer, and 35e857c changed get_irq_desc_chip to irq_desc_get_chip
> 

Thanks for pointing that out, I'll check the rest of the series too. I
thought the rebase onto -rc1 went a little too smoothly...

> > -       desc->irq_data.chip->irq_ack(&desc->irq_data);
> > +       chained_irq_enter(chip, desc);
> asm/mach/irq.h needs to be included to use this function
> 

Yup, assumed that was already included. Will add the #include line.

Thanks for looking at this,

Will

[PATCH 3/6] ARM: s5pv310: update IRQ combiner to use chained entry/exit functions

[Will Deacon]

This patch updates the IRQ combiner chained IRQ handler code to use the
chained IRQ enter/exit functions in order to function correctly on
primary controllers with different methods of flow control.

This is required for the GIC to move to fasteoi interrupt handling.

Cc: Kyungmin Park <kyungmin.park@samsung.com>
Signed-off-by: Will Deacon <will.deacon@arm.com>
---
 arch/arm/mach-exynos4/irq-combiner.c |    6 ++----
 1 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/arch/arm/mach-exynos4/irq-combiner.c b/arch/arm/mach-exynos4/irq-combiner.c
index f488b66..5a2758a 100644
--- a/arch/arm/mach-exynos4/irq-combiner.c
+++ b/arch/arm/mach-exynos4/irq-combiner.c
@@ -59,8 +59,7 @@ static void combiner_handle_cascade_irq(unsigned int irq, struct irq_desc *desc)
 	unsigned int cascade_irq, combiner_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 = __raw_readl(chip_data->base + COMBINER_INT_STATUS);
@@ -79,8 +78,7 @@ static void combiner_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 combiner_chip = {
-- 
1.7.0.4

[PATCH 4/6] ARM: msm: update GPIO chained IRQ handler to use entry/exit functions

[Will Deacon]

This patch updates the MSM gpio chained IRQ handler to use the chained
IRQ enter/exit functions in order to function correctly on primary
controllers with different methods of flow control.

Cc: Abhijeet Dharmapurikar <adharmap@codeaurora.org>
Signed-off-by: Will Deacon <will.deacon@arm.com>
---
 arch/arm/mach-msm/gpio-v2.c |    6 +++++-
 1 files changed, 5 insertions(+), 1 deletions(-)

diff --git a/arch/arm/mach-msm/gpio-v2.c b/arch/arm/mach-msm/gpio-v2.c
index 56a964e..483f007 100644
--- a/arch/arm/mach-msm/gpio-v2.c
+++ b/arch/arm/mach-msm/gpio-v2.c
@@ -311,6 +311,9 @@ static void msm_summary_irq_handler(unsigned int irq, struct irq_desc *desc)
 {
 	struct irq_data *data = irq_desc_get_irq_data(desc);
 	unsigned long i;
+	struct irq_chip *chip = get_irq_desc_chip(desc);
+
+	chained_irq_enter(chip, desc);
 
 	for (i = find_first_bit(msm_gpio.enabled_irqs, NR_GPIO_IRQS);
 	     i < NR_GPIO_IRQS;
@@ -319,7 +322,8 @@ static void msm_summary_irq_handler(unsigned int irq, struct irq_desc *desc)
 			generic_handle_irq(msm_gpio_to_irq(&msm_gpio.gpio_chip,
 							   i));
 	}
-	data->chip->irq_ack(data);
+
+	chained_irq_exit(chip, desc);
 }
 
 static int msm_gpio_irq_set_wake(struct irq_data *d, unsigned int on)
-- 
1.7.0.4

[PATCH 5/6] ARM: nmk: update GPIO chained IRQ handler to entry/exit functions

[Will Deacon]

This patch updates the Nomadik gpio chained IRQ handler to use the
chained IRQ enter/exit functions in order to function correctly on
primary controllers with different methods of flow control.

Cc: Rabin Vincent <rabin@rab.in>
Signed-off-by: Will Deacon <will.deacon@arm.com>
---
 arch/arm/plat-nomadik/gpio.c |   10 ++--------
 1 files changed, 2 insertions(+), 8 deletions(-)

diff --git a/arch/arm/plat-nomadik/gpio.c b/arch/arm/plat-nomadik/gpio.c
index f49748e..025a54c 100644
--- a/arch/arm/plat-nomadik/gpio.c
+++ b/arch/arm/plat-nomadik/gpio.c
@@ -681,13 +681,7 @@ static void __nmk_gpio_irq_handler(unsigned int irq, struct irq_desc *desc,
 	struct irq_chip *host_chip = irq_get_chip(irq);
 	unsigned int first_irq;
 
-	if (host_chip->irq_mask_ack)
-		host_chip->irq_mask_ack(&desc->irq_data);
-	else {
-		host_chip->irq_mask(&desc->irq_data);
-		if (host_chip->irq_ack)
-			host_chip->irq_ack(&desc->irq_data);
-	}
+	chained_irq_enter(host_chip, desc);
 
 	nmk_chip = irq_get_handler_data(irq);
 	first_irq = NOMADIK_GPIO_TO_IRQ(nmk_chip->chip.base);
@@ -698,7 +692,7 @@ static void __nmk_gpio_irq_handler(unsigned int irq, struct irq_desc *desc,
 		status &= ~BIT(bit);
 	}
 
-	host_chip->irq_unmask(&desc->irq_data);
+	chained_irq_exit(host_chip, desc);
 }
 
 static void nmk_gpio_irq_handler(unsigned int irq, struct irq_desc *desc)
-- 
1.7.0.4

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

[Will Deacon]

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@codeaurora.org>
Cc: Russell King - ARM Linux <linux@arm.linux.org.uk>
Acked-by: Catalin Marinas <catalin.marinas@arm.com>
Signed-off-by: Will Deacon <will.deacon@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

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

[Colin Cross]

On Fri, Apr 1, 2011 at 7:50 AM, Will Deacon <will.deacon@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@codeaurora.org>
> Cc: Russell King - ARM Linux <linux@arm.linux.org.uk>
> Acked-by: Catalin Marinas <catalin.marinas@arm.com>
> Signed-off-by: Will Deacon <will.deacon@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@android.com>
Works on tegra after fixing the problems with the tegra gpio patch.

[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@google.com> wrote:
> On Fri, Apr 1, 2011 at 7:50 AM, Will Deacon <will.deacon@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@codeaurora.org>
>> Cc: Russell King - ARM Linux <linux@arm.linux.org.uk>
>> Acked-by: Catalin Marinas <catalin.marinas@arm.com>
>> Signed-off-by: Will Deacon <will.deacon@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@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)

[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@google.com>  wrote:
>> On Fri, Apr 1, 2011 at 7:50 AM, Will Deacon<will.deacon@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@codeaurora.org>
>>> Cc: Russell King - ARM Linux<linux@arm.linux.org.uk>
>>> Acked-by: Catalin Marinas<catalin.marinas@arm.com>
>>> Signed-off-by: Will Deacon<will.deacon@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@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)

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

[Will Deacon]

On Sun, 2011-04-03 at 07:06 +0100, Santosh Shilimkar wrote:
> Will,

Hi Santosh, 

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

If you hold your horses then I'll post a newer series addressing the
points from Colin. Testing that on OMAP4 would be much appreciated!

Thanks for your patience,

Will

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

[Santosh Shilimkar]

On 4/3/2011 5:48 PM, Will Deacon wrote:
> On Sun, 2011-04-03 at 07:06 +0100, Santosh Shilimkar wrote:
>> Will,
>
> Hi Santosh,
>
>> 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 ?
>
> If you hold your horses then I'll post a newer series addressing the
> points from Colin. Testing that on OMAP4 would be much appreciated!
>
Sure.

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

[Will Deacon]

Hello,


On Sun, 2011-04-03 at 04:27 +0100, Colin Cross wrote:
> 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?).

Ok, I'll do some grepping around for those guys. I didn't realise they
were already being used.

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


Hmm, I don't like that spinlock. Are the gic_arch_extn.* pointers ever
modified after being set initially? If not, can we make the locking
conditional on that pointer being non-NULL?

Will

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

[Colin Cross]

On Sun, Apr 3, 2011 at 5:17 AM, Will Deacon <will.deacon@arm.com> wrote:
> Hello,
>
>
> On Sun, 2011-04-03 at 04:27 +0100, Colin Cross wrote:
>> 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?).
>
> Ok, I'll do some grepping around for those guys. I didn't realise they
> were already being used.
>
>> 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)
>
>
> Hmm, I don't like that spinlock. Are the gic_arch_extn.* pointers ever
> modified after being set initially? If not, can we make the locking
> conditional on that pointer being non-NULL?

The gic_arch_extn pointers should be initialized in init_irq and
untouched later, so it should be fine to make the locking conditional.
 There is currently no way to set the pointer under the lock anyways.

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

[Will Deacon]

Hi Colin,

On Sun, 2011-04-03 at 04:27 +0100, Colin Cross wrote:
> 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?).

Is something like the following correct for updating mach-tegra/irq.c?


diff --git a/arch/arm/mach-tegra/irq.c b/arch/arm/mach-tegra/irq.c
index 4330d89..2424a3b 100644
--- a/arch/arm/mach-tegra/irq.c
+++ b/arch/arm/mach-tegra/irq.c
@@ -48,7 +48,7 @@ static u32 tegra_lp0_wake_level_any;
 
 static void (*tegra_gic_mask_irq)(struct irq_data *d);
 static void (*tegra_gic_unmask_irq)(struct irq_data *d);
-static void (*tegra_gic_ack_irq)(struct irq_data *d);
+static void (*tegra_gic_eoi_irq)(struct irq_data *d);
 
 /* ensures that sufficient time is passed for a register write to
  * serialize into the 32KHz domain */
@@ -113,10 +113,10 @@ static void tegra_unmask(struct irq_data *d)
        tegra_legacy_unmask_irq(d->irq);
 }
 
-static void tegra_ack(struct irq_data *d)
+static void tegra_eoi(struct irq_data *d)
 {
        tegra_legacy_force_irq_clr(d->irq);
-       tegra_gic_ack_irq(d);
+       tegra_gic_eoi_irq(d);
 }
 
 static int tegra_retrigger(struct irq_data *d)
@@ -127,7 +127,7 @@ static int tegra_retrigger(struct irq_data *d)
 
 static struct irq_chip tegra_irq = {
        .name                   = "PPI",
-       .irq_ack                = tegra_ack,
+       .irq_eoi                = tegra_eoi,
        .irq_mask               = tegra_mask,
        .irq_unmask             = tegra_unmask,
        .irq_retrigger          = tegra_retrigger,
@@ -147,14 +147,14 @@ void __init tegra_init_irq(void)
        gic = irq_get_chip(29);
        tegra_gic_unmask_irq = gic->irq_unmask;
        tegra_gic_mask_irq = gic->irq_mask;
-       tegra_gic_ack_irq = gic->irq_ack;
+       tegra_gic_eoi_irq = gic->irq_eoi;
 #ifdef CONFIG_SMP
        tegra_irq.irq_set_affinity = gic->irq_set_affinity;
 #endif
 
        for (i = 0; i < INT_MAIN_NR; i++) {
                irq = INT_PRI_BASE + i;
-               irq_set_chip_and_handler(irq, &tegra_irq, handle_level_irq);
+               irq_set_chip_and_handler(irq, &tegra_irq, handle_fasteoi_irq);
                set_irq_flags(irq, IRQF_VALID);
        }
 }


I'm not 100% about the ordering in tegra_eoi, so I left it alone.

As it stands, this will be a pain to merge because the GIC code may be
out-of-sync with mach-tegra and we could get the flow control type
wrong. Any thoughts about how to resolve that? One [ugly] option is to
check the gic->irq_eoi pointer and choose the flow type based on that,
leaving the irq_ack pointer intact. This could be removed once the GIC
changes are in mainline.

Cheers,

Will