* [PATCH 1/6] ARM: omap: update GPIO chained IRQ handler to use entry/exit functions
2011-04-12 18:35 [PATCH v2 0/6] Use chained handler entry/exit functions in platform code Will Deacon
@ 2011-04-12 18:35 ` Will Deacon
2011-04-12 18:35 ` [PATCH 2/6] ARM: tegra: " Will Deacon
` (4 subsequent siblings)
5 siblings, 0 replies; 27+ messages in thread
From: Will Deacon @ 2011-04-12 18:35 UTC (permalink / raw)
To: linux-arm-kernel
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..a2478eb 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 = irq_desc_get_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
^ permalink raw reply related [flat|nested] 27+ messages in thread* [PATCH 2/6] ARM: tegra: update GPIO chained IRQ handler to use entry/exit functions
2011-04-12 18:35 [PATCH v2 0/6] Use chained handler entry/exit functions in platform code Will Deacon
2011-04-12 18:35 ` [PATCH 1/6] ARM: omap: update GPIO chained IRQ handler to use entry/exit functions Will Deacon
@ 2011-04-12 18:35 ` Will Deacon
2011-05-01 7:26 ` Colin Cross
2011-04-12 18:35 ` [PATCH 3/6] ARM: s5pv310: update IRQ combiner to use chained " Will Deacon
` (3 subsequent siblings)
5 siblings, 1 reply; 27+ messages in thread
From: Will Deacon @ 2011-04-12 18:35 UTC (permalink / raw)
To: linux-arm-kernel
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 | 9 ++++++---
arch/arm/mach-tegra/irq.c | 12 ++++++------
2 files changed, 12 insertions(+), 9 deletions(-)
diff --git a/arch/arm/mach-tegra/gpio.c b/arch/arm/mach-tegra/gpio.c
index 76a3f65..494e28f 100644
--- a/arch/arm/mach-tegra/gpio.c
+++ b/arch/arm/mach-tegra/gpio.c
@@ -24,6 +24,8 @@
#include <linux/io.h>
#include <linux/gpio.h>
+#include <asm/mach/irq.h>
+
#include <mach/iomap.h>
#include <mach/suspend.h>
@@ -221,8 +223,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 = irq_desc_get_chip(desc);
- desc->irq_data.chip->irq_ack(&desc->irq_data);
+ chained_irq_enter(chip, desc);
bank = irq_get_handler_data(irq);
@@ -241,7 +244,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 +252,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);
}
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);
}
}
--
1.7.0.4
^ permalink raw reply related [flat|nested] 27+ messages in thread* [PATCH 2/6] ARM: tegra: update GPIO chained IRQ handler to use entry/exit functions
2011-04-12 18:35 ` [PATCH 2/6] ARM: tegra: " Will Deacon
@ 2011-05-01 7:26 ` Colin Cross
2011-05-01 12:42 ` Will Deacon
0 siblings, 1 reply; 27+ messages in thread
From: Colin Cross @ 2011-05-01 7:26 UTC (permalink / raw)
To: linux-arm-kernel
On Tue, Apr 12, 2011 at 11:35 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 | ? ?9 ++++++---
> ?arch/arm/mach-tegra/irq.c ?| ? 12 ++++++------
> ?2 files changed, 12 insertions(+), 9 deletions(-)
>
> diff --git a/arch/arm/mach-tegra/gpio.c b/arch/arm/mach-tegra/gpio.c
> index 76a3f65..494e28f 100644
> --- a/arch/arm/mach-tegra/gpio.c
> +++ b/arch/arm/mach-tegra/gpio.c
> @@ -24,6 +24,8 @@
> ?#include <linux/io.h>
> ?#include <linux/gpio.h>
>
> +#include <asm/mach/irq.h>
> +
> ?#include <mach/iomap.h>
> ?#include <mach/suspend.h>
>
> @@ -221,8 +223,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 = irq_desc_get_chip(desc);
>
> - ? ? ? desc->irq_data.chip->irq_ack(&desc->irq_data);
> + ? ? ? chained_irq_enter(chip, desc);
>
> ? ? ? ?bank = irq_get_handler_data(irq);
>
> @@ -241,7 +244,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 +252,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);
>
> ?}
>
> 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);
> ? ? ? ?}
> ?}
> --
> 1.7.0.4
>
>
This version of the patch contains changes to gpio.c and irq.c. If
you drop the irq.c changes (which I will fix in my upcoming patch),
Acked-by: Colin Cross <ccross@android.com>
^ permalink raw reply [flat|nested] 27+ messages in thread* [PATCH 2/6] ARM: tegra: update GPIO chained IRQ handler to use entry/exit functions
2011-05-01 7:26 ` Colin Cross
@ 2011-05-01 12:42 ` Will Deacon
0 siblings, 0 replies; 27+ messages in thread
From: Will Deacon @ 2011-05-01 12:42 UTC (permalink / raw)
To: linux-arm-kernel
Hi Colin,
On Sun, 2011-05-01 at 08:26 +0100, Colin Cross wrote:
> On Tue, Apr 12, 2011 at 11:35 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 | 9 ++++++---
> > arch/arm/mach-tegra/irq.c | 12 ++++++------
> > 2 files changed, 12 insertions(+), 9 deletions(-)
[...]
> This version of the patch contains changes to gpio.c and irq.c. If
> you drop the irq.c changes (which I will fix in my upcoming patch),
Yes please! I'm more than happy to drop the changes to irq.c. Please CC
me on your patch so that I know when it's safe to continue pushing the
fasteoi change.
> Acked-by: Colin Cross <ccross@android.com>
Cheers,
Will
^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH 3/6] ARM: s5pv310: update IRQ combiner to use chained entry/exit functions
2011-04-12 18:35 [PATCH v2 0/6] Use chained handler entry/exit functions in platform code Will Deacon
2011-04-12 18:35 ` [PATCH 1/6] ARM: omap: update GPIO chained IRQ handler to use entry/exit functions Will Deacon
2011-04-12 18:35 ` [PATCH 2/6] ARM: tegra: " Will Deacon
@ 2011-04-12 18:35 ` Will Deacon
2011-04-12 18:35 ` [PATCH 4/6] ARM: msm: update GPIO chained IRQ handler to use " Will Deacon
` (2 subsequent siblings)
5 siblings, 0 replies; 27+ messages in thread
From: Will Deacon @ 2011-04-12 18:35 UTC (permalink / raw)
To: linux-arm-kernel
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
^ permalink raw reply related [flat|nested] 27+ messages in thread* [PATCH 4/6] ARM: msm: update GPIO chained IRQ handler to use entry/exit functions
2011-04-12 18:35 [PATCH v2 0/6] Use chained handler entry/exit functions in platform code Will Deacon
` (2 preceding siblings ...)
2011-04-12 18:35 ` [PATCH 3/6] ARM: s5pv310: update IRQ combiner to use chained " Will Deacon
@ 2011-04-12 18:35 ` Will Deacon
2011-04-15 18:27 ` David Brown
2011-04-16 1:51 ` Abhijeet Dharmapurikar
2011-04-12 18:35 ` [PATCH 5/6] ARM: nmk: update GPIO chained IRQ handler to " Will Deacon
2011-04-12 18:35 ` [PATCH 6/6] ARM: gic: use handle_fasteoi_irq for SPIs Will Deacon
5 siblings, 2 replies; 27+ messages in thread
From: Will Deacon @ 2011-04-12 18:35 UTC (permalink / raw)
To: linux-arm-kernel
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 | 10 ++++++++--
1 files changed, 8 insertions(+), 2 deletions(-)
diff --git a/arch/arm/mach-msm/gpio-v2.c b/arch/arm/mach-msm/gpio-v2.c
index 56a964e..cc9c4fd 100644
--- a/arch/arm/mach-msm/gpio-v2.c
+++ b/arch/arm/mach-msm/gpio-v2.c
@@ -27,6 +27,9 @@
#include <linux/module.h>
#include <linux/platform_device.h>
#include <linux/spinlock.h>
+
+#include <asm/mach/irq.h>
+
#include <mach/msm_iomap.h>
#include "gpiomux.h"
@@ -309,8 +312,10 @@ static int msm_gpio_irq_set_type(struct irq_data *d, unsigned int flow_type)
*/
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 = irq_desc_get_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 +324,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
^ permalink raw reply related [flat|nested] 27+ messages in thread* [PATCH 4/6] ARM: msm: update GPIO chained IRQ handler to use entry/exit functions
2011-04-12 18:35 ` [PATCH 4/6] ARM: msm: update GPIO chained IRQ handler to use " Will Deacon
@ 2011-04-15 18:27 ` David Brown
2011-04-18 17:57 ` Will Deacon
2011-04-16 1:51 ` Abhijeet Dharmapurikar
1 sibling, 1 reply; 27+ messages in thread
From: David Brown @ 2011-04-15 18:27 UTC (permalink / raw)
To: linux-arm-kernel
On Tue, Apr 12 2011, Will Deacon wrote:
> 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>
I'm okay acking this as long as Abhijeet is ok with it.
Thanks,
David
--
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.
^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH 4/6] ARM: msm: update GPIO chained IRQ handler to use entry/exit functions
2011-04-15 18:27 ` David Brown
@ 2011-04-18 17:57 ` Will Deacon
0 siblings, 0 replies; 27+ messages in thread
From: Will Deacon @ 2011-04-18 17:57 UTC (permalink / raw)
To: linux-arm-kernel
David,
On Fri, 2011-04-15 at 19:27 +0100, David Brown wrote:
> On Tue, Apr 12 2011, Will Deacon wrote:
>
> > 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>
>
> I'm okay acking this as long as Abhijeet is ok with it.
Ok, I'll add your Ack to this guy now that Abhijeet has tested/reviewed
it.
Thanks,
Will
^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH 4/6] ARM: msm: update GPIO chained IRQ handler to use entry/exit functions
2011-04-12 18:35 ` [PATCH 4/6] ARM: msm: update GPIO chained IRQ handler to use " Will Deacon
2011-04-15 18:27 ` David Brown
@ 2011-04-16 1:51 ` Abhijeet Dharmapurikar
2011-04-18 17:56 ` Will Deacon
1 sibling, 1 reply; 27+ messages in thread
From: Abhijeet Dharmapurikar @ 2011-04-16 1:51 UTC (permalink / raw)
To: linux-arm-kernel
Will Deacon wrote:
> 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 | 10 ++++++++--
> 1 files changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/arch/arm/mach-msm/gpio-v2.c b/arch/arm/mach-msm/gpio-v2.c
> index 56a964e..cc9c4fd 100644
> --- a/arch/arm/mach-msm/gpio-v2.c
> +++ b/arch/arm/mach-msm/gpio-v2.c
> @@ -27,6 +27,9 @@
> #include <linux/module.h>
> #include <linux/platform_device.h>
> #include <linux/spinlock.h>
> +
> +#include <asm/mach/irq.h>
> +
> #include <mach/msm_iomap.h>
> #include "gpiomux.h"
>
> @@ -309,8 +312,10 @@ static int msm_gpio_irq_set_type(struct irq_data *d, unsigned int flow_type)
> */
> 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 = irq_desc_get_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 +324,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)
Reviewed-by: Abhijeet Dharmapurikar <adharmap@codeaurora.org>
Tested-by: Abhijeet Dharmapurikar <adharmap@codeaurora.org>
--
--
Sent by an employee of the Qualcomm Innovation Center, Inc. The Qualcomm
Innovation Center, Inc. is a member of the Code Aurora Forum.
^ permalink raw reply [flat|nested] 27+ messages in thread* [PATCH 4/6] ARM: msm: update GPIO chained IRQ handler to use entry/exit functions
2011-04-16 1:51 ` Abhijeet Dharmapurikar
@ 2011-04-18 17:56 ` Will Deacon
0 siblings, 0 replies; 27+ messages in thread
From: Will Deacon @ 2011-04-18 17:56 UTC (permalink / raw)
To: linux-arm-kernel
Abhijeet,
On Sat, 2011-04-16 at 02:51 +0100, Abhijeet Dharmapurikar wrote:
> Will Deacon wrote:
> > 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 | 10 ++++++++--
> > 1 files changed, 8 insertions(+), 2 deletions(-)
[...]
> Reviewed-by: Abhijeet Dharmapurikar <adharmap@codeaurora.org>
> Tested-by: Abhijeet Dharmapurikar <adharmap@codeaurora.org>
Thanks for getting round to looking at this. I don't have any Qualcomm
hardware myself (feel free to send me some :) so I'm uneasy without the
Tested-by tags.
I'll add your tags for this patch and the final one in the series.
Cheers,
Will
^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH 5/6] ARM: nmk: update GPIO chained IRQ handler to entry/exit functions
2011-04-12 18:35 [PATCH v2 0/6] Use chained handler entry/exit functions in platform code Will Deacon
` (3 preceding siblings ...)
2011-04-12 18:35 ` [PATCH 4/6] ARM: msm: update GPIO chained IRQ handler to use " Will Deacon
@ 2011-04-12 18:35 ` Will Deacon
2011-04-18 18:26 ` Linus Walleij
2011-04-12 18:35 ` [PATCH 6/6] ARM: gic: use handle_fasteoi_irq for SPIs Will Deacon
5 siblings, 1 reply; 27+ messages in thread
From: Will Deacon @ 2011-04-12 18:35 UTC (permalink / raw)
To: linux-arm-kernel
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 | 12 ++++--------
1 files changed, 4 insertions(+), 8 deletions(-)
diff --git a/arch/arm/plat-nomadik/gpio.c b/arch/arm/plat-nomadik/gpio.c
index f49748e..307b813 100644
--- a/arch/arm/plat-nomadik/gpio.c
+++ b/arch/arm/plat-nomadik/gpio.c
@@ -23,6 +23,8 @@
#include <linux/irq.h>
#include <linux/slab.h>
+#include <asm/mach/irq.h>
+
#include <plat/pincfg.h>
#include <mach/hardware.h>
#include <mach/gpio.h>
@@ -681,13 +683,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 +694,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
^ permalink raw reply related [flat|nested] 27+ messages in thread* [PATCH 5/6] ARM: nmk: update GPIO chained IRQ handler to entry/exit functions
2011-04-12 18:35 ` [PATCH 5/6] ARM: nmk: update GPIO chained IRQ handler to " Will Deacon
@ 2011-04-18 18:26 ` Linus Walleij
2011-04-18 19:04 ` Will Deacon
0 siblings, 1 reply; 27+ messages in thread
From: Linus Walleij @ 2011-04-18 18:26 UTC (permalink / raw)
To: linux-arm-kernel
2011/4/12 Will Deacon <will.deacon@arm.com>:
> 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>
Acked-by: Linus Walleij <linus.walleij@linaro.org>
BUT I want it to go through Grants gpio tree. The reason is that
I plan to consolidate this driver into drivers/gpio/* and I don't want
collisions in merges.
And wait:
> +#include <asm/mach/irq.h>
> - ? ? ? 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);
(...)
> - ? ? ? host_chip->irq_unmask(&desc->irq_data);
> + ? ? ? chained_irq_exit(host_chip, desc);
This looks an awful lot like this from drivers/gpio/pl061.c:
desc->irq_data.chip->irq_ack(&desc->irq_data);
(...)
desc->irq_data.chip->irq_unmask(&desc->irq_data);
It gets me wondering what the proper way is to do this.
Isn't the proper place for chained_irq_[enter|exit]* in
include/linux/irq.h rather? It does not really look machine
or arch specific, is it, really?
I.e. this:
#ifndef __ASSEMBLY__
/*
* Entry/exit functions for chained handlers where the primary IRQ chip
* may implement either fasteoi or level-trigger flow control.
*/
static inline void chained_irq_enter(struct irq_chip *chip,
struct irq_desc *desc)
{
/* FastEOI controllers require no action on entry. */
if (chip->irq_eoi)
return;
if (chip->irq_mask_ack) {
chip->irq_mask_ack(&desc->irq_data);
} else {
chip->irq_mask(&desc->irq_data);
if (chip->irq_ack)
chip->irq_ack(&desc->irq_data);
}
}
static inline void chained_irq_exit(struct irq_chip *chip,
struct irq_desc *desc)
{
if (chip->irq_eoi)
chip->irq_eoi(&desc->irq_data);
else
chip->irq_unmask(&desc->irq_data);
}
#endif
Excuse me if this was discussed before...
Yours,
Linus Walleij
^ permalink raw reply [flat|nested] 27+ messages in thread* [PATCH 5/6] ARM: nmk: update GPIO chained IRQ handler to entry/exit functions
2011-04-18 18:26 ` Linus Walleij
@ 2011-04-18 19:04 ` Will Deacon
2011-04-18 23:46 ` Linus Walleij
0 siblings, 1 reply; 27+ messages in thread
From: Will Deacon @ 2011-04-18 19:04 UTC (permalink / raw)
To: linux-arm-kernel
Hi Linus,
On Mon, 2011-04-18 at 19:26 +0100, Linus Walleij wrote:
> 2011/4/12 Will Deacon <will.deacon@arm.com>:
>
> > 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>
>
> Acked-by: Linus Walleij <linus.walleij@linaro.org>
Thanks for the Ack.
> BUT I want it to go through Grants gpio tree. The reason is that
> I plan to consolidate this driver into drivers/gpio/* and I don't want
> collisions in merges.
Ok. Will Grant just pick it up when he's happy with it?
> And wait:
>
> > +#include <asm/mach/irq.h>
>
> > - 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);
>
> (...)
>
> > - host_chip->irq_unmask(&desc->irq_data);
> > + chained_irq_exit(host_chip, desc);
>
> This looks an awful lot like this from drivers/gpio/pl061.c:
>
> desc->irq_data.chip->irq_ack(&desc->irq_data);
> (...)
> desc->irq_data.chip->irq_unmask(&desc->irq_data);
The chained_{enter,exit} functions are there so that chained handlers
written for multiple parent IRQ chips can cope when the chips differ in
their method of flow control. I'd argue that we shouldn't be promoting
chained handlers like this (there should probably be a separate handler
for each primary chip implementation with which it's designed to work).
The *only* reason I abstracting this pattern is so that platforms can
survive the GIC migration from ACK => FastEOI without being broken in
mainline while the GIC code is in a state of flux.
> It gets me wondering what the proper way is to do this.
> Isn't the proper place for chained_irq_[enter|exit]* in
> include/linux/irq.h rather? It does not really look machine
> or arch specific, is it, really?
There are only a handful of machines under arch/arm/ that suffer from
this problem and need to be able to handle two flow types
simultaneously. If there's a demand for making this stuff more visible,
then fine, but I don't think there is.
Cheers,
Will
^ permalink raw reply [flat|nested] 27+ messages in thread* [PATCH 5/6] ARM: nmk: update GPIO chained IRQ handler to entry/exit functions
2011-04-18 19:04 ` Will Deacon
@ 2011-04-18 23:46 ` Linus Walleij
2011-04-19 19:52 ` Grant Likely
0 siblings, 1 reply; 27+ messages in thread
From: Linus Walleij @ 2011-04-18 23:46 UTC (permalink / raw)
To: linux-arm-kernel
2011/4/18 Will Deacon <will.deacon@arm.com>:
>> BUT I want it to go through Grants gpio tree. The reason is that
>> I plan to consolidate this driver into drivers/gpio/* and I don't want
>> collisions in merges.
>
> Ok. Will Grant just pick it up when he's happy with it?
If not picked up, I'll queue it on the top of my patch series when moving it
to drivers/gpio...
Thanks,
Linus Walleij
^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH 5/6] ARM: nmk: update GPIO chained IRQ handler to entry/exit functions
2011-04-18 23:46 ` Linus Walleij
@ 2011-04-19 19:52 ` Grant Likely
0 siblings, 0 replies; 27+ messages in thread
From: Grant Likely @ 2011-04-19 19:52 UTC (permalink / raw)
To: linux-arm-kernel
On Tue, Apr 19, 2011 at 01:46:53AM +0200, Linus Walleij wrote:
> 2011/4/18 Will Deacon <will.deacon@arm.com>:
>
> >> BUT I want it to go through Grants gpio tree. The reason is that
> >> I plan to consolidate this driver into drivers/gpio/* and I don't want
> >> collisions in merges.
> >
> > Ok. Will Grant just pick it up when he's happy with it?
>
> If not picked up, I'll queue it on the top of my patch series when moving it
> to drivers/gpio...
Go ahead and put it in your series. Since you're going to be doing
consolidation work on it, I'd rather deal with it all at once.
g.
^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH 6/6] ARM: gic: use handle_fasteoi_irq for SPIs
2011-04-12 18:35 [PATCH v2 0/6] Use chained handler entry/exit functions in platform code Will Deacon
` (4 preceding siblings ...)
2011-04-12 18:35 ` [PATCH 5/6] ARM: nmk: update GPIO chained IRQ handler to " Will Deacon
@ 2011-04-12 18:35 ` Will Deacon
2011-04-16 1:52 ` Abhijeet Dharmapurikar
` (2 more replies)
5 siblings, 3 replies; 27+ messages in thread
From: Will Deacon @ 2011-04-12 18:35 UTC (permalink / raw)
To: linux-arm-kernel
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 | 32 ++++++++++++++++----------------
1 files changed, 16 insertions(+), 16 deletions(-)
diff --git a/arch/arm/common/gic.c b/arch/arm/common/gic.c
index f70ec7d..e9c2ff8 100644
--- a/arch/arm/common/gic.c
+++ b/arch/arm/common/gic.c
@@ -49,7 +49,7 @@ struct gic_chip_data {
* Default make them NULL.
*/
struct irq_chip gic_arch_extn = {
- .irq_ack = NULL,
+ .irq_eoi = NULL,
.irq_mask = NULL,
.irq_unmask = NULL,
.irq_retrigger = NULL,
@@ -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,17 @@ static void gic_unmask_irq(struct irq_data *d)
spin_unlock(&irq_controller_lock);
}
+static void gic_eoi_irq(struct irq_data *d)
+{
+ if (gic_arch_extn.irq_eoi) {
+ spin_lock(&irq_controller_lock);
+ gic_arch_extn.irq_eoi(d);
+ spin_unlock(&irq_controller_lock);
+ }
+
+ 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 +220,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 +237,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 +319,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
^ permalink raw reply related [flat|nested] 27+ messages in thread* [PATCH 6/6] ARM: gic: use handle_fasteoi_irq for SPIs
2011-04-12 18:35 ` [PATCH 6/6] ARM: gic: use handle_fasteoi_irq for SPIs Will Deacon
@ 2011-04-16 1:52 ` Abhijeet Dharmapurikar
2011-04-19 11:20 ` Santosh Shilimkar
2011-04-30 2:38 ` Colin Cross
2 siblings, 0 replies; 27+ messages in thread
From: Abhijeet Dharmapurikar @ 2011-04-16 1:52 UTC (permalink / raw)
To: linux-arm-kernel
Will Deacon 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 | 32 ++++++++++++++++----------------
> 1 files changed, 16 insertions(+), 16 deletions(-)
>
> diff --git a/arch/arm/common/gic.c b/arch/arm/common/gic.c
> index f70ec7d..e9c2ff8 100644
> --- a/arch/arm/common/gic.c
> +++ b/arch/arm/common/gic.c
> @@ -49,7 +49,7 @@ struct gic_chip_data {
> * Default make them NULL.
> */
> struct irq_chip gic_arch_extn = {
> - .irq_ack = NULL,
> + .irq_eoi = NULL,
> .irq_mask = NULL,
> .irq_unmask = NULL,
> .irq_retrigger = NULL,
> @@ -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,17 @@ static void gic_unmask_irq(struct irq_data *d)
> spin_unlock(&irq_controller_lock);
> }
>
> +static void gic_eoi_irq(struct irq_data *d)
> +{
> + if (gic_arch_extn.irq_eoi) {
> + spin_lock(&irq_controller_lock);
> + gic_arch_extn.irq_eoi(d);
> + spin_unlock(&irq_controller_lock);
> + }
> +
> + 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 +220,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 +237,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 +319,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);
> }
Reviewed-by: Abhijeet Dharmapurikar <adharmap@codeaurora.org>
Tested-by: Abhijeet Dharmapurikar <adharmap@codeaurora.org>
--
Sent by an employee of the Qualcomm Innovation Center, Inc. The Qualcomm
Innovation Center, Inc. is a member of the Code Aurora Forum.
^ permalink raw reply [flat|nested] 27+ messages in thread* [PATCH 6/6] ARM: gic: use handle_fasteoi_irq for SPIs
2011-04-12 18:35 ` [PATCH 6/6] ARM: gic: use handle_fasteoi_irq for SPIs Will Deacon
2011-04-16 1:52 ` Abhijeet Dharmapurikar
@ 2011-04-19 11:20 ` Santosh Shilimkar
2011-04-19 15:16 ` Will Deacon
2011-04-30 2:38 ` Colin Cross
2 siblings, 1 reply; 27+ messages in thread
From: Santosh Shilimkar @ 2011-04-19 11:20 UTC (permalink / raw)
To: linux-arm-kernel
On 4/13/2011 12:05 AM, Will Deacon 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>
> ---
Tested with OMAP4.
Acked-by: Santosh Shilimkar <santosh.shilimkar@ti.com>
Tested-by: Santosh Shilimkar <santosh.shilimkar@ti.com>
^ permalink raw reply [flat|nested] 27+ messages in thread* [PATCH 6/6] ARM: gic: use handle_fasteoi_irq for SPIs
2011-04-19 11:20 ` Santosh Shilimkar
@ 2011-04-19 15:16 ` Will Deacon
2011-04-20 4:20 ` Santosh Shilimkar
0 siblings, 1 reply; 27+ messages in thread
From: Will Deacon @ 2011-04-19 15:16 UTC (permalink / raw)
To: linux-arm-kernel
Hi Santosh,
On Tue, 2011-04-19 at 12:20 +0100, Santosh Shilimkar wrote:
> On 4/13/2011 12:05 AM, Will Deacon 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>
> > ---
> Tested with OMAP4.
> Acked-by: Santosh Shilimkar <santosh.shilimkar@ti.com>
> Tested-by: Santosh Shilimkar <santosh.shilimkar@ti.com>
Cheers for this. I'll add this and the tags for the OMAP-specific patch
(for some reason you dropped the list when you replied with your Ack).
Thanks!
Will
^ permalink raw reply [flat|nested] 27+ messages in thread* [PATCH 6/6] ARM: gic: use handle_fasteoi_irq for SPIs
2011-04-19 15:16 ` Will Deacon
@ 2011-04-20 4:20 ` Santosh Shilimkar
0 siblings, 0 replies; 27+ messages in thread
From: Santosh Shilimkar @ 2011-04-20 4:20 UTC (permalink / raw)
To: linux-arm-kernel
On 4/19/2011 8:46 PM, Will Deacon wrote:
> Hi Santosh,
>
> On Tue, 2011-04-19 at 12:20 +0100, Santosh Shilimkar wrote:
>> On 4/13/2011 12:05 AM, Will Deacon 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>
>>> ---
>> Tested with OMAP4.
>> Acked-by: Santosh Shilimkar<santosh.shilimkar@ti.com>
>> Tested-by: Santosh Shilimkar<santosh.shilimkar@ti.com>
>
> Cheers for this. I'll add this and the tags for the OMAP-specific patch
> (for some reason you dropped the list when you replied with your Ack).
>
Sure. By mistake I did reply instead of reply all :(
Regards
Santosh
^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH 6/6] ARM: gic: use handle_fasteoi_irq for SPIs
2011-04-12 18:35 ` [PATCH 6/6] ARM: gic: use handle_fasteoi_irq for SPIs Will Deacon
2011-04-16 1:52 ` Abhijeet Dharmapurikar
2011-04-19 11:20 ` Santosh Shilimkar
@ 2011-04-30 2:38 ` Colin Cross
2011-04-30 9:54 ` Thomas Gleixner
2 siblings, 1 reply; 27+ messages in thread
From: Colin Cross @ 2011-04-30 2:38 UTC (permalink / raw)
To: linux-arm-kernel
On Tue, Apr 12, 2011 at 11:35 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>
After further testing, I'm having a problem with this patch, although
I think its more of a core issue than the fault of this patch. One of
my interrupts is getting stuck with the PENDING flag set, and
preventing suspend. The flow that triggers this is:
level triggered irq
handle_fasteoi_irq
handle_irq_event
isr
disable_irq_nosync
schedule_work (because it takes a sleeping i2c transaction to
deassert the irq pin)
irq_eoi
same irq
handle_fasteoi_irq
mark irq pending
mask_irq
work function
causes level triggered irq to go low
enable_irq
unmask_irq
check_irq_resend (returns immediately)
At this point, the irq is unmasked, but not being asserted, and marked
as pending. check_irq_resend doesn't clear the pending flag for level
triggered interrupts, so the pending flag will stay set until the next
time the interrupt occurs.
Should handle_fasteoi_irq mask the interrupt before eoi if it was
disabled by the interrupt handler? Otherwise, every level triggered
interrupt that is not deasserted by the interrupt handler will
interrupt the cpu twice. There is still the case where a driver
disables the irq, the interrupt goes high, then goes low again before
enable_irq is called.
This patch fixes the double interrupt problem, but not the vanishing
interrupt problem:
diff --git a/kernel/irq/chip.c b/kernel/irq/chip.c
index 4af1e2b..424bdfc 100644
--- a/kernel/irq/chip.c
+++ b/kernel/irq/chip.c
@@ -398,6 +398,9 @@ handle_fasteoi_irq(unsigned int irq, struct irq_desc *desc)
preflow_handler(desc);
handle_irq_event(desc);
+ if (irqd_irq_disabled(&desc->irq_data) &&
+ !irqd_irq_masked(&desc->irq_data))
+ mask_irq(desc);
out_eoi:
desc->irq_data.chip->irq_eoi(&desc->irq_data);
out_unlock:
^ permalink raw reply related [flat|nested] 27+ messages in thread* [PATCH 6/6] ARM: gic: use handle_fasteoi_irq for SPIs
2011-04-30 2:38 ` Colin Cross
@ 2011-04-30 9:54 ` Thomas Gleixner
2011-04-30 16:42 ` Colin Cross
0 siblings, 1 reply; 27+ messages in thread
From: Thomas Gleixner @ 2011-04-30 9:54 UTC (permalink / raw)
To: linux-arm-kernel
On Fri, 29 Apr 2011, Colin Cross wrote:
> > 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>
>
> After further testing, I'm having a problem with this patch, although
> I think its more of a core issue than the fault of this patch. One of
> my interrupts is getting stuck with the PENDING flag set, and
> preventing suspend. The flow that triggers this is:
>
> level triggered irq
> handle_fasteoi_irq
> handle_irq_event
> isr
> disable_irq_nosync
> schedule_work (because it takes a sleeping i2c transaction to
> deassert the irq pin)
If you'd use a threaded irq handler the IRQ_ONESHOT mechanism would
handle that problem for you. It masks the irq line before calling the
handler and unmask happens after the threaded handler has run.
disable_irq_nosync from an interrupt handler plus scheduling work is
the historic "threaded" interrupt handler mechanism. It's kinda murky
nowadays due to the lazy irq disable mechanism.
> irq_eoi
> same irq
> handle_fasteoi_irq
> mark irq pending
> mask_irq
> work function
> causes level triggered irq to go low
> enable_irq
> unmask_irq
> check_irq_resend (returns immediately)
>
> At this point, the irq is unmasked, but not being asserted, and marked
> as pending. check_irq_resend doesn't clear the pending flag for level
> triggered interrupts, so the pending flag will stay set until the next
> time the interrupt occurs.
Yes, that should be cleared unconditionally in check_irq_resend.
> Should handle_fasteoi_irq mask the interrupt before eoi if it was
> disabled by the interrupt handler? Otherwise, every level triggered
> interrupt that is not deasserted by the interrupt handler will
> interrupt the cpu twice. There is still the case where a driver
No, we should stop doing the disable_irq_nosync from handlers and use
threaded interrupts instead.
> disables the irq, the interrupt goes high, then goes low again before
> enable_irq is called.
So what you're saying is:
irq ____ _______
|________________| |_____________________
| |
ISR/mask/eoi enable_irq/unmask
So after the unmask the asserted new interrupt is not delivered?
That's not a software problem :)
Thanks,
tglx
^ permalink raw reply [flat|nested] 27+ messages in thread* [PATCH 6/6] ARM: gic: use handle_fasteoi_irq for SPIs
2011-04-30 9:54 ` Thomas Gleixner
@ 2011-04-30 16:42 ` Colin Cross
0 siblings, 0 replies; 27+ messages in thread
From: Colin Cross @ 2011-04-30 16:42 UTC (permalink / raw)
To: linux-arm-kernel
On Sat, Apr 30, 2011 at 2:54 AM, Thomas Gleixner <tglx@linutronix.de> wrote:
> On Fri, 29 Apr 2011, Colin Cross wrote:
>> > 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>
>>
>> After further testing, I'm having a problem with this patch, although
>> I think its more of a core issue than the fault of this patch. ?One of
>> my interrupts is getting stuck with the PENDING flag set, and
>> preventing suspend. ?The flow that triggers this is:
>>
>> level triggered irq
>> ?handle_fasteoi_irq
>> ? handle_irq_event
>> ? ?isr
>> ? ? disable_irq_nosync
>> ? ? schedule_work (because it takes a sleeping i2c transaction to
>> deassert the irq pin)
>
> If you'd use a threaded irq handler the IRQ_ONESHOT mechanism would
> handle that problem for you. It masks the irq line before calling the
> handler and unmask happens after the threaded handler has run.
>
> disable_irq_nosync from an interrupt handler plus scheduling work is
> the historic "threaded" interrupt handler mechanism. It's kinda murky
> nowadays due to the lazy irq disable mechanism.
Yes, and in the specific case that reproduces this problem for me a
threaded handler would be appropriate. However, there are cases where
disable irq can legitimately be called from an interrupt handler. One
example I was given is a debounce timer - the interrupt handler
disables the irq and then sets a timer.
>> ? ?irq_eoi
>> same irq
>> ?handle_fasteoi_irq
>> ? mark irq pending
>> ? mask_irq
>> work function
>> ?causes level triggered irq to go low
>> ?enable_irq
>> ? unmask_irq
>> ? check_irq_resend (returns immediately)
>>
>> At this point, the irq is unmasked, but not being asserted, and marked
>> as pending. ?check_irq_resend doesn't clear the pending flag for level
>> triggered interrupts, so the pending flag will stay set until the next
>> time the interrupt occurs.
>
> Yes, that should be cleared unconditionally in check_irq_resend.
>
>> Should handle_fasteoi_irq mask the interrupt before eoi if it was
>> disabled by the interrupt handler? ?Otherwise, every level triggered
>> interrupt that is not deasserted by the interrupt handler will
>> interrupt the cpu twice. ?There is still the case where a driver
>
> No, we should stop doing the disable_irq_nosync from handlers and use
> threaded interrupts instead.
As in the example above, I don't think threaded interrupts cover all
the cases where disable_irq_nosync is used in an interrupt handler,
although 99% of the time they do. handle_level_irq already has this
optimization - it doesn't unmask the irq line if the handler returns
with the irq disabled. I think the patch I posted would need to check
for a level triggered interrupt, masking the line on an edge triggered
interrupt could lose the next interrupt.
>> disables the irq, the interrupt goes high, then goes low again before
>> enable_irq is called.
>
> So what you're saying is:
>
> irq ____ ? ? ? ? ? ? ? ? ?_______
> ? ? ? ?|________________| ? ? ? |_____________________
>
> ? ? ? ? ? ?| ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?|
> ? ? ? ? ISR/mask/eoi ? ? ? ? ? ? ? ?enable_irq/unmask
>
> So after the unmask the asserted new interrupt is not delivered?
> That's not a software problem :)
Of course its not delivered, the problem is that it is marked pending,
check_wakeup_irqs returns true and prevents suspend. Always clearing
the pending flag in check_if_resend fixes suspend after calling
enable_irq, but suspend will still fail if attempted between when the
irq line goes low and enable_irq is called (which could be never if
the driver has been disabled for some reason).
^ permalink raw reply [flat|nested] 27+ messages in thread