[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <2844522.H6KZbzGzjr@vostro.rjw.lan>
Date: Wed, 29 Oct 2014 23:11 +0100
From: "Rafael J. Wysocki" <rjw@...ysocki.net>
To: Mika Westerberg <mika.westerberg@...ux.intel.com>
Cc: Linus Walleij <linus.walleij@...aro.org>,
Alexandre Courbot <gnurou@...il.com>,
Heikki Krogerus <heikki.krogerus@...ux.intel.com>,
Mathias Nyman <mathias.nyman@...ux.intel.com>,
Ning Li <ning.li@...el.com>, Alan Cox <alan@...ux.intel.com>,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH 1/4] gpio / ACPI: Add knowledge about pin controllers to acpi_get_gpiod()
On Monday, October 27, 2014 10:08:29 AM Mika Westerberg wrote:
> The GPIO resources (GpioIo/GpioInt) used in ACPI contain a GPIO number
> which is relative to the hardware GPIO controller. Typically this number
> can be translated directly to Linux GPIO number because the mapping is
> pretty much 1:1.
>
> However, when the GPIO driver is using pins exported by a pin controller
> driver via set of GPIO ranges, the mapping might not be 1:1 anymore and
> direct translation does not work.
>
> In such cases we need to translate the ACPI GPIO number to be suitable for
> the GPIO controller driver in question by checking all the pin controller
> GPIO ranges under the given device and using those to get the proper GPIO
> number.
>
> Signed-off-by: Mika Westerberg <mika.westerberg@...ux.intel.com>
> ---
> Rafael, are you OK with this change?
Yes, I am, so
Acked-by: Rafael J. Wysocki <rafael.j.wysocki@...el.com>
And I have no idea how to do that in a more straightforward way.
Of course ->
>
> drivers/gpio/gpiolib-acpi.c | 62 ++++++++++++++++++++++++++++++++++++++++++---
> 1 file changed, 59 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpio/gpiolib-acpi.c b/drivers/gpio/gpiolib-acpi.c
> index 05c6275da224..4f2c4adccb8f 100644
> --- a/drivers/gpio/gpiolib-acpi.c
> +++ b/drivers/gpio/gpiolib-acpi.c
> @@ -11,12 +11,14 @@
> */
>
> #include <linux/errno.h>
> +#include <linux/gpio.h>
> #include <linux/gpio/consumer.h>
> #include <linux/gpio/driver.h>
> #include <linux/export.h>
> #include <linux/acpi.h>
> #include <linux/interrupt.h>
> #include <linux/mutex.h>
> +#include <linux/pinctrl/pinctrl.h>
>
> #include "gpiolib.h"
>
> @@ -55,6 +57,58 @@ static int acpi_gpiochip_find(struct gpio_chip *gc, void *data)
> return ACPI_HANDLE(gc->dev) == data;
> }
>
> +#ifdef CONFIG_PINCTRL
> +/**
> + * acpi_gpiochip_pin_to_gpio_offset() - translates ACPI GPIO to Linux GPIO
> + * @chip: GPIO chip
> + * @pin: ACPI GPIO pin number from GpioIo/GpioInt resource
> + *
> + * Function takes ACPI GpioIo/GpioInt pin number as a parameter and
> + * translates it to a corresponding offset suitable to be passed to a
> + * GPIO controller driver.
> + *
> + * Typically the returned offset is same as @pin, but if the GPIO
> + * controller uses pin controller and the mapping is not contigous the
> + * offset might be different.
> + */
> +static int acpi_gpiochip_pin_to_gpio_offset(struct gpio_chip *chip, int pin)
> +{
> + struct gpio_pin_range *pin_range;
> +
> + /* If there are no ranges in this chip, use 1:1 mapping */
> + if (list_empty(&chip->pin_ranges))
> + return pin;
> +
> + list_for_each_entry(pin_range, &chip->pin_ranges, node) {
> + const struct pinctrl_gpio_range *range = &pin_range->range;
> + int i;
> +
> + if (range->pins) {
> + for (i = 0; i < range->npins; i++) {
> + if (range->pins[i] == pin)
> + return range->base + i - chip->base;
> + }
> + } else {
> + if (pin >= range->pin_base &&
> + pin < range->pin_base + range->npins) {
> + unsigned gpio_base;
> +
> + gpio_base = range->base - chip->base;
> + return gpio_base + pin - range->pin_base;
> + }
> + }
-> this is only going to work if the pin mapping in chip->pin_ranges doesn't
change after it has returned the offset, but I'm assiming that this is the case.
> + }
> +
> + return -EINVAL;
> +}
> +#else
> +static inline int acpi_gpiochip_pin_to_gpio_offset(struct gpio_chip *chip,
> + int pin)
> +{
> + return pin;
> +}
> +#endif
> +
> /**
> * acpi_get_gpiod() - Translate ACPI GPIO pin to GPIO descriptor usable with GPIO API
> * @path: ACPI GPIO controller full path name, (e.g. "\\_SB.GPO1")
> @@ -69,6 +123,7 @@ static struct gpio_desc *acpi_get_gpiod(char *path, int pin)
> struct gpio_chip *chip;
> acpi_handle handle;
> acpi_status status;
> + int offset;
>
> status = acpi_get_handle(NULL, path, &handle);
> if (ACPI_FAILURE(status))
> @@ -78,10 +133,11 @@ static struct gpio_desc *acpi_get_gpiod(char *path, int pin)
> if (!chip)
> return ERR_PTR(-ENODEV);
>
> - if (pin < 0 || pin > chip->ngpio)
> - return ERR_PTR(-EINVAL);
> + offset = acpi_gpiochip_pin_to_gpio_offset(chip, pin);
> + if (offset < 0)
> + return ERR_PTR(offset);
>
> - return gpiochip_get_desc(chip, pin);
> + return gpiochip_get_desc(chip, (u16)offset);
Silly question: Why do we need the explicit u16 cast now?
> }
>
> static irqreturn_t acpi_gpio_irq_handler(int irq, void *data)
>
--
I speak only for myself.
Rafael J. Wysocki, 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/
Powered by blists - more mailing lists