[Fwd: [PATCH Resend 01/02] Add Linux ASMP support for MPC8641D]

[Kumar Gala]

On Mar 22, 2007, at 10:51 AM, Olof Johansson wrote:

> On Thu, Mar 22, 2007 at 08:32:33AM -0700, Yoder Stuart-B08248 wrote:
>>
>>> +		mpic: pic at 40000 {
>>> +			clock-frequency = <0>;
>>> +			interrupt-controller;
>>> +			#address-cells = <0>;
>>> +			#interrupt-cells = <2>;
>>> +			reg = <40000 40000>;
>>> +			built-in;
>>> +			compatible = "chrp,open-pic";
>>> +			device_type = "open-pic";
>>> +			big-endian;
>>> +			interrupts = <
>>> +			18 2 49 2 19 2
>>> +			2a 2 2b 2 4a 1
>>> +			1d 2 1e 2 22 2
>>> +			23 2 24 2 28 2
>>> +			>;
>>> +		};
>>> +	};
>>> +};
>>
>> I think using the 'interrupts' property in this way is bad.  It's
>> being used to tell the pic which interrupts belong to it.
>>
>> The problem is that the interrupts property is being overloaded
>> and used for a completely different purpose than that which
>> it was originally intended and documented.  Interrupt controllers
>> should not have an interrupts property.
>>
>> The only reason I can see to do this is as a convenience to avoid
>> walking the device tree.   The pic code should walk the device tree
>> to determine the irq numbers the running core.
>
> Not speaking for the case of the freescale boards here, but not all
> interrupts are in the device tree, nor do they have to. Especially for
> PCI devices, etc.
>
> This is quite a special case. Having a brand new property for it might
> be a good idea. It's a pretty big hack anyway, so adding another  
> one to
> the device tree isn't that big a deal.

I think the issue is you don't want the per interrupt registers  
initialized twice.  Thus I suggest we just let set_irq_type handle it.

There is still an issue for shared interrupts, but I don't think you  
can solve that.  W/o moving the interrupt processing to some common  
level.

- k