[PATCH 1/2] gpio: userspace ABI for reading/writing GPIO lines

[PATCH 1/2] gpio: userspace ABI for reading/writing GPIO lines

[Linus Walleij]

This adds a userspace ABI for reading and writing GPIO lines.
The mechanism returns an anonymous file handle to a request
to read/write n offsets from a gpiochip. This file handle
in turn accepts two ioctl()s: one that reads and one that
writes values to the selected lines.

- Handles can be requested as input/output, active low,
  open drain, open source, however when you issue a request
  for n lines with GPIO_GET_LINEHANDLE_IOCTL, they must all
  have the same flags, i.e. all inputs or all outputs, all
  open drain etc. If a granular control of the flags for
  each line is desired, they need to be requested
  individually, not in a batch.

- The GPIOHANDLE_GET_LINE_VALUES_IOCTL read ioctl() can be
  issued also to output lines to verify that the hardware
  is in the expected state.

- It reads and writes up to GPIOHANDLES_MAX lines at once,
  utilizing the .set_multiple() call in the driver if
  possible, making the call efficient if several lines
  can be written with a single register update.

The limitation of GPIOHANDLES_MAX to 64 lines is done under
the assumption that we may expect hardware that can issue a
transaction updating 64 bits at an instant but unlikely
anything larger than that.

Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
 drivers/gpio/gpiolib.c    | 190 ++++++++++++++++++++++++++++++++++++++++++++++
 include/uapi/linux/gpio.h |  61 ++++++++++++++-
 2 files changed, 248 insertions(+), 3 deletions(-)

diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index bb3195d5e3af..f72dfb6094a7 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -20,6 +20,7 @@
 #include <linux/cdev.h>
 #include <linux/fs.h>
 #include <linux/uaccess.h>
+#include <linux/anon_inodes.h>
 #include <uapi/linux/gpio.h>
 
 #include "gpiolib.h"
@@ -308,6 +309,193 @@ static int gpiochip_set_desc_names(struct gpio_chip *gc)
 	return 0;
 }
 
+
+/*
+ * GPIO line handle management
+ */
+
+/**
+ * struct linehandle_state - contains the state of a userspace handle
+ * @gdev: the GPIO device the handle pertains to
+ * @label: consumer label used to tag descriptors
+ * @descs: the GPIO descriptors held by this handle
+ * @numdescs: the number of descriptors held in the descs array
+ */
+struct linehandle_state {
+	struct gpio_device *gdev;
+	char *label;
+	struct gpio_desc *descs[GPIOHANDLES_MAX];
+	u32 numdescs;
+};
+
+static long linehandle_ioctl(struct file *filep, unsigned int cmd,
+			     unsigned long arg)
+{
+	struct linehandle_state *lh = filep->private_data;
+	int __user *ip = (int __user *)arg;
+	struct gpiohandle_data ghd;
+	int i;
+
+	if (cmd == GPIOHANDLE_GET_LINE_VALUES_IOCTL) {
+		int val;
+
+		/* TODO: check if descriptors are really input */
+		for (i = 0; i < lh->numdescs; i++) {
+			val = gpiod_get_value(lh->descs[i]);
+			if (val < 0)
+				return val;
+			ghd.values[i] = val;
+		}
+
+		if (copy_to_user(ip, &ghd, sizeof(ghd)))
+			return -EFAULT;
+
+		return 0;
+	} else if (cmd == GPIOHANDLE_SET_LINE_VALUES_IOCTL) {
+		int vals[GPIOHANDLES_MAX];
+
+		/* TODO: check if descriptors are really output */
+		if (copy_from_user(&ghd, ip, sizeof(ghd)))
+			return -EFAULT;
+
+		/* Clamp all values to [0,1] */
+		for (i = 0; i < lh->numdescs; i++)
+			vals[i] = !!ghd.values[i];
+
+		/* Reuse the array setting function */
+		gpiod_set_array_value_complex(false,
+					      true,
+					      lh->numdescs,
+					      lh->descs,
+					      vals);
+		return 0;
+	}
+	return -EINVAL;
+}
+
+static int linehandle_release(struct inode *inode, struct file *filep)
+{
+	struct linehandle_state *lh = filep->private_data;
+	struct gpio_device *gdev = lh->gdev;
+	int i;
+
+	for (i = 0; i < lh->numdescs; i++)
+		gpiod_free(lh->descs[i]);
+	devm_kfree(&gdev->dev, lh->label);
+	devm_kfree(&gdev->dev, lh);
+	put_device(&gdev->dev);
+	return 0;
+}
+
+static const struct file_operations linehandle_fileops = {
+	.release = linehandle_release,
+	.owner = THIS_MODULE,
+	.llseek = noop_llseek,
+	.unlocked_ioctl = linehandle_ioctl,
+	.compat_ioctl = linehandle_ioctl,
+};
+
+static int linehandle_create(struct gpio_device *gdev, int __user *ip)
+{
+	struct gpiohandle_request handlereq;
+	struct linehandle_state *lh;
+	int fd, i, ret;
+
+	if (copy_from_user(&handlereq, ip, sizeof(handlereq)))
+		return -EFAULT;
+
+	/*
+	 * Use devm_* calls with devm_*free() so we can request
+	 * and free the memory for these while still be sure that
+	 * they will eventually go away with the device if not
+	 * before that.
+	 */
+	lh = devm_kzalloc(&gdev->dev, sizeof(*lh), GFP_KERNEL);
+	if (!lh)
+		return -ENOMEM;
+	lh->gdev = gdev;
+	get_device(&gdev->dev);
+
+	fd = anon_inode_getfd("gpio-linehandle",
+			      &linehandle_fileops,
+			      lh,
+			      O_RDONLY | O_CLOEXEC);
+	if (fd < 0) {
+		ret = fd;
+		goto out_free_lh;
+	}
+
+	/* Make sure this is terminated */
+	handlereq.consumer_label[sizeof(handlereq.consumer_label)-1] = '\0';
+	if (strlen(handlereq.consumer_label)) {
+		lh->label = devm_kstrdup(&gdev->dev,
+					 handlereq.consumer_label,
+					 GFP_KERNEL);
+		if (!lh->label) {
+			ret = -ENOMEM;
+			goto out_free_lh;
+		}
+	}
+
+	/* Request each GPIO */
+	for (i = 0; i < handlereq.lines; i++) {
+		u32 offset = handlereq.lineoffsets[i];
+		u32 lflags = handlereq.flags;
+		struct gpio_desc *desc;
+
+		desc = &gdev->descs[offset];
+		ret = gpiod_request(desc, lh->label);
+		if (ret)
+			goto out_free_descs;
+		lh->descs[i] = desc;
+
+		if (lflags & GPIOHANDLE_REQUEST_ACTIVE_LOW)
+			set_bit(FLAG_ACTIVE_LOW, &desc->flags);
+		if (lflags & GPIOHANDLE_REQUEST_OPEN_DRAIN)
+			set_bit(FLAG_OPEN_DRAIN, &desc->flags);
+		if (lflags & GPIOHANDLE_REQUEST_OPEN_SOURCE)
+			set_bit(FLAG_OPEN_SOURCE, &desc->flags);
+
+		/*
+		 * Lines have to be requested explicitly for input
+		 * or output, else the line will be treated "as is".
+		 */
+		if (lflags & GPIOHANDLE_REQUEST_OUTPUT) {
+			int val = !!handlereq.default_values[i];
+
+			ret = gpiod_direction_output(desc, val);
+			if (ret)
+				goto out_free_descs;
+		} else if (lflags & GPIOHANDLE_REQUEST_INPUT) {
+			ret = gpiod_direction_input(desc);
+			if (ret)
+				goto out_free_descs;
+		}
+		dev_dbg(&gdev->dev, "registered chardev handle for pin %d\n",
+			offset);
+	}
+	lh->numdescs = handlereq.lines;
+	dev_dbg(&gdev->dev, "registered chardev handle for %d lines\n",
+		lh->numdescs);
+
+	handlereq.fd = fd;
+	if (copy_to_user(ip, &handlereq, sizeof(handlereq))) {
+		ret = -EFAULT;
+		goto out_free_descs;
+	}
+
+	return 0;
+
+out_free_descs:
+	for (; i >= 0; i--)
+		gpiod_free(lh->descs[i]);
+	devm_kfree(&gdev->dev, lh->label);
+out_free_lh:
+	devm_kfree(&gdev->dev, lh);
+	put_device(&gdev->dev);
+	return ret;
+}
+
 /**
  * gpio_ioctl() - ioctl handler for the GPIO chardev
  */
@@ -383,6 +571,8 @@ static long gpio_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
 		if (copy_to_user(ip, &lineinfo, sizeof(lineinfo)))
 			return -EFAULT;
 		return 0;
+	} else if (cmd == GPIO_GET_LINEHANDLE_IOCTL) {
+		return linehandle_create(gdev, ip);
 	}
 	return -EINVAL;
 }
diff --git a/include/uapi/linux/gpio.h b/include/uapi/linux/gpio.h
index d0a3cac72250..8ffcde7b1c95 100644
--- a/include/uapi/linux/gpio.h
+++ b/include/uapi/linux/gpio.h
@@ -1,7 +1,7 @@
 /*
  * <linux/gpio.h> - userspace ABI for the GPIO character devices
  *
- * Copyright (C) 2015 Linus Walleij
+ * Copyright (C) 2016 Linus Walleij
  *
  * 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
@@ -26,8 +26,8 @@ struct gpiochip_info {
 	__u32 lines;
 };
 
-/* Line is in use by the kernel */
-#define GPIOLINE_FLAG_KERNEL		(1UL << 0)
+/* Informational flags */
+#define GPIOLINE_FLAG_KERNEL		(1UL << 0) /* Line used by the kernel */
 #define GPIOLINE_FLAG_IS_OUT		(1UL << 1)
 #define GPIOLINE_FLAG_ACTIVE_LOW	(1UL << 2)
 #define GPIOLINE_FLAG_OPEN_DRAIN	(1UL << 3)
@@ -52,7 +52,62 @@ struct gpioline_info {
 	char consumer[32];
 };
 
+/* Maximum number of requested handles */
+#define GPIOHANDLES_MAX 64
+
+/* Request flags */
+#define GPIOHANDLE_REQUEST_INPUT	(1UL << 0)
+#define GPIOHANDLE_REQUEST_OUTPUT	(1UL << 1)
+#define GPIOHANDLE_REQUEST_ACTIVE_LOW	(1UL << 2)
+#define GPIOHANDLE_REQUEST_OPEN_DRAIN	(1UL << 3)
+#define GPIOHANDLE_REQUEST_OPEN_SOURCE	(1UL << 4)
+
+/**
+ * struct gpiohandle_request - Information about a GPIO handle request
+ * @lineoffsets: an array desired lines, specified by offset index for the
+ * associated GPIO device
+ * @flags: desired flags for the desired GPIO lines, such as
+ * GPIOLINE_IS_OUT, GPIOLINE_FLAG_ACTIVE_LOW etc, OR:ed together.
+ * Note that even if multiple lines are requested, the same flags
+ * must be applicable to all of them, if you want lines with individual
+ * flags set, request them one by one. It is possible to select
+ * a batch of input or output lines, but they must all have the same
+ * characteristics, i.e. all inputs or all outputs, all active low etc
+ * @default_values: if the GPIOLINE_FLAG_IS_OUT is set for a requested
+ * line, this specifies the default output value, should be 0 (low) or
+ * 1 (high), anything else than 0 or 1 will be interpreted as 1 (high)
+ * @consumer_label: a desired consumer label for the selected GPIO line(s)
+ * such as "my-bitbanged-relay"
+ * @lines: number of lines requested in this request, i.e. the number of
+ * valid fields in the above arrays, set to 1 to request a single line
+ * @fd: if successful this field will contain a valid anonymous file handle
+ * after a GPIO_GET_LINEHANDLE_IOCTL operation, zero or negative value
+ * means error
+ */
+struct gpiohandle_request {
+	__u32 lineoffsets[GPIOHANDLES_MAX];
+	__u32 flags;
+	__u8 default_values[GPIOHANDLES_MAX];
+	char consumer_label[32];
+	__u32 lines;
+	int fd;
+};
+
 #define GPIO_GET_CHIPINFO_IOCTL _IOR(0xB4, 0x01, struct gpiochip_info)
 #define GPIO_GET_LINEINFO_IOCTL _IOWR(0xB4, 0x02, struct gpioline_info)
+#define GPIO_GET_LINEHANDLE_IOCTL _IOWR(0xB4, 0x03, struct gpiohandle_request)
+
+/**
+ * struct gpiohandle_data - Information of values on a GPIO handle
+ * @values: when getting the state of lines this contains the current
+ * state of a line, when setting the state of lines these should contain
+ * the desired target state
+ */
+struct gpiohandle_data {
+	__u8 values[GPIOHANDLES_MAX];
+};
+
+#define GPIOHANDLE_GET_LINE_VALUES_IOCTL _IOR(0xB4, 0x08, struct gpiohandle_data)
+#define GPIOHANDLE_SET_LINE_VALUES_IOCTL _IOR(0xB4, 0x09, struct gpiohandle_data)
 
 #endif /* _UAPI_GPIO_H_ */
-- 
2.4.11

[PATCH 2/2] tools/gpio: add the gpio-hammer tool

[Linus Walleij]

The gpio-hammer is used from userspace as an example of how
to retrieve a GPIO handle for one or several GPIO lines and
hammer the outputs from low to high and back again. It will
pulse the selected lines once per second for a specified
number of times or indefinitely if no loop count is
supplied.

Example output:
$ gpio-hammer -n gpiochip0 -o5 -o6 -o7
Hammer lines [5, 6, 7] on gpiochip0, initial states: [1, 1, 1]
[-] [5: 0, 6: 0, 7: 0]

Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
 tools/gpio/Makefile      |   5 +-
 tools/gpio/gpio-hammer.c | 189 +++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 192 insertions(+), 2 deletions(-)
 create mode 100644 tools/gpio/gpio-hammer.c

diff --git a/tools/gpio/Makefile b/tools/gpio/Makefile
index c155d6bc47a7..aea23949054e 100644
--- a/tools/gpio/Makefile
+++ b/tools/gpio/Makefile
@@ -1,12 +1,13 @@
 CC = $(CROSS_COMPILE)gcc
 CFLAGS += -O2 -Wall -g -D_GNU_SOURCE
 
-all: lsgpio
+all: lsgpio gpio-hammer
 
 lsgpio: lsgpio.o gpio-utils.o
+gpio-hammer: gpio-hammer.o gpio-utils.o
 
 %.o: %.c gpio-utils.h
 
 .PHONY: clean
 clean:
-	rm -f *.o lsgpio
+	rm -f *.o lsgpio gpio-hammer
diff --git a/tools/gpio/gpio-hammer.c b/tools/gpio/gpio-hammer.c
new file mode 100644
index 000000000000..37b3f141053d
--- /dev/null
+++ b/tools/gpio/gpio-hammer.c
@@ -0,0 +1,189 @@
+/*
+ * gpio-hammer - example swiss army knife to shake GPIO lines on a system
+ *
+ * Copyright (C) 2016 Linus Walleij
+ *
+ * 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.
+ *
+ * Usage:
+ *	gpio-hammer -n <device-name> -o <offset1> -o <offset2>
+ */
+
+#include <unistd.h>
+#include <stdlib.h>
+#include <stdbool.h>
+#include <stdio.h>
+#include <dirent.h>
+#include <errno.h>
+#include <string.h>
+#include <poll.h>
+#include <fcntl.h>
+#include <getopt.h>
+#include <sys/ioctl.h>
+#include <linux/gpio.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;
+		fprintf(stderr, "Failed to issue GPIOHANDLE GET LINE "
+			"VALUES IOCTL (%d)\n",
+			ret);
+		goto exit_close_error;
+	}
+	fprintf(stdout, "Hammer lines [");
+	for (i = 0; i < nlines; i++) {
+		fprintf(stdout, "%d", lines[i]);
+		if (i != (nlines - 1))
+			fprintf(stdout, ", ");
+	}
+	fprintf(stdout, "] on %s, initial states: [", device_name);
+	for (i = 0; i < nlines; i++) {
+		fprintf(stdout, "%d", data.values[i]);
+		if (i != (nlines - 1))
+			fprintf(stdout, ", ");
+	}
+	fprintf(stdout, "]\n");
+
+	/* Hammertime! */
+	j = 0;
+	while (1) {
+		/* Invert all lines so we blink */
+		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;
+			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;
+			fprintf(stderr, "Failed to issue GPIOHANDLE GET LINE "
+				"VALUES IOCTL (%d)\n",
+				ret);
+			goto exit_close_error;
+		}
+
+		fprintf(stdout, "[%c] ", swirr[j]);
+		j++;
+		if (j == sizeof(swirr)-1)
+			j = 0;
+
+		fprintf(stdout, "[");
+		for (i = 0; i < nlines; i++) {
+			fprintf(stdout, "%d: %d", lines[i], data.values[i]);
+			if (i != (nlines - 1))
+				fprintf(stdout, ", ");
+		}
+		fprintf(stdout, "]\r");
+		fflush(stdout);
+		sleep(1);
+		iteration++;
+		if (loops && iteration == loops)
+			break;
+	}
+	fprintf(stdout, "\n");
+	ret = 0;
+
+exit_close_error:
+	if (close(fd) == -1)
+		perror("Failed to close GPIO character device file");
+	free(chrdev_name);
+	return ret;
+}
+
+void print_usage(void)
+{
+	fprintf(stderr, "Usage: gpio-hammer [options]...\n"
+		"Hammer GPIO lines, 0->1->0->1...\n"
+		"  -n <name>  Hammer GPIOs on a named device (must be stated)\n"
+		"  -o <n>     Offset[s] to hammer, at least one, several can be stated\n"
+		" [-c <n>]    Do <n> loops (optional, infinite loop if not stated)\n"
+		"  -?         This helptext\n"
+		"\n"
+		"Example:\n"
+		"gpio-hammer -n gpiochip0 -o 4\n"
+	);
+}
+
+int main(int argc, char **argv)
+{
+	const char *device_name = NULL;
+	unsigned int lines[GPIOHANDLES_MAX];
+	unsigned int loops = 0;
+	int nlines;
+	int c;
+	int i;
+
+	i = 0;
+	while ((c = getopt(argc, argv, "c:n:o:?")) != -1) {
+		switch (c) {
+		case 'c':
+			loops = strtoul(optarg, NULL, 10);
+			break;
+		case 'n':
+			device_name = optarg;
+			break;
+		case 'o':
+			lines[i] = strtoul(optarg, NULL, 10);
+			i++;
+			break;
+		case '?':
+			print_usage();
+			return -1;
+		}
+	}
+	nlines = i;
+
+	if (!device_name || !nlines) {
+		print_usage();
+		return -1;
+	}
+	return hammer_device(device_name, lines, nlines, loops);
+}
-- 
2.4.11

Re: [PATCH 1/2] gpio: userspace ABI for reading/writing GPIO lines

[Dmitry Torokhov]

Hi Linus,

On Tue, Apr 26, 2016 at 10:54:25AM +0200, Linus Walleij wrote:
> This adds a userspace ABI for reading and writing GPIO lines.
> The mechanism returns an anonymous file handle to a request
> to read/write n offsets from a gpiochip. This file handle
> in turn accepts two ioctl()s: one that reads and one that
> writes values to the selected lines.
> 
> - Handles can be requested as input/output, active low,
>   open drain, open source, however when you issue a request
>   for n lines with GPIO_GET_LINEHANDLE_IOCTL, they must all
>   have the same flags, i.e. all inputs or all outputs, all
>   open drain etc. If a granular control of the flags for
>   each line is desired, they need to be requested
>   individually, not in a batch.
> 
> - The GPIOHANDLE_GET_LINE_VALUES_IOCTL read ioctl() can be
>   issued also to output lines to verify that the hardware
>   is in the expected state.
> 
> - It reads and writes up to GPIOHANDLES_MAX lines at once,
>   utilizing the .set_multiple() call in the driver if
>   possible, making the call efficient if several lines
>   can be written with a single register update.
> 
> The limitation of GPIOHANDLES_MAX to 64 lines is done under
> the assumption that we may expect hardware that can issue a
> transaction updating 64 bits at an instant but unlikely
> anything larger than that.
> 
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> ---
>  drivers/gpio/gpiolib.c    | 190 ++++++++++++++++++++++++++++++++++++++++++++++
>  include/uapi/linux/gpio.h |  61 ++++++++++++++-
>  2 files changed, 248 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
> index bb3195d5e3af..f72dfb6094a7 100644
> --- a/drivers/gpio/gpiolib.c
> +++ b/drivers/gpio/gpiolib.c
> @@ -20,6 +20,7 @@
>  #include <linux/cdev.h>
>  #include <linux/fs.h>
>  #include <linux/uaccess.h>
> +#include <linux/anon_inodes.h>
>  #include <uapi/linux/gpio.h>
>  
>  #include "gpiolib.h"
> @@ -308,6 +309,193 @@ static int gpiochip_set_desc_names(struct gpio_chip *gc)
>  	return 0;
>  }
>  
> +
> +/*
> + * GPIO line handle management
> + */
> +
> +/**
> + * struct linehandle_state - contains the state of a userspace handle
> + * @gdev: the GPIO device the handle pertains to
> + * @label: consumer label used to tag descriptors
> + * @descs: the GPIO descriptors held by this handle
> + * @numdescs: the number of descriptors held in the descs array
> + */
> +struct linehandle_state {
> +	struct gpio_device *gdev;
> +	char *label;

const char *?

> +	struct gpio_desc *descs[GPIOHANDLES_MAX];
> +	u32 numdescs;
> +};
> +
> +static long linehandle_ioctl(struct file *filep, unsigned int cmd,
> +			     unsigned long arg)
> +{
> +	struct linehandle_state *lh = filep->private_data;
> +	int __user *ip = (int __user *)arg;

You seem to be using the same handler for native and compat calls, but
for compat you need to use compat_ptr() because not all arches employ
straight cast conversions.

> +	struct gpiohandle_data ghd;
> +	int i;
> +
> +	if (cmd == GPIOHANDLE_GET_LINE_VALUES_IOCTL) {

I am fond of switch/case for flows like this.

> +		int val;
> +
> +		/* TODO: check if descriptors are really input */
> +		for (i = 0; i < lh->numdescs; i++) {
> +			val = gpiod_get_value(lh->descs[i]);
> +			if (val < 0)
> +				return val;
> +			ghd.values[i] = val;
> +		}
> +
> +		if (copy_to_user(ip, &ghd, sizeof(ghd)))
> +			return -EFAULT;
> +
> +		return 0;
> +	} else if (cmd == GPIOHANDLE_SET_LINE_VALUES_IOCTL) {
> +		int vals[GPIOHANDLES_MAX];
> +
> +		/* TODO: check if descriptors are really output */
> +		if (copy_from_user(&ghd, ip, sizeof(ghd)))
> +			return -EFAULT;
> +
> +		/* Clamp all values to [0,1] */
> +		for (i = 0; i < lh->numdescs; i++)
> +			vals[i] = !!ghd.values[i];
> +
> +		/* Reuse the array setting function */
> +		gpiod_set_array_value_complex(false,
> +					      true,
> +					      lh->numdescs,
> +					      lh->descs,
> +					      vals);

I wonder if we should be returning errors to userspace in case we failed
to toggle one or more gpios (since we seem to moving towards gpio
transitions being allowed to fail).

> +		return 0;
> +	}
> +	return -EINVAL;
> +}
> +
> +static int linehandle_release(struct inode *inode, struct file *filep)
> +{
> +	struct linehandle_state *lh = filep->private_data;
> +	struct gpio_device *gdev = lh->gdev;
> +	int i;
> +
> +	for (i = 0; i < lh->numdescs; i++)
> +		gpiod_free(lh->descs[i]);
> +	devm_kfree(&gdev->dev, lh->label);
> +	devm_kfree(&gdev->dev, lh);
> +	put_device(&gdev->dev);
> +	return 0;
> +}
> +
> +static const struct file_operations linehandle_fileops = {
> +	.release = linehandle_release,
> +	.owner = THIS_MODULE,
> +	.llseek = noop_llseek,
> +	.unlocked_ioctl = linehandle_ioctl,
> +	.compat_ioctl = linehandle_ioctl,
> +};
> +
> +static int linehandle_create(struct gpio_device *gdev, int __user *ip)
> +{
> +	struct gpiohandle_request handlereq;
> +	struct linehandle_state *lh;
> +	int fd, i, ret;
> +
> +	if (copy_from_user(&handlereq, ip, sizeof(handlereq)))
> +		return -EFAULT;
> +
> +	/*
> +	 * Use devm_* calls with devm_*free() so we can request
> +	 * and free the memory for these while still be sure that
> +	 * they will eventually go away with the device if not
> +	 * before that.
> +	 */

This is quite "interesting" statement. Consider what happens if driver
gets unbound from the device, but userspace still holds character device
open and calls ioctl on it.

IOW any time I see calls to devm_kfree I think it is a mistake and in
99% cases I am right.

> +	lh = devm_kzalloc(&gdev->dev, sizeof(*lh), GFP_KERNEL);
> +	if (!lh)
> +		return -ENOMEM;
> +	lh->gdev = gdev;
> +	get_device(&gdev->dev);
> +
> +	fd = anon_inode_getfd("gpio-linehandle",
> +			      &linehandle_fileops,
> +			      lh,
> +			      O_RDONLY | O_CLOEXEC);
> +	if (fd < 0) {
> +		ret = fd;
> +		goto out_free_lh;
> +	}
> +
> +	/* Make sure this is terminated */
> +	handlereq.consumer_label[sizeof(handlereq.consumer_label)-1] = '\0';
> +	if (strlen(handlereq.consumer_label)) {
> +		lh->label = devm_kstrdup(&gdev->dev,
> +					 handlereq.consumer_label,
> +					 GFP_KERNEL);
> +		if (!lh->label) {
> +			ret = -ENOMEM;
> +			goto out_free_lh;
> +		}
> +	}
> +
> +	/* Request each GPIO */
> +	for (i = 0; i < handlereq.lines; i++) {
> +		u32 offset = handlereq.lineoffsets[i];
> +		u32 lflags = handlereq.flags;
> +		struct gpio_desc *desc;
> +
> +		desc = &gdev->descs[offset];
> +		ret = gpiod_request(desc, lh->label);
> +		if (ret)
> +			goto out_free_descs;
> +		lh->descs[i] = desc;
> +
> +		if (lflags & GPIOHANDLE_REQUEST_ACTIVE_LOW)
> +			set_bit(FLAG_ACTIVE_LOW, &desc->flags);
> +		if (lflags & GPIOHANDLE_REQUEST_OPEN_DRAIN)
> +			set_bit(FLAG_OPEN_DRAIN, &desc->flags);
> +		if (lflags & GPIOHANDLE_REQUEST_OPEN_SOURCE)
> +			set_bit(FLAG_OPEN_SOURCE, &desc->flags);
> +
> +		/*
> +		 * Lines have to be requested explicitly for input
> +		 * or output, else the line will be treated "as is".
> +		 */
> +		if (lflags & GPIOHANDLE_REQUEST_OUTPUT) {
> +			int val = !!handlereq.default_values[i];
> +
> +			ret = gpiod_direction_output(desc, val);
> +			if (ret)
> +				goto out_free_descs;
> +		} else if (lflags & GPIOHANDLE_REQUEST_INPUT) {
> +			ret = gpiod_direction_input(desc);
> +			if (ret)
> +				goto out_free_descs;
> +		}
> +		dev_dbg(&gdev->dev, "registered chardev handle for pin %d\n",
> +			offset);
> +	}
> +	lh->numdescs = handlereq.lines;
> +	dev_dbg(&gdev->dev, "registered chardev handle for %d lines\n",
> +		lh->numdescs);
> +
> +	handlereq.fd = fd;
> +	if (copy_to_user(ip, &handlereq, sizeof(handlereq))) {
> +		ret = -EFAULT;
> +		goto out_free_descs;
> +	}
> +
> +	return 0;
> +
> +out_free_descs:
> +	for (; i >= 0; i--)
> +		gpiod_free(lh->descs[i]);
> +	devm_kfree(&gdev->dev, lh->label);
> +out_free_lh:
> +	devm_kfree(&gdev->dev, lh);
> +	put_device(&gdev->dev);
> +	return ret;
> +}
> +
>  /**
>   * gpio_ioctl() - ioctl handler for the GPIO chardev
>   */
> @@ -383,6 +571,8 @@ static long gpio_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
>  		if (copy_to_user(ip, &lineinfo, sizeof(lineinfo)))
>  			return -EFAULT;
>  		return 0;
> +	} else if (cmd == GPIO_GET_LINEHANDLE_IOCTL) {
> +		return linehandle_create(gdev, ip);
>  	}
>  	return -EINVAL;
>  }
> diff --git a/include/uapi/linux/gpio.h b/include/uapi/linux/gpio.h
> index d0a3cac72250..8ffcde7b1c95 100644
> --- a/include/uapi/linux/gpio.h
> +++ b/include/uapi/linux/gpio.h
> @@ -1,7 +1,7 @@
>  /*
>   * <linux/gpio.h> - userspace ABI for the GPIO character devices
>   *
> - * Copyright (C) 2015 Linus Walleij
> + * Copyright (C) 2016 Linus Walleij
>   *
>   * 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
> @@ -26,8 +26,8 @@ struct gpiochip_info {
>  	__u32 lines;
>  };
>  
> -/* Line is in use by the kernel */
> -#define GPIOLINE_FLAG_KERNEL		(1UL << 0)
> +/* Informational flags */
> +#define GPIOLINE_FLAG_KERNEL		(1UL << 0) /* Line used by the kernel */
>  #define GPIOLINE_FLAG_IS_OUT		(1UL << 1)
>  #define GPIOLINE_FLAG_ACTIVE_LOW	(1UL << 2)
>  #define GPIOLINE_FLAG_OPEN_DRAIN	(1UL << 3)
> @@ -52,7 +52,62 @@ struct gpioline_info {
>  	char consumer[32];
>  };
>  
> +/* Maximum number of requested handles */
> +#define GPIOHANDLES_MAX 64
> +
> +/* Request flags */
> +#define GPIOHANDLE_REQUEST_INPUT	(1UL << 0)
> +#define GPIOHANDLE_REQUEST_OUTPUT	(1UL << 1)
> +#define GPIOHANDLE_REQUEST_ACTIVE_LOW	(1UL << 2)
> +#define GPIOHANDLE_REQUEST_OPEN_DRAIN	(1UL << 3)
> +#define GPIOHANDLE_REQUEST_OPEN_SOURCE	(1UL << 4)
> +
> +/**
> + * struct gpiohandle_request - Information about a GPIO handle request
> + * @lineoffsets: an array desired lines, specified by offset index for the
> + * associated GPIO device
> + * @flags: desired flags for the desired GPIO lines, such as
> + * GPIOLINE_IS_OUT, GPIOLINE_FLAG_ACTIVE_LOW etc, OR:ed together.
> + * Note that even if multiple lines are requested, the same flags
> + * must be applicable to all of them, if you want lines with individual
> + * flags set, request them one by one. It is possible to select
> + * a batch of input or output lines, but they must all have the same
> + * characteristics, i.e. all inputs or all outputs, all active low etc
> + * @default_values: if the GPIOLINE_FLAG_IS_OUT is set for a requested
> + * line, this specifies the default output value, should be 0 (low) or
> + * 1 (high), anything else than 0 or 1 will be interpreted as 1 (high)
> + * @consumer_label: a desired consumer label for the selected GPIO line(s)
> + * such as "my-bitbanged-relay"
> + * @lines: number of lines requested in this request, i.e. the number of
> + * valid fields in the above arrays, set to 1 to request a single line
> + * @fd: if successful this field will contain a valid anonymous file handle
> + * after a GPIO_GET_LINEHANDLE_IOCTL operation, zero or negative value
> + * means error
> + */
> +struct gpiohandle_request {
> +	__u32 lineoffsets[GPIOHANDLES_MAX];
> +	__u32 flags;
> +	__u8 default_values[GPIOHANDLES_MAX];
> +	char consumer_label[32];
> +	__u32 lines;
> +	int fd;
> +};
> +
>  #define GPIO_GET_CHIPINFO_IOCTL _IOR(0xB4, 0x01, struct gpiochip_info)
>  #define GPIO_GET_LINEINFO_IOCTL _IOWR(0xB4, 0x02, struct gpioline_info)
> +#define GPIO_GET_LINEHANDLE_IOCTL _IOWR(0xB4, 0x03, struct gpiohandle_request)
> +
> +/**
> + * struct gpiohandle_data - Information of values on a GPIO handle
> + * @values: when getting the state of lines this contains the current
> + * state of a line, when setting the state of lines these should contain
> + * the desired target state
> + */
> +struct gpiohandle_data {
> +	__u8 values[GPIOHANDLES_MAX];
> +};
> +
> +#define GPIOHANDLE_GET_LINE_VALUES_IOCTL _IOR(0xB4, 0x08, struct gpiohandle_data)
> +#define GPIOHANDLE_SET_LINE_VALUES_IOCTL _IOR(0xB4, 0x09, struct gpiohandle_data)

_IOW for set?

>  
>  #endif /* _UAPI_GPIO_H_ */
> -- 
> 2.4.11
> 

Thanks.

-- 
Dmitry

Re: [PATCH 2/2] tools/gpio: add the gpio-hammer tool

[Michael Welling]

On Tue, Apr 26, 2016 at 10:54:26AM +0200, Linus Walleij wrote:
> The gpio-hammer is used from userspace as an example of how
> to retrieve a GPIO handle for one or several GPIO lines and
> hammer the outputs from low to high and back again. It will
> pulse the selected lines once per second for a specified
> number of times or indefinitely if no loop count is
> supplied.
> 
> Example output:
> $ gpio-hammer -n gpiochip0 -o5 -o6 -o7
> Hammer lines [5, 6, 7] on gpiochip0, initial states: [1, 1, 1]
> [-] [5: 0, 6: 0, 7: 0]
> 
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>

Tested-by: Michael Welling <mwelling@ieee.org>

Below is the output from my target. You will notice that a kernel warning is spewed out if I use the mcp23s08.
The processor GPIOs work fine as verified by blinking LEDs.

root@som3517-som200:~# ./lsgpio 
GPIO chip: gpiochip4, "mcp23s08", 8 GPIO lines
        line  0: unnamed unused
        line  1: unnamed unused
        line  2: unnamed unused
        line  3: unnamed unused
        line  4: unnamed unused
        line  5: unnamed unused
        line  6: unnamed unused
        line  7: unnamed unused
GPIO chip: gpiochip3, "gpio", 32 GPIO lines
        line  0: unnamed unused
        line  1: unnamed unused
        line  2: unnamed unused
        line  3: unnamed unused
        line  4: unnamed unused
        line  5: unnamed unused
        line  6: unnamed unused
        line  7: unnamed unused
        line  8: unnamed "spi1.3" [kernel output]
        line  9: unnamed unused
        line 10: unnamed unused
        line 11: unnamed unused
        line 12: unnamed unused
        line 13: unnamed unused
        line 14: unnamed unused
        line 15: unnamed unused
        line 16: unnamed unused
        line 17: unnamed unused
        line 18: unnamed unused
        line 19: unnamed unused
        line 20: unnamed unused
        line 21: unnamed unused
        line 22: unnamed unused
        line 23: unnamed unused
        line 24: unnamed unused
        line 25: unnamed unused
        line 26: unnamed unused
        line 27: unnamed unused
        line 28: unnamed unused
        line 29: unnamed unused
        line 30: unnamed unused
        line 31: unnamed unused
GPIO chip: gpiochip2, "gpio", 32 GPIO lines
        line  0: unnamed unused
        line  1: unnamed unused
        line  2: unnamed unused
        line  3: unnamed unused
        line  4: unnamed unused
        line  5: unname[   40.820783] random: nonblocking pool is initialized
d unused
        line  6: unnamed unused
        line  7: unnamed unused
        line  8: unnamed unused
        line  9: unnamed unused
        line 10: unnamed unused
        line 11: unnamed unused
        line 12: unnamed unused
        line 13: unnamed unused
        line 14: unnamed unused
        line 15: unnamed unused
        line 16: unnamed unused
        line 17: unnamed unused
        line 18: unnamed unused
        line 19: unnamed unused
        line 20: unnamed unused
        line 21: unnamed unused
        line 22: unnamed unused
        line 23: unnamed unused
        line 24: unnamed unused
        line 25: unnamed unused
        line 26: unnamed unused
        line 27: unnamed unused
        line 28: unnamed unused
        line 29: unnamed unused
        line 30: unnamed unused
        line 31: unnamed unused
GPIO chip: gpiochip1, "gpio", 32 GPIO lines
        line  0: unnamed unused
        line  1: unnamed unused
        line  2: unnamed unused
        line  3: unnamed unused
        line  4: unnamed unused
        line  5: unnamed unused
        line  6: unnamed unused
        line  7: unnamed unused
        line  8: unnamed unused
        line  9: unnamed unused
        line 10: unnamed unused
        line 11: unnamed unused
        line 12: unnamed "cd" [kernel active-low]
        line 13: unnamed "enable" [kernel output]
        line 14: unnamed "spi1.2" [kernel output]
        line 15: unnamed unused
        line 16: unnamed unused
        line 17: unnamed unused
        line 18: unnamed unused
        line 19: unnamed unused
        line 20: unnamed unused
        line 21: unnamed unused
        line 22: unnamed unused
        line 23: unnamed unused
        line 24: unnamed unused
        line 25: unnamed unused
        line 26: unnamed unused
        line 27: unnamed unused
        line 28: unnamed unused
        line 29: unnamed unused
        line 30: unnamed unused
        line 31: unnamed unused
GPIO chip: gpiochip0, "gpio", 32 GPIO lines
        line  0: unnamed unused
        line  1: unnamed unused
        line  2: unnamed unused
        line  3: unnamed unused
        line  4: unnamed unused
        line  5: unnamed "spi1.0" [kernel output]
        line  6: unnamed "spi1.1" [kernel output]
        line  7: unnamed unused
        line  8: unnamed unused
        line  9: unnamed unused
        line 10: unnamed unused
        line 11: unnamed unused
        line 12: unnamed unused
        line 13: unnamed unused
        line 14: unnamed unused
        line 15: unnamed unused
        line 16: unnamed unused
        line 17: unnamed unused
        line 18: unnamed unused
        line 19: unnamed unused
        line 20: unnamed unused
        line 21: unnamed unused
        line 22: unnamed unused
        line 23: unnamed unused
        line 24: unnamed unused
        line 25: unnamed unused
        line 26: unnamed unused
        line 27: unnamed unused
        line 28: unnamed unused
        line 29: unnamed unused
        line 30: unnamed unused
        line 31: unnamed unused
root@som3517-som200:~# ./gpio-hammer -n gpiochip0 -o7
Hammer lines [7] on gpiochip0, initial states: [1]
^C] [7: 1]
root@som3517-som200:~# ./gpio-hammer -n gpiochip3 -o4                                                                                                                                                                                        
Hammer lines [4] on gpiochip3, initial states: [1]
^C] [4: 1]
root@som3517-som200:~# ./gpio-hammer -n gpiochip1 -o9                                                                                                                                                                                        
Hammer lines [9] on gpiochip1, initial states: [1]
^C] [9: 0]
root@som3517-som200:~# ./gpio-hammer -n gpiochip4 -o0                                                                                                                                                                                        
[  187.511606] ------------[ cut here ]------------
[  187.516949] WARNING: CPU: 0 PID: 830 at /home/michael/projects/linux/linux-git/drivers/gpio/gpiolib.c:1907 gpiod_get_value+0x60/0xa4
[  187.529770] Modules linked in:
[  187.533082] CPU: 0 PID: 830 Comm: gpio-hammer Tainted: G        W       4.6.0-rc1-00084-g6f4ee91-dirty #2
[  187.543231] Hardware name: Generic AM33XX (Flattened Device Tree)
[  187.549753] [<c010fee4>] (unwind_backtrace) from [<c010c10c>] (show_stack+0x10/0x14)
[  187.557993] [<c010c10c>] (show_stack) from [<c0464074>] (dump_stack+0xb0/0xe4)
[  187.565695] [<c0464074>] (dump_stack) from [<c01345d4>] (__warn+0xd4/0x100)
[  187.573109] [<c01345d4>] (__warn) from [<c01346ac>] (warn_slowpath_null+0x20/0x28)
[  187.581163] [<c01346ac>] (warn_slowpath_null) from [<c049e3d8>] (gpiod_get_value+0x60/0xa4)
[  187.590046] [<c049e3d8>] (gpiod_get_value) from [<c049f7a8>] (linehandle_ioctl+0x114/0x19c)
[  187.598932] [<c049f7a8>] (linehandle_ioctl) from [<c0296710>] (do_vfs_ioctl+0x8c/0xa18)
[  187.607445] [<c0296710>] (do_vfs_ioctl) from [<c0297108>] (SyS_ioctl+0x6c/0x7c)
[  187.615233] [<c0297108>] (SyS_ioctl) from [<c0107820>] (ret_fast_syscall+0x0/0x1c)
[  187.623616] ---[ end trace 9b8ac986d34a8efa ]---
.
.

root@som3517-som200:~# uname -a
Linux som3517-som200 4.6.0-rc1-00084-g6f4ee91-dirty #2 SMP Wed Apr 27 10:46:00 CDT 2016 armv7l GNU/Linux
root@som3517-som200:~# cat /proc/cpuinfo 
processor       : 0
model name      : ARMv7 Processor rev 2 (v7l)
BogoMIPS        : 273.94
Features        : half thumb fastmult vfp edsp thumbee neon vfpv3 tls vfpd32 
CPU implementer : 0x41
CPU architecture: 7
CPU variant     : 0x3
CPU part        : 0xc08
CPU revision    : 2

Hardware        : Generic AM33XX (Flattened Device Tree)
Revision        : 0000
Serial          : 0000000000000000

> ---
>  tools/gpio/Makefile      |   5 +-
>  tools/gpio/gpio-hammer.c | 189 +++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 192 insertions(+), 2 deletions(-)
>  create mode 100644 tools/gpio/gpio-hammer.c
> 
> diff --git a/tools/gpio/Makefile b/tools/gpio/Makefile
> index c155d6bc47a7..aea23949054e 100644
> --- a/tools/gpio/Makefile
> +++ b/tools/gpio/Makefile
> @@ -1,12 +1,13 @@
>  CC = $(CROSS_COMPILE)gcc
>  CFLAGS += -O2 -Wall -g -D_GNU_SOURCE
>  
> -all: lsgpio
> +all: lsgpio gpio-hammer
>  
>  lsgpio: lsgpio.o gpio-utils.o
> +gpio-hammer: gpio-hammer.o gpio-utils.o
>  
>  %.o: %.c gpio-utils.h
>  
>  .PHONY: clean
>  clean:
> -	rm -f *.o lsgpio
> +	rm -f *.o lsgpio gpio-hammer
> diff --git a/tools/gpio/gpio-hammer.c b/tools/gpio/gpio-hammer.c
> new file mode 100644
> index 000000000000..37b3f141053d
> --- /dev/null
> +++ b/tools/gpio/gpio-hammer.c
> @@ -0,0 +1,189 @@
> +/*
> + * gpio-hammer - example swiss army knife to shake GPIO lines on a system
> + *
> + * Copyright (C) 2016 Linus Walleij
> + *
> + * 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.
> + *
> + * Usage:
> + *	gpio-hammer -n <device-name> -o <offset1> -o <offset2>
> + */
> +
> +#include <unistd.h>
> +#include <stdlib.h>
> +#include <stdbool.h>
> +#include <stdio.h>
> +#include <dirent.h>
> +#include <errno.h>
> +#include <string.h>
> +#include <poll.h>
> +#include <fcntl.h>
> +#include <getopt.h>
> +#include <sys/ioctl.h>
> +#include <linux/gpio.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;
> +		fprintf(stderr, "Failed to issue GPIOHANDLE GET LINE "
> +			"VALUES IOCTL (%d)\n",
> +			ret);
> +		goto exit_close_error;
> +	}
> +	fprintf(stdout, "Hammer lines [");
> +	for (i = 0; i < nlines; i++) {
> +		fprintf(stdout, "%d", lines[i]);
> +		if (i != (nlines - 1))
> +			fprintf(stdout, ", ");
> +	}
> +	fprintf(stdout, "] on %s, initial states: [", device_name);
> +	for (i = 0; i < nlines; i++) {
> +		fprintf(stdout, "%d", data.values[i]);
> +		if (i != (nlines - 1))
> +			fprintf(stdout, ", ");
> +	}
> +	fprintf(stdout, "]\n");
> +
> +	/* Hammertime! */
> +	j = 0;
> +	while (1) {
> +		/* Invert all lines so we blink */
> +		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;
> +			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;
> +			fprintf(stderr, "Failed to issue GPIOHANDLE GET LINE "
> +				"VALUES IOCTL (%d)\n",
> +				ret);
> +			goto exit_close_error;
> +		}
> +
> +		fprintf(stdout, "[%c] ", swirr[j]);
> +		j++;
> +		if (j == sizeof(swirr)-1)
> +			j = 0;
> +
> +		fprintf(stdout, "[");
> +		for (i = 0; i < nlines; i++) {
> +			fprintf(stdout, "%d: %d", lines[i], data.values[i]);
> +			if (i != (nlines - 1))
> +				fprintf(stdout, ", ");
> +		}
> +		fprintf(stdout, "]\r");
> +		fflush(stdout);
> +		sleep(1);
> +		iteration++;
> +		if (loops && iteration == loops)
> +			break;
> +	}
> +	fprintf(stdout, "\n");
> +	ret = 0;
> +
> +exit_close_error:
> +	if (close(fd) == -1)
> +		perror("Failed to close GPIO character device file");
> +	free(chrdev_name);
> +	return ret;
> +}
> +
> +void print_usage(void)
> +{
> +	fprintf(stderr, "Usage: gpio-hammer [options]...\n"
> +		"Hammer GPIO lines, 0->1->0->1...\n"
> +		"  -n <name>  Hammer GPIOs on a named device (must be stated)\n"
> +		"  -o <n>     Offset[s] to hammer, at least one, several can be stated\n"
> +		" [-c <n>]    Do <n> loops (optional, infinite loop if not stated)\n"
> +		"  -?         This helptext\n"
> +		"\n"
> +		"Example:\n"
> +		"gpio-hammer -n gpiochip0 -o 4\n"
> +	);
> +}
> +
> +int main(int argc, char **argv)
> +{
> +	const char *device_name = NULL;
> +	unsigned int lines[GPIOHANDLES_MAX];
> +	unsigned int loops = 0;
> +	int nlines;
> +	int c;
> +	int i;
> +
> +	i = 0;
> +	while ((c = getopt(argc, argv, "c:n:o:?")) != -1) {
> +		switch (c) {
> +		case 'c':
> +			loops = strtoul(optarg, NULL, 10);
> +			break;
> +		case 'n':
> +			device_name = optarg;
> +			break;
> +		case 'o':
> +			lines[i] = strtoul(optarg, NULL, 10);
> +			i++;
> +			break;
> +		case '?':
> +			print_usage();
> +			return -1;
> +		}
> +	}
> +	nlines = i;
> +
> +	if (!device_name || !nlines) {
> +		print_usage();
> +		return -1;
> +	}
> +	return hammer_device(device_name, lines, nlines, loops);
> +}
> -- 
> 2.4.11
> 

Re: [PATCH 2/2] tools/gpio: add the gpio-hammer tool

[Alexander Stein]

On Tuesday 26 April 2016 10:54:26, Linus Walleij wrote:
> The gpio-hammer is used from userspace as an example of how
> to retrieve a GPIO handle for one or several GPIO lines and
> hammer the outputs from low to high and back again. It will
> pulse the selected lines once per second for a specified
> number of times or indefinitely if no loop count is
> supplied.
> 
> Example output:
> $ gpio-hammer -n gpiochip0 -o5 -o6 -o7
> Hammer lines [5, 6, 7] on gpiochip0, initial states: [1, 1, 1]
> [-] [5: 0, 6: 0, 7: 0]
> 
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>

This might be a stupid question, but how do I build those tools? I've never 
done this before and failed with the following command:
> ARCH=arm CROSS_COMPILE=/opt/OSELAS.Toolchain-2013.12.2/arm-cortexa8-linux-
gnueabi/gcc-4.8.3-glibc-2.18-binutils-2.24-kernel-3.12-sanitized/bin/arm-
cortexa8-linux-gnueabi- make O=build_arm V=1 tools/gpio

results:

make -C /home/alex/repo/linux/build_arm KBUILD_SRC=/home/alex/repo/linux \
-f /home/alex/repo/linux/Makefile tools/gpio
make[1]: Verzeichnis „/home/alex/repo/linux/build_arm“ wird betreten
mkdir -p ./tools
make LDFLAGS= MAKEFLAGS="" O=/home/alex/repo/linux/build_arm subdir=tools -C 
../tools/ gpio
mkdir -p /home/alex/repo/linux/build_arm/tools/gpio && make 
O=/home/alex/repo/linux/build_arm subdir=tools/gpio --no-print-directory -C 
gpio 
/opt/OSELAS.Toolchain-2013.12.2/arm-cortexa8-linux-gnueabi/gcc-4.8.3-
glibc-2.18-binutils-2.24-kernel-3.12-sanitized/bin/arm-cortexa8-linux-gnueabi-
gcc -O2 -Wall -g -D_GNU_SOURCE   -c -o lsgpio.o lsgpio.c
lsgpio.c:25:24: fatal error: linux/gpio.h: No such file or directory
 #include <linux/gpio.h>

Apparently the includes to $(srcdir)/include and 
$(srcdir)/arch/${ARCH}/include are missing.

Best regards,
Alexander

--
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 1/2] gpio: userspace ABI for reading/writing GPIO lines

[Linus Walleij]

Hi Dmitry,

*thanks* for the effort to review this, it's much appreciated.

On Tue, Apr 26, 2016 at 6:44 PM, Dmitry Torokhov
<dmitry.torokhov@gmail.com> wrote:
> On Tue, Apr 26, 2016 at 10:54:25AM +0200, Linus Walleij wrote:

>> +/**
>> + * struct linehandle_state - contains the state of a userspace handle
>> + * @gdev: the GPIO device the handle pertains to
>> + * @label: consumer label used to tag descriptors
>> + * @descs: the GPIO descriptors held by this handle
>> + * @numdescs: the number of descriptors held in the descs array
>> + */
>> +struct linehandle_state {
>> +     struct gpio_device *gdev;
>> +     char *label;
>
> const char *?

Actually no, but I made an extra check.

>> +static long linehandle_ioctl(struct file *filep, unsigned int cmd,
>> +                          unsigned long arg)
>> +{
>> +     struct linehandle_state *lh = filep->private_data;
>> +     int __user *ip = (int __user *)arg;
>
> You seem to be using the same handler for native and compat calls, but
> for compat you need to use compat_ptr() because not all arches employ
> straight cast conversions.

Argh and that is actually a bug in my primary ioctl(). I sent a separate
patch fixing this up that should be in your inbox, and also fixed it in this
patch of course.

>> +     if (cmd == GPIOHANDLE_GET_LINE_VALUES_IOCTL) {
>
> I am fond of switch/case for flows like this.

I like the if-else-if because it enables me to:

>> +             int val;

Declare base block scope-local variables. switch()
requires a confusing { } to get the same local block.
It's a bit of taste question I guess.

>> +             /* Reuse the array setting function */
>> +             gpiod_set_array_value_complex(false,
>> +                                           true,
>> +                                           lh->numdescs,
>> +                                           lh->descs,
>> +                                           vals);
>
> I wonder if we should be returning errors to userspace in case we failed
> to toggle one or more gpios (since we seem to moving towards gpio
> transitions being allowed to fail).

OK I was actually thinking of that too, so now that you point
it out I should just fix that with a separate refactoring patch
making gpiod_set_array_value_complex() return an error
code.

>> +     /*
>> +      * Use devm_* calls with devm_*free() so we can request
>> +      * and free the memory for these while still be sure that
>> +      * they will eventually go away with the device if not
>> +      * before that.
>> +      */
>
> This is quite "interesting" statement. Consider what happens if driver
> gets unbound from the device, but userspace still holds character device
> open and calls ioctl on it.

That is actually a usecase I have spent much time with
speculating about, and I am trying to make that work. But it's
kind of complex.

First I must actually make sure that the device itself doesn't
go away randomly, so five lines down from this I have this:

>> +     get_device(&gdev->dev);

IIUC get_device() will inhibit the device from being unregistered
so that will not happen, and anything allocated with devm_*
will still be around, like the linehandles. Notice that this is not
the physical device but the gpiolib-intrinsic device that gets
registered from a say platform_device, i2c_device or so.
The latter is only the parent of this device.

Even if the parent i2c_device, platform_device or whatever
goes away, this will stay around because it is referenced
from userspace. (Or something else that hold a reference
count to it.)

Now for the individual lines we also have this in gpiod_get()
everytime a line is requested from a chip:

int gpiod_request(struct gpio_desc *desc, const char *label)
{
(...)
    if (try_module_get(gdev->owner)) {
        status = __gpiod_request(desc, label);
        if (status < 0)
            module_put(gdev->owner);
        else
            get_device(&gdev->dev);

So we also reference the device (AND the driver module if it's
a module) every time a line is taken. So none of these go
away if any line is in use.

So what remains is the case where say a device really goes
away (USB cable unplugged) or someone use the unbind file
in sysfs (which is the greatest tool on the planet to shoot oneself
in the foot indeed).

What happens in gpiochip_remove() is that the device does really
go away like this:

    /* Numb the device, cancelling all outstanding operations */
    gdev->chip = NULL;

As you can see from an earlier patch, all operations like
gpiod_get_value() etc contains the macro VALIDATE_DESC()
which does this:

#define VALIDATE_DESC(desc) do { \
    if (!desc || !desc->gdev) { \
        pr_warn("%s: invalid GPIO\n", __func__); \
        return -EINVAL; \
    } \
    if ( !desc->gdev->chip ) { \
        dev_warn(&desc->gdev->dev, \
             "%s: backing chip is gone\n", __func__); \
        return 0; \
    } } while (0)

So what happens (I hope) if you e.g. unplug a USB GPIO
controller is that it seems like your GPIO operations
(drivers or userspace or whatever) succeed, but you get
this warning in the dmesg "foo: backing chip is gone".

> IOW any time I see calls to devm_kfree I think it is a mistake and in
> 99% cases I am right.

In this case I intend it to work like this: if userspace
closes the character device (or crashes) the linehandles
will be released with devm_kfree().

But maybe I should not even use devm_* in this case,
but rather kmalloc()/kfree(). It just felt safer to also
connect it with the device.

>> +#define GPIOHANDLE_GET_LINE_VALUES_IOCTL _IOR(0xB4, 0x08, struct gpiohandle_data)
>> +#define GPIOHANDLE_SET_LINE_VALUES_IOCTL _IOR(0xB4, 0x09, struct gpiohandle_data)
>
> _IOW for set?

_IOWR for both actually now that I look at it...

I need to fill in a parameter in the struct, pass it to kernelspace and
have kernelspace fill in the requested data then pass it back to
userspace.

Again thanks a lot!

Yours,
Linus Walleij

Re: [PATCH 1/2] gpio: userspace ABI for reading/writing GPIO lines

[Dmitry Torokhov]

Hi Linus,

On Fri, May 27, 2016 at 03:22:52PM +0200, Linus Walleij wrote:
> Hi Dmitry,
> 
> *thanks* for the effort to review this, it's much appreciated.
> 
> On Tue, Apr 26, 2016 at 6:44 PM, Dmitry Torokhov
> <dmitry.torokhov@gmail.com> wrote:
> > On Tue, Apr 26, 2016 at 10:54:25AM +0200, Linus Walleij wrote:
> 
> >> +/**
> >> + * struct linehandle_state - contains the state of a userspace handle
> >> + * @gdev: the GPIO device the handle pertains to
> >> + * @label: consumer label used to tag descriptors
> >> + * @descs: the GPIO descriptors held by this handle
> >> + * @numdescs: the number of descriptors held in the descs array
> >> + */
> >> +struct linehandle_state {
> >> +     struct gpio_device *gdev;
> >> +     char *label;
> >
> > const char *?
> 
> Actually no, but I made an extra check.

Why not? Do you expect users of linehandle_state object to change the
name after the object has been created?

> 
> >> +static long linehandle_ioctl(struct file *filep, unsigned int cmd,
> >> +                          unsigned long arg)
> >> +{
> >> +     struct linehandle_state *lh = filep->private_data;
> >> +     int __user *ip = (int __user *)arg;
> >
> > You seem to be using the same handler for native and compat calls, but
> > for compat you need to use compat_ptr() because not all arches employ
> > straight cast conversions.
> 
> Argh and that is actually a bug in my primary ioctl(). I sent a separate
> patch fixing this up that should be in your inbox, and also fixed it in this
> patch of course.
> 
> >> +     if (cmd == GPIOHANDLE_GET_LINE_VALUES_IOCTL) {
> >
> > I am fond of switch/case for flows like this.
> 
> I like the if-else-if because it enables me to:
> 
> >> +             int val;
> 
> Declare base block scope-local variables. switch()
> requires a confusing { } to get the same local block.
> It's a bit of taste question I guess.
> 
> >> +             /* Reuse the array setting function */
> >> +             gpiod_set_array_value_complex(false,
> >> +                                           true,
> >> +                                           lh->numdescs,
> >> +                                           lh->descs,
> >> +                                           vals);
> >
> > I wonder if we should be returning errors to userspace in case we failed
> > to toggle one or more gpios (since we seem to moving towards gpio
> > transitions being allowed to fail).
> 
> OK I was actually thinking of that too, so now that you point
> it out I should just fix that with a separate refactoring patch
> making gpiod_set_array_value_complex() return an error
> code.
> 
> >> +     /*
> >> +      * Use devm_* calls with devm_*free() so we can request
> >> +      * and free the memory for these while still be sure that
> >> +      * they will eventually go away with the device if not
> >> +      * before that.
> >> +      */
> >
> > This is quite "interesting" statement. Consider what happens if driver
> > gets unbound from the device, but userspace still holds character device
> > open and calls ioctl on it.
> 
> That is actually a usecase I have spent much time with
> speculating about, and I am trying to make that work. But it's
> kind of complex.
> 
> First I must actually make sure that the device itself doesn't
> go away randomly, so five lines down from this I have this:
> 
> >> +     get_device(&gdev->dev);
> 
> IIUC get_device() will inhibit the device from being unregistered
> so that will not happen, and anything allocated with devm_*
> will still be around, like the linehandles. Notice that this is not
> the physical device but the gpiolib-intrinsic device that gets
> registered from a say platform_device, i2c_device or so.
> The latter is only the parent of this device.

This is wrong assumption(s). First of all devres resources are not tried
to lifetime of device, bit rather to lifetime of the bond between device
and driver. I.e. they go away when driver is unbound from the device,
even though device object is still in memory (either because physical
device is still present in the system, or because of existing references
to the "struct device" object).

Second, get_device() only increments reference count of the device in
question, preventing memory occupied by device object (and its parents)
from being freed. It has no effect whatsoever on the state of the
driver. In other words get_device() does not stop driver core from
unbinding the driver or unregistering the device (either because driver
module is being unloaded, hot-pluggable physical device gone away, or
user unbound the driver through sysfs).

> 
> Even if the parent i2c_device, platform_device or whatever
> goes away, this will stay around because it is referenced
> from userspace. (Or something else that hold a reference
> count to it.)
> 
> Now for the individual lines we also have this in gpiod_get()
> everytime a line is requested from a chip:
> 
> int gpiod_request(struct gpio_desc *desc, const char *label)
> {
> (...)
>     if (try_module_get(gdev->owner)) {
>         status = __gpiod_request(desc, label);
>         if (status < 0)
>             module_put(gdev->owner);
>         else
>             get_device(&gdev->dev);
> 
> So we also reference the device (AND the driver module if it's
> a module) every time a line is taken. So none of these go
> away if any line is in use.
> 
> So what remains is the case where say a device really goes
> away (USB cable unplugged) or someone use the unbind file
> in sysfs (which is the greatest tool on the planet to shoot oneself
> in the foot indeed).
> 
> What happens in gpiochip_remove() is that the device does really
> go away like this:
> 
>     /* Numb the device, cancelling all outstanding operations */
>     gdev->chip = NULL;
> 
> As you can see from an earlier patch, all operations like
> gpiod_get_value() etc contains the macro VALIDATE_DESC()
> which does this:
> 
> #define VALIDATE_DESC(desc) do { \
>     if (!desc || !desc->gdev) { \
>         pr_warn("%s: invalid GPIO\n", __func__); \
>         return -EINVAL; \
>     } \
>     if ( !desc->gdev->chip ) { \
>         dev_warn(&desc->gdev->dev, \
>              "%s: backing chip is gone\n", __func__); \
>         return 0; \
>     } } while (0)
> 
> So what happens (I hope) if you e.g. unplug a USB GPIO
> controller is that it seems like your GPIO operations
> (drivers or userspace or whatever) succeed, but you get
> this warning in the dmesg "foo: backing chip is gone".

That might work, I'll have to go back and look through the patch again,
as long as we do not use devm.

> 
> > IOW any time I see calls to devm_kfree I think it is a mistake and in
> > 99% cases I am right.
> 
> In this case I intend it to work like this: if userspace
> closes the character device (or crashes) the linehandles
> will be released with devm_kfree().
> 
> But maybe I should not even use devm_* in this case,
> but rather kmalloc()/kfree(). It just felt safer to also
> connect it with the device.

No, devm* should only be used in probe/remove paths, everywhere else you
need kmalloc/kfree, etc. Otherwise your memory may disappear too early.

Thanks.

-- 
Dmitry

Re: [PATCH 2/2] tools/gpio: add the gpio-hammer tool

[Linus Walleij]

On Wed, Apr 27, 2016 at 6:00 PM, Michael Welling <mwelling@ieee.org> wrote:
> On Tue, Apr 26, 2016 at 10:54:26AM +0200, Linus Walleij wrote:
>> The gpio-hammer is used from userspace as an example of how
>> to retrieve a GPIO handle for one or several GPIO lines and
>> hammer the outputs from low to high and back again. It will
>> pulse the selected lines once per second for a specified
>> number of times or indefinitely if no loop count is
>> supplied.
>>
>> Example output:
>> $ gpio-hammer -n gpiochip0 -o5 -o6 -o7
>> Hammer lines [5, 6, 7] on gpiochip0, initial states: [1, 1, 1]
>> [-] [5: 0, 6: 0, 7: 0]
>>
>> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
>
> Tested-by: Michael Welling <mwelling@ieee.org>

Thanks! :)

> Below is the output from my target.

You can now get rid of all the "unnamed" in the first column
by supplying the line/rail names in gpio-line-names = "A", "B" ...;
in the GPIO controller node in the device tree for the platform,
if it uses device tree too boot.

> You will notice that a kernel warning is
> spewed out if I use the mcp23s08.
> The processor GPIOs work fine as verified by blinking LEDs.
>
> root@som3517-som200:~# ./lsgpio
> GPIO chip: gpiochip4, "mcp23s08", 8 GPIO lines
>         line  0: unnamed unused
>         line  1: unnamed unused
>         line  2: unnamed unused
>         line  3: unnamed unused
>         line  4: unnamed unused
>         line  5: unnamed unused
>         line  6: unnamed unused
>         line  7: unnamed unused

> root@som3517-som200:~# ./gpio-hammer -n gpiochip4 -o0
> [  187.511606] ------------[ cut here ]------------
> [  187.516949] WARNING: CPU: 0 PID: 830 at /home/michael/projects/linux/linux-git/drivers/gpio/gpiolib.c:1907 gpiod_get_value+0x60/0xa4

Ah that's right, I have to use gpiod_get_value_cansleep(). Will
fix that.

Yours,
Linus Walleij

Re: [PATCH 2/2] tools/gpio: add the gpio-hammer tool

[Michael Welling]

On Tue, May 31, 2016 at 01:59:42PM +0200, Linus Walleij wrote:
> On Wed, Apr 27, 2016 at 6:00 PM, Michael Welling <mwelling@ieee.org> wrote:
> > On Tue, Apr 26, 2016 at 10:54:26AM +0200, Linus Walleij wrote:
> >> The gpio-hammer is used from userspace as an example of how
> >> to retrieve a GPIO handle for one or several GPIO lines and
> >> hammer the outputs from low to high and back again. It will
> >> pulse the selected lines once per second for a specified
> >> number of times or indefinitely if no loop count is
> >> supplied.
> >>
> >> Example output:
> >> $ gpio-hammer -n gpiochip0 -o5 -o6 -o7
> >> Hammer lines [5, 6, 7] on gpiochip0, initial states: [1, 1, 1]
> >> [-] [5: 0, 6: 0, 7: 0]
> >>
> >> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> >
> > Tested-by: Michael Welling <mwelling@ieee.org>
> 
> Thanks! :)

No problem. I wish I could help more.
I like how the support has progressed.

Did you ever find a solution for the exhausted char dev problem?

> 
> > Below is the output from my target.
> 
> You can now get rid of all the "unnamed" in the first column
> by supplying the line/rail names in gpio-line-names = "A", "B" ...;
> in the GPIO controller node in the device tree for the platform,
> if it uses device tree too boot.

Yeah I saw that.. not bored enough to name all of the GPIOs.

> 
> > You will notice that a kernel warning is
> > spewed out if I use the mcp23s08.
> > The processor GPIOs work fine as verified by blinking LEDs.
> >
> > root@som3517-som200:~# ./lsgpio
> > GPIO chip: gpiochip4, "mcp23s08", 8 GPIO lines
> >         line  0: unnamed unused
> >         line  1: unnamed unused
> >         line  2: unnamed unused
> >         line  3: unnamed unused
> >         line  4: unnamed unused
> >         line  5: unnamed unused
> >         line  6: unnamed unused
> >         line  7: unnamed unused
> 
> > root@som3517-som200:~# ./gpio-hammer -n gpiochip4 -o0
> > [  187.511606] ------------[ cut here ]------------
> > [  187.516949] WARNING: CPU: 0 PID: 830 at /home/michael/projects/linux/linux-git/drivers/gpio/gpiolib.c:1907 gpiod_get_value+0x60/0xa4
> 
> Ah that's right, I have to use gpiod_get_value_cansleep(). Will
> fix that.
> 
> Yours,
> Linus Walleij

Re: [PATCH 2/2] tools/gpio: add the gpio-hammer tool

[Linus Walleij]

On Wed, Jun 1, 2016 at 5:43 AM, Michael Welling <mwelling@ieee.org> wrote:

> Did you ever find a solution for the exhausted char dev problem?

Not really. It seems it turned up that all systems that bugged were randomly
generated QEMU models, not real systems.

They were also really doing far out stuff already, overwriting existing chardevs
all over the place, which can be seen in logs after commit
49db08c35887 "chrdev: emit a warning when we go below dynamic major range"
which I got merged.

I'm now working on an event ABI so we get feature complete and also surpass
the old sysfs.

Yours,
Linus Walleij

Re: [PATCH 2/2] tools/gpio: add the gpio-hammer tool

[Michael Welling]

On Wed, Jun 01, 2016 at 07:35:28PM +0200, Linus Walleij wrote:
> On Wed, Jun 1, 2016 at 5:43 AM, Michael Welling <mwelling@ieee.org> wrote:
> 
> > Did you ever find a solution for the exhausted char dev problem?
> 
> Not really. It seems it turned up that all systems that bugged were randomly
> generated QEMU models, not real systems.
> 
> They were also really doing far out stuff already, overwriting existing chardevs
> all over the place, which can be seen in logs after commit
> 49db08c35887 "chrdev: emit a warning when we go below dynamic major range"
> which I got merged.
>

Yeah I noticed that their config was pretty crazy.
 
> I'm now working on an event ABI so we get feature complete and also surpass
> the old sysfs.
>

Okay I will review and test it when the patch shows up.

Is there a way to name a group of GPIOs?

It seems you are passing around a byte of data for each GPIO state.
Is there a reason why the bits couldn't bit masked into single variable
given the max number of handles is 64?

> Yours,
> Linus Walleij

Re: [PATCH 2/2] tools/gpio: add the gpio-hammer tool

[Linus Walleij]

On Wed, Jun 1, 2016 at 8:09 PM, Michael Welling <mwelling@ieee.org> wrote:

> Is there a way to name a group of GPIOs?

Do you mean from the producer side or the consumer side?
There is gpio-line-names in DT for the producer side.

>From the consumer side the same consumer name will be used
on all lines if more than one is selected, which I think makes
sense. (It's just a label after all.)

> It seems you are passing around a byte of data for each GPIO state.
> Is there a reason why the bits couldn't bit masked into single variable
> given the max number of handles is 64?

Sorry not following, I guess you need to post me some part of
the patch or so...

Yours,
Linus Walleij

Re: [PATCH 2/2] tools/gpio: add the gpio-hammer tool

[Michael Welling]

On Wed, Jun 01, 2016 at 11:40:04PM +0200, Linus Walleij wrote:
> On Wed, Jun 1, 2016 at 8:09 PM, Michael Welling <mwelling@ieee.org> wrote:
> 
> > Is there a way to name a group of GPIOs?
> 
> Do you mean from the producer side or the consumer side?

Producer side is what I am thinking.

> There is gpio-line-names in DT for the producer side.
> 
The gpio-line-names are naming individual GPIO on a controller.
I am looking to take a set of GPIOs and name them as a group and
be able to access them by that name. Either individually or
simultaneously.
 
> From the consumer side the same consumer name will be used
> on all lines if more than one is selected, which I think makes
> sense. (It's just a label after all.)
> 
> > It seems you are passing around a byte of data for each GPIO state.
> > Is there a reason why the bits couldn't bit masked into single variable
> > given the max number of handles is 64?
> 
> Sorry not following, I guess you need to post me some part of
> the patch or so...

+       fprintf(stdout, "] on %s, initial states: [", device_name);
+       for (i = 0; i < nlines; i++) {
+               fprintf(stdout, "%d", data.values[i]);

data.values[i];

Each bit is stored in a byte.

> 
> Yours,
> Linus Walleij

Re: [PATCH 2/2] tools/gpio: add the gpio-hammer tool

[Linus Walleij]

On Thu, Jun 2, 2016 at 4:59 PM, Michael Welling <mwelling@ieee.org> wrote:
> On Wed, Jun 01, 2016 at 11:40:04PM +0200, Linus Walleij wrote:
>> On Wed, Jun 1, 2016 at 8:09 PM, Michael Welling <mwelling@ieee.org> wrote:
>>
>> > Is there a way to name a group of GPIOs?
>>
>> Do you mean from the producer side or the consumer side?
>
> Producer side is what I am thinking.
>
>> There is gpio-line-names in DT for the producer side.
>>
> The gpio-line-names are naming individual GPIO on a controller.
> I am looking to take a set of GPIOs and name them as a group and
> be able to access them by that name. Either individually or
> simultaneously.

There is no such mechanism currently, all lines are individual
resources.

>> Sorry not following, I guess you need to post me some part of
>> the patch or so...
>
> +       fprintf(stdout, "] on %s, initial states: [", device_name);
> +       for (i = 0; i < nlines; i++) {
> +               fprintf(stdout, "%d", data.values[i]);
>
> data.values[i];
>
> Each bit is stored in a byte.

Yeah the userspace interface is wasteful in that sense. It's a trade-off
between storage and simplicity and I had to choose something.

If I try to bitstuff the bits then I need to add another ABI to describe
how the bits are stuffed in the words. So I get complexity somewhere
else instead.

Yours,
Linus Walleij