From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752883AbaBYOkI (ORCPT ); Tue, 25 Feb 2014 09:40:08 -0500 Received: from v094114.home.net.pl ([79.96.170.134]:57167 "HELO v094114.home.net.pl" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with SMTP id S1752444AbaBYOkF (ORCPT ); Tue, 25 Feb 2014 09:40:05 -0500 From: "Rafael J. Wysocki" To: Mika Westerberg Cc: Linus Walleij , Alexandre Courbot , Lan Tianyu , Lv Zheng , Alan Cox , Mathias Nyman , linux-acpi@vger.kernel.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH 6/6] gpio / ACPI: Add support for ACPI GPIO operation regions Date: Tue, 25 Feb 2014 15:55:02 +0100 Message-ID: <2863081.yf9Ke2teV0@vostro.rjw.lan> User-Agent: KMail/4.11.5 (Linux/3.13.0+; KDE/4.11.5; x86_64; ; ) In-Reply-To: <1393257611-18031-7-git-send-email-mika.westerberg@linux.intel.com> References: <1393257611-18031-1-git-send-email-mika.westerberg@linux.intel.com> <1393257611-18031-7-git-send-email-mika.westerberg@linux.intel.com> MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="utf-8" Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Monday, February 24, 2014 06:00:11 PM Mika Westerberg wrote: > GPIO operation regions is a new feature introduced in ACPI 5.0 > specification. This feature adds a way for platform ASL code to call back > to OS GPIO driver and toggle GPIO pins. > > An example ASL code from Lenovo Miix 2 tablet with only relevant part > listed: > > Device (\_SB.GPO0) > { > Name (AVBL, Zero) > Method (_REG, 2, NotSerialized) > { > If (LEqual (Arg0, 0x08)) > { > // Marks the region available > Store (Arg1, AVBL) > } > } > > OperationRegion (GPOP, GeneralPurposeIo, Zero, 0x0C) > Field (GPOP, ByteAcc, NoLock, Preserve) > { > Connection ( > GpioIo (Exclusive, PullDefault, 0, 0, IoRestrictionOutputOnly, > "\\_SB.GPO0", 0x00, ResourceConsumer,,) > { > 0x003B > } > ), > SHD3, 1, > } > } > > Device (SHUB) > { > Method (_PS0, 0, Serialized) > { > If (LEqual (\_SB.GPO0.AVBL, One)) > { > Store (One, \_SB.GPO0.SHD3) > Sleep (0x32) > } > } > Method (_PS3, 0, Serialized) > { > If (LEqual (\_SB.GPO0.AVBL, One)) > { > Store (Zero, \_SB.GPO0.SHD3) > } > } > } > > The sensor hub (SHUB) device uses GPIO connection SHD3 to power the device > whenever the GPIO operation region is available. I would add more explanation of the ASL above here. Basically, how it is supposed to work and what's the handler's role in it. > Implement the support by registering GPIO operation region handlers for all > GPIO devices that have an ACPI handle. First time the GPIO is used by the > ASL code we make sure that the GPIO stays requested until the GPIO chip > driver itself is unloaded. If we find out that the GPIO is already > requested we just toggle it according to the value got from ASL code. The patch itself looks good to me. > Signed-off-by: Mika Westerberg > --- > drivers/gpio/gpiolib-acpi.c | 156 ++++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 156 insertions(+) > > diff --git a/drivers/gpio/gpiolib-acpi.c b/drivers/gpio/gpiolib-acpi.c > index 275735f390af..0133d38c447f 100644 > --- a/drivers/gpio/gpiolib-acpi.c > +++ b/drivers/gpio/gpiolib-acpi.c > @@ -16,6 +16,7 @@ > #include > #include > #include > +#include > > #include "gpiolib.h" > > @@ -26,7 +27,20 @@ struct acpi_gpio_event { > unsigned int irq; > }; > > +struct acpi_gpio_connection { > + struct list_head node; > + struct gpio_desc *desc; > +}; > + > struct acpi_gpio_chip { > + /* > + * ACPICA requires that the first field of the context parameter > + * passed to acpi_install_address_space_handler() is large enough > + * to hold struct acpi_connection_info. > + */ > + struct acpi_connection_info conn_info; > + struct list_head conns; > + struct mutex conn_lock; > struct gpio_chip *chip; > struct list_head events; > }; > @@ -334,6 +348,146 @@ struct gpio_desc *acpi_get_gpiod_by_index(struct device *dev, int index, > return lookup.desc ? lookup.desc : ERR_PTR(-ENOENT); > } > > +static acpi_status > +acpi_gpio_adr_space_handler(u32 function, acpi_physical_address address, > + u32 bits, u64 *value, void *handler_context, > + void *region_context) > +{ > + struct acpi_gpio_chip *achip = region_context; > + struct gpio_chip *chip = achip->chip; > + struct acpi_resource_gpio *agpio; > + struct acpi_resource *ares; > + acpi_status status; > + bool pull; > + int i; > + > + status = acpi_buffer_to_resource(achip->conn_info.connection, > + achip->conn_info.length, &ares); > + if (ACPI_FAILURE(status)) > + return status; > + > + if (WARN_ON(ares->type != ACPI_RESOURCE_TYPE_GPIO)) { > + ACPI_FREE(ares); > + return AE_BAD_PARAMETER; > + } > + > + agpio = &ares->data.gpio; > + pull = agpio->pin_config == ACPI_PIN_CONFIG_PULLUP; > + > + if (WARN_ON(agpio->io_restriction == ACPI_IO_RESTRICT_INPUT && > + function == ACPI_WRITE)) { > + ACPI_FREE(ares); > + return AE_BAD_PARAMETER; > + } > + > + for (i = 0; i < agpio->pin_table_length; i++) { > + unsigned pin = agpio->pin_table[i]; > + struct acpi_gpio_connection *conn; > + struct gpio_desc *desc; > + bool found; > + > + desc = gpiochip_get_desc(chip, pin); > + if (IS_ERR(desc)) { > + status = AE_ERROR; > + goto out; > + } > + > + mutex_lock(&achip->conn_lock); > + > + found = false; > + list_for_each_entry(conn, &achip->conns, node) { > + if (conn->desc == desc) { > + found = true; > + break; > + } > + } > + if (!found) { > + int ret; > + > + ret = gpiochip_request_own_desc(desc, "ACPI:OpRegion"); > + if (ret) { > + status = AE_ERROR; > + mutex_unlock(&achip->conn_lock); > + goto out; > + } > + > + switch (agpio->io_restriction) { > + case ACPI_IO_RESTRICT_INPUT: > + gpiod_direction_input(desc); > + break; > + case ACPI_IO_RESTRICT_OUTPUT: > + gpiod_direction_output(desc, pull); > + break; > + default: > + /* > + * Assume that the BIOS has configured the > + * direction and pull accordingly. > + */ > + break; > + } > + > + conn = kzalloc(sizeof(*conn), GFP_KERNEL); > + if (!conn) { > + status = AE_NO_MEMORY; > + mutex_unlock(&achip->conn_lock); > + goto out; > + } > + > + conn->desc = desc; > + > + list_add_tail(&conn->node, &achip->conns); > + } > + > + mutex_unlock(&achip->conn_lock); > + > + > + if (function == ACPI_WRITE) > + gpiod_set_raw_value(desc, !!((1 << i) & *value)); > + else > + *value |= gpiod_get_raw_value(desc) << i; > + } > + > +out: > + ACPI_FREE(ares); > + return status; > +} > + > +static void acpi_gpiochip_request_regions(struct acpi_gpio_chip *achip) > +{ > + struct gpio_chip *chip = achip->chip; > + acpi_handle handle = ACPI_HANDLE(chip->dev); > + acpi_status status; > + > + INIT_LIST_HEAD(&achip->conns); > + mutex_init(&achip->conn_lock); > + status = acpi_install_address_space_handler(handle, ACPI_ADR_SPACE_GPIO, > + acpi_gpio_adr_space_handler, > + NULL, achip); > + if (ACPI_FAILURE(status)) > + dev_err(chip->dev, "Failed to install GPIO OpRegion handler\n"); > +} > + > +static void acpi_gpiochip_free_regions(struct acpi_gpio_chip *achip) > +{ > + struct gpio_chip *chip = achip->chip; > + acpi_handle handle = ACPI_HANDLE(chip->dev); > + struct acpi_gpio_connection *conn, *tmp; > + acpi_status status; > + > + status = acpi_remove_address_space_handler(handle, ACPI_ADR_SPACE_GPIO, > + acpi_gpio_adr_space_handler); > + if (ACPI_FAILURE(status)) { > + dev_err(chip->dev, "Failed to remove GPIO OpRegion handler\n"); > + return; > + } > + > + list_for_each_entry_safe_reverse(conn, tmp, &achip->conns, node) { > + gpiochip_free_own_desc(conn->desc); > + list_del(&conn->node); > + kfree(conn); > + } > +} > + > void acpi_gpiochip_add(struct gpio_chip *chip) > { > struct acpi_gpio_chip *achip; > @@ -361,6 +515,7 @@ void acpi_gpiochip_add(struct gpio_chip *chip) > } > > acpi_gpiochip_request_interrupts(achip); > + acpi_gpiochip_request_regions(achip); > } > > void acpi_gpiochip_remove(struct gpio_chip *chip) > @@ -379,6 +534,7 @@ void acpi_gpiochip_remove(struct gpio_chip *chip) > return; > } > > + acpi_gpiochip_free_regions(achip); > acpi_gpiochip_free_interrupts(achip); > > acpi_detach_data(handle, acpi_gpio_chip_dh); > -- I speak only for myself. Rafael J. Wysocki, Intel Open Source Technology Center.