* [PATCH v4 1/8] iio: imu: adis: Add optional self_test callback
2026-10-23 9:30 [PATCH v4 0/8] iio: imu: Add support for the ADI ADIS16607 Radu Sabau via B4 Relay
@ 2026-10-23 9:30 ` Radu Sabau via B4 Relay
2026-10-23 9:30 ` [PATCH v4 2/8] iio: imu: adis: Fix custom reset dispatch in adislib Radu Sabau via B4 Relay
` (6 subsequent siblings)
7 siblings, 0 replies; 19+ messages in thread
From: Radu Sabau via B4 Relay @ 2026-10-23 9:30 UTC (permalink / raw)
To: Lars-Peter Clausen, Michael Hennerich, Nuno Sa, Jonathan Cameron,
David Lechner, Andy Shevchenko, Alexandru Ardelean, Radu Sabau,
Rob Herring, Krzysztof Kozlowski, Conor Dooley
Cc: linux-iio, linux-kernel, devicetree, linux, Jonathan Cameron
From: Radu Sabau <radu.sabau@analog.com>
Add an optional self_test callback to struct adis_data, for devices
whose self-test doesn't fit the generic mask-write/sleep/check-status
sequence. Purely additive: defaults to NULL, no change for existing
drivers.
Signed-off-by: Radu Sabau <radu.sabau@analog.com>
---
drivers/iio/imu/adis.c | 3 +++
include/linux/iio/imu/adis.h | 7 +++++++
2 files changed, 10 insertions(+)
diff --git a/drivers/iio/imu/adis.c b/drivers/iio/imu/adis.c
index a2bc1d14ed91..388518ebba2e 100644
--- a/drivers/iio/imu/adis.c
+++ b/drivers/iio/imu/adis.c
@@ -360,6 +360,9 @@ static int adis_self_test(struct adis *adis)
int ret;
const struct adis_timeout *timeouts = adis->data->timeouts;
+ if (adis->data->self_test)
+ return adis->data->self_test(adis);
+
ret = __adis_write_reg_16(adis, adis->data->self_test_reg,
adis->data->self_test_mask);
if (ret) {
diff --git a/include/linux/iio/imu/adis.h b/include/linux/iio/imu/adis.h
index bfb6df68e6c9..14f9b304c252 100644
--- a/include/linux/iio/imu/adis.h
+++ b/include/linux/iio/imu/adis.h
@@ -51,6 +51,12 @@ struct adis_timeout {
* @self_test_mask: Bitmask of supported self-test operations
* @self_test_reg: Register address to request self test command
* @self_test_no_autoclear: True if device's self-test needs clear of ctrl reg
+ * @self_test: Optional device-specific self-test override. When set,
+ * it is called instead of the generic mask-write/sleep/
+ * check-status sequence. The callback owns its own pass/fail
+ * determination entirely (it should call __adis_check_status()
+ * itself if it wants status-register checks). Must return 0 on
+ * success, a negative error code otherwise.
* @status_error_msgs: Array of error messages
* @status_error_mask: Bitmask of errors supported by the device
* @timeouts: Chip specific delays
@@ -81,6 +87,7 @@ struct adis_data {
unsigned int self_test_mask;
unsigned int self_test_reg;
bool self_test_no_autoclear;
+ int (*self_test)(struct adis *adis);
const struct adis_timeout *timeouts;
const char * const *status_error_msgs;
--
2.43.0
^ permalink raw reply [flat|nested] 19+ messages in thread* [PATCH v4 2/8] iio: imu: adis: Fix custom reset dispatch in adislib
2026-10-23 9:30 [PATCH v4 0/8] iio: imu: Add support for the ADI ADIS16607 Radu Sabau via B4 Relay
2026-10-23 9:30 ` [PATCH v4 1/8] iio: imu: adis: Add optional self_test callback Radu Sabau via B4 Relay
@ 2026-10-23 9:30 ` Radu Sabau via B4 Relay
2026-10-23 9:30 ` [PATCH v4 3/8] iio: imu: adis: Add optional post_reset_setup callback Radu Sabau via B4 Relay
` (5 subsequent siblings)
7 siblings, 0 replies; 19+ messages in thread
From: Radu Sabau via B4 Relay @ 2026-10-23 9:30 UTC (permalink / raw)
To: Lars-Peter Clausen, Michael Hennerich, Nuno Sa, Jonathan Cameron,
David Lechner, Andy Shevchenko, Alexandru Ardelean, Radu Sabau,
Rob Herring, Krzysztof Kozlowski, Conor Dooley
Cc: linux-iio, linux-kernel, devicetree, linux, Jonathan Cameron
From: Radu Sabau <radu.sabau@analog.com>
__adis_initial_startup()'s no-reset-gpio path called __adis_reset()
directly instead of adis->ops->reset(), silently ignoring any driver's
custom reset implementation. Invisible until now since
adis_default_ops.reset is __adis_reset() itself. Dispatch through
adis->ops->reset() instead - the same bug existed in the public
adis_reset() wrapper, fixed here too.
Move the settle-time sleep out of __adis_reset() and into both call
sites instead, so every reset op (default or custom) is treated
uniformly: adis->ops->reset() is expected to just do the write, and
the library always sleeps the settle time afterwards. This removes
the previous asymmetric "sleep here only for custom ops" special
case entirely.
Fixes: ecb010d44108 ("iio: imu: adis: Refactor adis_initial_startup")
Signed-off-by: Radu Sabau <radu.sabau@analog.com>
---
drivers/iio/imu/adis.c | 6 ++----
include/linux/iio/imu/adis.h | 12 +++++++++++-
2 files changed, 13 insertions(+), 5 deletions(-)
diff --git a/drivers/iio/imu/adis.c b/drivers/iio/imu/adis.c
index 388518ebba2e..5c3bb7259037 100644
--- a/drivers/iio/imu/adis.c
+++ b/drivers/iio/imu/adis.c
@@ -340,7 +340,6 @@ EXPORT_SYMBOL_NS_GPL(__adis_check_status, "IIO_ADISLIB");
int __adis_reset(struct adis *adis)
{
int ret;
- const struct adis_timeout *timeouts = adis->data->timeouts;
ret = __adis_write_reg_8(adis, adis->data->glob_cmd_reg,
ADIS_GLOB_CMD_SW_RESET);
@@ -349,8 +348,6 @@ int __adis_reset(struct adis *adis)
return ret;
}
- msleep(timeouts->sw_reset_ms);
-
return 0;
}
EXPORT_SYMBOL_NS_GPL(__adis_reset, "IIO_ADIS_LIB");
@@ -416,9 +413,10 @@ int __adis_initial_startup(struct adis *adis)
gpiod_set_value_cansleep(gpio, 0);
msleep(timeouts->reset_ms);
} else {
- ret = __adis_reset(adis);
+ ret = adis->ops->reset(adis);
if (ret)
return ret;
+ msleep(timeouts->sw_reset_ms);
}
ret = adis_self_test(adis);
diff --git a/include/linux/iio/imu/adis.h b/include/linux/iio/imu/adis.h
index 14f9b304c252..c09b381eb619 100644
--- a/include/linux/iio/imu/adis.h
+++ b/include/linux/iio/imu/adis.h
@@ -10,6 +10,7 @@
#define __IIO_ADIS_H__
#include <linux/cleanup.h>
+#include <linux/delay.h>
#include <linux/spi/spi.h>
#include <linux/interrupt.h>
#include <linux/iio/iio.h>
@@ -178,8 +179,17 @@ int __adis_reset(struct adis *adis);
*/
static inline int adis_reset(struct adis *adis)
{
+ int ret;
+
guard(mutex)(&adis->state_lock);
- return __adis_reset(adis);
+
+ ret = adis->ops->reset(adis);
+ if (ret)
+ return ret;
+
+ msleep(adis->data->timeouts->sw_reset_ms);
+
+ return 0;
}
int __adis_write_reg(struct adis *adis, unsigned int reg,
--
2.43.0
^ permalink raw reply [flat|nested] 19+ messages in thread* [PATCH v4 3/8] iio: imu: adis: Add optional post_reset_setup callback
2026-10-23 9:30 [PATCH v4 0/8] iio: imu: Add support for the ADI ADIS16607 Radu Sabau via B4 Relay
2026-10-23 9:30 ` [PATCH v4 1/8] iio: imu: adis: Add optional self_test callback Radu Sabau via B4 Relay
2026-10-23 9:30 ` [PATCH v4 2/8] iio: imu: adis: Fix custom reset dispatch in adislib Radu Sabau via B4 Relay
@ 2026-10-23 9:30 ` Radu Sabau via B4 Relay
2026-10-25 23:52 ` Jonathan Cameron
2026-10-23 9:30 ` [PATCH v4 4/8] dt-bindings: iio: imu: Add bindings for ADI ADIS16607 Radu Sabau via B4 Relay
` (4 subsequent siblings)
7 siblings, 1 reply; 19+ messages in thread
From: Radu Sabau via B4 Relay @ 2026-10-23 9:30 UTC (permalink / raw)
To: Lars-Peter Clausen, Michael Hennerich, Nuno Sa, Jonathan Cameron,
David Lechner, Andy Shevchenko, Alexandru Ardelean, Radu Sabau,
Rob Herring, Krzysztof Kozlowski, Conor Dooley
Cc: linux-iio, linux-kernel, devicetree, linux, Jonathan Cameron
From: Radu Sabau <radu.sabau@analog.com>
Add an optional post_reset_setup callback to struct adis_data, called
right after reset and before self-test. Not all ADIS devices need
extra bring-up between reset and self-test (SPI mode locking, status
or bootloader checks, pin or burst-mode configuration, etc.) - purely
additive: defaults to NULL, no change for existing drivers.
Signed-off-by: Radu Sabau <radu.sabau@analog.com>
---
drivers/iio/imu/adis.c | 6 ++++++
include/linux/iio/imu/adis.h | 7 +++++++
2 files changed, 13 insertions(+)
diff --git a/drivers/iio/imu/adis.c b/drivers/iio/imu/adis.c
index 5c3bb7259037..332cc7b0ff35 100644
--- a/drivers/iio/imu/adis.c
+++ b/drivers/iio/imu/adis.c
@@ -419,6 +419,12 @@ int __adis_initial_startup(struct adis *adis)
msleep(timeouts->sw_reset_ms);
}
+ if (adis->data->post_reset_setup) {
+ ret = adis->data->post_reset_setup(adis);
+ if (ret)
+ return ret;
+ }
+
ret = adis_self_test(adis);
if (ret)
return ret;
diff --git a/include/linux/iio/imu/adis.h b/include/linux/iio/imu/adis.h
index c09b381eb619..3d7c7febaec6 100644
--- a/include/linux/iio/imu/adis.h
+++ b/include/linux/iio/imu/adis.h
@@ -52,6 +52,12 @@ struct adis_timeout {
* @self_test_mask: Bitmask of supported self-test operations
* @self_test_reg: Register address to request self test command
* @self_test_no_autoclear: True if device's self-test needs clear of ctrl reg
+ * @post_reset_setup: Optional device-specific hook called after reset,
+ * before self-test. Not all ADIS devices need extra bring-up
+ * between reset and self-test (SPI mode locking, status or
+ * bootloader checks, pin or burst-mode configuration, etc.) -
+ * only called when set. Must return 0 on success, a negative
+ * error code otherwise.
* @self_test: Optional device-specific self-test override. When set,
* it is called instead of the generic mask-write/sleep/
* check-status sequence. The callback owns its own pass/fail
@@ -88,6 +94,7 @@ struct adis_data {
unsigned int self_test_mask;
unsigned int self_test_reg;
bool self_test_no_autoclear;
+ int (*post_reset_setup)(struct adis *adis);
int (*self_test)(struct adis *adis);
const struct adis_timeout *timeouts;
--
2.43.0
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: [PATCH v4 3/8] iio: imu: adis: Add optional post_reset_setup callback
2026-10-23 9:30 ` [PATCH v4 3/8] iio: imu: adis: Add optional post_reset_setup callback Radu Sabau via B4 Relay
@ 2026-10-25 23:52 ` Jonathan Cameron
2026-10-26 9:52 ` Sabau, Radu bogdan
0 siblings, 1 reply; 19+ messages in thread
From: Jonathan Cameron @ 2026-10-25 23:52 UTC (permalink / raw)
To: Radu Sabau via B4 Relay
Cc: radu.sabau, Lars-Peter Clausen, Michael Hennerich, Nuno Sa,
David Lechner, Andy Shevchenko, Alexandru Ardelean, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, linux-iio, linux-kernel,
devicetree, linux
On Fri, 23 Oct 2026 12:30:20 +0300
Radu Sabau via B4 Relay <devnull+radu.sabau.analog.com@kernel.org> wrote:
> From: Radu Sabau <radu.sabau@analog.com>
>
> Add an optional post_reset_setup callback to struct adis_data, called
> right after reset and before self-test. Not all ADIS devices need
> extra bring-up between reset and self-test (SPI mode locking, status
> or bootloader checks, pin or burst-mode configuration, etc.) - purely
> additive: defaults to NULL, no change for existing drivers.
>
> Signed-off-by: Radu Sabau <radu.sabau@analog.com>
Hi Radu,
As it seems you will be doing a v5 for other feedback, time to
argue about the bikeshed colour.
> diff --git a/include/linux/iio/imu/adis.h b/include/linux/iio/imu/adis.h
> index c09b381eb619..3d7c7febaec6 100644
> --- a/include/linux/iio/imu/adis.h
> +++ b/include/linux/iio/imu/adis.h
> @@ -52,6 +52,12 @@ struct adis_timeout {
> * @self_test_mask: Bitmask of supported self-test operations
> * @self_test_reg: Register address to request self test command
> * @self_test_no_autoclear: True if device's self-test needs clear of ctrl reg
> + * @post_reset_setup: Optional device-specific hook called after reset,
> + * before self-test. Not all ADIS devices need extra bring-up
> + * between reset and self-test (SPI mode locking, status or
> + * bootloader checks, pin or burst-mode configuration, etc.) -
> + * only called when set. Must return 0 on success, a negative
> + * error code otherwise.
Trivial but I wonder if this naming focuses unnecessarily on bit being
after reset. Maybe simply calling it init() would be enough with
assumption that any intialization is always after reset if there
is one done?
> * @self_test: Optional device-specific self-test override. When set,
> * it is called instead of the generic mask-write/sleep/
> * check-status sequence. The callback owns its own pass/fail
> @@ -88,6 +94,7 @@ struct adis_data {
> unsigned int self_test_mask;
> unsigned int self_test_reg;
> bool self_test_no_autoclear;
> + int (*post_reset_setup)(struct adis *adis);
> int (*self_test)(struct adis *adis);
> const struct adis_timeout *timeouts;
>
>
^ permalink raw reply [flat|nested] 19+ messages in thread* RE: [PATCH v4 3/8] iio: imu: adis: Add optional post_reset_setup callback
2026-10-25 23:52 ` Jonathan Cameron
@ 2026-10-26 9:52 ` Sabau, Radu bogdan
0 siblings, 0 replies; 19+ messages in thread
From: Sabau, Radu bogdan @ 2026-10-26 9:52 UTC (permalink / raw)
To: Jonathan Cameron, Radu Sabau via B4 Relay
Cc: Lars-Peter Clausen, Hennerich, Michael, Sa, Nuno, David Lechner,
Andy Shevchenko, Alexandru Ardelean, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, linux-iio, linux-kernel,
devicetree, linux
> -----Original Message-----
> From: Jonathan Cameron <jic23@kernel.org>
> Sent: Monday, October 26, 2026 1:52 AM
...
> > * @self_test_reg: Register address to request self test command
> > * @self_test_no_autoclear: True if device's self-test needs clear of ctrl reg
> > + * @post_reset_setup: Optional device-specific hook called after reset,
> > + * before self-test. Not all ADIS devices need extra bring-up
> > + * between reset and self-test (SPI mode locking, status or
> > + * bootloader checks, pin or burst-mode configuration, etc.) -
> > + * only called when set. Must return 0 on success, a negative
> > + * error code otherwise.
>
> Trivial but I wonder if this naming focuses unnecessarily on bit being
> after reset. Maybe simply calling it init() would be enough with
> assumption that any intialization is always after reset if there
> is one done?
>
Totally agree - renaming it to init() for v5.
> > * @self_test: Optional device-specific self-test override. When set,
> > * it is called instead of the generic mask-write/sleep/
> > * check-status sequence. The callback owns its own pass/fail
> > @@ -88,6 +94,7 @@ struct adis_data {
> > unsigned int self_test_mask;
> > unsigned int self_test_reg;
> > bool self_test_no_autoclear;
> > + int (*post_reset_setup)(struct adis *adis);
> > int (*self_test)(struct adis *adis);
> > const struct adis_timeout *timeouts;
> >
> >
^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH v4 4/8] dt-bindings: iio: imu: Add bindings for ADI ADIS16607
2026-10-23 9:30 [PATCH v4 0/8] iio: imu: Add support for the ADI ADIS16607 Radu Sabau via B4 Relay
` (2 preceding siblings ...)
2026-10-23 9:30 ` [PATCH v4 3/8] iio: imu: adis: Add optional post_reset_setup callback Radu Sabau via B4 Relay
@ 2026-10-23 9:30 ` Radu Sabau via B4 Relay
2026-10-25 23:54 ` Jonathan Cameron
2026-10-23 9:30 ` [PATCH v4 5/8] iio: imu: Add driver for the " Radu Sabau via B4 Relay
` (3 subsequent siblings)
7 siblings, 1 reply; 19+ messages in thread
From: Radu Sabau via B4 Relay @ 2026-10-23 9:30 UTC (permalink / raw)
To: Lars-Peter Clausen, Michael Hennerich, Nuno Sa, Jonathan Cameron,
David Lechner, Andy Shevchenko, Alexandru Ardelean, Radu Sabau,
Rob Herring, Krzysztof Kozlowski, Conor Dooley
Cc: linux-iio, linux-kernel, devicetree, linux, Jonathan Cameron
From: Radu Sabau <radu.sabau@analog.com>
Add device tree bindings for the Analog Devices ADIS16607 Precision
MEMS Inertial Measurement Unit. The ADIS16607 integrates a triaxial
gyroscope, triaxial accelerometer, and temperature sensor, and
communicates over SPI Half-Duplex.
Two variants are supported, adis16607-2 and adis16607-3, differing
only in gyroscope full-scale range (±450°/s and ±2000°/s
respectively).
The binding exposes optional properties for:
- An external clock for sync mode (falling back to the internal 8 kHz
clock when absent)
- A hardware reset GPIO (active low)
Reviewed-by: Rob Herring (Arm) <robh@kernel.org>
Signed-off-by: Radu Sabau <radu.sabau@analog.com>
---
.../devicetree/bindings/iio/imu/adi,adis16607.yaml | 94 ++++++++++++++++++++++
MAINTAINERS | 7 ++
2 files changed, 101 insertions(+)
diff --git a/Documentation/devicetree/bindings/iio/imu/adi,adis16607.yaml b/Documentation/devicetree/bindings/iio/imu/adi,adis16607.yaml
new file mode 100644
index 000000000000..4545f942365c
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/imu/adi,adis16607.yaml
@@ -0,0 +1,94 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/iio/imu/adi,adis16607.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Analog Devices ADIS16607 IMU
+
+maintainers:
+ - Radu Sabau <radu.sabau@analog.com>
+
+description: |
+ Analog Devices ADIS16607 Precision MEMS Inertial Measurement Unit (IMU)
+ with a triaxial gyroscope, triaxial accelerometer, and temperature sensor.
+ Uses SPI Half-Duplex communication protocol.
+
+ The device has five GPIO pins (GPIO0-GPIO4 in the datasheet), each
+ independently selectable for a single fixed function. This binding
+ exposes GPIO0's reset function as reset-gpios; GPIO2's sync function
+ is modeled via the clocks property, and GPIO4's DEV_NOK interrupt via
+ the interrupts property. GPIO3 (data-ready) is required internally
+ regardless of board wiring and isn't exposed. GPIO1 (wake) gates
+ whether the bootloader auto-loads its NVM contents after reset; it
+ defaults to enabled with a weak pull-up, which already gives normal
+ operation with no configuration needed, so it isn't exposed either.
+
+properties:
+ compatible:
+ enum:
+ - adi,adis16607-2
+ - adi,adis16607-3
+
+ reg:
+ maxItems: 1
+
+ spi-cpha: true
+
+ spi-cpol: true
+
+ spi-max-frequency:
+ maximum: 10000000
+
+ vdd-supply: true
+
+ interrupts:
+ maxItems: 1
+
+ clocks:
+ description:
+ Optional external clock for sync mode. If not provided, the internal
+ 8 kHz clock is used.
+ maxItems: 1
+
+ reset-gpios:
+ description:
+ Optional GPIO for hardware reset, active low. Routes to the
+ device's GPIO0 pin, which can be independently set to the reset
+ function or left idle via an on-chip register.
+ maxItems: 1
+
+required:
+ - compatible
+ - reg
+ - interrupts
+ - spi-cpha
+ - spi-cpol
+ - vdd-supply
+
+allOf:
+ - $ref: /schemas/spi/spi-peripheral-props.yaml#
+
+unevaluatedProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/interrupt-controller/irq.h>
+ #include <dt-bindings/gpio/gpio.h>
+ spi {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ imu@0 {
+ compatible = "adi,adis16607-2";
+ reg = <0>;
+ spi-cpha;
+ spi-cpol;
+ spi-max-frequency = <10000000>;
+ vdd-supply = <&vdd>;
+ interrupts = <4 IRQ_TYPE_LEVEL_HIGH>;
+ interrupt-parent = <&gpio>;
+ clocks = <&sync_clk>;
+ reset-gpios = <&gpio 10 GPIO_ACTIVE_LOW>;
+ };
+ };
diff --git a/MAINTAINERS b/MAINTAINERS
index f53047aa8d3c..d5679e8d0ffd 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1792,6 +1792,13 @@ S: Supported
W: https://ez.analog.com/linux-software-drivers
F: Documentation/devicetree/bindings/iio/imu/adi,adis16550.yaml
+ANALOG DEVICES INC ADIS16607 DRIVER
+M: Radu Sabau <radu.sabau@analog.com>
+L: linux-iio@vger.kernel.org
+S: Supported
+W: https://ez.analog.com/linux-software-drivers
+F: Documentation/devicetree/bindings/iio/imu/adi,adis16607.yaml
+
ANALOG DEVICES INC ADL8113 DRIVER
M: Antoniu Miclaus <antoniu.miclaus@analog.com>
L: linux-iio@vger.kernel.org
--
2.43.0
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: [PATCH v4 4/8] dt-bindings: iio: imu: Add bindings for ADI ADIS16607
2026-10-23 9:30 ` [PATCH v4 4/8] dt-bindings: iio: imu: Add bindings for ADI ADIS16607 Radu Sabau via B4 Relay
@ 2026-10-25 23:54 ` Jonathan Cameron
0 siblings, 0 replies; 19+ messages in thread
From: Jonathan Cameron @ 2026-10-25 23:54 UTC (permalink / raw)
To: Radu Sabau via B4 Relay
Cc: radu.sabau, Lars-Peter Clausen, Michael Hennerich, Nuno Sa,
David Lechner, Andy Shevchenko, Alexandru Ardelean, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, linux-iio, linux-kernel,
devicetree, linux
On Fri, 23 Oct 2026 12:30:21 +0300
Radu Sabau via B4 Relay <devnull+radu.sabau.analog.com@kernel.org> wrote:
> From: Radu Sabau <radu.sabau@analog.com>
>
> Add device tree bindings for the Analog Devices ADIS16607 Precision
> MEMS Inertial Measurement Unit. The ADIS16607 integrates a triaxial
> gyroscope, triaxial accelerometer, and temperature sensor, and
> communicates over SPI Half-Duplex.
>
> Two variants are supported, adis16607-2 and adis16607-3, differing
> only in gyroscope full-scale range (±450°/s and ±2000°/s
> respectively).
>
> The binding exposes optional properties for:
> - An external clock for sync mode (falling back to the internal 8 kHz
> clock when absent)
> - A hardware reset GPIO (active low)
>
> Reviewed-by: Rob Herring (Arm) <robh@kernel.org>
There is enough new stuff in here I'd have dropped Rob's tag.
> Signed-off-by: Radu Sabau <radu.sabau@analog.com>
> ---
and noted you had done so here with a breadcrumb of why, which
is the gpio handling
> .../devicetree/bindings/iio/imu/adi,adis16607.yaml | 94 ++++++++++++++++++++++
> MAINTAINERS | 7 ++
> 2 files changed, 101 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/iio/imu/adi,adis16607.yaml b/Documentation/devicetree/bindings/iio/imu/adi,adis16607.yaml
> new file mode 100644
> index 000000000000..4545f942365c
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/imu/adi,adis16607.yaml
> @@ -0,0 +1,94 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/iio/imu/adi,adis16607.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Analog Devices ADIS16607 IMU
> +
> +maintainers:
> + - Radu Sabau <radu.sabau@analog.com>
> +
> +description: |
> + Analog Devices ADIS16607 Precision MEMS Inertial Measurement Unit (IMU)
> + with a triaxial gyroscope, triaxial accelerometer, and temperature sensor.
> + Uses SPI Half-Duplex communication protocol.
> +
> + The device has five GPIO pins (GPIO0-GPIO4 in the datasheet), each
> + independently selectable for a single fixed function. This binding
> + exposes GPIO0's reset function as reset-gpios; GPIO2's sync function
> + is modeled via the clocks property, and GPIO4's DEV_NOK interrupt via
> + the interrupts property. GPIO3 (data-ready) is required internally
> + regardless of board wiring and isn't exposed. GPIO1 (wake) gates
> + whether the bootloader auto-loads its NVM contents after reset; it
> + defaults to enabled with a weak pull-up, which already gives normal
> + operation with no configuration needed, so it isn't exposed either.
> +
> +properties:
> + compatible:
> + enum:
> + - adi,adis16607-2
> + - adi,adis16607-3
> +
> + reg:
> + maxItems: 1
> +
> + spi-cpha: true
> +
> + spi-cpol: true
> +
> + spi-max-frequency:
> + maximum: 10000000
> +
> + vdd-supply: true
> +
> + interrupts:
> + maxItems: 1
> +
> + clocks:
> + description:
> + Optional external clock for sync mode. If not provided, the internal
> + 8 kHz clock is used.
> + maxItems: 1
> +
> + reset-gpios:
> + description:
> + Optional GPIO for hardware reset, active low. Routes to the
> + device's GPIO0 pin, which can be independently set to the reset
> + function or left idle via an on-chip register.
> + maxItems: 1
> +
> +required:
> + - compatible
> + - reg
> + - interrupts
> + - spi-cpha
> + - spi-cpol
> + - vdd-supply
> +
> +allOf:
> + - $ref: /schemas/spi/spi-peripheral-props.yaml#
> +
> +unevaluatedProperties: false
> +
> +examples:
> + - |
> + #include <dt-bindings/interrupt-controller/irq.h>
> + #include <dt-bindings/gpio/gpio.h>
> + spi {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + imu@0 {
> + compatible = "adi,adis16607-2";
> + reg = <0>;
> + spi-cpha;
> + spi-cpol;
> + spi-max-frequency = <10000000>;
> + vdd-supply = <&vdd>;
> + interrupts = <4 IRQ_TYPE_LEVEL_HIGH>;
> + interrupt-parent = <&gpio>;
> + clocks = <&sync_clk>;
> + reset-gpios = <&gpio 10 GPIO_ACTIVE_LOW>;
> + };
> + };
> diff --git a/MAINTAINERS b/MAINTAINERS
> index f53047aa8d3c..d5679e8d0ffd 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -1792,6 +1792,13 @@ S: Supported
> W: https://ez.analog.com/linux-software-drivers
> F: Documentation/devicetree/bindings/iio/imu/adi,adis16550.yaml
>
> +ANALOG DEVICES INC ADIS16607 DRIVER
> +M: Radu Sabau <radu.sabau@analog.com>
> +L: linux-iio@vger.kernel.org
> +S: Supported
> +W: https://ez.analog.com/linux-software-drivers
> +F: Documentation/devicetree/bindings/iio/imu/adi,adis16607.yaml
> +
> ANALOG DEVICES INC ADL8113 DRIVER
> M: Antoniu Miclaus <antoniu.miclaus@analog.com>
> L: linux-iio@vger.kernel.org
>
^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH v4 5/8] iio: imu: Add driver for the ADI ADIS16607
2026-10-23 9:30 [PATCH v4 0/8] iio: imu: Add support for the ADI ADIS16607 Radu Sabau via B4 Relay
` (3 preceding siblings ...)
2026-10-23 9:30 ` [PATCH v4 4/8] dt-bindings: iio: imu: Add bindings for ADI ADIS16607 Radu Sabau via B4 Relay
@ 2026-10-23 9:30 ` Radu Sabau via B4 Relay
2026-10-23 10:26 ` Andy Shevchenko
2026-10-23 9:30 ` [PATCH v4 6/8] iio: imu: adis16607: Add FIFO-based buffered/triggered capture Radu Sabau via B4 Relay
` (2 subsequent siblings)
7 siblings, 1 reply; 19+ messages in thread
From: Radu Sabau via B4 Relay @ 2026-10-23 9:30 UTC (permalink / raw)
To: Lars-Peter Clausen, Michael Hennerich, Nuno Sa, Jonathan Cameron,
David Lechner, Andy Shevchenko, Alexandru Ardelean, Radu Sabau,
Rob Herring, Krzysztof Kozlowski, Conor Dooley
Cc: linux-iio, linux-kernel, devicetree, linux, Jonathan Cameron
From: Radu Sabau <radu.sabau@analog.com>
The ADIS16607 is a Precision MEMS IMU from Analog Devices with
triaxial gyroscope and accelerometer, delta angle/velocity, and
temperature channels, communicating over a custom SPI Half-Duplex
protocol. ADIS16607-2 has a ±450°/s gyroscope range, ADIS16607-3
±2000°/s; both share a ±40g accelerometer range and an 8 kHz internal
clock.
This is the base driver: probe, raw channel reads, sampling-frequency
control, startup/self-test via adislib, and debugfs identity entries.
No buffered capture, external clock sync, or calibration bias yet -
those follow in later patches.
Self-test leaves the device's SELF_TEST register cleared on every exit
path, not just on success, so a read failure or a failed delta check
doesn't leave the sensor stuck in its self-test/force state until the
next reset.
Signed-off-by: Radu Sabau <radu.sabau@analog.com>
---
MAINTAINERS | 1 +
drivers/iio/imu/Kconfig | 13 +
drivers/iio/imu/Makefile | 1 +
drivers/iio/imu/adis16607.c | 842 ++++++++++++++++++++++++++++++++++++++++++++
4 files changed, 857 insertions(+)
diff --git a/MAINTAINERS b/MAINTAINERS
index d5679e8d0ffd..268a82adeb1d 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1798,6 +1798,7 @@ L: linux-iio@vger.kernel.org
S: Supported
W: https://ez.analog.com/linux-software-drivers
F: Documentation/devicetree/bindings/iio/imu/adi,adis16607.yaml
+F: drivers/iio/imu/adis16607.c
ANALOG DEVICES INC ADL8113 DRIVER
M: Antoniu Miclaus <antoniu.miclaus@analog.com>
diff --git a/drivers/iio/imu/Kconfig b/drivers/iio/imu/Kconfig
index 8bab4616be20..0b39057d0a7c 100644
--- a/drivers/iio/imu/Kconfig
+++ b/drivers/iio/imu/Kconfig
@@ -65,6 +65,19 @@ config ADIS16550
To compile this driver as a module, choose M here: the module will be
called adis16550.
+config ADIS16607
+ tristate "Analog Devices ADIS16607 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 ADIS16607-2
+ and ADIS16607-3 inertial sensors containing triaxis
+ gyroscope and triaxis accelerometer.
+
+ To compile this driver as a module, choose M here: the module will be
+ called adis16607.
+
source "drivers/iio/imu/bmi160/Kconfig"
source "drivers/iio/imu/bmi270/Kconfig"
source "drivers/iio/imu/bmi323/Kconfig"
diff --git a/drivers/iio/imu/Makefile b/drivers/iio/imu/Makefile
index 3268dc2371ae..6eeec1ee387f 100644
--- a/drivers/iio/imu/Makefile
+++ b/drivers/iio/imu/Makefile
@@ -9,6 +9,7 @@ obj-$(CONFIG_ADIS16460) += adis16460.o
obj-$(CONFIG_ADIS16475) += adis16475.o
obj-$(CONFIG_ADIS16480) += adis16480.o
obj-$(CONFIG_ADIS16550) += adis16550.o
+obj-$(CONFIG_ADIS16607) += adis16607.o
adis_lib-y += adis.o
adis_lib-$(CONFIG_IIO_ADIS_LIB_BUFFER) += adis_trigger.o
diff --git a/drivers/iio/imu/adis16607.c b/drivers/iio/imu/adis16607.c
new file mode 100644
index 000000000000..2dda306d7b54
--- /dev/null
+++ b/drivers/iio/imu/adis16607.c
@@ -0,0 +1,842 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * ADIS16607 IMU driver
+ *
+ * Copyright 2026 Analog Devices Inc.
+ */
+#include <linux/array_size.h>
+#include <linux/bitfield.h>
+#include <linux/bitops.h>
+#include <linux/debugfs.h>
+#include <linux/delay.h>
+#include <linux/iio/buffer.h>
+#include <linux/iio/iio.h>
+#include <linux/iio/imu/adis.h>
+#include <linux/math.h>
+#include <linux/minmax.h>
+#include <linux/module.h>
+#include <linux/property.h>
+#include <linux/regulator/consumer.h>
+#include <linux/seq_file.h>
+#include <linux/spi/spi.h>
+#include <linux/unaligned.h>
+#include <linux/units.h>
+
+/* Register addresses */
+#define ADIS16607_REG_DEV_ID 0x00
+#define ADIS16607_REG_REV_ID 0x01
+#define ADIS16607_REG_SERIAL_NUM0 0x02
+#define ADIS16607_REG_SERIAL_NUM1 0x03
+#define ADIS16607_REG_SERIAL_NUM2 0x04
+#define ADIS16607_REG_DIAG_STAT 0x05
+#define ADIS16607_REG_X_ACCEL 0x06
+#define ADIS16607_REG_Y_ACCEL 0x08
+#define ADIS16607_REG_Z_ACCEL 0x0A
+#define ADIS16607_REG_X_GYRO 0x0C
+#define ADIS16607_REG_Y_GYRO 0x0E
+#define ADIS16607_REG_Z_GYRO 0x10
+#define ADIS16607_REG_X_DELTVEL 0x12
+#define ADIS16607_REG_Y_DELTVEL 0x14
+#define ADIS16607_REG_Z_DELTVEL 0x16
+#define ADIS16607_REG_X_DELTANG 0x18
+#define ADIS16607_REG_Y_DELTANG 0x1A
+#define ADIS16607_REG_Z_DELTANG 0x1C
+#define ADIS16607_REG_TEMP 0x20
+#define ADIS16607_REG_USER_GPIO_CFG 0x2F
+#define ADIS16607_REG_SPI_HALFDUPLEX 0x32
+#define ADIS16607_REG_SOFT_RESET 0x36
+#define ADIS16607_REG_SELF_TEST 0x39
+#define ADIS16607_REG_DEC_RATE 0x3A
+#define ADIS16607_REG_DIGITAL_STATUS 0x4E
+
+/* SPI Half-Duplex lock value */
+#define ADIS16607_LOCK_SPI_HALFDUPLEX 0xB4B4
+
+/* Masks */
+#define ADIS16607_RESET_GPIO_MASK GENMASK(2, 0)
+#define ADIS16607_BOOTLOADER_BUSY_MASK BIT(0)
+
+/* Self-test registers and masks */
+#define ADIS16607_REG_SELF_TEST_DATA(x) ((x) + 0x23)
+#define ADIS16607_SNSR_SELF_TEST_MASK BIT(6)
+#define ADIS16607_ST_FORCE_MASK BIT(7)
+
+/* Self-test delta check limits */
+#define ADIS16607_ACCEL_XY_DELTA_MAX 260
+#define ADIS16607_ACCEL_Z_DELTA_MAX 4000
+#define ADIS16607_GYRO_DELTA_MAX 2600
+
+struct adis16607_chip_info {
+ const struct iio_chan_spec *channels;
+ const char *name;
+ u32 num_channels;
+ u32 gyro_max_val;
+ u32 gyro_max_scale;
+ u32 accel_max_val;
+ u32 accel_max_scale;
+ u32 temp_scale;
+ u32 deltang_max_val;
+ u32 deltvel_max_val;
+ u32 int_clk_hz;
+ u16 max_dec;
+};
+
+struct adis16607 {
+ struct adis adis;
+ const struct adis16607_chip_info *info;
+ unsigned long clk_freq_mhz;
+ u64 sample_period_ns;
+};
+
+enum {
+ ADIS16607_SCAN_ACCEL_X,
+ ADIS16607_SCAN_ACCEL_Y,
+ ADIS16607_SCAN_ACCEL_Z,
+ ADIS16607_SCAN_GYRO_X,
+ ADIS16607_SCAN_GYRO_Y,
+ ADIS16607_SCAN_GYRO_Z,
+ ADIS16607_SCAN_DELTVEL_X,
+ ADIS16607_SCAN_DELTVEL_Y,
+ ADIS16607_SCAN_DELTVEL_Z,
+ ADIS16607_SCAN_DELTANG_X,
+ ADIS16607_SCAN_DELTANG_Y,
+ ADIS16607_SCAN_DELTANG_Z,
+ ADIS16607_SCAN_TEMP,
+};
+
+/*
+ * SPI Half-Duplex read operation for ADIS16607.
+ * Protocol:
+ * - First transfer: Send [reg | 0x80, 0, 0, 0] - device latches command
+ * - Second transfer: Send same command, device responds in bytes 2-3
+ * - CS toggles between transfers (cs_change = 1 on first xfer)
+ * For 4-byte reads (24-bit sensor data):
+ * - Read MSW from reg, read LSW from reg+1
+ * - Combine as: ((val_msw << 8) & 0xFFFF00) | val_lsw
+ */
+static int adis16607_spi_read(struct adis *adis, unsigned int reg,
+ unsigned int *val, unsigned int size)
+{
+ struct spi_message msg;
+ struct spi_transfer xfers[2] = {
+ {
+ .tx_buf = adis->tx,
+ .len = 4,
+ .bits_per_word = 8,
+ .cs_change = 1,
+ },
+ {
+ .tx_buf = adis->tx,
+ .rx_buf = adis->rx,
+ .len = 4,
+ .bits_per_word = 8,
+ },
+ };
+ unsigned int val_msw, val_lsw;
+ int ret;
+
+ switch (size) {
+ case 2:
+ adis->tx[0] = reg | BIT(7);
+ adis->tx[1] = 0;
+ adis->tx[2] = 0;
+ adis->tx[3] = 0;
+
+ spi_message_init_with_transfers(&msg, xfers, ARRAY_SIZE(xfers));
+
+ ret = spi_sync(adis->spi, &msg);
+ if (ret)
+ return ret;
+
+ *val = get_unaligned_be16(&adis->rx[2]);
+ return 0;
+
+ case 4:
+ /* Read MSW from reg */
+ adis->tx[0] = reg | BIT(7);
+ adis->tx[1] = 0;
+ adis->tx[2] = 0;
+ adis->tx[3] = 0;
+
+ spi_message_init_with_transfers(&msg, xfers, ARRAY_SIZE(xfers));
+
+ ret = spi_sync(adis->spi, &msg);
+ if (ret)
+ return ret;
+
+ val_msw = get_unaligned_be16(&adis->rx[2]);
+
+ /* Read LSW from reg+1 */
+ adis->tx[0] = (reg + 1) | BIT(7);
+
+ spi_message_init_with_transfers(&msg, xfers, ARRAY_SIZE(xfers));
+
+ ret = spi_sync(adis->spi, &msg);
+ if (ret)
+ return ret;
+
+ val_lsw = adis->rx[2];
+
+ /* Combine: 24-bit value with MSW in upper 16 bits, LSW in lower 8 bits */
+ *val = ((val_msw << 8) & 0xFFFF00) | val_lsw;
+ return 0;
+
+ default:
+ return -EINVAL;
+ }
+}
+
+/*
+ * SPI Half-Duplex write operation for ADIS16607.
+ * Protocol:
+ * - Send 3 bytes: [reg, val_high, val_low]
+ * - Only 16-bit writes are supported
+ */
+static int adis16607_spi_write(struct adis *adis, unsigned int reg,
+ unsigned int val, unsigned int size)
+{
+ if (size != 2)
+ return -EINVAL;
+
+ adis->tx[0] = reg;
+ put_unaligned_be16(val, &adis->tx[1]);
+
+ return spi_write(adis->spi, adis->tx, 3);
+}
+
+static int adis16607_reset(struct adis *adis)
+{
+ return adis16607_spi_write(adis, ADIS16607_REG_SOFT_RESET, BIT(0), 2);
+}
+
+static const struct adis_ops adis16607_ops = {
+ .read = adis16607_spi_read,
+ .write = adis16607_spi_write,
+ .reset = adis16607_reset,
+};
+
+/*
+ * Check if the delta between two values is within specified range.
+ */
+static bool adis16607_delta_check(s16 val1, s16 val2, s16 delta_max)
+{
+ return abs(val1 - val2) <= delta_max;
+}
+
+/*
+ * Perform sensor self-test for ADIS16607.
+ * Reads self-test data before and after forcing self-test,
+ * then verifies delta values are within acceptable ranges.
+ */
+static int adis16607_sensor_self_test(struct adis16607 *st)
+{
+ struct adis *adis = &st->adis;
+ u16 st1_val[6], st2_val[6];
+ s16 delta_max[6] = {
+ ADIS16607_ACCEL_XY_DELTA_MAX,
+ ADIS16607_ACCEL_XY_DELTA_MAX,
+ ADIS16607_ACCEL_Z_DELTA_MAX,
+ ADIS16607_GYRO_DELTA_MAX,
+ ADIS16607_GYRO_DELTA_MAX,
+ ADIS16607_GYRO_DELTA_MAX,
+ };
+ int ret, i;
+
+ /* Enable sensor self-test */
+ ret = __adis_write_reg_16(adis, ADIS16607_REG_SELF_TEST,
+ ADIS16607_SNSR_SELF_TEST_MASK);
+ if (ret)
+ return ret;
+
+ /* Wait for self-test to settle before reading results */
+ msleep(adis->data->timeouts->self_test_ms);
+
+ /* Read first set of self-test data */
+ for (i = 0; i < ARRAY_SIZE(st1_val); i++) {
+ ret = __adis_read_reg_16(adis, ADIS16607_REG_SELF_TEST_DATA(i),
+ &st1_val[i]);
+ if (ret)
+ goto clear_self_test;
+ }
+
+ /* Force self-test */
+ ret = __adis_write_reg_16(adis, ADIS16607_REG_SELF_TEST,
+ ADIS16607_SNSR_SELF_TEST_MASK | ADIS16607_ST_FORCE_MASK);
+ if (ret)
+ goto clear_self_test;
+
+ /* Wait for forced self-test to settle before reading results */
+ msleep(adis->data->timeouts->self_test_ms);
+
+ /* Read second set of self-test data */
+ for (i = 0; i < ARRAY_SIZE(st2_val); i++) {
+ ret = __adis_read_reg_16(adis, ADIS16607_REG_SELF_TEST_DATA(i),
+ &st2_val[i]);
+ if (ret)
+ goto clear_self_test;
+
+ if (!adis16607_delta_check((s16)st1_val[i], (s16)st2_val[i],
+ delta_max[i])) {
+ dev_err(&adis->spi->dev,
+ "Self-test failed for axis %d: delta out of range\n", i);
+ ret = -EINVAL;
+ goto clear_self_test;
+ }
+ }
+
+ /* Clear self-test register */
+ return __adis_write_reg_16(adis, ADIS16607_REG_SELF_TEST, 0);
+
+clear_self_test:
+ /*
+ * Best-effort cleanup after a failure above - leave the device out
+ * of its self-test/force state rather than stuck in it until the
+ * next reset. Warn rather than returning this write's own result,
+ * so a cleanup failure doesn't mask the real error that got us here.
+ */
+ if (__adis_write_reg_16(adis, ADIS16607_REG_SELF_TEST, 0))
+ dev_warn(&adis->spi->dev, "failed to clear self-test register\n");
+
+ return ret;
+}
+
+/*
+ * Post-reset bring-up callback for struct adis_data (see adis16607_data
+ * below). __adis_initial_startup() calls this, unlocked, right after it
+ * has handled reset (hardware GPIO or SW reset, adislib's own job) and
+ * before self-test - no mutex is taken here either, matching that
+ * convention. Does the device-specific startup steps adislib has no
+ * concept of: SPI Half-Duplex locking, dev_id verification, and the
+ * bootloader-busy check.
+ */
+static int adis16607_post_reset_setup(struct adis *adis)
+{
+ struct device *dev = &adis->spi->dev;
+ u16 diag_stat, digital_status, dev_id;
+ int ret;
+
+ /* Lock the device into SPI Half-Duplex mode */
+ ret = __adis_write_reg_16(adis, ADIS16607_REG_SPI_HALFDUPLEX,
+ ADIS16607_LOCK_SPI_HALFDUPLEX);
+ if (ret) {
+ dev_err(dev, "failed to lock SPI Half-Duplex mode: %d\n", ret);
+ return ret;
+ }
+
+ /* Verify the correct device is attached */
+ ret = __adis_read_reg_16(adis, ADIS16607_REG_DEV_ID, &dev_id);
+ if (ret)
+ return ret;
+
+ if (dev_id != adis->data->prod_id) {
+ dev_err(dev, "device ID mismatch: expected 0x%04x, got 0x%04x\n",
+ adis->data->prod_id, dev_id);
+ return -ENODEV;
+ }
+
+ /* Read diag_stat to clear any latched faults */
+ ret = __adis_read_reg_16(adis, ADIS16607_REG_DIAG_STAT, &diag_stat);
+ if (ret)
+ return ret;
+
+ ret = __adis_check_status(adis);
+ if (ret) {
+ dev_err(dev, "status check failed: %d\n", ret);
+ return ret;
+ }
+
+ /* Check that bootloader is not busy */
+ ret = __adis_read_reg_16(adis, ADIS16607_REG_DIGITAL_STATUS, &digital_status);
+ if (ret)
+ return ret;
+
+ if (digital_status & ADIS16607_BOOTLOADER_BUSY_MASK) {
+ dev_err(dev, "bootloader is busy\n");
+ return -EBUSY;
+ }
+
+ return 0;
+}
+
+/*
+ * Self-test callback for struct adis_data (see adis16607_data below).
+ * Called by __adis_initial_startup() right after
+ * adis16607_post_reset_setup().
+ */
+static int adis16607_self_test(struct adis *adis)
+{
+ struct adis16607 *st = container_of(adis, struct adis16607, adis);
+ struct device *dev = &adis->spi->dev;
+ u16 diag_stat;
+ int ret;
+
+ /* Inertial Settling Time (Table 1), needed before self-test reads sensor data */
+ msleep(40);
+
+ ret = adis16607_sensor_self_test(st);
+ if (ret)
+ return ret;
+
+ /* Read diag_stat again to clear any faults latched during self-test */
+ ret = __adis_read_reg_16(adis, ADIS16607_REG_DIAG_STAT, &diag_stat);
+ if (ret)
+ return ret;
+
+ ret = __adis_check_status(adis);
+ if (ret)
+ dev_err(dev, "status check after self-test failed: %d\n", ret);
+
+ return ret;
+}
+
+static int adis16607_firmware_revision_show(struct seq_file *s, void *arg)
+{
+ struct adis16607 *st = s->private;
+ u16 rev;
+ int ret;
+
+ ret = adis_read_reg_16(&st->adis, ADIS16607_REG_REV_ID, &rev);
+ if (ret)
+ return ret;
+
+ seq_printf(s, "%u.%u\n", (rev >> 8) & 0xff, rev & 0xff);
+ return 0;
+}
+DEFINE_SHOW_ATTRIBUTE(adis16607_firmware_revision);
+
+static int adis16607_show_serial_number(void *arg, u64 *val)
+{
+ struct adis16607 *st = arg;
+ u64 serial;
+ u16 tmp;
+ int ret;
+
+ ret = adis_read_reg_16(&st->adis, ADIS16607_REG_SERIAL_NUM0, &tmp);
+ if (ret)
+ return ret;
+
+ serial = (u64)tmp;
+
+ ret = adis_read_reg_16(&st->adis, ADIS16607_REG_SERIAL_NUM1, &tmp);
+ if (ret)
+ return ret;
+
+ serial |= ((u64)tmp << 16);
+
+ ret = adis_read_reg_16(&st->adis, ADIS16607_REG_SERIAL_NUM2, &tmp);
+ if (ret)
+ return ret;
+
+ serial |= ((u64)tmp << 32);
+
+ *val = serial;
+ return 0;
+}
+DEFINE_DEBUGFS_ATTRIBUTE(adis16607_serial_number_fops,
+ adis16607_show_serial_number, NULL, "0x%.8llx\n");
+
+static int adis16607_show_dev_id(void *arg, u64 *val)
+{
+ struct adis16607 *st = arg;
+ u16 dev_id;
+ int ret;
+
+ ret = adis_read_reg_16(&st->adis, ADIS16607_REG_DEV_ID, &dev_id);
+ if (ret)
+ return ret;
+
+ *val = dev_id;
+ return 0;
+}
+DEFINE_DEBUGFS_ATTRIBUTE(adis16607_dev_id_fops,
+ adis16607_show_dev_id, NULL, "%llu\n");
+
+static void adis16607_debugfs_init(struct iio_dev *indio_dev)
+{
+ struct adis16607 *st = iio_priv(indio_dev);
+ struct dentry *d = iio_get_debugfs_dentry(indio_dev);
+
+ debugfs_create_file_unsafe("serial_number", 0400, d, st,
+ &adis16607_serial_number_fops);
+ debugfs_create_file_unsafe("dev_id", 0400, d, st,
+ &adis16607_dev_id_fops);
+ debugfs_create_file("firmware_revision", 0400, d, st,
+ &adis16607_firmware_revision_fops);
+}
+
+static int adis16607_get_freq(struct adis16607 *st, u32 *freq)
+{
+ u16 dec = 0;
+ int ret;
+
+ adis_dev_auto_lock(&st->adis);
+
+ ret = __adis_read_reg_16(&st->adis, ADIS16607_REG_DEC_RATE, &dec);
+ if (ret)
+ return ret;
+
+ *freq = DIV_ROUND_CLOSEST(st->clk_freq_mhz, dec + 1);
+
+ return 0;
+}
+
+static int adis16607_set_freq(struct adis16607 *st, u32 freq)
+{
+ u32 dec;
+ int ret;
+
+ if (!freq)
+ return -EINVAL;
+
+ adis_dev_auto_lock(&st->adis);
+
+ dec = DIV_ROUND_CLOSEST(st->clk_freq_mhz, freq);
+ if (dec)
+ dec--;
+
+ dec = min_t(u32, dec, st->info->max_dec);
+
+ ret = __adis_write_reg_16(&st->adis, ADIS16607_REG_DEC_RATE, dec);
+ if (ret)
+ return ret;
+
+ st->sample_period_ns = DIV_ROUND_CLOSEST_ULL((u64)NSEC_PER_SEC * 1000ULL * (dec + 1),
+ st->clk_freq_mhz);
+
+ return 0;
+}
+
+static int adis16607_read_raw(struct iio_dev *indio_dev,
+ const struct iio_chan_spec *chan,
+ int *val, int *val2, long info)
+{
+ struct adis16607 *st = iio_priv(indio_dev);
+ int ret;
+ u32 tmp;
+
+ 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->info->gyro_max_val;
+ *val2 = st->info->gyro_max_scale;
+ return IIO_VAL_FRACTIONAL;
+ case IIO_ACCEL:
+ *val = st->info->accel_max_val;
+ *val2 = st->info->accel_max_scale;
+ return IIO_VAL_FRACTIONAL;
+ case IIO_TEMP:
+ *val = st->info->temp_scale;
+ return IIO_VAL_INT;
+ case IIO_DELTA_ANGL:
+ *val = st->info->deltang_max_val;
+ *val2 = 23;
+ return IIO_VAL_FRACTIONAL_LOG2;
+ case IIO_DELTA_VELOCITY:
+ *val = st->info->deltvel_max_val;
+ *val2 = 23;
+ return IIO_VAL_FRACTIONAL_LOG2;
+ default:
+ return -EINVAL;
+ }
+
+ case IIO_CHAN_INFO_OFFSET:
+ /* Temperature: 0 raw counts = 25°C; offset = 25000 mdeg / 5 mdeg per LSB = 5000 */
+ *val = 5000;
+ return IIO_VAL_INT;
+
+ case IIO_CHAN_INFO_SAMP_FREQ:
+ ret = adis16607_get_freq(st, &tmp);
+ if (ret)
+ return ret;
+ *val = tmp / 1000;
+ *val2 = (tmp % 1000) * 1000;
+ return IIO_VAL_INT_PLUS_MICRO;
+
+ default:
+ return -EINVAL;
+ }
+}
+
+static int adis16607_write_raw(struct iio_dev *indio_dev,
+ const struct iio_chan_spec *chan,
+ int val, int val2, long info)
+{
+ struct adis16607 *st = iio_priv(indio_dev);
+ u32 tmp;
+
+ switch (info) {
+ case IIO_CHAN_INFO_SAMP_FREQ:
+ tmp = val * 1000 + val2 / 1000;
+ return adis16607_set_freq(st, tmp);
+
+ default:
+ return -EINVAL;
+ }
+}
+
+#define ADIS16607_MOD_CHAN(_type, _mod, _address, _si) { \
+ .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 = 24, \
+ .storagebits = 32, \
+ .endianness = IIO_BE, \
+ }, \
+}
+
+#define ADIS16607_ACCEL_CHANNEL(_mod) \
+ ADIS16607_MOD_CHAN(IIO_ACCEL, IIO_MOD_ ## _mod, \
+ ADIS16607_REG_ ## _mod ## _ACCEL, \
+ ADIS16607_SCAN_ACCEL_ ## _mod)
+
+#define ADIS16607_GYRO_CHANNEL(_mod) \
+ ADIS16607_MOD_CHAN(IIO_ANGL_VEL, IIO_MOD_ ## _mod, \
+ ADIS16607_REG_ ## _mod ## _GYRO, \
+ ADIS16607_SCAN_GYRO_ ## _mod)
+
+#define ADIS16607_MOD_CHAN_DELTA(_type, _mod, _address, _si) { \
+ .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 = 24, \
+ .storagebits = 32, \
+ .endianness = IIO_BE, \
+ }, \
+}
+
+#define ADIS16607_DELTANG_CHAN(_mod) \
+ ADIS16607_MOD_CHAN_DELTA(IIO_DELTA_ANGL, IIO_MOD_ ## _mod, \
+ ADIS16607_REG_ ## _mod ## _DELTANG, \
+ ADIS16607_SCAN_DELTANG_ ## _mod)
+
+#define ADIS16607_DELTVEL_CHAN(_mod) \
+ ADIS16607_MOD_CHAN_DELTA(IIO_DELTA_VELOCITY, IIO_MOD_ ## _mod, \
+ ADIS16607_REG_ ## _mod ## _DELTVEL, \
+ ADIS16607_SCAN_DELTVEL_ ## _mod)
+
+static const struct iio_chan_spec adis16607_channels[] = {
+ ADIS16607_ACCEL_CHANNEL(X),
+ ADIS16607_ACCEL_CHANNEL(Y),
+ ADIS16607_ACCEL_CHANNEL(Z),
+ ADIS16607_GYRO_CHANNEL(X),
+ ADIS16607_GYRO_CHANNEL(Y),
+ ADIS16607_GYRO_CHANNEL(Z),
+ ADIS16607_DELTVEL_CHAN(X),
+ ADIS16607_DELTVEL_CHAN(Y),
+ ADIS16607_DELTVEL_CHAN(Z),
+ ADIS16607_DELTANG_CHAN(X),
+ ADIS16607_DELTANG_CHAN(Y),
+ ADIS16607_DELTANG_CHAN(Z),
+ {
+ .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 = ADIS16607_REG_TEMP,
+ .scan_index = ADIS16607_SCAN_TEMP,
+ .scan_type = {
+ .sign = 's',
+ .realbits = 16,
+ .storagebits = 16,
+ .endianness = IIO_BE,
+ },
+ },
+ IIO_CHAN_SOFT_TIMESTAMP(13),
+};
+
+static const struct iio_info adis16607_info = {
+ .read_raw = adis16607_read_raw,
+ .write_raw = adis16607_write_raw,
+ .debugfs_reg_access = adis_debugfs_reg_access,
+};
+
+static const char * const adis16607_status_error_msgs[] = {
+ [9] = "Boot Memory Failure",
+ [11] = "Power Supply Failure",
+ [12] = "Accelerometer Failure",
+ [13] = "Gyroscope Failure",
+};
+
+static const struct adis_timeout adis16607_timeouts = {
+ .reset_ms = 130,
+ .sw_reset_ms = 50,
+ .self_test_ms = 15,
+};
+
+static const struct adis_data adis16607_data = {
+ .diag_stat_reg = ADIS16607_REG_DIAG_STAT,
+ .diag_stat_size = 2,
+ .prod_id_reg = ADIS16607_REG_DEV_ID,
+ .prod_id = 0x6000,
+ .unmasked_drdy = true,
+ .post_reset_setup = adis16607_post_reset_setup,
+ .self_test = adis16607_self_test,
+ .status_error_msgs = adis16607_status_error_msgs,
+ .status_error_mask = BIT(9) | BIT(11) | BIT(12) | BIT(13),
+ .timeouts = &adis16607_timeouts,
+};
+
+/*
+ * ADIS16607-2: 450 deg/s gyro, 40g accel
+ * ADIS16607-3: 2000 deg/s gyro, 40g accel
+ */
+static const struct adis16607_chip_info adis16607_2_chip_info = {
+ .channels = adis16607_channels,
+ .num_channels = ARRAY_SIZE(adis16607_channels),
+ .name = "adis16607-2",
+ .gyro_max_val = IIO_DEGREE_TO_RAD(450),
+ .gyro_max_scale = 30000 << 8,
+ .accel_max_val = IIO_G_TO_M_S_2(40),
+ .accel_max_scale = 31250 << 8,
+ .temp_scale = 5,
+ .deltang_max_val = IIO_DEGREE_TO_RAD(450),
+ .deltvel_max_val = IIO_G_TO_M_S_2(40),
+ .int_clk_hz = 8000,
+ .max_dec = 65535,
+};
+
+static const struct adis16607_chip_info adis16607_3_chip_info = {
+ .channels = adis16607_channels,
+ .num_channels = ARRAY_SIZE(adis16607_channels),
+ .name = "adis16607-3",
+ .gyro_max_val = IIO_DEGREE_TO_RAD(2000),
+ .gyro_max_scale = 31250 << 8,
+ .accel_max_val = IIO_G_TO_M_S_2(40),
+ .accel_max_scale = 31250 << 8,
+ .temp_scale = 5,
+ .deltang_max_val = IIO_DEGREE_TO_RAD(2000),
+ .deltvel_max_val = IIO_G_TO_M_S_2(40),
+ .int_clk_hz = 8000,
+ .max_dec = 65535,
+};
+
+/*
+ * No external clock sync support yet: always run on the internal clock.
+ * Default dec=0 after reset, so period = 1 / clk_freq_mhz.
+ */
+static int adis16607_config_clk(struct adis16607 *st)
+{
+ st->clk_freq_mhz = st->info->int_clk_hz * MILLI;
+ st->sample_period_ns =
+ DIV_ROUND_CLOSEST_ULL((u64)NSEC_PER_SEC * MILLI, st->clk_freq_mhz);
+ return 0;
+}
+
+static int adis16607_probe(struct spi_device *spi)
+{
+ struct device *dev = &spi->dev;
+ struct iio_dev *indio_dev;
+ struct adis16607 *st;
+ int ret;
+
+ indio_dev = devm_iio_device_alloc(dev, sizeof(*st));
+ if (!indio_dev)
+ return -ENOMEM;
+
+ st = iio_priv(indio_dev);
+ st->info = spi_get_device_match_data(spi);
+ if (!st->info)
+ return -ENODATA;
+
+ indio_dev->name = st->info->name;
+ indio_dev->channels = st->info->channels;
+ indio_dev->num_channels = st->info->num_channels;
+ indio_dev->info = &adis16607_info;
+ indio_dev->modes = INDIO_DIRECT_MODE;
+
+ st->adis.ops = &adis16607_ops;
+
+ ret = devm_regulator_get_enable(dev, "vdd");
+ if (ret)
+ return dev_err_probe(dev, ret, "Failed to get vdd regulator\n");
+
+ ret = adis_init(&st->adis, indio_dev, spi, &adis16607_data);
+ if (ret)
+ return ret;
+
+ /*
+ * Force GPIO0 back to its RESET function before relying on toggling
+ * it below - RESET_GPIO_MASK is runtime-writable on this part, so a
+ * prior write (buggy driver, direct reg access, etc.) could have
+ * left it de-routed, which would make the GPIO toggle in
+ * __adis_initial_startup() silently do nothing. This write goes out
+ * over SPI, which stays live regardless of this mask's state, so it
+ * can re-arm the mux as long as the device is merely
+ * reset-pin-defeated and not fully unresponsive. Only warn on
+ * failure instead of erroring out - bailing here would throw away
+ * whatever chance the GPIO toggle below still has of working.
+ */
+ if (device_property_present(dev, "reset-gpios")) {
+ ret = __adis_update_bits_base(&st->adis, ADIS16607_REG_USER_GPIO_CFG,
+ ADIS16607_RESET_GPIO_MASK,
+ FIELD_PREP(ADIS16607_RESET_GPIO_MASK, 1), 2);
+ if (ret)
+ dev_warn(dev, "failed to force reset-pin routing: %d\n", ret);
+ }
+
+ ret = __adis_initial_startup(&st->adis);
+ if (ret)
+ return ret;
+
+ ret = adis16607_config_clk(st);
+ if (ret)
+ return ret;
+
+ ret = devm_iio_device_register(dev, indio_dev);
+ if (ret)
+ return ret;
+
+ adis16607_debugfs_init(indio_dev);
+
+ return 0;
+}
+
+static const struct spi_device_id adis16607_id[] = {
+ { .name = "adis16607-2", .driver_data = (kernel_ulong_t)&adis16607_2_chip_info },
+ { .name = "adis16607-3", .driver_data = (kernel_ulong_t)&adis16607_3_chip_info },
+ { }
+};
+MODULE_DEVICE_TABLE(spi, adis16607_id);
+
+static const struct of_device_id adis16607_of_match[] = {
+ { .compatible = "adi,adis16607-2", .data = &adis16607_2_chip_info },
+ { .compatible = "adi,adis16607-3", .data = &adis16607_3_chip_info },
+ { }
+};
+MODULE_DEVICE_TABLE(of, adis16607_of_match);
+
+static struct spi_driver adis16607_driver = {
+ .driver = {
+ .name = "adis16607",
+ .of_match_table = adis16607_of_match,
+ },
+ .probe = adis16607_probe,
+ .id_table = adis16607_id,
+};
+module_spi_driver(adis16607_driver);
+
+MODULE_AUTHOR("Radu Sabau <radu.sabau@analog.com>");
+MODULE_DESCRIPTION("Analog Devices ADIS16607 IMU driver");
+MODULE_IMPORT_NS("IIO_ADISLIB");
+MODULE_LICENSE("GPL");
--
2.43.0
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: [PATCH v4 5/8] iio: imu: Add driver for the ADI ADIS16607
2026-10-23 9:30 ` [PATCH v4 5/8] iio: imu: Add driver for the " Radu Sabau via B4 Relay
@ 2026-10-23 10:26 ` Andy Shevchenko
2026-10-26 10:22 ` Sabau, Radu bogdan
0 siblings, 1 reply; 19+ messages in thread
From: Andy Shevchenko @ 2026-10-23 10:26 UTC (permalink / raw)
To: radu.sabau
Cc: Lars-Peter Clausen, Michael Hennerich, Nuno Sa, Jonathan Cameron,
David Lechner, Andy Shevchenko, Alexandru Ardelean, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, linux-iio, linux-kernel,
devicetree, linux
On Fri, Oct 23, 2026 at 12:30:22PM +0300, Radu Sabau via B4 Relay wrote:
> The ADIS16607 is a Precision MEMS IMU from Analog Devices with
> triaxial gyroscope and accelerometer, delta angle/velocity, and
> temperature channels, communicating over a custom SPI Half-Duplex
> protocol. ADIS16607-2 has a ±450°/s gyroscope range, ADIS16607-3
> ±2000°/s; both share a ±40g accelerometer range and an 8 kHz internal
> clock.
>
> This is the base driver: probe, raw channel reads, sampling-frequency
> control, startup/self-test via adislib, and debugfs identity entries.
> No buffered capture, external clock sync, or calibration bias yet -
> those follow in later patches.
>
> Self-test leaves the device's SELF_TEST register cleared on every exit
> path, not just on success, so a read failure or a failed delta check
> doesn't leave the sensor stuck in its self-test/force state until the
> next reset.
...
> +++ b/drivers/iio/imu/Kconfig
> + tristate "Analog Devices ADIS16607 and similar IMU driver"
(see below)
...
> +struct adis16607_chip_info {
> + const struct iio_chan_spec *channels;
> + const char *name;
> + u32 num_channels;
> + u32 gyro_max_val;
> + u32 gyro_max_scale;
> + u32 accel_max_val;
> + u32 accel_max_scale;
> + u32 temp_scale;
> + u32 deltang_max_val;
> + u32 deltvel_max_val;
> + u32 int_clk_hz;
_Hz
> + u16 max_dec;
> +};
...
> +struct adis16607 {
> + struct adis adis;
> + const struct adis16607_chip_info *info;
> + unsigned long clk_freq_mhz;
_MHz or _mHz? (I believe it's the first one.)
This is why case-sensitive units (as per physics) are important.
> + u64 sample_period_ns;
> +};
...
> + /* Combine: 24-bit value with MSW in upper 16 bits, LSW in lower 8 bits */
Make it multi-line and to fix 80 characters.
> + *val = ((val_msw << 8) & 0xFFFF00) | val_lsw;
Why not GENMASK()? Can val_msw be u16 or other 16-bit type?
...
> +static int adis16607_sensor_self_test(struct adis16607 *st)
> +{
> + struct adis *adis = &st->adis;
> + u16 st1_val[6], st2_val[6];
> + s16 delta_max[6] = {
> + ADIS16607_ACCEL_XY_DELTA_MAX,
> + ADIS16607_ACCEL_XY_DELTA_MAX,
> + ADIS16607_ACCEL_Z_DELTA_MAX,
> + ADIS16607_GYRO_DELTA_MAX,
> + ADIS16607_GYRO_DELTA_MAX,
> + ADIS16607_GYRO_DELTA_MAX,
> + };
> + int ret, i;
Why is 'i' signed?
> + /* Enable sensor self-test */
> + ret = __adis_write_reg_16(adis, ADIS16607_REG_SELF_TEST,
> + ADIS16607_SNSR_SELF_TEST_MASK);
> + if (ret)
> + return ret;
> +
> + /* Wait for self-test to settle before reading results */
> + msleep(adis->data->timeouts->self_test_ms);
> +
> + /* Read first set of self-test data */
> + for (i = 0; i < ARRAY_SIZE(st1_val); i++) {
> + ret = __adis_read_reg_16(adis, ADIS16607_REG_SELF_TEST_DATA(i),
> + &st1_val[i]);
> + if (ret)
> + goto clear_self_test;
> + }
> +
> + /* Force self-test */
> + ret = __adis_write_reg_16(adis, ADIS16607_REG_SELF_TEST,
> + ADIS16607_SNSR_SELF_TEST_MASK | ADIS16607_ST_FORCE_MASK);
> + if (ret)
> + goto clear_self_test;
> +
> + /* Wait for forced self-test to settle before reading results */
> + msleep(adis->data->timeouts->self_test_ms);
> +
> + /* Read second set of self-test data */
> + for (i = 0; i < ARRAY_SIZE(st2_val); i++) {
In both for-loops iterator is local, so
for (unsigned int i = 0; i < ARRAY_SIZE(st2_val); i++) {
> + ret = __adis_read_reg_16(adis, ADIS16607_REG_SELF_TEST_DATA(i),
> + &st2_val[i]);
> + if (ret)
> + goto clear_self_test;
> +
> + if (!adis16607_delta_check((s16)st1_val[i], (s16)st2_val[i],
> + delta_max[i])) {
> + dev_err(&adis->spi->dev,
> + "Self-test failed for axis %d: delta out of range\n", i);
> + ret = -EINVAL;
> + goto clear_self_test;
> + }
> + }
> +
> + /* Clear self-test register */
> + return __adis_write_reg_16(adis, ADIS16607_REG_SELF_TEST, 0);
> +
> +clear_self_test:
> + /*
> + * Best-effort cleanup after a failure above - leave the device out
> + * of its self-test/force state rather than stuck in it until the
> + * next reset. Warn rather than returning this write's own result,
> + * so a cleanup failure doesn't mask the real error that got us here.
> + */
> + if (__adis_write_reg_16(adis, ADIS16607_REG_SELF_TEST, 0))
> + dev_warn(&adis->spi->dev, "failed to clear self-test register\n");
> +
> + return ret;
> +}
...
> +static int adis16607_self_test(struct adis *adis)
> +{
> + struct adis16607 *st = container_of(adis, struct adis16607, adis);
> + struct device *dev = &adis->spi->dev;
> + u16 diag_stat;
> + int ret;
> +
> + /* Inertial Settling Time (Table 1), needed before self-test reads sensor data */
Make it fit 80.
> + msleep(40);
> +
> + ret = adis16607_sensor_self_test(st);
> + if (ret)
> + return ret;
> +
> + /* Read diag_stat again to clear any faults latched during self-test */
> + ret = __adis_read_reg_16(adis, ADIS16607_REG_DIAG_STAT, &diag_stat);
> + if (ret)
> + return ret;
> +
> + ret = __adis_check_status(adis);
> + if (ret)
> + dev_err(dev, "status check after self-test failed: %d\n", ret);
> +
> + return ret;
> +}
...
> +static int adis16607_firmware_revision_show(struct seq_file *s, void *arg)
> +{
> + struct adis16607 *st = s->private;
> + u16 rev;
Can this be
struct revision {
u8 minor;
u8 major;
} rev;
?
> + int ret;
> +
> + ret = adis_read_reg_16(&st->adis, ADIS16607_REG_REV_ID, &rev);
> + if (ret)
> + return ret;
> +
> + seq_printf(s, "%u.%u\n", (rev >> 8) & 0xff, rev & 0xff);
> + return 0;
> +}
...
> +static int adis16607_show_serial_number(void *arg, u64 *val)
> +{
> + struct adis16607 *st = arg;
> + u64 serial;
> + u16 tmp;
> + int ret;
> +
> + ret = adis_read_reg_16(&st->adis, ADIS16607_REG_SERIAL_NUM0, &tmp);
> + if (ret)
> + return ret;
> +
> + serial = (u64)tmp;
How is explicit casting helpful here?
> + ret = adis_read_reg_16(&st->adis, ADIS16607_REG_SERIAL_NUM1, &tmp);
> + if (ret)
> + return ret;
> +
> + serial |= ((u64)tmp << 16);
> +
> + ret = adis_read_reg_16(&st->adis, ADIS16607_REG_SERIAL_NUM2, &tmp);
> + if (ret)
> + return ret;
> +
> + serial |= ((u64)tmp << 32);
What about reversing the order and shift serial instead?
u64 serial = 0;
ret = adis_read_reg_16(&st->adis, ADIS16607_REG_SERIAL_NUM2, &tmp);
if (ret)
return ret;
serial |= tmp;
ret = adis_read_reg_16(&st->adis, ADIS16607_REG_SERIAL_NUM1, &tmp);
if (ret)
return ret;
serial <<= 16;
serial |= tmp;
ret = adis_read_reg_16(&st->adis, ADIS16607_REG_SERIAL_NUM0, &tmp);
if (ret)
return ret;
serial <<= 16;
serial |= tmp;
> + *val = serial;
> + return 0;
> +}
...
> +static int adis16607_set_freq(struct adis16607 *st, u32 freq)
> +{
> + u32 dec;
> + int ret;
> +
> + if (!freq)
> + return -EINVAL;
> +
> + adis_dev_auto_lock(&st->adis);
> +
> + dec = DIV_ROUND_CLOSEST(st->clk_freq_mhz, freq);
> + if (dec)
> + dec--;
> +
> + dec = min_t(u32, dec, st->info->max_dec);
No min_t(). Make the types to be compatible for min().
> + ret = __adis_write_reg_16(&st->adis, ADIS16607_REG_DEC_RATE, dec);
> + if (ret)
> + return ret;
> +
> + st->sample_period_ns = DIV_ROUND_CLOSEST_ULL((u64)NSEC_PER_SEC * 1000ULL * (dec + 1),
> + st->clk_freq_mhz);
Make it fit. Using temporary variable usually helps. Also you don't need (u64)
casting or ULL suffix. On top of that, what is another 1000 multiplier for?
I would expect a comment on top with explaining magic(s).
> + return 0;
> +}
...
> + case IIO_CHAN_INFO_SAMP_FREQ:
> + ret = adis16607_get_freq(st, &tmp);
> + if (ret)
> + return ret;
> + *val = tmp / 1000;
> + *val2 = (tmp % 1000) * 1000;
What is 1000? HZ_PER_KHZ? KHZ_PER_MHZ?
> + return IIO_VAL_INT_PLUS_MICRO;
...
> +static int adis16607_write_raw(struct iio_dev *indio_dev,
> + const struct iio_chan_spec *chan,
> + int val, int val2, long info)
> +{
> + struct adis16607 *st = iio_priv(indio_dev);
> + u32 tmp;
> +
> + switch (info) {
> + case IIO_CHAN_INFO_SAMP_FREQ:
> + tmp = val * 1000 + val2 / 1000;
> + return adis16607_set_freq(st, tmp);
Ditto.
> + default:
> + return -EINVAL;
> + }
> +}
...
> +#define ADIS16607_MOD_CHAN(_type, _mod, _address, _si) { \
> + .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 = 24, \
> + .storagebits = 32, \
> + .endianness = IIO_BE, \
> + }, \
> +}
This is not a function, I would rather shift it right by a tab with a split
(also indent the backslashes, it makes it easier to read).
#define ADIS16607_MOD_CHAN(_type, _mod, _address, _si) \
{ \
.type = (_type), \
...
}
...
> +#define ADIS16607_MOD_CHAN_DELTA(_type, _mod, _address, _si) { \
> + .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 = 24, \
> + .storagebits = 32, \
> + .endianness = IIO_BE, \
> + }, \
> +}
Ditto.
...
> +/*
> + * No external clock sync support yet: always run on the internal clock.
> + * Default dec=0 after reset, so period = 1 / clk_freq_mhz.
> + */
> +static int adis16607_config_clk(struct adis16607 *st)
> +{
> + st->clk_freq_mhz = st->info->int_clk_hz * MILLI;
So, it's milli-hertz then? We have MILLIHZ_PER_HZ.
> + st->sample_period_ns =
> + DIV_ROUND_CLOSEST_ULL((u64)NSEC_PER_SEC * MILLI, st->clk_freq_mhz);
> + return 0;
> +}
Yeah, this magic 1000 should be explained everywhere.
...
> +MODULE_DESCRIPTION("Analog Devices ADIS16607 IMU driver");
Make the text the same as per Kconfig. Whatever wording you choose.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 19+ messages in thread* RE: [PATCH v4 5/8] iio: imu: Add driver for the ADI ADIS16607
2026-10-23 10:26 ` Andy Shevchenko
@ 2026-10-26 10:22 ` Sabau, Radu bogdan
2026-10-26 10:50 ` Andy Shevchenko
0 siblings, 1 reply; 19+ messages in thread
From: Sabau, Radu bogdan @ 2026-10-26 10:22 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Lars-Peter Clausen, Hennerich, Michael, Sa, Nuno,
Jonathan Cameron, David Lechner, Andy Shevchenko,
Alexandru Ardelean, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, linux-iio, linux-kernel, devicetree, linux
> -----Original Message-----
> From: Andy Shevchenko <andriy.shevchenko@intel.com>
> Sent: Friday, October 23, 2026 1:27 PM
...
> > +static int adis16607_firmware_revision_show(struct seq_file *s, void *arg)
> > +{
> > + struct adis16607 *st = s->private;
> > + u16 rev;
>
> Can this be
>
> struct revision {
> u8 minor;
> u8 major;
> } rev;
>
> ?
>
Hi Andy! Thank you also for looking at this series.
Although I like the idea, I have a question here.
If u16 rev would be replaced by the struct and the read was done
directly on the struct pointer, could it mess things up on big-endian
targets? A native 16-bit store would place the value's low byte at
the lower address on little-endian, but the high byte at the lower
address on big-endian - so minor (at the lower offset) would only
get the right byte on little-endian, and would actually get major's
byte on big-endian, right?
What about we declare this struct and also have a u16 tmp declared,
do the read on tmp, and then rev.minor/rev.major get assigned via
FIELD_GET() - keeping the readability improvement without that risk?
struct {
u8 minor;
u8 major;
} rev;
u16 tmp;
int ret;
ret = adis_read_reg_16(&st->adis, ADIS16607_REG_REV_ID, &tmp);
if (ret)
return ret;
rev.major = FIELD_GET(GENMASK(15, 8), tmp);
rev.minor = FIELD_GET(GENMASK(7, 0), tmp);
seq_printf(s, "%u.%u\n", rev.major, rev.minor);
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: [PATCH v4 5/8] iio: imu: Add driver for the ADI ADIS16607
2026-10-26 10:22 ` Sabau, Radu bogdan
@ 2026-10-26 10:50 ` Andy Shevchenko
0 siblings, 0 replies; 19+ messages in thread
From: Andy Shevchenko @ 2026-10-26 10:50 UTC (permalink / raw)
To: Sabau, Radu bogdan
Cc: Lars-Peter Clausen, Hennerich, Michael, Sa, Nuno,
Jonathan Cameron, David Lechner, Andy Shevchenko,
Alexandru Ardelean, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, linux-iio, linux-kernel, devicetree, linux
On Mon, Oct 26, 2026 at 10:22:22AM +0000, Sabau, Radu bogdan wrote:
> > From: Andy Shevchenko <andriy.shevchenko@intel.com>
> > Sent: Friday, October 23, 2026 1:27 PM
...
> > > +static int adis16607_firmware_revision_show(struct seq_file *s, void *arg)
> > > +{
> > > + struct adis16607 *st = s->private;
> > > + u16 rev;
> >
> > Can this be
> >
> > struct revision {
> > u8 minor;
> > u8 major;
> > } rev;
> >
> > ?
>
> Although I like the idea, I have a question here.
> If u16 rev would be replaced by the struct and the read was done
> directly on the struct pointer, could it mess things up on big-endian
> targets? A native 16-bit store would place the value's low byte at
> the lower address on little-endian, but the high byte at the lower
> address on big-endian - so minor (at the lower offset) would only
> get the right byte on little-endian, and would actually get major's
> byte on big-endian, right?
>
> What about we declare this struct and also have a u16 tmp declared,
> do the read on tmp, and then rev.minor/rev.major get assigned via
> FIELD_GET() - keeping the readability improvement without that risk?
>
> struct {
> u8 minor;
> u8 major;
> } rev;
> u16 tmp;
> int ret;
>
> ret = adis_read_reg_16(&st->adis, ADIS16607_REG_REV_ID, &tmp);
> if (ret)
> return ret;
>
> rev.major = FIELD_GET(GENMASK(15, 8), tmp);
> rev.minor = FIELD_GET(GENMASK(7, 0), tmp);
>
> seq_printf(s, "%u.%u\n", rev.major, rev.minor);
Yes, this will work for me.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH v4 6/8] iio: imu: adis16607: Add FIFO-based buffered/triggered capture
2026-10-23 9:30 [PATCH v4 0/8] iio: imu: Add support for the ADI ADIS16607 Radu Sabau via B4 Relay
` (4 preceding siblings ...)
2026-10-23 9:30 ` [PATCH v4 5/8] iio: imu: Add driver for the " Radu Sabau via B4 Relay
@ 2026-10-23 9:30 ` Radu Sabau via B4 Relay
2026-10-26 0:09 ` Jonathan Cameron
2026-10-23 9:30 ` [PATCH v4 7/8] iio: imu: adis16607: Add external clock sync support Radu Sabau via B4 Relay
2026-10-23 9:30 ` [PATCH v4 8/8] iio: imu: adis16607: Add calibration bias support for gyro/accel Radu Sabau via B4 Relay
7 siblings, 1 reply; 19+ messages in thread
From: Radu Sabau via B4 Relay @ 2026-10-23 9:30 UTC (permalink / raw)
To: Lars-Peter Clausen, Michael Hennerich, Nuno Sa, Jonathan Cameron,
David Lechner, Andy Shevchenko, Alexandru Ardelean, Radu Sabau,
Rob Herring, Krzysztof Kozlowski, Conor Dooley
Cc: linux-iio, linux-kernel, devicetree, linux, Jonathan Cameron
From: Radu Sabau <radu.sabau@analog.com>
Add buffered capture via the device's hardware FIFO - the only
capture mode this driver supports. Popping the FIFO uses a dedicated
command and payload format (FIFO_DATA_CMD, 52 bytes, no checksum),
handled directly rather than through adislib's generic burst-read
helpers, which assume one fixed-shape read per trigger rather than a
variable-count drain loop.
The watermark-met condition is signaled via DEV_NOK on the device's
one interrupt pin, which stays asserted (level-triggered) until
DIAG_STAT is read and cleared - matching adislib's has_fifo contract.
DEV_NOK fires on any DIAG_STAT flag, not just FIFO_THRESH_MET, so the
trigger handler checks that specific bit before popping anything.
DR_GPIO_MASK stays enabled unconditionally: DR pulses drive the FIFO
controller's internal sample enqueue, independent of pin routing.
Watermark is configurable via the standard hwfifo_set_watermark
callback and the usual hwfifo_* sysfs attributes.
Signed-off-by: Radu Sabau <radu.sabau@analog.com>
---
drivers/iio/imu/adis16607.c | 349 +++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 346 insertions(+), 3 deletions(-)
diff --git a/drivers/iio/imu/adis16607.c b/drivers/iio/imu/adis16607.c
index 2dda306d7b54..b1f3667d8ce3 100644
--- a/drivers/iio/imu/adis16607.c
+++ b/drivers/iio/imu/adis16607.c
@@ -12,6 +12,9 @@
#include <linux/iio/buffer.h>
#include <linux/iio/iio.h>
#include <linux/iio/imu/adis.h>
+#include <linux/iio/sysfs.h>
+#include <linux/iio/trigger_consumer.h>
+#include <linux/interrupt.h>
#include <linux/math.h>
#include <linux/minmax.h>
#include <linux/module.h>
@@ -42,8 +45,11 @@
#define ADIS16607_REG_Y_DELTANG 0x1A
#define ADIS16607_REG_Z_DELTANG 0x1C
#define ADIS16607_REG_TEMP 0x20
+#define ADIS16607_REG_FIFO_WORD_CNT 0x2B
#define ADIS16607_REG_USER_GPIO_CFG 0x2F
#define ADIS16607_REG_SPI_HALFDUPLEX 0x32
+#define ADIS16607_REG_USER_DATA_CFG 0x34
+#define ADIS16607_REG_USER_FIFO_CFG 0x35
#define ADIS16607_REG_SOFT_RESET 0x36
#define ADIS16607_REG_SELF_TEST 0x39
#define ADIS16607_REG_DEC_RATE 0x3A
@@ -54,8 +60,26 @@
/* Masks */
#define ADIS16607_RESET_GPIO_MASK GENMASK(2, 0)
+#define ADIS16607_DR_GPIO_MASK GENMASK(11, 9)
+#define ADIS16607_DEV_NOK_MASK GENMASK(14, 12)
+#define ADIS16607_DATA_CNTR_EN_MASK BIT(14)
+#define ADIS16607_BURST32_MASK BIT(15)
#define ADIS16607_BOOTLOADER_BUSY_MASK BIT(0)
+/* FIFO masks */
+#define ADIS16607_FIFO_THR_MASK GENMASK(10, 0)
+#define ADIS16607_FIFO_FLUSH_MASK BIT(15)
+#define ADIS16607_FIFO_THR_MET_MASK BIT(0)
+#define ADIS16607_FIFO_DATA_CMD 0xA9
+
+/* FIFO limits */
+#define ADIS16607_MAX_FIFO_WM 38
+#define ADIS16607_FIFO_WORDS_PER_SAMPLE 26
+#define ADIS16607_FIFO_DATA_LEN 52
+
+/* Burst read - Half-Duplex mode (DATA_CNTR enabled) */
+#define ADIS16607_BURST_CMD_SIZE 4
+
/* Self-test registers and masks */
#define ADIS16607_REG_SELF_TEST_DATA(x) ((x) + 0x23)
#define ADIS16607_SNSR_SELF_TEST_MASK BIT(6)
@@ -86,6 +110,11 @@ struct adis16607 {
const struct adis16607_chip_info *info;
unsigned long clk_freq_mhz;
u64 sample_period_ns;
+ u16 fifo_watermark;
+ /* Scratch buffer used for FIFO pop operations, see adis16607_fifo_pop() */
+ u8 fifo_buf[ADIS16607_BURST_CMD_SIZE + ADIS16607_FIFO_DATA_LEN] __aligned(IIO_DMA_MINALIGN);
+ /* IIO buffer for reordered data - sized for max channels */
+ u8 data[64] __aligned(IIO_DMA_MINALIGN);
};
enum {
@@ -215,6 +244,286 @@ static const struct adis_ops adis16607_ops = {
.reset = adis16607_reset,
};
+/*
+ * FIFO/burst data offsets for each channel (in bytes from the data start,
+ * i.e. after the 4-byte command prefix).
+ * Each 32-bit inertial channel occupies 4 bytes (MSW + LSW), TEMP is 2 bytes.
+ *
+ * FIFO data layout (52 bytes, DATA_CNTR enabled, no checksum):
+ * Offset 0-3: ACCEL_X (32-bit: MSW, LSW)
+ * Offset 4-7: ACCEL_Y
+ * Offset 8-11: ACCEL_Z
+ * Offset 12-15: GYRO_X
+ * Offset 16-19: GYRO_Y
+ * Offset 20-23: GYRO_Z
+ * Offset 24-27: DELTVEL_X
+ * Offset 28-31: DELTVEL_Y
+ * Offset 32-35: DELTVEL_Z
+ * Offset 36-39: DELTANG_X
+ * Offset 40-43: DELTANG_Y
+ * Offset 44-47: DELTANG_Z
+ * Offset 48-49: TEMP (16-bit)
+ * Offset 50-51: DATA_CNTR (16-bit)
+ *
+ * Each 32-bit value encodes 24-bit sensor data as:
+ * value_24bit = (MSW << 8) | (LSW >> 8)
+ */
+static const u8 adis16607_burst_offsets[] = {
+ [ADIS16607_SCAN_ACCEL_X] = 0,
+ [ADIS16607_SCAN_ACCEL_Y] = 4,
+ [ADIS16607_SCAN_ACCEL_Z] = 8,
+ [ADIS16607_SCAN_GYRO_X] = 12,
+ [ADIS16607_SCAN_GYRO_Y] = 16,
+ [ADIS16607_SCAN_GYRO_Z] = 20,
+ [ADIS16607_SCAN_DELTVEL_X] = 24,
+ [ADIS16607_SCAN_DELTVEL_Y] = 28,
+ [ADIS16607_SCAN_DELTVEL_Z] = 32,
+ [ADIS16607_SCAN_DELTANG_X] = 36,
+ [ADIS16607_SCAN_DELTANG_Y] = 40,
+ [ADIS16607_SCAN_DELTANG_Z] = 44,
+ [ADIS16607_SCAN_TEMP] = 48,
+};
+
+/*
+ * Pop a single sample from FIFO using FIFO_DATA command (0xA9).
+ * Returns the raw FIFO data in the provided buffer.
+ */
+static int adis16607_fifo_pop(struct adis16607 *st, u8 *data)
+{
+ struct spi_message msg;
+ struct spi_transfer xfer = {
+ .tx_buf = st->fifo_buf,
+ .rx_buf = st->fifo_buf,
+ .len = ADIS16607_BURST_CMD_SIZE + ADIS16607_FIFO_DATA_LEN,
+ .bits_per_word = 8,
+ };
+ int ret;
+
+ /* FIFO data command */
+ st->fifo_buf[0] = ADIS16607_FIFO_DATA_CMD;
+ st->fifo_buf[1] = 0;
+ st->fifo_buf[2] = 0;
+ st->fifo_buf[3] = 0;
+
+ spi_message_init(&msg);
+ spi_message_add_tail(&xfer, &msg);
+
+ ret = spi_sync(st->adis.spi, &msg);
+ if (ret)
+ return ret;
+
+ /* Copy data starting after command bytes */
+ memcpy(data, st->fifo_buf + ADIS16607_BURST_CMD_SIZE, ADIS16607_FIFO_DATA_LEN);
+ return 0;
+}
+
+/*
+ * Process a single FIFO sample and push to IIO buffer.
+ * FIFO data format differs from regular burst - no DIAG_STAT, data starts immediately.
+ *
+ * FIFO data layout (52 bytes):
+ * Offset 0-3: ACCEL_X (32-bit)
+ * Offset 4-7: ACCEL_Y (32-bit)
+ * Offset 8-11: ACCEL_Z (32-bit)
+ * Offset 12-15: GYRO_X (32-bit)
+ * Offset 16-19: GYRO_Y (32-bit)
+ * Offset 20-23: GYRO_Z (32-bit)
+ * Offset 24-27: DELTVEL_X (32-bit)
+ * Offset 28-31: DELTVEL_Y (32-bit)
+ * Offset 32-35: DELTVEL_Z (32-bit)
+ * Offset 36-39: DELTANG_X (32-bit)
+ * Offset 40-43: DELTANG_Y (32-bit)
+ * Offset 44-47: DELTANG_Z (32-bit)
+ * Offset 48-49: TEMP (16-bit)
+ * Offset 50-51: DATA_CNTR (16-bit)
+ */
+static void adis16607_fifo_push_sample(struct iio_dev *indio_dev,
+ u8 *fifo_data, s64 timestamp)
+{
+ struct adis16607 *st = iio_priv(indio_dev);
+ u16 msw, lsw;
+ s32 val24;
+ int bit, offset = 0;
+
+ memset(st->data, 0, sizeof(st->data));
+
+ for_each_set_bit(bit, indio_dev->active_scan_mask, indio_dev->masklength) {
+ u8 burst_off;
+
+ /* Skip the soft timestamp channel - it has no burst offset */
+ if (bit >= ARRAY_SIZE(adis16607_burst_offsets))
+ continue;
+
+ burst_off = adis16607_burst_offsets[bit];
+
+ if (bit == ADIS16607_SCAN_TEMP) {
+ /* TEMP is 16-bit */
+ put_unaligned_be16(get_unaligned_be16(&fifo_data[burst_off]),
+ &st->data[offset]);
+ offset += 2;
+ } else {
+ /* All other channels are 24-bit stored as 32-bit */
+ msw = get_unaligned_be16(&fifo_data[burst_off]);
+ lsw = get_unaligned_be16(&fifo_data[burst_off + 2]);
+ val24 = sign_extend32(((u32)msw << 8) | (lsw >> 8), 23);
+ put_unaligned_be32(val24, &st->data[offset]);
+ offset += 4;
+ }
+ }
+
+ iio_push_to_buffers_with_timestamp(indio_dev, st->data, timestamp);
+}
+
+/*
+ * Trigger handler - reads samples from the device FIFO. Triggered by the
+ * DEV_NOK interrupt, which fires on any DIAG_STAT flag, not just
+ * FIFO_THRESH_MET - the DIAG_STAT read below both checks which flag(s)
+ * raised the interrupt and clears the latched status, and must happen
+ * before any FIFO pop, or we'd pop for nothing on an unrelated flag.
+ */
+static irqreturn_t adis16607_trigger_handler(int irq, void *p)
+{
+ struct iio_poll_func *pf = p;
+ struct iio_dev *indio_dev = pf->indio_dev;
+ struct adis16607 *st = iio_priv(indio_dev);
+ u8 fifo_sample[ADIS16607_FIFO_DATA_LEN];
+ u16 diag_stat, word_cnt, sample_cnt;
+ s64 timestamp;
+ int ret, i;
+
+ mutex_lock(&st->adis.state_lock);
+
+ /* Check if FIFO threshold is met */
+ ret = __adis_read_reg_16(&st->adis, ADIS16607_REG_DIAG_STAT, &diag_stat);
+ if (ret)
+ goto unlock;
+
+ if (!(diag_stat & ADIS16607_FIFO_THR_MET_MASK))
+ goto unlock;
+
+ /* Read FIFO word count */
+ ret = __adis_read_reg_16(&st->adis, ADIS16607_REG_FIFO_WORD_CNT, &word_cnt);
+ if (ret)
+ goto unlock;
+
+ /* Calculate number of complete samples */
+ sample_cnt = word_cnt / ADIS16607_FIFO_WORDS_PER_SAMPLE;
+ if (!sample_cnt)
+ goto unlock;
+
+ /*
+ * pf->timestamp is the IRQ-fire time, i.e. when the newest (last
+ * enqueued) sample tipped the FIFO over watermark. The FIFO pops
+ * oldest-first, so back-date the first popped sample's timestamp by
+ * the batch duration - the loop then counts back up to pf->timestamp
+ * for the newest sample, instead of projecting past it.
+ */
+ timestamp = pf->timestamp - (s64)(sample_cnt - 1) * st->sample_period_ns;
+
+ /* Pop and process each sample */
+ for (i = 0; i < sample_cnt; i++) {
+ ret = adis16607_fifo_pop(st, fifo_sample);
+ if (ret)
+ goto unlock;
+
+ adis16607_fifo_push_sample(indio_dev, fifo_sample, timestamp);
+ timestamp += st->sample_period_ns;
+ }
+
+unlock:
+ mutex_unlock(&st->adis.state_lock);
+ iio_trigger_notify_done(indio_dev->trig);
+ return IRQ_HANDLED;
+}
+
+/* FIFO sysfs attributes */
+IIO_STATIC_CONST_DEVICE_ATTR(hwfifo_watermark_min, "1");
+IIO_STATIC_CONST_DEVICE_ATTR(hwfifo_watermark_max, __stringify(ADIS16607_MAX_FIFO_WM));
+
+static ssize_t hwfifo_watermark_show(struct device *dev,
+ struct device_attribute *attr,
+ char *buf)
+{
+ struct iio_dev *indio_dev = dev_to_iio_dev(dev);
+ struct adis16607 *st = iio_priv(indio_dev);
+
+ return sysfs_emit(buf, "%u\n", st->fifo_watermark);
+}
+
+static ssize_t hwfifo_enabled_show(struct device *dev,
+ struct device_attribute *attr,
+ char *buf)
+{
+ struct iio_dev *indio_dev = dev_to_iio_dev(dev);
+ struct adis16607 *st = iio_priv(indio_dev);
+ u16 val;
+ int ret;
+
+ ret = adis_read_reg_16(&st->adis, ADIS16607_REG_USER_FIFO_CFG, &val);
+ if (ret)
+ return ret;
+
+ return sysfs_emit(buf, "%d\n", (val & ADIS16607_FIFO_THR_MASK) ? 1 : 0);
+}
+
+static IIO_DEVICE_ATTR_RO(hwfifo_watermark, 0);
+static IIO_DEVICE_ATTR_RO(hwfifo_enabled, 0);
+
+static const struct iio_dev_attr *adis16607_fifo_attributes[] = {
+ &iio_dev_attr_hwfifo_watermark_min,
+ &iio_dev_attr_hwfifo_watermark_max,
+ &iio_dev_attr_hwfifo_watermark,
+ &iio_dev_attr_hwfifo_enabled,
+ NULL
+};
+
+static int adis16607_buffer_postenable(struct iio_dev *indio_dev)
+{
+ struct adis16607 *st = iio_priv(indio_dev);
+ struct adis *adis = &st->adis;
+ u16 threshold;
+ int ret;
+
+ adis_dev_auto_lock(adis);
+
+ /* Flush FIFO to start clean */
+ ret = __adis_write_reg_16(adis, ADIS16607_REG_USER_FIFO_CFG,
+ ADIS16607_FIFO_FLUSH_MASK);
+ if (ret)
+ return ret;
+
+ /* Enable FIFO with watermark threshold (in word count) */
+ threshold = st->fifo_watermark * ADIS16607_FIFO_WORDS_PER_SAMPLE;
+ return __adis_write_reg_16(adis, ADIS16607_REG_USER_FIFO_CFG,
+ threshold & ADIS16607_FIFO_THR_MASK);
+}
+
+static int adis16607_buffer_postdisable(struct iio_dev *indio_dev)
+{
+ struct adis16607 *st = iio_priv(indio_dev);
+ struct adis *adis = &st->adis;
+
+ adis_dev_auto_lock(adis);
+
+ /* Disable FIFO */
+ return __adis_write_reg_16(adis, ADIS16607_REG_USER_FIFO_CFG, 0);
+}
+
+static const struct iio_buffer_setup_ops adis16607_buffer_ops = {
+ .postenable = adis16607_buffer_postenable,
+ .postdisable = adis16607_buffer_postdisable,
+};
+
+static int adis16607_set_watermark(struct iio_dev *indio_dev, unsigned int val)
+{
+ struct adis16607 *st = iio_priv(indio_dev);
+
+ val = clamp(val, 1, ADIS16607_MAX_FIFO_WM);
+ st->fifo_watermark = val;
+
+ return 0;
+}
+
/*
* Check if the delta between two values is within specified range.
*/
@@ -306,8 +615,9 @@ static int adis16607_sensor_self_test(struct adis16607 *st)
* has handled reset (hardware GPIO or SW reset, adislib's own job) and
* before self-test - no mutex is taken here either, matching that
* convention. Does the device-specific startup steps adislib has no
- * concept of: SPI Half-Duplex locking, dev_id verification, and the
- * bootloader-busy check.
+ * concept of: SPI Half-Duplex locking, dev_id verification, the
+ * bootloader-busy check, enabling 32-bit burst mode + DATA_CNTR, and
+ * the DR/DEV_NOK pin routing.
*/
static int adis16607_post_reset_setup(struct adis *adis)
{
@@ -355,7 +665,29 @@ static int adis16607_post_reset_setup(struct adis *adis)
return -EBUSY;
}
- return 0;
+ /* Enable 32-bit burst mode and DATA_CNTR - one-time, used by FIFO pops */
+ ret = __adis_update_bits_base(adis, ADIS16607_REG_USER_DATA_CFG,
+ ADIS16607_BURST32_MASK | ADIS16607_DATA_CNTR_EN_MASK,
+ ADIS16607_BURST32_MASK | ADIS16607_DATA_CNTR_EN_MASK, 2);
+ if (ret)
+ return ret;
+
+ /*
+ * DR_GPIO_MASK and DEV_NOK_MASK are updated here rather than written
+ * blind, so this doesn't clobber the RESET_GPIO_MASK bit already
+ * configured earlier in probe(). Per datasheet, DR pulses are what
+ * drive the FIFO controller's internal enqueue logic. DR and
+ * DEV_NOK are separate physical pins, so enabling DR_GPIO_MASK here
+ * does not put DR's full-sample-rate pulses onto the interrupt line
+ * - DEV_NOK_MASK routes the aggregate "any DIAG_STAT flag asserted"
+ * signal onto the one physically-wired interrupt pin, which is what
+ * actually fires on FIFO_THRESH_MET, at the (much lower) watermark
+ * rate, not the sample rate.
+ */
+ return __adis_update_bits_base(adis, ADIS16607_REG_USER_GPIO_CFG,
+ ADIS16607_DR_GPIO_MASK | ADIS16607_DEV_NOK_MASK,
+ FIELD_PREP(ADIS16607_DR_GPIO_MASK, 1) |
+ FIELD_PREP(ADIS16607_DEV_NOK_MASK, 1), 2);
}
/*
@@ -669,6 +1001,7 @@ static const struct iio_info adis16607_info = {
.read_raw = adis16607_read_raw,
.write_raw = adis16607_write_raw,
.debugfs_reg_access = adis_debugfs_reg_access,
+ .hwfifo_set_watermark = adis16607_set_watermark,
};
static const char * const adis16607_status_error_msgs[] = {
@@ -690,6 +1023,7 @@ static const struct adis_data adis16607_data = {
.prod_id_reg = ADIS16607_REG_DEV_ID,
.prod_id = 0x6000,
.unmasked_drdy = true,
+ .has_fifo = true,
.post_reset_setup = adis16607_post_reset_setup,
.self_test = adis16607_self_test,
.status_error_msgs = adis16607_status_error_msgs,
@@ -765,6 +1099,8 @@ static int adis16607_probe(struct spi_device *spi)
indio_dev->info = &adis16607_info;
indio_dev->modes = INDIO_DIRECT_MODE;
+ st->fifo_watermark = ADIS16607_MAX_FIFO_WM;
+
st->adis.ops = &adis16607_ops;
ret = devm_regulator_get_enable(dev, "vdd");
@@ -803,6 +1139,13 @@ static int adis16607_probe(struct spi_device *spi)
if (ret)
return ret;
+ ret = devm_adis_setup_buffer_and_trigger_with_attrs(&st->adis, indio_dev,
+ adis16607_trigger_handler,
+ &adis16607_buffer_ops,
+ adis16607_fifo_attributes);
+ if (ret)
+ return ret;
+
ret = devm_iio_device_register(dev, indio_dev);
if (ret)
return ret;
--
2.43.0
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: [PATCH v4 6/8] iio: imu: adis16607: Add FIFO-based buffered/triggered capture
2026-10-23 9:30 ` [PATCH v4 6/8] iio: imu: adis16607: Add FIFO-based buffered/triggered capture Radu Sabau via B4 Relay
@ 2026-10-26 0:09 ` Jonathan Cameron
0 siblings, 0 replies; 19+ messages in thread
From: Jonathan Cameron @ 2026-10-26 0:09 UTC (permalink / raw)
To: Radu Sabau via B4 Relay
Cc: radu.sabau, Lars-Peter Clausen, Michael Hennerich, Nuno Sa,
David Lechner, Andy Shevchenko, Alexandru Ardelean, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, linux-iio, linux-kernel,
devicetree, linux
On Fri, 23 Oct 2026 12:30:23 +0300
Radu Sabau via B4 Relay <devnull+radu.sabau.analog.com@kernel.org> wrote:
> From: Radu Sabau <radu.sabau@analog.com>
>
> Add buffered capture via the device's hardware FIFO - the only
> capture mode this driver supports. Popping the FIFO uses a dedicated
> command and payload format (FIFO_DATA_CMD, 52 bytes, no checksum),
> handled directly rather than through adislib's generic burst-read
> helpers, which assume one fixed-shape read per trigger rather than a
> variable-count drain loop.
>
> The watermark-met condition is signaled via DEV_NOK on the device's
> one interrupt pin, which stays asserted (level-triggered) until
> DIAG_STAT is read and cleared - matching adislib's has_fifo contract.
> DEV_NOK fires on any DIAG_STAT flag, not just FIFO_THRESH_MET, so the
> trigger handler checks that specific bit before popping anything.
> DR_GPIO_MASK stays enabled unconditionally: DR pulses drive the FIFO
> controller's internal sample enqueue, independent of pin routing.
>
> Watermark is configurable via the standard hwfifo_set_watermark
> callback and the usual hwfifo_* sysfs attributes.
>
> Signed-off-by: Radu Sabau <radu.sabau@analog.com>
> ---
> drivers/iio/imu/adis16607.c | 349 +++++++++++++++++++++++++++++++++++++++++++-
> 1 file changed, 346 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/iio/imu/adis16607.c b/drivers/iio/imu/adis16607.c
> index 2dda306d7b54..b1f3667d8ce3 100644
> --- a/drivers/iio/imu/adis16607.c
> +++ b/drivers/iio/imu/adis16607.c
> /* Self-test registers and masks */
> #define ADIS16607_REG_SELF_TEST_DATA(x) ((x) + 0x23)
> #define ADIS16607_SNSR_SELF_TEST_MASK BIT(6)
> @@ -86,6 +110,11 @@ struct adis16607 {
> const struct adis16607_chip_info *info;
> unsigned long clk_freq_mhz;
> u64 sample_period_ns;
> + u16 fifo_watermark;
> + /* Scratch buffer used for FIFO pop operations, see adis16607_fifo_pop() */
> + u8 fifo_buf[ADIS16607_BURST_CMD_SIZE + ADIS16607_FIFO_DATA_LEN] __aligned(IIO_DMA_MINALIGN);
> + /* IIO buffer for reordered data - sized for max channels */
That rather implies you never DMA into it. If so just move it before
the fifo_buf and no need to mark it IIO_DMA_MINALIGN.
I'd also prefer that 64 was sized by maths, not a comment saying
it is big enough. That may include use of IIO_DECLARE_BUFFER_WITH_TS()
to ensure the timestamp spacing is there.
> + u8 data[64] __aligned(IIO_DMA_MINALIGN);
> };
> +/*
> + * Process a single FIFO sample and push to IIO buffer.
> + * FIFO data format differs from regular burst - no DIAG_STAT, data starts immediately.
> + *
> + * FIFO data layout (52 bytes):
> + * Offset 0-3: ACCEL_X (32-bit)
> + * Offset 4-7: ACCEL_Y (32-bit)
> + * Offset 8-11: ACCEL_Z (32-bit)
> + * Offset 12-15: GYRO_X (32-bit)
> + * Offset 16-19: GYRO_Y (32-bit)
> + * Offset 20-23: GYRO_Z (32-bit)
> + * Offset 24-27: DELTVEL_X (32-bit)
> + * Offset 28-31: DELTVEL_Y (32-bit)
> + * Offset 32-35: DELTVEL_Z (32-bit)
> + * Offset 36-39: DELTANG_X (32-bit)
> + * Offset 40-43: DELTANG_Y (32-bit)
> + * Offset 44-47: DELTANG_Z (32-bit)
> + * Offset 48-49: TEMP (16-bit)
> + * Offset 50-51: DATA_CNTR (16-bit)
> + */
> +static void adis16607_fifo_push_sample(struct iio_dev *indio_dev,
> + u8 *fifo_data, s64 timestamp)
> +{
> + struct adis16607 *st = iio_priv(indio_dev);
> + u16 msw, lsw;
> + s32 val24;
> + int bit, offset = 0;
> +
> + memset(st->data, 0, sizeof(st->data));
> +
> + for_each_set_bit(bit, indio_dev->active_scan_mask, indio_dev->masklength) {
iio_for_each_active_channel()
As sashiko notes, masklength is private.
There are a load of places we need to update to stop accessing these
directly but please don't add another one.
It's on my list to throw some LLM tokens ago once I get out from
under review backlog.
https://sashiko.dev/#/patchset/20261023-adis16607-v4-0-a4d541ebfd13%40analog.com
Check everything else reported.
FWIW there are also some nasty race conditions around active_scan_mask
being available when the buffer is being torn down. You might
want to close those off by claiming buffered mode to prevent
such a transition.
> + u8 burst_off;
> +
> + /* Skip the soft timestamp channel - it has no burst offset */
> + if (bit >= ARRAY_SIZE(adis16607_burst_offsets))
> + continue;
> +
> + burst_off = adis16607_burst_offsets[bit];
> +
> + if (bit == ADIS16607_SCAN_TEMP) {
> + /* TEMP is 16-bit */
> + put_unaligned_be16(get_unaligned_be16(&fifo_data[burst_off]),
> + &st->data[offset]);
> + offset += 2;
> + } else {
> + /* All other channels are 24-bit stored as 32-bit */
> + msw = get_unaligned_be16(&fifo_data[burst_off]);
> + lsw = get_unaligned_be16(&fifo_data[burst_off + 2]);
> + val24 = sign_extend32(((u32)msw << 8) | (lsw >> 8), 23);
> + put_unaligned_be32(val24, &st->data[offset]);
> + offset += 4;
> + }
> + }
> +
> + iio_push_to_buffers_with_timestamp(indio_dev, st->data, timestamp);
Use iio_push_to_buffers_with_ts() as for once in an ADIS driver we actually
have the size of data. Note that fixing the whole adis library is something
that needs doing though I took a look at it on Friday and decided to leave
it for now!
Jonathan
> +}
^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH v4 7/8] iio: imu: adis16607: Add external clock sync support
2026-10-23 9:30 [PATCH v4 0/8] iio: imu: Add support for the ADI ADIS16607 Radu Sabau via B4 Relay
` (5 preceding siblings ...)
2026-10-23 9:30 ` [PATCH v4 6/8] iio: imu: adis16607: Add FIFO-based buffered/triggered capture Radu Sabau via B4 Relay
@ 2026-10-23 9:30 ` Radu Sabau via B4 Relay
2026-10-26 0:11 ` Jonathan Cameron
2026-10-23 9:30 ` [PATCH v4 8/8] iio: imu: adis16607: Add calibration bias support for gyro/accel Radu Sabau via B4 Relay
7 siblings, 1 reply; 19+ messages in thread
From: Radu Sabau via B4 Relay @ 2026-10-23 9:30 UTC (permalink / raw)
To: Lars-Peter Clausen, Michael Hennerich, Nuno Sa, Jonathan Cameron,
David Lechner, Andy Shevchenko, Alexandru Ardelean, Radu Sabau,
Rob Herring, Krzysztof Kozlowski, Conor Dooley
Cc: linux-iio, linux-kernel, devicetree, linux, Jonathan Cameron
From: Radu Sabau <radu.sabau@analog.com>
Add support for an optional external clock (clocks property),
selecting between SYNC_DIRECT and SYNC_SCALED modes depending on the
supplied frequency, with the internal 8 kHz clock used as the default
when no clock is provided.
Signed-off-by: Radu Sabau <radu.sabau@analog.com>
---
drivers/iio/imu/adis16607.c | 121 +++++++++++++++++++++++++++++++++++++++++---
1 file changed, 114 insertions(+), 7 deletions(-)
diff --git a/drivers/iio/imu/adis16607.c b/drivers/iio/imu/adis16607.c
index b1f3667d8ce3..3c5b59820be1 100644
--- a/drivers/iio/imu/adis16607.c
+++ b/drivers/iio/imu/adis16607.c
@@ -7,6 +7,7 @@
#include <linux/array_size.h>
#include <linux/bitfield.h>
#include <linux/bitops.h>
+#include <linux/clk.h>
#include <linux/debugfs.h>
#include <linux/delay.h>
#include <linux/iio/buffer.h>
@@ -48,6 +49,7 @@
#define ADIS16607_REG_FIFO_WORD_CNT 0x2B
#define ADIS16607_REG_USER_GPIO_CFG 0x2F
#define ADIS16607_REG_SPI_HALFDUPLEX 0x32
+#define ADIS16607_REG_USER_SYNC 0x33
#define ADIS16607_REG_USER_DATA_CFG 0x34
#define ADIS16607_REG_USER_FIFO_CFG 0x35
#define ADIS16607_REG_SOFT_RESET 0x36
@@ -60,10 +62,12 @@
/* Masks */
#define ADIS16607_RESET_GPIO_MASK GENMASK(2, 0)
+#define ADIS16607_SYNC_GPIO_MASK GENMASK(8, 6)
#define ADIS16607_DR_GPIO_MASK GENMASK(11, 9)
#define ADIS16607_DEV_NOK_MASK GENMASK(14, 12)
#define ADIS16607_DATA_CNTR_EN_MASK BIT(14)
#define ADIS16607_BURST32_MASK BIT(15)
+#define ADIS16607_SYNC_MODE_MASK BIT(15)
#define ADIS16607_BOOTLOADER_BUSY_MASK BIT(0)
/* FIFO masks */
@@ -90,8 +94,22 @@
#define ADIS16607_ACCEL_Z_DELTA_MAX 4000
#define ADIS16607_GYRO_DELTA_MAX 2600
+/* Sync modes */
+enum adis16607_sync_mode {
+ ADIS16607_SYNC_DEFAULT,
+ ADIS16607_SYNC_DIRECT,
+ ADIS16607_SYNC_SCALED,
+};
+
+struct adis16607_sync {
+ u16 sync_mode;
+ u16 min_rate;
+ u16 max_rate;
+};
+
struct adis16607_chip_info {
const struct iio_chan_spec *channels;
+ const struct adis16607_sync *sync_mode;
const char *name;
u32 num_channels;
u32 gyro_max_val;
@@ -103,6 +121,7 @@ struct adis16607_chip_info {
u32 deltvel_max_val;
u32 int_clk_hz;
u16 max_dec;
+ u8 num_sync;
};
struct adis16607 {
@@ -110,6 +129,7 @@ struct adis16607 {
const struct adis16607_chip_info *info;
unsigned long clk_freq_mhz;
u64 sample_period_ns;
+ u32 sync_mode;
u16 fifo_watermark;
/* Scratch buffer used for FIFO pop operations, see adis16607_fifo_pop() */
u8 fifo_buf[ADIS16607_BURST_CMD_SIZE + ADIS16607_FIFO_DATA_LEN] __aligned(IIO_DMA_MINALIGN);
@@ -1004,6 +1024,12 @@ static const struct iio_info adis16607_info = {
.hwfifo_set_watermark = adis16607_set_watermark,
};
+static const struct adis16607_sync adis16607_sync_modes[] = {
+ { ADIS16607_SYNC_DEFAULT, 0, 0 },
+ { ADIS16607_SYNC_DIRECT, 401, 8000 },
+ { ADIS16607_SYNC_SCALED, 1, 400 },
+};
+
static const char * const adis16607_status_error_msgs[] = {
[9] = "Boot Memory Failure",
[11] = "Power Supply Failure",
@@ -1048,6 +1074,8 @@ static const struct adis16607_chip_info adis16607_2_chip_info = {
.deltvel_max_val = IIO_G_TO_M_S_2(40),
.int_clk_hz = 8000,
.max_dec = 65535,
+ .sync_mode = adis16607_sync_modes,
+ .num_sync = ARRAY_SIZE(adis16607_sync_modes),
};
static const struct adis16607_chip_info adis16607_3_chip_info = {
@@ -1063,17 +1091,96 @@ static const struct adis16607_chip_info adis16607_3_chip_info = {
.deltvel_max_val = IIO_G_TO_M_S_2(40),
.int_clk_hz = 8000,
.max_dec = 65535,
+ .sync_mode = adis16607_sync_modes,
+ .num_sync = ARRAY_SIZE(adis16607_sync_modes),
};
-/*
- * No external clock sync support yet: always run on the internal clock.
- * Default dec=0 after reset, so period = 1 / clk_freq_mhz.
- */
static int adis16607_config_clk(struct adis16607 *st)
{
- st->clk_freq_mhz = st->info->int_clk_hz * MILLI;
- st->sample_period_ns =
- DIV_ROUND_CLOSEST_ULL((u64)NSEC_PER_SEC * MILLI, st->clk_freq_mhz);
+ struct device *dev = &st->adis.spi->dev;
+ const struct adis16607_sync *sync_mode_data = NULL;
+ unsigned long ext_clk_hz = 0;
+ u16 sync_scale = 0;
+ u16 dec;
+ struct clk *clk;
+ int ret, i;
+
+ clk = devm_clk_get_optional_enabled(dev, NULL);
+ if (IS_ERR(clk))
+ return PTR_ERR(clk);
+
+ if (clk) {
+ ext_clk_hz = clk_get_rate(clk);
+
+ for (i = 1; i < st->info->num_sync; i++) {
+ if (ext_clk_hz >= st->info->sync_mode[i].min_rate &&
+ ext_clk_hz <= st->info->sync_mode[i].max_rate) {
+ sync_mode_data = &st->info->sync_mode[i];
+ break;
+ }
+ }
+
+ if (!sync_mode_data)
+ return dev_err_probe(dev, -EINVAL,
+ "Clk rate: %lu not in a valid range\n",
+ ext_clk_hz);
+
+ st->sync_mode = sync_mode_data->sync_mode;
+ } else {
+ st->sync_mode = ADIS16607_SYNC_DEFAULT;
+ }
+
+ /*
+ * Write the full desired sync state explicitly for every mode,
+ * rather than only when enabling non-default sync: USER_SYNC's
+ * documented reset default (0x0000, direct mode / scale 0) means
+ * this isn't fixing a known reset-state issue, but each mode
+ * establishing its own complete state here, instead of only
+ * touching bits conditionally, is simpler to reason about and
+ * doesn't depend on that default holding true.
+ */
+ ret = adis_update_bits_base(&st->adis, ADIS16607_REG_USER_GPIO_CFG,
+ ADIS16607_SYNC_GPIO_MASK,
+ FIELD_PREP(ADIS16607_SYNC_GPIO_MASK,
+ st->sync_mode != ADIS16607_SYNC_DEFAULT), 2);
+ if (ret)
+ return ret;
+
+ if (st->sync_mode == ADIS16607_SYNC_SCALED)
+ sync_scale = DIV_ROUND_CLOSEST(st->info->int_clk_hz, ext_clk_hz);
+
+ ret = adis_write_reg_16(&st->adis, ADIS16607_REG_USER_SYNC,
+ (st->sync_mode == ADIS16607_SYNC_SCALED) ?
+ ADIS16607_SYNC_MODE_MASK | sync_scale : 0);
+ if (ret)
+ return ret;
+
+ switch (st->sync_mode) {
+ case ADIS16607_SYNC_SCALED:
+ /*
+ * sync_scale is a rounded integer, so the hardware's actual
+ * effective rate is ext_clk_hz * sync_scale, not necessarily
+ * int_clk_hz - use the real value here so sample_period_ns (and
+ * therefore FIFO sample timestamps) don't drift from it.
+ */
+ st->clk_freq_mhz = (unsigned long)ext_clk_hz * sync_scale * 1000;
+ break;
+ case ADIS16607_SYNC_DIRECT:
+ /* DIRECT: external clock is the effective internal rate */
+ st->clk_freq_mhz = ext_clk_hz * 1000;
+ break;
+ default:
+ st->clk_freq_mhz = (unsigned long)st->info->int_clk_hz * 1000;
+ break;
+ }
+
+ ret = adis_read_reg_16(&st->adis, ADIS16607_REG_DEC_RATE, &dec);
+ if (ret)
+ return ret;
+
+ st->sample_period_ns = DIV_ROUND_CLOSEST_ULL((u64)NSEC_PER_SEC * 1000ULL * (dec + 1),
+ st->clk_freq_mhz);
+
return 0;
}
--
2.43.0
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: [PATCH v4 7/8] iio: imu: adis16607: Add external clock sync support
2026-10-23 9:30 ` [PATCH v4 7/8] iio: imu: adis16607: Add external clock sync support Radu Sabau via B4 Relay
@ 2026-10-26 0:11 ` Jonathan Cameron
2026-10-26 10:54 ` Sabau, Radu bogdan
0 siblings, 1 reply; 19+ messages in thread
From: Jonathan Cameron @ 2026-10-26 0:11 UTC (permalink / raw)
To: Radu Sabau via B4 Relay
Cc: radu.sabau, Lars-Peter Clausen, Michael Hennerich, Nuno Sa,
David Lechner, Andy Shevchenko, Alexandru Ardelean, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, linux-iio, linux-kernel,
devicetree, linux
On Fri, 23 Oct 2026 12:30:24 +0300
Radu Sabau via B4 Relay <devnull+radu.sabau.analog.com@kernel.org> wrote:
> From: Radu Sabau <radu.sabau@analog.com>
>
> Add support for an optional external clock (clocks property),
> selecting between SYNC_DIRECT and SYNC_SCALED modes depending on the
> supplied frequency, with the internal 8 kHz clock used as the default
> when no clock is provided.
>
> Signed-off-by: Radu Sabau <radu.sabau@analog.com>
> ---
> -/*
> - * No external clock sync support yet: always run on the internal clock.
> - * Default dec=0 after reset, so period = 1 / clk_freq_mhz.
> - */
> static int adis16607_config_clk(struct adis16607 *st)
> {
> - st->clk_freq_mhz = st->info->int_clk_hz * MILLI;
> - st->sample_period_ns =
> - DIV_ROUND_CLOSEST_ULL((u64)NSEC_PER_SEC * MILLI, st->clk_freq_mhz);
> + struct device *dev = &st->adis.spi->dev;
> + const struct adis16607_sync *sync_mode_data = NULL;
> + unsigned long ext_clk_hz = 0;
> + u16 sync_scale = 0;
> + u16 dec;
> + struct clk *clk;
> + int ret, i;
where there is no other particular reason for ordering, reverse xmas tree
preferred.
> +
> + clk = devm_clk_get_optional_enabled(dev, NULL);
> + if (IS_ERR(clk))
> + return PTR_ERR(clk);
> +
> + if (clk) {
> + ext_clk_hz = clk_get_rate(clk);
Hmm. What if it is a controllable clock?
> +
> + for (i = 1; i < st->info->num_sync; i++) {
for (unsigned int i = 1; ...
> + if (ext_clk_hz >= st->info->sync_mode[i].min_rate &&
> + ext_clk_hz <= st->info->sync_mode[i].max_rate) {
> + sync_mode_data = &st->info->sync_mode[i];
> + break;
> + }
> + }
> +
> + if (!sync_mode_data)
> + return dev_err_probe(dev, -EINVAL,
> + "Clk rate: %lu not in a valid range\n",
> + ext_clk_hz);
> +
> + st->sync_mode = sync_mode_data->sync_mode;
> + } else {
> + st->sync_mode = ADIS16607_SYNC_DEFAULT;
> + }
> +
> + /*
> + * Write the full desired sync state explicitly for every mode,
> + * rather than only when enabling non-default sync: USER_SYNC's
> + * documented reset default (0x0000, direct mode / scale 0) means
> + * this isn't fixing a known reset-state issue, but each mode
> + * establishing its own complete state here, instead of only
> + * touching bits conditionally, is simpler to reason about and
> + * doesn't depend on that default holding true.
> + */
> + ret = adis_update_bits_base(&st->adis, ADIS16607_REG_USER_GPIO_CFG,
> + ADIS16607_SYNC_GPIO_MASK,
> + FIELD_PREP(ADIS16607_SYNC_GPIO_MASK,
> + st->sync_mode != ADIS16607_SYNC_DEFAULT), 2);
> + if (ret)
> + return ret;
> +
> + if (st->sync_mode == ADIS16607_SYNC_SCALED)
> + sync_scale = DIV_ROUND_CLOSEST(st->info->int_clk_hz, ext_clk_hz);
> +
> + ret = adis_write_reg_16(&st->adis, ADIS16607_REG_USER_SYNC,
> + (st->sync_mode == ADIS16607_SYNC_SCALED) ?
> + ADIS16607_SYNC_MODE_MASK | sync_scale : 0);
> + if (ret)
> + return ret;
> +
> + switch (st->sync_mode) {
> + case ADIS16607_SYNC_SCALED:
> + /*
> + * sync_scale is a rounded integer, so the hardware's actual
> + * effective rate is ext_clk_hz * sync_scale, not necessarily
> + * int_clk_hz - use the real value here so sample_period_ns (and
> + * therefore FIFO sample timestamps) don't drift from it.
> + */
> + st->clk_freq_mhz = (unsigned long)ext_clk_hz * sync_scale * 1000;
> + break;
> + case ADIS16607_SYNC_DIRECT:
> + /* DIRECT: external clock is the effective internal rate */
> + st->clk_freq_mhz = ext_clk_hz * 1000;
> + break;
> + default:
> + st->clk_freq_mhz = (unsigned long)st->info->int_clk_hz * 1000;
> + break;
> + }
> +
> + ret = adis_read_reg_16(&st->adis, ADIS16607_REG_DEC_RATE, &dec);
> + if (ret)
> + return ret;
> +
> + st->sample_period_ns = DIV_ROUND_CLOSEST_ULL((u64)NSEC_PER_SEC * 1000ULL * (dec + 1),
> + st->clk_freq_mhz);
> +
> return 0;
> }
>
>
^ permalink raw reply [flat|nested] 19+ messages in thread* RE: [PATCH v4 7/8] iio: imu: adis16607: Add external clock sync support
2026-10-26 0:11 ` Jonathan Cameron
@ 2026-10-26 10:54 ` Sabau, Radu bogdan
0 siblings, 0 replies; 19+ messages in thread
From: Sabau, Radu bogdan @ 2026-10-26 10:54 UTC (permalink / raw)
To: Jonathan Cameron, Radu Sabau via B4 Relay
Cc: Lars-Peter Clausen, Hennerich, Michael, Sa, Nuno, David Lechner,
Andy Shevchenko, Alexandru Ardelean, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, linux-iio, linux-kernel,
devicetree, linux
> -----Original Message-----
> From: Jonathan Cameron <jic23@kernel.org>
> Sent: Monday, October 26, 2026 2:12 AM
...
> > +
> > + clk = devm_clk_get_optional_enabled(dev, NULL);
> > + if (IS_ERR(clk))
> > + return PTR_ERR(clk);
> > +
> > + if (clk) {
> > + ext_clk_hz = clk_get_rate(clk);
>
> Hmm. What if it is a controllable clock?
Good point. What I'm thinking: add a clk_notifier (same pattern as
adf4377_freq_change() in drivers/iio/frequency/adf4377.c) that
re-runs the sync/scale setup on POST_RATE_CHANGE, under the same
state_lock the trigger handler already uses, so it can't race with
an in-flight FIFO drain. Splitting adis16607_config_clk() into a
probe-time wrapper (gets the clk, registers the notifier if one's
present) and the actual setup logic, so the notifier can re-run just
the latter. A fixed-rate clock would simply never trigger it in
practice, so this covers both cases with the same code.
Does that work for you?
^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH v4 8/8] iio: imu: adis16607: Add calibration bias support for gyro/accel
2026-10-23 9:30 [PATCH v4 0/8] iio: imu: Add support for the ADI ADIS16607 Radu Sabau via B4 Relay
` (6 preceding siblings ...)
2026-10-23 9:30 ` [PATCH v4 7/8] iio: imu: adis16607: Add external clock sync support Radu Sabau via B4 Relay
@ 2026-10-23 9:30 ` Radu Sabau via B4 Relay
2026-10-26 0:15 ` Jonathan Cameron
7 siblings, 1 reply; 19+ messages in thread
From: Radu Sabau via B4 Relay @ 2026-10-23 9:30 UTC (permalink / raw)
To: Lars-Peter Clausen, Michael Hennerich, Nuno Sa, Jonathan Cameron,
David Lechner, Andy Shevchenko, Alexandru Ardelean, Radu Sabau,
Rob Herring, Krzysztof Kozlowski, Conor Dooley
Cc: linux-iio, linux-kernel, devicetree, linux, Jonathan Cameron
From: Radu Sabau <radu.sabau@analog.com>
Add IIO_CHAN_INFO_CALIBBIAS support for the gyroscope and
accelerometer axes, backed by the device's per-axis bias registers
(XA/YA/ZA_BIAS, XG/YG/ZG_BIAS).
Signed-off-by: Radu Sabau <radu.sabau@analog.com>
---
drivers/iio/imu/adis16607.c | 35 ++++++++++++++++++++++++++++++++++-
1 file changed, 34 insertions(+), 1 deletion(-)
diff --git a/drivers/iio/imu/adis16607.c b/drivers/iio/imu/adis16607.c
index 3c5b59820be1..f73df022e819 100644
--- a/drivers/iio/imu/adis16607.c
+++ b/drivers/iio/imu/adis16607.c
@@ -55,6 +55,12 @@
#define ADIS16607_REG_SOFT_RESET 0x36
#define ADIS16607_REG_SELF_TEST 0x39
#define ADIS16607_REG_DEC_RATE 0x3A
+#define ADIS16607_REG_XA_BIAS 0x3B
+#define ADIS16607_REG_YA_BIAS 0x3C
+#define ADIS16607_REG_ZA_BIAS 0x3D
+#define ADIS16607_REG_XG_BIAS 0x3E
+#define ADIS16607_REG_YG_BIAS 0x3F
+#define ADIS16607_REG_ZG_BIAS 0x40
#define ADIS16607_REG_DIGITAL_STATUS 0x4E
/* SPI Half-Duplex lock value */
@@ -153,6 +159,15 @@ enum {
ADIS16607_SCAN_TEMP,
};
+static const u32 adis16607_calib_bias[] = {
+ [ADIS16607_SCAN_ACCEL_X] = ADIS16607_REG_XA_BIAS,
+ [ADIS16607_SCAN_ACCEL_Y] = ADIS16607_REG_YA_BIAS,
+ [ADIS16607_SCAN_ACCEL_Z] = ADIS16607_REG_ZA_BIAS,
+ [ADIS16607_SCAN_GYRO_X] = ADIS16607_REG_XG_BIAS,
+ [ADIS16607_SCAN_GYRO_Y] = ADIS16607_REG_YG_BIAS,
+ [ADIS16607_SCAN_GYRO_Z] = ADIS16607_REG_ZG_BIAS,
+};
+
/*
* SPI Half-Duplex read operation for ADIS16607.
* Protocol:
@@ -863,6 +878,7 @@ static int adis16607_read_raw(struct iio_dev *indio_dev,
int *val, int *val2, long info)
{
struct adis16607 *st = iio_priv(indio_dev);
+ const int idx = chan->scan_index;
int ret;
u32 tmp;
@@ -900,6 +916,16 @@ static int adis16607_read_raw(struct iio_dev *indio_dev,
*val = 5000;
return IIO_VAL_INT;
+ case IIO_CHAN_INFO_CALIBBIAS: {
+ u16 tmp16;
+
+ ret = adis_read_reg_16(&st->adis, adis16607_calib_bias[idx], &tmp16);
+ if (ret)
+ return ret;
+ *val = sign_extend32(tmp16, 15);
+ return IIO_VAL_INT;
+ }
+
case IIO_CHAN_INFO_SAMP_FREQ:
ret = adis16607_get_freq(st, &tmp);
if (ret)
@@ -918,6 +944,7 @@ static int adis16607_write_raw(struct iio_dev *indio_dev,
int val, int val2, long info)
{
struct adis16607 *st = iio_priv(indio_dev);
+ const int idx = chan->scan_index;
u32 tmp;
switch (info) {
@@ -925,6 +952,11 @@ static int adis16607_write_raw(struct iio_dev *indio_dev,
tmp = val * 1000 + val2 / 1000;
return adis16607_set_freq(st, tmp);
+ case IIO_CHAN_INFO_CALIBBIAS:
+ if (val < S16_MIN || val > S16_MAX)
+ return -EINVAL;
+ return adis_write_reg_16(&st->adis, adis16607_calib_bias[idx], val);
+
default:
return -EINVAL;
}
@@ -934,7 +966,8 @@ static int adis16607_write_raw(struct iio_dev *indio_dev,
.type = (_type), \
.modified = 1, \
.channel2 = (_mod), \
- .info_mask_separate = BIT(IIO_CHAN_INFO_RAW), \
+ .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) | \
+ BIT(IIO_CHAN_INFO_CALIBBIAS), \
.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE), \
.info_mask_shared_by_all = BIT(IIO_CHAN_INFO_SAMP_FREQ), \
.address = (_address), \
--
2.43.0
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: [PATCH v4 8/8] iio: imu: adis16607: Add calibration bias support for gyro/accel
2026-10-23 9:30 ` [PATCH v4 8/8] iio: imu: adis16607: Add calibration bias support for gyro/accel Radu Sabau via B4 Relay
@ 2026-10-26 0:15 ` Jonathan Cameron
0 siblings, 0 replies; 19+ messages in thread
From: Jonathan Cameron @ 2026-10-26 0:15 UTC (permalink / raw)
To: Radu Sabau via B4 Relay
Cc: radu.sabau, Lars-Peter Clausen, Michael Hennerich, Nuno Sa,
David Lechner, Andy Shevchenko, Alexandru Ardelean, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, linux-iio, linux-kernel,
devicetree, linux
On Fri, 23 Oct 2026 12:30:25 +0300
Radu Sabau via B4 Relay <devnull+radu.sabau.analog.com@kernel.org> wrote:
> From: Radu Sabau <radu.sabau@analog.com>
>
> Add IIO_CHAN_INFO_CALIBBIAS support for the gyroscope and
> accelerometer axes, backed by the device's per-axis bias registers
> (XA/YA/ZA_BIAS, XG/YG/ZG_BIAS).
>
> Signed-off-by: Radu Sabau <radu.sabau@analog.com>
I'd missed the use of scan_index in here. That seems like very
fragile coding to me. Moving over to address is going to require
changes a few patches back.
> +
> /*
> * SPI Half-Duplex read operation for ADIS16607.
> * Protocol:
> @@ -863,6 +878,7 @@ static int adis16607_read_raw(struct iio_dev *indio_dev,
> int *val, int *val2, long info)
> {
> struct adis16607 *st = iio_priv(indio_dev);
> + const int idx = chan->scan_index;
As below. This to me is hurting readability a little.
Using scan_index for this is a bit fragile anyway as those can
be changed by people expecting them to only about buffered capture.
This should be using address and that may well mean you need that
to become a lookup into an array of multiple register addresses
needed for a given channel rather than directly to one register as
currently.
> int ret;
> u32 tmp;
>
> @@ -900,6 +916,16 @@ static int adis16607_read_raw(struct iio_dev *indio_dev,
> *val = 5000;
> return IIO_VAL_INT;
>
> + case IIO_CHAN_INFO_CALIBBIAS: {
> + u16 tmp16;
> +
> + ret = adis_read_reg_16(&st->adis, adis16607_calib_bias[idx], &tmp16);
> + if (ret)
> + return ret;
> + *val = sign_extend32(tmp16, 15);
> + return IIO_VAL_INT;
> + }
> +
> case IIO_CHAN_INFO_SAMP_FREQ:
> ret = adis16607_get_freq(st, &tmp);
> if (ret)
> @@ -918,6 +944,7 @@ static int adis16607_write_raw(struct iio_dev *indio_dev,
> int val, int val2, long info)
> {
> struct adis16607 *st = iio_priv(indio_dev);
> + const int idx = chan->scan_index;
I'd just go long on the line that uses this or at least push declaring it
down to the case statement that uses it.
> u32 tmp;
>
> switch (info) {
> @@ -925,6 +952,11 @@ static int adis16607_write_raw(struct iio_dev *indio_dev,
> tmp = val * 1000 + val2 / 1000;
> return adis16607_set_freq(st, tmp);
>
> + case IIO_CHAN_INFO_CALIBBIAS:
> + if (val < S16_MIN || val > S16_MAX)
> + return -EINVAL;
> + return adis_write_reg_16(&st->adis, adis16607_calib_bias[idx], val);
> +
> default:
> return -EINVAL;
> }
> @@ -934,7 +966,8 @@ static int adis16607_write_raw(struct iio_dev *indio_dev,
> .type = (_type), \
> .modified = 1, \
> .channel2 = (_mod), \
> - .info_mask_separate = BIT(IIO_CHAN_INFO_RAW), \
> + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) | \
> + BIT(IIO_CHAN_INFO_CALIBBIAS), \
> .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE), \
> .info_mask_shared_by_all = BIT(IIO_CHAN_INFO_SAMP_FREQ), \
> .address = (_address), \
>
^ permalink raw reply [flat|nested] 19+ messages in thread