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 <[email protected]>
Cc: Ralf Baechle <[email protected]>
Cc: [email protected]
---
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)
On Mon, Jul 13, 2015 at 10:46 PM, Thomas Gleixner <[email protected]> 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
On Tue, 14 Jul 2015, Manuel Lauss wrote:
> On Mon, Jul 13, 2015 at 10:46 PM, Thomas Gleixner <[email protected]> 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)
On Tue, Jul 14, 2015 at 10:16 AM, Thomas Gleixner <[email protected]> wrote:
> On Tue, 14 Jul 2015, Manuel Lauss wrote:
>
>> On Mon, Jul 13, 2015 at 10:46 PM, Thomas Gleixner <[email protected]> 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
On Tue, Jul 14, 2015 at 10:55:08AM +0200, Manuel Lauss wrote:
> On Tue, Jul 14, 2015 at 10:16 AM, Thomas Gleixner <[email protected]> wrote:
> > On Tue, 14 Jul 2015, Manuel Lauss wrote:
> >
> >> On Mon, Jul 13, 2015 at 10:46 PM, Thomas Gleixner <[email protected]> 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
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.