[PATCH v5 00/17] gpiolib: acpi: pin configuration fixes

[PATCH v5 00/17] gpiolib: acpi: pin configuration fixes

[Andy Shevchenko]

There are fixes (and plenty cleanups) that allow to take into consideration
more parameters in ACPI, i.e. bias for GpioInt() and debounce timeout
for Operation Regions, Events and GpioInt() resources.

During review Hans noted, that gpiod_set_debounce() returns -ENOTSUPP for
the cases when feature is not supported either by driver or a controller.

It appears that we have slightly messy API here:

  FUNC			Relation with ENOTSUPP

  gpiod_set_config()	 returns if not supported
  gpiod_set_debounce()	 as gpiod_set_config() above
  gpio_set_debounce()	 legacy wrapper on top of gpiod_set_debounce()
  gpiod_set_transitory() skips it (returns okay) with debug message
  gpio_set_config()	 returns if not supported
  gpio_set_bias()	 skips it (returns okay)

Last two functions are internal to GPIO library, while the rest is
exported API. In order to be consistent with both naming schemas
the series introduces gpio_set_debounce_timeout() that considers
the feature optional. New API is only for internal use.

While at it, the few first patches do clean up the current GPIO library
code to unify it to some extend.

The above is followed by changes made in ACPI GPIO library part.

The bias patch highly depends on Intel pin control driver changes
(they are material for v5.10 [1]), due to this and amount of the
prerequisite changes this series is probably not supposed to be
backported (at least right now).

The last patch adds Intel GPIO tree as official one for ACPI GPIO
changes.

Assuming [1] makes v5.10 this series can be sent as PR to Linus
for v5.11 cycle.

Note, some patches are also depend to the code from GPIO fixes / for-next
repositories. Unfortunately there is no one repository which contains all
up to date for-next changes against GPIO subsystem. That's why I have merged
Bart's for-current followed by Linus' fixes followed by Bart's for-next
followed by Linus' for-next branches as prerequisites to the series.

Cc: Jamie McClymont <jamie@kwiius.com>

[1]: https://lore.kernel.org/linux-gpio/20201106181938.GA41213@black.fi.intel.com/

Changelog v5:
- introduced gpio_set_debounce_timeout()
- made a prerequisite refactoring in GPIO library code
- updated the rest accordingly

Changelog v4:
- extended debounce setting to ACPI events and Operation Regions
- added Ack (Linus)
- added few more cleanup patches, including MAINTAINERS update

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

Andy Shevchenko (17):
  gpiolib: Replace unsigned by unsigned int
  gpiolib: add missed break statement
  gpiolib: use proper API to pack pin configuration parameters
  gpiolib: Add temporary variable to gpiod_set_transitory() for cleaner
    code
  gpiolib: Extract gpio_set_config_with_argument() for future use
  gpiolib: move bias related code from gpio_set_config() to
    gpio_set_bias()
  gpiolib: Extract gpio_set_config_with_argument_optional() helper
  gpiolib: Extract gpio_set_debounce_timeout() for internal use
  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: Move acpi_gpio_to_gpiod_flags() upper in the code
  gpiolib: acpi: Make acpi_gpio_to_gpiod_flags() usable for GpioInt()
  gpiolib: acpi: Extract acpi_request_own_gpiod() helper
  gpiolib: acpi: Convert pin_index to be u16
  gpiolib: acpi: Use BIT() macro to increase readability
  gpiolib: acpi: Make Intel GPIO tree official for GPIO ACPI work

 MAINTAINERS                   |   1 +
 drivers/gpio/gpiolib-acpi.c   | 130 ++++++++++++++++++++--------------
 drivers/gpio/gpiolib-acpi.h   |   2 +
 drivers/gpio/gpiolib.c        |  97 ++++++++++++++-----------
 drivers/gpio/gpiolib.h        |   1 +
 include/linux/gpio/consumer.h |   4 +-
 6 files changed, 141 insertions(+), 94 deletions(-)

-- 
2.28.0

[PATCH v5 01/17] gpiolib: Replace unsigned by unsigned int

[Andy Shevchenko]

Replace unsigned by unsigned int in GPIO library code.
Note, legacy API left untouched.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/gpio/gpiolib.c        | 16 ++++++++--------
 include/linux/gpio/consumer.h |  4 ++--
 2 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index bb5a9557b350..f94225d7817b 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -211,7 +211,7 @@ static int gpiochip_find_base(int ngpio)
 int gpiod_get_direction(struct gpio_desc *desc)
 {
 	struct gpio_chip *gc;
-	unsigned offset;
+	unsigned int offset;
 	int ret;
 
 	gc = gpiod_to_chip(desc);
@@ -1333,7 +1333,7 @@ void gpiochip_irq_domain_deactivate(struct irq_domain *domain,
 }
 EXPORT_SYMBOL_GPL(gpiochip_irq_domain_deactivate);
 
-static int gpiochip_to_irq(struct gpio_chip *gc, unsigned offset)
+static int gpiochip_to_irq(struct gpio_chip *gc, unsigned int offset)
 {
 	struct irq_domain *domain = gc->irq.domain;
 
@@ -1635,7 +1635,7 @@ static inline void gpiochip_irqchip_free_valid_mask(struct gpio_chip *gc)
  * @gc: the gpiochip owning the GPIO
  * @offset: the offset of the GPIO to request for GPIO function
  */
-int gpiochip_generic_request(struct gpio_chip *gc, unsigned offset)
+int gpiochip_generic_request(struct gpio_chip *gc, unsigned int offset)
 {
 #ifdef CONFIG_PINCTRL
 	if (list_empty(&gc->gpiodev->pin_ranges))
@@ -1651,7 +1651,7 @@ EXPORT_SYMBOL_GPL(gpiochip_generic_request);
  * @gc: the gpiochip to request the gpio function for
  * @offset: the offset of the GPIO to free from GPIO function
  */
-void gpiochip_generic_free(struct gpio_chip *gc, unsigned offset)
+void gpiochip_generic_free(struct gpio_chip *gc, unsigned int offset)
 {
 	pinctrl_gpio_free(gc->gpiodev->base + offset);
 }
@@ -1663,7 +1663,7 @@ EXPORT_SYMBOL_GPL(gpiochip_generic_free);
  * @offset: the offset of the GPIO to apply the configuration
  * @config: the configuration to be applied
  */
-int gpiochip_generic_config(struct gpio_chip *gc, unsigned offset,
+int gpiochip_generic_config(struct gpio_chip *gc, unsigned int offset,
 			    unsigned long config)
 {
 	return pinctrl_gpio_set_config(gc->gpiodev->base + offset, config);
@@ -1994,7 +1994,7 @@ void gpiod_free(struct gpio_desc *desc)
  * help with diagnostics, and knowing that the signal is used as a GPIO
  * can help avoid accidentally multiplexing it to another controller.
  */
-const char *gpiochip_is_requested(struct gpio_chip *gc, unsigned offset)
+const char *gpiochip_is_requested(struct gpio_chip *gc, unsigned int offset)
 {
 	struct gpio_desc *desc;
 
@@ -2098,7 +2098,7 @@ static int gpio_set_config(struct gpio_desc *desc, enum pin_config_param mode)
 {
 	struct gpio_chip *gc = desc->gdev->chip;
 	unsigned long config;
-	unsigned arg;
+	unsigned int arg;
 
 	switch (mode) {
 	case PIN_CONFIG_BIAS_PULL_DOWN:
@@ -2354,7 +2354,7 @@ EXPORT_SYMBOL_GPL(gpiod_set_config);
  * 0 on success, %-ENOTSUPP if the controller doesn't support setting the
  * debounce time.
  */
-int gpiod_set_debounce(struct gpio_desc *desc, unsigned debounce)
+int gpiod_set_debounce(struct gpio_desc *desc, unsigned int debounce)
 {
 	unsigned long config;
 
diff --git a/include/linux/gpio/consumer.h b/include/linux/gpio/consumer.h
index 901aab89d025..ef49307611d2 100644
--- a/include/linux/gpio/consumer.h
+++ b/include/linux/gpio/consumer.h
@@ -158,7 +158,7 @@ int gpiod_set_raw_array_value_cansleep(unsigned int array_size,
 				       unsigned long *value_bitmap);
 
 int gpiod_set_config(struct gpio_desc *desc, unsigned long config);
-int gpiod_set_debounce(struct gpio_desc *desc, unsigned debounce);
+int gpiod_set_debounce(struct gpio_desc *desc, unsigned int debounce);
 int gpiod_set_transitory(struct gpio_desc *desc, bool transitory);
 void gpiod_toggle_active_low(struct gpio_desc *desc);
 
@@ -481,7 +481,7 @@ static inline int gpiod_set_config(struct gpio_desc *desc, unsigned long config)
 	return -ENOSYS;
 }
 
-static inline int gpiod_set_debounce(struct gpio_desc *desc, unsigned debounce)
+static inline int gpiod_set_debounce(struct gpio_desc *desc, unsigned int debounce)
 {
 	/* GPIO can never have been requested */
 	WARN_ON(desc);
-- 
2.28.0

[PATCH v5 02/17] gpiolib: add missed break statement

[Andy Shevchenko]

It's no difference in the functionality, but after the change the code
is less error prone to various checkers.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/gpio/gpiolib.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index f94225d7817b..b45487aace7e 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -2108,6 +2108,7 @@ static int gpio_set_config(struct gpio_desc *desc, enum pin_config_param mode)
 
 	default:
 		arg = 0;
+		break;
 	}
 
 	config = PIN_CONF_PACKED(mode, arg);
-- 
2.28.0

[PATCH v5 03/17] gpiolib: use proper API to pack pin configuration parameters

[Andy Shevchenko]

Instead of open coded macro use, call pinconf_to_config_packed().

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

diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index b45487aace7e..87bb73991337 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -2111,7 +2111,7 @@ static int gpio_set_config(struct gpio_desc *desc, enum pin_config_param mode)
 		break;
 	}
 
-	config = PIN_CONF_PACKED(mode, arg);
+	config = pinconf_to_config_packed(mode, arg);
 	return gpio_do_set_config(gc, gpio_chip_hwgpio(desc), config);
 }
 
-- 
2.28.0

[PATCH v5 04/17] gpiolib: Add temporary variable to gpiod_set_transitory() for cleaner code

[Andy Shevchenko]

Temporary variable that keeps mode allows to neat the code a bit.
It will also help for the future changes.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/gpio/gpiolib.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index 87bb73991337..134f11a84b91 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -2374,6 +2374,7 @@ EXPORT_SYMBOL_GPL(gpiod_set_debounce);
  */
 int gpiod_set_transitory(struct gpio_desc *desc, bool transitory)
 {
+	enum pin_config_param mode = PIN_CONFIG_PERSIST_STATE;
 	struct gpio_chip *gc;
 	unsigned long packed;
 	int gpio;
@@ -2391,8 +2392,7 @@ int gpiod_set_transitory(struct gpio_desc *desc, bool transitory)
 	if (!gc->set_config)
 		return 0;
 
-	packed = pinconf_to_config_packed(PIN_CONFIG_PERSIST_STATE,
-					  !transitory);
+	packed = pinconf_to_config_packed(mode, !transitory);
 	gpio = gpio_chip_hwgpio(desc);
 	rc = gpio_do_set_config(gc, gpio, packed);
 	if (rc == -ENOTSUPP) {
-- 
2.28.0

[PATCH v5 05/17] gpiolib: Extract gpio_set_config_with_argument() for future use

[Andy Shevchenko]

In the future we will need to have a separate function
that takes an arbitrary argument value.

Extract gpio_set_config_with_argument() for that purpose.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/gpio/gpiolib.c | 14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index 134f11a84b91..1e545546fb09 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -2094,10 +2094,19 @@ static int gpio_do_set_config(struct gpio_chip *gc, unsigned int offset,
 	return gc->set_config(gc, offset, config);
 }
 
-static int gpio_set_config(struct gpio_desc *desc, enum pin_config_param mode)
+static int gpio_set_config_with_argument(struct gpio_desc *desc,
+					 enum pin_config_param mode,
+					 u32 argument)
 {
 	struct gpio_chip *gc = desc->gdev->chip;
 	unsigned long config;
+
+	config = pinconf_to_config_packed(mode, argument);
+	return gpio_do_set_config(gc, gpio_chip_hwgpio(desc), config);
+}
+
+static int gpio_set_config(struct gpio_desc *desc, enum pin_config_param mode)
+{
 	unsigned int arg;
 
 	switch (mode) {
@@ -2111,8 +2120,7 @@ static int gpio_set_config(struct gpio_desc *desc, enum pin_config_param mode)
 		break;
 	}
 
-	config = pinconf_to_config_packed(mode, arg);
-	return gpio_do_set_config(gc, gpio_chip_hwgpio(desc), config);
+	return gpio_set_config_with_argument(desc, mode, arg);
 }
 
 static int gpio_set_bias(struct gpio_desc *desc)
-- 
2.28.0

[PATCH v5 06/17] gpiolib: move bias related code from gpio_set_config() to gpio_set_bias()

[Andy Shevchenko]

Move bias related code from gpio_set_config() to gpio_set_bias().

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/gpio/gpiolib.c | 29 ++++++++++++++---------------
 1 file changed, 14 insertions(+), 15 deletions(-)

diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index 1e545546fb09..0691ecbd9a16 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -2107,25 +2107,13 @@ static int gpio_set_config_with_argument(struct gpio_desc *desc,
 
 static int gpio_set_config(struct gpio_desc *desc, enum pin_config_param mode)
 {
-	unsigned int arg;
-
-	switch (mode) {
-	case PIN_CONFIG_BIAS_PULL_DOWN:
-	case PIN_CONFIG_BIAS_PULL_UP:
-		arg = 1;
-		break;
-
-	default:
-		arg = 0;
-		break;
-	}
-
-	return gpio_set_config_with_argument(desc, mode, arg);
+	return gpio_set_config_with_argument(desc, mode, 0);
 }
 
 static int gpio_set_bias(struct gpio_desc *desc)
 {
 	enum pin_config_param bias;
+	unsigned int arg;
 	int ret;
 
 	if (test_bit(FLAG_BIAS_DISABLE, &desc->flags))
@@ -2137,7 +2125,18 @@ static int gpio_set_bias(struct gpio_desc *desc)
 	else
 		return 0;
 
-	ret = gpio_set_config(desc, bias);
+	switch (bias) {
+	case PIN_CONFIG_BIAS_PULL_DOWN:
+	case PIN_CONFIG_BIAS_PULL_UP:
+		arg = 1;
+		break;
+
+	default:
+		arg = 0;
+		break;
+	}
+
+	ret = gpio_set_config_with_argument(desc, bias, arg);
 	if (ret != -ENOTSUPP)
 		return ret;
 
-- 
2.28.0

[PATCH v5 07/17] gpiolib: Extract gpio_set_config_with_argument_optional() helper

[Andy Shevchenko]

This function is useful for internal use in GPIO library.
There will be new user coming, prepare a helper for the new comer
and the existing ones.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/gpio/gpiolib.c | 50 ++++++++++++++++++++++--------------------
 1 file changed, 26 insertions(+), 24 deletions(-)

diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index 0691ecbd9a16..4558af08df23 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -2105,6 +2105,30 @@ static int gpio_set_config_with_argument(struct gpio_desc *desc,
 	return gpio_do_set_config(gc, gpio_chip_hwgpio(desc), config);
 }
 
+static int gpio_set_config_with_argument_optional(struct gpio_desc *desc,
+						  enum pin_config_param mode,
+						  u32 argument)
+{
+	struct device *dev = &desc->gdev->dev;
+	int gpio = gpio_chip_hwgpio(desc);
+	int ret;
+
+	ret = gpio_set_config_with_argument(desc, mode, argument);
+	if (ret != -ENOTSUPP)
+		return ret;
+
+	switch (mode) {
+	case PIN_CONFIG_PERSIST_STATE:
+		if (ret)
+			dev_dbg(dev, "Persistence not supported for GPIO %d\n", gpio);
+		break;
+	default:
+		break;
+	}
+
+	return 0;
+}
+
 static int gpio_set_config(struct gpio_desc *desc, enum pin_config_param mode)
 {
 	return gpio_set_config_with_argument(desc, mode, 0);
@@ -2114,7 +2138,6 @@ static int gpio_set_bias(struct gpio_desc *desc)
 {
 	enum pin_config_param bias;
 	unsigned int arg;
-	int ret;
 
 	if (test_bit(FLAG_BIAS_DISABLE, &desc->flags))
 		bias = PIN_CONFIG_BIAS_DISABLE;
@@ -2136,11 +2159,7 @@ static int gpio_set_bias(struct gpio_desc *desc)
 		break;
 	}
 
-	ret = gpio_set_config_with_argument(desc, bias, arg);
-	if (ret != -ENOTSUPP)
-		return ret;
-
-	return 0;
+	return gpio_set_config_with_argument_optional(desc, bias, arg);
 }
 
 /**
@@ -2382,10 +2401,6 @@ EXPORT_SYMBOL_GPL(gpiod_set_debounce);
 int gpiod_set_transitory(struct gpio_desc *desc, bool transitory)
 {
 	enum pin_config_param mode = PIN_CONFIG_PERSIST_STATE;
-	struct gpio_chip *gc;
-	unsigned long packed;
-	int gpio;
-	int rc;
 
 	VALIDATE_DESC(desc);
 	/*
@@ -2395,20 +2410,7 @@ int gpiod_set_transitory(struct gpio_desc *desc, bool transitory)
 	assign_bit(FLAG_TRANSITORY, &desc->flags, transitory);
 
 	/* If the driver supports it, set the persistence state now */
-	gc = desc->gdev->chip;
-	if (!gc->set_config)
-		return 0;
-
-	packed = pinconf_to_config_packed(mode, !transitory);
-	gpio = gpio_chip_hwgpio(desc);
-	rc = gpio_do_set_config(gc, gpio, packed);
-	if (rc == -ENOTSUPP) {
-		dev_dbg(&desc->gdev->dev, "Persistence not supported for GPIO %d\n",
-				gpio);
-		return 0;
-	}
-
-	return rc;
+	return gpio_set_config_with_argument_optional(desc, mode, !transitory);
 }
 EXPORT_SYMBOL_GPL(gpiod_set_transitory);
 
-- 
2.28.0

[PATCH v5 08/17] gpiolib: Extract gpio_set_debounce_timeout() for internal use

[Andy Shevchenko]

In some cases we would like to have debounce setter which doesn't fail
when feature is not supported by a controller.

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

diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index 4558af08df23..9d23fbf1f7cd 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -2162,6 +2162,13 @@ static int gpio_set_bias(struct gpio_desc *desc)
 	return gpio_set_config_with_argument_optional(desc, bias, arg);
 }
 
+int gpio_set_debounce_timeout(struct gpio_desc *desc, unsigned int debounce)
+{
+	enum pin_config_param mode = PIN_CONFIG_PERSIST_STATE;
+
+	return gpio_set_config_with_argument_optional(desc, mode, debounce);
+}
+
 /**
  * gpiod_direction_input - set the GPIO direction to input
  * @desc:	GPIO to set to input
diff --git a/drivers/gpio/gpiolib.h b/drivers/gpio/gpiolib.h
index 16bc5731673c..9b1a1c782704 100644
--- a/drivers/gpio/gpiolib.h
+++ b/drivers/gpio/gpiolib.h
@@ -136,6 +136,7 @@ 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 gpio_set_debounce_timeout(struct gpio_desc *desc, unsigned int debounce);
 int gpiod_hog(struct gpio_desc *desc, const char *name,
 		unsigned long lflags, enum gpiod_flags dflags);
 
-- 
2.28.0

[PATCH v5 09/17] 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>
Acked-by: Linus Walleij <linus.walleij@linaro.org>
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 v5 10/17] 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>
Acked-by: Linus Walleij <linus.walleij@linaro.org>
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 v5 11/17] 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>
Acked-by: Linus Walleij <linus.walleij@linaro.org>
---
 drivers/gpio/gpiolib-acpi.c | 18 ++++++++++++++++++
 drivers/gpio/gpiolib-acpi.h |  2 ++
 2 files changed, 20 insertions(+)

diff --git a/drivers/gpio/gpiolib-acpi.c b/drivers/gpio/gpiolib-acpi.c
index c127b410a7a2..6cbad96be866 100644
--- a/drivers/gpio/gpiolib-acpi.c
+++ b/drivers/gpio/gpiolib-acpi.c
@@ -299,6 +299,10 @@ static acpi_status acpi_gpiochip_alloc_event(struct acpi_resource *ares,
 		return AE_OK;
 	}
 
+	ret = gpio_set_debounce_timeout(desc, agpio->debounce_timeout);
+	if (ret)
+		goto fail_free_desc;
+
 	ret = gpiochip_lock_as_irq(chip, pin);
 	if (ret) {
 		dev_err(chip->parent,
@@ -664,6 +668,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 +966,10 @@ int acpi_dev_gpio_irq_get(struct acpi_device *adev, int index)
 			if (ret < 0)
 				return ret;
 
+			ret = gpio_set_debounce_timeout(desc, info.debounce);
+			if (ret)
+				return ret;
+
 			irq_flags = acpi_dev_get_irq_type(info.triggering,
 							  info.polarity);
 
@@ -1048,6 +1057,7 @@ acpi_gpio_adr_space_handler(u32 function, acpi_physical_address address,
 		if (!found) {
 			enum gpiod_flags flags = acpi_gpio_to_gpiod_flags(agpio);
 			const char *label = "ACPI:OpRegion";
+			int ret;
 
 			desc = gpiochip_request_own_desc(chip, pin, label,
 							 GPIO_ACTIVE_HIGH,
@@ -1058,6 +1068,14 @@ acpi_gpio_adr_space_handler(u32 function, acpi_physical_address address,
 				goto out;
 			}
 
+			ret = gpio_set_debounce_timeout(desc, agpio->debounce_timeout);
+			if (ret) {
+				status = AE_ERROR;
+				gpiochip_free_own_desc(desc);
+				mutex_unlock(&achip->conn_lock);
+				goto out;
+			}
+
 			conn = kzalloc(sizeof(*conn), GFP_KERNEL);
 			if (!conn) {
 				status = AE_NO_MEMORY;
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 v5 12/17] gpiolib: acpi: Move acpi_gpio_to_gpiod_flags() upper in the code

[Andy Shevchenko]

Move acpi_gpio_to_gpiod_flags() upper in the code to allow further refactoring.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/gpio/gpiolib-acpi.c | 66 ++++++++++++++++++-------------------
 1 file changed, 33 insertions(+), 33 deletions(-)

diff --git a/drivers/gpio/gpiolib-acpi.c b/drivers/gpio/gpiolib-acpi.c
index 6cbad96be866..999fd3816298 100644
--- a/drivers/gpio/gpiolib-acpi.c
+++ b/drivers/gpio/gpiolib-acpi.c
@@ -205,6 +205,39 @@ static void acpi_gpiochip_request_irqs(struct acpi_gpio_chip *acpi_gpio)
 		acpi_gpiochip_request_irq(acpi_gpio, event);
 }
 
+static enum gpiod_flags
+acpi_gpio_to_gpiod_flags(const struct acpi_resource_gpio *agpio)
+{
+	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, if it is pulled down we assume default to be low,
+		 * otherwise we leave pin untouched.
+		 */
+		switch (agpio->pin_config) {
+		case ACPI_PIN_CONFIG_PULLUP:
+			return GPIOD_OUT_HIGH;
+		case ACPI_PIN_CONFIG_PULLDOWN:
+			return GPIOD_OUT_LOW;
+		default:
+			break;
+		}
+	default:
+		break;
+	}
+
+	/*
+	 * Assume that the BIOS has configured the direction and pull
+	 * accordingly.
+	 */
+	return GPIOD_ASIS;
+}
+
 static bool acpi_gpio_in_ignore_list(const char *controller_in, int pin_in)
 {
 	const char *controller, *pin_str;
@@ -530,39 +563,6 @@ 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)
-{
-	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, if it is pulled down we assume default to be low,
-		 * otherwise we leave pin untouched.
-		 */
-		switch (agpio->pin_config) {
-		case ACPI_PIN_CONFIG_PULLUP:
-			return GPIOD_OUT_HIGH;
-		case ACPI_PIN_CONFIG_PULLDOWN:
-			return GPIOD_OUT_LOW;
-		default:
-			break;
-		}
-	default:
-		break;
-	}
-
-	/*
-	 * Assume that the BIOS has configured the direction and pull
-	 * accordingly.
-	 */
-	return GPIOD_ASIS;
-}
-
 static int
 __acpi_gpio_update_gpiod_flags(enum gpiod_flags *flags, enum gpiod_flags update)
 {
-- 
2.28.0

[PATCH v5 13/17] gpiolib: acpi: Make acpi_gpio_to_gpiod_flags() usable for GpioInt()

[Andy Shevchenko]

GpioInt() implies input configuration of the pin. Add this to
the acpi_gpio_to_gpiod_flags() and make usable for GpioInt().

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

diff --git a/drivers/gpio/gpiolib-acpi.c b/drivers/gpio/gpiolib-acpi.c
index 999fd3816298..ada6371f6c2d 100644
--- a/drivers/gpio/gpiolib-acpi.c
+++ b/drivers/gpio/gpiolib-acpi.c
@@ -208,6 +208,10 @@ 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)
 {
+	/* GpioInt() implies input configuration */
+	if (agpio->connection_type == ACPI_RESOURCE_GPIO_TYPE_INT)
+		return GPIOD_IN;
+
 	switch (agpio->io_restriction) {
 	case ACPI_IO_RESTRICT_INPUT:
 		return GPIOD_IN;
@@ -669,6 +673,7 @@ static int acpi_populate_gpio_lookup(struct acpi_resource *ares, void *data)
 					      agpio->pin_table[pin_index]);
 		lookup->info.pin_config = agpio->pin_config;
 		lookup->info.debounce = agpio->debounce_timeout;
+		lookup->info.flags = acpi_gpio_to_gpiod_flags(agpio);
 		lookup->info.gpioint = gpioint;
 
 		/*
@@ -679,11 +684,9 @@ 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);
 			lookup->info.polarity = lookup->active_low;
 		}
 	}
-- 
2.28.0

[PATCH v5 14/17] gpiolib: acpi: Extract acpi_request_own_gpiod() helper

[Andy Shevchenko]

It appears that we are using similar code excerpts for ACPI OpRegion
and event handling. Deduplicate those excerpts by extracting a new
acpi_request_own_gpiod() helper.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/gpio/gpiolib-acpi.c | 44 +++++++++++++++++++------------------
 1 file changed, 23 insertions(+), 21 deletions(-)

diff --git a/drivers/gpio/gpiolib-acpi.c b/drivers/gpio/gpiolib-acpi.c
index ada6371f6c2d..f8f1db0e3ccb 100644
--- a/drivers/gpio/gpiolib-acpi.c
+++ b/drivers/gpio/gpiolib-acpi.c
@@ -242,6 +242,27 @@ acpi_gpio_to_gpiod_flags(const struct acpi_resource_gpio *agpio)
 	return GPIOD_ASIS;
 }
 
+static struct gpio_desc *acpi_request_own_gpiod(struct gpio_chip *chip,
+						struct acpi_resource_gpio *agpio,
+						unsigned int index,
+						const char *label)
+{
+	enum gpiod_flags flags = acpi_gpio_to_gpiod_flags(agpio);
+	unsigned int pin = agpio->pin_table[index];
+	struct gpio_desc *desc;
+	int ret;
+
+	desc = gpiochip_request_own_desc(chip, pin, label, GPIO_ACTIVE_HIGH, flags);
+	if (IS_ERR(desc))
+		return desc;
+
+	ret = gpio_set_debounce_timeout(desc, agpio->debounce_timeout);
+	if (ret)
+		gpiochip_free_own_desc(desc);
+
+	return ret ? ERR_PTR(ret) : desc;
+}
+
 static bool acpi_gpio_in_ignore_list(const char *controller_in, int pin_in)
 {
 	const char *controller, *pin_str;
@@ -327,8 +348,7 @@ static acpi_status acpi_gpiochip_alloc_event(struct acpi_resource *ares,
 	if (!handler)
 		return AE_OK;
 
-	desc = gpiochip_request_own_desc(chip, pin, "ACPI:Event",
-					 GPIO_ACTIVE_HIGH, GPIOD_IN);
+	desc = acpi_request_own_gpiod(chip, agpio, 0, "ACPI:Event");
 	if (IS_ERR(desc)) {
 		dev_err(chip->parent,
 			"Failed to request GPIO for pin 0x%04X, err %ld\n",
@@ -336,10 +356,6 @@ static acpi_status acpi_gpiochip_alloc_event(struct acpi_resource *ares,
 		return AE_OK;
 	}
 
-	ret = gpio_set_debounce_timeout(desc, agpio->debounce_timeout);
-	if (ret)
-		goto fail_free_desc;
-
 	ret = gpiochip_lock_as_irq(chip, pin);
 	if (ret) {
 		dev_err(chip->parent,
@@ -1058,27 +1074,13 @@ acpi_gpio_adr_space_handler(u32 function, acpi_physical_address address,
 		}
 
 		if (!found) {
-			enum gpiod_flags flags = acpi_gpio_to_gpiod_flags(agpio);
-			const char *label = "ACPI:OpRegion";
-			int ret;
-
-			desc = gpiochip_request_own_desc(chip, pin, label,
-							 GPIO_ACTIVE_HIGH,
-							 flags);
+			desc = acpi_request_own_gpiod(chip, agpio, i, "ACPI:OpRegion");
 			if (IS_ERR(desc)) {
 				status = AE_ERROR;
 				mutex_unlock(&achip->conn_lock);
 				goto out;
 			}
 
-			ret = gpio_set_debounce_timeout(desc, agpio->debounce_timeout);
-			if (ret) {
-				status = AE_ERROR;
-				gpiochip_free_own_desc(desc);
-				mutex_unlock(&achip->conn_lock);
-				goto out;
-			}
-
 			conn = kzalloc(sizeof(*conn), GFP_KERNEL);
 			if (!conn) {
 				status = AE_NO_MEMORY;
-- 
2.28.0

[PATCH v5 15/17] 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>
Acked-by: Linus Walleij <linus.walleij@linaro.org>
---
 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 f8f1db0e3ccb..31008b0aef77 100644
--- a/drivers/gpio/gpiolib-acpi.c
+++ b/drivers/gpio/gpiolib-acpi.c
@@ -657,7 +657,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;
@@ -673,7 +673,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++;
@@ -818,7 +818,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 {
@@ -1014,7 +1014,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;
@@ -1037,7 +1037,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

[PATCH v5 16/17] gpiolib: acpi: Use BIT() macro to increase readability

[Andy Shevchenko]

We may use BIT() macro to increase readability in
acpi_gpio_adr_space_handler().

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

diff --git a/drivers/gpio/gpiolib-acpi.c b/drivers/gpio/gpiolib-acpi.c
index 31008b0aef77..b9c3140cbd6d 100644
--- a/drivers/gpio/gpiolib-acpi.c
+++ b/drivers/gpio/gpiolib-acpi.c
@@ -1097,8 +1097,7 @@ acpi_gpio_adr_space_handler(u32 function, acpi_physical_address address,
 		mutex_unlock(&achip->conn_lock);
 
 		if (function == ACPI_WRITE)
-			gpiod_set_raw_value_cansleep(desc,
-						     !!((1 << i) & *value));
+			gpiod_set_raw_value_cansleep(desc, !!(*value & BIT(i)));
 		else
 			*value |= (u64)gpiod_get_raw_value_cansleep(desc) << i;
 	}
-- 
2.28.0

[PATCH v5 17/17] gpiolib: acpi: Make Intel GPIO tree official for GPIO ACPI work

[Andy Shevchenko]

Make Intel GPIO tree official for GPIO ACPI work.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 MAINTAINERS | 1 +
 1 file changed, 1 insertion(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index e73636b75f29..53236b2ea0af 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -7483,6 +7483,7 @@ M:	Andy Shevchenko <andriy.shevchenko@linux.intel.com>
 L:	linux-gpio@vger.kernel.org
 L:	linux-acpi@vger.kernel.org
 S:	Maintained
+T:	git git://git.kernel.org/pub/scm/linux/kernel/git/andy/linux-gpio-intel.git
 F:	Documentation/firmware-guide/acpi/gpio-properties.rst
 F:	drivers/gpio/gpiolib-acpi.c
 F:	drivers/gpio/gpiolib-acpi.h
-- 
2.28.0

Re: [PATCH v5 00/17] gpiolib: acpi: pin configuration fixes

[Hans de Goede]

Hi,

On 11/9/20 9:53 PM, Andy Shevchenko wrote:
> There are fixes (and plenty cleanups) that allow to take into consideration
> more parameters in ACPI, i.e. bias for GpioInt() and debounce timeout
> for Operation Regions, Events and GpioInt() resources.
> 
> During review Hans noted, that gpiod_set_debounce() returns -ENOTSUPP for
> the cases when feature is not supported either by driver or a controller.
> 
> It appears that we have slightly messy API here:
> 
>   FUNC			Relation with ENOTSUPP
> 
>   gpiod_set_config()	 returns if not supported
>   gpiod_set_debounce()	 as gpiod_set_config() above
>   gpio_set_debounce()	 legacy wrapper on top of gpiod_set_debounce()
>   gpiod_set_transitory() skips it (returns okay) with debug message
>   gpio_set_config()	 returns if not supported
>   gpio_set_bias()	 skips it (returns okay)
> 
> Last two functions are internal to GPIO library, while the rest is
> exported API. In order to be consistent with both naming schemas
> the series introduces gpio_set_debounce_timeout() that considers
> the feature optional. New API is only for internal use.
> 
> While at it, the few first patches do clean up the current GPIO library
> code to unify it to some extend.
> 
> The above is followed by changes made in ACPI GPIO library part.
> 
> The bias patch highly depends on Intel pin control driver changes
> (they are material for v5.10 [1]), due to this and amount of the
> prerequisite changes this series is probably not supposed to be
> backported (at least right now).

The entire series looks good to me, feel free to add my:

Reviewed-by: Hans de Goede <hdegoede@redhat.com>

to patches 1-16 (patch 17 makes sense to me, but I do not feel
I'm the right person to ack it).

Regards,

Hans




> 
> The last patch adds Intel GPIO tree as official one for ACPI GPIO
> changes.
> 
> Assuming [1] makes v5.10 this series can be sent as PR to Linus
> for v5.11 cycle.
> 
> Note, some patches are also depend to the code from GPIO fixes / for-next
> repositories. Unfortunately there is no one repository which contains all
> up to date for-next changes against GPIO subsystem. That's why I have merged
> Bart's for-current followed by Linus' fixes followed by Bart's for-next
> followed by Linus' for-next branches as prerequisites to the series.
> 
> Cc: Jamie McClymont <jamie@kwiius.com>
> 
> [1]: https://lore.kernel.org/linux-gpio/20201106181938.GA41213@black.fi.intel.com/
> 
> Changelog v5:
> - introduced gpio_set_debounce_timeout()
> - made a prerequisite refactoring in GPIO library code
> - updated the rest accordingly
> 
> Changelog v4:
> - extended debounce setting to ACPI events and Operation Regions
> - added Ack (Linus)
> - added few more cleanup patches, including MAINTAINERS update
> 
> Changelog v3:
> - dropped upstreamed OF patch
> - added debounce fix
> 
> Andy Shevchenko (17):
>   gpiolib: Replace unsigned by unsigned int
>   gpiolib: add missed break statement
>   gpiolib: use proper API to pack pin configuration parameters
>   gpiolib: Add temporary variable to gpiod_set_transitory() for cleaner
>     code
>   gpiolib: Extract gpio_set_config_with_argument() for future use
>   gpiolib: move bias related code from gpio_set_config() to
>     gpio_set_bias()
>   gpiolib: Extract gpio_set_config_with_argument_optional() helper
>   gpiolib: Extract gpio_set_debounce_timeout() for internal use
>   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: Move acpi_gpio_to_gpiod_flags() upper in the code
>   gpiolib: acpi: Make acpi_gpio_to_gpiod_flags() usable for GpioInt()
>   gpiolib: acpi: Extract acpi_request_own_gpiod() helper
>   gpiolib: acpi: Convert pin_index to be u16
>   gpiolib: acpi: Use BIT() macro to increase readability
>   gpiolib: acpi: Make Intel GPIO tree official for GPIO ACPI work
> 
>  MAINTAINERS                   |   1 +
>  drivers/gpio/gpiolib-acpi.c   | 130 ++++++++++++++++++++--------------
>  drivers/gpio/gpiolib-acpi.h   |   2 +
>  drivers/gpio/gpiolib.c        |  97 ++++++++++++++-----------
>  drivers/gpio/gpiolib.h        |   1 +
>  include/linux/gpio/consumer.h |   4 +-
>  6 files changed, 141 insertions(+), 94 deletions(-)
> 

Re: [PATCH v5 00/17] gpiolib: acpi: pin configuration fixes

[Andy Shevchenko]

On Tue, Nov 10, 2020 at 4:10 PM Hans de Goede <hdegoede@redhat.com> wrote:
> On 11/9/20 9:53 PM, Andy Shevchenko wrote:
> > There are fixes (and plenty cleanups) that allow to take into consideration
> > more parameters in ACPI, i.e. bias for GpioInt() and debounce timeout
> > for Operation Regions, Events and GpioInt() resources.
> >
> > During review Hans noted, that gpiod_set_debounce() returns -ENOTSUPP for
> > the cases when feature is not supported either by driver or a controller.
> >
> > It appears that we have slightly messy API here:
> >
> >   FUNC                        Relation with ENOTSUPP
> >
> >   gpiod_set_config()   returns if not supported
> >   gpiod_set_debounce()         as gpiod_set_config() above
> >   gpio_set_debounce()  legacy wrapper on top of gpiod_set_debounce()
> >   gpiod_set_transitory() skips it (returns okay) with debug message
> >   gpio_set_config()    returns if not supported
> >   gpio_set_bias()      skips it (returns okay)
> >
> > Last two functions are internal to GPIO library, while the rest is
> > exported API. In order to be consistent with both naming schemas
> > the series introduces gpio_set_debounce_timeout() that considers
> > the feature optional. New API is only for internal use.
> >
> > While at it, the few first patches do clean up the current GPIO library
> > code to unify it to some extend.
> >
> > The above is followed by changes made in ACPI GPIO library part.
> >
> > The bias patch highly depends on Intel pin control driver changes
> > (they are material for v5.10 [1]), due to this and amount of the
> > prerequisite changes this series is probably not supposed to be
> > backported (at least right now).
>
> The entire series looks good to me, feel free to add my:
>
> Reviewed-by: Hans de Goede <hdegoede@redhat.com>

Thanks!

I still would like Mika to have a look and perform a couple more tests myself.
And I have one emergency task right now, so I think we have time to
let others have a look and comment / test / etc.

>
> to patches 1-16 (patch 17 makes sense to me, but I do not feel
> I'm the right person to ack it).
>
> Regards,
>
> Hans
>
>
>
>
> >
> > The last patch adds Intel GPIO tree as official one for ACPI GPIO
> > changes.
> >
> > Assuming [1] makes v5.10 this series can be sent as PR to Linus
> > for v5.11 cycle.
> >
> > Note, some patches are also depend to the code from GPIO fixes / for-next
> > repositories. Unfortunately there is no one repository which contains all
> > up to date for-next changes against GPIO subsystem. That's why I have merged
> > Bart's for-current followed by Linus' fixes followed by Bart's for-next
> > followed by Linus' for-next branches as prerequisites to the series.
> >
> > Cc: Jamie McClymont <jamie@kwiius.com>
> >
> > [1]: https://lore.kernel.org/linux-gpio/20201106181938.GA41213@black.fi.intel.com/
> >
> > Changelog v5:
> > - introduced gpio_set_debounce_timeout()
> > - made a prerequisite refactoring in GPIO library code
> > - updated the rest accordingly
> >
> > Changelog v4:
> > - extended debounce setting to ACPI events and Operation Regions
> > - added Ack (Linus)
> > - added few more cleanup patches, including MAINTAINERS update
> >
> > Changelog v3:
> > - dropped upstreamed OF patch
> > - added debounce fix
> >
> > Andy Shevchenko (17):
> >   gpiolib: Replace unsigned by unsigned int
> >   gpiolib: add missed break statement
> >   gpiolib: use proper API to pack pin configuration parameters
> >   gpiolib: Add temporary variable to gpiod_set_transitory() for cleaner
> >     code
> >   gpiolib: Extract gpio_set_config_with_argument() for future use
> >   gpiolib: move bias related code from gpio_set_config() to
> >     gpio_set_bias()
> >   gpiolib: Extract gpio_set_config_with_argument_optional() helper
> >   gpiolib: Extract gpio_set_debounce_timeout() for internal use
> >   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: Move acpi_gpio_to_gpiod_flags() upper in the code
> >   gpiolib: acpi: Make acpi_gpio_to_gpiod_flags() usable for GpioInt()
> >   gpiolib: acpi: Extract acpi_request_own_gpiod() helper
> >   gpiolib: acpi: Convert pin_index to be u16
> >   gpiolib: acpi: Use BIT() macro to increase readability
> >   gpiolib: acpi: Make Intel GPIO tree official for GPIO ACPI work
> >
> >  MAINTAINERS                   |   1 +
> >  drivers/gpio/gpiolib-acpi.c   | 130 ++++++++++++++++++++--------------
> >  drivers/gpio/gpiolib-acpi.h   |   2 +
> >  drivers/gpio/gpiolib.c        |  97 ++++++++++++++-----------
> >  drivers/gpio/gpiolib.h        |   1 +
> >  include/linux/gpio/consumer.h |   4 +-
> >  6 files changed, 141 insertions(+), 94 deletions(-)
> >
>


-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v5 01/17] gpiolib: Replace unsigned by unsigned int

[Mika Westerberg]

On Mon, Nov 09, 2020 at 10:53:16PM +0200, Andy Shevchenko wrote:
> Replace unsigned by unsigned int in GPIO library code.
> Note, legacy API left untouched.
> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

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

Re: [PATCH v5 02/17] gpiolib: add missed break statement

[Mika Westerberg]

On Mon, Nov 09, 2020 at 10:53:17PM +0200, Andy Shevchenko wrote:
> It's no difference in the functionality, but after the change the code
> is less error prone to various checkers.
> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

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

Re: [PATCH v5 03/17] gpiolib: use proper API to pack pin configuration parameters

[Mika Westerberg]

On Mon, Nov 09, 2020 at 10:53:18PM +0200, Andy Shevchenko wrote:
> Instead of open coded macro use, call pinconf_to_config_packed().
> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

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

Re: [PATCH v5 04/17] gpiolib: Add temporary variable to gpiod_set_transitory() for cleaner code

[Mika Westerberg]

On Mon, Nov 09, 2020 at 10:53:19PM +0200, Andy Shevchenko wrote:
> Temporary variable that keeps mode allows to neat the code a bit.
> It will also help for the future changes.
> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

Not really sure if this is good change or not. Instead of constant you
now introduce a useless variable just to get them to the same line.

To me this looks better and reads easier:

	packed = pinconf_to_config_packed(PIN_CONFIG_PERSIST_STATE, !transitory);

But not insisting so if GPIO maintainers are fine then no objections :)

Re: [PATCH v5 05/17] gpiolib: Extract gpio_set_config_with_argument() for future use

[Mika Westerberg]

On Mon, Nov 09, 2020 at 10:53:20PM +0200, Andy Shevchenko wrote:
> In the future we will need to have a separate function
> that takes an arbitrary argument value.
> 
> Extract gpio_set_config_with_argument() for that purpose.
> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

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

Re: [PATCH v5 06/17] gpiolib: move bias related code from gpio_set_config() to gpio_set_bias()

[Mika Westerberg]

On Mon, Nov 09, 2020 at 10:53:21PM +0200, Andy Shevchenko wrote:
> Move bias related code from gpio_set_config() to gpio_set_bias().
> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

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

Re: [PATCH v5 07/17] gpiolib: Extract gpio_set_config_with_argument_optional() helper

[Mika Westerberg]

On Mon, Nov 09, 2020 at 10:53:22PM +0200, Andy Shevchenko wrote:
> This function is useful for internal use in GPIO library.
> There will be new user coming, prepare a helper for the new comer
> and the existing ones.
> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

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

Re: [PATCH v5 08/17] gpiolib: Extract gpio_set_debounce_timeout() for internal use

[Mika Westerberg]

On Mon, Nov 09, 2020 at 10:53:23PM +0200, Andy Shevchenko wrote:
> In some cases we would like to have debounce setter which doesn't fail
> when feature is not supported by a controller.
> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
>  drivers/gpio/gpiolib.c | 7 +++++++
>  drivers/gpio/gpiolib.h | 1 +
>  2 files changed, 8 insertions(+)
> 
> diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
> index 4558af08df23..9d23fbf1f7cd 100644
> --- a/drivers/gpio/gpiolib.c
> +++ b/drivers/gpio/gpiolib.c
> @@ -2162,6 +2162,13 @@ static int gpio_set_bias(struct gpio_desc *desc)
>  	return gpio_set_config_with_argument_optional(desc, bias, arg);
>  }
>  
> +int gpio_set_debounce_timeout(struct gpio_desc *desc, unsigned int debounce)
> +{
> +	enum pin_config_param mode = PIN_CONFIG_PERSIST_STATE;
> +
> +	return gpio_set_config_with_argument_optional(desc, mode, debounce);

Again I think mode variable is pretty useless here and does not improve
readability.

Re: [PATCH v5 04/17] gpiolib: Add temporary variable to gpiod_set_transitory() for cleaner code

[Andy Shevchenko]

On Wed, Nov 11, 2020 at 5:33 PM Mika Westerberg
<mika.westerberg@linux.intel.com> wrote:
>
> On Mon, Nov 09, 2020 at 10:53:19PM +0200, Andy Shevchenko wrote:
> > Temporary variable that keeps mode allows to neat the code a bit.
> > It will also help for the future changes.
> >
> > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
>
> Not really sure if this is good change or not. Instead of constant you
> now introduce a useless variable just to get them to the same line.

No, it's useful in a patch in the series as promised in the commit
message. That variable reduces a line length a lot there.

> To me this looks better and reads easier:
>
>         packed = pinconf_to_config_packed(PIN_CONFIG_PERSIST_STATE, !transitory);
>
> But not insisting so if GPIO maintainers are fine then no objections :)

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v5 11/17] gpiolib: acpi: Take into account debounce settings

[Mika Westerberg]

On Mon, Nov 09, 2020 at 10:53:26PM +0200, Andy Shevchenko wrote:
> 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>
> Acked-by: Linus Walleij <linus.walleij@linaro.org>
> ---
>  drivers/gpio/gpiolib-acpi.c | 18 ++++++++++++++++++
>  drivers/gpio/gpiolib-acpi.h |  2 ++
>  2 files changed, 20 insertions(+)
> 
> diff --git a/drivers/gpio/gpiolib-acpi.c b/drivers/gpio/gpiolib-acpi.c
> index c127b410a7a2..6cbad96be866 100644
> --- a/drivers/gpio/gpiolib-acpi.c
> +++ b/drivers/gpio/gpiolib-acpi.c
> @@ -299,6 +299,10 @@ static acpi_status acpi_gpiochip_alloc_event(struct acpi_resource *ares,
>  		return AE_OK;
>  	}
>  
> +	ret = gpio_set_debounce_timeout(desc, agpio->debounce_timeout);
> +	if (ret)
> +		goto fail_free_desc;
> +
>  	ret = gpiochip_lock_as_irq(chip, pin);
>  	if (ret) {
>  		dev_err(chip->parent,
> @@ -664,6 +668,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 +966,10 @@ int acpi_dev_gpio_irq_get(struct acpi_device *adev, int index)
>  			if (ret < 0)
>  				return ret;
>  
> +			ret = gpio_set_debounce_timeout(desc, info.debounce);
> +			if (ret)
> +				return ret;
> +
>  			irq_flags = acpi_dev_get_irq_type(info.triggering,
>  							  info.polarity);
>  
> @@ -1048,6 +1057,7 @@ acpi_gpio_adr_space_handler(u32 function, acpi_physical_address address,
>  		if (!found) {
>  			enum gpiod_flags flags = acpi_gpio_to_gpiod_flags(agpio);
>  			const char *label = "ACPI:OpRegion";
> +			int ret;
>  
>  			desc = gpiochip_request_own_desc(chip, pin, label,
>  							 GPIO_ACTIVE_HIGH,
> @@ -1058,6 +1068,14 @@ acpi_gpio_adr_space_handler(u32 function, acpi_physical_address address,
>  				goto out;
>  			}
>  
> +			ret = gpio_set_debounce_timeout(desc, agpio->debounce_timeout);
> +			if (ret) {
> +				status = AE_ERROR;
> +				gpiochip_free_own_desc(desc);
> +				mutex_unlock(&achip->conn_lock);

Nit: I think you can set status outside of the critical section.

Otherwise looks good,

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

Re: [PATCH v5 07/17] gpiolib: Extract gpio_set_config_with_argument_optional() helper

[Andy Shevchenko]

On Mon, Nov 9, 2020 at 10:55 PM Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
>
> This function is useful for internal use in GPIO library.
> There will be new user coming, prepare a helper for the new comer
> and the existing ones.

Just to show how temporary variable is re-used here

>         enum pin_config_param mode = PIN_CONFIG_PERSIST_STATE;

> +       return gpio_set_config_with_argument_optional(desc, mode, !transitory);

Leaving constant in the parameter line will make it either long or ugly split.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v5 08/17] gpiolib: Extract gpio_set_debounce_timeout() for internal use

[Andy Shevchenko]

On Wed, Nov 11, 2020 at 5:40 PM Mika Westerberg
<mika.westerberg@linux.intel.com> wrote:
> On Mon, Nov 09, 2020 at 10:53:23PM +0200, Andy Shevchenko wrote:

...

> Again I think mode variable is pretty useless here and does not improve
> readability.

You mean something like

    return gpio_set_config_with_argument_optional(desc,
                PIN_CONFIG_INPUT_DEBOUNCE, debounce);

is better?

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v5 12/17] gpiolib: acpi: Move acpi_gpio_to_gpiod_flags() upper in the code

[Mika Westerberg]

On Mon, Nov 09, 2020 at 10:53:27PM +0200, Andy Shevchenko wrote:
> Move acpi_gpio_to_gpiod_flags() upper in the code to allow further refactoring.
> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

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

Re: [PATCH v5 13/17] gpiolib: acpi: Make acpi_gpio_to_gpiod_flags() usable for GpioInt()

[Mika Westerberg]

On Mon, Nov 09, 2020 at 10:53:28PM +0200, Andy Shevchenko wrote:
> GpioInt() implies input configuration of the pin. Add this to
> the acpi_gpio_to_gpiod_flags() and make usable for GpioInt().
> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

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

Re: [PATCH v5 14/17] gpiolib: acpi: Extract acpi_request_own_gpiod() helper

[Mika Westerberg]

On Mon, Nov 09, 2020 at 10:53:29PM +0200, Andy Shevchenko wrote:
> It appears that we are using similar code excerpts for ACPI OpRegion
> and event handling. Deduplicate those excerpts by extracting a new
> acpi_request_own_gpiod() helper.
> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

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

Re: [PATCH v5 08/17] gpiolib: Extract gpio_set_debounce_timeout() for internal use

[Mika Westerberg]

On Wed, Nov 11, 2020 at 05:46:55PM +0200, Andy Shevchenko wrote:
> On Wed, Nov 11, 2020 at 5:40 PM Mika Westerberg
> <mika.westerberg@linux.intel.com> wrote:
> > On Mon, Nov 09, 2020 at 10:53:23PM +0200, Andy Shevchenko wrote:
> 
> ...
> 
> > Again I think mode variable is pretty useless here and does not improve
> > readability.
> 
> You mean something like
> 
>     return gpio_set_config_with_argument_optional(desc,
>                 PIN_CONFIG_INPUT_DEBOUNCE, debounce);
> 
> is better?

Yes.

Re: [PATCH v5 15/17] gpiolib: acpi: Convert pin_index to be u16

[Mika Westerberg]

On Mon, Nov 09, 2020 at 10:53:30PM +0200, Andy Shevchenko wrote:
> 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>
> Acked-by: Linus Walleij <linus.walleij@linaro.org>

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

Re: [PATCH v5 16/17] gpiolib: acpi: Use BIT() macro to increase readability

[Mika Westerberg]

On Mon, Nov 09, 2020 at 10:53:31PM +0200, Andy Shevchenko wrote:
> We may use BIT() macro to increase readability in
> acpi_gpio_adr_space_handler().
> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
>  drivers/gpio/gpiolib-acpi.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/drivers/gpio/gpiolib-acpi.c b/drivers/gpio/gpiolib-acpi.c
> index 31008b0aef77..b9c3140cbd6d 100644
> --- a/drivers/gpio/gpiolib-acpi.c
> +++ b/drivers/gpio/gpiolib-acpi.c
> @@ -1097,8 +1097,7 @@ acpi_gpio_adr_space_handler(u32 function, acpi_physical_address address,
>  		mutex_unlock(&achip->conn_lock);
>  
>  		if (function == ACPI_WRITE)
> -			gpiod_set_raw_value_cansleep(desc,
> -						     !!((1 << i) & *value));
> +			gpiod_set_raw_value_cansleep(desc, !!(*value & BIT(i)));

Nit: Here I would use a helper variable to make it (much) more readable.

Anyway,

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

Re: [PATCH v5 08/17] gpiolib: Extract gpio_set_debounce_timeout() for internal use

[Andy Shevchenko]

On Wed, Nov 11, 2020 at 5:54 PM Mika Westerberg
<mika.westerberg@linux.intel.com> wrote:
> On Wed, Nov 11, 2020 at 05:46:55PM +0200, Andy Shevchenko wrote:
> > On Wed, Nov 11, 2020 at 5:40 PM Mika Westerberg
> > <mika.westerberg@linux.intel.com> wrote:
> > > On Mon, Nov 09, 2020 at 10:53:23PM +0200, Andy Shevchenko wrote:

...

> > > Again I think mode variable is pretty useless here and does not improve
> > > readability.
> >
> > You mean something like
> >
> >     return gpio_set_config_with_argument_optional(desc,
> >                 PIN_CONFIG_INPUT_DEBOUNCE, debounce);
> >
> > is better?

Hmm... Not sure I can agree with this, but I can change.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v5 16/17] gpiolib: acpi: Use BIT() macro to increase readability

[Andy Shevchenko]

On Wed, Nov 11, 2020 at 05:57:17PM +0200, Mika Westerberg wrote:
> On Mon, Nov 09, 2020 at 10:53:31PM +0200, Andy Shevchenko wrote:
> > We may use BIT() macro to increase readability in
> > acpi_gpio_adr_space_handler().
> > 
> > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> > ---
> >  drivers/gpio/gpiolib-acpi.c | 3 +--
> >  1 file changed, 1 insertion(+), 2 deletions(-)
> > 
> > diff --git a/drivers/gpio/gpiolib-acpi.c b/drivers/gpio/gpiolib-acpi.c
> > index 31008b0aef77..b9c3140cbd6d 100644
> > --- a/drivers/gpio/gpiolib-acpi.c
> > +++ b/drivers/gpio/gpiolib-acpi.c
> > @@ -1097,8 +1097,7 @@ acpi_gpio_adr_space_handler(u32 function, acpi_physical_address address,
> >  		mutex_unlock(&achip->conn_lock);
> >  
> >  		if (function == ACPI_WRITE)
> > -			gpiod_set_raw_value_cansleep(desc,
> > -						     !!((1 << i) & *value));
> > +			gpiod_set_raw_value_cansleep(desc, !!(*value & BIT(i)));
> 
> Nit: Here I would use a helper variable to make it (much) more readable.
> 
> Anyway,
> 
> Reviewed-by: Mika Westerberg <mika.westerberg@linux.intel.com>

Thank you for review of almost all patches in the series!

I will try to address your comments against temporary variable in other
patches, but my motivation is quite similar to yours here.


-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v5 04/17] gpiolib: Add temporary variable to gpiod_set_transitory() for cleaner code

[Andy Shevchenko]

On Wed, Nov 11, 2020 at 05:40:16PM +0200, Andy Shevchenko wrote:
> On Wed, Nov 11, 2020 at 5:33 PM Mika Westerberg
> <mika.westerberg@linux.intel.com> wrote:
> > On Mon, Nov 09, 2020 at 10:53:19PM +0200, Andy Shevchenko wrote:

...

> > To me this looks better and reads easier:
> >
> >         packed = pinconf_to_config_packed(PIN_CONFIG_PERSIST_STATE, !transitory);
> >
> > But not insisting so if GPIO maintainers are fine then no objections :)

I have dropped this patch.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v5 17/17] gpiolib: acpi: Make Intel GPIO tree official for GPIO ACPI work

[Andy Shevchenko]

On Mon, Nov 09, 2020 at 10:53:32PM +0200, Andy Shevchenko wrote:
> Make Intel GPIO tree official for GPIO ACPI work.

Mika, do you think it's a good idea?

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v5 17/17] gpiolib: acpi: Make Intel GPIO tree official for GPIO ACPI work

[Mika Westerberg]

On Wed, Nov 11, 2020 at 09:27:39PM +0200, Andy Shevchenko wrote:
> On Mon, Nov 09, 2020 at 10:53:32PM +0200, Andy Shevchenko wrote:
> > Make Intel GPIO tree official for GPIO ACPI work.
> 
> Mika, do you think it's a good idea?

If it helps the GPIO maintainers work then yes sure, and in that case
you can add:

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

Re: [PATCH v5 17/17] gpiolib: acpi: Make Intel GPIO tree official for GPIO ACPI work

[Linus Walleij]

On Thu, Nov 12, 2020 at 8:00 AM Mika Westerberg
<mika.westerberg@linux.intel.com> wrote:
> On Wed, Nov 11, 2020 at 09:27:39PM +0200, Andy Shevchenko wrote:
> > On Mon, Nov 09, 2020 at 10:53:32PM +0200, Andy Shevchenko wrote:
> > > Make Intel GPIO tree official for GPIO ACPI work.
> >
> > Mika, do you think it's a good idea?
>
> If it helps the GPIO maintainers work then yes sure, and in that case
> you can add:
>
> Reviewed-by: Mika Westerberg <mika.westerberg@linux.intel.com>

Was requested by me :)

And yes: Andy's collecting Intel patches has been a tremendous help,
and if he can do the same for ACPI I am grateful.

Yours,
Linus Walleij