All of lore.kernel.org
 help / color / mirror / Atom feed
* MPC8272- Porting HDLC driver from 2.6.14 to 2.6.27- "no_irq_chip" error
@ 2009-05-28  7:37 Daniel Ng
  2009-05-28 10:33 ` Norbert van Bolhuis
  0 siblings, 1 reply; 13+ messages in thread
From: Daniel Ng @ 2009-05-28  7:37 UTC (permalink / raw)
  To: linuxppc-dev

Hi,

I'm attempting to port our Ethos HDLC driver from 2.6.14 to 2.6.27, on
a MPC8272-based platform.

So far, the kernel crashes when the driver tries to open (see below).

It seems that the interrupt handler fails to register, with the
following condition in setup_irq() in manage.c:

desc->chip == &no_irq_chip

I notice that the only place where desc->chip is assigned to anything
else besides &no_irq_chip is in __set_irq_handler() in
kernel/irq/chip.c

In that file, __set_irq_handler() assigns desc->chip to
&dummy_irq_chip, but this seems to be done for a special ARM-specific
case, according to the commenting:

/*
 * Some ARM implementations install a handler for really dumb
 * interrupt hardware without setting an irq_chip. This worked
 * with the ARM no_irq_chip but the check in setup_irq would
 * prevent us to setup the interrupt at all. Switch it to
 * dummy_irq_chip for easy transition.
 */

Should I try to somehow call __set_irq_handler(), or should I be
looking elsewhere?

If I should be calling __set_irq_handler(), it seems the only relevant
function that calls this is cpm2_pic_host_map().

The only relevant functions which call cpm2_pic_host_map() are
irq_setup_virq() or irq_alloc_hosts() with the IRQ_HOST_MAP_LEGACY
parameter. IRQ_HOST_MAP_LEGACY seems to be irrelevant. Can someone
tell me what a virq is?

Cheers,
Daniel



Badness at c00224ec [verbose debug info unavailable]
NIP: c00224ec LR: c019b254 CTR: c01aa9f8
REGS: c1a49c70 TRAP: 0700   Not tainted  (2.6.27.19-800-OS-03050107)
MSR: 00021032 <ME,IR,DR>  CR: 22002022  XER: 00000000
TASK = c1bda400[306] 'pppd' THREAD: c1a48000
GPR00: 00000001 c1a49d20 c1bda400 00000000 c0318880 c19c4d80 c1b92211 00000000
GPR08: 00001032 c02cb240 00000000 00000000 22002022 fffffffe 01ff8000 00000000
GPR16: 10344020 00000000 00000002 10049ac0 c194f800 ffff8914 c18cd900 c18cd90c
GPR24: c1a49e48 00009032 c1a48000 c02b5fdc 00000002 c19c4d80 c1a48000 c1a48000
NIP [c00224ec] local_bh_enable+0x94/0xb4
LR [c019b254] dev_queue_xmit+0x108/0x580
Call Trace:
[c1a49d20] [c19c4d80] 0xc19c4d80 (unreliable)
[c1a49d30] [c019b254] dev_queue_xmit+0x108/0x580
[c1a49d50] [c016ac98] sppp_flush_xmit+0x20/0x44
[c1a49d60] [c016c0b4] sppp_open+0x80/0xac
[c1a49d80] [c016a104] ppp_open+0x70/0x98
--- Exception: bfd26bb0 at 0x8914
    LR = 0xc1a49e90
[c1a49da0] [c01699e0] hdlc_open+0x3c/0x104 (unreliable)
[c1a49dc0] [c016cdd4] ethos_wan_genhdlc_open+0xb0/0xef8
[c1a49df0] [c019c490] dev_open+0xbc/0x120
[c1a49e00] [c019bbc8] dev_change_flags+0x8c/0x1d0
[c1a49e20] [c01e1678] devinet_ioctl+0x700/0x7ac
[c1a49e90] [c01e2538] inet_ioctl+0xcc/0xf8
[c1a49ea0] [c018b584] sock_ioctl+0x60/0x268
[c1a49ec0] [c0084ab0] vfs_ioctl+0x3c/0xc4
[c1a49ee0] [c0084bb8] do_vfs_ioctl+0x80/0x454
[c1a49f10] [c0084fcc] sys_ioctl+0x40/0x88
[c1a49f40] [c000f928] ret_from_syscall+0x0/0x38
--- Exception: c01 at 0x480af50c
    LR = 0x480af5e4
Instruction dump:
41a20008 482044e1 80010014 83e1000c 38210010 7c0803a6 4e800020 3d20c02d
3929b240 800900dc 7c000034 5400d97e <0f000000> 2f800000 41beff90 38000001
hdlc2: Carrier detected
setup_irq()- desc->chip == &no_irq_chip
request_irq()- setup_irq() FAILED
ethos_wan_genhdlc_open(): request_irq() FAILED! ethos_wan->io_addr: 0xc5080000

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

* Re: MPC8272- Porting HDLC driver from 2.6.14 to 2.6.27- "no_irq_chip" error
  2009-05-28  7:37 MPC8272- Porting HDLC driver from 2.6.14 to 2.6.27- "no_irq_chip" error Daniel Ng
@ 2009-05-28 10:33 ` Norbert van Bolhuis
  2009-05-28 12:33   ` Wolfram Sang
  2009-06-02  0:46   ` Benjamin Herrenschmidt
  0 siblings, 2 replies; 13+ messages in thread
From: Norbert van Bolhuis @ 2009-05-28 10:33 UTC (permalink / raw)
  To: Daniel Ng; +Cc: linuxppc-dev

Hi Daniel,

"Ethos" driver... hmm. sounds familiar!
(good to hear that it is still used in active development)

About your question.

Since almost 2 years (kernel 2.6.22 from july 2007) the rule is that you can't
directly map a hardware irq number because the powerpc kernel keeps a
mapping between hardware irq numbers and virtual irq numbers.
request_irq() expects a virtual irq number.

Here's some background info why the linux PowerPC kernel works this way:

The basic request_irq() function is generic, but the value of the
arguments (especially the number for the IRQ line) is architecture
specific in many ways. This is one difference between the i386 code
and the powerpc code inside Linux. Most i386 hardware is standard
PC hardware with very clearly defined interrupt sources. Because of
this, the mapping from the numeric IRQ value to a real hardware
interrupt source is defined pretty clearly.
(In fact, not even clearly anymore  :-)   IE, there are still some legacy
  interrupts at fixed numbers but most things are remapped on x86 too
  nowadays when using IO_APICs, the kernel obtains numbers from ACPI,
  remaps them etc...)
The powerpc architecture code has to support almost arbitrarily complex
hardware, and the embedded world is the source of most of the complexity.
Because of this, the powerpc code has to dynamically allocate those numeric
IRQ sources and tie them to a specific hardware interrupt.
This is why there's a mapping between hardware irq numbers and virtual
irq numbers.


this is an example of how a simple 8313 Periodic Interval Timer (PIT) kernel driver
registers for the PIT IRQ (Interrupt ID 65)

#define PIT_IRQ 65

     virq = irq_create_mapping(NULL, PIT_IRQ);
     set_irq_type(virq, IRQ_TYPE_LEVEL_LOW);

     if(request_irq(virq, (irq_handler_t)timerEvent, 0, "timer2", (void *)0)) {
         printk(KERN_ERR "request_irq() returned error for irq=%d virq=%d\n", PIT_IRQ, virq);
     }

All the above info comes from this mailing (and the linuxppc-embedd list) though.
If you search these lists you'll find plenty of answers to similar questions.

---
N. van Bolhuis
AimValley




Daniel Ng wrote:
> Hi,
> 
> I'm attempting to port our Ethos HDLC driver from 2.6.14 to 2.6.27, on
> a MPC8272-based platform.
> 
> So far, the kernel crashes when the driver tries to open (see below).
> 
> It seems that the interrupt handler fails to register, with the
> following condition in setup_irq() in manage.c:
> 
> desc->chip == &no_irq_chip
> 
> I notice that the only place where desc->chip is assigned to anything
> else besides &no_irq_chip is in __set_irq_handler() in
> kernel/irq/chip.c
> 
> In that file, __set_irq_handler() assigns desc->chip to
> &dummy_irq_chip, but this seems to be done for a special ARM-specific
> case, according to the commenting:
> 
> /*
>  * Some ARM implementations install a handler for really dumb
>  * interrupt hardware without setting an irq_chip. This worked
>  * with the ARM no_irq_chip but the check in setup_irq would
>  * prevent us to setup the interrupt at all. Switch it to
>  * dummy_irq_chip for easy transition.
>  */
> 
> Should I try to somehow call __set_irq_handler(), or should I be
> looking elsewhere?
> 
> If I should be calling __set_irq_handler(), it seems the only relevant
> function that calls this is cpm2_pic_host_map().
> 
> The only relevant functions which call cpm2_pic_host_map() are
> irq_setup_virq() or irq_alloc_hosts() with the IRQ_HOST_MAP_LEGACY
> parameter. IRQ_HOST_MAP_LEGACY seems to be irrelevant. Can someone
> tell me what a virq is?
> 
> Cheers,
> Daniel
> 
> 
> 
> Badness at c00224ec [verbose debug info unavailable]
> NIP: c00224ec LR: c019b254 CTR: c01aa9f8
> REGS: c1a49c70 TRAP: 0700   Not tainted  (2.6.27.19-800-OS-03050107)
> MSR: 00021032 <ME,IR,DR>  CR: 22002022  XER: 00000000
> TASK = c1bda400[306] 'pppd' THREAD: c1a48000
> GPR00: 00000001 c1a49d20 c1bda400 00000000 c0318880 c19c4d80 c1b92211 00000000
> GPR08: 00001032 c02cb240 00000000 00000000 22002022 fffffffe 01ff8000 00000000
> GPR16: 10344020 00000000 00000002 10049ac0 c194f800 ffff8914 c18cd900 c18cd90c
> GPR24: c1a49e48 00009032 c1a48000 c02b5fdc 00000002 c19c4d80 c1a48000 c1a48000
> NIP [c00224ec] local_bh_enable+0x94/0xb4
> LR [c019b254] dev_queue_xmit+0x108/0x580
> Call Trace:
> [c1a49d20] [c19c4d80] 0xc19c4d80 (unreliable)
> [c1a49d30] [c019b254] dev_queue_xmit+0x108/0x580
> [c1a49d50] [c016ac98] sppp_flush_xmit+0x20/0x44
> [c1a49d60] [c016c0b4] sppp_open+0x80/0xac
> [c1a49d80] [c016a104] ppp_open+0x70/0x98
> --- Exception: bfd26bb0 at 0x8914
>     LR = 0xc1a49e90
> [c1a49da0] [c01699e0] hdlc_open+0x3c/0x104 (unreliable)
> [c1a49dc0] [c016cdd4] ethos_wan_genhdlc_open+0xb0/0xef8
> [c1a49df0] [c019c490] dev_open+0xbc/0x120
> [c1a49e00] [c019bbc8] dev_change_flags+0x8c/0x1d0
> [c1a49e20] [c01e1678] devinet_ioctl+0x700/0x7ac
> [c1a49e90] [c01e2538] inet_ioctl+0xcc/0xf8
> [c1a49ea0] [c018b584] sock_ioctl+0x60/0x268
> [c1a49ec0] [c0084ab0] vfs_ioctl+0x3c/0xc4
> [c1a49ee0] [c0084bb8] do_vfs_ioctl+0x80/0x454
> [c1a49f10] [c0084fcc] sys_ioctl+0x40/0x88
> [c1a49f40] [c000f928] ret_from_syscall+0x0/0x38
> --- Exception: c01 at 0x480af50c
>     LR = 0x480af5e4
> Instruction dump:
> 41a20008 482044e1 80010014 83e1000c 38210010 7c0803a6 4e800020 3d20c02d
> 3929b240 800900dc 7c000034 5400d97e <0f000000> 2f800000 41beff90 38000001
> hdlc2: Carrier detected
> setup_irq()- desc->chip == &no_irq_chip
> request_irq()- setup_irq() FAILED
> ethos_wan_genhdlc_open(): request_irq() FAILED! ethos_wan->io_addr: 0xc5080000
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@ozlabs.org
> https://ozlabs.org/mailman/listinfo/linuxppc-dev
> 

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

* Re: MPC8272- Porting HDLC driver from 2.6.14 to 2.6.27- "no_irq_chip" error
  2009-05-28 10:33 ` Norbert van Bolhuis
@ 2009-05-28 12:33   ` Wolfram Sang
  2009-05-29  0:46     ` Daniel Ng
                       ` (2 more replies)
  2009-06-02  0:46   ` Benjamin Herrenschmidt
  1 sibling, 3 replies; 13+ messages in thread
From: Wolfram Sang @ 2009-05-28 12:33 UTC (permalink / raw)
  To: Norbert van Bolhuis; +Cc: linuxppc-dev, Daniel Ng

[-- Attachment #1: Type: text/plain, Size: 1495 bytes --]

> this is an example of how a simple 8313 Periodic Interval Timer (PIT) kernel driver
> registers for the PIT IRQ (Interrupt ID 65)
>
> #define PIT_IRQ 65
>
>     virq = irq_create_mapping(NULL, PIT_IRQ);
>     set_irq_type(virq, IRQ_TYPE_LEVEL_LOW);
>
>     if(request_irq(virq, (irq_handler_t)timerEvent, 0, "timer2", (void *)0)) {
>         printk(KERN_ERR "request_irq() returned error for irq=%d virq=%d\n", PIT_IRQ, virq);
>     }

It is some time ago, but when I did something similar I needed the
following patch in order to use NULL for irq_create_mapping(). Have a
try, and if it is still needed (as it looks from a glimpse), then maybe
we should get it merged?

===

From: Wolfram Sang <w.sang@pengutronix.de>
Subject: [PATCH] powerpc/cpm2: make cpm2_pic the default host

Signed-off-by: Wolfram Sang <w.sang@pengutronix.de>
---
 arch/powerpc/sysdev/cpm2_pic.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/arch/powerpc/sysdev/cpm2_pic.c b/arch/powerpc/sysdev/cpm2_pic.c
index 78f1f7c..7a7d4e5 100644
--- a/arch/powerpc/sysdev/cpm2_pic.c
+++ b/arch/powerpc/sysdev/cpm2_pic.c
@@ -272,4 +272,5 @@ void cpm2_pic_init(struct device_node *node)
 		printk(KERN_ERR "CPM2 PIC: failed to allocate irq host!\n");
 		return;
 	}
+	irq_set_default_host(cpm2_pic_host);
 }

-- 
Pengutronix e.K.                           | Wolfram Sang                |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

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

* Re: MPC8272- Porting HDLC driver from 2.6.14 to 2.6.27- "no_irq_chip" error
  2009-05-28 12:33   ` Wolfram Sang
@ 2009-05-29  0:46     ` Daniel Ng
  2009-05-29  8:31       ` [PATCH] powerpc/cpm2: make cpm2_pic the default host Wolfram Sang
  2009-05-29 10:56     ` MPC8272- Porting HDLC driver from 2.6.14 to 2.6.27- "no_irq_chip" error Frank Svendsbøe
  2009-06-02  0:47     ` Benjamin Herrenschmidt
  2 siblings, 1 reply; 13+ messages in thread
From: Daniel Ng @ 2009-05-29  0:46 UTC (permalink / raw)
  To: Wolfram Sang; +Cc: linuxppc-dev, Norbert van Bolhuis

On Thu, May 28, 2009 at 10:33 PM, Wolfram Sang <w.sang@pengutronix.de> wrote:
>> this is an example of how a simple 8313 Periodic Interval Timer (PIT) kernel driver
>> registers for the PIT IRQ (Interrupt ID 65)
>>
>> #define PIT_IRQ 65
>>
>>     virq = irq_create_mapping(NULL, PIT_IRQ);
>>     set_irq_type(virq, IRQ_TYPE_LEVEL_LOW);
>>
>>     if(request_irq(virq, (irq_handler_t)timerEvent, 0, "timer2", (void *)0)) {
>>         printk(KERN_ERR "request_irq() returned error for irq=%d virq=%d\n", PIT_IRQ, virq);
>>     }
>
> It is some time ago, but when I did something similar I needed the
> following patch in order to use NULL for irq_create_mapping(). Have a
> try, and if it is still needed (as it looks from a glimpse), then maybe
> we should get it merged?
>
> ===
>
> From: Wolfram Sang <w.sang@pengutronix.de>
> Subject: [PATCH] powerpc/cpm2: make cpm2_pic the default host
>
> Signed-off-by: Wolfram Sang <w.sang@pengutronix.de>
> ---
>  arch/powerpc/sysdev/cpm2_pic.c |    1 +
>  1 files changed, 1 insertions(+), 0 deletions(-)
>
> diff --git a/arch/powerpc/sysdev/cpm2_pic.c b/arch/powerpc/sysdev/cpm2_pic.c
> index 78f1f7c..7a7d4e5 100644
> --- a/arch/powerpc/sysdev/cpm2_pic.c
> +++ b/arch/powerpc/sysdev/cpm2_pic.c
> @@ -272,4 +272,5 @@ void cpm2_pic_init(struct device_node *node)
>                printk(KERN_ERR "CPM2 PIC: failed to allocate irq host!\n");
>                return;
>        }
> +       irq_set_default_host(cpm2_pic_host);
>  }

Thanks guys.

I can confirm that Wolfram's patch above is required to get Norbet's
suggestion to work. Without the patch, I still get the original
symptoms. I have tested this on 2.6.27.19.

Daniel

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

* [PATCH] powerpc/cpm2: make cpm2_pic the default host
  2009-05-29  0:46     ` Daniel Ng
@ 2009-05-29  8:31       ` Wolfram Sang
  0 siblings, 0 replies; 13+ messages in thread
From: Wolfram Sang @ 2009-05-29  8:31 UTC (permalink / raw)
  To: w.sang; +Cc: linuxppc-dev

As stated in the source, this one is usually the only
interrupt-controller. To ease creating virq mappings, let it be the
default host.

Signed-off-by: Wolfram Sang <w.sang@pengutronix.de>
Acked-by: Daniel Ng <daniel.ng1234@gmail.com>
Cc: Vitaly Bordug <vitb@kernel.crashing.org>
Cc: Kumar Gala <galak@kernel.crashing.org>
Cc: linuxppc-dev@ozlabs.org
---
 arch/powerpc/sysdev/cpm2_pic.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/arch/powerpc/sysdev/cpm2_pic.c b/arch/powerpc/sysdev/cpm2_pic.c
index 78f1f7c..7a7d4e5 100644
--- a/arch/powerpc/sysdev/cpm2_pic.c
+++ b/arch/powerpc/sysdev/cpm2_pic.c
@@ -272,4 +272,5 @@ void cpm2_pic_init(struct device_node *node)
 		printk(KERN_ERR "CPM2 PIC: failed to allocate irq host!\n");
 		return;
 	}
+	irq_set_default_host(cpm2_pic_host);
 }
-- 
1.6.2

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

* Re: MPC8272- Porting HDLC driver from 2.6.14 to 2.6.27- "no_irq_chip" error
  2009-05-28 12:33   ` Wolfram Sang
  2009-05-29  0:46     ` Daniel Ng
@ 2009-05-29 10:56     ` Frank Svendsbøe
  2009-05-29 17:18       ` Scott Wood
  2009-06-02  0:47     ` Benjamin Herrenschmidt
  2 siblings, 1 reply; 13+ messages in thread
From: Frank Svendsbøe @ 2009-05-29 10:56 UTC (permalink / raw)
  To: Wolfram Sang; +Cc: linuxppc-dev, Daniel Ng, Norbert van Bolhuis

FYI. The same applies to mpc8xx targets: No default host interrupt controll=
er.
The following patch was needed for our target:
---
diff --git a/arch/powerpc/sysdev/mpc8xx_pic.c b/arch/powerpc/sysdev/mpc8xx_=
pic.c
index 5d2d552..92b2b66 100644
--- a/arch/powerpc/sysdev/mpc8xx_pic.c
+++ b/arch/powerpc/sysdev/mpc8xx_pic.c
@@ -186,6 +186,7 @@ int mpc8xx_pic_init(void)
                ret =3D -ENOMEM;
                goto out;
        }
+        irq_set_default_host(mpc8xx_pic_host);
        return 0;

 out:
---
Maybe setting a default host ought to be mandatory? Or is doing the
mapping manually
(without device tree descriptions) considered being a hack?

Frank


On Thu, May 28, 2009 at 2:33 PM, Wolfram Sang <w.sang@pengutronix.de> wrote=
:
>> this is an example of how a simple 8313 Periodic Interval Timer (PIT) ke=
rnel driver
>> registers for the PIT IRQ (Interrupt ID 65)
>>
>> #define PIT_IRQ 65
>>
>> =A0 =A0 virq =3D irq_create_mapping(NULL, PIT_IRQ);
>> =A0 =A0 set_irq_type(virq, IRQ_TYPE_LEVEL_LOW);
>>
>> =A0 =A0 if(request_irq(virq, (irq_handler_t)timerEvent, 0, "timer2", (vo=
id *)0)) {
>> =A0 =A0 =A0 =A0 printk(KERN_ERR "request_irq() returned error for irq=3D=
%d virq=3D%d\n", PIT_IRQ, virq);
>> =A0 =A0 }
>
> It is some time ago, but when I did something similar I needed the
> following patch in order to use NULL for irq_create_mapping(). Have a
> try, and if it is still needed (as it looks from a glimpse), then maybe
> we should get it merged?
>
> =3D=3D=3D
>
> From: Wolfram Sang <w.sang@pengutronix.de>
> Subject: [PATCH] powerpc/cpm2: make cpm2_pic the default host
>
> Signed-off-by: Wolfram Sang <w.sang@pengutronix.de>
> ---
> =A0arch/powerpc/sysdev/cpm2_pic.c | =A0 =A01 +
> =A01 files changed, 1 insertions(+), 0 deletions(-)
>
> diff --git a/arch/powerpc/sysdev/cpm2_pic.c b/arch/powerpc/sysdev/cpm2_pi=
c.c
> index 78f1f7c..7a7d4e5 100644
> --- a/arch/powerpc/sysdev/cpm2_pic.c
> +++ b/arch/powerpc/sysdev/cpm2_pic.c
> @@ -272,4 +272,5 @@ void cpm2_pic_init(struct device_node *node)
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0printk(KERN_ERR "CPM2 PIC: failed to alloc=
ate irq host!\n");
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0return;
> =A0 =A0 =A0 =A0}
> + =A0 =A0 =A0 irq_set_default_host(cpm2_pic_host);
> =A0}
>
> --
> Pengutronix e.K. =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 | Wo=
lfram Sang =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0|
> Industrial Linux Solutions =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 | http://www.p=
engutronix.de/ =A0|
>
> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG v1.4.9 (GNU/Linux)
>
> iEYEARECAAYFAkoehIYACgkQD27XaX1/VRsAygCePysW72eSPbW0rdM5DZ6lJS+7
> lEwAoItsU+K2CO9Eqfrwj64TgwEskB85
> =3D+3mh
> -----END PGP SIGNATURE-----
>
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@ozlabs.org
> https://ozlabs.org/mailman/listinfo/linuxppc-dev
>

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

* Re: MPC8272- Porting HDLC driver from 2.6.14 to 2.6.27- "no_irq_chip" error
  2009-05-29 10:56     ` MPC8272- Porting HDLC driver from 2.6.14 to 2.6.27- "no_irq_chip" error Frank Svendsbøe
@ 2009-05-29 17:18       ` Scott Wood
  2009-05-30 20:22         ` Frank Svendsbøe
  0 siblings, 1 reply; 13+ messages in thread
From: Scott Wood @ 2009-05-29 17:18 UTC (permalink / raw)
  To: Frank Svendsbøe; +Cc: linuxppc-dev, Daniel Ng, Norbert van Bolhuis

On Fri, May 29, 2009 at 12:56:13PM +0200, Frank Svendsbøe wrote:
> FYI. The same applies to mpc8xx targets: No default host interrupt controller.
> The following patch was needed for our target:
> ---
> diff --git a/arch/powerpc/sysdev/mpc8xx_pic.c b/arch/powerpc/sysdev/mpc8xx_pic.c
> index 5d2d552..92b2b66 100644
> --- a/arch/powerpc/sysdev/mpc8xx_pic.c
> +++ b/arch/powerpc/sysdev/mpc8xx_pic.c
> @@ -186,6 +186,7 @@ int mpc8xx_pic_init(void)
>                 ret = -ENOMEM;
>                 goto out;
>         }
> +        irq_set_default_host(mpc8xx_pic_host);
>         return 0;

This patch is whitespace mangled.

> 
>  out:
> ---
> Maybe setting a default host ought to be mandatory? Or is doing the
> mapping manually
> (without device tree descriptions) considered being a hack?

I consider it a hack -- not so much doing it manually (though the device
tree is better), but relying on a default interrupt controller when doing
so.  IRQ numbers only make sense in the context of a specific
controller.  It's especially misleading on 8xx, which has separate
regular and CPM PICs.

-Scott

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

* Re: MPC8272- Porting HDLC driver from 2.6.14 to 2.6.27- "no_irq_chip" error
  2009-05-29 17:18       ` Scott Wood
@ 2009-05-30 20:22         ` Frank Svendsbøe
  2009-06-02  0:48           ` Benjamin Herrenschmidt
  0 siblings, 1 reply; 13+ messages in thread
From: Frank Svendsbøe @ 2009-05-30 20:22 UTC (permalink / raw)
  To: Scott Wood; +Cc: linuxppc-dev, Daniel Ng, Norbert van Bolhuis

On Fri, May 29, 2009 at 7:18 PM, Scott Wood <scottwood@freescale.com> wrote=
:
> On Fri, May 29, 2009 at 12:56:13PM +0200, Frank Svendsb=F8e wrote:
>> FYI. The same applies to mpc8xx targets: No default host interrupt contr=
oller.
>> The following patch was needed for our target:
>> ---
>> diff --git a/arch/powerpc/sysdev/mpc8xx_pic.c b/arch/powerpc/sysdev/mpc8=
xx_pic.c
>> index 5d2d552..92b2b66 100644
>> --- a/arch/powerpc/sysdev/mpc8xx_pic.c
>> +++ b/arch/powerpc/sysdev/mpc8xx_pic.c
>> @@ -186,6 +186,7 @@ int mpc8xx_pic_init(void)
>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 ret =3D -ENOMEM;
>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 goto out;
>> =A0 =A0 =A0 =A0 }
>> + =A0 =A0 =A0 =A0irq_set_default_host(mpc8xx_pic_host);
>> =A0 =A0 =A0 =A0 return 0;
>
> This patch is whitespace mangled.
>
>>
>> =A0out:
>> ---
>> Maybe setting a default host ought to be mandatory? Or is doing the
>> mapping manually
>> (without device tree descriptions) considered being a hack?
>
> I consider it a hack -- not so much doing it manually (though the device
> tree is better), but relying on a default interrupt controller when doing
> so. =A0IRQ numbers only make sense in the context of a specific
> controller. =A0It's especially misleading on 8xx, which has separate
> regular and CPM PICs.
>
> -Scott
>

I agree, and was the reason I mentioned "hack". The patch wasn't meant
for commit,
just for reference (sorry for whitemangling ;-)

Regarding doing manual mapping: Is there another way to retrieve the
host controller
from a driver module without modifying kernel source? In case not, do you t=
hink
exporting the mpc8xx_pic_host symbol is a better solution?

Anyway, now that I'm beginning to understand dts I guess I might as
well just do it properly.

- Frank

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

* Re: MPC8272- Porting HDLC driver from 2.6.14 to 2.6.27- "no_irq_chip" error
  2009-05-28 10:33 ` Norbert van Bolhuis
  2009-05-28 12:33   ` Wolfram Sang
@ 2009-06-02  0:46   ` Benjamin Herrenschmidt
  1 sibling, 0 replies; 13+ messages in thread
From: Benjamin Herrenschmidt @ 2009-06-02  0:46 UTC (permalink / raw)
  To: Norbert van Bolhuis; +Cc: linuxppc-dev, Daniel Ng


> 
> #define PIT_IRQ 65

In addition, the interrupt should be provided by the device-tree of
course, in which case a single function will look it up for you -and-
do the appropriate mapping.

Cheers,
Ben.

>      virq = irq_create_mapping(NULL, PIT_IRQ);
>      set_irq_type(virq, IRQ_TYPE_LEVEL_LOW);
> 
>      if(request_irq(virq, (irq_handler_t)timerEvent, 0, "timer2", (void *)0)) {
>          printk(KERN_ERR "request_irq() returned error for irq=%d virq=%d\n", PIT_IRQ, virq);
>      }
> 
> All the above info comes from this mailing (and the linuxppc-embedd list) though.
> If you search these lists you'll find plenty of answers to similar questions.
> 
> ---
> N. van Bolhuis
> AimValley
> 
> 
> 
> 
> Daniel Ng wrote:
> > Hi,
> > 
> > I'm attempting to port our Ethos HDLC driver from 2.6.14 to 2.6.27, on
> > a MPC8272-based platform.
> > 
> > So far, the kernel crashes when the driver tries to open (see below).
> > 
> > It seems that the interrupt handler fails to register, with the
> > following condition in setup_irq() in manage.c:
> > 
> > desc->chip == &no_irq_chip
> > 
> > I notice that the only place where desc->chip is assigned to anything
> > else besides &no_irq_chip is in __set_irq_handler() in
> > kernel/irq/chip.c
> > 
> > In that file, __set_irq_handler() assigns desc->chip to
> > &dummy_irq_chip, but this seems to be done for a special ARM-specific
> > case, according to the commenting:
> > 
> > /*
> >  * Some ARM implementations install a handler for really dumb
> >  * interrupt hardware without setting an irq_chip. This worked
> >  * with the ARM no_irq_chip but the check in setup_irq would
> >  * prevent us to setup the interrupt at all. Switch it to
> >  * dummy_irq_chip for easy transition.
> >  */
> > 
> > Should I try to somehow call __set_irq_handler(), or should I be
> > looking elsewhere?
> > 
> > If I should be calling __set_irq_handler(), it seems the only relevant
> > function that calls this is cpm2_pic_host_map().
> > 
> > The only relevant functions which call cpm2_pic_host_map() are
> > irq_setup_virq() or irq_alloc_hosts() with the IRQ_HOST_MAP_LEGACY
> > parameter. IRQ_HOST_MAP_LEGACY seems to be irrelevant. Can someone
> > tell me what a virq is?
> > 
> > Cheers,
> > Daniel
> > 
> > 
> > 
> > Badness at c00224ec [verbose debug info unavailable]
> > NIP: c00224ec LR: c019b254 CTR: c01aa9f8
> > REGS: c1a49c70 TRAP: 0700   Not tainted  (2.6.27.19-800-OS-03050107)
> > MSR: 00021032 <ME,IR,DR>  CR: 22002022  XER: 00000000
> > TASK = c1bda400[306] 'pppd' THREAD: c1a48000
> > GPR00: 00000001 c1a49d20 c1bda400 00000000 c0318880 c19c4d80 c1b92211 00000000
> > GPR08: 00001032 c02cb240 00000000 00000000 22002022 fffffffe 01ff8000 00000000
> > GPR16: 10344020 00000000 00000002 10049ac0 c194f800 ffff8914 c18cd900 c18cd90c
> > GPR24: c1a49e48 00009032 c1a48000 c02b5fdc 00000002 c19c4d80 c1a48000 c1a48000
> > NIP [c00224ec] local_bh_enable+0x94/0xb4
> > LR [c019b254] dev_queue_xmit+0x108/0x580
> > Call Trace:
> > [c1a49d20] [c19c4d80] 0xc19c4d80 (unreliable)
> > [c1a49d30] [c019b254] dev_queue_xmit+0x108/0x580
> > [c1a49d50] [c016ac98] sppp_flush_xmit+0x20/0x44
> > [c1a49d60] [c016c0b4] sppp_open+0x80/0xac
> > [c1a49d80] [c016a104] ppp_open+0x70/0x98
> > --- Exception: bfd26bb0 at 0x8914
> >     LR = 0xc1a49e90
> > [c1a49da0] [c01699e0] hdlc_open+0x3c/0x104 (unreliable)
> > [c1a49dc0] [c016cdd4] ethos_wan_genhdlc_open+0xb0/0xef8
> > [c1a49df0] [c019c490] dev_open+0xbc/0x120
> > [c1a49e00] [c019bbc8] dev_change_flags+0x8c/0x1d0
> > [c1a49e20] [c01e1678] devinet_ioctl+0x700/0x7ac
> > [c1a49e90] [c01e2538] inet_ioctl+0xcc/0xf8
> > [c1a49ea0] [c018b584] sock_ioctl+0x60/0x268
> > [c1a49ec0] [c0084ab0] vfs_ioctl+0x3c/0xc4
> > [c1a49ee0] [c0084bb8] do_vfs_ioctl+0x80/0x454
> > [c1a49f10] [c0084fcc] sys_ioctl+0x40/0x88
> > [c1a49f40] [c000f928] ret_from_syscall+0x0/0x38
> > --- Exception: c01 at 0x480af50c
> >     LR = 0x480af5e4
> > Instruction dump:
> > 41a20008 482044e1 80010014 83e1000c 38210010 7c0803a6 4e800020 3d20c02d
> > 3929b240 800900dc 7c000034 5400d97e <0f000000> 2f800000 41beff90 38000001
> > hdlc2: Carrier detected
> > setup_irq()- desc->chip == &no_irq_chip
> > request_irq()- setup_irq() FAILED
> > ethos_wan_genhdlc_open(): request_irq() FAILED! ethos_wan->io_addr: 0xc5080000
> > _______________________________________________
> > Linuxppc-dev mailing list
> > Linuxppc-dev@ozlabs.org
> > https://ozlabs.org/mailman/listinfo/linuxppc-dev
> > 
> 
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@ozlabs.org
> https://ozlabs.org/mailman/listinfo/linuxppc-dev

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

* Re: MPC8272- Porting HDLC driver from 2.6.14 to 2.6.27- "no_irq_chip" error
  2009-05-28 12:33   ` Wolfram Sang
  2009-05-29  0:46     ` Daniel Ng
  2009-05-29 10:56     ` MPC8272- Porting HDLC driver from 2.6.14 to 2.6.27- "no_irq_chip" error Frank Svendsbøe
@ 2009-06-02  0:47     ` Benjamin Herrenschmidt
  2009-06-02  4:38       ` Wolfram Sang
  2 siblings, 1 reply; 13+ messages in thread
From: Benjamin Herrenschmidt @ 2009-06-02  0:47 UTC (permalink / raw)
  To: Wolfram Sang; +Cc: linuxppc-dev, Daniel Ng, Norbert van Bolhuis

On Thu, 2009-05-28 at 14:33 +0200, Wolfram Sang wrote:
> > this is an example of how a simple 8313 Periodic Interval Timer (PIT) kernel driver
> > registers for the PIT IRQ (Interrupt ID 65)
> >
> > #define PIT_IRQ 65
> >
> >     virq = irq_create_mapping(NULL, PIT_IRQ);
> >     set_irq_type(virq, IRQ_TYPE_LEVEL_LOW);
> >
> >     if(request_irq(virq, (irq_handler_t)timerEvent, 0, "timer2", (void *)0)) {
> >         printk(KERN_ERR "request_irq() returned error for irq=%d virq=%d\n", PIT_IRQ, virq);
> >     }
> 
> It is some time ago, but when I did something similar I needed the
> following patch in order to use NULL for irq_create_mapping(). Have a
> try, and if it is still needed (as it looks from a glimpse), then maybe
> we should get it merged?

I would object that you wouldn't have this problem if you weren't hard
wiring your interrupt number and were using the device-tree properly
instead. As to getting your patch merged, you'll have to argue with
Scott Wood who, I think, maintains the CPM2 stuff lately.

Cheers,
Ben.

> ===
> 
> From: Wolfram Sang <w.sang@pengutronix.de>
> Subject: [PATCH] powerpc/cpm2: make cpm2_pic the default host
> 
> Signed-off-by: Wolfram Sang <w.sang@pengutronix.de>
> ---
>  arch/powerpc/sysdev/cpm2_pic.c |    1 +
>  1 files changed, 1 insertions(+), 0 deletions(-)
> 
> diff --git a/arch/powerpc/sysdev/cpm2_pic.c b/arch/powerpc/sysdev/cpm2_pic.c
> index 78f1f7c..7a7d4e5 100644
> --- a/arch/powerpc/sysdev/cpm2_pic.c
> +++ b/arch/powerpc/sysdev/cpm2_pic.c
> @@ -272,4 +272,5 @@ void cpm2_pic_init(struct device_node *node)
>  		printk(KERN_ERR "CPM2 PIC: failed to allocate irq host!\n");
>  		return;
>  	}
> +	irq_set_default_host(cpm2_pic_host);
>  }
> 
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@ozlabs.org
> https://ozlabs.org/mailman/listinfo/linuxppc-dev

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

* Re: MPC8272- Porting HDLC driver from 2.6.14 to 2.6.27- "no_irq_chip" error
  2009-05-30 20:22         ` Frank Svendsbøe
@ 2009-06-02  0:48           ` Benjamin Herrenschmidt
  0 siblings, 0 replies; 13+ messages in thread
From: Benjamin Herrenschmidt @ 2009-06-02  0:48 UTC (permalink / raw)
  To: Frank Svendsbøe
  Cc: Scott Wood, linuxppc-dev, Daniel Ng, Norbert van Bolhuis

On Sat, 2009-05-30 at 22:22 +0200, Frank Svendsbøe wrote:
> Regarding doing manual mapping: Is there another way to retrieve the
> host controller
> from a driver module without modifying kernel source? In case not, do
> you think
> exporting the mpc8xx_pic_host symbol is a better solution?
> 
> Anyway, now that I'm beginning to understand dts I guess I might as
> well just do it properly.

Well, precisely :-) The DTS allows to contain the linkage to the PIC and
let the kernel resolve it all nicely for you.

Cheers,
Ben.

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

* Re: MPC8272- Porting HDLC driver from 2.6.14 to 2.6.27- "no_irq_chip" error
  2009-06-02  0:47     ` Benjamin Herrenschmidt
@ 2009-06-02  4:38       ` Wolfram Sang
  2009-06-02  4:42         ` Benjamin Herrenschmidt
  0 siblings, 1 reply; 13+ messages in thread
From: Wolfram Sang @ 2009-06-02  4:38 UTC (permalink / raw)
  To: Benjamin Herrenschmidt; +Cc: linuxppc-dev, Daniel Ng, Norbert van Bolhuis

[-- Attachment #1: Type: text/plain, Size: 931 bytes --]

> I would object that you wouldn't have this problem if you weren't hard
> wiring your interrupt number and were using the device-tree properly
> instead. As to getting your patch merged, you'll have to argue with
> Scott Wood who, I think, maintains the CPM2 stuff lately.

I fully agree that the proper way is using the device tree. I can't recall
exactly why this wasn't an option to me a year ago, but I assume it had
something to do with this ugly out-of-tree-driver. I wouldn't really argue
about this patch, I just thought it might be useful as it seems there are
people trying to do the same for some reason. Then again, maybe it should be
skipped, as it makes it easier to not use the proper solution (= device tree)
:)

Kind regards,

   Wolfram

-- 
Pengutronix e.K.                           | Wolfram Sang                |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

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

* Re: MPC8272- Porting HDLC driver from 2.6.14 to 2.6.27- "no_irq_chip" error
  2009-06-02  4:38       ` Wolfram Sang
@ 2009-06-02  4:42         ` Benjamin Herrenschmidt
  0 siblings, 0 replies; 13+ messages in thread
From: Benjamin Herrenschmidt @ 2009-06-02  4:42 UTC (permalink / raw)
  To: Wolfram Sang; +Cc: linuxppc-dev, Daniel Ng, Norbert van Bolhuis

On Tue, 2009-06-02 at 06:38 +0200, Wolfram Sang wrote:
> I fully agree that the proper way is using the device tree. I can't recall
> exactly why this wasn't an option to me a year ago, but I assume it had
> something to do with this ugly out-of-tree-driver. I wouldn't really argue
> about this patch, I just thought it might be useful as it seems there are
> people trying to do the same for some reason. Then again, maybe it should be
> skipped, as it makes it easier to not use the proper solution (= device tree)
> :)

Whether the driver is in or out of tree shouldn't affect the device-tree
which represents the -devices- on the system regardless of whether a
driver is in-tree or not.

So while some people might flinch, I don't necessarily object to adding
nodes in the device-trees we ship with the kernel for things for which
we don't have an in-tree driver.

It does mean however that the risk of such nodes containing busted
informations that aren't spotted because nobody has the driver to test
is higher but that's a risk you are taking with out of tree drivers
anyway.

Cheers,
Ben.

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

end of thread, other threads:[~2009-06-02  4:42 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2009-05-28  7:37 MPC8272- Porting HDLC driver from 2.6.14 to 2.6.27- "no_irq_chip" error Daniel Ng
2009-05-28 10:33 ` Norbert van Bolhuis
2009-05-28 12:33   ` Wolfram Sang
2009-05-29  0:46     ` Daniel Ng
2009-05-29  8:31       ` [PATCH] powerpc/cpm2: make cpm2_pic the default host Wolfram Sang
2009-05-29 10:56     ` MPC8272- Porting HDLC driver from 2.6.14 to 2.6.27- "no_irq_chip" error Frank Svendsbøe
2009-05-29 17:18       ` Scott Wood
2009-05-30 20:22         ` Frank Svendsbøe
2009-06-02  0:48           ` Benjamin Herrenschmidt
2009-06-02  0:47     ` Benjamin Herrenschmidt
2009-06-02  4:38       ` Wolfram Sang
2009-06-02  4:42         ` Benjamin Herrenschmidt
2009-06-02  0:46   ` Benjamin Herrenschmidt

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.