Re: [RFC PATCH 1/4] iio: core: Introduce iio_push_to_buffers_with_ts_na() for non aligned case.

[Andy Shevchenko <andy.shevchenko@gmail.com>]

On Sat, May 1, 2021 at 8:28 PM Jonathan Cameron <jic23@kernel.org> wrote:
>
> From: Jonathan Cameron <Jonathan.Cameron@huawei.com>
>
> Whilst it is almost always possible to arrange for scan data to be
> read directly into a buffer that is suitable for passing to
> iio_push_to_buffers_with_timestamp(), there are a few places where
> leading data needs to be skipped over.
>
> For these cases introduce a function that will allocate an appropriate
> sized and aligned bounce buffer (if not already allocated) and copy
> the unaligned data into that before calling
> iio_push_to_buffers_with_timestamp() on the bounce buffer.
> We tie the lifespace of this buffer to that of the iio_dev.dev
> which should ensure no memory leaks occur.

...

> +/**
> + * iio_push_to_buffers_with_ts_na() - push to registered buffer,
> + *    no alignment or space requirements.
> + * @indio_dev:         iio_dev structure for device.
> + * @data:              channel data excluding the timestamp.
> + * @data_sz:           size of data.
> + * @timestamp:         timestamp for the sample data.
> + *
> + * This special variant of iio_push_to_buffers_with_timetamp() does
> + * not require space for the timestamp, or 8 byte alignment of data.
> + * It does however require an allocation on first call and additional
> + * coppies on all calls, so should be avoided if possible

copies

> + */

I do not like the _na part in the name (My first impression was with a
Timestamp that was not available, what?!). Can we spell it better?

> +int iio_push_to_buffers_with_ts_na(struct iio_dev *indio_dev,
> +                                  const void *data,
> +                                  size_t data_sz,
> +                                  int64_t timestamp)
> +{
> +       struct iio_dev_opaque *iio_dev_opaque = to_iio_dev_opaque(indio_dev);
> +
> +       data_sz = min_t(size_t, indio_dev->scan_bytes, data_sz);
> +       if (iio_dev_opaque->bounce_buffer_size !=  indio_dev->scan_bytes) {

> +               iio_dev_opaque->bounce_buffer =
> +                       devm_krealloc(&indio_dev->dev,
> +                                     iio_dev_opaque->bounce_buffer,

Oh la la, foo = realloc(foo, ...) is 101 type of mistakes.
Please, don't do this.

> +                                     indio_dev->scan_bytes, GFP_KERNEL);
> +               if (!iio_dev_opaque)
> +                       return -ENOMEM;
> +       }
> +       memcpy(iio_dev_opaque->bounce_buffer, data, data_sz);
> +       return iio_push_to_buffers_with_timestamp(indio_dev,
> +                                                 iio_dev_opaque->bounce_buffer,
> +                                                 timestamp);
> +}

-- 
With Best Regards,
Andy Shevchenko