[PATCH v3 0/4] gpiolib: acpi: pin configuration fixes

[PATCH v3 0/4] gpiolib: acpi: pin configuration fixes

[Andy Shevchenko]

There are two fixes (and two small cleanups) that allow to take into
consideration more parameters in ACPI, i.e. bias for GpioInt() and
debounce time for GpioInt() and GpioIo() resources.

The first patch highly depends on Intel pin control driver changes
(for now [1], but might be more), so it's probably not supposed to be
backported (at least right now).

I think the best way is to collect tags from GPIO maintainers and I
can incorporate this into our Intel pin control branch which I will
share with you as PR against GPIO and pin control subsystems.

I'm also all ears for alternatives.

Cc: Jamie McClymont <jamie@kwiius.com>
Cc: Mika Westerberg <mika.westerberg@linux.intel.com>
Cc: Hans de Goede <hdegoede@redhat.com>

[1]: https://lore.kernel.org/linux-gpio/20201014104638.84043-1-andriy.shevchenko@linux.intel.com/T/

Changelog v3:
- dropped upstreamed OF patch
- added debounce fix

Andy Shevchenko (4):
  gpiolib: acpi: Respect bias settings for GpioInt() resource
  gpiolib: acpi: Use named item for enum gpiod_flags variable
  gpiolib: acpi: Take into account debounce settings
  gpiolib: acpi: Convert pin_index to be u16

 drivers/gpio/gpiolib-acpi.c | 23 ++++++++++++++++-------
 drivers/gpio/gpiolib-acpi.h |  2 ++
 2 files changed, 18 insertions(+), 7 deletions(-)

-- 
2.28.0

[PATCH v3 1/4] gpiolib: acpi: Respect bias settings for GpioInt() resource

[Andy Shevchenko]

In some cases the GpioInt() resource is coming with bias settings
which may affect system functioning. Respect bias settings for
GpioInt() resource by calling acpi_gpio_update_gpiod_*flags() API
in acpi_dev_gpio_irq_get().

Reported-by: Jamie McClymont <jamie@kwiius.com>
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Reviewed-by: Mika Westerberg <mika.westerberg@linux.intel.com>
---
 drivers/gpio/gpiolib-acpi.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/gpio/gpiolib-acpi.c b/drivers/gpio/gpiolib-acpi.c
index 834a12f3219e..3a39e8a93226 100644
--- a/drivers/gpio/gpiolib-acpi.c
+++ b/drivers/gpio/gpiolib-acpi.c
@@ -942,6 +942,7 @@ int acpi_dev_gpio_irq_get(struct acpi_device *adev, int index)
 
 		if (info.gpioint && idx++ == index) {
 			unsigned long lflags = GPIO_LOOKUP_FLAGS_DEFAULT;
+			enum gpiod_flags dflags = GPIOD_ASIS;
 			char label[32];
 			int irq;
 
@@ -952,8 +953,11 @@ int acpi_dev_gpio_irq_get(struct acpi_device *adev, int index)
 			if (irq < 0)
 				return irq;
 
+			acpi_gpio_update_gpiod_flags(&dflags, &info);
+			acpi_gpio_update_gpiod_lookup_flags(&lflags, &info);
+
 			snprintf(label, sizeof(label), "GpioInt() %d", index);
-			ret = gpiod_configure_flags(desc, label, lflags, info.flags);
+			ret = gpiod_configure_flags(desc, label, lflags, dflags);
 			if (ret < 0)
 				return ret;
 
-- 
2.28.0

[PATCH v3 2/4] gpiolib: acpi: Use named item for enum gpiod_flags variable

[Andy Shevchenko]

Use named item instead of plain integer for enum gpiod_flags
to make it clear that even 0 has its own meaning.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Reviewed-by: Mika Westerberg <mika.westerberg@linux.intel.com>
---
 drivers/gpio/gpiolib-acpi.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpio/gpiolib-acpi.c b/drivers/gpio/gpiolib-acpi.c
index 3a39e8a93226..c127b410a7a2 100644
--- a/drivers/gpio/gpiolib-acpi.c
+++ b/drivers/gpio/gpiolib-acpi.c
@@ -1136,7 +1136,7 @@ acpi_gpiochip_parse_own_gpio(struct acpi_gpio_chip *achip,
 	int ret;
 
 	*lflags = GPIO_LOOKUP_FLAGS_DEFAULT;
-	*dflags = 0;
+	*dflags = GPIOD_ASIS;
 	*name = NULL;
 
 	ret = fwnode_property_read_u32_array(fwnode, "gpios", gpios,
-- 
2.28.0

[PATCH v3 3/4] gpiolib: acpi: Take into account debounce settings

[Andy Shevchenko]

We didn't take into account the debounce settings supplied by ACPI.
This change is targeting the mentioned gap.

Reported-by: Coiby Xu <coiby.xu@gmail.com>
Cc: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/gpio/gpiolib-acpi.c | 5 +++++
 drivers/gpio/gpiolib-acpi.h | 2 ++
 2 files changed, 7 insertions(+)

diff --git a/drivers/gpio/gpiolib-acpi.c b/drivers/gpio/gpiolib-acpi.c
index c127b410a7a2..ad245b2a536e 100644
--- a/drivers/gpio/gpiolib-acpi.c
+++ b/drivers/gpio/gpiolib-acpi.c
@@ -664,6 +664,7 @@ static int acpi_populate_gpio_lookup(struct acpi_resource *ares, void *data)
 		lookup->desc = acpi_get_gpiod(agpio->resource_source.string_ptr,
 					      agpio->pin_table[pin_index]);
 		lookup->info.pin_config = agpio->pin_config;
+		lookup->info.debounce = agpio->debounce_timeout;
 		lookup->info.gpioint = gpioint;
 
 		/*
@@ -961,6 +962,10 @@ int acpi_dev_gpio_irq_get(struct acpi_device *adev, int index)
 			if (ret < 0)
 				return ret;
 
+			ret = gpiod_set_debounce(desc, info.debounce);
+			if (ret)
+				return ret;
+
 			irq_flags = acpi_dev_get_irq_type(info.triggering,
 							  info.polarity);
 
diff --git a/drivers/gpio/gpiolib-acpi.h b/drivers/gpio/gpiolib-acpi.h
index 1c6d65cf0629..e2edb632b2cc 100644
--- a/drivers/gpio/gpiolib-acpi.h
+++ b/drivers/gpio/gpiolib-acpi.h
@@ -18,6 +18,7 @@ struct acpi_device;
  * @pin_config: pin bias as provided by ACPI
  * @polarity: interrupt polarity as provided by ACPI
  * @triggering: triggering type as provided by ACPI
+ * @debounce: debounce timeout as provided by ACPI
  * @quirks: Linux specific quirks as provided by struct acpi_gpio_mapping
  */
 struct acpi_gpio_info {
@@ -27,6 +28,7 @@ struct acpi_gpio_info {
 	int pin_config;
 	int polarity;
 	int triggering;
+	unsigned int debounce;
 	unsigned int quirks;
 };
 
-- 
2.28.0

[PATCH v3 4/4] gpiolib: acpi: Convert pin_index to be u16

[Andy Shevchenko]

As defined by ACPI the pin index is 16-bit unsigned integer.
Define the variable, which holds it, accordingly.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@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 ad245b2a536e..a571dbf2093b 100644
--- a/drivers/gpio/gpiolib-acpi.c
+++ b/drivers/gpio/gpiolib-acpi.c
@@ -633,7 +633,7 @@ int acpi_gpio_update_gpiod_lookup_flags(unsigned long *lookupflags,
 struct acpi_gpio_lookup {
 	struct acpi_gpio_info info;
 	int index;
-	int pin_index;
+	u16 pin_index;
 	bool active_low;
 	struct gpio_desc *desc;
 	int n;
@@ -649,7 +649,7 @@ static int acpi_populate_gpio_lookup(struct acpi_resource *ares, void *data)
 	if (!lookup->desc) {
 		const struct acpi_resource_gpio *agpio = &ares->data.gpio;
 		bool gpioint = agpio->connection_type == ACPI_RESOURCE_GPIO_TYPE_INT;
-		int pin_index;
+		u16 pin_index;
 
 		if (lookup->info.quirks & ACPI_GPIO_QUIRK_ONLY_GPIOIO && gpioint)
 			lookup->index++;
@@ -795,7 +795,7 @@ static struct gpio_desc *acpi_get_gpiod_by_index(struct acpi_device *adev,
 		if (ret)
 			return ERR_PTR(ret);
 
-		dev_dbg(&adev->dev, "GPIO: _DSD returned %s %d %d %u\n",
+		dev_dbg(&adev->dev, "GPIO: _DSD returned %s %d %u %u\n",
 			dev_name(&lookup.info.adev->dev), lookup.index,
 			lookup.pin_index, lookup.active_low);
 	} else {
@@ -991,7 +991,7 @@ acpi_gpio_adr_space_handler(u32 function, acpi_physical_address address,
 	struct gpio_chip *chip = achip->chip;
 	struct acpi_resource_gpio *agpio;
 	struct acpi_resource *ares;
-	int pin_index = (int)address;
+	u16 pin_index = address;
 	acpi_status status;
 	int length;
 	int i;
@@ -1014,7 +1014,7 @@ acpi_gpio_adr_space_handler(u32 function, acpi_physical_address address,
 		return AE_BAD_PARAMETER;
 	}
 
-	length = min(agpio->pin_table_length, (u16)(pin_index + bits));
+	length = min_t(u16, agpio->pin_table_length, pin_index + bits);
 	for (i = pin_index; i < length; ++i) {
 		int pin = agpio->pin_table[i];
 		struct acpi_gpio_connection *conn;
-- 
2.28.0

Re: [PATCH v3 0/4] gpiolib: acpi: pin configuration fixes

[Linus Walleij]

On Mon, Nov 2, 2020 at 8:17 PM Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:

> I think the best way is to collect tags from GPIO maintainers and I
> can incorporate this into our Intel pin control branch which I will
> share with you as PR against GPIO and pin control subsystems.
>
> I'm also all ears for alternatives.

Acked-by: Linus Walleij <linus.walleij@linaro.org>

However I definitely trust you on any changes to gpiolib-acpi.c
so if you want a definitive formal control over that I do not mind
if you list yourself as maintainer of this file. I would
definitely pull any GPIO-ACPI-stuff from you without any
hesitation anyways.

Yours,
Linus Walleij

Re: [PATCH v3 0/4] gpiolib: acpi: pin configuration fixes

[Andy Shevchenko]

On Fri, Nov 6, 2020 at 3:53 PM Linus Walleij <linus.walleij@linaro.org> wrote:
>
> On Mon, Nov 2, 2020 at 8:17 PM Andy Shevchenko
> <andriy.shevchenko@linux.intel.com> wrote:
>
> > I think the best way is to collect tags from GPIO maintainers and I
> > can incorporate this into our Intel pin control branch which I will
> > share with you as PR against GPIO and pin control subsystems.
> >
> > I'm also all ears for alternatives.
>
> Acked-by: Linus Walleij <linus.walleij@linaro.org>

Thanks!

> However I definitely trust you on any changes to gpiolib-acpi.c
> so if you want a definitive formal control over that I do not mind
> if you list yourself as maintainer of this file. I would
> definitely pull any GPIO-ACPI-stuff from you without any
> hesitation anyways.

Let's hear what Mika says. I'm fine to use my Intel GPIO tree for that purpose.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v3 0/4] gpiolib: acpi: pin configuration fixes

[Andy Shevchenko]

On Fri, Nov 06, 2020 at 05:10:34PM +0200, Andy Shevchenko wrote:
> On Fri, Nov 6, 2020 at 3:53 PM Linus Walleij <linus.walleij@linaro.org> wrote:
> >
> > On Mon, Nov 2, 2020 at 8:17 PM Andy Shevchenko
> > <andriy.shevchenko@linux.intel.com> wrote:
> >
> > > I think the best way is to collect tags from GPIO maintainers and I
> > > can incorporate this into our Intel pin control branch which I will
> > > share with you as PR against GPIO and pin control subsystems.
> > >
> > > I'm also all ears for alternatives.
> >
> > Acked-by: Linus Walleij <linus.walleij@linaro.org>
> 
> Thanks!
> 
> > However I definitely trust you on any changes to gpiolib-acpi.c
> > so if you want a definitive formal control over that I do not mind
> > if you list yourself as maintainer of this file. I would
> > definitely pull any GPIO-ACPI-stuff from you without any
> > hesitation anyways.
> 
> Let's hear what Mika says. I'm fine to use my Intel GPIO tree for that purpose.

Okay, I have sent v4 with updated MAINTAINERS.

-- 
With Best Regards,
Andy Shevchenko