Re: [Patch v6 3/6] IIO: core: Modify scan element type

[Srinivas Pandruvada <srinivas.pandruvada@xxxxxxxxxxxxxxx>]

On 04/25/2014 11:02 AM, Jonathan Cameron wrote:
> On 15/04/14 19:21, Srinivas Pandruvada wrote:
> > The current scan element type uses the following format:
> >    [be|le]:[s|u]bits/storagebits[>>shift].
> > To specify multiple elements in this type, added a repeat value.
> > So new format is:
> >    [be|le]:[s|u]bits/storagebits{X[repeat]}[>>shift].
> > Here X is specifying how may times, real/storage bits are repeating.
> >
> > When X is value is 0 or 1, then repeat value is not used in the format,
> > and it will be same as existing format.
> >
> > Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada@xxxxxxxxxxxxxxx>
> Hi Srinivas,
>
> I'm afraid I've been rather useless in my reviewing of earlier 
> versions of this.
> What I originally meant to suggest was that we'd have descriptions 
> along the lines
> of
> u12/16{X4}>>2 (or u12/16X4>>2 without the brackets) rather than 
> u12/16{4repeat}>>2
I don't mind using u12/16X4>>2, will submit series again.

Thanks,
Srinivas
> Now obviously you've gone a long way with this approach so I guess 
> perhaps people
> actually like the word repeat in there?
>
> Sorry again that I didn't pick up on this in any of the last 4 
> versions you have posted.
> Sometimes the eye sees what it expects to rather than what is there!
>
> I don't particularly mind the version using the word 'repeat' if 
> others like, it simply
> isn't what I thought we had here!
>
> (the rest of this series are fine now btw)
>
> J
> > ---
> >   drivers/iio/industrialio-buffer.c | 41 
> > +++++++++++++++++++++++++++++++++------
> >   include/linux/iio/iio.h           |  7 +++++++
> >   2 files changed, 42 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/iio/industrialio-buffer.c 
> > b/drivers/iio/industrialio-buffer.c
> > index e108f2a..aa90c68 100644
> > --- a/drivers/iio/industrialio-buffer.c
> > +++ b/drivers/iio/industrialio-buffer.c
> > @@ -150,7 +150,16 @@ static ssize_t iio_show_fixed_type(struct device 
> > *dev,
> >           type = IIO_BE;
> >   #endif
> >       }
> > -    return sprintf(buf, "%s:%c%d/%d>>%u\n",
> > +    if (this_attr->c->scan_type.repeat > 1)
> > +        return sprintf(buf, "%s:%c%d/%d{%d[repeat]}>>%u\n",
> > +               iio_endian_prefix[type],
> > +               this_attr->c->scan_type.sign,
> > +               this_attr->c->scan_type.realbits,
> > +               this_attr->c->scan_type.storagebits,
> > +               this_attr->c->scan_type.repeat,
> > +               this_attr->c->scan_type.shift);
> > +        else
> > +            return sprintf(buf, "%s:%c%d/%d>>%u\n",
> >                  iio_endian_prefix[type],
> >                  this_attr->c->scan_type.sign,
> >                  this_attr->c->scan_type.realbits,
> > @@ -474,14 +483,22 @@ static int iio_compute_scan_bytes(struct 
> > iio_dev *indio_dev,
> >       for_each_set_bit(i, mask,
> >                indio_dev->masklength) {
> >           ch = iio_find_channel_from_si(indio_dev, i);
> > -        length = ch->scan_type.storagebits / 8;
> > +        if (ch->scan_type.repeat > 1)
> > +            length = ch->scan_type.storagebits / 8 *
> > +                ch->scan_type.repeat;
> > +        else
> > +            length = ch->scan_type.storagebits / 8;
> >           bytes = ALIGN(bytes, length);
> >           bytes += length;
> >       }
> >       if (timestamp) {
> >           ch = iio_find_channel_from_si(indio_dev,
> >                             indio_dev->scan_index_timestamp);
> > -        length = ch->scan_type.storagebits / 8;
> > +        if (ch->scan_type.repeat > 1)
> > +            length = ch->scan_type.storagebits / 8 *
> > +                ch->scan_type.repeat;
> > +        else
> > +            length = ch->scan_type.storagebits / 8;
> >           bytes = ALIGN(bytes, length);
> >           bytes += length;
> >       }
> > @@ -957,7 +974,11 @@ static int iio_buffer_update_demux(struct 
> > iio_dev *indio_dev,
> >                              indio_dev->masklength,
> >                              in_ind + 1);
> >               ch = iio_find_channel_from_si(indio_dev, in_ind);
> > -            length = ch->scan_type.storagebits/8;
> > +            if (ch->scan_type.repeat > 1)
> > +                length = ch->scan_type.storagebits / 8 *
> > +                    ch->scan_type.repeat;
> > +            else
> > +                length = ch->scan_type.storagebits / 8;
> >               /* Make sure we are aligned */
> >               in_loc += length;
> >               if (in_loc % length)
> > @@ -969,7 +990,11 @@ static int iio_buffer_update_demux(struct 
> > iio_dev *indio_dev,
> >               goto error_clear_mux_table;
> >           }
> >           ch = iio_find_channel_from_si(indio_dev, in_ind);
> > -        length = ch->scan_type.storagebits/8;
> > +        if (ch->scan_type.repeat > 1)
> > +            length = ch->scan_type.storagebits / 8 *
> > +                ch->scan_type.repeat;
> > +        else
> > +            length = ch->scan_type.storagebits / 8;
> >           if (out_loc % length)
> >               out_loc += length - out_loc % length;
> >           if (in_loc % length)
> > @@ -990,7 +1015,11 @@ static int iio_buffer_update_demux(struct 
> > iio_dev *indio_dev,
> >           }
> >           ch = iio_find_channel_from_si(indio_dev,
> >               indio_dev->scan_index_timestamp);
> > -        length = ch->scan_type.storagebits/8;
> > +        if (ch->scan_type.repeat > 1)
> > +            length = ch->scan_type.storagebits / 8 *
> > +                ch->scan_type.repeat;
> > +        else
> > +            length = ch->scan_type.storagebits / 8;
> >           if (out_loc % length)
> >               out_loc += length - out_loc % length;
> >           if (in_loc % length)
> > diff --git a/include/linux/iio/iio.h b/include/linux/iio/iio.h
> > index 5629c92..ccde917 100644
> > --- a/include/linux/iio/iio.h
> > +++ b/include/linux/iio/iio.h
> > @@ -177,6 +177,12 @@ struct iio_event_spec {
> >    *            shift:        Shift right by this before masking out
> >    *                    realbits.
> >    *            endianness:    little or big endian
> > + *            repeat:        Number of times real/storage bits
> > + *                    repeats. When the repeat element is
> > + *                    more than 1, then the type element in
> > + *                    sysfs will show a repeat value.
> > + *                    Otherwise, the number of repetitions is
> > + *                    omitted.
> >    * @info_mask_separate: What information is to be exported that is 
> > specific to
> >    *            this channel.
> >    * @info_mask_shared_by_type: What information is to be exported 
> > that is shared
> > @@ -219,6 +225,7 @@ struct iio_chan_spec {
> >           u8    realbits;
> >           u8    storagebits;
> >           u8    shift;
> > +        u8    repeat;
> >           enum iio_endian endianness;
> >       } scan_type;
> >       long            info_mask_separate;

--
To unsubscribe from this list: send the line "unsubscribe linux-iio" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html