[lkml]   [2012]   [Nov]   [9]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
Messages in this thread
SubjectRe: [PATCH 1/3] gpio / ACPI: add ACPI support
On 11/08/2012 09:38 PM, Mika Westerberg wrote:
>>> +#include<linux/errno.h>
>>> +#include<linux/gpio.h>
>>> +#include<linux/module.h>
>>> +#include<linux/acpi_gpio.h>
>>> +#include<linux/acpi.h>
>>> +
>>> +static int acpi_gpiochip_find(struct gpio_chip *gc, void *data)
>>> +{
>>> + acpi_handle handle = data;
>>> + acpi_handle gc_handle;
>>> +
>>> + if (!gc->dev)
>>> + return false;
>>> +
>>> + gc_handle = gc->dev->acpi_handle;
>>> + if (!gc_handle)
>>> + return false;
>> This test is redundant with the next one... unless 'handle' is also NULL :-)
>> Is it at all possible for multiple gpiochips to be used for a single
>> ACPI gpio controller node? Such as if the gpio controller has multiple
>> banks that should be controlled separately? If so then this won't be
>> sufficient. I've got the same issue with DT support where the find
>> function needs to also check if the pin is provided by that specific
>> gpiochip.
> AFAIK no but I'll let Mathias to answer that as he knows this better.

I'm interpreting it the same way as Mika, max one actual controller per
ACPI device node

The path (called ResourceSource in ACPI5 specs) in GpioIO/GpioInt
resources is a "string which uniquely identifies the GPIO controller
referred to by this descriptor." The pin number is zero based
controller relative.

The ACPI device controller node includes all other resources needed by
the controller driver (ioport/mem base, range, interrupt, and Hardware
ID used to pair with a driver)

Checked a board with two identical gpio controllers on it and it had two
separate ACPI device node entries. (with only different io address base
and interrupt resources)


>> Overall the patch looks good, but I need to see how it is used. It
>> would be really nice if device drivers could use basically the same
>> interface to obtain Linux gpio numbers regardless of if the backing
>> data was ACPI or DT. This API is one level below that.
> Yeah, this patch just mimics the DT version but in general it would be
> better if there was only one API to get the GPIO. There has been discussion
> about adding gpio_get() or something similar which could perhaps be used to
> abstract away DT or ACPI.
> We use this in a driver so that we walk through the ACPI resources for a
> given device (if we have the ACPI handle) and parse the GpioIO/GpioInt
> resources like:
> struct acpi_resource_gpio *acpi_gpio;
> struct acpi_device *adev;
> acpi_resource *res;
> int gpio;
> /* obtain the ACPI device from handle */
> ...
> /* walk through the resources attached to adev */
> ...
> switch (res->type) {
> acpi_gpio =&res->data.gpio;
> gpio = acpi_get_gpio(acpi_gpio->resource_source.string_ptr,
> acpi_gpio->pin_table[0]);
> ...
> }

 \ /
  Last update: 2012-11-09 15:41    [W:0.214 / U:0.096 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site