[PATCH 2/4] ARM: OMAP2/3: intc: Add DT support for TI interrupt controller

[Rob Herring]

On 12/09/2011 10:06 AM, Cousson, Benoit wrote:
> On 12/9/2011 4:22 PM, Rob Herring wrote:
>> On 12/09/2011 08:52 AM, Cousson, Benoit wrote:
>>> On 12/9/2011 2:20 PM, Rob Herring wrote:
>>>> On 12/08/2011 08:59 AM, Cousson, Benoit wrote:
>>>>> On 12/7/2011 10:20 PM, Rob Herring wrote:
>>>>>> On 12/07/2011 02:50 PM, Benoit Cousson wrote:
>>>>>
>>>>> [...]
>>>>>
>>>>>>> +#ifdef CONFIG_OF
>>>>>>> +int __init intc_of_init(struct device_node *node, struct
>>>>>>> device_node
>>>>>>> *parent)
>>>>>>> +{
>>>>>>> +    struct resource res;
>>>>>>> +    u32 nr_irqs;
>>>>>>> +
>>>>>>> +    if (WARN_ON(!node))
>>>>>>> +        return -ENODEV;
>>>>>>> +
>>>>>>> +    if (of_address_to_resource(node, 0,&res)) {
>>>>>>> +        WARN(1, "unable to get intc registers\n");
>>>>>>> +        return -EINVAL;
>>>>>>> +    }
>>>>>>> +
>>>>>>> +    if (of_property_read_u32(node, "ti,intc-size",&nr_irqs)) {
>>>>>>> +        WARN(1, "unable to get intc-size\n");
>>>>>>> +        return -EINVAL;
>>>>>>
>>>>>> There is no default value that makes sense?
>>>>>
>>>>> So far we have 96 or 128, so I can put 96 as a default and just
>>>>> keep the
>>>>> warning without returning an error.
>>>>>
>>>>>>> +    }
>>>>>>> +
>>>>>>> +    omap_init_irq(res.start, nr_irqs);
>>>>>>> +    irq_domain_add_simple(node, 0);
>>>>>>
>>>>>> Have you read the NO_IRQ thread...
>>>>>
>>>>> Yeah, I tried, but that's a long email thread with some unclear
>>>>> conclusion...
>>>>> The point is that the few users of that API today are using 0 as a
>>>>> base
>>>>> as well, so I thought it was still valid.
>>>>>
>>>>>> Is 0 ever a valid interrupt for a driver? If so, you must not use
>>>>>> 0 for
>>>>>> the base. I would pick 16 to skip over legacy ISA irqs.
>>>>>
>>>>> I do not have any 0 interrupts right now, my concern is that I still
>>>>> have some legacy non-DT drivers that expect the previous hwirq = irq
>>>>> mapping.
>>>>
>>>> I guess it depends how easily you can shift all the irq defines. You
>>>> allow specifying the base so that you can set it to 0 for non-DT and -1
>>>> (dynamic allocation) for DT.
>>>
>>> The issue, is that the IRQs are not defined anymore but hard coded in
>>> the hwmod database. And the idea is that this is reflecting exactly the
>>> hwirq from the spec, so I do not want to add any artificial offset for
>>> the domain in the original data.
>>>
>>>>>> irqdomains should always be enabled regardless of CONFIG_OF. So
>>>>>> either
>>>>>> you can leave it as is if OF is always enabled for OMAP, or you
>>>>>> should
>>>>>> move domain setup into omap_init_irq.
>>>>>
>>>>> OK, but it looks like I cannot really modify the current INTC to DT
>>>>> without having fully adapted the OMAP drivers to use
>>>>> irq_of_parse_and_map. Or did I miss something?
>>>>>
>>>>
>>>> No, the drivers should not need to be modified as long as they get the
>>>> irq's from platform device resources. You just want to make sure the
>>>> INTC has no knowledge of it's irq base so it can change.
>>>
>>> OK, the driver will not have to change but the IRQ value will not be the
>>> same in the case of DT since it will use the irq_create_of_mapping.
>>>
>>> Currently, the driver IRQ resource is 7 for example for the twl, this is
>>> the hwirq (= irq). If I use a domain with a base of 16, the resource
>>> will still be 7 except if the resource is created from OF, then it will
>>> be 23.
>>>
>>> The only way I have today to maintain the legacy drivers to work without
>>> hacking the OMAP resource code is to keep the base_irq at 0.
>>>
>>
>> For non-DT case, that is fine. It should not be hard to support 0 for
>> legacy and !0 for DT.
> 
> Mmm, I'm probably missing something here.
> 
> My point is that even in the DT case I do have some devices that are
> initialized without DT for the moment and thus cannot get access to the
> interrupt-controller node and then cannot retrieve the domain information.
> 
> How can I ensure the proper hwirq -> irq translation then for such devices?
> Only the one created by DT will have the correct irq number.
> 

Okay, I missed that aspect of it. So I guess 0 base is fine for now.

Rob

> Adding some hacks to add a +16 is clearly possible, but that will be a
> dirty hacks in the OMAP core code that will have to be removed later. So
> I'd rather keep the domain with a 0 offset to avoid all these temporary
> hacks.
> 
>>> Since I do not have any hwirq at 0 so far, is it a big deal to keep the
>>> base_irq at 0 for the moment?
>>> It will be easier to transition to DT that way without breaking the
>>> existing drivers.
>>
>> As long as it is trivial to change later. I'm afraid that if it is not
>> changed, then we won't know if it is trivial.
> 
> As soon as every devices are initialized from DT, the irq_base will
> become transparent, since both the IRQ handler and the IRQ resource will
> use the irq_domain_to_irq to get the irq from the hwirq. At that time
> adding 16 will then be straightforward.
> That's why I prefer that approach which is the one that generate the
> minimal temporary effort.
> 
> Regards,
> Benoit