Re: [PATCH v6 24/24] tools: iio: convert iio_generic_buffer to use new IIO buffer API

2021-02-15 Thread Jonathan Cameron
On Mon, 15 Feb 2021 12:40:43 +0200
Alexandru Ardelean  wrote:

> This change makes use of the new IIO buffer API to read data from an IIO
> buffer.
> It doesn't read the /sys/bus/iio/devices/iio:deviceX/scan_elements dir
> anymore, it reads /sys/bus/iio/devices/iio:deviceX/bufferY, where all the
> scan_elements have been merged together with the old/classical buffer
> attributes.
> 
> And it makes use of the new IIO_BUFFER_GET_FD_IOCTL ioctl to get an FD for
> the IIO buffer for which to read data from.
> It also does a quick sanity check to see that -EBUSY is returned if reading
> the chardev after the ioctl() has succeeded.
> 
> This was tested with the following cases:
>  1. Tested buffer0 works with ioctl()
>  2. Tested that buffer0 can't be opened via /dev/iio:deviceX after ioctl()
> This check should be omitted under normal operation; it's being done
> here to check that the driver change is sane
>  3. Moved valid buffer0 to be buffer1, and tested that data comes from it
> 
> Signed-off-by: Alexandru Ardelean 

Whole series applied to the togreg branch of iio.git and pushed out as testing
to see what we missed in review.

There is that one patch doing rework of a load of drivers early on in the
series that may get a few more reviews.

Otherwise, reviews on any of this from others still definitely welcome.
I'm not going to push out as a non rebasing tree for at least a couple of
weeks (given merge window is open).

Thanks,

Jonathan

> ---
>  tools/iio/Makefile |   1 +
>  tools/iio/iio_generic_buffer.c | 122 ++---
>  tools/iio/iio_utils.c  |  13 ++--
>  tools/iio/iio_utils.h  |   4 +-
>  4 files changed, 107 insertions(+), 33 deletions(-)
> 
> diff --git a/tools/iio/Makefile b/tools/iio/Makefile
> index 3de763d9ab70..5d12ac4e7f8f 100644
> --- a/tools/iio/Makefile
> +++ b/tools/iio/Makefile
> @@ -27,6 +27,7 @@ include $(srctree)/tools/build/Makefile.include
>  #
>  $(OUTPUT)include/linux/iio: ../../include/uapi/linux/iio
>   mkdir -p $(OUTPUT)include/linux/iio 2>&1 || true
> + ln -sf $(CURDIR)/../../include/uapi/linux/iio/buffer.h $@
>   ln -sf $(CURDIR)/../../include/uapi/linux/iio/events.h $@
>   ln -sf $(CURDIR)/../../include/uapi/linux/iio/types.h $@
>  
> diff --git a/tools/iio/iio_generic_buffer.c b/tools/iio/iio_generic_buffer.c
> index 7c7240553777..2491c54a5e4f 100644
> --- a/tools/iio/iio_generic_buffer.c
> +++ b/tools/iio/iio_generic_buffer.c
> @@ -30,6 +30,8 @@
>  #include 
>  #include 
>  #include 
> +#include 
> +#include 
>  #include "iio_utils.h"
>  
>  /**
> @@ -197,7 +199,7 @@ static void process_scan(char *data, struct 
> iio_channel_info *channels,
>   printf("\n");
>  }
>  
> -static int enable_disable_all_channels(char *dev_dir_name, int enable)
> +static int enable_disable_all_channels(char *dev_dir_name, int buffer_idx, 
> int enable)
>  {
>   const struct dirent *ent;
>   char scanelemdir[256];
> @@ -205,7 +207,7 @@ static int enable_disable_all_channels(char 
> *dev_dir_name, int enable)
>   int ret;
>  
>   snprintf(scanelemdir, sizeof(scanelemdir),
> -  FORMAT_SCAN_ELEMENTS_DIR, dev_dir_name);
> +  FORMAT_SCAN_ELEMENTS_DIR, dev_dir_name, buffer_idx);
>   scanelemdir[sizeof(scanelemdir)-1] = '\0';
>  
>   dp = opendir(scanelemdir);
> @@ -243,6 +245,7 @@ static void print_usage(void)
>   "Capture, convert and output data from IIO device buffer\n"
>   "  -a Auto-activate all available channels\n"
>   "  -A Force-activate ALL channels\n"
> + "  -b  The buffer which to open (by index), default 0\n"
>   "  -c  Do n conversions, or loop forever if n < 0\n"
>   "  -e Disable wait for event (new data)\n"
>   "  -g Use trigger-less mode\n"
> @@ -259,6 +262,7 @@ static void print_usage(void)
>  static enum autochan autochannels = AUTOCHANNELS_DISABLED;
>  static char *dev_dir_name = NULL;
>  static char *buf_dir_name = NULL;
> +static int buffer_idx = 0;
>  static bool current_trigger_set = false;
>  
>  static void cleanup(void)
> @@ -286,7 +290,7 @@ static void cleanup(void)
>  
>   /* Disable channels if auto-enabled */
>   if (dev_dir_name && autochannels == AUTOCHANNELS_ACTIVE) {
> - ret = enable_disable_all_channels(dev_dir_name, 0);
> + ret = enable_disable_all_channels(dev_dir_name, buffer_idx, 0);
>   if (ret)
>   fprintf(stderr, "Failed to disable all channels\n");
>   autochannels = AUTOCHANNELS_DISABLED;
> @@ -333,7 +337,9 @@ int main(int argc, char **argv)
>   unsigned long long j;
>   unsigned long toread;
>   int ret, c;
> - int fp = -1;
> + struct stat st;
> + int fd = -1;
> + int buf_fd = -1;
>  
>   int num_channels = 0;
>   char *trigger_name = NULL, *device_name = NULL;
> @@ -352,7 +358,7 @@ int main(int 

[PATCH v6 24/24] tools: iio: convert iio_generic_buffer to use new IIO buffer API

2021-02-15 Thread Alexandru Ardelean
This change makes use of the new IIO buffer API to read data from an IIO
buffer.
It doesn't read the /sys/bus/iio/devices/iio:deviceX/scan_elements dir
anymore, it reads /sys/bus/iio/devices/iio:deviceX/bufferY, where all the
scan_elements have been merged together with the old/classical buffer
attributes.

And it makes use of the new IIO_BUFFER_GET_FD_IOCTL ioctl to get an FD for
the IIO buffer for which to read data from.
It also does a quick sanity check to see that -EBUSY is returned if reading
the chardev after the ioctl() has succeeded.

This was tested with the following cases:
 1. Tested buffer0 works with ioctl()
 2. Tested that buffer0 can't be opened via /dev/iio:deviceX after ioctl()
This check should be omitted under normal operation; it's being done
here to check that the driver change is sane
 3. Moved valid buffer0 to be buffer1, and tested that data comes from it

Signed-off-by: Alexandru Ardelean 
---
 tools/iio/Makefile |   1 +
 tools/iio/iio_generic_buffer.c | 122 ++---
 tools/iio/iio_utils.c  |  13 ++--
 tools/iio/iio_utils.h  |   4 +-
 4 files changed, 107 insertions(+), 33 deletions(-)

diff --git a/tools/iio/Makefile b/tools/iio/Makefile
index 3de763d9ab70..5d12ac4e7f8f 100644
--- a/tools/iio/Makefile
+++ b/tools/iio/Makefile
@@ -27,6 +27,7 @@ include $(srctree)/tools/build/Makefile.include
 #
 $(OUTPUT)include/linux/iio: ../../include/uapi/linux/iio
mkdir -p $(OUTPUT)include/linux/iio 2>&1 || true
+   ln -sf $(CURDIR)/../../include/uapi/linux/iio/buffer.h $@
ln -sf $(CURDIR)/../../include/uapi/linux/iio/events.h $@
ln -sf $(CURDIR)/../../include/uapi/linux/iio/types.h $@
 
diff --git a/tools/iio/iio_generic_buffer.c b/tools/iio/iio_generic_buffer.c
index 7c7240553777..2491c54a5e4f 100644
--- a/tools/iio/iio_generic_buffer.c
+++ b/tools/iio/iio_generic_buffer.c
@@ -30,6 +30,8 @@
 #include 
 #include 
 #include 
+#include 
+#include 
 #include "iio_utils.h"
 
 /**
@@ -197,7 +199,7 @@ static void process_scan(char *data, struct 
iio_channel_info *channels,
printf("\n");
 }
 
-static int enable_disable_all_channels(char *dev_dir_name, int enable)
+static int enable_disable_all_channels(char *dev_dir_name, int buffer_idx, int 
enable)
 {
const struct dirent *ent;
char scanelemdir[256];
@@ -205,7 +207,7 @@ static int enable_disable_all_channels(char *dev_dir_name, 
int enable)
int ret;
 
snprintf(scanelemdir, sizeof(scanelemdir),
-FORMAT_SCAN_ELEMENTS_DIR, dev_dir_name);
+FORMAT_SCAN_ELEMENTS_DIR, dev_dir_name, buffer_idx);
scanelemdir[sizeof(scanelemdir)-1] = '\0';
 
dp = opendir(scanelemdir);
@@ -243,6 +245,7 @@ static void print_usage(void)
"Capture, convert and output data from IIO device buffer\n"
"  -a Auto-activate all available channels\n"
"  -A Force-activate ALL channels\n"
+   "  -b  The buffer which to open (by index), default 0\n"
"  -c  Do n conversions, or loop forever if n < 0\n"
"  -e Disable wait for event (new data)\n"
"  -g Use trigger-less mode\n"
@@ -259,6 +262,7 @@ static void print_usage(void)
 static enum autochan autochannels = AUTOCHANNELS_DISABLED;
 static char *dev_dir_name = NULL;
 static char *buf_dir_name = NULL;
+static int buffer_idx = 0;
 static bool current_trigger_set = false;
 
 static void cleanup(void)
@@ -286,7 +290,7 @@ static void cleanup(void)
 
/* Disable channels if auto-enabled */
if (dev_dir_name && autochannels == AUTOCHANNELS_ACTIVE) {
-   ret = enable_disable_all_channels(dev_dir_name, 0);
+   ret = enable_disable_all_channels(dev_dir_name, buffer_idx, 0);
if (ret)
fprintf(stderr, "Failed to disable all channels\n");
autochannels = AUTOCHANNELS_DISABLED;
@@ -333,7 +337,9 @@ int main(int argc, char **argv)
unsigned long long j;
unsigned long toread;
int ret, c;
-   int fp = -1;
+   struct stat st;
+   int fd = -1;
+   int buf_fd = -1;
 
int num_channels = 0;
char *trigger_name = NULL, *device_name = NULL;
@@ -352,7 +358,7 @@ int main(int argc, char **argv)
 
register_cleanup();
 
-   while ((c = getopt_long(argc, argv, "aAc:egl:n:N:t:T:w:?", longopts,
+   while ((c = getopt_long(argc, argv, "aAb:c:egl:n:N:t:T:w:?", longopts,
NULL)) != -1) {
switch (c) {
case 'a':
@@ -361,7 +367,20 @@ int main(int argc, char **argv)
case 'A':
autochannels = AUTOCHANNELS_ENABLED;
force_autochannels = true;
-   break;  
+   break;
+   case 'b':
+   errno = 0;
+