On Mon, Mar 23, 2020 at 10:21:07AM +0100, Mike Looijmans wrote:
> On 22-03-2020 01:16, Andy Shevchenko wrote:
> > On Thu, Mar 19, 2020 at 04:48:42PM +0100, Mike Looijmans wrote:
...
> > > +static int bmi088_accel_get_temp(struct bmi088_accel_data *data, int *val)
> > > +{
> > > + int ret;
> > > + __s16 temp;
> > > +
> > > + mutex_lock(&data->mutex);
> >
> > > + ret = regmap_bulk_read(data->regmap, BMI088_ACCEL_REG_TEMP,
> > > + &data->buffer, 2);
> >
> > sizeof() ?
>
> The buffer is a shared buffer, it will grow to accommodate reading all axis
> and timestamp in a single read (9 bytes) and for FIFO reads in foreseeable
> future.
>
> I could use sizeof(temp) here though, but that wouldn't that be more
> confusing?
Yeah, perhaps comment explaining why 2 is being used there and why you write
directly to the buffer (no temporary variable being involved)?
> > > + temp = get_unaligned_be16(data->buffer);
> > > +
> > > + mutex_unlock(&data->mutex);
> > > +
> > > + if (ret < 0)
> > > + return ret;
> > > +
> > > + *val = temp >> 5;
> > > +
> > > + return IIO_VAL_INT;
> > > +}
--
With Best Regards,
Andy Shevchenko
