Date: Thu, 4 Aug 2016 05:34:50 -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: <1946069a-d0f9-2c19-80a5-0b490682574b@freebsd.org> In-Reply-To: <57A30B72.7070809@FreeBSD.org> References: <201606051620.u55GKD5S066398@repo.freebsd.org> <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> <a5d43044-1733-6cc7-2e99-e85b60b0fcf3@freebsd.org> <579A25BB.8070206@FreeBSD.org> <30790e40-58b4-3371-c0f0-b7545571f389@freebsd.org> <579AFFC5.1040005@FreeBSD.org> <eb603349-eb88-866d-7a26-9e026518fd39@freebsd.org> <579CD355.1050203@FreeBSD.org> <460fa0b3-ddb7-6247-2412-3d75a589d5e7@freebsd.org> <579CF7C8.1040302@FreeBSD.org> <24107713-6d50-c21d-ccf1-7dbdb36cc484@freebsd.org> <579E1BE2.7020500@FreeBSD.org> <7f053bb8-ab03-e46c-1c72-d757348e4e54@freebsd.org> <cefdfaab-a95f-2a92-89bd-3d0cef2a75ab@freebsd.org> <57A09F34.4050400@FreeBSD.org> <ad1e6337-468e-f35d-7454-444a561cb103@freebsd.org> <57A30B72.7070809@FreeBSD.org>
next in thread | previous in thread | raw e-mail | index | archive | help
This is a multi-part message in MIME format. --------------3EFCE033AD3743BB7374BC07 Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit On 08/04/16 02:31, Michal Meloun wrote: > Dne 02.08.2016 v 17:31 Nathan Whitehorn napsal(a): >> >> On 08/02/16 06:25, Michal Meloun wrote: >>> I'm sorry for delayed response. We have serious trouble @work so I >>> cannot spend to much time on FreeBSB for next 2 or 3 days. >> Understood. >> >>> Dne 02.08.2016 v 6:00 Nathan Whitehorn napsal(a): >>>> On 07/31/16 11:43, Nathan Whitehorn wrote: >>>>> On 07/31/16 08:40, Michal Meloun wrote: >>>>>> Dne 31.07.2016 v 8:11 Nathan Whitehorn napsal(a): >>>>>> [snip] >>>>>>>> The current API is certainly a bit of a hack and I would be >>>>>>>> pleased to >>>>>>>>> see it go; but the replacement needs to be more functional and >>>>>>>>> more >>>>>>>>> robust. As far as I can tell, I literally *cannot* move PowerPC >>>>>>>>> over >>>>>>>>> to this architecture. >>>>>>>> Yes, at this time, I agree. If you can accept enhancement of >>>>>>>> (owf_)bus_map_intr() then we can move to next, significantly less >>>>>>>> hackish, state. >>>>>>> OK, sure. I wrote some code this afternoon (attached) to sketch >>>>>>> this. >>>>>>> It does not compile or run or anything useful, but it should >>>>>>> illustrate the important parts of what I think is an API that should >>>>>>> work for everyone. I'm happy to finish it if it looks reasonable >>>>>>> -- I >>>>>>> may in any case just to replace PowerPC's increasingly teetering >>>>>>> intr_machdep.c. >>>>>>> >>>>>>> The OF part is essentially unchanged from how it currently works: >>>>>>> there's a method that you pass the interrupt specifier and interrupt >>>>>>> parent to, and it spits back an IRQ that means that combination of >>>>>>> things in perpetuity. OFW_BUS_MAP_INTR() in nexus.c, with its >>>>>>> current >>>>>>> API, can be implemented in terms of it in one line. Whenever the >>>>>>> relevant PIC attaches, it is told to use the given IRQ to refer to >>>>>>> that opaque bytestring. >>>>>>> >>>>>>> I've added a set of equivalent functions for ACPI that take the >>>>>>> contents of an ACPI interrupt specifier and do the same things. It >>>>>>> tries to have the IRQ be human-meaningful, if possible, by matching >>>>>>> the ACPI interrupt number. Having separate functions seemed a little >>>>>>> cleaner than exposing the enums and unions as part of the KPI, >>>>>>> but I'm >>>>>>> not wedded to it -- this is just an example. >>>>>>> >>>>>>> PICs register (and deregister) themselves with either an OF >>>>>>> phandle or >>>>>>> an ACPI resource string or (god help us) both as needed. The PICs >>>>>>> have >>>>>>> corresponding methods for interpreting various kinds of interrupt >>>>>>> specifiers. >>>>>>> >>>>>>> Whether it allows bus_alloc_resource(), bus_setup_intr(), etc. to >>>>>>> succeed before the PICs are attached is gated on an #ifdef. That can >>>>>>> probably be off by default on ARM. A PowerPC version of this code >>>>>>> would have to keep it on until various bus drivers are fixed. >>>>>>> >>>>>>> Here's the general flow: >>>>>>> - Parent bus enumerates children, maps IRQs as needed, adds to >>>>>>> resource list. The struct resources involved aren't special (just a >>>>>>> single number), so PCIB_ROUTE_INTERRUPT(), MSIs, etc. are trivial to >>>>>>> implement (and already are implemented, in general, for OF/FDT >>>>>>> drivers). >>>>>>> - bus_alloc_resource() does nothing special >>>>>>> - bus_setup_intr() calls into the PIC and does what is needed, as in >>>>>>> r301453 (to within that #ifdef) >>>>>>> >>>>>>> This should keep all the best pieces of everything: >>>>>>> - ACPI and OF are supported together, and easy to extend for other >>>>>>> types of mapping technologies without breaking either KBI or KPI >>>>>>> (just >>>>>>> add new functions) >>>>>>> - No APIs change for existing Open Firmware code >>>>>>> - The support code can live entirely in machine-dependent >>>>>>> directories, >>>>>>> with no code changes at all required in kern/ or dev/ofw/ >>>>>>> - bus_setup_intr() and friends behave as in r301453 and succeed or >>>>>>> fail for real >>>>>>> - PCI code is not an issue >>>>>>> - No disconnected interrupt state maintained in mapping table >>>>>>> (unless >>>>>>> the transitional #ifdef is on) and the mapping table content is only >>>>>>> larger than r301453 by having a copy of the original interrupt >>>>>>> specifier. >>>>>>> >>>>>>> If and when we manage to fix the kernel APIs to allow non-scalar >>>>>>> interrupt specifiers, this should also be easy to gradually >>>>>>> transmogrify to support that case since there exists a 1:1 >>>>>>> mapping of >>>>>>> scalar IRQs to rich data and the control flow would be identical: >>>>>>> interrupt specifiers are read at enumeration time and a resulting >>>>>>> handle -- struct resource or scalar IRQ -- used afterward to refer >>>>>>> to it. >>>>>>> >>>>>> Nice, nice, i think that we are very close at this point. >>>>>> The problem with the above workflow is that maps IRQ function is >>>>>> called >>>>>> at parent bus enumeration time, generally at BUS_PASS_BUS pass. >>>>>> And at >>>>>> this time, PICs are not exist. >>>>>> Also, 'static struct intr_irqsrc *irq_sources[NIRQ];' is not a >>>>>> mapping >>>>>> table, it doesn't provide this functionality. >>>>>> But yes, i understand that mapping table is important and necessary >>>>>> for you. >>>>>> >>>>>> So, if i can extend our concept a little bit, then: >>>>>> We can add new table (map_data_tbl for example) that holds a >>>>>> copy of >>>>>> the original interrupt specifier, index to irq_sources table and >>>>>> probably some flags. >>>>>> And we can modify the above workflow to: >>>>>> - Parent bus enumerates children, allocates entries from >>>>>> map_data_tbl, >>>>>> adds to resource list. The struct resources involved aren't special >>>>>> (just a single number), so PCIB_ROUTE_INTERRUPT(), MSIs, etc. are >>>>>> trivial to implement (and already are implemented, in general, for >>>>>> OF/FDT drivers). Index to map_data_tbl is used as resource ID. >>>>>> - bus_alloc_resource() sets 'ALLOCATED' in map_data_tbl flags field, >>>>>> *maps IRQs* and stores result in 'index' field. >>>>>> - bus_setup_intr() sets 'SETUP_DONE' in map_data_tbl flags field, >>>>>> calls >>>>>> into the PIC and does what is needed, as in r301453. (to within that >>>>>> #ifdef) >>>>>> >>>>>> And, in PPC case, newly attached PIC scans whole map_data_tbl table, >>>>>> finds his entries and makes what needs. >>>>>> >>>>>> Does that make sense? >>>>>> I hope that postponing of map IRQ call is not a problem for PPC, >>>>>> everything else looks easy. >>>>>> Moreover, this additional level of indirection also solves all my >>>>>> 'hypothetical corner cases' with N:1 mappings (between original >>>>>> interrupt specifier and real interrupt number). >>>>> Yes, I think we are converging. >>>>> >>>>> My qualm about bus_alloc_resource() is twofold: >>>>> 1. Traditionally, with interrupts, bus_alloc_resource() has no side >>>>> effects and I'm not sure it propagates cleanly down the tree in all >>>>> cases. >>>>> 2. There are a few bus APIs (bus_config_intr() comes to mind) that >>>>> use raw IRQ IDs and, so far as I know, can be, and sometimes are >>>>> called before bus_alloc_resource(). If the PIC doesn't know about the >>>>> IRQ yet when bus_config_intr() (etc.) is called, things will just >>>>> break. >>>>> >>>>> So you would need to make sure that any bus method handling a >>>>> resource ID causes it to be mapped on the PIC at that time. It's OK >>>>> if that happens in bus_alloc_resource() so long as bus_config_intr(), >>>>> bus_setup_intr(), etc. cause that to happen if it hasn't happened yet >>>>> -- I don't care when these calls are made to the PIC driver so long >>>>> as *what* calls will be made is set at enumeration time. >>>>> >>>>> I am also totally fine with having, on ARM, bus_config_intr(), >>>>> bus_setup_intr() etc. fail if the PIC hasn't attached yet (On >>>>> PowerPC, we can't do that yet, but after this conversation, I regard >>>>> that as a bug and would fix that later), as well as delaying setup on >>>>> the PIC to the first time any bus driver actually tries to *use* the >>>>> interrupt (alloc_resource(), setup_intr(), config_intr(), whatever) >>>>> rather than when the interrupt is originally allocated. >>>>> >>> I understand. And yes, bus_alloc_resource() isn't propagated cleanly >>> down the tree in all cases. That's why we have 'INTRNG' hook in >>> subr_bus.c, which is suboptimal. >>> >>> About bus_config_intr(). The only consumer of bus_config_intr() is ACPI >>> code, in little hackish manner, as workaround for problem which is >>> fully solved by INTRNG. >>> Also, the semantic of bus_config_intr() is not clear, from INTRNG point >>> of view. >>> So i have tendency to ignore it :) >> Heh. Fair enough. I hadn't noticed that -- though I can see legitimate >> uses for it in the context of GPIOs. In any case, I'm a little wary of >> bus_alloc_resource() having side effects. Usually >> bus_activate_resource() would do that kind of thing. > Yes, good catch!! bus_activate_resource() is definitively best method. > >>>>> ------------ The following is a large parenthesis ------------------- >>>>> >>>>> One other possible route here that would also work well would be to >>>>> make ofwbus.c MD (it's a trivial piece of code anyway, so we don't >>>>> gain a lot by sharing it) and implement ofw_bus_map_intr() locally as >>>>> an ofwbus bus method. Then you could have the mapping table stored in >>>>> ofwbus's softc -- the API was designed for this initially. You would >>>>> need MD extensions for doing PIC registration there (which is fine), >>>>> but that segregates all the OFW-specific information into >>>>> OFW-specific code and would let the bus methods and the OFW interrupt >>>>> mapping table interact cleanly in the same place. This still >>>>> preserves the pre-r301453 API, makes PowerPC work, and maybe >>>>> address's skra@'s concern about extensibility and letting core >>>>> interrupt code know about FDT (or ACPI). I'd be happy to mock this up >>>>> as well if you think it's a good approach. >>>>> >>>> [snip] >>>>> This has the following features: >>>>> - Existing OFW API and semantics unchanged >>>>> - As such, PowerPC, PCI, etc. work fine with no changes >>>>> - Details encapsulated in MD code, so individual platforms can >>>>> implement this however they like >>>>> - arm/arm/intr.c (or whatever) only needs a method to allocate a >>>>> fresh interrupt, with no state, and anoter to set the device_t for an >>>>> interrupt sometime later. >>>>> - The internal table in the platform interrupt code has no knowledge >>>>> of any mappings whatsoever except having the appropriate device_t for >>>>> the PIC stored with the interrupt vector. >>>>> - Device tree bits handled purely in device tree code >>>>> - No action need be taken on any mapping until the interrupt is >>>>> actually allocated/set up, like r301453 >>>>> - Easy to add more mapping mechanisms (e.g. ACPI) by having similar >>>>> enumeration-mechanism-specific code in the root bus for that mapping >>>>> mechanism. >>>>> >>>>> -------------- End parenthesis ------------------------------- >>>> Here's an implementation of the parenthesis I wrote on an airplane >>>> this afternoon. It should be complete, though has not been tested. The >>>> code is short and simple (+70 lines in ofwbus.c). This preserves the >>>> pre-r301453 API and semantics relative to drivers, which means PowerPC >>>> and PCI work out of the box, while keeping the semantics relative to >>>> the interrupt layer of r301453 (PIC methods only called on resource >>>> allocation, no allocatable IRQs on unattached PICs, encapsulation of >>>> OFW-specific code in OFW-specific bits of the tree). It turns out >>>> those two things are compatible, somewhat to my surprise, and that >>>> makes the result very clean. I like this approach and would be happy >>>> to move forward with it. There are five functions of interest: >>>> >>>> 1. OFW_BUS_MAP_INTR(). This has the semantics and API it has now: you >>>> pass an interrupt specifier and parent, you get back an IRQ. No >>>> changes. This is the core of the normal OFW interrupt API. >>>> >>>> 2. OFW_BUS_REGISTER_PIC(device_t pic, phandle_t phandle). This is a >>>> new function that PIC drivers are supposed to use to register control >>>> of an interrupt domain. This replaces machine-specific code like >>>> powerpc_register_pic() to allow the PIC table to be in a bus parent >>>> rather than in the interrupt core. >>>> >>>> 3. PIC_MAP_OFW_INTR(device_t pic, int irq, interrupt specifier). This >>>> is a new function that PIC drivers that know how to handle device-tree >>>> interrupt descriptors implement (analogous to various existing ones >>>> that vary by platform). It tells the PIC that the given abstract IRQ >>>> means the given opaque interrupt specifier. >>>> >>>> 4. arm_allocate_irq(int suggested). This allocates a new IRQ number >>>> not (yet) attached to a PIC, as in r301453. I've added a parameter >>>> that lets you pass a suggested number to try in case it is possible to >>>> make it match an interrupt pin or something for human-readability. >>>> >>>> 5. arm_set_pic_for_irq(int irq, device_t). This tells the MD interrupt >>>> layer to associate a given PIC device_t with an IRQ. That is all the >>>> information the MD layer ever has about the IRQ mapping. >>>> >>>> Functions #1 and #2 are now implemented completely in ofwbus.c: there >>>> are no callouts anywhere else and the interrupt mapping table is >>>> maintained entirely internally to ofwbus (in its softc). In order to >>>> implement ACPI, or some other strategy, you would implement analogs to >>>> functions #1 and #2 that live somewhere in the bus hierarchy that is >>>> guaranteed to be above all devices that might want that type of >>>> interrupt (e.g. in acpi0), and some analog to #3 that PIC drivers >>>> implementing the mapping scheme would provide. >>>> >>>> Since the system interrupt code has no knowledge at all of interrupt >>>> mapping, of any type, in this scheme, adding new mapping types is >>>> trivial and can be done on a driver-by-driver basis if necessary >>>> without changing KPI and without any other part of the system even >>>> being aware. For example, GPIOs can use a completely different >>>> mechanism if they want and can do setup purely in the GPIO controller >>>> driver. You could have a method GPIO_GET_INTERRUPT_FOR_PIN() on a GPIO >>>> controller in which the GPIO controller allocates a generic IRQ, >>>> assigns through some internal table just in the GPIO driver, and >>>> returns to it to a consumer in some other device driver -- without a >>>> GPIO mapping type, new bus functions, or modifications to the platform >>>> interrupt code. >>>> >>>> The control flow goes like this: >>>> - Bus driver enumerates children, parses interrupts properties, calls >>>> OFW_BUS_MAP_INTR() to get IRQs for them (as pre-r301453), adds to >>>> interrupt list. >>>> - ofwbus receives the OFW_BUS_MAP_INTR() call, allocates a blank >>>> disconnected IRQ from the MD interrupt layer, and stores the mapping >>>> from the new IRQ to the given interrupt specifier and phandle in an >>>> internal table in ofwbus's softc. >>>> NB: Nothing else happens here, like post-r301453. Changing this does >>>> not change any semantics of the API pre-r301453, which means it >>>> remains fully compatible with PCI and PowerPC. Also, like >>>> post-r301453, there is no involvement of nexus. >>>> - PICs attach and call OFW_BUS_REGISTER_PIC(). ofwbus receives these >>>> messages and adds a (device_t, phandle_t) mapping to a second internal >>>> table. Note that the interrupt layer does not need to handle PIC >>>> registration anymore at all (except for the root PIC). >>>> - Bus child eventually calls a function that tries to set the >>>> interrupt up (e.g. bus_setup_intr()). That propagates up the bus >>>> hierarchy, eventually getting to ofwbus. ofwbus notes the IRQ number, >>>> looks it up in the table, looks up the appropriate PIC from the PIC >>>> table, then: >>>> A) calls arm_set_pic_for_irq(irq, pic_device_t) -- this is the >>>> interrupt layer's only interaction with the mapping code. All it deals >>>> with is device_ts and abstract IRQ numbers. >>>> B) calls PIC_MAP_OFW_INTR(pic, irq_number, interrupt-cells, >>>> interrupt-specifier) to tell the PIC that the interrupt layer's IRQ >>>> irq_number means the given specifier >>>> C) finally, passes the call onto nexus, which will do whatever would >>>> normally happen (unmasking the interrupt, setting handlers, etc.) in >>>> terms only of the abstract IRQ and the device_t assigned by ofwbus. >>>> >>>> You would implement ACPI just by doing a s/OFW/ACPI/g >>>> search-and-replace above -- since the interrupt layer doesn't know >>>> about OFW or ACPI or anything else, there is no need to touch it. This >>>> seems clean, simple, compartmentalized, preserves the existing API, >>>> and should work on all of our various hardware. PowerPC can't quite >>>> work with it yet without some multipass foo, but, since the API is >>>> preserved, that transition can happen gradually without KPI changes. >>>> For the same reason that it is API-preserving, I think this code is >>>> also MFC-able. >>>> -Nathan >>> I think that we are slightly diverges in this place: >>> - please note that PIC API (in kern/pic_if.m) is different from the >>> one >>> that PPC uses. >> Yes, which is fine (this is machine-dependent code anyway). On >> consideration, the PIC_MAP_OFW_INTR() function should probably be >> called OFW_BUS_PIC_MAP_INTR() and live in ofw_bus_if.m anyway, then be >> implemented by PICs. >> >>> - we must be able to store configuration data (original interrupt >>> specifier) in all cases, not only for OFW. That's reason why I have >>> proposed >>> to create 'mapping table'. >> It is stored in all cases, just not in the core interrupt code. I've >> only mocked this up for OFW here. For ACPI, you would have the >> equivalent table in acpi.c, along with ACPI_MAP_INTR(), >> ACPI_REGISTER_PIC(), etc. in acpi_if.m and implemented in >> dev/acpica/acpi.c. >> >> For GPIOs, you would have a mechanism that just traces however you are >> representing GPIOs anyway. >> >>> Anyway, i think that we both talking about +/- same solution, only i >>> want to move it from OFW specific level implemented at OFW bus level to >>> bus/interrupt specifier format independent level implemented in >>> subr_intr.c. >> This implements the same API in any case (identical to that >> pre-r301453), so the implementation doesn't really matter at all in >> terms of my needs. > Perfect. >> That said, having it in the root bus for the mapping domain (ofwbus0, >> acpi0) etc. seems cleaner to me, with no loss of functionality. The >> core interrupt code (subr_intr.c or whatever) doesn't have any obvious >> need to know anything at all about the mapping information so long as >> it knows the PIC device_t that corresponds to every abstract IRQ so >> that it can mask it or do other operations. Since, presumably, nothing >> in an ACPI device tree references an interrupt in the OFW device tree >> (if you even had both -- and how would you specify that, anyway?), >> implementing the relevant bits one step up the bus hierarchy doesn't >> change any behavior. >> >> And nothing can possibly be more flexible in terms of the mapping >> table in subr_intr.c than not having a mapping table: you don't need >> enums for mapping types, or unions that need to be expanded, breaking >> KBI, or anything. You can delete all the PIC registration (and MSI) >> code, all the switches, and all the unions from subr_intr.c and have a >> totally mapping-mechanism-agnostic KPI. >> > Where you see "all the switches, and all the unions" in subr_intr.c? > subr_intr.c uses nested structures approach, in exactly same way as is > used in OFW bus. Moreover, subr_intr.c *IS* currently totally > mapping-mechanism-agnostic KPI, and any form of mapping table must > follow this rule. That was hyperbolic; my apologies. The point was that you don't need intr_map_data, or the intr_map_data_type enum, this way. One of the nice things about that is that you don't need to add more entries to the enum to add new one-off mapping types. For example, the PS3 platform on PPC has its own non-device-tree, non-ACPI bus description and interrupt system. Needing to make new enums (or anything) like INTR_MAP_DATA_PS3 in sys/bus.h seems a little silly, though hardly very bad. I've attached a version of PowerPC's intr_machdep.c and pic_if.m that fully implements this distributed-table scheme. The code ends up being very short and clean (a third the number of lines of code as kern/subr_intr.c, for example) and the diff to PowerPC, including the new ofwbus.c code, ends up being net-negative. > But allow me to make short summary: > - we must support 3 'main' cases - OFW, ACPI and HINTS (for older arm > and mips boards) based systems > - single kernel must support all above cases, but only one can by > 'active' (kernel must be able to select right one at runtime). > - we must support 'direct' (interrupt specifier is defined by one of > above methods) or 'indirect' (where interrupt is associated with other > function - GPIO pin, but don't have direct description). > - we must be able to access single physical pin by both methods, > 'direct' and/or 'indirect'. > - we must be able to add new interrupt type as simple as possible Agreed with all of this. > > For interrupt controllers: > - single controller must be able to parse multiple formats. 'direct' or > 'indirect'. OFW or ACPI. ARM GIC must accept OFW or ACPI arguments (in > single kernel), GPIO based interrupt controller must accept 'direct' or > 'indirect' formats. > > For interrupt consumers: > - 'direct' interrupts are easy > - driver must be able to consume 'indirect' interrupt in 'root > bus'/'mapping-mechanism' agnostic manner. > For example, MMC driver must be able to get interrupt from CD gpio > pin in all possible cases/combinations . > Are we still connected? And this. > I don't think that all this is possible without single universal format > of interrupt mapping specifier. > And, if we must have universal format then why we needs different > mapping databases? It actually works just fine in this mapping-table-in-the-root-bus model. If you have an OFW-type of interrupt, it is set up by ofwbus.c. Since everything that would use an OFW interrupt is a child of ofwbus, this is totally indistinguishable from a global table from the perspective of client code. For ACPI interrupts, they are set up by acpi.c, which is indistinguishable from a global table for the same reason. For hints -- and single-domain systems generally -- you can just have the machine-dependent code assume that interrupts it doesn't explicitly know about belong to the root PIC. This is what you call the "direct" case. For the "indirect" case, there are as usual a few things you could do. They match the set of things you would do in any implementation: 1. Set up one or more global registries for "indirect" PICs, with corresponding mapping methods, either as bus methods on some high-level bus or on the machine-specific nexus, or just a global function that you call. 2. Use the global registry that you already have to have for GPIO controllers and provide a mapping method on the GPIO controller (GPIO_GET_INTERRUPT_FOR_PIN() or something) that returns a mapped IRQ. One of the nice things about distributing the tables this way is that you have lots of flexibility with things like these "indirect" interrupts and are free to do things like #2 at will locally in some machine-specific set of drivers. For example, in the PS3 code, I don't need to add a new "PS3" mapping type *anywhere*. The ps3bus root driver just has a table when it hands out interrupts and talks to ps3pic internally. -Nathan >>> Most of your control flow can be implemented at general level as is, or >>> already exist [intr_map_irq(), intr_pic_register()] . >>> Also, impact for current PPC code is, by me, minimal. >>> I can sketch my idea in more details, if you found it acceptable. >> All ideas are fine -- and the solution does not need to apply to all >> platforms anyway, so long as the OFW_BUS_MAP_INTR() semantics (and, >> ideally, API) are preserved. >> >>> Again, I'm sorry for delayed and very brief response. >> No worries; good luck with the work emergencies. >> -Nathan > All problems have been solved, sleep deficit remains :) > 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" > --------------3EFCE033AD3743BB7374BC07 Content-Type: text/plain; charset=UTF-8; name="intr_machdep.c" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="intr_machdep.c" /*- * Copyright (c) 1991 The Regents of the University of California. * All rights reserved. * * This code is derived from software contributed to Berkeley by * William Jolitz. * * Redistribution and use in source and binary forms, with or without * modification, are permitted provided that the following conditions * are met: * 1. Redistributions of source code must retain the above copyright * notice, this list of conditions and the following disclaimer. * 2. Redistributions in binary form must reproduce the above copyright * notice, this list of conditions and the following disclaimer in the * documentation and/or other materials provided with the distribution. * 4. Neither the name of the University nor the names of its contributors * may be used to endorse or promote products derived from this software * without specific prior written permission. * * THIS SOFTWARE IS PROVIDED BY THE REGENTS AND CONTRIBUTORS ``AS IS'' AND * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE * ARE DISCLAIMED. IN NO EVENT SHALL THE REGENTS OR CONTRIBUTORS BE LIABLE * FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS * OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) * HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT * LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY * OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF * SUCH DAMAGE. */ /*- * Copyright (c) 2002 Benno Rice. * All rights reserved. * * Redistribution and use in source and binary forms, with or without * modification, are permitted provided that the following conditions * are met: * 1. Redistributions of source code must retain the above copyright * notice, this list of conditions and the following disclaimer. * 2. Redistributions in binary form must reproduce the above copyright * notice, this list of conditions and the following disclaimer in the * documentation and/or other materials provided with the distribution. * * THIS SOFTWARE IS PROVIDED BY THE AUTHOR AND CONTRIBUTORS ``AS IS'' AND * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE * ARE DISCLAIMED. IN NO EVENT SHALL THE AUTHOR OR CONTRIBUTORS BE LIABLE * FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS * OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) * HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT * LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY * OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF * SUCH DAMAGE. * * from: @(#)isa.c 7.2 (Berkeley) 5/13/91 * form: src/sys/i386/isa/intr_machdep.c,v 1.57 2001/07/20 * * $FreeBSD: head/sys/powerpc/powerpc/intr_machdep.c 296177 2016-02-29 03:38:00Z jhibbits $ */ #include "opt_isa.h" #include <sys/param.h> #include <sys/systm.h> #include <sys/kernel.h> #include <sys/queue.h> #include <sys/bus.h> #include <sys/cpuset.h> #include <sys/interrupt.h> #include <sys/ktr.h> #include <sys/lock.h> #include <sys/malloc.h> #include <sys/mutex.h> #include <sys/pcpu.h> #include <sys/smp.h> #include <sys/syslog.h> #include <sys/vmmeter.h> #include <sys/proc.h> #include <machine/frame.h> #include <machine/intr_machdep.h> #include <machine/md_var.h> #include <machine/smp.h> #include <machine/trap.h> #include "pic_if.h" #define MAX_STRAY_LOG 5 static MALLOC_DEFINE(M_INTR, "intr", "interrupt handler data"); struct powerpc_intr { struct intr_event *event; long *cntp; u_int irq; u_int vector; device_t pic; u_int cntindex; cpuset_t cpu; int ipi; }; static u_int intrcnt_index = 0; static struct mtx intr_table_lock; static struct powerpc_intr *powerpc_intrs[INTR_VECTORS]; static u_int nvectors; /* Allocated vectors */ static u_int nirqs = 0; /* Allocated IRQs. */ static u_int stray_count; u_long intrcnt[INTR_VECTORS]; char intrnames[INTR_VECTORS * MAXCOMLEN]; size_t sintrcnt = sizeof(intrcnt); size_t sintrnames = sizeof(intrnames); device_t root_pic; #ifdef SMP static void *ipi_cookie; #endif static void intr_init(void *dummy __unused) { mtx_init(&intr_table_lock, "intr sources lock", NULL, MTX_DEF); } SYSINIT(intr_init, SI_SUB_INTR, SI_ORDER_FIRST, intr_init, NULL); #ifdef SMP static void smp_intr_init(void *dummy __unused) { struct powerpc_intr *i; int vector; for (vector = 0; vector < nvectors; vector++) { i = powerpc_intrs[vector]; if (i != NULL && i->event != NULL && i->pic == root_pic) PIC_BIND(i->pic, i->irq, i->cpu); } } SYSINIT(smp_intr_init, SI_SUB_SMP, SI_ORDER_ANY, smp_intr_init, NULL); #endif static void intrcnt_setname(const char *name, int index) { snprintf(intrnames + (MAXCOMLEN + 1) * index, MAXCOMLEN + 1, "%-*s", MAXCOMLEN, name); } void intrcnt_add(const char *name, u_long **countp) { int idx; idx = atomic_fetchadd_int(&intrcnt_index, 1); *countp = &intrcnt[idx]; intrcnt_setname(name, idx); } static struct powerpc_intr * intr_lookup(u_int irq); { struct powerpc_intr *i; int vector; mtx_lock(&intr_table_lock); for (vector = 0; vector < nvectors; vector++) { i = powerpc_intrs[vector]; if (i != NULL && i->irq == irq) { mtx_unlock(&intr_table_lock); return (i); } } mtx_unlock(&intr_table_lock); return (NULL); } int powerpc_alloc_intr(int suggested) { char intrname[16]; struct powerpc_intr *i, *iscan; int vector, irq; mtx_lock(&intr_table_lock); if (suggested <= 0) { for (vector = 0; vector < nvectors; vector++) { i = powerpc_intrs[vector]; if (i != NULL && i->irq == suggested) { suggested = 0; break; } } } /* * If no suggestion, or we couldn't provide it, use max(1000, max_irq+1) */ if (suggested <= 0) { irq = 1000; for (vector = 0; vector < nvectors; vector++) { i = powerpc_intrs[vector]; if (i != NULL && i->irq == suggested) irq = max(irq, i->irq + 1); } } i = malloc(sizeof(*i), M_INTR, M_NOWAIT); if (i == NULL) { mtx_unlock(&intr_table_lock); return (NULL); } i->event = NULL; i->cntp = NULL; i->irq = irq; i->pic = NULL; i->vector = -1; i->ipi = 0; #ifdef SMP i->cpu = all_cpus; #else CPU_SETOF(0, &i->cpu); #endif for (vector = 0; vector < INTR_VECTORS; vector++) { iscan = powerpc_intrs[vector]; if (iscan == NULL && i->vector == -1) { i->vector = vector; break; } } if (i->vector != -1) { powerpc_intrs[i->vector] = i; i->cntindex = atomic_fetchadd_int(&intrcnt_index, 1); i->cntp = &intrcnt[i->cntindex]; sprintf(intrname, "irq%u:", i->irq); intrcnt_setname(intrname, i->cntindex); nvectors++; } mtx_unlock(&intr_table_lock); if (i->vector == -1) { free(i, M_INTR); i = NULL; } return (i); } int powerpc_set_pic_for_irq(int irq, device_t pic) { struct powerpc_intr *i; i = irq_lookup(irq); KASSERT(i != NULL, ("%s: NULL IRQ %d", __func__, irq)); i->pic = pic; return (0); } static void powerpc_intr_eoi(void *arg) { struct powerpc_intr *i = arg; PIC_EOI(i->pic, i->irq); } static void powerpc_intr_pre_ithread(void *arg) { struct powerpc_intr *i = arg; PIC_MASK(i->pic, i->irq); PIC_EOI(i->pic, i->irq); } static void powerpc_intr_post_ithread(void *arg) { struct powerpc_intr *i = arg; PIC_UNMASK(i->pic, i->irq); } static int powerpc_assign_intr_cpu(void *arg, int cpu) { #ifdef SMP struct powerpc_intr *i = arg; if (cpu == NOCPU) i->cpu = all_cpus; else CPU_SETOF(cpu, &i->cpu); if (!cold && i->pic != NULL && i->pic == root_pic) PIC_BIND(i->pic, i->irq, i->cpu); return (0); #else return (EOPNOTSUPP); #endif } int powerpc_enable_intr(void) { struct powerpc_intr *i; int error, vector; #ifdef SMP int ipi_irq; #endif if (npics == 0) panic("no PIC detected\n"); if (root_pic == NULL) panic("no root PIC\n"); #ifdef SMP /* Install an IPI handler. */ if (mp_ncpus > 1) { ipi_irq = powerpc_alloc_intr(4096); error = PIC_MAP_IPI(root_pic, ipi_irq); KASSERT(error == 0, ("%s: SMP root PIC failed to map IPI", __func__, error)); error = powerpc_setup_intr("IPI", ipi_irq, powerpc_ipi_handler, NULL, NULL, INTR_TYPE_MISC | INTR_EXCL, &ipi_cookie); if (error) { printf("unable to setup IPI handler\n"); return (error); } /* * Some subterfuge: disable late EOI and mark this * as an IPI to the dispatch layer. */ i = intr_lookup(ipi_irq, FALSE); i->event->ie_post_filter = NULL; i->ipi = 1; } #endif for (vector = 0; vector < nvectors; vector++) { i = powerpc_intrs[vector]; if (i == NULL) continue; if (i->pic == NULL) continue; if (i->event != NULL) PIC_ENABLE(i->pic, i->irq, vector); } return (0); } int powerpc_setup_intr(const char *name, u_int irq, driver_filter_t filter, driver_intr_t handler, void *arg, enum intr_type flags, void **cookiep) { struct powerpc_intr *i; int error, enable = 0; i = intr_lookup(irq, FALSE); if (i == NULL) return (ENOMEM); if (i->event == NULL) { error = intr_event_create(&i->event, (void *)i, 0, irq, powerpc_intr_pre_ithread, powerpc_intr_post_ithread, powerpc_intr_eoi, powerpc_assign_intr_cpu, "irq%u:", irq); if (error) return (error); enable = 1; } error = intr_event_add_handler(i->event, name, filter, handler, arg, intr_priority(flags), flags, cookiep); mtx_lock(&intr_table_lock); intrcnt_setname(i->event->ie_fullname, i->cntindex); mtx_unlock(&intr_table_lock); if (!cold && i->pic != NULL) if (i->pic == root_pic) PIC_BIND(i->pic, i->irq, i->cpu); if (enable) PIC_ENABLE(i->pic, i->irq, i->vector); } return (error); } int powerpc_teardown_intr(void *cookie) { return (intr_event_remove_handler(cookie)); } #ifdef SMP int powerpc_bind_intr(u_int irq, u_char cpu) { struct powerpc_intr *i; i = intr_lookup(irq); if (i == NULL) return (ENOMEM); return (intr_event_bind(i->event, cpu)); } #endif int powerpc_config_intr(int irq, enum intr_trigger trig, enum intr_polarity pol) { struct powerpc_intr *i; i = intr_lookup(irq); if (i == NULL) return (ENOMEM); i->trig = trig; i->pol = pol; if (i->pic != NULL) PIC_CONFIG(i->pic, i->irq, trig, pol); return (0); } void powerpc_dispatch_intr(u_int vector, struct trapframe *tf) { struct powerpc_intr *i; struct intr_event *ie; i = powerpc_intrs[vector]; if (i == NULL) goto stray; (*i->cntp)++; ie = i->event; KASSERT(ie != NULL, ("%s: interrupt without an event", __func__)); /* * IPIs are magical and need to be EOI'ed before filtering. * This prevents races in IPI handling. */ if (i->ipi) PIC_EOI(i->pic, i->irq); if (intr_event_handle(ie, tf) != 0) { goto stray; } return; stray: stray_count++; if (stray_count <= MAX_STRAY_LOG) { printf("stray irq %d\n", i ? i->irq : -1); if (stray_count >= MAX_STRAY_LOG) { printf("got %d stray interrupts, not logging anymore\n", MAX_STRAY_LOG); } } if (i != NULL) PIC_MASK(i->pic, i->irq); } void powerpc_intr_mask(u_int irq) { struct powerpc_intr *i; i = intr_lookup(irq); if (i == NULL || i->pic == NULL) return; PIC_MASK(i->pic, i->irq); } void powerpc_intr_unmask(u_int irq) { struct powerpc_intr *i; i = intr_lookup(irq); if (i == NULL || i->pic == NULL) return; PIC_UNMASK(i->pic, i->irq); } --------------3EFCE033AD3743BB7374BC07 Content-Type: text/plain; charset=UTF-8; name="pic_if.m" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="pic_if.m" #- # Copyright (c) 1998 Doug Rabson # All rights reserved. # # Redistribution and use in source and binary forms, with or without # modification, are permitted provided that the following conditions # are met: # 1. Redistributions of source code must retain the above copyright # notice, this list of conditions and the following disclaimer. # 2. Redistributions in binary form must reproduce the above copyright # notice, this list of conditions and the following disclaimer in the # documentation and/or other materials provided with the distribution. # # THIS SOFTWARE IS PROVIDED BY THE AUTHOR AND CONTRIBUTORS ``AS IS'' AND # ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE # IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE # ARE DISCLAIMED. IN NO EVENT SHALL THE AUTHOR OR CONTRIBUTORS BE LIABLE # FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL # DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS # OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) # HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT # LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY # OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF # SUCH DAMAGE. # # from: src/sys/kern/bus_if.m,v 1.21 2002/04/21 11:16:10 markm Exp # $FreeBSD: head/sys/powerpc/powerpc/pic_if.m 257059 2013-10-24 15:37:32Z nwhitehorn $ # #include <sys/bus.h> #include <sys/cpuset.h> #include <machine/frame.h> INTERFACE pic; METHOD void bind { device_t dev; u_int irq; cpuset_t cpumask; }; METHOD void config { device_t dev; u_int irq; enum intr_trigger trig; enum intr_polarity pol; }; METHOD void dispatch { device_t dev; struct trapframe *tf; }; METHOD void enable { device_t dev; u_int irq; u_int vector; }; METHOD void eoi { device_t dev; u_int irq; }; METHOD void ipi { device_t dev; u_int cpu; }; METHOD void mask { device_t dev; u_int irq; }; METHOD void unmask { device_t dev; u_int irq; }; METHOD int map_ipi { device_t dev; }; --------------3EFCE033AD3743BB7374BC07--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?1946069a-d0f9-2c19-80a5-0b490682574b>