diff mbox series

[v7,13/18] gpiolib: acpi: Set initial value for output pin based on bias and polarity

Message ID 20201111222008.39993-14-andriy.shevchenko@linux.intel.com
State New
Headers show
Series gpiolib: acpi: pin configuration fixes | expand

Commit Message

Andy Shevchenko Nov. 11, 2020, 10:20 p.m. UTC 
From: Vasile-Laurentiu Stanimir <vasile-laurentiu.stanimir@windriver.com>

GpioIo() resources don't contain an initial value for the output pin.
Therefore instead of deducting its value solely based on bias field
we should deduce that value from the polarity and the bias fields.
Typical scenario is, when pin is defined in the table and its polarity,
specified in _DSD or via platform code, is defined as active low,
in the following call chain:

  -> acpi_populate_gpio_lookup()
     -> acpi_gpio_to_gpiod_flags()

it will return GPIOD_OUT_HIGH if bias is set no matter if polarity
is GPIO_ACTIVE_LOW, so it will return the current level instead of
the logical level.

Cc: Hans de Goede <hdegoede@redhat.com>
Cc: Mika Westerberg <mika.westerberg@linux.intel.com>
Signed-off-by: Vasile-Laurentiu Stanimir <vasile-laurentiu.stanimir@windriver.com>
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/gpio/gpiolib-acpi.c | 21 ++++++++++++---------
 1 file changed, 12 insertions(+), 9 deletions(-)

Comments

Mika Westerberg Nov. 12, 2020, 9:10 a.m. UTC | #1 
On Thu, Nov 12, 2020 at 12:20:03AM +0200, Andy Shevchenko wrote:
> From: Vasile-Laurentiu Stanimir <vasile-laurentiu.stanimir@windriver.com>
> 
> GpioIo() resources don't contain an initial value for the output pin.
> Therefore instead of deducting its value solely based on bias field
> we should deduce that value from the polarity and the bias fields.
> Typical scenario is, when pin is defined in the table and its polarity,
> specified in _DSD or via platform code, is defined as active low,
> in the following call chain:
> 
>   -> acpi_populate_gpio_lookup()
>      -> acpi_gpio_to_gpiod_flags()
> 
> it will return GPIOD_OUT_HIGH if bias is set no matter if polarity
> is GPIO_ACTIVE_LOW, so it will return the current level instead of
> the logical level.
> 
> Cc: Hans de Goede <hdegoede@redhat.com>
> Cc: Mika Westerberg <mika.westerberg@linux.intel.com>

Reviewed-by: Mika Westerberg <mika.westerberg@linux.intel.com>
diff mbox series

Patch

diff --git a/drivers/gpio/gpiolib-acpi.c b/drivers/gpio/gpiolib-acpi.c
index ac1bde0720f2..b47d5e8edaeb 100644
--- a/drivers/gpio/gpiolib-acpi.c
+++ b/drivers/gpio/gpiolib-acpi.c
@@ -206,7 +206,7 @@  static void acpi_gpiochip_request_irqs(struct acpi_gpio_chip *acpi_gpio)
 }
 
 static enum gpiod_flags
-acpi_gpio_to_gpiod_flags(const struct acpi_resource_gpio *agpio)
+acpi_gpio_to_gpiod_flags(const struct acpi_resource_gpio *agpio, int polarity)
 {
 	switch (agpio->io_restriction) {
 	case ACPI_IO_RESTRICT_INPUT:
@@ -215,15 +215,17 @@  acpi_gpio_to_gpiod_flags(const struct acpi_resource_gpio *agpio)
 		/*
 		 * ACPI GPIO resources don't contain an initial value for the
 		 * GPIO. Therefore we deduce that value from the pull field
-		 * instead. If the pin is pulled up we assume default to be
-		 * high, if it is pulled down we assume default to be low,
-		 * otherwise we leave pin untouched.
+		 * and the polarity instead. If the pin is pulled up we assume
+		 * default to be high, if it is pulled down we assume default
+		 * to be low, otherwise we leave pin untouched. For active low
+		 * polarity values will be switched. See also
+		 * Documentation/firmware-guide/acpi/gpio-properties.rst.
 		 */
 		switch (agpio->pin_config) {
 		case ACPI_PIN_CONFIG_PULLUP:
-			return GPIOD_OUT_HIGH;
+			return polarity == GPIO_ACTIVE_LOW ? GPIOD_OUT_LOW : GPIOD_OUT_HIGH;
 		case ACPI_PIN_CONFIG_PULLDOWN:
-			return GPIOD_OUT_LOW;
+			return polarity == GPIO_ACTIVE_LOW ? GPIOD_OUT_HIGH : GPIOD_OUT_LOW;
 		default:
 			break;
 		}
@@ -683,8 +685,8 @@  static int acpi_populate_gpio_lookup(struct acpi_resource *ares, void *data)
 			lookup->info.polarity = agpio->polarity;
 			lookup->info.triggering = agpio->triggering;
 		} else {
-			lookup->info.flags = acpi_gpio_to_gpiod_flags(agpio);
 			lookup->info.polarity = lookup->active_low;
+			lookup->info.flags = acpi_gpio_to_gpiod_flags(agpio, lookup->info.polarity);
 		}
 	}
 
@@ -1055,12 +1057,13 @@  acpi_gpio_adr_space_handler(u32 function, acpi_physical_address address,
 		}
 
 		if (!found) {
-			enum gpiod_flags flags = acpi_gpio_to_gpiod_flags(agpio);
+			int polarity = GPIO_ACTIVE_HIGH;
+			enum gpiod_flags flags = acpi_gpio_to_gpiod_flags(agpio, polarity);
 			const char *label = "ACPI:OpRegion";
 			int ret;
 
 			desc = gpiochip_request_own_desc(chip, pin, label,
-							 GPIO_ACTIVE_HIGH,
+							 polarity,
 							 flags);
 			if (IS_ERR(desc)) {
 				mutex_unlock(&achip->conn_lock);