* [PATCH v3 0/2] pressure: bmp280: Improve pushing of data to buffer
@ 2024-09-30 20:23 Vasileios Amoiridis
2024-09-30 20:23 ` [PATCH v3 1/2] iio: pressure: bmp280: Use unsigned type for raw values Vasileios Amoiridis
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Vasileios Amoiridis @ 2024-09-30 20:23 UTC (permalink / raw)
To: jic23; +Cc: dan.carpenter, linux-iio, linux-kernel, Vasileios Amoiridis
Changes in v3:
[PATCH v3 1/2]:
- Remove fixes tag
[PATCH v3 2/2]:
- Use internal s32 *chans variable to better visualize what is
taking place in the data->sensor_data.
- Use proper size/alignment to the data->sensor_data.
P.S. After this patchseries is applied, I will rebase this [1] and resend it.
[1]: https://lore.kernel.org/linux-iio/20240914002900.45158-1-vassilisamir@gmail.com/
---
v2: https://lore.kernel.org/linux-iio/20240929112511.100292-1-vassilisamir@gmail.com/
v1: https://lore.kernel.org/linux-iio/20240823172017.9028-1-vassilisamir@gmail.com/
Vasileios Amoiridis (2):
iio: pressure: bmp280: Use unsigned type for raw values
iio: pressure: bmp280: Use char instead of s32 for data buffer
drivers/iio/pressure/bmp280-core.c | 69 +++++++++++++++++++-----------
drivers/iio/pressure/bmp280.h | 4 +-
2 files changed, 46 insertions(+), 27 deletions(-)
--
2.25.1
^ permalink raw reply [flat|nested] 6+ messages in thread* [PATCH v3 1/2] iio: pressure: bmp280: Use unsigned type for raw values 2024-09-30 20:23 [PATCH v3 0/2] pressure: bmp280: Improve pushing of data to buffer Vasileios Amoiridis @ 2024-09-30 20:23 ` Vasileios Amoiridis 2024-09-30 20:23 ` [PATCH v3 2/2] iio: pressure: bmp280: Use char instead of s32 for data buffer Vasileios Amoiridis 2024-10-06 14:05 ` [PATCH v3 0/2] pressure: bmp280: Improve pushing of data to buffer Jonathan Cameron 2 siblings, 0 replies; 6+ messages in thread From: Vasileios Amoiridis @ 2024-09-30 20:23 UTC (permalink / raw) To: jic23; +Cc: dan.carpenter, linux-iio, linux-kernel, Vasileios Amoiridis The adc values coming directly from the sensor in the BM{E,P}{2,3}xx sensors are unsigned values so treat them as such. Signed-off-by: Vasileios Amoiridis <vassilisamir@gmail.com> --- drivers/iio/pressure/bmp280-core.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/drivers/iio/pressure/bmp280-core.c b/drivers/iio/pressure/bmp280-core.c index 6c2606f34ec4..472a6696303b 100644 --- a/drivers/iio/pressure/bmp280-core.c +++ b/drivers/iio/pressure/bmp280-core.c @@ -1023,7 +1023,8 @@ static irqreturn_t bmp280_trigger_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, t_fine; + u32 adc_temp, adc_press; + s32 t_fine; int ret; guard(mutex)(&data->lock); @@ -1137,7 +1138,8 @@ static irqreturn_t bme280_trigger_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, adc_humidity, t_fine; + u32 adc_temp, adc_press, adc_humidity; + s32 t_fine; int ret; guard(mutex)(&data->lock); @@ -1616,7 +1618,8 @@ static irqreturn_t bmp380_trigger_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, t_fine; + u32 adc_temp, adc_press; + s32 t_fine; int ret; guard(mutex)(&data->lock); -- 2.25.1 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH v3 2/2] iio: pressure: bmp280: Use char instead of s32 for data buffer 2024-09-30 20:23 [PATCH v3 0/2] pressure: bmp280: Improve pushing of data to buffer Vasileios Amoiridis 2024-09-30 20:23 ` [PATCH v3 1/2] iio: pressure: bmp280: Use unsigned type for raw values Vasileios Amoiridis @ 2024-09-30 20:23 ` Vasileios Amoiridis 2024-10-11 18:59 ` Kees Bakker 2024-10-06 14:05 ` [PATCH v3 0/2] pressure: bmp280: Improve pushing of data to buffer Jonathan Cameron 2 siblings, 1 reply; 6+ messages in thread From: Vasileios Amoiridis @ 2024-09-30 20:23 UTC (permalink / raw) To: jic23; +Cc: dan.carpenter, linux-iio, linux-kernel, Vasileios Amoiridis As it was reported and discussed here [1], storing the sensor data in an endian aware s32 buffer is not optimal. Advertising the timestamp as an addition of 2 s32 variables which is also implied is again not the best practice. For that reason, change the s32 sensor_data buffer to a u8 buffer and align it properly. [1]: https://lore.kernel.org/linux-iio/73d13cc0-afb9-4306-b498-5d821728c3ba@stanley.mountain/ Signed-off-by: Vasileios Amoiridis <vassilisamir@gmail.com> --- drivers/iio/pressure/bmp280-core.c | 72 ++++++++++++++++++------------ drivers/iio/pressure/bmp280.h | 4 +- 2 files changed, 46 insertions(+), 30 deletions(-) diff --git a/drivers/iio/pressure/bmp280-core.c b/drivers/iio/pressure/bmp280-core.c index 472a6696303b..6811619c6f11 100644 --- a/drivers/iio/pressure/bmp280-core.c +++ b/drivers/iio/pressure/bmp280-core.c @@ -1023,8 +1023,9 @@ static irqreturn_t bmp280_trigger_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); - u32 adc_temp, adc_press; - s32 t_fine; + u32 adc_temp, adc_press, comp_press; + s32 t_fine, comp_temp; + s32 *chans = (s32 *)data->sensor_data; int ret; guard(mutex)(&data->lock); @@ -1044,7 +1045,7 @@ static irqreturn_t bmp280_trigger_handler(int irq, void *p) goto out; } - data->sensor_data[1] = bmp280_compensate_temp(data, adc_temp); + comp_temp = bmp280_compensate_temp(data, adc_temp); /* Pressure calculations */ adc_press = FIELD_GET(BMP280_MEAS_TRIM_MASK, get_unaligned_be24(&data->buf[0])); @@ -1054,10 +1055,12 @@ static irqreturn_t bmp280_trigger_handler(int irq, void *p) } t_fine = bmp280_calc_t_fine(data, adc_temp); + comp_press = bmp280_compensate_press(data, adc_press, t_fine); - data->sensor_data[0] = bmp280_compensate_press(data, adc_press, t_fine); + chans[0] = comp_press; + chans[1] = comp_temp; - iio_push_to_buffers_with_timestamp(indio_dev, &data->sensor_data, + iio_push_to_buffers_with_timestamp(indio_dev, data->sensor_data, iio_get_time_ns(indio_dev)); out: @@ -1138,8 +1141,9 @@ static irqreturn_t bme280_trigger_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); - u32 adc_temp, adc_press, adc_humidity; - s32 t_fine; + u32 adc_temp, adc_press, adc_humidity, comp_press, comp_humidity; + s32 t_fine, comp_temp; + s32 *chans = (s32 *)data->sensor_data; int ret; guard(mutex)(&data->lock); @@ -1159,7 +1163,7 @@ static irqreturn_t bme280_trigger_handler(int irq, void *p) goto out; } - data->sensor_data[1] = bmp280_compensate_temp(data, adc_temp); + comp_temp = bmp280_compensate_temp(data, adc_temp); /* Pressure calculations */ adc_press = FIELD_GET(BMP280_MEAS_TRIM_MASK, get_unaligned_be24(&data->buf[0])); @@ -1169,8 +1173,7 @@ static irqreturn_t bme280_trigger_handler(int irq, void *p) } t_fine = bmp280_calc_t_fine(data, adc_temp); - - data->sensor_data[0] = bmp280_compensate_press(data, adc_press, t_fine); + comp_press = bmp280_compensate_press(data, adc_press, t_fine); /* Humidity calculations */ adc_humidity = get_unaligned_be16(&data->buf[6]); @@ -1179,9 +1182,14 @@ static irqreturn_t bme280_trigger_handler(int irq, void *p) dev_err(data->dev, "reading humidity skipped\n"); goto out; } - data->sensor_data[2] = bme280_compensate_humidity(data, adc_humidity, t_fine); - iio_push_to_buffers_with_timestamp(indio_dev, &data->sensor_data, + comp_humidity = bme280_compensate_humidity(data, adc_humidity, t_fine); + + chans[0] = comp_press; + chans[1] = comp_temp; + chans[2] = comp_humidity; + + iio_push_to_buffers_with_timestamp(indio_dev, data->sensor_data, iio_get_time_ns(indio_dev)); out: @@ -1618,8 +1626,9 @@ static irqreturn_t bmp380_trigger_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); - u32 adc_temp, adc_press; - s32 t_fine; + u32 adc_temp, adc_press, comp_press; + s32 t_fine, comp_temp; + s32 *chans = (s32 *)data->sensor_data; int ret; guard(mutex)(&data->lock); @@ -1639,7 +1648,7 @@ static irqreturn_t bmp380_trigger_handler(int irq, void *p) goto out; } - data->sensor_data[1] = bmp380_compensate_temp(data, adc_temp); + comp_temp = bmp380_compensate_temp(data, adc_temp); /* Pressure calculations */ adc_press = get_unaligned_le24(&data->buf[0]); @@ -1649,10 +1658,12 @@ static irqreturn_t bmp380_trigger_handler(int irq, void *p) } t_fine = bmp380_calc_t_fine(data, adc_temp); + comp_press = bmp380_compensate_press(data, adc_press, t_fine); - data->sensor_data[0] = bmp380_compensate_press(data, adc_press, t_fine); + chans[0] = comp_press; + chans[1] = comp_temp; - iio_push_to_buffers_with_timestamp(indio_dev, &data->sensor_data, + iio_push_to_buffers_with_timestamp(indio_dev, data->sensor_data, iio_get_time_ns(indio_dev)); out: @@ -2199,7 +2210,7 @@ static irqreturn_t bmp580_trigger_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; + int ret, offset; guard(mutex)(&data->lock); @@ -2211,13 +2222,15 @@ static irqreturn_t bmp580_trigger_handler(int irq, void *p) goto out; } - /* Temperature calculations */ - memcpy(&data->sensor_data[1], &data->buf[0], 3); - /* Pressure calculations */ - memcpy(&data->sensor_data[0], &data->buf[3], 3); + memcpy(&data->sensor_data[offset], &data->buf[3], 3); + + offset += sizeof(s32); + + /* Temperature calculations */ + memcpy(&data->sensor_data[offset], &data->buf[0], 3); - iio_push_to_buffers_with_timestamp(indio_dev, &data->sensor_data, + iio_push_to_buffers_with_timestamp(indio_dev, data->sensor_data, iio_get_time_ns(indio_dev)); out: @@ -2523,23 +2536,24 @@ static irqreturn_t bmp180_trigger_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; + int ret, comp_temp, comp_press; + s32 *chans = (s32 *)data->sensor_data; guard(mutex)(&data->lock); - ret = bmp180_read_temp(data, &chan_value); + ret = bmp180_read_temp(data, &comp_temp); if (ret) goto out; - data->sensor_data[1] = chan_value; - ret = bmp180_read_press(data, &chan_value); + ret = bmp180_read_press(data, &comp_press); if (ret) goto out; - data->sensor_data[0] = chan_value; + chans[0] = comp_press; + chans[1] = comp_temp; - iio_push_to_buffers_with_timestamp(indio_dev, &data->sensor_data, + iio_push_to_buffers_with_timestamp(indio_dev, data->sensor_data, iio_get_time_ns(indio_dev)); out: diff --git a/drivers/iio/pressure/bmp280.h b/drivers/iio/pressure/bmp280.h index a9f220c1f77a..dc1bf04cb0b5 100644 --- a/drivers/iio/pressure/bmp280.h +++ b/drivers/iio/pressure/bmp280.h @@ -322,6 +322,7 @@ BMP280_NUM_TEMP_BYTES + \ BME280_NUM_HUMIDITY_BYTES) +#define BME280_NUM_MAX_CHANNELS 3 /* Core exported structs */ static const char *const bmp280_supply_names[] = { @@ -419,7 +420,8 @@ struct bmp280_data { * Data to push to userspace triggered buffer. Up to 3 channels and * s64 timestamp, aligned. */ - s32 sensor_data[6] __aligned(8); + u8 sensor_data[ALIGN(sizeof(s32) * BME280_NUM_MAX_CHANNELS, sizeof(s64)) + + sizeof(s64)] __aligned(sizeof(s64)); /* * DMA (thus cache coherency maintenance) may require the -- 2.25.1 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v3 2/2] iio: pressure: bmp280: Use char instead of s32 for data buffer 2024-09-30 20:23 ` [PATCH v3 2/2] iio: pressure: bmp280: Use char instead of s32 for data buffer Vasileios Amoiridis @ 2024-10-11 18:59 ` Kees Bakker 0 siblings, 0 replies; 6+ messages in thread From: Kees Bakker @ 2024-10-11 18:59 UTC (permalink / raw) To: Vasileios Amoiridis, jic23; +Cc: dan.carpenter, linux-iio, linux-kernel Op 30-09-2024 om 22:23 schreef Vasileios Amoiridis: > As it was reported and discussed here [1], storing the sensor data in an > endian aware s32 buffer is not optimal. Advertising the timestamp as an > addition of 2 s32 variables which is also implied is again not the best > practice. For that reason, change the s32 sensor_data buffer to a u8 > buffer and align it properly. > > [1]: https://lore.kernel.org/linux-iio/73d13cc0-afb9-4306-b498-5d821728c3ba@stanley.mountain/ > > Signed-off-by: Vasileios Amoiridis <vassilisamir@gmail.com> > --- > drivers/iio/pressure/bmp280-core.c | 72 ++++++++++++++++++------------ > drivers/iio/pressure/bmp280.h | 4 +- > 2 files changed, 46 insertions(+), 30 deletions(-) > [...] > @@ -2199,7 +2210,7 @@ static irqreturn_t bmp580_trigger_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; > + int ret, offset; > > guard(mutex)(&data->lock); > > @@ -2211,13 +2222,15 @@ static irqreturn_t bmp580_trigger_handler(int irq, void *p) > goto out; > } > > - /* Temperature calculations */ > - memcpy(&data->sensor_data[1], &data->buf[0], 3); > - > /* Pressure calculations */ > - memcpy(&data->sensor_data[0], &data->buf[3], 3); > + memcpy(&data->sensor_data[offset], &data->buf[3], 3); Variable offset is not initialized. > + > + offset += sizeof(s32); > + > + /* Temperature calculations */ > + memcpy(&data->sensor_data[offset], &data->buf[0], 3); > > - iio_push_to_buffers_with_timestamp(indio_dev, &data->sensor_data, > + iio_push_to_buffers_with_timestamp(indio_dev, data->sensor_data, > iio_get_time_ns(indio_dev)); > > out: > ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v3 0/2] pressure: bmp280: Improve pushing of data to buffer 2024-09-30 20:23 [PATCH v3 0/2] pressure: bmp280: Improve pushing of data to buffer Vasileios Amoiridis 2024-09-30 20:23 ` [PATCH v3 1/2] iio: pressure: bmp280: Use unsigned type for raw values Vasileios Amoiridis 2024-09-30 20:23 ` [PATCH v3 2/2] iio: pressure: bmp280: Use char instead of s32 for data buffer Vasileios Amoiridis @ 2024-10-06 14:05 ` Jonathan Cameron 2024-10-07 19:35 ` Vasileios Amoiridis 2 siblings, 1 reply; 6+ messages in thread From: Jonathan Cameron @ 2024-10-06 14:05 UTC (permalink / raw) To: Vasileios Amoiridis; +Cc: dan.carpenter, linux-iio, linux-kernel On Mon, 30 Sep 2024 22:23:51 +0200 Vasileios Amoiridis <vassilisamir@gmail.com> wrote: > Changes in v3: > > [PATCH v3 1/2]: > - Remove fixes tag > > [PATCH v3 2/2]: > - Use internal s32 *chans variable to better visualize what is > taking place in the data->sensor_data. > - Use proper size/alignment to the data->sensor_data. > > P.S. After this patchseries is applied, I will rebase this [1] and resend it. > > [1]: https://lore.kernel.org/linux-iio/20240914002900.45158-1-vassilisamir@gmail.com/ Applied to the togreg branch of iio.git and pushed out as testing for all the normal boring reasons. Thanks, Jonathan > > --- > v2: https://lore.kernel.org/linux-iio/20240929112511.100292-1-vassilisamir@gmail.com/ > v1: https://lore.kernel.org/linux-iio/20240823172017.9028-1-vassilisamir@gmail.com/ > > Vasileios Amoiridis (2): > iio: pressure: bmp280: Use unsigned type for raw values > iio: pressure: bmp280: Use char instead of s32 for data buffer > > drivers/iio/pressure/bmp280-core.c | 69 +++++++++++++++++++----------- > drivers/iio/pressure/bmp280.h | 4 +- > 2 files changed, 46 insertions(+), 27 deletions(-) > ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v3 0/2] pressure: bmp280: Improve pushing of data to buffer 2024-10-06 14:05 ` [PATCH v3 0/2] pressure: bmp280: Improve pushing of data to buffer Jonathan Cameron @ 2024-10-07 19:35 ` Vasileios Amoiridis 0 siblings, 0 replies; 6+ messages in thread From: Vasileios Amoiridis @ 2024-10-07 19:35 UTC (permalink / raw) To: Jonathan Cameron Cc: Vasileios Amoiridis, dan.carpenter, linux-iio, linux-kernel On Sun, Oct 06, 2024 at 03:05:17PM +0100, Jonathan Cameron wrote: > On Mon, 30 Sep 2024 22:23:51 +0200 > Vasileios Amoiridis <vassilisamir@gmail.com> wrote: > > > Changes in v3: > > > > [PATCH v3 1/2]: > > - Remove fixes tag > > > > [PATCH v3 2/2]: > > - Use internal s32 *chans variable to better visualize what is > > taking place in the data->sensor_data. > > - Use proper size/alignment to the data->sensor_data. > > > > P.S. After this patchseries is applied, I will rebase this [1] and resend it. > > > > [1]: https://lore.kernel.org/linux-iio/20240914002900.45158-1-vassilisamir@gmail.com/ > Applied to the togreg branch of iio.git and pushed out as testing for all the normal > boring reasons. > > Thanks, > Jonathan > Hi Jonathan, thanks a lot! Cheers, Vasilis > > > > --- > > v2: https://lore.kernel.org/linux-iio/20240929112511.100292-1-vassilisamir@gmail.com/ > > v1: https://lore.kernel.org/linux-iio/20240823172017.9028-1-vassilisamir@gmail.com/ > > > > Vasileios Amoiridis (2): > > iio: pressure: bmp280: Use unsigned type for raw values > > iio: pressure: bmp280: Use char instead of s32 for data buffer > > > > drivers/iio/pressure/bmp280-core.c | 69 +++++++++++++++++++----------- > > drivers/iio/pressure/bmp280.h | 4 +- > > 2 files changed, 46 insertions(+), 27 deletions(-) > > > ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2024-10-11 18:59 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2024-09-30 20:23 [PATCH v3 0/2] pressure: bmp280: Improve pushing of data to buffer Vasileios Amoiridis 2024-09-30 20:23 ` [PATCH v3 1/2] iio: pressure: bmp280: Use unsigned type for raw values Vasileios Amoiridis 2024-09-30 20:23 ` [PATCH v3 2/2] iio: pressure: bmp280: Use char instead of s32 for data buffer Vasileios Amoiridis 2024-10-11 18:59 ` Kees Bakker 2024-10-06 14:05 ` [PATCH v3 0/2] pressure: bmp280: Improve pushing of data to buffer Jonathan Cameron 2024-10-07 19:35 ` Vasileios Amoiridis
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).