Date: Wed, 27 Jul 2016 18:27:48 +0200 From: Michal Meloun <mmel@FreeBSD.org> To: Nathan Whitehorn <nwhitehorn@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: <5798E104.5020104@FreeBSD.org> In-Reply-To: <f2edac8f-2859-cd98-754e-881e2b2d1e63@freebsd.org> References: <201606051620.u55GKD5S066398@repo.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> <f2edac8f-2859-cd98-754e-881e2b2d1e63@freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
Dne 26.07.2016 v 16:56 Nathan Whitehorn napsal(a):
>
>
> 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.
See [1].
>
> 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.
Nathan, I'm talking about pre /post r301453 state. Current INTRNG it
has never supported. But yes, I'm fully understand why you want it. See
[2].
>
>>
>> - 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.
Well, from my point of view, only "get interrupt data from
OFW/GPIO/.." part of bus_alloc_resource() follows bus hierarchy, real
execution not changed.
In any case, I see many possible variants how we can modify current
implementation. For rest, see [2].
>
>>
>>> 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
Firstly, please, ignore dependency problem. It will be explained later
in [2].
[1]
The original (your) INTRNG assume several things:
- value of interrupt parent xref together with byte contents of
'interrupts' property forms some sort of 'key' (aka virtual IRQ)
- byte contents of 'interrupts' property cannot be parsed in any way
inside INTRNG core.
- data forming this key are sufficient for subsequent mapping (and/or
configuring) to real IRQ.
- there must exist bidirectional unique relation between virtual and
real IRQs. So one key (virtual IRQ) can be mappable to one and just one
real IRQ. And only one virtual IRQ can be mapped to any given real
IRQ.
- we have cache that stores all observed 'keys' and associated real IRQ
) the they already mapped.
Unfortunately, necessity of unique mapping is very problematic.
We cannot handle situation, where shared interrupt is declared by
different but compatible 'interrupts' properties.
For example:
-------------------------
foo1: bar@12345678 {
interrupt-parent = <&pic1>;
interrupts = <1 IRQ_TYPE_LEVEL_HIGH>;
}
foo2: bar@1234567C {
interrupt-parent = <&pic1>;
interrupts = <1 IRQ_TYPE_NONE>;
}
Is this valid (from DT point of view) configuration? I think that yes.
Is this frequent configuration? I'm sure, isn't.
Is this possible configuration? I'm afraid that yes.
--------------------------
Next example are GPIO interrupts. These are encoded differently, so two
'keys' may point to to same real IRQ.
After that we decided to change our approach and utilize standard
resources and resource list entries to deliveryconfiguration data from
various sources (OFW/GPIO/...) to consumers.
[2]
Each OFW device has a lot of dependencies. It must enable clocks, power
regulators..., it must be taken from reset. All these action must be
done before we can access single register.
Most of these action can be done only with attached clock/power/reset
controller.
Within this, interrupts are not special. Moreover, 'cross type' circular
dependencies are not that rare.
I want to say:
Resource dependencies are must solved (and resolved) at different level
than is INTRNG.
Blind resource allocation is not universal solution because given
resource may/must be accessible immediately after allocation (in many
cases).
Unfortunately, and at time time, I known only one really working solution:
staggered driver initialization combined with multipass bus initialization.
I'm sorry, I'm not able to express all this accurately and clearly, but
I've really tried...
Michal
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?5798E104.5020104>