[patch 00/12] MIPS: Interrupt cleanups and API change preparation

[patch 00/12] MIPS: Interrupt cleanups and API change preparation

[Thomas Gleixner]

The following patch series contains the following changes:

    - Consolidation of chained interrupt handler setup/removal

    - Switch to functions which avoid a redundant interrupt
      descriptor lookup

    - Preparation of interrupt flow handlers for the 'irq' argument
      removal

The series has no dependencies and is also available as a git branch
for your convenience:

 git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git irq/mips

If you want me to carry the patches in the irq/core branch of tip,
please let me know.

Thanks,

	tglx

[patch 01/12] MIPS/jz4740: Consolidate chained IRQ handler install/remove

[Thomas Gleixner]

[-- Attachment #1: MIPS-jz4740-Consolidate-chained-IRQ-handler-install-.patch --]
[-- Type: text/plain, Size: 1152 bytes --]

Chained irq handlers usually set up handler data as well. We now have
a function to set both under irq_desc->lock. Replace the two calls
with one.

Search and conversion was done with coccinelle.

Reported-by: Russell King <rmk+kernel@arm.linux.org.uk>
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Cc: Julia Lawall <Julia.Lawall@lip6.fr>
Cc: Ralf Baechle <ralf@linux-mips.org>
Cc: linux-mips@linux-mips.org
---
 arch/mips/jz4740/gpio.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Index: tip/arch/mips/jz4740/gpio.c
===================================================================
--- tip.orig/arch/mips/jz4740/gpio.c
+++ tip/arch/mips/jz4740/gpio.c
@@ -423,8 +423,8 @@ static void jz4740_gpio_chip_init(struct
 	chip->base = ioremap(JZ4740_GPIO_BASE_ADDR + (id * 0x100), 0x100);
 
 	chip->irq = JZ4740_IRQ_INTC_GPIO(id);
-	irq_set_handler_data(chip->irq, chip);
-	irq_set_chained_handler(chip->irq, jz_gpio_irq_demux_handler);
+	irq_set_chained_handler_and_data(chip->irq,
+					 jz_gpio_irq_demux_handler, chip);
 
 	gc = irq_alloc_generic_chip(chip->gpio_chip.label, 1, chip->irq_base,
 		chip->base, handle_level_irq);

[patch 02/12] MIPS/pci-ar71xx: Consolidate chained IRQ handler install/remove

[Thomas Gleixner]

[-- Attachment #1: MIPS-pci-ar71xx-Consolidate-chained-IRQ-handler-inst.patch --]
[-- Type: text/plain, Size: 1011 bytes --]

Chained irq handlers usually set up handler data as well. We now have
a function to set both under irq_desc->lock. Replace the two calls
with one.

Search and conversion was done with coccinelle.

Reported-by: Russell King <rmk+kernel@arm.linux.org.uk>
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Cc: Julia Lawall <Julia.Lawall@lip6.fr>
Cc: Ralf Baechle <ralf@linux-mips.org>
Cc: linux-mips@linux-mips.org
---
 arch/mips/pci/pci-ar71xx.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Index: tip/arch/mips/pci/pci-ar71xx.c
===================================================================
--- tip.orig/arch/mips/pci/pci-ar71xx.c
+++ tip/arch/mips/pci/pci-ar71xx.c
@@ -312,8 +312,8 @@ static void ar71xx_pci_irq_init(struct a
 		irq_set_chip_data(i, apc);
 	}
 
-	irq_set_handler_data(apc->irq, apc);
-	irq_set_chained_handler(apc->irq, ar71xx_pci_irq_handler);
+	irq_set_chained_handler_and_data(apc->irq, ar71xx_pci_irq_handler,
+					 apc);
 }
 
 static void ar71xx_pci_reset(void)

[patch 03/12] MIPS/pci-ar724x: Consolidate chained IRQ handler install/remove

[Thomas Gleixner]

[-- Attachment #1: MIPS-pci-ar724x-Consolidate-chained-IRQ-handler-inst.patch --]
[-- Type: text/plain, Size: 1034 bytes --]

Chained irq handlers usually set up handler data as well. We now have
a function to set both under irq_desc->lock. Replace the two calls
with one.

Search and conversion was done with coccinelle.

Reported-by: Russell King <rmk+kernel@arm.linux.org.uk>
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Cc: Julia Lawall <Julia.Lawall@lip6.fr>
Cc: Ralf Baechle <ralf@linux-mips.org>
Cc: linux-mips@linux-mips.org
---
 arch/mips/pci/pci-ar724x.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Index: tip/arch/mips/pci/pci-ar724x.c
===================================================================
--- tip.orig/arch/mips/pci/pci-ar724x.c
+++ tip/arch/mips/pci/pci-ar724x.c
@@ -321,8 +321,8 @@ static void ar724x_pci_irq_init(struct a
 		irq_set_chip_data(i, apc);
 	}
 
-	irq_set_handler_data(apc->irq, apc);
-	irq_set_chained_handler(apc->irq, ar724x_pci_irq_handler);
+	irq_set_chained_handler_and_data(apc->irq, ar724x_pci_irq_handler,
+					 apc);
 }
 
 static int ar724x_pci_probe(struct platform_device *pdev)

[patch 04/12] MIPS/pci-rt3883: Consolidate chained IRQ handler install/remove

[Thomas Gleixner]

[-- Attachment #1: MIPS-pci-rt3883-Consolidate-chained-IRQ-handler-inst.patch --]
[-- Type: text/plain, Size: 952 bytes --]

Chained irq handlers usually set up handler data as well. We now have
a function to set both under irq_desc->lock. Replace the two calls
with one.

Search and conversion was done with coccinelle.

Reported-by: Russell King <rmk+kernel@arm.linux.org.uk>
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Cc: Julia Lawall <Julia.Lawall@lip6.fr>
Cc: Ralf Baechle <ralf@linux-mips.org>
Cc: linux-mips@linux-mips.org
---
 arch/mips/pci/pci-rt3883.c |    3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

Index: tip/arch/mips/pci/pci-rt3883.c
===================================================================
--- tip.orig/arch/mips/pci/pci-rt3883.c
+++ tip/arch/mips/pci/pci-rt3883.c
@@ -225,8 +225,7 @@ static int rt3883_pci_irq_init(struct de
 		return -ENODEV;
 	}
 
-	irq_set_handler_data(irq, rpc);
-	irq_set_chained_handler(irq, rt3883_pci_irq_handler);
+	irq_set_chained_handler_and_data(irq, rt3883_pci_irq_handler, rpc);
 
 	return 0;
 }

Re: [patch 04/12] MIPS/pci-rt3883: Consolidate chained IRQ handler install/remove

[Julia Lawall]

On Mon, 13 Jul 2015, Thomas Gleixner wrote:

> Chained irq handlers usually set up handler data as well. We now have
> a function to set both under irq_desc->lock. Replace the two calls
> with one.

Are the original calls remaining?  If so, should there be a semantic patch
in the kernel to check for this, in case people ut the two calls in teh
future.

julia

>
> Search and conversion was done with coccinelle.
>
> Reported-by: Russell King <rmk+kernel@arm.linux.org.uk>
> Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
> Cc: Julia Lawall <Julia.Lawall@lip6.fr>
> Cc: Ralf Baechle <ralf@linux-mips.org>
> Cc: linux-mips@linux-mips.org
> ---
>  arch/mips/pci/pci-rt3883.c |    3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
>
> Index: tip/arch/mips/pci/pci-rt3883.c
> ===================================================================
> --- tip.orig/arch/mips/pci/pci-rt3883.c
> +++ tip/arch/mips/pci/pci-rt3883.c
> @@ -225,8 +225,7 @@ static int rt3883_pci_irq_init(struct de
>  		return -ENODEV;
>  	}
>
> -	irq_set_handler_data(irq, rpc);
> -	irq_set_chained_handler(irq, rt3883_pci_irq_handler);
> +	irq_set_chained_handler_and_data(irq, rt3883_pci_irq_handler, rpc);
>
>  	return 0;
>  }
>
>
>

Re: [patch 04/12] MIPS/pci-rt3883: Consolidate chained IRQ handler install/remove

[Thomas Gleixner]

On Mon, 13 Jul 2015, Julia Lawall wrote:
> On Mon, 13 Jul 2015, Thomas Gleixner wrote:
> 
> > Chained irq handlers usually set up handler data as well. We now have
> > a function to set both under irq_desc->lock. Replace the two calls
> > with one.
> 
> Are the original calls remaining?  If so, should there be a semantic patch
> in the kernel to check for this, in case people ut the two calls in teh
> future.

irq_set_handler_data() can be used in a different context as well.

irq_set_chained_handler() has to stay for now, but we probably can
replace it with irq_set_chained_handler_and_data(irq, handler, NULL).
Have not yet done the analysis.

But yes, a semantic check for this would be nice.

Thanks,

	tglx

[patch 05/12] MIPS/irq: Use access helper irq_data_get_affinity_mask()

[Thomas Gleixner]

[-- Attachment #1: mips-irq-Use-access-helper-irq_data_get_affinity_mas.patch --]
[-- Type: text/plain, Size: 3079 bytes --]

From: Jiang Liu <jiang.liu@linux.intel.com>

This is a preparatory patch for moving irq_data struct members.

Signed-off-by: Jiang Liu <jiang.liu@linux.intel.com>
Cc: Ralf Baechle <ralf@linux-mips.org>
Cc: linux-mips@linux-mips.org
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
---
 arch/mips/bcm63xx/irq.c              |    2 +-
 arch/mips/cavium-octeon/octeon-irq.c |   14 ++++++++------
 arch/mips/pmcs-msp71xx/msp_irq_cic.c |    3 ++-
 3 files changed, 11 insertions(+), 8 deletions(-)

Index: tip/arch/mips/bcm63xx/irq.c
===================================================================
--- tip.orig/arch/mips/bcm63xx/irq.c
+++ tip/arch/mips/bcm63xx/irq.c
@@ -60,7 +60,7 @@ static inline int enable_irq_for_cpu(int
 	if (m)
 		enable &= cpumask_test_cpu(cpu, m);
 	else if (irqd_affinity_was_set(d))
-		enable &= cpumask_test_cpu(cpu, d->affinity);
+		enable &= cpumask_test_cpu(cpu, irq_data_get_affinity_mask(d));
 #endif
 	return enable;
 }
Index: tip/arch/mips/cavium-octeon/octeon-irq.c
===================================================================
--- tip.orig/arch/mips/cavium-octeon/octeon-irq.c
+++ tip/arch/mips/cavium-octeon/octeon-irq.c
@@ -225,13 +225,14 @@ static int next_cpu_for_irq(struct irq_d
 
 #ifdef CONFIG_SMP
 	int cpu;
-	int weight = cpumask_weight(data->affinity);
+	struct cpumask *mask = irq_data_get_affinity_mask(data);
+	int weight = cpumask_weight(mask);
 	struct octeon_ciu_chip_data *cd = irq_data_get_irq_chip_data(data);
 
 	if (weight > 1) {
 		cpu = cd->current_cpu;
 		for (;;) {
-			cpu = cpumask_next(cpu, data->affinity);
+			cpu = cpumask_next(cpu, mask);
 			if (cpu >= nr_cpu_ids) {
 				cpu = -1;
 				continue;
@@ -240,7 +241,7 @@ static int next_cpu_for_irq(struct irq_d
 			}
 		}
 	} else if (weight == 1) {
-		cpu = cpumask_first(data->affinity);
+		cpu = cpumask_first(mask);
 	} else {
 		cpu = smp_processor_id();
 	}
@@ -712,16 +713,17 @@ static void octeon_irq_cpu_offline_ciu(s
 {
 	int cpu = smp_processor_id();
 	cpumask_t new_affinity;
+	struct cpumask *mask = irq_data_get_affinity_mask(data);
 
-	if (!cpumask_test_cpu(cpu, data->affinity))
+	if (!cpumask_test_cpu(cpu, mask))
 		return;
 
-	if (cpumask_weight(data->affinity) > 1) {
+	if (cpumask_weight(mask) > 1) {
 		/*
 		 * It has multi CPU affinity, just remove this CPU
 		 * from the affinity set.
 		 */
-		cpumask_copy(&new_affinity, data->affinity);
+		cpumask_copy(&new_affinity, mask);
 		cpumask_clear_cpu(cpu, &new_affinity);
 	} else {
 		/* Otherwise, put it on lowest numbered online CPU. */
Index: tip/arch/mips/pmcs-msp71xx/msp_irq_cic.c
===================================================================
--- tip.orig/arch/mips/pmcs-msp71xx/msp_irq_cic.c
+++ tip/arch/mips/pmcs-msp71xx/msp_irq_cic.c
@@ -88,7 +88,8 @@ static void unmask_cic_irq(struct irq_da
 	* Make sure we have IRQ affinity.  It may have changed while
 	* we were processing the IRQ.
 	*/
-	if (!cpumask_test_cpu(smp_processor_id(), d->affinity))
+	if (!cpumask_test_cpu(smp_processor_id(),
+			      irq_data_get_affinity_mask(d)))
 		return;
 #endif
 

[patch 06/12] MIPS/alchemy: Use irq_set_chip_handler_name_locked()

[Thomas Gleixner]

[-- Attachment #1: MIPS-alchemy-Use-irq_set_chip_handler_name_locked.patch --]
[-- Type: text/plain, Size: 996 bytes --]

Hand in irq_data and avoid the redundant lookup of irq_desc.

Originally-from: Jiang Liu <jiang.liu@linux.intel.com>
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
---
 arch/mips/alchemy/common/irq.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Index: tip/arch/mips/alchemy/common/irq.c
===================================================================
--- tip.orig/arch/mips/alchemy/common/irq.c
+++ tip/arch/mips/alchemy/common/irq.c
@@ -491,7 +491,7 @@ static int au1x_ic_settype(struct irq_da
 	default:
 		ret = -EINVAL;
 	}
-	__irq_set_chip_handler_name_locked(d->irq, chip, handler, name);
+	irq_set_chip_handler_name_locked(d, chip, handler, name);
 
 	wmb();
 
@@ -703,7 +703,7 @@ static int au1300_gpic_settype(struct ir
 		return -EINVAL;
 	}
 
-	__irq_set_chip_handler_name_locked(d->irq, &au1300_gpic, hdl, name);
+	irq_set_chip_handler_name_locked(d, &au1300_gpic, hdl, name);
 
 	au1300_gpic_chgcfg(d->irq - ALCHEMY_GPIC_INT_BASE, GPIC_CFG_IC_MASK, s);
 

[patch 07/12] MIPS/bcm63xx: Use irq_set_handler_locked()

[Thomas Gleixner]

[-- Attachment #1: mips-bcm63xx-Use-irq_set_handler_locked.patch --]
[-- Type: text/plain, Size: 1016 bytes --]

Use irq_set_handler_locked() as it avoids a redundant lookup of the
irq descriptor.

Search and replacement was done with coccinelle.

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Cc: Jiang Liu <jiang.liu@linux.intel.com>
Cc: Julia Lawall <julia.lawall@lip6.fr>
Cc: Ralf Baechle <ralf@linux-mips.org>
Cc: linux-mips@linux-mips.org
---
 arch/mips/bcm63xx/irq.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Index: tip/arch/mips/bcm63xx/irq.c
===================================================================
--- tip.orig/arch/mips/bcm63xx/irq.c
+++ tip/arch/mips/bcm63xx/irq.c
@@ -365,9 +365,9 @@ static int bcm63xx_external_irq_set_type
 
 	irqd_set_trigger_type(d, flow_type);
 	if (flow_type & (IRQ_TYPE_LEVEL_LOW | IRQ_TYPE_LEVEL_HIGH))
-		__irq_set_handler_locked(d->irq, handle_level_irq);
+		irq_set_handler_locked(d, handle_level_irq);
 	else
-		__irq_set_handler_locked(d->irq, handle_edge_irq);
+		irq_set_handler_locked(d, handle_edge_irq);
 
 	return IRQ_SET_MASK_OK_NOCOPY;
 }

[patch 08/12] MIPS/alchemy: Remove pointless irqdisable/enable

[Thomas Gleixner]

[-- Attachment #1: MIPS-alchemy--Remove-pointless-irqdisable-enable.patch --]
[-- Type: text/plain, Size: 1195 bytes --]

bcsr_csc_handler() is a cascading interrupt handler. It has a
disable_irq_nosync()/enable_irq() pair around the generic_handle_irq()
call. The value of this disable/enable is zero because its a complete
noop:

disable_irq_nosync() merily increments the disable count without
actually masking the interrupt. enable_irq() soleley decrements the
disable count without touching the interrupt chip. The interrupt
cannot arrive again because the complete call chain runs with
interrupts disabled.

Remove it.

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Cc: Ralf Baechle <ralf@linux-mips.org>
Cc: linux-mips@linux-mips.org
---
 arch/mips/alchemy/devboards/bcsr.c |    2 --
 1 file changed, 2 deletions(-)

Index: tip/arch/mips/alchemy/devboards/bcsr.c
===================================================================
--- tip.orig/arch/mips/alchemy/devboards/bcsr.c
+++ tip/arch/mips/alchemy/devboards/bcsr.c
@@ -89,9 +89,7 @@ static void bcsr_csc_handler(unsigned in
 {
 	unsigned short bisr = __raw_readw(bcsr_virt + BCSR_REG_INTSTAT);
 
-	disable_irq_nosync(irq);
 	generic_handle_irq(bcsr_csc_base + __ffs(bisr));
-	enable_irq(irq);
 }
 
 static void bcsr_irq_mask(struct irq_data *d)

Re: [patch 08/12] MIPS/alchemy: Remove pointless irqdisable/enable

[Manuel Lauss]

On Mon, Jul 13, 2015 at 10:46 PM, Thomas Gleixner <tglx@linutronix.de> wrote:
> bcsr_csc_handler() is a cascading interrupt handler. It has a
> disable_irq_nosync()/enable_irq() pair around the generic_handle_irq()
> call. The value of this disable/enable is zero because its a complete
> noop:
>
> disable_irq_nosync() merily increments the disable count without
> actually masking the interrupt. enable_irq() soleley decrements the
> disable count without touching the interrupt chip. The interrupt
> cannot arrive again because the complete call chain runs with
> interrupts disabled.
>
> Remove it.

Is there another patch this one depends on?  The DB1300 board doesn't
boot (i.e. interrupts from the cpld aren't serviced) with this patch applied:
(irq 136 is the first serviced by the bcsr cpld):

irq 136: nobody cared (try booting with the "irqpoll" option)
CPU: 0 PID: 50 Comm: kworker/u2:2 Not tainted
4.1.0-db1xxx-12807-g1ced2d0-dirty #8
Workqueue: events_unbound async_run_entry_fn
Stack : 8090c3ec 8090c3c4 00000000 809d0000 00000000 80153668 80908814 00000032
          80a03828 8090c3c4 8093b2fc 8fb736e4 80908814 807e7f3c
00000000 8013f8dc
          00000000 00000000 8fb736e4 8fb73704 80908814 8013726c
00000000 00000002
          00000000 00000000 00000000 00000000 00000000 00000000
00000000 00000000
          6e657665 755f7374 756f626e 0000646e 00000000 00000000
8fc32500 8fc32c00
          ...
Call Trace:
[<8010ee6c>] show_stack+0x64/0x7c
[<801571c4>] __report_bad_irq.isra.0+0x40/0x100
[<801574d8>] note_interrupt+0x1e0/0x338
[<80154d3c>] handle_irq_event_percpu+0xe8/0x1a0
[<80154e34>] handle_irq_event+0x40/0x6c
[<80157c8c>] handle_level_irq+0xac/0x16c
[<801543e8>] generic_handle_irq+0x44/0x5c
[<801543e8>] generic_handle_irq+0x44/0x5c
[<801543e8>] generic_handle_irq+0x44/0x5c
[<8010bd04>] do_IRQ+0x18/0x24
[<8010a018>] ret_from_irq+0x0/0x4
[<801d95fc>] kmem_cache_alloc+0x0/0xf8
[<80216b38>] alloc_buffer_head+0x1c/0x70
[<80216cb0>] alloc_page_buffers+0xbc/0x134
[<80216d4c>] create_empty_buffers+0x24/0x14c
[<80216ee0>] create_page_buffers+0x6c/0x94
[<8021896c>] block_read_full_page+0x48/0x4b8
[<8019dac8>] do_read_cache_page+0xac/0x278
[<8019dcb4>] read_cache_page+0x20/0x2c
[<803da774>] read_dev_sector+0x34/0xc0
[<803dc7ec>] read_lba.isra.0+0xe8/0x200
[<803dcb80>] is_gpt_valid+0x27c/0x318
[<803dcd40>] efi_partition+0x124/0xb44
[<803db9b4>] check_partition+0x108/0x254
[<803dae84>] rescan_partitions+0x104/0x384
[<8021cc8c>] __blkdev_get+0x318/0x440
[<8021d8d0>] blkdev_get+0x11c/0x330
[<803d8c08>] add_disk+0x380/0x488
[<8048e350>] sd_probe_async+0x100/0x228
[<8013d7ec>] async_run_entry_fn+0x4c/0x118
[<80135080>] process_one_work+0x130/0x40c
[<801354c8>] worker_thread+0x16c/0x5a8
[<8013af04>] kthread+0xd4/0xec
[<8010a068>] ret_from_kernel_thread+0x14/0x1c

handlers:
[<804ab75c>] ata_sff_interrupt
Disabling IRQ #136


Manuel

Re: [patch 08/12] MIPS/alchemy: Remove pointless irqdisable/enable

[Thomas Gleixner]

On Tue, 14 Jul 2015, Manuel Lauss wrote:

> On Mon, Jul 13, 2015 at 10:46 PM, Thomas Gleixner <tglx@linutronix.de> wrote:
> > bcsr_csc_handler() is a cascading interrupt handler. It has a
> > disable_irq_nosync()/enable_irq() pair around the generic_handle_irq()
> > call. The value of this disable/enable is zero because its a complete
> > noop:
> >
> > disable_irq_nosync() merily increments the disable count without
> > actually masking the interrupt. enable_irq() soleley decrements the
> > disable count without touching the interrupt chip. The interrupt
> > cannot arrive again because the complete call chain runs with
> > interrupts disabled.
> >
> > Remove it.
> 
> Is there another patch this one depends on?  The DB1300 board doesn't

No.

> boot (i.e. interrupts from the cpld aren't serviced) with this patch applied:
> (irq 136 is the first serviced by the bcsr cpld):
> 
> irq 136: nobody cared (try booting with the "irqpoll" option)

That's weird. Looking deeper, enable_irq() actually calls
chip->unmask() unconditionally. So it seems the chip is sensitive to
that.

Does the following patch on top fix things again?

Thanks,

	tglx
----
diff --git a/arch/mips/alchemy/devboards/bcsr.c b/arch/mips/alchemy/devboards/bcsr.c
index 3a24f2d6ecfd..ec47abe580c6 100644
--- a/arch/mips/alchemy/devboards/bcsr.c
+++ b/arch/mips/alchemy/devboards/bcsr.c
@@ -88,8 +88,11 @@ EXPORT_SYMBOL_GPL(bcsr_mod);
 static void bcsr_csc_handler(unsigned int irq, struct irq_desc *d)
 {
 	unsigned short bisr = __raw_readw(bcsr_virt + BCSR_REG_INTSTAT);
+	struct irq_chip *chip = irq_desc_get_chip(d);
 
+	chained_irq_enter(chip, d);
 	generic_handle_irq(bcsr_csc_base + __ffs(bisr));
+	chained_irq_exit(chip, d);
 }
 
 static void bcsr_irq_mask(struct irq_data *d)

Re: [patch 08/12] MIPS/alchemy: Remove pointless irqdisable/enable

[Manuel Lauss]

On Tue, Jul 14, 2015 at 10:16 AM, Thomas Gleixner <tglx@linutronix.de> wrote:
> On Tue, 14 Jul 2015, Manuel Lauss wrote:
>
>> On Mon, Jul 13, 2015 at 10:46 PM, Thomas Gleixner <tglx@linutronix.de> wrote:
>> > bcsr_csc_handler() is a cascading interrupt handler. It has a
>> > disable_irq_nosync()/enable_irq() pair around the generic_handle_irq()
>> > call. The value of this disable/enable is zero because its a complete
>> > noop:
>> >
>> > disable_irq_nosync() merily increments the disable count without
>> > actually masking the interrupt. enable_irq() soleley decrements the
>> > disable count without touching the interrupt chip. The interrupt
>> > cannot arrive again because the complete call chain runs with
>> > interrupts disabled.
>> >
>> > Remove it.
>>
>> Is there another patch this one depends on?  The DB1300 board doesn't
>
> No.
>
>> boot (i.e. interrupts from the cpld aren't serviced) with this patch applied:
>> (irq 136 is the first serviced by the bcsr cpld):
>>
>> irq 136: nobody cared (try booting with the "irqpoll" option)
>
> That's weird. Looking deeper, enable_irq() actually calls
> chip->unmask() unconditionally. So it seems the chip is sensitive to
> that.
>
> Does the following patch on top fix things again?
>
> Thanks,
>
>         tglx
> ----
> diff --git a/arch/mips/alchemy/devboards/bcsr.c b/arch/mips/alchemy/devboards/bcsr.c
> index 3a24f2d6ecfd..ec47abe580c6 100644
> --- a/arch/mips/alchemy/devboards/bcsr.c
> +++ b/arch/mips/alchemy/devboards/bcsr.c
> @@ -88,8 +88,11 @@ EXPORT_SYMBOL_GPL(bcsr_mod);
>  static void bcsr_csc_handler(unsigned int irq, struct irq_desc *d)
>  {
>         unsigned short bisr = __raw_readw(bcsr_virt + BCSR_REG_INTSTAT);
> +       struct irq_chip *chip = irq_desc_get_chip(d);
>
> +       chained_irq_enter(chip, d);
>         generic_handle_irq(bcsr_csc_base + __ffs(bisr));
> +       chained_irq_exit(chip, d);
>  }
>
>  static void bcsr_irq_mask(struct irq_data *d)


Yes.  Add #include <linux/irqchip/chained_irq.h> on top and it works again.
This hardware is problematic, an older variant with identical verilog
code in the cpld's
irq unit works fine without this.

Thanks,
    Manuel

Re: [patch 08/12] MIPS/alchemy: Remove pointless irqdisable/enable

[Ralf Baechle]

On Tue, Jul 14, 2015 at 10:55:08AM +0200, Manuel Lauss wrote:

> On Tue, Jul 14, 2015 at 10:16 AM, Thomas Gleixner <tglx@linutronix.de> wrote:
> > On Tue, 14 Jul 2015, Manuel Lauss wrote:
> >
> >> On Mon, Jul 13, 2015 at 10:46 PM, Thomas Gleixner <tglx@linutronix.de> wrote:
> >> > bcsr_csc_handler() is a cascading interrupt handler. It has a
> >> > disable_irq_nosync()/enable_irq() pair around the generic_handle_irq()
> >> > call. The value of this disable/enable is zero because its a complete
> >> > noop:
> >> >
> >> > disable_irq_nosync() merily increments the disable count without
> >> > actually masking the interrupt. enable_irq() soleley decrements the
> >> > disable count without touching the interrupt chip. The interrupt
> >> > cannot arrive again because the complete call chain runs with
> >> > interrupts disabled.
> >> >
> >> > Remove it.
> >>
> >> Is there another patch this one depends on?  The DB1300 board doesn't
> >
> > No.
> >
> >> boot (i.e. interrupts from the cpld aren't serviced) with this patch applied:
> >> (irq 136 is the first serviced by the bcsr cpld):
> >>
> >> irq 136: nobody cared (try booting with the "irqpoll" option)
> >
> > That's weird. Looking deeper, enable_irq() actually calls
> > chip->unmask() unconditionally. So it seems the chip is sensitive to
> > that.
> >
> > Does the following patch on top fix things again?
> >
> > Thanks,
> >
> >         tglx
> > ----
> > diff --git a/arch/mips/alchemy/devboards/bcsr.c b/arch/mips/alchemy/devboards/bcsr.c
> > index 3a24f2d6ecfd..ec47abe580c6 100644
> > --- a/arch/mips/alchemy/devboards/bcsr.c
> > +++ b/arch/mips/alchemy/devboards/bcsr.c
> > @@ -88,8 +88,11 @@ EXPORT_SYMBOL_GPL(bcsr_mod);
> >  static void bcsr_csc_handler(unsigned int irq, struct irq_desc *d)
> >  {
> >         unsigned short bisr = __raw_readw(bcsr_virt + BCSR_REG_INTSTAT);
> > +       struct irq_chip *chip = irq_desc_get_chip(d);
> >
> > +       chained_irq_enter(chip, d);
> >         generic_handle_irq(bcsr_csc_base + __ffs(bisr));
> > +       chained_irq_exit(chip, d);
> >  }
> >
> >  static void bcsr_irq_mask(struct irq_data *d)
> 
> 
> Yes.  Add #include <linux/irqchip/chained_irq.h> on top and it works again.
> This hardware is problematic, an older variant with identical verilog
> code in the cpld's
> irq unit works fine without this.

So shall I merge both patches and the header file change together or?

  Ralf

Re: [patch 08/12] MIPS/alchemy: Remove pointless irqdisable/enable

[Thomas Gleixner]

On Tue, 14 Jul 2015, Ralf Baechle wrote:
> On Tue, Jul 14, 2015 at 10:55:08AM +0200, Manuel Lauss wrote:
> > Yes.  Add #include <linux/irqchip/chained_irq.h> on top and it works again.
> > This hardware is problematic, an older variant with identical verilog
> > code in the cpld's
> > irq unit works fine without this.
> 
> So shall I merge both patches and the header file change together or?

Yes please.

[patch 09/12] MIPS/ath91: Remove pointless irqdisable/enable

[Thomas Gleixner]

[-- Attachment #1: MIPS-ath91--Remove-pointless-irqdisable-enable.patch --]
[-- Type: text/plain, Size: 2502 bytes --]

The various interrupt flow handlers in ath79 are cascading interrupt
handlers. They all have a disable_irq_nosync()/enable_irq() pair
around the generic_handle_irq() call. The value of this disable/enable
is zero because its a complete noop:

disable_irq_nosync() merily increments the disable count without
actually masking the interrupt. enable_irq() soleley decrements the
disable count without touching the interrupt chip. The interrupt
cannot arrive again because the complete call chain runs with
interrupts disabled.

Remove it.

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Cc: Ralf Baechle <ralf@linux-mips.org>
Cc: linux-mips@linux-mips.org
---
 arch/mips/ath79/irq.c |   18 ++----------------
 1 file changed, 2 insertions(+), 16 deletions(-)

Index: tip/arch/mips/ath79/irq.c
===================================================================
--- tip.orig/arch/mips/ath79/irq.c
+++ tip/arch/mips/ath79/irq.c
@@ -124,8 +124,6 @@ static void ar934x_ip2_irq_dispatch(unsi
 {
 	u32 status;
 
-	disable_irq_nosync(irq);
-
 	status = ath79_reset_rr(AR934X_RESET_REG_PCIE_WMAC_INT_STATUS);
 
 	if (status & AR934X_PCIE_WMAC_INT_PCIE_ALL) {
@@ -137,8 +135,6 @@ static void ar934x_ip2_irq_dispatch(unsi
 	} else {
 		spurious_interrupt();
 	}
-
-	enable_irq(irq);
 }
 
 static void ar934x_ip2_irq_init(void)
@@ -157,14 +153,12 @@ static void qca955x_ip2_irq_dispatch(uns
 {
 	u32 status;
 
-	disable_irq_nosync(irq);
-
 	status = ath79_reset_rr(QCA955X_RESET_REG_EXT_INT_STATUS);
 	status &= QCA955X_EXT_INT_PCIE_RC1_ALL | QCA955X_EXT_INT_WMAC_ALL;
 
 	if (status == 0) {
 		spurious_interrupt();
-		goto enable;
+		return;
 	}
 
 	if (status & QCA955X_EXT_INT_PCIE_RC1_ALL) {
@@ -176,17 +170,12 @@ static void qca955x_ip2_irq_dispatch(uns
 		/* TODO: flush DDR? */
 		generic_handle_irq(ATH79_IP2_IRQ(1));
 	}
-
-enable:
-	enable_irq(irq);
 }
 
 static void qca955x_ip3_irq_dispatch(unsigned int irq, struct irq_desc *desc)
 {
 	u32 status;
 
-	disable_irq_nosync(irq);
-
 	status = ath79_reset_rr(QCA955X_RESET_REG_EXT_INT_STATUS);
 	status &= QCA955X_EXT_INT_PCIE_RC2_ALL |
 		  QCA955X_EXT_INT_USB1 |
@@ -194,7 +183,7 @@ static void qca955x_ip3_irq_dispatch(uns
 
 	if (status == 0) {
 		spurious_interrupt();
-		goto enable;
+		return;
 	}
 
 	if (status & QCA955X_EXT_INT_USB1) {
@@ -211,9 +200,6 @@ static void qca955x_ip3_irq_dispatch(uns
 		/* TODO: flush DDR? */
 		generic_handle_irq(ATH79_IP3_IRQ(2));
 	}
-
-enable:
-	enable_irq(irq);
 }
 
 static void qca955x_irq_init(void)

[patch 10/12] MIPS/cavium/octeon: Replace the homebrewn flow handler

[Thomas Gleixner]

[-- Attachment #1: MIPS-cavium-octeon--Replace-the-homebrewn-flow-handler.patch --]
[-- Type: text/plain, Size: 2372 bytes --]

The gpio interrupt handling of octeon contains a homebrewn flow
handler which calls either handle_level_irq or handle_edge_irq
depending on the trigger type. Thats an extra conditional and call in
the interrupt handling path. The proper way to handle different types
and therefor different flows is to update the handler in the
irq_set_type() callback.

Remove the extra indirection and add the handler update to
octeon_irq_ciu_gpio_set_type(). At mapping time it defaults to
handle_level_irq which gets updated if the device tree contains a
different trigger type.

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Cc: Ralf Baechle <ralf@linux-mips.org>
Cc: David Daney <david.daney@cavium.com>
Cc: Jiang Liu <jiang.liu@linux.intel.com>
Cc: linux-mips@linux-mips.org
---
 arch/mips/cavium-octeon/octeon-irq.c |   22 +++++++++++-----------
 1 file changed, 11 insertions(+), 11 deletions(-)

Index: tip/arch/mips/cavium-octeon/octeon-irq.c
===================================================================
--- tip.orig/arch/mips/cavium-octeon/octeon-irq.c
+++ tip/arch/mips/cavium-octeon/octeon-irq.c
@@ -663,6 +663,11 @@ static int octeon_irq_ciu_gpio_set_type(
 	irqd_set_trigger_type(data, t);
 	octeon_irq_gpio_setup(data);
 
+	if (irqd_get_trigger_type(data) & IRQ_TYPE_EDGE_BOTH)
+		irq_set_handler_locked(data, handle_edge_irq);
+	else
+		irq_set_handler_locked(data, handle_level_irq);
+
 	return IRQ_SET_MASK_OK;
 }
 
@@ -697,16 +702,6 @@ static void octeon_irq_ciu_gpio_ack(stru
 	cvmx_write_csr(CVMX_GPIO_INT_CLR, mask);
 }
 
-static void octeon_irq_handle_trigger(unsigned int irq, struct irq_desc *desc)
-{
-	struct irq_data *data = irq_desc_get_irq_data(desc);
-
-	if (irqd_get_trigger_type(data) & IRQ_TYPE_EDGE_BOTH)
-		handle_edge_irq(irq, desc);
-	else
-		handle_level_irq(irq, desc);
-}
-
 #ifdef CONFIG_SMP
 
 static void octeon_irq_cpu_offline_ciu(struct irq_data *data)
@@ -1229,8 +1224,13 @@ static int octeon_irq_gpio_map(struct ir
 		octeon_irq_ciu_to_irq[line][bit] != 0)
 		return -EINVAL;
 
+	/*
+	 * Default to handle_level_irq. If the DT contains a different
+	 * trigger type, it will call the irq_set_type callback and
+	 * the handler gets updated.
+	 */
 	r = octeon_irq_set_ciu_mapping(virq, line, bit, hw,
-		octeon_irq_gpio_chip, octeon_irq_handle_trigger);
+				       octeon_irq_gpio_chip, handle_level_irq);
 	return r;
 }
 

Re: [patch 10/12] MIPS/cavium/octeon: Replace the homebrewn flow handler

[David Daney]

On 07/13/2015 01:46 PM, Thomas Gleixner wrote:
> The gpio interrupt handling of octeon contains a homebrewn flow
> handler which calls either handle_level_irq or handle_edge_irq
> depending on the trigger type. Thats an extra conditional and call in
> the interrupt handling path. The proper way to handle different types
> and therefor different flows is to update the handler in the
> irq_set_type() callback.
>
> Remove the extra indirection and add the handler update to
> octeon_irq_ciu_gpio_set_type(). At mapping time it defaults to
> handle_level_irq which gets updated if the device tree contains a
> different trigger type.
>
> Signed-off-by: Thomas Gleixner<tglx@linutronix.de>

This looks sane, not tested, but ...

Acked-by: David Daney <david.daney@cavium.com>



> Cc: Ralf Baechle<ralf@linux-mips.org>
> Cc: David Daney<david.daney@cavium.com>
> Cc: Jiang Liu<jiang.liu@linux.intel.com>
> Cc:linux-mips@linux-mips.org
> ---
>   arch/mips/cavium-octeon/octeon-irq.c |   22 +++++++++++-----------
>   1 file changed, 11 insertions(+), 11 deletions(-)
>
> Index: tip/arch/mips/cavium-octeon/octeon-irq.c
> ===================================================================
> --- tip.orig/arch/mips/cavium-octeon/octeon-irq.c
> +++ tip/arch/mips/cavium-octeon/octeon-irq.c
> @@ -663,6 +663,11 @@ static int octeon_irq_ciu_gpio_set_type(
>   	irqd_set_trigger_type(data, t);
>   	octeon_irq_gpio_setup(data);
>
> +	if (irqd_get_trigger_type(data) & IRQ_TYPE_EDGE_BOTH)
> +		irq_set_handler_locked(data, handle_edge_irq);
> +	else
> +		irq_set_handler_locked(data, handle_level_irq);
> +
>   	return IRQ_SET_MASK_OK;
>   }
>
> @@ -697,16 +702,6 @@ static void octeon_irq_ciu_gpio_ack(stru
>   	cvmx_write_csr(CVMX_GPIO_INT_CLR, mask);
>   }
>
> -static void octeon_irq_handle_trigger(unsigned int irq, struct irq_desc *desc)
> -{
> -	struct irq_data *data = irq_desc_get_irq_data(desc);
> -
> -	if (irqd_get_trigger_type(data) & IRQ_TYPE_EDGE_BOTH)
> -		handle_edge_irq(irq, desc);
> -	else
> -		handle_level_irq(irq, desc);
> -}
> -
>   #ifdef CONFIG_SMP
>
>   static void octeon_irq_cpu_offline_ciu(struct irq_data *data)
> @@ -1229,8 +1224,13 @@ static int octeon_irq_gpio_map(struct ir
>   		octeon_irq_ciu_to_irq[line][bit] != 0)
>   		return -EINVAL;
>
> +	/*
> +	 * Default to handle_level_irq. If the DT contains a different
> +	 * trigger type, it will call the irq_set_type callback and
> +	 * the handler gets updated.
> +	 */
>   	r = octeon_irq_set_ciu_mapping(virq, line, bit, hw,
> -		octeon_irq_gpio_chip, octeon_irq_handle_trigger);
> +				       octeon_irq_gpio_chip, handle_level_irq);
>   	return r;
>   }
>
>
>
>
>
>

[patch 11/12] MIPS/netlogic: Prepare ipi handlers for irq argument removal

[Thomas Gleixner]

[-- Attachment #1: MIPS-netlogic--Prepare-ipi-handlers-for-irq-argument-removal.patch --]
[-- Type: text/plain, Size: 1526 bytes --]

The irq argument of most interrupt flow handlers is unused or merily
used instead of a local variable. The handlers which need the irq
argument can retrieve the irq number from the irq descriptor.

Search and update was done with coccinelle and the invaluable help of
Julia Lawall.

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Cc: Julia Lawall <Julia.Lawall@lip6.fr>
Cc: Jiang Liu <jiang.liu@linux.intel.com>
Cc: Ralf Baechle <ralf@linux-mips.org>
Cc: linux-mips@linux-mips.org
---
 arch/mips/netlogic/common/smp.c |    6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

Index: tip/arch/mips/netlogic/common/smp.c
===================================================================
--- tip.orig/arch/mips/netlogic/common/smp.c
+++ tip/arch/mips/netlogic/common/smp.c
@@ -82,8 +82,9 @@ void nlm_send_ipi_mask(const struct cpum
 }
 
 /* IRQ_IPI_SMP_FUNCTION Handler */
-void nlm_smp_function_ipi_handler(unsigned int irq, struct irq_desc *desc)
+void nlm_smp_function_ipi_handler(unsigned int __irq, struct irq_desc *desc)
 {
+	unsigned int irq = irq_desc_get_irq(desc);
 	clear_c0_eimr(irq);
 	ack_c0_eirr(irq);
 	smp_call_function_interrupt();
@@ -91,8 +92,9 @@ void nlm_smp_function_ipi_handler(unsign
 }
 
 /* IRQ_IPI_SMP_RESCHEDULE  handler */
-void nlm_smp_resched_ipi_handler(unsigned int irq, struct irq_desc *desc)
+void nlm_smp_resched_ipi_handler(unsigned int __irq, struct irq_desc *desc)
 {
+	unsigned int irq = irq_desc_get_irq(desc);
 	clear_c0_eimr(irq);
 	ack_c0_eirr(irq);
 	scheduler_ipi();

[patch 12/12] MIPS/PCI/rt3883: Prepare rt3883_pci_irq_handler for irq argument removal

[Thomas Gleixner]

[-- Attachment #1: MIPS-PCI-rt3883--Prepare-rt3883_pci_irq_handler-for-irq-argument-removal.patch --]
[-- Type: text/plain, Size: 1335 bytes --]

The irq argument of most interrupt flow handlers is unused or merily
used instead of a local variable. The handlers which need the irq
argument can retrieve the irq number from the irq descriptor.

Search and update was done with coccinelle and the invaluable help of
Julia Lawall.

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Cc: Julia Lawall <Julia.Lawall@lip6.fr>
Cc: Jiang Liu <jiang.liu@linux.intel.com>
Cc: Ralf Baechle <ralf@linux-mips.org>
Cc: linux-mips@linux-mips.org
---
---
 arch/mips/pci/pci-rt3883.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Index: tip/arch/mips/pci/pci-rt3883.c
===================================================================
--- tip.orig/arch/mips/pci/pci-rt3883.c
+++ tip/arch/mips/pci/pci-rt3883.c
@@ -129,7 +129,7 @@ static void rt3883_pci_write_cfg32(struc
 	rt3883_pci_w32(rpc, val, RT3883_PCI_REG_CFGDATA);
 }
 
-static void rt3883_pci_irq_handler(unsigned int irq, struct irq_desc *desc)
+static void rt3883_pci_irq_handler(unsigned int __irq, struct irq_desc *desc)
 {
 	struct rt3883_pci_controller *rpc;
 	u32 pending;
@@ -145,7 +145,7 @@ static void rt3883_pci_irq_handler(unsig
 	}
 
 	while (pending) {
-		unsigned bit = __ffs(pending);
+		unsigned irq, bit = __ffs(pending);
 
 		irq = irq_find_mapping(rpc->irq_domain, bit);
 		generic_handle_irq(irq);