[PATCH v2 00/12] gpio: acpi: Make it working

[PATCH v2 00/12] gpio: acpi: Make it working

[Andy Shevchenko]

This is second iteration of the fix of GPIO ACPI library to make it
robust against different ACPI specification generations and thus
firmwares.

Currently GPIO ACPI library provides an API to get a GPIO resources
(IO or interrupt) from ACPI tables for the individual drivers.

This library has few flaws which makes some devices not working:
- the library allows to abuse ACPI by using a _CRS fallback mechanism
- the library neglects flags of the resource
- the PNP ACPI is not ready for GpioInt() resources for interrupts

In this series:
- the _CRS fallback is forbidden
- the pin configuration follows what firmware wants to
- the documentation is updated in order to clarify corner cases
- the PNP ACPI is extended to support GpioInt() for interrupts

After this series it's possible to use GPIO pins for input (interrupt)
which were set as output due to HW reset default is input buffer
disabled and BIOS didn't touch that pin at all. It's a crucial
functionality for Internet of Things (IoT) open connected boards where
user may choose any of available pin for almost any of available
function, including GPIO input (interrupt).

Current bad behaviour was first reported by Jarkko Nikula few months ago.

Jagadish, it would be nice if you can re-test entire series in your
environment and give a Tested-by tag.

In v2:
- add 4 patches to fix an issue with device that takes GpioInt resource
- amend documentation
- fix several spelling typos
- add Mika's and Jarkko's tags
- rebase on top of recent linux-next

Andy Shevchenko (11):
  gpiolib: Export gpiod_configure_flags() to internal users
  gpio: acpi: Align acpi_find_gpio() with DT version
  gpio: acpi: Do sanity check for GpioInt in acpi_find_gpio()
  gpio: acpi: Even more tighten up ACPI GPIO lookups
  gpio: acpi: Synchronize acpi_find_gpio() and acpi_gpio_count()
  gpio: acpi: Explain how to get GPIO descriptors in ACPI case
  gpio: acpi: Factor out acpi_gpio_to_gpiod_flags() helper
  gpio: acpi: Override GPIO initialization flags
  gpio: acpi: Split out acpi_gpio_get_irq_resource() helper
  PNP / ACPI: join strings back for better maintenance
  PNP / ACPI: remove FSF address

Jagadish Krishnamoorthy (1):
  PNP / ACPI: add support for GpioInt resource type

 Documentation/acpi/gpio-properties.txt |  65 ++++++++++++
 drivers/gpio/gpiolib-acpi.c            | 185 ++++++++++++++++++++-------------
 drivers/gpio/gpiolib.c                 |  10 +-
 drivers/gpio/gpiolib.h                 |  17 ++-
 drivers/pnp/pnpacpi/rsparser.c         |  37 ++++---
 include/linux/acpi.h                   |   7 ++
 6 files changed, 230 insertions(+), 91 deletions(-)

-- 
2.11.0

[PATCH v2 01/12] gpiolib: Export gpiod_configure_flags() to internal users

[Andy Shevchenko]

This is preparatory patch for enabling GPIO ACPI to configure a pin
accordingly.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Tested-by: Jarkko Nikula <jarkko.nikula@linux.intel.com>
Reviewed-by: Mika Westerberg <mika.westerberg@linux.intel.com>
---
 drivers/gpio/gpiolib.c | 2 +-
 drivers/gpio/gpiolib.h | 2 ++
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index 5db44139cef8..99e07a2b7e02 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -3212,7 +3212,7 @@ EXPORT_SYMBOL_GPL(gpiod_get_optional);
  * requested function and/or index, or another IS_ERR() code if an error
  * occurred while trying to acquire the GPIO.
  */
-static int gpiod_configure_flags(struct gpio_desc *desc, const char *con_id,
+int gpiod_configure_flags(struct gpio_desc *desc, const char *con_id,
 		unsigned long lflags, enum gpiod_flags dflags)
 {
 	int status;
diff --git a/drivers/gpio/gpiolib.h b/drivers/gpio/gpiolib.h
index 2495b7ee1b42..e36a0bdc7740 100644
--- a/drivers/gpio/gpiolib.h
+++ b/drivers/gpio/gpiolib.h
@@ -199,6 +199,8 @@ struct gpio_desc {
 
 int gpiod_request(struct gpio_desc *desc, const char *label);
 void gpiod_free(struct gpio_desc *desc);
+int gpiod_configure_flags(struct gpio_desc *desc, const char *con_id,
+		unsigned long lflags, enum gpiod_flags dflags);
 int gpiod_hog(struct gpio_desc *desc, const char *name,
 		unsigned long lflags, enum gpiod_flags dflags);
 
-- 
2.11.0

Re: [PATCH v2 01/12] gpiolib: Export gpiod_configure_flags() to internal users

[Linus Walleij]

On Tue, May 23, 2017 at 7:03 PM, Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:

> This is preparatory patch for enabling GPIO ACPI to configure a pin
> accordingly.
>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Tested-by: Jarkko Nikula <jarkko.nikula@linux.intel.com>
> Reviewed-by: Mika Westerberg <mika.westerberg@linux.intel.com>

Patch applied.

Yours,
Linus Walleij

[PATCH v2 02/12] gpio: acpi: Align acpi_find_gpio() with DT version

[Andy Shevchenko]

By some reason acpi_find_gpio() and acpi_gpio_count() have compared
connection ID to "gpios" when tries to check if suffix is needed or not.

Don't do any assumptions about what connection ID can be and, when defined,
use it only with suffix as it's done in the device tree version.

Reviewed-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Tested-by: Jarkko Nikula <jarkko.nikula@linux.intel.com>
Reviewed-by: Mika Westerberg <mika.westerberg@linux.intel.com>
---
 drivers/gpio/gpiolib-acpi.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpio/gpiolib-acpi.c b/drivers/gpio/gpiolib-acpi.c
index 2185232da823..055a8a255a40 100644
--- a/drivers/gpio/gpiolib-acpi.c
+++ b/drivers/gpio/gpiolib-acpi.c
@@ -599,7 +599,7 @@ struct gpio_desc *acpi_find_gpio(struct device *dev,
 
 	/* Try first from _DSD */
 	for (i = 0; i < ARRAY_SIZE(gpio_suffixes); i++) {
-		if (con_id && strcmp(con_id, "gpios")) {
+		if (con_id) {
 			snprintf(propname, sizeof(propname), "%s-%s",
 				 con_id, gpio_suffixes[i]);
 		} else {
@@ -1089,7 +1089,7 @@ int acpi_gpio_count(struct device *dev, const char *con_id)
 
 	/* Try first from _DSD */
 	for (i = 0; i < ARRAY_SIZE(gpio_suffixes); i++) {
-		if (con_id && strcmp(con_id, "gpios"))
+		if (con_id)
 			snprintf(propname, sizeof(propname), "%s-%s",
 				 con_id, gpio_suffixes[i]);
 		else
-- 
2.11.0

Re: [PATCH v2 02/12] gpio: acpi: Align acpi_find_gpio() with DT version

[Linus Walleij]

On Tue, May 23, 2017 at 7:03 PM, Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:

> By some reason acpi_find_gpio() and acpi_gpio_count() have compared
> connection ID to "gpios" when tries to check if suffix is needed or not.
>
> Don't do any assumptions about what connection ID can be and, when defined,
> use it only with suffix as it's done in the device tree version.
>
> Reviewed-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Tested-by: Jarkko Nikula <jarkko.nikula@linux.intel.com>
> Reviewed-by: Mika Westerberg <mika.westerberg@linux.intel.com>

Patch applied.

Yours,
Linus Walleij

[PATCH v2 03/12] gpio: acpi: Do sanity check for GpioInt in acpi_find_gpio()

[Andy Shevchenko]

Check that we don't ask for output direction on GpioInt resource
in cases with or without _DSD defined.

Reviewed-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Tested-by: Jarkko Nikula <jarkko.nikula@linux.intel.com>
Reviewed-by: Mika Westerberg <mika.westerberg@linux.intel.com>
---
 drivers/gpio/gpiolib-acpi.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/gpio/gpiolib-acpi.c b/drivers/gpio/gpiolib-acpi.c
index 055a8a255a40..28f35a9de86b 100644
--- a/drivers/gpio/gpiolib-acpi.c
+++ b/drivers/gpio/gpiolib-acpi.c
@@ -622,12 +622,12 @@ struct gpio_desc *acpi_find_gpio(struct device *dev,
 		desc = acpi_get_gpiod_by_index(adev, NULL, idx, &info);
 		if (IS_ERR(desc))
 			return desc;
+	}
 
-		if ((flags == GPIOD_OUT_LOW || flags == GPIOD_OUT_HIGH) &&
-		    info.gpioint) {
-			dev_dbg(dev, "refusing GpioInt() entry when doing GPIOD_OUT_* lookup\n");
-			return ERR_PTR(-ENOENT);
-		}
+	if (info.gpioint &&
+	    (flags == GPIOD_OUT_LOW || flags == GPIOD_OUT_HIGH)) {
+		dev_dbg(dev, "refusing GpioInt() entry when doing GPIOD_OUT_* lookup\n");
+		return ERR_PTR(-ENOENT);
 	}
 
 	if (info.polarity == GPIO_ACTIVE_LOW)
-- 
2.11.0

Re: [PATCH v2 03/12] gpio: acpi: Do sanity check for GpioInt in acpi_find_gpio()

[Linus Walleij]

On Tue, May 23, 2017 at 7:03 PM, Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:

> Check that we don't ask for output direction on GpioInt resource
> in cases with or without _DSD defined.
>
> Reviewed-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Tested-by: Jarkko Nikula <jarkko.nikula@linux.intel.com>
> Reviewed-by: Mika Westerberg <mika.westerberg@linux.intel.com>

Patch applied.

Yours,
Linus Walleij

[PATCH v2 04/12] gpio: acpi: Even more tighten up ACPI GPIO lookups

[Andy Shevchenko]

The commit 10cf4899f8af ("gpiolib: tighten up ACPI legacy gpio lookups")
prevents to getting same resource twice if the driver asks twice using
different connection ID.

But the whole idea of fallback might bring some problems. Imagine the case when
we have two versions of BIOS/hardware where in one _DSD is introduced along
with GPIO resources, but the other one uses just plain GPIO resource for
another purpose

Case 1:

    Device (DEVX)
    {
        ...
        Name (_CRS, ResourceTemplate ()
        {
            GpioIo (Exclusive, PullUp, 0, 0, IoRestrictionInputOnly,
                    "\\_SB.GPO0", 0, ResourceConsumer) {15}
        })
        Name (_DSD, Package ()
        {
            ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
            Package ()
            {
                Package () {"some-gpios", Package() {^DEVX, 0, 0, 0 }},
            }
        })
    }

Case 2:

    Device (DEVX)
    {
        ...
        Name (_CRS, ResourceTemplate ()
        {
            GpioIo (Exclusive, PullUp, 0, 0, IoRestrictionInputOnly,
                    "\\_SB.GPO0", 0, ResourceConsumer) {27}
        })
    }

To prevent the possible misconfiguration tighten up even more GPIO ACPI lookups
for case without connection ID provided.

In the past the issue had been triggered by "use mctrl_gpio helpers" series
[1,2].

[1] commit 4ef03d328769 ("tty/serial/8250: use mctrl_gpio helpers")
[2] https://patchwork.kernel.org/patch/9283745/

Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Cc: Bastien Nocera <hadess@hadess.net>
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Tested-by: Jarkko Nikula <jarkko.nikula@linux.intel.com>
Reviewed-by: Mika Westerberg <mika.westerberg@linux.intel.com>
---
 drivers/gpio/gpiolib-acpi.c | 36 +-----------------------------------
 1 file changed, 1 insertion(+), 35 deletions(-)

diff --git a/drivers/gpio/gpiolib-acpi.c b/drivers/gpio/gpiolib-acpi.c
index 28f35a9de86b..0392d8ed332f 100644
--- a/drivers/gpio/gpiolib-acpi.c
+++ b/drivers/gpio/gpiolib-acpi.c
@@ -1129,45 +1129,11 @@ int acpi_gpio_count(struct device *dev, const char *con_id)
 	return count ? count : -ENOENT;
 }
 
-struct acpi_crs_lookup {
-	struct list_head node;
-	struct acpi_device *adev;
-	const char *con_id;
-};
-
-static DEFINE_MUTEX(acpi_crs_lookup_lock);
-static LIST_HEAD(acpi_crs_lookup_list);
-
 bool acpi_can_fallback_to_crs(struct acpi_device *adev, const char *con_id)
 {
-	struct acpi_crs_lookup *l, *lookup = NULL;
-
 	/* Never allow fallback if the device has properties */
 	if (adev->data.properties || adev->driver_gpios)
 		return false;
 
-	mutex_lock(&acpi_crs_lookup_lock);
-
-	list_for_each_entry(l, &acpi_crs_lookup_list, node) {
-		if (l->adev == adev) {
-			lookup = l;
-			break;
-		}
-	}
-
-	if (!lookup) {
-		lookup = kmalloc(sizeof(*lookup), GFP_KERNEL);
-		if (lookup) {
-			lookup->adev = adev;
-			lookup->con_id = kstrdup(con_id, GFP_KERNEL);
-			list_add_tail(&lookup->node, &acpi_crs_lookup_list);
-		}
-	}
-
-	mutex_unlock(&acpi_crs_lookup_lock);
-
-	return lookup &&
-		((!lookup->con_id && !con_id) ||
-		 (lookup->con_id && con_id &&
-		  strcmp(lookup->con_id, con_id) == 0));
+	return con_id == NULL;
 }
-- 
2.11.0

Re: [PATCH v2 04/12] gpio: acpi: Even more tighten up ACPI GPIO lookups

[Linus Walleij]

On Tue, May 23, 2017 at 7:03 PM, Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:

> The commit 10cf4899f8af ("gpiolib: tighten up ACPI legacy gpio lookups")
> prevents to getting same resource twice if the driver asks twice using
> different connection ID.
>
> But the whole idea of fallback might bring some problems. Imagine the case when
> we have two versions of BIOS/hardware where in one _DSD is introduced along
> with GPIO resources, but the other one uses just plain GPIO resource for
> another purpose
>
> Case 1:
>
>     Device (DEVX)
>     {
>         ...
>         Name (_CRS, ResourceTemplate ()
>         {
>             GpioIo (Exclusive, PullUp, 0, 0, IoRestrictionInputOnly,
>                     "\\_SB.GPO0", 0, ResourceConsumer) {15}
>         })
>         Name (_DSD, Package ()
>         {
>             ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
>             Package ()
>             {
>                 Package () {"some-gpios", Package() {^DEVX, 0, 0, 0 }},
>             }
>         })
>     }
>
> Case 2:
>
>     Device (DEVX)
>     {
>         ...
>         Name (_CRS, ResourceTemplate ()
>         {
>             GpioIo (Exclusive, PullUp, 0, 0, IoRestrictionInputOnly,
>                     "\\_SB.GPO0", 0, ResourceConsumer) {27}
>         })
>     }
>
> To prevent the possible misconfiguration tighten up even more GPIO ACPI lookups
> for case without connection ID provided.
>
> In the past the issue had been triggered by "use mctrl_gpio helpers" series
> [1,2].
>
> [1] commit 4ef03d328769 ("tty/serial/8250: use mctrl_gpio helpers")
> [2] https://patchwork.kernel.org/patch/9283745/
>
> Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> Cc: Bastien Nocera <hadess@hadess.net>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Tested-by: Jarkko Nikula <jarkko.nikula@linux.intel.com>
> Reviewed-by: Mika Westerberg <mika.westerberg@linux.intel.com>

Patch applied.

Yours,
Linus Walleij

[PATCH v2 05/12] gpio: acpi: Synchronize acpi_find_gpio() and acpi_gpio_count()

[Andy Shevchenko]

If we pass connection ID to the both functions and at the same time
acpi_can_fallback_to_crs() returns false we will get different results,
i.e. the number of GPIO resources returned by acpi_gpio_count() might be
not correct.

Fix this by calling acpi_can_fallback_to_crs() in acpi_gpio_count()
before trying to fallback.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Tested-by: Jarkko Nikula <jarkko.nikula@linux.intel.com>
Reviewed-by: Mika Westerberg <mika.westerberg@linux.intel.com>
---
 drivers/gpio/gpiolib-acpi.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/gpio/gpiolib-acpi.c b/drivers/gpio/gpiolib-acpi.c
index 0392d8ed332f..740df0e9dcb3 100644
--- a/drivers/gpio/gpiolib-acpi.c
+++ b/drivers/gpio/gpiolib-acpi.c
@@ -1119,6 +1119,9 @@ int acpi_gpio_count(struct device *dev, const char *con_id)
 		struct list_head resource_list;
 		unsigned int crs_count = 0;
 
+		if (!acpi_can_fallback_to_crs(adev, con_id))
+			return count;
+
 		INIT_LIST_HEAD(&resource_list);
 		acpi_dev_get_resources(adev, &resource_list,
 				       acpi_find_gpio_count, &crs_count);
-- 
2.11.0

Re: [PATCH v2 05/12] gpio: acpi: Synchronize acpi_find_gpio() and acpi_gpio_count()

[Linus Walleij]

On Tue, May 23, 2017 at 7:03 PM, Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:

> If we pass connection ID to the both functions and at the same time
> acpi_can_fallback_to_crs() returns false we will get different results,
> i.e. the number of GPIO resources returned by acpi_gpio_count() might be
> not correct.
>
> Fix this by calling acpi_can_fallback_to_crs() in acpi_gpio_count()
> before trying to fallback.
>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Tested-by: Jarkko Nikula <jarkko.nikula@linux.intel.com>
> Reviewed-by: Mika Westerberg <mika.westerberg@linux.intel.com>

Patch applied.

Yours,
Linus Walleij

[PATCH v2 06/12] gpio: acpi: Explain how to get GPIO descriptors in ACPI case

[Andy Shevchenko]

Documentation lacks of explanation how we actually use device properties
for GPIO resources.

Add a section to the documentation about that.

Suggested-by: Mika Westerberg <mika.westerberg@linux.intel.com>
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Tested-by: Jarkko Nikula <jarkko.nikula@linux.intel.com>
Reviewed-by: Mika Westerberg <mika.westerberg@linux.intel.com>
---
 Documentation/acpi/gpio-properties.txt | 65 ++++++++++++++++++++++++++++++++++
 1 file changed, 65 insertions(+)

diff --git a/Documentation/acpi/gpio-properties.txt b/Documentation/acpi/gpio-properties.txt
index 2aff0349facd..88c65cb5bf0a 100644
--- a/Documentation/acpi/gpio-properties.txt
+++ b/Documentation/acpi/gpio-properties.txt
@@ -156,3 +156,68 @@ pointed to by its first argument.  That should be done in the driver's .probe()
 routine.  On removal, the driver should unregister its GPIO mapping table by
 calling acpi_dev_remove_driver_gpios() on the ACPI device object where that
 table was previously registered.
+
+Using the _CRS fallback
+-----------------------
+
+If a device does not have _DSD or the driver does not create ACPI GPIO
+mapping, the Linux GPIO framework refuses to return any GPIOs. This is
+because the driver does not know what it actually gets. For example if we
+have a device like below:
+
+  Device (BTH)
+  {
+      Name (_HID, ...)
+
+      Name (_CRS, ResourceTemplate () {
+          GpioIo (Exclusive, PullNone, 0, 0, IoRestrictionNone,
+                  "\\_SB.GPO0", 0, ResourceConsumer) {15}
+          GpioIo (Exclusive, PullNone, 0, 0, IoRestrictionNone,
+                  "\\_SB.GPO0", 0, ResourceConsumer) {27}
+      })
+  }
+
+The driver might expect to get the right GPIO when it does:
+
+  desc = gpiod_get(dev, "reset", GPIOD_OUT_LOW);
+
+but since there is no way to know the mapping between "reset" and
+the GpioIo() in _CRS desc will hold ERR_PTR(-ENOENT).
+
+The driver author can solve this by passing the mapping explictly
+(the recommended way and documented in the above chapter).
+
+The ACPI GPIO mapping tables should not contaminate drivers that are not
+knowing about which exact device they are servicing on. It implies that
+the ACPI GPIO mapping tables are hardly linked to ACPI ID and certain
+objects, as listed in the above chapter, of the device in question.
+
+Getting GPIO descriptor
+-----------------------
+
+There are two main approaches to get GPIO resource from ACPI:
+	desc = gpiod_get(dev, connection_id, flags);
+	desc = gpiod_get_index(dev, connection_id, index, flags);
+
+We may consider two different cases here, i.e. when connection ID is
+provided and otherwise.
+
+Case 1:
+	desc = gpiod_get(dev, "non-null-connection-id", flags);
+	desc = gpiod_get_index(dev, "non-null-connection-id", index, flags);
+
+Case 2:
+	desc = gpiod_get(dev, NULL, flags);
+	desc = gpiod_get_index(dev, NULL, index, flags);
+
+Case 1 assumes that corresponding ACPI device description must have
+defined device properties and will prevent to getting any GPIO resources
+otherwise.
+
+Case 2 explicitly tells GPIO core to look for resources in _CRS.
+
+Be aware that gpiod_get_index() in cases 1 and 2, assuming that there
+are two versions of ACPI device description provided and no mapping is
+present in the driver, will return different resources. That's why a
+certain driver has to handle them carefully as explained in previous
+chapter.
-- 
2.11.0

Re: [PATCH v2 06/12] gpio: acpi: Explain how to get GPIO descriptors in ACPI case

[Linus Walleij]

On Tue, May 23, 2017 at 7:03 PM, Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:

> Documentation lacks of explanation how we actually use device properties
> for GPIO resources.
>
> Add a section to the documentation about that.
>
> Suggested-by: Mika Westerberg <mika.westerberg@linux.intel.com>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Tested-by: Jarkko Nikula <jarkko.nikula@linux.intel.com>
> Reviewed-by: Mika Westerberg <mika.westerberg@linux.intel.com>

Patch applied.

Yours,
Linus Walleij

[PATCH v2 07/12] gpio: acpi: Factor out acpi_gpio_to_gpiod_flags() helper

[Andy Shevchenko]

The helper function acpi_gpio_to_gpiod_flags() will be used later to configure
pin properly whenever it's requested.

While here, introduce a checking error code returned by gpiod_configure_flags()
and bail out if it's not okay.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Tested-by: Jarkko Nikula <jarkko.nikula@linux.intel.com>
Reviewed-by: Mika Westerberg <mika.westerberg@linux.intel.com>
---
 drivers/gpio/gpiolib-acpi.c | 61 ++++++++++++++++++++++++++-------------------
 1 file changed, 36 insertions(+), 25 deletions(-)

diff --git a/drivers/gpio/gpiolib-acpi.c b/drivers/gpio/gpiolib-acpi.c
index 740df0e9dcb3..cb61e11558a5 100644
--- a/drivers/gpio/gpiolib-acpi.c
+++ b/drivers/gpio/gpiolib-acpi.c
@@ -423,6 +423,31 @@ static bool acpi_get_driver_gpio_data(struct acpi_device *adev,
 	return false;
 }
 
+static enum gpiod_flags
+acpi_gpio_to_gpiod_flags(const struct acpi_resource_gpio *agpio)
+{
+	bool pull_up = agpio->pin_config == ACPI_PIN_CONFIG_PULLUP;
+
+	switch (agpio->io_restriction) {
+	case ACPI_IO_RESTRICT_INPUT:
+		return GPIOD_IN;
+	case ACPI_IO_RESTRICT_OUTPUT:
+		/*
+		 * 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, otherwise low.
+		 */
+		return pull_up ? GPIOD_OUT_HIGH : GPIOD_OUT_LOW;
+	default:
+		/*
+		 * Assume that the BIOS has configured the direction and pull
+		 * accordingly.
+		 */
+		return GPIOD_ASIS;
+	}
+}
+
 struct acpi_gpio_lookup {
 	struct acpi_gpio_info info;
 	int index;
@@ -740,7 +765,6 @@ acpi_gpio_adr_space_handler(u32 function, acpi_physical_address address,
 	struct acpi_resource *ares;
 	int pin_index = (int)address;
 	acpi_status status;
-	bool pull_up;
 	int length;
 	int i;
 
@@ -755,7 +779,6 @@ acpi_gpio_adr_space_handler(u32 function, acpi_physical_address address,
 	}
 
 	agpio = &ares->data.gpio;
-	pull_up = agpio->pin_config == ACPI_PIN_CONFIG_PULLUP;
 
 	if (WARN_ON(agpio->io_restriction == ACPI_IO_RESTRICT_INPUT &&
 	    function == ACPI_WRITE)) {
@@ -806,35 +829,23 @@ acpi_gpio_adr_space_handler(u32 function, acpi_physical_address address,
 		}
 
 		if (!found) {
-			desc = gpiochip_request_own_desc(chip, pin,
-							 "ACPI:OpRegion");
+			enum gpiod_flags flags = acpi_gpio_to_gpiod_flags(agpio);
+			const char *label = "ACPI:OpRegion";
+			int err;
+
+			desc = gpiochip_request_own_desc(chip, pin, label);
 			if (IS_ERR(desc)) {
 				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:
-				/*
-				 * 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, otherwise
-				 * low.
-				 */
-				gpiod_direction_output(desc, pull_up);
-				break;
-			default:
-				/*
-				 * Assume that the BIOS has configured the
-				 * direction and pull accordingly.
-				 */
-				break;
+			err = gpiod_configure_flags(desc, label, 0, flags);
+			if (err < 0) {
+				status = AE_NOT_CONFIGURED;
+				gpiochip_free_own_desc(desc);
+				mutex_unlock(&achip->conn_lock);
+				goto out;
 			}
 
 			conn = kzalloc(sizeof(*conn), GFP_KERNEL);
-- 
2.11.0

Re: [PATCH v2 07/12] gpio: acpi: Factor out acpi_gpio_to_gpiod_flags() helper

[Linus Walleij]

On Tue, May 23, 2017 at 7:03 PM, Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:

> The helper function acpi_gpio_to_gpiod_flags() will be used later to configure
> pin properly whenever it's requested.
>
> While here, introduce a checking error code returned by gpiod_configure_flags()
> and bail out if it's not okay.
>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Tested-by: Jarkko Nikula <jarkko.nikula@linux.intel.com>
> Reviewed-by: Mika Westerberg <mika.westerberg@linux.intel.com>

Patch applied.

Yours,
Linus Walleij

[PATCH v2 08/12] gpio: acpi: Override GPIO initialization flags

[Andy Shevchenko]

This allows ACPI GPIO code to modify flags based on
ACPI GpioIo() / GpioInt() resources.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Tested-by: Jarkko Nikula <jarkko.nikula@linux.intel.com>
Reviewed-by: Mika Westerberg <mika.westerberg@linux.intel.com>
---
 drivers/gpio/gpiolib-acpi.c | 50 +++++++++++++++++++++++++++++++++++++++++++--
 drivers/gpio/gpiolib.c      |  8 ++++++--
 drivers/gpio/gpiolib.h      | 15 ++++++++++++--
 3 files changed, 67 insertions(+), 6 deletions(-)

diff --git a/drivers/gpio/gpiolib-acpi.c b/drivers/gpio/gpiolib-acpi.c
index cb61e11558a5..e431222edc2b 100644
--- a/drivers/gpio/gpiolib-acpi.c
+++ b/drivers/gpio/gpiolib-acpi.c
@@ -448,6 +448,34 @@ acpi_gpio_to_gpiod_flags(const struct acpi_resource_gpio *agpio)
 	}
 }
 
+int
+acpi_gpio_update_gpiod_flags(enum gpiod_flags *flags, enum gpiod_flags update)
+{
+	int ret = 0;
+
+	/*
+	 * Check if the BIOS has IoRestriction with explicitly set direction
+	 * and update @flags accordingly. Otherwise use whatever caller asked
+	 * for.
+	 */
+	if (update & GPIOD_FLAGS_BIT_DIR_SET) {
+		enum gpiod_flags diff = *flags ^ update;
+
+		/*
+		 * Check if caller supplied incompatible GPIO initialization
+		 * flags.
+		 *
+		 * Return %-EINVAL to notify that firmware has different
+		 * settings and we are going to use them.
+		 */
+		if (((*flags & GPIOD_FLAGS_BIT_DIR_SET) && (diff & GPIOD_FLAGS_BIT_DIR_OUT)) ||
+		    ((*flags & GPIOD_FLAGS_BIT_DIR_OUT) && (diff & GPIOD_FLAGS_BIT_DIR_VAL)))
+			ret = -EINVAL;
+		*flags = update;
+	}
+	return ret;
+}
+
 struct acpi_gpio_lookup {
 	struct acpi_gpio_info info;
 	int index;
@@ -485,8 +513,11 @@ static int acpi_populate_gpio_lookup(struct acpi_resource *ares, void *data)
 		 * - ACPI_ACTIVE_HIGH == GPIO_ACTIVE_HIGH
 		 */
 		if (lookup->info.gpioint) {
+			lookup->info.flags = GPIOD_IN;
 			lookup->info.polarity = agpio->polarity;
 			lookup->info.triggering = agpio->triggering;
+		} else {
+			lookup->info.flags = acpi_gpio_to_gpiod_flags(agpio);
 		}
 
 	}
@@ -613,13 +644,14 @@ static struct gpio_desc *acpi_get_gpiod_by_index(struct acpi_device *adev,
 struct gpio_desc *acpi_find_gpio(struct device *dev,
 				 const char *con_id,
 				 unsigned int idx,
-				 enum gpiod_flags flags,
+				 enum gpiod_flags *dflags,
 				 enum gpio_lookup_flags *lookupflags)
 {
 	struct acpi_device *adev = ACPI_COMPANION(dev);
 	struct acpi_gpio_info info;
 	struct gpio_desc *desc;
 	char propname[32];
+	int err;
 	int i;
 
 	/* Try first from _DSD */
@@ -650,7 +682,7 @@ struct gpio_desc *acpi_find_gpio(struct device *dev,
 	}
 
 	if (info.gpioint &&
-	    (flags == GPIOD_OUT_LOW || flags == GPIOD_OUT_HIGH)) {
+	    (*dflags == GPIOD_OUT_LOW || *dflags == GPIOD_OUT_HIGH)) {
 		dev_dbg(dev, "refusing GpioInt() entry when doing GPIOD_OUT_* lookup\n");
 		return ERR_PTR(-ENOENT);
 	}
@@ -658,6 +690,10 @@ struct gpio_desc *acpi_find_gpio(struct device *dev,
 	if (info.polarity == GPIO_ACTIVE_LOW)
 		*lookupflags |= GPIO_ACTIVE_LOW;
 
+	err = acpi_gpio_update_gpiod_flags(dflags, info.flags);
+	if (err)
+		dev_dbg(dev, "Override GPIO initialization flags\n");
+
 	return desc;
 }
 
@@ -711,12 +747,16 @@ struct gpio_desc *acpi_node_get_gpiod(struct fwnode_handle *fwnode,
  * used to translate from the GPIO offset in the resource to the Linux IRQ
  * number.
  *
+ * The function is idempotent, though each time it runs it will configure GPIO
+ * pin direction according to the flags in GpioInt resource.
+ *
  * Return: Linux IRQ number (>%0) on success, negative errno on failure.
  */
 int acpi_dev_gpio_irq_get(struct acpi_device *adev, int index)
 {
 	int idx, i;
 	unsigned int irq_flags;
+	int ret;
 
 	for (i = 0, idx = 0; idx <= index; i++) {
 		struct acpi_gpio_info info;
@@ -729,6 +769,7 @@ int acpi_dev_gpio_irq_get(struct acpi_device *adev, int index)
 			return PTR_ERR(desc);
 
 		if (info.gpioint && idx++ == index) {
+			char label[32];
 			int irq;
 
 			if (IS_ERR(desc))
@@ -738,6 +779,11 @@ int acpi_dev_gpio_irq_get(struct acpi_device *adev, int index)
 			if (irq < 0)
 				return irq;
 
+			snprintf(label, sizeof(label), "GpioInt() %d", index);
+			ret = gpiod_configure_flags(desc, label, 0, info.flags);
+			if (ret < 0)
+				return ret;
+
 			irq_flags = acpi_dev_get_irq_type(info.triggering,
 							  info.polarity);
 
diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index 99e07a2b7e02..70d854a4173d 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -3272,7 +3272,7 @@ struct gpio_desc *__must_check gpiod_get_index(struct device *dev,
 			desc = of_find_gpio(dev, con_id, idx, &lookupflags);
 		} else if (ACPI_COMPANION(dev)) {
 			dev_dbg(dev, "using ACPI for GPIO lookup\n");
-			desc = acpi_find_gpio(dev, con_id, idx, flags, &lookupflags);
+			desc = acpi_find_gpio(dev, con_id, idx, &flags, &lookupflags);
 		}
 	}
 
@@ -3353,8 +3353,12 @@ struct gpio_desc *fwnode_get_named_gpiod(struct fwnode_handle *fwnode,
 		struct acpi_gpio_info info;
 
 		desc = acpi_node_get_gpiod(fwnode, propname, index, &info);
-		if (!IS_ERR(desc))
+		if (!IS_ERR(desc)) {
 			active_low = info.polarity == GPIO_ACTIVE_LOW;
+			ret = acpi_gpio_update_gpiod_flags(&dflags, info.flags);
+			if (ret)
+				pr_debug("Override GPIO initialization flags\n");
+		}
 	}
 
 	if (IS_ERR(desc))
diff --git a/drivers/gpio/gpiolib.h b/drivers/gpio/gpiolib.h
index e36a0bdc7740..cff398cbb545 100644
--- a/drivers/gpio/gpiolib.h
+++ b/drivers/gpio/gpiolib.h
@@ -75,11 +75,13 @@ struct gpio_device {
 
 /**
  * struct acpi_gpio_info - ACPI GPIO specific information
+ * @flags: GPIO initialization flags
  * @gpioint: if %true this GPIO is of type GpioInt otherwise type is GpioIo
  * @polarity: interrupt polarity as provided by ACPI
  * @triggering: triggering type as provided by ACPI
  */
 struct acpi_gpio_info {
+	enum gpiod_flags flags;
 	bool gpioint;
 	int polarity;
 	int triggering;
@@ -121,10 +123,13 @@ void acpi_gpiochip_remove(struct gpio_chip *chip);
 void acpi_gpiochip_request_interrupts(struct gpio_chip *chip);
 void acpi_gpiochip_free_interrupts(struct gpio_chip *chip);
 
+int acpi_gpio_update_gpiod_flags(enum gpiod_flags *flags,
+				 enum gpiod_flags update);
+
 struct gpio_desc *acpi_find_gpio(struct device *dev,
 				 const char *con_id,
 				 unsigned int idx,
-				 enum gpiod_flags flags,
+				 enum gpiod_flags *dflags,
 				 enum gpio_lookup_flags *lookupflags);
 struct gpio_desc *acpi_node_get_gpiod(struct fwnode_handle *fwnode,
 				      const char *propname, int index,
@@ -143,9 +148,15 @@ acpi_gpiochip_request_interrupts(struct gpio_chip *chip) { }
 static inline void
 acpi_gpiochip_free_interrupts(struct gpio_chip *chip) { }
 
+static inline int
+acpi_gpio_update_gpiod_flags(enum gpiod_flags *flags, enum gpiod_flags update)
+{
+	return 0;
+}
+
 static inline struct gpio_desc *
 acpi_find_gpio(struct device *dev, const char *con_id,
-	       unsigned int idx, enum gpiod_flags flags,
+	       unsigned int idx, enum gpiod_flags *dflags,
 	       enum gpio_lookup_flags *lookupflags)
 {
 	return ERR_PTR(-ENOENT);
-- 
2.11.0

Re: [PATCH v2 08/12] gpio: acpi: Override GPIO initialization flags

[Linus Walleij]

On Tue, May 23, 2017 at 7:03 PM, Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:

> This allows ACPI GPIO code to modify flags based on
> ACPI GpioIo() / GpioInt() resources.
>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Tested-by: Jarkko Nikula <jarkko.nikula@linux.intel.com>
> Reviewed-by: Mika Westerberg <mika.westerberg@linux.intel.com>

Patch applied.

Yours,
Linus Walleij

[PATCH v2 09/12] gpio: acpi: Split out acpi_gpio_get_irq_resource() helper

[Andy Shevchenko]

The helper does retrieve pointer to struct acpi_resource_gpio from
struct acpi_resource if it represents GpioInt() resource.

It will be used by PNP code later on.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/gpio/gpiolib-acpi.c | 23 ++++++++++++++++++-----
 include/linux/acpi.h        |  7 +++++++
 2 files changed, 25 insertions(+), 5 deletions(-)

diff --git a/drivers/gpio/gpiolib-acpi.c b/drivers/gpio/gpiolib-acpi.c
index e431222edc2b..6bea176b066c 100644
--- a/drivers/gpio/gpiolib-acpi.c
+++ b/drivers/gpio/gpiolib-acpi.c
@@ -165,6 +165,23 @@ static void acpi_gpio_chip_dh(acpi_handle handle, void *data)
 	/* The address of this function is used as a key. */
 }
 
+bool acpi_gpio_get_irq_resource(struct acpi_resource *ares,
+				struct acpi_resource_gpio **agpio)
+{
+	struct acpi_resource_gpio *gpio;
+
+	if (ares->type != ACPI_RESOURCE_TYPE_GPIO)
+		return false;
+
+	gpio = &ares->data.gpio;
+	if (gpio->connection_type != ACPI_RESOURCE_GPIO_TYPE_INT)
+		return false;
+
+	*agpio = gpio;
+	return true;
+}
+EXPORT_SYMBOL_GPL(acpi_gpio_get_irq_resource);
+
 static acpi_status acpi_gpiochip_request_interrupt(struct acpi_resource *ares,
 						   void *context)
 {
@@ -178,11 +195,7 @@ static acpi_status acpi_gpiochip_request_interrupt(struct acpi_resource *ares,
 	unsigned long irqflags;
 	int ret, pin, irq;
 
-	if (ares->type != ACPI_RESOURCE_TYPE_GPIO)
-		return AE_OK;
-
-	agpio = &ares->data.gpio;
-	if (agpio->connection_type != ACPI_RESOURCE_GPIO_TYPE_INT)
+	if (!acpi_gpio_get_irq_resource(ares, &agpio))
 		return AE_OK;
 
 	handle = ACPI_HANDLE(chip->parent);
diff --git a/include/linux/acpi.h b/include/linux/acpi.h
index 137e4a3d89c5..d5aa3c42f64d 100644
--- a/include/linux/acpi.h
+++ b/include/linux/acpi.h
@@ -964,6 +964,8 @@ int devm_acpi_dev_add_driver_gpios(struct device *dev,
 				   const struct acpi_gpio_mapping *gpios);
 void devm_acpi_dev_remove_driver_gpios(struct device *dev);
 
+bool acpi_gpio_get_irq_resource(struct acpi_resource *ares,
+				struct acpi_resource_gpio **agpio);
 int acpi_dev_gpio_irq_get(struct acpi_device *adev, int index);
 #else
 static inline int acpi_dev_add_driver_gpios(struct acpi_device *adev,
@@ -980,6 +982,11 @@ static inline int devm_acpi_dev_add_driver_gpios(struct device *dev,
 }
 static inline void devm_acpi_dev_remove_driver_gpios(struct device *dev) {}
 
+static inline bool acpi_gpio_get_irq_resource(struct acpi_resource *ares,
+					      struct acpi_resource_gpio **agpio)
+{
+	return false;
+}
 static inline int acpi_dev_gpio_irq_get(struct acpi_device *adev, int index)
 {
 	return -ENXIO;
-- 
2.11.0

Re: [PATCH v2 09/12] gpio: acpi: Split out acpi_gpio_get_irq_resource() helper

[Mika Westerberg]

On Tue, May 23, 2017 at 08:03:24PM +0300, Andy Shevchenko wrote:
> The helper does retrieve pointer to struct acpi_resource_gpio from
> struct acpi_resource if it represents GpioInt() resource.
> 
> It will be used by PNP code later on.
> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
>  drivers/gpio/gpiolib-acpi.c | 23 ++++++++++++++++++-----
>  include/linux/acpi.h        |  7 +++++++
>  2 files changed, 25 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpio/gpiolib-acpi.c b/drivers/gpio/gpiolib-acpi.c
> index e431222edc2b..6bea176b066c 100644
> --- a/drivers/gpio/gpiolib-acpi.c
> +++ b/drivers/gpio/gpiolib-acpi.c
> @@ -165,6 +165,23 @@ static void acpi_gpio_chip_dh(acpi_handle handle, void *data)
>  	/* The address of this function is used as a key. */
>  }

Since this is exported, it might be good idea to provide kernel-doc
here.

Anyway,

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

> +bool acpi_gpio_get_irq_resource(struct acpi_resource *ares,
> +				struct acpi_resource_gpio **agpio)
> +{
> +	struct acpi_resource_gpio *gpio;
> +
> +	if (ares->type != ACPI_RESOURCE_TYPE_GPIO)
> +		return false;
> +
> +	gpio = &ares->data.gpio;
> +	if (gpio->connection_type != ACPI_RESOURCE_GPIO_TYPE_INT)
> +		return false;
> +
> +	*agpio = gpio;
> +	return true;
> +}
> +EXPORT_SYMBOL_GPL(acpi_gpio_get_irq_resource);

Re: [PATCH v2 09/12] gpio: acpi: Split out acpi_gpio_get_irq_resource() helper

[Linus Walleij]

On Tue, May 23, 2017 at 7:03 PM, Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:

> The helper does retrieve pointer to struct acpi_resource_gpio from
> struct acpi_resource if it represents GpioInt() resource.
>
> It will be used by PNP code later on.
>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

Patch applied with Mika's review tag.

Yours,
Linus Walleij

[PATCH v2 10/12] PNP / ACPI: add support for GpioInt resource type

[Andy Shevchenko]

From: Jagadish Krishnamoorthy <jagadish.krishnamoorthy@intel.com>

The PNP ACPI driver parses ACPI interrupt resource but not
GpioInt resource. When the firmware passes GpioInt resource
for IRQ the PNP ACPI driver ignores it and hence the interrupt for
the particular driver will not work.
One such example is 8042 keyboard which uses PNP driver for obtaining
the interrupt resource. On Intel Braswell project GpioInt is used
instead of interrupt resource and the keyboard driver fails to
register interrupt.
Fix the issue by parsing GpioInt resource type.

Signed-off-by: Jagadish Krishnamoorthy <jagadish.krishnamoorthy@intel.com>
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/pnp/pnpacpi/rsparser.c | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/drivers/pnp/pnpacpi/rsparser.c b/drivers/pnp/pnpacpi/rsparser.c
index 4b717c699313..af44e57f5148 100644
--- a/drivers/pnp/pnpacpi/rsparser.c
+++ b/drivers/pnp/pnpacpi/rsparser.c
@@ -180,6 +180,7 @@ static acpi_status pnpacpi_allocated_resource(struct acpi_resource *res,
 	struct pnp_dev *dev = data;
 	struct acpi_resource_dma *dma;
 	struct acpi_resource_vendor_typed *vendor_typed;
+	struct acpi_resource_gpio *gpio;
 	struct resource_win win = {{0}, 0};
 	struct resource *r = &win.res;
 	int i, flags;
@@ -210,6 +211,20 @@ static acpi_status pnpacpi_allocated_resource(struct acpi_resource *res,
 			}
 		}
 		return AE_OK;
+	} else if (acpi_gpio_get_irq_resource(res, &gpio)) {
+		/*
+		 * If the resource is GpioInt() type then extract the IRQ
+		 * from GPIO resource and fill it into IRQ resource type.
+		 */
+		i = acpi_dev_gpio_irq_get(dev->data, 0);
+		if (i >= 0) {
+			flags = acpi_dev_irq_flags(gpio->triggering,
+						   gpio->polarity,
+						   gpio->sharable);
+		} else
+			flags = IORESOURCE_DISABLED;
+		pnp_add_irq_resource(dev, i, flags);
+		return AE_OK;
 	} else if (r->flags & IORESOURCE_DISABLED) {
 		pnp_add_irq_resource(dev, 0, IORESOURCE_DISABLED);
 		return AE_OK;
-- 
2.11.0

Re: [PATCH v2 10/12] PNP / ACPI: add support for GpioInt resource type

[Mika Westerberg]

On Tue, May 23, 2017 at 08:03:25PM +0300, Andy Shevchenko wrote:
> From: Jagadish Krishnamoorthy <jagadish.krishnamoorthy@intel.com>
> 
> The PNP ACPI driver parses ACPI interrupt resource but not
> GpioInt resource. When the firmware passes GpioInt resource
> for IRQ the PNP ACPI driver ignores it and hence the interrupt for
> the particular driver will not work.
> One such example is 8042 keyboard which uses PNP driver for obtaining
> the interrupt resource. On Intel Braswell project GpioInt is used
> instead of interrupt resource and the keyboard driver fails to
> register interrupt.
> Fix the issue by parsing GpioInt resource type.

Maybe you can add link to the bugzilla entry here?

> Signed-off-by: Jagadish Krishnamoorthy <jagadish.krishnamoorthy@intel.com>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
>  drivers/pnp/pnpacpi/rsparser.c | 15 +++++++++++++++
>  1 file changed, 15 insertions(+)
> 
> diff --git a/drivers/pnp/pnpacpi/rsparser.c b/drivers/pnp/pnpacpi/rsparser.c
> index 4b717c699313..af44e57f5148 100644
> --- a/drivers/pnp/pnpacpi/rsparser.c
> +++ b/drivers/pnp/pnpacpi/rsparser.c
> @@ -180,6 +180,7 @@ static acpi_status pnpacpi_allocated_resource(struct acpi_resource *res,
>  	struct pnp_dev *dev = data;
>  	struct acpi_resource_dma *dma;
>  	struct acpi_resource_vendor_typed *vendor_typed;
> +	struct acpi_resource_gpio *gpio;
>  	struct resource_win win = {{0}, 0};
>  	struct resource *r = &win.res;
>  	int i, flags;
> @@ -210,6 +211,20 @@ static acpi_status pnpacpi_allocated_resource(struct acpi_resource *res,
>  			}
>  		}
>  		return AE_OK;
> +	} else if (acpi_gpio_get_irq_resource(res, &gpio)) {
> +		/*
> +		 * If the resource is GpioInt() type then extract the IRQ
> +		 * from GPIO resource and fill it into IRQ resource type.
> +		 */
> +		i = acpi_dev_gpio_irq_get(dev->data, 0);
> +		if (i >= 0) {
> +			flags = acpi_dev_irq_flags(gpio->triggering,
> +						   gpio->polarity,
> +						   gpio->sharable);
> +		} else
> +			flags = IORESOURCE_DISABLED;

You need to add {} here as well.

With that done you can add my

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

> +		pnp_add_irq_resource(dev, i, flags);
> +		return AE_OK;
>  	} else if (r->flags & IORESOURCE_DISABLED) {
>  		pnp_add_irq_resource(dev, 0, IORESOURCE_DISABLED);
>  		return AE_OK;
> -- 
> 2.11.0

Re: [PATCH v2 10/12] PNP / ACPI: add support for GpioInt resource type

[Linus Walleij]

On Tue, May 23, 2017 at 7:03 PM, Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:

> From: Jagadish Krishnamoorthy <jagadish.krishnamoorthy@intel.com>
>
> The PNP ACPI driver parses ACPI interrupt resource but not
> GpioInt resource. When the firmware passes GpioInt resource
> for IRQ the PNP ACPI driver ignores it and hence the interrupt for
> the particular driver will not work.
> One such example is 8042 keyboard which uses PNP driver for obtaining
> the interrupt resource. On Intel Braswell project GpioInt is used
> instead of interrupt resource and the keyboard driver fails to
> register interrupt.
> Fix the issue by parsing GpioInt resource type.
>
> Signed-off-by: Jagadish Krishnamoorthy <jagadish.krishnamoorthy@intel.com>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

Fixed the paranthesis pointed out by Mika and applied
with his review tag.

Yours,
Linus Walleij

[PATCH v2 11/12] PNP / ACPI: join strings back for better maintenance

[Andy Shevchenko]

Simply join string literals back for better maintenance and debugging.

No functional changes intended.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/pnp/pnpacpi/rsparser.c | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/drivers/pnp/pnpacpi/rsparser.c b/drivers/pnp/pnpacpi/rsparser.c
index af44e57f5148..02ae95a70b7d 100644
--- a/drivers/pnp/pnpacpi/rsparser.c
+++ b/drivers/pnp/pnpacpi/rsparser.c
@@ -149,8 +149,8 @@ static int vendor_resource_matches(struct pnp_dev *dev,
 	    uuid_len == sizeof(match->data) &&
 	    memcmp(uuid, match->data, uuid_len) == 0) {
 		if (expected_len && expected_len != actual_len) {
-			dev_err(&dev->dev, "wrong vendor descriptor size; "
-				"expected %d, found %d bytes\n",
+			dev_err(&dev->dev,
+				"wrong vendor descriptor size; expected %d, found %d bytes\n",
 				expected_len, actual_len);
 			return 0;
 		}
@@ -204,9 +204,8 @@ static acpi_status pnpacpi_allocated_resource(struct acpi_resource *res,
 			 * one interrupt, we won't be able to re-encode it.
 			 */
 			if (pnp_can_write(dev)) {
-				dev_warn(&dev->dev, "multiple interrupts in "
-					 "_CRS descriptor; configuration can't "
-					 "be changed\n");
+				dev_warn(&dev->dev,
+					 "multiple interrupts in _CRS descriptor; configuration can't be changed\n");
 				dev->capabilities &= ~PNP_WRITE;
 			}
 		}
@@ -346,8 +345,8 @@ static __init void pnpacpi_parse_ext_irq_option(struct pnp_dev *dev,
 			if (p->interrupts[i] < PNP_IRQ_NR)
 				__set_bit(p->interrupts[i], map.bits);
 			else
-				dev_err(&dev->dev, "ignoring IRQ %d option "
-					"(too large for %d entry bitmap)\n",
+				dev_err(&dev->dev,
+					"ignoring IRQ %d option (too large for %d entry bitmap)\n",
 					p->interrupts[i], PNP_IRQ_NR);
 		}
 	}
@@ -948,8 +947,9 @@ int pnpacpi_encode_resources(struct pnp_dev *dev, struct acpi_buffer *buffer)
 		case ACPI_RESOURCE_TYPE_EXTENDED_ADDRESS64:
 		case ACPI_RESOURCE_TYPE_GENERIC_REGISTER:
 		default:	/* other type */
-			dev_warn(&dev->dev, "can't encode unknown resource "
-				 "type %d\n", resource->type);
+			dev_warn(&dev->dev,
+				 "can't encode unknown resource type %d\n",
+				 resource->type);
 			return -EINVAL;
 		}
 		resource++;
-- 
2.11.0

Re: [PATCH v2 11/12] PNP / ACPI: join strings back for better maintenance

[Mika Westerberg]

On Tue, May 23, 2017 at 08:03:26PM +0300, Andy Shevchenko wrote:
> Simply join string literals back for better maintenance and debugging.
> 
> No functional changes intended.
> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

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

Re: [PATCH v2 11/12] PNP / ACPI: join strings back for better maintenance

[Linus Walleij]

On Tue, May 23, 2017 at 7:03 PM, Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:

> Simply join string literals back for better maintenance and debugging.
>
> No functional changes intended.
>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

Patch applied with Mika's review tag.

Yours,
Linus Walleij

[PATCH v2 12/12] PNP / ACPI: remove FSF address

[Andy Shevchenko]

There is no point in keeping an address in the file since it's subject
to change.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/pnp/pnpacpi/rsparser.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/drivers/pnp/pnpacpi/rsparser.c b/drivers/pnp/pnpacpi/rsparser.c
index 02ae95a70b7d..562270a0f264 100644
--- a/drivers/pnp/pnpacpi/rsparser.c
+++ b/drivers/pnp/pnpacpi/rsparser.c
@@ -15,10 +15,6 @@
  * WITHOUT ANY WARRANTY; without even the implied warranty of
  * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
  * General Public License for more details.
- *
- * You should have received a copy of the GNU General Public License
- * along with this program; if not, write to the Free Software
- * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
  */
 #include <linux/kernel.h>
 #include <linux/acpi.h>
-- 
2.11.0

Re: [PATCH v2 12/12] PNP / ACPI: remove FSF address

[Mika Westerberg]

On Tue, May 23, 2017 at 08:03:27PM +0300, Andy Shevchenko wrote:
> There is no point in keeping an address in the file since it's subject
> to change.
> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

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

Re: [PATCH v2 12/12] PNP / ACPI: remove FSF address

[Linus Walleij]

On Tue, May 23, 2017 at 7:03 PM, Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:

> There is no point in keeping an address in the file since it's subject
> to change.
>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

Patch applied with Mika's review tag.

Yours,
Linus Walleij

Re: [PATCH v2 00/12] gpio: acpi: Make it working

[Linus Walleij]

On Tue, May 23, 2017 at 7:03 PM, Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:

> This is second iteration of the fix of GPIO ACPI library to make it
> robust against different ACPI specification generations and thus
> firmwares.

I applied it all so we get some rotation.

Was a bit worried about hitting drivers/pnp/* but I suspect that is
actually your turf so I just applied it.

Yours,
Linus Walleij

Re: [PATCH v2 00/12] gpio: acpi: Make it working

[Andy Shevchenko]

On Mon, 2017-05-29 at 11:31 +0200, Linus Walleij wrote:
> On Tue, May 23, 2017 at 7:03 PM, Andy Shevchenko
> <andriy.shevchenko@linux.intel.com> wrote:
> 
> > This is second iteration of the fix of GPIO ACPI library to make it
> > robust against different ACPI specification generations and thus
> > firmwares.
> 
> I applied it all so we get some rotation.
> 
> Was a bit worried about hitting drivers/pnp/* but I suspect that is
> actually your turf so I just applied it.
> 

Thanks!


-- 
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy