Re: [PATCH 6/6] gpio / ACPI: Add support for ACPI GPIO operation regions

2014-02-26 Thread Mika Westerberg
On Tue, Feb 25, 2014 at 03:55:02PM +0100, Rafael J. Wysocki wrote:
> 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.

OK, I will do that in the next revision.

> > 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.

Thanks for reviewing this :)
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 6/6] gpio / ACPI: Add support for ACPI GPIO operation regions

2014-02-26 Thread Mika Westerberg
On Tue, Feb 25, 2014 at 03:55:02PM +0100, Rafael J. Wysocki wrote:
 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.

OK, I will do that in the next revision.

  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.

Thanks for reviewing this :)
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 6/6] gpio / ACPI: Add support for ACPI GPIO operation regions

2014-02-25 Thread Rafael J. Wysocki
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, );
> + if (ACPI_FAILURE(status))
> + return status;
> +
> + if (WARN_ON(ares->type != ACPI_RESOURCE_TYPE_GPIO)) {
> + ACPI_FREE(ares);
> + return AE_BAD_PARAMETER;
> + }
> +
> + agpio = >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(>conn_lock);
> +
> + found = false;
> + list_for_each_entry(conn, >conns, node) {
> + if (conn->desc == 

Re: [PATCH 6/6] gpio / ACPI: Add support for ACPI GPIO operation regions

2014-02-25 Thread Rafael J. Wysocki
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 mika.westerb...@linux.intel.com
 ---
  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 linux/export.h
  #include linux/acpi.h
  #include linux/interrupt.h
 +#include linux/mutex.h
  
  #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;
 +