[PATCH 1/5] MAINTAINERS: add ADIS IMU driver library entry

[PATCH 1/5] MAINTAINERS: add ADIS IMU driver library entry

[Alexandru Ardelean]

This change adds the ADIS driver library to the MAINTAINERS list, and adds
myself as the current maintainer of this library.

Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
---
 MAINTAINERS | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 1eb971608ac4..544e23753e96 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -930,6 +930,13 @@ S:	Supported
 F:	drivers/mux/adgs1408.c
 F:	Documentation/devicetree/bindings/mux/adi,adgs1408.txt
 
+ANALOG DEVICES INC ADIS DRIVER LIBRARY
+M:	Alexandru Ardelean <alexandru.ardelean@analog.com>
+S:	Supported
+L:	linux-iio@vger.kernel.org
+F:	include/linux/iio/imu/adis.h
+F:	drivers/iio/imu/adis.c
+
 ANALOG DEVICES INC ADP5061 DRIVER
 M:	Stefan Popa <stefan.popa@analog.com>
 L:	linux-pm@vger.kernel.org
-- 
2.20.1

[PATCH 2/5] drivers: spi: core: Add optional stall delay between cs_change transfers

[Alexandru Ardelean]

Some devices like the ADIS16460 IMU require a stall period between
transfers, i.e. between when the CS is de-asserted and re-asserted. The
default value of 10us is not enough. This change makes the delay
configurable for when the next CS change goes active.

Signed-off-by: Michael Hennerich <michael.hennerich@analog.com>
Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
---
 drivers/spi/spi.c       | 3 ++-
 include/linux/spi/spi.h | 3 +++
 2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
index 5e75944ad5d1..739de0118ee1 100644
--- a/drivers/spi/spi.c
+++ b/drivers/spi/spi.c
@@ -1163,7 +1163,8 @@ static int spi_transfer_one_message(struct spi_controller *ctlr,
 				keep_cs = true;
 			} else {
 				spi_set_cs(msg->spi, false);
-				udelay(10);
+				udelay(xfer->cs_change_stall_delay_us ?
+				       xfer->cs_change_stall_delay_us : 10);
 				spi_set_cs(msg->spi, true);
 			}
 		}
diff --git a/include/linux/spi/spi.h b/include/linux/spi/spi.h
index 053abd22ad31..d23add3b4790 100644
--- a/include/linux/spi/spi.h
+++ b/include/linux/spi/spi.h
@@ -734,6 +734,8 @@ extern void spi_res_release(struct spi_controller *ctlr,
  *      transfer. If 0 the default (from @spi_device) is used.
  * @bits_per_word: select a bits_per_word other than the device default
  *      for this transfer. If 0 the default (from @spi_device) is used.
+ * @cs_change_stall_delay_us: microseconds to delay between cs_change
+ * 	transfers.
  * @cs_change: affects chipselect after this transfer completes
  * @delay_usecs: microseconds to delay after this transfer before
  *	(optionally) changing the chipselect status, then starting
@@ -823,6 +825,7 @@ struct spi_transfer {
 #define	SPI_NBITS_QUAD		0x04 /* 4bits transfer */
 	u8		bits_per_word;
 	u8		word_delay_usecs;
+	u8		cs_change_stall_delay_us;
 	u16		delay_usecs;
 	u32		speed_hz;
 	u16		word_delay;
-- 
2.20.1

[PATCH 3/5] iio: imu: adis: Add support for SPI transfer cs_change_stall_delay_us

[Alexandru Ardelean]

The ADIS16460 requires a higher delay before the next transfer. Since the
SPI framework supports configuring the delay before the next transfer, this
driver will become the first user of it.

The support for this functionality in ADIS16460 requires an addition to the
ADIS lib to support the `cs_change_stall_delay_us` functionality in SPI.

Signed-off-by: Michael Hennerich <michael.hennerich@analog.com>
Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
---
 drivers/iio/imu/adis.c       | 9 +++++++++
 include/linux/iio/imu/adis.h | 2 ++
 2 files changed, 11 insertions(+)

diff --git a/drivers/iio/imu/adis.c b/drivers/iio/imu/adis.c
index c771ae6803a9..90dac69910b3 100644
--- a/drivers/iio/imu/adis.c
+++ b/drivers/iio/imu/adis.c
@@ -40,28 +40,33 @@ int adis_write_reg(struct adis *adis, unsigned int reg,
 			.len = 2,
 			.cs_change = 1,
 			.delay_usecs = adis->data->write_delay,
+			.cs_change_stall_delay_us = adis->data->cs_stall_delay,
 		}, {
 			.tx_buf = adis->tx + 2,
 			.bits_per_word = 8,
 			.len = 2,
 			.cs_change = 1,
 			.delay_usecs = adis->data->write_delay,
+			.cs_change_stall_delay_us = adis->data->cs_stall_delay,
 		}, {
 			.tx_buf = adis->tx + 4,
 			.bits_per_word = 8,
 			.len = 2,
 			.cs_change = 1,
 			.delay_usecs = adis->data->write_delay,
+			.cs_change_stall_delay_us = adis->data->cs_stall_delay,
 		}, {
 			.tx_buf = adis->tx + 6,
 			.bits_per_word = 8,
 			.len = 2,
 			.delay_usecs = adis->data->write_delay,
+			.cs_change_stall_delay_us = adis->data->cs_stall_delay,
 		}, {
 			.tx_buf = adis->tx + 8,
 			.bits_per_word = 8,
 			.len = 2,
 			.delay_usecs = adis->data->write_delay,
+			.cs_change_stall_delay_us = adis->data->cs_stall_delay,
 		},
 	};
 
@@ -134,12 +139,14 @@ int adis_read_reg(struct adis *adis, unsigned int reg,
 			.len = 2,
 			.cs_change = 1,
 			.delay_usecs = adis->data->write_delay,
+			.cs_change_stall_delay_us = adis->data->cs_stall_delay,
 		}, {
 			.tx_buf = adis->tx + 2,
 			.bits_per_word = 8,
 			.len = 2,
 			.cs_change = 1,
 			.delay_usecs = adis->data->read_delay,
+			.cs_change_stall_delay_us = adis->data->cs_stall_delay,
 		}, {
 			.tx_buf = adis->tx + 4,
 			.rx_buf = adis->rx,
@@ -147,11 +154,13 @@ int adis_read_reg(struct adis *adis, unsigned int reg,
 			.len = 2,
 			.cs_change = 1,
 			.delay_usecs = adis->data->read_delay,
+			.cs_change_stall_delay_us = adis->data->cs_stall_delay,
 		}, {
 			.rx_buf = adis->rx + 2,
 			.bits_per_word = 8,
 			.len = 2,
 			.delay_usecs = adis->data->read_delay,
+			.cs_change_stall_delay_us = adis->data->cs_stall_delay,
 		},
 	};
 
diff --git a/include/linux/iio/imu/adis.h b/include/linux/iio/imu/adis.h
index 469a493f7ae0..4aa248b6b3bd 100644
--- a/include/linux/iio/imu/adis.h
+++ b/include/linux/iio/imu/adis.h
@@ -27,6 +27,7 @@ struct adis_burst;
  * struct adis_data - ADIS chip variant specific data
  * @read_delay: SPI delay for read operations in us
  * @write_delay: SPI delay for write operations in us
+ * @cs_stall_delay: SPI stall delay between transfers in us
  * @glob_cmd_reg: Register address of the GLOB_CMD register
  * @msc_ctrl_reg: Register address of the MSC_CTRL register
  * @diag_stat_reg: Register address of the DIAG_STAT register
@@ -36,6 +37,7 @@ struct adis_burst;
 struct adis_data {
 	unsigned int read_delay;
 	unsigned int write_delay;
+	unsigned int cs_stall_delay;
 
 	unsigned int glob_cmd_reg;
 	unsigned int msc_ctrl_reg;
-- 
2.20.1

[PATCH 4/5] iio: imu: Add support for the ADIS16460 IMU

[Alexandru Ardelean]

The ADIS16460 device is a complete inertial system that includes a triaxial
gyroscope and a triaxial accelerometer. It's more simplified design than
that of the ADIS16480, and does not offer the triaxial magnetometers &
pressure sensors. It does also have a temperature sensor (like the
ADIS16480).
Since it is part of the ADIS16XXX family, it re-uses parts of the ADIS
library.

Naturally, the register map is different and much more simplified than the
ADIS16480 subfamily, so it cannot be integrated into that driver. A major
difference is that the registers are not paged.

One thing that is particularly special about it, is that it requires a
higher CS stall delay between data (around 16 uS vs other chips from the
family requiring around 2 uS).

Datasheet:
  https://www.analog.com/media/en/technical-documentation/data-sheets/adis16460.pdf

Signed-off-by: Dragos Bogdan <dragos.bogdan@analog.com>
Signed-off-by: Michael Hennerich <michael.hennerich@analog.com>
Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
---
 MAINTAINERS                 |   7 +
 drivers/iio/imu/Kconfig     |   9 +
 drivers/iio/imu/Makefile    |   1 +
 drivers/iio/imu/adis16460.c | 490 ++++++++++++++++++++++++++++++++++++
 4 files changed, 507 insertions(+)
 create mode 100644 drivers/iio/imu/adis16460.c

diff --git a/MAINTAINERS b/MAINTAINERS
index 544e23753e96..ef2e2cee32e1 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -937,6 +937,13 @@ L:	linux-iio@vger.kernel.org
 F:	include/linux/iio/imu/adis.h
 F:	drivers/iio/imu/adis.c
 
+ANALOG DEVICES INC ADIS16460 DRIVER
+M:	Dragos Bogdan <dragos.bogdan@analog.com>
+S:	Supported
+L:	linux-iio@vger.kernel.org
+W:	http://ez.analog.com/community/linux-device-drivers
+F:	drivers/iio/imu/adis16460.c
+
 ANALOG DEVICES INC ADP5061 DRIVER
 M:	Stefan Popa <stefan.popa@analog.com>
 L:	linux-pm@vger.kernel.org
diff --git a/drivers/iio/imu/Kconfig b/drivers/iio/imu/Kconfig
index 156630a21696..a29a481c20d2 100644
--- a/drivers/iio/imu/Kconfig
+++ b/drivers/iio/imu/Kconfig
@@ -16,6 +16,15 @@ config ADIS16400
 	  adis16365, adis16400 and adis16405 triaxial inertial sensors
 	  (adis16400 series also have magnetometers).
 
+config ADIS16460
+	tristate "Analog Devices ADIS16460 and similar IMU driver"
+	depends on SPI
+	select IIO_ADIS_LIB
+	select IIO_ADIS_LIB_BUFFER if IIO_BUFFER
+	help
+	  Say yes here to build support for Analog Devices ADIS16460 inertial
+	  sensor.
+
 config ADIS16480
 	tristate "Analog Devices ADIS16480 and similar IMU driver"
 	depends on SPI
diff --git a/drivers/iio/imu/Makefile b/drivers/iio/imu/Makefile
index 9e452fce1aaf..4a6958865504 100644
--- a/drivers/iio/imu/Makefile
+++ b/drivers/iio/imu/Makefile
@@ -5,6 +5,7 @@
 
 # When adding new entries keep the list in alphabetical order
 obj-$(CONFIG_ADIS16400) += adis16400.o
+obj-$(CONFIG_ADIS16460) += adis16460.o
 obj-$(CONFIG_ADIS16480) += adis16480.o
 
 adis_lib-y += adis.o
diff --git a/drivers/iio/imu/adis16460.c b/drivers/iio/imu/adis16460.c
new file mode 100644
index 000000000000..6c86af06b5d1
--- /dev/null
+++ b/drivers/iio/imu/adis16460.c
@@ -0,0 +1,490 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * ADIS16460 IMU driver
+ *
+ * Copyright 2019 Analog Devices Inc.
+ *
+ */
+
+#include <linux/delay.h>
+#include <linux/module.h>
+#include <linux/spi/spi.h>
+
+#include <linux/iio/iio.h>
+#include <linux/iio/imu/adis.h>
+
+#include <linux/debugfs.h>
+
+#define ADIS16460_REG_FLASH_CNT		0x00
+#define ADIS16460_REG_DIAG_STAT		0x02
+#define ADIS16460_REG_X_GYRO_LOW	0x04
+#define ADIS16460_REG_X_GYRO_OUT	0x06
+#define ADIS16460_REG_Y_GYRO_LOW	0x08
+#define ADIS16460_REG_Y_GYRO_OUT	0x0A
+#define ADIS16460_REG_Z_GYRO_LOW	0x0C
+#define ADIS16460_REG_Z_GYRO_OUT	0x0E
+#define ADIS16460_REG_X_ACCL_LOW	0x10
+#define ADIS16460_REG_X_ACCL_OUT	0x12
+#define ADIS16460_REG_Y_ACCL_LOW	0x14
+#define ADIS16460_REG_Y_ACCL_OUT	0x16
+#define ADIS16460_REG_Z_ACCL_LOW	0x18
+#define ADIS16460_REG_Z_ACCL_OUT	0x1A
+#define ADIS16460_REG_SMPL_CNTR		0x1C
+#define ADIS16460_REG_TEMP_OUT		0x1E
+#define ADIS16460_REG_X_DELT_ANG	0x24
+#define ADIS16460_REG_Y_DELT_ANG	0x26
+#define ADIS16460_REG_Z_DELT_ANG	0x28
+#define ADIS16460_REG_X_DELT_VEL	0x2A
+#define ADIS16460_REG_Y_DELT_VEL	0x2C
+#define ADIS16460_REG_Z_DELT_VEL	0x2E
+#define ADIS16460_REG_MSC_CTRL		0x32
+#define ADIS16460_REG_SYNC_SCAL		0x34
+#define ADIS16460_REG_DEC_RATE		0x36
+#define ADIS16460_REG_FLTR_CTRL		0x38
+#define ADIS16460_REG_GLOB_CMD		0x3E
+#define ADIS16460_REG_X_GYRO_OFF	0x40
+#define ADIS16460_REG_Y_GYRO_OFF	0x42
+#define ADIS16460_REG_Z_GYRO_OFF	0x44
+#define ADIS16460_REG_X_ACCL_OFF	0x46
+#define ADIS16460_REG_Y_ACCL_OFF	0x48
+#define ADIS16460_REG_Z_ACCL_OFF	0x4A
+#define ADIS16460_REG_LOT_ID1 R		0x52
+#define ADIS16460_REG_LOT_ID2 R		0x54
+#define ADIS16460_REG_PROD_ID		0x56
+#define ADIS16460_REG_SERIAL_NUM	0x58
+#define ADIS16460_REG_CAL_SGNTR		0x60
+#define ADIS16460_REG_CAL_CRC		0x62
+#define ADIS16460_REG_CODE_SGNTR	0x64
+#define ADIS16460_REG_CODE_CRC		0x66
+
+struct adis16460_chip_info {
+	unsigned int num_channels;
+	const struct iio_chan_spec *channels;
+	unsigned int gyro_max_val;
+	unsigned int gyro_max_scale;
+	unsigned int accel_max_val;
+	unsigned int accel_max_scale;
+};
+
+struct adis16460 {
+	const struct adis16460_chip_info *chip_info;
+	struct adis adis;
+};
+
+#ifdef CONFIG_DEBUG_FS
+
+static int adis16460_show_serial_number(void *arg, u64 *val)
+{
+	struct adis16460 *adis16460 = arg;
+	u16 serial;
+	int ret;
+
+	ret = adis_read_reg_16(&adis16460->adis, ADIS16460_REG_SERIAL_NUM,
+		&serial);
+	if (ret < 0)
+		return ret;
+
+	*val = serial;
+
+	return 0;
+}
+DEFINE_SIMPLE_ATTRIBUTE(adis16460_serial_number_fops,
+	adis16460_show_serial_number, NULL, "0x%.4llx\n");
+
+static int adis16460_show_product_id(void *arg, u64 *val)
+{
+	struct adis16460 *adis16460 = arg;
+	u16 prod_id;
+	int ret;
+
+	ret = adis_read_reg_16(&adis16460->adis, ADIS16460_REG_PROD_ID,
+		&prod_id);
+	if (ret < 0)
+		return ret;
+
+	*val = prod_id;
+
+	return 0;
+}
+DEFINE_SIMPLE_ATTRIBUTE(adis16460_product_id_fops,
+	adis16460_show_product_id, NULL, "%llu\n");
+
+static int adis16460_show_flash_count(void *arg, u64 *val)
+{
+	struct adis16460 *adis16460 = arg;
+	u32 flash_count;
+	int ret;
+
+	ret = adis_read_reg_32(&adis16460->adis, ADIS16460_REG_FLASH_CNT,
+		&flash_count);
+	if (ret < 0)
+		return ret;
+
+	*val = flash_count;
+
+	return 0;
+}
+DEFINE_SIMPLE_ATTRIBUTE(adis16460_flash_count_fops,
+	adis16460_show_flash_count, NULL, "%lld\n");
+
+static int adis16460_debugfs_init(struct iio_dev *indio_dev)
+{
+	struct adis16460 *adis16460 = iio_priv(indio_dev);
+
+	debugfs_create_file("serial_number", 0400, indio_dev->debugfs_dentry,
+		adis16460, &adis16460_serial_number_fops);
+	debugfs_create_file("product_id", 0400, indio_dev->debugfs_dentry,
+		adis16460, &adis16460_product_id_fops);
+	debugfs_create_file("flash_count", 0400, indio_dev->debugfs_dentry,
+		adis16460, &adis16460_flash_count_fops);
+
+	return 0;
+}
+
+#else
+
+static int adis16460_debugfs_init(struct iio_dev *indio_dev)
+{
+	return 0;
+}
+
+#endif
+
+static int adis16460_set_freq(struct iio_dev *indio_dev, int val, int val2)
+{
+	struct adis16460 *st = iio_priv(indio_dev);
+	unsigned int t;
+
+	t =  val * 1000 + val2 / 1000;
+	if (t <= 0)
+		return -EINVAL;
+
+	t = 2048000 / t;
+	if (t > 2048)
+		t = 2048;
+
+	if (t != 0)
+		t--;
+
+	return adis_write_reg_16(&st->adis, ADIS16460_REG_DEC_RATE, t);
+}
+
+static int adis16460_get_freq(struct iio_dev *indio_dev, int *val, int *val2)
+{
+	struct adis16460 *st = iio_priv(indio_dev);
+	uint16_t t;
+	int ret;
+	unsigned freq;
+
+	ret = adis_read_reg_16(&st->adis, ADIS16460_REG_DEC_RATE, &t);
+	if (ret < 0)
+		return ret;
+
+	freq = 2048000 / (t + 1);
+	*val = freq / 1000;
+	*val2 = (freq % 1000) * 1000;
+
+	return IIO_VAL_INT_PLUS_MICRO;
+}
+
+static int adis16460_read_raw(struct iio_dev *indio_dev,
+	const struct iio_chan_spec *chan, int *val, int *val2, long info)
+{
+	struct adis16460 *st = iio_priv(indio_dev);
+
+	switch (info) {
+	case IIO_CHAN_INFO_RAW:
+		return adis_single_conversion(indio_dev, chan, 0, val);
+	case IIO_CHAN_INFO_SCALE:
+		switch (chan->type) {
+		case IIO_ANGL_VEL:
+			*val = st->chip_info->gyro_max_scale;
+			*val2 = st->chip_info->gyro_max_val;
+			return IIO_VAL_FRACTIONAL;
+		case IIO_ACCEL:
+			*val = st->chip_info->accel_max_scale;
+			*val2 = st->chip_info->accel_max_val;
+			return IIO_VAL_FRACTIONAL;
+		case IIO_TEMP:
+			*val = 50; /* 50 milli degrees Celsius/LSB */
+			return IIO_VAL_INT;
+		default:
+			return -EINVAL;
+		}
+	case IIO_CHAN_INFO_OFFSET:
+		*val = 500; /* 25 degrees Celsius = 0x0000 */
+		return IIO_VAL_INT;
+	case IIO_CHAN_INFO_SAMP_FREQ:
+		return adis16460_get_freq(indio_dev, val, val2);
+	default:
+		return -EINVAL;
+	}
+}
+
+static int adis16460_write_raw(struct iio_dev *indio_dev,
+	const struct iio_chan_spec *chan, int val, int val2, long info)
+{
+	switch (info) {
+	case IIO_CHAN_INFO_SAMP_FREQ:
+		return adis16460_set_freq(indio_dev, val, val2);
+	default:
+		return -EINVAL;
+	}
+}
+
+enum {
+	ADIS16460_SCAN_GYRO_X,
+	ADIS16460_SCAN_GYRO_Y,
+	ADIS16460_SCAN_GYRO_Z,
+	ADIS16460_SCAN_ACCEL_X,
+	ADIS16460_SCAN_ACCEL_Y,
+	ADIS16460_SCAN_ACCEL_Z,
+	ADIS16460_SCAN_TEMP,
+};
+
+#define ADIS16460_MOD_CHANNEL(_type, _mod, _address, _si, _bits) \
+	{ \
+		.type = (_type), \
+		.modified = 1, \
+		.channel2 = (_mod), \
+		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW), \
+		.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE), \
+		.info_mask_shared_by_all = BIT(IIO_CHAN_INFO_SAMP_FREQ), \
+		.address = (_address), \
+		.scan_index = (_si), \
+		.scan_type = { \
+			.sign = 's', \
+			.realbits = (_bits), \
+			.storagebits = (_bits), \
+			.endianness = IIO_BE, \
+		}, \
+	}
+
+#define ADIS16460_GYRO_CHANNEL(_mod) \
+	ADIS16460_MOD_CHANNEL(IIO_ANGL_VEL, IIO_MOD_ ## _mod, \
+	ADIS16460_REG_ ## _mod ## _GYRO_LOW, ADIS16460_SCAN_GYRO_ ## _mod, \
+	32)
+
+#define ADIS16460_ACCEL_CHANNEL(_mod) \
+	ADIS16460_MOD_CHANNEL(IIO_ACCEL, IIO_MOD_ ## _mod, \
+	ADIS16460_REG_ ## _mod ## _ACCL_LOW, ADIS16460_SCAN_ACCEL_ ## _mod, \
+	32)
+
+#define ADIS16460_TEMP_CHANNEL() { \
+		.type = IIO_TEMP, \
+		.indexed = 1, \
+		.channel = 0, \
+		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) | \
+			BIT(IIO_CHAN_INFO_SCALE) | \
+			BIT(IIO_CHAN_INFO_OFFSET), \
+		.info_mask_shared_by_all = BIT(IIO_CHAN_INFO_SAMP_FREQ), \
+		.address = ADIS16460_REG_TEMP_OUT, \
+		.scan_index = ADIS16460_SCAN_TEMP, \
+		.scan_type = { \
+			.sign = 's', \
+			.realbits = 16, \
+			.storagebits = 16, \
+			.endianness = IIO_BE, \
+		}, \
+	}
+
+static const struct iio_chan_spec adis16460_channels[] = {
+	ADIS16460_GYRO_CHANNEL(X),
+	ADIS16460_GYRO_CHANNEL(Y),
+	ADIS16460_GYRO_CHANNEL(Z),
+	ADIS16460_ACCEL_CHANNEL(X),
+	ADIS16460_ACCEL_CHANNEL(Y),
+	ADIS16460_ACCEL_CHANNEL(Z),
+	ADIS16460_TEMP_CHANNEL(),
+	IIO_CHAN_SOFT_TIMESTAMP(7)
+};
+
+static const struct adis16460_chip_info adis16460_chip_info = {
+	.channels = adis16460_channels,
+	.num_channels = ARRAY_SIZE(adis16460_channels),
+	/*
+	 * storing the value in rad/degree and the scale in degree
+	 * gives us the result in rad and better precession than
+	 * storing the scale directly in rad.
+	 */
+	.gyro_max_val = IIO_RAD_TO_DEGREE(200 << 16),
+	.gyro_max_scale = 1,
+	.accel_max_val = IIO_M_S_2_TO_G(20000 << 16),
+	.accel_max_scale = 5,
+};
+
+static const struct iio_info adis16460_info = {
+	.read_raw = &adis16460_read_raw,
+	.write_raw = &adis16460_write_raw,
+	.update_scan_mode = adis_update_scan_mode,
+	.debugfs_reg_access = adis_debugfs_reg_access,
+};
+
+static int adis16460_enable_irq(struct adis *adis, bool enable)
+{
+	/*
+	 * There is no way to gate the data-ready signal internally inside the
+	 * ADIS16460 :(
+	 */
+	if (enable)
+		enable_irq(adis->spi->irq);
+	else
+		disable_irq(adis->spi->irq);
+
+	return 0;
+}
+
+static int adis16460_initial_setup(struct iio_dev *indio_dev)
+{
+	struct adis16460 *st = iio_priv(indio_dev);
+	uint16_t prod_id;
+	unsigned int device_id;
+	int ret;
+
+	adis_reset(&st->adis);
+	msleep(222);
+
+	ret = adis_write_reg_16(&st->adis, ADIS16460_REG_GLOB_CMD, BIT(1));
+	if (ret)
+		return ret;
+	msleep(75);
+
+	ret = adis_check_status(&st->adis);
+	if (ret)
+		return ret;
+
+	ret = adis_read_reg_16(&st->adis, ADIS16460_REG_PROD_ID, &prod_id);
+	if (ret)
+		return ret;
+
+	ret = sscanf(indio_dev->name, "adis%u\n", &device_id);
+	if (ret != 1)
+		return -EINVAL;
+
+	if (prod_id != device_id)
+		dev_warn(&indio_dev->dev, "Device ID(%u) and product ID(%u) do not match.",
+				device_id, prod_id);
+
+	return 0;
+}
+
+#define ADIS16460_DIAG_STAT_IN_CLK_OOS	7
+#define ADIS16460_DIAG_STAT_FLASH_MEM	6
+#define ADIS16460_DIAG_STAT_SELF_TEST	5
+#define ADIS16460_DIAG_STAT_OVERRANGE	4
+#define ADIS16460_DIAG_STAT_SPI_COMM	3
+#define ADIS16460_DIAG_STAT_FLASH_UPT	2
+
+static const char * const adis16460_status_error_msgs[] = {
+	[ADIS16460_DIAG_STAT_IN_CLK_OOS] = "Input clock out of sync",
+	[ADIS16460_DIAG_STAT_FLASH_MEM] = "Flash memory failure",
+	[ADIS16460_DIAG_STAT_SELF_TEST] = "Self test diagnostic failure",
+	[ADIS16460_DIAG_STAT_OVERRANGE] = "Sensor overrange",
+	[ADIS16460_DIAG_STAT_SPI_COMM] = "SPI communication failure",
+	[ADIS16460_DIAG_STAT_FLASH_UPT] = "Flash update failure",
+};
+
+static const struct adis_data adis16460_data = {
+	.diag_stat_reg = ADIS16460_REG_DIAG_STAT,
+	.glob_cmd_reg = ADIS16460_REG_GLOB_CMD,
+	.has_paging = false,
+	.read_delay = 5,
+	.write_delay = 5,
+	.cs_stall_delay = 16,
+	.status_error_msgs = adis16460_status_error_msgs,
+	.status_error_mask = BIT(ADIS16460_DIAG_STAT_IN_CLK_OOS) |
+		BIT(ADIS16460_DIAG_STAT_FLASH_MEM) |
+		BIT(ADIS16460_DIAG_STAT_SELF_TEST) |
+		BIT(ADIS16460_DIAG_STAT_OVERRANGE) |
+		BIT(ADIS16460_DIAG_STAT_SPI_COMM) |
+		BIT(ADIS16460_DIAG_STAT_FLASH_UPT),
+	.enable_irq = adis16460_enable_irq,
+};
+
+static int adis16460_probe(struct spi_device *spi)
+{
+	struct iio_dev *indio_dev;
+	struct adis16460 *st;
+	int ret;
+
+	indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*st));
+	if (indio_dev == NULL)
+		return -ENOMEM;
+
+	spi_set_drvdata(spi, indio_dev);
+
+	st = iio_priv(indio_dev);
+
+	st->chip_info = &adis16460_chip_info;
+	indio_dev->dev.parent = &spi->dev;
+	indio_dev->name = spi_get_device_id(spi)->name;
+	indio_dev->channels = st->chip_info->channels;
+	indio_dev->num_channels = st->chip_info->num_channels;
+	indio_dev->info = &adis16460_info;
+	indio_dev->modes = INDIO_DIRECT_MODE;
+
+	ret = adis_init(&st->adis, indio_dev, spi, &adis16460_data);
+	if (ret)
+		return ret;
+
+	ret = adis_setup_buffer_and_trigger(&st->adis, indio_dev, NULL);
+	if (ret)
+		return ret;
+
+	adis16460_enable_irq(&st->adis, 0);
+
+	ret = adis16460_initial_setup(indio_dev);
+	if (ret)
+		goto error_cleanup_buffer;
+
+	ret = iio_device_register(indio_dev);
+	if (ret)
+		goto error_cleanup_buffer;
+
+	adis16460_debugfs_init(indio_dev);
+
+	return 0;
+
+error_cleanup_buffer:
+	adis_cleanup_buffer_and_trigger(&st->adis, indio_dev);
+	return ret;
+}
+
+static int adis16460_remove(struct spi_device *spi)
+{
+	struct iio_dev *indio_dev = spi_get_drvdata(spi);
+	struct adis16460 *st = iio_priv(indio_dev);
+
+	iio_device_unregister(indio_dev);
+
+	adis_cleanup_buffer_and_trigger(&st->adis, indio_dev);
+
+	return 0;
+}
+
+static const struct spi_device_id adis16460_ids[] = {
+	{ "adis16460", 0 },
+	{}
+};
+MODULE_DEVICE_TABLE(spi, adis16460_id);
+
+static const struct of_device_id adis16460_of_match[] = {
+	{ .compatible = "adi,adis16460" },
+	{}
+};
+MODULE_DEVICE_TABLE(of, adis16460_of_match);
+
+static struct spi_driver adis16460_driver = {
+	.driver = {
+		.name = "adis16460",
+		.of_match_table = adis16460_of_match,
+	},
+	.id_table = adis16460_ids,
+	.probe = adis16460_probe,
+	.remove = adis16460_remove,
+};
+module_spi_driver(adis16460_driver);
+
+MODULE_AUTHOR("Dragos Bogdan <dragos.bogdan@analog.com>");
+MODULE_DESCRIPTION("Analog Devices ADIS16460 IMU driver");
+MODULE_LICENSE("GPL");
-- 
2.20.1

Re: [PATCH 1/5] MAINTAINERS: add ADIS IMU driver library entry

[Jonathan Cameron]

On Tue, 25 Jun 2019 16:13:24 +0300
Alexandru Ardelean <alexandru.ardelean@analog.com> wrote:

> This change adds the ADIS driver library to the MAINTAINERS list, and adds
> myself as the current maintainer of this library.
> 
> Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
Hi Alexandru,

Given this only has a tangential relationship to the rest of the patches
I'll pick up now.

More generally this series is complex enough a cover letter would have
been good + 5/5 isn't anywhere that I can find?


Thanks,

Jonathan

> ---
>  MAINTAINERS | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 1eb971608ac4..544e23753e96 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -930,6 +930,13 @@ S:	Supported
>  F:	drivers/mux/adgs1408.c
>  F:	Documentation/devicetree/bindings/mux/adi,adgs1408.txt
>  
> +ANALOG DEVICES INC ADIS DRIVER LIBRARY
> +M:	Alexandru Ardelean <alexandru.ardelean@analog.com>
> +S:	Supported
> +L:	linux-iio@vger.kernel.org
> +F:	include/linux/iio/imu/adis.h
> +F:	drivers/iio/imu/adis.c
> +
>  ANALOG DEVICES INC ADP5061 DRIVER
>  M:	Stefan Popa <stefan.popa@analog.com>
>  L:	linux-pm@vger.kernel.org

Re: [PATCH 2/5] drivers: spi: core: Add optional stall delay between cs_change transfers

[Jonathan Cameron]

On Tue, 25 Jun 2019 16:13:25 +0300
Alexandru Ardelean <alexandru.ardelean@analog.com> wrote:

> Some devices like the ADIS16460 IMU require a stall period between
> transfers, i.e. between when the CS is de-asserted and re-asserted. The
> default value of 10us is not enough. This change makes the delay
> configurable for when the next CS change goes active.
> 
> Signed-off-by: Michael Hennerich <michael.hennerich@analog.com>
> Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>

General principle seems fine to me, though naming may need some work.

cs_low_time or something more specific than stall perhaps?

+CC Mark.

> ---
>  drivers/spi/spi.c       | 3 ++-
>  include/linux/spi/spi.h | 3 +++
>  2 files changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
> index 5e75944ad5d1..739de0118ee1 100644
> --- a/drivers/spi/spi.c
> +++ b/drivers/spi/spi.c
> @@ -1163,7 +1163,8 @@ static int spi_transfer_one_message(struct spi_controller *ctlr,
>  				keep_cs = true;
>  			} else {
>  				spi_set_cs(msg->spi, false);
> -				udelay(10);
> +				udelay(xfer->cs_change_stall_delay_us ?
> +				       xfer->cs_change_stall_delay_us : 10);
>  				spi_set_cs(msg->spi, true);
>  			}
>  		}
> diff --git a/include/linux/spi/spi.h b/include/linux/spi/spi.h
> index 053abd22ad31..d23add3b4790 100644
> --- a/include/linux/spi/spi.h
> +++ b/include/linux/spi/spi.h
> @@ -734,6 +734,8 @@ extern void spi_res_release(struct spi_controller *ctlr,
>   *      transfer. If 0 the default (from @spi_device) is used.
>   * @bits_per_word: select a bits_per_word other than the device default
>   *      for this transfer. If 0 the default (from @spi_device) is used.
> + * @cs_change_stall_delay_us: microseconds to delay between cs_change
> + * 	transfers.
>   * @cs_change: affects chipselect after this transfer completes
>   * @delay_usecs: microseconds to delay after this transfer before
>   *	(optionally) changing the chipselect status, then starting
> @@ -823,6 +825,7 @@ struct spi_transfer {
>  #define	SPI_NBITS_QUAD		0x04 /* 4bits transfer */
>  	u8		bits_per_word;
>  	u8		word_delay_usecs;
> +	u8		cs_change_stall_delay_us;
>  	u16		delay_usecs;
>  	u32		speed_hz;
>  	u16		word_delay;

Re: [PATCH 3/5] iio: imu: adis: Add support for SPI transfer cs_change_stall_delay_us

[Jonathan Cameron]

On Tue, 25 Jun 2019 16:13:26 +0300
Alexandru Ardelean <alexandru.ardelean@analog.com> wrote:

> The ADIS16460 requires a higher delay before the next transfer. Since the
> SPI framework supports configuring the delay before the next transfer, this
> driver will become the first user of it.
> 
> The support for this functionality in ADIS16460 requires an addition to the
> ADIS lib to support the `cs_change_stall_delay_us` functionality in SPI.
> 
> Signed-off-by: Michael Hennerich <michael.hennerich@analog.com>
> Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
Subject to previous patch naming etc, this is fine and I'll pick it up once
that's sorted.

Thanks,

Jonathan

> ---
>  drivers/iio/imu/adis.c       | 9 +++++++++
>  include/linux/iio/imu/adis.h | 2 ++
>  2 files changed, 11 insertions(+)
> 
> diff --git a/drivers/iio/imu/adis.c b/drivers/iio/imu/adis.c
> index c771ae6803a9..90dac69910b3 100644
> --- a/drivers/iio/imu/adis.c
> +++ b/drivers/iio/imu/adis.c
> @@ -40,28 +40,33 @@ int adis_write_reg(struct adis *adis, unsigned int reg,
>  			.len = 2,
>  			.cs_change = 1,
>  			.delay_usecs = adis->data->write_delay,
> +			.cs_change_stall_delay_us = adis->data->cs_stall_delay,
>  		}, {
>  			.tx_buf = adis->tx + 2,
>  			.bits_per_word = 8,
>  			.len = 2,
>  			.cs_change = 1,
>  			.delay_usecs = adis->data->write_delay,
> +			.cs_change_stall_delay_us = adis->data->cs_stall_delay,
>  		}, {
>  			.tx_buf = adis->tx + 4,
>  			.bits_per_word = 8,
>  			.len = 2,
>  			.cs_change = 1,
>  			.delay_usecs = adis->data->write_delay,
> +			.cs_change_stall_delay_us = adis->data->cs_stall_delay,
>  		}, {
>  			.tx_buf = adis->tx + 6,
>  			.bits_per_word = 8,
>  			.len = 2,
>  			.delay_usecs = adis->data->write_delay,
> +			.cs_change_stall_delay_us = adis->data->cs_stall_delay,
>  		}, {
>  			.tx_buf = adis->tx + 8,
>  			.bits_per_word = 8,
>  			.len = 2,
>  			.delay_usecs = adis->data->write_delay,
> +			.cs_change_stall_delay_us = adis->data->cs_stall_delay,
>  		},
>  	};
>  
> @@ -134,12 +139,14 @@ int adis_read_reg(struct adis *adis, unsigned int reg,
>  			.len = 2,
>  			.cs_change = 1,
>  			.delay_usecs = adis->data->write_delay,
> +			.cs_change_stall_delay_us = adis->data->cs_stall_delay,
>  		}, {
>  			.tx_buf = adis->tx + 2,
>  			.bits_per_word = 8,
>  			.len = 2,
>  			.cs_change = 1,
>  			.delay_usecs = adis->data->read_delay,
> +			.cs_change_stall_delay_us = adis->data->cs_stall_delay,
>  		}, {
>  			.tx_buf = adis->tx + 4,
>  			.rx_buf = adis->rx,
> @@ -147,11 +154,13 @@ int adis_read_reg(struct adis *adis, unsigned int reg,
>  			.len = 2,
>  			.cs_change = 1,
>  			.delay_usecs = adis->data->read_delay,
> +			.cs_change_stall_delay_us = adis->data->cs_stall_delay,
>  		}, {
>  			.rx_buf = adis->rx + 2,
>  			.bits_per_word = 8,
>  			.len = 2,
>  			.delay_usecs = adis->data->read_delay,
> +			.cs_change_stall_delay_us = adis->data->cs_stall_delay,
>  		},
>  	};
>  
> diff --git a/include/linux/iio/imu/adis.h b/include/linux/iio/imu/adis.h
> index 469a493f7ae0..4aa248b6b3bd 100644
> --- a/include/linux/iio/imu/adis.h
> +++ b/include/linux/iio/imu/adis.h
> @@ -27,6 +27,7 @@ struct adis_burst;
>   * struct adis_data - ADIS chip variant specific data
>   * @read_delay: SPI delay for read operations in us
>   * @write_delay: SPI delay for write operations in us
> + * @cs_stall_delay: SPI stall delay between transfers in us
>   * @glob_cmd_reg: Register address of the GLOB_CMD register
>   * @msc_ctrl_reg: Register address of the MSC_CTRL register
>   * @diag_stat_reg: Register address of the DIAG_STAT register
> @@ -36,6 +37,7 @@ struct adis_burst;
>  struct adis_data {
>  	unsigned int read_delay;
>  	unsigned int write_delay;
> +	unsigned int cs_stall_delay;
>  
>  	unsigned int glob_cmd_reg;
>  	unsigned int msc_ctrl_reg;

Re: [PATCH 4/5] iio: imu: Add support for the ADIS16460 IMU

[Jonathan Cameron]

On Tue, 25 Jun 2019 16:13:27 +0300
Alexandru Ardelean <alexandru.ardelean@analog.com> wrote:

> The ADIS16460 device is a complete inertial system that includes a triaxial
> gyroscope and a triaxial accelerometer. It's more simplified design than
> that of the ADIS16480, and does not offer the triaxial magnetometers &
> pressure sensors. It does also have a temperature sensor (like the
> ADIS16480).
> Since it is part of the ADIS16XXX family, it re-uses parts of the ADIS
> library.
> 
> Naturally, the register map is different and much more simplified than the
> ADIS16480 subfamily, so it cannot be integrated into that driver. A major
> difference is that the registers are not paged.
> 
> One thing that is particularly special about it, is that it requires a
> higher CS stall delay between data (around 16 uS vs other chips from the
> family requiring around 2 uS).
> 
> Datasheet:
>   https://www.analog.com/media/en/technical-documentation/data-sheets/adis16460.pdf
> 
> Signed-off-by: Dragos Bogdan <dragos.bogdan@analog.com>
> Signed-off-by: Michael Hennerich <michael.hennerich@analog.com>
> Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>

A few comments inline, but only one that matters. Licenses disagree
(you would almost have thought I'd just fixed a load of these and
have it fresh in my mind!)

Thanks,

Jonathan

> ---
>  MAINTAINERS                 |   7 +
>  drivers/iio/imu/Kconfig     |   9 +
>  drivers/iio/imu/Makefile    |   1 +
>  drivers/iio/imu/adis16460.c | 490 ++++++++++++++++++++++++++++++++++++
>  4 files changed, 507 insertions(+)
>  create mode 100644 drivers/iio/imu/adis16460.c
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 544e23753e96..ef2e2cee32e1 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -937,6 +937,13 @@ L:	linux-iio@vger.kernel.org
>  F:	include/linux/iio/imu/adis.h
>  F:	drivers/iio/imu/adis.c
>  
> +ANALOG DEVICES INC ADIS16460 DRIVER
> +M:	Dragos Bogdan <dragos.bogdan@analog.com>
> +S:	Supported
> +L:	linux-iio@vger.kernel.org
> +W:	http://ez.analog.com/community/linux-device-drivers
> +F:	drivers/iio/imu/adis16460.c
> +
>  ANALOG DEVICES INC ADP5061 DRIVER
>  M:	Stefan Popa <stefan.popa@analog.com>
>  L:	linux-pm@vger.kernel.org
> diff --git a/drivers/iio/imu/Kconfig b/drivers/iio/imu/Kconfig
> index 156630a21696..a29a481c20d2 100644
> --- a/drivers/iio/imu/Kconfig
> +++ b/drivers/iio/imu/Kconfig
> @@ -16,6 +16,15 @@ config ADIS16400
>  	  adis16365, adis16400 and adis16405 triaxial inertial sensors
>  	  (adis16400 series also have magnetometers).
>  
> +config ADIS16460
> +	tristate "Analog Devices ADIS16460 and similar IMU driver"
> +	depends on SPI
> +	select IIO_ADIS_LIB
> +	select IIO_ADIS_LIB_BUFFER if IIO_BUFFER
> +	help
> +	  Say yes here to build support for Analog Devices ADIS16460 inertial
> +	  sensor.
I have a nasty feeling this is short enough that one of the static
analysis tools will moan and we'll get a patch adding the name of the module
or something like that, mostly to shut up the warning.

> +
>  config ADIS16480
>  	tristate "Analog Devices ADIS16480 and similar IMU driver"
>  	depends on SPI
> diff --git a/drivers/iio/imu/Makefile b/drivers/iio/imu/Makefile
> index 9e452fce1aaf..4a6958865504 100644
> --- a/drivers/iio/imu/Makefile
> +++ b/drivers/iio/imu/Makefile
> @@ -5,6 +5,7 @@
>  
>  # When adding new entries keep the list in alphabetical order
>  obj-$(CONFIG_ADIS16400) += adis16400.o
> +obj-$(CONFIG_ADIS16460) += adis16460.o
>  obj-$(CONFIG_ADIS16480) += adis16480.o
>  
>  adis_lib-y += adis.o
> diff --git a/drivers/iio/imu/adis16460.c b/drivers/iio/imu/adis16460.c
> new file mode 100644
> index 000000000000..6c86af06b5d1
> --- /dev/null
> +++ b/drivers/iio/imu/adis16460.c
> @@ -0,0 +1,490 @@
> +// SPDX-License-Identifier: GPL-2.0

MODULE_LICENSE(GPL) means GPL-2.0+ IIRC so fix one or the other.

> +/*
> + * ADIS16460 IMU driver
> + *
> + * Copyright 2019 Analog Devices Inc.
> + *
Nitpick, This blank line doesn't add anything.

> + */
> +
> +#include <linux/delay.h>
> +#include <linux/module.h>
> +#include <linux/spi/spi.h>
> +
> +#include <linux/iio/iio.h>
> +#include <linux/iio/imu/adis.h>
> +
> +#include <linux/debugfs.h>
> +
> +#define ADIS16460_REG_FLASH_CNT		0x00
> +#define ADIS16460_REG_DIAG_STAT		0x02
> +#define ADIS16460_REG_X_GYRO_LOW	0x04
> +#define ADIS16460_REG_X_GYRO_OUT	0x06
> +#define ADIS16460_REG_Y_GYRO_LOW	0x08
> +#define ADIS16460_REG_Y_GYRO_OUT	0x0A
> +#define ADIS16460_REG_Z_GYRO_LOW	0x0C
> +#define ADIS16460_REG_Z_GYRO_OUT	0x0E
> +#define ADIS16460_REG_X_ACCL_LOW	0x10
> +#define ADIS16460_REG_X_ACCL_OUT	0x12
> +#define ADIS16460_REG_Y_ACCL_LOW	0x14
> +#define ADIS16460_REG_Y_ACCL_OUT	0x16
> +#define ADIS16460_REG_Z_ACCL_LOW	0x18
> +#define ADIS16460_REG_Z_ACCL_OUT	0x1A
> +#define ADIS16460_REG_SMPL_CNTR		0x1C
> +#define ADIS16460_REG_TEMP_OUT		0x1E
> +#define ADIS16460_REG_X_DELT_ANG	0x24
> +#define ADIS16460_REG_Y_DELT_ANG	0x26
> +#define ADIS16460_REG_Z_DELT_ANG	0x28
> +#define ADIS16460_REG_X_DELT_VEL	0x2A
> +#define ADIS16460_REG_Y_DELT_VEL	0x2C
> +#define ADIS16460_REG_Z_DELT_VEL	0x2E
> +#define ADIS16460_REG_MSC_CTRL		0x32
> +#define ADIS16460_REG_SYNC_SCAL		0x34
> +#define ADIS16460_REG_DEC_RATE		0x36
> +#define ADIS16460_REG_FLTR_CTRL		0x38
> +#define ADIS16460_REG_GLOB_CMD		0x3E
> +#define ADIS16460_REG_X_GYRO_OFF	0x40
> +#define ADIS16460_REG_Y_GYRO_OFF	0x42
> +#define ADIS16460_REG_Z_GYRO_OFF	0x44
> +#define ADIS16460_REG_X_ACCL_OFF	0x46
> +#define ADIS16460_REG_Y_ACCL_OFF	0x48
> +#define ADIS16460_REG_Z_ACCL_OFF	0x4A
> +#define ADIS16460_REG_LOT_ID1 R		0x52
> +#define ADIS16460_REG_LOT_ID2 R		0x54
> +#define ADIS16460_REG_PROD_ID		0x56
> +#define ADIS16460_REG_SERIAL_NUM	0x58
> +#define ADIS16460_REG_CAL_SGNTR		0x60
> +#define ADIS16460_REG_CAL_CRC		0x62
> +#define ADIS16460_REG_CODE_SGNTR	0x64
> +#define ADIS16460_REG_CODE_CRC		0x66
> +
> +struct adis16460_chip_info {

Future proofing, or another part going to be added shortly?
Given the history of these drivers and the large number of parts
that have shown up, I'll give this one the benefit of the doubt!

> +	unsigned int num_channels;
> +	const struct iio_chan_spec *channels;
> +	unsigned int gyro_max_val;
> +	unsigned int gyro_max_scale;
> +	unsigned int accel_max_val;
> +	unsigned int accel_max_scale;
> +};
> +
> +struct adis16460 {
> +	const struct adis16460_chip_info *chip_info;
> +	struct adis adis;
> +};
> +
> +#ifdef CONFIG_DEBUG_FS
> +
> +static int adis16460_show_serial_number(void *arg, u64 *val)
> +{
> +	struct adis16460 *adis16460 = arg;
> +	u16 serial;
> +	int ret;
> +
> +	ret = adis_read_reg_16(&adis16460->adis, ADIS16460_REG_SERIAL_NUM,
> +		&serial);
> +	if (ret < 0)
> +		return ret;
> +
> +	*val = serial;
> +
> +	return 0;
> +}
> +DEFINE_SIMPLE_ATTRIBUTE(adis16460_serial_number_fops,
> +	adis16460_show_serial_number, NULL, "0x%.4llx\n");
> +
> +static int adis16460_show_product_id(void *arg, u64 *val)
> +{
> +	struct adis16460 *adis16460 = arg;
> +	u16 prod_id;
> +	int ret;
> +
> +	ret = adis_read_reg_16(&adis16460->adis, ADIS16460_REG_PROD_ID,
> +		&prod_id);
> +	if (ret < 0)
> +		return ret;
> +
> +	*val = prod_id;
> +
> +	return 0;
> +}
> +DEFINE_SIMPLE_ATTRIBUTE(adis16460_product_id_fops,
> +	adis16460_show_product_id, NULL, "%llu\n");
> +
> +static int adis16460_show_flash_count(void *arg, u64 *val)
> +{
> +	struct adis16460 *adis16460 = arg;
> +	u32 flash_count;
> +	int ret;
> +
> +	ret = adis_read_reg_32(&adis16460->adis, ADIS16460_REG_FLASH_CNT,
> +		&flash_count);
> +	if (ret < 0)
> +		return ret;
> +
> +	*val = flash_count;
> +
> +	return 0;
> +}
> +DEFINE_SIMPLE_ATTRIBUTE(adis16460_flash_count_fops,
> +	adis16460_show_flash_count, NULL, "%lld\n");
> +
> +static int adis16460_debugfs_init(struct iio_dev *indio_dev)
> +{
> +	struct adis16460 *adis16460 = iio_priv(indio_dev);
> +
> +	debugfs_create_file("serial_number", 0400, indio_dev->debugfs_dentry,
> +		adis16460, &adis16460_serial_number_fops);
> +	debugfs_create_file("product_id", 0400, indio_dev->debugfs_dentry,
> +		adis16460, &adis16460_product_id_fops);
> +	debugfs_create_file("flash_count", 0400, indio_dev->debugfs_dentry,
> +		adis16460, &adis16460_flash_count_fops);
> +
> +	return 0;
> +}
> +
> +#else
> +
> +static int adis16460_debugfs_init(struct iio_dev *indio_dev)
> +{
> +	return 0;
> +}
> +
> +#endif
> +
> +static int adis16460_set_freq(struct iio_dev *indio_dev, int val, int val2)
> +{
> +	struct adis16460 *st = iio_priv(indio_dev);
> +	unsigned int t;
> +
> +	t =  val * 1000 + val2 / 1000;
> +	if (t <= 0)
> +		return -EINVAL;
> +
> +	t = 2048000 / t;
> +	if (t > 2048)
> +		t = 2048;
> +
> +	if (t != 0)
> +		t--;
> +
> +	return adis_write_reg_16(&st->adis, ADIS16460_REG_DEC_RATE, t);
> +}
> +
> +static int adis16460_get_freq(struct iio_dev *indio_dev, int *val, int *val2)
> +{
> +	struct adis16460 *st = iio_priv(indio_dev);
> +	uint16_t t;
> +	int ret;
> +	unsigned freq;
> +
> +	ret = adis_read_reg_16(&st->adis, ADIS16460_REG_DEC_RATE, &t);
> +	if (ret < 0)
> +		return ret;
> +
> +	freq = 2048000 / (t + 1);
> +	*val = freq / 1000;
> +	*val2 = (freq % 1000) * 1000;
> +
> +	return IIO_VAL_INT_PLUS_MICRO;
> +}
> +
> +static int adis16460_read_raw(struct iio_dev *indio_dev,
> +	const struct iio_chan_spec *chan, int *val, int *val2, long info)
> +{
> +	struct adis16460 *st = iio_priv(indio_dev);
> +
> +	switch (info) {
> +	case IIO_CHAN_INFO_RAW:
> +		return adis_single_conversion(indio_dev, chan, 0, val);
> +	case IIO_CHAN_INFO_SCALE:
> +		switch (chan->type) {
> +		case IIO_ANGL_VEL:
> +			*val = st->chip_info->gyro_max_scale;
> +			*val2 = st->chip_info->gyro_max_val;
> +			return IIO_VAL_FRACTIONAL;
> +		case IIO_ACCEL:
> +			*val = st->chip_info->accel_max_scale;
> +			*val2 = st->chip_info->accel_max_val;
> +			return IIO_VAL_FRACTIONAL;
> +		case IIO_TEMP:
> +			*val = 50; /* 50 milli degrees Celsius/LSB */
> +			return IIO_VAL_INT;
> +		default:
> +			return -EINVAL;
> +		}
> +	case IIO_CHAN_INFO_OFFSET:
> +		*val = 500; /* 25 degrees Celsius = 0x0000 */
> +		return IIO_VAL_INT;
> +	case IIO_CHAN_INFO_SAMP_FREQ:
> +		return adis16460_get_freq(indio_dev, val, val2);
> +	default:
> +		return -EINVAL;
> +	}
> +}
> +
> +static int adis16460_write_raw(struct iio_dev *indio_dev,
> +	const struct iio_chan_spec *chan, int val, int val2, long info)
> +{
> +	switch (info) {
> +	case IIO_CHAN_INFO_SAMP_FREQ:
> +		return adis16460_set_freq(indio_dev, val, val2);
> +	default:
> +		return -EINVAL;
> +	}
> +}
> +
> +enum {
> +	ADIS16460_SCAN_GYRO_X,
> +	ADIS16460_SCAN_GYRO_Y,
> +	ADIS16460_SCAN_GYRO_Z,
> +	ADIS16460_SCAN_ACCEL_X,
> +	ADIS16460_SCAN_ACCEL_Y,
> +	ADIS16460_SCAN_ACCEL_Z,
> +	ADIS16460_SCAN_TEMP,
> +};
> +
> +#define ADIS16460_MOD_CHANNEL(_type, _mod, _address, _si, _bits) \
> +	{ \
> +		.type = (_type), \
> +		.modified = 1, \
> +		.channel2 = (_mod), \
> +		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW), \
> +		.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE), \
> +		.info_mask_shared_by_all = BIT(IIO_CHAN_INFO_SAMP_FREQ), \
> +		.address = (_address), \
> +		.scan_index = (_si), \
> +		.scan_type = { \
> +			.sign = 's', \
> +			.realbits = (_bits), \
> +			.storagebits = (_bits), \
> +			.endianness = IIO_BE, \
> +		}, \
> +	}
> +
> +#define ADIS16460_GYRO_CHANNEL(_mod) \
> +	ADIS16460_MOD_CHANNEL(IIO_ANGL_VEL, IIO_MOD_ ## _mod, \
> +	ADIS16460_REG_ ## _mod ## _GYRO_LOW, ADIS16460_SCAN_GYRO_ ## _mod, \
> +	32)
> +
> +#define ADIS16460_ACCEL_CHANNEL(_mod) \
> +	ADIS16460_MOD_CHANNEL(IIO_ACCEL, IIO_MOD_ ## _mod, \
> +	ADIS16460_REG_ ## _mod ## _ACCL_LOW, ADIS16460_SCAN_ACCEL_ ## _mod, \
> +	32)
> +
> +#define ADIS16460_TEMP_CHANNEL() { \
> +		.type = IIO_TEMP, \
> +		.indexed = 1, \
> +		.channel = 0, \
> +		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) | \
> +			BIT(IIO_CHAN_INFO_SCALE) | \
> +			BIT(IIO_CHAN_INFO_OFFSET), \
> +		.info_mask_shared_by_all = BIT(IIO_CHAN_INFO_SAMP_FREQ), \
> +		.address = ADIS16460_REG_TEMP_OUT, \
> +		.scan_index = ADIS16460_SCAN_TEMP, \
> +		.scan_type = { \
> +			.sign = 's', \
> +			.realbits = 16, \
> +			.storagebits = 16, \
> +			.endianness = IIO_BE, \
> +		}, \
> +	}
> +
> +static const struct iio_chan_spec adis16460_channels[] = {
> +	ADIS16460_GYRO_CHANNEL(X),
> +	ADIS16460_GYRO_CHANNEL(Y),
> +	ADIS16460_GYRO_CHANNEL(Z),
> +	ADIS16460_ACCEL_CHANNEL(X),
> +	ADIS16460_ACCEL_CHANNEL(Y),
> +	ADIS16460_ACCEL_CHANNEL(Z),
> +	ADIS16460_TEMP_CHANNEL(),
> +	IIO_CHAN_SOFT_TIMESTAMP(7)
> +};
> +
> +static const struct adis16460_chip_info adis16460_chip_info = {
> +	.channels = adis16460_channels,
> +	.num_channels = ARRAY_SIZE(adis16460_channels),
> +	/*
> +	 * storing the value in rad/degree and the scale in degree
> +	 * gives us the result in rad and better precession than
> +	 * storing the scale directly in rad.
> +	 */
> +	.gyro_max_val = IIO_RAD_TO_DEGREE(200 << 16),
> +	.gyro_max_scale = 1,
> +	.accel_max_val = IIO_M_S_2_TO_G(20000 << 16),
> +	.accel_max_scale = 5,
> +};
> +
> +static const struct iio_info adis16460_info = {
> +	.read_raw = &adis16460_read_raw,
> +	.write_raw = &adis16460_write_raw,
> +	.update_scan_mode = adis_update_scan_mode,
> +	.debugfs_reg_access = adis_debugfs_reg_access,
> +};
> +
> +static int adis16460_enable_irq(struct adis *adis, bool enable)
> +{
> +	/*
> +	 * There is no way to gate the data-ready signal internally inside the
> +	 * ADIS16460 :(

Yuk. That's not a nice thing to drop in a simplified design!
oh well, it's far from the first time.

> +	 */
> +	if (enable)
> +		enable_irq(adis->spi->irq);
> +	else
> +		disable_irq(adis->spi->irq);
> +
> +	return 0;
> +}
> +
> +static int adis16460_initial_setup(struct iio_dev *indio_dev)
> +{
> +	struct adis16460 *st = iio_priv(indio_dev);
> +	uint16_t prod_id;
> +	unsigned int device_id;
> +	int ret;
> +
> +	adis_reset(&st->adis);
> +	msleep(222);
> +
> +	ret = adis_write_reg_16(&st->adis, ADIS16460_REG_GLOB_CMD, BIT(1));
> +	if (ret)
> +		return ret;
> +	msleep(75);
> +
> +	ret = adis_check_status(&st->adis);
> +	if (ret)
> +		return ret;
> +
> +	ret = adis_read_reg_16(&st->adis, ADIS16460_REG_PROD_ID, &prod_id);
> +	if (ret)
> +		return ret;
> +
> +	ret = sscanf(indio_dev->name, "adis%u\n", &device_id);
> +	if (ret != 1)
> +		return -EINVAL;
> +
> +	if (prod_id != device_id)
> +		dev_warn(&indio_dev->dev, "Device ID(%u) and product ID(%u) do not match.",
> +				device_id, prod_id);
> +
> +	return 0;
> +}
> +
> +#define ADIS16460_DIAG_STAT_IN_CLK_OOS	7
> +#define ADIS16460_DIAG_STAT_FLASH_MEM	6
> +#define ADIS16460_DIAG_STAT_SELF_TEST	5
> +#define ADIS16460_DIAG_STAT_OVERRANGE	4
> +#define ADIS16460_DIAG_STAT_SPI_COMM	3
> +#define ADIS16460_DIAG_STAT_FLASH_UPT	2
> +
> +static const char * const adis16460_status_error_msgs[] = {
> +	[ADIS16460_DIAG_STAT_IN_CLK_OOS] = "Input clock out of sync",
> +	[ADIS16460_DIAG_STAT_FLASH_MEM] = "Flash memory failure",
> +	[ADIS16460_DIAG_STAT_SELF_TEST] = "Self test diagnostic failure",
> +	[ADIS16460_DIAG_STAT_OVERRANGE] = "Sensor overrange",
> +	[ADIS16460_DIAG_STAT_SPI_COMM] = "SPI communication failure",
> +	[ADIS16460_DIAG_STAT_FLASH_UPT] = "Flash update failure",
> +};
> +
> +static const struct adis_data adis16460_data = {
> +	.diag_stat_reg = ADIS16460_REG_DIAG_STAT,
> +	.glob_cmd_reg = ADIS16460_REG_GLOB_CMD,
> +	.has_paging = false,
> +	.read_delay = 5,
> +	.write_delay = 5,
> +	.cs_stall_delay = 16,
> +	.status_error_msgs = adis16460_status_error_msgs,
> +	.status_error_mask = BIT(ADIS16460_DIAG_STAT_IN_CLK_OOS) |
> +		BIT(ADIS16460_DIAG_STAT_FLASH_MEM) |
> +		BIT(ADIS16460_DIAG_STAT_SELF_TEST) |
> +		BIT(ADIS16460_DIAG_STAT_OVERRANGE) |
> +		BIT(ADIS16460_DIAG_STAT_SPI_COMM) |
> +		BIT(ADIS16460_DIAG_STAT_FLASH_UPT),
> +	.enable_irq = adis16460_enable_irq,
> +};
> +
> +static int adis16460_probe(struct spi_device *spi)
> +{
> +	struct iio_dev *indio_dev;
> +	struct adis16460 *st;
> +	int ret;
> +
> +	indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*st));
> +	if (indio_dev == NULL)
> +		return -ENOMEM;
> +
> +	spi_set_drvdata(spi, indio_dev);
> +
> +	st = iio_priv(indio_dev);
> +
> +	st->chip_info = &adis16460_chip_info;
> +	indio_dev->dev.parent = &spi->dev;
> +	indio_dev->name = spi_get_device_id(spi)->name;
> +	indio_dev->channels = st->chip_info->channels;
> +	indio_dev->num_channels = st->chip_info->num_channels;
> +	indio_dev->info = &adis16460_info;
> +	indio_dev->modes = INDIO_DIRECT_MODE;
> +
> +	ret = adis_init(&st->adis, indio_dev, spi, &adis16460_data);
> +	if (ret)
> +		return ret;
> +
> +	ret = adis_setup_buffer_and_trigger(&st->adis, indio_dev, NULL);
> +	if (ret)
> +		return ret;
> +
> +	adis16460_enable_irq(&st->adis, 0);
> +
> +	ret = adis16460_initial_setup(indio_dev);
> +	if (ret)
> +		goto error_cleanup_buffer;
> +
> +	ret = iio_device_register(indio_dev);
> +	if (ret)
> +		goto error_cleanup_buffer;
> +
> +	adis16460_debugfs_init(indio_dev);
> +
> +	return 0;
> +
> +error_cleanup_buffer:
> +	adis_cleanup_buffer_and_trigger(&st->adis, indio_dev);
> +	return ret;
> +}
> +
> +static int adis16460_remove(struct spi_device *spi)
> +{
> +	struct iio_dev *indio_dev = spi_get_drvdata(spi);
> +	struct adis16460 *st = iio_priv(indio_dev);
> +
> +	iio_device_unregister(indio_dev);
> +
> +	adis_cleanup_buffer_and_trigger(&st->adis, indio_dev);
It might be worth thinking about a devm_ variation of the
function that pairs with this so as to tidy up (get rid of!) this
rather minimal remove...

> +
> +	return 0;
> +}
> +
> +static const struct spi_device_id adis16460_ids[] = {
> +	{ "adis16460", 0 },
> +	{}
> +};
> +MODULE_DEVICE_TABLE(spi, adis16460_id);
> +
> +static const struct of_device_id adis16460_of_match[] = {
> +	{ .compatible = "adi,adis16460" },
> +	{}
> +};
> +MODULE_DEVICE_TABLE(of, adis16460_of_match);
> +
> +static struct spi_driver adis16460_driver = {
> +	.driver = {
> +		.name = "adis16460",
> +		.of_match_table = adis16460_of_match,
> +	},
> +	.id_table = adis16460_ids,
> +	.probe = adis16460_probe,
> +	.remove = adis16460_remove,
> +};
> +module_spi_driver(adis16460_driver);
> +
> +MODULE_AUTHOR("Dragos Bogdan <dragos.bogdan@analog.com>");
> +MODULE_DESCRIPTION("Analog Devices ADIS16460 IMU driver");
> +MODULE_LICENSE("GPL");

Re: [PATCH 1/5] MAINTAINERS: add ADIS IMU driver library entry

[Ardelean, Alexandru]

On Wed, 2019-06-26 at 19:32 +0100, Jonathan Cameron wrote:
> [External]
> 
> On Tue, 25 Jun 2019 16:13:24 +0300
> Alexandru Ardelean <alexandru.ardelean@analog.com> wrote:
> 
> > This change adds the ADIS driver library to the MAINTAINERS list, and adds
> > myself as the current maintainer of this library.
> > 
> > Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
> Hi Alexandru,
> 
> Given this only has a tangential relationship to the rest of the patches
> I'll pick up now.
> 
> More generally this series is complex enough a cover letter would have
> been good + 5/5 isn't anywhere that I can find?

That is a bit weird.
I guess our mail server went a little bust when sending the series and dropped the series.

Sorry about that.
Alex

> 
> 
> Thanks,
> 
> Jonathan
> 
> > ---
> >  MAINTAINERS | 7 +++++++
> >  1 file changed, 7 insertions(+)
> > 
> > diff --git a/MAINTAINERS b/MAINTAINERS
> > index 1eb971608ac4..544e23753e96 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -930,6 +930,13 @@ S:	Supported
> >  F:	drivers/mux/adgs1408.c
> >  F:	Documentation/devicetree/bindings/mux/adi,adgs1408.txt
> >  
> > +ANALOG DEVICES INC ADIS DRIVER LIBRARY
> > +M:	Alexandru Ardelean <alexandru.ardelean@analog.com>
> > +S:	Supported
> > +L:	linux-iio@vger.kernel.org
> > +F:	include/linux/iio/imu/adis.h
> > +F:	drivers/iio/imu/adis.c
> > +
> >  ANALOG DEVICES INC ADP5061 DRIVER
> >  M:	Stefan Popa <stefan.popa@analog.com>
> >  L:	linux-pm@vger.kernel.org

Re: [PATCH 2/5] drivers: spi: core: Add optional stall delay between cs_change transfers

[Ardelean, Alexandru]

On Wed, 2019-06-26 at 19:34 +0100, Jonathan Cameron wrote:
> [External]
> 
> On Tue, 25 Jun 2019 16:13:25 +0300
> Alexandru Ardelean <alexandru.ardelean@analog.com> wrote:
> 
> > Some devices like the ADIS16460 IMU require a stall period between
> > transfers, i.e. between when the CS is de-asserted and re-asserted. The
> > default value of 10us is not enough. This change makes the delay
> > configurable for when the next CS change goes active.
> > 
> > Signed-off-by: Michael Hennerich <michael.hennerich@analog.com>
> > Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
> 
> General principle seems fine to me, though naming may need some work.
> 
> cs_low_time or something more specific than stall perhaps?

I agree the naming could do some work.
It was one of the items [in this series] for which I was expecting a bit of a discussion.

Thanks
Alex

> 
> +CC Mark.
> 
> > ---
> >  drivers/spi/spi.c       | 3 ++-
> >  include/linux/spi/spi.h | 3 +++
> >  2 files changed, 5 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
> > index 5e75944ad5d1..739de0118ee1 100644
> > --- a/drivers/spi/spi.c
> > +++ b/drivers/spi/spi.c
> > @@ -1163,7 +1163,8 @@ static int spi_transfer_one_message(struct spi_controller *ctlr,
> >  				keep_cs = true;
> >  			} else {
> >  				spi_set_cs(msg->spi, false);
> > -				udelay(10);
> > +				udelay(xfer->cs_change_stall_delay_us ?
> > +				       xfer->cs_change_stall_delay_us : 10);
> >  				spi_set_cs(msg->spi, true);
> >  			}
> >  		}
> > diff --git a/include/linux/spi/spi.h b/include/linux/spi/spi.h
> > index 053abd22ad31..d23add3b4790 100644
> > --- a/include/linux/spi/spi.h
> > +++ b/include/linux/spi/spi.h
> > @@ -734,6 +734,8 @@ extern void spi_res_release(struct spi_controller *ctlr,
> >   *      transfer. If 0 the default (from @spi_device) is used.
> >   * @bits_per_word: select a bits_per_word other than the device default
> >   *      for this transfer. If 0 the default (from @spi_device) is used.
> > + * @cs_change_stall_delay_us: microseconds to delay between cs_change
> > + * 	transfers.
> >   * @cs_change: affects chipselect after this transfer completes
> >   * @delay_usecs: microseconds to delay after this transfer before
> >   *	(optionally) changing the chipselect status, then starting
> > @@ -823,6 +825,7 @@ struct spi_transfer {
> >  #define	SPI_NBITS_QUAD		0x04 /* 4bits transfer */
> >  	u8		bits_per_word;
> >  	u8		word_delay_usecs;
> > +	u8		cs_change_stall_delay_us;
> >  	u16		delay_usecs;
> >  	u32		speed_hz;
> >  	u16		word_delay;

Re: [PATCH 4/5] iio: imu: Add support for the ADIS16460 IMU

[Ardelean, Alexandru]

On Wed, 2019-06-26 at 19:47 +0100, Jonathan Cameron wrote:
> [External]
> 
> On Tue, 25 Jun 2019 16:13:27 +0300
> Alexandru Ardelean <alexandru.ardelean@analog.com> wrote:
> 
> > The ADIS16460 device is a complete inertial system that includes a triaxial
> > gyroscope and a triaxial accelerometer. It's more simplified design than
> > that of the ADIS16480, and does not offer the triaxial magnetometers &
> > pressure sensors. It does also have a temperature sensor (like the
> > ADIS16480).
> > Since it is part of the ADIS16XXX family, it re-uses parts of the ADIS
> > library.
> > 
> > Naturally, the register map is different and much more simplified than the
> > ADIS16480 subfamily, so it cannot be integrated into that driver. A major
> > difference is that the registers are not paged.
> > 
> > One thing that is particularly special about it, is that it requires a
> > higher CS stall delay between data (around 16 uS vs other chips from the
> > family requiring around 2 uS).
> > 
> > Datasheet:
> >   https://www.analog.com/media/en/technical-documentation/data-sheets/adis16460.pdf
> > 
> > Signed-off-by: Dragos Bogdan <dragos.bogdan@analog.com>
> > Signed-off-by: Michael Hennerich <michael.hennerich@analog.com>
> > Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
> 
> A few comments inline, but only one that matters. Licenses disagree
> (you would almost have thought I'd just fixed a load of these and
> have it fresh in my mind!)
> 
> Thanks,
> 
> Jonathan
> 
> > ---
> >  MAINTAINERS                 |   7 +
> >  drivers/iio/imu/Kconfig     |   9 +
> >  drivers/iio/imu/Makefile    |   1 +
> >  drivers/iio/imu/adis16460.c | 490 ++++++++++++++++++++++++++++++++++++
> >  4 files changed, 507 insertions(+)
> >  create mode 100644 drivers/iio/imu/adis16460.c
> > 
> > diff --git a/MAINTAINERS b/MAINTAINERS
> > index 544e23753e96..ef2e2cee32e1 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -937,6 +937,13 @@ L:	linux-iio@vger.kernel.org
> >  F:	include/linux/iio/imu/adis.h
> >  F:	drivers/iio/imu/adis.c
> >  
> > +ANALOG DEVICES INC ADIS16460 DRIVER
> > +M:	Dragos Bogdan <dragos.bogdan@analog.com>
> > +S:	Supported
> > +L:	linux-iio@vger.kernel.org
> > +W:	http://ez.analog.com/community/linux-device-drivers
> > +F:	drivers/iio/imu/adis16460.c
> > +
> >  ANALOG DEVICES INC ADP5061 DRIVER
> >  M:	Stefan Popa <stefan.popa@analog.com>
> >  L:	linux-pm@vger.kernel.org
> > diff --git a/drivers/iio/imu/Kconfig b/drivers/iio/imu/Kconfig
> > index 156630a21696..a29a481c20d2 100644
> > --- a/drivers/iio/imu/Kconfig
> > +++ b/drivers/iio/imu/Kconfig
> > @@ -16,6 +16,15 @@ config ADIS16400
> >  	  adis16365, adis16400 and adis16405 triaxial inertial sensors
> >  	  (adis16400 series also have magnetometers).
> >  
> > +config ADIS16460
> > +	tristate "Analog Devices ADIS16460 and similar IMU driver"
> > +	depends on SPI
> > +	select IIO_ADIS_LIB
> > +	select IIO_ADIS_LIB_BUFFER if IIO_BUFFER
> > +	help
> > +	  Say yes here to build support for Analog Devices ADIS16460 inertial
> > +	  sensor.
> I have a nasty feeling this is short enough that one of the static
> analysis tools will moan and we'll get a patch adding the name of the module
> or something like that, mostly to shut up the warning.

ack

> 
> > +
> >  config ADIS16480
> >  	tristate "Analog Devices ADIS16480 and similar IMU driver"
> >  	depends on SPI
> > diff --git a/drivers/iio/imu/Makefile b/drivers/iio/imu/Makefile
> > index 9e452fce1aaf..4a6958865504 100644
> > --- a/drivers/iio/imu/Makefile
> > +++ b/drivers/iio/imu/Makefile
> > @@ -5,6 +5,7 @@
> >  
> >  # When adding new entries keep the list in alphabetical order
> >  obj-$(CONFIG_ADIS16400) += adis16400.o
> > +obj-$(CONFIG_ADIS16460) += adis16460.o
> >  obj-$(CONFIG_ADIS16480) += adis16480.o
> >  
> >  adis_lib-y += adis.o
> > diff --git a/drivers/iio/imu/adis16460.c b/drivers/iio/imu/adis16460.c
> > new file mode 100644
> > index 000000000000..6c86af06b5d1
> > --- /dev/null
> > +++ b/drivers/iio/imu/adis16460.c
> > @@ -0,0 +1,490 @@
> > +// SPDX-License-Identifier: GPL-2.0
> 
> MODULE_LICENSE(GPL) means GPL-2.0+ IIRC so fix one or the other.

ack
I'll update this

> 
> > +/*
> > + * ADIS16460 IMU driver
> > + *
> > + * Copyright 2019 Analog Devices Inc.
> > + *
> Nitpick, This blank line doesn't add anything.

ack

> 
> > + */
> > +
> > +#include <linux/delay.h>
> > +#include <linux/module.h>
> > +#include <linux/spi/spi.h>
> > +
> > +#include <linux/iio/iio.h>
> > +#include <linux/iio/imu/adis.h>
> > +
> > +#include <linux/debugfs.h>
> > +
> > +#define ADIS16460_REG_FLASH_CNT		0x00
> > +#define ADIS16460_REG_DIAG_STAT		0x02
> > +#define ADIS16460_REG_X_GYRO_LOW	0x04
> > +#define ADIS16460_REG_X_GYRO_OUT	0x06
> > +#define ADIS16460_REG_Y_GYRO_LOW	0x08
> > +#define ADIS16460_REG_Y_GYRO_OUT	0x0A
> > +#define ADIS16460_REG_Z_GYRO_LOW	0x0C
> > +#define ADIS16460_REG_Z_GYRO_OUT	0x0E
> > +#define ADIS16460_REG_X_ACCL_LOW	0x10
> > +#define ADIS16460_REG_X_ACCL_OUT	0x12
> > +#define ADIS16460_REG_Y_ACCL_LOW	0x14
> > +#define ADIS16460_REG_Y_ACCL_OUT	0x16
> > +#define ADIS16460_REG_Z_ACCL_LOW	0x18
> > +#define ADIS16460_REG_Z_ACCL_OUT	0x1A
> > +#define ADIS16460_REG_SMPL_CNTR		0x1C
> > +#define ADIS16460_REG_TEMP_OUT		0x1E
> > +#define ADIS16460_REG_X_DELT_ANG	0x24
> > +#define ADIS16460_REG_Y_DELT_ANG	0x26
> > +#define ADIS16460_REG_Z_DELT_ANG	0x28
> > +#define ADIS16460_REG_X_DELT_VEL	0x2A
> > +#define ADIS16460_REG_Y_DELT_VEL	0x2C
> > +#define ADIS16460_REG_Z_DELT_VEL	0x2E
> > +#define ADIS16460_REG_MSC_CTRL		0x32
> > +#define ADIS16460_REG_SYNC_SCAL		0x34
> > +#define ADIS16460_REG_DEC_RATE		0x36
> > +#define ADIS16460_REG_FLTR_CTRL		0x38
> > +#define ADIS16460_REG_GLOB_CMD		0x3E
> > +#define ADIS16460_REG_X_GYRO_OFF	0x40
> > +#define ADIS16460_REG_Y_GYRO_OFF	0x42
> > +#define ADIS16460_REG_Z_GYRO_OFF	0x44
> > +#define ADIS16460_REG_X_ACCL_OFF	0x46
> > +#define ADIS16460_REG_Y_ACCL_OFF	0x48
> > +#define ADIS16460_REG_Z_ACCL_OFF	0x4A
> > +#define ADIS16460_REG_LOT_ID1 R		0x52
> > +#define ADIS16460_REG_LOT_ID2 R		0x54
> > +#define ADIS16460_REG_PROD_ID		0x56
> > +#define ADIS16460_REG_SERIAL_NUM	0x58
> > +#define ADIS16460_REG_CAL_SGNTR		0x60
> > +#define ADIS16460_REG_CAL_CRC		0x62
> > +#define ADIS16460_REG_CODE_SGNTR	0x64
> > +#define ADIS16460_REG_CODE_CRC		0x66
> > +
> > +struct adis16460_chip_info {
> 
> Future proofing, or another part going to be added shortly?
> Given the history of these drivers and the large number of parts
> that have shown up, I'll give this one the benefit of the doubt!

Future proofing.
[In the short-term] No new parts will be added to this driver.
But we will see later, if we get an internal request for something that fits here.

From what I can tell, this driver was adapted from either the adis16400 or adis16480 driver(s).
Usually the market driver for these seems to be the automotive industry, but the current driver for this particular one
is one of our reference designs:
https://wiki.analog.com/resources/eval/user-guides/pzsdr/carriers/packrf
https://wiki.analog.com/resources/eval/user-guides/pzsdr/carriers/packrf/hardware

Somewhere in the schematics, there's a mention that this IMU is used.

> 
> > +	unsigned int num_channels;
> > +	const struct iio_chan_spec *channels;
> > +	unsigned int gyro_max_val;
> > +	unsigned int gyro_max_scale;
> > +	unsigned int accel_max_val;
> > +	unsigned int accel_max_scale;
> > +};
> > +
> > +struct adis16460 {
> > +	const struct adis16460_chip_info *chip_info;
> > +	struct adis adis;
> > +};
> > +
> > +#ifdef CONFIG_DEBUG_FS
> > +
> > +static int adis16460_show_serial_number(void *arg, u64 *val)
> > +{
> > +	struct adis16460 *adis16460 = arg;
> > +	u16 serial;
> > +	int ret;
> > +
> > +	ret = adis_read_reg_16(&adis16460->adis, ADIS16460_REG_SERIAL_NUM,
> > +		&serial);
> > +	if (ret < 0)
> > +		return ret;
> > +
> > +	*val = serial;
> > +
> > +	return 0;
> > +}
> > +DEFINE_SIMPLE_ATTRIBUTE(adis16460_serial_number_fops,
> > +	adis16460_show_serial_number, NULL, "0x%.4llx\n");
> > +
> > +static int adis16460_show_product_id(void *arg, u64 *val)
> > +{
> > +	struct adis16460 *adis16460 = arg;
> > +	u16 prod_id;
> > +	int ret;
> > +
> > +	ret = adis_read_reg_16(&adis16460->adis, ADIS16460_REG_PROD_ID,
> > +		&prod_id);
> > +	if (ret < 0)
> > +		return ret;
> > +
> > +	*val = prod_id;
> > +
> > +	return 0;
> > +}
> > +DEFINE_SIMPLE_ATTRIBUTE(adis16460_product_id_fops,
> > +	adis16460_show_product_id, NULL, "%llu\n");
> > +
> > +static int adis16460_show_flash_count(void *arg, u64 *val)
> > +{
> > +	struct adis16460 *adis16460 = arg;
> > +	u32 flash_count;
> > +	int ret;
> > +
> > +	ret = adis_read_reg_32(&adis16460->adis, ADIS16460_REG_FLASH_CNT,
> > +		&flash_count);
> > +	if (ret < 0)
> > +		return ret;
> > +
> > +	*val = flash_count;
> > +
> > +	return 0;
> > +}
> > +DEFINE_SIMPLE_ATTRIBUTE(adis16460_flash_count_fops,
> > +	adis16460_show_flash_count, NULL, "%lld\n");
> > +
> > +static int adis16460_debugfs_init(struct iio_dev *indio_dev)
> > +{
> > +	struct adis16460 *adis16460 = iio_priv(indio_dev);
> > +
> > +	debugfs_create_file("serial_number", 0400, indio_dev->debugfs_dentry,
> > +		adis16460, &adis16460_serial_number_fops);
> > +	debugfs_create_file("product_id", 0400, indio_dev->debugfs_dentry,
> > +		adis16460, &adis16460_product_id_fops);
> > +	debugfs_create_file("flash_count", 0400, indio_dev->debugfs_dentry,
> > +		adis16460, &adis16460_flash_count_fops);
> > +
> > +	return 0;
> > +}
> > +
> > +#else
> > +
> > +static int adis16460_debugfs_init(struct iio_dev *indio_dev)
> > +{
> > +	return 0;
> > +}
> > +
> > +#endif
> > +
> > +static int adis16460_set_freq(struct iio_dev *indio_dev, int val, int val2)
> > +{
> > +	struct adis16460 *st = iio_priv(indio_dev);
> > +	unsigned int t;
> > +
> > +	t =  val * 1000 + val2 / 1000;
> > +	if (t <= 0)
> > +		return -EINVAL;
> > +
> > +	t = 2048000 / t;
> > +	if (t > 2048)
> > +		t = 2048;
> > +
> > +	if (t != 0)
> > +		t--;
> > +
> > +	return adis_write_reg_16(&st->adis, ADIS16460_REG_DEC_RATE, t);
> > +}
> > +
> > +static int adis16460_get_freq(struct iio_dev *indio_dev, int *val, int *val2)
> > +{
> > +	struct adis16460 *st = iio_priv(indio_dev);
> > +	uint16_t t;
> > +	int ret;
> > +	unsigned freq;
> > +
> > +	ret = adis_read_reg_16(&st->adis, ADIS16460_REG_DEC_RATE, &t);
> > +	if (ret < 0)
> > +		return ret;
> > +
> > +	freq = 2048000 / (t + 1);
> > +	*val = freq / 1000;
> > +	*val2 = (freq % 1000) * 1000;
> > +
> > +	return IIO_VAL_INT_PLUS_MICRO;
> > +}
> > +
> > +static int adis16460_read_raw(struct iio_dev *indio_dev,
> > +	const struct iio_chan_spec *chan, int *val, int *val2, long info)
> > +{
> > +	struct adis16460 *st = iio_priv(indio_dev);
> > +
> > +	switch (info) {
> > +	case IIO_CHAN_INFO_RAW:
> > +		return adis_single_conversion(indio_dev, chan, 0, val);
> > +	case IIO_CHAN_INFO_SCALE:
> > +		switch (chan->type) {
> > +		case IIO_ANGL_VEL:
> > +			*val = st->chip_info->gyro_max_scale;
> > +			*val2 = st->chip_info->gyro_max_val;
> > +			return IIO_VAL_FRACTIONAL;
> > +		case IIO_ACCEL:
> > +			*val = st->chip_info->accel_max_scale;
> > +			*val2 = st->chip_info->accel_max_val;
> > +			return IIO_VAL_FRACTIONAL;
> > +		case IIO_TEMP:
> > +			*val = 50; /* 50 milli degrees Celsius/LSB */
> > +			return IIO_VAL_INT;
> > +		default:
> > +			return -EINVAL;
> > +		}
> > +	case IIO_CHAN_INFO_OFFSET:
> > +		*val = 500; /* 25 degrees Celsius = 0x0000 */
> > +		return IIO_VAL_INT;
> > +	case IIO_CHAN_INFO_SAMP_FREQ:
> > +		return adis16460_get_freq(indio_dev, val, val2);
> > +	default:
> > +		return -EINVAL;
> > +	}
> > +}
> > +
> > +static int adis16460_write_raw(struct iio_dev *indio_dev,
> > +	const struct iio_chan_spec *chan, int val, int val2, long info)
> > +{
> > +	switch (info) {
> > +	case IIO_CHAN_INFO_SAMP_FREQ:
> > +		return adis16460_set_freq(indio_dev, val, val2);
> > +	default:
> > +		return -EINVAL;
> > +	}
> > +}
> > +
> > +enum {
> > +	ADIS16460_SCAN_GYRO_X,
> > +	ADIS16460_SCAN_GYRO_Y,
> > +	ADIS16460_SCAN_GYRO_Z,
> > +	ADIS16460_SCAN_ACCEL_X,
> > +	ADIS16460_SCAN_ACCEL_Y,
> > +	ADIS16460_SCAN_ACCEL_Z,
> > +	ADIS16460_SCAN_TEMP,
> > +};
> > +
> > +#define ADIS16460_MOD_CHANNEL(_type, _mod, _address, _si, _bits) \
> > +	{ \
> > +		.type = (_type), \
> > +		.modified = 1, \
> > +		.channel2 = (_mod), \
> > +		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW), \
> > +		.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE), \
> > +		.info_mask_shared_by_all = BIT(IIO_CHAN_INFO_SAMP_FREQ), \
> > +		.address = (_address), \
> > +		.scan_index = (_si), \
> > +		.scan_type = { \
> > +			.sign = 's', \
> > +			.realbits = (_bits), \
> > +			.storagebits = (_bits), \
> > +			.endianness = IIO_BE, \
> > +		}, \
> > +	}
> > +
> > +#define ADIS16460_GYRO_CHANNEL(_mod) \
> > +	ADIS16460_MOD_CHANNEL(IIO_ANGL_VEL, IIO_MOD_ ## _mod, \
> > +	ADIS16460_REG_ ## _mod ## _GYRO_LOW, ADIS16460_SCAN_GYRO_ ## _mod, \
> > +	32)
> > +
> > +#define ADIS16460_ACCEL_CHANNEL(_mod) \
> > +	ADIS16460_MOD_CHANNEL(IIO_ACCEL, IIO_MOD_ ## _mod, \
> > +	ADIS16460_REG_ ## _mod ## _ACCL_LOW, ADIS16460_SCAN_ACCEL_ ## _mod, \
> > +	32)
> > +
> > +#define ADIS16460_TEMP_CHANNEL() { \
> > +		.type = IIO_TEMP, \
> > +		.indexed = 1, \
> > +		.channel = 0, \
> > +		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) | \
> > +			BIT(IIO_CHAN_INFO_SCALE) | \
> > +			BIT(IIO_CHAN_INFO_OFFSET), \
> > +		.info_mask_shared_by_all = BIT(IIO_CHAN_INFO_SAMP_FREQ), \
> > +		.address = ADIS16460_REG_TEMP_OUT, \
> > +		.scan_index = ADIS16460_SCAN_TEMP, \
> > +		.scan_type = { \
> > +			.sign = 's', \
> > +			.realbits = 16, \
> > +			.storagebits = 16, \
> > +			.endianness = IIO_BE, \
> > +		}, \
> > +	}
> > +
> > +static const struct iio_chan_spec adis16460_channels[] = {
> > +	ADIS16460_GYRO_CHANNEL(X),
> > +	ADIS16460_GYRO_CHANNEL(Y),
> > +	ADIS16460_GYRO_CHANNEL(Z),
> > +	ADIS16460_ACCEL_CHANNEL(X),
> > +	ADIS16460_ACCEL_CHANNEL(Y),
> > +	ADIS16460_ACCEL_CHANNEL(Z),
> > +	ADIS16460_TEMP_CHANNEL(),
> > +	IIO_CHAN_SOFT_TIMESTAMP(7)
> > +};
> > +
> > +static const struct adis16460_chip_info adis16460_chip_info = {
> > +	.channels = adis16460_channels,
> > +	.num_channels = ARRAY_SIZE(adis16460_channels),
> > +	/*
> > +	 * storing the value in rad/degree and the scale in degree
> > +	 * gives us the result in rad and better precession than
> > +	 * storing the scale directly in rad.
> > +	 */
> > +	.gyro_max_val = IIO_RAD_TO_DEGREE(200 << 16),
> > +	.gyro_max_scale = 1,
> > +	.accel_max_val = IIO_M_S_2_TO_G(20000 << 16),
> > +	.accel_max_scale = 5,
> > +};
> > +
> > +static const struct iio_info adis16460_info = {
> > +	.read_raw = &adis16460_read_raw,
> > +	.write_raw = &adis16460_write_raw,
> > +	.update_scan_mode = adis_update_scan_mode,
> > +	.debugfs_reg_access = adis_debugfs_reg_access,
> > +};
> > +
> > +static int adis16460_enable_irq(struct adis *adis, bool enable)
> > +{
> > +	/*
> > +	 * There is no way to gate the data-ready signal internally inside the
> > +	 * ADIS16460 :(
> 
> Yuk. That's not a nice thing to drop in a simplified design!
> oh well, it's far from the first time.
> 
> > +	 */
> > +	if (enable)
> > +		enable_irq(adis->spi->irq);
> > +	else
> > +		disable_irq(adis->spi->irq);
> > +
> > +	return 0;
> > +}
> > +
> > +static int adis16460_initial_setup(struct iio_dev *indio_dev)
> > +{
> > +	struct adis16460 *st = iio_priv(indio_dev);
> > +	uint16_t prod_id;
> > +	unsigned int device_id;
> > +	int ret;
> > +
> > +	adis_reset(&st->adis);
> > +	msleep(222);
> > +
> > +	ret = adis_write_reg_16(&st->adis, ADIS16460_REG_GLOB_CMD, BIT(1));
> > +	if (ret)
> > +		return ret;
> > +	msleep(75);
> > +
> > +	ret = adis_check_status(&st->adis);
> > +	if (ret)
> > +		return ret;
> > +
> > +	ret = adis_read_reg_16(&st->adis, ADIS16460_REG_PROD_ID, &prod_id);
> > +	if (ret)
> > +		return ret;
> > +
> > +	ret = sscanf(indio_dev->name, "adis%u\n", &device_id);
> > +	if (ret != 1)
> > +		return -EINVAL;
> > +
> > +	if (prod_id != device_id)
> > +		dev_warn(&indio_dev->dev, "Device ID(%u) and product ID(%u) do not match.",
> > +				device_id, prod_id);
> > +
> > +	return 0;
> > +}
> > +
> > +#define ADIS16460_DIAG_STAT_IN_CLK_OOS	7
> > +#define ADIS16460_DIAG_STAT_FLASH_MEM	6
> > +#define ADIS16460_DIAG_STAT_SELF_TEST	5
> > +#define ADIS16460_DIAG_STAT_OVERRANGE	4
> > +#define ADIS16460_DIAG_STAT_SPI_COMM	3
> > +#define ADIS16460_DIAG_STAT_FLASH_UPT	2
> > +
> > +static const char * const adis16460_status_error_msgs[] = {
> > +	[ADIS16460_DIAG_STAT_IN_CLK_OOS] = "Input clock out of sync",
> > +	[ADIS16460_DIAG_STAT_FLASH_MEM] = "Flash memory failure",
> > +	[ADIS16460_DIAG_STAT_SELF_TEST] = "Self test diagnostic failure",
> > +	[ADIS16460_DIAG_STAT_OVERRANGE] = "Sensor overrange",
> > +	[ADIS16460_DIAG_STAT_SPI_COMM] = "SPI communication failure",
> > +	[ADIS16460_DIAG_STAT_FLASH_UPT] = "Flash update failure",
> > +};
> > +
> > +static const struct adis_data adis16460_data = {
> > +	.diag_stat_reg = ADIS16460_REG_DIAG_STAT,
> > +	.glob_cmd_reg = ADIS16460_REG_GLOB_CMD,
> > +	.has_paging = false,
> > +	.read_delay = 5,
> > +	.write_delay = 5,
> > +	.cs_stall_delay = 16,
> > +	.status_error_msgs = adis16460_status_error_msgs,
> > +	.status_error_mask = BIT(ADIS16460_DIAG_STAT_IN_CLK_OOS) |
> > +		BIT(ADIS16460_DIAG_STAT_FLASH_MEM) |
> > +		BIT(ADIS16460_DIAG_STAT_SELF_TEST) |
> > +		BIT(ADIS16460_DIAG_STAT_OVERRANGE) |
> > +		BIT(ADIS16460_DIAG_STAT_SPI_COMM) |
> > +		BIT(ADIS16460_DIAG_STAT_FLASH_UPT),
> > +	.enable_irq = adis16460_enable_irq,
> > +};
> > +
> > +static int adis16460_probe(struct spi_device *spi)
> > +{
> > +	struct iio_dev *indio_dev;
> > +	struct adis16460 *st;
> > +	int ret;
> > +
> > +	indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*st));
> > +	if (indio_dev == NULL)
> > +		return -ENOMEM;
> > +
> > +	spi_set_drvdata(spi, indio_dev);
> > +
> > +	st = iio_priv(indio_dev);
> > +
> > +	st->chip_info = &adis16460_chip_info;
> > +	indio_dev->dev.parent = &spi->dev;
> > +	indio_dev->name = spi_get_device_id(spi)->name;
> > +	indio_dev->channels = st->chip_info->channels;
> > +	indio_dev->num_channels = st->chip_info->num_channels;
> > +	indio_dev->info = &adis16460_info;
> > +	indio_dev->modes = INDIO_DIRECT_MODE;
> > +
> > +	ret = adis_init(&st->adis, indio_dev, spi, &adis16460_data);
> > +	if (ret)
> > +		return ret;
> > +
> > +	ret = adis_setup_buffer_and_trigger(&st->adis, indio_dev, NULL);
> > +	if (ret)
> > +		return ret;
> > +
> > +	adis16460_enable_irq(&st->adis, 0);
> > +
> > +	ret = adis16460_initial_setup(indio_dev);
> > +	if (ret)
> > +		goto error_cleanup_buffer;
> > +
> > +	ret = iio_device_register(indio_dev);
> > +	if (ret)
> > +		goto error_cleanup_buffer;
> > +
> > +	adis16460_debugfs_init(indio_dev);
> > +
> > +	return 0;
> > +
> > +error_cleanup_buffer:
> > +	adis_cleanup_buffer_and_trigger(&st->adis, indio_dev);
> > +	return ret;
> > +}
> > +
> > +static int adis16460_remove(struct spi_device *spi)
> > +{
> > +	struct iio_dev *indio_dev = spi_get_drvdata(spi);
> > +	struct adis16460 *st = iio_priv(indio_dev);
> > +
> > +	iio_device_unregister(indio_dev);
> > +
> > +	adis_cleanup_buffer_and_trigger(&st->adis, indio_dev);
> It might be worth thinking about a devm_ variation of the
> function that pairs with this so as to tidy up (get rid of!) this
> rather minimal remove...

I agree.
I'll make a note of it.

I was looking through the ADIS lib (when preparing this driver) and it seems that some more cleanup could be added in
there.

> 
> > +
> > +	return 0;
> > +}
> > +
> > +static const struct spi_device_id adis16460_ids[] = {
> > +	{ "adis16460", 0 },
> > +	{}
> > +};
> > +MODULE_DEVICE_TABLE(spi, adis16460_id);
> > +
> > +static const struct of_device_id adis16460_of_match[] = {
> > +	{ .compatible = "adi,adis16460" },
> > +	{}
> > +};
> > +MODULE_DEVICE_TABLE(of, adis16460_of_match);
> > +
> > +static struct spi_driver adis16460_driver = {
> > +	.driver = {
> > +		.name = "adis16460",
> > +		.of_match_table = adis16460_of_match,
> > +	},
> > +	.id_table = adis16460_ids,
> > +	.probe = adis16460_probe,
> > +	.remove = adis16460_remove,
> > +};
> > +module_spi_driver(adis16460_driver);
> > +
> > +MODULE_AUTHOR("Dragos Bogdan <dragos.bogdan@analog.com>");
> > +MODULE_DESCRIPTION("Analog Devices ADIS16460 IMU driver");
> > +MODULE_LICENSE("GPL");

Re: [PATCH 2/5] drivers: spi: core: Add optional stall delay between cs_change transfers

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 796 bytes --]

On Wed, Jun 26, 2019 at 07:34:38PM +0100, Jonathan Cameron wrote:
> On Tue, 25 Jun 2019 16:13:25 +0300
> Alexandru Ardelean <alexandru.ardelean@analog.com> wrote:
> 
> > Some devices like the ADIS16460 IMU require a stall period between
> > transfers, i.e. between when the CS is de-asserted and re-asserted. The
> > default value of 10us is not enough. This change makes the delay
> > configurable for when the next CS change goes active.

This looks like cs_change_delay.

As documented in SubmittingPatches please send patches to the 
maintainers for the code you would like to change.  The normal kernel
workflow is that people apply patches from their inboxes, if they aren't
copied they are likely to not see the patch at all and it is much more
difficult to apply patches.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH 2/5] drivers: spi: core: Add optional stall delay between cs_change transfers

[Ardelean, Alexandru]

On Tue, 2019-07-09 at 15:12 +0100, Mark Brown wrote:
> On Wed, Jun 26, 2019 at 07:34:38PM +0100, Jonathan Cameron wrote:
> > On Tue, 25 Jun 2019 16:13:25 +0300
> > Alexandru Ardelean <alexandru.ardelean@analog.com> wrote:
> > 
> > > Some devices like the ADIS16460 IMU require a stall period between
> > > transfers, i.e. between when the CS is de-asserted and re-asserted. The
> > > default value of 10us is not enough. This change makes the delay
> > > configurable for when the next CS change goes active.
> 
> This looks like cs_change_delay.
> 
> As documented in SubmittingPatches please send patches to the 
> maintainers for the code you would like to change.  The normal kernel
> workflow is that people apply patches from their inboxes, if they aren't
> copied they are likely to not see the patch at all and it is much more
> difficult to apply patches.

Ack.
[Sorry for the late reply; I'm balancing other stuff as well and terrible at it]

I'll probably update my practice to also include maintainers via --cc to `git send-email`.
Up until now, I would send emails to lists [as much as possible] and try to not include people directly.
My assumption was that the list is enough.

I'm still adjusting to how things get done in the various Linux kernel subsystems/subgroups.