All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/4][V2] iio: imu: Add support for the ADIS16460 IMU
@ 2019-07-17 11:51 ` Alexandru Ardelean
  0 siblings, 0 replies; 13+ messages in thread
From: Alexandru Ardelean @ 2019-07-17 11:51 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.

Support for this chip, requires changes in both IIO & SPI, in order to
support configurable/longer CS change delays.

The default CS change delay is 10 uS, while the ADIS16460 requires a
minimum of 16 uS. In order to accomodate this, the SPI transfer struct
requires a `cs_change_delay_usecs` parameter that is used when `cs_change`
is set.

The ADIS library also requires a small update to support the new SPI
`cs_change_delay_usecs`, and after that, support for ADIS16460 is added,
since all the required parts for operating the chip are in the kernel.

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 (4):
  drivers: spi: core: Add optional stall delay between cs_change
    transfers
  iio: imu: adis: Add support for SPI transfer cs_change_stall_delay_us
  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                        |   6 +
 drivers/iio/imu/adis16460.c                   | 489 ++++++++++++++++++
 drivers/spi/spi.c                             |   3 +-
 include/linux/iio/imu/adis.h                  |   2 +
 include/linux/spi/spi.h                       |   3 +
 9 files changed, 576 insertions(+), 1 deletion(-)
 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] 13+ messages in thread

* [PATCH 0/4][V2] iio: imu: Add support for the ADIS16460 IMU
@ 2019-07-17 11:51 ` Alexandru Ardelean
  0 siblings, 0 replies; 13+ messages in thread
From: Alexandru Ardelean @ 2019-07-17 11:51 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.

Support for this chip, requires changes in both IIO & SPI, in order to
support configurable/longer CS change delays.

The default CS change delay is 10 uS, while the ADIS16460 requires a
minimum of 16 uS. In order to accomodate this, the SPI transfer struct
requires a `cs_change_delay_usecs` parameter that is used when `cs_change`
is set.

The ADIS library also requires a small update to support the new SPI
`cs_change_delay_usecs`, and after that, support for ADIS16460 is added,
since all the required parts for operating the chip are in the kernel.

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 (4):
  drivers: spi: core: Add optional stall delay between cs_change
    transfers
  iio: imu: adis: Add support for SPI transfer cs_change_stall_delay_us
  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                        |   6 +
 drivers/iio/imu/adis16460.c                   | 489 ++++++++++++++++++
 drivers/spi/spi.c                             |   3 +-
 include/linux/iio/imu/adis.h                  |   2 +
 include/linux/spi/spi.h                       |   3 +
 9 files changed, 576 insertions(+), 1 deletion(-)
 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] 13+ messages in thread

* [PATCH 1/4][V2] drivers: spi: core: Add optional delay between cs_change transfers
  2019-07-17 11:51 ` Alexandru Ardelean
@ 2019-07-17 11:51   ` Alexandru Ardelean
  -1 siblings, 0 replies; 13+ messages in thread
From: Alexandru Ardelean @ 2019-07-17 11:51 UTC (permalink / raw)
  To: linux-iio, linux-spi, devicetree, linux-kernel
  Cc: jic23, robh+dt, mark.rutland, broonie, Alexandru Ardelean,
	Michael Hennerich

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..02fd00bcaace 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_delay_usecs ?
+				       xfer->cs_change_delay_usecs : 10);
 				spi_set_cs(msg->spi, true);
 			}
 		}
diff --git a/include/linux/spi/spi.h b/include/linux/spi/spi.h
index 053abd22ad31..c884b3b94841 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_delay_usecs: 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_delay_usecs;
 	u16		delay_usecs;
 	u32		speed_hz;
 	u16		word_delay;
-- 
2.20.1


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

* [PATCH 1/4][V2] drivers: spi: core: Add optional delay between cs_change transfers
@ 2019-07-17 11:51   ` Alexandru Ardelean
  0 siblings, 0 replies; 13+ messages in thread
From: Alexandru Ardelean @ 2019-07-17 11:51 UTC (permalink / raw)
  To: linux-iio, linux-spi, devicetree, linux-kernel
  Cc: jic23, robh+dt, mark.rutland, broonie, Alexandru Ardelean,
	Michael Hennerich

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..02fd00bcaace 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_delay_usecs ?
+				       xfer->cs_change_delay_usecs : 10);
 				spi_set_cs(msg->spi, true);
 			}
 		}
diff --git a/include/linux/spi/spi.h b/include/linux/spi/spi.h
index 053abd22ad31..c884b3b94841 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_delay_usecs: 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_delay_usecs;
 	u16		delay_usecs;
 	u32		speed_hz;
 	u16		word_delay;
-- 
2.20.1

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

* [PATCH 2/4][V2] iio: imu: adis: Add support for SPI transfer cs_change_delay_usecs
  2019-07-17 11:51 ` Alexandru Ardelean
@ 2019-07-17 11:51   ` Alexandru Ardelean
  -1 siblings, 0 replies; 13+ messages in thread
From: Alexandru Ardelean @ 2019-07-17 11:51 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_stall_delay_us` functionality in SPI.

Not all transfers set `cs_change` to 1. Only those that do, have the
`cs_change_delay` assigned.

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

diff --git a/drivers/iio/imu/adis.c b/drivers/iio/imu/adis.c
index c771ae6803a9..6fdb6f4cebd4 100644
--- a/drivers/iio/imu/adis.c
+++ b/drivers/iio/imu/adis.c
@@ -40,18 +40,21 @@ int adis_write_reg(struct adis *adis, unsigned int reg,
 			.len = 2,
 			.cs_change = 1,
 			.delay_usecs = adis->data->write_delay,
+			.cs_change_delay_usecs = adis->data->cs_change_delay,
 		}, {
 			.tx_buf = adis->tx + 2,
 			.bits_per_word = 8,
 			.len = 2,
 			.cs_change = 1,
 			.delay_usecs = adis->data->write_delay,
+			.cs_change_delay_usecs = adis->data->cs_change_delay,
 		}, {
 			.tx_buf = adis->tx + 4,
 			.bits_per_word = 8,
 			.len = 2,
 			.cs_change = 1,
 			.delay_usecs = adis->data->write_delay,
+			.cs_change_delay_usecs = adis->data->cs_change_delay,
 		}, {
 			.tx_buf = adis->tx + 6,
 			.bits_per_word = 8,
@@ -134,12 +137,14 @@ int adis_read_reg(struct adis *adis, unsigned int reg,
 			.len = 2,
 			.cs_change = 1,
 			.delay_usecs = adis->data->write_delay,
+			.cs_change_delay_usecs = adis->data->cs_change_delay,
 		}, {
 			.tx_buf = adis->tx + 2,
 			.bits_per_word = 8,
 			.len = 2,
 			.cs_change = 1,
 			.delay_usecs = adis->data->read_delay,
+			.cs_change_delay_usecs = adis->data->cs_change_delay,
 		}, {
 			.tx_buf = adis->tx + 4,
 			.rx_buf = adis->rx,
@@ -147,6 +152,7 @@ int adis_read_reg(struct adis *adis, unsigned int reg,
 			.len = 2,
 			.cs_change = 1,
 			.delay_usecs = adis->data->read_delay,
+			.cs_change_delay_usecs = adis->data->cs_change_delay,
 		}, {
 			.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 469a493f7ae0..fd884b45ed45 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_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
@@ -36,6 +37,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] 13+ messages in thread

* [PATCH 2/4][V2] iio: imu: adis: Add support for SPI transfer cs_change_delay_usecs
@ 2019-07-17 11:51   ` Alexandru Ardelean
  0 siblings, 0 replies; 13+ messages in thread
From: Alexandru Ardelean @ 2019-07-17 11:51 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_stall_delay_us` functionality in SPI.

Not all transfers set `cs_change` to 1. Only those that do, have the
`cs_change_delay` assigned.

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

diff --git a/drivers/iio/imu/adis.c b/drivers/iio/imu/adis.c
index c771ae6803a9..6fdb6f4cebd4 100644
--- a/drivers/iio/imu/adis.c
+++ b/drivers/iio/imu/adis.c
@@ -40,18 +40,21 @@ int adis_write_reg(struct adis *adis, unsigned int reg,
 			.len = 2,
 			.cs_change = 1,
 			.delay_usecs = adis->data->write_delay,
+			.cs_change_delay_usecs = adis->data->cs_change_delay,
 		}, {
 			.tx_buf = adis->tx + 2,
 			.bits_per_word = 8,
 			.len = 2,
 			.cs_change = 1,
 			.delay_usecs = adis->data->write_delay,
+			.cs_change_delay_usecs = adis->data->cs_change_delay,
 		}, {
 			.tx_buf = adis->tx + 4,
 			.bits_per_word = 8,
 			.len = 2,
 			.cs_change = 1,
 			.delay_usecs = adis->data->write_delay,
+			.cs_change_delay_usecs = adis->data->cs_change_delay,
 		}, {
 			.tx_buf = adis->tx + 6,
 			.bits_per_word = 8,
@@ -134,12 +137,14 @@ int adis_read_reg(struct adis *adis, unsigned int reg,
 			.len = 2,
 			.cs_change = 1,
 			.delay_usecs = adis->data->write_delay,
+			.cs_change_delay_usecs = adis->data->cs_change_delay,
 		}, {
 			.tx_buf = adis->tx + 2,
 			.bits_per_word = 8,
 			.len = 2,
 			.cs_change = 1,
 			.delay_usecs = adis->data->read_delay,
+			.cs_change_delay_usecs = adis->data->cs_change_delay,
 		}, {
 			.tx_buf = adis->tx + 4,
 			.rx_buf = adis->rx,
@@ -147,6 +152,7 @@ int adis_read_reg(struct adis *adis, unsigned int reg,
 			.len = 2,
 			.cs_change = 1,
 			.delay_usecs = adis->data->read_delay,
+			.cs_change_delay_usecs = adis->data->cs_change_delay,
 		}, {
 			.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 469a493f7ae0..fd884b45ed45 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_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
@@ -36,6 +37,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] 13+ messages in thread

* [PATCH 3/4][V2] iio: imu: Add support for the ADIS16460 IMU
  2019-07-17 11:51 ` Alexandru Ardelean
@ 2019-07-17 11:51   ` Alexandru Ardelean
  -1 siblings, 0 replies; 13+ messages in thread
From: Alexandru Ardelean @ 2019-07-17 11:51 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 ad498428b38c..8e679504c087 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..f048d0d757b1 100644
--- a/drivers/iio/imu/Kconfig
+++ b/drivers/iio/imu/Kconfig
@@ -16,6 +16,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] 13+ messages in thread

* [PATCH 3/4][V2] iio: imu: Add support for the ADIS16460 IMU
@ 2019-07-17 11:51   ` Alexandru Ardelean
  0 siblings, 0 replies; 13+ messages in thread
From: Alexandru Ardelean @ 2019-07-17 11:51 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 ad498428b38c..8e679504c087 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..f048d0d757b1 100644
--- a/drivers/iio/imu/Kconfig
+++ b/drivers/iio/imu/Kconfig
@@ -16,6 +16,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] 13+ messages in thread

* [PATCH 4/4][V2] dt-bindings: iio: imu: add bindings for ADIS16460
  2019-07-17 11:51 ` Alexandru Ardelean
@ 2019-07-17 11:51   ` Alexandru Ardelean
  -1 siblings, 0 replies; 13+ messages in thread
From: Alexandru Ardelean @ 2019-07-17 11:51 UTC (permalink / raw)
  To: linux-iio, linux-spi, devicetree, linux-kernel
  Cc: jic23, robh+dt, mark.rutland, broonie, Alexandru Ardelean

This change adds device-tree bindings for the ADIS16460.

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 8e679504c087..c44fbe8e91e9 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -943,6 +943,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] 13+ messages in thread

* [PATCH 4/4][V2] dt-bindings: iio: imu: add bindings for ADIS16460
@ 2019-07-17 11:51   ` Alexandru Ardelean
  0 siblings, 0 replies; 13+ messages in thread
From: Alexandru Ardelean @ 2019-07-17 11:51 UTC (permalink / raw)
  To: linux-iio, linux-spi, devicetree, linux-kernel
  Cc: jic23, robh+dt, mark.rutland, broonie, Alexandru Ardelean

This change adds device-tree bindings for the ADIS16460.

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 8e679504c087..c44fbe8e91e9 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -943,6 +943,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] 13+ messages in thread

* Re: [PATCH 1/4][V2] drivers: spi: core: Add optional delay between cs_change transfers
  2019-07-17 11:51   ` Alexandru Ardelean
  (?)
@ 2019-07-18 12:50   ` Mark Brown
  2019-07-18 13:36     ` Ardelean, Alexandru
  -1 siblings, 1 reply; 13+ messages in thread
From: Mark Brown @ 2019-07-18 12:50 UTC (permalink / raw)
  To: Alexandru Ardelean
  Cc: linux-iio, linux-spi, devicetree, linux-kernel, jic23, robh+dt,
	mark.rutland, Michael Hennerich

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

On Wed, Jul 17, 2019 at 02:51:06PM +0300, Alexandru Ardelean 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.

To repeat my previous feedback:

| This looks like cs_change_delay.

Please use subject lines matching the style for the subsystem.  This
makes it easier for people to identify relevant patches.

Please don't ignore review comments, people are generally making them
for a reason and are likely to have the same concerns if issues remain
unaddressed.  Having to repeat the same comments can get repetitive and
make people question the value of time spent reviewing.  If you disagree
with the review comments that's fine but you need to reply and discuss
your concerns so that the reviewer can understand your decisions.

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

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

* Re: [PATCH 1/4][V2] drivers: spi: core: Add optional delay between cs_change transfers
  2019-07-18 12:50   ` Mark Brown
@ 2019-07-18 13:36     ` Ardelean, Alexandru
  0 siblings, 0 replies; 13+ messages in thread
From: Ardelean, Alexandru @ 2019-07-18 13:36 UTC (permalink / raw)
  To: broonie
  Cc: linux-spi, linux-iio, mark.rutland, devicetree, jic23,
	linux-kernel, Hennerich, Michael, robh+dt

On Thu, 2019-07-18 at 13:50 +0100, Mark Brown wrote:
> On Wed, Jul 17, 2019 at 02:51:06PM +0300, Alexandru Ardelean 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.
> 
> To repeat my previous feedback:
> 
> > This looks like cs_change_delay.

Ack.
Will use `cs_change_delay`.
I have no strong preference regarding the name.

> 
> Please use subject lines matching the style for the subsystem.  This
> makes it easier for people to identify relevant patches.

Ack.
Will look for SPI subsystem specific subject lines and use them.

> 
> Please don't ignore review comments, people are generally making them
> for a reason and are likely to have the same concerns if issues remain
> unaddressed.  Having to repeat the same comments can get repetitive and
> make people question the value of time spent reviewing.  If you disagree
> with the review comments that's fine but you need to reply and discuss
> your concerns so that the reviewer can understand your decisions.

[ the following part should not be considered in a disrespectful tone ; the intent is nowhere near that, but text-
communication has a design-flaw where a disrespectful tone may be interpreted [where there isn't one] ]

My intent wasn't to ignore the review comment.
Sorry if it came out like that.

I assumed a patch re-spin was preferred vs a verbal discussion.
Some people prefer patch re-spins as a basis for discussion.
Various people have various preferences.

Also, I wasn't sure how soon I'd get a reply back on this, since various people/maintainers have various reply-times.
And I also [sometimes] have longer reply-back-times [which doesn't help either].
And I wasn't sure if `cs_change_delay` was fully intentional/ad-literam, or whether it was a feedback of the sorts
"along-the-lines of `cs_change_delay`".

While looking at `struct spi_transfer` the other "delay" fields seem to be: `word_delay_usecs` & `delay_usecs`, which is
why I assumed `cs_change_delay_usecs` was preferred [though I will admit, it is a long var-name].

And the conclusion [from my side] is: maybe I rushed this a bit and maybe it annoyed you.
Not my intention, and it'll take me a bit to adjust to your style.

Moving forward.

1. I will use `cs_change_delay` as field name 
2. I will use SPI subsystem subject line; I will admit I forget this stuff periodically

Is there anything else I should consider?
Or anything else to discuss?

I'm open to elements I may have forgotten/omitted unintentionally.

Thanks
Alex

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

* Re: [PATCH 4/4][V2] dt-bindings: iio: imu: add bindings for ADIS16460
  2019-07-17 11:51   ` Alexandru Ardelean
  (?)
@ 2019-07-19 22:49   ` Rob Herring
  -1 siblings, 0 replies; 13+ messages in thread
From: Rob Herring @ 2019-07-19 22:49 UTC (permalink / raw)
  To: Alexandru Ardelean
  Cc: open list:IIO SUBSYSTEM AND DRIVERS, linux-spi, devicetree,
	linux-kernel, Jonathan Cameron, Mark Rutland, Mark Brown

On Wed, Jul 17, 2019 at 5:51 AM Alexandru Ardelean
<alexandru.ardelean@analog.com> wrote:
>
> This change adds device-tree bindings for the ADIS16460.
>
> 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

Reviewed-by: Rob Herring <robh@kernel.org>

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

end of thread, other threads:[~2019-07-19 22:49 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-07-17 11:51 [PATCH 0/4][V2] iio: imu: Add support for the ADIS16460 IMU Alexandru Ardelean
2019-07-17 11:51 ` Alexandru Ardelean
2019-07-17 11:51 ` [PATCH 1/4][V2] drivers: spi: core: Add optional delay between cs_change transfers Alexandru Ardelean
2019-07-17 11:51   ` Alexandru Ardelean
2019-07-18 12:50   ` Mark Brown
2019-07-18 13:36     ` Ardelean, Alexandru
2019-07-17 11:51 ` [PATCH 2/4][V2] iio: imu: adis: Add support for SPI transfer cs_change_delay_usecs Alexandru Ardelean
2019-07-17 11:51   ` Alexandru Ardelean
2019-07-17 11:51 ` [PATCH 3/4][V2] iio: imu: Add support for the ADIS16460 IMU Alexandru Ardelean
2019-07-17 11:51   ` Alexandru Ardelean
2019-07-17 11:51 ` [PATCH 4/4][V2] dt-bindings: iio: imu: add bindings for ADIS16460 Alexandru Ardelean
2019-07-17 11:51   ` Alexandru Ardelean
2019-07-19 22:49   ` Rob Herring

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.