linux-kernel - Re: [PATCH v4 08/11] iio: accel: adxl313: add inactivity sensing

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <20250608171425.11269bf0@jic23-huawei>
Date: Sun, 8 Jun 2025 17:14:25 +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 v4 08/11] iio: accel: adxl313: add inactivity sensing

On Sun,  1 Jun 2025 17:21:36 +0000
Lothar Rubusch <l.rubusch@...il.com> wrote:

> Extend the interrupt handler to process interrupts as inactivity events.
> Add functions to set threshold and period registers for inactivity. Add
> functions to enable / disable inactivity. Extend the fake iio channel to
> deal with inactivity events on x, y and z combined with AND.
> 
> Signed-off-by: Lothar Rubusch <l.rubusch@...il.com>

> @@ -555,16 +621,30 @@ static int adxl313_write_event_value(struct iio_dev *indio_dev,
>  	if (type != IIO_EV_TYPE_MAG)
>  		return -EINVAL;
>  
> -	if (info != IIO_EV_INFO_VALUE)
> -		return -EINVAL;
> -
> -	/* Scale factor 15.625 mg/LSB */
> -	regval = DIV_ROUND_CLOSEST(MICRO * val + val2, 15625);
> -	switch (dir) {
> -	case IIO_EV_DIR_RISING:
> -		ret = regmap_write(data->regmap,
> -				   adxl313_act_thresh_reg[ADXL313_ACTIVITY],
> -				   regval);
> +	switch (info) {
> +	case IIO_EV_INFO_VALUE:
> +		/* Scale factor 15.625 mg/LSB */
> +		regval = DIV_ROUND_CLOSEST(MICRO * val + val2, 15625);
> +		switch (dir) {
> +		case IIO_EV_DIR_RISING:
> +			ret = regmap_write(data->regmap,
> +					   adxl313_act_thresh_reg[ADXL313_ACTIVITY],
> +					   regval);
> +			if (ret)
> +				return ret;
> +			return adxl313_set_measure_en(data, true);
> +		case IIO_EV_DIR_FALLING:
> +			ret = regmap_write(data->regmap,
> +					   adxl313_act_thresh_reg[ADXL313_INACTIVITY],
> +					   regval);
> +			if (ret)
> +				return ret;
> +			return adxl313_set_measure_en(data, true);
> +		default:
> +			return -EINVAL;
> +		}
> +	case IIO_EV_INFO_PERIOD:
> +		ret = adxl313_set_inact_time_s(data, val);
>  		if (ret)
>  			return ret;
>  		return adxl313_set_measure_en(data, true);

Having the enable in the case statement but he disable outside is misbalanced.
Do it one way or the other (either always disable / enable outside, or both
inside each relevant case statement.   If we need to do this, I'd have
a helper function do_adxl313_write_event_value() or similar that is called
between disabling and enabling measurement mode and contains all the other
stuff in this function (or along those lines anyway).  You can chose
whether or not to reenable measurement mode depending on what is returned
by the helper function.