[PATCH v4 0/2] iio: imu: inv_icm42600: add support of accel low power

[PATCH v4 0/2] iio: imu: inv_icm42600: add support of accel low power

[inv.git-commit]

From: Jean-Baptiste Maneyrol <jean-baptiste.maneyrol@tdk.com>

Add accel low power support, enabling use of lower ODR below 12.5Hz.

Add new sysfs API for controlling the power mode and enforce power mode used
when using an ODR that supports only 1 mode (ODR < 12.5Hz or ODR > 500Hz).

Changelog

V2:
- fix patch missing Signed-off tag

V3:
- make all ODRs available and switch power mode automatically for ODR supporting
  only 1 mode.
 
V4:
- prevent power mode setting when the ODR is not supported
- return the actual power mode when sensor is on, otherwise the requested one.
- add sysfs ABI documentation

Jean-Baptiste Maneyrol (1):
  iio: imu: inv_icm42600: add support of accel low-power mode

 drivers/iio/imu/inv_icm42600/inv_icm42600.h   |   4 +
 .../iio/imu/inv_icm42600/inv_icm42600_accel.c | 124 +++++++++++++++++-
 .../iio/imu/inv_icm42600/inv_icm42600_core.c  |  27 ++++
 3 files changed, 151 insertions(+), 4 deletions(-)

-- 
2.34.1

[PATCH v4 1/2] iio: document inv_icm42600 driver private sysfs attributes

[inv.git-commit]

From: Jean-Baptiste Maneyrol <jean-baptiste.maneyrol@tdk.com>

Add ABI documentation for inv_icm42600 private sysfs attributes.

Signed-off-by: Jean-Baptiste Maneyrol <jean-baptiste.maneyrol@tdk.com>
---
 .../ABI/testing/sysfs-bus-iio-inv_icm42600     | 18 ++++++++++++++++++
 MAINTAINERS                                    |  1 +
 2 files changed, 19 insertions(+)
 create mode 100644 Documentation/ABI/testing/sysfs-bus-iio-inv_icm42600

diff --git a/Documentation/ABI/testing/sysfs-bus-iio-inv_icm42600 b/Documentation/ABI/testing/sysfs-bus-iio-inv_icm42600
new file mode 100644
index 000000000000..7eeacfb7650d
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-bus-iio-inv_icm42600
@@ -0,0 +1,18 @@
+What:		/sys/bus/iio/devices/iio:deviceX/in_accel_power_mode
+KernelVersion:	6.11
+Contact:	linux-iio@vger.kernel.org
+Description:
+		Accelerometer power mode. Setting this attribute will set the
+		requested power mode to use if the ODR support it. If ODR
+		support only 1 mode, power mode will be enforced.
+		Reading this attribute will return the current accelerometer
+		power mode if the sensor is on, or the requested value if the
+		sensor is off. The value between real and requested value can
+		be different for ODR supporting only 1 mode.
+
+What:		/sys/bus/iio/devices/iio:deviceX/in_accel_power_mode_available
+KernelVersion:	6.11
+Contact:	linux-iio@vger.kernel.org
+Description:
+		List of available accelerometer power modes that can be set in
+		in_accel_power_mode attribute.
diff --git a/MAINTAINERS b/MAINTAINERS
index 304429f9bfc6..23526cb6ac7e 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -11336,6 +11336,7 @@ M:	Jean-Baptiste Maneyrol <jmaneyrol@invensense.com>
 L:	linux-iio@vger.kernel.org
 S:	Maintained
 W:	https://invensense.tdk.com/
+F:	Documentation/ABI/testing/sysfs-bus-iio-inv_icm42600
 F:	Documentation/devicetree/bindings/iio/imu/invensense,icm42600.yaml
 F:	drivers/iio/imu/inv_icm42600/
 
-- 
2.34.1

[PATCH v4 2/2] iio: imu: inv_icm42600: add support of accel low-power mode

[inv.git-commit]

From: Jean-Baptiste Maneyrol <jean-baptiste.maneyrol@tdk.com>

Add ODRs accessible only in low-power mode. Switch automatically to
low-power or low-noise depending on the ODR set.

Add channel attributes "power_mode" and "power_mode_available" for
setting the power mode to use (low-noise or low-power) for ODRs
supporting both mode. Reading "power_mode" when the sensor is on
will return the actual mode and not the requested one. It will be
different when using ODRs not supported by the requested mode.

Use low-power mode by default.

Signed-off-by: Jean-Baptiste Maneyrol <jean-baptiste.maneyrol@tdk.com>
---
 drivers/iio/imu/inv_icm42600/inv_icm42600.h   |   4 +
 .../iio/imu/inv_icm42600/inv_icm42600_accel.c | 124 +++++++++++++++++-
 .../iio/imu/inv_icm42600/inv_icm42600_core.c  |  27 ++++
 3 files changed, 151 insertions(+), 4 deletions(-)

diff --git a/drivers/iio/imu/inv_icm42600/inv_icm42600.h b/drivers/iio/imu/inv_icm42600/inv_icm42600.h
index c4ac91f6bafe..3a07e43e4cf1 100644
--- a/drivers/iio/imu/inv_icm42600/inv_icm42600.h
+++ b/drivers/iio/imu/inv_icm42600/inv_icm42600.h
@@ -177,11 +177,15 @@ struct inv_icm42600_state {
  * struct inv_icm42600_sensor_state - sensor state variables
  * @scales:		table of scales.
  * @scales_len:		length (nb of items) of the scales table.
+ * @power_mode:		sensor requested power mode (for common frequencies)
+ * @filter:		sensor filter.
  * @ts:			timestamp module states.
  */
 struct inv_icm42600_sensor_state {
 	const int *scales;
 	size_t scales_len;
+	enum inv_icm42600_sensor_mode power_mode;
+	enum inv_icm42600_filter filter;
 	struct inv_sensors_timestamp ts;
 };
 
diff --git a/drivers/iio/imu/inv_icm42600/inv_icm42600_accel.c b/drivers/iio/imu/inv_icm42600/inv_icm42600_accel.c
index 83d8504ebfff..3927829dd344 100644
--- a/drivers/iio/imu/inv_icm42600/inv_icm42600_accel.c
+++ b/drivers/iio/imu/inv_icm42600/inv_icm42600_accel.c
@@ -55,8 +55,108 @@ enum inv_icm42600_accel_scan {
 	INV_ICM42600_ACCEL_SCAN_TIMESTAMP,
 };
 
+static const char * const inv_icm42600_accel_power_mode_items[] = {
+	"low-noise",
+	"low-power",
+};
+static const int inv_icm42600_accel_power_mode_values[] = {
+	INV_ICM42600_SENSOR_MODE_LOW_NOISE,
+	INV_ICM42600_SENSOR_MODE_LOW_POWER,
+};
+static const int inv_icm42600_accel_filter_values[] = {
+	INV_ICM42600_FILTER_BW_ODR_DIV_2,
+	INV_ICM42600_FILTER_AVG_16X,
+};
+
+static int inv_icm42600_accel_power_mode_set(struct iio_dev *indio_dev,
+					     const struct iio_chan_spec *chan,
+					     unsigned int idx)
+{
+	struct inv_icm42600_state *st = iio_device_get_drvdata(indio_dev);
+	struct inv_icm42600_sensor_state *accel_st = iio_priv(indio_dev);
+	int power_mode, filter;
+
+	if (chan->type != IIO_ACCEL)
+		return -EINVAL;
+
+	if (idx >= ARRAY_SIZE(inv_icm42600_accel_power_mode_values))
+		return -EINVAL;
+
+	if (iio_buffer_enabled(indio_dev))
+		return -EBUSY;
+
+	power_mode = inv_icm42600_accel_power_mode_values[idx];
+	filter = inv_icm42600_accel_filter_values[idx];
+
+	guard(mutex)(&st->lock);
+
+	/* prevent change if power mode is not supported by the ODR */
+	switch (power_mode) {
+	case INV_ICM42600_SENSOR_MODE_LOW_NOISE:
+		if (st->conf.accel.odr >= INV_ICM42600_ODR_6_25HZ_LP &&
+		    st->conf.accel.odr <= INV_ICM42600_ODR_1_5625HZ_LP)
+			return -EPERM;
+		break;
+	case INV_ICM42600_SENSOR_MODE_LOW_POWER:
+	default:
+		if (st->conf.accel.odr <= INV_ICM42600_ODR_1KHZ_LN)
+			return -EPERM;
+		break;
+	}
+
+	accel_st->power_mode = power_mode;
+	accel_st->filter = filter;
+
+	return 0;
+}
+
+static int inv_icm42600_accel_power_mode_get(struct iio_dev *indio_dev,
+					     const struct iio_chan_spec *chan)
+{
+	struct inv_icm42600_state *st = iio_device_get_drvdata(indio_dev);
+	struct inv_icm42600_sensor_state *accel_st = iio_priv(indio_dev);
+	unsigned int idx;
+	int power_mode;
+
+	if (chan->type != IIO_ACCEL)
+		return -EINVAL;
+
+	guard(mutex)(&st->lock);
+
+	/* if sensor is on, returns actual power mode and not configured one */
+	switch (st->conf.accel.mode) {
+	case INV_ICM42600_SENSOR_MODE_LOW_POWER:
+	case INV_ICM42600_SENSOR_MODE_LOW_NOISE:
+		power_mode = st->conf.accel.mode;
+		break;
+	default:
+		power_mode = accel_st->power_mode;
+		break;
+	}
+
+	for (idx = 0; idx < ARRAY_SIZE(inv_icm42600_accel_power_mode_values); ++idx) {
+		if (power_mode == inv_icm42600_accel_power_mode_values[idx])
+			break;
+	}
+	if (idx >= ARRAY_SIZE(inv_icm42600_accel_power_mode_values))
+		return -EINVAL;
+
+	return idx;
+}
+
+static const struct iio_enum inv_icm42600_accel_power_mode_enum = {
+	.items = inv_icm42600_accel_power_mode_items,
+	.num_items = ARRAY_SIZE(inv_icm42600_accel_power_mode_items),
+	.set = inv_icm42600_accel_power_mode_set,
+	.get = inv_icm42600_accel_power_mode_get,
+};
+
 static const struct iio_chan_spec_ext_info inv_icm42600_accel_ext_infos[] = {
 	IIO_MOUNT_MATRIX(IIO_SHARED_BY_ALL, inv_icm42600_get_mount_matrix),
+	IIO_ENUM_AVAILABLE("power_mode", IIO_SHARED_BY_TYPE,
+			   &inv_icm42600_accel_power_mode_enum),
+	IIO_ENUM("power_mode", IIO_SHARED_BY_TYPE,
+		 &inv_icm42600_accel_power_mode_enum),
 	{},
 };
 
@@ -120,7 +220,8 @@ static int inv_icm42600_accel_update_scan_mode(struct iio_dev *indio_dev,
 
 	if (*scan_mask & INV_ICM42600_SCAN_MASK_ACCEL_3AXIS) {
 		/* enable accel sensor */
-		conf.mode = INV_ICM42600_SENSOR_MODE_LOW_NOISE;
+		conf.mode = accel_st->power_mode;
+		conf.filter = accel_st->filter;
 		ret = inv_icm42600_set_accel_conf(st, &conf, &sleep_accel);
 		if (ret)
 			goto out_unlock;
@@ -144,10 +245,12 @@ static int inv_icm42600_accel_update_scan_mode(struct iio_dev *indio_dev,
 	return ret;
 }
 
-static int inv_icm42600_accel_read_sensor(struct inv_icm42600_state *st,
+static int inv_icm42600_accel_read_sensor(struct iio_dev *indio_dev,
 					  struct iio_chan_spec const *chan,
 					  int16_t *val)
 {
+	struct inv_icm42600_state *st = iio_device_get_drvdata(indio_dev);
+	struct inv_icm42600_sensor_state *accel_st = iio_priv(indio_dev);
 	struct device *dev = regmap_get_device(st->map);
 	struct inv_icm42600_sensor_conf conf = INV_ICM42600_SENSOR_CONF_INIT;
 	unsigned int reg;
@@ -175,7 +278,8 @@ static int inv_icm42600_accel_read_sensor(struct inv_icm42600_state *st,
 	mutex_lock(&st->lock);
 
 	/* enable accel sensor */
-	conf.mode = INV_ICM42600_SENSOR_MODE_LOW_NOISE;
+	conf.mode = accel_st->power_mode;
+	conf.filter = accel_st->filter;
 	ret = inv_icm42600_set_accel_conf(st, &conf, NULL);
 	if (ret)
 		goto exit;
@@ -277,6 +381,12 @@ static int inv_icm42600_accel_write_scale(struct iio_dev *indio_dev,
 
 /* IIO format int + micro */
 static const int inv_icm42600_accel_odr[] = {
+	/* 1.5625Hz */
+	1, 562500,
+	/* 3.125Hz */
+	3, 125000,
+	/* 6.25Hz */
+	6, 250000,
 	/* 12.5Hz */
 	12, 500000,
 	/* 25Hz */
@@ -296,6 +406,9 @@ static const int inv_icm42600_accel_odr[] = {
 };
 
 static const int inv_icm42600_accel_odr_conv[] = {
+	INV_ICM42600_ODR_1_5625HZ_LP,
+	INV_ICM42600_ODR_3_125HZ_LP,
+	INV_ICM42600_ODR_6_25HZ_LP,
 	INV_ICM42600_ODR_12_5HZ,
 	INV_ICM42600_ODR_25HZ,
 	INV_ICM42600_ODR_50HZ,
@@ -581,7 +694,7 @@ static int inv_icm42600_accel_read_raw(struct iio_dev *indio_dev,
 		ret = iio_device_claim_direct_mode(indio_dev);
 		if (ret)
 			return ret;
-		ret = inv_icm42600_accel_read_sensor(st, chan, &data);
+		ret = inv_icm42600_accel_read_sensor(indio_dev, chan, &data);
 		iio_device_release_direct_mode(indio_dev);
 		if (ret)
 			return ret;
@@ -754,6 +867,9 @@ struct iio_dev *inv_icm42600_accel_init(struct inv_icm42600_state *st)
 		accel_st->scales_len = ARRAY_SIZE(inv_icm42600_accel_scale);
 		break;
 	}
+	/* low-power by default at init */
+	accel_st->power_mode = INV_ICM42600_SENSOR_MODE_LOW_POWER;
+	accel_st->filter = INV_ICM42600_FILTER_AVG_16X;
 
 	/*
 	 * clock period is 32kHz (31250ns)
diff --git a/drivers/iio/imu/inv_icm42600/inv_icm42600_core.c b/drivers/iio/imu/inv_icm42600/inv_icm42600_core.c
index 96116a68ab29..edf19a8a2f39 100644
--- a/drivers/iio/imu/inv_icm42600/inv_icm42600_core.c
+++ b/drivers/iio/imu/inv_icm42600/inv_icm42600_core.c
@@ -248,6 +248,23 @@ int inv_icm42600_set_accel_conf(struct inv_icm42600_state *st,
 	if (conf->filter < 0)
 		conf->filter = oldconf->filter;
 
+	/* force power mode against ODR when sensor is on */
+	switch (conf->mode) {
+	case INV_ICM42600_SENSOR_MODE_LOW_POWER:
+	case INV_ICM42600_SENSOR_MODE_LOW_NOISE:
+		if (conf->odr <= INV_ICM42600_ODR_1KHZ_LN) {
+			conf->mode = INV_ICM42600_SENSOR_MODE_LOW_NOISE;
+			conf->filter = INV_ICM42600_FILTER_BW_ODR_DIV_2;
+		} else if (conf->odr >= INV_ICM42600_ODR_6_25HZ_LP &&
+			   conf->odr <= INV_ICM42600_ODR_1_5625HZ_LP) {
+			conf->mode = INV_ICM42600_SENSOR_MODE_LOW_POWER;
+			conf->filter = INV_ICM42600_FILTER_AVG_16X;
+		}
+		break;
+	default:
+		break;
+	}
+
 	/* set ACCEL_CONFIG0 register (accel fullscale & odr) */
 	if (conf->fs != oldconf->fs || conf->odr != oldconf->odr) {
 		val = INV_ICM42600_ACCEL_CONFIG0_FS(conf->fs) |
@@ -441,6 +458,16 @@ static int inv_icm42600_setup(struct inv_icm42600_state *st,
 	if (ret)
 		return ret;
 
+	/*
+	 * Use RC clock for accel low-power to fix glitches when switching
+	 * gyro on/off while accel low-power is on.
+	 */
+	ret = regmap_update_bits(st->map, INV_ICM42600_REG_INTF_CONFIG1,
+				 INV_ICM42600_INTF_CONFIG1_ACCEL_LP_CLK_RC,
+				 INV_ICM42600_INTF_CONFIG1_ACCEL_LP_CLK_RC);
+	if (ret)
+		return ret;
+
 	return inv_icm42600_set_conf(st, hw->conf);
 }
 
-- 
2.34.1

Re: [PATCH v4 0/2] iio: imu: inv_icm42600: add support of accel low power

[Jonathan Cameron]

On Wed,  5 Jun 2024 19:59:47 +0000
inv.git-commit@tdk.com wrote:

> From: Jean-Baptiste Maneyrol <jean-baptiste.maneyrol@tdk.com>
> 
> Add accel low power support, enabling use of lower ODR below 12.5Hz.
> 
> Add new sysfs API for controlling the power mode and enforce power mode used
> when using an ODR that supports only 1 mode (ODR < 12.5Hz or ODR > 500Hz).
Applied to the togreg branch of iio.git and pushed out as testing for 0-day to
poke at it.

Thanks,

Jonathan

> 
> Changelog
> 
> V2:
> - fix patch missing Signed-off tag
> 
> V3:
> - make all ODRs available and switch power mode automatically for ODR supporting
>   only 1 mode.
>  
> V4:
> - prevent power mode setting when the ODR is not supported
> - return the actual power mode when sensor is on, otherwise the requested one.
> - add sysfs ABI documentation
> 
> Jean-Baptiste Maneyrol (1):
>   iio: imu: inv_icm42600: add support of accel low-power mode
> 
>  drivers/iio/imu/inv_icm42600/inv_icm42600.h   |   4 +
>  .../iio/imu/inv_icm42600/inv_icm42600_accel.c | 124 +++++++++++++++++-
>  .../iio/imu/inv_icm42600/inv_icm42600_core.c  |  27 ++++
>  3 files changed, 151 insertions(+), 4 deletions(-)
> 

Re: [PATCH v4 0/2] iio: imu: inv_icm42600: add support of accel low power

[Jean-Baptiste Maneyrol]

Hi Jonathan,

I'm sorry I am not able to find this patch in togreg branch currently.

Am I not looking at the correct place or there is a problem somewhere?

Thanks,
JB

________________________________________
From: Jonathan Cameron <jic23@kernel.org>
Sent: Sunday, June 9, 2024 12:09
To: INV Git Commit <INV.git-commit@tdk.com>
Cc: lars@metafoo.de <lars@metafoo.de>; linux-iio@vger.kernel.org <linux-iio@vger.kernel.org>; Jean-Baptiste Maneyrol <Jean-Baptiste.Maneyrol@tdk.com>
Subject: Re: [PATCH v4 0/2] iio: imu: inv_icm42600: add support of accel low power
 
This Message Is From an External Sender
This message came from outside your organization.
 
On Wed,  5 Jun 2024 19:59:47 +0000
inv.git-commit@tdk.com wrote:

> From: Jean-Baptiste Maneyrol <jean-baptiste.maneyrol@tdk.com>
> 
> Add accel low power support, enabling use of lower ODR below 12.5Hz.
> 
> Add new sysfs API for controlling the power mode and enforce power mode used
> when using an ODR that supports only 1 mode (ODR < 12.5Hz or ODR > 500Hz).
Applied to the togreg branch of iio.git and pushed out as testing for 0-day to
poke at it.

Thanks,

Jonathan

> 
> Changelog
> 
> V2:
> - fix patch missing Signed-off tag
> 
> V3:
> - make all ODRs available and switch power mode automatically for ODR supporting
>   only 1 mode.
>  
> V4:
> - prevent power mode setting when the ODR is not supported
> - return the actual power mode when sensor is on, otherwise the requested one.
> - add sysfs ABI documentation
> 
> Jean-Baptiste Maneyrol (1):
>   iio: imu: inv_icm42600: add support of accel low-power mode
> 
>  drivers/iio/imu/inv_icm42600/inv_icm42600.h   |   4 +
>  .../iio/imu/inv_icm42600/inv_icm42600_accel.c | 124 +++++++++++++++++-
>  .../iio/imu/inv_icm42600/inv_icm42600_core.c  |  27 ++++
>  3 files changed, 151 insertions(+), 4 deletions(-)
> 

Re: [PATCH v4 0/2] iio: imu: inv_icm42600: add support of accel low power

[Jonathan Cameron]

On Wed, 12 Jun 2024 12:28:50 +0000
Jean-Baptiste Maneyrol <Jean-Baptiste.Maneyrol@tdk.com> wrote:

> Hi Jonathan,
> 
> I'm sorry I am not able to find this patch in togreg branch currently.
> 
> Am I not looking at the correct place or there is a problem somewhere?
As so often happens I was being dozy at end of a reviewing session
and failed to push out.
Done now.

> 
> Thanks,
> JB
> 
> ________________________________________
> From: Jonathan Cameron <jic23@kernel.org>
> Sent: Sunday, June 9, 2024 12:09
> To: INV Git Commit <INV.git-commit@tdk.com>
> Cc: lars@metafoo.de <lars@metafoo.de>; linux-iio@vger.kernel.org <linux-iio@vger.kernel.org>; Jean-Baptiste Maneyrol <Jean-Baptiste.Maneyrol@tdk.com>
> Subject: Re: [PATCH v4 0/2] iio: imu: inv_icm42600: add support of accel low power
>  
> This Message Is From an External Sender
> This message came from outside your organization.
>  
> On Wed,  5 Jun 2024 19:59:47 +0000
> inv.git-commit@tdk.com wrote:
> 
> > From: Jean-Baptiste Maneyrol <jean-baptiste.maneyrol@tdk.com>
> > 
> > Add accel low power support, enabling use of lower ODR below 12.5Hz.
> > 
> > Add new sysfs API for controlling the power mode and enforce power mode used
> > when using an ODR that supports only 1 mode (ODR < 12.5Hz or ODR > 500Hz).  
> Applied to the togreg branch of iio.git and pushed out as testing for 0-day to
> poke at it.
> 
> Thanks,
> 
> Jonathan
> 
> > 
> > Changelog
> > 
> > V2:
> > - fix patch missing Signed-off tag
> > 
> > V3:
> > - make all ODRs available and switch power mode automatically for ODR supporting
> >   only 1 mode.
> >  
> > V4:
> > - prevent power mode setting when the ODR is not supported
> > - return the actual power mode when sensor is on, otherwise the requested one.
> > - add sysfs ABI documentation
> > 
> > Jean-Baptiste Maneyrol (1):
> >   iio: imu: inv_icm42600: add support of accel low-power mode
> > 
> >  drivers/iio/imu/inv_icm42600/inv_icm42600.h   |   4 +
> >  .../iio/imu/inv_icm42600/inv_icm42600_accel.c | 124 +++++++++++++++++-
> >  .../iio/imu/inv_icm42600/inv_icm42600_core.c  |  27 ++++
> >  3 files changed, 151 insertions(+), 4 deletions(-)
> >   
>