diff mbox series

[v3,2/2] iio: imu: smi240: imu driver

Message ID 20240815152545.7705-3-Jianping.Shen@de.bosch.com (mailing list archive)
State Changes Requested
Headers show
Series iio: imu: smi240: cover-letter | expand

Commit Message

Shen Jianping (ME-SE/EAD2) Aug. 15, 2024, 3:25 p.m. UTC
From: Shen Jianping <Jianping.Shen@de.bosch.com>

iio: imu: smi240: add sensor driver
Signed-off-by: Shen Jianping <Jianping.Shen@de.bosch.com>
---

Notes:
    v1 -> v2
    - Use regmap for register access
    - Redefine channel for each singel axis
    - Provide triggered buffer
    - Fix findings in Kconfig
    - Remove unimportant functions
    
    v2 -> v3
    - Use enum für capture mode
    - Using spi default init value instead manual init
    - remove duplicated module declaration
    - Fix code to avoid warning

 drivers/iio/imu/Kconfig              |   1 +
 drivers/iio/imu/Makefile             |   1 +
 drivers/iio/imu/smi240/Kconfig       |  12 +
 drivers/iio/imu/smi240/Makefile      |   7 +
 drivers/iio/imu/smi240/smi240.h      |  32 +++
 drivers/iio/imu/smi240/smi240_core.c | 386 +++++++++++++++++++++++++++
 drivers/iio/imu/smi240/smi240_spi.c  | 164 ++++++++++++
 7 files changed, 603 insertions(+)
 create mode 100644 drivers/iio/imu/smi240/Kconfig
 create mode 100644 drivers/iio/imu/smi240/Makefile
 create mode 100644 drivers/iio/imu/smi240/smi240.h
 create mode 100644 drivers/iio/imu/smi240/smi240_core.c
 create mode 100644 drivers/iio/imu/smi240/smi240_spi.c

Comments

kernel test robot Aug. 17, 2024, 1:56 a.m. UTC | #1
Hi,

kernel test robot noticed the following build warnings:

[auto build test WARNING on jic23-iio/togreg]
[also build test WARNING on linus/master v6.11-rc3 next-20240816]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Jianping-Shen-de-bosch-com/dt-bindings-iio-imu-smi240-devicetree-binding/20240815-234739
base:   https://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git togreg
patch link:    https://lore.kernel.org/r/20240815152545.7705-3-Jianping.Shen%40de.bosch.com
patch subject: [PATCH v3 2/2] iio: imu: smi240: imu driver
config: x86_64-randconfig-123-20240817 (https://download.01.org/0day-ci/archive/20240817/202408170910.aR1gYef3-lkp@intel.com/config)
compiler: clang version 18.1.5 (https://github.com/llvm/llvm-project 617a15a9eac96088ae5e9134248d8236e34b91b1)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240817/202408170910.aR1gYef3-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202408170910.aR1gYef3-lkp@intel.com/

sparse warnings: (new ones prefixed by >>)
>> drivers/iio/imu/smi240/smi240_core.c:207:9: sparse: sparse: dereference of noderef expression
>> drivers/iio/imu/smi240/smi240_core.c:207:9: sparse: sparse: dereference of noderef expression
>> drivers/iio/imu/smi240/smi240_core.c:207:9: sparse: sparse: dereference of noderef expression
--
>> drivers/iio/imu/smi240/smi240_spi.c:69:17: sparse: sparse: incorrect type in assignment (different base types) @@     expected restricted __be32 [usertype] request @@     got int @@
   drivers/iio/imu/smi240/smi240_spi.c:69:17: sparse:     expected restricted __be32 [usertype] request
   drivers/iio/imu/smi240/smi240_spi.c:69:17: sparse:     got int
>> drivers/iio/imu/smi240/smi240_spi.c:70:17: sparse: sparse: invalid assignment: |=
   drivers/iio/imu/smi240/smi240_spi.c:70:17: sparse:    left side has type restricted __be32
   drivers/iio/imu/smi240/smi240_spi.c:70:17: sparse:    right side has type int
   drivers/iio/imu/smi240/smi240_spi.c:71:17: sparse: sparse: invalid assignment: |=
   drivers/iio/imu/smi240/smi240_spi.c:71:17: sparse:    left side has type restricted __be32
   drivers/iio/imu/smi240/smi240_spi.c:71:17: sparse:    right side has type unsigned long
>> drivers/iio/imu/smi240/smi240_spi.c:72:32: sparse: sparse: incorrect type in argument 1 (different base types) @@     expected unsigned int [usertype] data @@     got restricted __be32 [usertype] request @@
   drivers/iio/imu/smi240/smi240_spi.c:72:32: sparse:     expected unsigned int [usertype] data
   drivers/iio/imu/smi240/smi240_spi.c:72:32: sparse:     got restricted __be32 [usertype] request
   drivers/iio/imu/smi240/smi240_spi.c:72:17: sparse: sparse: invalid assignment: |=
   drivers/iio/imu/smi240/smi240_spi.c:72:17: sparse:    left side has type restricted __be32
   drivers/iio/imu/smi240/smi240_spi.c:72:17: sparse:    right side has type unsigned char
>> drivers/iio/imu/smi240/smi240_spi.c:73:19: sparse: sparse: cast from restricted __be32
>> drivers/iio/imu/smi240/smi240_spi.c:89:18: sparse: sparse: incorrect type in assignment (different base types) @@     expected restricted __be32 [addressable] [usertype] response @@     got unsigned int [usertype] @@
   drivers/iio/imu/smi240/smi240_spi.c:89:18: sparse:     expected restricted __be32 [addressable] [usertype] response
   drivers/iio/imu/smi240/smi240_spi.c:89:18: sparse:     got unsigned int [usertype]
>> drivers/iio/imu/smi240/smi240_spi.c:91:42: sparse: sparse: incorrect type in argument 1 (different base types) @@     expected unsigned int [usertype] data @@     got restricted __be32 [addressable] [usertype] response @@
   drivers/iio/imu/smi240/smi240_spi.c:91:42: sparse:     expected unsigned int [usertype] data
   drivers/iio/imu/smi240/smi240_spi.c:91:42: sparse:     got restricted __be32 [addressable] [usertype] response
>> drivers/iio/imu/smi240/smi240_spi.c:94:20: sparse: sparse: cast to restricted __be32
>> drivers/iio/imu/smi240/smi240_spi.c:94:20: sparse: sparse: restricted __be32 degrades to integer
>> drivers/iio/imu/smi240/smi240_spi.c:94:20: sparse: sparse: restricted __be32 degrades to integer
>> drivers/iio/imu/smi240/smi240_spi.c:94:18: sparse: sparse: incorrect type in assignment (different base types) @@     expected restricted __be32 [addressable] [usertype] response @@     got unsigned long @@
   drivers/iio/imu/smi240/smi240_spi.c:94:18: sparse:     expected restricted __be32 [addressable] [usertype] response
   drivers/iio/imu/smi240/smi240_spi.c:94:18: sparse:     got unsigned long
   drivers/iio/imu/smi240/smi240_spi.c:108:17: sparse: sparse: incorrect type in assignment (different base types) @@     expected restricted __be32 [usertype] request @@     got int @@
   drivers/iio/imu/smi240/smi240_spi.c:108:17: sparse:     expected restricted __be32 [usertype] request
   drivers/iio/imu/smi240/smi240_spi.c:108:17: sparse:     got int
   drivers/iio/imu/smi240/smi240_spi.c:109:17: sparse: sparse: invalid assignment: |=
   drivers/iio/imu/smi240/smi240_spi.c:109:17: sparse:    left side has type restricted __be32
   drivers/iio/imu/smi240/smi240_spi.c:109:17: sparse:    right side has type int
   drivers/iio/imu/smi240/smi240_spi.c:110:17: sparse: sparse: invalid assignment: |=
   drivers/iio/imu/smi240/smi240_spi.c:110:17: sparse:    left side has type restricted __be32
   drivers/iio/imu/smi240/smi240_spi.c:110:17: sparse:    right side has type unsigned long
   drivers/iio/imu/smi240/smi240_spi.c:111:17: sparse: sparse: invalid assignment: |=
   drivers/iio/imu/smi240/smi240_spi.c:111:17: sparse:    left side has type restricted __be32
   drivers/iio/imu/smi240/smi240_spi.c:111:17: sparse:    right side has type unsigned long
   drivers/iio/imu/smi240/smi240_spi.c:112:32: sparse: sparse: incorrect type in argument 1 (different base types) @@     expected unsigned int [usertype] data @@     got restricted __be32 [usertype] request @@
   drivers/iio/imu/smi240/smi240_spi.c:112:32: sparse:     expected unsigned int [usertype] data
   drivers/iio/imu/smi240/smi240_spi.c:112:32: sparse:     got restricted __be32 [usertype] request
   drivers/iio/imu/smi240/smi240_spi.c:112:17: sparse: sparse: invalid assignment: |=
   drivers/iio/imu/smi240/smi240_spi.c:112:17: sparse:    left side has type restricted __be32
   drivers/iio/imu/smi240/smi240_spi.c:112:17: sparse:    right side has type unsigned char
   drivers/iio/imu/smi240/smi240_spi.c:113:19: sparse: sparse: cast from restricted __be32

vim +207 drivers/iio/imu/smi240/smi240_core.c

   197	
   198	static irqreturn_t smi240_trigger_handler(int irq, void *p)
   199	{
   200		struct iio_poll_func *pf = p;
   201		struct iio_dev *indio_dev = pf->indio_dev;
   202		struct smi240_data *data = iio_priv(indio_dev);
   203		int ret, sample, chan, i = 0;
   204	
   205		data->capture = SMI240_CAPTURE_ON;
   206	
 > 207		for_each_set_bit(chan, indio_dev->active_scan_mask,
   208				 indio_dev->masklength) {
   209			ret = regmap_read(data->regmap,
   210					  SMI240_DATA_CAP_FIRST_REG + chan, &sample);
   211			data->capture = SMI240_CAPTURE_OFF;
   212			if (ret)
   213				break;
   214			data->buf[i++] = sample;
   215		}
   216	
   217		if (ret == 0)
   218			iio_push_to_buffers_with_timestamp(indio_dev, data->buf,
   219							   pf->timestamp);
   220	
   221		iio_trigger_notify_done(indio_dev->trig);
   222		return IRQ_HANDLED;
   223	}
   224
kernel test robot Aug. 17, 2024, 4:13 p.m. UTC | #2
Hi,

kernel test robot noticed the following build warnings:

[auto build test WARNING on jic23-iio/togreg]
[also build test WARNING on linus/master v6.11-rc3 next-20240816]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Jianping-Shen-de-bosch-com/dt-bindings-iio-imu-smi240-devicetree-binding/20240815-234739
base:   https://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git togreg
patch link:    https://lore.kernel.org/r/20240815152545.7705-3-Jianping.Shen%40de.bosch.com
patch subject: [PATCH v3 2/2] iio: imu: smi240: imu driver
config: microblaze-randconfig-r132-20240817 (https://download.01.org/0day-ci/archive/20240817/202408172325.mMMFMiZJ-lkp@intel.com/config)
compiler: microblaze-linux-gcc (GCC) 14.1.0
reproduce: (https://download.01.org/0day-ci/archive/20240817/202408172325.mMMFMiZJ-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202408172325.mMMFMiZJ-lkp@intel.com/

sparse warnings: (new ones prefixed by >>)
   drivers/iio/imu/smi240/smi240_spi.c:69:17: sparse: sparse: incorrect type in assignment (different base types) @@     expected restricted __be32 [usertype] request @@     got int @@
   drivers/iio/imu/smi240/smi240_spi.c:69:17: sparse:     expected restricted __be32 [usertype] request
   drivers/iio/imu/smi240/smi240_spi.c:69:17: sparse:     got int
   drivers/iio/imu/smi240/smi240_spi.c:70:17: sparse: sparse: invalid assignment: |=
   drivers/iio/imu/smi240/smi240_spi.c:70:17: sparse:    left side has type restricted __be32
   drivers/iio/imu/smi240/smi240_spi.c:70:17: sparse:    right side has type int
   drivers/iio/imu/smi240/smi240_spi.c:71:17: sparse: sparse: invalid assignment: |=
   drivers/iio/imu/smi240/smi240_spi.c:71:17: sparse:    left side has type restricted __be32
   drivers/iio/imu/smi240/smi240_spi.c:71:17: sparse:    right side has type unsigned long
   drivers/iio/imu/smi240/smi240_spi.c:72:32: sparse: sparse: incorrect type in argument 1 (different base types) @@     expected unsigned int [usertype] data @@     got restricted __be32 [usertype] request @@
   drivers/iio/imu/smi240/smi240_spi.c:72:32: sparse:     expected unsigned int [usertype] data
   drivers/iio/imu/smi240/smi240_spi.c:72:32: sparse:     got restricted __be32 [usertype] request
   drivers/iio/imu/smi240/smi240_spi.c:72:17: sparse: sparse: invalid assignment: |=
   drivers/iio/imu/smi240/smi240_spi.c:72:17: sparse:    left side has type restricted __be32
   drivers/iio/imu/smi240/smi240_spi.c:72:17: sparse:    right side has type unsigned char
>> drivers/iio/imu/smi240/smi240_spi.c:73:19: sparse: sparse: incorrect type in argument 1 (different base types) @@     expected unsigned int [usertype] val @@     got restricted __be32 [usertype] request @@
   drivers/iio/imu/smi240/smi240_spi.c:73:19: sparse:     expected unsigned int [usertype] val
   drivers/iio/imu/smi240/smi240_spi.c:73:19: sparse:     got restricted __be32 [usertype] request
   drivers/iio/imu/smi240/smi240_spi.c:73:19: sparse: sparse: cast from restricted __be32
   drivers/iio/imu/smi240/smi240_spi.c:73:19: sparse: sparse: cast from restricted __be32
   drivers/iio/imu/smi240/smi240_spi.c:73:19: sparse: sparse: cast from restricted __be32
   drivers/iio/imu/smi240/smi240_spi.c:73:19: sparse: sparse: cast from restricted __be32
   drivers/iio/imu/smi240/smi240_spi.c:89:18: sparse: sparse: incorrect type in assignment (different base types) @@     expected restricted __be32 [addressable] [usertype] response @@     got unsigned int [usertype] @@
   drivers/iio/imu/smi240/smi240_spi.c:89:18: sparse:     expected restricted __be32 [addressable] [usertype] response
   drivers/iio/imu/smi240/smi240_spi.c:89:18: sparse:     got unsigned int [usertype]
   drivers/iio/imu/smi240/smi240_spi.c:91:42: sparse: sparse: incorrect type in argument 1 (different base types) @@     expected unsigned int [usertype] data @@     got restricted __be32 [addressable] [usertype] response @@
   drivers/iio/imu/smi240/smi240_spi.c:91:42: sparse:     expected unsigned int [usertype] data
   drivers/iio/imu/smi240/smi240_spi.c:91:42: sparse:     got restricted __be32 [addressable] [usertype] response
   drivers/iio/imu/smi240/smi240_spi.c:94:20: sparse: sparse: cast to restricted __be32
   drivers/iio/imu/smi240/smi240_spi.c:94:20: sparse: sparse: restricted __be32 degrades to integer
   drivers/iio/imu/smi240/smi240_spi.c:94:20: sparse: sparse: restricted __be32 degrades to integer
   drivers/iio/imu/smi240/smi240_spi.c:94:18: sparse: sparse: incorrect type in assignment (different base types) @@     expected restricted __be32 [addressable] [usertype] response @@     got unsigned long @@
   drivers/iio/imu/smi240/smi240_spi.c:94:18: sparse:     expected restricted __be32 [addressable] [usertype] response
   drivers/iio/imu/smi240/smi240_spi.c:94:18: sparse:     got unsigned long
   drivers/iio/imu/smi240/smi240_spi.c:108:17: sparse: sparse: incorrect type in assignment (different base types) @@     expected restricted __be32 [usertype] request @@     got int @@
   drivers/iio/imu/smi240/smi240_spi.c:108:17: sparse:     expected restricted __be32 [usertype] request
   drivers/iio/imu/smi240/smi240_spi.c:108:17: sparse:     got int
   drivers/iio/imu/smi240/smi240_spi.c:109:17: sparse: sparse: invalid assignment: |=
   drivers/iio/imu/smi240/smi240_spi.c:109:17: sparse:    left side has type restricted __be32
   drivers/iio/imu/smi240/smi240_spi.c:109:17: sparse:    right side has type int
   drivers/iio/imu/smi240/smi240_spi.c:110:17: sparse: sparse: invalid assignment: |=
   drivers/iio/imu/smi240/smi240_spi.c:110:17: sparse:    left side has type restricted __be32
   drivers/iio/imu/smi240/smi240_spi.c:110:17: sparse:    right side has type unsigned long
   drivers/iio/imu/smi240/smi240_spi.c:111:17: sparse: sparse: invalid assignment: |=
   drivers/iio/imu/smi240/smi240_spi.c:111:17: sparse:    left side has type restricted __be32
   drivers/iio/imu/smi240/smi240_spi.c:111:17: sparse:    right side has type unsigned long
   drivers/iio/imu/smi240/smi240_spi.c:112:32: sparse: sparse: incorrect type in argument 1 (different base types) @@     expected unsigned int [usertype] data @@     got restricted __be32 [usertype] request @@
   drivers/iio/imu/smi240/smi240_spi.c:112:32: sparse:     expected unsigned int [usertype] data
   drivers/iio/imu/smi240/smi240_spi.c:112:32: sparse:     got restricted __be32 [usertype] request
   drivers/iio/imu/smi240/smi240_spi.c:112:17: sparse: sparse: invalid assignment: |=
   drivers/iio/imu/smi240/smi240_spi.c:112:17: sparse:    left side has type restricted __be32
   drivers/iio/imu/smi240/smi240_spi.c:112:17: sparse:    right side has type unsigned char
   drivers/iio/imu/smi240/smi240_spi.c:113:19: sparse: sparse: incorrect type in argument 1 (different base types) @@     expected unsigned int [usertype] val @@     got restricted __be32 [usertype] request @@
   drivers/iio/imu/smi240/smi240_spi.c:113:19: sparse:     expected unsigned int [usertype] val
   drivers/iio/imu/smi240/smi240_spi.c:113:19: sparse:     got restricted __be32 [usertype] request
   drivers/iio/imu/smi240/smi240_spi.c:113:19: sparse: sparse: cast from restricted __be32
   drivers/iio/imu/smi240/smi240_spi.c:113:19: sparse: sparse: cast from restricted __be32
   drivers/iio/imu/smi240/smi240_spi.c:113:19: sparse: sparse: cast from restricted __be32
   drivers/iio/imu/smi240/smi240_spi.c:113:19: sparse: sparse: cast from restricted __be32

vim +73 drivers/iio/imu/smi240/smi240_spi.c

    58	
    59	static int smi240_regmap_spi_read(void *context, const void *reg_buf,
    60					  size_t reg_size, void *val_buf,
    61					  size_t val_size)
    62	{
    63		int ret;
    64		__be32 request, response;
    65		struct spi_device *spi = context;
    66		struct iio_dev *indio_dev = dev_get_drvdata(&spi->dev);
    67		struct smi240_data *data = iio_priv(indio_dev);
    68	
    69		request = SMI240_BUS_ID << 30;
    70		request |= FIELD_PREP(SMI240_CAP_BIT_MASK, data->capture);
    71		request |= FIELD_PREP(SMI240_WRITE_ADDR_MASK, *(u8 *)reg_buf);
    72		request |= smi240_crc3(request, SMI240_CRC_INIT, SMI240_CRC_POLY);
  > 73		request = cpu_to_be32(request);
    74	
    75		/*
    76		 * SMI240 module consists of a 32Bit Out Of Frame (OOF)
    77		 * SPI protocol, where the slave interface responds to
    78		 * the Master request in the next frame.
    79		 * CS signal must toggle (> 700 ns) between the frames.
    80		 */
    81		ret = spi_write(spi, &request, sizeof(request));
    82		if (ret)
    83			return ret;
    84	
    85		ret = spi_read(spi, &response, sizeof(response));
    86		if (ret)
    87			return ret;
    88	
    89		response = be32_to_cpu(response);
    90	
    91		if (!smi240_sensor_data_is_valid(response))
    92			return -EIO;
    93	
    94		response = FIELD_GET(SMI240_READ_DATA_MASK, response);
    95		memcpy(val_buf, &response, val_size);
    96	
    97		return 0;
    98	}
    99
Jonathan Cameron Aug. 17, 2024, 4:17 p.m. UTC | #3
On Thu, 15 Aug 2024 17:25:45 +0200
<Jianping.Shen@de.bosch.com> wrote:

> From: Shen Jianping <Jianping.Shen@de.bosch.com>
> 
> iio: imu: smi240: add sensor driver
Blank line + tell us something about features supported, + what the device
is etc here. Don't repeat the title.

Also good to provide a
Datasheet: tag here if there is a public datasheet available
> Signed-off-by: Shen Jianping <Jianping.Shen@de.bosch.com>

Various other comments inline,

Thanks,

Jonathan


> diff --git a/drivers/iio/imu/smi240/smi240.h b/drivers/iio/imu/smi240/smi240.h
> new file mode 100644
> index 00000000000..a165bbd9f0b
> --- /dev/null
> +++ b/drivers/iio/imu/smi240/smi240.h
> @@ -0,0 +1,32 @@
> +/* SPDX-License-Identifier: BSD-3-Clause OR GPL-2.0 */
> +/*
> + * Copyright (c) 2024 Robert Bosch GmbH.
> + *
As below I'd drop this blank line.

> + */
> +#ifndef _SMI240_H
> +#define _SMI240_H
> +
> +#include <linux/device.h>
> +#include <linux/regmap.h>
Use a forwards def only as this doesn't need to know about regmap.h contents.

struct regmap;
struct device;

Then include regmap.h and device.h in the appropriate c files.

> +
> +enum capture_mode { SMI240_CAPTURE_OFF = 0, SMI240_CAPTURE_ON = 1 };
> +
> +struct smi240_data {
> +	struct regmap *regmap;
> +	u16 accel_filter_freq;
> +	u16 anglvel_filter_freq;
> +	u8 bite_reps;
> +	enum capture_mode capture;
> +	/*
> +	 * Ensure natural alignment for timestamp if present.
> +	 * Channel size: 2 bytes.
> +	 * Max length needed: 2 * 3 channels + temp channel + 2 bytes padding + 8 byte ts.
> +	 * If fewer channels are enabled, less space may be needed, as
> +	 * long as the timestamp is still aligned to 8 bytes.
> +	 */
> +	s16 buf[12] __aligned(8);
Add a dma safe buffer here
	__be32 spi_buf __aligned(IIO_DMA_MINALIGN);
to use for the spi transfers 

See spi_sync() docs for why or this talk
Wolfram did a few years ago.
https://www.youtube.com/watch?v=JDwaMClvV-s
DMA safety in buffers for Linux Kernel device drivers

> +};
> +
> +int smi240_core_probe(struct device *dev, struct regmap *regmap);
> +
> +#endif /* _SMI240_H */
> diff --git a/drivers/iio/imu/smi240/smi240_core.c b/drivers/iio/imu/smi240/smi240_core.c
> new file mode 100644
> index 00000000000..9e7269b90c9
> --- /dev/null
> +++ b/drivers/iio/imu/smi240/smi240_core.c
> @@ -0,0 +1,386 @@
> +// SPDX-License-Identifier: BSD-3-Clause OR GPL-2.0
> +/*
> + * Copyright (c) 2024 Robert Bosch GmbH.
> + *
Trivial: Drop this blanke line. Doesn't add anything,.
> + */
> +#include <linux/regmap.h>
> +#include <linux/delay.h>
> +#include <linux/bits.h>
> +#include <linux/bitfield.h>
> +#include <linux/iio/iio.h>
> +#include <linux/iio/sysfs.h>
With suggested changes below you won't need sysfs.h

> +#include <linux/iio/triggered_buffer.h>
> +#include <linux/iio/trigger_consumer.h>
> +#include <linux/iio/buffer.h>
> +#include <linux/iio/trigger.h>
> +
> +#include "smi240.h"

> +#define SMI240_DATA_CHANNEL(_type, _axis, _index)                          \
> +	{                                                                  \
> +		.type = _type, .modified = 1, .channel2 = IIO_MOD_##_axis, \
as below.

> +		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),              \
> +		.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),  \
> +		.scan_index = _index,                                      \
> +		.scan_type = {                                             \
> +			.sign = 's',                                       \
> +			.realbits = 16,                                    \
> +			.storagebits = 16,                                 \
> +			.endianness = IIO_LE,                              \
> +		},                                                         \
> +	}
> +
> +#define SMI240_TEMP_CHANNEL(_index)                           \
> +	{                                                     \
> +		.type = IIO_TEMP, .modified = 1,              \

I'd put .modified on next line.
When doing c99 style assignment it can be easy to miss wher there
are multiple elements on one line. It's fine to combine them if everything
on a single line, but not mix and match as done here.

> +		.channel2 = IIO_MOD_TEMP_OBJECT,              \
> +		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW), \
> +		.scan_index = _index,                         \
> +		.scan_type = {                                \
> +			.sign = 's',                          \
> +			.realbits = 16,                       \
> +			.storagebits = 16,                    \
> +			.endianness = IIO_LE,                 \
> +		},                                            \
> +	}
> +
> +static const struct iio_chan_spec smi240_channels[] = {
> +	SMI240_TEMP_CHANNEL(SMI240_TEMP_OBJECT),
> +	SMI240_DATA_CHANNEL(IIO_ACCEL, X, SMI240_SCAN_ACCEL_X),
> +	SMI240_DATA_CHANNEL(IIO_ACCEL, Y, SMI240_SCAN_ACCEL_Y),
> +	SMI240_DATA_CHANNEL(IIO_ACCEL, Z, SMI240_SCAN_ACCEL_Z),
> +	SMI240_DATA_CHANNEL(IIO_ANGL_VEL, X, SMI240_SCAN_GYRO_X),
> +	SMI240_DATA_CHANNEL(IIO_ANGL_VEL, Y, SMI240_SCAN_GYRO_Y),
> +	SMI240_DATA_CHANNEL(IIO_ANGL_VEL, Z, SMI240_SCAN_GYRO_Z),
> +	IIO_CHAN_SOFT_TIMESTAMP(SMI240_SCAN_TIMESTAMP),
> +};
> +
> +static const int smi240_low_pass_freqs[] = { SMI240_LOW_BANDWIDTH_HZ,
> +					     SMI240_HIGH_BANDWIDTH_HZ };
> +
> +static int smi240_soft_reset(struct smi240_data *data)
> +{
> +	int ret;
> +
> +	ret = regmap_write(data->regmap, SMI240_CMD_REG, SMI240_SOFT_RESET_CMD);
> +	if (ret)
> +		return ret;
> +	fsleep(SMI240_DIGITAL_STARTUP_DELAY_US);

blank line here.

> +	return 0;
> +}
> +
> +static int smi240_soft_config(struct smi240_data *data)
> +{
> +	int ret;
> +	u8 acc_bw, gyr_bw;
> +	u16 request;
> +
> +	switch (data->accel_filter_freq) {
> +	case SMI240_LOW_BANDWIDTH_HZ:
> +		acc_bw = 0x1;
> +		break;
> +	case SMI240_HIGH_BANDWIDTH_HZ:
> +		acc_bw = 0x0;
> +		break;
> +	default:
> +		return -EINVAL;
> +	}
> +
> +	switch (data->anglvel_filter_freq) {
> +	case SMI240_LOW_BANDWIDTH_HZ:
> +		gyr_bw = 0x1;
> +		break;
> +	case SMI240_HIGH_BANDWIDTH_HZ:
> +		gyr_bw = 0x0;
> +		break;
> +	default:
> +		return -EINVAL;
> +	}
> +
> +	request = FIELD_PREP(SMI240_SOFT_CONFIG_EOC_MASK, 1);
> +	request |= FIELD_PREP(SMI240_SOFT_CONFIG_GYR_BW_MASK, gyr_bw);
> +	request |= FIELD_PREP(SMI240_SOFT_CONFIG_ACC_BW_MASK, acc_bw);
> +	request |= FIELD_PREP(SMI240_SOFT_CONFIG_BITE_AUTO_MASK, 1);
> +	request |= FIELD_PREP(SMI240_SOFT_CONFIG_BITE_REP_MASK,
> +			      data->bite_reps - 1);
> +
> +	ret = regmap_write(data->regmap, SMI240_SOFT_CONFIG_REG, request);
> +	if (ret)
> +		return ret;
> +
> +	fsleep(SMI240_MECH_STARTUP_DELAY_US +
> +	       data->bite_reps * SMI240_BITE_SEQUENCE_DELAY_US +
> +	       SMI240_FILTER_FLUSH_DELAY_US);

blank line here slightly helps readability.


> +	return 0;
> +}
> +
> +static int smi240_get_low_pass_filter_freq(struct smi240_data *data,
> +					   int chan_type, int *val)
> +{
> +	switch (chan_type) {
> +	case IIO_ACCEL:
> +		*val = data->accel_filter_freq;
> +		break;
		return 0;
> +	case IIO_ANGL_VEL:
> +		*val = data->anglvel_filter_freq;
> +		break;
		return 0;
> +	default:
> +		return -EINVAL;
> +	}
drop the rest as won't get there.

If you can return early it allows the reader following a particular
code path to not bother looking at places where nothing happens
anyway.  So in general it's preferred unless there is shared cleanup
to do that applies to multiple paths.

> +
> +	return 0;
> +}

> +
> +static irqreturn_t smi240_trigger_handler(int irq, void *p)
> +{
> +	struct iio_poll_func *pf = p;
> +	struct iio_dev *indio_dev = pf->indio_dev;
> +	struct smi240_data *data = iio_priv(indio_dev);
> +	int ret, sample, chan, i = 0;
> +
> +	data->capture = SMI240_CAPTURE_ON;
> +
> +	for_each_set_bit(chan, indio_dev->active_scan_mask,
> +			 indio_dev->masklength) {
> +		ret = regmap_read(data->regmap,
> +				  SMI240_DATA_CAP_FIRST_REG + chan, &sample);
The read should be directly into the buffer. Not point in copying it elsewhere
earlier.

		ret = regmap_read(data->regmap,
				  SMI240_DATA_CAP_FIRST_REG + chan, &data->buf[i]);
		if (ret)
			goto out;

		i++;


> +		data->capture = SMI240_CAPTURE_OFF;
> +		if (ret)
> +			break;
> +		data->buf[i++] = sample;
> +	}
> +
> +	if (ret == 0)
> +		iio_push_to_buffers_with_timestamp(indio_dev, data->buf,
> +						   pf->timestamp);
Keep good path inline.
	if (ret)
		goto out;

	iio_push_to_buffers_with...

out:
	iio_trigger_notify_done(indio_dev->trig)
etc

Whilst it's a little more code, it's more consistent and lets a reviewer
see this is correct slightly more quickly.

> +
> +	iio_trigger_notify_done(indio_dev->trig);
> +	return IRQ_HANDLED;
> +}

> +static int smi240_write_raw(struct iio_dev *indio_dev,
> +			    struct iio_chan_spec const *chan, int val, int val2,
> +			    long mask)
> +{
> +	int ret, i;
> +	struct smi240_data *data = iio_priv(indio_dev);
> +
> +	switch (mask) {
> +	case IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY:
> +		for (i = 0; i < ARRAY_SIZE(smi240_low_pass_freqs); i++) {
> +			if (val == smi240_low_pass_freqs[i])
> +				break;
> +		}
> +
> +		if (i == ARRAY_SIZE(smi240_low_pass_freqs))
> +			return -EINVAL;
> +
> +		switch (chan->type) {
> +		case IIO_ACCEL:
> +			data->accel_filter_freq = val;
> +			break;
> +		case IIO_ANGL_VEL:
> +			data->anglvel_filter_freq = val;
> +			break;
> +		default:
> +			return -EINVAL;
> +		}
> +		break;
> +	default:
> +		return -EINVAL;
> +	}
> +
> +	// Write access to soft config is locked until hard/soft reset

Look at local comment syntax. Always use
/* */
in IIO

> +	ret = smi240_soft_reset(data);
> +	if (ret)
> +		return ret;
> +
> +	ret = smi240_soft_config(data);
> +	if (ret)
> +		return ret;
	return smi240_soft_config(data);

> +
> +	return 0;
> +}
> +
> +static int smi240_init(struct smi240_data *data)
> +{
> +	data->accel_filter_freq = SMI240_HIGH_BANDWIDTH_HZ;
> +	data->anglvel_filter_freq = SMI240_HIGH_BANDWIDTH_HZ;
> +	data->bite_reps = 3;
> +
> +	return smi240_soft_config(data);
> +}
> +
> +static IIO_CONST_ATTR_TEMP_SCALE("1/256");
> +static IIO_CONST_ATTR_TEMP_OFFSET("25");
> +
> +static struct attribute *smi240_attrs[] = {
> +	&iio_const_attr_in_temp_scale.dev_attr.attr,
> +	&iio_const_attr_in_temp_offset.dev_attr.attr,

Why can't you do these with normal channel info_mask elements?
This is not custom ABI, so shouldn't be done with specific
attributes like this.  We go through that dance with getting
IIO to create them because it enforced ABI + enables in kernel
users to access the channel.  Temperature may well be of interest
to other kernle drivrs.

> +	NULL,
If this was something you were keeping, not trailing comma on the
null terminator.

> +};
> +
> +static const struct attribute_group smi240_attrs_group = {
> +	.attrs = smi240_attrs,
> +};
> +
> +static const struct iio_info smi240_info = {
> +	.read_avail = smi240_read_avail,
> +	.read_raw = smi240_read_raw,
> +	.write_raw = smi240_write_raw,
> +	.attrs = &smi240_attrs_group,
> +};
> +


> +int smi240_core_probe(struct device *dev, struct regmap *regmap)
> +{
...

> +}
> +EXPORT_SYMBOL_GPL(smi240_core_probe);

All one module, no need to export symbols.


> diff --git a/drivers/iio/imu/smi240/smi240_spi.c b/drivers/iio/imu/smi240/smi240_spi.c
> new file mode 100644
> index 00000000000..d1ed92dce79
> --- /dev/null
> +++ b/drivers/iio/imu/smi240/smi240_spi.c
> @@ -0,0 +1,164 @@

...

> +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;
> +	__be32 request, response;
> +	struct spi_device *spi = context;
> +	struct iio_dev *indio_dev = dev_get_drvdata(&spi->dev);
> +	struct smi240_data *data = iio_priv(indio_dev);
> +
> +	request = SMI240_BUS_ID << 30;

As below. Build the u32 value up then convert to store in the __be32


> +	request |= FIELD_PREP(SMI240_CAP_BIT_MASK, data->capture);
> +	request |= FIELD_PREP(SMI240_WRITE_ADDR_MASK, *(u8 *)reg_buf);
> +	request |= smi240_crc3(request, SMI240_CRC_INIT, SMI240_CRC_POLY);
> +	request = 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, &request, sizeof(request));
> +	if (ret)
> +		return ret;
> +
> +	ret = spi_read(spi, &response, sizeof(response));
> +	if (ret)
> +		return ret;
> +
> +	response = be32_to_cpu(response);

Again, use two variables to avoid messing up u32 and __be32 types.

> +
> +	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)
> +{
> +	__be32 request;
> +	struct spi_device *spi = context;
> +	u8 reg_addr = ((u8 *)data)[0];
> +	u16 reg_data = ((u8 *)data)[2] << 8 | ((u8 *)data)[1];

Hmm. So this converts from little endian to CPU endian for this
value and then we convert it to big endian below. Odd, but maybe valid.

	u16 reg_data = get_unaligned_le16(&data[1]);

> +
> +	request = SMI240_BUS_ID << 30;
> +	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);
> +	request = cpu_to_be32(request);

Build the register in a u32 then at the end convert to be32 so that
the types are correct at all stages.
As you have probably noted, the automated testing really doesn't
like mixing up endian types like this.


> +
> +	return spi_write(spi, &request, sizeof(request));

Spi requires DMA safe buffers. Put somewhere to store this after your
buffer at the end of your iio_priv() structure.
Need to mark it __aligned(IIO_DMA_MINALIGN).


> +}

> +
> +static const struct spi_device_id smi240_spi_id[] = { { "smi240", 0 }, {} };
Lay that out as
static const struct spi_device_id smi240_spi_id[] = {
	{ "smi240", 0 },
	{ }
};

> +MODULE_DEVICE_TABLE(spi, smi240_spi_id);
> +
> +static const struct of_device_id smi240_of_match[] = {
> +	{ .compatible = "bosch,smi240" },
> +	{},
	{ }

so space and no trailing comma.

> +};
> +MODULE_DEVICE_TABLE(of, smi240_of_match);
diff mbox series

Patch

diff --git a/drivers/iio/imu/Kconfig b/drivers/iio/imu/Kconfig
index 52a155ff325..8808074513d 100644
--- a/drivers/iio/imu/Kconfig
+++ b/drivers/iio/imu/Kconfig
@@ -96,6 +96,7 @@  config KMX61
 
 source "drivers/iio/imu/inv_icm42600/Kconfig"
 source "drivers/iio/imu/inv_mpu6050/Kconfig"
+source "drivers/iio/imu/smi240/Kconfig"
 source "drivers/iio/imu/st_lsm6dsx/Kconfig"
 source "drivers/iio/imu/st_lsm9ds0/Kconfig"
 
diff --git a/drivers/iio/imu/Makefile b/drivers/iio/imu/Makefile
index 7e2d7d5c3b7..b6f162ae4ed 100644
--- a/drivers/iio/imu/Makefile
+++ b/drivers/iio/imu/Makefile
@@ -27,5 +27,6 @@  obj-y += inv_mpu6050/
 
 obj-$(CONFIG_KMX61) += kmx61.o
 
+obj-y += smi240/
 obj-y += st_lsm6dsx/
 obj-y += st_lsm9ds0/
diff --git a/drivers/iio/imu/smi240/Kconfig b/drivers/iio/imu/smi240/Kconfig
new file mode 100644
index 00000000000..b7f3598f6c4
--- /dev/null
+++ b/drivers/iio/imu/smi240/Kconfig
@@ -0,0 +1,12 @@ 
+config SMI240
+	tristate "Bosch Sensor SMI240 Inertial Measurement Unit"
+	depends on SPI
+	select REGMAP_SPI
+	select IIO_BUFFER
+	select IIO_TRIGGERED_BUFFER
+	help
+	  If you say yes here you get support for SMI240 IMU on SPI with
+	  accelerometer and gyroscope.
+
+	  This driver can also be built as a module. If so, the module will be
+	  called smi240.
diff --git a/drivers/iio/imu/smi240/Makefile b/drivers/iio/imu/smi240/Makefile
new file mode 100644
index 00000000000..0e5f7db7d78
--- /dev/null
+++ b/drivers/iio/imu/smi240/Makefile
@@ -0,0 +1,7 @@ 
+# SPDX-License-Identifier: BSD-3-Clause OR GPL-2.0
+#
+# Makefile for Bosch SMI240
+#
+obj-$(CONFIG_SMI240) += smi240.o
+smi240-objs := smi240_core.o
+smi240-objs += smi240_spi.o
diff --git a/drivers/iio/imu/smi240/smi240.h b/drivers/iio/imu/smi240/smi240.h
new file mode 100644
index 00000000000..a165bbd9f0b
--- /dev/null
+++ b/drivers/iio/imu/smi240/smi240.h
@@ -0,0 +1,32 @@ 
+/* SPDX-License-Identifier: BSD-3-Clause OR GPL-2.0 */
+/*
+ * Copyright (c) 2024 Robert Bosch GmbH.
+ *
+ */
+#ifndef _SMI240_H
+#define _SMI240_H
+
+#include <linux/device.h>
+#include <linux/regmap.h>
+
+enum capture_mode { SMI240_CAPTURE_OFF = 0, SMI240_CAPTURE_ON = 1 };
+
+struct smi240_data {
+	struct regmap *regmap;
+	u16 accel_filter_freq;
+	u16 anglvel_filter_freq;
+	u8 bite_reps;
+	enum capture_mode capture;
+	/*
+	 * Ensure natural alignment for timestamp if present.
+	 * Channel size: 2 bytes.
+	 * Max length needed: 2 * 3 channels + temp channel + 2 bytes padding + 8 byte ts.
+	 * If fewer channels are enabled, less space may be needed, as
+	 * long as the timestamp is still aligned to 8 bytes.
+	 */
+	s16 buf[12] __aligned(8);
+};
+
+int smi240_core_probe(struct device *dev, struct regmap *regmap);
+
+#endif /* _SMI240_H */
diff --git a/drivers/iio/imu/smi240/smi240_core.c b/drivers/iio/imu/smi240/smi240_core.c
new file mode 100644
index 00000000000..9e7269b90c9
--- /dev/null
+++ b/drivers/iio/imu/smi240/smi240_core.c
@@ -0,0 +1,386 @@ 
+// SPDX-License-Identifier: BSD-3-Clause OR GPL-2.0
+/*
+ * Copyright (c) 2024 Robert Bosch GmbH.
+ *
+ */
+#include <linux/regmap.h>
+#include <linux/delay.h>
+#include <linux/bits.h>
+#include <linux/bitfield.h>
+#include <linux/iio/iio.h>
+#include <linux/iio/sysfs.h>
+#include <linux/iio/triggered_buffer.h>
+#include <linux/iio/trigger_consumer.h>
+#include <linux/iio/buffer.h>
+#include <linux/iio/trigger.h>
+
+#include "smi240.h"
+
+enum {
+	SMI240_TEMP_OBJECT,
+	SMI240_SCAN_ACCEL_X,
+	SMI240_SCAN_ACCEL_Y,
+	SMI240_SCAN_ACCEL_Z,
+	SMI240_SCAN_GYRO_X,
+	SMI240_SCAN_GYRO_Y,
+	SMI240_SCAN_GYRO_Z,
+	SMI240_SCAN_TIMESTAMP,
+};
+
+#define SMI240_CHIP_ID 0x0024
+
+#define SMI240_SOFT_CONFIG_EOC_MASK BIT_MASK(0)
+#define SMI240_SOFT_CONFIG_GYR_BW_MASK BIT_MASK(1)
+#define SMI240_SOFT_CONFIG_ACC_BW_MASK BIT_MASK(2)
+#define SMI240_SOFT_CONFIG_BITE_AUTO_MASK BIT_MASK(3)
+#define SMI240_SOFT_CONFIG_BITE_REP_MASK GENMASK(6, 4)
+
+#define SMI240_CHIP_ID_REG 0x00
+#define SMI240_SOFT_CONFIG_REG 0x0A
+#define SMI240_TEMP_CUR_REG 0x10
+#define SMI240_ACCEL_X_CUR_REG 0x11
+#define SMI240_GYRO_X_CUR_REG 0x14
+#define SMI240_DATA_CAP_FIRST_REG 0x17
+#define SMI240_CMD_REG 0x2F
+
+#define SMI240_SOFT_RESET_CMD 0xB6
+
+#define SMI240_BITE_SEQUENCE_DELAY_US 140000
+#define SMI240_FILTER_FLUSH_DELAY_US 60000
+#define SMI240_DIGITAL_STARTUP_DELAY_US 120000
+#define SMI240_MECH_STARTUP_DELAY_US 100000
+
+#define SMI240_LOW_BANDWIDTH_HZ 50
+#define SMI240_HIGH_BANDWIDTH_HZ 400
+
+#define SMI240_DATA_CHANNEL(_type, _axis, _index)                          \
+	{                                                                  \
+		.type = _type, .modified = 1, .channel2 = IIO_MOD_##_axis, \
+		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),              \
+		.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),  \
+		.scan_index = _index,                                      \
+		.scan_type = {                                             \
+			.sign = 's',                                       \
+			.realbits = 16,                                    \
+			.storagebits = 16,                                 \
+			.endianness = IIO_LE,                              \
+		},                                                         \
+	}
+
+#define SMI240_TEMP_CHANNEL(_index)                           \
+	{                                                     \
+		.type = IIO_TEMP, .modified = 1,              \
+		.channel2 = IIO_MOD_TEMP_OBJECT,              \
+		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW), \
+		.scan_index = _index,                         \
+		.scan_type = {                                \
+			.sign = 's',                          \
+			.realbits = 16,                       \
+			.storagebits = 16,                    \
+			.endianness = IIO_LE,                 \
+		},                                            \
+	}
+
+static const struct iio_chan_spec smi240_channels[] = {
+	SMI240_TEMP_CHANNEL(SMI240_TEMP_OBJECT),
+	SMI240_DATA_CHANNEL(IIO_ACCEL, X, SMI240_SCAN_ACCEL_X),
+	SMI240_DATA_CHANNEL(IIO_ACCEL, Y, SMI240_SCAN_ACCEL_Y),
+	SMI240_DATA_CHANNEL(IIO_ACCEL, Z, SMI240_SCAN_ACCEL_Z),
+	SMI240_DATA_CHANNEL(IIO_ANGL_VEL, X, SMI240_SCAN_GYRO_X),
+	SMI240_DATA_CHANNEL(IIO_ANGL_VEL, Y, SMI240_SCAN_GYRO_Y),
+	SMI240_DATA_CHANNEL(IIO_ANGL_VEL, Z, SMI240_SCAN_GYRO_Z),
+	IIO_CHAN_SOFT_TIMESTAMP(SMI240_SCAN_TIMESTAMP),
+};
+
+static const int smi240_low_pass_freqs[] = { SMI240_LOW_BANDWIDTH_HZ,
+					     SMI240_HIGH_BANDWIDTH_HZ };
+
+static int smi240_soft_reset(struct smi240_data *data)
+{
+	int ret;
+
+	ret = regmap_write(data->regmap, SMI240_CMD_REG, SMI240_SOFT_RESET_CMD);
+	if (ret)
+		return ret;
+	fsleep(SMI240_DIGITAL_STARTUP_DELAY_US);
+	return 0;
+}
+
+static int smi240_soft_config(struct smi240_data *data)
+{
+	int ret;
+	u8 acc_bw, gyr_bw;
+	u16 request;
+
+	switch (data->accel_filter_freq) {
+	case SMI240_LOW_BANDWIDTH_HZ:
+		acc_bw = 0x1;
+		break;
+	case SMI240_HIGH_BANDWIDTH_HZ:
+		acc_bw = 0x0;
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	switch (data->anglvel_filter_freq) {
+	case SMI240_LOW_BANDWIDTH_HZ:
+		gyr_bw = 0x1;
+		break;
+	case SMI240_HIGH_BANDWIDTH_HZ:
+		gyr_bw = 0x0;
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	request = FIELD_PREP(SMI240_SOFT_CONFIG_EOC_MASK, 1);
+	request |= FIELD_PREP(SMI240_SOFT_CONFIG_GYR_BW_MASK, gyr_bw);
+	request |= FIELD_PREP(SMI240_SOFT_CONFIG_ACC_BW_MASK, acc_bw);
+	request |= FIELD_PREP(SMI240_SOFT_CONFIG_BITE_AUTO_MASK, 1);
+	request |= FIELD_PREP(SMI240_SOFT_CONFIG_BITE_REP_MASK,
+			      data->bite_reps - 1);
+
+	ret = regmap_write(data->regmap, SMI240_SOFT_CONFIG_REG, request);
+	if (ret)
+		return ret;
+
+	fsleep(SMI240_MECH_STARTUP_DELAY_US +
+	       data->bite_reps * SMI240_BITE_SEQUENCE_DELAY_US +
+	       SMI240_FILTER_FLUSH_DELAY_US);
+	return 0;
+}
+
+static int smi240_get_low_pass_filter_freq(struct smi240_data *data,
+					   int chan_type, int *val)
+{
+	switch (chan_type) {
+	case IIO_ACCEL:
+		*val = data->accel_filter_freq;
+		break;
+	case IIO_ANGL_VEL:
+		*val = data->anglvel_filter_freq;
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+static int smi240_get_data(struct smi240_data *data, int chan_type, int axis,
+			   int *val)
+{
+	u8 reg;
+	int ret, sample;
+
+	if (chan_type == IIO_TEMP)
+		reg = SMI240_TEMP_CUR_REG;
+	else if (chan_type == IIO_ACCEL)
+		reg = SMI240_ACCEL_X_CUR_REG + (axis - IIO_MOD_X);
+	else if (chan_type == IIO_ANGL_VEL)
+		reg = SMI240_GYRO_X_CUR_REG + (axis - IIO_MOD_X);
+	else
+		return -EINVAL;
+
+	ret = regmap_read(data->regmap, reg, &sample);
+	if (ret)
+		return ret;
+
+	*val = sign_extend32(sample, 15);
+
+	return 0;
+}
+
+static irqreturn_t smi240_trigger_handler(int irq, void *p)
+{
+	struct iio_poll_func *pf = p;
+	struct iio_dev *indio_dev = pf->indio_dev;
+	struct smi240_data *data = iio_priv(indio_dev);
+	int ret, sample, chan, i = 0;
+
+	data->capture = SMI240_CAPTURE_ON;
+
+	for_each_set_bit(chan, indio_dev->active_scan_mask,
+			 indio_dev->masklength) {
+		ret = regmap_read(data->regmap,
+				  SMI240_DATA_CAP_FIRST_REG + chan, &sample);
+		data->capture = SMI240_CAPTURE_OFF;
+		if (ret)
+			break;
+		data->buf[i++] = sample;
+	}
+
+	if (ret == 0)
+		iio_push_to_buffers_with_timestamp(indio_dev, data->buf,
+						   pf->timestamp);
+
+	iio_trigger_notify_done(indio_dev->trig);
+	return IRQ_HANDLED;
+}
+
+static int smi240_read_avail(struct iio_dev *indio_dev,
+			     struct iio_chan_spec const *chan, const int **vals,
+			     int *type, int *length, long mask)
+{
+	switch (mask) {
+	case IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY:
+		*vals = smi240_low_pass_freqs;
+		*length = ARRAY_SIZE(smi240_low_pass_freqs);
+		*type = IIO_VAL_INT;
+		return IIO_AVAIL_LIST;
+	default:
+		return -EINVAL;
+	}
+}
+
+static int smi240_read_raw(struct iio_dev *indio_dev,
+			   struct iio_chan_spec const *chan, int *val,
+			   int *val2, long mask)
+{
+	int ret;
+	struct smi240_data *data = iio_priv(indio_dev);
+
+	switch (mask) {
+	case IIO_CHAN_INFO_RAW:
+		if (iio_buffer_enabled(indio_dev))
+			return -EBUSY;
+		ret = smi240_get_data(data, chan->type, chan->channel2, val);
+		if (ret)
+			return ret;
+		return IIO_VAL_INT;
+
+	case IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY:
+		ret = smi240_get_low_pass_filter_freq(data, chan->type, val);
+		if (ret)
+			return ret;
+		return IIO_VAL_INT;
+
+	default:
+		return -EINVAL;
+	}
+}
+
+static int smi240_write_raw(struct iio_dev *indio_dev,
+			    struct iio_chan_spec const *chan, int val, int val2,
+			    long mask)
+{
+	int ret, i;
+	struct smi240_data *data = iio_priv(indio_dev);
+
+	switch (mask) {
+	case IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY:
+		for (i = 0; i < ARRAY_SIZE(smi240_low_pass_freqs); i++) {
+			if (val == smi240_low_pass_freqs[i])
+				break;
+		}
+
+		if (i == ARRAY_SIZE(smi240_low_pass_freqs))
+			return -EINVAL;
+
+		switch (chan->type) {
+		case IIO_ACCEL:
+			data->accel_filter_freq = val;
+			break;
+		case IIO_ANGL_VEL:
+			data->anglvel_filter_freq = val;
+			break;
+		default:
+			return -EINVAL;
+		}
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	// Write access to soft config is locked until hard/soft reset
+	ret = smi240_soft_reset(data);
+	if (ret)
+		return ret;
+
+	ret = smi240_soft_config(data);
+	if (ret)
+		return ret;
+
+	return 0;
+}
+
+static int smi240_init(struct smi240_data *data)
+{
+	data->accel_filter_freq = SMI240_HIGH_BANDWIDTH_HZ;
+	data->anglvel_filter_freq = SMI240_HIGH_BANDWIDTH_HZ;
+	data->bite_reps = 3;
+
+	return smi240_soft_config(data);
+}
+
+static IIO_CONST_ATTR_TEMP_SCALE("1/256");
+static IIO_CONST_ATTR_TEMP_OFFSET("25");
+
+static struct attribute *smi240_attrs[] = {
+	&iio_const_attr_in_temp_scale.dev_attr.attr,
+	&iio_const_attr_in_temp_offset.dev_attr.attr,
+	NULL,
+};
+
+static const struct attribute_group smi240_attrs_group = {
+	.attrs = smi240_attrs,
+};
+
+static const struct iio_info smi240_info = {
+	.read_avail = smi240_read_avail,
+	.read_raw = smi240_read_raw,
+	.write_raw = smi240_write_raw,
+	.attrs = &smi240_attrs_group,
+};
+
+int smi240_core_probe(struct device *dev, struct regmap *regmap)
+{
+	struct iio_dev *indio_dev;
+	struct smi240_data *data;
+	int ret, response;
+
+	indio_dev = devm_iio_device_alloc(dev, sizeof(*data));
+	if (!indio_dev)
+		return -ENOMEM;
+
+	data = iio_priv(indio_dev);
+	dev_set_drvdata(dev, indio_dev);
+	data->regmap = regmap;
+	data->capture = SMI240_CAPTURE_OFF;
+
+	ret = regmap_read(data->regmap, SMI240_CHIP_ID_REG, &response);
+	if (ret)
+		return dev_err_probe(dev, ret, "Read chip id failed\n");
+
+	if (response != SMI240_CHIP_ID)
+		dev_info(dev, "Unknown chip id: 0x%04x\n", response);
+
+	ret = smi240_init(data);
+	if (ret)
+		return dev_err_probe(dev, ret,
+				     "Device initialization failed\n");
+
+	indio_dev->channels = smi240_channels;
+	indio_dev->num_channels = ARRAY_SIZE(smi240_channels);
+	indio_dev->name = "smi240";
+	indio_dev->modes = INDIO_DIRECT_MODE;
+	indio_dev->info = &smi240_info;
+
+	ret = devm_iio_triggered_buffer_setup(dev, indio_dev,
+					      iio_pollfunc_store_time,
+					      smi240_trigger_handler, NULL);
+	if (ret)
+		return dev_err_probe(dev, ret,
+				     "Setup triggered buffer failed\n");
+
+	ret = devm_iio_device_register(dev, indio_dev);
+	if (ret)
+		return dev_err_probe(dev, ret, "Register IIO device failed\n");
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(smi240_core_probe);
diff --git a/drivers/iio/imu/smi240/smi240_spi.c b/drivers/iio/imu/smi240/smi240_spi.c
new file mode 100644
index 00000000000..d1ed92dce79
--- /dev/null
+++ b/drivers/iio/imu/smi240/smi240_spi.c
@@ -0,0 +1,164 @@ 
+// 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/regmap.h>
+#include <linux/spi/spi.h>
+#include <linux/bitfield.h>
+#include <linux/iio/iio.h>
+
+#include "smi240.h"
+
+#define SMI240_CRC_INIT 0x05
+#define SMI240_CRC_POLY 0x0B
+#define SMI240_BUS_ID 0x00
+
+#define SMI240_SD_BIT_MASK 0x80000000
+#define SMI240_CS_BIT_MASK 0x00000008
+
+#define SMI240_WRITE_ADDR_MASK GENMASK(29, 22)
+#define SMI240_WRITE_BIT_MASK 0x00200000
+#define SMI240_WRITE_DATA_MASK GENMASK(18, 3)
+#define SMI240_CAP_BIT_MASK 0x00100000
+#define SMI240_READ_DATA_MASK GENMASK(19, 4)
+
+static u8 smi240_crc3(u32 data, u8 init, u8 poly)
+{
+	u8 crc = init;
+	u8 do_xor;
+	s8 i = 31;
+
+	do {
+		do_xor = crc & 0x04;
+		crc <<= 1;
+		crc |= 0x01 & (data >> i);
+		if (do_xor)
+			crc ^= poly;
+
+		crc &= 0x07;
+	} while (--i >= 0);
+
+	return crc;
+}
+
+static bool smi240_sensor_data_is_valid(u32 data)
+{
+	if (smi240_crc3(data, SMI240_CRC_INIT, SMI240_CRC_POLY) != 0)
+		return false;
+
+	if (FIELD_GET(SMI240_SD_BIT_MASK, data) &
+	    FIELD_GET(SMI240_CS_BIT_MASK, data))
+		return false;
+
+	return true;
+}
+
+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;
+	__be32 request, response;
+	struct spi_device *spi = context;
+	struct iio_dev *indio_dev = dev_get_drvdata(&spi->dev);
+	struct smi240_data *data = iio_priv(indio_dev);
+
+	request = SMI240_BUS_ID << 30;
+	request |= FIELD_PREP(SMI240_CAP_BIT_MASK, data->capture);
+	request |= FIELD_PREP(SMI240_WRITE_ADDR_MASK, *(u8 *)reg_buf);
+	request |= smi240_crc3(request, SMI240_CRC_INIT, SMI240_CRC_POLY);
+	request = 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, &request, sizeof(request));
+	if (ret)
+		return ret;
+
+	ret = spi_read(spi, &response, sizeof(response));
+	if (ret)
+		return ret;
+
+	response = be32_to_cpu(response);
+
+	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)
+{
+	__be32 request;
+	struct spi_device *spi = context;
+	u8 reg_addr = ((u8 *)data)[0];
+	u16 reg_data = ((u8 *)data)[2] << 8 | ((u8 *)data)[1];
+
+	request = SMI240_BUS_ID << 30;
+	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);
+	request = cpu_to_be32(request);
+
+	return spi_write(spi, &request, sizeof(request));
+}
+
+static const struct regmap_bus smi240_regmap_bus = {
+	.read = smi240_regmap_spi_read,
+	.write = smi240_regmap_spi_write,
+};
+
+static const struct regmap_config smi240_regmap_config = {
+	.reg_bits = 8,
+	.val_bits = 16,
+	.val_format_endian = REGMAP_ENDIAN_LITTLE,
+};
+
+static int smi240_spi_probe(struct spi_device *spi)
+{
+	struct regmap *regmap;
+
+	regmap = devm_regmap_init(&spi->dev, &smi240_regmap_bus, &spi->dev,
+				  &smi240_regmap_config);
+	if (IS_ERR(regmap))
+		return dev_err_probe(&spi->dev, PTR_ERR(regmap),
+				     "Failed to initialize SPI Regmap\n");
+
+	return smi240_core_probe(&spi->dev, regmap);
+}
+
+static const struct spi_device_id smi240_spi_id[] = { { "smi240", 0 }, {} };
+MODULE_DEVICE_TABLE(spi, smi240_spi_id);
+
+static const struct of_device_id smi240_of_match[] = {
+	{ .compatible = "bosch,smi240" },
+	{},
+};
+MODULE_DEVICE_TABLE(of, smi240_of_match);
+
+static struct spi_driver smi240_spi_driver = {
+	.probe = smi240_spi_probe,
+	.id_table = smi240_spi_id,
+	.driver = {
+		.of_match_table = smi240_of_match,
+		.name = "smi240",
+	},
+};
+module_spi_driver(smi240_spi_driver);
+
+MODULE_AUTHOR("Markus Lochmann <markus.lochmann@de.bosch.com>");
+MODULE_AUTHOR("Stefan Gutmann <stefan.gutmann@de.bosch.com>");
+MODULE_DESCRIPTION("Bosch SMI240 SPI driver");
+MODULE_LICENSE("Dual BSD/GPL");