2011-05-28 18:32:15

by Geert Uytterhoeven

[permalink] [raw]
Subject: m68k: Convert to genirq (WIP)

Hi all,

Now sparc32 has beaten me, I pushed out my work in progress of converting m68k
to use the generic hardirq framework:

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

It contains the following commits:

[1] ide-{cd,floppy,tape}, keyboard: Do not include <linux/irq.>
[2] m68k/irq: Rename irq_controller to irq_chip
[3] m68k/irq: Kill irq_node_t typedef, always use struct irq_node
[4] m68k/irq: Rename irq_node to irq_data
[5] m68k/irq: Switch irq_chip methods to "struct irq_data *data"
[6] m68k/irq: Rename setup_irq() to m68k_setup_irq() and make it static
[7] m68k/irq: Extract irq_set_chip()
[8] m68k/irq: Add genirq support
[9] m68k/atari: Convert Atari to genirq

[1] is a temporary(?) fix to avoid redefinitions during the remainder of
the series,
[2-7] refactor the current m68k irq framework to match the generic hardirq
framework more closely w.r.t. to its (platform) users,
[8] adds generic hardirq support to the core, which can be enabled through
a config option, so both legacy and generic hardirq can coexist at the
source level during the migration,
[9] converts Atari to use generic hardirqs.

It's working on ARAnyM[*], which means user (non-autovectored) interrupts are
working.

Next on my list is Amiga, which will cover autovectored and chained interrupts.

Hopefully I'll have enough confidence after that to blindly convert the other
platforms I cannot test myself ;-)
Of course any help is welcome, especially for Mac (with its maze of interrupt
controllers) and Q40 (with its weird ISA interrupt mapping). The remainder of
the platforms seems to be fairly standard and simple.

Thanks!

P.S. The branch m68k-genirq may be rebased in the future.

[*] There are 2 warnings during boot though:

| NR_IRQS:72
| ------------[ cut here ]------------
| WARNING: at linux/kernel/irq/chip.c:559 0x2a191a()
| Modules linked in:
| Call Trace: [<0002767e>] warn_slowpath_common+0x4c/0x64
| [<0004e81c>] irq_set_chip+0x0/0x66
| [<000276aa>] warn_slowpath_null+0x14/0x1a
| [<0004e728>] __irq_set_handler+0x11c/0x13c
| [<0004ec7e>] handle_level_irq+0x0/0x9c
| [<00003ef4>] m68k_setup_irq_controller+0x3c/0x50
| [<0004ec7e>] handle_level_irq+0x0/0x9c
| [<0004e81c>] irq_set_chip+0x0/0x66
| [<00027dc8>] printk+0x0/0x1a
| [<00322022>] __alloc_bootmem+0x0/0x1a
| [<0031dea4>] atari_init_IRQ+0x2a/0xe8
| [<0004ec7e>] handle_level_irq+0x0/0x9c
| [<0031c450>] init_IRQ+0x28/0x2e
| [<00027dc8>] printk+0x0/0x1a
| [<0031a012>] start_kernel+0x196/0x3b0
| [<0031931e>] _sinittext+0x31e/0x9c0
|
| ---[ end trace 139ce121c98e96c9 ]---

and:

| ------------[ cut here ]------------
| WARNING: at linux/kernel/irq/handle.c:130 handle_irq_event_percpu+0xe6/0x148()
| irq 3 handler nfeth_interrupt+0x0/0x126 enabled interrupts
| Modules linked in:
| Call Trace: [<00027670>] warn_slowpath_common+0x3e/0x64
| [<0002767e>] warn_slowpath_common+0x4c/0x64
| [<0002770c>] warn_slowpath_fmt+0x2a/0x32
| [<0004d01a>] handle_irq_event_percpu+0xe6/0x148
| [<0004d01a>] handle_irq_event_percpu+0xe6/0x148
| [<00009d64>] nfeth_interrupt+0x0/0x126
| [<00027dc8>] printk+0x0/0x1a
| [<0004d09c>] handle_irq_event+0x20/0x2c
| [<0004ecce>] handle_level_irq+0x50/0x9c
| [<002496b8>] schedule+0x0/0x31e
| [<00006782>] do_IRQ+0x2e/0x44
| [<00027dc8>] printk+0x0/0x1a
| [<00003eb4>] __m68k_handle_int+0xe/0x12
| [<000026e2>] auto_irqhandler_fixup+0x4/0x6
| [<00027dc8>] printk+0x0/0x1a
| [<00002c46>] default_idle+0x0/0xe
| [<00002b0c>] cpu_idle+0x16/0x22
| [<00002b18>] kernel_thread+0x0/0x4e
| [<00248610>] rest_init+0x5c/0x62
| [<0031a220>] start_kernel+0x3a4/0x3b0
| [<000106aa>] ssincos+0x27e/0x2ac
| [<000106aa>] ssincos+0x27e/0x2ac
| [<0031931e>] _sinittext+0x31e/0x9c0
|
| ---[ end trace 139ce121c98e96cb ]---

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

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


2011-05-31 08:35:10

by Geert Uytterhoeven

[permalink] [raw]
Subject: Re: m68k: Convert to genirq (WIP)

On Sat, May 28, 2011 at 20:32, Geert Uytterhoeven <[email protected]> wrote:
> | NR_IRQS:72
> | ------------[ cut here ]------------
> | WARNING: at linux/kernel/irq/chip.c:559 0x2a191a()
> | Modules linked in:
> | Call Trace: [<0002767e>] warn_slowpath_common+0x4c/0x64
> |  [<0004e81c>] irq_set_chip+0x0/0x66
> |  [<000276aa>] warn_slowpath_null+0x14/0x1a
> |  [<0004e728>] __irq_set_handler+0x11c/0x13c
> |  [<0004ec7e>] handle_level_irq+0x0/0x9c
> |  [<00003ef4>] m68k_setup_irq_controller+0x3c/0x50
> |  [<0004ec7e>] handle_level_irq+0x0/0x9c
> |  [<0004e81c>] irq_set_chip+0x0/0x66
> |  [<00027dc8>] printk+0x0/0x1a
> |  [<00322022>] __alloc_bootmem+0x0/0x1a
> |  [<0031dea4>] atari_init_IRQ+0x2a/0xe8
> |  [<0004ec7e>] handle_level_irq+0x0/0x9c
> |  [<0031c450>] init_IRQ+0x28/0x2e
> |  [<00027dc8>] printk+0x0/0x1a
> |  [<0031a012>] start_kernel+0x196/0x3b0
> |  [<0031931e>] _sinittext+0x31e/0x9c0
> |
> | ---[ end trace 139ce121c98e96c9 ]---

whitespace-challenged diff --git a/arch/m68k/kernel/ints.c
b/arch/m68k/kernel/ints.c
index f4804f3..8d8a1e9 100644
--- a/arch/m68k/kernel/ints.c
+++ b/arch/m68k/kernel/ints.c
@@ -285,7 +285,7 @@ void m68k_setup_irq_controller(struct irq_chip *chip,
for (i = 0; i < cnt; i++) {
irq_set_chip(irq + i, chip);
if (handle)
- irq_set_handler(i, handle);
+ irq_set_handler(irq + i, handle);
}
}

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

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

2011-05-31 09:50:35

by Thomas Gleixner

[permalink] [raw]
Subject: Re: m68k: Convert to genirq (WIP)

Geert,

On Sat, 28 May 2011, Geert Uytterhoeven wrote:

> Hi all,
>
> Now sparc32 has beaten me, I pushed out my work in progress of converting m68k
> to use the generic hardirq framework:

That leaves only s390 in the rain, but I don't expect that they
replace their magic interrupt opcode ever :)

> 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
>
> It contains the following commits:
>
> [1] ide-{cd,floppy,tape}, keyboard: Do not include <linux/irq.>
> [2] m68k/irq: Rename irq_controller to irq_chip
> [3] m68k/irq: Kill irq_node_t typedef, always use struct irq_node
> [4] m68k/irq: Rename irq_node to irq_data
> [5] m68k/irq: Switch irq_chip methods to "struct irq_data *data"
> [6] m68k/irq: Rename setup_irq() to m68k_setup_irq() and make it static
> [7] m68k/irq: Extract irq_set_chip()
> [8] m68k/irq: Add genirq support
> [9] m68k/atari: Convert Atari to genirq
>
> [1] is a temporary(?) fix to avoid redefinitions during the remainder of
> the series,
> [2-7] refactor the current m68k irq framework to match the generic hardirq
> framework more closely w.r.t. to its (platform) users,
> [8] adds generic hardirq support to the core, which can be enabled through
> a config option, so both legacy and generic hardirq can coexist at the
> source level during the migration,
> [9] converts Atari to use generic hardirqs.

Very sensible approach! Patches are looking good so far! Feel free to
add Acked-by-me!

Thanks,

tglx

2011-05-31 19:54:30

by Geert Uytterhoeven

[permalink] [raw]
Subject: Re: m68k: Convert to genirq (WIP)

Hi Thomas,

On Sat, May 28, 2011 at 20:32, Geert Uytterhoeven <[email protected]> wrote:
> Next on my list is Amiga, which will cover autovectored and chained interrupts.

Plain chained interrupts should be handled by calling irq_set_chained_handler()
to register an irq_flow_handler_t, and let the handler call
generic_handle_irq() for
each successive interrupt, right?

However, on Amiga we have interrupts (IRQ_AMIGA_PORTS and
IRQ_AMIGA_EXTER) that are a combination of chained (for the CIA interrupt
controllers) and shared level interrupts (for motherboard hardware and
Zorro expansion
boards).
But irq_set_chained_handler() sets IRQ_NOREQUEST, i.e. the interrupt cannot
be requested anymore via request_irq(), so this rules out using it with shared
level interrupts.

What's the preferred way to handle this? Is there are standard way, or should I
write my own specialized irq_flow_handler_t that handles both?

In the old scheme, we just registered the chain handlers with
request_irq(), so they
worked nicely together with the shared level interrupts (cfr.
arch/m68k/amiga/cia.c)

Thanks!

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

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

2011-05-31 20:11:02

by Thomas Gleixner

[permalink] [raw]
Subject: Re: m68k: Convert to genirq (WIP)

Geert,

On Tue, 31 May 2011, Geert Uytterhoeven wrote:
> On Sat, May 28, 2011 at 20:32, Geert Uytterhoeven <[email protected]> wrote:
> > Next on my list is Amiga, which will cover autovectored and chained interrupts.
>
> Plain chained interrupts should be handled by calling irq_set_chained_handler()
> to register an irq_flow_handler_t, and let the handler call
> generic_handle_irq() for
> each successive interrupt, right?
>
> However, on Amiga we have interrupts (IRQ_AMIGA_PORTS and
> IRQ_AMIGA_EXTER) that are a combination of chained (for the CIA interrupt
> controllers) and shared level interrupts (for motherboard hardware and
> Zorro expansion
> boards).
> But irq_set_chained_handler() sets IRQ_NOREQUEST, i.e. the interrupt cannot
> be requested anymore via request_irq(), so this rules out using it with shared
> level interrupts.
>
> What's the preferred way to handle this? Is there are standard way, or should I
> write my own specialized irq_flow_handler_t that handles both?
>
> In the old scheme, we just registered the chain handlers with
> request_irq(), so they
> worked nicely together with the shared level interrupts (cfr.
> arch/m68k/amiga/cia.c)

You still can setup all those handlers setup via request_irq() and
handle the demux part by calling generic_handle_irq() from the handler
which deals with that demux hardware. irq_set_chained_handler() is
merily an optimization which does not apply to potentially shared
primary interrupts.

Thanks,

tglx

2011-06-03 11:45:24

by Finn Thain

[permalink] [raw]
Subject: Re: m68k: Convert to genirq (WIP)


On Sat, 28 May 2011, Geert Uytterhoeven wrote:

> Of course any help is welcome, especially for Mac

What kind of help are you looking for? I read through your patch series
but I'm not sure how much work is left. Probably I need to look at a
genirq reference implementation to get an idea of the goal...

> (with its maze of interrupt controllers)

Perhaps we do not need to model that complexity in generic irq terms?

The mac port could probably make good use of the irq startup and shutdown
hooks though.

Finn

2011-06-03 11:33:39

by Geert Uytterhoeven

[permalink] [raw]
Subject: Re: m68k: Convert to genirq (WIP)

On Fri, Jun 3, 2011 at 13:20, Finn Thain <[email protected]> wrote:
> On Sat, 28 May 2011, Geert Uytterhoeven wrote:
>> Of course any help is welcome, especially for Mac
>
> What kind of help are you looking for? I read through your patch series
> but I'm not sure how much work is left. Probably I need to look at a
> genirq reference implementation to get an idea of the goal...

I could use help in the form of converting the various m68k subplatforms over to
genirq.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

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

2011-06-04 17:56:59

by Geert Uytterhoeven

[permalink] [raw]
Subject: Re: m68k: Convert to genirq (WIP)

On Sat, May 28, 2011 at 20:32, Geert Uytterhoeven <[email protected]> wrote:
> Now sparc32 has beaten me, I pushed out my work in progress of converting m68k
> to use the generic hardirq framework:
>
> 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

> Next on my list is Amiga, which will cover autovectored and chained interrupts.

I rebased to 3.0-rc1 and added (not in that order):
[10] m68k/irq: Add m68k_setup_irq_controller()
[11] m68k/irq: Adding missing "irq" offset in m68k_setup_irq_controller()
[12] m68k/irq: Fix auto and user chips and handlers
[13] m68k/amiga: Refactor amiints.c
[14] m68k/amiga: Convert Amiga to genirq
[15] m68k/amiga: Optimize interrupts using chain handlers

It works on Amiga now, too. It can still use some optimizations in the
irq_{enable,disable,ack,mask,mask_ack,unmask} area, as my BogoMIPS
rating dropped by ca 2.5% and is now under 16, for a 25 MHz 68040.

BTW, the second warning on Atari is still there.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

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

2011-06-05 06:44:18

by Brad Boyer

[permalink] [raw]
Subject: Re: m68k: Convert to genirq (WIP)

On Fri, Jun 03, 2011 at 09:20:23PM +1000, Finn Thain wrote:
> On Sat, 28 May 2011, Geert Uytterhoeven wrote:
> > Of course any help is welcome, especially for Mac
>
> What kind of help are you looking for? I read through your patch series
> but I'm not sure how much work is left. Probably I need to look at a
> genirq reference implementation to get an idea of the goal...

If there are any good documents on genirq, I would like to look at it
as well. I didn't see anything obvious in Documentation/ in the tree.

> > (with its maze of interrupt controllers)
>
> Perhaps we do not need to model that complexity in generic irq terms?
>
> The mac port could probably make good use of the irq startup and shutdown
> hooks though.

If we could really map all the interrupt controllers, we could probably
cut down on the largest interrupt number. Most Macs really only have
20 or 30 actual interrupt sources. The problem is the two or three
layers of cascaded mess. None of them have just a single layer. Just
getting rid of the fixed global mapping could take us down to 64 since
nothing has both Baboon and PSC, but we have unique numbers for them.
We have 3 whole blocks of 8 just for PSC, even though that's just in
two models. We don't even know what most of those do, if anything.

We would probably need to move everything to platform devices if we
do that so that drivers don't need to use hard-coded interrupts. I
suppose we really should do that anyway.

Brad Boyer
[email protected]

2011-06-05 07:45:17

by Geert Uytterhoeven

[permalink] [raw]
Subject: Re: m68k: Convert to genirq (WIP)

On Sun, Jun 5, 2011 at 08:27, Brad Boyer <[email protected]> wrote:
> On Fri, Jun 03, 2011 at 09:20:23PM +1000, Finn Thain wrote:
>> On Sat, 28 May 2011, Geert Uytterhoeven wrote:
>> > Of course any help is welcome, especially for Mac
>>
>> What kind of help are you looking for? I read through your patch series
>> but I'm not sure how much work is left. Probably I need to look at a
>> genirq reference implementation to get an idea of the goal...
>
> If there are any good documents on genirq, I would like to look at it
> as well. I didn't see anything obvious in Documentation/ in the tree.

Documentation/DocBook/genericirq.tmpl

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

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

2011-06-15 19:45:03

by Geert Uytterhoeven

[permalink] [raw]
Subject: Re: m68k: Convert to genirq (WIP)

On Sat, Jun 4, 2011 at 19:56, Geert Uytterhoeven <[email protected]> wrote:
> On Sat, May 28, 2011 at 20:32, Geert Uytterhoeven <[email protected]> wrote:
>> Now sparc32 has beaten me, I pushed out my work in progress of converting m68k
>> to use the generic hardirq framework:
>>
>> 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
>
>> Next on my list is Amiga, which will cover autovectored and chained interrupts.
>
> I rebased to 3.0-rc1 and added (not in that order):
>  [10] m68k/irq: Add m68k_setup_irq_controller()
>  [11] m68k/irq: Adding missing "irq" offset in m68k_setup_irq_controller()
>  [12] m68k/irq: Fix auto and user chips and handlers
>  [13] m68k/amiga: Refactor amiints.c
>  [14] m68k/amiga: Convert Amiga to genirq
>  [15] m68k/amiga: Optimize interrupts using chain handlers
>
> It works on Amiga now, too. It can still use some optimizations in the
> irq_{enable,disable,ack,mask,mask_ack,unmask} area, as my BogoMIPS
> rating dropped by ca 2.5% and is now under 16, for a 25 MHz 68040.

Seems like everything (Atari/ARAnyM and Amiga) still works when using
handle_simple_irq instead of handle_level_irq.
As a bonus, BogoMIPS is above 16 again.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

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

2011-06-16 19:31:00

by Geert Uytterhoeven

[permalink] [raw]
Subject: Re: m68k: Convert to genirq (WIP)

On Wed, Jun 15, 2011 at 21:44, Geert Uytterhoeven <[email protected]> wrote:
> On Sat, Jun 4, 2011 at 19:56, Geert Uytterhoeven <[email protected]> wrote:
>> On Sat, May 28, 2011 at 20:32, Geert Uytterhoeven <[email protected]> wrote:
>>> Now sparc32 has beaten me, I pushed out my work in progress of converting m68k
>>> to use the generic hardirq framework:
>>>
>>> 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
>>
>>> Next on my list is Amiga, which will cover autovectored and chained interrupts.
>>
>> I rebased to 3.0-rc1 and added (not in that order):
>>  [10] m68k/irq: Add m68k_setup_irq_controller()
>>  [11] m68k/irq: Adding missing "irq" offset in m68k_setup_irq_controller()
>>  [12] m68k/irq: Fix auto and user chips and handlers
>>  [13] m68k/amiga: Refactor amiints.c
>>  [14] m68k/amiga: Convert Amiga to genirq
>>  [15] m68k/amiga: Optimize interrupts using chain handlers
>>
>> It works on Amiga now, too. It can still use some optimizations in the
>> irq_{enable,disable,ack,mask,mask_ack,unmask} area, as my BogoMIPS
>> rating dropped by ca 2.5% and is now under 16, for a 25 MHz 68040.
>
> Seems like everything (Atari/ARAnyM and Amiga) still works when using
> handle_simple_irq instead of handle_level_irq.
> As a bonus, BogoMIPS is above 16 again.

With handle_simple_irq(), we no longer need to define irq_{,un}mask() methods
in our irq_chips. Hence the "old" m68k platform interrupt code seems to be much
closer to genirq than I thought...

Does this make sense?

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

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

2011-06-16 19:45:15

by Geert Uytterhoeven

[permalink] [raw]
Subject: Re: m68k: Convert to genirq (WIP)

On Sat, May 28, 2011 at 20:32, Geert Uytterhoeven <[email protected]> wrote:
> | ------------[ cut here ]------------
> | WARNING: at linux/kernel/irq/handle.c:130 handle_irq_event_percpu+0xe6/0x148()
> | irq 3 handler nfeth_interrupt+0x0/0x126 enabled interrupts
> | Modules linked in:
> | Call Trace: [<00027670>] warn_slowpath_common+0x3e/0x64
> |  [<0002767e>] warn_slowpath_common+0x4c/0x64
> |  [<0002770c>] warn_slowpath_fmt+0x2a/0x32
> |  [<0004d01a>] handle_irq_event_percpu+0xe6/0x148
> |  [<0004d01a>] handle_irq_event_percpu+0xe6/0x148
> |  [<00009d64>] nfeth_interrupt+0x0/0x126
> |  [<00027dc8>] printk+0x0/0x1a
> |  [<0004d09c>] handle_irq_event+0x20/0x2c
> |  [<0004ecce>] handle_level_irq+0x50/0x9c
> |  [<002496b8>] schedule+0x0/0x31e
> |  [<00006782>] do_IRQ+0x2e/0x44
> |  [<00027dc8>] printk+0x0/0x1a
> |  [<00003eb4>] __m68k_handle_int+0xe/0x12
> |  [<000026e2>] auto_irqhandler_fixup+0x4/0x6
> |  [<00027dc8>] printk+0x0/0x1a
> |  [<00002c46>] default_idle+0x0/0xe
> |  [<00002b0c>] cpu_idle+0x16/0x22
> |  [<00002b18>] kernel_thread+0x0/0x4e
> |  [<00248610>] rest_init+0x5c/0x62
> |  [<0031a220>] start_kernel+0x3a4/0x3b0
> |  [<000106aa>] ssincos+0x27e/0x2ac
> |  [<000106aa>] ssincos+0x27e/0x2ac
> |  [<0031931e>] _sinittext+0x31e/0x9c0
> |
> | ---[ end trace 139ce121c98e96cb ]---

This is the WARN_ONCE(!irqs_disabled()) check.

static inline bool arch_irqs_disabled_flags(unsigned long flags)
{
return (flags & ~ALLOWINT) != 0;
}

with flags = 0x2300. Due to the "special" value of ALLOWINT on Atari:

#if defined(MACH_ATARI_ONLY)
/* block out HSYNC on the atari */
#define ALLOWINT (~0x400)
#define MAX_NOINT_IPL 3
#else
/* portable version */
#define ALLOWINT (~0x700)
#define MAX_NOINT_IPL 0
#endif /* machine compilation types */

the test fails.

Would it harm to always use the "portable" version?
That one is used on multi-platform kernels anyway?
Or would it cause too many HBLANK interrupts?

BTW, MAX_NOINT_IPL is no longer used.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

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

2011-06-16 20:03:39

by Thomas Gleixner

[permalink] [raw]
Subject: Re: m68k: Convert to genirq (WIP)

On Thu, 16 Jun 2011, Geert Uytterhoeven wrote:
> > Seems like everything (Atari/ARAnyM and Amiga) still works when using
> > handle_simple_irq instead of handle_level_irq.
> > As a bonus, BogoMIPS is above 16 again.
>
> With handle_simple_irq(), we no longer need to define irq_{,un}mask() methods
> in our irq_chips. Hence the "old" m68k platform interrupt code seems to be much
> closer to genirq than I thought...
>
> Does this make sense?

It makes sense, when your interrupt chips do not require any action
when an interrupt is handled. In that case you can avoid the hardware
access.

Thanks,

tglx

2011-06-16 21:10:12

by Michael Schmitz

[permalink] [raw]
Subject: Re: m68k: Convert to genirq (WIP)

On Fri, Jun 17, 2011 at 7:45 AM, Geert Uytterhoeven
<[email protected]> wrote:
Hi Geert,

> This is the WARN_ONCE(!irqs_disabled()) check.
>
> static inline bool arch_irqs_disabled_flags(unsigned long flags)
> {
> ? ? ? ?return (flags & ~ALLOWINT) != 0;
> }
>
> with flags = 0x2300. Due to the "special" value of ALLOWINT on Atari:
>
> #if defined(MACH_ATARI_ONLY)
> ? ? ? ?/* block out HSYNC on the atari */
> #define ALLOWINT ? ? ? ?(~0x400)
> #define MAX_NOINT_IPL ? 3
> #else
> ? ? ? ?/* portable version */
> #define ALLOWINT ? ? ? ?(~0x700)
> #define MAX_NOINT_IPL ? 0
> #endif /* machine compilation types */
>
> the test fails.
>
> Would it harm to always use the "portable" version?
> That one is used on multi-platform kernels anyway?
> Or would it cause too many HBLANK interrupts?

I'd say it would cause too many unnecessary interrupts. At least with
the original Falcon hardware that was a problem (haven't ever tried
this on the CT60). Not sure I tried multi platform kernels in a long
time, either. For these, it would probably required to male ALLOWINT a
runtime optiion in order to avoid this problem (I seem to recall we
used the corresponding hbl interrrupt handler for this originally
since it only was a problem on Falcon, not on TT. Does the TT use IPL
1 and 2, Andreas?).

MAX_NOINT_IPL may not needed any longer because all interrupt and
signal return is now done from assembler code in entry.S, I guess.

Probably best to ignore the two lowest IRQ bits on Atari for the
purpose of this test since these are always going to be disabled.

Cheers,

Michael


> BTW, MAX_NOINT_IPL is no longer used.
>
> Gr{oetje,eeting}s,
>
> ? ? ? ? ? ? ? ? ? ? ? ? Geert
>
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]
>
> 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
>

2011-06-17 03:44:58

by Finn Thain

[permalink] [raw]
Subject: Re: m68k: Convert to genirq (WIP)


On Thu, 16 Jun 2011, Geert Uytterhoeven wrote:

> On Wed, Jun 15, 2011 at 21:44, Geert Uytterhoeven <[email protected]> wrote:
> > On Sat, Jun 4, 2011 at 19:56, Geert Uytterhoeven <[email protected]> wrote:
> >> On Sat, May 28, 2011 at 20:32, Geert Uytterhoeven <[email protected]> wrote:
> >>
> >> It works on Amiga now, too. It can still use some optimizations in
> >> the irq_{enable,disable,ack,mask,mask_ack,unmask} area, as my
> >> BogoMIPS rating dropped by ca 2.5% and is now under 16, for a 25 MHz
> >> 68040.
> >
> > Seems like everything (Atari/ARAnyM and Amiga) still works when using
> > handle_simple_irq instead of handle_level_irq. As a bonus, BogoMIPS is
> > above 16 again.

Your bogomips benchmark would be the best-case penalty, right?

Have you tried say, sending a ping flood to measure throughput and
latency?

>
> With handle_simple_irq(), we no longer need to define irq_{,un}mask()
> methods in our irq_chips. Hence the "old" m68k platform interrupt code
> seems to be much closer to genirq than I thought...
>
> Does this make sense?

If the simple irq model gets us closer to the goal and doesn't kill
performance then it makes sense to me.

Finn

>
> Gr{oetje,eeting}s,
>
>                         Geert
>

2011-06-17 05:01:35

by Geert Uytterhoeven

[permalink] [raw]
Subject: Re: m68k: Convert to genirq (WIP)

On Fri, Jun 17, 2011 at 05:45, Finn Thain <[email protected]> wrote:
> On Thu, 16 Jun 2011, Geert Uytterhoeven wrote:
>> On Wed, Jun 15, 2011 at 21:44, Geert Uytterhoeven <[email protected]> wrote:
>> > On Sat, Jun 4, 2011 at 19:56, Geert Uytterhoeven <[email protected]> wrote:
>> >> On Sat, May 28, 2011 at 20:32, Geert Uytterhoeven <[email protected]> wrote:
>> >>
>> >> It works on Amiga now, too. It can still use some optimizations in
>> >> the irq_{enable,disable,ack,mask,mask_ack,unmask} area, as my
>> >> BogoMIPS rating dropped by ca 2.5% and is now under 16, for a 25 MHz
>> >> 68040.
>> >
>> > Seems like everything (Atari/ARAnyM and Amiga) still works when using
>> > handle_simple_irq instead of handle_level_irq. As a bonus, BogoMIPS is
>> > above 16 again.
>
> Your bogomips benchmark would be the best-case penalty, right?
>
> Have you tried say, sending a ping flood to measure throughput and
> latency?

No, I haven't compared network performance.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

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

2011-06-17 17:22:42

by Andreas Schwab

[permalink] [raw]
Subject: Re: m68k: Convert to genirq (WIP)

Geert Uytterhoeven <[email protected]> writes:

> This is the WARN_ONCE(!irqs_disabled()) check.
>
> static inline bool arch_irqs_disabled_flags(unsigned long flags)
> {
> return (flags & ~ALLOWINT) != 0;

That should be

return (flags & 0x700) == 0x700;

It doesn't make sense to use ALLOWINT here, since arch_local_irq_disable
doesn't use it either.

Andreas.

--
Andreas Schwab, [email protected]
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5
"And now for something completely different."

2011-06-17 17:25:18

by Andreas Schwab

[permalink] [raw]
Subject: Re: m68k: Convert to genirq (WIP)

Michael Schmitz <[email protected]> writes:

> Does the TT use IPL 1 and 2, Andreas?).

The TT has the same interrupts.

Andreas.

--
Andreas Schwab, [email protected]
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5
"And now for something completely different."

2011-06-19 10:17:37

by Geert Uytterhoeven

[permalink] [raw]
Subject: Re: m68k: Convert to genirq (WIP)

On Fri, Jun 17, 2011 at 19:22, Andreas Schwab <[email protected]> wrote:
> Geert Uytterhoeven <[email protected]> writes:
>
>> This is the WARN_ONCE(!irqs_disabled()) check.
>>
>> static inline bool arch_irqs_disabled_flags(unsigned long flags)
>> {
>>         return (flags & ~ALLOWINT) != 0;
>
> That should be
>
>          return (flags & 0x700) == 0x700;
>
> It doesn't make sense to use ALLOWINT here, since arch_local_irq_disable
> doesn't use it either.

Unfortunately this makes it worse:

| Calibrating delay loop... kernel BUG at kernel/posix-cpu-timers.c:1314!

BUG_ON(!irqs_disabled()) triggers as flags is 0x2619 there.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

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

2011-06-19 11:19:10

by Andreas Schwab

[permalink] [raw]
Subject: Re: m68k: Convert to genirq (WIP)

Geert Uytterhoeven <[email protected]> writes:

> BUG_ON(!irqs_disabled()) triggers as flags is 0x2619 there.

Which is correct, since arch_local_irq_disable would set it to 2719.
You need to find out why arch_local_irq_disable wasn't called.

Andreas.

--
Andreas Schwab, [email protected]
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5
"And now for something completely different."

2011-06-19 13:37:01

by Geert Uytterhoeven

[permalink] [raw]
Subject: Re: m68k: Convert to genirq (WIP)

On Sun, Jun 19, 2011 at 13:19, Andreas Schwab <[email protected]> wrote:
> Geert Uytterhoeven <[email protected]> writes:
>
>> BUG_ON(!irqs_disabled()) triggers as flags is 0x2619 there.
>
> Which is correct, since arch_local_irq_disable would set it to 2719.
> You need to find out why arch_local_irq_disable wasn't called.

I don't think we ever explicitly disable interrupts in interrupt handlers.
The CPU disables all interupts of the same or lower level anyway.
As all Atari interrupts (except for hblank and vblank) run at level 6,
this is why flags & 0x700 == 0x600 above.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

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

2011-06-19 13:59:02

by Andreas Schwab

[permalink] [raw]
Subject: Re: m68k: Convert to genirq (WIP)

Geert Uytterhoeven <[email protected]> writes:

> On Sun, Jun 19, 2011 at 13:19, Andreas Schwab <[email protected]> wrote:
>> Geert Uytterhoeven <[email protected]> writes:
>>
>>> BUG_ON(!irqs_disabled()) triggers as flags is 0x2619 there.
>>
>> Which is correct, since arch_local_irq_disable would set it to 2719.
>> You need to find out why arch_local_irq_disable wasn't called.
>
> I don't think we ever explicitly disable interrupts in interrupt handlers.

Then the BUG_ON is wrong.

Andreas.

--
Andreas Schwab, [email protected]
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5
"And now for something completely different."