On Tue, 12 Apr 2022 02:01:29 +0530
Jagath Jog J <jagathjog1996@xxxxxxxxx> wrote:
> Added channel for step counter which can be enable or disable
> through the sysfs interface.
>
> Signed-off-by: Jagath Jog J <jagathjog1996@xxxxxxxxx>
> ---
> drivers/iio/accel/bma400.h | 1 +
> drivers/iio/accel/bma400_core.c | 48 ++++++++++++++++++++++++++++++---
> 2 files changed, 45 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/iio/accel/bma400.h b/drivers/iio/accel/bma400.h
> index a7482a66b36b..52f9ea95de81 100644
> --- a/drivers/iio/accel/bma400.h
> +++ b/drivers/iio/accel/bma400.h
> @@ -53,6 +53,7 @@
> #define BMA400_STEP_CNT1_REG 0x16
> #define BMA400_STEP_CNT3_REG 0x17
> #define BMA400_STEP_STAT_REG 0x18
> +#define BMA400_STEP_INT_MSK BIT(0)
>
> /*
> * Read-write configuration registers
> diff --git a/drivers/iio/accel/bma400_core.c b/drivers/iio/accel/bma400_core.c
> index b7b2b67aef31..c8f147163d3c 100644
> --- a/drivers/iio/accel/bma400_core.c
> +++ b/drivers/iio/accel/bma400_core.c
> @@ -20,6 +20,8 @@
> #include <linux/regmap.h>
> #include <linux/regulator/consumer.h>
>
> +#include <asm/unaligned.h>
> +
> #include <linux/iio/iio.h>
> #include <linux/iio/buffer.h>
> #include <linux/iio/trigger.h>
> @@ -67,6 +69,7 @@ struct bma400_data {
> int oversampling_ratio;
> int scale;
> struct iio_trigger *trig;
> + int steps_enabled;
> /* Correct time stamp alignment */
> struct {
> __le16 buff[3];
> @@ -201,6 +204,12 @@ static const struct iio_chan_spec bma400_channels[] = {
> .endianness = IIO_LE,
> },
> },
> + {
> + .type = IIO_STEPS,
> + .info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED) |
> + BIT(IIO_CHAN_INFO_ENABLE),
> + .scan_index = -1, /* No buffer support */
> + },
> IIO_CHAN_SOFT_TIMESTAMP(4),
> };
>
> @@ -705,13 +714,28 @@ static int bma400_read_raw(struct iio_dev *indio_dev,
> {
> struct bma400_data *data = iio_priv(indio_dev);
> int ret;
> + u8 steps_raw[3];
>
> switch (mask) {
> case IIO_CHAN_INFO_PROCESSED:
> - mutex_lock(&data->mutex);
> - ret = bma400_get_temp_reg(data, val, val2);
> - mutex_unlock(&data->mutex);
> - return ret;
> + switch (chan->type) {
> + case IIO_TEMP:
> + mutex_lock(&data->mutex);
> + ret = bma400_get_temp_reg(data, val, val2);
> + mutex_unlock(&data->mutex);
> + return ret;
> + case IIO_STEPS:
> + mutex_lock(&data->mutex);
> + ret = regmap_bulk_read(data->regmap, BMA400_STEP_CNT0_REG,
> + &steps_raw, sizeof(steps_raw));
Bulk read to a non DMA safe buffer. Similar fix to before needed.
Or given this is a slow path, just use a local kmalloc() for the buffer.
The reality is you are probably fine today without such care, but last
time we discussed this the conclusion was that it would be a perfectly
valid optimisation in regmap to return to requiring DMA safe buffers
for bulk accesses.
> + mutex_unlock(&data->mutex);
> + if (ret)
> + return ret;
> + *val = get_unaligned_le24(steps_raw);
> + return IIO_VAL_INT;
> + default:
> + return -EINVAL;
> + }
> case IIO_CHAN_INFO_RAW:
> mutex_lock(&data->mutex);
> ret = bma400_get_accel_reg(data, chan, val);
> @@ -752,6 +776,9 @@ static int bma400_read_raw(struct iio_dev *indio_dev,
>
> *val = data->oversampling_ratio;
> return IIO_VAL_INT;
> + case IIO_CHAN_INFO_ENABLE:
> + *val = data->steps_enabled;
> + return IIO_VAL_INT;
> default:
> return -EINVAL;
> }
> @@ -817,6 +844,17 @@ static int bma400_write_raw(struct iio_dev *indio_dev,
> ret = bma400_set_accel_oversampling_ratio(data, val);
> mutex_unlock(&data->mutex);
> return ret;
> + case IIO_CHAN_INFO_ENABLE:
> + if (data->steps_enabled == val)
> + return 0;
> +
> + mutex_lock(&data->mutex);
> + ret = regmap_update_bits(data->regmap, BMA400_INT_CONFIG1_REG,
> + BMA400_STEP_INT_MSK,
> + FIELD_PREP(BMA400_STEP_INT_MSK, !!val));
Why the lock for this one?
> + mutex_unlock(&data->mutex);
> + data->steps_enabled = val;
> + return ret;
> default:
> return -EINVAL;
> }
> @@ -833,6 +871,8 @@ static int bma400_write_raw_get_fmt(struct iio_dev *indio_dev,
> return IIO_VAL_INT_PLUS_MICRO;
> case IIO_CHAN_INFO_OVERSAMPLING_RATIO:
> return IIO_VAL_INT;
> + case IIO_CHAN_INFO_ENABLE:
> + return IIO_VAL_INT;
> default:
> return -EINVAL;
> }
