Date: Wed, 27 Jul 2016 10:19:41 -0700 From: Nathan Whitehorn <nwhitehorn@freebsd.org> To: Michal Meloun <mmel@freebsd.org> Cc: "freebsd-arm@freebsd.org" <freebsd-arm@freebsd.org>, Svatopluk Kraus <skra@freebsd.org>, "freebsd-arch@freebsd.org" <freebsd-arch@freebsd.org> Subject: Re: INTRNG (Was: svn commit: r301453....) Message-ID: <a5d43044-1733-6cc7-2e99-e85b60b0fcf3@freebsd.org> In-Reply-To: <5798E104.5020104@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> <5798E104.5020104@FreeBSD.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On 07/27/16 09:27, Michal Meloun wrote: [snip] >> 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]. Current "INTRNG", at least on PowerPC, has supported this for 10 years. If it doesn't on ARM, it's because of some trivial implementation bug. > >>> - 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]. But that's an important part, for three reasons: 1. The PIC may not exist when bus_alloc_resource() is called 2. The bus parents have no useful information to contribute to this since the hierarchy doesn't go that way 3. If you try to use the interrupt pin as the resource ID, which r301453 does, you end up with rman conflicts if, for example, a bus has two children with interrupts on identically numbered pins on two different interrupt controllers. > >>> >>>> 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]. Except that that isn't a workable solution (see the end) > > [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. There isn't actually a requirement of bidirectional uniqueness, as I explained the last time you brought this up. One "virtual" IRQ does need to map to exactly one interrupt specifier, but the reverse is not the case. The PIC driver is absolutely free to map and dispatch multiple virtual IRQs from the same shared interrupt pin, with no more overhead than you usually get from shared interrupt lines. > > 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. No, it's malformed and a violation of the standard. > Is this frequent configuration? I'm sure, isn't. > Is this possible configuration? I'm afraid that yes. People do in fact do deranged non-conformant things with device trees sometimes, unfortunately, so it is indeed worth worrying about. > -------------------------- > > 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. This is a non-issue. You make the GPIO controller a PIC, it handles interrupts however you like. If you got a weird device tree that uses two encodings (one for "interrupts" properties with GPIOs and one for "GPIO" properties on which you need to take interrupts but that weren't added to the "interrupts" list for mysterious reasons), you introduce a helper function for the GPIO case. Are there any actual problems with the pre-existing interrupt mapping code? I have not seen any so far. > [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. They are actually quite special in that the kernel enables them late and so you can defer setup. They are also special in that, for that reason, it is possible to design systems with circular dependency graphs with interrupts. It is not possible to do this with, for example, clocks: if I need to apply a clock to the clock controller with itself, the system is just not bootable and such a system will not be built or shipped. Interrupts need only happen later, after device setup, and so such systems *are* designed and shipped. The same is true for power. > 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). Absolutely: for GPIOs, clocks, power, etc. you want a system that looks different than the interrupt virtualization system. Probably with extra resource types and maybe with some API similar to r301453. But interrupts have different, and more complex, requirements and cannot be mapped this way. > > Unfortunately, and at time time, I known only one really working solution: > staggered driver initialization combined with multipass bus initialization. That does not solve the interrupt problem. If devices have interrupts on their own children, no amount of multipass initialization can possibly break the loop. Multipass would work for other kinds of resources (clocks, power, etc.) and is a perfect match for those resource types. A dynamic multipass (where a driver can return EAGAIN or something from attach if its resources don't exist yet) would work great. *But* in the specific, special case of interrupts, it is not a workable model. You could imagine some change to initialization that gives devices a late attach and an early attach method and does interrupts in the late part, but that is hugely invasive change that would need to touch every single driver in the tree -- to solve a problem that is already solved by interrupt virtualization. -Nathan > I'm sorry, I'm not able to express all this accurately and clearly, but > I've really tried... > Michal > > > _______________________________________________ > freebsd-arch@freebsd.org mailing list > https://lists.freebsd.org/mailman/listinfo/freebsd-arch > To unsubscribe, send any mail to "freebsd-arch-unsubscribe@freebsd.org" >
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?a5d43044-1733-6cc7-2e99-e85b60b0fcf3>