MPC8272- Porting HDLC driver from 2.6.14 to 2.6.27- "no_irq_chip" error

FYI. The same applies to mpc8xx targets: No default host interrupt controller.
The following patch was needed for our target:
---
---
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) 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/  |
>
> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG v1.4.9 (GNU/Linux)
>
> iEYEARECAAYFAkoehIYACgkQD27XaX1/VRsAygCePysW72eSPbW0rdM5DZ6lJS+7
> lEwAoItsU+K2CO9Eqfrwj64TgwEskB85
> =+3mh
> -----END PGP SIGNATURE-----
>
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@ozlabs.org
> https://ozlabs.org/mailman/listinfo/linuxppc-dev
>

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

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ř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
>

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 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.

- Frank

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.