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)); > +}