On Fri, 16 Nov 2018 19:31:47 +0100
Lorenzo Bianconi <lorenzo.bianconi@xxxxxxxxxx> wrote:
> > On Sun, 11 Nov 2018 23:28:28 +0100
> > Lorenzo Bianconi <lorenzo.bianconi@xxxxxxxxxx> wrote:
> >
>
> [...]
>
> > > - __le16 data;
> > > + int ret, delay, len = ch->scan_type.realbits >> 3;
> > > + u8 *data;
> > >
> > > - err = st_lsm6dsx_shub_set_enable(sensor, true);
> > > - if (err < 0)
> > > - return err;
> > > + ret = st_lsm6dsx_shub_set_enable(sensor, true);
> > > + if (ret < 0)
> > > + return ret;
> > > +
> > > + data = kmalloc(len, GFP_KERNEL);
> > > + if (!data)
> > > + return -ENOMEM;
> >
> > Do we not want to disable should this fail?
> > Easier might be to just do the allocation before the set_enable
> > call.
> >
> > I'd also be tempted to just put a big enough buffer on the stack, or
> > into the sensor structure.
> >
>
> ack, right. I will fix it in v2 using a u8 data[4] on the stack.
> I guess it is reasonable, agree?
Can always be expanded later if we need it!
Thanks,
Jonathan
>
> Regards,
> Lorenzo
>
> > Also, I haven't checked but is there potential for DMA into this
> > buffer?
> >
> >
> > >
> > > delay = 1000000 / sensor->odr;
> > > usleep_range(delay, 2 * delay);
> > >
> > > - err = st_lsm6dsx_shub_read(sensor, ch->address, (u8 *)&data, len);
> > > - if (err < 0)
> > > - return err;
> > > + ret = st_lsm6dsx_shub_read(sensor, ch->address, data, len);
> > > + if (ret < 0)
> > > + goto out;
> > >
> > > st_lsm6dsx_shub_set_enable(sensor, false);
> > >
> > > switch (len) {
> > > case 2:
> > > - *val = (s16)le16_to_cpu(data);
> > > + *val = (s16)le16_to_cpu(*((__le16 *)data));
> > > + ret = IIO_VAL_INT;
> > > break;
> > > default:
> > > - return -EINVAL;
> > > + ret = -EINVAL;
> > > + break;
> > > }
> > >
> > > - return IIO_VAL_INT;
> > > +out:
> > > + kfree(data);
> > > + return ret;
> > > }
> > >
> > > static int
> >
