linux-iio.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/3][V4] iio: imu: Add support for the ADIS16460 IMU
@ 2019-07-23  7:36 Alexandru Ardelean
  2019-07-23  7:36 ` [PATCH 1/3][V4] iio: imu: adis: Add support for SPI transfer cs_change_delay Alexandru Ardelean
                   ` (2 more replies)
  0 siblings, 3 replies; 9+ messages in thread
From: Alexandru Ardelean @ 2019-07-23  7:36 UTC (permalink / raw)
  To: linux-iio, linux-spi, devicetree, linux-kernel
  Cc: jic23, robh+dt, mark.rutland, broonie, Alexandru Ardelean

This changeset adds support for the ADIS16460.
The default CS change delay is 10 uS, while the ADIS16460 requires a
minimum of 16 uS.

As it turns out, the SPI framework support already has support for this
feature, which is currently present in the for-next branch:
   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git/commit/?h=0ff2de8bb163551ec4230a5a6f3c40c1f6adec4f

This changeset now makes use of that feature, to allow longer CS change
times (as needed) for ADIS16460.

The SPI patch is present in the iio/testing branch, but not present in the
iio/togreg branch.

Changelog v3 -> v4:
* for SPI: no change
* for ADIS lib: use existing `cs_change_delay` feature (from SPI)
* for DT: no change

Changelog v2 -> v3:
* for SPI:
  * used `cs_change_delay` instead of `cs_change_delay_usecs` (i.e. removed
    `_usecs` suffix
  * used SPI specific subject line for SPI patch
* for ADIS lib:
  * updated to use the `cs_change_delay`
* for DT:
  * added Rob's `Reviewed-by` tag

Changelog v1 -> v2:
* for SPI:
  * renamed `cs_change_stall_delay_us` -> `cs_change_delay_usecs`
    initial recommendation was `cs_change_delay`, but decided to name this
    `cs_change_delay_usecs`, since the convention for these delays seems
    to add the `_usecs` suffix
* for ADIS lib:
  * renamed `stall_delay` -> `cs_change_delay`
  * removed some assignments of `cs_change_delay`
    where `cs_change` is not set
* for ADIS16460 driver:
  * fixed license
  * adjusted to new `cs_change_delay[_usecs]`

Alexandru Ardelean (3):
  iio: imu: adis: Add support for SPI transfer cs_change_delay
  iio: imu: Add support for the ADIS16460 IMU
  dt-bindings: iio: imu: add bindings for ADIS16460

 .../bindings/iio/imu/adi,adis16460.yaml       |  53 ++
 MAINTAINERS                                   |   8 +
 drivers/iio/imu/Kconfig                       |  12 +
 drivers/iio/imu/Makefile                      |   1 +
 drivers/iio/imu/adis.c                        |  12 +
 drivers/iio/imu/adis16460.c                   | 489 ++++++++++++++++++
 include/linux/iio/imu/adis.h                  |   2 +
 7 files changed, 577 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/iio/imu/adi,adis16460.yaml
 create mode 100644 drivers/iio/imu/adis16460.c

-- 
2.20.1


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH 1/3][V4] iio: imu: adis: Add support for SPI transfer cs_change_delay
  2019-07-23  7:36 [PATCH 0/3][V4] iio: imu: Add support for the ADIS16460 IMU Alexandru Ardelean
@ 2019-07-23  7:36 ` Alexandru Ardelean
  2019-07-27 18:48   ` Jonathan Cameron
  2019-07-23  7:36 ` [PATCH 2/3][V4] iio: imu: Add support for the ADIS16460 IMU Alexandru Ardelean
  2019-07-23  7:36 ` [PATCH 3/3][V4] dt-bindings: iio: imu: add bindings for ADIS16460 Alexandru Ardelean
  2 siblings, 1 reply; 9+ messages in thread
From: Alexandru Ardelean @ 2019-07-23  7:36 UTC (permalink / raw)
  To: linux-iio, linux-spi, devicetree, linux-kernel
  Cc: jic23, robh+dt, mark.rutland, broonie, Alexandru Ardelean,
	Michael Hennerich

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_delay` functionality from the SPI
subsystem.

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

diff --git a/drivers/iio/imu/adis.c b/drivers/iio/imu/adis.c
index 30281e91dbf9..1631c255deab 100644
--- a/drivers/iio/imu/adis.c
+++ b/drivers/iio/imu/adis.c
@@ -39,18 +39,24 @@ int adis_write_reg(struct adis *adis, unsigned int reg,
 			.len = 2,
 			.cs_change = 1,
 			.delay_usecs = adis->data->write_delay,
+			.cs_change_delay = adis->data->cs_change_delay,
+			.cs_change_delay_unit = SPI_DELAY_UNIT_USECS,
 		}, {
 			.tx_buf = adis->tx + 2,
 			.bits_per_word = 8,
 			.len = 2,
 			.cs_change = 1,
 			.delay_usecs = adis->data->write_delay,
+			.cs_change_delay = adis->data->cs_change_delay,
+			.cs_change_delay_unit = SPI_DELAY_UNIT_USECS,
 		}, {
 			.tx_buf = adis->tx + 4,
 			.bits_per_word = 8,
 			.len = 2,
 			.cs_change = 1,
 			.delay_usecs = adis->data->write_delay,
+			.cs_change_delay = adis->data->cs_change_delay,
+			.cs_change_delay_unit = SPI_DELAY_UNIT_USECS,
 		}, {
 			.tx_buf = adis->tx + 6,
 			.bits_per_word = 8,
@@ -133,12 +139,16 @@ int adis_read_reg(struct adis *adis, unsigned int reg,
 			.len = 2,
 			.cs_change = 1,
 			.delay_usecs = adis->data->write_delay,
+			.cs_change_delay = adis->data->cs_change_delay,
+			.cs_change_delay_unit = SPI_DELAY_UNIT_USECS,
 		}, {
 			.tx_buf = adis->tx + 2,
 			.bits_per_word = 8,
 			.len = 2,
 			.cs_change = 1,
 			.delay_usecs = adis->data->read_delay,
+			.cs_change_delay = adis->data->cs_change_delay,
+			.cs_change_delay_unit = SPI_DELAY_UNIT_USECS,
 		}, {
 			.tx_buf = adis->tx + 4,
 			.rx_buf = adis->rx,
@@ -146,6 +156,8 @@ int adis_read_reg(struct adis *adis, unsigned int reg,
 			.len = 2,
 			.cs_change = 1,
 			.delay_usecs = adis->data->read_delay,
+			.cs_change_delay = adis->data->cs_change_delay,
+			.cs_change_delay_unit = SPI_DELAY_UNIT_USECS,
 		}, {
 			.rx_buf = adis->rx + 2,
 			.bits_per_word = 8,
diff --git a/include/linux/iio/imu/adis.h b/include/linux/iio/imu/adis.h
index 3428d06b2f44..4c53815bb729 100644
--- a/include/linux/iio/imu/adis.h
+++ b/include/linux/iio/imu/adis.h
@@ -26,6 +26,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_change_delay: SPI delay between CS changes 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
@@ -35,6 +36,7 @@ struct adis_burst;
 struct adis_data {
 	unsigned int read_delay;
 	unsigned int write_delay;
+	unsigned int cs_change_delay;
 
 	unsigned int glob_cmd_reg;
 	unsigned int msc_ctrl_reg;
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 2/3][V4] iio: imu: Add support for the ADIS16460 IMU
  2019-07-23  7:36 [PATCH 0/3][V4] iio: imu: Add support for the ADIS16460 IMU Alexandru Ardelean
  2019-07-23  7:36 ` [PATCH 1/3][V4] iio: imu: adis: Add support for SPI transfer cs_change_delay Alexandru Ardelean
@ 2019-07-23  7:36 ` Alexandru Ardelean
  2019-07-27 18:54   ` Jonathan Cameron
  2019-07-23  7:36 ` [PATCH 3/3][V4] dt-bindings: iio: imu: add bindings for ADIS16460 Alexandru Ardelean
  2 siblings, 1 reply; 9+ messages in thread
From: Alexandru Ardelean @ 2019-07-23  7:36 UTC (permalink / raw)
  To: linux-iio, linux-spi, devicetree, linux-kernel
  Cc: jic23, robh+dt, mark.rutland, broonie, Alexandru Ardelean,
	Dragos Bogdan, Michael Hennerich

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 delay between CS changes (i.e. when CS goes up, the spec recommends
that it be brought down after a minimum of 16 uS).
Other ADIS chips require (via spec) a minimum of 2 uS between CS changes.
The kernel's 10 uS default should be fine for those other chips; they
haven't been tested with lower CS change delays yet.

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     |  12 +
 drivers/iio/imu/Makefile    |   1 +
 drivers/iio/imu/adis16460.c | 489 ++++++++++++++++++++++++++++++++++++
 4 files changed, 509 insertions(+)
 create mode 100644 drivers/iio/imu/adis16460.c

diff --git a/MAINTAINERS b/MAINTAINERS
index 783569e3c4b4..f7de89e82e35 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -945,6 +945,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 4957e6df447e..f3c7282321a8 100644
--- a/drivers/iio/imu/Kconfig
+++ b/drivers/iio/imu/Kconfig
@@ -17,6 +17,18 @@ 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.
+
+	  To compile this driver as a module, choose M here: the module will be
+	  called adis16460.
+
 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..db713cba75a2
--- /dev/null
+++ b/drivers/iio/imu/adis16460.c
@@ -0,0 +1,489 @@
+// 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		0x52
+#define ADIS16460_REG_LOT_ID2		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 int 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_change_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


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 3/3][V4] dt-bindings: iio: imu: add bindings for ADIS16460
  2019-07-23  7:36 [PATCH 0/3][V4] iio: imu: Add support for the ADIS16460 IMU Alexandru Ardelean
  2019-07-23  7:36 ` [PATCH 1/3][V4] iio: imu: adis: Add support for SPI transfer cs_change_delay Alexandru Ardelean
  2019-07-23  7:36 ` [PATCH 2/3][V4] iio: imu: Add support for the ADIS16460 IMU Alexandru Ardelean
@ 2019-07-23  7:36 ` Alexandru Ardelean
  2019-07-27 18:56   ` Jonathan Cameron
  2 siblings, 1 reply; 9+ messages in thread
From: Alexandru Ardelean @ 2019-07-23  7:36 UTC (permalink / raw)
  To: linux-iio, linux-spi, devicetree, linux-kernel
  Cc: jic23, robh+dt, mark.rutland, broonie, Alexandru Ardelean, Rob Herring

This change adds device-tree bindings for the ADIS16460.

Reviewed-by: Rob Herring <robh@kernel.org>
Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
---
 .../bindings/iio/imu/adi,adis16460.yaml       | 53 +++++++++++++++++++
 MAINTAINERS                                   |  1 +
 2 files changed, 54 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/iio/imu/adi,adis16460.yaml

diff --git a/Documentation/devicetree/bindings/iio/imu/adi,adis16460.yaml b/Documentation/devicetree/bindings/iio/imu/adi,adis16460.yaml
new file mode 100644
index 000000000000..0c53009ba7d6
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/imu/adi,adis16460.yaml
@@ -0,0 +1,53 @@
+# SPDX-License-Identifier: GPL-2.0
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/iio/imu/adi,adis16460.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Analog Devices ADIS16460 and similar IMUs
+
+maintainers:
+  - Dragos Bogdan <dragos.bogdan@analog.com>
+
+description: |
+  Analog Devices ADIS16460 and similar IMUs
+  https://www.analog.com/media/en/technical-documentation/data-sheets/ADIS16460.pdf
+
+properties:
+  compatible:
+    enum:
+      - adi,adis16460
+
+  reg:
+    maxItems: 1
+
+  spi-cpha: true
+
+  spi-cpol: true
+
+  interrupts:
+    maxItems: 1
+
+required:
+  - compatible
+  - reg
+  - interrupts
+
+examples:
+  - |
+    #include <dt-bindings/gpio/gpio.h>
+    #include <dt-bindings/interrupt-controller/irq.h>
+    spi0 {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        imu@0 {
+            compatible = "adi,adis16460";
+            reg = <0>;
+            spi-max-frequency = <5000000>;
+            spi-cpol;
+            spi-cpha;
+            interrupt-parent = <&gpio0>;
+            interrupts = <0 IRQ_TYPE_LEVEL_HIGH>;
+        };
+    };
diff --git a/MAINTAINERS b/MAINTAINERS
index f7de89e82e35..07105e43ea1e 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -951,6 +951,7 @@ S:	Supported
 L:	linux-iio@vger.kernel.org
 W:	http://ez.analog.com/community/linux-device-drivers
 F:	drivers/iio/imu/adis16460.c
+F:	Documentation/devicetree/bindings/iio/imu/adi,adis16460.yaml
 
 ANALOG DEVICES INC ADP5061 DRIVER
 M:	Stefan Popa <stefan.popa@analog.com>
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH 1/3][V4] iio: imu: adis: Add support for SPI transfer cs_change_delay
  2019-07-23  7:36 ` [PATCH 1/3][V4] iio: imu: adis: Add support for SPI transfer cs_change_delay Alexandru Ardelean
@ 2019-07-27 18:48   ` Jonathan Cameron
  0 siblings, 0 replies; 9+ messages in thread
From: Jonathan Cameron @ 2019-07-27 18:48 UTC (permalink / raw)
  To: Alexandru Ardelean
  Cc: linux-iio, linux-spi, devicetree, linux-kernel, robh+dt,
	mark.rutland, broonie, Michael Hennerich

On Tue, 23 Jul 2019 10:36:38 +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_delay` functionality from the SPI
> subsystem.
> 
> Signed-off-by: Michael Hennerich <michael.hennerich@analog.com>
> Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
Applied to the togreg branch of iio.git and pushed out as testing for
the autobuilders to play with it.

Thanks,

Jonathan

> ---
>  drivers/iio/imu/adis.c       | 12 ++++++++++++
>  include/linux/iio/imu/adis.h |  2 ++
>  2 files changed, 14 insertions(+)
> 
> diff --git a/drivers/iio/imu/adis.c b/drivers/iio/imu/adis.c
> index 30281e91dbf9..1631c255deab 100644
> --- a/drivers/iio/imu/adis.c
> +++ b/drivers/iio/imu/adis.c
> @@ -39,18 +39,24 @@ int adis_write_reg(struct adis *adis, unsigned int reg,
>  			.len = 2,
>  			.cs_change = 1,
>  			.delay_usecs = adis->data->write_delay,
> +			.cs_change_delay = adis->data->cs_change_delay,
> +			.cs_change_delay_unit = SPI_DELAY_UNIT_USECS,
>  		}, {
>  			.tx_buf = adis->tx + 2,
>  			.bits_per_word = 8,
>  			.len = 2,
>  			.cs_change = 1,
>  			.delay_usecs = adis->data->write_delay,
> +			.cs_change_delay = adis->data->cs_change_delay,
> +			.cs_change_delay_unit = SPI_DELAY_UNIT_USECS,
>  		}, {
>  			.tx_buf = adis->tx + 4,
>  			.bits_per_word = 8,
>  			.len = 2,
>  			.cs_change = 1,
>  			.delay_usecs = adis->data->write_delay,
> +			.cs_change_delay = adis->data->cs_change_delay,
> +			.cs_change_delay_unit = SPI_DELAY_UNIT_USECS,
>  		}, {
>  			.tx_buf = adis->tx + 6,
>  			.bits_per_word = 8,
> @@ -133,12 +139,16 @@ int adis_read_reg(struct adis *adis, unsigned int reg,
>  			.len = 2,
>  			.cs_change = 1,
>  			.delay_usecs = adis->data->write_delay,
> +			.cs_change_delay = adis->data->cs_change_delay,
> +			.cs_change_delay_unit = SPI_DELAY_UNIT_USECS,
>  		}, {
>  			.tx_buf = adis->tx + 2,
>  			.bits_per_word = 8,
>  			.len = 2,
>  			.cs_change = 1,
>  			.delay_usecs = adis->data->read_delay,
> +			.cs_change_delay = adis->data->cs_change_delay,
> +			.cs_change_delay_unit = SPI_DELAY_UNIT_USECS,
>  		}, {
>  			.tx_buf = adis->tx + 4,
>  			.rx_buf = adis->rx,
> @@ -146,6 +156,8 @@ int adis_read_reg(struct adis *adis, unsigned int reg,
>  			.len = 2,
>  			.cs_change = 1,
>  			.delay_usecs = adis->data->read_delay,
> +			.cs_change_delay = adis->data->cs_change_delay,
> +			.cs_change_delay_unit = SPI_DELAY_UNIT_USECS,
>  		}, {
>  			.rx_buf = adis->rx + 2,
>  			.bits_per_word = 8,
> diff --git a/include/linux/iio/imu/adis.h b/include/linux/iio/imu/adis.h
> index 3428d06b2f44..4c53815bb729 100644
> --- a/include/linux/iio/imu/adis.h
> +++ b/include/linux/iio/imu/adis.h
> @@ -26,6 +26,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_change_delay: SPI delay between CS changes 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
> @@ -35,6 +36,7 @@ struct adis_burst;
>  struct adis_data {
>  	unsigned int read_delay;
>  	unsigned int write_delay;
> +	unsigned int cs_change_delay;
>  
>  	unsigned int glob_cmd_reg;
>  	unsigned int msc_ctrl_reg;


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 2/3][V4] iio: imu: Add support for the ADIS16460 IMU
  2019-07-23  7:36 ` [PATCH 2/3][V4] iio: imu: Add support for the ADIS16460 IMU Alexandru Ardelean
@ 2019-07-27 18:54   ` Jonathan Cameron
  0 siblings, 0 replies; 9+ messages in thread
From: Jonathan Cameron @ 2019-07-27 18:54 UTC (permalink / raw)
  To: Alexandru Ardelean
  Cc: linux-iio, linux-spi, devicetree, linux-kernel, robh+dt,
	mark.rutland, broonie, Dragos Bogdan, Michael Hennerich

On Tue, 23 Jul 2019 10:36:39 +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 delay between CS changes (i.e. when CS goes up, the spec recommends
> that it be brought down after a minimum of 16 uS).
> Other ADIS chips require (via spec) a minimum of 2 uS between CS changes.
> The kernel's 10 uS default should be fine for those other chips; they
> haven't been tested with lower CS change delays yet.
> 
> 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>
Applied to the togreg branch of iio.git and pushed out as testing.

Note there was a typo / variable naming inconsistency that I fixed up.
See inline.

Thanks,

Jonathan

> ---
>  MAINTAINERS                 |   7 +
>  drivers/iio/imu/Kconfig     |  12 +
>  drivers/iio/imu/Makefile    |   1 +
>  drivers/iio/imu/adis16460.c | 489 ++++++++++++++++++++++++++++++++++++
>  4 files changed, 509 insertions(+)
>  create mode 100644 drivers/iio/imu/adis16460.c
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 783569e3c4b4..f7de89e82e35 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -945,6 +945,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 4957e6df447e..f3c7282321a8 100644
> --- a/drivers/iio/imu/Kconfig
> +++ b/drivers/iio/imu/Kconfig
> @@ -17,6 +17,18 @@ 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.
> +
> +	  To compile this driver as a module, choose M here: the module will be
> +	  called adis16460.
> +
>  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..db713cba75a2
> --- /dev/null
> +++ b/drivers/iio/imu/adis16460.c
> @@ -0,0 +1,489 @@
> +// 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		0x52
> +#define ADIS16460_REG_LOT_ID2		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 int 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_change_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[] = {
this is plural, it's used below as singular
*_id;

I've fixed it up as ids everywhere.

Please check the result.

Jonathan

> +	{ "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");


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 3/3][V4] dt-bindings: iio: imu: add bindings for ADIS16460
  2019-07-23  7:36 ` [PATCH 3/3][V4] dt-bindings: iio: imu: add bindings for ADIS16460 Alexandru Ardelean
@ 2019-07-27 18:56   ` Jonathan Cameron
  2019-07-29 23:24     ` Rob Herring
  0 siblings, 1 reply; 9+ messages in thread
From: Jonathan Cameron @ 2019-07-27 18:56 UTC (permalink / raw)
  To: Alexandru Ardelean
  Cc: linux-iio, linux-spi, devicetree, linux-kernel, robh+dt,
	mark.rutland, broonie, Rob Herring

On Tue, 23 Jul 2019 10:36:40 +0300
Alexandru Ardelean <alexandru.ardelean@analog.com> wrote:

> This change adds device-tree bindings for the ADIS16460.
> 
> Reviewed-by: Rob Herring <robh@kernel.org>
> Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>

Really trivial, but convention (as driven by what git am -s does if nothing
else, is to add extra tags in chronological order.  So Rob would be after
you.  I tweaked it which I don't always remember to do.

It's not consistent across the kernel but I'll fight for my little corner
to be :)

Applied.

Thanks,

Jonathan

> ---
>  .../bindings/iio/imu/adi,adis16460.yaml       | 53 +++++++++++++++++++
>  MAINTAINERS                                   |  1 +
>  2 files changed, 54 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/iio/imu/adi,adis16460.yaml
> 
> diff --git a/Documentation/devicetree/bindings/iio/imu/adi,adis16460.yaml b/Documentation/devicetree/bindings/iio/imu/adi,adis16460.yaml
> new file mode 100644
> index 000000000000..0c53009ba7d6
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/imu/adi,adis16460.yaml
> @@ -0,0 +1,53 @@
> +# SPDX-License-Identifier: GPL-2.0
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/iio/imu/adi,adis16460.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Analog Devices ADIS16460 and similar IMUs
> +
> +maintainers:
> +  - Dragos Bogdan <dragos.bogdan@analog.com>
> +
> +description: |
> +  Analog Devices ADIS16460 and similar IMUs
> +  https://www.analog.com/media/en/technical-documentation/data-sheets/ADIS16460.pdf
> +
> +properties:
> +  compatible:
> +    enum:
> +      - adi,adis16460
> +
> +  reg:
> +    maxItems: 1
> +
> +  spi-cpha: true
> +
> +  spi-cpol: true
> +
> +  interrupts:
> +    maxItems: 1
> +
> +required:
> +  - compatible
> +  - reg
> +  - interrupts
> +
> +examples:
> +  - |
> +    #include <dt-bindings/gpio/gpio.h>
> +    #include <dt-bindings/interrupt-controller/irq.h>
> +    spi0 {
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +
> +        imu@0 {
> +            compatible = "adi,adis16460";
> +            reg = <0>;
> +            spi-max-frequency = <5000000>;
> +            spi-cpol;
> +            spi-cpha;
> +            interrupt-parent = <&gpio0>;
> +            interrupts = <0 IRQ_TYPE_LEVEL_HIGH>;
> +        };
> +    };
> diff --git a/MAINTAINERS b/MAINTAINERS
> index f7de89e82e35..07105e43ea1e 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -951,6 +951,7 @@ S:	Supported
>  L:	linux-iio@vger.kernel.org
>  W:	http://ez.analog.com/community/linux-device-drivers
>  F:	drivers/iio/imu/adis16460.c
> +F:	Documentation/devicetree/bindings/iio/imu/adi,adis16460.yaml
>  
>  ANALOG DEVICES INC ADP5061 DRIVER
>  M:	Stefan Popa <stefan.popa@analog.com>


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 3/3][V4] dt-bindings: iio: imu: add bindings for ADIS16460
  2019-07-27 18:56   ` Jonathan Cameron
@ 2019-07-29 23:24     ` Rob Herring
  2019-08-01 12:41       ` Jonathan Cameron
  0 siblings, 1 reply; 9+ messages in thread
From: Rob Herring @ 2019-07-29 23:24 UTC (permalink / raw)
  To: Jonathan Cameron
  Cc: Alexandru Ardelean, open list:IIO SUBSYSTEM AND DRIVERS,
	linux-spi, devicetree, linux-kernel, Mark Rutland, Mark Brown

On Sat, Jul 27, 2019 at 12:56 PM Jonathan Cameron <jic23@kernel.org> wrote:
>
> On Tue, 23 Jul 2019 10:36:40 +0300
> Alexandru Ardelean <alexandru.ardelean@analog.com> wrote:
>
> > This change adds device-tree bindings for the ADIS16460.
> >
> > Reviewed-by: Rob Herring <robh@kernel.org>
> > Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
>
> Really trivial, but convention (as driven by what git am -s does if nothing
> else, is to add extra tags in chronological order.  So Rob would be after
> you.  I tweaked it which I don't always remember to do.

I'd argue it is in chronological order as the submitter added my tag
and then sent it out. If you applied it and added my tag, then it
would be after (but before yours).

> It's not consistent across the kernel but I'll fight for my little corner
> to be :)

More consistency would be nice then there's less tribal knowledge
about maintainers for submitters to learn.

Rob

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 3/3][V4] dt-bindings: iio: imu: add bindings for ADIS16460
  2019-07-29 23:24     ` Rob Herring
@ 2019-08-01 12:41       ` Jonathan Cameron
  0 siblings, 0 replies; 9+ messages in thread
From: Jonathan Cameron @ 2019-08-01 12:41 UTC (permalink / raw)
  To: Rob Herring
  Cc: Jonathan Cameron, Alexandru Ardelean,
	open list:IIO SUBSYSTEM AND DRIVERS, linux-spi, devicetree,
	linux-kernel, Mark Rutland, Mark Brown

On Mon, 29 Jul 2019 17:24:40 -0600
Rob Herring <robh+dt@kernel.org> wrote:

> On Sat, Jul 27, 2019 at 12:56 PM Jonathan Cameron <jic23@kernel.org> wrote:
> >
> > On Tue, 23 Jul 2019 10:36:40 +0300
> > Alexandru Ardelean <alexandru.ardelean@analog.com> wrote:
> >  
> > > This change adds device-tree bindings for the ADIS16460.
> > >
> > > Reviewed-by: Rob Herring <robh@kernel.org>
> > > Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>  
> >
> > Really trivial, but convention (as driven by what git am -s does if nothing
> > else, is to add extra tags in chronological order.  So Rob would be after
> > you.  I tweaked it which I don't always remember to do.  
> 
> I'd argue it is in chronological order as the submitter added my tag
> and then sent it out. If you applied it and added my tag, then it
> would be after (but before yours).

Bike shedding to follow...

Possibly but given you gave the Reviewed-by for v2, and it hasn't changed
is Alex's the same Signed-off-by as seen on V2, or a new one reflecting the
addition of your Reviewed-by? 

:)


> 
> > It's not consistent across the kernel but I'll fight for my little corner
> > to be :)  
> 
> More consistency would be nice then there's less tribal knowledge
> about maintainers for submitters to learn.

Agreed.

Jonathan

> 
> Rob



^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2019-08-01 12:41 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-07-23  7:36 [PATCH 0/3][V4] iio: imu: Add support for the ADIS16460 IMU Alexandru Ardelean
2019-07-23  7:36 ` [PATCH 1/3][V4] iio: imu: adis: Add support for SPI transfer cs_change_delay Alexandru Ardelean
2019-07-27 18:48   ` Jonathan Cameron
2019-07-23  7:36 ` [PATCH 2/3][V4] iio: imu: Add support for the ADIS16460 IMU Alexandru Ardelean
2019-07-27 18:54   ` Jonathan Cameron
2019-07-23  7:36 ` [PATCH 3/3][V4] dt-bindings: iio: imu: add bindings for ADIS16460 Alexandru Ardelean
2019-07-27 18:56   ` Jonathan Cameron
2019-07-29 23:24     ` Rob Herring
2019-08-01 12:41       ` Jonathan Cameron

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).