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

[Yoder Stuart-B08248]

> -----Original Message-----
> From: Kumar Gala [mailto:galak at kernel.crashing.org] 
> Sent: Thursday, March 22, 2007 10:44 AM
> To: Olof Johansson
> Cc: Yoder Stuart-B08248; linuxppc-dev at ozlabs.org
> Subject: Re: [Fwd: [PATCH Resend 01/02] Add Linux ASMP 
> support for MPC8641D]
> 
> 
> 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.

As I understand the patch, there are two DTS files once for each core
and each having a partitioned set of devices for that core.  In the 
current patch the pic in each DTS has a unique 'interrupts' property.

This shouldn't be necessary-- to initialize the pic should simply be
a matter of each core walking it's device tree, which only has the
devices 
assigned to it, and programming the pic appropriately.   No registers
should be programmed twice.

Stuart