Re: [Outreachy kernel] [PATCH] iio: imu: bmi160: bmi160_core: Fix sparse warning

[Alison Schofield <amsfield22@gmail.com>]

On Mon, Oct 03, 2016 at 07:07:39PM +0530, sayli karnik wrote:
> On Mon, Oct 3, 2016 at 4:55 AM, Alison Schofield <amsfield22@gmail.com> wrote:
> > On Sun, Oct 02, 2016 at 05:53:08PM +0200, Lars-Peter Clausen wrote:
> >> On 10/02/2016 07:00 AM, Alison Schofield wrote:
> >> [...]
> >> >> ---
> >> >>  drivers/iio/imu/bmi160/bmi160_core.c | 3 ++-
> >> >>  1 file changed, 2 insertions(+), 1 deletion(-)
> >> >>
> >> >> diff --git a/drivers/iio/imu/bmi160/bmi160_core.c b/drivers/iio/imu/bmi160/bmi160_core.c
> >> >> index e0251b8..5355507 100644
> >> >> --- a/drivers/iio/imu/bmi160/bmi160_core.c
> >> >> +++ b/drivers/iio/imu/bmi160/bmi160_core.c
> >> >> @@ -398,7 +398,8 @@ static irqreturn_t bmi160_trigger_handler(int irq, void *p)
> >> >>    struct iio_poll_func *pf = p;
> >> >>    struct iio_dev *indio_dev = pf->indio_dev;
> >> >>    struct bmi160_data *data = iio_priv(indio_dev);
> >> >> -  s16 buf[16]; /* 3 sens x 3 axis x s16 + 3 x s16 pad + 4 x s16 tstamp */
> >> >> +  __le16 buf[16];
> >> >> +  /* 3 sens x 3 axis x __le16 + 3 x __le16 pad + 4 x __le16 tstamp */
> >> >>    int i, ret, j = 0, base = BMI160_REG_DATA_MAGN_XOUT_L;
> >> >>    __le16 sample;
> >> >
> >> > Wondering about this option below.  Data was read into an __le16, so that
> >> > was good diligence on drivers part.  Seems we can use le16_to_cpu() for the
> >> > conversion into the buf.
> >> >
> >> > --- a/drivers/iio/imu/bmi160/bmi160_core.c
> >> > +++ b/drivers/iio/imu/bmi160/bmi160_core.c
> >> > @@ -408,7 +408,7 @@ static irqreturn_t bmi160_trigger_handler(int irq, void *p)
> >> >                                    &sample, sizeof(__le16));
> >> >             if (ret < 0)
> >> >                     goto done;
> >> > -           buf[j++] = sample;
> >> > +           buf[j++] = le16_to_cpu(sample);
> >> >     }
> >>
> >> This conversion is usually skipped on purpose and delayed until it is
> >> actually needed by the user. The IIO channel is accordingly marked that it
> >> will produce LE data.
> > Thanks Lars.  I knew that for buffers, overlooked it, now I'll know it
> > better!
> >
> > So, Sayli, you probably got this from the analysis of the last patch.
> > In buffered mode, we'll go ahead and return the data in it's 'native'
> > order.  So, my suggestion to convert it here, is wrong.  Ignore ;)
> >
> Oh I see! So should I resend the patch with an updated
> description?(cosmetic/bug fix)

Yes.  In the commit message, you can leave out the subdirs (imu: bmc160:)
so that you have more space for a descriptive message of the change.
alisons

> 
> thanks,
> sayli
> 
> > alisons
> >
> >>