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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20250316113203.695b69e8@jic23-huawei>
Date: Sun, 16 Mar 2025 11:32:03 +0000
From: Jonathan Cameron <jic23@...nel.org>
To: Lothar Rubusch <l.rubusch@...il.com>
Cc: lars@...afoo.de, Michael.Hennerich@...log.com,
 linux-iio@...r.kernel.org, linux-kernel@...r.kernel.org,
 eraretuya@...il.com
Subject: Re: [PATCH v4 11/14] iio: accel: adxl345: add activity event
 feature

On Thu, 13 Mar 2025 16:50:46 +0000
Lothar Rubusch <l.rubusch@...il.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@...il.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;
>  	}
>