Date: Sat, 29 Jan 2011 15:18:32 -0800 From: Marcel Moolenaar <xcllnt@mac.com> To: Nathan Whitehorn <nwhitehorn@freebsd.org> Cc: powerpc@freebsd.org Subject: Re: Beware of revision 218075 Message-ID: <C1C88C89-C7AC-4F9D-ADFC-F9124F032B90@mac.com> In-Reply-To: <4D449D3B.5050008@freebsd.org> References: <550814DC-3E80-401C-B725-BFCB391726DF@mac.com> <4D449504.7030209@freebsd.org> <5C0CFF6E-94C9-43F5-A22F-5A0F6ECC0448@mac.com> <4D4498FF.9050808@freebsd.org> <F60BB9A4-E8D8-4DA9-A179-C395B0BFA568@mac.com> <4D449D3B.5050008@freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
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. > > 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. -- Marcel Moolenaar xcllnt@mac.com
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?C1C88C89-C7AC-4F9D-ADFC-F9124F032B90>