Re: OMAP34xx

Cousson, Benoit
2012-02-08T18:40:03Z

On 2/7/2012 9:41 AM, Russell King - ARM Linux wrote:
> On Tue, Feb 07, 2012 at 01:54:57AM +0100, Cousson, Benoit wrote:
>> Hi Russell,
>>
>> On 2/7/2012 1:24 AM, Russell King - ARM Linux wrote:
>>> On Tue, Feb 07, 2012 at 12:19:50AM +0100, Cousson, Benoit wrote:
>>>> In theory that patch should not be even needed.
>>>
>>> In theory that change is needed to fix the obviously broken code which
>>> is there at the moment.
>>
>> Well, both patches were supposed to be merged during the last merge
>> window, so this code is not broken
>
> You're talking out your arse.

Yep, this requires years of practice, but the harder is not necessarily
to talk but mainly to handle the keyboard.

> It's fucked, because:
>
> domain.irq_base = pdata->irq_base;
> domain.nr_irq = nr_irqs;
> #ifdef CONFIG_OF_IRQ
> domain.of_node = of_node_get(node);
> domain.ops =&irq_domain_simple_ops;
> #endif
> irq_domain_add(&domain);
>
> YOU MUST NEVER EVER REGISTER AN IRQ DOMAIN WITH A NULL .ops MEMBER OTHERWISE
> THE KERNEL IS GUARANTEED TO OOPS.

OK, I think I get it now. AFAIR the domain.ops was added inside the
CONFIG_OF_IRQ because irq_domain_simple_ops was just defined if
CONFIG_OF_IRQ was defined at that time and thus the build was broken for
non-DT build.

It appears that this dependency was indeed broken because after googling
a little bit based on Mark's comment I found that a proper patch to fix
that was sent during 3.2-rc5.

commit c87fb57346fc7653ace98769f148e0dcd88ac1ee
Author: Jamie Iles <jamie@jamieiles.com>
Date: Wed Dec 14 23:43:16 2011 +0100

ARM: 7235/1: irqdomain: export irq_domain_simple_ops for !CONFIG_OF

So indeed, it is now safe and better to remove this CONFIG_OF_IRQ to
avoid the oops before Grant nuke that code with the new
irq_domain_add_legacy.

Regards,
Benoit