linux-kernel - Re: [PATCH v5 7/8] iio: accel: adxl313: add AC coupled activity/inactivity events

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 for Android: free password hash cracker in your pocket

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

Message-ID: <20250622123752.1834fbe2@jic23-huawei>
Date: Sun, 22 Jun 2025 12:37:52 +0100
From: Jonathan Cameron <jic23@...nel.org>
To: Lothar Rubusch <l.rubusch@...il.com>
Cc: dlechner@...libre.com, nuno.sa@...log.com, andy@...nel.org,
 corbet@....net, lucas.p.stankus@...il.com, lars@...afoo.de,
 Michael.Hennerich@...log.com, bagasdotme@...il.com,
 linux-iio@...r.kernel.org, linux-doc@...r.kernel.org,
 linux-kernel@...r.kernel.org
Subject: Re: [PATCH v5 7/8] iio: accel: adxl313: add AC coupled
 activity/inactivity events

On Sun, 15 Jun 2025 22:22:57 +0000
Lothar Rubusch <l.rubusch@...il.com> wrote:

> Introduce AC-coupled activity and inactivity as MAG_ADAPTIVE events.
> This adds a new set of threshold and duration configuration options,
> ensures proper handling of event disabling, and extends the use of the
> link bit to support complementary event configurations.
> 
> For example, either ACTIVITY or ACTIVITY_AC can be enabled, but only the
> most recently set configuration will remain active. Disabling ACTIVITY
> will have no effect if ACTIVITY_AC is currently enabled, as the event
> types must match (i.e., ACTIVITY_AC must be explicitly disabled). When
> either INACTIVITY or INACTIVITY_AC is enabled alongside an activity
> event, the link bit is set.
> 
> With the link bit and auto-sleep enabled, activity and inactivity events
> represent changes in the sensor's power-saving state and are only
> triggered upon actual state transitions. Since AC coupling uses separate
> bits for activity and inactivity, each can be configured independently.
> For instance, ACTIVITY can be linked with INACTIVITY_AC.
> 
> If one of the linked events is disabled, the link bit is cleared. In
> that case, the remaining event will no longer reflect a state transition
> but will instead trigger based on periodic inactivity or whenever the
> activity threshold is exceeded.
> 
> Signed-off-by: Lothar Rubusch <l.rubusch@...il.com>

One small suggestion below.

>  
>  static int adxl313_set_act_inact_linkbit(struct adxl313_data *data, bool en)
>  {
> -	int act_en, inact_en;
> +	int act_en, inact_en, act_ac_en, inact_ac_en;
>  
>  	act_en = adxl313_is_act_inact_en(data, ADXL313_ACTIVITY);
>  	if (act_en < 0)
>  		return act_en;
>  
> +	act_ac_en = adxl313_is_act_inact_en(data, ADXL313_ACTIVITY_AC);
> +	if (act_ac_en < 0)
> +		return act_ac_en;
> +
> +	act_en = act_en || act_ac_en;

All ends up a little confusing as act_en changes meaning as it is built
up.  Maybe better to just have act_dc_en in earlier patch then so
the boolean complexity all in one place?...

> +
>  	inact_en = adxl313_is_act_inact_en(data, ADXL313_INACTIVITY);
>  	if (inact_en < 0)
>  		return inact_en;
>  
> +	inact_ac_en = adxl313_is_act_inact_en(data, ADXL313_INACTIVITY_AC);
> +	if (inact_ac_en < 0)
> +		return inact_ac_en;
> +
> +	inact_en = inact_en || inact_ac_en;
> +
>  	en = en && act_en && inact_en;

	en = en && (act_dc_en || act_ac_en) && (inact_dc_en || inact_ac_en);

>  
>  	return regmap_assign_bits(data->regmap, ADXL313_REG_POWER_CTL,