Re: [PATCH v2 02/12] iio: buffer-dma: Enable buffer write support

Andy Shevchenko
2022-03-28T20:40:18Z

On Wed, Feb 9, 2022 at 9:10 AM Paul Cercueil <paul@crapouillou.net> wrote:
>
> Adding write support to the buffer-dma code is easy - the write()
> function basically needs to do the exact same thing as the read()
> function: dequeue a block, read or write the data, enqueue the block
> when entirely processed.
>
> Therefore, the iio_buffer_dma_read() and the new iio_buffer_dma_write()
> now both call a function iio_buffer_dma_io(), which will perform this
> task.
>
> The .space_available() callback can return the exact same value as the
> .data_available() callback for input buffers, since in both cases we
> count the exact same thing (the number of bytes in each available
> block).
>
> Note that we preemptively reset block->bytes_used to the buffer's size
> in iio_dma_buffer_request_update(), as in the future the
> iio_dma_buffer_enqueue() function won't reset it.

...

> v2: - Fix block->state not being reset in
> iio_dma_buffer_request_update() for output buffers.
> - Only update block->bytes_used once and add a comment about why we
> update it.
> - Add a comment about why we're setting a different state for output
> buffers in iio_dma_buffer_request_update()
> - Remove useless cast to bool (!!) in iio_dma_buffer_io()

Usual place for changelog is after the cutter '--- ' line below...

> Signed-off-by: Paul Cercueil <paul@crapouillou.net>
> Reviewed-by: Alexandru Ardelean <ardeleanalex@gmail.com>
> ---

...somewhere here.

> drivers/iio/buffer/industrialio-buffer-dma.c | 88 ++++++++++++++++----
> include/linux/iio/buffer-dma.h | 7 ++

...

> +static int iio_dma_buffer_io(struct iio_buffer *buffer,
> + size_t n, char __user *user_buffer, bool is_write)

I believe there is a room for size_t n on the previous line.

...

> + if (is_write)

I would name it is_from_user.

> + ret = copy_from_user(addr, user_buffer, n);
> + else
> + ret = copy_to_user(user_buffer, addr, n);

...

> +int iio_dma_buffer_write(struct iio_buffer *buffer, size_t n,
> + const char __user *user_buffer)
> +{
> + return iio_dma_buffer_io(buffer, n, (__force char *)user_buffer, true);

Why do you drop address space markers?

> +}

--
With Best Regards,
Andy Shevchenko