On Mon, 26 Aug 2024 16:05:45 +0200
<Jianping.Shen@xxxxxxxxxxxx> wrote:
> From: Shen Jianping <Jianping.Shen@xxxxxxxxxxxx>
>
> This patch adds the iio driver for bosch imu smi240. The smi240
> is a combined three axis angular rate and three axis acceleration
> sensor module with a measurement range of +/-300°/s and up to 16g.
> Smi240 does not support interrupt. A synchronous acc and gyro
> sampling can be triggered by setting the capture bit in spi read
> command.
>
> Implemented features:
> * raw data access for each axis through sysfs
> * tiggered buffer for continuous sampling
> * synchronous acc and gyro data from tiggered buffer
>
> Signed-off-by: Shen Jianping <Jianping.Shen@xxxxxxxxxxxx>
As per the other review, I'd really prefer this as one simple file.
I know you want to keep it looking like other IMUs, but it is a much
simpler devices, so the complexity they need is not appropriate here.
A few things inline,
Jonathan
> diff --git a/drivers/iio/imu/smi240/smi240_core.c b/drivers/iio/imu/smi240/smi240_core.c
> new file mode 100644
> index 00000000000..fc0226af843
> --- /dev/null
> +++ b/drivers/iio/imu/smi240/smi240_core.c
> @@ -0,0 +1,385 @@
...
> +#define SMI240_DATA_CHANNEL(_type, _axis, _index) \
> + { \
> + .type = _type, \
> + .modified = 1, \
> + .channel2 = IIO_MOD_##_axis, \
> + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW), \
No scaling? I'd expect the offset + scale for an accelerometer or gyro
channel to be well defined in the datasheet.
> + .info_mask_shared_by_type = \
> + BIT(IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY), \
> + .info_mask_shared_by_type_available = \
> + BIT(IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY), \
> diff --git a/drivers/iio/imu/smi240/smi240_spi.c b/drivers/iio/imu/smi240/smi240_spi.c
> new file mode 100644
> index 00000000000..d308f6407da
> --- /dev/null
> +++ b/drivers/iio/imu/smi240/smi240_spi.c
> @@ -0,0 +1,172 @@
> +// SPDX-License-Identifier: BSD-3-Clause OR GPL-2.0
> +/*
> + * Copyright (c) 2024 Robert Bosch GmbH.
> + */
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/device.h>
> +#include <linux/regmap.h>
> +#include <linux/spi/spi.h>
> +#include <linux/bitfield.h>
> +#include <linux/iio/iio.h>
> +#include <asm/unaligned.h>
Alphabetical order preferred.
> +static int smi240_regmap_spi_read(void *context, const void *reg_buf,
> + size_t reg_size, void *val_buf,
> + size_t val_size)
> +{
> + int ret;
> + u32 request, response;
> + struct spi_device *spi = context;
> + struct iio_dev *indio_dev = dev_get_drvdata(&spi->dev);
> + struct smi240_data *iio_priv_data = iio_priv(indio_dev);
> +
> + request = SMI240_BUS_ID << 30;
FIELD_PREP for this field as well (and define the mask etc).
> + request |= FIELD_PREP(SMI240_CAP_BIT_MASK, iio_priv_data->capture);
> + request |= FIELD_PREP(SMI240_WRITE_ADDR_MASK, *(u8 *)reg_buf);
> + request |= smi240_crc3(request, SMI240_CRC_INIT, SMI240_CRC_POLY);
> +
> + iio_priv_data->spi_buf = cpu_to_be32(request);
> +
> + /*
> + * SMI240 module consists of a 32Bit Out Of Frame (OOF)
> + * SPI protocol, where the slave interface responds to
> + * the Master request in the next frame.
> + * CS signal must toggle (> 700 ns) between the frames.
> + */
> + ret = spi_write(spi, &iio_priv_data->spi_buf, sizeof(request));
> + if (ret)
> + return ret;
> +
> + ret = spi_read(spi, &iio_priv_data->spi_buf, sizeof(response));
> + if (ret)
> + return ret;
> +
> + response = be32_to_cpu(iio_priv_data->spi_buf);
> +
> + if (!smi240_sensor_data_is_valid(response))
> + return -EIO;
> +
> + response = FIELD_GET(SMI240_READ_DATA_MASK, response);
> + memcpy(val_buf, &response, val_size);
> +
> + return 0;
> +}
> +
> +static int smi240_regmap_spi_write(void *context, const void *data,
> + size_t count)
> +{
> + u32 request;
> + struct spi_device *spi = context;
> + struct iio_dev *indio_dev = dev_get_drvdata(&spi->dev);
> + struct smi240_data *iio_priv_data = iio_priv(indio_dev);
> + u8 reg_addr = ((u8 *)data)[0];
> + u16 reg_data = get_unaligned_le16(&((u8 *)data)[1]);
> +
> + request = SMI240_BUS_ID << 30;
FIELD_PREP() for this one as well with appropriate mask.
> + request |= FIELD_PREP(SMI240_WRITE_BIT_MASK, 1);
> + request |= FIELD_PREP(SMI240_WRITE_ADDR_MASK, reg_addr);
> + request |= FIELD_PREP(SMI240_WRITE_DATA_MASK, reg_data);
> + request |= smi240_crc3(request, SMI240_CRC_INIT, SMI240_CRC_POLY);
> +
> + iio_priv_data->spi_buf = cpu_to_be32(request);
> +
> + return spi_write(spi, &iio_priv_data->spi_buf, sizeof(request));
> +}
