From owner-freebsd-arm@FreeBSD.ORG Tue Apr 15 15:02:34 2014 Return-Path: Delivered-To: freebsd-arm@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) (using TLSv1 with cipher ADH-AES256-SHA (256/256 bits)) (No client certificate requested) by hub.freebsd.org (Postfix) with ESMTPS id 29B1C437 for ; Tue, 15 Apr 2014 15:02:34 +0000 (UTC) Received: from c.mail.sonic.net (c.mail.sonic.net [64.142.111.80]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client did not present a certificate) by mx1.freebsd.org (Postfix) with ESMTPS id 0A3031638 for ; Tue, 15 Apr 2014 15:02:33 +0000 (UTC) Received: from zeppelin.tachypleus.net ([128.135.100.108]) (authenticated bits=0) by c.mail.sonic.net (8.14.4/8.14.4) with ESMTP id s3FF2Rrw003237 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES128-SHA bits=128 verify=NOT); Tue, 15 Apr 2014 08:02:28 -0700 Message-ID: <534D4A01.2020804@freebsd.org> Date: Tue, 15 Apr 2014 08:02:25 -0700 From: Nathan Whitehorn User-Agent: Mozilla/5.0 (X11; FreeBSD amd64; rv:24.0) Gecko/20100101 Thunderbird/24.4.0 MIME-Version: 1.0 To: Warner Losh Subject: Re: [RFC] Refactored interrupt handling on ARM References: <3e7f866f4bc774975ae3c85e0df78ec2@uj.edu.pl> <53418D13.7030107@freebsd.org> <534C0F48.2090302@freebsd.org> <534C5A6A.1090707@freebsd.org> <246c2ef842c2b47eb2400c1f700ad441@uj.edu.pl> <534CC733.7010009@freebsd.org> In-Reply-To: Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 8bit X-Sonic-ID: C;snl29a7E4xGvVrilf7iULg== M;fs/l9a7E4xGvVrilf7iULg== Cc: freebsd-arm@freebsd.org X-BeenThere: freebsd-arm@freebsd.org X-Mailman-Version: 2.1.17 Precedence: list List-Id: "Porting FreeBSD to ARM processors." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 15 Apr 2014 15:02:34 -0000 On 04/15/14 07:11, Warner Losh wrote: > On Apr 14, 2014, at 11:44 PM, Nathan Whitehorn wrote: > >> On 04/14/14 15:44, Jakub Klama wrote: >>> On Mon, 14 Apr 2014 15:00:10 -0700, Nathan Whitehorn wrote: >>>> I had deliberately made it private with the last round of interrupt >>>> changes. The idea was to rely completely on newbus for interrupt >>>> mapping. Having a public interface allows code to bypass the bus >>>> hierarchy, which usually isn't a good thing. This ended up happening >>>> all over the place on PowerPC, for example. This made a lot of drivers >>>> less MI than they should have been and took a lot of time to get rid >>>> of. >>> Agree. In fact, we can completely remove FDT_MAP_IRQ macro, as well as >>> FDT_INTR_MAX. >>> >>> Jakub >>> >> Great! Very nice work with all of this, by the way. It's a thorny problem and I'm glad to see it being solved. > 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. > > 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. 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? 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? > > 255 is too few interrupts to have. We need easily twice that for Atmel. And chance we can remove NIRQ as well? > > 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. > > Warner > > > On a related note, I'm confused by the simplebus.c changes. machine/fdt.h isn't an MI header (it doesn't exist on PowerPC, for example). Neither is FDT_DESCRIBE_IRQ() an MI interface. If you want an interface like this, it seems like it should be a standard part of the MI interrupt layer rather than yet more hacks in simplebus. -Nathan