Re: [PATCH v2 2/2] iio: imu: smi240: imu driver

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

 



Le 09/08/2024 à 13:16, Jianping.Shen-V5te9oGctAVWk0Htik3J/w@xxxxxxxxxxxxxxxx a écrit :
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





[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