Re: [PATCH v2 3/3] tools: iio: add example for high-speed buffer support

2021-02-15 Thread Alexandru Ardelean
On Sun, Feb 14, 2021 at 5:58 PM Jonathan Cameron  wrote:
>
> On Fri, 12 Feb 2021 12:11:43 +0200
> Alexandru Ardelean  wrote:
>
> > Following a recent update to the IIO buffer infrastructure, this change
> > adds a basic example on how to access an IIO buffer via the new mmap()
> > interface.
> >
> > The ioctl() for the high-speed mode needs to be enabled right from the
> > start, before setting any parameters via sysfs (length, enable, etc), to
> > make sure that the mmap mode is used and not the fileio mode.
> >
> > Signed-off-by: Alexandru Ardelean 
>
> Just one small question on error handling. Otherwise this looks fine to me.
>
> thanks,
>
> Jonathan
>
> > ---
> >  tools/iio/iio_generic_buffer.c | 183 +++--
> >  1 file changed, 177 insertions(+), 6 deletions(-)
> >
> > diff --git a/tools/iio/iio_generic_buffer.c b/tools/iio/iio_generic_buffer.c
> > index fdd08514d556..675a7e6047e0 100644
> > --- a/tools/iio/iio_generic_buffer.c
> > +++ b/tools/iio/iio_generic_buffer.c
> > @@ -31,6 +31,7 @@
> >  #include 
> >  #include 
> >  #include 
> > +#include 
> >  #include 
> >  #include "iio_utils.h"
> >
> > @@ -239,6 +240,132 @@ static int enable_disable_all_channels(char 
> > *dev_dir_name, int buffer_idx, int e
> >   return 0;
> >  }
> >
> > +struct mmap_block {
> > + struct iio_buffer_block block;
> > + void *addr;
> > +};
> > +
> > +static struct mmap_block *enable_high_speed(int buf_fd, unsigned int 
> > block_size,
> > + int nblocks)
> > +{
> > + struct iio_buffer_block_alloc_req req = { 0 };
> > + struct mmap_block *mmaps = NULL;
> > + int mmaps_cnt = 0;
> > + int i, ret;
> > +
> > + /**
> > +  * Validate we can do high-speed by issuing BLOCK_FREE ioctl.
> > +  * If using just BLOCK_ALLOC it's distinguish between ENOSYS
> > +  * and other error types.
> > +  */
> > + ret = ioctl(buf_fd, IIO_BUFFER_BLOCK_FREE_IOCTL, 0);
> > + if (ret < 0) {
> > + errno = ENOSYS;
> > + return NULL;
> > + }
> > +
> > + /* for now, this */
> > + req.id = 0;
> > + req.type = 0;
> > + req.size = block_size;
> > + req.count = nblocks;
> > +
> > + ret = ioctl(buf_fd, IIO_BUFFER_BLOCK_ALLOC_IOCTL, );
> > + if (ret < 0)
> > + return NULL;
> > +
> > + if (req.count == 0) {
> > + errno = ENOMEM;
> > + return NULL;
> > + }
> > +
> > + if (req.count < nblocks) {
> > + fprintf(stderr, "Requested %d blocks, got %d\n",
> > + nblocks, req.count);
> > + errno = ENOMEM;
> > + return NULL;
> > + }
> > +
> > + mmaps = calloc(req.count, sizeof(*mmaps));
> > + if (!mmaps) {
> > + errno = ENOMEM;
> > + return NULL;
> > + }
> > +
> > + for (i = 0; i < req.count; i++) {
> > + mmaps[i].block.id = i;
> > + ret = ioctl(buf_fd, IIO_BUFFER_BLOCK_QUERY_IOCTL, 
> > [i].block);
> > + if (ret < 0)
> > + goto error;
> > +
> > + ret = ioctl(buf_fd, IIO_BUFFER_BLOCK_ENQUEUE_IOCTL, 
> > [i].block);
> > + if (ret < 0)
> > + goto error;
>
> > +
> > + mmaps[i].addr = mmap(0, mmaps[i].block.size,
> > +   PROT_READ | PROT_WRITE, MAP_SHARED,
> > +   buf_fd, mmaps[i].block.data.offset);
> > +
> > + if (mmaps[i].addr == MAP_FAILED)
> > + goto error;
> > +
> > + mmaps_cnt++;
> > + }
> > +
> > + return mmaps;
> > +
> > +error:
> > + for (i = 0; i < mmaps_cnt; i++)
> > + munmap(mmaps[i].addr, mmaps[i].block.size);
> > + free(mmaps);
>
> No free of the blocks?  We have unmapped them but I'd imagine we'd also
> need to free them from the driver side.

Good catch.

>
> > + return NULL;
> > +}
> > +
> > +static int read_high_speed(int buf_fd, char *data, unsigned int block_size,
> > +struct mmap_block *mmaps, unsigned int mmaps_cnt)
> > +{
> > + struct iio_buffer_block block;
> > + int ret;
> > +
> > + /**
> > +  * This is where some buffer-pool management can do wonders,
> > +  * but for the sake of this sample-code, we're just going to
> > +  * copy the data and re-enqueue it back
> > +  */
> > + memset(, 0, sizeof(block));
> > + ret = ioctl(buf_fd, IIO_BUFFER_BLOCK_DEQUEUE_IOCTL, );
> > + if (ret < 0)
> > + return ret;
> > +
> > + /* check for weird conditions */
> > + if (block.bytes_used > block_size) {
> > + fprintf(stderr,
> > + "Got a bigger block (%u) than expected (%u)\n",
> > + block.bytes_used, block_size);
> > + return -EFBIG;
> > + }
> > +
> > + if (block.bytes_used < block_size) {
> > + /**
> > +  * This can be 

Re: [PATCH v2 3/3] tools: iio: add example for high-speed buffer support

2021-02-14 Thread Jonathan Cameron
On Fri, 12 Feb 2021 12:11:43 +0200
Alexandru Ardelean  wrote:

> Following a recent update to the IIO buffer infrastructure, this change
> adds a basic example on how to access an IIO buffer via the new mmap()
> interface.
> 
> The ioctl() for the high-speed mode needs to be enabled right from the
> start, before setting any parameters via sysfs (length, enable, etc), to
> make sure that the mmap mode is used and not the fileio mode.
> 
> Signed-off-by: Alexandru Ardelean 

Just one small question on error handling. Otherwise this looks fine to me.

thanks,

Jonathan

> ---
>  tools/iio/iio_generic_buffer.c | 183 +++--
>  1 file changed, 177 insertions(+), 6 deletions(-)
> 
> diff --git a/tools/iio/iio_generic_buffer.c b/tools/iio/iio_generic_buffer.c
> index fdd08514d556..675a7e6047e0 100644
> --- a/tools/iio/iio_generic_buffer.c
> +++ b/tools/iio/iio_generic_buffer.c
> @@ -31,6 +31,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  #include "iio_utils.h"
>  
> @@ -239,6 +240,132 @@ static int enable_disable_all_channels(char 
> *dev_dir_name, int buffer_idx, int e
>   return 0;
>  }
>  
> +struct mmap_block {
> + struct iio_buffer_block block;
> + void *addr;
> +};
> +
> +static struct mmap_block *enable_high_speed(int buf_fd, unsigned int 
> block_size,
> + int nblocks)
> +{
> + struct iio_buffer_block_alloc_req req = { 0 };
> + struct mmap_block *mmaps = NULL;
> + int mmaps_cnt = 0;
> + int i, ret;
> +
> + /**
> +  * Validate we can do high-speed by issuing BLOCK_FREE ioctl.
> +  * If using just BLOCK_ALLOC it's distinguish between ENOSYS
> +  * and other error types.
> +  */
> + ret = ioctl(buf_fd, IIO_BUFFER_BLOCK_FREE_IOCTL, 0);
> + if (ret < 0) {
> + errno = ENOSYS;
> + return NULL;
> + }
> +
> + /* for now, this */
> + req.id = 0;
> + req.type = 0;
> + req.size = block_size;
> + req.count = nblocks;
> +
> + ret = ioctl(buf_fd, IIO_BUFFER_BLOCK_ALLOC_IOCTL, );
> + if (ret < 0)
> + return NULL;
> +
> + if (req.count == 0) {
> + errno = ENOMEM;
> + return NULL;
> + }
> +
> + if (req.count < nblocks) {
> + fprintf(stderr, "Requested %d blocks, got %d\n",
> + nblocks, req.count);
> + errno = ENOMEM;
> + return NULL;
> + }
> +
> + mmaps = calloc(req.count, sizeof(*mmaps));
> + if (!mmaps) {
> + errno = ENOMEM;
> + return NULL;
> + }
> +
> + for (i = 0; i < req.count; i++) {
> + mmaps[i].block.id = i;
> + ret = ioctl(buf_fd, IIO_BUFFER_BLOCK_QUERY_IOCTL, 
> [i].block);
> + if (ret < 0)
> + goto error;
> +
> + ret = ioctl(buf_fd, IIO_BUFFER_BLOCK_ENQUEUE_IOCTL, 
> [i].block);
> + if (ret < 0)
> + goto error;

> +
> + mmaps[i].addr = mmap(0, mmaps[i].block.size,
> +   PROT_READ | PROT_WRITE, MAP_SHARED,
> +   buf_fd, mmaps[i].block.data.offset);
> +
> + if (mmaps[i].addr == MAP_FAILED)
> + goto error;
> +
> + mmaps_cnt++;
> + }
> +
> + return mmaps;
> +
> +error:
> + for (i = 0; i < mmaps_cnt; i++)
> + munmap(mmaps[i].addr, mmaps[i].block.size);
> + free(mmaps);

No free of the blocks?  We have unmapped them but I'd imagine we'd also
need to free them from the driver side.

> + return NULL;
> +}
> +
> +static int read_high_speed(int buf_fd, char *data, unsigned int block_size,
> +struct mmap_block *mmaps, unsigned int mmaps_cnt)
> +{
> + struct iio_buffer_block block;
> + int ret;
> +
> + /**
> +  * This is where some buffer-pool management can do wonders,
> +  * but for the sake of this sample-code, we're just going to
> +  * copy the data and re-enqueue it back
> +  */
> + memset(, 0, sizeof(block));
> + ret = ioctl(buf_fd, IIO_BUFFER_BLOCK_DEQUEUE_IOCTL, );
> + if (ret < 0)
> + return ret;
> +
> + /* check for weird conditions */
> + if (block.bytes_used > block_size) {
> + fprintf(stderr,
> + "Got a bigger block (%u) than expected (%u)\n",
> + block.bytes_used, block_size);
> + return -EFBIG;
> + }
> +
> + if (block.bytes_used < block_size) {
> + /**
> +  * This can be normal, with some real-world data
> +  * terminating abruptly. But log it.
> +  */
> + fprintf(stderr,
> + "Got a smaller block (%u) than expected (%u)\n",
> + block.bytes_used, block_size);
> + }
> +
> + /* memcpy() the data, we lose some more performance here :p */
> + memcpy(data, 

[PATCH v2 3/3] tools: iio: add example for high-speed buffer support

2021-02-12 Thread Alexandru Ardelean
Following a recent update to the IIO buffer infrastructure, this change
adds a basic example on how to access an IIO buffer via the new mmap()
interface.

The ioctl() for the high-speed mode needs to be enabled right from the
start, before setting any parameters via sysfs (length, enable, etc), to
make sure that the mmap mode is used and not the fileio mode.

Signed-off-by: Alexandru Ardelean 
---
 tools/iio/iio_generic_buffer.c | 183 +++--
 1 file changed, 177 insertions(+), 6 deletions(-)

diff --git a/tools/iio/iio_generic_buffer.c b/tools/iio/iio_generic_buffer.c
index fdd08514d556..675a7e6047e0 100644
--- a/tools/iio/iio_generic_buffer.c
+++ b/tools/iio/iio_generic_buffer.c
@@ -31,6 +31,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include "iio_utils.h"
 
@@ -239,6 +240,132 @@ static int enable_disable_all_channels(char 
*dev_dir_name, int buffer_idx, int e
return 0;
 }
 
+struct mmap_block {
+   struct iio_buffer_block block;
+   void *addr;
+};
+
+static struct mmap_block *enable_high_speed(int buf_fd, unsigned int 
block_size,
+   int nblocks)
+{
+   struct iio_buffer_block_alloc_req req = { 0 };
+   struct mmap_block *mmaps = NULL;
+   int mmaps_cnt = 0;
+   int i, ret;
+
+   /**
+* Validate we can do high-speed by issuing BLOCK_FREE ioctl.
+* If using just BLOCK_ALLOC it's distinguish between ENOSYS
+* and other error types.
+*/
+   ret = ioctl(buf_fd, IIO_BUFFER_BLOCK_FREE_IOCTL, 0);
+   if (ret < 0) {
+   errno = ENOSYS;
+   return NULL;
+   }
+
+   /* for now, this */
+   req.id = 0;
+   req.type = 0;
+   req.size = block_size;
+   req.count = nblocks;
+
+   ret = ioctl(buf_fd, IIO_BUFFER_BLOCK_ALLOC_IOCTL, );
+   if (ret < 0)
+   return NULL;
+
+   if (req.count == 0) {
+   errno = ENOMEM;
+   return NULL;
+   }
+
+   if (req.count < nblocks) {
+   fprintf(stderr, "Requested %d blocks, got %d\n",
+   nblocks, req.count);
+   errno = ENOMEM;
+   return NULL;
+   }
+
+   mmaps = calloc(req.count, sizeof(*mmaps));
+   if (!mmaps) {
+   errno = ENOMEM;
+   return NULL;
+   }
+
+   for (i = 0; i < req.count; i++) {
+   mmaps[i].block.id = i;
+   ret = ioctl(buf_fd, IIO_BUFFER_BLOCK_QUERY_IOCTL, 
[i].block);
+   if (ret < 0)
+   goto error;
+
+   ret = ioctl(buf_fd, IIO_BUFFER_BLOCK_ENQUEUE_IOCTL, 
[i].block);
+   if (ret < 0)
+   goto error;
+
+   mmaps[i].addr = mmap(0, mmaps[i].block.size,
+ PROT_READ | PROT_WRITE, MAP_SHARED,
+ buf_fd, mmaps[i].block.data.offset);
+
+   if (mmaps[i].addr == MAP_FAILED)
+   goto error;
+
+   mmaps_cnt++;
+   }
+
+   return mmaps;
+
+error:
+   for (i = 0; i < mmaps_cnt; i++)
+   munmap(mmaps[i].addr, mmaps[i].block.size);
+   free(mmaps);
+   return NULL;
+}
+
+static int read_high_speed(int buf_fd, char *data, unsigned int block_size,
+  struct mmap_block *mmaps, unsigned int mmaps_cnt)
+{
+   struct iio_buffer_block block;
+   int ret;
+
+   /**
+* This is where some buffer-pool management can do wonders,
+* but for the sake of this sample-code, we're just going to
+* copy the data and re-enqueue it back
+*/
+   memset(, 0, sizeof(block));
+   ret = ioctl(buf_fd, IIO_BUFFER_BLOCK_DEQUEUE_IOCTL, );
+   if (ret < 0)
+   return ret;
+
+   /* check for weird conditions */
+   if (block.bytes_used > block_size) {
+   fprintf(stderr,
+   "Got a bigger block (%u) than expected (%u)\n",
+   block.bytes_used, block_size);
+   return -EFBIG;
+   }
+
+   if (block.bytes_used < block_size) {
+   /**
+* This can be normal, with some real-world data
+* terminating abruptly. But log it.
+*/
+   fprintf(stderr,
+   "Got a smaller block (%u) than expected (%u)\n",
+   block.bytes_used, block_size);
+   }
+
+   /* memcpy() the data, we lose some more performance here :p */
+   memcpy(data, mmaps[block.id].addr, block.bytes_used);
+
+   /* and re-queue this back */
+   ret = ioctl(buf_fd, IIO_BUFFER_BLOCK_ENQUEUE_IOCTL, 
[block.id].block);
+   if (ret < 0)
+   return ret;
+
+   return block.bytes_used;
+}
+
 static void print_usage(void)
 {
fprintf(stderr, "Usage: generic_buffer [options]...\n"
@@ -249,6 +376,7 @@ static void