Re: [PATCH v3 11/15] iio: accel: adxl345: add g-range configuration

[Jonathan Cameron <jic23@kernel.org>]

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

> Introduce means to configure and work with the available g-ranges
> keeping the precision of 13 digits.
> 
> This is in preparation for the activity/inactivity feature.

I'm not really following why adding range control is anything
much to do with that. Mostly we do this to improve accuracy for
low accelerations.

> 
> Signed-off-by: Lothar Rubusch <l.rubusch@gmail.com>


> @@ -483,12 +518,48 @@ static int adxl345_set_odr(struct adxl345_state *st, enum adxl345_odr odr)
>  	return 0;
>  }
>  
> +static int adxl345_find_range(struct adxl345_state *st, int val, int val2,
> +			      enum adxl345_range *range)
> +{
> +	int i;
> +
> +	for (i = 0; i < ARRAY_SIZE(adxl345_fullres_range_tbl); i++)
> +		if (val == adxl345_fullres_range_tbl[i][0] &&
> +		    val2 == adxl345_fullres_range_tbl[i][1])
> +			break;
Similar to case in earlier patch, maybe set *range and return in here
so that any finish of the loop is an error.
> +
> +	if (i == ARRAY_SIZE(adxl345_fullres_range_tbl))
> +		return -EINVAL;
> +
> +	*range = i;
> +
> +	return 0;
> +}
> +
> +static int adxl345_set_range(struct adxl345_state *st, enum adxl345_range range)
> +{
> +	int 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;
> +

return regmap_update_bits() unless this gets more complex in later patch.

> +	return 0;
> +}
> +

> @@ -558,6 +634,7 @@ static int adxl345_write_raw(struct iio_dev *indio_dev,
>  			     int val, int val2, long mask)
>  {
>  	struct adxl345_state *st = iio_priv(indio_dev);
> +	enum adxl345_range range;
>  	enum adxl345_odr odr;
>  	int ret;
>  
> @@ -581,6 +658,12 @@ static int adxl345_write_raw(struct iio_dev *indio_dev,
>  			return ret;
>  		ret = adxl345_set_odr(st, odr);
>  		break;
> +	case IIO_CHAN_INFO_SCALE:
> +		ret = adxl345_find_range(st, val, val2,	&range);
> +		if (ret)
> +			return ret;
> +		ret = adxl345_set_range(st, range);
as in previous I'd have the 	
		if (ret)
			return ret;
here for consistency with the other one immediately above this.
> +		break;
>  	default:
>  		return -EINVAL;
>  	}