[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20240623172330.0f94cae9@jic23-huawei>
Date: Sun, 23 Jun 2024 17:23:30 +0100
From: Jonathan Cameron <jic23@...nel.org>
To: Vasileios Amoiridis <vassilisamir@...il.com>, ak@...klinger.de,
phil@...pberrypi.com
Cc: lars@...afoo.de, andriy.shevchenko@...ux.intel.com,
ang.iglesiasg@...il.com, mazziesaccount@...il.com,
petre.rodan@...dimension.ro, 579lpy@...il.com, linus.walleij@...aro.org,
semen.protsenko@...aro.org, linux-iio@...r.kernel.org,
linux-kernel@...r.kernel.org, Adam Rizkalla <ajarizzo@...il.com>
Subject: Re: [PATCH v8 1/3] iio: pressure: bmp280: Generalize read_*()
functions
On Sat, 22 Jun 2024 14:19:18 +0200
Vasileios Amoiridis <vassilisamir@...il.com> wrote:
> On Sat, Jun 22, 2024 at 10:28:26AM +0100, Jonathan Cameron wrote:
> > On Tue, 18 Jun 2024 01:05:38 +0200
> > Vasileios Amoiridis <vassilisamir@...il.com> wrote:
> >
> > > Add the coefficients for the IIO standard units and the IIO value
> > > inside the chip_info structure.
> > >
> > > Move the calculations for the IIO unit compatibility from inside the
> > > read_{temp,press,humid}() functions and move them to the general
> > > read_raw() function.
> > >
> > > In this way, all the data for the calculation of the value are
> > > located in the chip_info structure of the respective sensor.
> > >
> > > Signed-off-by: Vasileios Amoiridis <vassilisamir@...il.com>
> > Does this incorporate the fix? I'm a little confused looking at
> > what is visible here, so I'd like Adam to take a look.
> >
> > Btw, you missed cc'ing Adam.
> >
>
> Ah, I only used the output of get_maintainer...
always be careful to sanity check that :)
> ...
>
> > > @@ -518,11 +511,29 @@ static int bmp280_read_raw_impl(struct iio_dev *indio_dev,
> > > case IIO_CHAN_INFO_PROCESSED:
> > > switch (chan->type) {
> > > case IIO_HUMIDITYRELATIVE:
> > > - return data->chip_info->read_humid(data, val, val2);
> > > + ret = data->chip_info->read_humid(data, &chan_value);
> > > + if (ret)
> > > + return ret;
> > > +
> > > + *val = data->chip_info->humid_coeffs[0] * chan_value;
> > > + *val2 = data->chip_info->humid_coeffs[1];
> > > + return data->chip_info->humid_coeffs_type;
> > > case IIO_PRESSURE:
> > > - return data->chip_info->read_press(data, val, val2);
> > > + ret = data->chip_info->read_press(data, &chan_value);
> > > + if (ret)
> > > + return ret;
> > > +
> > > + *val = data->chip_info->press_coeffs[0] * chan_value;
> > > + *val2 = data->chip_info->press_coeffs[1];
> > > + return data->chip_info->press_coeffs_type;
> > > case IIO_TEMP:
> > > - return data->chip_info->read_temp(data, val, val2);
> > > + ret = data->chip_info->read_temp(data, &chan_value);
> > > + if (ret)
> > > + return ret;
> > > +
> > > + *val = data->chip_info->temp_coeffs[0] * (s64)chan_value;
>
> This is the first difference with the previous version where I incorporated
> the typecasting to (s64).
On a 32 bit platform that will then get pushed into a 32 bit int and overflow
I think. Back when IIO got started everything was 32 bit so it didn't make sense
to make these 64 bit or indeed to worry about forcing the size.
Jonathan
Powered by blists - more mailing lists