On Sun, Jun 23, 2024 at 05:26:15PM +0100, Jonathan Cameron wrote: > On Sat, 22 Jun 2024 16:09:11 +0200 > Vasileios Amoiridis <vassilisamir@xxxxxxxxx> wrote: > > > On Sat, Jun 22, 2024 at 10:40:39AM +0100, Jonathan Cameron wrote: > > > On Tue, 18 Jun 2024 01:05:40 +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> > > > > Link: https://lore.kernel.org/r/20240512230524.53990-6-vassilisamir@xxxxxxxxx > > > > --- > > > The sign extend in buffered path doesn't make much sense as we should be > > > advertising the correct bit depth for the channel and making that a userspace > > > problem. > > > > > > I'd failed to notice you are doing endian conversions just to check > > > the skipped values. Ideally we'd leave the channels little endian > > > and include that in the channel spec. > > > > > > Hmm. I guess this works and if we have to do the endian conversion > > > anyway isn't too bad. It does provide slightly wrong information > > > to userspace though. > > > > > > So even with this in place I think these channels should be real_bits 24. > > > > > > > > > > > > > +static irqreturn_t bmp580_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); > > > > + s32 adc_temp, adc_press; > > > > + int ret; > > > > + > > > > + guard(mutex)(&data->lock); > > > > + > > > > + /* Burst read data registers */ > > > > + ret = regmap_bulk_read(data->regmap, BMP580_REG_TEMP_XLSB, > > > > + data->buf, BMP280_BURST_READ_BYTES); > > > > With the bulk read here, we have 24 bits of temperature and 24 bits > > of pressure, so in total 6 bytes. The only way I can see to not do > > the endian conversion is that I memcpy the first 3 bytes to the > > data->sensor_data[1], and the last 3 bytes to the data->sensor_data[0]. > > > > If you can see any other better way please let me know, otherwise the > > other solution is to use get_unaligned_24(). Still, we won't do the > > skipping part. > > If you return in cpu endian because it's convenient that is fine, but > still set the number of realbits to 24 or whatever is appropriate. > > Or as you say memcpy the 3 bytes. There is probably an arch out there > in which that is much more efficient than the endian fun, but I > can't be bothered to figure out how ;) > > Jonathan Well, thanks for the advice :) Cheers, Vasilis
