[PATCH 0/5] gpio / ACPI: convert users to gpiod_* and drop acpi_gpio.h

[PATCH 0/5] gpio / ACPI: convert users to gpiod_* and drop acpi_gpio.h

[Mika Westerberg]

Hi all,

Now that the mainline kernel has full ACPI support for the GPIO descriptor
interface we can get rid of ACPI specific GPIO functions in favor of GPIO
descriptor (gpiod_*) interfaces.

This series first converts the existing two users to this interface and
then modifies gpiolib and gpiolib-acpi so that the ACPI functions are only
called internally in drivers/gpio. We then remove the acpi_gpio.h and
require all users to user gpiod_* interfaces.

I suppose it would make sense to merge the whole series via GPIO or ACPI
trees because there's a dependency that the first two patches need to be
applied before last three. Otherwise the drivers in question fail to
compile.

Thanks.

Heikki Krogerus (1):
  net: rfkill: gpio: convert to descriptor-based GPIO interface

Mika Westerberg (4):
  mmc: sdhci-acpi: covert to use GPIO descriptor API
  gpio / ACPI: register to ACPI events automatically
  gpio / ACPI: get rid of acpi_gpio.h
  Documentation / ACPI: update to GPIO descriptor API

 Documentation/acpi/enumeration.txt |  36 +++----------
 drivers/gpio/gpiolib-acpi.c        |  20 +++++--
 drivers/gpio/gpiolib.c             |   5 +-
 drivers/gpio/gpiolib.h             |  46 ++++++++++++++++
 drivers/mmc/host/sdhci-acpi.c      |  26 ++++-----
 drivers/pinctrl/pinctrl-baytrail.c |   4 --
 include/linux/acpi_gpio.h          |  51 ------------------
 net/rfkill/rfkill-gpio.c           | 108 +++++++++++++++++++++++--------------
 8 files changed, 150 insertions(+), 146 deletions(-)
 create mode 100644 drivers/gpio/gpiolib.h
 delete mode 100644 include/linux/acpi_gpio.h

-- 
1.8.4.3

[PATCH 1/5] net: rfkill: gpio: convert to descriptor-based GPIO interface

[Mika Westerberg]

From: Heikki Krogerus <heikki.krogerus@linux.intel.com>

Convert to the safer gpiod_* family of API functions.

Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
---
 net/rfkill/rfkill-gpio.c | 108 +++++++++++++++++++++++++++++------------------
 1 file changed, 68 insertions(+), 40 deletions(-)

diff --git a/net/rfkill/rfkill-gpio.c b/net/rfkill/rfkill-gpio.c
index 5620d3c07479..a3e703d9d59b 100644
--- a/net/rfkill/rfkill-gpio.c
+++ b/net/rfkill/rfkill-gpio.c
@@ -25,15 +25,15 @@
 #include <linux/clk.h>
 #include <linux/slab.h>
 #include <linux/acpi.h>
-#include <linux/acpi_gpio.h>
+#include <linux/gpio/consumer.h>
 
 #include <linux/rfkill-gpio.h>
 
 struct rfkill_gpio_data {
 	const char		*name;
 	enum rfkill_type	type;
-	int			reset_gpio;
-	int			shutdown_gpio;
+	struct gpio_desc	*reset_gpio;
+	struct gpio_desc	*shutdown_gpio;
 
 	struct rfkill		*rfkill_dev;
 	char			*reset_name;
@@ -48,19 +48,15 @@ static int rfkill_gpio_set_power(void *data, bool blocked)
 	struct rfkill_gpio_data *rfkill = data;
 
 	if (blocked) {
-		if (gpio_is_valid(rfkill->shutdown_gpio))
-			gpio_set_value(rfkill->shutdown_gpio, 0);
-		if (gpio_is_valid(rfkill->reset_gpio))
-			gpio_set_value(rfkill->reset_gpio, 0);
+		gpiod_set_value(rfkill->shutdown_gpio, 0);
+		gpiod_set_value(rfkill->reset_gpio, 0);
 		if (!IS_ERR(rfkill->clk) && rfkill->clk_enabled)
 			clk_disable(rfkill->clk);
 	} else {
 		if (!IS_ERR(rfkill->clk) && !rfkill->clk_enabled)
 			clk_enable(rfkill->clk);
-		if (gpio_is_valid(rfkill->reset_gpio))
-			gpio_set_value(rfkill->reset_gpio, 1);
-		if (gpio_is_valid(rfkill->shutdown_gpio))
-			gpio_set_value(rfkill->shutdown_gpio, 1);
+		gpiod_set_value(rfkill->reset_gpio, 1);
+		gpiod_set_value(rfkill->shutdown_gpio, 1);
 	}
 
 	rfkill->clk_enabled = blocked;
@@ -72,6 +68,35 @@ static const struct rfkill_ops rfkill_gpio_ops = {
 	.set_block = rfkill_gpio_set_power,
 };
 
+static int rfkill_gpio_convert_to_desc(struct platform_device *pdev,
+				       struct rfkill_gpio_data *rfkill)
+{
+	struct rfkill_gpio_platform_data *pdata = pdev->dev.platform_data;
+	int ret = 0;
+
+	if (gpio_is_valid(pdata->reset_gpio)) {
+		ret = devm_gpio_request_one(&pdev->dev, pdata->reset_gpio,
+					    0, rfkill->reset_name);
+		if (ret) {
+			pr_warn("%s: failed to get reset gpio.\n", __func__);
+			return ret;
+		}
+		rfkill->reset_gpio = gpio_to_desc(pdata->reset_gpio);
+	}
+
+	if (gpio_is_valid(pdata->shutdown_gpio)) {
+		ret = devm_gpio_request_one(&pdev->dev, pdata->shutdown_gpio,
+					    0, rfkill->shutdown_name);
+		if (ret) {
+			pr_warn("%s: failed to get shutdown gpio.\n", __func__);
+			return ret;
+		}
+		rfkill->shutdown_gpio = gpio_to_desc(pdata->shutdown_gpio);
+	}
+
+	return 0;
+}
+
 static int rfkill_gpio_acpi_probe(struct device *dev,
 				  struct rfkill_gpio_data *rfkill)
 {
@@ -83,8 +108,6 @@ static int rfkill_gpio_acpi_probe(struct device *dev,
 
 	rfkill->name = dev_name(dev);
 	rfkill->type = (unsigned)id->driver_data;
-	rfkill->reset_gpio = acpi_get_gpio_by_index(dev, 0, NULL);
-	rfkill->shutdown_gpio = acpi_get_gpio_by_index(dev, 1, NULL);
 
 	return 0;
 }
@@ -109,28 +132,10 @@ static int rfkill_gpio_probe(struct platform_device *pdev)
 		clk_name = pdata->power_clk_name;
 		rfkill->name = pdata->name;
 		rfkill->type = pdata->type;
-		rfkill->reset_gpio = pdata->reset_gpio;
-		rfkill->shutdown_gpio = pdata->shutdown_gpio;
 	} else {
 		return -ENODEV;
 	}
 
-	/* make sure at-least one of the GPIO is defined and that
-	 * a name is specified for this instance */
-	if ((!gpio_is_valid(rfkill->reset_gpio) &&
-	     !gpio_is_valid(rfkill->shutdown_gpio)) || !rfkill->name) {
-		pr_warn("%s: invalid platform data\n", __func__);
-		return -EINVAL;
-	}
-
-	if (pdata && pdata->gpio_runtime_setup) {
-		ret = pdata->gpio_runtime_setup(pdev);
-		if (ret) {
-			pr_warn("%s: can't set up gpio\n", __func__);
-			return ret;
-		}
-	}
-
 	len = strlen(rfkill->name);
 	rfkill->reset_name = devm_kzalloc(&pdev->dev, len + 7, GFP_KERNEL);
 	if (!rfkill->reset_name)
@@ -145,20 +150,43 @@ static int rfkill_gpio_probe(struct platform_device *pdev)
 
 	rfkill->clk = devm_clk_get(&pdev->dev, clk_name);
 
-	if (gpio_is_valid(rfkill->reset_gpio)) {
-		ret = devm_gpio_request_one(&pdev->dev, rfkill->reset_gpio,
-					    0, rfkill->reset_name);
-		if (ret) {
-			pr_warn("%s: failed to get reset gpio.\n", __func__);
+	if (pdata && (pdata->reset_gpio || pdata->shutdown_gpio)) {
+		ret = rfkill_gpio_convert_to_desc(pdev, rfkill);
+		if (ret)
 			return ret;
+	} else {
+		struct gpio_desc *gpio;
+
+		gpio = devm_gpiod_get_index(&pdev->dev, rfkill->reset_name, 0);
+		if (!IS_ERR(gpio)) {
+			ret = gpiod_direction_output(gpio, 0);
+			if (ret)
+				return ret;
+			rfkill->reset_gpio = gpio;
+		}
+
+		gpio = devm_gpiod_get_index(&pdev->dev,
+					    rfkill->shutdown_name, 1);
+		if (!IS_ERR(gpio)) {
+			ret = gpiod_direction_output(gpio, 0);
+			if (ret)
+				return ret;
+			rfkill->shutdown_gpio = gpio;
 		}
 	}
 
-	if (gpio_is_valid(rfkill->shutdown_gpio)) {
-		ret = devm_gpio_request_one(&pdev->dev, rfkill->shutdown_gpio,
-					    0, rfkill->shutdown_name);
+	/* Make sure at-least one of the GPIO is defined and that
+	 * a name is specified for this instance
+	 */
+	if ((!rfkill->reset_gpio && !rfkill->shutdown_gpio) || !rfkill->name) {
+		pr_warn("%s: invalid platform data\n", __func__);
+		return -EINVAL;
+	}
+
+	if (pdata && pdata->gpio_runtime_setup) {
+		ret = pdata->gpio_runtime_setup(pdev);
 		if (ret) {
-			pr_warn("%s: failed to get shutdown gpio.\n", __func__);
+			pr_warn("%s: can't set up gpio\n", __func__);
 			return ret;
 		}
 	}
-- 
1.8.4.3

[PATCH 2/5] mmc: sdhci-acpi: covert to use GPIO descriptor API

[Mika Westerberg]

The new descriptor based GPIO interface is now the recommend and more safer
way of using GPIOs from device drivers. Convert the ACPI SDHCI driver to
use that interface.

Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
---
 drivers/mmc/host/sdhci-acpi.c | 26 ++++++++++----------------
 1 file changed, 10 insertions(+), 16 deletions(-)

diff --git a/drivers/mmc/host/sdhci-acpi.c b/drivers/mmc/host/sdhci-acpi.c
index ef19874fcd1f..379ddfe14bc7 100644
--- a/drivers/mmc/host/sdhci-acpi.c
+++ b/drivers/mmc/host/sdhci-acpi.c
@@ -31,10 +31,9 @@
 #include <linux/bitops.h>
 #include <linux/types.h>
 #include <linux/err.h>
-#include <linux/gpio.h>
+#include <linux/gpio/consumer.h>
 #include <linux/interrupt.h>
 #include <linux/acpi.h>
-#include <linux/acpi_gpio.h>
 #include <linux/pm.h>
 #include <linux/pm_runtime.h>
 #include <linux/delay.h>
@@ -199,22 +198,19 @@ static irqreturn_t sdhci_acpi_sd_cd(int irq, void *dev_id)
 	return IRQ_HANDLED;
 }
 
-static int sdhci_acpi_add_own_cd(struct device *dev, int gpio,
-				 struct mmc_host *mmc)
+static int sdhci_acpi_add_own_cd(struct device *dev, struct mmc_host *mmc)
 {
+	struct gpio_desc *desc;
 	unsigned long flags;
 	int err, irq;
 
-	if (gpio < 0) {
-		err = gpio;
+	desc = devm_gpiod_get_index(dev, NULL, 0);
+	if (IS_ERR(desc)) {
+		err = PTR_ERR(desc);
 		goto out;
 	}
 
-	err = devm_gpio_request_one(dev, gpio, GPIOF_DIR_IN, "sd_cd");
-	if (err)
-		goto out;
-
-	irq = gpio_to_irq(gpio);
+	irq = gpiod_to_irq(desc);
 	if (irq < 0) {
 		err = irq;
 		goto out_free;
@@ -228,7 +224,7 @@ static int sdhci_acpi_add_own_cd(struct device *dev, int gpio,
 	return 0;
 
 out_free:
-	devm_gpio_free(dev, gpio);
+	devm_gpiod_put(dev, desc);
 out:
 	dev_warn(dev, "failed to setup card detect wake up\n");
 	return err;
@@ -254,7 +250,7 @@ static int sdhci_acpi_probe(struct platform_device *pdev)
 	struct resource *iomem;
 	resource_size_t len;
 	const char *hid;
-	int err, gpio;
+	int err;
 
 	if (acpi_bus_get_device(handle, &device))
 		return -ENODEV;
@@ -279,8 +275,6 @@ static int sdhci_acpi_probe(struct platform_device *pdev)
 	if (IS_ERR(host))
 		return PTR_ERR(host);
 
-	gpio = acpi_get_gpio_by_index(dev, 0, NULL);
-
 	c = sdhci_priv(host);
 	c->host = host;
 	c->slot = sdhci_acpi_get_slot(handle, hid);
@@ -338,7 +332,7 @@ static int sdhci_acpi_probe(struct platform_device *pdev)
 		goto err_free;
 
 	if (sdhci_acpi_flag(c, SDHCI_ACPI_SD_CD)) {
-		if (sdhci_acpi_add_own_cd(dev, gpio, host->mmc))
+		if (sdhci_acpi_add_own_cd(dev, host->mmc))
 			c->use_runtime_pm = false;
 	}
 
-- 
1.8.4.3

[PATCH 3/5] gpio / ACPI: register to ACPI events automatically

[Mika Westerberg]

Instead of asking each driver to register to ACPI events we can just call
acpi_gpiochip_register_interrupts() for each chip that has an ACPI handle.
The function checks chip->to_irq and if it is set to NULL (a GPIO driver
that doesn't do interrupts) the function does nothing.

We also add the a new header drivers/gpio/gpiolib.h that is used for
functions internal to gpiolib and add ACPI GPIO chip registering functions
to that header.

Once that is done we can remove call to acpi_gpiochip_register_interrupts()
from its only user, pinctrl-baytrail.c

Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
---
 drivers/gpio/gpiolib-acpi.c        | 16 ++++++++++++----
 drivers/gpio/gpiolib.c             |  4 ++++
 drivers/gpio/gpiolib.h             | 23 +++++++++++++++++++++++
 drivers/pinctrl/pinctrl-baytrail.c |  4 ----
 include/linux/acpi_gpio.h          |  6 ------
 5 files changed, 39 insertions(+), 14 deletions(-)
 create mode 100644 drivers/gpio/gpiolib.h

diff --git a/drivers/gpio/gpiolib-acpi.c b/drivers/gpio/gpiolib-acpi.c
index ae0ffdce8bd5..cb2da66fbbfe 100644
--- a/drivers/gpio/gpiolib-acpi.c
+++ b/drivers/gpio/gpiolib-acpi.c
@@ -94,7 +94,7 @@ static void acpi_gpio_evt_dh(acpi_handle handle, void *data)
  * gpio pins have acpi event methods and assigns interrupt handlers that calls
  * the acpi event methods for those pins.
  */
-void acpi_gpiochip_request_interrupts(struct gpio_chip *chip)
+static void acpi_gpiochip_request_interrupts(struct gpio_chip *chip)
 {
 	struct acpi_buffer buf = {ACPI_ALLOCATE_BUFFER, NULL};
 	struct acpi_resource *res;
@@ -192,7 +192,6 @@ void acpi_gpiochip_request_interrupts(struct gpio_chip *chip)
 				irq);
 	}
 }
-EXPORT_SYMBOL(acpi_gpiochip_request_interrupts);
 
 /**
  * acpi_gpiochip_free_interrupts() - Free GPIO _EVT ACPI event interrupts.
@@ -203,7 +202,7 @@ EXPORT_SYMBOL(acpi_gpiochip_request_interrupts);
  * The remaining ACPI event interrupts associated with the chip are freed
  * automatically.
  */
-void acpi_gpiochip_free_interrupts(struct gpio_chip *chip)
+static void acpi_gpiochip_free_interrupts(struct gpio_chip *chip)
 {
 	acpi_handle handle;
 	acpi_status status;
@@ -230,7 +229,6 @@ void acpi_gpiochip_free_interrupts(struct gpio_chip *chip)
 	acpi_detach_data(handle, acpi_gpio_evt_dh);
 	kfree(evt_pins);
 }
-EXPORT_SYMBOL(acpi_gpiochip_free_interrupts);
 
 struct acpi_gpio_lookup {
 	struct acpi_gpio_info info;
@@ -310,3 +308,13 @@ struct gpio_desc *acpi_get_gpiod_by_index(struct device *dev, int index,
 	return lookup.desc ? lookup.desc : ERR_PTR(-ENODEV);
 }
 EXPORT_SYMBOL_GPL(acpi_get_gpiod_by_index);
+
+void acpi_gpiochip_add(struct gpio_chip *chip)
+{
+	acpi_gpiochip_request_interrupts(chip);
+}
+
+void acpi_gpiochip_remove(struct gpio_chip *chip)
+{
+	acpi_gpiochip_free_interrupts(chip);
+}
diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index 4e10b10d3ddd..70abccf0d144 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -15,6 +15,8 @@
 #include <linux/slab.h>
 #include <linux/acpi.h>
 
+#include "gpiolib.h"
+
 #define CREATE_TRACE_POINTS
 #include <trace/events/gpio.h>
 
@@ -1212,6 +1214,7 @@ int gpiochip_add(struct gpio_chip *chip)
 #endif
 
 	of_gpiochip_add(chip);
+	acpi_gpiochip_add(chip);
 
 	if (status)
 		goto fail;
@@ -1253,6 +1256,7 @@ int gpiochip_remove(struct gpio_chip *chip)
 
 	gpiochip_remove_pin_ranges(chip);
 	of_gpiochip_remove(chip);
+	acpi_gpiochip_remove(chip);
 
 	for (id = 0; id < chip->ngpio; id++) {
 		if (test_bit(FLAG_REQUESTED, &chip->desc[id].flags)) {
diff --git a/drivers/gpio/gpiolib.h b/drivers/gpio/gpiolib.h
new file mode 100644
index 000000000000..2ed23ab8298c
--- /dev/null
+++ b/drivers/gpio/gpiolib.h
@@ -0,0 +1,23 @@
+/*
+ * Internal GPIO functions.
+ *
+ * Copyright (C) 2013, Intel Corporation
+ * Author: Mika Westerberg <mika.westerberg@linux.intel.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#ifndef GPIOLIB_H
+#define GPIOLIB_H
+
+#ifdef CONFIG_ACPI
+void acpi_gpiochip_add(struct gpio_chip *chip);
+void acpi_gpiochip_remove(struct gpio_chip *chip);
+#else
+static inline void acpi_gpiochip_add(struct gpio_chip *chip) { }
+static inline void acpi_gpiochip_remove(struct gpio_chip *chip) { }
+#endif
+
+#endif /* GPIOLIB_H */
diff --git a/drivers/pinctrl/pinctrl-baytrail.c b/drivers/pinctrl/pinctrl-baytrail.c
index 2832576d8b12..98b87a59df2d 100644
--- a/drivers/pinctrl/pinctrl-baytrail.c
+++ b/drivers/pinctrl/pinctrl-baytrail.c
@@ -29,7 +29,6 @@
 #include <linux/gpio.h>
 #include <linux/irqdomain.h>
 #include <linux/acpi.h>
-#include <linux/acpi_gpio.h>
 #include <linux/platform_device.h>
 #include <linux/seq_file.h>
 #include <linux/io.h>
@@ -485,9 +484,6 @@ static int byt_gpio_probe(struct platform_device *pdev)
 
 		irq_set_handler_data(hwirq, vg);
 		irq_set_chained_handler(hwirq, byt_gpio_irq_handler);
-
-		/* Register interrupt handlers for gpio signaled acpi events */
-		acpi_gpiochip_request_interrupts(gc);
 	}
 
 	pm_runtime_enable(dev);
diff --git a/include/linux/acpi_gpio.h b/include/linux/acpi_gpio.h
index d875bc3dba3c..af96a0d452f6 100644
--- a/include/linux/acpi_gpio.h
+++ b/include/linux/acpi_gpio.h
@@ -21,9 +21,6 @@ struct acpi_gpio_info {
 
 struct gpio_desc *acpi_get_gpiod_by_index(struct device *dev, int index,
 					  struct acpi_gpio_info *info);
-void acpi_gpiochip_request_interrupts(struct gpio_chip *chip);
-void acpi_gpiochip_free_interrupts(struct gpio_chip *chip);
-
 #else /* CONFIG_GPIO_ACPI */
 
 static inline struct gpio_desc *
@@ -33,9 +30,6 @@ acpi_get_gpiod_by_index(struct device *dev, int index,
 	return ERR_PTR(-ENOSYS);
 }
 
-static inline void acpi_gpiochip_request_interrupts(struct gpio_chip *chip) { }
-static inline void acpi_gpiochip_free_interrupts(struct gpio_chip *chip) { }
-
 #endif /* CONFIG_GPIO_ACPI */
 
 static inline int acpi_get_gpio_by_index(struct device *dev, int index,
-- 
1.8.4.3

[PATCH 4/5] gpio / ACPI: get rid of acpi_gpio.h

[Mika Westerberg]

Now that all users of acpi_gpio.h have been moved to user either the GPIO
descriptor interface or to the internal gpiolib.h we can get rid of
acpi_gpio.h entirely.

Once this is done the only interface to get GPIOs to drivers enumerated
from ACPI namespace is the descriptor based interface.

Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
---
 drivers/gpio/gpiolib-acpi.c |  4 +++-
 drivers/gpio/gpiolib.c      |  1 -
 drivers/gpio/gpiolib.h      | 23 +++++++++++++++++++++++
 include/linux/acpi_gpio.h   | 45 ---------------------------------------------
 4 files changed, 26 insertions(+), 47 deletions(-)
 delete mode 100644 include/linux/acpi_gpio.h

diff --git a/drivers/gpio/gpiolib-acpi.c b/drivers/gpio/gpiolib-acpi.c
index cb2da66fbbfe..6c158d9a6efa 100644
--- a/drivers/gpio/gpiolib-acpi.c
+++ b/drivers/gpio/gpiolib-acpi.c
@@ -12,11 +12,13 @@
 
 #include <linux/errno.h>
 #include <linux/gpio/consumer.h>
+#include <linux/gpio/driver.h>
 #include <linux/export.h>
-#include <linux/acpi_gpio.h>
 #include <linux/acpi.h>
 #include <linux/interrupt.h>
 
+#include "gpiolib.h"
+
 struct acpi_gpio_evt_pin {
 	struct list_head node;
 	acpi_handle *evt_handle;
diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index 70abccf0d144..a6b82413c290 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -10,7 +10,6 @@
 #include <linux/seq_file.h>
 #include <linux/gpio.h>
 #include <linux/of_gpio.h>
-#include <linux/acpi_gpio.h>
 #include <linux/idr.h>
 #include <linux/slab.h>
 #include <linux/acpi.h>
diff --git a/drivers/gpio/gpiolib.h b/drivers/gpio/gpiolib.h
index 2ed23ab8298c..82be586c1f90 100644
--- a/drivers/gpio/gpiolib.h
+++ b/drivers/gpio/gpiolib.h
@@ -12,12 +12,35 @@
 #ifndef GPIOLIB_H
 #define GPIOLIB_H
 
+#include <linux/err.h>
+#include <linux/device.h>
+
+/**
+ * struct acpi_gpio_info - ACPI GPIO specific information
+ * @gpioint: if %true this GPIO is of type GpioInt otherwise type is GpioIo
+ * @active_low: in case of @gpioint, the pin is active low
+ */
+struct acpi_gpio_info {
+	bool gpioint;
+	bool active_low;
+};
+
 #ifdef CONFIG_ACPI
 void acpi_gpiochip_add(struct gpio_chip *chip);
 void acpi_gpiochip_remove(struct gpio_chip *chip);
+
+struct gpio_desc *acpi_get_gpiod_by_index(struct device *dev, int index,
+					  struct acpi_gpio_info *info);
 #else
 static inline void acpi_gpiochip_add(struct gpio_chip *chip) { }
 static inline void acpi_gpiochip_remove(struct gpio_chip *chip) { }
+
+static inline struct gpio_desc *
+acpi_get_gpiod_by_index(struct device *dev, int index,
+			struct acpi_gpio_info *info)
+{
+	return ERR_PTR(-ENOSYS);
+}
 #endif
 
 #endif /* GPIOLIB_H */
diff --git a/include/linux/acpi_gpio.h b/include/linux/acpi_gpio.h
deleted file mode 100644
index af96a0d452f6..000000000000
--- a/include/linux/acpi_gpio.h
+++ /dev/null
@@ -1,45 +0,0 @@
-#ifndef _LINUX_ACPI_GPIO_H_
-#define _LINUX_ACPI_GPIO_H_
-
-#include <linux/device.h>
-#include <linux/err.h>
-#include <linux/errno.h>
-#include <linux/gpio.h>
-#include <linux/gpio/consumer.h>
-
-/**
- * struct acpi_gpio_info - ACPI GPIO specific information
- * @gpioint: if %true this GPIO is of type GpioInt otherwise type is GpioIo
- * @active_low: in case of @gpioint, the pin is active low
- */
-struct acpi_gpio_info {
-	bool gpioint;
-	bool active_low;
-};
-
-#ifdef CONFIG_GPIO_ACPI
-
-struct gpio_desc *acpi_get_gpiod_by_index(struct device *dev, int index,
-					  struct acpi_gpio_info *info);
-#else /* CONFIG_GPIO_ACPI */
-
-static inline struct gpio_desc *
-acpi_get_gpiod_by_index(struct device *dev, int index,
-			struct acpi_gpio_info *info)
-{
-	return ERR_PTR(-ENOSYS);
-}
-
-#endif /* CONFIG_GPIO_ACPI */
-
-static inline int acpi_get_gpio_by_index(struct device *dev, int index,
-					 struct acpi_gpio_info *info)
-{
-	struct gpio_desc *desc = acpi_get_gpiod_by_index(dev, index, info);
-
-	if (IS_ERR(desc))
-		return PTR_ERR(desc);
-	return desc_to_gpio(desc);
-}
-
-#endif /* _LINUX_ACPI_GPIO_H_ */
-- 
1.8.4.3

[PATCH 5/5] Documentation / ACPI: update to GPIO descriptor API

[Mika Westerberg]

Update the documentation also to reflect the fact that there are no ACPI
specific GPIO interfaces anymore but drivers should instead use the
descriptor based GPIO APIs.

Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
---
 Documentation/acpi/enumeration.txt | 36 +++++++-----------------------------
 1 file changed, 7 insertions(+), 29 deletions(-)

diff --git a/Documentation/acpi/enumeration.txt b/Documentation/acpi/enumeration.txt
index b994bcb32b92..2a1519b87177 100644
--- a/Documentation/acpi/enumeration.txt
+++ b/Documentation/acpi/enumeration.txt
@@ -293,36 +293,13 @@ the device to the driver. For example:
 
 These GPIO numbers are controller relative and path "\\_SB.PCI0.GPI0"
 specifies the path to the controller. In order to use these GPIOs in Linux
-we need to translate them to the Linux GPIO numbers.
+we need to translate them to the corresponding Linux GPIO descriptors.
 
-In a simple case of just getting the Linux GPIO number from device
-resources one can use acpi_get_gpio_by_index() helper function. It takes
-pointer to the device and index of the GpioIo/GpioInt descriptor in the
-device resources list. For example:
+There is a standard GPIO API for that and is documented in
+Documentation/gpio.txt.
 
-	int gpio_irq, gpio_power;
-	int ret;
-
-	gpio_irq = acpi_get_gpio_by_index(dev, 1, NULL);
-	if (gpio_irq < 0)
-		/* handle error */
-
-	gpio_power = acpi_get_gpio_by_index(dev, 0, NULL);
-	if (gpio_power < 0)
-		/* handle error */
-
-	/* Now we can use the GPIO numbers */
-
-Other GpioIo parameters must be converted first by the driver to be
-suitable to the gpiolib before passing them.
-
-In case of GpioInt resource an additional call to gpio_to_irq() must be
-done before calling request_irq().
-
-Note that the above API is ACPI specific and not recommended for drivers
-that need to support non-ACPI systems. The recommended way is to use
-the descriptor based GPIO interfaces. The above example looks like this
-when converted to the GPIO desc:
+In the above example we can get the corresponding two GPIO descriptors with
+a code like this:
 
 	#include <linux/gpio/consumer.h>
 	...
@@ -339,4 +316,5 @@ when converted to the GPIO desc:
 
 	/* Now we can use the GPIO descriptors */
 
-See also Documentation/gpio.txt.
+There are also devm_* versions of these functions which release the
+descriptors once the device is released.
-- 
1.8.4.3

Re: [PATCH 0/5] gpio / ACPI: convert users to gpiod_* and drop acpi_gpio.h

[Johannes Berg]

On Thu, 2013-11-21 at 16:45 +0200, Mika Westerberg wrote:

> I suppose it would make sense to merge the whole series via GPIO or ACPI
> trees because there's a dependency that the first two patches need to be
> applied before last three. Otherwise the drivers in question fail to
> compile.

Totally. I wouldn't want to track it all, so ack for rfkill. Not that I
understand/know that code well anyway.

johannes

Re: [PATCH 1/5] net: rfkill: gpio: convert to descriptor-based GPIO interface

[Heikki Krogerus]

Hi Rhyland,

On Thu, Nov 21, 2013 at 04:45:49PM +0200, Mika Westerberg wrote:
> +static int rfkill_gpio_convert_to_desc(struct platform_device *pdev,
> +				       struct rfkill_gpio_data *rfkill)
> +{
> +	struct rfkill_gpio_platform_data *pdata = pdev->dev.platform_data;
> +	int ret = 0;
> +
> +	if (gpio_is_valid(pdata->reset_gpio)) {
> +		ret = devm_gpio_request_one(&pdev->dev, pdata->reset_gpio,
> +					    0, rfkill->reset_name);
> +		if (ret) {
> +			pr_warn("%s: failed to get reset gpio.\n", __func__);
> +			return ret;
> +		}
> +		rfkill->reset_gpio = gpio_to_desc(pdata->reset_gpio);
> +	}
> +
> +	if (gpio_is_valid(pdata->shutdown_gpio)) {
> +		ret = devm_gpio_request_one(&pdev->dev, pdata->shutdown_gpio,
> +					    0, rfkill->shutdown_name);
> +		if (ret) {
> +			pr_warn("%s: failed to get shutdown gpio.\n", __func__);
> +			return ret;
> +		}
> +		rfkill->shutdown_gpio = gpio_to_desc(pdata->shutdown_gpio);
> +	}
> +
> +	return 0;
> +}

We could drop this conversion if you guys added gpiod_lookup table to
your platform code. I think something like this is enough. Please note
that I have not even tried compile it but you get the idea from it.

diff --git a/arch/arm/mach-tegra/board-paz00.c b/arch/arm/mach-tegra/board-paz00.c
index 06f0240..19bce88 100644
--- a/arch/arm/mach-tegra/board-paz00.c
+++ b/arch/arm/mach-tegra/board-paz00.c
@@ -18,13 +18,12 @@
  */
 
 #include <linux/platform_device.h>
+#include <linux/gpio/driver.h>
 #include <linux/rfkill-gpio.h>
 #include "board.h"
 
 static struct rfkill_gpio_platform_data wifi_rfkill_platform_data = {
        .name           = "wifi_rfkill",
-       .reset_gpio     = 25, /* PD1 */
-       .shutdown_gpio  = 85, /* PK5 */
        .type   = RFKILL_TYPE_WLAN,
 };
 
@@ -36,7 +35,13 @@ static struct platform_device wifi_rfkill_device = {
        },
 };
 
+static struct gpiod_lookup wifi_gpio_lookup[] = {
+       GPIO_LOOKUP_IDX("tegra-gpio", 25, "rfkill_gpio", NULL, 0, NULL),
+       GPIO_LOOKUP_IDX("tegra-gpio", 85, "rfkill_gpio", NULL, 1, NULL),
+};
+
 void __init tegra_paz00_wifikill_init(void)
 {
+       gpiod_add_table(wifi_lookup, ARRAY_SIZE(wifi_gpio_lookup));
        platform_device_register(&wifi_rfkill_device);
 }


-- 
heikki

Re: [PATCH 1/5] net: rfkill: gpio: convert to descriptor-based GPIO interface

[Andy Shevchenko]

On Thu, Nov 21, 2013 at 4:45 PM, Mika Westerberg
<mika.westerberg@linux.intel.com> wrote:
> From: Heikki Krogerus <heikki.krogerus@linux.intel.com>
>
> Convert to the safer gpiod_* family of API functions.

Few comments below.

> --- a/net/rfkill/rfkill-gpio.c
> +++ b/net/rfkill/rfkill-gpio.c

> +static int rfkill_gpio_convert_to_desc(struct platform_device *pdev,
> +                                      struct rfkill_gpio_data *rfkill)
> +{
> +       struct rfkill_gpio_platform_data *pdata = pdev->dev.platform_data;
> +       int ret = 0;

Unneeded assignment.

> +
> +       if (gpio_is_valid(pdata->reset_gpio)) {
> +               ret = devm_gpio_request_one(&pdev->dev, pdata->reset_gpio,
> +                                           0, rfkill->reset_name);
> +               if (ret) {
> +                       pr_warn("%s: failed to get reset gpio.\n", __func__);
> +                       return ret;
> +               }
> +               rfkill->reset_gpio = gpio_to_desc(pdata->reset_gpio);
> +       }
> +
> +       if (gpio_is_valid(pdata->shutdown_gpio)) {
> +               ret = devm_gpio_request_one(&pdev->dev, pdata->shutdown_gpio,
> +                                           0, rfkill->shutdown_name);
> +               if (ret) {
> +                       pr_warn("%s: failed to get shutdown gpio.\n", __func__);
> +                       return ret;
> +               }
> +               rfkill->shutdown_gpio = gpio_to_desc(pdata->shutdown_gpio);
> +       }
> +
> +       return 0;
> +}

[]

> +       /* Make sure at-least one of the GPIO is defined and that
> +        * a name is specified for this instance
> +        */
> +       if ((!rfkill->reset_gpio && !rfkill->shutdown_gpio) || !rfkill->name) {
> +               pr_warn("%s: invalid platform data\n", __func__);
> +               return -EINVAL;
> +       }
> +
> +       if (pdata && pdata->gpio_runtime_setup) {
> +               ret = pdata->gpio_runtime_setup(pdev);
>                 if (ret) {
> -                       pr_warn("%s: failed to get shutdown gpio.\n", __func__);
> +                       pr_warn("%s: can't set up gpio\n", __func__);

Could you use dev_* family of functions to print messages?

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 1/5] net: rfkill: gpio: convert to descriptor-based GPIO interface

[Rhyland Klein]

On 11/21/2013 10:24 AM, Heikki Krogerus wrote:
> Hi Rhyland,
> 
> On Thu, Nov 21, 2013 at 04:45:49PM +0200, Mika Westerberg wrote:
>> +static int rfkill_gpio_convert_to_desc(struct platform_device *pdev,
>> +				       struct rfkill_gpio_data *rfkill)
>> +{
>> +	struct rfkill_gpio_platform_data *pdata = pdev->dev.platform_data;
>> +	int ret = 0;
>> +
>> +	if (gpio_is_valid(pdata->reset_gpio)) {
>> +		ret = devm_gpio_request_one(&pdev->dev, pdata->reset_gpio,
>> +					    0, rfkill->reset_name);
>> +		if (ret) {
>> +			pr_warn("%s: failed to get reset gpio.\n", __func__);
>> +			return ret;
>> +		}
>> +		rfkill->reset_gpio = gpio_to_desc(pdata->reset_gpio);
>> +	}
>> +
>> +	if (gpio_is_valid(pdata->shutdown_gpio)) {
>> +		ret = devm_gpio_request_one(&pdev->dev, pdata->shutdown_gpio,
>> +					    0, rfkill->shutdown_name);
>> +		if (ret) {
>> +			pr_warn("%s: failed to get shutdown gpio.\n", __func__);
>> +			return ret;
>> +		}
>> +		rfkill->shutdown_gpio = gpio_to_desc(pdata->shutdown_gpio);
>> +	}
>> +
>> +	return 0;
>> +}
> 
> We could drop this conversion if you guys added gpiod_lookup table to
> your platform code. I think something like this is enough. Please note
> that I have not even tried compile it but you get the idea from it.
> 
> diff --git a/arch/arm/mach-tegra/board-paz00.c b/arch/arm/mach-tegra/board-paz00.c
> index 06f0240..19bce88 100644
> --- a/arch/arm/mach-tegra/board-paz00.c
> +++ b/arch/arm/mach-tegra/board-paz00.c
> @@ -18,13 +18,12 @@
>   */
>  
>  #include <linux/platform_device.h>
> +#include <linux/gpio/driver.h>
>  #include <linux/rfkill-gpio.h>
>  #include "board.h"
>  
>  static struct rfkill_gpio_platform_data wifi_rfkill_platform_data = {
>         .name           = "wifi_rfkill",
> -       .reset_gpio     = 25, /* PD1 */
> -       .shutdown_gpio  = 85, /* PK5 */
>         .type   = RFKILL_TYPE_WLAN,
>  };
>  
> @@ -36,7 +35,13 @@ static struct platform_device wifi_rfkill_device = {
>         },
>  };
>  
> +static struct gpiod_lookup wifi_gpio_lookup[] = {
> +       GPIO_LOOKUP_IDX("tegra-gpio", 25, "rfkill_gpio", NULL, 0, NULL),
> +       GPIO_LOOKUP_IDX("tegra-gpio", 85, "rfkill_gpio", NULL, 1, NULL),
> +};
> +
>  void __init tegra_paz00_wifikill_init(void)
>  {
> +       gpiod_add_table(wifi_lookup, ARRAY_SIZE(wifi_gpio_lookup));
>         platform_device_register(&wifi_rfkill_device);
>  }
> 
> 

This seems like a reasonable patch, and likely a good way to go. I don't
have a AC100 to test this with though, so I can't verify this
personally. Also, would the paz00 patch become a dependency of this
patchset? Mika, maybe you can therefore include a patch for paz00 in
your patchset?

-rhyland

-- 
nvpublic

Re: [PATCH 1/5] net: rfkill: gpio: convert to descriptor-based GPIO interface

[Mika Westerberg]

On Thu, Nov 21, 2013 at 12:05:34PM -0500, Rhyland Klein wrote:
> This seems like a reasonable patch, and likely a good way to go. I don't
> have a AC100 to test this with though, so I can't verify this
> personally. Also, would the paz00 patch become a dependency of this
> patchset? Mika, maybe you can therefore include a patch for paz00 in
> your patchset?

Sure I can, if everyone is OK with that.

Re: [PATCH 1/5] net: rfkill: gpio: convert to descriptor-based GPIO interface

[Rafael J. Wysocki]

On Thursday, November 21, 2013 07:22:17 PM Mika Westerberg wrote:
> On Thu, Nov 21, 2013 at 12:05:34PM -0500, Rhyland Klein wrote:
> > This seems like a reasonable patch, and likely a good way to go. I don't
> > have a AC100 to test this with though, so I can't verify this
> > personally. Also, would the paz00 patch become a dependency of this
> > patchset? Mika, maybe you can therefore include a patch for paz00 in
> > your patchset?
> 
> Sure I can, if everyone is OK with that.

No objections from me.

Thanks!

-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

Re: [PATCH 2/5] mmc: sdhci-acpi: covert to use GPIO descriptor API

[Adrian Hunter]

On 21/11/13 16:45, Mika Westerberg wrote:
> The new descriptor based GPIO interface is now the recommend and more safer

"recommend and more safer" -> "recommended and safer"

> way of using GPIOs from device drivers. Convert the ACPI SDHCI driver to
> use that interface.
> 
> Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
> ---
>  drivers/mmc/host/sdhci-acpi.c | 26 ++++++++++----------------
>  1 file changed, 10 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/mmc/host/sdhci-acpi.c b/drivers/mmc/host/sdhci-acpi.c
> index ef19874fcd1f..379ddfe14bc7 100644
> --- a/drivers/mmc/host/sdhci-acpi.c
> +++ b/drivers/mmc/host/sdhci-acpi.c
> @@ -31,10 +31,9 @@
>  #include <linux/bitops.h>
>  #include <linux/types.h>
>  #include <linux/err.h>
> -#include <linux/gpio.h>
> +#include <linux/gpio/consumer.h>
>  #include <linux/interrupt.h>
>  #include <linux/acpi.h>
> -#include <linux/acpi_gpio.h>
>  #include <linux/pm.h>
>  #include <linux/pm_runtime.h>
>  #include <linux/delay.h>
> @@ -199,22 +198,19 @@ static irqreturn_t sdhci_acpi_sd_cd(int irq, void *dev_id)
>  	return IRQ_HANDLED;
>  }
>  
> -static int sdhci_acpi_add_own_cd(struct device *dev, int gpio,
> -				 struct mmc_host *mmc)
> +static int sdhci_acpi_add_own_cd(struct device *dev, struct mmc_host *mmc)
>  {
> +	struct gpio_desc *desc;
>  	unsigned long flags;
>  	int err, irq;
>  
> -	if (gpio < 0) {
> -		err = gpio;
> +	desc = devm_gpiod_get_index(dev, NULL, 0);

I notice that:

	devm_gpio_request_one(.., label)
		-> gpio_request_one(..., label)
			-> gpiod_request(..., label)

but:

	devm_gpiod_get_index(..., con_id, ...)
		-> gpiod_get_index(..., con_id, ...)
			-> gpiod_request(..., con_id)

which suggests that 'con_id' is equivalent to 'label'.
i.e. "sd_cd".  Is it?

> +	if (IS_ERR(desc)) {
> +		err = PTR_ERR(desc);
>  		goto out;
>  	}
>  
> -	err = devm_gpio_request_one(dev, gpio, GPIOF_DIR_IN, "sd_cd");
> -	if (err)
> -		goto out;
> -
> -	irq = gpio_to_irq(gpio);
> +	irq = gpiod_to_irq(desc);
>  	if (irq < 0) {
>  		err = irq;
>  		goto out_free;
> @@ -228,7 +224,7 @@ static int sdhci_acpi_add_own_cd(struct device *dev, int gpio,
>  	return 0;
>  
>  out_free:
> -	devm_gpio_free(dev, gpio);
> +	devm_gpiod_put(dev, desc);
>  out:
>  	dev_warn(dev, "failed to setup card detect wake up\n");
>  	return err;
> @@ -254,7 +250,7 @@ static int sdhci_acpi_probe(struct platform_device *pdev)
>  	struct resource *iomem;
>  	resource_size_t len;
>  	const char *hid;
> -	int err, gpio;
> +	int err;
>  
>  	if (acpi_bus_get_device(handle, &device))
>  		return -ENODEV;
> @@ -279,8 +275,6 @@ static int sdhci_acpi_probe(struct platform_device *pdev)
>  	if (IS_ERR(host))
>  		return PTR_ERR(host);
>  
> -	gpio = acpi_get_gpio_by_index(dev, 0, NULL);
> -
>  	c = sdhci_priv(host);
>  	c->host = host;
>  	c->slot = sdhci_acpi_get_slot(handle, hid);
> @@ -338,7 +332,7 @@ static int sdhci_acpi_probe(struct platform_device *pdev)
>  		goto err_free;
>  
>  	if (sdhci_acpi_flag(c, SDHCI_ACPI_SD_CD)) {
> -		if (sdhci_acpi_add_own_cd(dev, gpio, host->mmc))
> +		if (sdhci_acpi_add_own_cd(dev, host->mmc))
>  			c->use_runtime_pm = false;
>  	}
>  
> 

Re: [PATCH 2/5] mmc: sdhci-acpi: covert to use GPIO descriptor API

[Mika Westerberg]

On Fri, Nov 22, 2013 at 10:53:18AM +0200, Adrian Hunter wrote:
> On 21/11/13 16:45, Mika Westerberg wrote:
> > The new descriptor based GPIO interface is now the recommend and more safer
> 
> "recommend and more safer" -> "recommended and safer"

OK.

> 
> > way of using GPIOs from device drivers. Convert the ACPI SDHCI driver to
> > use that interface.
> > 
> > Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
> > ---
> >  drivers/mmc/host/sdhci-acpi.c | 26 ++++++++++----------------
> >  1 file changed, 10 insertions(+), 16 deletions(-)
> > 
> > diff --git a/drivers/mmc/host/sdhci-acpi.c b/drivers/mmc/host/sdhci-acpi.c
> > index ef19874fcd1f..379ddfe14bc7 100644
> > --- a/drivers/mmc/host/sdhci-acpi.c
> > +++ b/drivers/mmc/host/sdhci-acpi.c
> > @@ -31,10 +31,9 @@
> >  #include <linux/bitops.h>
> >  #include <linux/types.h>
> >  #include <linux/err.h>
> > -#include <linux/gpio.h>
> > +#include <linux/gpio/consumer.h>
> >  #include <linux/interrupt.h>
> >  #include <linux/acpi.h>
> > -#include <linux/acpi_gpio.h>
> >  #include <linux/pm.h>
> >  #include <linux/pm_runtime.h>
> >  #include <linux/delay.h>
> > @@ -199,22 +198,19 @@ static irqreturn_t sdhci_acpi_sd_cd(int irq, void *dev_id)
> >  	return IRQ_HANDLED;
> >  }
> >  
> > -static int sdhci_acpi_add_own_cd(struct device *dev, int gpio,
> > -				 struct mmc_host *mmc)
> > +static int sdhci_acpi_add_own_cd(struct device *dev, struct mmc_host *mmc)
> >  {
> > +	struct gpio_desc *desc;
> >  	unsigned long flags;
> >  	int err, irq;
> >  
> > -	if (gpio < 0) {
> > -		err = gpio;
> > +	desc = devm_gpiod_get_index(dev, NULL, 0);
> 
> I notice that:
> 
> 	devm_gpio_request_one(.., label)
> 		-> gpio_request_one(..., label)
> 			-> gpiod_request(..., label)
> 
> but:
> 
> 	devm_gpiod_get_index(..., con_id, ...)
> 		-> gpiod_get_index(..., con_id, ...)
> 			-> gpiod_request(..., con_id)
> 
> which suggests that 'con_id' is equivalent to 'label'.
> i.e. "sd_cd".  Is it?

Indeed, it is. Even though ACPI doesn't use con_id for lookup gpiolib uses
it as a label once the GPIO descriptor is found.

I'll change that from NULL to "sd_cd".

Re: [PATCH 1/5] net: rfkill: gpio: convert to descriptor-based GPIO interface

[Heikki Krogerus]

Hi Andy,

On Thu, Nov 21, 2013 at 05:32:28PM +0200, Andy Shevchenko wrote:
> > +       /* Make sure at-least one of the GPIO is defined and that
> > +        * a name is specified for this instance
> > +        */
> > +       if ((!rfkill->reset_gpio && !rfkill->shutdown_gpio) || !rfkill->name) {
> > +               pr_warn("%s: invalid platform data\n", __func__);
> > +               return -EINVAL;
> > +       }
> > +
> > +       if (pdata && pdata->gpio_runtime_setup) {
> > +               ret = pdata->gpio_runtime_setup(pdev);
> >                 if (ret) {
> > -                       pr_warn("%s: failed to get shutdown gpio.\n", __func__);
> > +                       pr_warn("%s: can't set up gpio\n", __func__);
> 
> Could you use dev_* family of functions to print messages?

OK, I'll change them. Thanks!

-- 
heikki