Re: [PATCH v3 14/15] iio: accel: adxl345: add coupling detection for activity/inactivity

[Jonathan Cameron <jic23@kernel.org>]

On Thu, 20 Feb 2025 10:42:33 +0000
Lothar Rubusch <l.rubusch@gmail.com> wrote:

> Add coupling activity/inactivity detection by the AC/DC bit. This is an
> addititional enhancement for the detection of activity states and
> completes the activity / inactivity feature of the ADXL345.
> 
> Signed-off-by: Lothar Rubusch <l.rubusch@gmail.com>
> @@ -337,6 +356,69 @@ static int adxl345_write_act_axis(struct adxl345_state *st,
>  	return 0;
>  }
>  
> +static int adxl345_is_act_inact_ac(struct adxl345_state *st,
> +				   enum adxl345_activity_type type, bool *ac)
> +{
> +	unsigned int regval;
> +	int ret;
> +
> +	ret = regmap_read(st->regmap, ADXL345_REG_ACT_INACT_CTRL, &regval);
> +	if (ret)
> +		return ret;
> +
> +	if (type == ADXL345_ACTIVITY)
> +		*ac = (FIELD_GET(ADXL345_REG_ACT_ACDC_MSK, regval) > 0) ? true : false;
> +	else
> +		*ac = (FIELD_GET(ADXL345_REG_INACT_ACDC_MSK, regval) > 0) ? true : false;
> +
Why the ternaries? 
		*ac = FIELD_GET() > 0;
is a boolean right hand side anyway.

> +	return 0;
> +}
> +
> +static int adxl345_set_act_inact_ac(struct adxl345_state *st,
> +				    enum adxl345_activity_type type, bool ac)
> +{
> +	unsigned int act_inact_ac = ac ? 0xff : 0x00;
> +
> +	/*
> +	 * A setting of 0 selects dc-coupled operation, and a setting of 1
false and true rather than 0 / 1?
> +	 * enables ac-coupled operation. In dc-coupled operation, the current
> +	 * acceleration magnitude is compared directly with
> +	 * ADXL345_REG_THRESH_ACT and ADXL345_REG_THRESH_INACT to determine
> +	 * whether activity or inactivity is detected.
...
>  static int adxl345_is_act_inact_en(struct adxl345_state *st,
>  				   enum adxl345_activity_type type, bool *en)
>  {
> @@ -695,6 +777,11 @@ static int adxl345_set_odr(struct adxl345_state *st, enum adxl345_odr odr)
>  	if (ret)
>  		return ret;
>  
> +	/* update inactivity time by ODR */
> +	ret = adxl345_set_inact_time_s(st, 0);
> +	if (ret)
> +		return ret;
> +
>  	return 0;

return adxl345_set_inact_time_s()

>  }
>  
> @@ -718,14 +805,54 @@ static int adxl345_find_range(struct adxl345_state *st, int val, int val2,
>  
>  static int adxl345_set_range(struct adxl345_state *st, enum adxl345_range range)
>  {
> +	unsigned int act_threshold, inact_threshold;
> +	unsigned int range_old;
> +	unsigned int regval;
>  	int ret;
>  
> +	ret = regmap_read(st->regmap, ADXL345_REG_DATA_FORMAT, &regval);
> +	if (ret)
> +		return ret;
> +	range_old = FIELD_GET(ADXL345_DATA_FORMAT_RANGE, regval);
> +
> +	ret = regmap_read(st->regmap,
> +			  adxl345_act_thresh_reg[ADXL345_ACTIVITY],
> +			  &act_threshold);
> +	if (ret)
> +		return ret;
> +
> +	ret = regmap_read(st->regmap,
> +			  adxl345_act_thresh_reg[ADXL345_INACTIVITY],
> +			  &inact_threshold);
> +	if (ret)
> +		return ret;
> +
>  	ret = regmap_update_bits(st->regmap, ADXL345_REG_DATA_FORMAT,
>  				 ADXL345_DATA_FORMAT_RANGE,
>  				 FIELD_PREP(ADXL345_DATA_FORMAT_RANGE, range));
>  	if (ret)
>  		return ret;
>  
> +	act_threshold = act_threshold
> +		* adxl345_range_factor_tbl[range_old]
> +		/ adxl345_range_factor_tbl[range];
> +	act_threshold = min(255, max(1, inact_threshold));
> +
> +	inact_threshold = inact_threshold
> +		* adxl345_range_factor_tbl[range_old]
> +		/ adxl345_range_factor_tbl[range];
> +	inact_threshold = min(255, max(1, inact_threshold));
> +
> +	ret = regmap_write(st->regmap, adxl345_act_thresh_reg[ADXL345_ACTIVITY],
> +			   act_threshold);
> +	if (ret)
> +		return ret;
> +
> +	ret = regmap_write(st->regmap, adxl345_act_thresh_reg[ADXL345_INACTIVITY],
> +			   inact_threshold);
> +	if (ret)
> +		return ret;
return regmap_write()
> +
>  	return 0;
>  }