On Mon, 10 Feb 2025 11:01:15 +0000
Lothar Rubusch <l.rubusch@xxxxxxxxx> 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.
>
> Signed-off-by: Lothar Rubusch <l.rubusch@xxxxxxxxx>
A few really trivial comments on this one.
> ---
> drivers/iio/accel/adxl345_core.c | 92 ++++++++++++++++++++++++++++++--
> 1 file changed, 89 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/iio/accel/adxl345_core.c b/drivers/iio/accel/adxl345_core.c
> index 08ad71875c5e..ea7bfe193d31 100644
> --- a/drivers/iio/accel/adxl345_core.c
> +++ b/drivers/iio/accel/adxl345_core.c
> @@ -86,6 +86,13 @@ enum adxl345_odr {
> ADXL345_ODR_3200HZ,
> };
>
> +enum adxl345_range {
> + ADXL345_2G_RANGE = 0,
> + ADXL345_4G_RANGE,
> + ADXL345_8G_RANGE,
> + ADXL345_16G_RANGE,
> +};
> +
> /* Certain features recommend 12.5 Hz - 400 Hz ODR */
> static const int adxl345_odr_tbl[][2] = {
> [ADXL345_ODR_0P10HZ] = { 0, 97000},
> @@ -106,6 +113,33 @@ static const int adxl345_odr_tbl[][2] = {
> [ADXL345_ODR_3200HZ] = {3200, 0},
> };
>
> +/*
> + * Full resolution frequency table:
> + * (g * 2 * 9.80665) / (2^(resolution) - 1)
> + *
> + * resolution := 13 (full)
> + * g := 2|4|8|16
> + *
> + * 2g at 13bit: 0.004789
> + * 4g at 13bit: 0.009578
> + * 8g at 13bit: 0.019156
> + * 16g at 16bit: 0.038312
> + */
> +static const int adxl345_fullres_range_tbl[][2] = {
> + [ADXL345_2G_RANGE] = {0, 4789},
I'm slowly trying to standardize a few more corners of the kernel style for IIO at least.
This hits one of them. So if you don't mind adding spaces after { and before }
it will one day save me time on a cleanup series.
> + [ADXL345_4G_RANGE] = {0, 9578},
> + [ADXL345_8G_RANGE] = {0, 19156},
> + [ADXL345_16G_RANGE] = {0, 38312},
> +};
> @@ -1252,6 +1335,9 @@ int adxl345_core_probe(struct device *dev, struct regmap *regmap,
> * undesired behavior.
> */
> ret = adxl345_set_odr(st, ADXL345_ODR_200HZ);
> + if (ret)
> + return ret;
Trivial but I'd put a blank line here for slightly improved readability.
> + ret = adxl345_set_range(st, ADXL345_16G_RANGE);
> if (ret)
> return ret;
>
