Re: [PATCH v11 4/8] iio: accel: adxl345: add inactivity feature

[Andy Shevchenko <andriy.shevchenko@intel.com>]

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