I stumbled over this gem when looking through the drivers which are
not yet converted to the new irq_chip functions.
Thanks,
tglx
Applied, thanks.
--
Matthew Garrett | [email protected]
On Mon, 7 Feb 2011, Matthew Garrett wrote:
> Applied, thanks.
Btw, there is no real reason to have this as a chained handler. I
guess I need to find some time for educational documentation :)
See below.
Thanks,
tglx
------->
Subject: platform-drivers: x86: pmic: Use request_irq instead of chained handler
From: Thomas Gleixner <[email protected]>
Date: Mon, 07 Feb 2011 21:24:29 +0100
There is no need to install a chained handler for this hardware. This
is a plain x86 IOAPIC interrupt which is handled by the core code
perfectly fine. There is nothing special about demultiplexing these
gpio interrupts which justifies a custom hack. Replace it by a plain
old interrupt handler installed with request_irq. That makes the code
agnostic about the underlying primary interrupt hardware. The overhead
for this is minimal, but it gives us the advantage of accounting,
balancing and to detect interrupt storms. gpio interrupts are not
really that performance critical.
Signed-off-by: Thomas Gleixner <[email protected]>
---
drivers/platform/x86/intel_pmic_gpio.c | 14 +++++++++-----
1 file changed, 9 insertions(+), 5 deletions(-)
Index: linux-2.6/drivers/platform/x86/intel_pmic_gpio.c
===================================================================
--- linux-2.6.orig/drivers/platform/x86/intel_pmic_gpio.c
+++ linux-2.6/drivers/platform/x86/intel_pmic_gpio.c
@@ -211,9 +211,9 @@ static struct irq_chip pmic_irqchip = {
.irq_set_type = pmic_irq_type,
};
-static void pmic_irq_handler(unsigned irq, struct irq_desc *desc)
+static irqreturn_t pmic_irq_handler(unsigned int irq, void *data)
{
- struct pmic_gpio *pg = (struct pmic_gpio *)get_irq_data(irq);
+ struct pmic_gpio *pg = data;
u8 intsts = *((u8 *)pg->gpiointr + 4);
int gpio;
@@ -223,7 +223,6 @@ static void pmic_irq_handler(unsigned ir
generic_handle_irq(pg->irq_base + gpio);
}
}
- desc->chip->irq_eoi(get_irq_desc_chip_data(desc));
}
static int __devinit platform_pmic_gpio_probe(struct platform_device *pdev)
@@ -280,8 +279,13 @@ static int __devinit platform_pmic_gpio_
printk(KERN_ERR "%s: Can not add pmic gpio chip.\n", __func__);
goto err;
}
- set_irq_data(pg->irq, pg);
- set_irq_chained_handler(pg->irq, pmic_irq_handler);
+
+ retval = request_irq(pg->irq, pmic_irq_handler, 0, "pmic", pg);
+ if (retval) {
+ printk(KERN_WARN "pmic: Interrupt request failed\n");
+ goto err;
+ }
+
for (i = 0; i < 8; i++) {
set_irq_chip_and_handler_name(i + pg->irq_base, &pmic_irqchip,
handle_simple_irq, "demux");
On Mon, 7 Feb 2011 21:41:30 +0100 (CET)
Thomas Gleixner <[email protected]> wrote:
> On Mon, 7 Feb 2011, Matthew Garrett wrote:
>
> > Applied, thanks.
>
> Btw, there is no real reason to have this as a chained handler. I
> guess I need to find some time for educational documentation :)
>
> See below.
>
> Thanks,
>
> tglx
>
> ------->
> Subject: platform-drivers: x86: pmic: Use request_irq instead of chained handler
> From: Thomas Gleixner <[email protected]>
> Date: Mon, 07 Feb 2011 21:24:29 +0100
>
> There is no need to install a chained handler for this hardware. This
> is a plain x86 IOAPIC interrupt which is handled by the core code
> perfectly fine. There is nothing special about demultiplexing these
> gpio interrupts which justifies a custom hack. Replace it by a plain
> old interrupt handler installed with request_irq. That makes the code
> agnostic about the underlying primary interrupt hardware. The overhead
> for this is minimal, but it gives us the advantage of accounting,
> balancing and to detect interrupt storms. gpio interrupts are not
> really that performance critical.
>
> Signed-off-by: Thomas Gleixner <[email protected]>
> ---
> drivers/platform/x86/intel_pmic_gpio.c | 14 +++++++++-----
> 1 file changed, 9 insertions(+), 5 deletions(-)
>
> Index: linux-2.6/drivers/platform/x86/intel_pmic_gpio.c
> ===================================================================
> --- linux-2.6.orig/drivers/platform/x86/intel_pmic_gpio.c
> +++ linux-2.6/drivers/platform/x86/intel_pmic_gpio.c
> @@ -211,9 +211,9 @@ static struct irq_chip pmic_irqchip = {
> .irq_set_type = pmic_irq_type,
> };
>
> -static void pmic_irq_handler(unsigned irq, struct irq_desc *desc)
> +static irqreturn_t pmic_irq_handler(unsigned int irq, void *data)
> {
> - struct pmic_gpio *pg = (struct pmic_gpio *)get_irq_data(irq);
> + struct pmic_gpio *pg = data;
> u8 intsts = *((u8 *)pg->gpiointr + 4);
> int gpio;
>
> @@ -223,7 +223,6 @@ static void pmic_irq_handler(unsigned ir
> generic_handle_irq(pg->irq_base + gpio);
> }
> }
> - desc->chip->irq_eoi(get_irq_desc_chip_data(desc));
> }
>
> static int __devinit platform_pmic_gpio_probe(struct platform_device *pdev)
> @@ -280,8 +279,13 @@ static int __devinit platform_pmic_gpio_
> printk(KERN_ERR "%s: Can not add pmic gpio chip.\n", __func__);
> goto err;
> }
> - set_irq_data(pg->irq, pg);
> - set_irq_chained_handler(pg->irq, pmic_irq_handler);
> +
> + retval = request_irq(pg->irq, pmic_irq_handler, 0, "pmic", pg);
> + if (retval) {
> + printk(KERN_WARN "pmic: Interrupt request failed\n");
> + goto err;
> + }
> +
> for (i = 0; i < 8; i++) {
> set_irq_chip_and_handler_name(i + pg->irq_base, &pmic_irqchip,
> handle_simple_irq, "demux");
That didn't work out too well.
Subject: drivers/platform/x86/intel_pmic_gpio.c: fix big mess with i386 allmodconfig
From: Andrew Morton <[email protected]>
Fix build error:
drivers/platform/x86/intel_pmic_gpio.c:285: error: 'KERN_WARN' undeclared (first use in this function)
And a bunch of warnings:
drivers/platform/x86/intel_pmic_gpio.c: In function 'pmic_irq_handler':
drivers/platform/x86/intel_pmic_gpio.c:226: warning: no return statement in function returning non-void
drivers/platform/x86/intel_pmic_gpio.c: In function 'platform_pmic_gpio_probe':
drivers/platform/x86/intel_pmic_gpio.c:283: warning: passing argument 2 of 'request_irq' from incompatible pointer type
drivers/platform/x86/intel_pmic_gpio.c: At top level:
drivers/platform/x86/intel_pmic_gpio.c:183: warning: 'pmic_bus_lock' defined but not used
drivers/platform/x86/intel_pmic_gpio.c:190: warning: 'pmic_bus_sync_unlock' defined but not used
This repairs linux-next's "platform-drivers: x86: pmic: Use request_irq
instead of chained handler". Please don't introduce bisection holes into
mainline!
Cc: Thomas Gleixner <[email protected]>
Cc: Matthew Garrett <[email protected]>
Signed-off-by: Andrew Morton <[email protected]>
---
drivers/platform/x86/intel_pmic_gpio.c | 40 ++---------------------
1 file changed, 5 insertions(+), 35 deletions(-)
diff -puN drivers/platform/x86/intel_pmic_gpio.c~drivers-platform-x86-intel_pmic_gpioc-fix-big-mess-with-i386-allmodconfig drivers/platform/x86/intel_pmic_gpio.c
--- a/drivers/platform/x86/intel_pmic_gpio.c~drivers-platform-x86-intel_pmic_gpioc-fix-big-mess-with-i386-allmodconfig
+++ a/drivers/platform/x86/intel_pmic_gpio.c
@@ -74,19 +74,6 @@ struct pmic_gpio {
u32 trigger_type;
};
-static void pmic_program_irqtype(int gpio, int type)
-{
- if (type & IRQ_TYPE_EDGE_RISING)
- intel_scu_ipc_update_register(GPIO0 + gpio, 0x20, 0x20);
- else
- intel_scu_ipc_update_register(GPIO0 + gpio, 0x00, 0x20);
-
- if (type & IRQ_TYPE_EDGE_FALLING)
- intel_scu_ipc_update_register(GPIO0 + gpio, 0x10, 0x10);
- else
- intel_scu_ipc_update_register(GPIO0 + gpio, 0x00, 0x10);
-};
-
static int pmic_gpio_direction_input(struct gpio_chip *chip, unsigned offset)
{
if (offset > 8) {
@@ -179,26 +166,6 @@ static int pmic_gpio_to_irq(struct gpio_
return pg->irq_base + offset;
}
-static void pmic_bus_lock(struct irq_data *data)
-{
- struct pmic_gpio *pg = irq_data_get_irq_chip_data(data);
-
- mutex_lock(&pg->buslock);
-}
-
-static void pmic_bus_sync_unlock(struct irq_data *data)
-{
- struct pmic_gpio *pg = irq_data_get_irq_chip_data(data);
-
- if (pg->update_type) {
- unsigned int gpio = pg->update_type & ~GPIO_UPDATE_TYPE;
-
- pmic_program_irqtype(gpio, pg->trigger_type);
- pg->update_type = 0;
- }
- mutex_unlock(&pg->buslock);
-}
-
/* the gpiointr register is read-clear, so just do nothing. */
static void pmic_irq_unmask(struct irq_data *data) { }
@@ -211,18 +178,21 @@ static struct irq_chip pmic_irqchip = {
.irq_set_type = pmic_irq_type,
};
-static irqreturn_t pmic_irq_handler(unsigned int irq, void *data)
+static irqreturn_t pmic_irq_handler(int irq, void *data)
{
struct pmic_gpio *pg = data;
u8 intsts = *((u8 *)pg->gpiointr + 4);
int gpio;
+ irqreturn_t ret = IRQ_NONE;
for (gpio = 0; gpio < 8; gpio++) {
if (intsts & (1 << gpio)) {
pr_debug("pmic pin %d triggered\n", gpio);
generic_handle_irq(pg->irq_base + gpio);
+ ret = IRQ_HANDLED;
}
}
+ return ret;
}
static int __devinit platform_pmic_gpio_probe(struct platform_device *pdev)
@@ -282,7 +252,7 @@ static int __devinit platform_pmic_gpio_
retval = request_irq(pg->irq, pmic_irq_handler, 0, "pmic", pg);
if (retval) {
- printk(KERN_WARN "pmic: Interrupt request failed\n");
+ printk(KERN_WARNING "pmic: Interrupt request failed\n");
goto err;
}
_