Date: Tue, 26 Jul 2016 07:56:09 -0700 From: Nathan Whitehorn <nwhitehorn@freebsd.org> To: Michal Meloun <mmel@FreeBSD.org> Cc: Svatopluk Kraus <skra@FreeBSD.org>, "freebsd-arch@freebsd.org" <freebsd-arch@FreeBSD.org>, "freebsd-arm@freebsd.org" <freebsd-arm@FreeBSD.org> Subject: Re: INTRNG (Was: svn commit: r301453....) Message-ID: <f2edac8f-2859-cd98-754e-881e2b2d1e63@freebsd.org> In-Reply-To: <57976867.6080705@FreeBSD.org> References: <201606051620.u55GKD5S066398@repo.freebsd.org> <b9606755-69cb-2cb0-04d7-6be32e4cb89e@freebsd.org> <578E0B5D.3070105@FreeBSD.org> <e026f6fc-76ed-5dbe-00fc-365b6d7bcf94@freebsd.org> <578F6075.7010500@FreeBSD.org> <05a80ac6-4285-ec9d-36e9-2f92c609f746@freebsd.org> <57907B0F.9070204@FreeBSD.org> <9d2a224c-b787-2875-5984-a7a2354e8695@freebsd.org> <57934ABD.6010807@FreeBSD.org> <4e7a3e8f-cc21-f5f2-e3e0-4dbd554a4cd0@freebsd.org> <5794720F.4050303@FreeBSD.org> <8bfd8668-bc49-e109-e610-b5cd470be3ec@freebsd.org> <57950005.6070403@FreeBSD.org> <f82018ee-51e7-60fa-2682-f0ef307a52b5@freebsd.org> <57961549.4020105@FreeBSD.org> <e2cace17-0924-2084-5fcf-626f87e41cc3@freebsd.org> <CANCZdfr%2BZ4XxXRY0yMiWXwp=8iKq54y3uJ9-OfAOdfxAs1qdtw@mail.gmail.com> <f94bfd25-fabf-efc3-55c9-cfdfd9e4d6e6@freebsd.org> <CANCZdfpz=z3gc3pyb_Qssa3vGJSnPv_r6J-SWDPPpE9zPYB9=w@mail.gmail.com> <ab44ddb1-515b-94ac-6b12-673b7c53d658@freebsd.org> <57976867.6080705@FreeBSD.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On 07/26/16 06:40, Michal Meloun wrote:
> Dne 26.07.2016 v 7:41 Nathan Whitehorn napsal(a):
>>
>> On 07/25/16 21:24, Warner Losh wrote:
>>> On Mon, Jul 25, 2016 at 10:48 AM, Nathan Whitehorn
>>> <nwhitehorn@freebsd.org> wrote:
>>>> On 07/25/16 09:32, Warner Losh wrote:
>>>>> On Mon, Jul 25, 2016 at 8:05 AM, Nathan Whitehorn
>>>>> <nwhitehorn@freebsd.org> wrote:
>>>>>> That wasn't my question. Are these particular device drivers
>>>>>> allocating
>>>>>> interrupts both on the GPIOs in their "interrupts" property (which
>>>>>> are
>>>>>> entirely GPIOs in this example) *and* on the GPIOs listed as
>>>>>> resources
>>>>>> but
>>>>>> not listed as interrupts? If they are, then you need a switching
>>>>>> mechanism,
>>>>>> but that seems pretty unlikely given the names of the
>>>>>> non-interrupt GPIOs
>>>>>> (they look like outputs). It would also be a somewhat deranged way
>>>>>> to set
>>>>>> up
>>>>>> a device tree -- not that that rules it out or anything.
>>>>> On Atmel, there's a situation that this covers, I think.
>>>>>
>>>>> The MCI device has an interrupt in the core:
>>>>>
>>>>> mmc0: mmc@fffa8000 {
>>>>> compatible = "atmel,hsmci";
>>>>> reg = <0xfffa8000 0x600>;
>>>>> interrupts = <9
>>>>> IRQ_TYPE_LEVEL_HIGH 0>;
>>>>> #address-cells = <1>;
>>>>> #size-cells = <0>;
>>>>> pinctrl-names = "default";
>>>>> clocks = <&mci0_clk>;
>>>>> clock-names = "mci_clk";
>>>>> status = "disabled";
>>>>> };
>>>>>
>>>>> and in other places wires in GPIO interrupts for things like card
>>>>> eject / insertion.
>>>>>
>>>>> mmc0: mmc@f0008000 {
>>>>> pinctrl-0 = <
>>>>> &pinctrl_board_mmc0
>>>>>
>>>>> &pinctrl_mmc0_slot0_clk_cmd_dat0
>>>>> &pinctrl_mmc0_slot0_dat1_3>;
>>>>> status = "okay";
>>>>> slot@0 {
>>>>> reg = <0>;
>>>>> bus-width = <4>;
>>>>> cd-gpios = <&pioD 15
>>>>> GPIO_ACTIVE_HIGH>;
>>>>> };
>>>>> };
>>>>>
>>>>> an interrupt is registered on the cd-gpios pin for when the card
>>>>> changes.
>>>>> At
>>>>> least in linux, FreeBSD doesn't (yet) implement this, but will
>>>>> someday if
>>>>> I get
>>>>> back to the armv6 atmel work I started (see at91-cosino.dts for
>>>>> example,
>>>>> there's
>>>>> others).
>>>>>
>>>>> I think this is an example of what you are asking about, or did I get
>>>>> lost in the
>>>>> twisty maze of conversation zigs and zags...
>>>>>
>>>>> Warner
>>>>>
>>>> Where we would run into (minor) problems is if the interrupt parent
>>>> for the
>>>> first mmc0 is the GPIO controller. More generally, if &pioD has
>>>> interrupt
>>>> children specified in some way that is not a <pin, active
>>>> high/whatever>
>>>> tuple somewhere else in the tree then you would have to have methods to
>>>> parse both interrupt specifiers
>>>> as-obtained-from-interrupts-properties (or
>>>> equivalent) and specifiers as-obtained-from-gpio-properties. If the
>>>> tree
>>>> picks one format and sticks with it, you can get away with just the
>>>> one.
>>>> Glancing through the DTS source for this board, that doesn't appear
>>>> to be
>>>> the case and the property formatting is uniform, but I might have
>>>> missed
>>>> something in one of the many #includes.
>>> Interrupts and GPIO specifiers are different in subtle ways. The
>>> interrupt
>>> parent for mmc0 is an AIC, which is also the ultimate parent of the GPIO
>>> controller.
>> That is what it looked like.
>>
>>> But the properties for the GPIO pins that act as interrupts and
>>> the interrupt specifiers are different.
>> So there are devices with both interrupts = <foo bar>,
>> #interrupt-parent = <&gpio> and gpios = <&gpio bleh baz> where "Bleh
>> baz" is formatted different than "foo bar" and both are meant to be
>> treated as interrupts?
>>
>> It's fine if there are, but I haven't seen any such device trees yet.
> I think that yes, but I'm not ready to search all 1372 Linux DT files
> only for explicit example.
> But your question is invalid from beginning.
> 1) Format of each single 'interrupts' and '*-gpios' property is
> different because each single one are defined differently.
> 2) The question should not be 'Are there device' but 'Is this
> combination valid'
>
> Moreover, I'm curious why you want OFW property for gpio interrupts at all,
> given gpio can be instantiated by other entity (PCIe, USB).
>
> Please see:
> https://svnweb.freebsd.org/base/head/sys/dev/gpio/gpiobus.c?revision=301539&view=markup#l92
> and, for example this one controller
> https://svnweb.freebsd.org/base/head/sys/arm/nvidia/tegra_gpio.c?revision=300149&view=markup#l575
Well, it's part of the device tree standard, so dealing with GPIO
properties seems reasonable where they occur. You could, of course, have
systems without device trees and GPIOs. I'm not sure where you are going
with this.
>
>>>> As a general point, GPIO weirdness would be easy enough case to
>>>> handle if it
>>>> did come up (add some mapping method, as above) that I think we
>>>> shouldn't
>>>> worry too much about it from an architectural point of view. If a board
>>>> appears that is set up this way, we can roll with the punches at
>>>> that point
>>>> and add whatever small amount of shim code that is required. It
>>>> would be
>>>> annoyance, sure, but not a real complication.
>>> I suspect that either I don't understand the issue, or we'll have
>>> such boards
>>> very quickly. The Atmel design is fairly clean in comparison to other
>>> franken-horrors
>>> I've seen...
>> People do weird things for sure. My point is just that the details of
>> how (implicit) GPIO interrupts are formatted just isn't that
>> important. It's easy enough to add special code for devices that are
>> set up in bizarre ways as they are spotted in the wild and that
>> special code is so minor that it doesn't matter for the design of the
>> API. This is a point I was just curious about, since I had never
>> actually seen device trees set up that way.
>>
>> The issue with this patch is, at its core, whether you have an
>> architecture that relies on the newbus hierarchy (like r301453) or
>> that allows links outside of that hierarchy that can cross branches or
>> go the "wrong" way, like the previously existing code.
> OK. The r301453 :
> - it removes unnecessary idea of virtual IRQs. I gave you some examples
> where virtual IRQ concept fails.
I have never seen any of these examples.
The concept is *extremely* necessary, which is why both Linux and
FreeBSD decided to use it independently There is no way to handle parent
buses with a single rman and devices on multiple PICs with overlapping
interrupt ranges without them; neither is there a way to decode
arbitrary-length interrupt specifiers or to handle things like MSIs.
Please see the list of cases at
https://wiki.freebsd.org/Complicated_Interrupts and in my earlier email
for some examples of things that you just can't represent with this new
system, as far as I can tell.
> Also, both variants needs attached PIC at bus_alloc_resource() time,
> so timing wasn't been changed.
They absolutely do not, as I have explained repeatedly. Due to parent
devices with interrupts handled by their bus children, this is a hard
requirement of any workable system. This is not a theoretical issue; I
have lots of hardware like this.
>
> - it implements new BUS_MAP_INTR(). As I understand it, this is
> problematic for you, and I'm ready to change it. But I need more details
> than "it's fundamentally broken".
Please explain (a) what cases it handles that the existing code and
does, and (b) how you would resolve each of the cases on the wiki page I
sent.
The general issue is that it traces the newbus hierarchy, when
interrupts often do not, and so breaks when you have links to
as-yet-unattached parts of the hierarchy. It also relies on the assigned
"IRQ" numbers being nonoverlapping to avoid rman errors, but that's a
relatively minor thing.
>
>> There are some other differences, of varying degrees of importance,
>> but that's the really fundamental one. I haven't seen any cases where
>> r301453 provides functionality absent in the already existing system,
>> but there seem to be a large number of cases (see the first email I
>> sent to freebsd-arch or
>> https://wiki.freebsd.org/Complicated_Interrupts) that the API in
>> r301453 cannot accommodate and that are needed to support a variety of
>> hardware.
> Which single feature has been removed by r301453?
Nothing has been removed by it, because the normal code is intact.
However, the new code is less functional than the old code, so cannot
replace it. In particular, it is architecturally incapable of working
with the kinds of device trees found on PowerPC systems (see the wiki
page). That means we would have to keep both indefinitely, which is a
significant maintenance burden to no gain whatsoever.
-Nathan
> Michal
>
>> Since having two APIs will be a significant maintenance burden, if
>> there are cases the existing code can't handle, I would like to know
>> about them; otherwise, I think we should back out r301453 and stick to
>> the standard device tree interrupt mapping mechanism on ARM instead.
>> I'm happy to help implement any necessary enhancements there (for
>> example, dealing with weirdly-encoded GPIOs).
>> -Nathan
>>
>>> Warner
>>>
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?f2edac8f-2859-cd98-754e-881e2b2d1e63>