linux-kernel - Re: [PATCH] ACPI / GPIO: Pass index to acpi_get_gpiod_by

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Sun, 02 Nov 2014 20:49:37 -0800
From:	Darren Hart <dvhart@...ux.intel.com>
To:	Grant Likely <grant.likely@...retlab.ca>,
	"Rafael J. Wysocki" <rjw@...ysocki.net>,
	Mika Westerberg <mika.westerberg@...ux.intel.com>,
	Alexandre Courbot <acourbot@...dia.com>,
	Linus Walleij <linus.walleij@...aro.org>,
	Arnd Bergmann <arnd@...db.de>
CC:	linux-acpi@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] ACPI / GPIO: Pass index to acpi_get_gpiod_by_index()
 when using properties



On 11/1/14 4:11, Grant Likely wrote:
> On Tue, 28 Oct 2014 22:59:57 +0100
> , "Rafael J. Wysocki" <rjw@...ysocki.net>
>  wrote:
>> On Tuesday, October 28, 2014 01:15:27 PM Mika Westerberg wrote:
>>> acpi_dev_add_driver_gpios() makes it possible to set up mapping between
>>> properties and ACPI GpioIo resources in a driver, so we can take index
>>> parameter in acpi_find_gpio() into use with _DSD device properties now.
>>>
>>> This index can be used to select a GPIO from a property with multiple
>>> GPIOs:
>>>
>>>   Package () {
>>>   	"data-gpios",
>>>   	Package () {
>>>   		\_SB.GPIO, 0, 0, 0,
>>>   		\_SB.GPIO, 1, 0, 0,
>>>   		\_SB.GPIO, 2, 0, 1,
>>>   	}
>>>   }
>>>
>>> In order to retrieve the last GPIO from a driver we can simply do:
>>>
>>>   desc = devm_gpiod_get_index(dev, "data", 2);
>>>
>>> and so on.
>>>
>>> Signed-off-by: Mika Westerberg <mika.westerberg@...ux.intel.com>
>>
>> Cool. :-)
>>
>> Any objections anyone?
> 
> Actually, I do. Not in the idea, but in the implementation. The way this gets encoded:
> 
> 	Package () {
> 		\_SB.GPIO, 0, 0, 0,
> 		\_SB.GPIO, 1, 0, 0,
> 		\_SB.GPIO, 2, 0, 1,
> 	}
> 
> Means that decoding each GPIO tuple requires the length of a tuple to be
> fixed, or to implement a DT-like #gpio-cells. If it is fixed, then there
> is no way to expand the binding later. Can this be done in the following
> way instead?
> 
> 	Package () {
> 		Package () { \_SB.GPIO, 0, 0, 0 },
> 		Package () { \_SB.GPIO, 1, 0, 0 },
> 		Package () { \_SB.GPIO, 2, 0, 1 },
> 	}
> 
> This is one of the biggest pains in device tree. We don't have any way
> to group tuples so it requires looking up stuff across the tree to
> figure out how to parse each multi-item property.
> 
> I know that last year we talked about how bios vendors would get
> complicated properties wrong, but I think there is little risk in this
> case. If the property is encoded wrong, the driver simply won't work and
> it is unlikely to get shipped before being fixed.

This particular nesting of Packages is expressly prohibited by the
Device Properties UUID for the reasons you mention.

http://www.uefi.org/sites/default/files/resources/_DSD-device-properties-UUID.pdf

2.1 Data Format Definition
...
" a Package consisting entirely of Integer, String, or Reference objects
(and
specifically not containing a nested Package)."

That said, I am not fond of the many properties mixed in as a single
Package. We discussed this at some length while this was being proposed,
and this was deemed the lesser evil.

-- 
Darren Hart
Intel Open Source Technology Center
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/