[Ksummit-discuss] [TECH TOPIC] Driver model/resources, ACPI, DT, etc (sigh)

[Darren Vincent Hart]

On Sun, 2014-05-18 at 22:28 +0200, Linus Walleij wrote:
> On Sun, May 18, 2014 at 1:03 AM, Benjamin Herrenschmidt
> <benh at kernel.crashing.org> wrote:
> > On Fri, 2014-05-16 at 18:54 -0700, Darren Vincent Hart wrote:
> >
> >> In my opinion, the common dev_get_property() interface which calls the
> >> appropriate firmware accessor function makes the most sense. Creating
> >> another intermediate format which we then have to make into something
> >> useful (like pdata) strikes me as unnecessary and likely limiting.
> >
> > So in the end it will really depend on whether people are good enough to
> > use the same property/value "names" and format accross the
> > representations.
> >
> > So yes, maybe something like dev_get_property() would work fine for
> > most cases, which would be great. And for the always necessary quirks
> > where for example the ACPI variant used a wrong spelling or the DT
> > variant used a different size or something, the driver can either
> > openly call different of and acpi variants or we could have quirks in
> > the driver itself... ie, a pointer in struct device to a quirk table,
> > possibly based on hash of the name for fast lookup. But let's wait
> > for some real implementations to see how necessary that really is.
> 
> This has already happened with GPIO as DT had named GPIOs
> and ACPI yet had not, but could get GPIOs from a certain
> index, which DT also could.
> 
> So:
> 
> gpios = <a, b>;
> 
> or:
> 
> foo-gpio = <a>;
> bar-gpio = <b>;
> 
> Whereas in ACPI it would only be the former representation.
> So the prototype had to be something like:
> 
> GPIO = gpiod_get_index(device, name, index);
> 
> So we first look for a named GPIO and if that doesn't work
> we look for an indexed GPIO. All fine.
> 
> Anyway, then ACPI said they are going to introduce named
> GPIOs so all is good. Or is it?
> 
> No, they can still choose a totally different name from what
> DT is using. So we end up with code like this:
> 
> if (gpio = gpiod_get_index(device, "foo", index))
> ...
> else if (gpio = giod_get_index(device, "bar", index))
> ...
> 
> That is however not enough since they can also disagree
> with indexed values so that whereas the two GPIO pins
> may be gpios = <a, b>; in DT nothing stops the ACPI
> guys from putting it in order <b, a> and we get code to
> compensate for that instead.

I see two distinct problems being raised. One on equivalent mechanisms,
and one on parameterization.

With respect to mechanism, it is my understanding that ACPI 5.0 does
provide for GpioIO pin lists, but for some reason, those are rarely used
(and not implemented in the Linux ACPICA). However, it seems to me those
could be used in the same way as <function>-gpios in DT land.

While independent from the ACPI Device Properties work we have been
working on, I wonder if we could address this in some of the
documentation we're preparing as well - as it is intended to document
best practice, and the properties need to interact with the already
defined resource types, such as GPIO.

Rafael - what do you think?

As for parameterization like the function name ("foo" or "bar"), or
which "index", that is what the DT and ACPI Device Properties are meant
to abstract, so I don't see this as a problem. Rather than if/else
blocks in the code as you've described above, those names and indexes
should be read from the device properties.

> 
> So there are, with the simple example of GPIO, already
> a multitude of ways of shooting oneself in the foot, defining
> bindings for the same hardware in incompatible ways and
> generally screwing up.
> 
> And this almost already happened for RFkill but luckily
> eventually we stayed clear of some of it by managing
> to DEFINE that the RESET GPIO comes at index 0 and
> SHUTDOWN GPIO comes at index 1, in BOTH
> representations UNLESS they are named, and in that
> case the name takes precedence and this file:
> net/rfkill/rfkill-gpio.c
> 
> Is actually a good example of how things should look.
> 
> Looking at that file, do we all think this looks good?

Thanks, concrete examples are usually helpful.

It is a good example I think. The only bit I don't like is that
acpi_find_gpio ignores con_id. I'm sure there is a lot of context here
as to why - probably mostly surrounding the fact that while ACPI *CAN*
do this, there hasn't been a standard way of doing it and therefor
nothing we could reliably abstract... I'd like to see if we can improve
that.

-- 
Darren Hart
Intel Open Source Technology Center