Date: Sat, 23 Jul 2016 11:36:46 -0700 From: Nathan Whitehorn <nwhitehorn@freebsd.org> To: Michal Meloun <mmel@FreeBSD.org>, John Baldwin <jhb@FreeBSD.org>, Andrew Turner <andrew@fubar.geek.nz> Cc: Svatopluk Kraus <skra@FreeBSD.org>, src-committers@FreeBSD.org, svn-src-all@FreeBSD.org, svn-src-head@FreeBSD.org 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 Message-ID: <2d0b7e5f-bdd8-095e-dd1f-1044164edc40@freebsd.org> In-Reply-To: <57934F27.9070800@FreeBSD.org> References: <201606051620.u55GKD5S066398@repo.freebsd.org> <578F6075.7010500@FreeBSD.org> <20160721133742.05f0e045@zapp> <13301107.Hm25rxUxW2@ralph.baldwin.cx> <57934F27.9070800@FreeBSD.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On 07/23/16 04:04, Michal Meloun wrote:
> Dne 21.07.2016 v 23:35 John Baldwin napsal(a):
>> On Thursday, July 21, 2016 01:37:42 PM Andrew Turner wrote:
>>> On Wed, 20 Jul 2016 13:28:53 +0200
>>> Michal Meloun <mmel@FreeBSD.org> 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(). 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. 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.
>>
> I like this idea. Mainly if we can add 'struct alloc_resource_args' into
> 'struct resource_list_entry' and, eventually, also into struct resource_i.
> Inability to pass something more complex as single integer between bus
> enumerator (aka resource_list_entry creator) and bus_alloc_resource()
> (aka resource_list_entry consumer) is serious limitation. At lest for me :)
> Michal
>
>
Unfortunately, it doesn't actually work for resources that don't follow
the bus hierarchy, however (see earlier follow-up emails to jhb from
myself and others).
-Nathan
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?2d0b7e5f-bdd8-095e-dd1f-1044164edc40>