All of lore.kernel.org
 help / color / mirror / Atom feed
* m68k: [v5] Convert to genirq (WIP)
@ 2011-08-26 20:34 Geert Uytterhoeven
  2011-08-28  2:08 ` Finn Thain
  0 siblings, 1 reply; 10+ messages in thread
From: Geert Uytterhoeven @ 2011-08-26 20:34 UTC (permalink / raw)
  To: Linux/m68k; +Cc: linux-kernel, Thomas Gleixner

I rebased and updated my m68k-genirq branch at

git://git.kernel.org:/pub/scm/linux/kernel/git/geert/linux-m68k.git m68k-genirq
http://git.kernel.org/?p=linux/kernel/git/geert/linux-m68k.git;a=shortlog;h=refs/heads/m68k-genirq

[PATCH 01/24] ide-{cd,floppy,tape}, keyboard: Do not include <linux/irq.>
[PATCH 02/24] m68k/irq: Rename irq_controller to irq_chip
[PATCH 03/24] m68k/irq: Kill irq_node_t typedef, always use struct irq_node
[PATCH 04/24] m68k/irq: Rename irq_node to irq_data
[PATCH 05/24] m68k/irq: Switch irq_chip methods to "struct irq_data *data"
[PATCH 06/24] m68k/irq: Rename setup_irq() to m68k_setup_irq() and
make it static
[PATCH 07/24] m68k/irq: Extract irq_set_chip()
[PATCH 08/24] m68k/irq: Add m68k_setup_irq_controller()
[PATCH 09/24] m68k/irq: Rename {,__}m68k_handle_int()
[PATCH 10/24] m68k/irq: Remove obsolete IRQ_FLG_* definitions and users
[PATCH 11/24] m68k/irq: Add genirq support
[PATCH 12/24] m68k/atari: Convert Atari to genirq
[PATCH 13/24] m68k/atari: Remove code and comments about different irq types
[PATCH 14/24] m68k/amiga: Refactor amiints.c
[PATCH 15/24] m68k/amiga: Convert Amiga to genirq
[PATCH 16/24] m68k/amiga: Optimize interrupts using chain handlers
[PATCH 17/24] m68k/mac: Convert Mac to genirq
[PATCH 18/24] m68k/mac: Optimize interrupts using chain handlers
[PATCH 19/24] m68k/hp300: Convert HP9000/300 and HP9000/400 to genirq
[PATCH 20/24] m68k/vme: Convert VME to genirq
[PATCH 21/24] m68k/sun3: Use the kstat_irqs_cpu() wrapper
[PATCH 22/24] m68k/apollo: Convert Apollo to genirq
[PATCH 23/24] m68k/sun3: Convert Sun3/3x to genirq
[PATCH 24/24] m68k/q40: Convert Q40/Q60 to genirq

Changes:
  - [PATCH 11/24]: Keep m68k_setup_auto_interrupt(), it's still needed for Q40
  - [PATCH 18/24]: m68k/mac: Optimize interrupts using chain handlers (NEW)
  - [PATCH 24/24]: m68k/q40: Convert Q40/Q60 to genirq (NEW)

Todo:
 - Removal of legacy code inside #ifndef CONFIG_GENERIC_HARDIRQ,
 - Proper patch submission emails,
 - Get it merged (For 3.3? Or even 3.2?).

Oh, I almost forgot: testing ;-)! So far I tested it on Atari (ARAnyM)
and Amiga only.

Thanks for testing and feedback!

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: m68k: [v5] Convert to genirq (WIP)
  2011-08-26 20:34 m68k: [v5] Convert to genirq (WIP) Geert Uytterhoeven
@ 2011-08-28  2:08 ` Finn Thain
  2011-08-28  7:53   ` Geert Uytterhoeven
  2011-09-02 12:14   ` Finn Thain
  0 siblings, 2 replies; 10+ messages in thread
From: Finn Thain @ 2011-08-28  2:08 UTC (permalink / raw)
  To: Geert Uytterhoeven; +Cc: Linux/m68k, linux-kernel, Thomas Gleixner


On Fri, 26 Aug 2011, Geert Uytterhoeven wrote:

> - [PATCH 18/24]: m68k/mac: Optimize interrupts using chain handlers (NEW)

That patch has a mistake --

	irq_set_chained_handler(IRQ_AUTO_4, psc_irq);
	irq_set_handler_data(IRQ_AUTO_3, (void *)0x40);
	irq_set_chained_handler(IRQ_AUTO_5, psc_irq);
	irq_set_handler_data(IRQ_AUTO_3, (void *)0x50);
	irq_set_chained_handler(IRQ_AUTO_6, psc_irq);
	irq_set_handler_data(IRQ_AUTO_3, (void *)0x60);

should be

	irq_set_chained_handler(IRQ_AUTO_4, psc_irq);
	irq_set_handler_data(IRQ_AUTO_4, (void *)0x40);
	irq_set_chained_handler(IRQ_AUTO_5, psc_irq);
	irq_set_handler_data(IRQ_AUTO_5, (void *)0x50);
	irq_set_chained_handler(IRQ_AUTO_6, psc_irq);
	irq_set_handler_data(IRQ_AUTO_6, (void *)0x60);

I also found that the mac_esp driver probe (when in PIO mode) locks up the 
machine. I worked around this by changing the 
disable_irq(IRQ_VIA2_3)/enable_irq(IRQ_VIA2_3) calls to 
local_irq_save/local_irq_restore, but this is not a good solution.

Apparently genirq doesn't permit nested disable_irq/enable_irq calls. But 
I haven't found any outer disable_irq/enable_irq pair yet, so I'm only 
guessing as to the cause of the problem. I'll keep looking.

On one VIA-based machine that I tested, !CONFIG_USE_GENERIC_HARDIRQS 
gives:

# cat /proc/interrupts
auto       1:       2299 via1
auto       2:       3876 via2
auto       4:        562 SCC
auto       7:          0 NMI
mac       10:        590 pmu-shift
mac       12:         90 pmu-clock
mac       14:       1635 timer
mac       17:       3876 nubus
mac       56:       3879 sonic

whereas, CONFIG_USE_GENERIC_HARDIRQS=y gives:

# cat /proc/interrupts
           CPU0       
  4:       5303      auto  SCC
  7:          0      auto  NMI
 10:       2136       mac  pmu-shift
 12:        607       mac  pmu-clock
 14:      41858       mac  timer
 56:      66876       mac  sonic
ERR:          0

Are there no counters for chained IRQs?

Other than these issues everything looks OK. I will test OSS and Baboon 
when I get the relevant hardware set up.

Finn

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: m68k: [v5] Convert to genirq (WIP)
  2011-08-28  2:08 ` Finn Thain
@ 2011-08-28  7:53   ` Geert Uytterhoeven
  2011-08-29  7:36     ` Geert Uytterhoeven
  2011-09-02 12:14   ` Finn Thain
  1 sibling, 1 reply; 10+ messages in thread
From: Geert Uytterhoeven @ 2011-08-28  7:53 UTC (permalink / raw)
  To: Finn Thain; +Cc: Linux/m68k, linux-kernel, Thomas Gleixner

On Sun, Aug 28, 2011 at 04:08, Finn Thain <fthain@telegraphics.com.au> wrote:
> On Fri, 26 Aug 2011, Geert Uytterhoeven wrote:
>> - [PATCH 18/24]: m68k/mac: Optimize interrupts using chain handlers (NEW)
>
> That patch has a mistake --
>
>        irq_set_chained_handler(IRQ_AUTO_4, psc_irq);
>        irq_set_handler_data(IRQ_AUTO_3, (void *)0x40);
>        irq_set_chained_handler(IRQ_AUTO_5, psc_irq);
>        irq_set_handler_data(IRQ_AUTO_3, (void *)0x50);
>        irq_set_chained_handler(IRQ_AUTO_6, psc_irq);
>        irq_set_handler_data(IRQ_AUTO_3, (void *)0x60);
>
> should be
>
>        irq_set_chained_handler(IRQ_AUTO_4, psc_irq);
>        irq_set_handler_data(IRQ_AUTO_4, (void *)0x40);
>        irq_set_chained_handler(IRQ_AUTO_5, psc_irq);
>        irq_set_handler_data(IRQ_AUTO_5, (void *)0x50);
>        irq_set_chained_handler(IRQ_AUTO_6, psc_irq);
>        irq_set_handler_data(IRQ_AUTO_6, (void *)0x60);

Woops, of course. Thanks!

> On one VIA-based machine that I tested, !CONFIG_USE_GENERIC_HARDIRQS
> gives:
>
> # cat /proc/interrupts
> auto       1:       2299 via1
> auto       2:       3876 via2
> auto       4:        562 SCC
> auto       7:          0 NMI
> mac       10:        590 pmu-shift
> mac       12:         90 pmu-clock
> mac       14:       1635 timer
> mac       17:       3876 nubus
> mac       56:       3879 sonic
>
> whereas, CONFIG_USE_GENERIC_HARDIRQS=y gives:
>
> # cat /proc/interrupts
>           CPU0
>  4:       5303      auto  SCC
>  7:          0      auto  NMI
>  10:       2136       mac  pmu-shift
>  12:        607       mac  pmu-clock
>  14:      41858       mac  timer
>  56:      66876       mac  sonic
> ERR:          0
>
> Are there no counters for chained IRQs?

I noticed the same for Amiga.

> Other than these issues everything looks OK. I will test OSS and Baboon
> when I get the relevant hardware set up.

Thanks again!

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: m68k: [v5] Convert to genirq (WIP)
  2011-08-28  7:53   ` Geert Uytterhoeven
@ 2011-08-29  7:36     ` Geert Uytterhoeven
  0 siblings, 0 replies; 10+ messages in thread
From: Geert Uytterhoeven @ 2011-08-29  7:36 UTC (permalink / raw)
  To: Thomas Gleixner; +Cc: Linux/m68k, linux-kernel, Finn Thain

On Sun, Aug 28, 2011 at 09:53, Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> On Sun, Aug 28, 2011 at 04:08, Finn Thain <fthain@telegraphics.com.au> wrote:
>> On one VIA-based machine that I tested, !CONFIG_USE_GENERIC_HARDIRQS
>> gives:
>>
>> # cat /proc/interrupts
>> auto       1:       2299 via1
>> auto       2:       3876 via2
>> auto       4:        562 SCC
>> auto       7:          0 NMI
>> mac       10:        590 pmu-shift
>> mac       12:         90 pmu-clock
>> mac       14:       1635 timer
>> mac       17:       3876 nubus
>> mac       56:       3879 sonic
>>
>> whereas, CONFIG_USE_GENERIC_HARDIRQS=y gives:
>>
>> # cat /proc/interrupts
>>           CPU0
>>  4:       5303      auto  SCC
>>  7:          0      auto  NMI
>>  10:       2136       mac  pmu-shift
>>  12:        607       mac  pmu-clock
>>  14:      41858       mac  timer
>>  56:      66876       mac  sonic
>> ERR:          0
>>
>> Are there no counters for chained IRQs?
>
> I noticed the same for Amiga.

Thomas, is it intentional that chain handlers don't show counters?
What if we get unexpected interrupts the chain handler can't handle?

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: m68k: [v5] Convert to genirq (WIP)
  2011-08-28  2:08 ` Finn Thain
  2011-08-28  7:53   ` Geert Uytterhoeven
@ 2011-09-02 12:14   ` Finn Thain
  2011-09-02 17:24     ` Geert Uytterhoeven
  1 sibling, 1 reply; 10+ messages in thread
From: Finn Thain @ 2011-09-02 12:14 UTC (permalink / raw)
  To: Geert Uytterhoeven; +Cc: Linux/m68k, linux-kernel, Thomas Gleixner


On Sun, 28 Aug 2011, I wrote:

>
> I also found that the mac_esp driver probe (when in PIO mode) locks up 
> the machine. I worked around this by changing the 
> disable_irq(IRQ_VIA2_3)/enable_irq(IRQ_VIA2_3) calls to 
> local_irq_save/local_irq_restore, but this is not a good solution.

This seems to be a bug in my mac_esp code. m68k's disable_irq() is 
actually equivalent to genirq's disable_irq_nosync(). Changing mac_esp to 
use disable_irq_nosync() fixes the issue under genirq.

I think these disable_irq/enable_irq calls are probably redundant. They 
don't seem to make any difference in practice. I'll send a patch when I've 
studied the code some more.

However, I found another problem. pmac_zilog oopses when its TTY is closed 
(see below). And macsonic does the same when the NIC is closed. The trace 
says that irq_shutdown() died trying to call the chip irq_mask routine, 
when desc->irq_data.chip (in a0) was NULL. Any ideas?

Finn


Unable to handle kernel NULL pointer dereference at virtual address   (null)
Oops: 00000000
Modules linked in:
PC: [<00000000>]   (null)
SR: 2700  SP: 02019d84  a2: 02017960
d0: 00000004    d1: 00000007    d2: 0031cadc    d3: 00000004
d4: 00002000    d5: 01c00710    a0: 00000000    a1: 00000000
Process init (pid: 1, task=02017960)
Frame format=7 eff addr=02019e10 ssw=05e6 faddr=00000000
wb 1 stat/addr/data: 0005 02109460 01c13e88
wb 2 stat/addr/data: 0000 0007b474 000771ac
wb 3 stat/addr/data: 0005 01c10b64 00000000
push data: 01c13e88 0201ac80 00000024 00000000
Stack from 02019dec:
        0004ce34 002f7fdc 0210edd0 0004c870 002f7fdc 00000004 0031cadc 00000bb
        002f7fdc 0031cadc 0004c8c6 00000004 0031cadc 00002000 00168b1c 0031cadc
        0017c296 00000004 0031cadc 00302000 00002000 020ca800 0017986e 0031cadc
        020ca820 00000001 00000001 00000000 00002008 020ca800 020f5000 0017a61a
        020f5000 020ca800 0210a7b0 00000000 00000002 01c00710 00000000 00000000
        020f5000 00000000 02109820 02019f34 01c00710 00000000 00164002 ffffffff
Call Trace: [<0004ce34>] irq_shutdown+0x48/0x54
 [<0004c870>] __free_irq+0x128/0x14e
 [<0004c8c6>] free_irq+0x30/0x80
 [<00002000>] _start+0x0/0x8
 [<00168b1c>] tty_chars_in_buffer+0x0/0x1a
 [<0017c296>] pmz_shutdown+0x28/0xe8
 [<00002000>] _start+0x0/0x8
 [<0017986e>] uart_shutdown+0x7e/0xb4
 [<00002008>] do_one_initcall+0x0/0x1c0
 [<0017a61a>] uart_close+0xe0/0x2a0
 [<00164002>] tty_release+0x58/0x472
 [<001640a4>] tty_release+0xfa/0x472
 [<00001000>] kernel_pg_dir+0x0/0x1000
 [<00001000>] kernel_pg_dir+0x0/0x1000
 [<0008dace>] alloc_fd+0x82/0x162
 [<00079b94>] fput+0x1aa/0x23a
 [<00079a6e>] fput+0x84/0x23a [<00076bca>] filp_close+0x4a/0x70
 [<00076c50>] sys_close+0x60/0x88
 [<0000269c>] syscall+0x8/0xc
 [<00001000>] kernel_pg_dir+0x0/0x1000
 [<0000c012>] res_func+0x48e/0x141a
 [<000021e4>] run_init_process+0x1c/0x22
 [<00002228>] init_post+0x3e/0xce
 [<000862aa>] sys_dup+0x0/0x58
 [<00327ad6>] kernel_init+0xe0/0xe8
 [<003279f6>] kernel_init+0x0/0xe8
 [<00002000>] _start+0x0/0x8
 [<00002960>] kernel_thread+0x3a/0x4e

Code: 0000 0000 0000 0000 0000 0000 0000 0000 Bad PC value.
Disabling lock debugging due to kernel taint



^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: m68k: [v5] Convert to genirq (WIP)
  2011-09-02 12:14   ` Finn Thain
@ 2011-09-02 17:24     ` Geert Uytterhoeven
  2011-09-02 18:24       ` Geert Uytterhoeven
                         ` (2 more replies)
  0 siblings, 3 replies; 10+ messages in thread
From: Geert Uytterhoeven @ 2011-09-02 17:24 UTC (permalink / raw)
  To: Finn Thain, Thomas Gleixner; +Cc: Linux/m68k, linux-kernel

On Fri, Sep 2, 2011 at 14:14, Finn Thain <fthain@telegraphics.com.au> wrote:
> On Sun, 28 Aug 2011, I wrote:
> However, I found another problem. pmac_zilog oopses when its TTY is closed
> (see below). And macsonic does the same when the NIC is closed. The trace
> says that irq_shutdown() died trying to call the chip irq_mask routine,
> when desc->irq_data.chip (in a0) was NULL. Any ideas?

> Unable to handle kernel NULL pointer dereference at virtual address   (null)
> Oops: 00000000
> Modules linked in:
> PC: [<00000000>]   (null)

No, it calls a function pointer that's NULL.

void irq_shutdown(struct irq_desc *desc)
{
        irq_state_set_disabled(desc);
        desc->depth = 1;
        if (desc->irq_data.chip->irq_shutdown)
                desc->irq_data.chip->irq_shutdown(&desc->irq_data);
        if (desc->irq_data.chip->irq_disable)
                desc->irq_data.chip->irq_disable(&desc->irq_data);
        else
                desc->irq_data.chip->irq_mask(&desc->irq_data);
        irq_state_set_masked(desc);
}

Oops, seems I misread the code and assumed the second "if" was an "else if",
as I wrote down in my notes that it calls only one of the 3 functions :-(
But it does need either .irq_disable() or .irq_mask().

The following irq_chip structs have both .irq_disable() and .irq_mask() NULL:
  - arch/m68k/apollo/dn_ints.c:apollo_irq_chip
  - arch/m68k/kernel/ints.c:auto_irq_chip
  - arch/m68k/kernel/ints.c:user_irq_chip

Since this is on Mac and mac_irq_chip is OK, I guess this is a non-PSC Mac,
i.e. the IRQ number is IRQ_MAC_SCC == IRQ_AUTO_4?

Thomas: As we cannot disable or mask autovector interrupts at the
autovector level,
IMHO it doesn't make much sense do add a dummy .irq_mask() function.
Can we add an extra check in irq_shutdown() to not call .irq_mask() if
it's NULL?

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: m68k: [v5] Convert to genirq (WIP)
  2011-09-02 17:24     ` Geert Uytterhoeven
@ 2011-09-02 18:24       ` Geert Uytterhoeven
       [not found]         ` <4E63291D.5050801@gmail.com>
  2011-09-02 19:53       ` Thomas Gleixner
  2011-09-03  2:40       ` Finn Thain
  2 siblings, 1 reply; 10+ messages in thread
From: Geert Uytterhoeven @ 2011-09-02 18:24 UTC (permalink / raw)
  To: Finn Thain, Thomas Gleixner; +Cc: Linux/m68k, linux-kernel

On Fri, Sep 2, 2011 at 19:24, Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> On Fri, Sep 2, 2011 at 14:14, Finn Thain <fthain@telegraphics.com.au> wrote:
>> On Sun, 28 Aug 2011, I wrote:
>> However, I found another problem. pmac_zilog oopses when its TTY is closed
>> (see below). And macsonic does the same when the NIC is closed. The trace
>> says that irq_shutdown() died trying to call the chip irq_mask routine,
>> when desc->irq_data.chip (in a0) was NULL. Any ideas?
>
>> Unable to handle kernel NULL pointer dereference at virtual address   (null)
>> Oops: 00000000
>> Modules linked in:
>> PC: [<00000000>]   (null)
>
> No, it calls a function pointer that's NULL.
>
> void irq_shutdown(struct irq_desc *desc)
> {
>        irq_state_set_disabled(desc);
>        desc->depth = 1;
>        if (desc->irq_data.chip->irq_shutdown)
>                desc->irq_data.chip->irq_shutdown(&desc->irq_data);
>        if (desc->irq_data.chip->irq_disable)
>                desc->irq_data.chip->irq_disable(&desc->irq_data);
>        else
>                desc->irq_data.chip->irq_mask(&desc->irq_data);
>        irq_state_set_masked(desc);
> }
>
> Oops, seems I misread the code and assumed the second "if" was an "else if",
> as I wrote down in my notes that it calls only one of the 3 functions :-(
> But it does need either .irq_disable() or .irq_mask().

However, include/linux/irq.h says:

 * @irq_shutdown:       shut down the interrupt (defaults to ->disable if NULL)

which gives me the impression the original intention was to have an "else if",
and don't require .irq_disable() or .irq_mask() if .irq_shutdown() is available.

That's also what  __free_irq() used to do, cfr.
3b56f0585fd4c02d047dc406668cb40159b2d340

"genirq: Remove bogus conditional

 The if (chip->irq_shutdown) check will always evaluate to true, as we
 fill in chip->irq_shutdown with default_shutdown in
 irq_chip_set_defaults() if the chip does not provide its own function."

diff --git a/kernel/irq/manage.c b/kernel/irq/manage.c
index 33a6ee0..30bc8de 100644
--- a/kernel/irq/manage.c
+++ b/kernel/irq/manage.c
@@ -1057,10 +1057,7 @@ static struct irqaction *__free_irq(unsigned int irq, voi
        /* If this was the last handler, shut down the IRQ line: */
        if (!desc->action) {
                desc->status |= IRQ_DISABLED;
-               if (desc->irq_data.chip->irq_shutdown)
-                       desc->irq_data.chip->irq_shutdown(&desc->irq_data);
-               else
-                       desc->irq_data.chip->irq_disable(&desc->irq_data);
+               desc->irq_data.chip->irq_shutdown(&desc->irq_data);
        }

 #ifdef CONFIG_SMP

(at that time the check had become bogus, as .irq_shutdown() was always set to
.irq_disable() if it was NULL).

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: m68k: [v5] Convert to genirq (WIP)
  2011-09-02 17:24     ` Geert Uytterhoeven
  2011-09-02 18:24       ` Geert Uytterhoeven
@ 2011-09-02 19:53       ` Thomas Gleixner
  2011-09-03  2:40       ` Finn Thain
  2 siblings, 0 replies; 10+ messages in thread
From: Thomas Gleixner @ 2011-09-02 19:53 UTC (permalink / raw)
  To: Geert Uytterhoeven; +Cc: Finn Thain, Linux/m68k, linux-kernel

[-- Attachment #1: Type: TEXT/PLAIN, Size: 2075 bytes --]

On Fri, 2 Sep 2011, Geert Uytterhoeven wrote:

> On Fri, Sep 2, 2011 at 14:14, Finn Thain <fthain@telegraphics.com.au> wrote:
> > On Sun, 28 Aug 2011, I wrote:
> > However, I found another problem. pmac_zilog oopses when its TTY is closed
> > (see below). And macsonic does the same when the NIC is closed. The trace
> > says that irq_shutdown() died trying to call the chip irq_mask routine,
> > when desc->irq_data.chip (in a0) was NULL. Any ideas?
> 
> > Unable to handle kernel NULL pointer dereference at virtual address   (null)
> > Oops: 00000000
> > Modules linked in:
> > PC: [<00000000>]   (null)
> 
> No, it calls a function pointer that's NULL.
> 
> void irq_shutdown(struct irq_desc *desc)
> {
>         irq_state_set_disabled(desc);
>         desc->depth = 1;
>         if (desc->irq_data.chip->irq_shutdown)
>                 desc->irq_data.chip->irq_shutdown(&desc->irq_data);
>         if (desc->irq_data.chip->irq_disable)
>                 desc->irq_data.chip->irq_disable(&desc->irq_data);
>         else
>                 desc->irq_data.chip->irq_mask(&desc->irq_data);
>         irq_state_set_masked(desc);
> }
> 
> Oops, seems I misread the code and assumed the second "if" was an "else if",

I think that was supposed to be an "else if". Lemme look at the code
before the overhaul to be sure.

> as I wrote down in my notes that it calls only one of the 3 functions :-(
> But it does need either .irq_disable() or .irq_mask().
> 
> The following irq_chip structs have both .irq_disable() and .irq_mask() NULL:
>   - arch/m68k/apollo/dn_ints.c:apollo_irq_chip
>   - arch/m68k/kernel/ints.c:auto_irq_chip
>   - arch/m68k/kernel/ints.c:user_irq_chip
> 
> Since this is on Mac and mac_irq_chip is OK, I guess this is a non-PSC Mac,
> i.e. the IRQ number is IRQ_MAC_SCC == IRQ_AUTO_4?
> 
> Thomas: As we cannot disable or mask autovector interrupts at the
> autovector level,
> IMHO it doesn't make much sense do add a dummy .irq_mask() function.
> Can we add an extra check in irq_shutdown() to not call .irq_mask() if
> it's NULL?

Yes.

Thanks,

	tglx

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: m68k: [v5] Convert to genirq (WIP)
  2011-09-02 17:24     ` Geert Uytterhoeven
  2011-09-02 18:24       ` Geert Uytterhoeven
  2011-09-02 19:53       ` Thomas Gleixner
@ 2011-09-03  2:40       ` Finn Thain
  2 siblings, 0 replies; 10+ messages in thread
From: Finn Thain @ 2011-09-03  2:40 UTC (permalink / raw)
  To: Geert Uytterhoeven; +Cc: Thomas Gleixner, Linux/m68k, linux-kernel


On Fri, 2 Sep 2011, Geert Uytterhoeven wrote:

> Since this is on Mac and mac_irq_chip is OK, I guess this is a non-PSC 
> Mac, i.e. the IRQ number is IRQ_MAC_SCC == IRQ_AUTO_4?

Right. Similarly, the sonic driver was using IRQ_AUTO_3 when the same 
issue cropped up there. (I was wondering why the PSC machine didn't 
exhibit the problem -- as you point out, it's only auto vector interrupts 
that are affected.)

Finn

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: m68k: [v5] Convert to genirq (WIP)
       [not found]               ` <4E647F80.8050903@gmail.com>
@ 2011-09-05 20:18                 ` Michael Schmitz
  0 siblings, 0 replies; 10+ messages in thread
From: Michael Schmitz @ 2011-09-05 20:18 UTC (permalink / raw)
  To: Geert Uytterhoeven; +Cc: Michael Schmitz, Linux/m68k

Hi Geert,

> Works a treat - and gives about 1.5 to 2 fold higher throughput on scp than
> the 3.0 I used before.

Well, not quite:I have seen the following messages after a while of uptime

irq 12: nobody cared (try booting with the "irqpoll" option)
Call Trace: [<0004dc86>] __report_bad_irq+0xa2/0xae
 [<0004dc18>] __report_bad_irq+0x34/0xae
 [<0004dd64>] note_interrupt+0xaa/0x13c
 [<2102401c>] atari_ei_interrupt+0x1c/0x24 [atari_ethernec]
 [<0004cb32>] handle_irq_event_percpu+0x7e/0x110
 [<000a6cdc>] load_script+0xb4/0x1d8
 [<0009ae0c>] __blkdev_get+0x244/0x34a
 [<00001000>] kernel_pg_dir+0x0/0x1000
 [<000c6668>] ext3_link+0x48/0x124
 [<0004cbe4>] handle_irq_event+0x20/0x2c
 [<0004e258>] handle_simple_irq+0x3a/0x4c
 [<0004c5be>] generic_handle_irq+0x20/0x28
 [<00005e9c>] do_IRQ+0x20/0x30
 [<00237970>] schedule+0x0/0x2b6
 [<0000275e>] user_irqhandler_fixup+0x4/0x1a
 [<000028d0>] default_idle+0x0/0x14
 [<00002904>] cpu_idle+0x20/0x30
 [<000029f2>] kernel_thread+0x0/0x50
 [<00236928>] rest_init+0x68/0x6c
 [<000279f4>] printk+0x0/0x18
 [<0031407e>] start_kernel+0x29e/0x2a8
 [<00001000>] kernel_pg_dir+0x0/0x1000
 [<0031331e>] _sinittext+0x31e/0x9c0

handlers:
[<21024000>] atari_ei_interrupt
Disabling IRQ #12

Network dead after this; trying to ping out from the Falcon gives:

------------[ cut here ]------------
WARNING: at net/sched/sch_generic.c:255 dev_watchdog+0x194/0x1c6()
NETDEV WATCHDOG: eth0 (): transmit queue 0 timed out
Modules linked in: ipv6 atari_ethernec 8390 genrtc [last unloaded: atari_91C111]
Call Trace: [<000271fa>] warn_slowpath_common+0x4c/0x64
 [<0002723c>] warn_slowpath_fmt+0x2a/0x32
 [<001ccc98>] dev_watchdog+0x194/0x1c6
 [<0002ff10>] call_timer_fn+0x0/0x70
 [<001ccc98>] dev_watchdog+0x194/0x1c6
 [<001ccb04>] dev_watchdog+0x0/0x1c6
 [<0004cb32>] handle_irq_event_percpu+0x7e/0x110
 [<0002ff22>] call_timer_fn+0x12/0x70
 [<001ccb04>] dev_watchdog+0x0/0x1c6
 [<00030082>] run_timer_softirq+0xc0/0x16e
 [<001ccb04>] dev_watchdog+0x0/0x1c6
 [<00001000>] kernel_pg_dir+0x0/0x1000
 [<000c6668>] ext3_link+0x48/0x124
 [<0004c5be>] generic_handle_irq+0x20/0x28
 [<0002b24a>] __do_softirq+0x78/0xc6
 [<00237970>] schedule+0x0/0x2b6
 [<0002b2be>] do_softirq+0x26/0x2c
 [<00002664>] ret_from_exception+0x0/0xc
 [<000028d0>] default_idle+0x0/0x14
 [<00002904>] cpu_idle+0x20/0x30
 [<000029f2>] kernel_thread+0x0/0x50
 [<00236928>] rest_init+0x68/0x6c
 [<000279f4>] printk+0x0/0x18
 [<0031407e>] start_kernel+0x29e/0x2a8
 [<00001000>] kernel_pg_dir+0x0/0x1000
 [<0031331e>] _sinittext+0x31e/0x9c0

---[ end trace a81b161261829977 ]---

Unloading / reloading the module makes it work again:

ne.c:v1.10 9/23/94 Donald Becker (becker@scyld.com)
<6>atari_ethernec.c 11/10/06 Michael Schmitz (schmitz@debian.org)
 failed to detect IRQ line. Assuming irq 12
 00 00 e8 5d 58 cd
eth%d: RTL8019 found at 0x300, using IRQ 12.
eth0: no IPv6 routers present

Until the interrupt is disabled again:

irq 12: nobody cared (try booting with the "irqpoll" option)
Call Trace: [<0004dc86>] __report_bad_irq+0xa2/0xae
 [<0004dc18>] __report_bad_irq+0x34/0xae
 [<0004dd64>] note_interrupt+0xaa/0x13c
 [<2102401c>] atari_ei_interrupt+0x1c/0x24 [atari_ethernec]
 [<0004cb32>] handle_irq_event_percpu+0x7e/0x110
 [<000a6cdc>] load_script+0xb4/0x1d8
 [<0009ae0c>] __blkdev_get+0x244/0x34a
 [<00001000>] kernel_pg_dir+0x0/0x1000
 [<000c6668>] ext3_link+0x48/0x124
 [<0004cbe4>] handle_irq_event+0x20/0x2c
 [<0004e258>] handle_simple_irq+0x3a/0x4c
 [<0004c5be>] generic_handle_irq+0x20/0x28
 [<00005e9c>] do_IRQ+0x20/0x30
 [<00237970>] schedule+0x0/0x2b6
 [<0000275e>] user_irqhandler_fixup+0x4/0x1a
 [<000028d0>] default_idle+0x0/0x14
 [<00002904>] cpu_idle+0x20/0x30
 [<000029f2>] kernel_thread+0x0/0x50
 [<00236928>] rest_init+0x68/0x6c
 [<000279f4>] printk+0x0/0x18
 [<0031407e>] start_kernel+0x29e/0x2a8
 [<00001000>] kernel_pg_dir+0x0/0x1000
 [<0031331e>] _sinittext+0x31e/0x9c0

handlers:
[<21024000>] atari_ei_interrupt
Disabling IRQ #12

What will the irqpoll option do in this context?

Are these in the interrupt path, or another (disk) interrupt
preempting the timer D interrupt?

 [<000a6cdc>] load_script+0xb4/0x1d8
 [<0009ae0c>] __blkdev_get+0x244/0x34a
 [<00001000>] kernel_pg_dir+0x0/0x1000
 [<000c6668>] ext3_link+0x48/0x124

Cheers,

  Michael

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2011-09-05 20:18 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-08-26 20:34 m68k: [v5] Convert to genirq (WIP) Geert Uytterhoeven
2011-08-28  2:08 ` Finn Thain
2011-08-28  7:53   ` Geert Uytterhoeven
2011-08-29  7:36     ` Geert Uytterhoeven
2011-09-02 12:14   ` Finn Thain
2011-09-02 17:24     ` Geert Uytterhoeven
2011-09-02 18:24       ` Geert Uytterhoeven
     [not found]         ` <4E63291D.5050801@gmail.com>
     [not found]           ` <CAMuHMdXB5nBRVvUmqake-kV1mbWq+k8vOGHPOSL+2ix6PJW80w@mail.gmail.com>
     [not found]             ` <CAOmrzkKCGhL2GfDTC_gqKtKfU2vKeqDh1CMZ+BdUCtLEufQhqw@mail.gmail.com>
     [not found]               ` <4E647F80.8050903@gmail.com>
2011-09-05 20:18                 ` Michael Schmitz
2011-09-02 19:53       ` Thomas Gleixner
2011-09-03  2:40       ` Finn Thain

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.