lkml.org 
[lkml]   [2022]   [Mar]   [28]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    From
    Date
    SubjectRe: [PATCH v2 02/12] iio: buffer-dma: Enable buffer write support
    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

    \
     
     \ /
      Last update: 2022-03-28 22:41    [W:5.142 / U:0.076 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site