linux-kernel - Re: [PATCH v11 4/8] iio: accel: adxl345: add inactivity 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: <aGaTH6gVqHxn9Xct@smile.fi.intel.com>
Date: Thu, 3 Jul 2025 17:26:39 +0300
From: Andy Shevchenko <andriy.shevchenko@...el.com>
To: Lothar Rubusch <l.rubusch@...il.com>
Cc: lars@...afoo.de, Michael.Hennerich@...log.com, jic23@...nel.org,
	dlechner@...libre.com, nuno.sa@...log.com, andy@...nel.org,
	corbet@....net, linux-iio@...r.kernel.org,
	linux-kernel@...r.kernel.org, linux-doc@...r.kernel.org,
	eraretuya@...il.com
Subject: Re: [PATCH v11 4/8] iio: accel: adxl345: add inactivity feature

On Wed, Jul 02, 2025 at 11:03:11PM +0000, Lothar Rubusch wrote:
> Add support for the sensor’s inactivity feature in the driver. When both
> activity and inactivity detection are enabled, the sensor sets a link bit
> that ties the two functions together. This also enables auto-sleep mode,
> allowing the sensor to automatically enter sleep state upon detecting
> inactivity.
> 
> Inactivity detection relies on a configurable threshold and a specified
> time period. If sensor measurements remain below the threshold for the
> defined duration, the sensor transitions to the inactivity state.
> 
> When an Output Data Rate (ODR) is set, the inactivity time period is
> automatically adjusted to a sensible default. Higher ODRs result in shorter
> inactivity timeouts, while lower ODRs allow longer durations-within
> reasonable upper and lower bounds. This is important because features like
> auto-sleep operate effectively only between 12.5 Hz and 400 Hz. These
> defaults are applied when the sample rate is modified, but users can
> override them by explicitly setting a custom inactivity timeout.
> 
> Similarly, configuring the g-range provides default threshold values for
> both activity and inactivity detection. These are implicit defaults meant
> to simplify configuration, but they can also be manually overridden as
> needed.

...

>  #define ADXL345_REG_TAP_SUPPRESS_MSK	BIT(3)
>  #define ADXL345_REG_TAP_SUPPRESS	BIT(3)
>  #define ADXL345_REG_ACT_AXIS_MSK	GENMASK(6, 4)
> +#define ADXL345_REG_INACT_AXIS_MSK	GENMASK(2, 0)
> +#define ADXL345_POWER_CTL_INACT_MSK	(ADXL345_POWER_CTL_AUTO_SLEEP | ADXL345_POWER_CTL_LINK)
>  
>  #define ADXL345_TAP_Z_EN		BIT(0)
>  #define ADXL345_TAP_Y_EN		BIT(1)
>  #define ADXL345_TAP_X_EN		BIT(2)
>  
> +#define ADXL345_INACT_Z_EN		BIT(0)
> +#define ADXL345_INACT_Y_EN		BIT(1)
> +#define ADXL345_INACT_X_EN		BIT(2)
> +#define ADXL345_INACT_XYZ_EN		(ADXL345_INACT_Z_EN | ADXL345_INACT_Y_EN | ADXL345_INACT_X_EN)
> +
>  #define ADXL345_ACT_Z_EN		BIT(4)
>  #define ADXL345_ACT_Y_EN		BIT(5)
>  #define ADXL345_ACT_X_EN		BIT(6)

Now it's even more mess. I am lost in understanding which bits/masks are from
the same offset and which are not.

...

> +		.mask_shared_by_type = BIT(IIO_EV_INFO_VALUE) |
> +			BIT(IIO_EV_INFO_PERIOD),

Same comment about indentation style.

-- 
With Best Regards,
Andy Shevchenko