Beware of revision 218075

[Nathan Whitehorn]

On 01/29/11 17:18, Marcel Moolenaar wrote:
> On Jan 29, 2011, at 3:05 PM, Nathan Whitehorn wrote:
>
>> On 01/29/11 16:58, Marcel Moolenaar wrote:
>>> On Jan 29, 2011, at 2:47 PM, Nathan Whitehorn wrote:
>>>
>>>> On 01/29/11 16:38, Marcel Moolenaar wrote:
>>>>> On Jan 29, 2011, at 2:30 PM, Nathan Whitehorn wrote:
>>>>>
>>>>>>> I'll be causing some more churn in the coming weeks, but that
>>>>>>> should be mostly related to the FDT support and as such should not
>>>>>>> impact any powerpc platforms other than mpc85xx.
>>>>>>>
>>>>>> I would really have appreciated a heads up and a chance to test this before it went into the tree. I would also note that you appear well on your way to duplicating all the PCI logic in /sys/powerpc/ofw, and that unifying OFW/FDT, since they are the same thing, would probably save a lot of pain down the road.
>>>>> Yes, unification between FDT and OFW is on my mind. However, I first
>>>>> need FDT to work well before I can unify. Unifying powerpc with arm
>>>>> is the first step.
>>>> Understood. Also, G5 machines are totally broken by your commit.
>>> Ok. What exactly is broken?
>> It's actually not G5 machines, but SMP machines. The problem is this part of the commit:
>>
>> Index: intr_machdep.c
>> ===================================================================
>> --- intr_machdep.c      (revision 218074)
>> +++ intr_machdep.c      (revision 218075)
>> @@ -373,6 +426,9 @@
>>                     i->pol != INTR_POLARITY_CONFORM)
>>                         PIC_CONFIG(i->pic, i->intline, i->trig, i->pol);
>>
>> +               if (i != NULL&&  i->pic == root_pic)
>> +                       PIC_BIND(i->pic, i->intline, i->cpu);
>> +
>>                 if (i->event != NULL)
>>                         PIC_ENABLE(i->pic, i->intline, vector);
>>         }
>
> That can be reverted without harm AFAICT. One of my earlier
> changes removed the SYSINIT to do the binding and instead do
> it here. I partially reverted that (i.e. the SYSINIT is
> still there, but the change above remained), because I didn't
> know the consequence of such a change.
I've reverted it in r218081. Everything seems OK now.
>> The "max IRQ = 128" thing also likely breaks Cell systems, like the PS3, where the PIC has 256 interrupts, but I haven't looked into that in detail yet.
> This is where we need to implement multiple passes. I tried to
> do that in the same commit, but ended up with a lot more
> churn than I was comfortable handling. But ideally, we probe
> busses and interrupt controllers first, so that we don't have
> this problem -- all interrupt controllers will have registered
> before we need to map from PIC+pin to IRQ.
>
> There's probably a quick fix in case it's broken. If the pin
> is higher than the number of IRQs we have recorded for the PIC,
> we can simply bump the number of irqs the PIC has (i.e. extend
> the IRQ range assigned to the PIC). Unless we have other PICs
> recorded this can always be done -- since the PS3 only has 1
> PIC, this should be a quick kluge to get it working again.

Ah, OK. My PS3 seems to boot fine, so don't worry about it. Good luck 
navigating through the rest of this minefield!
-Nathan