[PATCH v3 0/5] Add gpio test framework

[PATCH v3 0/5] Add gpio test framework

[bamvor.zhangjian]

From: Bamvor Jian Zhang <bamvor.zhangjian@linaro.org>

These series of patches try to add support for testing of gpio
subsystem based on the proposal from Linus Walleij. The first two
version is here[1][2].

The basic idea is implement a virtual gpio device(gpio-mockup) base
on gpiolib. Tester could test the gpiolib by manipulating gpio-mockup
device through sysfs or char device and check the result from debugfs.
Reference the following figure:

   sysfs/char device  debugfs
     |                   |
  gpiolib----------------/
     |
 gpio-mockup

Currently, this test script will use chardev interface by default.

In order to avoid conflict with other gpio exist in the system,
only dynamic allocation is tested by default. User could pass -f to
do full test.

[1] http://comments.gmane.org/gmane.linux.kernel.gpio/11883
[2] http://www.spinics.net/lists/linux-gpio/msg11700.html

Changes since v1:
1.  Change value of gpio to boolean.
2.  Only test dynamic allocation by default.
Changes since v2:
1.  Switch to chardev.
2.  Add basic gpio operation for chardev.

Bamvor Jian Zhang (5):
  tools/gpio: add gpio basic opereations
  tools/gpio: re-work gpio hammer with gpio operations
  gpio/mockup: add virtual gpio device
  selftest/gpio: add gpio test case
  gpio: MAINTAINERS: Add an entry for GPIO mockup driver

 Documentation/kernel-parameters.txt                |   4 +
 MAINTAINERS                                        |   7 +
 drivers/gpio/Kconfig                               |  12 +
 drivers/gpio/Makefile                              |   1 +
 drivers/gpio/gpio-mockup.c                         | 214 ++++++++++++++
 tools/gpio/gpio-hammer.c                           |  52 +---
 tools/gpio/gpio-utils.c                            | 163 +++++++++++
 tools/gpio/gpio-utils.h                            | 121 ++++++++
 tools/testing/selftests/Makefile                   |   1 +
 tools/testing/selftests/gpio/Makefile              |  23 ++
 tools/testing/selftests/gpio/gpio-mockup-chardev.c | 321 +++++++++++++++++++++
 tools/testing/selftests/gpio/gpio-mockup-sysfs.sh  | 134 +++++++++
 tools/testing/selftests/gpio/gpio-mockup.sh        | 200 +++++++++++++
 13 files changed, 1211 insertions(+), 42 deletions(-)
 create mode 100644 drivers/gpio/gpio-mockup.c
 create mode 100644 tools/testing/selftests/gpio/Makefile
 create mode 100644 tools/testing/selftests/gpio/gpio-mockup-chardev.c
 create mode 100755 tools/testing/selftests/gpio/gpio-mockup-sysfs.sh
 create mode 100755 tools/testing/selftests/gpio/gpio-mockup.sh

-- 
1.8.4.5

[PATCH v3 1/5] tools/gpio: add gpio basic opereations

[bamvor.zhangjian]

From: Bamvor Jian Zhang <bamvor.zhangjian@linaro.org>

Add basic gpio operations. User could get/set gpio value and/or flag for
specific gpio chardev.

Reference the "tools/testing/selftest/gpio/gpio-mockup-chardev.c" for
how to use it.

Signed-off-by: Bamvor Jian Zhang <bamvor.zhangjian@linaro.org>
---
 tools/gpio/gpio-utils.c | 163 ++++++++++++++++++++++++++++++++++++++++++++++++
 tools/gpio/gpio-utils.h | 121 +++++++++++++++++++++++++++++++++++
 2 files changed, 284 insertions(+)

diff --git a/tools/gpio/gpio-utils.c b/tools/gpio/gpio-utils.c
index 8208718..3c0a35c 100644
--- a/tools/gpio/gpio-utils.c
+++ b/tools/gpio/gpio-utils.c
@@ -2,10 +2,173 @@
  * GPIO tools - helpers library for the GPIO tools
  *
  * Copyright (C) 2015 Linus Walleij
+ * Copyright (C) 2016 Bamvor Jian Zhang
  *
  * 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 <unistd.h>
+#include <stdlib.h>
+#include <stdio.h>
+#include <errno.h>
+#include <string.h>
+#include <fcntl.h>
+#include <getopt.h>
+#include <sys/ioctl.h>
+#include <linux/gpio.h>
 #include "gpio-utils.h"
+
+#define COMSUMER "gpio-utils"
+
+int gpio_request(const char *device_name, unsigned int *lines,
+		 unsigned int nlines, unsigned int flag,
+		 struct gpiohandle_data *data, const char *consumer_label)
+{
+	struct gpiohandle_request req;
+	char *chrdev_name;
+	int fd;
+	int i;
+	int ret;
+
+	ret = asprintf(&chrdev_name, "/dev/%s", device_name);
+	if (ret < 0)
+		return -ENOMEM;
+
+	fd = open(chrdev_name, 0);
+	if (fd == -1) {
+		ret = -errno;
+		fprintf(stderr, "Failed to open %s\n", chrdev_name);
+		goto exit_close_error;
+	}
+
+	for (i = 0; i < nlines; i++)
+		req.lineoffsets[i] = lines[i];
+
+	req.flags = flag;
+	strcpy(req.consumer_label, consumer_label);
+	req.lines = nlines;
+	if (flag & GPIOHANDLE_REQUEST_OUTPUT)
+		memcpy(req.default_values, data, sizeof(req.default_values));
+
+	ret = ioctl(fd, GPIO_GET_LINEHANDLE_IOCTL, &req);
+	if (ret == -1) {
+		ret = -errno;
+		fprintf(stderr, "Failed to issue GET LINEHANDLE IOCTL (%d)\n",
+			ret);
+		goto exit_close_error;
+	}
+
+exit_close_error:
+	if (close(fd) == -1)
+		perror("Failed to close GPIO character device file");
+	free(chrdev_name);
+	return ret < 0 ? ret : req.fd;
+}
+
+int gpio_set_values(const int fd, struct gpiohandle_data *data)
+{
+	int ret;
+
+	ret = ioctl(fd, GPIOHANDLE_SET_LINE_VALUES_IOCTL, data);
+	if (ret == -1) {
+		ret = -errno;
+		fprintf(stderr, "Failed to issue %s (%d)\n",
+			"GPIOHANDLE_SET_LINE_VALUES_IOCTL", ret);
+		goto exit_close_error;
+	}
+
+exit_close_error:
+	return ret;
+}
+
+int gpio_get_values(const int fd, struct gpiohandle_data *data)
+{
+	int ret;
+
+	ret = ioctl(fd, GPIOHANDLE_GET_LINE_VALUES_IOCTL, data);
+	if (ret == -1) {
+		ret = -errno;
+		fprintf(stderr, "Failed to issue %s (%d)\n",
+			"GPIOHANDLE_GET_LINE_VALUES_IOCTL", ret);
+		goto exit_close_error;
+	}
+
+exit_close_error:
+	return ret;
+}
+
+int gpio_release(const int fd)
+{
+	int ret;
+
+	ret = close(fd);
+	if (ret < -1)
+		perror("Failed to close GPIO LINEHANDLE device file");
+
+	return ret;
+}
+
+int gpio_gets(const char *device_name, unsigned int *lines, unsigned int nlines,
+	      unsigned int flag, struct gpiohandle_data *data)
+{
+	int fd;
+	int ret;
+
+	ret = gpio_request(device_name, lines, nlines, flag, data, COMSUMER);
+	if (ret < 0)
+		return ret;
+
+	fd = ret;
+	ret = gpio_get_values(fd, data);
+	ret = gpio_release(fd);
+	return ret;
+}
+
+int gpio_sets(const char *device_name, unsigned int *lines, unsigned int nlines,
+	      unsigned int flag, struct gpiohandle_data *data)
+{
+	int ret;
+
+	ret = gpio_request(device_name, lines, nlines, flag, data, COMSUMER);
+	if (ret < 0)
+		return ret;
+
+	ret = gpio_release(ret);
+	return ret;
+}
+
+int gpio_get(const char *device_name, unsigned int line, unsigned int flag)
+{
+	struct gpiohandle_data data;
+	unsigned int lines[] = {line};
+
+	gpio_gets(device_name, lines, 1, flag, &data);
+	return data.values[0];
+}
+
+int gpio_set(const char *device_name, unsigned int line, unsigned int flag,
+	     unsigned int value)
+{
+	struct gpiohandle_data data;
+	unsigned int lines[] = {line};
+
+	data.values[0] = value;
+	return gpio_sets(device_name, lines, 1, flag, &data);
+}
+
+int gpio_set_flag(const char *device_name, unsigned int line, unsigned int flag)
+{
+	struct gpiohandle_data data;
+	unsigned int lines[] = {line};
+	int ret;
+
+	ret = gpio_request(device_name, lines, 1, flag, &data, COMSUMER);
+	if (ret < 0)
+		return ret;
+
+	ret = gpio_release(ret);
+	return ret;
+}
+
diff --git a/tools/gpio/gpio-utils.h b/tools/gpio/gpio-utils.h
index 5f57133..fcf87b6 100644
--- a/tools/gpio/gpio-utils.h
+++ b/tools/gpio/gpio-utils.h
@@ -24,4 +24,125 @@ static inline int check_prefix(const char *str, const char *prefix)
 		strncmp(str, prefix, strlen(prefix)) == 0;
 }
 
+/* Basic operation of gpio */
+/*
+ * Request the lines for gpio with device_name. Could set the default value
+ * in request.
+ * device_name:    the name of gpiochip in "/dev", such as gpiochip0.
+ * lines:	   the array of which line should be requested.
+ * nline:          the total number of line
+ * flag:           input, output and so on. Reference "linux/gpio.h" for the
+ *                 meaning for flag.
+ * data:	   default value when flag is GPIOHANDLE_REQUEST_OUTPUT.
+ * consumer_label: the name of consumer, such as "sysfs", "powerkey".
+ *
+ * Return value:   On success return the fd of specific gpiochip. It could be
+ *                 release by gpio_release.
+ *		   On failure return the errno.
+ */
+int gpio_request(const char *device_name, unsigned int *lines,
+		 unsigned int nlines, unsigned int flag,
+		 struct gpiohandle_data *data, const char *consumer_label);
+/*
+ * Set the value of gpio for fd
+ * fd:             the fd returned by gpio_request
+ * data:	   the value want to set
+ *
+ * Return value:   On success return 0
+ *		   On failure return the errno.
+ */
+int gpio_set_values(const int fd, struct gpiohandle_data *data);
+
+/*
+ * Get the value of gpio for fd
+ * fd:             the fd returned by gpio_request
+ * data:	   the valud get from gpiochip.
+ *
+ * Return value:   On success return 0
+ *		   On failure return the errno.
+ */
+int gpio_get_values(const int fd, struct gpiohandle_data *data);
+
+/*
+ * release the fd for gpiochip
+ */
+int gpio_release(const int fd);
+
+/* Easy operation for one time get/set */
+/*
+ * Get one pin value from line of device_name. Could change the flag if
+ * necessary.
+ * device_name: the name of gpiochip in "/dev", such as gpiochip0.
+ * line:        number of line, such as 2.
+ * flag:        input, output and so on. Reference "linux/gpio.h" for the
+ *              meaning for flag. Set to 0 if do not want to update the
+ *              flag. It is the recommandation value.
+ *
+ * Return value:   On success return the value get from gpiochip.
+ *		   On failure return the errno.
+ */
+int gpio_get(const char *device_name, unsigned int line, unsigned int flag);
+
+/*
+ * Get pins value from line of device_name. Could change the flag if
+ * necessary.
+ * device_name: the name of gpiochip in "/dev", such as gpiochip0.
+ * lines:	   the array of which line should be requested.
+ * nline:          the total number of line
+ * flag:           input, output and so on. Reference "linux/gpio.h" for the
+ *                 meaning for flag.
+ * data:	   the valud get from gpiochip.
+ *
+ * Return value:   On success return 0
+ *		   On failure return the errno.
+ */
+int gpio_gets(const char *device_name, unsigned int *lines, unsigned int nlines,
+	      unsigned int flag, struct gpiohandle_data *data);
+
+/*
+ * Set one pin value from line of device_name. Could change the flag if
+ * necessary.
+ * device_name: the name of gpiochip in "/dev", such as gpiochip0.
+ * line:        number of line, such as 2.
+ * flag:        input, output and so on. Reference "linux/gpio.h" for the
+ *              meaning for flag. Set to 0 if do not want to update the
+ *              flag. It is the recommandation value.
+ * value:	the value want to set the gpio line.
+ *
+ * Return value:   On success return 0
+ *		   On failure return the errno.
+ */
+int gpio_set(const char *device_name, unsigned int line, unsigned int flag,
+	     unsigned int value);
+
+/*
+ * Set pins value from line of device_name. Could change the flag if
+ * necessary.
+ * device_name: the name of gpiochip in "/dev", such as gpiochip0.
+ * lines:	   the array of which line should be requested.
+ * nline:          the total number of line
+ * flag:           input, output and so on. Reference "linux/gpio.h" for the
+ *                 meaning for flag.
+ * data:	   the value want to set
+ *
+ * Return value:   On success return 0
+ *		   On failure return the errno.
+ */
+int gpio_sets(const char *device_name, unsigned int *lines, unsigned int nlines,
+	      unsigned int flag, struct gpiohandle_data *data);
+
+/*
+ * set the flag for device_name. Mainly for changing the direction of gpio
+ *
+ * device_name: the name of gpiochip in "/dev", such as gpiochip0.
+ * line:        number of line, such as 2.
+ * flag:        input, output and so on. Reference "linux/gpio.h" for the
+ *              meaning for flag.
+ *
+ * Return value:   On success return 0
+ *		   On failure return the errno.
+ */
+int gpio_set_flag(const char *device_name, unsigned int line,
+		  unsigned int flag);
+
 #endif /* _GPIO_UTILS_H_ */
-- 
1.8.4.5

[PATCH v3 2/5] tools/gpio: re-work gpio hammer with gpio operations

[bamvor.zhangjian]

From: Bamvor Jian Zhang <bamvor.zhangjian@linaro.org>

Signed-off-by: Bamvor Jian Zhang <bamvor.zhangjian@linaro.org>
---
 tools/gpio/gpio-hammer.c | 52 ++++++++++--------------------------------------
 1 file changed, 10 insertions(+), 42 deletions(-)

diff --git a/tools/gpio/gpio-hammer.c b/tools/gpio/gpio-hammer.c
index 37b3f14..14dd20c 100644
--- a/tools/gpio/gpio-hammer.c
+++ b/tools/gpio/gpio-hammer.c
@@ -23,49 +23,20 @@
 #include <getopt.h>
 #include <sys/ioctl.h>
 #include <linux/gpio.h>
+#include "gpio-utils.h"
 
 int hammer_device(const char *device_name, unsigned int *lines, int nlines,
 		  unsigned int loops)
 {
-	struct gpiohandle_request req;
 	struct gpiohandle_data data;
-	char *chrdev_name;
 	char swirr[] = "-\\|/";
-	int fd;
 	int ret;
 	int i, j;
 	unsigned int iteration = 0;
 
-	ret = asprintf(&chrdev_name, "/dev/%s", device_name);
-	if (ret < 0)
-		return -ENOMEM;
-
-	fd = open(chrdev_name, 0);
-	if (fd == -1) {
-		ret = -errno;
-		fprintf(stderr, "Failed to open %s\n", chrdev_name);
-		goto exit_close_error;
-	}
-
-	/* Request lines as output */
-	for (i = 0; i < nlines; i++)
-		req.lineoffsets[i] = lines[i];
-	req.flags = GPIOHANDLE_REQUEST_OUTPUT; /* Request as output */
-	strcpy(req.consumer_label, "gpio-hammer");
-	req.lines = nlines;
-	ret = ioctl(fd, GPIO_GET_LINEHANDLE_IOCTL, &req);
-	if (ret == -1) {
-		ret = -errno;
-		fprintf(stderr, "Failed to issue GET LINEHANDLE "
-			"IOCTL (%d)\n",
-			ret);
-		goto exit_close_error;
-	}
-
-	/* Read initial states */
-	ret = ioctl(req.fd, GPIOHANDLE_GET_LINE_VALUES_IOCTL, &data);
-	if (ret == -1) {
-		ret = -errno;
+	/* Do not set input or output to avoid change direction or value */
+	ret = gpio_gets(device_name, lines, nlines, 0, &data);
+	if (ret < 0) {
 		fprintf(stderr, "Failed to issue GPIOHANDLE GET LINE "
 			"VALUES IOCTL (%d)\n",
 			ret);
@@ -92,18 +63,18 @@ int hammer_device(const char *device_name, unsigned int *lines, int nlines,
 		for (i = 0; i < nlines; i++)
 			data.values[i] = !data.values[i];
 
-		ret = ioctl(req.fd, GPIOHANDLE_SET_LINE_VALUES_IOCTL, &data);
-		if (ret == -1) {
-			ret = -errno;
+		ret = gpio_sets(device_name, lines, nlines,
+				GPIOHANDLE_REQUEST_OUTPUT, &data);
+		if (ret < 0) {
 			fprintf(stderr, "Failed to issue GPIOHANDLE SET LINE "
 				"VALUES IOCTL (%d)\n",
 				ret);
 			goto exit_close_error;
 		}
 		/* Re-read values to get status */
-		ret = ioctl(req.fd, GPIOHANDLE_GET_LINE_VALUES_IOCTL, &data);
-		if (ret == -1) {
-			ret = -errno;
+		ret = gpio_gets(device_name, lines, nlines,
+				GPIOHANDLE_REQUEST_OUTPUT, &data);
+		if (ret < 0) {
 			fprintf(stderr, "Failed to issue GPIOHANDLE GET LINE "
 				"VALUES IOCTL (%d)\n",
 				ret);
@@ -132,9 +103,6 @@ int hammer_device(const char *device_name, unsigned int *lines, int nlines,
 	ret = 0;
 
 exit_close_error:
-	if (close(fd) == -1)
-		perror("Failed to close GPIO character device file");
-	free(chrdev_name);
 	return ret;
 }
 
-- 
1.8.4.5

[PATCH v3 3/5] gpio/mockup: add virtual gpio device

[bamvor.zhangjian]

From: Bamvor Jian Zhang <bamvor.zhangjian@linaro.org>

This patch add basic structure of a virtual gpio device(gpio-mockup)
for testing gpio subsystem. The tester could manipulate such device
through userspace(sysfs or char device) and check the result from
debugfs.

Currently, it support one or more gpiochip(determined by module
parameters with base,ngpio pair). One could test the overlap of
different gpiochip and test the direction and/or output values of
these chips.

Signed-off-by: Kamlakant Patel <kamlakant.patel@broadcom.com>
Signed-off-by: Bamvor Jian Zhang <bamvor.zhangjian@linaro.org>
---
 Documentation/kernel-parameters.txt |   4 +
 drivers/gpio/Kconfig                |  12 ++
 drivers/gpio/Makefile               |   1 +
 drivers/gpio/gpio-mockup.c          | 214 ++++++++++++++++++++++++++++++++++++
 4 files changed, 231 insertions(+)
 create mode 100644 drivers/gpio/gpio-mockup.c

diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
index 46c030a..61f4dbf 100644
--- a/Documentation/kernel-parameters.txt
+++ b/Documentation/kernel-parameters.txt
@@ -1364,6 +1364,10 @@ bytes respectively. Such letter suffixes can also be entirely omitted.
 			Format: <unsigned int> such that (rxsize & ~0x1fffc0) == 0.
 			Default: 1024
 
+	gpio-mockup.gpio_mockup_ranges
+			[HW] Sets the ranges of gpiochip of for this device.
+			Format: <start1>,<end1>,<start2>,<end2>...
+
 	hardlockup_all_cpu_backtrace=
 			[KNL] Should the hard-lockup detector generate
 			backtraces on all cpus.
diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
index 5eb7c86..4393bdf 100644
--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -271,6 +271,18 @@ config GPIO_MM_LANTIQ
 	  (EBU) found on Lantiq SoCs. The gpios are output only as they are
 	  created by attaching a 16bit latch to the bus.
 
+config GPIO_MOCKUP
+	tristate "GPIO Testing Driver"
+	depends on GPIOLIB
+	select GPIO_SYSFS
+	help
+	  This enables GPIO Testing driver, which provides a way to test GPIO
+	  subsystem through sysfs(or char device) and debugfs. GPIO_SYSFS
+	  must be selected for this test.
+	  User could use it through the script in
+	  tools/testing/selftests/gpio/gpio-mockup.sh. Reference the usage in
+	  it.
+
 config GPIO_MOXART
 	bool "MOXART GPIO support"
 	depends on ARCH_MOXART || COMPILE_TEST
diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile
index 691fb3a..540d7ab 100644
--- a/drivers/gpio/Makefile
+++ b/drivers/gpio/Makefile
@@ -72,6 +72,7 @@ obj-$(CONFIG_GPIO_MC9S08DZ60)	+= gpio-mc9s08dz60.o
 obj-$(CONFIG_GPIO_MCP23S08)	+= gpio-mcp23s08.o
 obj-$(CONFIG_GPIO_ML_IOH)	+= gpio-ml-ioh.o
 obj-$(CONFIG_GPIO_MM_LANTIQ)	+= gpio-mm-lantiq.o
+obj-$(CONFIG_GPIO_MOCKUP)      += gpio-mockup.o
 obj-$(CONFIG_GPIO_MOXART)	+= gpio-moxart.o
 obj-$(CONFIG_GPIO_MPC5200)	+= gpio-mpc5200.o
 obj-$(CONFIG_GPIO_MPC8XXX)	+= gpio-mpc8xxx.o
diff --git a/drivers/gpio/gpio-mockup.c b/drivers/gpio/gpio-mockup.c
new file mode 100644
index 0000000..1ef85b0
--- /dev/null
+++ b/drivers/gpio/gpio-mockup.c
@@ -0,0 +1,214 @@
+/*
+ * GPIO Testing Device Driver
+ *
+ * Copyright (C) 2014  Kamlakant Patel <kamlakant.patel@broadcom.com>
+ * Copyright (C) 2015-2016  Bamvor Jian Zhang <bamvor.zhangjian@linaro.org>
+ *
+ * This program is free software; you can redistribute  it and/or modify it
+ * under  the terms of  the GNU General  Public License as published by the
+ * Free Software Foundation;  either version 2 of the  License, or (at your
+ * option) any later version.
+ *
+ */
+
+#include <linux/init.h>
+#include <linux/module.h>
+#include <linux/gpio/driver.h>
+#include <linux/platform_device.h>
+
+#define GPIO_NAME	"gpio-mockup"
+#define	MAX_GC		10
+
+enum direction {
+	OUT,
+	IN
+};
+
+/*
+ * struct gpio_pin_status - structure describing a GPIO status
+ * @dir:       Configures direction of gpio as "in" or "out", 0=in, 1=out
+ * @value:     Configures status of the gpio as 0(low) or 1(high)
+ */
+struct gpio_pin_status {
+	enum direction dir;
+	bool value;
+};
+
+struct mockup_gpio_controller {
+	struct gpio_chip gc;
+	struct gpio_pin_status *stats;
+};
+
+static int gpio_mockup_ranges[MAX_GC << 1];
+static int gpio_mockup_params_nr;
+module_param_array(gpio_mockup_ranges, int, &gpio_mockup_params_nr, 0400);
+
+const char pins_name_start = 'A';
+
+static int mockup_gpio_get(struct gpio_chip *gc, unsigned int offset)
+{
+	struct mockup_gpio_controller *cntr = gpiochip_get_data(gc);
+
+	return cntr->stats[offset].value;
+}
+
+static void mockup_gpio_set(struct gpio_chip *gc, unsigned int offset,
+			    int value)
+{
+	struct mockup_gpio_controller *cntr = gpiochip_get_data(gc);
+
+	cntr->stats[offset].value = !!value;
+}
+
+static int mockup_gpio_dirout(struct gpio_chip *gc, unsigned int offset,
+			      int value)
+{
+	struct mockup_gpio_controller *cntr = gpiochip_get_data(gc);
+
+	mockup_gpio_set(gc, offset, value);
+	cntr->stats[offset].dir = OUT;
+	return 0;
+}
+
+static int mockup_gpio_dirin(struct gpio_chip *gc, unsigned int offset)
+{
+	struct mockup_gpio_controller *cntr = gpiochip_get_data(gc);
+
+	cntr->stats[offset].dir = IN;
+	return 0;
+}
+
+static int mockup_gpio_get_direction(struct gpio_chip *gc, unsigned int offset)
+{
+	struct mockup_gpio_controller *cntr = gpiochip_get_data(gc);
+
+	return cntr->stats[offset].dir;
+}
+
+static int mockup_gpio_add(struct device *dev,
+			   struct mockup_gpio_controller *cntr,
+			   const char *name, int base, int ngpio)
+{
+	int ret;
+
+	cntr->gc.base = base;
+	cntr->gc.ngpio = ngpio;
+	cntr->gc.label = name;
+	cntr->gc.owner = THIS_MODULE;
+	cntr->gc.parent = dev;
+	cntr->gc.get = mockup_gpio_get;
+	cntr->gc.set = mockup_gpio_set;
+	cntr->gc.direction_output = mockup_gpio_dirout;
+	cntr->gc.direction_input = mockup_gpio_dirin;
+	cntr->gc.get_direction = mockup_gpio_get_direction;
+	cntr->stats = devm_kzalloc(dev, sizeof(*cntr->stats) * cntr->gc.ngpio,
+				   GFP_KERNEL);
+	if (!cntr->stats) {
+		ret = -ENOMEM;
+		goto err;
+	}
+	ret = devm_gpiochip_add_data(dev, &cntr->gc, cntr);
+	if (ret)
+		goto err;
+
+	dev_info(dev, "gpio<%d..%d> add successful!", base, base + ngpio);
+	return 0;
+err:
+	dev_err(dev, "gpio<%d..%d> add failed!", base, base + ngpio);
+	return ret;
+}
+
+static int mockup_gpio_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct mockup_gpio_controller *cntr;
+	int ret;
+	int i;
+	int base;
+	int ngpio;
+	char chip_name[sizeof(GPIO_NAME) + 3];
+
+	if (gpio_mockup_params_nr < 2)
+		return -EINVAL;
+
+	cntr = devm_kzalloc(dev, sizeof(*cntr) * (gpio_mockup_params_nr >> 1),
+			    GFP_KERNEL);
+	if (!cntr)
+		return -ENOMEM;
+
+	platform_set_drvdata(pdev, cntr);
+
+	for (i = 0; i < gpio_mockup_params_nr >> 1; i++) {
+		base = gpio_mockup_ranges[i * 2];
+		if (base == -1)
+			ngpio = gpio_mockup_ranges[i * 2 + 1];
+		else
+			ngpio = gpio_mockup_ranges[i * 2 + 1] - base;
+
+		if (ngpio >= 0) {
+			sprintf(chip_name, "%s-%c", GPIO_NAME,
+				pins_name_start + i);
+			ret = mockup_gpio_add(dev, &cntr[i],
+					      chip_name, base, ngpio);
+		} else {
+			ret = -1;
+		}
+		if (ret) {
+			if (base < 0)
+				dev_err(dev, "gpio<%d..%d> add failed\n",
+					base, ngpio);
+			else
+				dev_err(dev, "gpio<%d..%d> add failed\n",
+					base, base + ngpio);
+
+			return ret;
+		}
+	}
+
+	return 0;
+}
+
+static struct platform_driver mockup_gpio_driver = {
+	.driver = {
+		   .name = GPIO_NAME,
+		   },
+	.probe = mockup_gpio_probe,
+};
+
+static struct platform_device *pdev;
+static int __init mock_device_init(void)
+{
+	int err;
+
+	pdev = platform_device_alloc(GPIO_NAME, -1);
+	if (!pdev)
+		return -ENOMEM;
+
+	err = platform_device_add(pdev);
+	if (err) {
+		platform_device_put(pdev);
+		return err;
+	}
+
+	err = platform_driver_register(&mockup_gpio_driver);
+	if (err) {
+		platform_device_unregister(pdev);
+		return err;
+	}
+
+	return 0;
+}
+
+static void __exit mock_device_exit(void)
+{
+	platform_driver_unregister(&mockup_gpio_driver);
+	platform_device_unregister(pdev);
+}
+
+module_init(mock_device_init);
+module_exit(mock_device_exit);
+
+MODULE_AUTHOR("Kamlakant Patel <kamlakant.patel@broadcom.com>");
+MODULE_AUTHOR("Bamvor Jian Zhang <bamvor.zhangjian@linaro.org>");
+MODULE_DESCRIPTION("GPIO Testing driver");
+MODULE_LICENSE("GPL v2");
-- 
1.8.4.5

[PATCH v3 4/5] selftest/gpio: add gpio test case

[bamvor.zhangjian]

From: Bamvor Jian Zhang <bamvor.zhangjian@linaro.org>

This test script try to do whitebox testing for gpio subsystem(
based on gpiolib). It manipulate gpio-mockup device through sysfs
or char device and check the result from debugfs.

Test the following things:
1.  Add single, multi gpiochip with the checking of overlap.
2.  Test direction and output value for valid pin.
3.  Test dynamic allocation of gpio base.

Signed-off-by: Bamvor Jian Zhang <bamvor.zhangjian@linaro.org>
---
 tools/testing/selftests/Makefile                   |   1 +
 tools/testing/selftests/gpio/Makefile              |  23 ++
 tools/testing/selftests/gpio/gpio-mockup-chardev.c | 321 +++++++++++++++++++++
 tools/testing/selftests/gpio/gpio-mockup-sysfs.sh  | 134 +++++++++
 tools/testing/selftests/gpio/gpio-mockup.sh        | 200 +++++++++++++
 5 files changed, 679 insertions(+)
 create mode 100644 tools/testing/selftests/gpio/Makefile
 create mode 100644 tools/testing/selftests/gpio/gpio-mockup-chardev.c
 create mode 100755 tools/testing/selftests/gpio/gpio-mockup-sysfs.sh
 create mode 100755 tools/testing/selftests/gpio/gpio-mockup.sh

diff --git a/tools/testing/selftests/Makefile b/tools/testing/selftests/Makefile
index ff9e5f2..2437dbc 100644
--- a/tools/testing/selftests/Makefile
+++ b/tools/testing/selftests/Makefile
@@ -6,6 +6,7 @@ TARGETS += exec
 TARGETS += firmware
 TARGETS += ftrace
 TARGETS += futex
+TARGETS += gpio
 TARGETS += ipc
 TARGETS += kcmp
 TARGETS += lib
diff --git a/tools/testing/selftests/gpio/Makefile b/tools/testing/selftests/gpio/Makefile
new file mode 100644
index 0000000..e1144ba
--- /dev/null
+++ b/tools/testing/selftests/gpio/Makefile
@@ -0,0 +1,23 @@
+
+TEST_PROGS := gpio-mockup.sh
+TEST_FILES := gpio-mockup-sysfs.sh $(BINARIES)
+BINARIES := gpio-mockup-chardev
+
+include ../lib.mk
+
+all: $(BINARIES)
+
+clean:
+	$(RM) $(BINARIES)
+
+CFLAGS += -O2 -g -std=gnu99 -Wall -I../../../../usr/include/
+LDLIBS += -lmount -I/usr/include/libmount
+
+$(BINARIES): ../../../gpio/gpio-utils.o ../../../../usr/include/linux/gpio.h
+
+../../../gpio/gpio-utils.o:
+	make ARCH=$(ARCH) CROSS_COMPILE=$(CROSS_COMPILE) -C ../../../gpio
+
+../../../../usr/include/linux/gpio.h:
+	make -C ../../../.. headers_install
+
diff --git a/tools/testing/selftests/gpio/gpio-mockup-chardev.c b/tools/testing/selftests/gpio/gpio-mockup-chardev.c
new file mode 100644
index 0000000..1868db6
--- /dev/null
+++ b/tools/testing/selftests/gpio/gpio-mockup-chardev.c
@@ -0,0 +1,321 @@
+/*
+ * GPIO chardev test helper
+ *
+ * Copyright (C) 2016 Bamvor Jian Zhang
+ *
+ * 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.
+ */
+
+#define _GNU_SOURCE
+#include <unistd.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <stdio.h>
+#include <errno.h>
+#include <string.h>
+#include <fcntl.h>
+#include <getopt.h>
+#include <sys/ioctl.h>
+#include <libmount.h>
+#include <err.h>
+#include <dirent.h>
+#include <linux/gpio.h>
+#include "../../../gpio/gpio-utils.h"
+
+#define CONSUMER	"gpio-selftest"
+#define	GC_NUM		10
+enum direction {
+	OUT,
+	IN
+};
+
+static int get_debugfs(char **path)
+{
+	struct libmnt_context *cxt;
+	struct libmnt_table *tb;
+	struct libmnt_iter *itr = NULL;
+	struct libmnt_fs *fs;
+	int found = 0;
+
+	cxt = mnt_new_context();
+	if (!cxt)
+		err(EXIT_FAILURE, "libmount context allocation failed");
+
+	itr = mnt_new_iter(MNT_ITER_FORWARD);
+	if (!itr)
+		err(EXIT_FAILURE, "failed to initialize libmount iterator");
+
+	if (mnt_context_get_mtab(cxt, &tb))
+		err(EXIT_FAILURE, "failed to read mtab");
+
+	while (mnt_table_next_fs(tb, itr, &fs) == 0) {
+		const char *type = mnt_fs_get_fstype(fs);
+
+		if (!strcmp(type, "debugfs")) {
+			found = 1;
+			break;
+		}
+	}
+	if (found)
+		asprintf(path, "%s/gpio", mnt_fs_get_target(fs));
+
+	mnt_free_iter(itr);
+	mnt_free_context(cxt);
+
+	if (!found)
+		return -1;
+
+	return 0;
+}
+
+static int gpio_debugfs_get(const char *consumer, int *dir, int *value)
+{
+	char *debugfs;
+	FILE *f;
+	char *line = NULL;
+	size_t len = 0;
+	char *cur;
+	int found = 0;
+
+	if (get_debugfs(&debugfs) != 0)
+		err(EXIT_FAILURE, "debugfs is not mounted");
+
+	f = fopen(debugfs, "r");
+	if (!f)
+		err(EXIT_FAILURE, "read from gpio debugfs failed");
+
+	/*
+	 * gpio-2   (                    |gpio-selftest               ) in  lo
+	 */
+	while (getline(&line, &len, f) != -1) {
+		cur = strstr(line, consumer);
+		if (cur == NULL)
+			continue;
+
+		cur = strchr(line, ')');
+		if (!cur)
+			continue;
+
+		cur += 2;
+		if (!strncmp(cur, "out", 3)) {
+			*dir = OUT;
+			cur += 4;
+		} else if (!strncmp(cur, "in", 2)) {
+			*dir = IN;
+			cur += 4;
+		}
+
+		if (!strncmp(cur, "hi", 2))
+			*value = 1;
+		else if (!strncmp(cur, "lo", 2))
+			*value = 0;
+
+		found = 1;
+		break;
+	}
+	free(debugfs);
+	fclose(f);
+	free(line);
+
+	if (!found)
+		return -1;
+
+	return 0;
+}
+
+static struct gpiochip_info *list_gpiochip(const char *gpiochip_name, int *ret)
+{
+	struct gpiochip_info *cinfo;
+	struct gpiochip_info *current;
+	const struct dirent *ent;
+	DIR *dp;
+	char *chrdev_name;
+	int fd;
+	int i = 0;
+
+	cinfo = calloc(sizeof(struct gpiochip_info) * 4, GC_NUM + 1);
+	if (!cinfo)
+		err(EXIT_FAILURE, "gpiochip_info allocation failed");
+
+	current = cinfo;
+	dp = opendir("/dev");
+	if (!dp) {
+		*ret = -errno;
+		goto error_out;
+	}
+
+	while (ent = readdir(dp), ent) {
+		if (check_prefix(ent->d_name, "gpiochip")) {
+			*ret = asprintf(&chrdev_name, "/dev/%s", ent->d_name);
+			if (*ret < 0)
+				goto error_out;
+
+			fd = open(chrdev_name, 0);
+			if (fd == -1) {
+				*ret = -errno;
+				fprintf(stderr, "Failed to open %s\n",
+					chrdev_name);
+				goto error_close_dir;
+			}
+			*ret = ioctl(fd, GPIO_GET_CHIPINFO_IOCTL, current);
+			if (*ret == -1) {
+				perror("Failed to issue CHIPINFO IOCTL\n");
+				goto error_close_dir;
+			}
+			close(fd);
+			if (strcmp(current->label, gpiochip_name) == 0
+			    || check_prefix(current->label, gpiochip_name)) {
+				*ret = 0;
+				current++;
+				i++;
+			}
+		}
+	}
+
+	if ((!*ret && i == 0) || *ret < 0) {
+		free(cinfo);
+		cinfo = NULL;
+	}
+	if (!*ret && i > 0) {
+		cinfo = realloc(cinfo, sizeof(struct gpiochip_info) * 4 * i);
+		*ret = i;
+	}
+
+error_close_dir:
+	closedir(dp);
+error_out:
+	if (*ret < 0)
+		err(EXIT_FAILURE, "list gpiochip failed: %s", strerror(*ret));
+
+	return cinfo;
+}
+
+int gpio_pin_test(struct gpiochip_info *cinfo, int line, int flag, int value)
+{
+	struct gpiohandle_data data;
+	unsigned int lines[] = { line };
+	int fd;
+	int debugfs_dir;
+	int debugfs_value;
+	int ret;
+
+	data.values[0] = value;
+	ret = gpio_request(cinfo->name, lines, 1, flag, &data, CONSUMER);
+	if (ret < 0)
+		goto fail_out;
+	else
+		fd = ret;
+
+	ret = gpio_debugfs_get(CONSUMER, &debugfs_dir, &debugfs_value);
+	if (ret) {
+		ret = -EINVAL;
+		goto fail_out;
+	}
+	if (flag & GPIOHANDLE_REQUEST_INPUT) {
+		if (debugfs_dir != IN) {
+			errno = -EINVAL;
+			ret = -errno;
+		}
+	} else if (flag & GPIOHANDLE_REQUEST_OUTPUT) {
+		if (flag & GPIOHANDLE_REQUEST_ACTIVE_LOW)
+			debugfs_value = !debugfs_value;
+
+		if (!(debugfs_dir == OUT && value == debugfs_value))
+			errno = -EINVAL;
+		ret = -errno;
+
+	}
+
+	gpio_release(fd);
+fail_out:
+	if (ret)
+		err(EXIT_FAILURE, "gpio<%s> line<%d> test flag<0x%x> value<%d>",
+		    cinfo->name, line, flag, value);
+
+	return ret;
+}
+
+void gpio_pin_tests(struct gpiochip_info *cinfo, unsigned int line)
+{
+	printf("line<%d>", line);
+	gpio_pin_test(cinfo, line, GPIOHANDLE_REQUEST_OUTPUT, 0);
+	printf(".");
+	gpio_pin_test(cinfo, line, GPIOHANDLE_REQUEST_OUTPUT, 1);
+	printf(".");
+	gpio_pin_test(cinfo, line,
+		      GPIOHANDLE_REQUEST_OUTPUT | GPIOHANDLE_REQUEST_ACTIVE_LOW,
+		      0);
+	printf(".");
+	gpio_pin_test(cinfo, line,
+		      GPIOHANDLE_REQUEST_OUTPUT | GPIOHANDLE_REQUEST_ACTIVE_LOW,
+		      1);
+	printf(".");
+	gpio_pin_test(cinfo, line, GPIOHANDLE_REQUEST_INPUT, 0);
+	printf(".");
+}
+
+/*
+ * ./gpio-mockup-chardev gpio_chip_name_prefix is_valid_gpio_chip
+ * Return 0 if successful or exit with EXIT_FAILURE if test failed.
+ * gpio_chip_name_prefix: The prefix of gpiochip you want to test. E.g.
+ *			  gpio-mockup
+ * is_valid_gpio_chip:	  Whether the gpio_chip is valid. 1 means valid,
+ *			  0 means invalid which could not be found by
+ *			  list_gpiochip.
+ */
+int main(int argc, char *argv[])
+{
+	char *prefix;
+	int valid;
+	struct gpiochip_info *cinfo;
+	struct gpiochip_info *current;
+	int i;
+	int ret;
+
+	if (argc < 3) {
+		printf("Usage: %s prefix is_valid", argv[0]);
+		exit(EXIT_FAILURE);
+	}
+
+	prefix = argv[1];
+	valid = strcmp(argv[2], "true") == 0 ? 1 : 0;
+
+	printf("Test gpiochip %s: ", prefix);
+	cinfo = list_gpiochip(prefix, &ret);
+	if (!cinfo) {
+		if (!valid && ret == 0) {
+			printf("Invalid test successful\n");
+			ret = 0;
+			goto out;
+		} else {
+			ret = -EINVAL;
+			goto out;
+		}
+	} else if (cinfo && !valid) {
+		ret = -EINVAL;
+		goto out;
+	}
+	current = cinfo;
+	for (i = 0; i < ret; i++) {
+		gpio_pin_tests(current, 0);
+		gpio_pin_tests(current, current->lines - 1);
+		gpio_pin_tests(current, random() % current->lines);
+		current++;
+	}
+	ret = 0;
+	printf("successful\n");
+
+out:
+	if (ret)
+		fprintf(stderr, "gpio<%s> test failed\n", prefix);
+
+	if (cinfo)
+		free(cinfo);
+
+	if (ret)
+		exit(EXIT_FAILURE);
+
+	return ret;
+}
diff --git a/tools/testing/selftests/gpio/gpio-mockup-sysfs.sh b/tools/testing/selftests/gpio/gpio-mockup-sysfs.sh
new file mode 100755
index 0000000..085d7a3
--- /dev/null
+++ b/tools/testing/selftests/gpio/gpio-mockup-sysfs.sh
@@ -0,0 +1,134 @@
+
+is_consistent()
+{
+	val=
+
+	active_low_sysfs=`cat $GPIO_SYSFS/gpio$nr/active_low`
+	val_sysfs=`cat $GPIO_SYSFS/gpio$nr/value`
+	dir_sysfs=`cat $GPIO_SYSFS/gpio$nr/direction`
+
+	gpio_this_debugfs=`cat $GPIO_DEBUGFS |grep "gpio-$nr" | sed "s/(.*)//g"`
+	dir_debugfs=`echo $gpio_this_debugfs | awk '{print $2}'`
+	val_debugfs=`echo $gpio_this_debugfs | awk '{print $3}'`
+	if [ $val_debugfs = "lo" ]; then
+		val=0
+	elif [ $val_debugfs = "hi" ]; then
+		val=1
+	fi
+
+	if [ $active_low_sysfs = "1" ]; then
+		if [ $val = "0" ]; then
+			val="1"
+		else
+			val="0"
+		fi
+	fi
+
+	if [ $val_sysfs = $val ] && [ $dir_sysfs = $dir_debugfs ]; then
+		echo -n "."
+	else
+		echo "test fail, exit"
+		die
+	fi
+}
+
+test_pin_logic()
+{
+	nr=$1
+	direction=$2
+	active_low=$3
+	value=$4
+
+	echo $direction > $GPIO_SYSFS/gpio$nr/direction
+	echo $active_low > $GPIO_SYSFS/gpio$nr/active_low
+	if [ $direction = "out" ]; then
+		echo $value > $GPIO_SYSFS/gpio$nr/value
+	fi
+	is_consistent $nr
+}
+
+test_one_pin()
+{
+	nr=$1
+
+	echo -n "test pin<$nr>"
+
+	echo $nr > $GPIO_SYSFS/export 2>/dev/null
+
+	if [ X$? != X0 ]; then
+		echo "test GPIO pin $nr failed"
+		die
+	fi
+
+	#"Checking if the sysfs is consistent with debugfs: "
+	is_consistent $nr
+
+	#"Checking the logic of active_low: "
+	test_pin_logic $nr out 1 1
+	test_pin_logic $nr out 1 0
+	test_pin_logic $nr out 0 1
+	test_pin_logic $nr out 0 0
+
+	#"Checking the logic of direction: "
+	test_pin_logic $nr in 1 1
+	test_pin_logic $nr out 1 0
+	test_pin_logic $nr low 0 1
+	test_pin_logic $nr high 0 0
+
+	echo $nr > $GPIO_SYSFS/unexport
+
+	echo "successful"
+}
+
+test_one_pin_fail()
+{
+	nr=$1
+
+	echo $nr > $GPIO_SYSFS/export 2>/dev/null
+
+	if [ X$? != X0 ]; then
+		echo "test invalid pin $nr successful"
+	else
+		echo "test invalid pin $nr failed"
+		echo $nr > $GPIO_SYSFS/unexport 2>/dev/null
+		die
+	fi
+}
+
+list_chip()
+{
+	echo `ls -d $GPIO_DRV_SYSFS/gpiochip* 2>/dev/null`
+}
+
+test_chip()
+{
+	chip=$1
+	name=`basename $chip`
+	base=`cat $chip/base`
+	ngpio=`cat $chip/ngpio`
+	printf "%-10s %-5s %-5s\n" $name $base $ngpio
+	if [ $ngpio = "0" ]; then
+		echo "number of gpio is zero is not allowed".
+	fi
+	test_one_pin $base
+	test_one_pin $(($base + $ngpio - 1))
+	test_one_pin $((( RANDOM % $ngpio )  + $base ))
+}
+
+test_chips_sysfs()
+{
+       gpiochip=`list_chip $module`
+       if [ X"$gpiochip" = X ]; then
+               if [ X"$valid" = Xfalse ]; then
+                       echo "successful"
+               else
+                       echo "fail"
+                       die
+               fi
+       else
+               for chip in $gpiochip; do
+                       test_chip $chip
+               done
+       fi
+}
+
diff --git a/tools/testing/selftests/gpio/gpio-mockup.sh b/tools/testing/selftests/gpio/gpio-mockup.sh
new file mode 100755
index 0000000..256b094
--- /dev/null
+++ b/tools/testing/selftests/gpio/gpio-mockup.sh
@@ -0,0 +1,200 @@
+#!/bin/bash
+
+#exit status
+#1: run as non-root user
+#2: sysfs/debugfs not mount
+#3: insert module fail when gpio-mockup is a module.
+#4: other reason.
+
+SYSFS=
+GPIO_SYSFS=
+GPIO_DRV_SYSFS=
+DEBUGFS=
+GPIO_DEBUGFS=
+dev_type=
+module=
+
+usage()
+{
+	echo "Usage:"
+	echo "$0 [-f] [-m name] [-t type]"
+	echo "-f:  full test. It maybe conflict with existence gpio device."
+	echo "-m:  module name, default name is gpio-mockup. It could also test"
+	echo "     other gpio device."
+	echo "-t:  interface type: sysfs or chardev(char device). The latter"
+	echo "     one is default"
+	echo ""
+	echo "$0 -h"
+	echo "This usage"
+}
+
+prerequisite()
+{
+	msg="skip all tests:"
+	if [ $UID != 0 ]; then
+		echo $msg must be run as root >&2
+		exit 1
+	fi
+	SYSFS=`mount -t sysfs | head -1 | awk '{ print $3 }'`
+	if [ ! -d "$SYSFS" ]; then
+		echo $msg sysfs is not mounted >&2
+		exit 2
+	fi
+	GPIO_SYSFS=`echo $SYSFS/class/gpio`
+	GPIO_DRV_SYSFS=`echo $SYSFS/devices/platform/$module/gpio`
+	DEBUGFS=`mount -t debugfs | head -1 | awk '{ print $3 }'`
+	if [ ! -d "$DEBUGFS" ]; then
+		echo $msg debugfs is not mounted >&2
+		exit 2
+	fi
+	GPIO_DEBUGFS=`echo $DEBUGFS/gpio`
+	source gpio-mockup-sysfs.sh
+}
+
+try_insert_module()
+{
+	if [ -d "$GPIO_DRV_SYSFS" ]; then
+		echo "$GPIO_DRV_SYSFS exist. Skip insert module"
+	else
+		modprobe -q $module $1
+		if [ X$? != X0 ]; then
+			echo $msg insmod $module failed >&2
+			exit 3
+		fi
+	fi
+}
+
+remove_module()
+{
+	modprobe -r -q $module
+}
+
+die()
+{
+	remove_module
+	exit 4
+}
+
+test_chips()
+{
+	if [ X$dev_type = Xsysfs ]; then
+		test_chips_sysfs $*
+	else
+		$BASE/gpio-mockup-chardev $*
+	fi
+}
+
+gpio_test()
+{
+	param=$1
+	valid=$2
+
+	if [ X"$param" = X ]; then
+		die
+	fi
+	try_insert_module "gpio_mockup_ranges=$param"
+	echo -n "GPIO $module test with ranges: <"
+	echo "$param>: "
+	printf "%-10s %s\n" $param
+	test_chips $module $valid
+	remove_module
+}
+
+BASE=`dirname $0`
+
+dev_type=
+TEMP=`getopt -o fhm:t: -n '$0' -- "$@"`
+
+if [ "$?" != "0" ]; then
+        echo "Parameter process failed, Terminating..." >&2
+        exit 1
+fi
+
+# Note the quotes around `$TEMP': they are essential!
+eval set -- "$TEMP"
+
+while true; do
+	case $1 in
+	-f)
+		full_test=true
+		shift
+		;;
+	-h)
+		usage
+		exit
+		;;
+	-m)
+		module=$2
+		shift 2
+		;;
+	-t)
+		dev_type=$2
+		shift 2
+		;;
+	--)
+		shift
+		break
+		;;
+	*)
+		echo "Internal error!"
+		exit 1
+		;;
+	esac
+done
+
+if [ X"$module" = X ]; then
+	module="gpio-mockup"
+fi
+
+if [ X$dev_type != Xsysfs ]; then
+	dev_type="chardev"
+fi
+
+prerequisite
+
+echo "1.  Test dynamic allocation of gpio successful means insert gpiochip and"
+echo "    manipulate gpio pin successful"
+gpio_test "-1,32" true
+gpio_test "-1,32,-1,32" true
+gpio_test "-1,32,-1,32,-1,32" true
+if [ X$full_test = Xtrue ]; then
+	gpio_test "-1,32,32,64" true
+	gpio_test "-1,32,40,64,-1,5" true
+	gpio_test "-1,32,32,64,-1,32" true
+	gpio_test "0,32,32,64,-1,32,-1,32" true
+	gpio_test "-1,32,-1,32,0,32,32,64" true
+	echo "2.  Do basic test: successful means insert gpiochip and"
+	echo "    manipulate gpio pin successful"
+	gpio_test "0,32" true
+	gpio_test "0,32,32,64" true
+	gpio_test "0,32,40,64,64,96" true
+fi
+echo "3.  Error test: successful means insert gpiochip failed"
+echo "3.1 Test number of gpio overflow"
+#Currently: The max number of gpio(1024) is defined in arm architecture.
+gpio_test "-1,32,-1,1024" false
+if [ X$full_test = Xtrue ]; then
+	echo "3.2 Test zero line of gpio"
+	gpio_test "0,0" false
+	echo "3.3 Test range overlap"
+	echo "3.3.1 Test corner case"
+	gpio_test "0,32,0,1" false
+	gpio_test "0,32,32,64,32,40" false
+	gpio_test "0,32,35,64,35,45" false
+	gpio_test "0,32,31,32" false
+	gpio_test "0,32,32,64,36,37" false
+	gpio_test "0,32,35,64,34,36" false
+	echo "3.3.2 Test inserting invalid second gpiochip"
+	gpio_test "0,32,30,35" false
+	gpio_test "0,32,1,5" false
+	gpio_test "10,32,9,14" false
+	gpio_test "10,32,30,35" false
+	echo "3.3.3 Test others"
+	gpio_test "0,32,40,56,39,45" false
+	gpio_test "0,32,40,56,30,33" false
+	gpio_test "0,32,40,56,30,41" false
+	gpio_test "0,32,40,56,20,21" false
+fi
+
+echo GPIO test PASS
+
-- 
1.8.4.5

[PATCH v3 5/5] gpio: MAINTAINERS: Add an entry for GPIO mockup driver

[bamvor.zhangjian]

From: Bamvor Jian Zhang <bamvor.zhangjian@linaro.org>

Add an entry for the GPIO mockup driver with myself as maintainer.

Signed-off-by: Bamvor Jian Zhang <bamvor.zhangjian@linaro.org>
---
 MAINTAINERS | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index a9ffa78..2dc4f52 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -5241,6 +5241,13 @@ L:	linux-input@vger.kernel.org
 S:	Maintained
 F:	drivers/input/touchscreen/goodix.c
 
+GPIO MOCKUP DRIVER
+M:	Bamvor Jian Zhang <bamvor.zhangjian@linaro.org>
+L:	linux-gpio@vger.kernel.org
+S:	Maintained
+F:	drivers/gpio/gpio-mockup.c
+F:	tools/testing/selftests/gpio/
+
 GPIO SUBSYSTEM
 M:	Linus Walleij <linus.walleij@linaro.org>
 M:	Alexandre Courbot <gnurou@gmail.com>
-- 
1.8.4.5

Re: [PATCH v3 3/5] gpio/mockup: add virtual gpio device

[Linus Walleij]

On Wed, Aug 31, 2016 at 2:45 AM,  <bamvor.zhangjian@linaro.org> wrote:

> From: Bamvor Jian Zhang <bamvor.zhangjian@linaro.org>
>
> This patch add basic structure of a virtual gpio device(gpio-mockup)
> for testing gpio subsystem. The tester could manipulate such device
> through userspace(sysfs or char device) and check the result from
> debugfs.
>
> Currently, it support one or more gpiochip(determined by module
> parameters with base,ngpio pair). One could test the overlap of
> different gpiochip and test the direction and/or output values of
> these chips.
>
> Signed-off-by: Kamlakant Patel <kamlakant.patel@broadcom.com>
> Signed-off-by: Bamvor Jian Zhang <bamvor.zhangjian@linaro.org>

This patch applied. It's a very simple, straight-forward driver that does
exactly what we want.

Yours,
Linus Walleij

Re: [PATCH v3 5/5] gpio: MAINTAINERS: Add an entry for GPIO mockup driver

[Linus Walleij]

On Wed, Aug 31, 2016 at 2:45 AM,  <bamvor.zhangjian@linaro.org> wrote:

> From: Bamvor Jian Zhang <bamvor.zhangjian@linaro.org>
>
> Add an entry for the GPIO mockup driver with myself as maintainer.
>
> Signed-off-by: Bamvor Jian Zhang <bamvor.zhangjian@linaro.org>

Patch applied.

Yours,
Linus Walleij

Re: [PATCH v3 1/5] tools/gpio: add gpio basic opereations

[Linus Walleij]

On Wed, Aug 31, 2016 at 2:45 AM,  <bamvor.zhangjian@linaro.org> wrote:

> +int gpio_request(const char *device_name, unsigned int *lines,
> +                unsigned int nlines, unsigned int flag,
> +                struct gpiohandle_data *data, const char *consumer_label)

Could it be named
gpiotools_request_linehandle()?

Or the name is maybe a bit long.

It's nice if we have a name that tells all the function is doing.

> +int gpio_set_values(const int fd, struct gpiohandle_data *data)

Pls rename
gpiotools_set_values()

> +int gpio_get_values(const int fd, struct gpiohandle_data *data)

Pls rename
gpiotools_get_values()

> +int gpio_release(const int fd)

Pls rename
gpiotools_release_linehandle()

> +int gpio_gets(const char *device_name, unsigned int *lines, unsigned int nlines,
> +             unsigned int flag, struct gpiohandle_data *data)
> +int gpio_sets(const char *device_name, unsigned int *lines, unsigned int nlines,
> +             unsigned int flag, struct gpiohandle_data *data)
> +int gpio_get(const char *device_name, unsigned int line, unsigned int flag)
> +int gpio_set(const char *device_name, unsigned int line, unsigned int flag,
> +            unsigned int value)
> +int gpio_set_flag(const char *device_name, unsigned int line, unsigned int flag)

Prefix all gpiotools_*

> +/*
> + * Request the lines for gpio with device_name. Could set the default value
> + * in request.
> + * device_name:    the name of gpiochip in "/dev", such as gpiochip0.
> + * lines:         the array of which line should be requested.
> + * nline:          the total number of line
> + * flag:           input, output and so on. Reference "linux/gpio.h" for the
> + *                 meaning for flag.
> + * data:          default value when flag is GPIOHANDLE_REQUEST_OUTPUT.
> + * consumer_label: the name of consumer, such as "sysfs", "powerkey".
> + *
> + * Return value:   On success return the fd of specific gpiochip. It could be
> + *                 release by gpio_release.
> + *                On failure return the errno.
> + */
> +int gpio_request(const char *device_name, unsigned int *lines,
> +                unsigned int nlines, unsigned int flag,
> +                struct gpiohandle_data *data, const char *consumer_label);

For all of those, convert to kerneldoc (Documentation/kernel-doc-nano-HOWTO)
and move the kerneldoc above the actual implementation, keep the
header file spartan.

Yours,
Linus Walleij

Re: [PATCH v3 1/5] tools/gpio: add gpio basic opereations

[Michael Welling]

On Wed, Aug 31, 2016 at 11:45:44AM +0200, bamvor.zhangjian@linaro.org wrote:
> From: Bamvor Jian Zhang <bamvor.zhangjian@linaro.org>
> 
> Add basic gpio operations. User could get/set gpio value and/or flag for
> specific gpio chardev.
> 
> Reference the "tools/testing/selftest/gpio/gpio-mockup-chardev.c" for
> how to use it.
> 
> Signed-off-by: Bamvor Jian Zhang <bamvor.zhangjian@linaro.org>
> ---
>  tools/gpio/gpio-utils.c | 163 ++++++++++++++++++++++++++++++++++++++++++++++++
>  tools/gpio/gpio-utils.h | 121 +++++++++++++++++++++++++++++++++++
>  2 files changed, 284 insertions(+)
> 
> diff --git a/tools/gpio/gpio-utils.c b/tools/gpio/gpio-utils.c
> index 8208718..3c0a35c 100644
> --- a/tools/gpio/gpio-utils.c
> +++ b/tools/gpio/gpio-utils.c
> @@ -2,10 +2,173 @@
>   * GPIO tools - helpers library for the GPIO tools
>   *
>   * Copyright (C) 2015 Linus Walleij
> + * Copyright (C) 2016 Bamvor Jian Zhang
>   *
>   * 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 <unistd.h>
> +#include <stdlib.h>
> +#include <stdio.h>
> +#include <errno.h>
> +#include <string.h>
> +#include <fcntl.h>
> +#include <getopt.h>
> +#include <sys/ioctl.h>
> +#include <linux/gpio.h>
>  #include "gpio-utils.h"
> +
> +#define COMSUMER "gpio-utils"
> +
> +int gpio_request(const char *device_name, unsigned int *lines,
> +		 unsigned int nlines, unsigned int flag,
> +		 struct gpiohandle_data *data, const char *consumer_label)
> +{
> +	struct gpiohandle_request req;
> +	char *chrdev_name;
> +	int fd;
> +	int i;
> +	int ret;
> +
> +	ret = asprintf(&chrdev_name, "/dev/%s", device_name);
> +	if (ret < 0)
> +		return -ENOMEM;
> +
> +	fd = open(chrdev_name, 0);
> +	if (fd == -1) {
> +		ret = -errno;
> +		fprintf(stderr, "Failed to open %s\n", chrdev_name);
> +		goto exit_close_error;
> +	}
> +
> +	for (i = 0; i < nlines; i++)
> +		req.lineoffsets[i] = lines[i];
> +
> +	req.flags = flag;
> +	strcpy(req.consumer_label, consumer_label);
> +	req.lines = nlines;
> +	if (flag & GPIOHANDLE_REQUEST_OUTPUT)
> +		memcpy(req.default_values, data, sizeof(req.default_values));
> +
> +	ret = ioctl(fd, GPIO_GET_LINEHANDLE_IOCTL, &req);
> +	if (ret == -1) {
> +		ret = -errno;
> +		fprintf(stderr, "Failed to issue GET LINEHANDLE IOCTL (%d)\n",
> +			ret);
> +		goto exit_close_error;
> +	}
> +
> +exit_close_error:
> +	if (close(fd) == -1)
> +		perror("Failed to close GPIO character device file");
> +	free(chrdev_name);
> +	return ret < 0 ? ret : req.fd;
> +}
> +
> +int gpio_set_values(const int fd, struct gpiohandle_data *data)
> +{
> +	int ret;
> +
> +	ret = ioctl(fd, GPIOHANDLE_SET_LINE_VALUES_IOCTL, data);
> +	if (ret == -1) {
> +		ret = -errno;
> +		fprintf(stderr, "Failed to issue %s (%d)\n",
> +			"GPIOHANDLE_SET_LINE_VALUES_IOCTL", ret);
> +		goto exit_close_error;

This goto is unnecessary because there is no code to be skipped.

> +	}
> +
> +exit_close_error:
> +	return ret;
> +}
> +
> +int gpio_get_values(const int fd, struct gpiohandle_data *data)
> +{
> +	int ret;
> +
> +	ret = ioctl(fd, GPIOHANDLE_GET_LINE_VALUES_IOCTL, data);
> +	if (ret == -1) {
> +		ret = -errno;
> +		fprintf(stderr, "Failed to issue %s (%d)\n",
> +			"GPIOHANDLE_GET_LINE_VALUES_IOCTL", ret);
> +		goto exit_close_error;

Same here.

> +	}
> +
> +exit_close_error:
> +	return ret;
> +}
> +
> +int gpio_release(const int fd)
> +{
> +	int ret;
> +
> +	ret = close(fd);
> +	if (ret < -1)
> +		perror("Failed to close GPIO LINEHANDLE device file");
> +
> +	return ret;
> +}
> +
> +int gpio_gets(const char *device_name, unsigned int *lines, unsigned int nlines,
> +	      unsigned int flag, struct gpiohandle_data *data)
> +{
> +	int fd;
> +	int ret;
> +
> +	ret = gpio_request(device_name, lines, nlines, flag, data, COMSUMER);
> +	if (ret < 0)
> +		return ret;
> +
> +	fd = ret;
> +	ret = gpio_get_values(fd, data);

The error checking is missing here.

> +	ret = gpio_release(fd);

Shouldn't we leave it up the user how they want the deal with the file handle?
There is more system call overhead if we open and close the GPIO device for
each access.

> +	return ret;
> +}
> +
> +int gpio_sets(const char *device_name, unsigned int *lines, unsigned int nlines,
> +	      unsigned int flag, struct gpiohandle_data *data)
> +{
> +	int ret;
> +
> +	ret = gpio_request(device_name, lines, nlines, flag, data, COMSUMER);
> +	if (ret < 0)
> +		return ret;
> +
> +	ret = gpio_release(ret);
> +	return ret;
> +}
> +
> +int gpio_get(const char *device_name, unsigned int line, unsigned int flag)
> +{
> +	struct gpiohandle_data data;
> +	unsigned int lines[] = {line};

Is the lines variable really needed?

> +
> +	gpio_gets(device_name, lines, 1, flag, &data);
> +	return data.values[0];
> +}
> +
> +int gpio_set(const char *device_name, unsigned int line, unsigned int flag,
> +	     unsigned int value)
> +{
> +	struct gpiohandle_data data;
> +	unsigned int lines[] = {line};
> +
> +	data.values[0] = value;
> +	return gpio_sets(device_name, lines, 1, flag, &data);
> +}
> +
> +int gpio_set_flag(const char *device_name, unsigned int line, unsigned int flag)
> +{
> +	struct gpiohandle_data data;
> +	unsigned int lines[] = {line};
> +	int ret;
> +
> +	ret = gpio_request(device_name, lines, 1, flag, &data, COMSUMER);
> +	if (ret < 0)
> +		return ret;
> +
> +	ret = gpio_release(ret);
> +	return ret;
> +}
> +
> diff --git a/tools/gpio/gpio-utils.h b/tools/gpio/gpio-utils.h
> index 5f57133..fcf87b6 100644
> --- a/tools/gpio/gpio-utils.h
> +++ b/tools/gpio/gpio-utils.h
> @@ -24,4 +24,125 @@ static inline int check_prefix(const char *str, const char *prefix)
>  		strncmp(str, prefix, strlen(prefix)) == 0;
>  }
>  
> +/* Basic operation of gpio */
> +/*
> + * Request the lines for gpio with device_name. Could set the default value
> + * in request.
> + * device_name:    the name of gpiochip in "/dev", such as gpiochip0.
> + * lines:	   the array of which line should be requested.
> + * nline:          the total number of line
> + * flag:           input, output and so on. Reference "linux/gpio.h" for the
> + *                 meaning for flag.
> + * data:	   default value when flag is GPIOHANDLE_REQUEST_OUTPUT.
> + * consumer_label: the name of consumer, such as "sysfs", "powerkey".
> + *
> + * Return value:   On success return the fd of specific gpiochip. It could be
> + *                 release by gpio_release.
> + *		   On failure return the errno.
> + */
> +int gpio_request(const char *device_name, unsigned int *lines,
> +		 unsigned int nlines, unsigned int flag,
> +		 struct gpiohandle_data *data, const char *consumer_label);
> +/*
> + * Set the value of gpio for fd
> + * fd:             the fd returned by gpio_request
> + * data:	   the value want to set
> + *
> + * Return value:   On success return 0
> + *		   On failure return the errno.
> + */
> +int gpio_set_values(const int fd, struct gpiohandle_data *data);
> +
> +/*
> + * Get the value of gpio for fd
> + * fd:             the fd returned by gpio_request
> + * data:	   the valud get from gpiochip.
> + *
> + * Return value:   On success return 0
> + *		   On failure return the errno.
> + */
> +int gpio_get_values(const int fd, struct gpiohandle_data *data);
> +
> +/*
> + * release the fd for gpiochip
> + */
> +int gpio_release(const int fd);
> +
> +/* Easy operation for one time get/set */
> +/*
> + * Get one pin value from line of device_name. Could change the flag if
> + * necessary.
> + * device_name: the name of gpiochip in "/dev", such as gpiochip0.
> + * line:        number of line, such as 2.
> + * flag:        input, output and so on. Reference "linux/gpio.h" for the
> + *              meaning for flag. Set to 0 if do not want to update the
> + *              flag. It is the recommandation value.
> + *
> + * Return value:   On success return the value get from gpiochip.
> + *		   On failure return the errno.
> + */
> +int gpio_get(const char *device_name, unsigned int line, unsigned int flag);
> +
> +/*
> + * Get pins value from line of device_name. Could change the flag if
> + * necessary.
> + * device_name: the name of gpiochip in "/dev", such as gpiochip0.
> + * lines:	   the array of which line should be requested.
> + * nline:          the total number of line
> + * flag:           input, output and so on. Reference "linux/gpio.h" for the
> + *                 meaning for flag.
> + * data:	   the valud get from gpiochip.
> + *
> + * Return value:   On success return 0
> + *		   On failure return the errno.
> + */
> +int gpio_gets(const char *device_name, unsigned int *lines, unsigned int nlines,
> +	      unsigned int flag, struct gpiohandle_data *data);
> +
> +/*
> + * Set one pin value from line of device_name. Could change the flag if
> + * necessary.
> + * device_name: the name of gpiochip in "/dev", such as gpiochip0.
> + * line:        number of line, such as 2.
> + * flag:        input, output and so on. Reference "linux/gpio.h" for the
> + *              meaning for flag. Set to 0 if do not want to update the
> + *              flag. It is the recommandation value.
> + * value:	the value want to set the gpio line.
> + *
> + * Return value:   On success return 0
> + *		   On failure return the errno.
> + */
> +int gpio_set(const char *device_name, unsigned int line, unsigned int flag,
> +	     unsigned int value);
> +
> +/*
> + * Set pins value from line of device_name. Could change the flag if
> + * necessary.
> + * device_name: the name of gpiochip in "/dev", such as gpiochip0.
> + * lines:	   the array of which line should be requested.
> + * nline:          the total number of line
> + * flag:           input, output and so on. Reference "linux/gpio.h" for the
> + *                 meaning for flag.
> + * data:	   the value want to set
> + *
> + * Return value:   On success return 0
> + *		   On failure return the errno.
> + */
> +int gpio_sets(const char *device_name, unsigned int *lines, unsigned int nlines,
> +	      unsigned int flag, struct gpiohandle_data *data);
> +
> +/*
> + * set the flag for device_name. Mainly for changing the direction of gpio
> + *
> + * device_name: the name of gpiochip in "/dev", such as gpiochip0.
> + * line:        number of line, such as 2.
> + * flag:        input, output and so on. Reference "linux/gpio.h" for the
> + *              meaning for flag.
> + *
> + * Return value:   On success return 0
> + *		   On failure return the errno.
> + */
> +int gpio_set_flag(const char *device_name, unsigned int line,
> +		  unsigned int flag);
> +
>  #endif /* _GPIO_UTILS_H_ */
> -- 
> 1.8.4.5
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 1/5] tools/gpio: add gpio basic opereations

[Bamvor Zhang Jian]

Hi, Michael

On 28 September 2016 at 12:16, Michael Welling <mwelling@ieee.org> wrote:
> On Wed, Aug 31, 2016 at 11:45:44AM +0200, bamvor.zhangjian@linaro.org wrote:
>> From: Bamvor Jian Zhang <bamvor.zhangjian@linaro.org>
[...]
>> +     ret = ioctl(fd, GPIOHANDLE_SET_LINE_VALUES_IOCTL, data);
>> +     if (ret == -1) {
>> +             ret = -errno;
>> +             fprintf(stderr, "Failed to issue %s (%d)\n",
>> +                     "GPIOHANDLE_SET_LINE_VALUES_IOCTL", ret);
>> +             goto exit_close_error;
>
> This goto is unnecessary because there is no code to be skipped.
>
>> +     }
>> +
>> +exit_close_error:
>> +     return ret;
>> +}
>> +
>> +int gpio_get_values(const int fd, struct gpiohandle_data *data)
>> +{
>> +     int ret;
>> +
>> +     ret = ioctl(fd, GPIOHANDLE_GET_LINE_VALUES_IOCTL, data);
>> +     if (ret == -1) {
>> +             ret = -errno;
>> +             fprintf(stderr, "Failed to issue %s (%d)\n",
>> +                     "GPIOHANDLE_GET_LINE_VALUES_IOCTL", ret);
>> +             goto exit_close_error;
>
> Same here.
Thanks. I will fix above two in next version.
>
>> +     }
>> +
>> +exit_close_error:
>> +     return ret;
>> +}
>> +
>> +int gpio_release(const int fd)
>> +{
>> +     int ret;
>> +
>> +     ret = close(fd);
>> +     if (ret < -1)
>> +             perror("Failed to close GPIO LINEHANDLE device file");
>> +
>> +     return ret;
>> +}
>> +
>> +int gpio_gets(const char *device_name, unsigned int *lines, unsigned int nlines,
>> +           unsigned int flag, struct gpiohandle_data *data)
>> +{
>> +     int fd;
>> +     int ret;
>> +
>> +     ret = gpio_request(device_name, lines, nlines, flag, data, COMSUMER);
>> +     if (ret < 0)
>> +             return ret;
>> +
>> +     fd = ret;
>> +     ret = gpio_get_values(fd, data);
>
> The error checking is missing here.
Well, no matter the gpio_get_values fails or not. We both need to release the
line we request. But, we need the return value if gpio_get_values fails. In the
next version, I will return the errno of gpio_get_values if it fails.
>
>> +     ret = gpio_release(fd);
>
> Shouldn't we leave it up the user how they want the deal with the file handle?
> There is more system call overhead if we open and close the GPIO device for
> each access.
Yes. I understand and agree the syscall overhead you mentioned. So, I provide
the request, ops, release ops above. These gpio_[sg]ets? functions is the easy
way if the user only access the gpio in a few times.
>
>> +     return ret;
>> +}
>> +
>> +int gpio_sets(const char *device_name, unsigned int *lines, unsigned int nlines,
>> +           unsigned int flag, struct gpiohandle_data *data)
>> +{
>> +     int ret;
>> +
>> +     ret = gpio_request(device_name, lines, nlines, flag, data, COMSUMER);
>> +     if (ret < 0)
>> +             return ret;
>> +
>> +     ret = gpio_release(ret);
>> +     return ret;
>> +}
>> +
>> +int gpio_get(const char *device_name, unsigned int line, unsigned int flag)
>> +{
>> +     struct gpiohandle_data data;
>> +     unsigned int lines[] = {line};
>
> Is the lines variable really needed?
Do you mean something like gpio_gets(device_name, {line}, 1, flag, &data);?
Personally, I like the first version. Because it is more clear and compiler
will optimize it anyway. Ideas?

Regards

Bamvor

Re: [PATCH v3 1/5] tools/gpio: add gpio basic opereations

[Michael Welling]

On Wed, Sep 28, 2016 at 02:35:17PM -0700, Bamvor Zhang Jian wrote:
> Hi, Michael
> 
> On 28 September 2016 at 12:16, Michael Welling <mwelling@ieee.org> wrote:
> > On Wed, Aug 31, 2016 at 11:45:44AM +0200, bamvor.zhangjian@linaro.org wrote:
> >> From: Bamvor Jian Zhang <bamvor.zhangjian@linaro.org>
> [...]
> >> +     ret = ioctl(fd, GPIOHANDLE_SET_LINE_VALUES_IOCTL, data);
> >> +     if (ret == -1) {
> >> +             ret = -errno;
> >> +             fprintf(stderr, "Failed to issue %s (%d)\n",
> >> +                     "GPIOHANDLE_SET_LINE_VALUES_IOCTL", ret);
> >> +             goto exit_close_error;
> >
> > This goto is unnecessary because there is no code to be skipped.
> >
> >> +     }
> >> +
> >> +exit_close_error:
> >> +     return ret;
> >> +}
> >> +
> >> +int gpio_get_values(const int fd, struct gpiohandle_data *data)
> >> +{
> >> +     int ret;
> >> +
> >> +     ret = ioctl(fd, GPIOHANDLE_GET_LINE_VALUES_IOCTL, data);
> >> +     if (ret == -1) {
> >> +             ret = -errno;
> >> +             fprintf(stderr, "Failed to issue %s (%d)\n",
> >> +                     "GPIOHANDLE_GET_LINE_VALUES_IOCTL", ret);
> >> +             goto exit_close_error;
> >
> > Same here.
> Thanks. I will fix above two in next version.
> >
> >> +     }
> >> +
> >> +exit_close_error:
> >> +     return ret;
> >> +}
> >> +
> >> +int gpio_release(const int fd)
> >> +{
> >> +     int ret;
> >> +
> >> +     ret = close(fd);
> >> +     if (ret < -1)
> >> +             perror("Failed to close GPIO LINEHANDLE device file");
> >> +
> >> +     return ret;
> >> +}
> >> +
> >> +int gpio_gets(const char *device_name, unsigned int *lines, unsigned int nlines,
> >> +           unsigned int flag, struct gpiohandle_data *data)
> >> +{
> >> +     int fd;
> >> +     int ret;
> >> +
> >> +     ret = gpio_request(device_name, lines, nlines, flag, data, COMSUMER);
> >> +     if (ret < 0)
> >> +             return ret;
> >> +
> >> +     fd = ret;
> >> +     ret = gpio_get_values(fd, data);
> >
> > The error checking is missing here.
> Well, no matter the gpio_get_values fails or not. We both need to release the
> line we request. But, we need the return value if gpio_get_values fails. In the
> next version, I will return the errno of gpio_get_values if it fails.
> >
> >> +     ret = gpio_release(fd);
> >
> > Shouldn't we leave it up the user how they want the deal with the file handle?
> > There is more system call overhead if we open and close the GPIO device for
> > each access.
> Yes. I understand and agree the syscall overhead you mentioned. So, I provide
> the request, ops, release ops above. These gpio_[sg]ets? functions is the easy
> way if the user only access the gpio in a few times.
> >
> >> +     return ret;
> >> +}
> >> +
> >> +int gpio_sets(const char *device_name, unsigned int *lines, unsigned int nlines,
> >> +           unsigned int flag, struct gpiohandle_data *data)
> >> +{
> >> +     int ret;
> >> +
> >> +     ret = gpio_request(device_name, lines, nlines, flag, data, COMSUMER);
> >> +     if (ret < 0)
> >> +             return ret;
> >> +
> >> +     ret = gpio_release(ret);
> >> +     return ret;
> >> +}
> >> +
> >> +int gpio_get(const char *device_name, unsigned int line, unsigned int flag)
> >> +{
> >> +     struct gpiohandle_data data;
> >> +     unsigned int lines[] = {line};
> >
> > Is the lines variable really needed?
> Do you mean something like gpio_gets(device_name, {line}, 1, flag, &data);?
> Personally, I like the first version. Because it is more clear and compiler
> will optimize it anyway. Ideas?

gpio_gets(device_name, &line, 1, flag, &data);

It may be easier to understand the way you have it.

> 
> Regards
> 
> Bamvor

Re: [PATCH v3 2/5] tools/gpio: re-work gpio hammer with gpio operations

[Bamvor Zhang Jian]

Hi, Linus, Michael

When I am writing the updated version of gpio-hammer.c(base on the
gpio-util.c we discussed). I am thinking if the gpio line is
aleady output, maybe the user do not want to update the value. But
I found that I must update the default_values in struct
gpiohandle_request if I want to set gpio as output.  Because I could
not set the gpio direction after the gpio line request. And I could
not skip the default value in request. I am thinking if we need to
deal with it.

The first option is add another flag(e.g.
GPIOHANDLE_REQUEST_UPDATE_VALUE) to indicate the update the value of
gpio. This method break the current application which rely on setting
the default_values through GPIOHANDLE_REQUEST_OUTPUT. Another way is
allow user update the flag after request. It need to add flags to
struct linehandle_state.
diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index 53ff25a..a6965a3 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -329,6 +329,7 @@ struct linehandle_state {
        const char *label;
        struct gpio_desc *descs[GPIOHANDLES_MAX];
        u32 numdescs;
+       u32 flags;
 };

The easiest way seems set the gpio value to 0 in gpio-hammer.c which
is consistent with behavior of sysfs. But in the real senario, the
user need to get the value by setting 0 to the flag in gpio request,
then release and set GPIOHANDLE_REQUEST_OUTPUT with the value it
get from previous request. It lead to the request twice for setting/
flipping value of gpio.

Suggestion?

Regards

Bamvor


On 31 August 2016 at 17:45,  <bamvor.zhangjian@linaro.org> wrote:
> From: Bamvor Jian Zhang <bamvor.zhangjian@linaro.org>
>
> Signed-off-by: Bamvor Jian Zhang <bamvor.zhangjian@linaro.org>
> ---
>  tools/gpio/gpio-hammer.c | 52 ++++++++++--------------------------------------
>  1 file changed, 10 insertions(+), 42 deletions(-)
>
> diff --git a/tools/gpio/gpio-hammer.c b/tools/gpio/gpio-hammer.c
> index 37b3f14..14dd20c 100644
> --- a/tools/gpio/gpio-hammer.c
> +++ b/tools/gpio/gpio-hammer.c
> @@ -23,49 +23,20 @@
>  #include <getopt.h>
>  #include <sys/ioctl.h>
>  #include <linux/gpio.h>
> +#include "gpio-utils.h"
>
>  int hammer_device(const char *device_name, unsigned int *lines, int nlines,
>                   unsigned int loops)
>  {
> -       struct gpiohandle_request req;
>         struct gpiohandle_data data;
> -       char *chrdev_name;
>         char swirr[] = "-\\|/";
> -       int fd;
>         int ret;
>         int i, j;
>         unsigned int iteration = 0;
>
> -       ret = asprintf(&chrdev_name, "/dev/%s", device_name);
> -       if (ret < 0)
> -               return -ENOMEM;
> -
> -       fd = open(chrdev_name, 0);
> -       if (fd == -1) {
> -               ret = -errno;
> -               fprintf(stderr, "Failed to open %s\n", chrdev_name);
> -               goto exit_close_error;
> -       }
> -
> -       /* Request lines as output */
> -       for (i = 0; i < nlines; i++)
> -               req.lineoffsets[i] = lines[i];
> -       req.flags = GPIOHANDLE_REQUEST_OUTPUT; /* Request as output */
> -       strcpy(req.consumer_label, "gpio-hammer");
> -       req.lines = nlines;
> -       ret = ioctl(fd, GPIO_GET_LINEHANDLE_IOCTL, &req);
> -       if (ret == -1) {
> -               ret = -errno;
> -               fprintf(stderr, "Failed to issue GET LINEHANDLE "
> -                       "IOCTL (%d)\n",
> -                       ret);
> -               goto exit_close_error;
> -       }
> -
> -       /* Read initial states */
> -       ret = ioctl(req.fd, GPIOHANDLE_GET_LINE_VALUES_IOCTL, &data);
> -       if (ret == -1) {
> -               ret = -errno;
> +       /* Do not set input or output to avoid change direction or value */
> +       ret = gpio_gets(device_name, lines, nlines, 0, &data);
> +       if (ret < 0) {
>                 fprintf(stderr, "Failed to issue GPIOHANDLE GET LINE "
>                         "VALUES IOCTL (%d)\n",
>                         ret);
> @@ -92,18 +63,18 @@ int hammer_device(const char *device_name, unsigned int *lines, int nlines,
>                 for (i = 0; i < nlines; i++)
>                         data.values[i] = !data.values[i];
>
> -               ret = ioctl(req.fd, GPIOHANDLE_SET_LINE_VALUES_IOCTL, &data);
> -               if (ret == -1) {
> -                       ret = -errno;
> +               ret = gpio_sets(device_name, lines, nlines,
> +                               GPIOHANDLE_REQUEST_OUTPUT, &data);
> +               if (ret < 0) {
>                         fprintf(stderr, "Failed to issue GPIOHANDLE SET LINE "
>                                 "VALUES IOCTL (%d)\n",
>                                 ret);
>                         goto exit_close_error;
>                 }
>                 /* Re-read values to get status */
> -               ret = ioctl(req.fd, GPIOHANDLE_GET_LINE_VALUES_IOCTL, &data);
> -               if (ret == -1) {
> -                       ret = -errno;
> +               ret = gpio_gets(device_name, lines, nlines,
> +                               GPIOHANDLE_REQUEST_OUTPUT, &data);
> +               if (ret < 0) {
>                         fprintf(stderr, "Failed to issue GPIOHANDLE GET LINE "
>                                 "VALUES IOCTL (%d)\n",
>                                 ret);
> @@ -132,9 +103,6 @@ int hammer_device(const char *device_name, unsigned int *lines, int nlines,
>         ret = 0;
>
>  exit_close_error:
> -       if (close(fd) == -1)
> -               perror("Failed to close GPIO character device file");
> -       free(chrdev_name);
>         return ret;
>  }
>
> --
> 1.8.4.5
>