On Sun, May 05, 2024 at 08:34:56PM +0100, Jonathan Cameron wrote:
> On Mon, 29 Apr 2024 21:00:46 +0200
> Vasileios Amoiridis <vassilisamir@xxxxxxxxx> wrote:
>
> > BMP2xx, BME280, BMP3xx, and BMP5xx use continuous buffers for their
> > temperature, pressure and humidity readings. This facilitates the
> > use of burst/bulk reads in order to acquire data faster. The
> > approach is different from the one used in oneshot captures.
> >
> > BMP085 & BMP1xx devices use a completely different measurement
> > process that is well defined and is used in their buffer_handler().
> >
> > Suggested-by: Angel Iglesias <ang.iglesiasg@xxxxxxxxx>
> > Signed-off-by: Vasileios Amoiridis <vassilisamir@xxxxxxxxx>
> Hi Vasileois,
>
> Just one question on this inline. (patches 8 and 9 look good to me)
>
> For v6, only need to send the patches that I haven't already applied.
>
> Thanks,
>
> Jonathan
>
> >
> > +static irqreturn_t bmp180_buffer_handler(int irq, void *p)
> > +{
> > + struct iio_poll_func *pf = p;
> > + struct iio_dev *indio_dev = pf->indio_dev;
> > + struct bmp280_data *data = iio_priv(indio_dev);
> > + int ret, chan_value;
> > +
> > + guard(mutex)(&data->lock);
> > +
> > + ret = bmp180_read_temp(data, &chan_value);
> > + if (ret < 0)
> > + return IRQ_HANDLED;
> > +
> > + data->sensor_data[1] = chan_value;
> > +
> > + ret = bmp180_read_press(data, &chan_value);
>
> So I 'think' that after all the refactoring you end up reading the temperature
> twice. To avoid that you need to pull the read_temp() and read_press()
> function implementations here and only do the (currently duplicated) steps once.
>
> You seem to have done this for the other case, but missed the bmp180?
> Maybe I'm missing some reason it doesn't work for this one!
>
Hi Jonathan!
So, I didn't miss it. This is an old sensor and in order to get data out, the
procedure is much more constrained. As you can see in the datasheet [1] in page
11 there is a well defined process on how to read the data out. It's not
possible to make a burst read here. Hence, the strange bmp180_measure() function
in order to wait for an EOC before reading the values. Indeed I am reading the
temperature 2 times which is not optimal but in order to read both of them I
would have to:
a) either get the temperature out of the bmp180_read_press() function
(which would ruin a bit consistency with the other bmpxxx_read_press() functions)
b) make a bmp180_get_sensor_data() which would look like bmp180_get_press() but
also gives temperature (which won't look that good).
That's why I didn't touch it. If you think it makes more sense to do it, I can
follow one of the 2 approaches, whichever you think would make more sense.
Cheers,
Vasilis
[1]: https://cdn-shop.adafruit.com/datasheets/BST-BMP180-DS000-09.pdf
> > + if (ret < 0)
> > + return IRQ_HANDLED;
> > +
> > + data->sensor_data[0] = chan_value;
> > +
> > + iio_push_to_buffers_with_timestamp(indio_dev, &data->sensor_data,
> > + iio_get_time_ns(indio_dev));
> > +
> > + iio_trigger_notify_done(indio_dev->trig);
> > +
> > + return IRQ_HANDLED;
> > +}
