* [PATCH v3 0/2] iio: imu: smi240: cover-letter
@ 2024-08-15 15:25 Jianping.Shen
2024-08-15 15:25 ` [PATCH v3 1/2] dt-bindings: iio: imu: smi240: devicetree binding Jianping.Shen
2024-08-15 15:25 ` [PATCH v3 2/2] iio: imu: smi240: imu driver Jianping.Shen
0 siblings, 2 replies; 9+ messages in thread
From: Jianping.Shen @ 2024-08-15 15:25 UTC (permalink / raw)
To: jic23, lars, robh, krzk+dt, conor+dt, dima.fedrau,
marcelo.schmitt1, linux-iio, devicetree, linux-kernel,
Jianping.Shen, Christian.Lorenz3, Ulrike.Frauendorf, Kai.Dolde
From: Shen Jianping <Jianping.Shen@de.bosch.com>
This patchset adds the iio driver and the dt-binding 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.
dt-bindings:
v1 -> v2
- Add more detail in description
- Add maintainer
- Add vdd and vddio power supply
- Use generic node name
- Order the properties according to DTS coding style
v2 -> v3
- improve description
- improve supply definition
- make supply definition as required
- add supply definition in example
imu driver:
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
Shen Jianping (2):
dt-bindings: iio: imu: smi240: devicetree binding
iio: imu: smi240: imu driver
.../bindings/iio/imu/bosch,smi240.yaml | 52 +++
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 ++++++++
8 files changed, 655 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
--
2.34.1
^ permalink raw reply [flat|nested] 9+ messages in thread* [PATCH v3 1/2] dt-bindings: iio: imu: smi240: devicetree binding 2024-08-15 15:25 [PATCH v3 0/2] iio: imu: smi240: cover-letter Jianping.Shen @ 2024-08-15 15:25 ` Jianping.Shen 2024-08-15 15:34 ` Conor Dooley 2024-08-16 7:26 ` Krzysztof Kozlowski 2024-08-15 15:25 ` [PATCH v3 2/2] iio: imu: smi240: imu driver Jianping.Shen 1 sibling, 2 replies; 9+ messages in thread From: Jianping.Shen @ 2024-08-15 15:25 UTC (permalink / raw) To: jic23, lars, robh, krzk+dt, conor+dt, dima.fedrau, marcelo.schmitt1, linux-iio, devicetree, linux-kernel, Jianping.Shen, Christian.Lorenz3, Ulrike.Frauendorf, Kai.Dolde From: Shen Jianping <Jianping.Shen@de.bosch.com> dt-bindings: iio: imu: smi240: add sensor dt-binding Signed-off-by: Shen Jianping <Jianping.Shen@de.bosch.com> --- Notes: v1 -> v2 - Add more detail in description - Add maintainer - Add vdd and vddio power supply - Use generic node name - Order the properties according to DTS coding style v2 -> v3 - improve description - improve supply definition - make supply definition as required - add supply definition in example .../bindings/iio/imu/bosch,smi240.yaml | 52 +++++++++++++++++++ 1 file changed, 52 insertions(+) create mode 100644 Documentation/devicetree/bindings/iio/imu/bosch,smi240.yaml 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..6f1fb4b892c --- /dev/null +++ b/Documentation/devicetree/bindings/iio/imu/bosch,smi240.yaml @@ -0,0 +1,52 @@ +# 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 imu + +maintainers: + - Jianping Shen <Jianping.Shen@de.bosch.com> + +description: + 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. + https://www.bosch-semiconductors.com/mems-sensors/highly-automated-driving/smi240/ + +properties: + compatible: + const: bosch,smi240 + + reg: + maxItems: 1 + + vdd-supply: true + vddio-supply: true + +required: + - compatible + - reg + - vdd-supply + - vddio-supply + +allOf: + - $ref: /schemas/spi/spi-peripheral-props.yaml# + +unevaluatedProperties: false + +examples: + - | + spi { + #address-cells = <1>; + #size-cells = <0>; + + imu@0 { + compatible = "bosch,smi240"; + reg = <0>; + vdd-supply = <&vdd>; + vddio-supply = <&vddio>; + spi-max-frequency = <10000000>; + }; + }; -- 2.34.1 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH v3 1/2] dt-bindings: iio: imu: smi240: devicetree binding 2024-08-15 15:25 ` [PATCH v3 1/2] dt-bindings: iio: imu: smi240: devicetree binding Jianping.Shen @ 2024-08-15 15:34 ` Conor Dooley 2024-08-15 15:38 ` Conor Dooley 2024-08-16 7:26 ` Krzysztof Kozlowski 1 sibling, 1 reply; 9+ messages in thread From: Conor Dooley @ 2024-08-15 15:34 UTC (permalink / raw) To: Jianping.Shen Cc: jic23, lars, robh, krzk+dt, conor+dt, dima.fedrau, marcelo.schmitt1, linux-iio, devicetree, linux-kernel, Christian.Lorenz3, Ulrike.Frauendorf, Kai.Dolde [-- Attachment #1: Type: text/plain, Size: 462 bytes --] On Thu, Aug 15, 2024 at 05:25:44PM +0200, Jianping.Shen@de.bosch.com wrote: > From: Shen Jianping <Jianping.Shen@de.bosch.com> > > dt-bindings: iio: imu: smi240: add sensor dt-binding > Signed-off-by: Shen Jianping <Jianping.Shen@de.bosch.com> There should be a blank line between the body of the commit message and the signoff. That said, the body here just repeats $subject - but really should be something more like the description in the binding. [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 228 bytes --] ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v3 1/2] dt-bindings: iio: imu: smi240: devicetree binding 2024-08-15 15:34 ` Conor Dooley @ 2024-08-15 15:38 ` Conor Dooley 0 siblings, 0 replies; 9+ messages in thread From: Conor Dooley @ 2024-08-15 15:38 UTC (permalink / raw) To: Jianping.Shen Cc: jic23, lars, robh, krzk+dt, conor+dt, dima.fedrau, marcelo.schmitt1, linux-iio, devicetree, linux-kernel, Christian.Lorenz3, Ulrike.Frauendorf, Kai.Dolde [-- Attachment #1: Type: text/plain, Size: 676 bytes --] On Thu, Aug 15, 2024 at 04:34:05PM +0100, Conor Dooley wrote: > On Thu, Aug 15, 2024 at 05:25:44PM +0200, Jianping.Shen@de.bosch.com wrote: > > From: Shen Jianping <Jianping.Shen@de.bosch.com> > > > > dt-bindings: iio: imu: smi240: add sensor dt-binding > > Signed-off-by: Shen Jianping <Jianping.Shen@de.bosch.com> > > There should be a blank line between the body of the commit message and > the signoff. That said, the body here just repeats $subject - but really > should be something more like the description in the binding. Crap, I meant to say that with a fixed commit message, Reviewed-by: Conor Dooley <conor.dooley@microchip.com> Thanks, Conor. [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 228 bytes --] ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v3 1/2] dt-bindings: iio: imu: smi240: devicetree binding 2024-08-15 15:25 ` [PATCH v3 1/2] dt-bindings: iio: imu: smi240: devicetree binding Jianping.Shen 2024-08-15 15:34 ` Conor Dooley @ 2024-08-16 7:26 ` Krzysztof Kozlowski 1 sibling, 0 replies; 9+ messages in thread From: Krzysztof Kozlowski @ 2024-08-16 7:26 UTC (permalink / raw) To: Jianping.Shen, jic23, lars, robh, krzk+dt, conor+dt, dima.fedrau, marcelo.schmitt1, linux-iio, devicetree, linux-kernel, Christian.Lorenz3, Ulrike.Frauendorf, Kai.Dolde On 15/08/2024 17:25, Jianping.Shen@de.bosch.com wrote: > From: Shen Jianping <Jianping.Shen@de.bosch.com> > > dt-bindings: iio: imu: smi240: add sensor dt-binding Nothing improved. Please write proper commit msgs. See submitting-patches document. > Signed-off-by: Shen Jianping <Jianping.Shen@de.bosch.com> > --- > Best regards, Krzysztof ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v3 2/2] iio: imu: smi240: imu driver 2024-08-15 15:25 [PATCH v3 0/2] iio: imu: smi240: cover-letter Jianping.Shen 2024-08-15 15:25 ` [PATCH v3 1/2] dt-bindings: iio: imu: smi240: devicetree binding Jianping.Shen @ 2024-08-15 15:25 ` Jianping.Shen 2024-08-17 1:56 ` kernel test robot ` (2 more replies) 1 sibling, 3 replies; 9+ messages in thread From: Jianping.Shen @ 2024-08-15 15:25 UTC (permalink / raw) To: jic23, lars, robh, krzk+dt, conor+dt, dima.fedrau, marcelo.schmitt1, linux-iio, devicetree, linux-kernel, Jianping.Shen, Christian.Lorenz3, Ulrike.Frauendorf, Kai.Dolde 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 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"); -- 2.34.1 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH v3 2/2] iio: imu: smi240: imu driver 2024-08-15 15:25 ` [PATCH v3 2/2] iio: imu: smi240: imu driver Jianping.Shen @ 2024-08-17 1:56 ` kernel test robot 2024-08-17 16:13 ` kernel test robot 2024-08-17 16:17 ` Jonathan Cameron 2 siblings, 0 replies; 9+ messages in thread From: kernel test robot @ 2024-08-17 1:56 UTC (permalink / raw) To: Jianping.Shen, jic23, lars, robh, krzk+dt, conor+dt, dima.fedrau, marcelo.schmitt1, linux-iio, devicetree, linux-kernel, Christian.Lorenz3, Ulrike.Frauendorf, Kai.Dolde Cc: oe-kbuild-all 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 -- 0-DAY CI Kernel Test Service https://github.com/intel/lkp-tests/wiki ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v3 2/2] iio: imu: smi240: imu driver 2024-08-15 15:25 ` [PATCH v3 2/2] iio: imu: smi240: imu driver Jianping.Shen 2024-08-17 1:56 ` kernel test robot @ 2024-08-17 16:13 ` kernel test robot 2024-08-17 16:17 ` Jonathan Cameron 2 siblings, 0 replies; 9+ messages in thread From: kernel test robot @ 2024-08-17 16:13 UTC (permalink / raw) To: Jianping.Shen, jic23, lars, robh, krzk+dt, conor+dt, dima.fedrau, marcelo.schmitt1, linux-iio, devicetree, linux-kernel, Christian.Lorenz3, Ulrike.Frauendorf, Kai.Dolde Cc: oe-kbuild-all 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 -- 0-DAY CI Kernel Test Service https://github.com/intel/lkp-tests/wiki ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v3 2/2] iio: imu: smi240: imu driver 2024-08-15 15:25 ` [PATCH v3 2/2] iio: imu: smi240: imu driver Jianping.Shen 2024-08-17 1:56 ` kernel test robot 2024-08-17 16:13 ` kernel test robot @ 2024-08-17 16:17 ` Jonathan Cameron 2 siblings, 0 replies; 9+ messages in thread From: Jonathan Cameron @ 2024-08-17 16:17 UTC (permalink / raw) To: Jianping.Shen Cc: lars, robh, krzk+dt, conor+dt, dima.fedrau, marcelo.schmitt1, linux-iio, devicetree, linux-kernel, Christian.Lorenz3, Ulrike.Frauendorf, Kai.Dolde 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); ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2024-08-17 16:17 UTC | newest] Thread overview: 9+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2024-08-15 15:25 [PATCH v3 0/2] iio: imu: smi240: cover-letter Jianping.Shen 2024-08-15 15:25 ` [PATCH v3 1/2] dt-bindings: iio: imu: smi240: devicetree binding Jianping.Shen 2024-08-15 15:34 ` Conor Dooley 2024-08-15 15:38 ` Conor Dooley 2024-08-16 7:26 ` Krzysztof Kozlowski 2024-08-15 15:25 ` [PATCH v3 2/2] iio: imu: smi240: imu driver Jianping.Shen 2024-08-17 1:56 ` kernel test robot 2024-08-17 16:13 ` kernel test robot 2024-08-17 16:17 ` Jonathan Cameron
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox