Re: [PATCH v3 0/5] iio: core: Add mmap interface infrastructure

From: Alexandru Ardelean <ardeleanalex@gmail.com>
To: Jonathan Cameron <jic23@kernel.org>
Cc: "Alexandru Ardelean" <alexandru.ardelean@analog.com>,
	LKML <linux-kernel@vger.kernel.org>,
	linux-iio <linux-iio@vger.kernel.org>,
	"Lars-Peter Clausen" <lars@metafoo.de>,
	"Hennerich, Michael" <Michael.Hennerich@analog.com>,
	"Nuno Sá" <nuno.sa@analog.com>,
	"Bogdan, Dragos" <dragos.bogdan@analog.com>
Subject: Re: [PATCH v3 0/5] iio: core: Add mmap interface infrastructure
Date: Wed, 17 Feb 2021 09:18:03 +0200	[thread overview]
Message-ID: <CA+U=DspGd=x+iL2Smzoks6g_MN8kCT8dj8iambAWg3A4JwA1YQ@mail.gmail.com> (raw)
In-Reply-To: <20210216200023.6e4dcde9@archlinux>

On Tue, Feb 16, 2021 at 10:02 PM Jonathan Cameron <jic23@kernel.org> wrote:
>
> On Mon, 15 Feb 2021 16:32:29 +0200
> Alexandru Ardelean <alexandru.ardelean@analog.com> wrote:
>
>
> Hi Alex,
>
> I'm a little nervous about adding the userspace interface used in this
> series, but it seems reasonable and as you say below, is close to what
> Analog have been using for years.
>
> So I've applied it to the togreg branch of iio.git and pushed out as
> testing as normal.   I'm not going to push that out as anything I can't
> rebase for at least a few weeks, so if anyone else wants to take a look
> that would be great.
>

Oops.
I actually was preparing a new version yesterday with a fix for this
and a few tweaks.
This issue seems to appear on 64 bit only.

Will send it.

> One slight tweak was needed. I forced it with min_t(size_t,...
> as it should always be at least as bit as __u32 so should be fine.
>
>   CHECK   drivers/iio/industrialio-core.c
> In file included from ./include/linux/kernel.h:14,
>                  from ./include/asm-generic/bug.h:20,
>                  from ./arch/x86/include/asm/bug.h:93,
>                  from ./include/linux/bug.h:5,
>                  from ./include/linux/mmdebug.h:5,
>                  from ./include/linux/gfp.h:5,
>                  from ./include/linux/slab.h:15,
>                  from drivers/iio/buffer/industrialio-buffer-dmaengine.c:7:
> drivers/iio/buffer/industrialio-buffer-dmaengine.c: In function ‘iio_dmaengine_buffer_submit_block’:
> ./include/linux/minmax.h:18:28: warning: comparison of distinct pointer types lacks a cast
>    18 |  (!!(sizeof((typeof(x) *)1 == (typeof(y) *)1)))
>       |                            ^~
> ./include/linux/minmax.h:32:4: note: in expansion of macro ‘__typecheck’
>    32 |   (__typecheck(x, y) && __no_side_effects(x, y))
>       |    ^~~~~~~~~~~
> ./include/linux/minmax.h:42:24: note: in expansion of macro ‘__safe_cmp’
>    42 |  __builtin_choose_expr(__safe_cmp(x, y), \
>       |                        ^~~~~~~~~~
> ./include/linux/minmax.h:51:19: note: in expansion of macro ‘__careful_cmp’
>    51 | #define min(x, y) __careful_cmp(x, y, <)
>       |                   ^~~~~~~~~~~~~
> drivers/iio/buffer/industrialio-buffer-dmaengine.c:69:28: note: in expansion of macro ‘min’
>    69 |  block->block.bytes_used = min(block->block.size,
>       |                            ^~~
>   CHECK   drivers/iio/buffer/industrialio-buffer-dmaengine.c
>   CHECK   drivers/iio/industrialio-buffer.c
> drivers/iio/buffer/industrialio-buffer-dmaengine.c:69:35: error: incompatible types in comparison expression (different type sizes):
> drivers/iio/buffer/industrialio-buffer-dmaengine.c:69:35:    unsigned int *
> drivers/iio/buffer/industrialio-buffer-dmaengine.c:69:35:    unsigned long *
>
> Thanks,
>
> Jonathan
>
>
> > Changelog v2 -> v3:
> > * https://lore.kernel.org/linux-iio/20210212101143.18993-1-alexandru.ardelean@analog.com/T/#u
> > * added 'Documentation: iio: add doc for high-speed buffer API'
> > * add 'iio: buffer-dma: split iio_dma_buffer_fileio_free() function'
> > * patch 'iio: buffer-dma: Add mmap support'
> >    - unwind free on error path in iio_dma_buffer_alloc_blocks()
> >    - removed double mm.h include
> > * patch 'tools: iio: add example for high-speed buffer support'
> >    - call IIO_BUFFER_BLOCK_FREE_IOCTL on the error path of the
> >      enable_high_speed() function
> >
> > Changelog v1 -> v2:
> > * https://lore.kernel.org/linux-iio/20210211123353.78963-1-alexandru.ardelean@analog.com/T/#t
> > * removed IIO_BUFFER_BLOCK_FLAG_CYCLIC flag; will be added in a later
> >   patch
> > * removed extra line in tools/iio/iio_generic_buffer.c
> > * patch 'iio: core: Add mmap interface infrastructure'
> >   added docstrings for new hooks (alloc_blocks, mmap, etc)
> >
> > This is basically Lars' work adapted from branch:
> >   https://github.com/larsclausen/linux/commits/iio-high-speed-5.10
> > [hopefully i got the stuff correctly from that branch]
> >
> > What is different, is that this one is adapted on top of the multibuffer
> > support (currently at v5) discussed here:
> >   https://lore.kernel.org/linux-iio/20210211122452.78106-1-alexandru.ardelean@analog.com/T/#t
> >
> > Also, adapted an example for high-speed/mmap support in
> > 'tools/iio/iio_generic_buffer.c'
> >
> > The example is adapted from libiio:
> >   https://github.com/analogdevicesinc/libiio/blob/master/local.c#L51
> > but will all the ioctl()s organized after the one that are reserved
> > (hopefully) for IIO
> >
> > Tested that mmap() works.
> > Moved (artifically) valid buffer0 as buffer2 and the operation still
> > works.
> >
> > Alexandru Ardelean (3):
> >   Documentation: iio: add doc for high-speed buffer API
> >   iio: buffer-dma: split iio_dma_buffer_fileio_free() function
> >   tools: iio: add example for high-speed buffer support
> >
> > Lars-Peter Clausen (2):
> >   iio: core: Add mmap interface infrastructure
> >   iio: buffer-dma: Add mmap support
> >
> >  Documentation/iio/iio_high_speed_buffers.rst  | 100 ++++++
> >  Documentation/iio/index.rst                   |   2 +
> >  drivers/iio/buffer/industrialio-buffer-dma.c  | 324 ++++++++++++++++--
> >  .../buffer/industrialio-buffer-dmaengine.c    |  22 +-
> >  drivers/iio/industrialio-buffer.c             | 158 +++++++++
> >  include/linux/iio/buffer-dma.h                |  27 +-
> >  include/linux/iio/buffer_impl.h               |  23 ++
> >  include/uapi/linux/iio/buffer.h               |  49 +++
> >  tools/iio/iio_generic_buffer.c                | 184 +++++++++-
> >  9 files changed, 841 insertions(+), 48 deletions(-)
> >  create mode 100644 Documentation/iio/iio_high_speed_buffers.rst
> >
>