[PATCH v4 0/3] iio: generic_buffer: Select device/trigger by number

[PATCH v4 0/3] iio: generic_buffer: Select device/trigger by number

[Crestez Dan Leonard]

This patch series adds support for selecting iio devices/triggers by numeric id
rather than just name. This is useful because the names are not actually
guaranteed to be unique.

Changes since v3:
* Rebase on top of latest jic23/testing
* Fix handling of --trigger-num 0. Initialize trig_num to -1 and check if >= 0
for valid id.

Jonathan: you replied to v3 saying that they were applied but I don't see those
patches on any branch from your repo:
    https://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git

Crestez Dan Leonard (3):
  iio: generic_buffer: Cleanup when receiving signals
  iio: generic_buffer: Add --device-num option
  iio: generic_buffer: Add --trigger-num option

 tools/iio/iio_generic_buffer.c | 261 ++++++++++++++++++++++++++++-------------
 1 file changed, 181 insertions(+), 80 deletions(-)

-- 
2.5.5

[PATCH v4 1/3] iio: generic_buffer: Cleanup when receiving signals

[Crestez Dan Leonard]

This will clean (disable buffer/trigger/channels) when doing
something like a CTRL-C. Otherwise restarting generic_buffer requires a
manual echo 0 > buffer/enable

This also cleanup up all the code freeing string buffers at
the end of main. We initialize all pointers to NULL so that cleanup can
all be done under a single error label.

Signed-off-by: Crestez Dan Leonard <leonard.crestez@intel.com>
---
 tools/iio/iio_generic_buffer.c | 172 ++++++++++++++++++++++++++---------------
 1 file changed, 108 insertions(+), 64 deletions(-)

diff --git a/tools/iio/iio_generic_buffer.c b/tools/iio/iio_generic_buffer.c
index 2429c78..972f400 100644
--- a/tools/iio/iio_generic_buffer.c
+++ b/tools/iio/iio_generic_buffer.c
@@ -32,6 +32,8 @@
 #include <endian.h>
 #include <getopt.h>
 #include <inttypes.h>
+#include <stdbool.h>
+#include <signal.h>
 #include "iio_utils.h"
 
 /**
@@ -254,6 +256,65 @@ void print_usage(void)
 		"  -w <n>     Set delay between reads in us (event-less mode)\n");
 }
 
+enum autochan autochannels = AUTOCHANNELS_DISABLED;
+char *dev_dir_name = NULL;
+char *buf_dir_name = NULL;
+bool current_trigger_set = false;
+
+void cleanup(void)
+{
+	int ret;
+
+	/* Disable trigger */
+	if (dev_dir_name && current_trigger_set) {
+		/* Disconnect the trigger - just write a dummy name. */
+		ret = write_sysfs_string("trigger/current_trigger",
+					 dev_dir_name, "NULL");
+		if (ret < 0)
+			fprintf(stderr, "Failed to disable trigger: %s\n",
+				strerror(-ret));
+		current_trigger_set = false;
+	}
+
+	/* Disable buffer */
+	if (buf_dir_name) {
+		ret = write_sysfs_int("enable", buf_dir_name, 0);
+		if (ret < 0)
+			fprintf(stderr, "Failed to disable buffer: %s\n",
+				strerror(-ret));
+	}
+
+	/* Disable channels if auto-enabled */
+	if (dev_dir_name && autochannels == AUTOCHANNELS_ACTIVE) {
+		ret = enable_disable_all_channels(dev_dir_name, 0);
+		if (ret)
+			fprintf(stderr, "Failed to disable all channels\n");
+		autochannels = AUTOCHANNELS_DISABLED;
+	}
+}
+
+void sig_handler(int signum)
+{
+	fprintf(stderr, "Caught signal %d\n", signum);
+	cleanup();
+	exit(-signum);
+}
+
+void register_cleanup(void)
+{
+	struct sigaction sa = { .sa_handler = sig_handler };
+	const int signums[] = { SIGINT, SIGTERM, SIGABRT };
+	int ret, i;
+
+	for (i = 0; i < ARRAY_SIZE(signums); ++i) {
+		ret = sigaction(signums[i], &sa, NULL);
+		if (ret) {
+			perror("Failed to register signal handler");
+			exit(-1);
+		}
+	}
+}
+
 int main(int argc, char **argv)
 {
 	unsigned long num_loops = 2;
@@ -261,25 +322,24 @@ int main(int argc, char **argv)
 	unsigned long buf_len = 128;
 
 	int ret, c, i, j, toread;
-	int fp;
+	int fp = -1;
 
-	int num_channels;
+	int num_channels = 0;
 	char *trigger_name = NULL, *device_name = NULL;
-	char *dev_dir_name, *buf_dir_name;
 
-	int datardytrigger = 1;
-	char *data;
+	char *data = NULL;
 	ssize_t read_size;
 	int dev_num, trig_num;
-	char *buffer_access;
+	char *buffer_access = NULL;
 	int scan_size;
 	int noevents = 0;
 	int notrigger = 0;
-	enum autochan autochannels = AUTOCHANNELS_DISABLED;
 	char *dummy;
 
 	struct iio_channel_info *channels;
 
+	register_cleanup();
+
 	while ((c = getopt(argc, argv, "ac:egl:n:t:w:")) != -1) {
 		switch (c) {
 		case 'a':
@@ -288,8 +348,10 @@ int main(int argc, char **argv)
 		case 'c':
 			errno = 0;
 			num_loops = strtoul(optarg, &dummy, 10);
-			if (errno)
-				return -errno;
+			if (errno) {
+				ret = -errno;
+				goto error;
+			}
 
 			break;
 		case 'e':
@@ -301,26 +363,30 @@ int main(int argc, char **argv)
 		case 'l':
 			errno = 0;
 			buf_len = strtoul(optarg, &dummy, 10);
-			if (errno)
-				return -errno;
+			if (errno) {
+				ret = -errno;
+				goto error;
+			}
 
 			break;
 		case 'n':
 			device_name = optarg;
 			break;
 		case 't':
-			trigger_name = optarg;
-			datardytrigger = 0;
+			trigger_name = strdup(optarg);
 			break;
 		case 'w':
 			errno = 0;
 			timedelay = strtoul(optarg, &dummy, 10);
-			if (errno)
-				return -errno;
+			if (errno) {
+				ret = -errno;
+				goto error;
+			}
 			break;
 		case '?':
 			print_usage();
-			return -1;
+			ret = -1;
+			goto error;
 		}
 	}
 
@@ -334,14 +400,17 @@ int main(int argc, char **argv)
 	dev_num = find_type_by_name(device_name, "iio:device");
 	if (dev_num < 0) {
 		fprintf(stderr, "Failed to find the %s\n", device_name);
-		return dev_num;
+		ret = dev_num;
+		goto error;
 	}
 
 	printf("iio device number being used is %d\n", dev_num);
 
 	ret = asprintf(&dev_dir_name, "%siio:device%d", iio_dir, dev_num);
-	if (ret < 0)
-		return -ENOMEM;
+	if (ret < 0) {
+		ret = -ENOMEM;
+		goto error;
+	}
 
 	if (!notrigger) {
 		if (!trigger_name) {
@@ -354,7 +423,7 @@ int main(int argc, char **argv)
 				       "%s-dev%d", device_name, dev_num);
 			if (ret < 0) {
 				ret = -ENOMEM;
-				goto error_free_dev_dir_name;
+				goto error;
 			}
 		}
 
@@ -367,7 +436,7 @@ int main(int argc, char **argv)
 				       "%s-trigger", device_name);
 			if (ret < 0) {
 				ret = -ENOMEM;
-				goto error_free_dev_dir_name;
+				goto error;
 			}
 		}
 
@@ -376,7 +445,7 @@ int main(int argc, char **argv)
 			fprintf(stderr, "Failed to find the trigger %s\n",
 				trigger_name);
 			ret = trig_num;
-			goto error_free_triggername;
+			goto error;
 		}
 
 		printf("iio trigger number being used is %d\n", trig_num);
@@ -392,7 +461,7 @@ int main(int argc, char **argv)
 	if (ret) {
 		fprintf(stderr, "Problem reading scan element information\n"
 			"diag %s\n", dev_dir_name);
-		goto error_free_triggername;
+		goto error;
 	}
 	if (num_channels && autochannels == AUTOCHANNELS_ENABLED) {
 		fprintf(stderr, "Auto-channels selected but some channels "
@@ -407,7 +476,7 @@ int main(int argc, char **argv)
 		ret = enable_disable_all_channels(dev_dir_name, 1);
 		if (ret) {
 			fprintf(stderr, "Failed to enable all channels\n");
-			goto error_free_triggername;
+			goto error;
 		}
 
 		/* This flags that we need to disable the channels again */
@@ -419,12 +488,12 @@ int main(int argc, char **argv)
 			fprintf(stderr, "Problem reading scan element "
 				"information\n"
 				"diag %s\n", dev_dir_name);
-			goto error_disable_channels;
+			goto error;
 		}
 		if (!num_channels) {
 			fprintf(stderr, "Still no channels after "
 				"auto-enabling, giving up\n");
-			goto error_disable_channels;
+			goto error;
 		}
 	}
 
@@ -436,7 +505,7 @@ int main(int argc, char **argv)
 			"/*_en or pass -a to autoenable channels and "
 			"try again.\n", dev_dir_name);
 		ret = -ENOENT;
-		goto error_free_triggername;
+		goto error;
 	}
 
 	/*
@@ -448,7 +517,7 @@ int main(int argc, char **argv)
 		       "%siio:device%d/buffer", iio_dir, dev_num);
 	if (ret < 0) {
 		ret = -ENOMEM;
-		goto error_free_channels;
+		goto error;
 	}
 
 	if (!notrigger) {
@@ -463,34 +532,34 @@ int main(int argc, char **argv)
 		if (ret < 0) {
 			fprintf(stderr,
 				"Failed to write current_trigger file\n");
-			goto error_free_buf_dir_name;
+			goto error;
 		}
 	}
 
 	/* Setup ring buffer parameters */
 	ret = write_sysfs_int("length", buf_dir_name, buf_len);
 	if (ret < 0)
-		goto error_free_buf_dir_name;
+		goto error;
 
 	/* Enable the buffer */
 	ret = write_sysfs_int("enable", buf_dir_name, 1);
 	if (ret < 0) {
 		fprintf(stderr,
 			"Failed to enable buffer: %s\n", strerror(-ret));
-		goto error_free_buf_dir_name;
+		goto error;
 	}
 
 	scan_size = size_from_channelarray(channels, num_channels);
 	data = malloc(scan_size * buf_len);
 	if (!data) {
 		ret = -ENOMEM;
-		goto error_free_buf_dir_name;
+		goto error;
 	}
 
 	ret = asprintf(&buffer_access, "/dev/iio:device%d", dev_num);
 	if (ret < 0) {
 		ret = -ENOMEM;
-		goto error_free_data;
+		goto error;
 	}
 
 	/* Attempt to open non blocking the access dev */
@@ -498,7 +567,7 @@ int main(int argc, char **argv)
 	if (fp == -1) { /* TODO: If it isn't there make the node */
 		ret = -errno;
 		fprintf(stderr, "Failed to open %s\n", buffer_access);
-		goto error_free_buffer_access;
+		goto error;
 	}
 
 	for (j = 0; j < num_loops; j++) {
@@ -511,7 +580,7 @@ int main(int argc, char **argv)
 			ret = poll(&pfd, 1, -1);
 			if (ret < 0) {
 				ret = -errno;
-				goto error_close_buffer_access;
+				goto error;
 			} else if (ret == 0) {
 				continue;
 			}
@@ -536,45 +605,20 @@ int main(int argc, char **argv)
 				     num_channels);
 	}
 
-	/* Stop the buffer */
-	ret = write_sysfs_int("enable", buf_dir_name, 0);
-	if (ret < 0)
-		goto error_close_buffer_access;
-
-	if (!notrigger)
-		/* Disconnect the trigger - just write a dummy name. */
-		ret = write_sysfs_string("trigger/current_trigger",
-					 dev_dir_name, "NULL");
-		if (ret < 0)
-			fprintf(stderr, "Failed to write to %s\n",
-				dev_dir_name);
+error:
+	cleanup();
 
-error_close_buffer_access:
-	if (close(fp) == -1)
+	if (fp >= 0 && close(fp) == -1)
 		perror("Failed to close buffer");
-
-error_free_buffer_access:
 	free(buffer_access);
-error_free_data:
 	free(data);
-error_free_buf_dir_name:
 	free(buf_dir_name);
-error_free_channels:
 	for (i = num_channels - 1; i >= 0; i--) {
 		free(channels[i].name);
 		free(channels[i].generic_name);
 	}
 	free(channels);
-error_free_triggername:
-	if (datardytrigger)
-		free(trigger_name);
-error_disable_channels:
-	if (autochannels == AUTOCHANNELS_ACTIVE) {
-		ret = enable_disable_all_channels(dev_dir_name, 0);
-		if (ret)
-			fprintf(stderr, "Failed to disable all channels\n");
-	}
-error_free_dev_dir_name:
+	free(trigger_name);
 	free(dev_dir_name);
 
 	return ret;
-- 
2.5.5

[PATCH v4 2/3] iio: generic_buffer: Add --device-num option

[Crestez Dan Leonard]

This makes it possible to distinguish between iio devices with the same
name.

Signed-off-by: Crestez Dan Leonard <leonard.crestez@intel.com>
---
 tools/iio/iio_generic_buffer.c | 69 +++++++++++++++++++++++++++++++-----------
 1 file changed, 51 insertions(+), 18 deletions(-)

diff --git a/tools/iio/iio_generic_buffer.c b/tools/iio/iio_generic_buffer.c
index 972f400..3f16e9f 100644
--- a/tools/iio/iio_generic_buffer.c
+++ b/tools/iio/iio_generic_buffer.c
@@ -251,7 +251,9 @@ void print_usage(void)
 		"  -e         Disable wait for event (new data)\n"
 		"  -g         Use trigger-less mode\n"
 		"  -l <n>     Set buffer length to n samples\n"
-		"  -n <name>  Set device name (mandatory)\n"
+		"  --device-name -n <name>\n"
+		"  --device-num -N <num>\n"
+		"        Set device by name or number (mandatory)\n"
 		"  -t <name>  Set trigger name\n"
 		"  -w <n>     Set delay between reads in us (event-less mode)\n");
 }
@@ -315,6 +317,12 @@ void register_cleanup(void)
 	}
 }
 
+static const struct option longopts[] = {
+	{ "device-name",	1, 0, 'n' },
+	{ "device-num",		1, 0, 'N' },
+	{ },
+};
+
 int main(int argc, char **argv)
 {
 	unsigned long num_loops = 2;
@@ -329,7 +337,7 @@ int main(int argc, char **argv)
 
 	char *data = NULL;
 	ssize_t read_size;
-	int dev_num, trig_num;
+	int dev_num = -1, trig_num;
 	char *buffer_access = NULL;
 	int scan_size;
 	int noevents = 0;
@@ -340,7 +348,7 @@ int main(int argc, char **argv)
 
 	register_cleanup();
 
-	while ((c = getopt(argc, argv, "ac:egl:n:t:w:")) != -1) {
+	while ((c = getopt_long(argc, argv, "ac:egl:n:N:t:w:", longopts, NULL)) != -1) {
 		switch (c) {
 		case 'a':
 			autochannels = AUTOCHANNELS_ENABLED;
@@ -370,7 +378,15 @@ int main(int argc, char **argv)
 
 			break;
 		case 'n':
-			device_name = optarg;
+			device_name = strdup(optarg);
+			break;
+		case 'N':
+			errno = 0;
+			dev_num = strtoul(optarg, &dummy, 10);
+			if (errno) {
+				ret = -errno;
+				goto error;
+			}
 			break;
 		case 't':
 			trigger_name = strdup(optarg);
@@ -390,26 +406,42 @@ int main(int argc, char **argv)
 		}
 	}
 
-	if (!device_name) {
-		fprintf(stderr, "Device name not set\n");
-		print_usage();
-		return -1;
-	}
-
 	/* Find the device requested */
-	dev_num = find_type_by_name(device_name, "iio:device");
-	if (dev_num < 0) {
-		fprintf(stderr, "Failed to find the %s\n", device_name);
-		ret = dev_num;
+	if (dev_num < 0 && !device_name) {
+		fprintf(stderr, "Device not set\n");
+		print_usage();
+		ret = -1;
+		goto error;
+	} else if (dev_num >= 0 && device_name) {
+		fprintf(stderr, "Only one of --device-num or --device-name needs to be set\n");
+		print_usage();
+		ret = -1;
 		goto error;
+	} else if (dev_num < 0) {
+		dev_num = find_type_by_name(device_name, "iio:device");
+		if (dev_num < 0) {
+			fprintf(stderr, "Failed to find the %s\n", device_name);
+			ret = dev_num;
+			goto error;
+		}
 	}
-
 	printf("iio device number being used is %d\n", dev_num);
 
 	ret = asprintf(&dev_dir_name, "%siio:device%d", iio_dir, dev_num);
-	if (ret < 0) {
-		ret = -ENOMEM;
-		goto error;
+	if (ret < 0)
+		return -ENOMEM;
+	/* Fetch device_name if specified by number */
+	if (!device_name) {
+		device_name = malloc(IIO_MAX_NAME_LENGTH);
+		if (!device_name) {
+			ret = -ENOMEM;
+			goto error;
+		}
+		ret = read_sysfs_string("name", dev_dir_name, device_name);
+		if (ret < 0) {
+			fprintf(stderr, "Failed to read name of device %d\n", dev_num);
+			goto error;
+		}
 	}
 
 	if (!notrigger) {
@@ -619,6 +651,7 @@ error:
 	}
 	free(channels);
 	free(trigger_name);
+	free(device_name);
 	free(dev_dir_name);
 
 	return ret;
-- 
2.5.5

[PATCH v4 3/3] iio: generic_buffer: Add --trigger-num option

[Crestez Dan Leonard]

Signed-off-by: Crestez Dan Leonard <leonard.crestez@intel.com>
---
 tools/iio/iio_generic_buffer.c | 36 ++++++++++++++++++++++++++++++------
 1 file changed, 30 insertions(+), 6 deletions(-)

diff --git a/tools/iio/iio_generic_buffer.c b/tools/iio/iio_generic_buffer.c
index 3f16e9f..0e8a1f7 100644
--- a/tools/iio/iio_generic_buffer.c
+++ b/tools/iio/iio_generic_buffer.c
@@ -254,7 +254,9 @@ void print_usage(void)
 		"  --device-name -n <name>\n"
 		"  --device-num -N <num>\n"
 		"        Set device by name or number (mandatory)\n"
-		"  -t <name>  Set trigger name\n"
+		"  --trigger-name -t <name>\n"
+		"  --trigger-num -T <num>\n"
+		"        Set trigger by name or number\n"
 		"  -w <n>     Set delay between reads in us (event-less mode)\n");
 }
 
@@ -320,6 +322,8 @@ void register_cleanup(void)
 static const struct option longopts[] = {
 	{ "device-name",	1, 0, 'n' },
 	{ "device-num",		1, 0, 'N' },
+	{ "trigger-name",	1, 0, 't' },
+	{ "trigger-num",	1, 0, 'T' },
 	{ },
 };
 
@@ -337,7 +341,7 @@ int main(int argc, char **argv)
 
 	char *data = NULL;
 	ssize_t read_size;
-	int dev_num = -1, trig_num;
+	int dev_num = -1, trig_num = -1;
 	char *buffer_access = NULL;
 	int scan_size;
 	int noevents = 0;
@@ -348,7 +352,7 @@ int main(int argc, char **argv)
 
 	register_cleanup();
 
-	while ((c = getopt_long(argc, argv, "ac:egl:n:N:t:w:", longopts, NULL)) != -1) {
+	while ((c = getopt_long(argc, argv, "ac:egl:n:N:t:T:w:", longopts, NULL)) != -1) {
 		switch (c) {
 		case 'a':
 			autochannels = AUTOCHANNELS_ENABLED;
@@ -391,6 +395,12 @@ int main(int argc, char **argv)
 		case 't':
 			trigger_name = strdup(optarg);
 			break;
+		case 'T':
+			errno = 0;
+			trig_num = strtoul(optarg, &dummy, 10);
+			if (errno)
+				return -errno;
+			break;
 		case 'w':
 			errno = 0;
 			timedelay = strtoul(optarg, &dummy, 10);
@@ -444,7 +454,23 @@ int main(int argc, char **argv)
 		}
 	}
 
-	if (!notrigger) {
+	if (notrigger) {
+		printf("trigger-less mode selected\n");
+	} if (trig_num >= 0) {
+		char *trig_dev_name;
+		ret = asprintf(&trig_dev_name, "%strigger%d", iio_dir, trig_num);
+		if (ret < 0) {
+			return -ENOMEM;
+		}
+		trigger_name = malloc(IIO_MAX_NAME_LENGTH);
+		ret = read_sysfs_string("name", trig_dev_name, trigger_name);
+		free(trig_dev_name);
+		if (ret < 0) {
+			fprintf(stderr, "Failed to read trigger%d name from\n", trig_num);
+			return ret;
+		}
+		printf("iio trigger number being used is %d\n", trig_num);
+	} else {
 		if (!trigger_name) {
 			/*
 			 * Build the trigger name. If it is device associated
@@ -481,8 +507,6 @@ int main(int argc, char **argv)
 		}
 
 		printf("iio trigger number being used is %d\n", trig_num);
-	} else {
-		printf("trigger-less mode selected\n");
 	}
 
 	/*
-- 
2.5.5

Re: [PATCH v4 0/3] iio: generic_buffer: Select device/trigger by number

[Crestez Dan Leonard]

On 05/30/2016 06:19 PM, Crestez Dan Leonard wrote:
> This patch series adds support for selecting iio devices/triggers by numeric id
> rather than just name. This is useful because the names are not actually
> guaranteed to be unique.
> 
> Changes since v3:
> * Rebase on top of latest jic23/testing
> * Fix handling of --trigger-num 0. Initialize trig_num to -1 and check if >= 0
> for valid id.
> 
> Jonathan: you replied to v3 saying that they were applied but I don't see those
> patches on any branch from your repo:
>     https://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git

I see v3 made it's way into jic23/testing anyway:

https://git.kernel.org/cgit/linux/kernel/git/jic23/iio.git/commit/?h=testing&id=7c7e9dad7017ff5b5f0524ea6d85dcda3c62431e

Ignore this series, I'll send a fix instead.

-- 
Regards,
Leonard