| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20250927154428.381df54e@jic23-huawei>
Date: Sat, 27 Sep 2025 15:44:28 +0100
From: Jonathan Cameron <jic23@...nel.org>
To: <Jianping.Shen@...bosch.com>
Cc: <lars@...afoo.de>, <robh@...nel.org>, <krzk+dt@...nel.org>,
<conor+dt@...nel.org>, <dima.fedrau@...il.com>,
<marcelo.schmitt1@...il.com>, <linux-iio@...r.kernel.org>,
<devicetree@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
<Christian.Lorenz3@...bosch.com>, <Ulrike.Frauendorf@...bosch.com>,
<Kai.Dolde@...bosch.com>
Subject: Re: [PATCH v4 2/2] iio: imu: smi330: Add driver
On Mon, 15 Sep 2025 18:09:34 +0200
<Jianping.Shen@...bosch.com> wrote:
> From: Jianping Shen <Jianping.Shen@...bosch.com>
>
> Add the iio driver for bosch imu smi330. The smi330 is a combined
> three axis angular rate and three axis acceleration sensor.
>
> Signed-off-by: Jianping Shen <Jianping.Shen@...bosch.com>
Applied to the testing branch of iio.git (which will be rebased on rc1)
with following diff;
diff --git a/drivers/iio/imu/smi330/Kconfig b/drivers/iio/imu/smi330/Kconfig
index 95b06f2317e8..915a5b6fb974 100644
--- a/drivers/iio/imu/smi330/Kconfig
+++ b/drivers/iio/imu/smi330/Kconfig
@@ -7,6 +7,7 @@ config SMI330
tristate "Bosch Sensor SMI330 Inertial Measurement Unit"
select IIO_BUFFER
select IIO_TRIGGERED_BUFFER
+ depends on I2C || SPI
help
Enable support for the Bosch SMI330 IMU.
diff --git a/drivers/iio/imu/smi330/smi330_core.c b/drivers/iio/imu/smi330/smi330_core.c
index fdbc251d1f60..8571414d2a55 100644
--- a/drivers/iio/imu/smi330/smi330_core.c
+++ b/drivers/iio/imu/smi330/smi330_core.c
@@ -491,8 +491,6 @@ static int smi330_read_avail(struct iio_dev *indio_dev,
default:
return -EINVAL;
}
-
- return -EINVAL;
}
static int smi330_read_raw(struct iio_dev *indio_dev,
> diff --git a/drivers/iio/imu/smi330/Kconfig b/drivers/iio/imu/smi330/Kconfig
> new file mode 100644
> index 00000000000..95b06f2317e
> --- /dev/null
> +++ b/drivers/iio/imu/smi330/Kconfig
> @@ -0,0 +1,39 @@
> +# SPDX-License-Identifier: GPL-2.0
> +#
> +# SMI330 IMU driver
> +#
> +
> +config SMI330
> + tristate "Bosch Sensor SMI330 Inertial Measurement Unit"
> + select IIO_BUFFER
> + select IIO_TRIGGERED_BUFFER
Why expose this as an option, whether or not either bus is supported?
At minimum this should be
depends on I2C || SPI
Often we just don't make this a selectable symbol though that
complicates the select statements here hence I've just made this tweak.
> + help
> + Enable support for the Bosch SMI330 IMU.
> +
> + The driver supports different operation modes like polling,
> + data ready or fifo mode and advanced features like no-motion,
> + no-motion, any-motion or tilt detection.
> +
> +config SMI330_I2C
> + tristate "Bosch SMI330 I2C driver"
> + depends on I2C
> + select SMI330
> + select REGMAP_I2C
> + help
> + Enable support for the Bosch SMI330 6-Axis IMU connected to I2C
> + interface.
> +
> + This driver can also be built as a module. If so, the module will be
> + called smi330_i2c.
> +
> +config SMI330_SPI
> + tristate "Bosch SMI330 SPI driver"
> + depends on SPI
> + select SMI330
> + select REGMAP_SPI
> + help
> + Enable support for the Bosch SMI330 6-Axis IMU connected to SPI
> + interface.
> +
> + This driver can also be built as a module. If so, the module will be
> + called smi330_spi.
> diff --git a/drivers/iio/imu/smi330/smi330_core.c b/drivers/iio/imu/smi330/smi330_core.c
> new file mode 100644
> index 00000000000..fdbc251d1f6
> --- /dev/null
> +++ b/drivers/iio/imu/smi330/smi330_core.c
> +/* Register mask */
> +#define SMI330_CHIP_ID_MASK GENMASK(7, 0)
> +#define SMI330_ERR_FATAL_MASK BIT(0)
> +#define SMI330_ERR_ACC_CONF_MASK BIT(5)
> +#define SMI330_ERR_GYR_CONF_MASK BIT(6)
> +#define SMI330_STATUS_POR_MASK BIT(0)
> +#define SMI330_INT_STATUS_ACC_GYR_DRDY_MASK GENMASK(13, 12)
> +#define SMI330_CFG_ODR_MASK GENMASK(3, 0)
> +#define SMI330_CFG_RANGE_MASK GENMASK(6, 4)
> +#define SMI330_CFG_BW_MASK BIT(7)
> +#define SMI330_CFG_AVG_NUM_MASK GENMASK(10, 8)
> +#define SMI330_CFG_MODE_MASK GENMASK(14, 12)
> +#define SMI330_IO_INT_CTRL_INT1_MASK GENMASK(2, 0)
> +#define SMI330_IO_INT_CTRL_INT2_MASK GENMASK(10, 8)
> +#define SMI330_INT_CONF_LATCH_MASK BIT(0)
> +#define SMI330_INT_MAP2_ACC_DRDY_MASK GENMASK(11, 10)
> +#define SMI330_INT_MAP2_GYR_DRDY_MASK GENMASK(9, 8)
> +#define SMI330_INT_MAP2_DRDY_MASK \
> + (SMI330_INT_MAP2_ACC_DRDY_MASK | SMI330_INT_MAP2_GYR_DRDY_MASK)
Trivial but I'm not sure this composite mask is useful. I would just
have used the two masks inline. I didn't bother modifying this though.
> +
> +static int smi330_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_SCALE:
> + if (chan->type == IIO_ACCEL) {
> + *vals = smi330_accel_scale_attr.vals;
> + *length = smi330_accel_scale_attr.len;
> + *type = smi330_accel_scale_attr.type;
> + } else {
> + *vals = smi330_gyro_scale_attr.vals;
> + *length = smi330_gyro_scale_attr.len;
> + *type = smi330_gyro_scale_attr.type;
> + }
> + return IIO_AVAIL_LIST;
> + case IIO_CHAN_INFO_OVERSAMPLING_RATIO:
> + *vals = smi330_average_attr.vals;
> + *length = smi330_average_attr.len;
> + *type = smi330_average_attr.type;
> + *type = IIO_VAL_INT;
> + return IIO_AVAIL_LIST;
> + case IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY:
> + *vals = smi330_bandwidth_attr.vals;
> + *length = smi330_bandwidth_attr.len;
> + *type = smi330_bandwidth_attr.type;
> + return IIO_AVAIL_LIST;
> + case IIO_CHAN_INFO_SAMP_FREQ:
> + *vals = smi330_odr_attr.vals;
> + *length = smi330_odr_attr.len;
> + *type = smi330_odr_attr.type;
> + return IIO_AVAIL_LIST;
> + default:
> + return -EINVAL;
> + }
> +
Unreachable code. I'm a bit surprised the compiler didn't moan about this.
> + return -EINVAL;
> +}
Powered by blists - more mailing lists