From: "Shen Jianping (ME-SE/EAD2)" <Jianping.Shen-V5te9oGctAVWk0Htik3J/w@xxxxxxxxxxxxxxxx> iio: imu: smi240: driver improvements Signed-off-by: Shen Jianping (ME-SE/EAD2) <Jianping.Shen-V5te9oGctAVWk0Htik3J/w@xxxxxxxxxxxxxxxx> ---
Hi, ...
diff --git a/drivers/iio/imu/smi240/smi240_core.c b/drivers/iio/imu/smi240/smi240_core.c new file mode 100644 index 00000000000..4b2a4a290f3 --- /dev/null +++ b/drivers/iio/imu/smi240/smi240_core.c
...
+static int smi240_read_raw(struct iio_dev *indio_dev,
+ struct iio_chan_spec const *chan, int *val,
+ int *val2, long mask)
+{
+ int ret = 0;
No need to init (and in other places you don't)
+ 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;
Nitpick: missing new line?
+ ret = smi240_soft_config(data); + if (ret) + return ret; + + return 0; +}
...
+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 dev_err_probe(dev, -ENOMEM,
+ "Allocate iio device failed\n");
Usually messages related to memory allocation are not useful. Just: return -ENOMEM?
+ + data = iio_priv(indio_dev); + dev_set_drvdata(dev, indio_dev); + data->regmap = regmap; + data->capture = 0;
No need to explicitly initialize 'capture', devm_iio_device_alloc() already zeroes the allocated emmory.
It doesn't hurt to be explicit, but why this field and not the other ones?
+ + 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);
...
diff --git a/drivers/iio/imu/smi240/smi240_spi.c b/drivers/iio/imu/smi240/smi240_spi.c new file mode 100644 index 00000000000..ac9e37ffa37 --- /dev/null +++ b/drivers/iio/imu/smi240/smi240_spi.c
...
+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 struct regmap_bus smi240_regmap_bus = {
const?
+ .read = smi240_regmap_spi_read, + .write = smi240_regmap_spi_write, +};
... CJ
