Re: [PATCH] ACPI / GPIO: Pass index to acpi_get_gpiod_by_index() when using properties

[Grant Likely <grant.likely@secretlab.ca>]

On Tue, 28 Oct 2014 22:59:57 +0100
, "Rafael J. Wysocki" <rjw@rjwysocki.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@linux.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.

g.