[PATCH v3 0/4] ACPI / gpio: Updates to properties

[PATCH v3 0/4] ACPI / gpio: Updates to properties

[Mika Westerberg]

Hi,

This series brings couple of useful GPIO related properties from Device
Tree to ACPI _DSD device properties:

  - Names for GPIO lines
  - GPIO hogging
  - Holes in GPIO property lists

We are using these to get Intel Galileo better supported in the mainline
kernel (but these may be useful for other boards as well). For example SPI
chip select on Galileo is connected to a GPIO line so we need to be able to
describe it in ACPI, and at the same time allow native chip selects.

GPIO hogging can be used to set initial state of certain GPIOs available on
the headers regardless of the BIOS settings (which may be wrong as it knows
nothing about which devices have been connected).

The previous versions can be found here:

  v2: http://www.spinics.net/lists/linux-acpi/msg69469.html
  v1: http://www.spinics.net/lists/linux-acpi/msg69385.html

Changes from v2:
  - Rebased on top of v4.9-rc1 and dropped the already applied patches.
  - Reordered the series so that GPIO names which touches DT code is moved
    to be last one.
  - Renamed acpi_gpiochip_set_names() to devprop_gpiochip_set_names() and
    moved it to a new file: gpiolib-devprop.c.
  - Changed of_gpiochip_add() to call devprop_gpiochip_set_names() and
    removed of_gpiochip_set_names().
  - Added ACKs from Rafael to patches 1 and 2. I did not add ACK for
    patches 3 and 4 because they are changed from the previous version.

The DT part is compile tested only so it would be nice if someone with ARM
board could try if the GPIO names still work as expected.

Changes from v1:
  - Drop patch [1/5] as it has been applied already.
  - Move patch [4/5] to be the first.
  - Rename acpi_data_get_property_reference() to __acpi_node_get_property_reference().
  - Drop acpi_node_get_property_reference() as it is not necessary anymore.
  - Add static inline wrapper acpi_node_get_property_reference() that
    calls the previous passing MAX_ACPI_REFERENCE_ARGS to support existing
    drivers.

Mika Westerberg (4):
  ACPI / gpio: Allow holes in list of GPIOs for a device
  ACPI / gpio: Add hogging support
  gpio: Rework of_gpiochip_set_names() to use device property accessors
  ACPI / gpio: Add support for naming GPIOs

 Documentation/acpi/gpio-properties.txt |  62 +++++++++++++++++++
 drivers/gpio/Makefile                  |   1 +
 drivers/gpio/gpiolib-acpi.c            | 106 +++++++++++++++++++++++++++++----
 drivers/gpio/gpiolib-devprop.c         |  62 +++++++++++++++++++
 drivers/gpio/gpiolib-of.c              |  47 +--------------
 drivers/gpio/gpiolib.h                 |   2 +
 6 files changed, 223 insertions(+), 57 deletions(-)
 create mode 100644 drivers/gpio/gpiolib-devprop.c

-- 
2.9.3

[PATCH v3 1/4] ACPI / gpio: Allow holes in list of GPIOs for a device

[Mika Westerberg]

Make it possible to have an empty GPIOs in a GPIO list for device. For
example a SPI master may use both GPIOs and native pins as chip selects and
we need to be able to distinguish between the two.

This makes it mandatory to have exactly 3 arguments for GPIOs and then
converts gpiolib to use of __acpi_node_get_property_reference() instead. In
addition we make acpi_gpio_package_count() to handle holes as well (this
matches the DT version).

Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
Acked-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 Documentation/acpi/gpio-properties.txt | 15 +++++++++++++++
 drivers/gpio/gpiolib-acpi.c            | 32 +++++++++++++++++++++-----------
 2 files changed, 36 insertions(+), 11 deletions(-)

diff --git a/Documentation/acpi/gpio-properties.txt b/Documentation/acpi/gpio-properties.txt
index 5aafe0b351a1..09cff657b82c 100644
--- a/Documentation/acpi/gpio-properties.txt
+++ b/Documentation/acpi/gpio-properties.txt
@@ -51,6 +51,21 @@ it to 1 marks the GPIO as active low.
 In our Bluetooth example the "reset-gpios" refers to the second GpioIo()
 resource, second pin in that resource with the GPIO number of 31.
 
+It is possible to leave holes in the array of GPIOs. This is useful in
+cases like with SPI host controllers where some chip selects may be
+implemented as GPIOs and some as native signals. For example a SPI host
+controller can have chip selects 0 and 2 implemented as GPIOs and 1 as
+native:
+
+  Package () {
+      "cs-gpios",
+      Package () {
+          ^GPIO, 19, 0, 0, // chip select 0: GPIO
+          0,               // chip select 1: native signal
+          ^GPIO, 20, 0, 0, // chip select 2: GPIO
+      }
+  }
+
 ACPI GPIO Mappings Provided by Drivers
 --------------------------------------
 
diff --git a/drivers/gpio/gpiolib-acpi.c b/drivers/gpio/gpiolib-acpi.c
index 58ece201b8e6..700ea6ad609b 100644
--- a/drivers/gpio/gpiolib-acpi.c
+++ b/drivers/gpio/gpiolib-acpi.c
@@ -468,7 +468,8 @@ static int acpi_gpio_property_lookup(struct fwnode_handle *fwnode,
 	int ret;
 
 	memset(&args, 0, sizeof(args));
-	ret = acpi_node_get_property_reference(fwnode, propname, index, &args);
+	ret = __acpi_node_get_property_reference(fwnode, propname, index, 3,
+						 &args);
 	if (ret) {
 		struct acpi_device *adev = to_acpi_device_node(fwnode);
 
@@ -483,13 +484,13 @@ static int acpi_gpio_property_lookup(struct fwnode_handle *fwnode,
 	 * on returned args.
 	 */
 	lookup->adev = args.adev;
-	if (args.nargs >= 2) {
-		lookup->index = args.args[0];
-		lookup->pin_index = args.args[1];
-		/* 3rd argument, if present is used to specify active_low. */
-		if (args.nargs >= 3)
-			lookup->active_low = !!args.args[2];
-	}
+	if (args.nargs != 3)
+		return -EPROTO;
+
+	lookup->index = args.args[0];
+	lookup->pin_index = args.args[1];
+	lookup->active_low = !!args.args[2];
+
 	return 0;
 }
 
@@ -915,18 +916,27 @@ void acpi_gpiochip_remove(struct gpio_chip *chip)
 	kfree(acpi_gpio);
 }
 
-static unsigned int acpi_gpio_package_count(const union acpi_object *obj)
+static int acpi_gpio_package_count(const union acpi_object *obj)
 {
 	const union acpi_object *element = obj->package.elements;
 	const union acpi_object *end = element + obj->package.count;
 	unsigned int count = 0;
 
 	while (element < end) {
-		if (element->type == ACPI_TYPE_LOCAL_REFERENCE)
+		switch (element->type) {
+		case ACPI_TYPE_LOCAL_REFERENCE:
+			element += 3;
+			/* Fallthrough */
+		case ACPI_TYPE_INTEGER:
+			element++;
 			count++;
+			break;
 
-		element++;
+		default:
+			return -EPROTO;
+		}
 	}
+
 	return count;
 }
 
-- 
2.9.3

Re: [PATCH v3 1/4] ACPI / gpio: Allow holes in list of GPIOs for a device

[Linus Walleij]

On Fri, Oct 21, 2016 at 4:21 PM, Mika Westerberg
<mika.westerberg@linux.intel.com> wrote:

> Make it possible to have an empty GPIOs in a GPIO list for device. For
> example a SPI master may use both GPIOs and native pins as chip selects and
> we need to be able to distinguish between the two.
>
> This makes it mandatory to have exactly 3 arguments for GPIOs and then
> converts gpiolib to use of __acpi_node_get_property_reference() instead. In
> addition we make acpi_gpio_package_count() to handle holes as well (this
> matches the DT version).
>
> Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
> Acked-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

Patch applied.

Yours,
Linus Walleij

[PATCH v3 2/4] ACPI / gpio: Add hogging support

[Mika Westerberg]

GPIO hogging means that the GPIO controller can "hog" and configure certain
GPIOs without need for a driver or userspace to do that. This is useful in
open-connected boards where BIOS cannot possibly know beforehand which
devices will be connected to the board.

This adds GPIO hogging mechanism to ACPI analogous to Device Tree.

Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
Acked-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 Documentation/acpi/gpio-properties.txt | 35 +++++++++++++++++
 drivers/gpio/gpiolib-acpi.c            | 71 ++++++++++++++++++++++++++++++++++
 2 files changed, 106 insertions(+)

diff --git a/Documentation/acpi/gpio-properties.txt b/Documentation/acpi/gpio-properties.txt
index 09cff657b82c..ce7bc5c33c24 100644
--- a/Documentation/acpi/gpio-properties.txt
+++ b/Documentation/acpi/gpio-properties.txt
@@ -66,6 +66,41 @@ native:
       }
   }
 
+Other supported properties
+--------------------------
+
+Following Device Tree compatible device properties are also supported by
+_DSD device properties for GPIO controllers:
+
+- gpio-hog
+- output-high
+- output-low
+- input
+- line-name
+
+Example:
+
+  Name (_DSD, Package () {
+      // _DSD Hierarchical Properties Extension UUID
+      ToUUID("dbb8e3e6-5886-4ba6-8795-1319f52a966b"),
+      Package () {
+          Package () {"hog-gpio8", "G8PU"}
+      }
+  })
+
+  Name (G8PU, Package () {
+      ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
+      Package () {
+          Package () {"gpio-hog", 1},
+          Package () {"gpios", Package () {8, 0}},
+          Package () {"output-high", 1},
+          Package () {"line-name", "gpio8-pullup"},
+      }
+  })
+
+See Documentation/devicetree/bindings/gpio/gpio.tx for more information
+about these properties.
+
 ACPI GPIO Mappings Provided by Drivers
 --------------------------------------
 
diff --git a/drivers/gpio/gpiolib-acpi.c b/drivers/gpio/gpiolib-acpi.c
index 700ea6ad609b..4f46982ce982 100644
--- a/drivers/gpio/gpiolib-acpi.c
+++ b/drivers/gpio/gpiolib-acpi.c
@@ -857,6 +857,76 @@ static void acpi_gpiochip_free_regions(struct acpi_gpio_chip *achip)
 	}
 }
 
+struct gpio_desc *acpi_gpiochip_parse_own_gpio(struct acpi_gpio_chip *achip,
+	struct fwnode_handle *fwnode, const char **name, unsigned int *lflags,
+	unsigned int *dflags)
+{
+	struct gpio_chip *chip = achip->chip;
+	struct gpio_desc *desc;
+	u32 gpios[2];
+	int ret;
+
+	ret = fwnode_property_read_u32_array(fwnode, "gpios", gpios,
+					     ARRAY_SIZE(gpios));
+	if (ret < 0)
+		return ERR_PTR(ret);
+
+	ret = acpi_gpiochip_pin_to_gpio_offset(chip->gpiodev, gpios[0]);
+	if (ret < 0)
+		return ERR_PTR(ret);
+
+	desc = gpiochip_get_desc(chip, ret);
+	if (IS_ERR(desc))
+		return desc;
+
+	*lflags = 0;
+	*dflags = 0;
+	*name = NULL;
+
+	if (gpios[1])
+		*lflags |= GPIO_ACTIVE_LOW;
+
+	if (fwnode_property_present(fwnode, "input"))
+		*dflags |= GPIOD_IN;
+	else if (fwnode_property_present(fwnode, "output-low"))
+		*dflags |= GPIOD_OUT_LOW;
+	else if (fwnode_property_present(fwnode, "output-high"))
+		*dflags |= GPIOD_OUT_HIGH;
+	else
+		return ERR_PTR(-EINVAL);
+
+	fwnode_property_read_string(fwnode, "line-name", name);
+
+	return desc;
+}
+
+static void acpi_gpiochip_scan_gpios(struct acpi_gpio_chip *achip)
+{
+	struct gpio_chip *chip = achip->chip;
+	struct fwnode_handle *fwnode;
+
+	device_for_each_child_node(chip->parent, fwnode) {
+		unsigned int lflags, dflags;
+		struct gpio_desc *desc;
+		const char *name;
+		int ret;
+
+		if (!fwnode_property_present(fwnode, "gpio-hog"))
+			continue;
+
+		desc = acpi_gpiochip_parse_own_gpio(achip, fwnode, &name,
+						    &lflags, &dflags);
+		if (IS_ERR(desc))
+			continue;
+
+		ret = gpiod_hog(desc, name, lflags, dflags);
+		if (ret) {
+			dev_err(chip->parent, "Failed to hog GPIO\n");
+			return;
+		}
+	}
+}
+
 void acpi_gpiochip_add(struct gpio_chip *chip)
 {
 	struct acpi_gpio_chip *acpi_gpio;
@@ -888,6 +958,7 @@ void acpi_gpiochip_add(struct gpio_chip *chip)
 	}
 
 	acpi_gpiochip_request_regions(acpi_gpio);
+	acpi_gpiochip_scan_gpios(acpi_gpio);
 	acpi_walk_dep_device_list(handle);
 }
 
-- 
2.9.3

Re: [PATCH v3 2/4] ACPI / gpio: Add hogging support

[Mika Westerberg]

On Fri, Oct 21, 2016 at 05:21:30PM +0300, Mika Westerberg wrote:
> +See Documentation/devicetree/bindings/gpio/gpio.tx for more information

Andy noticed there is a typo in the above, should be .txt instead. Let
me know if you want me to resend the series with this fixed.

Re: [PATCH v3 2/4] ACPI / gpio: Add hogging support

[Linus Walleij]

On Fri, Oct 21, 2016 at 4:21 PM, Mika Westerberg
<mika.westerberg@linux.intel.com> wrote:

> GPIO hogging means that the GPIO controller can "hog" and configure certain
> GPIOs without need for a driver or userspace to do that. This is useful in
> open-connected boards where BIOS cannot possibly know beforehand which
> devices will be connected to the board.
>
> This adds GPIO hogging mechanism to ACPI analogous to Device Tree.
>
> Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
> Acked-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

Patch applied.

Fixed the small typo in the process. No need to resend.

Yours,
Linus Walleij

[PATCH v3 3/4] gpio: Rework of_gpiochip_set_names() to use device property accessors

[Mika Westerberg]

In order to use "gpio-line-names" property in systems not having DT as
their boot firmware, rework of_gpiochip_set_names() to use device property
accessors. This reworked function is placed in a separate file making it
clear it deals with universal device properties.

Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
---
 drivers/gpio/Makefile          |  1 +
 drivers/gpio/gpiolib-devprop.c | 62 ++++++++++++++++++++++++++++++++++++++++++
 drivers/gpio/gpiolib-of.c      | 47 +-------------------------------
 drivers/gpio/gpiolib.h         |  2 ++
 4 files changed, 66 insertions(+), 46 deletions(-)
 create mode 100644 drivers/gpio/gpiolib-devprop.c

diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile
index ab28a2daeacc..5cf9af1c0668 100644
--- a/drivers/gpio/Makefile
+++ b/drivers/gpio/Makefile
@@ -5,6 +5,7 @@ ccflags-$(CONFIG_DEBUG_GPIO)	+= -DDEBUG
 obj-$(CONFIG_GPIO_DEVRES)	+= devres.o
 obj-$(CONFIG_GPIOLIB)		+= gpiolib.o
 obj-$(CONFIG_GPIOLIB)		+= gpiolib-legacy.o
+obj-$(CONFIG_GPIOLIB)		+= gpiolib-devprop.o
 obj-$(CONFIG_OF_GPIO)		+= gpiolib-of.o
 obj-$(CONFIG_GPIO_SYSFS)	+= gpiolib-sysfs.o
 obj-$(CONFIG_GPIO_ACPI)		+= gpiolib-acpi.o
diff --git a/drivers/gpio/gpiolib-devprop.c b/drivers/gpio/gpiolib-devprop.c
new file mode 100644
index 000000000000..17bfc41692ef
--- /dev/null
+++ b/drivers/gpio/gpiolib-devprop.c
@@ -0,0 +1,62 @@
+/*
+ * Device property helpers for GPIO chips.
+ *
+ * Copyright (C) 2016, 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.
+ */
+
+#include <linux/property.h>
+#include <linux/slab.h>
+#include <linux/gpio/consumer.h>
+#include <linux/gpio/driver.h>
+
+#include "gpiolib.h"
+
+/**
+ * devprop_gpiochip_set_names - Set GPIO line names using device properties
+ * @chip: GPIO chip whose lines should be named, if possible
+ *
+ * Looks for device property "gpio-line-names" and if it exists assigns
+ * GPIO line names for the chip. The memory allocated for the assigned
+ * names belong to the underlying firmware node and should not be released
+ * by the caller.
+ */
+void devprop_gpiochip_set_names(struct gpio_chip *chip)
+{
+	struct gpio_device *gdev = chip->gpiodev;
+	const char **names;
+	int ret, i;
+
+	ret = device_property_read_string_array(chip->parent, "gpio-line-names",
+						NULL, 0);
+	if (ret < 0)
+		return;
+
+	if (ret != gdev->ngpio) {
+		dev_warn(chip->parent,
+			 "names %d do not match number of GPIOs %d\n", ret,
+			 gdev->ngpio);
+		return;
+	}
+
+	names = kcalloc(gdev->ngpio, sizeof(*names), GFP_KERNEL);
+	if (!names)
+		return;
+
+	ret = device_property_read_string_array(chip->parent, "gpio-line-names",
+						names, gdev->ngpio);
+	if (ret < 0) {
+		dev_warn(chip->parent, "failed to read GPIO line names\n");
+		kfree(names);
+		return;
+	}
+
+	for (i = 0; i < gdev->ngpio; i++)
+		gdev->descs[i].name = names[i];
+
+	kfree(names);
+}
diff --git a/drivers/gpio/gpiolib-of.c b/drivers/gpio/gpiolib-of.c
index ecad3f0e3b77..3fa4e84b4327 100644
--- a/drivers/gpio/gpiolib-of.c
+++ b/drivers/gpio/gpiolib-of.c
@@ -222,51 +222,6 @@ static struct gpio_desc *of_parse_own_gpio(struct device_node *np,
 }
 
 /**
- * of_gpiochip_set_names() - set up the names of the lines
- * @chip: GPIO chip whose lines should be named, if possible
- */
-static void of_gpiochip_set_names(struct gpio_chip *gc)
-{
-	struct gpio_device *gdev = gc->gpiodev;
-	struct device_node *np = gc->of_node;
-	int i;
-	int nstrings;
-
-	nstrings = of_property_count_strings(np, "gpio-line-names");
-	if (nstrings <= 0)
-		/* Lines names not present */
-		return;
-
-	/* This is normally not what you want */
-	if (gdev->ngpio != nstrings)
-		dev_info(&gdev->dev, "gpio-line-names specifies %d line "
-			 "names but there are %d lines on the chip\n",
-			 nstrings, gdev->ngpio);
-
-	/*
-	 * Make sure to not index beyond the end of the number of descriptors
-	 * of the GPIO device.
-	 */
-	for (i = 0; i < gdev->ngpio; i++) {
-		const char *name;
-		int ret;
-
-		ret = of_property_read_string_index(np,
-						    "gpio-line-names",
-						    i,
-						    &name);
-		if (ret) {
-			if (ret != -ENODATA)
-                                dev_err(&gdev->dev,
-                                        "unable to name line %d: %d\n",
-                                        i, ret);
-			break;
-		}
-		gdev->descs[i].name = name;
-	}
-}
-
-/**
  * of_gpiochip_scan_gpios - Scan gpio-controller for gpio definitions
  * @chip:	gpio chip to act on
  *
@@ -522,7 +477,7 @@ int of_gpiochip_add(struct gpio_chip *chip)
 
 	/* If the chip defines names itself, these take precedence */
 	if (!chip->names)
-		of_gpiochip_set_names(chip);
+		devprop_gpiochip_set_names(chip);
 
 	of_node_get(chip->of_node);
 
diff --git a/drivers/gpio/gpiolib.h b/drivers/gpio/gpiolib.h
index 346fbda39220..d10eaf520860 100644
--- a/drivers/gpio/gpiolib.h
+++ b/drivers/gpio/gpiolib.h
@@ -209,6 +209,8 @@ static int __maybe_unused gpio_chip_hwgpio(const struct gpio_desc *desc)
 	return desc - &desc->gdev->descs[0];
 }
 
+void devprop_gpiochip_set_names(struct gpio_chip *chip);
+
 /* With descriptor prefix */
 
 #define gpiod_emerg(desc, fmt, ...)					       \
-- 
2.9.3

Re: [PATCH v3 3/4] gpio: Rework of_gpiochip_set_names() to use device property accessors

[Linus Walleij]

On Fri, Oct 21, 2016 at 4:21 PM, Mika Westerberg
<mika.westerberg@linux.intel.com> wrote:

> In order to use "gpio-line-names" property in systems not having DT as
> their boot firmware, rework of_gpiochip_set_names() to use device property
> accessors. This reworked function is placed in a separate file making it
> clear it deals with universal device properties.
>
> Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>

Exactly like this, thanks!

Patch applied.

Yours,
Linus Walleij

[PATCH v3 4/4] ACPI / gpio: Add support for naming GPIOs

[Mika Westerberg]

Now that we have the new helper function that sets nice names for GPIO
lines based on "gpio-line-names" device property, we can take advantage of
this in acpi_gpiochip_add().

Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
---
 Documentation/acpi/gpio-properties.txt | 12 ++++++++++++
 drivers/gpio/gpiolib-acpi.c            |  3 +++
 2 files changed, 15 insertions(+)

diff --git a/Documentation/acpi/gpio-properties.txt b/Documentation/acpi/gpio-properties.txt
index ce7bc5c33c24..08fdc0ddee48 100644
--- a/Documentation/acpi/gpio-properties.txt
+++ b/Documentation/acpi/gpio-properties.txt
@@ -98,6 +98,18 @@ Example:
       }
   })
 
+- gpio-line-names
+
+Example:
+
+  Package () {
+      "gpio-line-names",
+      Package () {
+          "SPI0_CS_N", "EXP2_INT", "MUX6_IO", "UART0_RXD", "MUX7_IO",
+          "LVL_C_A1", "MUX0_IO", "SPI1_MISO"
+      }
+  }
+
 See Documentation/devicetree/bindings/gpio/gpio.tx for more information
 about these properties.
 
diff --git a/drivers/gpio/gpiolib-acpi.c b/drivers/gpio/gpiolib-acpi.c
index 4f46982ce982..53266ef12008 100644
--- a/drivers/gpio/gpiolib-acpi.c
+++ b/drivers/gpio/gpiolib-acpi.c
@@ -957,6 +957,9 @@ void acpi_gpiochip_add(struct gpio_chip *chip)
 		return;
 	}
 
+	if (!chip->names)
+		devprop_gpiochip_set_names(chip);
+
 	acpi_gpiochip_request_regions(acpi_gpio);
 	acpi_gpiochip_scan_gpios(acpi_gpio);
 	acpi_walk_dep_device_list(handle);
-- 
2.9.3

Re: [PATCH v3 4/4] ACPI / gpio: Add support for naming GPIOs

[Linus Walleij]

On Fri, Oct 21, 2016 at 4:21 PM, Mika Westerberg
<mika.westerberg@linux.intel.com> wrote:

> Now that we have the new helper function that sets nice names for GPIO
> lines based on "gpio-line-names" device property, we can take advantage of
> this in acpi_gpiochip_add().
>
> Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>

Sweet!

Patch applied.

Yours,
Linus Walleij