Skip site navigation (1)Skip section navigation (2)
Date:      Mon, 19 Jan 2015 15:47:32 -0800
From:      Nathan Whitehorn <nwhitehorn@freebsd.org>
To:        Svatopluk Kraus <onwahe@gmail.com>
Cc:        freebsd-arm@freebsd.org
Subject:   Re: interrupt framework
Message-ID:  <54BD9794.4080204@freebsd.org>
In-Reply-To: <CAFHCsPUqq-o4z9c5_8SYxcefUiFvGADB5FnB5NiQuu6XBrdyng@mail.gmail.com>
References:  <CAFHCsPX5kG_v-F-cjpyMQsT_b386eok=mqWW0%2BEUb_4-_1Otnw@mail.gmail.com>	<54BA9888.1020303@freebsd.org>	<CAFHCsPX-X-OG4jGLbhdH1BVtqorJKUeaVbzabX-%2BUfEM2fhD6A@mail.gmail.com>	<54BD3F86.3010901@freebsd.org> <CAFHCsPUqq-o4z9c5_8SYxcefUiFvGADB5FnB5NiQuu6XBrdyng@mail.gmail.com>

next in thread | previous in thread | raw e-mail | index | archive | help

On 01/19/15 15:00, Svatopluk Kraus wrote:
> On Mon, Jan 19, 2015 at 6:31 PM, Nathan Whitehorn
> <nwhitehorn@freebsd.org> wrote:
>> On 01/19/15 08:42, Svatopluk Kraus wrote:
>>> On Sat, Jan 17, 2015 at 6:14 PM, Nathan Whitehorn
>>> <nwhitehorn@freebsd.org> wrote:
>>>> On 01/15/15 05:51, Svatopluk Kraus wrote:
>>>>> Hi community,
>>>>>
>>>>> I and Michal Meloun have done some work on ARM interrupt framework and
>>>>> this is the result.
>>>>>
>>>>> We've started with intrng project with Ian's WIP changes, have looked
>>>>> at Andrew's ARM64 git repository, and this is how we think an
>>>>> interrupt framework should look like. We've implemented it with
>>>>> removable interrupt controllers in mind (PCI world). It's not finished
>>>>> from this point of view, however some functions are more complex
>>>>> because of it.
>>>>>
>>>>> It's tested on pandaboard and only GIC is implemented now. There is no
>>>>> problem to implement it to other controllers. We are open to questions
>>>>> and can finish our work considering any comments. Whoever is waiting
>>>>> for ARM interrupt framework as we were, you are welcome to test it.
>>>>> Whoever is welcome. The patches are done against FreeBSD-11-current
>>>>> revision 277210. There are two new files.
>>>>>
>>>>> ARM_INTRNG option must be added to board configuration file for new
>>>>> framework.
>>>>>
>>>>> There are still some things not implemented and some things which
>>>>> should be discussed like PPI support. For example, how to enable PPI
>>>>> interrupt on other CPUs when they are already running?
>>>>>
>>>>> We keep in mind that an interrupt framework should be helpfull but
>>>>> general enough to not dictate interrupt controlles too much. Thus we
>>>>> try to keep some things as much separated as possible. Each interrupt
>>>>> is represented by an interrupt source (ISRC) in the framework. An ISRC
>>>>> is described by an interrupt number which is much more an unique
>>>>> resource handle - totally independent on internal representation of
>>>>> interrupts in any interrupt controller.
>>>>>
>>>>> An interrupt is described by cells in FDT world. The cells can be
>>>>> decoded only by associated interrupt controller and as such, they are
>>>>> transparent for interrupt framework. The framework provides
>>>>> arm_fdt_map_irq() function which maps this transparent cells to an
>>>>> interrupt number. It creates an ISRC, saves cells on it, and once when
>>>>> associated interrupt controller is registered, it provides the ISRC
>>>>> with cells into the controller.
>>>>>
>>>>> It's a controller responsibility to save an ISRC associated with
>>>>> cells. An ISRC is transparent for any controller. However, an
>>>>> controller can set/get its data to/from an ISRC. Further, an
>>>>> controller should set a name to an ISRC according to internal
>>>>> representation of associated interrupt.
>>>>>
>>>>> An controller interrupt dispatch function can call framework only if
>>>>> it has associated ISRC to received interrupt.
>>>>>
>>>>> For legacy reason, there is arm_namespace_map_irq() function. An
>>>>> interrupt is described by namespace type and a number from the
>>>>> namespace. It's intented for use with no FDT drivers. Now, it's used
>>>>> for mapping an IPI on a controller.
>>>>>
>>>>> We think that it's better to call chained controllers (with filter
>>>>> only) without MI interrupt framework overhead, so we implemented
>>>>> shortcut. It could be utilized by INTR_SOLO flag during
>>>>> bus_setup_intr().
>>>>>
>>>>> Only an interrupt controller can really know its position in interrupt
>>>>> controller's tree. So root controller must claim itself as a root. In
>>>>> FDT world, according to ePAPR approved version 1.1 from 08 April 2011,
>>>>> page 30:
>>>>>
>>>>> "The root of the interrupt tree is determined when traversal of the
>>>>> interrupt tree reaches an interrupt controller node without an
>>>>> interrupts property and thus no explicit interrupt parent."
>>>>>
>>>>> Thus there are no need for any non-standard things in DTS files.
>>>>>
>>>>> Svata
>>>>>
>>>> I took a look through intrng.c and had a couple comments about the FDT
>>>> mapping stuff:
>>>>
>>>> 1. You use the device tree node handles as lookup keys rather than xref
>>>> handles. These are not necessarily stable, so you should use xref handles
>>>> instead.
>>>>
>>>> 2. If you make change (1), you don't depend on any OF_* stuff and can use
>>>> the same code with the PIC node ID as an opaque key on non-FDT platforms.
>>>> We
>>>> do this on PowerPC as well, which has been very useful. It will also save
>>>> some #ifdef.
>>>> -Nathan
>>>>
>>> Thanks. I did changes due to (1). Considering (2), I understand what
>>> you are doing in PowerPC, but it's not something I could adapt so
>>> easily. Hiding phandle_t behind uint32_t is clever, saves a few FDT
>>> #ifdefs, but makes things a little mysterious. Even if we will think
>>> about this uint32_t like some kind of key, there should be a function
>>> which convert phandle_t to that uint32_t key.
>>>
>>> I'm attaching new version of intrng.c with change (1) and with some
>>> more little adjustments.
>>>
>>> Svata
>>
>> Thanks! How do you plan to support multiple PICs on non-FDT platforms then?
>> It looks like it just fails at the moment.
>> -Nathan
>
> There is the following mapping function:
> u_int arm_namespace_map_irq(device_t dev, uint8_t type, uint16_t num);
>
> I named it "namespace" but it can be named another way. I think it
> does same like in PowerPC when node is NULL. However, there is one
> more argument - type. For example, it's used for IPI mapping in
> intrng.c.
>
> Svata
>
So you need the PIC's device_t to allocate an interrupt? That doesn't 
seem workable in the real world. What's wrong with just exposing the FDT 
interface with the phandle_t as an opaque key? You don't do anything 
with it except use it as a table lookup key, so it does not in any way 
matter what it actually is.
-Nathan



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?54BD9794.4080204>