Re: [PATCH 3/4] iio: add helper function for reading channel offset in buffer

[Artur Rojek]

On 2022-08-19 10:17, Andy Shevchenko wrote:
> On Wed, Aug 17, 2022 at 1:58 PM Artur Rojek <contact@xxxxxxxxxxxxxx> 
> wrote:
> > This is useful for consumers that wish to parse raw buffer data.
>
> ...
>
> > +int iio_find_channel_offset_in_buffer(struct iio_dev *indio_dev,
> > +                                     const struct iio_chan_spec 
> > *chan,
> > +                                     struct iio_buffer *buffer)
> > +{
> > +       int length, offset = 0;
> > +       unsigned int si;
> > +
> > +       if (chan->scan_index < 0 ||
> > +           !test_bit(chan->scan_index, buffer->scan_mask)) {
> > +               return -EINVAL;
> > +       }
>
> Have you run checkpatch? The {} are redundant. But personally I would
> split this into two separate conditionals.
I did run checkpatch on it - all patches were ready for submission.
I don't find the {} redundant for multi-line statements, like this one,
and I personally prefer to check conditions that return the same error
type together.
> > +       for (si = 0; si < chan->scan_index; ++si) {
>
> Just a side crying: where did you, people, get this pre-increment 
> pattern from?!
>
> > +               if (!test_bit(si, buffer->scan_mask))
> > +                       continue;
>
> NIH for_each_set_bit()
>
> > +               length = iio_storage_bytes_for_si(indio_dev, si);
> > +
> > +               /* Account for channel alignment. */
> > +               if (offset % length)
> > +                       offset += length - (offset % length);
> > +               offset += length;
> > +       }
> > +
> > +       return offset;
> > +}
> > +EXPORT_SYMBOL_GPL(iio_find_channel_offset_in_buffer);
>
> Same Q as per previous patch: IIO namespace?