Re: [PATCH] drivers: Bosch SMI240 IMU Driver

[Krzysztof Kozlowski <krzk@kernel.org>]

On 18/07/2024 14:24, Jianping.Shen@de.bosch.com wrote:
> From: "Shen Jianping (ME-SE/EAD2)" <she2rt@LR-C-0008DVM.rt.de.bosch.com>
> 
> Add Bosch SMI240 IMU IIO Driver to iio-for-6.10b

What is "iio-for-6.10b"? How is it relevant to git history? This is
supposed to say something about the driver and hardware.

> 
> Signed-off-by: Shen Jianping (ME-SE/EAD2) <she2rt@LR-C-0008DVM.rt.de.bosch.com>
> ---
>  .../bindings/iio/imu/bosch,smi240.yaml        |  45 +
>  drivers/iio/imu/Kconfig                       |   2 +
>  drivers/iio/imu/Makefile                      |   1 +
>  drivers/iio/imu/smi240/Kconfig                |  30 +
>  drivers/iio/imu/smi240/Makefile               |   8 +
>  drivers/iio/imu/smi240/smi240.h               |  31 +
>  drivers/iio/imu/smi240/smi240_core.c          | 814 ++++++++++++++++++
>  drivers/iio/imu/smi240/smi240_spi.c           | 153 ++++
>  8 files changed, 1084 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/iio/imu/bosch,smi240.yaml
>  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
> 
> diff --git a/Documentation/devicetree/bindings/iio/imu/bosch,smi240.yaml b/Documentation/devicetree/bindings/iio/imu/bosch,smi240.yaml
> new file mode 100644
> index 00000000000..972819cacff
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/imu/bosch,smi240.yaml
> @@ -0,0 +1,45 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/iio/imu/bosch,smi240.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: BOSCH SMI240

Also: BOSCH or Bosch?

> +
> +maintainers:
> +  - unknown
> +
> +description: |
> +  Inertial Measurement Unit with Accelerometer, Gyroscope 
> +  https://www.bosch-semiconductors.com/mems-sensors/highly-automated-driving/smi240/
> +
> +properties:
> +  compatible:
> +    const: BOSCH,SMI240
> +
> +  reg:
> +    maxItems: 1
> +
> +required:
> +  - compatible
> +  - spi-max-frequency
> +  - reg
> +
> +allOf:
> +  - $ref: /schemas/spi/spi-peripheral-props.yaml#
> +
> +unevaluatedProperties: false
> +
> +examples:
> +  - |
> +    // Example
> +    spi {
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +
> +        smi240@0 {
> +            compatible = "BOSCH,SMI240";
> +            spi-max-frequency = <10000000>;
> +            reg = <0>;
> +        };
> +    };
> diff --git a/drivers/iio/imu/Kconfig b/drivers/iio/imu/Kconfig
> index 52a155ff325..2c348ad686a 100644
> --- a/drivers/iio/imu/Kconfig
> +++ b/drivers/iio/imu/Kconfig
> @@ -96,6 +96,8 @@ config KMX61
>  
>  source "drivers/iio/imu/inv_icm42600/Kconfig"
>  source "drivers/iio/imu/inv_mpu6050/Kconfig"
> +source "/home/she2rt/dev/smi240-linux-driver-iio/drivers/iio/imu/smi240/Kconfig"

Yeah... this won't work, obviously.

> +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..7114c941cc3
> --- /dev/null
> +++ b/drivers/iio/imu/smi240/Kconfig
> @@ -0,0 +1,30 @@

Missing SPDX

> +config SMI240
> +	tristate "Bosch Sensor SMI240 Inertial Measurement Unit"
> +	depends on SPI_MASTER
> +	select IIO_BUFFER
> +	select IIO_TRIGGERED_BUFFER
> +	help
> +	  Build driver
> +	  for Bosch
> +	  SMI240 6-axis IMU
> +	  sensor.

Ooh my... what's this style?

Read coding style for Kconfig.


> +
> +config SMI240_MAX_BUFFER_LEN

Drop, dead code.

> +	depends on SMI240
> +	int "configue read buffer size"
> +	default "1024"
> +	help
> +	  1024 bytes are big
> +	  enough for most cases.
> +	  Do not change this value
> +	  if not sure.
> +
> +config SMI240_UNIT_TEST

Drop, dead code.

> +	tristate "Unit Test for SMI240"
> +	depends on KUNIT=y
> +	help
> +	  Build Unit Test
> +	  for Bosch
> +	  SMI240 6-axis
> +	  IMU sensor.
> +
> diff --git a/drivers/iio/imu/smi240/Makefile b/drivers/iio/imu/smi240/Makefile
> new file mode 100644
> index 00000000000..394eaecf5f3
> --- /dev/null
> +++ b/drivers/iio/imu/smi240/Makefile
> @@ -0,0 +1,8 @@
> +#
> +# Makefile for Bosch SMI240

Drop. It cannot be anything else. Do not say that "x" is a "x".

You miss SPDX on the other hand.

> +#
> +
> +obj-$(CONFIG_SMI240) += smi240.o
> +smi240-objs := smi240_core.o


Best regards,
Krzysztof