public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/2] iio: imu: st_lsm6dsx: Decouple sensor ODR from FIFO batch data rate
@ 2025-10-09 17:36 Francesco Lavra
  2025-10-09 17:36 ` [PATCH 1/2] iio: imu: st_lsm6dsx: Fix measurement unit for odr struct member Francesco Lavra
  2025-10-09 17:36 ` [PATCH 2/2] iio: imu: st_lsm6dsx: Decouple sensor ODR from FIFO batch data rate Francesco Lavra
  0 siblings, 2 replies; 20+ messages in thread
From: Francesco Lavra @ 2025-10-09 17:36 UTC (permalink / raw)
  To: Lorenzo Bianconi, Jonathan Cameron, David Lechner, Nuno Sá,
	Andy Shevchenko, linux-iio, linux-kernel

The rate at which accelerometer or gyroscope sensor samples are fed
to the hardware FIFO (batch data rate, or BDR) does not have to
coincide with the sensor sampling frequency (output data rate, or
ODR); the only requirement is for the BDR to not be greater than
the ODR. Having a BDR lower than the ODR is useful in cases where
an application requires a high sampling rate for accurate detection
of motion events (e.g. wakeup events), but wants to read sensor
sample values from the device buffer at a lower data rate.
This change set amends the st_lsm6dsx IIO driver to support the above
use case. The first commit is a trivial fix to the source code comment
that indicates what measurement unit is used for the `odr` field of
struct st_lsm6ds_sensor, while the second commit introduces a new `bdr`
field in the same struct to implement the new functionality.

Francesco Lavra (2):
  iio: imu: st_lsm6dsx: Fix measurement unit for odr struct member
  iio: imu: st_lsm6dsx: Decouple sensor ODR from FIFO batch data rate

 drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h       |  4 +-
 .../iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c    | 64 ++++++++++++++++---
 drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c  |  9 ++-
 drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_shub.c  |  4 +-
 4 files changed, 67 insertions(+), 14 deletions(-)

-- 
2.39.5


^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH 1/2] iio: imu: st_lsm6dsx: Fix measurement unit for odr struct member
  2025-10-09 17:36 [PATCH 0/2] iio: imu: st_lsm6dsx: Decouple sensor ODR from FIFO batch data rate Francesco Lavra
@ 2025-10-09 17:36 ` Francesco Lavra
  2025-10-09 20:40   ` Lorenzo Bianconi
  2025-10-09 17:36 ` [PATCH 2/2] iio: imu: st_lsm6dsx: Decouple sensor ODR from FIFO batch data rate Francesco Lavra
  1 sibling, 1 reply; 20+ messages in thread
From: Francesco Lavra @ 2025-10-09 17:36 UTC (permalink / raw)
  To: Lorenzo Bianconi, Jonathan Cameron, David Lechner, Nuno Sá,
	Andy Shevchenko, linux-iio, linux-kernel

The `odr` field in struct st_lsm6dsx_sensor contains a data rate
value expressed in mHz, not in Hz.

Signed-off-by: Francesco Lavra <flavra@baylibre.com>
---
 drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h
index c225b246c8a5..bd366c6e282a 100644
--- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h
+++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h
@@ -365,7 +365,7 @@ enum st_lsm6dsx_fifo_mode {
  * @id: Sensor identifier.
  * @hw: Pointer to instance of struct st_lsm6dsx_hw.
  * @gain: Configured sensor sensitivity.
- * @odr: Output data rate of the sensor [Hz].
+ * @odr: Output data rate of the sensor [mHz].
  * @samples_to_discard: Number of samples to discard for filters settling time.
  * @watermark: Sensor watermark level.
  * @decimator: Sensor decimation factor.
-- 
2.39.5


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH 2/2] iio: imu: st_lsm6dsx: Decouple sensor ODR from FIFO batch data rate
  2025-10-09 17:36 [PATCH 0/2] iio: imu: st_lsm6dsx: Decouple sensor ODR from FIFO batch data rate Francesco Lavra
  2025-10-09 17:36 ` [PATCH 1/2] iio: imu: st_lsm6dsx: Fix measurement unit for odr struct member Francesco Lavra
@ 2025-10-09 17:36 ` Francesco Lavra
  2025-10-09 22:30   ` Lorenzo Bianconi
                     ` (2 more replies)
  1 sibling, 3 replies; 20+ messages in thread
From: Francesco Lavra @ 2025-10-09 17:36 UTC (permalink / raw)
  To: Lorenzo Bianconi, Jonathan Cameron, David Lechner, Nuno Sá,
	Andy Shevchenko, linux-iio, linux-kernel

The rate at which accelerometer or gyroscope sensor samples are fed
to the hardware FIFO (batch data rate, or BDR) does not have to
coincide with the sensor sampling frequency (output data rate, or
ODR); the only requirement is for the BDR to not be greater than
the ODR. Having a BDR lower than the ODR is useful in cases where
an application requires a high sampling rate for accurate detection
of motion events (e.g. wakeup events), but wants to read sensor
sample values from the device buffer at a lower data rate.
To support the above use case, add a sampling_frequency sysfs
attribute to the buffer directory of st_lsm6dsx IIO devices, which
controls the BDR for a given sensor independently from the "main"
sampling_frequency attribute (which controls the ODR); introduce a
new `bdr` field in struct st_lsm6dsx_sensor to keep track of the
current BDR value, and use this field instead of the `odr` field in
the code that deals with the FIFO data rate. In the sensor hub
driver, make the bdr value always mirror the odr value, since there
is no separate configuration setting to control the BDR for data
produced by the sensor hub functionality.

Signed-off-by: Francesco Lavra <flavra@baylibre.com>
---
 drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h       |  2 +
 .../iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c    | 64 ++++++++++++++++---
 drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c  |  9 ++-
 drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_shub.c  |  4 +-
 4 files changed, 66 insertions(+), 13 deletions(-)

diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h
index bd366c6e282a..dc4aeea3a3b8 100644
--- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h
+++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h
@@ -366,6 +366,7 @@ enum st_lsm6dsx_fifo_mode {
  * @hw: Pointer to instance of struct st_lsm6dsx_hw.
  * @gain: Configured sensor sensitivity.
  * @odr: Output data rate of the sensor [mHz].
+ * @bdr: Batch data rate [mHz]
  * @samples_to_discard: Number of samples to discard for filters settling time.
  * @watermark: Sensor watermark level.
  * @decimator: Sensor decimation factor.
@@ -380,6 +381,7 @@ struct st_lsm6dsx_sensor {
 
 	u32 gain;
 	u32 odr;
+	u32 bdr;
 
 	u16 samples_to_discard;
 	u16 watermark;
diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c
index 8a9d2593576a..5912ea76d493 100644
--- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c
+++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c
@@ -56,6 +56,7 @@
 #include <linux/iio/kfifo_buf.h>
 #include <linux/iio/iio.h>
 #include <linux/iio/buffer.h>
+#include <linux/iio/sysfs.h>
 #include <linux/regmap.h>
 #include <linux/bitfield.h>
 
@@ -105,7 +106,7 @@ static int
 st_lsm6dsx_get_decimator_val(struct st_lsm6dsx_sensor *sensor, u32 max_odr)
 {
 	const int max_size = ARRAY_SIZE(st_lsm6dsx_decimator_table);
-	u32 decimator =  max_odr / sensor->odr;
+	u32 decimator =  max_odr / sensor->bdr;
 	int i;
 
 	if (decimator > 1)
@@ -136,14 +137,14 @@ static void st_lsm6dsx_get_max_min_odr(struct st_lsm6dsx_hw *hw,
 		if (!(hw->enable_mask & BIT(sensor->id)))
 			continue;
 
-		*max_odr = max_t(u32, *max_odr, sensor->odr);
-		*min_odr = min_t(u32, *min_odr, sensor->odr);
+		*max_odr = max_t(u32, *max_odr, sensor->bdr);
+		*min_odr = min_t(u32, *min_odr, sensor->bdr);
 	}
 }
 
 static u8 st_lsm6dsx_get_sip(struct st_lsm6dsx_sensor *sensor, u32 min_odr)
 {
-	u8 sip = sensor->odr / min_odr;
+	u8 sip = sensor->bdr / min_odr;
 
 	return sip > 1 ? round_down(sip, 2) : sip;
 }
@@ -231,7 +232,7 @@ static int st_lsm6dsx_set_fifo_odr(struct st_lsm6dsx_sensor *sensor,
 		if (enable) {
 			int err;
 
-			err = st_lsm6dsx_check_odr(sensor, sensor->odr,
+			err = st_lsm6dsx_check_odr(sensor, sensor->bdr,
 						   &data);
 			if (err < 0)
 				return err;
@@ -713,7 +714,7 @@ st_lsm6dsx_update_samples_to_discard(struct st_lsm6dsx_sensor *sensor)
 
 	data = &hw->settings->samples_to_discard[sensor->id];
 	for (i = 0; i < ST_LSM6DSX_ODR_LIST_SIZE; i++) {
-		if (data->val[i].milli_hz == sensor->odr) {
+		if (data->val[i].milli_hz == sensor->bdr) {
 			sensor->samples_to_discard = data->val[i].samples;
 			return;
 		}
@@ -799,6 +800,52 @@ static const struct iio_buffer_setup_ops st_lsm6dsx_buffer_ops = {
 	.postdisable = st_lsm6dsx_buffer_postdisable,
 };
 
+static ssize_t st_lsm6dsx_bdr_show(struct device *dev,
+				   struct device_attribute *attr, char *buf)
+{
+	struct st_lsm6dsx_sensor *sensor = iio_priv(dev_to_iio_dev(dev));
+	u32 bdr = sensor->bdr;
+
+	return sysfs_emit(buf, "%d.%03d\n", bdr / 1000, bdr % 1000);
+}
+
+static ssize_t st_lsm6dsx_bdr_store(struct device *dev,
+				    struct device_attribute *attr,
+				    const char *buf, size_t len)
+{
+	struct iio_dev *iio_dev = dev_to_iio_dev(dev);
+	struct st_lsm6dsx_sensor *sensor = iio_priv(iio_dev);
+	int integer, fract;
+	int ret;
+	u32 bdr;
+	u8 data;
+
+	ret = iio_str_to_fixpoint(buf, 100, &integer, &fract);
+	if (ret)
+		return ret;
+	bdr = integer * 1000 + fract;
+	ret = st_lsm6dsx_check_odr(sensor, bdr, &data);
+	if (ret < 0)
+		return ret;
+	bdr = ret;
+	if (!iio_device_claim_direct(iio_dev))
+		return -EBUSY;
+	/* the batch data rate must not exceed the sensor output data rate */
+	if (bdr <= sensor->odr)
+		sensor->bdr = bdr;
+	else
+		ret = -EINVAL;
+	iio_device_release_direct(iio_dev);
+	return (ret < 0) ? ret : len;
+}
+
+static IIO_DEV_ATTR_SAMP_FREQ(0664, st_lsm6dsx_bdr_show, st_lsm6dsx_bdr_store);
+
+static const struct iio_dev_attr *st_lsm6dsx_buffer_attrs[] = {
+	&iio_dev_attr_sampling_frequency,
+	NULL
+};
+
 int st_lsm6dsx_fifo_setup(struct st_lsm6dsx_hw *hw)
 {
 	int i, ret;
@@ -807,8 +854,9 @@ int st_lsm6dsx_fifo_setup(struct st_lsm6dsx_hw *hw)
 		if (!hw->iio_devs[i])
 			continue;
 
-		ret = devm_iio_kfifo_buffer_setup(hw->dev, hw->iio_devs[i],
-						  &st_lsm6dsx_buffer_ops);
+		ret = devm_iio_kfifo_buffer_setup_ext(hw->dev, hw->iio_devs[i],
+						      &st_lsm6dsx_buffer_ops,
+						      st_lsm6dsx_buffer_attrs);
 		if (ret)
 			return ret;
 	}
diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
index c65ad49829e7..e4922578329e 100644
--- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
+++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
@@ -1847,10 +1847,13 @@ static int st_lsm6dsx_write_raw(struct iio_dev *iio_dev,
 
 		val = val * 1000 + val2 / 1000;
 		val = st_lsm6dsx_check_odr(sensor, val, &data);
-		if (val < 0)
+		if (val < 0) {
 			err = val;
-		else
+		} else {
 			sensor->odr = val;
+			/* the batch data rate must not exceed the sensor ODR */
+			sensor->bdr = min_t(u32, sensor->bdr, sensor->odr);
+		}
 		break;
 	}
 	default:
@@ -2383,7 +2386,7 @@ static struct iio_dev *st_lsm6dsx_alloc_iiodev(struct st_lsm6dsx_hw *hw,
 	sensor = iio_priv(iio_dev);
 	sensor->id = id;
 	sensor->hw = hw;
-	sensor->odr = hw->settings->odr_table[id].odr_avl[0].milli_hz;
+	sensor->odr = sensor->bdr = hw->settings->odr_table[id].odr_avl[0].milli_hz;
 	sensor->gain = hw->settings->fs_table[id].fs_avl[0].gain;
 	sensor->watermark = 1;
 
diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_shub.c b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_shub.c
index 3c5e65dc0f97..01d73002e888 100644
--- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_shub.c
+++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_shub.c
@@ -639,7 +639,7 @@ __st_lsm6dsx_shub_write_raw(struct iio_dev *iio_dev,
 			return odr;
 
 		sensor->ext_info.slv_odr = val;
-		sensor->odr = odr;
+		sensor->odr = sensor->bdr = odr;
 		return 0;
 	}
 	case IIO_CHAN_INFO_SCALE:
@@ -745,7 +745,7 @@ st_lsm6dsx_shub_alloc_iiodev(struct st_lsm6dsx_hw *hw,
 	sensor = iio_priv(iio_dev);
 	sensor->id = id;
 	sensor->hw = hw;
-	sensor->odr = hw->settings->odr_table[ref_id].odr_avl[0].milli_hz;
+	sensor->odr = sensor->bdr = hw->settings->odr_table[ref_id].odr_avl[0].milli_hz;
 	sensor->ext_info.slv_odr = info->odr_table.odr_avl[0].milli_hz;
 	sensor->gain = info->fs_table.fs_avl[0].gain;
 	sensor->ext_info.settings = info;
-- 
2.39.5


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* Re: [PATCH 1/2] iio: imu: st_lsm6dsx: Fix measurement unit for odr struct member
  2025-10-09 17:36 ` [PATCH 1/2] iio: imu: st_lsm6dsx: Fix measurement unit for odr struct member Francesco Lavra
@ 2025-10-09 20:40   ` Lorenzo Bianconi
  0 siblings, 0 replies; 20+ messages in thread
From: Lorenzo Bianconi @ 2025-10-09 20:40 UTC (permalink / raw)
  To: Francesco Lavra
  Cc: Jonathan Cameron, David Lechner, Nuno Sá, Andy Shevchenko,
	linux-iio, linux-kernel

[-- Attachment #1: Type: text/plain, Size: 1063 bytes --]

> The `odr` field in struct st_lsm6dsx_sensor contains a data rate
> value expressed in mHz, not in Hz.

ack, can you please the proper Fixes tag?

Regards,
Lorenzo

> 
> Signed-off-by: Francesco Lavra <flavra@baylibre.com>
> ---
>  drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h
> index c225b246c8a5..bd366c6e282a 100644
> --- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h
> +++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h
> @@ -365,7 +365,7 @@ enum st_lsm6dsx_fifo_mode {
>   * @id: Sensor identifier.
>   * @hw: Pointer to instance of struct st_lsm6dsx_hw.
>   * @gain: Configured sensor sensitivity.
> - * @odr: Output data rate of the sensor [Hz].
> + * @odr: Output data rate of the sensor [mHz].
>   * @samples_to_discard: Number of samples to discard for filters settling time.
>   * @watermark: Sensor watermark level.
>   * @decimator: Sensor decimation factor.
> -- 
> 2.39.5
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 2/2] iio: imu: st_lsm6dsx: Decouple sensor ODR from FIFO batch data rate
  2025-10-09 17:36 ` [PATCH 2/2] iio: imu: st_lsm6dsx: Decouple sensor ODR from FIFO batch data rate Francesco Lavra
@ 2025-10-09 22:30   ` Lorenzo Bianconi
  2025-10-10  7:12     ` Francesco Lavra
  2025-10-10 14:55   ` Andy Shevchenko
  2025-10-10 17:44   ` Jonathan Cameron
  2 siblings, 1 reply; 20+ messages in thread
From: Lorenzo Bianconi @ 2025-10-09 22:30 UTC (permalink / raw)
  To: Francesco Lavra
  Cc: Jonathan Cameron, David Lechner, Nuno Sá, Andy Shevchenko,
	linux-iio, linux-kernel

[-- Attachment #1: Type: text/plain, Size: 9234 bytes --]

> The rate at which accelerometer or gyroscope sensor samples are fed
> to the hardware FIFO (batch data rate, or BDR) does not have to
> coincide with the sensor sampling frequency (output data rate, or
> ODR); the only requirement is for the BDR to not be greater than
> the ODR. Having a BDR lower than the ODR is useful in cases where
> an application requires a high sampling rate for accurate detection
> of motion events (e.g. wakeup events), but wants to read sensor
> sample values from the device buffer at a lower data rate.

can you please provide more details here? Are you using the hw fifo to read
data? If we configure the hw fifo according to the BDR (even assuming the
watermark is set 1) the hw will generate interrupts according to the BDR
(bdr < odr).

> To support the above use case, add a sampling_frequency sysfs
> attribute to the buffer directory of st_lsm6dsx IIO devices, which
> controls the BDR for a given sensor independently from the "main"
> sampling_frequency attribute (which controls the ODR); introduce a
> new `bdr` field in struct st_lsm6dsx_sensor to keep track of the
> current BDR value, and use this field instead of the `odr` field in
> the code that deals with the FIFO data rate. In the sensor hub
> driver, make the bdr value always mirror the odr value, since there
> is no separate configuration setting to control the BDR for data
> produced by the sensor hub functionality.
> 
> Signed-off-by: Francesco Lavra <flavra@baylibre.com>
> ---
>  drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h       |  2 +
>  .../iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c    | 64 ++++++++++++++++---
>  drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c  |  9 ++-
>  drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_shub.c  |  4 +-
>  4 files changed, 66 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h
> index bd366c6e282a..dc4aeea3a3b8 100644
> --- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h
> +++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h
> @@ -366,6 +366,7 @@ enum st_lsm6dsx_fifo_mode {
>   * @hw: Pointer to instance of struct st_lsm6dsx_hw.
>   * @gain: Configured sensor sensitivity.
>   * @odr: Output data rate of the sensor [mHz].
> + * @bdr: Batch data rate [mHz]
>   * @samples_to_discard: Number of samples to discard for filters settling time.
>   * @watermark: Sensor watermark level.
>   * @decimator: Sensor decimation factor.
> @@ -380,6 +381,7 @@ struct st_lsm6dsx_sensor {
>  
>  	u32 gain;
>  	u32 odr;
> +	u32 bdr;
>  
>  	u16 samples_to_discard;
>  	u16 watermark;
> diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c
> index 8a9d2593576a..5912ea76d493 100644
> --- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c
> +++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c
> @@ -56,6 +56,7 @@
>  #include <linux/iio/kfifo_buf.h>
>  #include <linux/iio/iio.h>
>  #include <linux/iio/buffer.h>
> +#include <linux/iio/sysfs.h>
>  #include <linux/regmap.h>
>  #include <linux/bitfield.h>
>  
> @@ -105,7 +106,7 @@ static int
>  st_lsm6dsx_get_decimator_val(struct st_lsm6dsx_sensor *sensor, u32 max_odr)
>  {
>  	const int max_size = ARRAY_SIZE(st_lsm6dsx_decimator_table);
> -	u32 decimator =  max_odr / sensor->odr;
> +	u32 decimator =  max_odr / sensor->bdr;
>  	int i;
>  
>  	if (decimator > 1)
> @@ -136,14 +137,14 @@ static void st_lsm6dsx_get_max_min_odr(struct st_lsm6dsx_hw *hw,
>  		if (!(hw->enable_mask & BIT(sensor->id)))
>  			continue;
>  
> -		*max_odr = max_t(u32, *max_odr, sensor->odr);
> -		*min_odr = min_t(u32, *min_odr, sensor->odr);
> +		*max_odr = max_t(u32, *max_odr, sensor->bdr);
> +		*min_odr = min_t(u32, *min_odr, sensor->bdr);
>  	}
>  }
>  
>  static u8 st_lsm6dsx_get_sip(struct st_lsm6dsx_sensor *sensor, u32 min_odr)
>  {
> -	u8 sip = sensor->odr / min_odr;
> +	u8 sip = sensor->bdr / min_odr;
>  
>  	return sip > 1 ? round_down(sip, 2) : sip;
>  }
> @@ -231,7 +232,7 @@ static int st_lsm6dsx_set_fifo_odr(struct st_lsm6dsx_sensor *sensor,
>  		if (enable) {
>  			int err;
>  
> -			err = st_lsm6dsx_check_odr(sensor, sensor->odr,
> +			err = st_lsm6dsx_check_odr(sensor, sensor->bdr,
>  						   &data);
>  			if (err < 0)
>  				return err;
> @@ -713,7 +714,7 @@ st_lsm6dsx_update_samples_to_discard(struct st_lsm6dsx_sensor *sensor)
>  
>  	data = &hw->settings->samples_to_discard[sensor->id];
>  	for (i = 0; i < ST_LSM6DSX_ODR_LIST_SIZE; i++) {
> -		if (data->val[i].milli_hz == sensor->odr) {
> +		if (data->val[i].milli_hz == sensor->bdr) {
>  			sensor->samples_to_discard = data->val[i].samples;
>  			return;
>  		}
> @@ -799,6 +800,52 @@ static const struct iio_buffer_setup_ops st_lsm6dsx_buffer_ops = {
>  	.postdisable = st_lsm6dsx_buffer_postdisable,
>  };
>  
> +static ssize_t st_lsm6dsx_bdr_show(struct device *dev,
> +				   struct device_attribute *attr, char *buf)
> +{
> +	struct st_lsm6dsx_sensor *sensor = iio_priv(dev_to_iio_dev(dev));
> +	u32 bdr = sensor->bdr;
> +
> +	return sysfs_emit(buf, "%d.%03d\n", bdr / 1000, bdr % 1000);
> +}
> +
> +static ssize_t st_lsm6dsx_bdr_store(struct device *dev,
> +				    struct device_attribute *attr,
> +				    const char *buf, size_t len)
> +{
> +	struct iio_dev *iio_dev = dev_to_iio_dev(dev);
> +	struct st_lsm6dsx_sensor *sensor = iio_priv(iio_dev);
> +	int integer, fract;
> +	int ret;
> +	u32 bdr;
> +	u8 data;
> +
> +	ret = iio_str_to_fixpoint(buf, 100, &integer, &fract);
> +	if (ret)
> +		return ret;
nit: new line here.

> +	bdr = integer * 1000 + fract;
> +	ret = st_lsm6dsx_check_odr(sensor, bdr, &data);
> +	if (ret < 0)
> +		return ret;

nit: new line here.

> +	bdr = ret;
> +	if (!iio_device_claim_direct(iio_dev))
> +		return -EBUSY;

I guess you can check it at the beginning of the routine.

> +	/* the batch data rate must not exceed the sensor output data rate */
> +	if (bdr <= sensor->odr)
> +		sensor->bdr = bdr;
> +	else
> +		ret = -EINVAL;

nit: new line here.

> +	iio_device_release_direct(iio_dev);

nit: new line here.

> +	return (ret < 0) ? ret : len;

nit: we do not need brackets here.

> +}
> +
> +static IIO_DEV_ATTR_SAMP_FREQ(0664, st_lsm6dsx_bdr_show, st_lsm6dsx_bdr_store);
> +
> +static const struct iio_dev_attr *st_lsm6dsx_buffer_attrs[] = {
> +	&iio_dev_attr_sampling_frequency,
> +	NULL
> +};
> +
>  int st_lsm6dsx_fifo_setup(struct st_lsm6dsx_hw *hw)
>  {
>  	int i, ret;
> @@ -807,8 +854,9 @@ int st_lsm6dsx_fifo_setup(struct st_lsm6dsx_hw *hw)
>  		if (!hw->iio_devs[i])
>  			continue;
>  
> -		ret = devm_iio_kfifo_buffer_setup(hw->dev, hw->iio_devs[i],
> -						  &st_lsm6dsx_buffer_ops);
> +		ret = devm_iio_kfifo_buffer_setup_ext(hw->dev, hw->iio_devs[i],
> +						      &st_lsm6dsx_buffer_ops,
> +						      st_lsm6dsx_buffer_attrs);
>  		if (ret)
>  			return ret;
>  	}
> diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
> index c65ad49829e7..e4922578329e 100644
> --- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
> +++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
> @@ -1847,10 +1847,13 @@ static int st_lsm6dsx_write_raw(struct iio_dev *iio_dev,
>  
>  		val = val * 1000 + val2 / 1000;
>  		val = st_lsm6dsx_check_odr(sensor, val, &data);
> -		if (val < 0)
> +		if (val < 0) {
>  			err = val;
> -		else
> +		} else {
>  			sensor->odr = val;
> +			/* the batch data rate must not exceed the sensor ODR */
> +			sensor->bdr = min_t(u32, sensor->bdr, sensor->odr);
> +		}
>  		break;
>  	}
>  	default:
> @@ -2383,7 +2386,7 @@ static struct iio_dev *st_lsm6dsx_alloc_iiodev(struct st_lsm6dsx_hw *hw,
>  	sensor = iio_priv(iio_dev);
>  	sensor->id = id;
>  	sensor->hw = hw;
> -	sensor->odr = hw->settings->odr_table[id].odr_avl[0].milli_hz;
> +	sensor->odr = sensor->bdr = hw->settings->odr_table[id].odr_avl[0].milli_hz;

please add a new line to set sensor->bdr here.

>  	sensor->gain = hw->settings->fs_table[id].fs_avl[0].gain;
>  	sensor->watermark = 1;
>  
> diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_shub.c b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_shub.c
> index 3c5e65dc0f97..01d73002e888 100644
> --- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_shub.c
> +++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_shub.c
> @@ -639,7 +639,7 @@ __st_lsm6dsx_shub_write_raw(struct iio_dev *iio_dev,
>  			return odr;
>  
>  		sensor->ext_info.slv_odr = val;
> -		sensor->odr = odr;
> +		sensor->odr = sensor->bdr = odr;
>  		return 0;
>  	}
>  	case IIO_CHAN_INFO_SCALE:
> @@ -745,7 +745,7 @@ st_lsm6dsx_shub_alloc_iiodev(struct st_lsm6dsx_hw *hw,
>  	sensor = iio_priv(iio_dev);
>  	sensor->id = id;
>  	sensor->hw = hw;
> -	sensor->odr = hw->settings->odr_table[ref_id].odr_avl[0].milli_hz;
> +	sensor->odr = sensor->bdr = hw->settings->odr_table[ref_id].odr_avl[0].milli_hz;

please add a new line to set sensor->bdr here.

>  	sensor->ext_info.slv_odr = info->odr_table.odr_avl[0].milli_hz;
>  	sensor->gain = info->fs_table.fs_avl[0].gain;
>  	sensor->ext_info.settings = info;
> -- 
> 2.39.5
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 2/2] iio: imu: st_lsm6dsx: Decouple sensor ODR from FIFO batch data rate
  2025-10-09 22:30   ` Lorenzo Bianconi
@ 2025-10-10  7:12     ` Francesco Lavra
  2025-10-10  8:13       ` Lorenzo Bianconi
  0 siblings, 1 reply; 20+ messages in thread
From: Francesco Lavra @ 2025-10-10  7:12 UTC (permalink / raw)
  To: Lorenzo Bianconi
  Cc: Jonathan Cameron, David Lechner, Nuno Sá, Andy Shevchenko,
	linux-iio, linux-kernel

[-- Attachment #1: Type: text/plain, Size: 1328 bytes --]

On Fri, 2025-10-10 at 00:30 +0200, Lorenzo Bianconi wrote:
> > The rate at which accelerometer or gyroscope sensor samples are fed
> > to the hardware FIFO (batch data rate, or BDR) does not have to
> > coincide with the sensor sampling frequency (output data rate, or
> > ODR); the only requirement is for the BDR to not be greater than
> > the ODR. Having a BDR lower than the ODR is useful in cases where
> > an application requires a high sampling rate for accurate detection
> > of motion events (e.g. wakeup events), but wants to read sensor
> > sample values from the device buffer at a lower data rate.
> 
> can you please provide more details here? Are you using the hw fifo to
> read
> data? If we configure the hw fifo according to the BDR (even assuming the
> watermark is set 1) the hw will generate interrupts according to the BDR
> (bdr < odr).

Yes, I'm using the hw fifo to read data. The use case is to enable event
detection (which works best at high sampling rates) and sensor data
streaming at the same time, without requiring the data stream to be at the
same rate as the sensor sampling rate. So the amount of I2C (or SPI)
traffic (as well as the rate of periodic interrupts) required by the data
stream is kept to a minimum without sacrificing the accuracy of event
detection.



[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 2/2] iio: imu: st_lsm6dsx: Decouple sensor ODR from FIFO batch data rate
  2025-10-10  7:12     ` Francesco Lavra
@ 2025-10-10  8:13       ` Lorenzo Bianconi
  2025-10-10  9:28         ` Francesco Lavra
  0 siblings, 1 reply; 20+ messages in thread
From: Lorenzo Bianconi @ 2025-10-10  8:13 UTC (permalink / raw)
  To: Francesco Lavra
  Cc: Jonathan Cameron, David Lechner, Nuno Sá, Andy Shevchenko,
	linux-iio, linux-kernel

[-- Attachment #1: Type: text/plain, Size: 1562 bytes --]

> On Fri, 2025-10-10 at 00:30 +0200, Lorenzo Bianconi wrote:
> > > The rate at which accelerometer or gyroscope sensor samples are fed
> > > to the hardware FIFO (batch data rate, or BDR) does not have to
> > > coincide with the sensor sampling frequency (output data rate, or
> > > ODR); the only requirement is for the BDR to not be greater than
> > > the ODR. Having a BDR lower than the ODR is useful in cases where
> > > an application requires a high sampling rate for accurate detection
> > > of motion events (e.g. wakeup events), but wants to read sensor
> > > sample values from the device buffer at a lower data rate.
> > 
> > can you please provide more details here? Are you using the hw fifo to
> > read
> > data? If we configure the hw fifo according to the BDR (even assuming the
> > watermark is set 1) the hw will generate interrupts according to the BDR
> > (bdr < odr).
> 
> Yes, I'm using the hw fifo to read data. The use case is to enable event
> detection (which works best at high sampling rates) and sensor data
> streaming at the same time, without requiring the data stream to be at the
> same rate as the sensor sampling rate. So the amount of I2C (or SPI)
> traffic (as well as the rate of periodic interrupts) required by the data
> stream is kept to a minimum without sacrificing the accuracy of event
> detection.

I guess you can get the same result (reduce sensor data interrupt rate
keeping high odr value) configuring the hw fifo watermark.
Does it work for you?

Regards,
Lorenzo

> 
> 



[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 2/2] iio: imu: st_lsm6dsx: Decouple sensor ODR from FIFO batch data rate
  2025-10-10  8:13       ` Lorenzo Bianconi
@ 2025-10-10  9:28         ` Francesco Lavra
  2025-10-10 13:15           ` Lorenzo Bianconi
  0 siblings, 1 reply; 20+ messages in thread
From: Francesco Lavra @ 2025-10-10  9:28 UTC (permalink / raw)
  To: Lorenzo Bianconi
  Cc: Jonathan Cameron, David Lechner, Nuno Sá, Andy Shevchenko,
	linux-iio, linux-kernel

[-- Attachment #1: Type: text/plain, Size: 1852 bytes --]

On Fri, 2025-10-10 at 10:13 +0200, Lorenzo Bianconi wrote:
> > On Fri, 2025-10-10 at 00:30 +0200, Lorenzo Bianconi wrote:
> > > > The rate at which accelerometer or gyroscope sensor samples are fed
> > > > to the hardware FIFO (batch data rate, or BDR) does not have to
> > > > coincide with the sensor sampling frequency (output data rate, or
> > > > ODR); the only requirement is for the BDR to not be greater than
> > > > the ODR. Having a BDR lower than the ODR is useful in cases where
> > > > an application requires a high sampling rate for accurate detection
> > > > of motion events (e.g. wakeup events), but wants to read sensor
> > > > sample values from the device buffer at a lower data rate.
> > > 
> > > can you please provide more details here? Are you using the hw fifo
> > > to
> > > read
> > > data? If we configure the hw fifo according to the BDR (even assuming
> > > the
> > > watermark is set 1) the hw will generate interrupts according to the
> > > BDR
> > > (bdr < odr).
> > 
> > Yes, I'm using the hw fifo to read data. The use case is to enable
> > event
> > detection (which works best at high sampling rates) and sensor data
> > streaming at the same time, without requiring the data stream to be at
> > the
> > same rate as the sensor sampling rate. So the amount of I2C (or SPI)
> > traffic (as well as the rate of periodic interrupts) required by the
> > data
> > stream is kept to a minimum without sacrificing the accuracy of event
> > detection.
> 
> I guess you can get the same result (reduce sensor data interrupt rate
> keeping high odr value) configuring the hw fifo watermark.
> Does it work for you?

Setting the hw fifo watermark to a high value reduces the rate of
interrupts, but doesn't do much to reduce the amount of I2C traffic, so the
issue would still be there.

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 2/2] iio: imu: st_lsm6dsx: Decouple sensor ODR from FIFO batch data rate
  2025-10-10  9:28         ` Francesco Lavra
@ 2025-10-10 13:15           ` Lorenzo Bianconi
  2025-10-10 15:50             ` David Lechner
  0 siblings, 1 reply; 20+ messages in thread
From: Lorenzo Bianconi @ 2025-10-10 13:15 UTC (permalink / raw)
  To: Francesco Lavra
  Cc: Jonathan Cameron, David Lechner, Nuno Sá, Andy Shevchenko,
	linux-iio, linux-kernel

[-- Attachment #1: Type: text/plain, Size: 2159 bytes --]

On Oct 10, Francesco Lavra wrote:
> On Fri, 2025-10-10 at 10:13 +0200, Lorenzo Bianconi wrote:
> > > On Fri, 2025-10-10 at 00:30 +0200, Lorenzo Bianconi wrote:
> > > > > The rate at which accelerometer or gyroscope sensor samples are fed
> > > > > to the hardware FIFO (batch data rate, or BDR) does not have to
> > > > > coincide with the sensor sampling frequency (output data rate, or
> > > > > ODR); the only requirement is for the BDR to not be greater than
> > > > > the ODR. Having a BDR lower than the ODR is useful in cases where
> > > > > an application requires a high sampling rate for accurate detection
> > > > > of motion events (e.g. wakeup events), but wants to read sensor
> > > > > sample values from the device buffer at a lower data rate.
> > > > 
> > > > can you please provide more details here? Are you using the hw fifo
> > > > to
> > > > read
> > > > data? If we configure the hw fifo according to the BDR (even assuming
> > > > the
> > > > watermark is set 1) the hw will generate interrupts according to the
> > > > BDR
> > > > (bdr < odr).
> > > 
> > > Yes, I'm using the hw fifo to read data. The use case is to enable
> > > event
> > > detection (which works best at high sampling rates) and sensor data
> > > streaming at the same time, without requiring the data stream to be at
> > > the
> > > same rate as the sensor sampling rate. So the amount of I2C (or SPI)
> > > traffic (as well as the rate of periodic interrupts) required by the
> > > data
> > > stream is kept to a minimum without sacrificing the accuracy of event
> > > detection.
> > 
> > I guess you can get the same result (reduce sensor data interrupt rate
> > keeping high odr value) configuring the hw fifo watermark.
> > Does it work for you?
> 
> Setting the hw fifo watermark to a high value reduces the rate of
> interrupts, but doesn't do much to reduce the amount of I2C traffic, so the
> issue would still be there.

ack, now I got the goal of the series. I think the series is mostly fine.
I guess hwfifo_odr instead of bdr is more meaningful, what do you think?
Naming is always hard.

Regards,
Lorenzo

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 2/2] iio: imu: st_lsm6dsx: Decouple sensor ODR from FIFO batch data rate
  2025-10-09 17:36 ` [PATCH 2/2] iio: imu: st_lsm6dsx: Decouple sensor ODR from FIFO batch data rate Francesco Lavra
  2025-10-09 22:30   ` Lorenzo Bianconi
@ 2025-10-10 14:55   ` Andy Shevchenko
  2025-10-10 18:44     ` Francesco Lavra
  2025-10-10 17:44   ` Jonathan Cameron
  2 siblings, 1 reply; 20+ messages in thread
From: Andy Shevchenko @ 2025-10-10 14:55 UTC (permalink / raw)
  To: Francesco Lavra
  Cc: Lorenzo Bianconi, Jonathan Cameron, David Lechner, Nuno Sá,
	Andy Shevchenko, linux-iio, linux-kernel

On Thu, Oct 9, 2025 at 8:36 PM Francesco Lavra <flavra@baylibre.com> wrote:
>
> The rate at which accelerometer or gyroscope sensor samples are fed
> to the hardware FIFO (batch data rate, or BDR) does not have to
> coincide with the sensor sampling frequency (output data rate, or
> ODR); the only requirement is for the BDR to not be greater than
> the ODR. Having a BDR lower than the ODR is useful in cases where
> an application requires a high sampling rate for accurate detection
> of motion events (e.g. wakeup events), but wants to read sensor
> sample values from the device buffer at a lower data rate.
> To support the above use case, add a sampling_frequency sysfs
> attribute to the buffer directory of st_lsm6dsx IIO devices, which
> controls the BDR for a given sensor independently from the "main"
> sampling_frequency attribute (which controls the ODR); introduce a
> new `bdr` field in struct st_lsm6dsx_sensor to keep track of the
> current BDR value, and use this field instead of the `odr` field in
> the code that deals with the FIFO data rate. In the sensor hub
> driver, make the bdr value always mirror the odr value, since there
> is no separate configuration setting to control the BDR for data
> produced by the sensor hub functionality.

...

> -               *max_odr = max_t(u32, *max_odr, sensor->odr);
> -               *min_odr = min_t(u32, *min_odr, sensor->odr);
> +               *max_odr = max_t(u32, *max_odr, sensor->bdr);
> +               *min_odr = min_t(u32, *min_odr, sensor->bdr);

Can we get rid of '_t' parts at some point? Or IOW what is the good
justification for typed macros here?

...

> +       ret = iio_str_to_fixpoint(buf, 100, &integer, &fract);
> +       if (ret)
> +               return ret;
> +       bdr = integer * 1000 + fract;

MILLI?

-- 
With Best Regards,
Andy Shevchenko

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 2/2] iio: imu: st_lsm6dsx: Decouple sensor ODR from FIFO batch data rate
  2025-10-10 13:15           ` Lorenzo Bianconi
@ 2025-10-10 15:50             ` David Lechner
  2025-10-10 16:22               ` Lorenzo Bianconi
  0 siblings, 1 reply; 20+ messages in thread
From: David Lechner @ 2025-10-10 15:50 UTC (permalink / raw)
  To: Lorenzo Bianconi, Francesco Lavra
  Cc: Jonathan Cameron, Nuno Sá, Andy Shevchenko, linux-iio,
	linux-kernel

On 10/10/25 8:15 AM, Lorenzo Bianconi wrote:
> On Oct 10, Francesco Lavra wrote:
>> On Fri, 2025-10-10 at 10:13 +0200, Lorenzo Bianconi wrote:
>>>> On Fri, 2025-10-10 at 00:30 +0200, Lorenzo Bianconi wrote:
>>>>>> The rate at which accelerometer or gyroscope sensor samples are fed
>>>>>> to the hardware FIFO (batch data rate, or BDR) does not have to
>>>>>> coincide with the sensor sampling frequency (output data rate, or
>>>>>> ODR); the only requirement is for the BDR to not be greater than
>>>>>> the ODR. Having a BDR lower than the ODR is useful in cases where
>>>>>> an application requires a high sampling rate for accurate detection
>>>>>> of motion events (e.g. wakeup events), but wants to read sensor
>>>>>> sample values from the device buffer at a lower data rate.
>>>>>
>>>>> can you please provide more details here? Are you using the hw fifo
>>>>> to
>>>>> read
>>>>> data? If we configure the hw fifo according to the BDR (even assuming
>>>>> the
>>>>> watermark is set 1) the hw will generate interrupts according to the
>>>>> BDR
>>>>> (bdr < odr).
>>>>
>>>> Yes, I'm using the hw fifo to read data. The use case is to enable
>>>> event
>>>> detection (which works best at high sampling rates) and sensor data
>>>> streaming at the same time, without requiring the data stream to be at
>>>> the
>>>> same rate as the sensor sampling rate. So the amount of I2C (or SPI)
>>>> traffic (as well as the rate of periodic interrupts) required by the
>>>> data
>>>> stream is kept to a minimum without sacrificing the accuracy of event
>>>> detection.
>>>
>>> I guess you can get the same result (reduce sensor data interrupt rate
>>> keeping high odr value) configuring the hw fifo watermark.
>>> Does it work for you?
>>
>> Setting the hw fifo watermark to a high value reduces the rate of
>> interrupts, but doesn't do much to reduce the amount of I2C traffic, so the
>> issue would still be there.
> 
> ack, now I got the goal of the series. I think the series is mostly fine.
> I guess hwfifo_odr instead of bdr is more meaningful, what do you think?
> Naming is always hard.
> 
> Regards,
> Lorenzo

In the IIO subsystem, we prefer to include the units in the variable/
field name as well, e.g. hw_fifo_odr_mHz.

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 2/2] iio: imu: st_lsm6dsx: Decouple sensor ODR from FIFO batch data rate
  2025-10-10 15:50             ` David Lechner
@ 2025-10-10 16:22               ` Lorenzo Bianconi
  2025-10-10 16:23                 ` Andy Shevchenko
  0 siblings, 1 reply; 20+ messages in thread
From: Lorenzo Bianconi @ 2025-10-10 16:22 UTC (permalink / raw)
  To: David Lechner
  Cc: Francesco Lavra, Jonathan Cameron, Nuno Sá, Andy Shevchenko,
	linux-iio, linux-kernel

[-- Attachment #1: Type: text/plain, Size: 2456 bytes --]

On Oct 10, David Lechner wrote:
> On 10/10/25 8:15 AM, Lorenzo Bianconi wrote:
> > On Oct 10, Francesco Lavra wrote:
> >> On Fri, 2025-10-10 at 10:13 +0200, Lorenzo Bianconi wrote:
> >>>> On Fri, 2025-10-10 at 00:30 +0200, Lorenzo Bianconi wrote:
> >>>>>> The rate at which accelerometer or gyroscope sensor samples are fed
> >>>>>> to the hardware FIFO (batch data rate, or BDR) does not have to
> >>>>>> coincide with the sensor sampling frequency (output data rate, or
> >>>>>> ODR); the only requirement is for the BDR to not be greater than
> >>>>>> the ODR. Having a BDR lower than the ODR is useful in cases where
> >>>>>> an application requires a high sampling rate for accurate detection
> >>>>>> of motion events (e.g. wakeup events), but wants to read sensor
> >>>>>> sample values from the device buffer at a lower data rate.
> >>>>>
> >>>>> can you please provide more details here? Are you using the hw fifo
> >>>>> to
> >>>>> read
> >>>>> data? If we configure the hw fifo according to the BDR (even assuming
> >>>>> the
> >>>>> watermark is set 1) the hw will generate interrupts according to the
> >>>>> BDR
> >>>>> (bdr < odr).
> >>>>
> >>>> Yes, I'm using the hw fifo to read data. The use case is to enable
> >>>> event
> >>>> detection (which works best at high sampling rates) and sensor data
> >>>> streaming at the same time, without requiring the data stream to be at
> >>>> the
> >>>> same rate as the sensor sampling rate. So the amount of I2C (or SPI)
> >>>> traffic (as well as the rate of periodic interrupts) required by the
> >>>> data
> >>>> stream is kept to a minimum without sacrificing the accuracy of event
> >>>> detection.
> >>>
> >>> I guess you can get the same result (reduce sensor data interrupt rate
> >>> keeping high odr value) configuring the hw fifo watermark.
> >>> Does it work for you?
> >>
> >> Setting the hw fifo watermark to a high value reduces the rate of
> >> interrupts, but doesn't do much to reduce the amount of I2C traffic, so the
> >> issue would still be there.
> > 
> > ack, now I got the goal of the series. I think the series is mostly fine.
> > I guess hwfifo_odr instead of bdr is more meaningful, what do you think?
> > Naming is always hard.
> > 
> > Regards,
> > Lorenzo
> 
> In the IIO subsystem, we prefer to include the units in the variable/
> field name as well, e.g. hw_fifo_odr_mHz.

ack, but please avoid camel case :)

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 2/2] iio: imu: st_lsm6dsx: Decouple sensor ODR from FIFO batch data rate
  2025-10-10 16:22               ` Lorenzo Bianconi
@ 2025-10-10 16:23                 ` Andy Shevchenko
  2025-10-10 18:35                   ` Francesco Lavra
  0 siblings, 1 reply; 20+ messages in thread
From: Andy Shevchenko @ 2025-10-10 16:23 UTC (permalink / raw)
  To: Lorenzo Bianconi
  Cc: David Lechner, Francesco Lavra, Jonathan Cameron, Nuno Sá,
	Andy Shevchenko, linux-iio, linux-kernel

On Fri, Oct 10, 2025 at 7:22 PM Lorenzo Bianconi <lorenzo@kernel.org> wrote:
> On Oct 10, David Lechner wrote:
> > On 10/10/25 8:15 AM, Lorenzo Bianconi wrote:

...

> > In the IIO subsystem, we prefer to include the units in the variable/
> > field name as well, e.g. hw_fifo_odr_mHz.
>
> ack, but please avoid camel case :)

Don't mix up a camel case with units.
Or do you have a way to distinguish milli from mega, please?

-- 
With Best Regards,
Andy Shevchenko

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 2/2] iio: imu: st_lsm6dsx: Decouple sensor ODR from FIFO batch data rate
  2025-10-09 17:36 ` [PATCH 2/2] iio: imu: st_lsm6dsx: Decouple sensor ODR from FIFO batch data rate Francesco Lavra
  2025-10-09 22:30   ` Lorenzo Bianconi
  2025-10-10 14:55   ` Andy Shevchenko
@ 2025-10-10 17:44   ` Jonathan Cameron
  2025-10-10 18:50     ` Francesco Lavra
  2 siblings, 1 reply; 20+ messages in thread
From: Jonathan Cameron @ 2025-10-10 17:44 UTC (permalink / raw)
  To: Francesco Lavra
  Cc: Lorenzo Bianconi, Jonathan Cameron, David Lechner, Nuno Sá,
	Andy Shevchenko, linux-iio, linux-kernel

On Thu,  9 Oct 2025 19:36:09 +0200
Francesco Lavra <flavra@baylibre.com> wrote:

> The rate at which accelerometer or gyroscope sensor samples are fed
> to the hardware FIFO (batch data rate, or BDR) does not have to
> coincide with the sensor sampling frequency (output data rate, or
> ODR); the only requirement is for the BDR to not be greater than
> the ODR. Having a BDR lower than the ODR is useful in cases where
> an application requires a high sampling rate for accurate detection
> of motion events (e.g. wakeup events), but wants to read sensor
> sample values from the device buffer at a lower data rate.
> To support the above use case, add a sampling_frequency sysfs
> attribute to the buffer directory of st_lsm6dsx IIO devices, which
> controls the BDR for a given sensor independently from the "main"
> sampling_frequency attribute (which controls the ODR); introduce a
> new `bdr` field in struct st_lsm6dsx_sensor to keep track of the
> current BDR value, and use this field instead of the `odr` field in
> the code that deals with the FIFO data rate. In the sensor hub
> driver, make the bdr value always mirror the odr value, since there
> is no separate configuration setting to control the BDR for data
> produced by the sensor hub functionality.
> 
> Signed-off-by: Francesco Lavra <flavra@baylibre.com>

A few additional trivial things from me.  In general this looks fine.
Whilst that buffer/sampling_frequency isn't common it's been part
of the ABI for a while for this sort of thing.

My only slight concern is backwards compatibility. 
Perhaps you can add something on what happens if main sampling_frequency
is modified by a user who doesn't know anything about buffer/sampling_frequency?

Given that's a new interface and the ABI always allows a write to one
value to change any other maybe we have to say the main sampling frequency
write updates the buffer one and a write to the buffer one after that is needed
to set it to a different value?

That is a bit ugly but it is backwards compatible I think.



> diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c
> index 8a9d2593576a..5912ea76d493 100644
> --- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c
> +++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c
> @@ -56,6 +56,7 @@
>  #include <linux/iio/kfifo_buf.h>
>  #include <linux/iio/iio.h>
>  #include <linux/iio/buffer.h>
> +#include <linux/iio/sysfs.h>
>  #include <linux/regmap.h>
>  #include <linux/bitfield.h>
>  
> @@ -105,7 +106,7 @@ static int
>  st_lsm6dsx_get_decimator_val(struct st_lsm6dsx_sensor *sensor, u32 max_odr)
>  {
>  	const int max_size = ARRAY_SIZE(st_lsm6dsx_decimator_table);
> -	u32 decimator =  max_odr / sensor->odr;
> +	u32 decimator =  max_odr / sensor->bdr;

No idea why there is a bonus space after = but good to cleanup whilst you are
here.

>  	int i;

> +static ssize_t st_lsm6dsx_bdr_store(struct device *dev,
> +				    struct device_attribute *attr,
> +				    const char *buf, size_t len)
> +{
> +	struct iio_dev *iio_dev = dev_to_iio_dev(dev);
> +	struct st_lsm6dsx_sensor *sensor = iio_priv(iio_dev);
> +	int integer, fract;
> +	int ret;
> +	u32 bdr;
> +	u8 data;
> +
> +	ret = iio_str_to_fixpoint(buf, 100, &integer, &fract);
> +	if (ret)
> +		return ret;

Add blank line after this sort of error handling return.  Slightly helps
with readability.

> +	bdr = integer * 1000 + fract;
> +	ret = st_lsm6dsx_check_odr(sensor, bdr, &data);
> +	if (ret < 0)
> +		return ret;
Here as well.
> +	bdr = ret;

Probably here as well.

> +	if (!iio_device_claim_direct(iio_dev))
> +		return -EBUSY;
> +	/* the batch data rate must not exceed the sensor output data rate */
> +	if (bdr <= sensor->odr)
> +		sensor->bdr = bdr;
> +	else
> +		ret = -EINVAL;
> +	iio_device_release_direct(iio_dev);
Add one before the final return.
> +	return (ret < 0) ? ret : len;
> +}



^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 2/2] iio: imu: st_lsm6dsx: Decouple sensor ODR from FIFO batch data rate
  2025-10-10 16:23                 ` Andy Shevchenko
@ 2025-10-10 18:35                   ` Francesco Lavra
  0 siblings, 0 replies; 20+ messages in thread
From: Francesco Lavra @ 2025-10-10 18:35 UTC (permalink / raw)
  To: Andy Shevchenko, Lorenzo Bianconi
  Cc: David Lechner, Jonathan Cameron, Nuno Sá, Andy Shevchenko,
	linux-iio, linux-kernel

[-- Attachment #1: Type: text/plain, Size: 563 bytes --]

On Fri, 2025-10-10 at 19:23 +0300, Andy Shevchenko wrote:
> On Fri, Oct 10, 2025 at 7:22 PM Lorenzo Bianconi <lorenzo@kernel.org>
> wrote:
> > On Oct 10, David Lechner wrote:
> > > On 10/10/25 8:15 AM, Lorenzo Bianconi wrote:
> 
> ...
> 
> > > In the IIO subsystem, we prefer to include the units in the variable/
> > > field name as well, e.g. hw_fifo_odr_mHz.
> > 
> > ack, but please avoid camel case :)
> 
> Don't mix up a camel case with units.
> Or do you have a way to distinguish milli from mega, please?

OK, I will use hwfifo_odr_mHz

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 2/2] iio: imu: st_lsm6dsx: Decouple sensor ODR from FIFO batch data rate
  2025-10-10 14:55   ` Andy Shevchenko
@ 2025-10-10 18:44     ` Francesco Lavra
  2025-10-15 14:34       ` Andy Shevchenko
  0 siblings, 1 reply; 20+ messages in thread
From: Francesco Lavra @ 2025-10-10 18:44 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Lorenzo Bianconi, Jonathan Cameron, David Lechner, Nuno Sá,
	Andy Shevchenko, linux-iio, linux-kernel

[-- Attachment #1: Type: text/plain, Size: 2217 bytes --]

On Fri, 2025-10-10 at 17:55 +0300, Andy Shevchenko wrote:
> On Thu, Oct 9, 2025 at 8:36 PM Francesco Lavra <flavra@baylibre.com>
> wrote:
> > 
> > The rate at which accelerometer or gyroscope sensor samples are fed
> > to the hardware FIFO (batch data rate, or BDR) does not have to
> > coincide with the sensor sampling frequency (output data rate, or
> > ODR); the only requirement is for the BDR to not be greater than
> > the ODR. Having a BDR lower than the ODR is useful in cases where
> > an application requires a high sampling rate for accurate detection
> > of motion events (e.g. wakeup events), but wants to read sensor
> > sample values from the device buffer at a lower data rate.
> > To support the above use case, add a sampling_frequency sysfs
> > attribute to the buffer directory of st_lsm6dsx IIO devices, which
> > controls the BDR for a given sensor independently from the "main"
> > sampling_frequency attribute (which controls the ODR); introduce a
> > new `bdr` field in struct st_lsm6dsx_sensor to keep track of the
> > current BDR value, and use this field instead of the `odr` field in
> > the code that deals with the FIFO data rate. In the sensor hub
> > driver, make the bdr value always mirror the odr value, since there
> > is no separate configuration setting to control the BDR for data
> > produced by the sensor hub functionality.
> 
> ...
> 
> > -               *max_odr = max_t(u32, *max_odr, sensor->odr);
> > -               *min_odr = min_t(u32, *min_odr, sensor->odr);
> > +               *max_odr = max_t(u32, *max_odr, sensor->bdr);
> > +               *min_odr = min_t(u32, *min_odr, sensor->bdr);
> 
> Can we get rid of '_t' parts at some point? Or IOW what is the good
> justification for typed macros here?

I think they are not justified here, I will get take this opportunity to
get rid of them.

> ...
> 
> > +       ret = iio_str_to_fixpoint(buf, 100, &integer, &fract);
> > +       if (ret)
> > +               return ret;
> > +       bdr = integer * 1000 + fract;
> 
> MILLI?

If you mean replacing fract with milli, will do.

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 2/2] iio: imu: st_lsm6dsx: Decouple sensor ODR from FIFO batch data rate
  2025-10-10 17:44   ` Jonathan Cameron
@ 2025-10-10 18:50     ` Francesco Lavra
  0 siblings, 0 replies; 20+ messages in thread
From: Francesco Lavra @ 2025-10-10 18:50 UTC (permalink / raw)
  To: Jonathan Cameron
  Cc: Lorenzo Bianconi, Jonathan Cameron, David Lechner, Nuno Sá,
	Andy Shevchenko, linux-iio, linux-kernel

On Fri, 2025-10-10 at 18:44 +0100, Jonathan Cameron wrote:
> On Thu,  9 Oct 2025 19:36:09 +0200
> Francesco Lavra <flavra@baylibre.com> wrote:
> 
> > The rate at which accelerometer or gyroscope sensor samples are fed
> > to the hardware FIFO (batch data rate, or BDR) does not have to
> > coincide with the sensor sampling frequency (output data rate, or
> > ODR); the only requirement is for the BDR to not be greater than
> > the ODR. Having a BDR lower than the ODR is useful in cases where
> > an application requires a high sampling rate for accurate detection
> > of motion events (e.g. wakeup events), but wants to read sensor
> > sample values from the device buffer at a lower data rate.
> > To support the above use case, add a sampling_frequency sysfs
> > attribute to the buffer directory of st_lsm6dsx IIO devices, which
> > controls the BDR for a given sensor independently from the "main"
> > sampling_frequency attribute (which controls the ODR); introduce a
> > new `bdr` field in struct st_lsm6dsx_sensor to keep track of the
> > current BDR value, and use this field instead of the `odr` field in
> > the code that deals with the FIFO data rate. In the sensor hub
> > driver, make the bdr value always mirror the odr value, since there
> > is no separate configuration setting to control the BDR for data
> > produced by the sensor hub functionality.
> > 
> > Signed-off-by: Francesco Lavra <flavra@baylibre.com>
> 
> A few additional trivial things from me.  In general this looks fine.
> Whilst that buffer/sampling_frequency isn't common it's been part
> of the ABI for a while for this sort of thing.
> 
> My only slight concern is backwards compatibility. 
> Perhaps you can add something on what happens if main sampling_frequency
> is modified by a user who doesn't know anything about
> buffer/sampling_frequency?
> 
> Given that's a new interface and the ABI always allows a write to one
> value to change any other maybe we have to say the main sampling
> frequency
> write updates the buffer one and a write to the buffer one after that is
> needed
> to set it to a different value?
> 
> That is a bit ugly but it is backwards compatible I think.

Yes, for backwards compatibility it makes sense to update the buffer
frequency whenever the main frequency is set. Will do.

OK also for the cosmetic changes below.

> > diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c
> > b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c
> > index 8a9d2593576a..5912ea76d493 100644
> > --- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c
> > +++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c
> > @@ -56,6 +56,7 @@
> >  #include <linux/iio/kfifo_buf.h>
> >  #include <linux/iio/iio.h>
> >  #include <linux/iio/buffer.h>
> > +#include <linux/iio/sysfs.h>
> >  #include <linux/regmap.h>
> >  #include <linux/bitfield.h>
> >  
> > @@ -105,7 +106,7 @@ static int
> >  st_lsm6dsx_get_decimator_val(struct st_lsm6dsx_sensor *sensor, u32
> > max_odr)
> >  {
> >         const int max_size = ARRAY_SIZE(st_lsm6dsx_decimator_table);
> > -       u32 decimator =  max_odr / sensor->odr;
> > +       u32 decimator =  max_odr / sensor->bdr;
> 
> No idea why there is a bonus space after = but good to cleanup whilst you
> are
> here.
> 
> >         int i;
> 
> > +static ssize_t st_lsm6dsx_bdr_store(struct device *dev,
> > +                                   struct device_attribute *attr,
> > +                                   const char *buf, size_t len)
> > +{
> > +       struct iio_dev *iio_dev = dev_to_iio_dev(dev);
> > +       struct st_lsm6dsx_sensor *sensor = iio_priv(iio_dev);
> > +       int integer, fract;
> > +       int ret;
> > +       u32 bdr;
> > +       u8 data;
> > +
> > +       ret = iio_str_to_fixpoint(buf, 100, &integer, &fract);
> > +       if (ret)
> > +               return ret;
> 
> Add blank line after this sort of error handling return.  Slightly helps
> with readability.
> 
> > +       bdr = integer * 1000 + fract;
> > +       ret = st_lsm6dsx_check_odr(sensor, bdr, &data);
> > +       if (ret < 0)
> > +               return ret;
> Here as well.
> > +       bdr = ret;
> 
> Probably here as well.
> 
> > +       if (!iio_device_claim_direct(iio_dev))
> > +               return -EBUSY;
> > +       /* the batch data rate must not exceed the sensor output data
> > rate */
> > +       if (bdr <= sensor->odr)
> > +               sensor->bdr = bdr;
> > +       else
> > +               ret = -EINVAL;
> > +       iio_device_release_direct(iio_dev);
> Add one before the final return.
> > +       return (ret < 0) ? ret : len;
> > +}

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 2/2] iio: imu: st_lsm6dsx: Decouple sensor ODR from FIFO batch data rate
  2025-10-10 18:44     ` Francesco Lavra
@ 2025-10-15 14:34       ` Andy Shevchenko
  0 siblings, 0 replies; 20+ messages in thread
From: Andy Shevchenko @ 2025-10-15 14:34 UTC (permalink / raw)
  To: Francesco Lavra
  Cc: Andy Shevchenko, Lorenzo Bianconi, Jonathan Cameron,
	David Lechner, Nuno Sá, Andy Shevchenko, linux-iio,
	linux-kernel

On Fri, Oct 10, 2025 at 08:44:50PM +0200, Francesco Lavra wrote:
> On Fri, 2025-10-10 at 17:55 +0300, Andy Shevchenko wrote:
> > On Thu, Oct 9, 2025 at 8:36 PM Francesco Lavra <flavra@baylibre.com>
> > wrote:

...

> > > +       ret = iio_str_to_fixpoint(buf, 100, &integer, &fract);
> > > +       if (ret)
> > > +               return ret;
> > > +       bdr = integer * 1000 + fract;
> > 
> > MILLI?
> 
> If you mean replacing fract with milli, will do.

I meant 1000 --> MILLI(?) or whatever SI prefix suits better?

-- 
With Best Regards,
Andy Shevchenko



^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH 2/2] iio: imu: st_lsm6dsx: Decouple sensor ODR from FIFO batch data rate
  2025-10-16 17:21 [PATCH v2 0/2] " Francesco Lavra
@ 2025-10-16 17:21 ` Francesco Lavra
  2025-10-16 20:22   ` Lorenzo Bianconi
  0 siblings, 1 reply; 20+ messages in thread
From: Francesco Lavra @ 2025-10-16 17:21 UTC (permalink / raw)
  To: Lorenzo Bianconi, Jonathan Cameron, David Lechner, Nuno Sá,
	Andy Shevchenko, linux-iio, linux-kernel

The rate at which accelerometer or gyroscope sensor samples are fed
to the hardware FIFO (batch data rate, or BDR) does not have to
coincide with the sensor sampling frequency (output data rate, or
ODR); the only requirement is for the BDR to not be greater than
the ODR. Having a BDR lower than the ODR is useful in cases where
an application requires a high sampling rate for accurate detection
of motion events (e.g. wakeup events), but wants to read sensor
sample values from the hardware FIFO at a lower data rate (e.g. to
minimize the amount of I2C or SPI traffic and the rate of periodic
interrupts).
To support the above use case, add a sampling_frequency sysfs
attribute to the buffer directory of st_lsm6dsx IIO devices, which
controls the BDR for a given sensor independently from the "main"
sampling_frequency attribute (which controls the ODR); introduce a
new `hwfifo_odr_mHz` field in struct st_lsm6dsx_sensor to keep
track of the current BDR value, and use this field instead of the
`odr` field in the code that deals with the FIFO data rate. In the
sensor hub driver, make the hwfifo_odr_mHz value always mirror the
odr value, since there is no separate configuration setting to
control the BDR for data produced by the sensor hub functionality.
For backwards compatibility, set the buffer frequency equal to the
main frequency whenever the latter is updated via sysfs; if
userspace wants a different buffer frequency, it has to write to
the relevant sysfs attribute after any writes to the main frequency
attribute.

Signed-off-by: Francesco Lavra <flavra@baylibre.com>
---
 drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h       |  2 +
 .../iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c    | 71 ++++++++++++++++---
 drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c  |  3 +-
 drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_shub.c  |  3 +-
 4 files changed, 69 insertions(+), 10 deletions(-)

diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h
index bd366c6e282a..3cd520bdec46 100644
--- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h
+++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h
@@ -366,6 +366,7 @@ enum st_lsm6dsx_fifo_mode {
  * @hw: Pointer to instance of struct st_lsm6dsx_hw.
  * @gain: Configured sensor sensitivity.
  * @odr: Output data rate of the sensor [mHz].
+ * hwfifo_odr_mHz: Batch data rate for hardware FIFO [mHz]
  * @samples_to_discard: Number of samples to discard for filters settling time.
  * @watermark: Sensor watermark level.
  * @decimator: Sensor decimation factor.
@@ -380,6 +381,7 @@ struct st_lsm6dsx_sensor {
 
 	u32 gain;
 	u32 odr;
+	u32 hwfifo_odr_mHz;
 
 	u16 samples_to_discard;
 	u16 watermark;
diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c
index 8a9d2593576a..7a66f06522c4 100644
--- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c
+++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c
@@ -56,6 +56,7 @@
 #include <linux/iio/kfifo_buf.h>
 #include <linux/iio/iio.h>
 #include <linux/iio/buffer.h>
+#include <linux/iio/sysfs.h>
 #include <linux/regmap.h>
 #include <linux/bitfield.h>
 
@@ -105,7 +106,7 @@ static int
 st_lsm6dsx_get_decimator_val(struct st_lsm6dsx_sensor *sensor, u32 max_odr)
 {
 	const int max_size = ARRAY_SIZE(st_lsm6dsx_decimator_table);
-	u32 decimator =  max_odr / sensor->odr;
+	u32 decimator = max_odr / sensor->hwfifo_odr_mHz;
 	int i;
 
 	if (decimator > 1)
@@ -136,14 +137,14 @@ static void st_lsm6dsx_get_max_min_odr(struct st_lsm6dsx_hw *hw,
 		if (!(hw->enable_mask & BIT(sensor->id)))
 			continue;
 
-		*max_odr = max_t(u32, *max_odr, sensor->odr);
-		*min_odr = min_t(u32, *min_odr, sensor->odr);
+		*max_odr = max(*max_odr, sensor->hwfifo_odr_mHz);
+		*min_odr = min(*min_odr, sensor->hwfifo_odr_mHz);
 	}
 }
 
 static u8 st_lsm6dsx_get_sip(struct st_lsm6dsx_sensor *sensor, u32 min_odr)
 {
-	u8 sip = sensor->odr / min_odr;
+	u8 sip = sensor->hwfifo_odr_mHz / min_odr;
 
 	return sip > 1 ? round_down(sip, 2) : sip;
 }
@@ -231,7 +232,7 @@ static int st_lsm6dsx_set_fifo_odr(struct st_lsm6dsx_sensor *sensor,
 		if (enable) {
 			int err;
 
-			err = st_lsm6dsx_check_odr(sensor, sensor->odr,
+			err = st_lsm6dsx_check_odr(sensor, sensor->hwfifo_odr_mHz,
 						   &data);
 			if (err < 0)
 				return err;
@@ -713,7 +714,7 @@ st_lsm6dsx_update_samples_to_discard(struct st_lsm6dsx_sensor *sensor)
 
 	data = &hw->settings->samples_to_discard[sensor->id];
 	for (i = 0; i < ST_LSM6DSX_ODR_LIST_SIZE; i++) {
-		if (data->val[i].milli_hz == sensor->odr) {
+		if (data->val[i].milli_hz == sensor->hwfifo_odr_mHz) {
 			sensor->samples_to_discard = data->val[i].samples;
 			return;
 		}
@@ -799,6 +800,59 @@ static const struct iio_buffer_setup_ops st_lsm6dsx_buffer_ops = {
 	.postdisable = st_lsm6dsx_buffer_postdisable,
 };
 
+static ssize_t st_lsm6dsx_hwfifo_odr_show(struct device *dev,
+					  struct device_attribute *attr, char *buf)
+{
+	struct st_lsm6dsx_sensor *sensor = iio_priv(dev_to_iio_dev(dev));
+	u32 hwfifo_odr = sensor->hwfifo_odr_mHz;
+
+	return sysfs_emit(buf, "%d.%03d\n", hwfifo_odr / 1000, hwfifo_odr % 1000);
+}
+
+static ssize_t st_lsm6dsx_hwfifo_odr_store(struct device *dev,
+					   struct device_attribute *attr,
+					   const char *buf, size_t len)
+{
+	struct iio_dev *iio_dev = dev_to_iio_dev(dev);
+	struct st_lsm6dsx_sensor *sensor = iio_priv(iio_dev);
+	int integer, milli;
+	int ret;
+	u32 hwfifo_odr;
+	u8 data;
+
+	if (!iio_device_claim_direct(iio_dev))
+		return -EBUSY;
+
+	ret = iio_str_to_fixpoint(buf, 100, &integer, &milli);
+	if (ret)
+		goto out;
+
+	hwfifo_odr = integer * 1000 + milli;
+	ret = st_lsm6dsx_check_odr(sensor, hwfifo_odr, &data);
+	if (ret < 0)
+		goto out;
+
+	hwfifo_odr = ret;
+
+	/* the batch data rate must not exceed the sensor output data rate */
+	if (hwfifo_odr <= sensor->odr)
+		sensor->hwfifo_odr_mHz = hwfifo_odr;
+	else
+		ret = -EINVAL;
+
+out:
+	iio_device_release_direct(iio_dev);
+
+	return ret < 0 ? ret : len;
+}
+
+static IIO_DEV_ATTR_SAMP_FREQ(0664, st_lsm6dsx_hwfifo_odr_show, st_lsm6dsx_hwfifo_odr_store);
+
+static const struct iio_dev_attr *st_lsm6dsx_buffer_attrs[] = {
+	&iio_dev_attr_sampling_frequency,
+	NULL
+};
+
 int st_lsm6dsx_fifo_setup(struct st_lsm6dsx_hw *hw)
 {
 	int i, ret;
@@ -807,8 +861,9 @@ int st_lsm6dsx_fifo_setup(struct st_lsm6dsx_hw *hw)
 		if (!hw->iio_devs[i])
 			continue;
 
-		ret = devm_iio_kfifo_buffer_setup(hw->dev, hw->iio_devs[i],
-						  &st_lsm6dsx_buffer_ops);
+		ret = devm_iio_kfifo_buffer_setup_ext(hw->dev, hw->iio_devs[i],
+						      &st_lsm6dsx_buffer_ops,
+						      st_lsm6dsx_buffer_attrs);
 		if (ret)
 			return ret;
 	}
diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
index c65ad49829e7..055424303a4f 100644
--- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
+++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
@@ -1850,7 +1850,7 @@ static int st_lsm6dsx_write_raw(struct iio_dev *iio_dev,
 		if (val < 0)
 			err = val;
 		else
-			sensor->odr = val;
+			sensor->odr = sensor->hwfifo_odr_mHz = val;
 		break;
 	}
 	default:
@@ -2384,6 +2384,7 @@ static struct iio_dev *st_lsm6dsx_alloc_iiodev(struct st_lsm6dsx_hw *hw,
 	sensor->id = id;
 	sensor->hw = hw;
 	sensor->odr = hw->settings->odr_table[id].odr_avl[0].milli_hz;
+	sensor->hwfifo_odr_mHz = sensor->odr;
 	sensor->gain = hw->settings->fs_table[id].fs_avl[0].gain;
 	sensor->watermark = 1;
 
diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_shub.c b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_shub.c
index 3c5e65dc0f97..039f588ef806 100644
--- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_shub.c
+++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_shub.c
@@ -639,7 +639,7 @@ __st_lsm6dsx_shub_write_raw(struct iio_dev *iio_dev,
 			return odr;
 
 		sensor->ext_info.slv_odr = val;
-		sensor->odr = odr;
+		sensor->odr = sensor->hwfifo_odr_mHz = odr;
 		return 0;
 	}
 	case IIO_CHAN_INFO_SCALE:
@@ -746,6 +746,7 @@ st_lsm6dsx_shub_alloc_iiodev(struct st_lsm6dsx_hw *hw,
 	sensor->id = id;
 	sensor->hw = hw;
 	sensor->odr = hw->settings->odr_table[ref_id].odr_avl[0].milli_hz;
+	sensor->hwfifo_odr_mHz = sensor->odr;
 	sensor->ext_info.slv_odr = info->odr_table.odr_avl[0].milli_hz;
 	sensor->gain = info->fs_table.fs_avl[0].gain;
 	sensor->ext_info.settings = info;
-- 
2.39.5


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* Re: [PATCH 2/2] iio: imu: st_lsm6dsx: Decouple sensor ODR from FIFO batch data rate
  2025-10-16 17:21 ` [PATCH 2/2] " Francesco Lavra
@ 2025-10-16 20:22   ` Lorenzo Bianconi
  0 siblings, 0 replies; 20+ messages in thread
From: Lorenzo Bianconi @ 2025-10-16 20:22 UTC (permalink / raw)
  To: Francesco Lavra
  Cc: Jonathan Cameron, David Lechner, Nuno Sá, Andy Shevchenko,
	linux-iio, linux-kernel

[-- Attachment #1: Type: text/plain, Size: 9288 bytes --]

> The rate at which accelerometer or gyroscope sensor samples are fed
> to the hardware FIFO (batch data rate, or BDR) does not have to
> coincide with the sensor sampling frequency (output data rate, or
> ODR); the only requirement is for the BDR to not be greater than
> the ODR. Having a BDR lower than the ODR is useful in cases where
> an application requires a high sampling rate for accurate detection
> of motion events (e.g. wakeup events), but wants to read sensor
> sample values from the hardware FIFO at a lower data rate (e.g. to
> minimize the amount of I2C or SPI traffic and the rate of periodic
> interrupts).
> To support the above use case, add a sampling_frequency sysfs
> attribute to the buffer directory of st_lsm6dsx IIO devices, which
> controls the BDR for a given sensor independently from the "main"
> sampling_frequency attribute (which controls the ODR); introduce a
> new `hwfifo_odr_mHz` field in struct st_lsm6dsx_sensor to keep
> track of the current BDR value, and use this field instead of the
> `odr` field in the code that deals with the FIFO data rate. In the
> sensor hub driver, make the hwfifo_odr_mHz value always mirror the
> odr value, since there is no separate configuration setting to
> control the BDR for data produced by the sensor hub functionality.
> For backwards compatibility, set the buffer frequency equal to the
> main frequency whenever the latter is updated via sysfs; if
> userspace wants a different buffer frequency, it has to write to
> the relevant sysfs attribute after any writes to the main frequency
> attribute.
> 
> Signed-off-by: Francesco Lavra <flavra@baylibre.com>

Hi Francesco,

I think the patch is fine, just few nits inline. Addressing them:

Acked-by: Lorenzo Bianconi <lorenzo@kernel.org>

> ---
>  drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h       |  2 +
>  .../iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c    | 71 ++++++++++++++++---
>  drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c  |  3 +-
>  drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_shub.c  |  3 +-
>  4 files changed, 69 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h
> index bd366c6e282a..3cd520bdec46 100644
> --- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h
> +++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h
> @@ -366,6 +366,7 @@ enum st_lsm6dsx_fifo_mode {
>   * @hw: Pointer to instance of struct st_lsm6dsx_hw.
>   * @gain: Configured sensor sensitivity.
>   * @odr: Output data rate of the sensor [mHz].
> + * hwfifo_odr_mHz: Batch data rate for hardware FIFO [mHz]
>   * @samples_to_discard: Number of samples to discard for filters settling time.
>   * @watermark: Sensor watermark level.
>   * @decimator: Sensor decimation factor.
> @@ -380,6 +381,7 @@ struct st_lsm6dsx_sensor {
>  
>  	u32 gain;
>  	u32 odr;
> +	u32 hwfifo_odr_mHz;
>  
>  	u16 samples_to_discard;
>  	u16 watermark;
> diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c
> index 8a9d2593576a..7a66f06522c4 100644
> --- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c
> +++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c
> @@ -56,6 +56,7 @@
>  #include <linux/iio/kfifo_buf.h>
>  #include <linux/iio/iio.h>
>  #include <linux/iio/buffer.h>
> +#include <linux/iio/sysfs.h>
>  #include <linux/regmap.h>
>  #include <linux/bitfield.h>
>  
> @@ -105,7 +106,7 @@ static int
>  st_lsm6dsx_get_decimator_val(struct st_lsm6dsx_sensor *sensor, u32 max_odr)
>  {
>  	const int max_size = ARRAY_SIZE(st_lsm6dsx_decimator_table);
> -	u32 decimator =  max_odr / sensor->odr;
> +	u32 decimator = max_odr / sensor->hwfifo_odr_mHz;
>  	int i;
>  
>  	if (decimator > 1)
> @@ -136,14 +137,14 @@ static void st_lsm6dsx_get_max_min_odr(struct st_lsm6dsx_hw *hw,
>  		if (!(hw->enable_mask & BIT(sensor->id)))
>  			continue;
>  
> -		*max_odr = max_t(u32, *max_odr, sensor->odr);
> -		*min_odr = min_t(u32, *min_odr, sensor->odr);
> +		*max_odr = max(*max_odr, sensor->hwfifo_odr_mHz);
> +		*min_odr = min(*min_odr, sensor->hwfifo_odr_mHz);
>  	}
>  }
>  
>  static u8 st_lsm6dsx_get_sip(struct st_lsm6dsx_sensor *sensor, u32 min_odr)
>  {
> -	u8 sip = sensor->odr / min_odr;
> +	u8 sip = sensor->hwfifo_odr_mHz / min_odr;
>  
>  	return sip > 1 ? round_down(sip, 2) : sip;
>  }
> @@ -231,7 +232,7 @@ static int st_lsm6dsx_set_fifo_odr(struct st_lsm6dsx_sensor *sensor,
>  		if (enable) {
>  			int err;
>  
> -			err = st_lsm6dsx_check_odr(sensor, sensor->odr,
> +			err = st_lsm6dsx_check_odr(sensor, sensor->hwfifo_odr_mHz,
>  						   &data);
>  			if (err < 0)
>  				return err;
> @@ -713,7 +714,7 @@ st_lsm6dsx_update_samples_to_discard(struct st_lsm6dsx_sensor *sensor)
>  
>  	data = &hw->settings->samples_to_discard[sensor->id];
>  	for (i = 0; i < ST_LSM6DSX_ODR_LIST_SIZE; i++) {
> -		if (data->val[i].milli_hz == sensor->odr) {
> +		if (data->val[i].milli_hz == sensor->hwfifo_odr_mHz) {
>  			sensor->samples_to_discard = data->val[i].samples;
>  			return;
>  		}
> @@ -799,6 +800,59 @@ static const struct iio_buffer_setup_ops st_lsm6dsx_buffer_ops = {
>  	.postdisable = st_lsm6dsx_buffer_postdisable,
>  };
>  
> +static ssize_t st_lsm6dsx_hwfifo_odr_show(struct device *dev,
> +					  struct device_attribute *attr, char *buf)
> +{
> +	struct st_lsm6dsx_sensor *sensor = iio_priv(dev_to_iio_dev(dev));
> +	u32 hwfifo_odr = sensor->hwfifo_odr_mHz;

nit: you can rid of this hwfifo_odr variable here and use
sensor->hwfifo_odr_mHz directly.

> +
> +	return sysfs_emit(buf, "%d.%03d\n", hwfifo_odr / 1000, hwfifo_odr % 1000);
> +}
> +
> +static ssize_t st_lsm6dsx_hwfifo_odr_store(struct device *dev,
> +					   struct device_attribute *attr,
> +					   const char *buf, size_t len)
> +{
> +	struct iio_dev *iio_dev = dev_to_iio_dev(dev);
> +	struct st_lsm6dsx_sensor *sensor = iio_priv(iio_dev);
> +	int integer, milli;
> +	int ret;
> +	u32 hwfifo_odr;
> +	u8 data;
> +
> +	if (!iio_device_claim_direct(iio_dev))
> +		return -EBUSY;
> +
> +	ret = iio_str_to_fixpoint(buf, 100, &integer, &milli);
> +	if (ret)
> +		goto out;
> +
> +	hwfifo_odr = integer * 1000 + milli;
> +	ret = st_lsm6dsx_check_odr(sensor, hwfifo_odr, &data);
> +	if (ret < 0)
> +		goto out;
> +
> +	hwfifo_odr = ret;
> +
> +	/* the batch data rate must not exceed the sensor output data rate */
> +	if (hwfifo_odr <= sensor->odr)
> +		sensor->hwfifo_odr_mHz = hwfifo_odr;
> +	else
> +		ret = -EINVAL;
> +
> +out:
> +	iio_device_release_direct(iio_dev);
> +
> +	return ret < 0 ? ret : len;
> +}
> +
> +static IIO_DEV_ATTR_SAMP_FREQ(0664, st_lsm6dsx_hwfifo_odr_show, st_lsm6dsx_hwfifo_odr_store);
> +
> +static const struct iio_dev_attr *st_lsm6dsx_buffer_attrs[] = {
> +	&iio_dev_attr_sampling_frequency,
> +	NULL
> +};
> +
>  int st_lsm6dsx_fifo_setup(struct st_lsm6dsx_hw *hw)
>  {
>  	int i, ret;
> @@ -807,8 +861,9 @@ int st_lsm6dsx_fifo_setup(struct st_lsm6dsx_hw *hw)
>  		if (!hw->iio_devs[i])
>  			continue;
>  
> -		ret = devm_iio_kfifo_buffer_setup(hw->dev, hw->iio_devs[i],
> -						  &st_lsm6dsx_buffer_ops);
> +		ret = devm_iio_kfifo_buffer_setup_ext(hw->dev, hw->iio_devs[i],
> +						      &st_lsm6dsx_buffer_ops,
> +						      st_lsm6dsx_buffer_attrs);
>  		if (ret)
>  			return ret;
>  	}
> diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
> index c65ad49829e7..055424303a4f 100644
> --- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
> +++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
> @@ -1850,7 +1850,7 @@ static int st_lsm6dsx_write_raw(struct iio_dev *iio_dev,
>  		if (val < 0)
>  			err = val;
>  		else
> -			sensor->odr = val;
> +			sensor->odr = sensor->hwfifo_odr_mHz = val;

please avoid multi-assignment here.

>  		break;
>  	}
>  	default:
> @@ -2384,6 +2384,7 @@ static struct iio_dev *st_lsm6dsx_alloc_iiodev(struct st_lsm6dsx_hw *hw,
>  	sensor->id = id;
>  	sensor->hw = hw;
>  	sensor->odr = hw->settings->odr_table[id].odr_avl[0].milli_hz;
> +	sensor->hwfifo_odr_mHz = sensor->odr;
>  	sensor->gain = hw->settings->fs_table[id].fs_avl[0].gain;
>  	sensor->watermark = 1;
>  
> diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_shub.c b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_shub.c
> index 3c5e65dc0f97..039f588ef806 100644
> --- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_shub.c
> +++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_shub.c
> @@ -639,7 +639,7 @@ __st_lsm6dsx_shub_write_raw(struct iio_dev *iio_dev,
>  			return odr;
>  
>  		sensor->ext_info.slv_odr = val;
> -		sensor->odr = odr;
> +		sensor->odr = sensor->hwfifo_odr_mHz = odr;

same here, one assignment per line.

>  		return 0;
>  	}
>  	case IIO_CHAN_INFO_SCALE:
> @@ -746,6 +746,7 @@ st_lsm6dsx_shub_alloc_iiodev(struct st_lsm6dsx_hw *hw,
>  	sensor->id = id;
>  	sensor->hw = hw;
>  	sensor->odr = hw->settings->odr_table[ref_id].odr_avl[0].milli_hz;
> +	sensor->hwfifo_odr_mHz = sensor->odr;
>  	sensor->ext_info.slv_odr = info->odr_table.odr_avl[0].milli_hz;
>  	sensor->gain = info->fs_table.fs_avl[0].gain;
>  	sensor->ext_info.settings = info;
> -- 
> 2.39.5
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

^ permalink raw reply	[flat|nested] 20+ messages in thread

end of thread, other threads:[~2025-10-18 17:34 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-10-09 17:36 [PATCH 0/2] iio: imu: st_lsm6dsx: Decouple sensor ODR from FIFO batch data rate Francesco Lavra
2025-10-09 17:36 ` [PATCH 1/2] iio: imu: st_lsm6dsx: Fix measurement unit for odr struct member Francesco Lavra
2025-10-09 20:40   ` Lorenzo Bianconi
2025-10-09 17:36 ` [PATCH 2/2] iio: imu: st_lsm6dsx: Decouple sensor ODR from FIFO batch data rate Francesco Lavra
2025-10-09 22:30   ` Lorenzo Bianconi
2025-10-10  7:12     ` Francesco Lavra
2025-10-10  8:13       ` Lorenzo Bianconi
2025-10-10  9:28         ` Francesco Lavra
2025-10-10 13:15           ` Lorenzo Bianconi
2025-10-10 15:50             ` David Lechner
2025-10-10 16:22               ` Lorenzo Bianconi
2025-10-10 16:23                 ` Andy Shevchenko
2025-10-10 18:35                   ` Francesco Lavra
2025-10-10 14:55   ` Andy Shevchenko
2025-10-10 18:44     ` Francesco Lavra
2025-10-15 14:34       ` Andy Shevchenko
2025-10-10 17:44   ` Jonathan Cameron
2025-10-10 18:50     ` Francesco Lavra
  -- strict thread matches above, loose matches on Subject: below --
2025-10-16 17:21 [PATCH v2 0/2] " Francesco Lavra
2025-10-16 17:21 ` [PATCH 2/2] " Francesco Lavra
2025-10-16 20:22   ` Lorenzo Bianconi

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox