> 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?
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
>
