linux-kernel - Re: [PATCH v7 09/11] 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: <20250427133847.6f3b164c@jic23-huawei>
Date: Sun, 27 Apr 2025 13:38:47 +0100
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 v7 09/11] iio: accel: adxl345: add inactivity feature

On Mon, 21 Apr 2025 22:06:39 +0000
Lothar Rubusch <l.rubusch@...il.com> wrote:

> Add the inactivity feature of the sensor. When activity and inactivity
> are enabled, a link bit will be set linking activity and inactivity
> handling. Additionally, the auto-sleep mode will be enabled. Due to the
> link bit the sensor is going to auto-sleep when inactivity was
> detected.
> 
> Inactivity detection needs a threshold to be configured, and a time
> after which it will go into inactivity state if measurements under
> threshold.
> 
> When a ODR is configured this time for inactivity is adjusted with a
> corresponding reasonable default value, in order to have higher
> frequencies and lower inactivity times, and lower sample frequency but
> give more time until inactivity. Both with reasonable upper and lower
> boundaries, since many of the sensor's features (e.g. auto-sleep) will
> need to operate beween 12.5 Hz and 400 Hz. This is a default setting
> when actively changing sample frequency, explicitly setting the time
> until inactivity will overwrite the default.
> 
> Similarly, setting the g-range will provide a default value for the
> activity and inactivity thresholds. Both are implicit defaults, but
> equally can be overwritten to be explicitly configured.
> 
> Signed-off-by: Lothar Rubusch <l.rubusch@...il.com>

Tweaked - see below for why.  Please sanity check I didn't mess it up!


diff --git a/drivers/iio/accel/adxl345_core.c b/drivers/iio/accel/adxl345_core.c
index dd8cea5c48f3..a61c97c086e9 100644
--- a/drivers/iio/accel/adxl345_core.c
+++ b/drivers/iio/accel/adxl345_core.c
@@ -241,16 +241,14 @@ static const struct iio_event_spec adxl345_all_event_spec[] = {
                .mask_shared_by_type = BIT(IIO_EV_INFO_VALUE) |
                BIT(IIO_EV_INFO_PERIOD),
        },
-};
-
-/* inactivity */
-static const struct iio_event_spec adxl345_inactivity_event_spec = {
+       /* inactivity */
+       {
                .type = IIO_EV_TYPE_THRESH,
                .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),
-
+               BIT(IIO_EV_INFO_PERIOD),
+       },
 };
 
 static const struct iio_chan_spec adxl345_channels[] = {
@@ -265,14 +263,6 @@ static const struct iio_chan_spec adxl345_channels[] = {
                .event_spec = adxl345_all_event_spec,
                .num_event_specs = ARRAY_SIZE(adxl345_all_event_spec),
        },
-       {
-               .type = IIO_ACCEL,
-               .modified = 1,
-               .channel2 = IIO_MOD_X_AND_Y_AND_Z,
-               .scan_index = -1, /* Fake channel */
-               .event_spec = &adxl345_inactivity_event_spec,
-               .num_event_specs = 1,
-       },
 };
 
 static const unsigned long adxl345_scan_masks[] = {

> ---
>  drivers/iio/accel/adxl345_core.c | 172 ++++++++++++++++++++++++++++++-
>  1 file changed, 167 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/iio/accel/adxl345_core.c b/drivers/iio/accel/adxl345_core.c
> index 680981609d83..b25efcad069b 100644
> --- a/drivers/iio/accel/adxl345_core.c
> +++ b/drivers/iio/accel/adxl345_core.c
> @@ -37,11 +37,17 @@

>  
> +/* inactivity */
> +static const struct iio_event_spec adxl345_inactivity_event_spec = {
> +		.type = IIO_EV_TYPE_THRESH,
> +		.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),
> +
> +};
> +
>  static const struct iio_chan_spec adxl345_channels[] = {
>  	ADXL345_CHANNEL(0, chan_x, X),
>  	ADXL345_CHANNEL(1, chan_y, Y),
> @@ -244,6 +263,14 @@ static const struct iio_chan_spec adxl345_channels[] = {
>  		.event_spec = &adxl345_freefall_event_spec,
>  		.num_event_specs = 1,
>  	},
> +	{
> +		.type = IIO_ACCEL,
> +		.modified = 1,
> +		.channel2 = IIO_MOD_X_AND_Y_AND_Z,
> +		.scan_index = -1, /* Fake channel */
> +		.event_spec = &adxl345_inactivity_event_spec,
> +		.num_event_specs = 1,
Why do we need a separate fake channel for this?

Should reuse the one that we have for freefall which has the same definition.

+	{
+		.type = IIO_ACCEL,
+		.modified = 1,
+		.channel2 = IIO_MOD_X_AND_Y_AND_Z,
+		.scan_index = -1, /* Fake channel */
+		.event_spec = &adxl345_freefall_event_spec,
+		.num_event_specs = 1,
+	},

> +	},
>  };
>  
>  static const unsigned long adxl345_scan_masks[] = {
> @@ -287,7 +314,8 @@ static int adxl345_set_measure_en(struct adxl345_state *st, bool en)
>  {
>  	unsigned int val = en ? ADXL345_POWER_CTL_MEASURE : ADXL345_POWER_CTL_STANDBY;
>  
> -	return regmap_write(st->regmap, ADXL345_REG_POWER_CTL, val);
> +	return regmap_update_bits(st->regmap, ADXL345_REG_POWER_CTL,
> +				  ADXL345_POWER_CTL_MEASURE, val);
>  }