RE: [PATCH] drivers: Bosch SMI240 IMU Driver

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Thank you for the fast feedback. We will rework on the findings and send you the new version soon.

Mit freundlichen Grüßen / Best regards

Jianping Shen

Mobility Electronics - Sensors, Engineering Advanced Development - MEMS Solutions Software (ME-SE/EAD2)
Robert Bosch GmbH | Postfach 13 42 | 72703 Reutlingen | GERMANY | www.bosch.com
Tel. +49 7121 35-37749 | Telefax +49 711 811-509378 | Jianping.Shen@xxxxxxxxxxxx

Sitz: Stuttgart, Registergericht: Amtsgericht Stuttgart, HRB 14000;
Aufsichtsratsvorsitzender: Prof. Dr. Stefan Asenkerschbaumer; 
Geschäftsführung: Dr. Stefan Hartung, Dr. Christian Fischer, Dr. Markus Forschner, 
Stefan Grosch, Dr. Markus Heyn, Dr. Frank Meyer, Dr. Tanja Rückert

-----Original Message-----
From: Krzysztof Kozlowski <krzk@xxxxxxxxxx> 
Sent: Thursday, July 18, 2024 2:47 PM
To: Shen Jianping (ME-SE/EAD2) <Jianping.Shen@xxxxxxxxxxxx>; jic23@xxxxxxxxxx; lars@xxxxxxxxxx; robh@xxxxxxxxxx; krzk+dt@xxxxxxxxxx; conor+dt@xxxxxxxxxx; dima.fedrau@xxxxxxxxx; marcelo.schmitt1@xxxxxxxxx; linux-iio@xxxxxxxxxxxxxxx; devicetree@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx; Lorenz Christian (ME-SE/EAD2) <Christian.Lorenz3@xxxxxxxxxxxx>; Frauendorf Ulrike (ME/PJ-SW3) <Ulrike.Frauendorf@xxxxxxxxxxxx>; Dolde Kai (ME-SE/PAE-A3) <Kai.Dolde@xxxxxxxxxxxx>
Subject: Re: [PATCH] drivers: Bosch SMI240 IMU Driver

On 18/07/2024 14:24, Jianping.Shen@xxxxxxxxxxxx wrote:
> From: "Shen Jianping (ME-SE/EAD2)" 
> <she2rt@xxxxxxxxxxxxxxxxxxxxxxxxxxxx>
> 
> 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@xxxxxxxxxxxxxxxxxxxxxxxxxxxx>
> ---
>  .../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: 
> +https://eur03.safelinks.protection.outlook.com/?url=http%3A%2F%2Fdevi
> +cetree.org%2Fschemas%2Fiio%2Fimu%2Fbosch%2Csmi240.yaml%23&data=05%7C0
> +2%7CJianping.Shen%40de.bosch.com%7C666fcf2fcad249cc892908dca727cdda%7
> +C0ae51e1907c84e4bbb6d648ee58410f4%7C0%7C0%7C638569036597203543%7CUnkn
> +own%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWw
> +iLCJXVCI6Mn0%3D%7C0%7C%7C%7C&sdata=ijO6JQTkrWJw5XLJMk%2FKey6Wq%2BvEk%
> +2B0FmIAwLiNaSxM%3D&reserved=0
> +$schema: 
> +https://eur03.safelinks.protection.outlook.com/?url=http%3A%2F%2Fdevi
> +cetree.org%2Fmeta-schemas%2Fcore.yaml%23&data=05%7C02%7CJianping.Shen
> +%40de.bosch.com%7C666fcf2fcad249cc892908dca727cdda%7C0ae51e1907c84e4b
> +bb6d648ee58410f4%7C0%7C0%7C638569036597212752%7CUnknown%7CTWFpbGZsb3d
> +8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7
> +C0%7C%7C%7C&sdata=mbYZyVVoFzmBDNT0YELRFQmV6Ag3AJNJ%2BjGJKN%2BTv2k%3D&
> +reserved=0
> +
> +title: BOSCH SMI240

Also: BOSCH or Bosch?

> +
> +maintainers:
> +  - unknown
> +
> +description: |
> +  Inertial Measurement Unit with Accelerometer, Gyroscope
> +  
> +https://eur03.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwww
> +.bosch-semiconductors.com%2Fmems-sensors%2Fhighly-automated-driving%2
> +Fsmi240%2F&data=05%7C02%7CJianping.Shen%40de.bosch.com%7C666fcf2fcad2
> +49cc892908dca727cdda%7C0ae51e1907c84e4bbb6d648ee58410f4%7C0%7C0%7C638
> +569036597218023%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV
> +2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C0%7C%7C%7C&sdata=1OK2dyvXLmN0
> +STEslacOHryAVJ%2F0%2BQcILMtirBmQDwc%3D&reserved=0
> +
> +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






[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux