Date: Tue, 15 Apr 2014 17:29:32 +0200 From: Jakub Klama <jakub.klama@uj.edu.pl> To: Warner Losh <imp@bsdimp.com> Cc: freebsd-arm@freebsd.org Subject: Re: [RFC] Refactored interrupt handling on ARM Message-ID: <619da7d72d2345b1fcac5426b45c6ead@uj.edu.pl> In-Reply-To: <ACD52C94-A44A-42FD-8016-61B0B21B12D9@bsdimp.com> 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>
next in thread | previous in thread | raw e-mail | index | archive | help
On Tue, 15 Apr 2014 08:11:57 -0600, Warner Losh wrote: > This is looking nice on the surface. I’ve done a small portion of > this sort of work for the Atmel port, so I’ll see how well this can > be > extended to work there. Unlike the gic device, there’s 3 or 4 pio > controllers that act as interrupt handlers, plus the need to wire up > pins relatively early... > > I’d clean up the name ‘intrng.c’ though, since ng things tend to > become dated… 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. Well, the naming thing is the least significant one I guess... and one which can be fixed in a moment. 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. > 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. Can you give an example? > Linux specifies the GPIO more directly (though each SoC is different > in the details). I haven’t 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? LPC changes purpose was rather to give an example, surely other platforms can benefit more from these. > While I appreciate the aesthetic appeal of having purely an interrupt > number and PIC (interrupt parent), I’m not sure that will work > everywhere. > > dosoftints() does nothing, is that on purpose? It's present to not break compatibility and I'm not even sure what should it do. However: % ack dosoftints arm/arm/intrng.c 417:void dosoftints(void); 419:dosoftints(void) arm/arm/intr.c 138:void dosoftints(void); 140:dosoftints(void) is it needed anyways? > 255 is too few interrupts to have. We need easily twice that for > Atmel. And chance we can remove NIRQ as well? 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. 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. > Finally, I’m not sure what the change to kern_intr.c is supposed to > accomplish. It seems weird to me. Can you explain it? I’d think that > would have a bad effect on other platforms. 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@. Jakub
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?619da7d72d2345b1fcac5426b45c6ead>