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
