iio: generic_buffer: Select device/trigger by number
v4 cover iio: generic_buffer: Select device/trigger by number
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(-)
Signature · 1 line
2.5.5
PATCH v4 1/3 iio: generic_buffer: Cleanup when receiving signals
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.
--- tools/iio/iio_generic_buffer.c | 172 ++++++++++++++++++++++++++--------------- 1 file changed, 108 insertions(+), 64 deletions(-)
tools/iio/iio_generic_buffer.c +108 -64
@@ -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;
Signature · 1 line
2.5.5
PATCH v4 2/3 iio: generic_buffer: Add --device-num option
In reply to Crestez Dan LeonardThis makes it possible to distinguish between iio devices with the same name.
--- tools/iio/iio_generic_buffer.c | 69 +++++++++++++++++++++++++++++++----------- 1 file changed, 51 insertions(+), 18 deletions(-)
tools/iio/iio_generic_buffer.c +51 -18
@@ -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;
Signature · 1 line
2.5.5
PATCH v4 3/3 iio: generic_buffer: Add --trigger-num option
In reply to Crestez Dan Leonard--- tools/iio/iio_generic_buffer.c | 36 ++++++++++++++++++++++++++++++------ 1 file changed, 30 insertions(+), 6 deletions(-)
tools/iio/iio_generic_buffer.c +30 -6
@@ -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" ); }/*
Signature · 1 line
2.5.5
Quoted 4 lines · 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.
Quoted 8 lines
Changes since v3:
* Rebase on top of latest jic23/testing
* Fix handling of --trigger-num 0. Initializetrig_numto -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:
Ignore this series, I'll send a fix instead.
Signature · 2 lines
Regards, Leonard