Re: OMAP34xx

["Cousson, Benoit" <b-cousson@ti.com>]

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