Re: [PATCH v4 11/14] iio: accel: adxl345: add activity event feature

[Jonathan Cameron <jic23@kernel.org>]

On Thu, 13 Mar 2025 16:50:46 +0000
Lothar Rubusch <l.rubusch@gmail.com> wrote:

> Make the sensor detect and issue interrupts at activity. Activity
> events are configured by a threshold stored in regmap cache. Initialize
> the activity threshold register to a reasonable default value in probe.
> The value is taken from the older ADXL345 input driver, to provide a
> similar behavior. Reset the activity/inactivity direction enabling
> register in probe. Reset and initialization shall bring the sensor in a
> defined initial state to prevent dangling settings when warm restarting
> the sensor.
> 
> Activity, ODR configuration together with the range setting prepare the
> activity/inactivity hystersesis setup, implemented in a follow up patch.
> 
> Signed-off-by: Lothar Rubusch <l.rubusch@gmail.com>

A few things inline.

Jonathan

> @@ -1176,7 +1373,8 @@ static irqreturn_t adxl345_irq_handler(int irq, void *p)
>  	if (ret)
>  		return IRQ_NONE;
>  
> -	if (FIELD_GET(ADXL345_REG_TAP_AXIS_MSK, axis_ctrl)) {
> +	if (FIELD_GET(ADXL345_REG_TAP_AXIS_MSK, axis_ctrl) ||
> +	    FIELD_GET(ADXL345_REG_ACT_AXIS_MSK, axis_ctrl)) {
>  		ret = regmap_read(st->regmap, ADXL345_REG_ACT_TAP_STATUS, &regval);
>  		if (ret)
>  			return IRQ_NONE;
> @@ -1187,12 +1385,22 @@ static irqreturn_t adxl345_irq_handler(int irq, void *p)
>  			tap_dir = IIO_MOD_Y;
>  		else if (FIELD_GET(ADXL345_X_EN, regval))
>  			tap_dir = IIO_MOD_X;
> +
> +		/* Activity direction is stored in the upper four bits */
> +		regval >>= act_shift;
As per feedback on v3 (late feedback that is), just add more defines
for these bits as the reuse adds connections that are kind of
coincidence rather than fundamental (bits 'could' have been defined
in any order or non contiguous) and lead to less readable code.
> +
> +		if (FIELD_GET(ADXL345_Z_EN, regval))
> +			act_dir = IIO_MOD_Z;
> +		else if (FIELD_GET(ADXL345_Y_EN, regval))
> +			act_dir = IIO_MOD_Y;
> +		else if (FIELD_GET(ADXL345_X_EN, regval))
> +			act_dir = IIO_MOD_X;
>  	}
>