* [PATCH] iio: imu: st_lsm6dsx: do not use a fixed read len in read_oneshot [not found] <cover.1541975146.git.lorenzo.bianconi@redhat.com> @ 2018-11-11 22:28 ` Lorenzo Bianconi 2018-11-16 18:21 ` Jonathan Cameron 0 siblings, 1 reply; 4+ messages in thread From: Lorenzo Bianconi @ 2018-11-11 22:28 UTC (permalink / raw) To: jic23; +Cc: linux-iio Generalize st_lsm6dsx_shub_read_oneshot in order to not use a fixed read length and take into account iio channel realbits for single read operations Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi@redhat.com> --- drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_shub.c | 31 +++++++++++++------- 1 file changed, 20 insertions(+), 11 deletions(-) diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_shub.c b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_shub.c index ee59b0cac84f..b908df0380b3 100644 --- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_shub.c +++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_shub.c @@ -27,6 +27,7 @@ #include <linux/iio/iio.h> #include <linux/iio/sysfs.h> #include <linux/bitfield.h> +#include <linux/slab.h> #include "st_lsm6dsx.h" @@ -432,31 +433,39 @@ st_lsm6dsx_shub_read_oneshot(struct st_lsm6dsx_sensor *sensor, struct iio_chan_spec const *ch, int *val) { - int err, delay, len = ch->scan_type.realbits >> 3; - __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; 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 -- 2.19.1 ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] iio: imu: st_lsm6dsx: do not use a fixed read len in read_oneshot 2018-11-11 22:28 ` [PATCH] iio: imu: st_lsm6dsx: do not use a fixed read len in read_oneshot Lorenzo Bianconi @ 2018-11-16 18:21 ` Jonathan Cameron 2018-11-16 18:31 ` Lorenzo Bianconi 0 siblings, 1 reply; 4+ messages in thread From: Jonathan Cameron @ 2018-11-16 18:21 UTC (permalink / raw) To: Lorenzo Bianconi; +Cc: linux-iio On Sun, 11 Nov 2018 23:28:28 +0100 Lorenzo Bianconi <lorenzo.bianconi@redhat.com> wrote: > Generalize st_lsm6dsx_shub_read_oneshot in order to not use a fixed > read length and take into account iio channel realbits for single > read operations > > Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi@redhat.com> > --- > drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_shub.c | 31 +++++++++++++------- > 1 file changed, 20 insertions(+), 11 deletions(-) > > diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_shub.c b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_shub.c > index ee59b0cac84f..b908df0380b3 100644 > --- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_shub.c > +++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_shub.c > @@ -27,6 +27,7 @@ > #include <linux/iio/iio.h> > #include <linux/iio/sysfs.h> > #include <linux/bitfield.h> > +#include <linux/slab.h> > > #include "st_lsm6dsx.h" > > @@ -432,31 +433,39 @@ st_lsm6dsx_shub_read_oneshot(struct st_lsm6dsx_sensor *sensor, > struct iio_chan_spec const *ch, > int *val) > { > - int err, delay, len = ch->scan_type.realbits >> 3; > - __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. 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 ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] iio: imu: st_lsm6dsx: do not use a fixed read len in read_oneshot 2018-11-16 18:21 ` Jonathan Cameron @ 2018-11-16 18:31 ` Lorenzo Bianconi 2018-11-17 16:27 ` Jonathan Cameron 0 siblings, 1 reply; 4+ messages in thread From: Lorenzo Bianconi @ 2018-11-16 18:31 UTC (permalink / raw) To: Jonathan Cameron; +Cc: linux-iio > On Sun, 11 Nov 2018 23:28:28 +0100 > Lorenzo Bianconi <lorenzo.bianconi@redhat.com> 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 > ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] iio: imu: st_lsm6dsx: do not use a fixed read len in read_oneshot 2018-11-16 18:31 ` Lorenzo Bianconi @ 2018-11-17 16:27 ` Jonathan Cameron 0 siblings, 0 replies; 4+ messages in thread From: Jonathan Cameron @ 2018-11-17 16:27 UTC (permalink / raw) To: Lorenzo Bianconi; +Cc: linux-iio On Fri, 16 Nov 2018 19:31:47 +0100 Lorenzo Bianconi <lorenzo.bianconi@redhat.com> wrote: > > On Sun, 11 Nov 2018 23:28:28 +0100 > > Lorenzo Bianconi <lorenzo.bianconi@redhat.com> 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 > > ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2018-11-18 2:45 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <cover.1541975146.git.lorenzo.bianconi@redhat.com>
2018-11-11 22:28 ` [PATCH] iio: imu: st_lsm6dsx: do not use a fixed read len in read_oneshot Lorenzo Bianconi
2018-11-16 18:21 ` Jonathan Cameron
2018-11-16 18:31 ` Lorenzo Bianconi
2018-11-17 16:27 ` Jonathan Cameron
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).