Date: Tue, 15 Apr 2014 14:02:05 -0600 From: Warner Losh <imp@bsdimp.com> To: Ian Lepore <ian@FreeBSD.org> Cc: freebsd-arm@FreeBSD.org Subject: Re: [RFC] Refactored interrupt handling on ARM Message-ID: <763F152E-B261-4BB4-A289-4E841C50BD81@bsdimp.com> In-Reply-To: <1397580102.1124.121.camel@revolution.hippie.lan> References: <3e7f866f4bc774975ae3c85e0df78ec2@uj.edu.pl> <53418D13.7030107@freebsd.org> <534C0F48.2090302@freebsd.org> <f2bebfa812ecb70f423b6be4779b217b@uj.edu.pl> <534C5A6A.1090707@freebsd.org> <246c2ef842c2b47eb2400c1f700ad441@uj.edu.pl> <534CC733.7010009@freebsd.org> <ACD52C94-A44A-42FD-8016-61B0B21B12D9@bsdimp.com> <619da7d72d2345b1fcac5426b45c6ead@uj.edu.pl> <1397580102.1124.121.camel@revolution.hippie.lan>
next in thread | previous in thread | raw e-mail | index | archive | help
On Apr 15, 2014, at 10:41 AM, Ian Lepore <ian@FreeBSD.org> wrote: > On Tue, 2014-04-15 at 17:29 +0200, Jakub Klama wrote: >> On Tue, 15 Apr 2014 08:11:57 -0600, Warner Losh wrote: >>> This is looking nice on the surface. I=92ve done a small portion of >>> this sort of work for the Atmel port, so I=92ll see how well this = can=20 >>> be >>> extended to work there. Unlike the gic device, there=92s 3 or 4 pio >>> controllers that act as interrupt handlers, plus the need to wire up >>> pins relatively early... >>>=20 >>> I=92d clean up the name =91intrng.c=92 though, since ng things tend = to >>> become dated=85 ARM_INTRNG also suffers from this naming flaw. Is = there >>> any reason not to have it on all the time? Also, the ARM_INTRNG = ifdef >>> is inconsistently applied. >>=20 >> Well, the naming thing is the least significant one I guess... and >> one which can be fixed in a moment. >>=20 >> ARM_INTRNG should be enabled only on platforms which have pic drivers >> using pic_if.m interface instead of the old one. The purpose for that >> option is to not break existing code. >>=20 >=20 > IMO we do too much of this. Unless there's a really good reason not = to > update the older platforms to use this new scheme, I think we should > just convert everything to the new way. I=92m 100% in agreement here. Those platforms that aren=92t upgraded = need to go away. >>> Looking at the FDT, it appears that the interrupt numbers are hard >>> coded for things that appear to be connected to GPIOs. This hard >>> coding (and changes to reflect differences in mapping) is really = bad. >>=20 >> Can you give an example? >>=20 >>> Linux specifies the GPIO more directly (though each SoC is different >>> in the details). I haven=92t looked at LPC (where I noticed the = change) >>> on Linux to see if these changes bring us closer to being able to = use >>> the stock FDT for that platform. Do these changes help with that? >>=20 >> LPC changes purpose was rather to give an example, surely other >> platforms can benefit more from these. >>=20 >>> While I appreciate the aesthetic appeal of having purely an = interrupt >>> number and PIC (interrupt parent), I=92m not sure that will work >>> everywhere. >>>=20 >>> dosoftints() does nothing, is that on purpose? >>=20 >> It's present to not break compatibility and I'm not even sure what >> should it do. However: >>=20 >> % ack dosoftints >> arm/arm/intrng.c >> 417:void dosoftints(void); >> 419:dosoftints(void) >>=20 >> arm/arm/intr.c >> 138:void dosoftints(void); >> 140:dosoftints(void) >>=20 >> is it needed anyways? >>=20 >>> 255 is too few interrupts to have. We need easily twice that for >>> Atmel. And chance we can remove NIRQ as well? >>=20 >> Currently we can have 255 pics and 256 irqs on each, but because irq >> number is stored in int, we easily (down to changing two lines of = code) >> divide it by 16:16 or 8:24, thus having 2^16 pics and 2^16 irqs on = each >> or 256 pics with 2^24 irqs on each maximum. >>=20 >> Regarding NIRQ, there's surely possible to get rid of it, but it will >> need also removing intrcnt/sintrcnt/intrnames and creating more >> flexible interface instead (for other archs too). Anyways, it's some >> step in removing it. >>=20 >>> Finally, I=92m not sure what the change to kern_intr.c is supposed = to >>> accomplish. It seems weird to me. Can you explain it? I=92d think = that >>> would have a bad effect on other platforms. >>=20 >> trapframe is passed to the first intr_event_handle() call and saved >> in td->td_intr_frame (just like it was before). If the consecutive >> calls to intr_event_handle() supply 0/NULL as trapframe, filter >> function which wants trapframe instead of argument will get the >> saved one. So the only situation where the behavior is different >> is case where you call intr_event_handle(ie, NULL). BTW This change >> was discussed some time ago with cognet@. >=20 > Since this is only needed by the new arm code, and the new arm code = only > calls intr_event_handle() from arm_dispatch_irq(), can't this logic = just > move into there? I agree with this as well=85 I don=92t think it is a safe assumption = that we can just change the MI behavior here... > Also, I have a feeling EOI isn't being done in all the right places, = but > it could be a matter of misreading the diff. I need to actually apply > the diff and read the code, and I'll get to that soon, really I will = (I > seem to be writing a new definition for "soon" every time I say that). Yea, I had that on my todo list to check carefully when I had time to look at this stuff in greater detail. Warner
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?763F152E-B261-4BB4-A289-4E841C50BD81>