On Tue, 10 May 2022 14:17:49 +0000
LI Qingwu <Qing-wu.Li@xxxxxxxxxxxxxxxxxxxxxxx> wrote:
> The sensor can set the scales by writing the range register 0x41,
> The current driver has no interface to configure it.
> The commit adds the interface for config the scales.
>
> Reviewed-by: Alexandru Ardelean <ardeleanalex@xxxxxxxxx>
> Signed-off-by: LI Qingwu <Qing-wu.Li@xxxxxxxxxxxxxxxxxxxxxxx>
Hi.
A few minor requested changes inline,
Thanks,
Jonathan
> ---
> drivers/iio/accel/bmi088-accel-core.c | 32 ++++++++++++++++++++++++++-
> 1 file changed, 31 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/iio/accel/bmi088-accel-core.c b/drivers/iio/accel/bmi088-accel-core.c
> index 9300313b63cb..8fee1d02e773 100644
> --- a/drivers/iio/accel/bmi088-accel-core.c
> +++ b/drivers/iio/accel/bmi088-accel-core.c
> @@ -237,6 +237,21 @@ static int bmi088_accel_set_sample_freq(struct bmi088_accel_data *data, int val)
> BMI088_ACCEL_MODE_ODR_MASK, regval);
> }
>
> +static int bmi088_accel_set_scale(struct bmi088_accel_data *data, int val, int val2)
> +{
> + unsigned int i;
> +
> + for (i = 0; i < 4; i++)
> + if (val == data->chip_info->scale_table[i][0] &&
> + val2 == data->chip_info->scale_table[i][1])
> + break;
> +
> + if (i >= 4)
== 4
If it's > 4 something very odd happened :)
> + return -EINVAL;
> +
> + return regmap_write(data->regmap, BMI088_ACCEL_REG_ACC_RANGE, i);
> +}
> +
> static int bmi088_accel_get_temp(struct bmi088_accel_data *data, int *val)
> {
> int ret;
> @@ -368,7 +383,13 @@ static int bmi088_accel_read_avail(struct iio_dev *indio_dev,
> const int **vals, int *type, int *length,
> long mask)
> {
> + struct bmi088_accel_data *data = iio_priv(indio_dev);
> switch (mask) {
> + case IIO_CHAN_INFO_SCALE:
> + *vals = (const int *)data->chip_info->scale_table;
> + *length = 8;
> + *type = IIO_VAL_INT_PLUS_MICRO;
> + return IIO_AVAIL_LIST;
> case IIO_CHAN_INFO_SAMP_FREQ:
> *type = IIO_VAL_INT_PLUS_MICRO;
> *vals = bmi088_sample_freqs;
> @@ -388,6 +409,14 @@ static int bmi088_accel_write_raw(struct iio_dev *indio_dev,
> int ret;
>
> switch (mask) {
> + case IIO_CHAN_INFO_SCALE:
> + ret = pm_runtime_resume_and_get(dev);
> + if (ret)
> + return ret;
Blank line here. That separates a functional call and it's error handling from
what happens next and makes the code a tiny bit easier to read.
Also consistent with SAMP_FREQ block that follows.
> + ret = bmi088_accel_set_scale(data, val, val2);
> + pm_runtime_mark_last_busy(dev);
> + pm_runtime_put_autosuspend(dev);
> + return ret;
> case IIO_CHAN_INFO_SAMP_FREQ:
> ret = pm_runtime_resume_and_get(dev);
> if (ret)
> @@ -409,7 +438,8 @@ static int bmi088_accel_write_raw(struct iio_dev *indio_dev,
> .info_mask_separate = BIT(IIO_CHAN_INFO_RAW), \
> .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE) | \
> BIT(IIO_CHAN_INFO_SAMP_FREQ), \
> - .info_mask_shared_by_type_available = BIT(IIO_CHAN_INFO_SAMP_FREQ), \
> + .info_mask_shared_by_type_available = BIT(IIO_CHAN_INFO_SAMP_FREQ) | \
> + BIT(IIO_CHAN_INFO_SCALE), \
> .scan_index = AXIS_##_axis, \
> }
>
