From owner-svn-src-head@freebsd.org Fri Jul 22 05:51:29 2016 Return-Path: Delivered-To: svn-src-head@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id BFDA7BA1E98; Fri, 22 Jul 2016 05:51:29 +0000 (UTC) (envelope-from nwhitehorn@freebsd.org) Received: from c.mail.sonic.net (c.mail.sonic.net [64.142.111.80]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (Client did not present a certificate) by mx1.freebsd.org (Postfix) with ESMTPS id A46DD1DE7; Fri, 22 Jul 2016 05:51:29 +0000 (UTC) (envelope-from nwhitehorn@freebsd.org) Received: from zeppelin.tachypleus.net (75-101-50-44.static.sonic.net [75.101.50.44]) (authenticated bits=0) by c.mail.sonic.net (8.15.1/8.15.1) with ESMTPSA id u6M5pKLv007017 (version=TLSv1.2 cipher=DHE-RSA-AES128-SHA bits=128 verify=NOT); Thu, 21 Jul 2016 22:51:20 -0700 Subject: Re: svn commit: r301453 - in head/sys: arm/arm arm64/arm64 dev/fdt dev/gpio dev/iicbus dev/ofw dev/pci dev/vnic kern mips/mips sys To: John Baldwin , Andrew Turner References: <201606051620.u55GKD5S066398@repo.freebsd.org> <578F6075.7010500@FreeBSD.org> <20160721133742.05f0e045@zapp> <13301107.Hm25rxUxW2@ralph.baldwin.cx> Cc: Michal Meloun , Svatopluk Kraus , src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org From: Nathan Whitehorn Message-ID: <03bcc081-5b93-c2ba-4e9b-e51d0b2e773a@freebsd.org> Date: Thu, 21 Jul 2016 22:51:20 -0700 User-Agent: Mozilla/5.0 (X11; FreeBSD amd64; rv:45.0) Gecko/20100101 Thunderbird/45.1.0 MIME-Version: 1.0 In-Reply-To: <13301107.Hm25rxUxW2@ralph.baldwin.cx> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit X-Sonic-CAuth: UmFuZG9tSVbYawnepQAe1PF0BqBTxPf5l0T1d5QKZisjH8BrcAFE4KoXgJ6UCg5YhgKJwnPBa/qDtRh7bBTel4+AuT1HRCcHnp8YLS1d+cI= X-Sonic-ID: C;8t5dUNBP5hG1l5tMTlz00w== M;AkC6UNBP5hG1l5tMTlz00w== X-Spam-Flag: No X-Sonic-Spam-Details: 0.0/5.0 by cerberusd X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.22 Precedence: list List-Id: SVN commit messages for the src tree for head/-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 22 Jul 2016 05:51:29 -0000 On 07/21/16 14:35, John Baldwin wrote: > On Thursday, July 21, 2016 01:37:42 PM Andrew Turner wrote: >> On Wed, 20 Jul 2016 13:28:53 +0200 >> Michal Meloun wrote: >>> Dne 19.07.2016 v 17:06 Nathan Whitehorn napsal(a): >>>> 2. It partially duplicates the functionality of OFW_BUS_MAP_INTR(), >>>> but is both problematically more general and less flexible (it has >>>> requirements on timing of PIC attachment vs. driver resource >>>> allocation) >>> OFW_BUS_MAP_INTR() can parse only OFW based data and expect that >>> parsed data are magicaly stored within the call. >>> The new method, bus_map_intr(), can parse data from multiple sources >>> (OFW, UEFI / ACPI, synthetic[gpio device + pin number]). It also >>> returns parsed data back to caller. >>> And no, it doesn't add any additional timing requirements . >> I've been looking at ACPI on arm64. So far I have not found the need >> for this with ACPI as we don't need to send the data to the interrupt >> controller driver to be parsed in the way OFW/FDT needs to. > ACPI though has a gross hack where we call BUS_CONFIG_INTR on the IRQ > in bus_alloc_resource(). I hadn't realized that. It looks like you could do essentially the same thing we do on PowerPC to clean this up by explicitly mapping the ACPI interrupt domains to different PICs with varying default interrupt properties. > What I had advocated in the discussions > leading up to this was to have some sort of opaque structure containing > a set of properties (the sort of thing bus_map_resource and make_dev_s > use) that was passed up at bus_setup_intr() time. > I think it should now > be passed up at bus_alloc_resource() time instead, but it would allow bus > drivers to "decorate" a SYS_RES_IRQ request as it goes up the device tree > with properties that the interrupt controller can then associate with > the IRQ cookie it allocates in its own code. We used to do this on PPC and MIPS, and the current code still supports it, but it turned out not to be useful in the end for IRQs. The hierarchy for IRQs rarely (read: almost never) follows the bus hierarchy and often is enumerated in a different order. I have hardware, for example, where the children of a single parent bus are all wired to different interrupt controllers and sometimes to a mixture of interrupt controllers. Those controllers are cascaded in ways that cross the newbus tree laterally and, on some of them, the parent device from the bus topology has interrupts handled by its own (bus) children. Trying to make the newbus parents do something sensible with all of this would be crazy and, in the case where parents depend on resources provided by their own children, impossible. This is all to say that, since you want the interrupts to be decorated along a path that usually has nothing to do with the newbus hierarchy, it doesn't add much to add extra features to resource allocation. ofw_bus_map_intr() is a newbus method to support this kind of thing but, on all supported platforms, it is implemented only in nexus and no cases have appeared where anyone ever wanted anything at the intermediate layers. > I would let the particular > structure have different layouts for different resource types. On x86 we > would replace bus_config_intr by passing the level and trigger-mode in > this structure. However, I could also see allowing the memattr to be > set for a SYS_RES_MEMORY resource so you could have a much shorter way > than an explicit bus_map_resource to map an entire BAR as WC for example: > > struct alloc_resource_args { > size_t len; > union { > struct { > enum intr_trigger trigger; > enum intr_polarity polarity; > } irq; > struct { > vm_memattr_t memattr; > } memory; > } > } > > ... > > union alloc_resource_args args; > > init_alloc_resource_args(&args, sizeof(args)); > args.memattr = VM_MEMATTR_WRITE_COMBINING; > > /* Uses WC for the implicit mapping. */ > res = bus_alloc_resource(...., &args); > > ... > > foobus_alloc_resource(..., union alloc_resource_args *args) > { > union alloc_resource_args args2; > > switch (type) { > case SYS_RES_IRQ: > if (args == NULL) { > init_alloc_resource_args(&args2, sizeof(args2)); > args = &args2; > } > /* Replace call to BUS_CONFIG_INTR on ACPI: */ > if (args->irq.polarity == INTR_POLARITY_CONFORMING && > device_has_polarity_from_CRS) > args->irq.polarity = polarity_from_CRS; > ... > } > > However, you could associate arbitrary data with a resource request by > adding more members to the approriate struct in the union. > For memory, I think this is an interesting concept, but it really doesn't match well with what you would want to do for interrupts or for, say, GPIOs in which the lines of control are fundamentally unrelated to the newbus hierarchy. -Nathan