[PATCH v5 0/8] iio: accel: adxl313: add power-save on activity/inactivity

[PATCH v5 0/8] iio: accel: adxl313: add power-save on activity/inactivity

[Lothar Rubusch]

The patch set covers the following topics:
- add debug register and regmap cache
- prepare iio channel scan_type and scan_index
- prepare interrupt handling
- implement fifo with watermark
- add activity/inactivity together with auto-sleep with link bit
- add ac coupled activity/inactivity, integrate with auto-sleep and link bit
- documentation

Since activity and inactivity here are implemented covering all axis, I
assumed x&y&z and x|y|z, respectively. Thus the driver uses a fake
channel for activity/inactiviy. AC-coupling is similar to other Analog Device
accelerometers, so MAG_ADAPTIVE events are chosen. Combinations are
documented and functionality tested and verified working.

Signed-off-by: Lothar Rubusch <l.rubusch@gmail.com>
---
v4 -> v5:
- [v4 01/11]: applied - debug register                                          
- [v4 03/11]: applied w/ changed commit message - regmap cache                  
- refrase all commit messages                                                   
- merge patches [v4 02/11] [v4 05/11] and [v4 06/11]                            
- add ADXL313_REG_INT_SOURCE to the initial regmap cache definition             
- `adxl313_set_watermark()`: replace plain hex numbers by defined bit masks     
- `adxl313_set_watermark()`: replace `regmap_update_bits()` by
  `regmap_set_bits()`
- `adxl313_get_samples()`: remove initialization of variable `samples`          
- `adxl313_buffer_postenable()`: add comment on turning off measurment          
- `adxl313_push_event()`: move WATERMARK separate out, focus on pushing events  
- `adxl313_irq_handler()`: add comment on draining the FIFO                     
- `adxl313_push_event()`: remove missleading comment on return statement        
- `adxl313_is_act_inact_en()`: If it's false, it will be false anyway -
  simplified now
- change order in multiplication with unit: `val * MICRO` which is read
  more naturally
- `adxl313_is_act_inact_en()`: remove check for ADXL313_ACTIVITY in the
  activity patch
- `adxl313_write_event_value()`: remove the general turning off measurement mode
- `adxl313_set_inact_time_s()`: replace plain number 255 by U8_MAX              
- `adxl313_read/write_event_config()`: encapsulate duplicate code into
  `adxl313_read/write_mag_config()`
- `adxl313_read/write_event_value()`: encapsulate duplicate code into
  `adxl313_read/write_mag_value()`
- `adxl313_is_act_inact_en()`: apply switch/case rather than if/else for
  readability; factor out variable `coupling`; convert all remaining `_en`
  variables there to bool, such that a negative error is evaluated from a
  `ret`, and     logic operates with `_en` variables
- `adxl313_set_act_inact_en()`: major rework due to issues discovered by
  automated testing (also affects related functions)
- fix kernel-doc issues 

v3 -> v4:
- squash patches [v3 02/12 + 03/12]: buffer usage into the patch that adds buffered support
- squash patches [v3 07/12 + 08/12]: interrupt handler with watermark implementation
- add patch: (in)activity / AC coupled as `MAG_ADAPTIVE` event
- `ADXL313_MEASUREMENT_MODE`: adjust commit message on removal of define and adding measurement enable function
- remove irq variable from driver data struct, make it a local variable
- `adxl313_core_probe()`: flip logic to condition `int_line != ADXL313_INT_NONE`
- `adxl313_core_probe()`: change mapping interrupts from 0xff to an explicit local mask
- `adxl313_core_probe()`: add comment on FIFO bypass mode
- reduce odd selection of headers to add [`adxl313_core.c`]
- `adxl313_set_fifo()`: this function was turning measurement off/on before changing `fifo_mode`,
   called in postenable and predisable this firstly excluded setting of interrupts, and secondly
   still configured watermark where unnecessary, this function was thus removed (covers unhandled
   return value, and refactoring of function parameters)
- `adxl313_fifo_transfer()`: simplify computation of `sizeof(i*count/2)`
- `adxl313_irq_handler()`: make call of `adxl313_reset_fifo()` conditional to OVERRUN one patch earlier
- includes: rework adding included headers
- activity: change to work with or'd axis and related changes to the fake channel and arrays
- `adxl313_set_act_inact_en()`: generally turn off measurement when adjusting config
  activity/inactivity related config registers, turn measurement on after
- doc: adjust code block highlighting and remove links

v2 -> v3:
- verify keeping trailing comma when it's multi-line assignment [v1 02/12]
- `adxl313_set_fifo()`: verify have two on one line to make it easier to read [v1 07/12]
- `adxl313_fifo_transfer()`: verify removal of useless initialization of ret [v1 07/12]
- `adxl313_fifo_transfer()`: verify usage of array_size() from overflow.h [v1 07/12]
- `adxl313_fifo_transfer()`: verify return 0 here [v1 07/12]
- `adxl313_irq_handler()`: verify "Why do we need the label?" / moving the call under the conditional [v1 07/12]
- verify reorganization of half condition for Activity [v1 09/12] and Inactivity [v1 10/12]
- verify usage of MICRO instead of 1000000
- `adxl313_is_act_inact_en()`: restructure according to return logic value, or negative error
- `adxl313_set_act_inact_en()`: restructure function, use regmap_assign_bits()
- `adxl313_set_act_inact_en()`: verify makeing it a logical split [v1 11/12]
- `adxl313_fifo_transfer()`: change iterator variable type from int to unsigned int [v2 07/12]
- `adxl313_fifo_reset()`: add comment on why reset status registers does not do error check ("At least comment...") [v2 07/12]
- `adxl313_fifo_push()`: change iterator variable from int to unsigned int [v2 08/12]
- `adxl313_fifo_push()`: remove duplicate check for samples being <0 [v2 08/12]
- apply `regmap_assign_bits()` in several places to replace regmap_update_bits() depending on bools
- `adxl313_set_watermark()`: rename mask variable to make it more comprehensive
- removal of duplicate blanks in various places (sry, my keyboard died) [v1 07/12]

v1 -> v2:
- usage of units.h
- simplify approach for return values
---

Lothar Rubusch (8):
  iio: accel: adxl313: make use of regmap cache
  iio: accel: adxl313: add function to enable measurement
  iio: accel: adxl313: add buffered FIFO watermark with interrupt
    handling
  iio: accel: adxl313: add activity sensing
  iio: accel: adxl313: add inactivity sensing
  iio: accel: adxl313: implement power-save on inactivity
  iio: accel: adxl313: add AC coupled activity/inactivity events
  docs: iio: add ADXL313 accelerometer

 Documentation/iio/adxl313.rst    | 289 ++++++++++
 Documentation/iio/index.rst      |   1 +
 drivers/iio/accel/adxl313.h      |  33 +-
 drivers/iio/accel/adxl313_core.c | 896 ++++++++++++++++++++++++++++++-
 drivers/iio/accel/adxl313_i2c.c  |   6 +
 drivers/iio/accel/adxl313_spi.c  |   6 +
 6 files changed, 1220 insertions(+), 11 deletions(-)
 create mode 100644 Documentation/iio/adxl313.rst


base-commit: 7461179e080df770240850a126cc7dbffad195c8
prerequisite-patch-id: 263cdbf28524f1edc96717db1461d7a4be2319c2
-- 
2.39.5

[PATCH v5 1/8] iio: accel: adxl313: make use of regmap cache

[Lothar Rubusch]

Setup regmap cache to cache register configuration, reducing bus traffic
for repeated accesses to non volatile registers.

Signed-off-by: Lothar Rubusch <l.rubusch@gmail.com>
---
 drivers/iio/accel/adxl313.h      |  3 +++
 drivers/iio/accel/adxl313_core.c | 18 ++++++++++++++++++
 drivers/iio/accel/adxl313_i2c.c  |  6 ++++++
 drivers/iio/accel/adxl313_spi.c  |  6 ++++++
 4 files changed, 33 insertions(+)

diff --git a/drivers/iio/accel/adxl313.h b/drivers/iio/accel/adxl313.h
index 72f624af4686..2bc86ac8ffd4 100644
--- a/drivers/iio/accel/adxl313.h
+++ b/drivers/iio/accel/adxl313.h
@@ -22,6 +22,7 @@
 #define ADXL313_REG_BW_RATE		0x2C
 #define ADXL313_REG_POWER_CTL		0x2D
 #define ADXL313_REG_INT_MAP		0x2F
+#define ADXL313_REG_INT_SOURCE		0x30
 #define ADXL313_REG_DATA_FORMAT		0x31
 #define ADXL313_REG_DATA_AXIS(index)	(0x32 + ((index) * 2))
 #define ADXL313_REG_FIFO_CTL		0x38
@@ -54,6 +55,8 @@ extern const struct regmap_access_table adxl312_writable_regs_table;
 extern const struct regmap_access_table adxl313_writable_regs_table;
 extern const struct regmap_access_table adxl314_writable_regs_table;
 
+bool adxl313_is_volatile_reg(struct device *dev, unsigned int reg);
+
 enum adxl313_device_type {
 	ADXL312,
 	ADXL313,
diff --git a/drivers/iio/accel/adxl313_core.c b/drivers/iio/accel/adxl313_core.c
index 2f26da5857d4..39f16f97bb4a 100644
--- a/drivers/iio/accel/adxl313_core.c
+++ b/drivers/iio/accel/adxl313_core.c
@@ -46,6 +46,24 @@ const struct regmap_access_table adxl314_readable_regs_table = {
 };
 EXPORT_SYMBOL_NS_GPL(adxl314_readable_regs_table, IIO_ADXL313);
 
+bool adxl313_is_volatile_reg(struct device *dev, unsigned int reg)
+{
+	switch (reg) {
+	case ADXL313_REG_DATA_AXIS(0):
+	case ADXL313_REG_DATA_AXIS(1):
+	case ADXL313_REG_DATA_AXIS(2):
+	case ADXL313_REG_DATA_AXIS(3):
+	case ADXL313_REG_DATA_AXIS(4):
+	case ADXL313_REG_DATA_AXIS(5):
+	case ADXL313_REG_FIFO_STATUS:
+	case ADXL313_REG_INT_SOURCE:
+		return true;
+	default:
+		return false;
+	}
+}
+EXPORT_SYMBOL_NS_GPL(adxl313_is_volatile_reg, "IIO_ADXL313");
+
 static int adxl312_check_id(struct device *dev,
 			    struct adxl313_data *data)
 {
diff --git a/drivers/iio/accel/adxl313_i2c.c b/drivers/iio/accel/adxl313_i2c.c
index a4cf0cf2c5aa..e8636e8ab14f 100644
--- a/drivers/iio/accel/adxl313_i2c.c
+++ b/drivers/iio/accel/adxl313_i2c.c
@@ -21,6 +21,8 @@ static const struct regmap_config adxl31x_i2c_regmap_config[] = {
 		.rd_table	= &adxl312_readable_regs_table,
 		.wr_table	= &adxl312_writable_regs_table,
 		.max_register	= 0x39,
+		.volatile_reg	= adxl313_is_volatile_reg,
+		.cache_type	= REGCACHE_MAPLE,
 	},
 	[ADXL313] = {
 		.reg_bits	= 8,
@@ -28,6 +30,8 @@ static const struct regmap_config adxl31x_i2c_regmap_config[] = {
 		.rd_table	= &adxl313_readable_regs_table,
 		.wr_table	= &adxl313_writable_regs_table,
 		.max_register	= 0x39,
+		.volatile_reg	= adxl313_is_volatile_reg,
+		.cache_type	= REGCACHE_MAPLE,
 	},
 	[ADXL314] = {
 		.reg_bits	= 8,
@@ -35,6 +39,8 @@ static const struct regmap_config adxl31x_i2c_regmap_config[] = {
 		.rd_table	= &adxl314_readable_regs_table,
 		.wr_table	= &adxl314_writable_regs_table,
 		.max_register	= 0x39,
+		.volatile_reg	= adxl313_is_volatile_reg,
+		.cache_type	= REGCACHE_MAPLE,
 	},
 };
 
diff --git a/drivers/iio/accel/adxl313_spi.c b/drivers/iio/accel/adxl313_spi.c
index 9a16b40bff34..68e323e81aeb 100644
--- a/drivers/iio/accel/adxl313_spi.c
+++ b/drivers/iio/accel/adxl313_spi.c
@@ -24,6 +24,8 @@ static const struct regmap_config adxl31x_spi_regmap_config[] = {
 		.max_register	= 0x39,
 		/* Setting bits 7 and 6 enables multiple-byte read */
 		.read_flag_mask	= BIT(7) | BIT(6),
+		.volatile_reg	= adxl313_is_volatile_reg,
+		.cache_type	= REGCACHE_MAPLE,
 	},
 	[ADXL313] = {
 		.reg_bits	= 8,
@@ -33,6 +35,8 @@ static const struct regmap_config adxl31x_spi_regmap_config[] = {
 		.max_register	= 0x39,
 		/* Setting bits 7 and 6 enables multiple-byte read */
 		.read_flag_mask	= BIT(7) | BIT(6),
+		.volatile_reg	= adxl313_is_volatile_reg,
+		.cache_type	= REGCACHE_MAPLE,
 	},
 	[ADXL314] = {
 		.reg_bits	= 8,
@@ -42,6 +46,8 @@ static const struct regmap_config adxl31x_spi_regmap_config[] = {
 		.max_register	= 0x39,
 		/* Setting bits 7 and 6 enables multiple-byte read */
 		.read_flag_mask	= BIT(7) | BIT(6),
+		.volatile_reg	= adxl313_is_volatile_reg,
+		.cache_type	= REGCACHE_MAPLE,
 	},
 };
 
-- 
2.39.5

[PATCH v5 2/8] iio: accel: adxl313: add function to enable measurement

[Lothar Rubusch]

Refactor the control of measurement and standby modes for the sensor.
Instead of directly writing to the register, encapsulate this operation
in a dedicated function that handles enabling and disabling measurement.
This approach will reduce code duplication wherever sensor configuration
changes are required. In subsequent patches, measurement mode will be
set to standby as part of this process.

Additionally, simplify the control mask to include only the measurement
bit. The sleep bit governs a different behavior—putting the sensor into
sleep mode, not just standby for configuration—and is currently unused.
Therefore, there's no need to include both the sleep and measurement
bits in the same mask.

Signed-off-by: Lothar Rubusch <l.rubusch@gmail.com>
---
 drivers/iio/accel/adxl313.h      |  3 +--
 drivers/iio/accel/adxl313_core.c | 10 +++++++---
 2 files changed, 8 insertions(+), 5 deletions(-)

diff --git a/drivers/iio/accel/adxl313.h b/drivers/iio/accel/adxl313.h
index 2bc86ac8ffd4..6958a00f5e8f 100644
--- a/drivers/iio/accel/adxl313.h
+++ b/drivers/iio/accel/adxl313.h
@@ -37,8 +37,7 @@
 #define ADXL313_RATE_MSK		GENMASK(3, 0)
 #define ADXL313_RATE_BASE		6
 
-#define ADXL313_POWER_CTL_MSK		GENMASK(3, 2)
-#define ADXL313_MEASUREMENT_MODE	BIT(3)
+#define ADXL313_POWER_CTL_MSK		BIT(3)
 
 #define ADXL313_RANGE_MSK		GENMASK(1, 0)
 #define ADXL313_RANGE_MAX		3
diff --git a/drivers/iio/accel/adxl313_core.c b/drivers/iio/accel/adxl313_core.c
index 39f16f97bb4a..99a7f3755031 100644
--- a/drivers/iio/accel/adxl313_core.c
+++ b/drivers/iio/accel/adxl313_core.c
@@ -64,6 +64,12 @@ bool adxl313_is_volatile_reg(struct device *dev, unsigned int reg)
 }
 EXPORT_SYMBOL_NS_GPL(adxl313_is_volatile_reg, "IIO_ADXL313");
 
+static int adxl313_set_measure_en(struct adxl313_data *data, bool en)
+{
+	return regmap_assign_bits(data->regmap, ADXL313_REG_POWER_CTL,
+				  ADXL313_POWER_CTL_MSK, en);
+}
+
 static int adxl312_check_id(struct device *dev,
 			    struct adxl313_data *data)
 {
@@ -398,9 +404,7 @@ static int adxl313_setup(struct device *dev, struct adxl313_data *data,
 	}
 
 	/* Enables measurement mode */
-	return regmap_update_bits(data->regmap, ADXL313_REG_POWER_CTL,
-				  ADXL313_POWER_CTL_MSK,
-				  ADXL313_MEASUREMENT_MODE);
+	return adxl313_set_measure_en(data, true);
 }
 
 /**
-- 
2.39.5

[PATCH v5 3/8] iio: accel: adxl313: add buffered FIFO watermark with interrupt handling

[Lothar Rubusch]

Cover the following tasks:
– Add scan_mask and scan_index to the IIO channel configuration. The
scan_index sets up buffer usage. According to the datasheet, the ADXL313
uses a 13-bit wide data field in full-resolution mode. Set the
signedness, number of storage bits, and endianness accordingly.

– Parse the devicetree for an optional interrupt line and configure the
interrupt mapping based on its presence. If no interrupt line is
specified, keep the FIFO in bypass mode as currently implemented.

– Set up the interrupt handler. Add register access to detect and
evaluate interrupts. Implement functions to clear status registers and
reset the FIFO.

– Implement FIFO watermark configuration and handling. Allow the
watermark level to be set, evaluate the corresponding interrupt, read
the FIFO contents, and push the data to the IIO channel.

Signed-off-by: Lothar Rubusch <l.rubusch@gmail.com>
---
 drivers/iio/accel/adxl313.h      |  22 +++
 drivers/iio/accel/adxl313_core.c | 259 ++++++++++++++++++++++++++++++-
 2 files changed, 275 insertions(+), 6 deletions(-)

diff --git a/drivers/iio/accel/adxl313.h b/drivers/iio/accel/adxl313.h
index 6958a00f5e8f..4f4a9fd39f6d 100644
--- a/drivers/iio/accel/adxl313.h
+++ b/drivers/iio/accel/adxl313.h
@@ -21,6 +21,7 @@
 #define ADXL313_REG_ACT_INACT_CTL	0x27
 #define ADXL313_REG_BW_RATE		0x2C
 #define ADXL313_REG_POWER_CTL		0x2D
+#define ADXL313_REG_INT_ENABLE		0x2E
 #define ADXL313_REG_INT_MAP		0x2F
 #define ADXL313_REG_INT_SOURCE		0x30
 #define ADXL313_REG_DATA_FORMAT		0x31
@@ -46,6 +47,25 @@
 #define ADXL313_SPI_3WIRE		BIT(6)
 #define ADXL313_I2C_DISABLE		BIT(6)
 
+#define ADXL313_INT_OVERRUN		BIT(0)
+#define ADXL313_INT_WATERMARK		BIT(1)
+#define ADXL313_INT_INACTIVITY		BIT(3)
+#define ADXL313_INT_ACTIVITY		BIT(4)
+#define ADXL313_INT_DREADY		BIT(7)
+
+/* FIFO entries: how many values are stored in the FIFO */
+#define ADXL313_REG_FIFO_STATUS_ENTRIES_MSK	GENMASK(5, 0)
+/* FIFO samples: number of samples needed for watermark (FIFO mode) */
+#define ADXL313_REG_FIFO_CTL_SAMPLES_MSK	GENMASK(4, 0)
+#define ADXL313_REG_FIFO_CTL_MODE_MSK		GENMASK(7, 6)
+
+#define ADXL313_FIFO_BYPASS			0
+#define ADXL313_FIFO_STREAM			2
+
+#define ADXL313_FIFO_SIZE			32
+
+#define ADXL313_NUM_AXIS			3
+
 extern const struct regmap_access_table adxl312_readable_regs_table;
 extern const struct regmap_access_table adxl313_readable_regs_table;
 extern const struct regmap_access_table adxl314_readable_regs_table;
@@ -66,7 +86,9 @@ struct adxl313_data {
 	struct regmap	*regmap;
 	const struct adxl313_chip_info *chip_info;
 	struct mutex	lock; /* lock to protect transf_buf */
+	u8 watermark;
 	__le16		transf_buf __aligned(IIO_DMA_MINALIGN);
+	__le16		fifo_buf[ADXL313_NUM_AXIS * ADXL313_FIFO_SIZE + 1];
 };
 
 struct adxl313_chip_info {
diff --git a/drivers/iio/accel/adxl313_core.c b/drivers/iio/accel/adxl313_core.c
index 99a7f3755031..488680807a8f 100644
--- a/drivers/iio/accel/adxl313_core.c
+++ b/drivers/iio/accel/adxl313_core.c
@@ -8,11 +8,23 @@
  */
 
 #include <linux/bitfield.h>
+#include <linux/interrupt.h>
 #include <linux/module.h>
+#include <linux/overflow.h>
+#include <linux/property.h>
 #include <linux/regmap.h>
 
+#include <linux/iio/buffer.h>
+#include <linux/iio/kfifo_buf.h>
+
 #include "adxl313.h"
 
+#define ADXL313_INT_NONE			U8_MAX
+#define ADXL313_INT1				1
+#define ADXL313_INT2				2
+
+#define ADXL313_REG_XYZ_BASE			ADXL313_REG_DATA_AXIS(0)
+
 static const struct regmap_range adxl312_readable_reg_range[] = {
 	regmap_reg_range(ADXL313_REG_DEVID0, ADXL313_REG_DEVID0),
 	regmap_reg_range(ADXL313_REG_OFS_AXIS(0), ADXL313_REG_OFS_AXIS(2)),
@@ -195,9 +207,10 @@ static const int adxl313_odr_freqs[][2] = {
 	[9] = { 3200, 0 },
 };
 
-#define ADXL313_ACCEL_CHANNEL(index, axis) {				\
+#define ADXL313_ACCEL_CHANNEL(index, reg, axis) {			\
 	.type = IIO_ACCEL,						\
-	.address = index,						\
+	.scan_index = (index),						\
+	.address = (reg),						\
 	.modified = 1,							\
 	.channel2 = IIO_MOD_##axis,					\
 	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |			\
@@ -207,14 +220,26 @@ static const int adxl313_odr_freqs[][2] = {
 	.info_mask_shared_by_type_available =				\
 		BIT(IIO_CHAN_INFO_SAMP_FREQ),				\
 	.scan_type = {							\
+		.sign = 's',						\
 		.realbits = 13,						\
+		.storagebits = 16,					\
+		.endianness = IIO_BE,					\
 	},								\
 }
 
+enum adxl313_chans {
+	chan_x, chan_y, chan_z,
+};
+
 static const struct iio_chan_spec adxl313_channels[] = {
-	ADXL313_ACCEL_CHANNEL(0, X),
-	ADXL313_ACCEL_CHANNEL(1, Y),
-	ADXL313_ACCEL_CHANNEL(2, Z),
+	ADXL313_ACCEL_CHANNEL(0, chan_x, X),
+	ADXL313_ACCEL_CHANNEL(1, chan_y, Y),
+	ADXL313_ACCEL_CHANNEL(2, chan_z, Z),
+};
+
+static const unsigned long adxl313_scan_masks[] = {
+	BIT(chan_x) | BIT(chan_y) | BIT(chan_z),
+	0
 };
 
 static int adxl313_set_odr(struct adxl313_data *data,
@@ -345,6 +370,173 @@ static int adxl313_write_raw(struct iio_dev *indio_dev,
 	}
 }
 
+static int adxl313_set_watermark(struct iio_dev *indio_dev, unsigned int value)
+{
+	struct adxl313_data *data = iio_priv(indio_dev);
+	int ret;
+
+	value = min(value, ADXL313_FIFO_SIZE - 1);
+
+	ret = adxl313_set_measure_en(data, false);
+	if (ret)
+		return ret;
+
+	ret = regmap_update_bits(data->regmap, ADXL313_REG_FIFO_CTL,
+				 ADXL313_REG_FIFO_CTL_MODE_MSK, value);
+	if (ret)
+		return ret;
+
+	data->watermark = value;
+
+	ret = regmap_set_bits(data->regmap, ADXL313_REG_INT_ENABLE,
+			      ADXL313_INT_WATERMARK);
+	if (ret)
+		return ret;
+
+	return adxl313_set_measure_en(data, true);
+}
+
+static int adxl313_get_samples(struct adxl313_data *data)
+{
+	unsigned int regval;
+	int ret;
+
+	ret = regmap_read(data->regmap, ADXL313_REG_FIFO_STATUS, &regval);
+	if (ret)
+		return ret;
+
+	return FIELD_GET(ADXL313_REG_FIFO_STATUS_ENTRIES_MSK, regval);
+}
+
+static int adxl313_fifo_transfer(struct adxl313_data *data, int samples)
+{
+	unsigned int i;
+	int ret;
+
+	for (i = 0; i < samples; i++) {
+		ret = regmap_bulk_read(data->regmap, ADXL313_REG_XYZ_BASE,
+				       data->fifo_buf + (i * ADXL313_NUM_AXIS),
+				       2 * ADXL313_NUM_AXIS);
+		if (ret)
+			return ret;
+	}
+
+	return 0;
+}
+
+/**
+ * adxl313_fifo_reset() - Reset the FIFO and interrupt status registers.
+ * @data: The device data.
+ *
+ * Reset the FIFO status registers. Reading out status registers clears the
+ * FIFO and interrupt configuration. Thus do not evaluate regmap return values.
+ * Ignore particular read register content. Register content is not processed
+ * any further. Therefore the function returns void.
+ */
+static void adxl313_fifo_reset(struct adxl313_data *data)
+{
+	unsigned int regval;
+	int samples;
+
+	adxl313_set_measure_en(data, false);
+
+	samples = adxl313_get_samples(data);
+	if (samples > 0)
+		adxl313_fifo_transfer(data, samples);
+
+	regmap_read(data->regmap, ADXL313_REG_INT_SOURCE, &regval);
+
+	adxl313_set_measure_en(data, true);
+}
+
+static int adxl313_buffer_postenable(struct iio_dev *indio_dev)
+{
+	struct adxl313_data *data = iio_priv(indio_dev);
+	int ret;
+
+	/* Set FIFO modes with measurement turned off, according to datasheet */
+	ret = adxl313_set_measure_en(data, false);
+	if (ret)
+		return ret;
+
+	ret = regmap_write(data->regmap, ADXL313_REG_FIFO_CTL,
+			   FIELD_PREP(ADXL313_REG_FIFO_CTL_SAMPLES_MSK,	data->watermark) |
+			   FIELD_PREP(ADXL313_REG_FIFO_CTL_MODE_MSK, ADXL313_FIFO_STREAM));
+	if (ret)
+		return ret;
+
+	return adxl313_set_measure_en(data, true);
+}
+
+static int adxl313_buffer_predisable(struct iio_dev *indio_dev)
+{
+	struct adxl313_data *data = iio_priv(indio_dev);
+	int ret;
+
+	ret = adxl313_set_measure_en(data, false);
+	if (ret)
+		return ret;
+
+	ret = regmap_write(data->regmap, ADXL313_REG_FIFO_CTL,
+			   FIELD_PREP(ADXL313_REG_FIFO_CTL_MODE_MSK, ADXL313_FIFO_BYPASS));
+
+	ret = regmap_write(data->regmap, ADXL313_REG_INT_ENABLE, 0);
+	if (ret)
+		return ret;
+
+	return adxl313_set_measure_en(data, true);
+}
+
+static const struct iio_buffer_setup_ops adxl313_buffer_ops = {
+	.postenable = adxl313_buffer_postenable,
+	.predisable = adxl313_buffer_predisable,
+};
+
+static int adxl313_fifo_push(struct iio_dev *indio_dev, int samples)
+{
+	struct adxl313_data *data = iio_priv(indio_dev);
+	unsigned int i;
+	int ret;
+
+	ret = adxl313_fifo_transfer(data, samples);
+	if (ret)
+		return ret;
+
+	for (i = 0; i < ADXL313_NUM_AXIS * samples; i += ADXL313_NUM_AXIS)
+		iio_push_to_buffers(indio_dev, &data->fifo_buf[i]);
+
+	return 0;
+}
+
+static irqreturn_t adxl313_irq_handler(int irq, void *p)
+{
+	struct iio_dev *indio_dev = p;
+	struct adxl313_data *data = iio_priv(indio_dev);
+	int samples, int_stat;
+
+	if (regmap_read(data->regmap, ADXL313_REG_INT_SOURCE, &int_stat))
+		return IRQ_NONE;
+
+	if (FIELD_GET(ADXL313_INT_WATERMARK, int_stat)) {
+		samples = adxl313_get_samples(data);
+		if (samples < 0)
+			goto err;
+
+		if (adxl313_fifo_push(indio_dev, samples))
+			goto err;
+	}
+
+	if (FIELD_GET(ADXL313_INT_OVERRUN, int_stat))
+		goto err;
+
+	return IRQ_HANDLED;
+
+err:
+	adxl313_fifo_reset(data);
+
+	return IRQ_HANDLED;
+}
+
 static int adxl313_reg_access(struct iio_dev *indio_dev, unsigned int reg,
 			      unsigned int writeval, unsigned int *readval)
 {
@@ -359,6 +551,7 @@ static const struct iio_info adxl313_info = {
 	.read_raw	= adxl313_read_raw,
 	.write_raw	= adxl313_write_raw,
 	.read_avail	= adxl313_read_freq_avail,
+	.hwfifo_set_watermark = adxl313_set_watermark,
 	.debugfs_reg_access = &adxl313_reg_access,
 };
 
@@ -424,7 +617,9 @@ int adxl313_core_probe(struct device *dev,
 {
 	struct adxl313_data *data;
 	struct iio_dev *indio_dev;
-	int ret;
+	u8 int_line;
+	u8 int_map_msk;
+	int irq, ret;
 
 	indio_dev = devm_iio_device_alloc(dev, sizeof(*data));
 	if (!indio_dev)
@@ -441,6 +636,7 @@ int adxl313_core_probe(struct device *dev,
 	indio_dev->modes = INDIO_DIRECT_MODE;
 	indio_dev->channels = adxl313_channels;
 	indio_dev->num_channels = ARRAY_SIZE(adxl313_channels);
+	indio_dev->available_scan_masks = adxl313_scan_masks;
 
 	ret = adxl313_setup(dev, data, setup);
 	if (ret) {
@@ -448,6 +644,57 @@ int adxl313_core_probe(struct device *dev,
 		return ret;
 	}
 
+	int_line = ADXL313_INT1;
+	irq = fwnode_irq_get_byname(dev_fwnode(dev), "INT1");
+	if (irq < 0) {
+		int_line = ADXL313_INT2;
+		irq = fwnode_irq_get_byname(dev_fwnode(dev), "INT2");
+		if (irq < 0)
+			int_line = ADXL313_INT_NONE;
+	}
+
+	if (int_line != ADXL313_INT_NONE) {
+		/* FIFO_STREAM mode */
+		int_map_msk = ADXL313_INT_DREADY | ADXL313_INT_ACTIVITY |
+			ADXL313_INT_INACTIVITY | ADXL313_INT_WATERMARK |
+			ADXL313_INT_OVERRUN;
+		ret = regmap_assign_bits(data->regmap, ADXL313_REG_INT_MAP,
+					 int_map_msk, int_line == ADXL313_INT2);
+		if (ret)
+			return ret;
+
+		ret = devm_iio_kfifo_buffer_setup(dev, indio_dev,
+						  &adxl313_buffer_ops);
+		if (ret)
+			return ret;
+
+		ret = devm_request_threaded_irq(dev, irq, NULL,
+						&adxl313_irq_handler,
+						IRQF_SHARED | IRQF_ONESHOT,
+						indio_dev->name, indio_dev);
+		if (ret)
+			return ret;
+	} else {
+		/*
+		 * FIFO_BYPASSED mode
+		 *
+		 * When no interrupt lines are specified, the driver falls back
+		 * to use the sensor in FIFO_BYPASS mode. This means turning off
+		 * internal FIFO and interrupt generation (since there is no
+		 * line specified). Unmaskable interrupts such as overrun or
+		 * data ready won't interfere. Even that a FIFO_STREAM mode w/o
+		 * connected interrupt line might allow for obtaining raw
+		 * measurements, a fallback to disable interrupts when no
+		 * interrupt lines are connected seems to be the cleaner
+		 * solution.
+		 */
+		ret = regmap_write(data->regmap, ADXL313_REG_FIFO_CTL,
+				   FIELD_PREP(ADXL313_REG_FIFO_CTL_MODE_MSK,
+					      ADXL313_FIFO_BYPASS));
+		if (ret)
+			return ret;
+	}
+
 	return devm_iio_device_register(dev, indio_dev);
 }
 EXPORT_SYMBOL_NS_GPL(adxl313_core_probe, IIO_ADXL313);
-- 
2.39.5

[PATCH v5 4/8] iio: accel: adxl313: add activity sensing

[Lothar Rubusch]

Add support for configuring an activity detection threshold. Extend the
interrupt handler to process activity-related interrupts, and provide
functions to set the threshold as well as to enable or disable activity
sensing. Additionally, introduce a virtual channel that represents the
logical AND of the x, y, and z axes in the IIO channel.

This patch serves as a preparatory step; some definitions and functions
introduced here are intended to be extended later to support inactivity
detection.

Signed-off-by: Lothar Rubusch <l.rubusch@gmail.com>
---
 drivers/iio/accel/adxl313_core.c | 244 +++++++++++++++++++++++++++++++
 1 file changed, 244 insertions(+)

diff --git a/drivers/iio/accel/adxl313_core.c b/drivers/iio/accel/adxl313_core.c
index 488680807a8f..b0d14ce60f01 100644
--- a/drivers/iio/accel/adxl313_core.c
+++ b/drivers/iio/accel/adxl313_core.c
@@ -13,8 +13,10 @@
 #include <linux/overflow.h>
 #include <linux/property.h>
 #include <linux/regmap.h>
+#include <linux/units.h>
 
 #include <linux/iio/buffer.h>
+#include <linux/iio/events.h>
 #include <linux/iio/kfifo_buf.h>
 
 #include "adxl313.h"
@@ -25,6 +27,21 @@
 
 #define ADXL313_REG_XYZ_BASE			ADXL313_REG_DATA_AXIS(0)
 
+#define ADXL313_ACT_XYZ_EN			GENMASK(6, 4)
+
+/* activity/inactivity */
+enum adxl313_activity_type {
+	ADXL313_ACTIVITY,
+};
+
+static const unsigned int adxl313_act_int_reg[] = {
+	[ADXL313_ACTIVITY] = ADXL313_INT_ACTIVITY,
+};
+
+static const unsigned int adxl313_act_thresh_reg[] = {
+	[ADXL313_ACTIVITY] = ADXL313_REG_THRESH_ACT,
+};
+
 static const struct regmap_range adxl312_readable_reg_range[] = {
 	regmap_reg_range(ADXL313_REG_DEVID0, ADXL313_REG_DEVID0),
 	regmap_reg_range(ADXL313_REG_OFS_AXIS(0), ADXL313_REG_OFS_AXIS(2)),
@@ -227,6 +244,15 @@ static const int adxl313_odr_freqs[][2] = {
 	},								\
 }
 
+static const struct iio_event_spec adxl313_activity_events[] = {
+	{
+		.type = IIO_EV_TYPE_MAG,
+		.dir = IIO_EV_DIR_RISING,
+		.mask_separate = BIT(IIO_EV_INFO_ENABLE),
+		.mask_shared_by_type = BIT(IIO_EV_INFO_VALUE),
+	},
+};
+
 enum adxl313_chans {
 	chan_x, chan_y, chan_z,
 };
@@ -235,6 +261,14 @@ static const struct iio_chan_spec adxl313_channels[] = {
 	ADXL313_ACCEL_CHANNEL(0, chan_x, X),
 	ADXL313_ACCEL_CHANNEL(1, chan_y, Y),
 	ADXL313_ACCEL_CHANNEL(2, chan_z, Z),
+	{
+		.type = IIO_ACCEL,
+		.modified = 1,
+		.channel2 = IIO_MOD_X_OR_Y_OR_Z,
+		.scan_index = -1, /* Fake channel for axis OR'ing */
+		.event_spec = adxl313_activity_events,
+		.num_event_specs = ARRAY_SIZE(adxl313_activity_events),
+	},
 };
 
 static const unsigned long adxl313_scan_masks[] = {
@@ -297,6 +331,73 @@ static int adxl313_read_freq_avail(struct iio_dev *indio_dev,
 	}
 }
 
+static int adxl313_is_act_inact_en(struct adxl313_data *data,
+				   enum adxl313_activity_type type)
+{
+	unsigned int axis_ctrl;
+	unsigned int regval;
+	int axis_en, ret;
+
+	ret = regmap_read(data->regmap, ADXL313_REG_ACT_INACT_CTL, &axis_ctrl);
+	if (ret)
+		return ret;
+
+	axis_en = FIELD_GET(ADXL313_ACT_XYZ_EN, axis_ctrl);
+
+	if (!axis_en)
+		return false;
+
+	/* Check if specific interrupt is enabled */
+	ret = regmap_read(data->regmap, ADXL313_REG_INT_ENABLE, &regval);
+	if (ret)
+		return ret;
+
+	return adxl313_act_int_reg[type] & regval;
+}
+
+static int adxl313_set_act_inact_en(struct adxl313_data *data,
+				    enum adxl313_activity_type type,
+				    bool cmd_en)
+{
+	unsigned int axis_ctrl;
+	unsigned int threshold;
+	int ret;
+
+	if (type != ADXL313_ACTIVITY)
+		return 0;
+
+	if (cmd_en) {
+		/* When turning on, check if threshold is valid */
+		ret = regmap_read(data->regmap, adxl313_act_thresh_reg[type],
+				  &threshold);
+		if (ret)
+			return ret;
+
+		if (!threshold) /* Just ignore the command if threshold is 0 */
+			return 0;
+	}
+
+	/* Start modifying configuration registers */
+	ret = adxl313_set_measure_en(data, false);
+	if (ret)
+		return ret;
+
+	/* Enable axis according to the command */
+	axis_ctrl = ADXL313_ACT_XYZ_EN;
+	ret = regmap_assign_bits(data->regmap, ADXL313_REG_ACT_INACT_CTL,
+				 axis_ctrl, cmd_en);
+	if (ret)
+		return ret;
+
+	/* Enable the interrupt line, according to the command */
+	ret = regmap_assign_bits(data->regmap, ADXL313_REG_INT_ENABLE,
+				 adxl313_act_int_reg[type], cmd_en);
+	if (ret)
+		return ret;
+
+	return adxl313_set_measure_en(data, true);
+}
+
 static int adxl313_read_raw(struct iio_dev *indio_dev,
 			    struct iio_chan_spec const *chan,
 			    int *val, int *val2, long mask)
@@ -370,6 +471,103 @@ static int adxl313_write_raw(struct iio_dev *indio_dev,
 	}
 }
 
+static int adxl313_read_event_config(struct iio_dev *indio_dev,
+				     const struct iio_chan_spec *chan,
+				     enum iio_event_type type,
+				     enum iio_event_direction dir)
+{
+	struct adxl313_data *data = iio_priv(indio_dev);
+
+	if (type != IIO_EV_TYPE_MAG)
+		return -EINVAL;
+
+	switch (dir) {
+	case IIO_EV_DIR_RISING:
+		return adxl313_is_act_inact_en(data, ADXL313_ACTIVITY);
+	default:
+		return -EINVAL;
+	}
+}
+
+static int adxl313_write_event_config(struct iio_dev *indio_dev,
+				      const struct iio_chan_spec *chan,
+				      enum iio_event_type type,
+				      enum iio_event_direction dir,
+				      bool state)
+{
+	struct adxl313_data *data = iio_priv(indio_dev);
+
+	if (type != IIO_EV_TYPE_MAG)
+		return -EINVAL;
+
+	switch (dir) {
+	case IIO_EV_DIR_RISING:
+		return adxl313_set_act_inact_en(data, ADXL313_ACTIVITY, state);
+	default:
+		return -EINVAL;
+	}
+}
+
+static int adxl313_read_event_value(struct iio_dev *indio_dev,
+				    const struct iio_chan_spec *chan,
+				    enum iio_event_type type,
+				    enum iio_event_direction dir,
+				    enum iio_event_info info,
+				    int *val, int *val2)
+{
+	struct adxl313_data *data = iio_priv(indio_dev);
+	unsigned int act_threshold;
+	int ret;
+
+	if (type != IIO_EV_TYPE_MAG)
+		return -EINVAL;
+
+	if (info != IIO_EV_INFO_VALUE)
+		return -EINVAL;
+
+	switch (dir) {
+	case IIO_EV_DIR_RISING:
+		ret = regmap_read(data->regmap,
+				  adxl313_act_thresh_reg[ADXL313_ACTIVITY],
+				  &act_threshold);
+		if (ret)
+			return ret;
+		*val = act_threshold * 15625;
+		*val2 = MICRO;
+		return IIO_VAL_FRACTIONAL;
+	default:
+		return -EINVAL;
+	}
+}
+
+static int adxl313_write_event_value(struct iio_dev *indio_dev,
+				     const struct iio_chan_spec *chan,
+				     enum iio_event_type type,
+				     enum iio_event_direction dir,
+				     enum iio_event_info info,
+				     int val, int val2)
+{
+	struct adxl313_data *data = iio_priv(indio_dev);
+	unsigned int regval;
+
+	if (type != IIO_EV_TYPE_MAG)
+		return -EINVAL;
+
+	if (info != IIO_EV_INFO_VALUE)
+		return -EINVAL;
+
+	/* Scale factor 15.625 mg/LSB */
+	regval = DIV_ROUND_CLOSEST(val * MICRO + val2, 15625);
+	switch (dir) {
+	case IIO_EV_DIR_RISING:
+		return regmap_write(data->regmap,
+				    adxl313_act_thresh_reg[ADXL313_ACTIVITY],
+				    regval);
+	default:
+		return -EINVAL;
+	}
+}
+
 static int adxl313_set_watermark(struct iio_dev *indio_dev, unsigned int value)
 {
 	struct adxl313_data *data = iio_priv(indio_dev);
@@ -508,6 +706,25 @@ static int adxl313_fifo_push(struct iio_dev *indio_dev, int samples)
 	return 0;
 }
 
+static int adxl313_push_events(struct iio_dev *indio_dev, int int_stat)
+{
+	s64 ts = iio_get_time_ns(indio_dev);
+	int ret = -ENOENT;
+
+	if (FIELD_GET(ADXL313_INT_ACTIVITY, int_stat)) {
+		ret = iio_push_event(indio_dev,
+				     IIO_MOD_EVENT_CODE(IIO_ACCEL, 0,
+							IIO_MOD_X_OR_Y_OR_Z,
+							IIO_EV_TYPE_MAG,
+							IIO_EV_DIR_RISING),
+				     ts);
+		if (ret)
+			return ret;
+	}
+
+	return ret;
+}
+
 static irqreturn_t adxl313_irq_handler(int irq, void *p)
 {
 	struct iio_dev *indio_dev = p;
@@ -517,6 +734,16 @@ static irqreturn_t adxl313_irq_handler(int irq, void *p)
 	if (regmap_read(data->regmap, ADXL313_REG_INT_SOURCE, &int_stat))
 		return IRQ_NONE;
 
+	/*
+	 * In cases of sensor events not handled (still not implemented) by
+	 * this driver, the FIFO needs to be drained to become operational
+	 * again. In general the sensor configuration only should issue events
+	 * which were configured by this driver. Anyway a miss-configuration
+	 * easily might end up in a hanging sensor FIFO.
+	 */
+	if (adxl313_push_events(indio_dev, int_stat))
+		goto err;
+
 	if (FIELD_GET(ADXL313_INT_WATERMARK, int_stat)) {
 		samples = adxl313_get_samples(data);
 		if (samples < 0)
@@ -550,6 +777,10 @@ static int adxl313_reg_access(struct iio_dev *indio_dev, unsigned int reg,
 static const struct iio_info adxl313_info = {
 	.read_raw	= adxl313_read_raw,
 	.write_raw	= adxl313_write_raw,
+	.read_event_config = adxl313_read_event_config,
+	.write_event_config = adxl313_write_event_config,
+	.read_event_value = adxl313_read_event_value,
+	.write_event_value = adxl313_write_event_value,
 	.read_avail	= adxl313_read_freq_avail,
 	.hwfifo_set_watermark = adxl313_set_watermark,
 	.debugfs_reg_access = &adxl313_reg_access,
@@ -663,6 +894,19 @@ int adxl313_core_probe(struct device *dev,
 		if (ret)
 			return ret;
 
+		/*
+		 * Reset or configure the registers with reasonable default
+		 * values. As having 0 in most cases may result in undesirable
+		 * behavior if the interrupts are enabled.
+		 */
+		ret = regmap_write(data->regmap, ADXL313_REG_ACT_INACT_CTL, 0x00);
+		if (ret)
+			return ret;
+
+		ret = regmap_write(data->regmap, ADXL313_REG_THRESH_ACT, 0x52);
+		if (ret)
+			return ret;
+
 		ret = devm_iio_kfifo_buffer_setup(dev, indio_dev,
 						  &adxl313_buffer_ops);
 		if (ret)
-- 
2.39.5

[PATCH v5 5/8] iio: accel: adxl313: add inactivity sensing

[Lothar Rubusch]

Enhance the interrupt handler to process inactivity events. Introduce
functions to configure the threshold and period registers for
inactivity detection, as well as to enable or disable the inactivity
feature. Extend the fake IIO channel to handle inactivity events by
combining the x, y, and z axes using a logical AND operation.

Signed-off-by: Lothar Rubusch <l.rubusch@gmail.com>
---
 drivers/iio/accel/adxl313.h      |   2 +
 drivers/iio/accel/adxl313_core.c | 161 ++++++++++++++++++++++++++-----
 2 files changed, 137 insertions(+), 26 deletions(-)

diff --git a/drivers/iio/accel/adxl313.h b/drivers/iio/accel/adxl313.h
index 4f4a9fd39f6d..d7e8cb44855b 100644
--- a/drivers/iio/accel/adxl313.h
+++ b/drivers/iio/accel/adxl313.h
@@ -18,6 +18,8 @@
 #define ADXL313_REG_SOFT_RESET		0x18
 #define ADXL313_REG_OFS_AXIS(index)	(0x1E + (index))
 #define ADXL313_REG_THRESH_ACT		0x24
+#define ADXL313_REG_THRESH_INACT	0x25
+#define ADXL313_REG_TIME_INACT		0x26
 #define ADXL313_REG_ACT_INACT_CTL	0x27
 #define ADXL313_REG_BW_RATE		0x2C
 #define ADXL313_REG_POWER_CTL		0x2D
diff --git a/drivers/iio/accel/adxl313_core.c b/drivers/iio/accel/adxl313_core.c
index b0d14ce60f01..95f52c95682a 100644
--- a/drivers/iio/accel/adxl313_core.c
+++ b/drivers/iio/accel/adxl313_core.c
@@ -28,18 +28,22 @@
 #define ADXL313_REG_XYZ_BASE			ADXL313_REG_DATA_AXIS(0)
 
 #define ADXL313_ACT_XYZ_EN			GENMASK(6, 4)
+#define ADXL313_INACT_XYZ_EN			GENMASK(2, 0)
 
 /* activity/inactivity */
 enum adxl313_activity_type {
 	ADXL313_ACTIVITY,
+	ADXL313_INACTIVITY,
 };
 
 static const unsigned int adxl313_act_int_reg[] = {
 	[ADXL313_ACTIVITY] = ADXL313_INT_ACTIVITY,
+	[ADXL313_INACTIVITY] = ADXL313_INT_INACTIVITY,
 };
 
 static const unsigned int adxl313_act_thresh_reg[] = {
 	[ADXL313_ACTIVITY] = ADXL313_REG_THRESH_ACT,
+	[ADXL313_INACTIVITY] = ADXL313_REG_THRESH_INACT,
 };
 
 static const struct regmap_range adxl312_readable_reg_range[] = {
@@ -253,6 +257,16 @@ static const struct iio_event_spec adxl313_activity_events[] = {
 	},
 };
 
+static const struct iio_event_spec adxl313_inactivity_events[] = {
+	{
+		.type = IIO_EV_TYPE_MAG,
+		.dir = IIO_EV_DIR_FALLING,
+		.mask_separate = BIT(IIO_EV_INFO_ENABLE),
+		.mask_shared_by_type = BIT(IIO_EV_INFO_VALUE) |
+			BIT(IIO_EV_INFO_PERIOD),
+	},
+};
+
 enum adxl313_chans {
 	chan_x, chan_y, chan_z,
 };
@@ -269,6 +283,14 @@ static const struct iio_chan_spec adxl313_channels[] = {
 		.event_spec = adxl313_activity_events,
 		.num_event_specs = ARRAY_SIZE(adxl313_activity_events),
 	},
+	{
+		.type = IIO_ACCEL,
+		.modified = 1,
+		.channel2 = IIO_MOD_X_AND_Y_AND_Z,
+		.scan_index = -1, /* Fake channel for axis AND'ing */
+		.event_spec = adxl313_inactivity_events,
+		.num_event_specs = ARRAY_SIZE(adxl313_inactivity_events),
+	},
 };
 
 static const unsigned long adxl313_scan_masks[] = {
@@ -331,6 +353,15 @@ static int adxl313_read_freq_avail(struct iio_dev *indio_dev,
 	}
 }
 
+static int adxl313_set_inact_time_s(struct adxl313_data *data,
+				    unsigned int val_s)
+{
+	unsigned int max_boundary = U8_MAX; /* by register size */
+	unsigned int val = min(val_s, max_boundary);
+
+	return regmap_write(data->regmap, ADXL313_REG_TIME_INACT, val);
+}
+
 static int adxl313_is_act_inact_en(struct adxl313_data *data,
 				   enum adxl313_activity_type type)
 {
@@ -342,7 +373,17 @@ static int adxl313_is_act_inact_en(struct adxl313_data *data,
 	if (ret)
 		return ret;
 
-	axis_en = FIELD_GET(ADXL313_ACT_XYZ_EN, axis_ctrl);
+	/* Check if axis for activity are enabled */
+	switch (type) {
+	case ADXL313_ACTIVITY:
+		axis_en = FIELD_GET(ADXL313_ACT_XYZ_EN, axis_ctrl);
+		break;
+	case ADXL313_INACTIVITY:
+		axis_en = FIELD_GET(ADXL313_INACT_XYZ_EN, axis_ctrl);
+		break;
+	default:
+		return -EINVAL;
+	}
 
 	if (!axis_en)
 		return false;
@@ -361,11 +402,9 @@ static int adxl313_set_act_inact_en(struct adxl313_data *data,
 {
 	unsigned int axis_ctrl;
 	unsigned int threshold;
+	unsigned int inact_time_s;
 	int ret;
 
-	if (type != ADXL313_ACTIVITY)
-		return 0;
-
 	if (cmd_en) {
 		/* When turning on, check if threshold is valid */
 		ret = regmap_read(data->regmap, adxl313_act_thresh_reg[type],
@@ -375,15 +414,33 @@ static int adxl313_set_act_inact_en(struct adxl313_data *data,
 
 		if (!threshold) /* Just ignore the command if threshold is 0 */
 			return 0;
+
+		/* When turning on inactivity, check if inact time is valid */
+		if (type == ADXL313_INACTIVITY) {
+			ret = regmap_read(data->regmap, ADXL313_REG_TIME_INACT, &inact_time_s);
+			if (ret)
+				return ret;
+
+			if (!inact_time_s)
+				return 0;
+		}
 	}
 
-	/* Start modifying configuration registers */
 	ret = adxl313_set_measure_en(data, false);
 	if (ret)
 		return ret;
 
 	/* Enable axis according to the command */
-	axis_ctrl = ADXL313_ACT_XYZ_EN;
+	switch (type) {
+	case ADXL313_ACTIVITY:
+		axis_ctrl = ADXL313_ACT_XYZ_EN;
+		break;
+	case ADXL313_INACTIVITY:
+		axis_ctrl = ADXL313_INACT_XYZ_EN;
+		break;
+	default:
+		return -EINVAL;
+	}
 	ret = regmap_assign_bits(data->regmap, ADXL313_REG_ACT_INACT_CTL,
 				 axis_ctrl, cmd_en);
 	if (ret)
@@ -484,6 +541,8 @@ static int adxl313_read_event_config(struct iio_dev *indio_dev,
 	switch (dir) {
 	case IIO_EV_DIR_RISING:
 		return adxl313_is_act_inact_en(data, ADXL313_ACTIVITY);
+	case IIO_EV_DIR_FALLING:
+		return adxl313_is_act_inact_en(data, ADXL313_INACTIVITY);
 	default:
 		return -EINVAL;
 	}
@@ -503,6 +562,8 @@ static int adxl313_write_event_config(struct iio_dev *indio_dev,
 	switch (dir) {
 	case IIO_EV_DIR_RISING:
 		return adxl313_set_act_inact_en(data, ADXL313_ACTIVITY, state);
+	case IIO_EV_DIR_FALLING:
+		return adxl313_set_act_inact_en(data, ADXL313_INACTIVITY, state);
 	default:
 		return -EINVAL;
 	}
@@ -517,24 +578,45 @@ static int adxl313_read_event_value(struct iio_dev *indio_dev,
 {
 	struct adxl313_data *data = iio_priv(indio_dev);
 	unsigned int act_threshold;
+	unsigned int inact_threshold;
+	unsigned int inact_time_s;
 	int ret;
 
 	if (type != IIO_EV_TYPE_MAG)
 		return -EINVAL;
 
-	if (info != IIO_EV_INFO_VALUE)
-		return -EINVAL;
-
-	switch (dir) {
-	case IIO_EV_DIR_RISING:
+	switch (info) {
+	case IIO_EV_INFO_VALUE:
+		switch (dir) {
+		case IIO_EV_DIR_RISING:
+			ret = regmap_read(data->regmap,
+					  adxl313_act_thresh_reg[ADXL313_ACTIVITY],
+					  &act_threshold);
+			if (ret)
+				return ret;
+			*val = act_threshold * 15625;
+			*val2 = MICRO;
+			return IIO_VAL_FRACTIONAL;
+		case IIO_EV_DIR_FALLING:
+			ret = regmap_read(data->regmap,
+					  adxl313_act_thresh_reg[ADXL313_INACTIVITY],
+					  &inact_threshold);
+			if (ret)
+				return ret;
+			*val = inact_threshold * 15625;
+			*val2 = MICRO;
+			return IIO_VAL_FRACTIONAL;
+		default:
+			return -EINVAL;
+		}
+	case IIO_EV_INFO_PERIOD:
 		ret = regmap_read(data->regmap,
-				  adxl313_act_thresh_reg[ADXL313_ACTIVITY],
-				  &act_threshold);
+				  ADXL313_REG_TIME_INACT,
+				  &inact_time_s);
 		if (ret)
 			return ret;
-		*val = act_threshold * 15625;
-		*val2 = MICRO;
-		return IIO_VAL_FRACTIONAL;
+		*val = inact_time_s;
+		return IIO_VAL_INT;
 	default:
 		return -EINVAL;
 	}
@@ -553,16 +635,24 @@ static int adxl313_write_event_value(struct iio_dev *indio_dev,
 	if (type != IIO_EV_TYPE_MAG)
 		return -EINVAL;
 
-	if (info != IIO_EV_INFO_VALUE)
-		return -EINVAL;
-
-	/* Scale factor 15.625 mg/LSB */
-	regval = DIV_ROUND_CLOSEST(val * MICRO + val2, 15625);
-	switch (dir) {
-	case IIO_EV_DIR_RISING:
-		return regmap_write(data->regmap,
-				    adxl313_act_thresh_reg[ADXL313_ACTIVITY],
-				    regval);
+	switch (info) {
+	case IIO_EV_INFO_VALUE:
+		/* Scale factor 15.625 mg/LSB */
+		regval = DIV_ROUND_CLOSEST(MICRO * val + val2, 15625);
+		switch (dir) {
+		case IIO_EV_DIR_RISING:
+			return regmap_write(data->regmap,
+					    adxl313_act_thresh_reg[ADXL313_ACTIVITY],
+					    regval);
+		case IIO_EV_DIR_FALLING:
+			return regmap_write(data->regmap,
+					    adxl313_act_thresh_reg[ADXL313_INACTIVITY],
+					    regval);
+		default:
+			return -EINVAL;
+		}
+	case IIO_EV_INFO_PERIOD:
+		return adxl313_set_inact_time_s(data, val);
 	default:
 		return -EINVAL;
 	}
@@ -722,6 +812,17 @@ static int adxl313_push_events(struct iio_dev *indio_dev, int int_stat)
 			return ret;
 	}
 
+	if (FIELD_GET(ADXL313_INT_INACTIVITY, int_stat)) {
+		ret = iio_push_event(indio_dev,
+				     IIO_MOD_EVENT_CODE(IIO_ACCEL, 0,
+							IIO_MOD_X_AND_Y_AND_Z,
+							IIO_EV_TYPE_MAG,
+							IIO_EV_DIR_FALLING),
+				     ts);
+		if (ret)
+			return ret;
+	}
+
 	return ret;
 }
 
@@ -903,6 +1004,14 @@ int adxl313_core_probe(struct device *dev,
 		if (ret)
 			return ret;
 
+		ret = regmap_write(data->regmap, ADXL313_REG_TIME_INACT, 5);
+		if (ret)
+			return ret;
+
+		ret = regmap_write(data->regmap, ADXL313_REG_THRESH_INACT, 0x4f);
+		if (ret)
+			return ret;
+
 		ret = regmap_write(data->regmap, ADXL313_REG_THRESH_ACT, 0x52);
 		if (ret)
 			return ret;
-- 
2.39.5

[PATCH v5 6/8] iio: accel: adxl313: implement power-save on inactivity

[Lothar Rubusch]

Configure the link bit to associate activity and inactivity sensing,
allowing the sensor to reflect its internal power-saving state.
Additionally, enable the auto-sleep bit to transition the sensor into
auto-sleep mode during periods of inactivity, as outlined in the
datasheet.

Signed-off-by: Lothar Rubusch <l.rubusch@gmail.com>
---
 drivers/iio/accel/adxl313.h      |  3 +++
 drivers/iio/accel/adxl313_core.c | 25 +++++++++++++++++++++++++
 2 files changed, 28 insertions(+)

diff --git a/drivers/iio/accel/adxl313.h b/drivers/iio/accel/adxl313.h
index d7e8cb44855b..75ef54b60f75 100644
--- a/drivers/iio/accel/adxl313.h
+++ b/drivers/iio/accel/adxl313.h
@@ -41,6 +41,9 @@
 #define ADXL313_RATE_BASE		6
 
 #define ADXL313_POWER_CTL_MSK		BIT(3)
+#define ADXL313_POWER_CTL_INACT_MSK	GENMASK(5, 4)
+#define ADXL313_POWER_CTL_LINK		BIT(5)
+#define ADXL313_POWER_CTL_AUTO_SLEEP	BIT(4)
 
 #define ADXL313_RANGE_MSK		GENMASK(1, 0)
 #define ADXL313_RANGE_MAX		3
diff --git a/drivers/iio/accel/adxl313_core.c b/drivers/iio/accel/adxl313_core.c
index 95f52c95682a..d8a263b2a6f6 100644
--- a/drivers/iio/accel/adxl313_core.c
+++ b/drivers/iio/accel/adxl313_core.c
@@ -396,6 +396,25 @@ static int adxl313_is_act_inact_en(struct adxl313_data *data,
 	return adxl313_act_int_reg[type] & regval;
 }
 
+static int adxl313_set_act_inact_linkbit(struct adxl313_data *data, bool en)
+{
+	int act_en, inact_en;
+
+	act_en = adxl313_is_act_inact_en(data, ADXL313_ACTIVITY);
+	if (act_en < 0)
+		return act_en;
+
+	inact_en = adxl313_is_act_inact_en(data, ADXL313_INACTIVITY);
+	if (inact_en < 0)
+		return inact_en;
+
+	en = en && act_en && inact_en;
+
+	return regmap_assign_bits(data->regmap, ADXL313_REG_POWER_CTL,
+				  (ADXL313_POWER_CTL_AUTO_SLEEP | ADXL313_POWER_CTL_LINK),
+				  en);
+}
+
 static int adxl313_set_act_inact_en(struct adxl313_data *data,
 				    enum adxl313_activity_type type,
 				    bool cmd_en)
@@ -426,6 +445,7 @@ static int adxl313_set_act_inact_en(struct adxl313_data *data,
 		}
 	}
 
+	/* Start modifying configuration registers */
 	ret = adxl313_set_measure_en(data, false);
 	if (ret)
 		return ret;
@@ -452,6 +472,11 @@ static int adxl313_set_act_inact_en(struct adxl313_data *data,
 	if (ret)
 		return ret;
 
+	/* Set link-bit and auto-sleep only when ACT and INACT are enabled */
+	ret = adxl313_set_act_inact_linkbit(data, cmd_en);
+	if (ret)
+		return ret;
+
 	return adxl313_set_measure_en(data, true);
 }
 
-- 
2.39.5

[PATCH v5 7/8] iio: accel: adxl313: add AC coupled activity/inactivity events

[Lothar Rubusch]

Introduce AC-coupled activity and inactivity as MAG_ADAPTIVE events.
This adds a new set of threshold and duration configuration options,
ensures proper handling of event disabling, and extends the use of the
link bit to support complementary event configurations.

For example, either ACTIVITY or ACTIVITY_AC can be enabled, but only the
most recently set configuration will remain active. Disabling ACTIVITY
will have no effect if ACTIVITY_AC is currently enabled, as the event
types must match (i.e., ACTIVITY_AC must be explicitly disabled). When
either INACTIVITY or INACTIVITY_AC is enabled alongside an activity
event, the link bit is set.

With the link bit and auto-sleep enabled, activity and inactivity events
represent changes in the sensor's power-saving state and are only
triggered upon actual state transitions. Since AC coupling uses separate
bits for activity and inactivity, each can be configured independently.
For instance, ACTIVITY can be linked with INACTIVITY_AC.

If one of the linked events is disabled, the link bit is cleared. In
that case, the remaining event will no longer reflect a state transition
but will instead trigger based on periodic inactivity or whenever the
activity threshold is exceeded.

Signed-off-by: Lothar Rubusch <l.rubusch@gmail.com>
---
 drivers/iio/accel/adxl313_core.c | 361 +++++++++++++++++++++++++------
 1 file changed, 296 insertions(+), 65 deletions(-)

diff --git a/drivers/iio/accel/adxl313_core.c b/drivers/iio/accel/adxl313_core.c
index d8a263b2a6f6..a04f28049f3e 100644
--- a/drivers/iio/accel/adxl313_core.c
+++ b/drivers/iio/accel/adxl313_core.c
@@ -30,20 +30,38 @@
 #define ADXL313_ACT_XYZ_EN			GENMASK(6, 4)
 #define ADXL313_INACT_XYZ_EN			GENMASK(2, 0)
 
+#define ADXL313_REG_ACT_ACDC_MSK		BIT(7)
+#define ADXL313_REG_INACT_ACDC_MSK		BIT(3)
+#define ADXL313_COUPLING_DC			0
+#define ADXL313_COUPLING_AC			1
+
 /* activity/inactivity */
 enum adxl313_activity_type {
 	ADXL313_ACTIVITY,
 	ADXL313_INACTIVITY,
+	ADXL313_ACTIVITY_AC,
+	ADXL313_INACTIVITY_AC,
 };
 
 static const unsigned int adxl313_act_int_reg[] = {
 	[ADXL313_ACTIVITY] = ADXL313_INT_ACTIVITY,
 	[ADXL313_INACTIVITY] = ADXL313_INT_INACTIVITY,
+	[ADXL313_ACTIVITY_AC] = ADXL313_INT_ACTIVITY,
+	[ADXL313_INACTIVITY_AC] = ADXL313_INT_INACTIVITY,
 };
 
 static const unsigned int adxl313_act_thresh_reg[] = {
 	[ADXL313_ACTIVITY] = ADXL313_REG_THRESH_ACT,
 	[ADXL313_INACTIVITY] = ADXL313_REG_THRESH_INACT,
+	[ADXL313_ACTIVITY_AC] = ADXL313_REG_THRESH_ACT,
+	[ADXL313_INACTIVITY_AC] = ADXL313_REG_THRESH_INACT,
+};
+
+static const unsigned int adxl313_act_acdc_msk[] = {
+	[ADXL313_ACTIVITY] = ADXL313_REG_ACT_ACDC_MSK,
+	[ADXL313_INACTIVITY] = ADXL313_REG_INACT_ACDC_MSK,
+	[ADXL313_ACTIVITY_AC] = ADXL313_REG_ACT_ACDC_MSK,
+	[ADXL313_INACTIVITY_AC] = ADXL313_REG_INACT_ACDC_MSK,
 };
 
 static const struct regmap_range adxl312_readable_reg_range[] = {
@@ -255,6 +273,13 @@ static const struct iio_event_spec adxl313_activity_events[] = {
 		.mask_separate = BIT(IIO_EV_INFO_ENABLE),
 		.mask_shared_by_type = BIT(IIO_EV_INFO_VALUE),
 	},
+	{
+		/* activity, AC bit set */
+		.type = IIO_EV_TYPE_MAG_ADAPTIVE,
+		.dir = IIO_EV_DIR_RISING,
+		.mask_separate = BIT(IIO_EV_INFO_ENABLE),
+		.mask_shared_by_type = BIT(IIO_EV_INFO_VALUE),
+	},
 };
 
 static const struct iio_event_spec adxl313_inactivity_events[] = {
@@ -265,6 +290,14 @@ static const struct iio_event_spec adxl313_inactivity_events[] = {
 		.mask_shared_by_type = BIT(IIO_EV_INFO_VALUE) |
 			BIT(IIO_EV_INFO_PERIOD),
 	},
+	{
+		/* inactivity, AC bit set */
+		.type = IIO_EV_TYPE_MAG_ADAPTIVE,
+		.dir = IIO_EV_DIR_FALLING,
+		.mask_separate = BIT(IIO_EV_INFO_ENABLE),
+		.mask_shared_by_type = BIT(IIO_EV_INFO_VALUE) |
+			BIT(IIO_EV_INFO_PERIOD),
+	},
 };
 
 enum adxl313_chans {
@@ -362,12 +395,50 @@ static int adxl313_set_inact_time_s(struct adxl313_data *data,
 	return regmap_write(data->regmap, ADXL313_REG_TIME_INACT, val);
 }
 
+/**
+ * adxl313_is_act_inact_ac() - Check if AC coupling is enabled.
+ * @data: The device data.
+ * @type: The activity or inactivity type.
+ *
+ * Provide a type of activity or inactivity, combined with either AC coupling
+ * set, or default to DC coupling. This function verifies, if the combination is
+ * currently enabled or not.
+ *
+ * Return: if the provided activity type has AC coupling enabled or a negative
+ * error value.
+ */
+static int adxl313_is_act_inact_ac(struct adxl313_data *data,
+				   enum adxl313_activity_type type)
+{
+	unsigned int regval;
+	bool coupling;
+	int ret;
+
+	ret = regmap_read(data->regmap, ADXL313_REG_ACT_INACT_CTL, &regval);
+	if (ret)
+		return ret;
+
+	coupling = adxl313_act_acdc_msk[type] & regval;
+
+	switch (type) {
+	case ADXL313_ACTIVITY:
+	case ADXL313_INACTIVITY:
+		return coupling == ADXL313_COUPLING_DC;
+	case ADXL313_ACTIVITY_AC:
+	case ADXL313_INACTIVITY_AC:
+		return coupling == ADXL313_COUPLING_AC;
+	default:
+		return -EINVAL;
+	}
+}
+
 static int adxl313_is_act_inact_en(struct adxl313_data *data,
 				   enum adxl313_activity_type type)
 {
 	unsigned int axis_ctrl;
 	unsigned int regval;
-	int axis_en, ret;
+	bool axis_en, int_en;
+	int ret;
 
 	ret = regmap_read(data->regmap, ADXL313_REG_ACT_INACT_CTL, &axis_ctrl);
 	if (ret)
@@ -376,9 +447,11 @@ static int adxl313_is_act_inact_en(struct adxl313_data *data,
 	/* Check if axis for activity are enabled */
 	switch (type) {
 	case ADXL313_ACTIVITY:
+	case ADXL313_ACTIVITY_AC:
 		axis_en = FIELD_GET(ADXL313_ACT_XYZ_EN, axis_ctrl);
 		break;
 	case ADXL313_INACTIVITY:
+	case ADXL313_INACTIVITY_AC:
 		axis_en = FIELD_GET(ADXL313_INACT_XYZ_EN, axis_ctrl);
 		break;
 	default:
@@ -393,21 +466,38 @@ static int adxl313_is_act_inact_en(struct adxl313_data *data,
 	if (ret)
 		return ret;
 
-	return adxl313_act_int_reg[type] & regval;
+	int_en = adxl313_act_int_reg[type] & regval;
+	if (!int_en)
+		return false;
+
+	/* Check if configured coupling matches provided type */
+	return adxl313_is_act_inact_ac(data, type);
 }
 
 static int adxl313_set_act_inact_linkbit(struct adxl313_data *data, bool en)
 {
-	int act_en, inact_en;
+	int act_en, inact_en, act_ac_en, inact_ac_en;
 
 	act_en = adxl313_is_act_inact_en(data, ADXL313_ACTIVITY);
 	if (act_en < 0)
 		return act_en;
 
+	act_ac_en = adxl313_is_act_inact_en(data, ADXL313_ACTIVITY_AC);
+	if (act_ac_en < 0)
+		return act_ac_en;
+
+	act_en = act_en || act_ac_en;
+
 	inact_en = adxl313_is_act_inact_en(data, ADXL313_INACTIVITY);
 	if (inact_en < 0)
 		return inact_en;
 
+	inact_ac_en = adxl313_is_act_inact_en(data, ADXL313_INACTIVITY_AC);
+	if (inact_ac_en < 0)
+		return inact_ac_en;
+
+	inact_en = inact_en || inact_ac_en;
+
 	en = en && act_en && inact_en;
 
 	return regmap_assign_bits(data->regmap, ADXL313_REG_POWER_CTL,
@@ -422,6 +512,7 @@ static int adxl313_set_act_inact_en(struct adxl313_data *data,
 	unsigned int axis_ctrl;
 	unsigned int threshold;
 	unsigned int inact_time_s;
+	bool act_inact_ac;
 	int ret;
 
 	if (cmd_en) {
@@ -435,7 +526,7 @@ static int adxl313_set_act_inact_en(struct adxl313_data *data,
 			return 0;
 
 		/* When turning on inactivity, check if inact time is valid */
-		if (type == ADXL313_INACTIVITY) {
+		if (type == ADXL313_INACTIVITY || type == ADXL313_INACTIVITY_AC) {
 			ret = regmap_read(data->regmap, ADXL313_REG_TIME_INACT, &inact_time_s);
 			if (ret)
 				return ret;
@@ -443,6 +534,14 @@ static int adxl313_set_act_inact_en(struct adxl313_data *data,
 			if (!inact_time_s)
 				return 0;
 		}
+	} else {
+		/* When turning off, check if the correct coupling event was
+		 * specified, this can be misused, e.g.: Having AC-coupled
+		 * activity turned on, and in current call trying to turning off
+		 * a DC-coupled activity shall be caught here.
+		 */
+		if (adxl313_is_act_inact_ac(data, type) <= 0)
+			return 0;
 	}
 
 	/* Start modifying configuration registers */
@@ -453,9 +552,11 @@ static int adxl313_set_act_inact_en(struct adxl313_data *data,
 	/* Enable axis according to the command */
 	switch (type) {
 	case ADXL313_ACTIVITY:
+	case ADXL313_ACTIVITY_AC:
 		axis_ctrl = ADXL313_ACT_XYZ_EN;
 		break;
 	case ADXL313_INACTIVITY:
+	case ADXL313_INACTIVITY_AC:
 		axis_ctrl = ADXL313_INACT_XYZ_EN;
 		break;
 	default:
@@ -466,6 +567,25 @@ static int adxl313_set_act_inact_en(struct adxl313_data *data,
 	if (ret)
 		return ret;
 
+	/* Update AC/DC-coupling according to the command */
+	switch (type) {
+	case ADXL313_ACTIVITY_AC:
+	case ADXL313_INACTIVITY_AC:
+		act_inact_ac = ADXL313_COUPLING_AC && cmd_en;
+		break;
+	case ADXL313_ACTIVITY:
+	case ADXL313_INACTIVITY:
+		act_inact_ac = ADXL313_COUPLING_DC && cmd_en;
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	ret = regmap_assign_bits(data->regmap, ADXL313_REG_ACT_INACT_CTL,
+				 adxl313_act_acdc_msk[type], act_inact_ac);
+	if (ret)
+		return ret;
+
 	/* Enable the interrupt line, according to the command */
 	ret = regmap_assign_bits(data->regmap, ADXL313_REG_INT_ENABLE,
 				 adxl313_act_int_reg[type], cmd_en);
@@ -553,6 +673,37 @@ static int adxl313_write_raw(struct iio_dev *indio_dev,
 	}
 }
 
+static int adxl313_read_mag_config(struct adxl313_data *data,
+				   enum iio_event_direction dir,
+				   enum adxl313_activity_type act_type,
+				   enum adxl313_activity_type inact_type)
+{
+	switch (dir) {
+	case IIO_EV_DIR_RISING:
+		return adxl313_is_act_inact_en(data, act_type);
+	case IIO_EV_DIR_FALLING:
+		return adxl313_is_act_inact_en(data, inact_type);
+	default:
+		return -EINVAL;
+	}
+}
+
+static int adxl313_write_mag_config(struct adxl313_data *data,
+				    enum iio_event_direction dir,
+				    enum adxl313_activity_type act_type,
+				    enum adxl313_activity_type inact_type,
+				    bool state)
+{
+	switch (dir) {
+	case IIO_EV_DIR_RISING:
+		return adxl313_set_act_inact_en(data, act_type, state);
+	case IIO_EV_DIR_FALLING:
+		return adxl313_set_act_inact_en(data, inact_type, state);
+	default:
+		return -EINVAL;
+	}
+}
+
 static int adxl313_read_event_config(struct iio_dev *indio_dev,
 				     const struct iio_chan_spec *chan,
 				     enum iio_event_type type,
@@ -560,14 +711,15 @@ static int adxl313_read_event_config(struct iio_dev *indio_dev,
 {
 	struct adxl313_data *data = iio_priv(indio_dev);
 
-	if (type != IIO_EV_TYPE_MAG)
-		return -EINVAL;
-
-	switch (dir) {
-	case IIO_EV_DIR_RISING:
-		return adxl313_is_act_inact_en(data, ADXL313_ACTIVITY);
-	case IIO_EV_DIR_FALLING:
-		return adxl313_is_act_inact_en(data, ADXL313_INACTIVITY);
+	switch (type) {
+	case IIO_EV_TYPE_MAG:
+		return adxl313_read_mag_config(data, dir,
+					       ADXL313_ACTIVITY,
+					       ADXL313_INACTIVITY);
+	case IIO_EV_TYPE_MAG_ADAPTIVE:
+		return adxl313_read_mag_config(data, dir,
+					       ADXL313_ACTIVITY_AC,
+					       ADXL313_INACTIVITY_AC);
 	default:
 		return -EINVAL;
 	}
@@ -581,54 +733,51 @@ static int adxl313_write_event_config(struct iio_dev *indio_dev,
 {
 	struct adxl313_data *data = iio_priv(indio_dev);
 
-	if (type != IIO_EV_TYPE_MAG)
-		return -EINVAL;
-
-	switch (dir) {
-	case IIO_EV_DIR_RISING:
-		return adxl313_set_act_inact_en(data, ADXL313_ACTIVITY, state);
-	case IIO_EV_DIR_FALLING:
-		return adxl313_set_act_inact_en(data, ADXL313_INACTIVITY, state);
+	switch (type) {
+	case IIO_EV_TYPE_MAG:
+		return adxl313_write_mag_config(data, dir,
+						ADXL313_ACTIVITY,
+						ADXL313_INACTIVITY,
+						state);
+	case IIO_EV_TYPE_MAG_ADAPTIVE:
+		return adxl313_write_mag_config(data, dir,
+						ADXL313_ACTIVITY_AC,
+						ADXL313_INACTIVITY_AC,
+						state);
 	default:
 		return -EINVAL;
 	}
 }
 
-static int adxl313_read_event_value(struct iio_dev *indio_dev,
-				    const struct iio_chan_spec *chan,
-				    enum iio_event_type type,
-				    enum iio_event_direction dir,
-				    enum iio_event_info info,
-				    int *val, int *val2)
+static int adxl313_read_mag_value(struct adxl313_data *data,
+				  enum iio_event_direction dir,
+				  enum iio_event_info info,
+				  enum adxl313_activity_type act_type,
+				  enum adxl313_activity_type inact_type,
+				  int *val, int *val2)
 {
-	struct adxl313_data *data = iio_priv(indio_dev);
-	unsigned int act_threshold;
-	unsigned int inact_threshold;
-	unsigned int inact_time_s;
+	unsigned int threshold, period;
 	int ret;
 
-	if (type != IIO_EV_TYPE_MAG)
-		return -EINVAL;
-
 	switch (info) {
 	case IIO_EV_INFO_VALUE:
 		switch (dir) {
 		case IIO_EV_DIR_RISING:
 			ret = regmap_read(data->regmap,
-					  adxl313_act_thresh_reg[ADXL313_ACTIVITY],
-					  &act_threshold);
+					  adxl313_act_thresh_reg[act_type],
+					  &threshold);
 			if (ret)
 				return ret;
-			*val = act_threshold * 15625;
+			*val = threshold * 15625;
 			*val2 = MICRO;
 			return IIO_VAL_FRACTIONAL;
 		case IIO_EV_DIR_FALLING:
 			ret = regmap_read(data->regmap,
-					  adxl313_act_thresh_reg[ADXL313_INACTIVITY],
-					  &inact_threshold);
+					  adxl313_act_thresh_reg[inact_type],
+					  &threshold);
 			if (ret)
 				return ret;
-			*val = inact_threshold * 15625;
+			*val = threshold * 15625;
 			*val2 = MICRO;
 			return IIO_VAL_FRACTIONAL;
 		default:
@@ -637,29 +786,25 @@ static int adxl313_read_event_value(struct iio_dev *indio_dev,
 	case IIO_EV_INFO_PERIOD:
 		ret = regmap_read(data->regmap,
 				  ADXL313_REG_TIME_INACT,
-				  &inact_time_s);
+				  &period);
 		if (ret)
 			return ret;
-		*val = inact_time_s;
+		*val = period;
 		return IIO_VAL_INT;
 	default:
 		return -EINVAL;
 	}
 }
 
-static int adxl313_write_event_value(struct iio_dev *indio_dev,
-				     const struct iio_chan_spec *chan,
-				     enum iio_event_type type,
-				     enum iio_event_direction dir,
-				     enum iio_event_info info,
-				     int val, int val2)
+static int adxl313_write_mag_value(struct adxl313_data *data,
+				   enum iio_event_direction dir,
+				   enum iio_event_info info,
+				   enum adxl313_activity_type act_type,
+				   enum adxl313_activity_type inact_type,
+				   int val, int val2)
 {
-	struct adxl313_data *data = iio_priv(indio_dev);
 	unsigned int regval;
 
-	if (type != IIO_EV_TYPE_MAG)
-		return -EINVAL;
-
 	switch (info) {
 	case IIO_EV_INFO_VALUE:
 		/* Scale factor 15.625 mg/LSB */
@@ -667,11 +812,11 @@ static int adxl313_write_event_value(struct iio_dev *indio_dev,
 		switch (dir) {
 		case IIO_EV_DIR_RISING:
 			return regmap_write(data->regmap,
-					    adxl313_act_thresh_reg[ADXL313_ACTIVITY],
+					    adxl313_act_thresh_reg[act_type],
 					    regval);
 		case IIO_EV_DIR_FALLING:
 			return regmap_write(data->regmap,
-					    adxl313_act_thresh_reg[ADXL313_INACTIVITY],
+					    adxl313_act_thresh_reg[inact_type],
 					    regval);
 		default:
 			return -EINVAL;
@@ -683,6 +828,56 @@ static int adxl313_write_event_value(struct iio_dev *indio_dev,
 	}
 }
 
+static int adxl313_read_event_value(struct iio_dev *indio_dev,
+				    const struct iio_chan_spec *chan,
+				    enum iio_event_type type,
+				    enum iio_event_direction dir,
+				    enum iio_event_info info,
+				    int *val, int *val2)
+{
+	struct adxl313_data *data = iio_priv(indio_dev);
+
+	switch (type) {
+	case IIO_EV_TYPE_MAG:
+		return adxl313_read_mag_value(data, dir, info,
+					      ADXL313_ACTIVITY,
+					      ADXL313_INACTIVITY,
+					      val, val2);
+	case IIO_EV_TYPE_MAG_ADAPTIVE:
+		return adxl313_read_mag_value(data, dir, info,
+					      ADXL313_ACTIVITY_AC,
+					      ADXL313_INACTIVITY_AC,
+					      val, val2);
+	default:
+		return -EINVAL;
+	}
+}
+
+static int adxl313_write_event_value(struct iio_dev *indio_dev,
+				     const struct iio_chan_spec *chan,
+				     enum iio_event_type type,
+				     enum iio_event_direction dir,
+				     enum iio_event_info info,
+				     int val, int val2)
+{
+	struct adxl313_data *data = iio_priv(indio_dev);
+
+	switch (type) {
+	case IIO_EV_TYPE_MAG:
+		return adxl313_write_mag_value(data, dir, info,
+					       ADXL313_ACTIVITY,
+					       ADXL313_INACTIVITY,
+					       val, val2);
+	case IIO_EV_TYPE_MAG_ADAPTIVE:
+		return adxl313_write_mag_value(data, dir, info,
+					       ADXL313_ACTIVITY_AC,
+					       ADXL313_INACTIVITY_AC,
+					       val, val2);
+	default:
+		return -EINVAL;
+	}
+}
+
 static int adxl313_set_watermark(struct iio_dev *indio_dev, unsigned int value)
 {
 	struct adxl313_data *data = iio_priv(indio_dev);
@@ -824,28 +1019,64 @@ static int adxl313_fifo_push(struct iio_dev *indio_dev, int samples)
 static int adxl313_push_events(struct iio_dev *indio_dev, int int_stat)
 {
 	s64 ts = iio_get_time_ns(indio_dev);
+	struct adxl313_data *data = iio_priv(indio_dev);
+	unsigned int regval;
 	int ret = -ENOENT;
 
 	if (FIELD_GET(ADXL313_INT_ACTIVITY, int_stat)) {
-		ret = iio_push_event(indio_dev,
-				     IIO_MOD_EVENT_CODE(IIO_ACCEL, 0,
-							IIO_MOD_X_OR_Y_OR_Z,
-							IIO_EV_TYPE_MAG,
-							IIO_EV_DIR_RISING),
-				     ts);
+		ret = regmap_read(data->regmap, ADXL313_REG_ACT_INACT_CTL, &regval);
 		if (ret)
 			return ret;
+
+		if (FIELD_GET(ADXL313_REG_ACT_ACDC_MSK, regval)) {
+			/* AC coupled */
+			ret = iio_push_event(indio_dev,
+					     IIO_MOD_EVENT_CODE(IIO_ACCEL, 0,
+								IIO_MOD_X_OR_Y_OR_Z,
+								IIO_EV_TYPE_MAG_ADAPTIVE,
+								IIO_EV_DIR_RISING),
+					     ts);
+			if (ret)
+				return ret;
+		} else {
+			/* DC coupled, relying on THRESH */
+			ret = iio_push_event(indio_dev,
+					     IIO_MOD_EVENT_CODE(IIO_ACCEL, 0,
+								IIO_MOD_X_OR_Y_OR_Z,
+								IIO_EV_TYPE_MAG,
+								IIO_EV_DIR_RISING),
+					     ts);
+			if (ret)
+				return ret;
+		}
 	}
 
 	if (FIELD_GET(ADXL313_INT_INACTIVITY, int_stat)) {
-		ret = iio_push_event(indio_dev,
-				     IIO_MOD_EVENT_CODE(IIO_ACCEL, 0,
-							IIO_MOD_X_AND_Y_AND_Z,
-							IIO_EV_TYPE_MAG,
-							IIO_EV_DIR_FALLING),
-				     ts);
+		ret = regmap_read(data->regmap, ADXL313_REG_ACT_INACT_CTL, &regval);
 		if (ret)
 			return ret;
+
+		if (FIELD_GET(ADXL313_REG_INACT_ACDC_MSK, regval)) {
+			/* AC coupled */
+			ret = iio_push_event(indio_dev,
+					     IIO_MOD_EVENT_CODE(IIO_ACCEL, 0,
+								IIO_MOD_X_AND_Y_AND_Z,
+								IIO_EV_TYPE_MAG_ADAPTIVE,
+								IIO_EV_DIR_FALLING),
+					     ts);
+			if (ret)
+				return ret;
+		} else {
+			/* DC coupled, relying on THRESH */
+			ret = iio_push_event(indio_dev,
+					     IIO_MOD_EVENT_CODE(IIO_ACCEL, 0,
+								IIO_MOD_X_AND_Y_AND_Z,
+								IIO_EV_TYPE_MAG,
+								IIO_EV_DIR_FALLING),
+					     ts);
+			if (ret)
+				return ret;
+		}
 	}
 
 	return ret;
-- 
2.39.5

[PATCH v5 8/8] docs: iio: add ADXL313 accelerometer

[Lothar Rubusch]

Add documentation for the ADXL313 accelerometer driver.

Signed-off-by: Lothar Rubusch <l.rubusch@gmail.com>
---
 Documentation/iio/adxl313.rst | 289 ++++++++++++++++++++++++++++++++++
 Documentation/iio/index.rst   |   1 +
 2 files changed, 290 insertions(+)
 create mode 100644 Documentation/iio/adxl313.rst

diff --git a/Documentation/iio/adxl313.rst b/Documentation/iio/adxl313.rst
new file mode 100644
index 000000000000..41b9cc37981c
--- /dev/null
+++ b/Documentation/iio/adxl313.rst
@@ -0,0 +1,289 @@
+.. SPDX-License-Identifier: GPL-2.0
+
+===============
+ADXL313 driver
+===============
+
+This driver supports Analog Device's ADXL313 on SPI/I2C bus.
+
+1. Supported devices
+====================
+
+* `ADXL313 <https://www.analog.com/ADXL313>`_
+
+The ADXL313is a low noise density, low power, 3-axis accelerometer with
+selectable measurement ranges. The ADXL313 supports the ±0.5 g, ±1 g, ±2 g and
+±4 g ranges.
+
+2. Device attributes
+====================
+
+Accelerometer measurements are always provided.
+
+Each IIO device, has a device folder under ``/sys/bus/iio/devices/iio:deviceX``,
+where X is the IIO index of the device. Under these folders reside a set of
+device files, depending on the characteristics and features of the hardware
+device in questions. These files are consistently generalized and documented in
+the IIO ABI documentation.
+
+The following tables show the adxl313 related device files, found in the
+specific device folder path ``/sys/bus/iio/devices/iio:deviceX``.
+
++---------------------------------------------------+----------------------------------------------------------+
+| 3-Axis Accelerometer related device files         | Description                                              |
++---------------------------------------------------+----------------------------------------------------------+
+| in_accel_scale                                    | Scale for the accelerometer channels.                    |
++---------------------------------------------------+----------------------------------------------------------+
+| in_accel_x_calibbias                              | Calibration offset for the X-axis accelerometer channel. |
++---------------------------------------------------+----------------------------------------------------------+
+| in_accel_x_raw                                    | Raw X-axis accelerometer channel value.                  |
++---------------------------------------------------+----------------------------------------------------------+
+| in_accel_y_calibbias                              | y-axis acceleration offset correction                    |
++---------------------------------------------------+----------------------------------------------------------+
+| in_accel_y_raw                                    | Raw Y-axis accelerometer channel value.                  |
++---------------------------------------------------+----------------------------------------------------------+
+| in_accel_z_calibbias                              | Calibration offset for the Z-axis accelerometer channel. |
++---------------------------------------------------+----------------------------------------------------------+
+| in_accel_z_raw                                    | Raw Z-axis accelerometer channel value.                  |
++---------------------------------------------------+----------------------------------------------------------+
+
++---------------------------------------+----------------------------------------------+
+| Miscellaneous device files            | Description                                  |
++---------------------------------------+----------------------------------------------+
+| name                                  | Name of the IIO device.                      |
++---------------------------------------+----------------------------------------------+
+| in_accel_sampling_frequency           | Currently selected sample rate.              |
++---------------------------------------+----------------------------------------------+
+| in_accel_sampling_frequency_available | Available sampling frequency configurations. |
++---------------------------------------+----------------------------------------------+
+
+The iio event related settings, found in ``/sys/bus/iio/devices/iio:deviceX/events``.
+
++---------------------------------------------------+----------------------------------------------------------+
+| in_accel_mag_adaptive_falling_period              | AC coupled inactivity time.                              |
++---------------------------------------------------+----------------------------------------------------------+
+| in_accel_mag_adaptive_falling_value               | AC coupled inactivity threshold.                         |
++---------------------------------------------------+----------------------------------------------------------+
+| in_accel_mag_adaptive_rising_value                | AC coupled activity threshold.                           |
++---------------------------------------------------+----------------------------------------------------------+
+| in_accel_mag_falling_period                       | Inactivity time.                                         |
++---------------------------------------------------+----------------------------------------------------------+
+| in_accel_mag_falling_value                        | Inactivity threshold.                                    |
++---------------------------------------------------+----------------------------------------------------------+
+| in_accel_mag_rising_value                         | Activity threshold.                                      |
++---------------------------------------------------+----------------------------------------------------------+
+| in_accel_x\&y\&z_mag_adaptive_falling_en          | Enable or disable AC coupled inactivity events.          |
++---------------------------------------------------+----------------------------------------------------------+
+| in_accel_x\|y\|z_mag_adaptive_rising_en           | Enable or disable AC coupled activity events.            |
++---------------------------------------------------+----------------------------------------------------------+
+| in_accel_x\&y\&z_mag_falling_en                   | Enable or disable inactivity events.                     |
++---------------------------------------------------+----------------------------------------------------------+
+| in_accel_x\|y\|z_mag_rising_en                    | Enable or disable activity events.                       |
++---------------------------------------------------+----------------------------------------------------------+
+
+The default coupling is DC coupled events. In this case the threshold will
+be in place as such, where for the AC coupled case an adaptive threshold
+(described in the datasheet) will be applied by the sensor. In general activity,
+i.e. ``ACTIVITY`` or ``ACTIVITY_AC`` and inactivity i.e. ``INACTIVITY`` or
+``INACTIVITY_AC``, will be linked with auto-sleep enabled when both are enabled.
+This means in particular ``ACTIVITY`` can also be linked to ``INACTIVITY_AC``
+and vice versa, without problem.
+
+Note here, that ``ACTIVITY`` and ``ACTIVITY_AC`` are mutually exclusive. This
+means, that the most recent configuration will be set. For instance, if
+``ACTIVITY`` is enabled, and ``ACTIVITY_AC`` will be enabled, the sensor driver
+will have ``ACTIVITY`` disabled, but ``ACTIVITY_AC`` enabled. The same is valid
+for inactivity. In case of turning off an event, it has to match to what is
+actually enabled, i.e. enabling ``ACTIVITY_AC`` and then disabling ``ACTIVITY``
+is simply ignored as it is already disabled. Or, as if it was any other not
+enabled event, too.
+
+Channels processed values
+-------------------------
+
+A channel value can be read from its _raw attribute. The value returned is the
+raw value as reported by the devices. To get the processed value of the channel,
+apply the following formula:
+
+.. code-block::
+
+        processed value = (_raw + _offset) * _scale
+
+Where _offset and _scale are device attributes. If no _offset attribute is
+present, simply assume its value is 0.
+
+The ADXL313 driver offers data for a single types of channels, the table below
+shows the measurement units for the processed value, which are defined by the
+IIO framework:
+
++-------------------------------------+---------------------------+
+| Channel type                        | Measurement unit          |
++-------------------------------------+---------------------------+
+| Acceleration on X, Y, and Z axis    | Meters per Second squared |
++-------------------------------------+---------------------------+
+
+Usage examples
+--------------
+
+Show device name:
+
+.. code-block:: bash
+
+        root:/sys/bus/iio/devices/iio:device0> cat name
+        adxl313
+
+Show accelerometer channels value:
+
+.. code-block:: bash
+
+        root:/sys/bus/iio/devices/iio:device0> cat in_accel_x_raw
+        2
+        root:/sys/bus/iio/devices/iio:device0> cat in_accel_y_raw
+        -57
+        root:/sys/bus/iio/devices/iio:device0> cat in_accel_z_raw
+        2
+        root:/sys/bus/iio/devices/iio:device0> cat in_accel_scale
+        0.009576806
+
+The accelerometer values will be:
+
+- X-axis acceleration = in_accel_x_raw * in_accel_scale = 0.0191536 m/s^2
+- Y-axis acceleration = in_accel_y_raw * in_accel_scale = -0.5458779 m/s^2
+- Z-axis acceleration = in_accel_z_raw * in_accel_scale = 0.0191536 m/s^2
+
+Set calibration offset for accelerometer channels. Note, that the calibration
+will be rounded according to the graduation of LSB units:
+
+.. code-block:: bash
+
+        root:/sys/bus/iio/devices/iio:device0> cat in_accel_x_calibbias
+        0
+
+        root:/sys/bus/iio/devices/iio:device0> echo 50 > in_accel_x_calibbias
+        root:/sys/bus/iio/devices/iio:device0> cat in_accel_x_calibbias
+        48
+
+Set sampling frequency:
+
+.. code-block:: bash
+
+        root:/sys/bus/iio/devices/iio:device0> cat in_accel_sampling_frequency
+        100.000000
+        root:/sys/bus/iio/devices/iio:device0> cat in_accel_sampling_frequency_available
+        6.250000 12.500000 25.000000 50.000000 100.000000 200.000000 400.000000 800.000000 1600.000000 3200.000000
+
+        root:/sys/bus/iio/devices/iio:device0> echo 400 > in_accel_sampling_frequency
+        root:/sys/bus/iio/devices/iio:device0> cat in_accel_sampling_frequency
+        400.000000
+
+3. Device buffers and triggers
+==============================
+
+This driver supports IIO buffers.
+
+All devices support retrieving the raw acceleration measurements using buffers.
+
+Usage examples
+--------------
+
+Select channels for buffer read:
+
+.. code-block:: bash
+
+        root:/sys/bus/iio/devices/iio:device0> echo 1 > scan_elements/in_accel_x_en
+        root:/sys/bus/iio/devices/iio:device0> echo 1 > scan_elements/in_accel_y_en
+        root:/sys/bus/iio/devices/iio:device0> echo 1 > scan_elements/in_accel_z_en
+
+Set the number of samples to be stored in the buffer:
+
+.. code-block:: bash
+
+        root:/sys/bus/iio/devices/iio:device0> echo 10 > buffer/length
+
+Enable buffer readings:
+
+.. code-block:: bash
+
+        root:/sys/bus/iio/devices/iio:device0> echo 1 > buffer/enable
+
+Obtain buffered data:
+
+.. code-block:: bash
+
+        root:/sys/bus/iio/devices/iio:device0> hexdump -C /dev/iio\:device0
+        ...
+        000000d0  01 fc 31 00 c7 ff 03 fc  31 00 c7 ff 04 fc 33 00  |..1.....1.....3.|
+        000000e0  c8 ff 03 fc 32 00 c5 ff  ff fc 32 00 c7 ff 0a fc  |....2.....2.....|
+        000000f0  30 00 c8 ff 06 fc 33 00  c7 ff 01 fc 2f 00 c8 ff  |0.....3...../...|
+        00000100  02 fc 32 00 c6 ff 04 fc  33 00 c8 ff 05 fc 33 00  |..2.....3.....3.|
+        00000110  ca ff 02 fc 31 00 c7 ff  02 fc 30 00 c9 ff 09 fc  |....1.....0.....|
+        00000120  35 00 c9 ff 08 fc 35 00  c8 ff 02 fc 31 00 c5 ff  |5.....5.....1...|
+        00000130  03 fc 32 00 c7 ff 04 fc  32 00 c7 ff 02 fc 31 00  |..2.....2.....1.|
+        00000140  c7 ff 08 fc 30 00 c7 ff  02 fc 32 00 c5 ff ff fc  |....0.....2.....|
+        00000150  31 00 c5 ff 04 fc 31 00  c8 ff 03 fc 32 00 c8 ff  |1.....1.....2...|
+        00000160  01 fc 31 00 c7 ff 05 fc  31 00 c3 ff 04 fc 31 00  |..1.....1.....1.|
+        00000170  c5 ff 04 fc 30 00 c7 ff  03 fc 31 00 c9 ff 03 fc  |....0.....1.....|
+        ...
+
+Enabling activity detection:
+
+.. code-block:: bash
+        root:/sys/bus/iio/devices/iio:device0> echo 1.28125 > ./events/in_accel_mag_rising_value
+        root:/sys/bus/iio/devices/iio:device0> echo 1 > ./events/in_accel_x\|y\|z_mag_rising_en
+
+        root:/sys/bus/iio/devices/iio:device0> iio_event_monitor adxl313
+        Found IIO device with name adxl313 with device number 0
+        <only while moving the sensor>
+        Event: time: 1748795762298351281, type: accel(x|y|z), channel: 0, evtype: mag, direction: rising
+        Event: time: 1748795762302653704, type: accel(x|y|z), channel: 0, evtype: mag, direction: rising
+        Event: time: 1748795762304340726, type: accel(x|y|z), channel: 0, evtype: mag, direction: rising
+        ...
+
+Disabling activity detection:
+
+.. code-block:: bash
+        root:/sys/bus/iio/devices/iio:device0> echo 0 > ./events/in_accel_x\|y\|z_mag_rising_en
+        root:/sys/bus/iio/devices/iio:device0> iio_event_monitor adxl313
+        <nothing>
+
+Enabling inactivity detection:
+
+.. code-block:: bash
+        root:/sys/bus/iio/devices/iio:device0> echo 1.234375 > ./events/in_accel_mag_falling_value
+        root:/sys/bus/iio/devices/iio:device0> echo 5 > ./events/in_accel_mag_falling_period
+        root:/sys/bus/iio/devices/iio:device0> echo 1 > ./events/in_accel_x\&y\&z_mag_falling_en
+
+        root:/sys/bus/iio/devices/iio:device0> iio_event_monitor adxl313
+        Found IIO device with name adxl313 with device number 0
+        Event: time: 1748796324115962975, type: accel(x&y&z), channel: 0, evtype: mag, direction: falling
+        Event: time: 1748796329329981772, type: accel(x&y&z), channel: 0, evtype: mag, direction: falling
+        Event: time: 1748796334543399706, type: accel(x&y&z), channel: 0, evtype: mag, direction: falling
+        ...
+        <every 5s now indicates inactivity>
+
+Now, enabling activity, e.g. the AC coupled counter-part ``ACTIVITY_AC``
+
+.. code-block:: bash
+        root:/sys/bus/iio/devices/iio:device0> echo 1.28125 > ./events/in_accel_mag_rising_value
+        root:/sys/bus/iio/devices/iio:device0> echo 1 > ./events/in_accel_x\|y\|z_mag_rising_en
+
+        root:/sys/bus/iio/devices/iio:device0> iio_event_monitor adxl313
+        Found IIO device with name adxl313 with device number 0
+        <some activity with the sensor>
+        Event: time: 1748796880354686777, type: accel(x|y|z), channel: 0, evtype: mag_adaptive, direction: rising
+        <5s of inactivity, then>
+        Event: time: 1748796885543252017, type: accel(x&y&z), channel: 0, evtype: mag, direction: falling
+        <some other activity detected by accelerating the sensor>
+        Event: time: 1748796887756634678, type: accel(x|y|z), channel: 0, evtype: mag_adaptive, direction: rising
+        <again, 5s of inactivity>
+        Event: time: 1748796892964368352, type: accel(x&y&z), channel: 0, evtype: mag, direction: falling
+        <stays like this until next activity in auto-sleep>
+
+Note, when AC coupling is in place, the event type will be of ``mag_adaptive``.
+AC- or DC-coupled (the default) events are used similarly.
+
+4. IIO Interfacing Tools
+========================
+
+See Documentation/iio/iio_tools.rst for the description of the available IIO
+interfacing tools.
diff --git a/Documentation/iio/index.rst b/Documentation/iio/index.rst
index 2d6afc5a8ed5..c106402a91f7 100644
--- a/Documentation/iio/index.rst
+++ b/Documentation/iio/index.rst
@@ -31,6 +31,7 @@ Industrial I/O Kernel Drivers
    adis16475
    adis16480
    adis16550
+   adxl313
    adxl380
    bno055
    ep93xx_adc
-- 
2.39.5

Re: [PATCH v5 3/8] iio: accel: adxl313: add buffered FIFO watermark with interrupt handling

[Andy Shevchenko]

On Mon, Jun 16, 2025 at 1:23 AM Lothar Rubusch <l.rubusch@gmail.com> wrote:
>
> Cover the following tasks:
> – Add scan_mask and scan_index to the IIO channel configuration. The
> scan_index sets up buffer usage. According to the datasheet, the ADXL313
> uses a 13-bit wide data field in full-resolution mode. Set the
> signedness, number of storage bits, and endianness accordingly.
>
> – Parse the devicetree for an optional interrupt line and configure the
> interrupt mapping based on its presence. If no interrupt line is
> specified, keep the FIFO in bypass mode as currently implemented.
>
> – Set up the interrupt handler. Add register access to detect and
> evaluate interrupts. Implement functions to clear status registers and
> reset the FIFO.
>
> – Implement FIFO watermark configuration and handling. Allow the
> watermark level to be set, evaluate the corresponding interrupt, read
> the FIFO contents, and push the data to the IIO channel.

...

> +       int_line = ADXL313_INT1;
> +       irq = fwnode_irq_get_byname(dev_fwnode(dev), "INT1");
> +       if (irq < 0) {
> +               int_line = ADXL313_INT2;
> +               irq = fwnode_irq_get_byname(dev_fwnode(dev), "INT2");
> +               if (irq < 0)
> +                       int_line = ADXL313_INT_NONE;
> +       }
> +
> +       if (int_line != ADXL313_INT_NONE) {

> +       } else {

> +       }

What I meant is something like this:


       int_line = ADXL313_INT_NONE;
       irq = fwnode_irq_get_byname(dev_fwnode(dev), "INT1");
       if (irq > 0) {
              int_line = ADXL313_INT1;
       } else {
               irq = fwnode_irq_get_byname(dev_fwnode(dev), "INT2");
               if (irq > 0)
                      int_line = ADXL313_INT2;
       }

       if (int_line == ADXL313_INT_NONE) {
   ...
       } else {
   ...
       }

Obviously with a helper you can unnest the if-else-if above.

static unsigned int _get_int_type(...)
{
    if (irq > 0)
        return ...
    return _NONE;
}

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v5 4/8] iio: accel: adxl313: add activity sensing

[Andy Shevchenko]

On Mon, Jun 16, 2025 at 1:23 AM Lothar Rubusch <l.rubusch@gmail.com> wrote:
>
> Add support for configuring an activity detection threshold. Extend the
> interrupt handler to process activity-related interrupts, and provide
> functions to set the threshold as well as to enable or disable activity
> sensing. Additionally, introduce a virtual channel that represents the
> logical AND of the x, y, and z axes in the IIO channel.
>
> This patch serves as a preparatory step; some definitions and functions
> introduced here are intended to be extended later to support inactivity
> detection.

...

> +static int adxl313_is_act_inact_en(struct adxl313_data *data,
> +                                  enum adxl313_activity_type type)
> +{
> +       unsigned int axis_ctrl;
> +       unsigned int regval;
> +       int axis_en, ret;
> +
> +       ret = regmap_read(data->regmap, ADXL313_REG_ACT_INACT_CTL, &axis_ctrl);
> +       if (ret)
> +               return ret;
> +
> +       axis_en = FIELD_GET(ADXL313_ACT_XYZ_EN, axis_ctrl);

> +

Remove this blank line. If needed, can be added later.

> +       if (!axis_en)
> +               return false;
> +
> +       /* Check if specific interrupt is enabled */
> +       ret = regmap_read(data->regmap, ADXL313_REG_INT_ENABLE, &regval);
> +       if (ret)
> +               return ret;
> +
> +       return adxl313_act_int_reg[type] & regval;
> +}

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v5 5/8] iio: accel: adxl313: add inactivity sensing

[Andy Shevchenko]

On Mon, Jun 16, 2025 at 1:23 AM Lothar Rubusch <l.rubusch@gmail.com> wrote:
>
> Enhance the interrupt handler to process inactivity events. Introduce
> functions to configure the threshold and period registers for
> inactivity detection, as well as to enable or disable the inactivity
> feature. Extend the fake IIO channel to handle inactivity events by
> combining the x, y, and z axes using a logical AND operation.

...

> -       axis_en = FIELD_GET(ADXL313_ACT_XYZ_EN, axis_ctrl);
> +       /* Check if axis for activity are enabled */
> +       switch (type) {
> +       case ADXL313_ACTIVITY:
> +               axis_en = FIELD_GET(ADXL313_ACT_XYZ_EN, axis_ctrl);
> +               break;
> +       case ADXL313_INACTIVITY:
> +               axis_en = FIELD_GET(ADXL313_INACT_XYZ_EN, axis_ctrl);
> +               break;
> +       default:
> +               return -EINVAL;
> +       }
>
>         if (!axis_en)
>                 return false;

So, this looks better without a variable?

  case FOO:
     if (!FIELD_GET(...))
         return false;
     break;

On the first glance it seems that next changes don't affect this.

...

> -       /* Start modifying configuration registers */

Stray change, the next patch restores this. So why change to  begin with?

>         ret = adxl313_set_measure_en(data, false);
>         if (ret)
>                 return ret;

...

>         /* Enable axis according to the command */
> -       axis_ctrl = ADXL313_ACT_XYZ_EN;
> +       switch (type) {

I was wondering if you can use switch-case earlier in the series.

> +       case ADXL313_ACTIVITY:
> +               axis_ctrl = ADXL313_ACT_XYZ_EN;
> +               break;
> +       case ADXL313_INACTIVITY:
> +               axis_ctrl = ADXL313_INACT_XYZ_EN;
> +               break;
> +       default:
> +               return -EINVAL;
> +       }

...

> -       if (info != IIO_EV_INFO_VALUE)
> -               return -EINVAL;
> -
> -       switch (dir) {
> -       case IIO_EV_DIR_RISING:
> +       switch (info) {
> +       case IIO_EV_INFO_VALUE:
> +               switch (dir) {
> +               case IIO_EV_DIR_RISING:
> +                       ret = regmap_read(data->regmap,
> +                                         adxl313_act_thresh_reg[ADXL313_ACTIVITY],
> +                                         &act_threshold);
> +                       if (ret)
> +                               return ret;
> +                       *val = act_threshold * 15625;
> +                       *val2 = MICRO;
> +                       return IIO_VAL_FRACTIONAL;
> +               case IIO_EV_DIR_FALLING:
> +                       ret = regmap_read(data->regmap,
> +                                         adxl313_act_thresh_reg[ADXL313_INACTIVITY],
> +                                         &inact_threshold);
> +                       if (ret)
> +                               return ret;

> +                       *val = inact_threshold * 15625;
> +                       *val2 = MICRO;
> +                       return IIO_VAL_FRACTIONAL;
> +               default:
> +                       return -EINVAL;
> +               }
> +       case IIO_EV_INFO_PERIOD:
>                 ret = regmap_read(data->regmap,
> -                                 adxl313_act_thresh_reg[ADXL313_ACTIVITY],
> -                                 &act_threshold);
> +                                 ADXL313_REG_TIME_INACT,
> +                                 &inact_time_s);
>                 if (ret)
>                         return ret;
> -               *val = act_threshold * 15625;
> -               *val2 = MICRO;
> -               return IIO_VAL_FRACTIONAL;
> +               *val = inact_time_s;
> +               return IIO_VAL_INT;
>         default:
>                 return -EINVAL;
>         }

I still don't get what's wrong with helpers for nested switches?
Instead of doing ping-pong with so many lines (due to indentation
changes), just create a helper from the beginning. In this case this
will look more like


  if (nfo == IIO_EV_INFO_VALUE)
    return my_cool_helper_for_THIS_case(...);

Note, I admit that not all the cases may be done like this, but just
look at this again and perhaps something can be optimised.

--
With Best Regards,
Andy Shevchenko

Re: [PATCH v5 0/8] iio: accel: adxl313: add power-save on activity/inactivity

[Andy Shevchenko]

On Mon, Jun 16, 2025 at 1:23 AM Lothar Rubusch <l.rubusch@gmail.com> wrote:
>
> The patch set covers the following topics:
> - add debug register and regmap cache
> - prepare iio channel scan_type and scan_index
> - prepare interrupt handling
> - implement fifo with watermark
> - add activity/inactivity together with auto-sleep with link bit
> - add ac coupled activity/inactivity, integrate with auto-sleep and link bit
> - documentation
>
> Since activity and inactivity here are implemented covering all axis, I
> assumed x&y&z and x|y|z, respectively. Thus the driver uses a fake
> channel for activity/inactiviy. AC-coupling is similar to other Analog Device
> accelerometers, so MAG_ADAPTIVE events are chosen. Combinations are
> documented and functionality tested and verified working.

Reviewed-by: Andy Shevchenko <andy@kernel.org>
for non-commented patches, or where nit-picks can be easily solved and
you agree with the suggestions.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v5 7/8] iio: accel: adxl313: add AC coupled activity/inactivity events

[Andy Shevchenko]

On Mon, Jun 16, 2025 at 1:23 AM Lothar Rubusch <l.rubusch@gmail.com> wrote:
>
> Introduce AC-coupled activity and inactivity as MAG_ADAPTIVE events.
> This adds a new set of threshold and duration configuration options,
> ensures proper handling of event disabling, and extends the use of the
> link bit to support complementary event configurations.
>
> For example, either ACTIVITY or ACTIVITY_AC can be enabled, but only the
> most recently set configuration will remain active. Disabling ACTIVITY
> will have no effect if ACTIVITY_AC is currently enabled, as the event
> types must match (i.e., ACTIVITY_AC must be explicitly disabled). When
> either INACTIVITY or INACTIVITY_AC is enabled alongside an activity
> event, the link bit is set.
>
> With the link bit and auto-sleep enabled, activity and inactivity events
> represent changes in the sensor's power-saving state and are only
> triggered upon actual state transitions. Since AC coupling uses separate
> bits for activity and inactivity, each can be configured independently.
> For instance, ACTIVITY can be linked with INACTIVITY_AC.
>
> If one of the linked events is disabled, the link bit is cleared. In
> that case, the remaining event will no longer reflect a state transition
> but will instead trigger based on periodic inactivity or whenever the
> activity threshold is exceeded.

...

> +/**
> + * adxl313_is_act_inact_ac() - Check if AC coupling is enabled.
> + * @data: The device data.
> + * @type: The activity or inactivity type.
> + *
> + * Provide a type of activity or inactivity, combined with either AC coupling
> + * set, or default to DC coupling. This function verifies, if the combination is

Comma is not needed before "if".

> + * currently enabled or not.
> + *
> + * Return: if the provided activity type has AC coupling enabled or a negative
> + * error value.
> + */

...

>  static int adxl313_is_act_inact_en(struct adxl313_data *data,
>                                    enum adxl313_activity_type type)
>  {
>         unsigned int axis_ctrl;
>         unsigned int regval;
> -       int axis_en, ret;
> +       bool axis_en, int_en;

Why is axis_en not bool to begin with?

> +       int ret;

> +       int_en = adxl313_act_int_reg[type] & regval;
> +       if (!int_en)
> +               return false;
> +
> +       /* Check if configured coupling matches provided type */
> +       return adxl313_is_act_inact_ac(data, type);
>  }

...

> +       act_en = act_en || act_ac_en;

Why is this done here and not after inact_*en checks?

>         inact_en = adxl313_is_act_inact_en(data, ADXL313_INACTIVITY);
>         if (inact_en < 0)
>                 return inact_en;
>
> +       inact_ac_en = adxl313_is_act_inact_en(data, ADXL313_INACTIVITY_AC);
> +       if (inact_ac_en < 0)
> +               return inact_ac_en;

...somewhere here?

> +       inact_en = inact_en || inact_ac_en;
> +
>         en = en && act_en && inact_en;

...

> +               /* When turning off, check if the correct coupling event was
> +                * specified, this can be misused, e.g.: Having AC-coupled
> +                * activity turned on, and in current call trying to turning off
> +                * a DC-coupled activity shall be caught here.
> +                */

/*
 * Wrong multi-line
 * style. Use this example.
 */

Also, please rephrase a bit or split to more sentences as it's harder
to understand now what you meant. Also this doesn't clearly explain
the ' < 0 --> return 0' cases.

> +               if (adxl313_is_act_inact_ac(data, type) <= 0)
> +                       return 0;

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v5 0/8] iio: accel: adxl313: add power-save on activity/inactivity

[Bagas Sanjaya]

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

On Sun, Jun 15, 2025 at 10:22:50PM +0000, Lothar Rubusch wrote:
> base-commit: 7461179e080df770240850a126cc7dbffad195c8
> prerequisite-patch-id: 263cdbf28524f1edc96717db1461d7a4be2319c2

What prerequisite patch?

Confused...

-- 
An old man doll... just what I always wanted! - Clara

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

Re: [PATCH v5 3/8] iio: accel: adxl313: add buffered FIFO watermark with interrupt handling

[Lothar Rubusch]

Hi Andy,

On Mon, Jun 16, 2025 at 11:33 AM Andy Shevchenko
<andy.shevchenko@gmail.com> wrote:
>
> On Mon, Jun 16, 2025 at 1:23 AM Lothar Rubusch <l.rubusch@gmail.com> wrote:
> >
> > Cover the following tasks:
> > – Add scan_mask and scan_index to the IIO channel configuration. The
> > scan_index sets up buffer usage. According to the datasheet, the ADXL313
> > uses a 13-bit wide data field in full-resolution mode. Set the
> > signedness, number of storage bits, and endianness accordingly.
> >
> > – Parse the devicetree for an optional interrupt line and configure the
> > interrupt mapping based on its presence. If no interrupt line is
> > specified, keep the FIFO in bypass mode as currently implemented.
> >
> > – Set up the interrupt handler. Add register access to detect and
> > evaluate interrupts. Implement functions to clear status registers and
> > reset the FIFO.
> >
> > – Implement FIFO watermark configuration and handling. Allow the
> > watermark level to be set, evaluate the corresponding interrupt, read
> > the FIFO contents, and push the data to the IIO channel.
>
> ...
>
> > +       int_line = ADXL313_INT1;
> > +       irq = fwnode_irq_get_byname(dev_fwnode(dev), "INT1");
> > +       if (irq < 0) {
> > +               int_line = ADXL313_INT2;
> > +               irq = fwnode_irq_get_byname(dev_fwnode(dev), "INT2");
> > +               if (irq < 0)
> > +                       int_line = ADXL313_INT_NONE;
> > +       }
> > +
> > +       if (int_line != ADXL313_INT_NONE) {
>
> > +       } else {
>
> > +       }
>
> What I meant is something like this:
>
>
>        int_line = ADXL313_INT_NONE;
>        irq = fwnode_irq_get_byname(dev_fwnode(dev), "INT1");
>        if (irq > 0) {
>               int_line = ADXL313_INT1;
>        } else {
>                irq = fwnode_irq_get_byname(dev_fwnode(dev), "INT2");
>                if (irq > 0)
>                       int_line = ADXL313_INT2;
>        }
>
>        if (int_line == ADXL313_INT_NONE) {
>    ...
>        } else {
>    ...
>        }
>

I probably got this wrong. I interpreted Jonathans review [PATCH v3
06/12] in the above way. Anyway, I did not read his second phrase. I
agree, flipping if / else case and going by '==' instead of '!='
simplifies it.
   ...
    > +
    > + if (int_line == ADXL313_INT1 || int_line == ADXL313_INT2) {

    Why not int_line != ADXL313_INT_NONE ?
    Or flip the logic so that you do that case first.
    ...
https://marc.info/?l=linux-iio&m=174817641830144&w=2


> Obviously with a helper you can unnest the if-else-if above.
>
> static unsigned int _get_int_type(...)
> {
>     if (irq > 0)
>         return ...
>     return _NONE;
> }
>

Well, indeed. That's definitely the obvious simplification needed
here. Thanks for pointing out.

> --
> With Best Regards,
> Andy Shevchenko

Re: [PATCH v5 5/8] iio: accel: adxl313: add inactivity sensing

[Lothar Rubusch]

Hi Andy / Hi Jonathan,

Two questions down below.

On Mon, Jun 16, 2025 at 12:59 PM Andy Shevchenko
<andy.shevchenko@gmail.com> wrote:
>
> On Mon, Jun 16, 2025 at 1:23 AM Lothar Rubusch <l.rubusch@gmail.com> wrote:
> >
[...]
> > +       switch (info) {
> > +       case IIO_EV_INFO_VALUE:
> > +               switch (dir) {
> > +               case IIO_EV_DIR_RISING:
> > +                       ret = regmap_read(data->regmap,
> > +                                         adxl313_act_thresh_reg[ADXL313_ACTIVITY],
> > +                                         &act_threshold);
> > +                       if (ret)
> > +                               return ret;
> > +                       *val = act_threshold * 15625;
> > +                       *val2 = MICRO;
> > +                       return IIO_VAL_FRACTIONAL;
> > +               case IIO_EV_DIR_FALLING:
> > +                       ret = regmap_read(data->regmap,
> > +                                         adxl313_act_thresh_reg[ADXL313_INACTIVITY],
> > +                                         &inact_threshold);
> > +                       if (ret)
> > +                               return ret;
>
> > +                       *val = inact_threshold * 15625;
> > +                       *val2 = MICRO;
> > +                       return IIO_VAL_FRACTIONAL;
> > +               default:
> > +                       return -EINVAL;
> > +               }
> > +       case IIO_EV_INFO_PERIOD:
> >                 ret = regmap_read(data->regmap,
> > -                                 adxl313_act_thresh_reg[ADXL313_ACTIVITY],
> > -                                 &act_threshold);
> > +                                 ADXL313_REG_TIME_INACT,
> > +                                 &inact_time_s);
> >                 if (ret)
> >                         return ret;
> > -               *val = act_threshold * 15625;
> > -               *val2 = MICRO;
> > -               return IIO_VAL_FRACTIONAL;
> > +               *val = inact_time_s;
> > +               return IIO_VAL_INT;
> >         default:
> >                 return -EINVAL;
> >         }
>
> I still don't get what's wrong with helpers for nested switches?
> Instead of doing ping-pong with so many lines (due to indentation
> changes), just create a helper from the beginning. In this case this
> will look more like
>
>
>   if (nfo == IIO_EV_INFO_VALUE)
>     return my_cool_helper_for_THIS_case(...);
>
> Note, I admit that not all the cases may be done like this, but just
> look at this again and perhaps something can be optimised.
>

First, about helpers dealing with nested switches. The resulting
structure then is like

switch (type) {
case IIO_EV_TYPE_MAG:
    switch (info) {
    case IIO_EV_INFO_VALUE:
        switch (dir) {
        case IIO_EV_DIR_RISING:  // activity
            ....
        case IIO_EV_DIR_FALLING: // inactivity
            ....
        }
        case IIO_EV_INFO_PERIOD:
            ...
    }
case IIO_EV_TYPE_MAG_ADAPTIVE:
      // same as above, again for _AC events
 ...
}

Actually I'm using a helper for nested switches. But currently I'm
adding it quite late, when I have cases for ACTIVITY, INACTIVITY and
ACTIVITY_AC and INACTIVITY_AC, since this results in code duplication.
The resulting structure the looks as follows.

helper_mag(...)
{
    switch (info) {
    case IIO_EV_INFO_VALUE:
        switch (dir) {
        case IIO_EV_DIR_RISING:  // activity
            ....
        case IIO_EV_DIR_FALLING: // inactivity
            ....
        }
        case IIO_EV_INFO_PERIOD:
            ...
    }
}

switch (type) {
case IIO_EV_TYPE_MAG:
    helper_mag(...);
case IIO_EV_TYPE_MAG_ADAPTIVE:
        // same as above, again for _AC events
    helper_mag(...);
}

Is this reasonable? For the v6 now, I plan on introducing this helper
when adding INACTIVITY sensing, having ACTIVITY already in place. This
is, because INACTIVITY as distinguishable type is still not available,
but would be needed as function argument as well. Does this make
sense? Or, should I start with the helper right at the beginning? Is
it ok to have once a nested switch in the helper?

Second question is about the adxl313_read_event_config() functions,
I'd like to have here 0 or 1 in regular cases (<0 for error). Is it ok
if I adjust the functions slightly to guarantee this? Currently it
generally returns >0 in cases of "true" which is correct. But this is
most of the times 1, in some cases can be 8 or something. I just like
it to be uniform for testing (which is not a valid argumentation). Is
this legitimate?

Best,
L

> --
> With Best Regards,
> Andy Shevchenko

Re: [PATCH v5 5/8] iio: accel: adxl313: add inactivity sensing

[Andy Shevchenko]

On Tue, Jun 17, 2025 at 1:10 PM Lothar Rubusch <l.rubusch@gmail.com> wrote:
> On Mon, Jun 16, 2025 at 12:59 PM Andy Shevchenko
> <andy.shevchenko@gmail.com> wrote:
> > On Mon, Jun 16, 2025 at 1:23 AM Lothar Rubusch <l.rubusch@gmail.com> wrote:

[...]

> > > +       switch (info) {
> > > +       case IIO_EV_INFO_VALUE:
> > > +               switch (dir) {
> > > +               case IIO_EV_DIR_RISING:
> > > +                       ret = regmap_read(data->regmap,
> > > +                                         adxl313_act_thresh_reg[ADXL313_ACTIVITY],
> > > +                                         &act_threshold);
> > > +                       if (ret)
> > > +                               return ret;
> > > +                       *val = act_threshold * 15625;
> > > +                       *val2 = MICRO;
> > > +                       return IIO_VAL_FRACTIONAL;
> > > +               case IIO_EV_DIR_FALLING:
> > > +                       ret = regmap_read(data->regmap,
> > > +                                         adxl313_act_thresh_reg[ADXL313_INACTIVITY],
> > > +                                         &inact_threshold);
> > > +                       if (ret)
> > > +                               return ret;
> >
> > > +                       *val = inact_threshold * 15625;
> > > +                       *val2 = MICRO;
> > > +                       return IIO_VAL_FRACTIONAL;
> > > +               default:
> > > +                       return -EINVAL;
> > > +               }
> > > +       case IIO_EV_INFO_PERIOD:
> > >                 ret = regmap_read(data->regmap,
> > > -                                 adxl313_act_thresh_reg[ADXL313_ACTIVITY],
> > > -                                 &act_threshold);
> > > +                                 ADXL313_REG_TIME_INACT,
> > > +                                 &inact_time_s);
> > >                 if (ret)
> > >                         return ret;
> > > -               *val = act_threshold * 15625;
> > > -               *val2 = MICRO;
> > > -               return IIO_VAL_FRACTIONAL;
> > > +               *val = inact_time_s;
> > > +               return IIO_VAL_INT;
> > >         default:
> > >                 return -EINVAL;
> > >         }
> >
> > I still don't get what's wrong with helpers for nested switches?
> > Instead of doing ping-pong with so many lines (due to indentation
> > changes), just create a helper from the beginning. In this case this
> > will look more like
> >
> >   if (nfo == IIO_EV_INFO_VALUE)
> >     return my_cool_helper_for_THIS_case(...);
> >
> > Note, I admit that not all the cases may be done like this, but just
> > look at this again and perhaps something can be optimised.
>
> First, about helpers dealing with nested switches. The resulting
> structure then is like
>
> switch (type) {
> case IIO_EV_TYPE_MAG:
>     switch (info) {
>     case IIO_EV_INFO_VALUE:
>         switch (dir) {
>         case IIO_EV_DIR_RISING:  // activity
>             ....
>         case IIO_EV_DIR_FALLING: // inactivity
>             ....
>         }
>         case IIO_EV_INFO_PERIOD:
>             ...
>     }
> case IIO_EV_TYPE_MAG_ADAPTIVE:
>       // same as above, again for _AC events
>  ...
> }
>
> Actually I'm using a helper for nested switches. But currently I'm
> adding it quite late, when I have cases for ACTIVITY, INACTIVITY and
> ACTIVITY_AC and INACTIVITY_AC, since this results in code duplication.
> The resulting structure the looks as follows.
>
> helper_mag(...)
> {
>     switch (info) {
>     case IIO_EV_INFO_VALUE:
>         switch (dir) {
>         case IIO_EV_DIR_RISING:  // activity
>             ....
>         case IIO_EV_DIR_FALLING: // inactivity
>             ....
>         }
>         case IIO_EV_INFO_PERIOD:
>             ...
>     }
> }
>
> switch (type) {
> case IIO_EV_TYPE_MAG:
>     helper_mag(...);
> case IIO_EV_TYPE_MAG_ADAPTIVE:
>         // same as above, again for _AC events
>     helper_mag(...);
> }
>
> Is this reasonable? For the v6 now, I plan on introducing this helper
> when adding INACTIVITY sensing, having ACTIVITY already in place. This
> is, because INACTIVITY as distinguishable type is still not available,
> but would be needed as function argument as well. Does this make
> sense? Or, should I start with the helper right at the beginning? Is
> it ok to have once a nested switch in the helper?

Yes, that's what even would propose here, is to make a helper with the
current code (if it's not empty) and then fill it with the content. In
any case try and see.

The (end) idea is to have only one level of the switch-case per
function in this case and use helpers for the inner ones:

foo()
{
  if (...)
    return -EINVAL;
  switch (dir) {
  case BAZ:
    ...
    break;
  }
}

switch (type) {
case FOO:
  return _do_foo();
}

> Second question is about the adxl313_read_event_config() functions,
> I'd like to have here 0 or 1 in regular cases (<0 for error). Is it ok
> if I adjust the functions slightly to guarantee this? Currently it
> generally returns >0 in cases of "true" which is correct. But this is
> most of the times 1, in some cases can be 8 or something. I just like
> it to be uniform for testing (which is not a valid argumentation). Is
> this legitimate?

If this is an ABI, better to unify this to have the same meaning of
each of the returned values independently on the functions, if this is
just an internal, who cares? However, there is, of course, a corner
case if MSB is set and you will return a (positive) value as an error
code. So, just look at the functions and decide which path you take.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v5 1/8] iio: accel: adxl313: make use of regmap cache

[Jonathan Cameron]

On Sun, 15 Jun 2025 22:22:51 +0000
Lothar Rubusch <l.rubusch@gmail.com> wrote:

> Setup regmap cache to cache register configuration, reducing bus traffic
> for repeated accesses to non volatile registers.
> 
> Signed-off-by: Lothar Rubusch <l.rubusch@gmail.com>
Applied but there was some fuzz. That makes me wonder what
is different between our trees.

Maybe it will become clear later in the patch set.

Thanks

Jonathan

> ---
>  drivers/iio/accel/adxl313.h      |  3 +++
>  drivers/iio/accel/adxl313_core.c | 18 ++++++++++++++++++
>  drivers/iio/accel/adxl313_i2c.c  |  6 ++++++
>  drivers/iio/accel/adxl313_spi.c  |  6 ++++++
>  4 files changed, 33 insertions(+)
> 
> diff --git a/drivers/iio/accel/adxl313.h b/drivers/iio/accel/adxl313.h
> index 72f624af4686..2bc86ac8ffd4 100644
> --- a/drivers/iio/accel/adxl313.h
> +++ b/drivers/iio/accel/adxl313.h
> @@ -22,6 +22,7 @@
>  #define ADXL313_REG_BW_RATE		0x2C
>  #define ADXL313_REG_POWER_CTL		0x2D
>  #define ADXL313_REG_INT_MAP		0x2F
> +#define ADXL313_REG_INT_SOURCE		0x30
>  #define ADXL313_REG_DATA_FORMAT		0x31
>  #define ADXL313_REG_DATA_AXIS(index)	(0x32 + ((index) * 2))
>  #define ADXL313_REG_FIFO_CTL		0x38
> @@ -54,6 +55,8 @@ extern const struct regmap_access_table adxl312_writable_regs_table;
>  extern const struct regmap_access_table adxl313_writable_regs_table;
>  extern const struct regmap_access_table adxl314_writable_regs_table;
>  
> +bool adxl313_is_volatile_reg(struct device *dev, unsigned int reg);
> +
>  enum adxl313_device_type {
>  	ADXL312,
>  	ADXL313,
> diff --git a/drivers/iio/accel/adxl313_core.c b/drivers/iio/accel/adxl313_core.c
> index 2f26da5857d4..39f16f97bb4a 100644
> --- a/drivers/iio/accel/adxl313_core.c
> +++ b/drivers/iio/accel/adxl313_core.c
> @@ -46,6 +46,24 @@ const struct regmap_access_table adxl314_readable_regs_table = {
>  };
>  EXPORT_SYMBOL_NS_GPL(adxl314_readable_regs_table, IIO_ADXL313);
>  
> +bool adxl313_is_volatile_reg(struct device *dev, unsigned int reg)
> +{
> +	switch (reg) {
> +	case ADXL313_REG_DATA_AXIS(0):
> +	case ADXL313_REG_DATA_AXIS(1):
> +	case ADXL313_REG_DATA_AXIS(2):
> +	case ADXL313_REG_DATA_AXIS(3):
> +	case ADXL313_REG_DATA_AXIS(4):
> +	case ADXL313_REG_DATA_AXIS(5):
> +	case ADXL313_REG_FIFO_STATUS:
> +	case ADXL313_REG_INT_SOURCE:
> +		return true;
> +	default:
> +		return false;
> +	}
> +}
> +EXPORT_SYMBOL_NS_GPL(adxl313_is_volatile_reg, "IIO_ADXL313");
> +
>  static int adxl312_check_id(struct device *dev,
>  			    struct adxl313_data *data)
>  {
> diff --git a/drivers/iio/accel/adxl313_i2c.c b/drivers/iio/accel/adxl313_i2c.c
> index a4cf0cf2c5aa..e8636e8ab14f 100644
> --- a/drivers/iio/accel/adxl313_i2c.c
> +++ b/drivers/iio/accel/adxl313_i2c.c
> @@ -21,6 +21,8 @@ static const struct regmap_config adxl31x_i2c_regmap_config[] = {
>  		.rd_table	= &adxl312_readable_regs_table,
>  		.wr_table	= &adxl312_writable_regs_table,
>  		.max_register	= 0x39,
> +		.volatile_reg	= adxl313_is_volatile_reg,
> +		.cache_type	= REGCACHE_MAPLE,
>  	},
>  	[ADXL313] = {
>  		.reg_bits	= 8,
> @@ -28,6 +30,8 @@ static const struct regmap_config adxl31x_i2c_regmap_config[] = {
>  		.rd_table	= &adxl313_readable_regs_table,
>  		.wr_table	= &adxl313_writable_regs_table,
>  		.max_register	= 0x39,
> +		.volatile_reg	= adxl313_is_volatile_reg,
> +		.cache_type	= REGCACHE_MAPLE,
>  	},
>  	[ADXL314] = {
>  		.reg_bits	= 8,
> @@ -35,6 +39,8 @@ static const struct regmap_config adxl31x_i2c_regmap_config[] = {
>  		.rd_table	= &adxl314_readable_regs_table,
>  		.wr_table	= &adxl314_writable_regs_table,
>  		.max_register	= 0x39,
> +		.volatile_reg	= adxl313_is_volatile_reg,
> +		.cache_type	= REGCACHE_MAPLE,
>  	},
>  };
>  
> diff --git a/drivers/iio/accel/adxl313_spi.c b/drivers/iio/accel/adxl313_spi.c
> index 9a16b40bff34..68e323e81aeb 100644
> --- a/drivers/iio/accel/adxl313_spi.c
> +++ b/drivers/iio/accel/adxl313_spi.c
> @@ -24,6 +24,8 @@ static const struct regmap_config adxl31x_spi_regmap_config[] = {
>  		.max_register	= 0x39,
>  		/* Setting bits 7 and 6 enables multiple-byte read */
>  		.read_flag_mask	= BIT(7) | BIT(6),
> +		.volatile_reg	= adxl313_is_volatile_reg,
> +		.cache_type	= REGCACHE_MAPLE,
>  	},
>  	[ADXL313] = {
>  		.reg_bits	= 8,
> @@ -33,6 +35,8 @@ static const struct regmap_config adxl31x_spi_regmap_config[] = {
>  		.max_register	= 0x39,
>  		/* Setting bits 7 and 6 enables multiple-byte read */
>  		.read_flag_mask	= BIT(7) | BIT(6),
> +		.volatile_reg	= adxl313_is_volatile_reg,
> +		.cache_type	= REGCACHE_MAPLE,
>  	},
>  	[ADXL314] = {
>  		.reg_bits	= 8,
> @@ -42,6 +46,8 @@ static const struct regmap_config adxl31x_spi_regmap_config[] = {
>  		.max_register	= 0x39,
>  		/* Setting bits 7 and 6 enables multiple-byte read */
>  		.read_flag_mask	= BIT(7) | BIT(6),
> +		.volatile_reg	= adxl313_is_volatile_reg,
> +		.cache_type	= REGCACHE_MAPLE,
>  	},
>  };
>  

Re: [PATCH v5 2/8] iio: accel: adxl313: add function to enable measurement

[Jonathan Cameron]

On Sun, 15 Jun 2025 22:22:52 +0000
Lothar Rubusch <l.rubusch@gmail.com> wrote:

> Refactor the control of measurement and standby modes for the sensor.
> Instead of directly writing to the register, encapsulate this operation
> in a dedicated function that handles enabling and disabling measurement.
> This approach will reduce code duplication wherever sensor configuration
> changes are required. In subsequent patches, measurement mode will be
> set to standby as part of this process.
> 
> Additionally, simplify the control mask to include only the measurement
> bit. The sleep bit governs a different behavior—putting the sensor into
> sleep mode, not just standby for configuration—and is currently unused.
> Therefore, there's no need to include both the sleep and measurement
> bits in the same mask.
> 
> Signed-off-by: Lothar Rubusch <l.rubusch@gmail.com>
Applied.

Re: [PATCH v5 3/8] iio: accel: adxl313: add buffered FIFO watermark with interrupt handling

[Jonathan Cameron]

On Sun, 15 Jun 2025 22:22:53 +0000
Lothar Rubusch <l.rubusch@gmail.com> wrote:

> Cover the following tasks:
> – Add scan_mask and scan_index to the IIO channel configuration. The
> scan_index sets up buffer usage. According to the datasheet, the ADXL313
> uses a 13-bit wide data field in full-resolution mode. Set the
> signedness, number of storage bits, and endianness accordingly.
> 
> – Parse the devicetree for an optional interrupt line and configure the
> interrupt mapping based on its presence. If no interrupt line is
> specified, keep the FIFO in bypass mode as currently implemented.
> 
> – Set up the interrupt handler. Add register access to detect and
> evaluate interrupts. Implement functions to clear status registers and
> reset the FIFO.
> 
> – Implement FIFO watermark configuration and handling. Allow the
> watermark level to be set, evaluate the corresponding interrupt, read
> the FIFO contents, and push the data to the IIO channel.
> 
> Signed-off-by: Lothar Rubusch <l.rubusch@gmail.com>

Just the follow on comment that I posted on v4 thread after you sent this.
(obviously in addition to what you've been discussing with Andy)

> diff --git a/drivers/iio/accel/adxl313_core.c b/drivers/iio/accel/adxl313_core.c
> index 99a7f3755031..488680807a8f 100644
> --- a/drivers/iio/accel/adxl313_core.c
> +++ b/drivers/iio/accel/adxl313_core.c

> +
> +static int adxl313_fifo_transfer(struct adxl313_data *data, int samples)
> +{
> +	unsigned int i;
> +	int ret;
> +
> +	for (i = 0; i < samples; i++) {
> +		ret = regmap_bulk_read(data->regmap, ADXL313_REG_XYZ_BASE,
> +				       data->fifo_buf + (i * ADXL313_NUM_AXIS),
> +				       2 * ADXL313_NUM_AXIS);

the sizeof() thing that I gave a late reply to in v4 thread applies here.

> +		if (ret)
> +			return ret;
> +	}
> +
> +	return 0;
> +}

Re: [PATCH v5 7/8] iio: accel: adxl313: add AC coupled activity/inactivity events

[Jonathan Cameron]

On Sun, 15 Jun 2025 22:22:57 +0000
Lothar Rubusch <l.rubusch@gmail.com> wrote:

> Introduce AC-coupled activity and inactivity as MAG_ADAPTIVE events.
> This adds a new set of threshold and duration configuration options,
> ensures proper handling of event disabling, and extends the use of the
> link bit to support complementary event configurations.
> 
> For example, either ACTIVITY or ACTIVITY_AC can be enabled, but only the
> most recently set configuration will remain active. Disabling ACTIVITY
> will have no effect if ACTIVITY_AC is currently enabled, as the event
> types must match (i.e., ACTIVITY_AC must be explicitly disabled). When
> either INACTIVITY or INACTIVITY_AC is enabled alongside an activity
> event, the link bit is set.
> 
> With the link bit and auto-sleep enabled, activity and inactivity events
> represent changes in the sensor's power-saving state and are only
> triggered upon actual state transitions. Since AC coupling uses separate
> bits for activity and inactivity, each can be configured independently.
> For instance, ACTIVITY can be linked with INACTIVITY_AC.
> 
> If one of the linked events is disabled, the link bit is cleared. In
> that case, the remaining event will no longer reflect a state transition
> but will instead trigger based on periodic inactivity or whenever the
> activity threshold is exceeded.
> 
> Signed-off-by: Lothar Rubusch <l.rubusch@gmail.com>

One small suggestion below.

>  
>  static int adxl313_set_act_inact_linkbit(struct adxl313_data *data, bool en)
>  {
> -	int act_en, inact_en;
> +	int act_en, inact_en, act_ac_en, inact_ac_en;
>  
>  	act_en = adxl313_is_act_inact_en(data, ADXL313_ACTIVITY);
>  	if (act_en < 0)
>  		return act_en;
>  
> +	act_ac_en = adxl313_is_act_inact_en(data, ADXL313_ACTIVITY_AC);
> +	if (act_ac_en < 0)
> +		return act_ac_en;
> +
> +	act_en = act_en || act_ac_en;

All ends up a little confusing as act_en changes meaning as it is built
up.  Maybe better to just have act_dc_en in earlier patch then so
the boolean complexity all in one place?...

> +
>  	inact_en = adxl313_is_act_inact_en(data, ADXL313_INACTIVITY);
>  	if (inact_en < 0)
>  		return inact_en;
>  
> +	inact_ac_en = adxl313_is_act_inact_en(data, ADXL313_INACTIVITY_AC);
> +	if (inact_ac_en < 0)
> +		return inact_ac_en;
> +
> +	inact_en = inact_en || inact_ac_en;
> +
>  	en = en && act_en && inact_en;

	en = en && (act_dc_en || act_ac_en) && (inact_dc_en || inact_ac_en);

>  
>  	return regmap_assign_bits(data->regmap, ADXL313_REG_POWER_CTL,

Re: [PATCH v5 0/8] iio: accel: adxl313: add power-save on activity/inactivity

[Lothar Rubusch]

Hi Bagas Sanjaya

Sry, for the late reply.

On Tue, Jun 17, 2025 at 3:14 AM Bagas Sanjaya <bagasdotme@gmail.com> wrote:
>
> On Sun, Jun 15, 2025 at 10:22:50PM +0000, Lothar Rubusch wrote:
> > base-commit: 7461179e080df770240850a126cc7dbffad195c8
> > prerequisite-patch-id: 263cdbf28524f1edc96717db1461d7a4be2319c2
>
> What prerequisite patch?
>
> Confused...
>

Interesting, tbh I'm not sure about the "prerequisite-patch-id",
either. AFAIK there is some git command to verify and use it as rather
an ID of the patch (series), as far as I can see in the documentation
for format-patch:
https://git-scm.com/docs/git-format-patch#_base_tree_information

As mentioned the series is based on this fork and branch:
https://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git/log/?h=testing

Actually, the base commit should be possible to identify (at least I
could identify it in tig). AFAIK - or, as I needed to learn myself in
the past - usually they check the series automatically first to rule
out such things. E.g. they execute checkpatch, smatch, in case DT
check,... on it. If anything fails here, I guess I would have received
feedback within about 24h. Since there was no feedback, I assume there
should not be an issue.

Best regards,
Lothar

> --
> An old man doll... just what I always wanted! - Clara