* [PATCH v3 1/6] iio: imu: adis: Add optional self_test callback and fix custom reset dispatch
2026-10-15 9:32 [PATCH v3 0/6] iio: imu: Add support for the ADI ADIS16607 Radu Sabau via B4 Relay
@ 2026-10-15 9:32 ` Radu Sabau via B4 Relay
2026-10-15 9:40 ` sashiko-bot
` (3 more replies)
2026-10-15 9:32 ` [PATCH v3 2/6] dt-bindings: iio: imu: Add bindings for ADI ADIS16607 Radu Sabau via B4 Relay
` (4 subsequent siblings)
5 siblings, 4 replies; 27+ messages in thread
From: Radu Sabau via B4 Relay @ 2026-10-15 9:32 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, 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.
Also fix __adis_initial_startup()'s no-reset-gpio path: it 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, adding the settle-time sleep at
the call site for custom ops (documented not to sleep internally)
while skipping it for the default op (which already sleeps
internally).
Fixes: ecb010d44108 ("iio: imu: adis: Refactor adis_initial_startup")
Signed-off-by: Radu Sabau <radu.sabau@analog.com>
---
drivers/iio/imu/adis.c | 30 +++++++++++++++++++++++-------
include/linux/iio/imu/adis.h | 32 +++++++++++++++++++++++++++++++-
2 files changed, 54 insertions(+), 8 deletions(-)
diff --git a/drivers/iio/imu/adis.c b/drivers/iio/imu/adis.c
index a2bc1d14ed91..448046fd2ada 100644
--- a/drivers/iio/imu/adis.c
+++ b/drivers/iio/imu/adis.c
@@ -355,11 +355,20 @@ int __adis_reset(struct adis *adis)
}
EXPORT_SYMBOL_NS_GPL(__adis_reset, "IIO_ADIS_LIB");
+static const struct adis_ops adis_default_ops = {
+ .read = __adis_read_reg,
+ .write = __adis_write_reg,
+ .reset = __adis_reset,
+};
+
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) {
@@ -413,7 +422,20 @@ 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;
+ /*
+ * __adis_reset() already sleeps the settle time internally.
+ * Custom reset ops are documented (struct adis_ops::reset)
+ * not to, so sleep here for them.
+ */
+ if (adis->ops->reset != __adis_reset)
+ msleep(timeouts->sw_reset_ms);
+ }
+
+ if (adis->data->post_reset_setup) {
+ ret = adis->data->post_reset_setup(adis);
if (ret)
return ret;
}
@@ -491,12 +513,6 @@ int adis_single_conversion(struct iio_dev *indio_dev,
}
EXPORT_SYMBOL_NS_GPL(adis_single_conversion, "IIO_ADISLIB");
-static const struct adis_ops adis_default_ops = {
- .read = __adis_read_reg,
- .write = __adis_write_reg,
- .reset = __adis_reset,
-};
-
/**
* adis_init() - Initialize adis device structure
* @adis: The adis device
diff --git a/include/linux/iio/imu/adis.h b/include/linux/iio/imu/adis.h
index bfb6df68e6c9..5d0baeded3c9 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>
@@ -51,6 +52,18 @@ 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
+ * 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 +94,8 @@ 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;
const char * const *status_error_msgs;
@@ -171,8 +186,23 @@ 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;
+
+ /*
+ * __adis_reset() already sleeps the settle time internally. Custom
+ * reset ops are documented (struct adis_ops::reset) not to, so sleep
+ * here for them.
+ */
+ if (adis->ops->reset != __adis_reset)
+ 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 related [flat|nested] 27+ messages in thread* Re: [PATCH v3 1/6] iio: imu: adis: Add optional self_test callback and fix custom reset dispatch
2026-10-15 9:32 ` [PATCH v3 1/6] iio: imu: adis: Add optional self_test callback and fix custom reset dispatch Radu Sabau via B4 Relay
@ 2026-10-15 9:40 ` sashiko-bot
2026-10-15 18:50 ` Andy Shevchenko
` (2 subsequent siblings)
3 siblings, 0 replies; 27+ messages in thread
From: sashiko-bot @ 2026-10-15 9:40 UTC (permalink / raw)
To: Radu Sabau; +Cc: devicetree, robh, conor+dt
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] iio: imu: adis: undocumented addition of post_reset_setup callback
--
commit 6c20542e733281223a77056f3199ffa32200b12f
Author: Radu Sabau <radu.sabau@analog.com>
iio: imu: adis: Add optional self_test callback and fix custom reset dispatch
This commit adds an optional self_test callback for devices with custom
test sequences and fixes the no-reset-gpio startup path to use the driver
provided reset ops instead of directly calling __adis_reset().
> diff --git a/drivers/iio/imu/adis.c b/drivers/iio/imu/adis.c
> index a2bc1d14ed91d..448046fd2ada4 100644
> --- a/drivers/iio/imu/adis.c
> +++ b/drivers/iio/imu/adis.c
[ ... ]
> @@ -413,7 +422,20 @@ int __adis_initial_startup(struct adis *adis)
[ ... ]
> - ret = __adis_reset(adis);
> + ret = adis->ops->reset(adis);
> + if (ret)
> + return ret;
> + /*
> + * __adis_reset() already sleeps the settle time internally.
> + * Custom reset ops are documented (struct adis_ops::reset)
> + * not to, so sleep here for them.
> + */
> + if (adis->ops->reset != __adis_reset)
> + msleep(timeouts->sw_reset_ms);
> + }
> +
> + if (adis->data->post_reset_setup) {
> + ret = adis->data->post_reset_setup(adis);
[Severity: Low]
The commit message explicitly details the self_test callback addition and the
changes to the reset dispatch path, but it doesn't mention this new
post_reset_setup callback being added to the initialization sequence. Could
the commit message be updated to document this new API addition?
> if (ret)
> return ret;
> }
[ ... ]
> diff --git a/include/linux/iio/imu/adis.h b/include/linux/iio/imu/adis.h
> index bfb6df68e6c94..5d0baeded3c92 100644
> --- a/include/linux/iio/imu/adis.h
> +++ b/include/linux/iio/imu/adis.h
[ ... ]
> @@ -81,6 +94,8 @@ 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);
[Severity: Low]
Similar to the above, should this struct adis_data API addition be mentioned
in the commit message so that developers are aware of the newly available
bring-up hook?
> + int (*self_test)(struct adis *adis);
> const struct adis_timeout *timeouts;
>
> const char * const *status_error_msgs;
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261015-adis16607-v3-0-f972d4efee85@analog.com?part=1
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [PATCH v3 1/6] iio: imu: adis: Add optional self_test callback and fix custom reset dispatch
2026-10-15 9:32 ` [PATCH v3 1/6] iio: imu: adis: Add optional self_test callback and fix custom reset dispatch Radu Sabau via B4 Relay
2026-10-15 9:40 ` sashiko-bot
@ 2026-10-15 18:50 ` Andy Shevchenko
2026-10-17 1:06 ` Jonathan Cameron
2026-10-22 14:37 ` Nuno Sá
3 siblings, 0 replies; 27+ messages in thread
From: Andy Shevchenko @ 2026-10-15 18:50 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
On Thu, Oct 15, 2026 at 12:32:58PM +0300, Radu Sabau via B4 Relay wrote:
> 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.
>
> Also fix __adis_initial_startup()'s no-reset-gpio path: it 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, adding the settle-time sleep at
> the call site for custom ops (documented not to sleep internally)
> while skipping it for the default op (which already sleeps
> internally).
...
> - ret = __adis_reset(adis);
> + ret = adis->ops->reset(adis);
> + if (ret)
> + return ret;
> + /*
> + * __adis_reset() already sleeps the settle time internally.
> + * Custom reset ops are documented (struct adis_ops::reset)
> + * not to, so sleep here for them.
> + */
> + if (adis->ops->reset != __adis_reset)
> + msleep(timeouts->sw_reset_ms);
Can't we simply stubbed ->reset to a wrapper on top of the msleep()?
...
> 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;
> +
> + /*
> + * __adis_reset() already sleeps the settle time internally. Custom
> + * reset ops are documented (struct adis_ops::reset) not to, so sleep
> + * here for them.
> + */
> + if (adis->ops->reset != __adis_reset)
> + msleep(adis->data->timeouts->sw_reset_ms);
It doesn't look scalable to have this accompanying each ->reset() call.
> + return 0;
> }
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [PATCH v3 1/6] iio: imu: adis: Add optional self_test callback and fix custom reset dispatch
2026-10-15 9:32 ` [PATCH v3 1/6] iio: imu: adis: Add optional self_test callback and fix custom reset dispatch Radu Sabau via B4 Relay
2026-10-15 9:40 ` sashiko-bot
2026-10-15 18:50 ` Andy Shevchenko
@ 2026-10-17 1:06 ` Jonathan Cameron
2026-10-22 14:37 ` Nuno Sá
3 siblings, 0 replies; 27+ messages in thread
From: Jonathan Cameron @ 2026-10-17 1:06 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
On Thu, 15 Oct 2026 12:32:58 +0300
Radu Sabau via B4 Relay <devnull+radu.sabau.analog.com@kernel.org> wrote:
> 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.
>
> Also fix __adis_initial_startup()'s no-reset-gpio path: it called
'Also' rings alarm bells. Why not a separate patch for this?
> __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, adding the settle-time sleep at
> the call site for custom ops (documented not to sleep internally)
> while skipping it for the default op (which already sleeps
> internally).
Why don't we stop __adis_reset() from having an internal sleep
and simplify the code?
There is a missing 'also'. The introduction of post_reset_setup().
That should be a third patch.
>
> Fixes: ecb010d44108 ("iio: imu: adis: Refactor adis_initial_startup")
> Signed-off-by: Radu Sabau <radu.sabau@analog.com>
> ---
> drivers/iio/imu/adis.c | 30 +++++++++++++++++++++++-------
> include/linux/iio/imu/adis.h | 32 +++++++++++++++++++++++++++++++-
> 2 files changed, 54 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/iio/imu/adis.c b/drivers/iio/imu/adis.c
> index a2bc1d14ed91..448046fd2ada 100644
> --- a/drivers/iio/imu/adis.c
> +++ b/drivers/iio/imu/adis.c
> @@ -355,11 +355,20 @@ int __adis_reset(struct adis *adis)
> }
> EXPORT_SYMBOL_NS_GPL(__adis_reset, "IIO_ADIS_LIB");
>
> +static const struct adis_ops adis_default_ops = {
> + .read = __adis_read_reg,
> + .write = __adis_write_reg,
> + .reset = __adis_reset,
> +};
> +
> 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) {
> @@ -413,7 +422,20 @@ 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;
> + /*
> + * __adis_reset() already sleeps the settle time internally.
> + * Custom reset ops are documented (struct adis_ops::reset)
> + * not to, so sleep here for them.
> + */
> + if (adis->ops->reset != __adis_reset)
> + msleep(timeouts->sw_reset_ms);
> + }
> +
> + if (adis->data->post_reset_setup) {
> + ret = adis->data->post_reset_setup(adis);
> if (ret)
> return ret;
> }
> @@ -491,12 +513,6 @@ int adis_single_conversion(struct iio_dev *indio_dev,
> }
> EXPORT_SYMBOL_NS_GPL(adis_single_conversion, "IIO_ADISLIB");
>
> -static const struct adis_ops adis_default_ops = {
> - .read = __adis_read_reg,
> - .write = __adis_write_reg,
> - .reset = __adis_reset,
> -};
> -
> /**
> * adis_init() - Initialize adis device structure
> * @adis: The adis device
> diff --git a/include/linux/iio/imu/adis.h b/include/linux/iio/imu/adis.h
> index bfb6df68e6c9..5d0baeded3c9 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>
> @@ -51,6 +52,18 @@ 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.
This doesn't seem related to the rest. Separate patch I think.
> + * @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 +94,8 @@ 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;
>
> const char * const *status_error_msgs;
> @@ -171,8 +186,23 @@ 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;
> +
> + /*
> + * __adis_reset() already sleeps the settle time internally. Custom
> + * reset ops are documented (struct adis_ops::reset) not to, so sleep
> + * here for them.
> + */
> + if (adis->ops->reset != __adis_reset)
> + msleep(adis->data->timeouts->sw_reset_ms);
> +
> + return 0;
> }
>
> int __adis_write_reg(struct adis *adis, unsigned int reg,
>
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [PATCH v3 1/6] iio: imu: adis: Add optional self_test callback and fix custom reset dispatch
2026-10-15 9:32 ` [PATCH v3 1/6] iio: imu: adis: Add optional self_test callback and fix custom reset dispatch Radu Sabau via B4 Relay
` (2 preceding siblings ...)
2026-10-17 1:06 ` Jonathan Cameron
@ 2026-10-22 14:37 ` Nuno Sá
3 siblings, 0 replies; 27+ messages in thread
From: Nuno Sá @ 2026-10-22 14:37 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
On Thu, 15 Oct 2026 12:32:58 +0300, Radu Sabau <radu.sabau@analog.com> wrote:
> diff --git a/drivers/iio/imu/adis.c b/drivers/iio/imu/adis.c
> index a2bc1d14ed91..448046fd2ada 100644
> --- a/drivers/iio/imu/adis.c
> +++ b/drivers/iio/imu/adis.c
> @@ -413,7 +422,20 @@ 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;
> + /*
> + * __adis_reset() already sleeps the settle time internally.
> + * Custom reset ops are documented (struct adis_ops::reset)
> + * not to, so sleep here for them.
> + */
> + if (adis->ops->reset != __adis_reset)
> + msleep(timeouts->sw_reset_ms);
Agree with Andy. Don't really like the above. Maybe we just have the one
explicit msleep() call in here (actually out of the if () else block)
and remove it from the internal __adis_reset(). Hence we can just call
the sleep() in adis_reset().
--
Nuno Sá <nuno.sa@analog.com>
^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH v3 2/6] dt-bindings: iio: imu: Add bindings for ADI ADIS16607
2026-10-15 9:32 [PATCH v3 0/6] iio: imu: Add support for the ADI ADIS16607 Radu Sabau via B4 Relay
2026-10-15 9:32 ` [PATCH v3 1/6] iio: imu: adis: Add optional self_test callback and fix custom reset dispatch Radu Sabau via B4 Relay
@ 2026-10-15 9:32 ` Radu Sabau via B4 Relay
2026-10-15 20:43 ` Rob Herring (Arm)
2026-10-17 1:11 ` Jonathan Cameron
2026-10-15 9:33 ` [PATCH v3 3/6] iio: imu: Add driver for the " Radu Sabau via B4 Relay
` (3 subsequent siblings)
5 siblings, 2 replies; 27+ messages in thread
From: Radu Sabau via B4 Relay @ 2026-10-15 9:32 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, 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.
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)
Signed-off-by: Radu Sabau <radu.sabau@analog.com>
---
.../devicetree/bindings/iio/imu/adi,adis16607.yaml | 82 ++++++++++++++++++++++
MAINTAINERS | 7 ++
2 files changed, 89 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..1cf969154885
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/imu/adi,adis16607.yaml
@@ -0,0 +1,82 @@
+# 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.
+
+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. The line is active low.
+ 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 a7fd31320b38..f6ad73be5260 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1717,6 +1717,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 related [flat|nested] 27+ messages in thread* Re: [PATCH v3 2/6] dt-bindings: iio: imu: Add bindings for ADI ADIS16607
2026-10-15 9:32 ` [PATCH v3 2/6] dt-bindings: iio: imu: Add bindings for ADI ADIS16607 Radu Sabau via B4 Relay
@ 2026-10-15 20:43 ` Rob Herring (Arm)
2026-10-17 1:11 ` Jonathan Cameron
1 sibling, 0 replies; 27+ messages in thread
From: Rob Herring (Arm) @ 2026-10-15 20:43 UTC (permalink / raw)
To: Radu Sabau
Cc: devicetree, Krzysztof Kozlowski, linux-iio, Jonathan Cameron,
Alexandru Ardelean, Nuno Sa, Andy Shevchenko, Lars-Peter Clausen,
linux-kernel, Michael Hennerich, David Lechner, Conor Dooley
On Thu, 15 Oct 2026 12:32:59 +0300, Radu Sabau wrote:
> 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.
>
> 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)
>
> Signed-off-by: Radu Sabau <radu.sabau@analog.com>
> ---
> .../devicetree/bindings/iio/imu/adi,adis16607.yaml | 82 ++++++++++++++++++++++
> MAINTAINERS | 7 ++
> 2 files changed, 89 insertions(+)
>
Reviewed-by: Rob Herring (Arm) <robh@kernel.org>
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v3 2/6] dt-bindings: iio: imu: Add bindings for ADI ADIS16607
2026-10-15 9:32 ` [PATCH v3 2/6] dt-bindings: iio: imu: Add bindings for ADI ADIS16607 Radu Sabau via B4 Relay
2026-10-15 20:43 ` Rob Herring (Arm)
@ 2026-10-17 1:11 ` Jonathan Cameron
2026-10-19 8:15 ` Sabau, Radu bogdan
1 sibling, 1 reply; 27+ messages in thread
From: Jonathan Cameron @ 2026-10-17 1: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
On Thu, 15 Oct 2026 12:32:59 +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.
Say briefly how they are different and hence require their
own compatibles.
The hardware rest being a configurable pin is an unusual
bit of hardware design given how do we reset the reset pin
to issue a reset if the device is hung?
>
> 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)
>
> Signed-off-by: Radu Sabau <radu.sabau@analog.com>
> ---
> .../devicetree/bindings/iio/imu/adi,adis16607.yaml | 82 ++++++++++++++++++++++
> MAINTAINERS | 7 ++
> 2 files changed, 89 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..1cf969154885
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/imu/adi,adis16607.yaml
> @@ -0,0 +1,82 @@
> +# 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.
> +
> +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. The line is active low.
> + maxItems: 1
The datasheet I looked at has 4 gpio pins. You are calling out one
as reset. What happens if we don't have controllable supplies
and someone else (firmware for instance) configured GPIO0 to not
be reset?
Anyhow, feels like this isn't a full description.
> +
> +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 a7fd31320b38..f6ad73be5260 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -1717,6 +1717,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] 27+ messages in thread* RE: [PATCH v3 2/6] dt-bindings: iio: imu: Add bindings for ADI ADIS16607
2026-10-17 1:11 ` Jonathan Cameron
@ 2026-10-19 8:15 ` Sabau, Radu bogdan
2026-10-21 19:07 ` Jonathan Cameron
0 siblings, 1 reply; 27+ messages in thread
From: Sabau, Radu bogdan @ 2026-10-19 8:15 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@vger.kernel.org,
linux-kernel@vger.kernel.org, devicetree@vger.kernel.org
> -----Original Message-----
> From: Jonathan Cameron <jic23@kernel.org>
> Sent: Saturday, October 17, 2026 4:12 AM
...
> >
> > 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.
>
> Say briefly how they are different and hence require their
> own compatibles.
Hi Jonathan, thank you for looking at this, as well.
The only difference between them is the gyroscope's fixed full-scale
range: ±450°/s for adis16607-2, ±2000°/s for adis16607-3. Everything
else (accelerometer range, decimation, internal clock, sync modes) is
identical, and both report the same device ID over SPI, so the driver
has no way to tell which gyro range is populated at runtime - the
compatible string is the only source of that information. I'll make
the commit message say this explicitly.
>
> The hardware rest being a configurable pin is an unusual
> bit of hardware design given how do we reset the reset pin
> to issue a reset if the device is hung?
>
I agree the hardware reset being configurable is unusual. GPIO0 is configured
as reset by default at power-up, so the driver probing at power-up is ok.
However, to answer your question if the USER_GPIO_CFG register is altered
by debugfs read or a bug or something else and the device is hung, the true
answer is you can't use hardware reset anymore and the only way out of this
would be a power-cycle.
Perhaps a force write of the RESET_GPIO_MASK before the initial_startup
would help in case the SPI communication would still be alive, and I guess
if the SPI communication is broken we would then get an errno code anyway,
right? I am not sure this is the best solution so I would like to know your
thoughts on this.
> >
> > 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)
> >
...
> > + reset-gpios:
> > + description:
> > + Optional GPIO for hardware reset. The line is active low.
> > + maxItems: 1
>
> The datasheet I looked at has 4 gpio pins. You are calling out one
> as reset. What happens if we don't have controllable supplies
> and someone else (firmware for instance) configured GPIO0 to not
> be reset?
>
> Anyhow, feels like this isn't a full description.
>
Indeed, this is not a full description. There are a total of 5 gpio pins to
this device and each one has a fixed function, the USER_GPIO_CFG is
more like an enable/disable register I would say. So RESET is either
GPIO0 or idle to say so.
Would you say that a description like this would be more suitable? :
reset-gpios:
description:
Optional GPIO for hardware reset, active low. Of the device's
five GPIO pins, this one is dedicated to the reset function,
which can be independently enabled or idled via an on-chip
register.
maxItems: 1
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [PATCH v3 2/6] dt-bindings: iio: imu: Add bindings for ADI ADIS16607
2026-10-19 8:15 ` Sabau, Radu bogdan
@ 2026-10-21 19:07 ` Jonathan Cameron
0 siblings, 0 replies; 27+ messages in thread
From: Jonathan Cameron @ 2026-10-21 19:07 UTC (permalink / raw)
To: Sabau, Radu bogdan
Cc: Radu Sabau via B4 Relay, Lars-Peter Clausen, Hennerich, Michael,
Sa, Nuno, David Lechner, Andy Shevchenko, Alexandru Ardelean,
Rob Herring, Krzysztof Kozlowski, Conor Dooley,
linux-iio@vger.kernel.org, linux-kernel@vger.kernel.org,
devicetree@vger.kernel.org
On Mon, 19 Oct 2026 08:15:41 +0000
"Sabau, Radu bogdan" <radu.sabau@analog.com> wrote:
> > -----Original Message-----
> > From: Jonathan Cameron <jic23@kernel.org>
> > Sent: Saturday, October 17, 2026 4:12 AM
>
> ...
>
> > >
> > > 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.
> >
> > Say briefly how they are different and hence require their
> > own compatibles.
>
> Hi Jonathan, thank you for looking at this, as well.
>
> The only difference between them is the gyroscope's fixed full-scale
> range: ±450°/s for adis16607-2, ±2000°/s for adis16607-3. Everything
> else (accelerometer range, decimation, internal clock, sync modes) is
> identical, and both report the same device ID over SPI, so the driver
> has no way to tell which gyro range is populated at runtime - the
> compatible string is the only source of that information. I'll make
> the commit message say this explicitly.
Good. A one liner to say scales differ in the commit message will do nicely.
>
> >
> > The hardware rest being a configurable pin is an unusual
> > bit of hardware design given how do we reset the reset pin
> > to issue a reset if the device is hung?
> >
>
> I agree the hardware reset being configurable is unusual. GPIO0 is configured
> as reset by default at power-up, so the driver probing at power-up is ok.
> However, to answer your question if the USER_GPIO_CFG register is altered
> by debugfs read or a bug or something else and the device is hung, the true
> answer is you can't use hardware reset anymore and the only way out of this
> would be a power-cycle.
>
> Perhaps a force write of the RESET_GPIO_MASK before the initial_startup
> would help in case the SPI communication would still be alive, and I guess
> if the SPI communication is broken we would then get an errno code anyway,
> right? I am not sure this is the best solution so I would like to know your
> thoughts on this.
Yeah. I think try to write it - but only warn if write fails rather
than error out. Gives us the best chance of recovering from any
problems.
>
> > >
> > > 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)
> > >
>
> ...
>
> > > + reset-gpios:
> > > + description:
> > > + Optional GPIO for hardware reset. The line is active low.
> > > + maxItems: 1
> >
> > The datasheet I looked at has 4 gpio pins. You are calling out one
> > as reset. What happens if we don't have controllable supplies
> > and someone else (firmware for instance) configured GPIO0 to not
> > be reset?
> >
> > Anyhow, feels like this isn't a full description.
> >
>
> Indeed, this is not a full description. There are a total of 5 gpio pins to
> this device and each one has a fixed function, the USER_GPIO_CFG is
> more like an enable/disable register I would say. So RESET is either
> GPIO0 or idle to say so.
>
> Would you say that a description like this would be more suitable? :
>
> reset-gpios:
> description:
> Optional GPIO for hardware reset, active low. Of the device's
> five GPIO pins, this one is dedicated to the reset function,
> which can be independently enabled or idled via an on-chip
> register.
Only talk about this one in this description.
But add the pins with appropriate names for the functions the others
support. The GPIOX naming is silly given they are only good for
one thing each. Just make sure to add something in the comment
to refer to what they are called on the datasheet.
Jonathan
> maxItems: 1
>
^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH v3 3/6] iio: imu: Add driver for the ADI ADIS16607
2026-10-15 9:32 [PATCH v3 0/6] iio: imu: Add support for the ADI ADIS16607 Radu Sabau via B4 Relay
2026-10-15 9:32 ` [PATCH v3 1/6] iio: imu: adis: Add optional self_test callback and fix custom reset dispatch Radu Sabau via B4 Relay
2026-10-15 9:32 ` [PATCH v3 2/6] dt-bindings: iio: imu: Add bindings for ADI ADIS16607 Radu Sabau via B4 Relay
@ 2026-10-15 9:33 ` Radu Sabau via B4 Relay
2026-10-15 9:44 ` sashiko-bot
` (3 more replies)
2026-10-15 9:33 ` [PATCH v3 4/6] iio: imu: adis16607: Add FIFO-based buffered/triggered capture Radu Sabau via B4 Relay
` (2 subsequent siblings)
5 siblings, 4 replies; 27+ messages in thread
From: Radu Sabau via B4 Relay @ 2026-10-15 9:33 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, 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.
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 | 855 ++++++++++++++++++++++++++++++++++++++++++++
4 files changed, 870 insertions(+)
diff --git a/MAINTAINERS b/MAINTAINERS
index f6ad73be5260..70b7a430c610 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1723,6 +1723,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 7e0181c27bb6..7781bd7081cd 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 13fb7846e9c9..a5e21687cb5d 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..1d471f59f043
--- /dev/null
+++ b/drivers/iio/imu/adis16607.c
@@ -0,0 +1,855 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * ADIS16607 IMU driver
+ *
+ * Copyright 2026 Analog Devices Inc.
+ */
+#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/kernel.h>
+#include <linux/math.h>
+#include <linux/units.h>
+#include <linux/module.h>
+#include <linux/property.h>
+#include <linux/regulator/consumer.h>
+#include <linux/spi/spi.h>
+#include <linux/unaligned.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;
+ u16 max_dec;
+};
+
+struct adis16607 {
+ const struct adis16607_chip_info *info;
+ struct adis adis;
+ unsigned long clk_freq; /* millihertz */
+ u64 sample_period_ns;
+ /* IIO buffer for reordered data - sized for max channels */
+ u8 data[64] __aligned(IIO_DMA_MINALIGN);
+};
+
+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(&msg);
+ spi_message_add_tail(&xfers[0], &msg);
+ spi_message_add_tail(&xfers[1], &msg);
+
+ ret = spi_sync(adis->spi, &msg);
+ if (ret)
+ return ret;
+
+ *val = get_unaligned_be16(&adis->rx[2]);
+ break;
+
+ 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(&msg);
+ spi_message_add_tail(&xfers[0], &msg);
+ spi_message_add_tail(&xfers[1], &msg);
+
+ 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(&msg);
+ spi_message_add_tail(&xfers[0], &msg);
+ spi_message_add_tail(&xfers[1], &msg);
+
+ 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;
+ break;
+
+ default:
+ return -EINVAL;
+ }
+
+ return 0;
+}
+
+/*
+ * 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;
+ adis->tx[1] = (val >> 8) & 0xFF;
+ adis->tx[2] = val & 0xFF;
+
+ 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 < 6; i++) {
+ ret = __adis_read_reg_16(adis, ADIS16607_REG_SELF_TEST_DATA(i),
+ &st1_val[i]);
+ if (ret)
+ return ret;
+ }
+
+ /* Force self-test */
+ ret = __adis_write_reg_16(adis, ADIS16607_REG_SELF_TEST,
+ ADIS16607_SNSR_SELF_TEST_MASK | ADIS16607_ST_FORCE_MASK);
+ if (ret)
+ return ret;
+
+ /* 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 < 6; i++) {
+ ret = __adis_read_reg_16(adis, ADIS16607_REG_SELF_TEST_DATA(i),
+ &st2_val[i]);
+ if (ret)
+ return ret;
+
+ 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);
+ return -EINVAL;
+ }
+ }
+
+ /* Clear self-test register */
+ ret = __adis_write_reg_16(adis, ADIS16607_REG_SELF_TEST, 0);
+ if (ret)
+ return ret;
+
+ return 0;
+}
+
+/*
+ * 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, the
+ * bootloader-busy check, and (if wired up) a reset-status GPIO output.
+ */
+static int adis16607_post_reset_setup(struct adis *adis)
+{
+ struct device *dev = &adis->spi->dev;
+ u16 diag_stat, digital_status, dev_id;
+ int ret;
+
+ /* Additional 100ms required at initial startup per datasheet */
+ msleep(100);
+
+ /* 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;
+ }
+
+ /* Route a reset-status indicator to its own pin, if reset-gpios is wired */
+ if (device_property_present(dev, "reset-gpios")) {
+ ret = __adis_update_bits_base(adis, ADIS16607_REG_USER_GPIO_CFG,
+ ADIS16607_RESET_GPIO_MASK,
+ FIELD_PREP(ADIS16607_RESET_GPIO_MASK, 1), 2);
+ if (ret)
+ return ret;
+ }
+
+ 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;
+
+ 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;
+}
+
+#ifdef CONFIG_DEBUG_FS
+static int adis16607_show_firmware_id(void *arg, u64 *val)
+{
+ struct adis16607 *st = arg;
+ u16 rev;
+ int ret;
+
+ ret = adis_read_reg_16(&st->adis, ADIS16607_REG_REV_ID, &rev);
+ if (ret)
+ return ret;
+
+ *val = (rev >> 8) & 0xff;
+ return 0;
+}
+DEFINE_DEBUGFS_ATTRIBUTE(adis16607_firmware_id_fops,
+ adis16607_show_firmware_id, NULL, "%llu\n");
+
+static int adis16607_show_revision_id(void *arg, u64 *val)
+{
+ struct adis16607 *st = arg;
+ u16 rev;
+ int ret;
+
+ ret = adis_read_reg_16(&st->adis, ADIS16607_REG_REV_ID, &rev);
+ if (ret)
+ return ret;
+
+ *val = rev & 0xff;
+ return 0;
+}
+DEFINE_DEBUGFS_ATTRIBUTE(adis16607_revision_id_fops,
+ adis16607_show_revision_id, NULL, "%llu\n");
+
+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_unsafe("firmware_id", 0400, d, st,
+ &adis16607_firmware_id_fops);
+ debugfs_create_file_unsafe("revision_id", 0400, d, st,
+ &adis16607_revision_id_fops);
+}
+#else
+static void adis16607_debugfs_init(struct iio_dev *indio_dev)
+{
+}
+#endif
+
+static int adis16607_get_freq(struct adis16607 *st, u32 *freq)
+{
+ u16 dec = 0;
+ int ret;
+
+ mutex_lock(&st->adis.state_lock);
+
+ ret = __adis_read_reg_16(&st->adis, ADIS16607_REG_DEC_RATE, &dec);
+ if (ret)
+ goto unlock;
+
+ *freq = DIV_ROUND_CLOSEST(st->clk_freq, dec + 1);
+
+unlock:
+ mutex_unlock(&st->adis.state_lock);
+ return ret;
+}
+
+static int adis16607_set_freq(struct adis16607 *st, u32 freq)
+{
+ u32 dec;
+ int ret;
+
+ if (!freq)
+ return -EINVAL;
+
+ mutex_lock(&st->adis.state_lock);
+
+ dec = DIV_ROUND_CLOSEST(st->clk_freq, 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)
+ st->sample_period_ns = DIV_ROUND_CLOSEST_ULL((u64)NSEC_PER_SEC * 1000ULL * (dec + 1),
+ st->clk_freq);
+
+ mutex_unlock(&st->adis.state_lock);
+ return ret;
+}
+
+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, _bits) { \
+ .type = (_type), \
+ .modified = 1, \
+ .channel2 = (_mod), \
+ .info_mask_separate = BIT(IIO_CHAN_INFO_RAW), \
+ .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE), \
+ .info_mask_shared_by_all = BIT(IIO_CHAN_INFO_SAMP_FREQ), \
+ .address = (_address), \
+ .scan_index = (_si), \
+ .scan_type = { \
+ .sign = 's', \
+ .realbits = (_bits), \
+ .storagebits = 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, 24)
+
+#define ADIS16607_GYRO_CHANNEL(_mod) \
+ ADIS16607_MOD_CHAN(IIO_ANGL_VEL, IIO_MOD_ ## _mod, \
+ ADIS16607_REG_ ## _mod ## _GYRO, \
+ ADIS16607_SCAN_GYRO_ ## _mod, 24)
+
+#define ADIS16607_TEMP_CHANNEL() { \
+ .type = IIO_TEMP, \
+ .indexed = 1, \
+ .channel = 0, \
+ .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) | \
+ BIT(IIO_CHAN_INFO_SCALE) | \
+ BIT(IIO_CHAN_INFO_OFFSET), \
+ .info_mask_shared_by_all = BIT(IIO_CHAN_INFO_SAMP_FREQ), \
+ .address = ADIS16607_REG_TEMP, \
+ .scan_index = ADIS16607_SCAN_TEMP, \
+ .scan_type = { \
+ .sign = 's', \
+ .realbits = 16, \
+ .storagebits = 16, \
+ .endianness = IIO_BE, \
+ }, \
+}
+
+#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),
+ ADIS16607_TEMP_CHANNEL(),
+ 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
+ */
+#define ADIS16607_CHIP_INFO_COMMON \
+ .channels = adis16607_channels, \
+ .num_channels = ARRAY_SIZE(adis16607_channels), \
+ .accel_max_val = IIO_G_TO_M_S_2(40), \
+ .accel_max_scale = 31250 << 8, \
+ .temp_scale = 5, \
+ .deltvel_max_val = IIO_G_TO_M_S_2(40), \
+ .int_clk = 8000, \
+ .max_dec = 65535
+
+static const struct adis16607_chip_info adis16607_2_chip_info = {
+ ADIS16607_CHIP_INFO_COMMON,
+ .name = "adis16607-2",
+ .gyro_max_val = IIO_DEGREE_TO_RAD(450),
+ .gyro_max_scale = 30000 << 8,
+ .deltang_max_val = IIO_DEGREE_TO_RAD(450),
+};
+
+static const struct adis16607_chip_info adis16607_3_chip_info = {
+ ADIS16607_CHIP_INFO_COMMON,
+ .name = "adis16607-3",
+ .gyro_max_val = IIO_DEGREE_TO_RAD(2000),
+ .gyro_max_scale = 31250 << 8,
+ .deltang_max_val = IIO_DEGREE_TO_RAD(2000),
+};
+
+/*
+ * No external clock sync support yet: always run on the internal clock.
+ * Default dec=0 after reset, so period = 1 / clk_freq.
+ */
+static int adis16607_config_clk(struct adis16607 *st)
+{
+ st->clk_freq = (unsigned long)st->info->int_clk * 1000;
+ st->sample_period_ns = DIV_ROUND_CLOSEST_ULL((u64)NSEC_PER_SEC * 1000ULL,
+ st->clk_freq);
+ 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 -EINVAL;
+
+ 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;
+
+ 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 related [flat|nested] 27+ messages in thread* Re: [PATCH v3 3/6] iio: imu: Add driver for the ADI ADIS16607
2026-10-15 9:33 ` [PATCH v3 3/6] iio: imu: Add driver for the " Radu Sabau via B4 Relay
@ 2026-10-15 9:44 ` sashiko-bot
2026-10-15 19:04 ` Andy Shevchenko
` (2 subsequent siblings)
3 siblings, 0 replies; 27+ messages in thread
From: sashiko-bot @ 2026-10-15 9:44 UTC (permalink / raw)
To: Radu Sabau; +Cc: devicetree, robh, conor+dt
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] spi: DMA cacheline sharing violation between rx and tx buffers
- [Medium] iio: missing hardware state teardown on self-test error
--
commit 7828515c587fd9239b2490c3f7b3aebfd3b79eaf
Author: Radu Sabau <radu.sabau@analog.com>
iio: imu: Add driver for the ADI ADIS16607
This commit introduces the base driver for the Analog Devices ADIS16607
series of IMUs. It includes probe functionality, raw channel reads,
frequency control, startup sequence, and debugfs support.
> diff --git a/drivers/iio/imu/adis16607.c b/drivers/iio/imu/adis16607.c
> new file mode 100644
> index 0000000000000..1d471f59f043d
> --- /dev/null
> +++ b/drivers/iio/imu/adis16607.c
> @@ -0,0 +1,855 @@
[ ... ]
> +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,
> + },
[Severity: Medium]
Does this simultaneous DMA mapping of both adis->tx and adis->rx violate
DMA cacheline coherency?
In include/linux/iio/imu/adis.h, struct adis defines tx and rx adjacently:
u8 tx[10] __aligned(IIO_DMA_MINALIGN);
u8 rx[4];
Because rx lacks its own alignment boundary and tx is 10 bytes, rx falls
within the same DMA cacheline as tx. On non-cache-coherent architectures,
mapping both buffers concurrently in opposite DMA directions can discard
ongoing cache flushes or CPU writes for tx during the cache invalidation
for rx, potentially leading to hardware data corruption.
[ ... ]
> +static int adis16607_sensor_self_test(struct adis16607 *st)
> +{
> + struct adis *adis = &st->adis;
[ ... ]
> + /* Read second set of self-test data */
> + for (i = 0; i < 6; i++) {
> + ret = __adis_read_reg_16(adis, ADIS16607_REG_SELF_TEST_DATA(i),
> + &st2_val[i]);
> + if (ret)
> + return ret;
> +
> + 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);
> + return -EINVAL;
> + }
> + }
> +
> + /* Clear self-test register */
> + ret = __adis_write_reg_16(adis, ADIS16607_REG_SELF_TEST, 0);
[Severity: Medium]
Will returning early from the loop bypass the teardown of the self-test mode?
If the second read fails or adis16607_delta_check() fails, the function
returns immediately. This skips the final __adis_write_reg_16() that clears
the ADIS16607_REG_SELF_TEST register, leaving the sensor hardware locked
in an active diagnostic/force state.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261015-adis16607-v3-0-f972d4efee85@analog.com?part=3
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [PATCH v3 3/6] iio: imu: Add driver for the ADI ADIS16607
2026-10-15 9:33 ` [PATCH v3 3/6] iio: imu: Add driver for the " Radu Sabau via B4 Relay
2026-10-15 9:44 ` sashiko-bot
@ 2026-10-15 19:04 ` Andy Shevchenko
2026-10-22 10:25 ` Sabau, Radu bogdan
2026-10-17 1:24 ` Jonathan Cameron
2026-10-22 14:37 ` Nuno Sá
3 siblings, 1 reply; 27+ messages in thread
From: Andy Shevchenko @ 2026-10-15 19:04 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
On Thu, Oct 15, 2026 at 12:33:00PM +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.
...
> +#include <linux/kernel.h>
No way. This header shouldn't appear in a simple driver like this.
> +#include <linux/units.h>
> +#include <linux/module.h>
Keep the list ordered.
> +#include <linux/property.h>
> +#include <linux/regulator/consumer.h>
> +#include <linux/spi/spi.h>
> +#include <linux/unaligned.h>
...
> +struct adis16607 {
> + const struct adis16607_chip_info *info;
> + struct adis adis;
If you ever use container_of() on this, the moving it to be the first member
saves some .text code.
> + unsigned long clk_freq; /* millihertz */
> + u64 sample_period_ns;
> + /* IIO buffer for reordered data - sized for max channels */
> + u8 data[64] __aligned(IIO_DMA_MINALIGN);
> +};
Note, whenever you optimize layout with `pahole` always check with
`bloat-o-meter` to see if the code generation is affected and how.
...
> + case 2:
> + adis->tx[0] = reg | BIT(7);
> + adis->tx[1] = 0;
> + adis->tx[2] = 0;
> + adis->tx[3] = 0;
> +
> + spi_message_init(&msg);
> + spi_message_add_tail(&xfers[0], &msg);
> + spi_message_add_tail(&xfers[1], &msg);
There is a function to add multiple transfers to the message.
> + ret = spi_sync(adis->spi, &msg);
> + if (ret)
> + return ret;
> +
> + *val = get_unaligned_be16(&adis->rx[2]);
> + break;
> +
> + 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(&msg);
> + spi_message_add_tail(&xfers[0], &msg);
> + spi_message_add_tail(&xfers[1], &msg);
Ditto.
> + 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(&msg);
> + spi_message_add_tail(&xfers[0], &msg);
> + spi_message_add_tail(&xfers[1], &msg);
> +
> + 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;
> + break;
> +
> + default:
> + return -EINVAL;
> + }
> +
> + return 0;
Replace breaks with return 0 and drop this one.
...
> +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;
> + adis->tx[1] = (val >> 8) & 0xFF;
> + adis->tx[2] = val & 0xFF;
put_unaligned_be16()
> + return spi_write(adis->spi, adis->tx, 3);
> +}
...
> + /* Read first set of self-test data */
> + for (i = 0; i < 6; i++) {
ARRAY_SIZE()
> + ret = __adis_read_reg_16(adis, ADIS16607_REG_SELF_TEST_DATA(i),
> + &st1_val[i]);
> + if (ret)
> + return ret;
> + }
...
> + /* Read second set of self-test data */
> + for (i = 0; i < 6; i++) {
Ditto.
> + ret = __adis_read_reg_16(adis, ADIS16607_REG_SELF_TEST_DATA(i),
> + &st2_val[i]);
> + if (ret)
> + return ret;
> +
> + 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);
> + return -EINVAL;
> + }
> + }
> + /* Clear self-test register */
> + ret = __adis_write_reg_16(adis, ADIS16607_REG_SELF_TEST, 0);
> + if (ret)
> + return ret;
> +
> + return 0;
return ..._write_reg_16(...);
...
> + /* Additional 100ms required at initial startup per datasheet */
Add a reference where in DS it's required.
> + msleep(100);
...
> + /* Route a reset-status indicator to its own pin, if reset-gpios is wired */
> + if (device_property_present(dev, "reset-gpios")) {
Huh?! Can we use reset framework instead and do something with help of it?
> + ret = __adis_update_bits_base(adis, ADIS16607_REG_USER_GPIO_CFG,
> + ADIS16607_RESET_GPIO_MASK,
> + FIELD_PREP(ADIS16607_RESET_GPIO_MASK, 1), 2);
> + if (ret)
> + return ret;
> + }
...
> +#ifdef CONFIG_DEBUG_FS
> +static int adis16607_show_firmware_id(void *arg, u64 *val)
> +{
> + struct adis16607 *st = arg;
> + u16 rev;
> + int ret;
> +
> + ret = adis_read_reg_16(&st->adis, ADIS16607_REG_REV_ID, &rev);
> + if (ret)
> + return ret;
> +
> + *val = (rev >> 8) & 0xff;
> + return 0;
> +}
> +DEFINE_DEBUGFS_ATTRIBUTE(adis16607_firmware_id_fops,
> + adis16607_show_firmware_id, NULL, "%llu\n");
> +
> +static int adis16607_show_revision_id(void *arg, u64 *val)
> +{
> + struct adis16607 *st = arg;
> + u16 rev;
> + int ret;
> +
> + ret = adis_read_reg_16(&st->adis, ADIS16607_REG_REV_ID, &rev);
> + if (ret)
> + return ret;
> +
> + *val = rev & 0xff;
> + return 0;
> +}
> +DEFINE_DEBUGFS_ATTRIBUTE(adis16607_revision_id_fops,
> + adis16607_show_revision_id, NULL, "%llu\n");
"%u.%u" (major.minor)
...
> +#else
> +static void adis16607_debugfs_init(struct iio_dev *indio_dev)
> +{
> +}
> +#endif
Why? The DebugFS is aware of this, no ugly ifdeffery needed.
...
> +static int adis16607_get_freq(struct adis16607 *st, u32 *freq)
> +{
> + u16 dec = 0;
> + int ret;
> +
> + mutex_lock(&st->adis.state_lock);
What's wrong with guard()()?
> + ret = __adis_read_reg_16(&st->adis, ADIS16607_REG_DEC_RATE, &dec);
> + if (ret)
> + goto unlock;
> +
> + *freq = DIV_ROUND_CLOSEST(st->clk_freq, dec + 1);
> +
> +unlock:
> + mutex_unlock(&st->adis.state_lock);
> + return ret;
> +}
...
> +static int adis16607_config_clk(struct adis16607 *st)
> +{
> + st->clk_freq = (unsigned long)st->info->int_clk * 1000;
Why casting? What is the meaning of 1000? Why members are without _Hz or alike
units?
> + st->sample_period_ns = DIV_ROUND_CLOSEST_ULL((u64)NSEC_PER_SEC * 1000ULL,
> + st->clk_freq);
Better to indent like
st->sample_period_ns =
DIV_ROUND_CLOSEST_ULL((u64)NSEC_PER_SEC * 1000ULL, st->clk_freq);
> + 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 -EINVAL;
-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;
> +
> + 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;
> +}
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 27+ messages in thread* RE: [PATCH v3 3/6] iio: imu: Add driver for the ADI ADIS16607
2026-10-15 19:04 ` Andy Shevchenko
@ 2026-10-22 10:25 ` Sabau, Radu bogdan
0 siblings, 0 replies; 27+ messages in thread
From: Sabau, Radu bogdan @ 2026-10-22 10:25 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@vger.kernel.org,
linux-kernel@vger.kernel.org, devicetree@vger.kernel.org
> -----Original Message-----
> From: Andy Shevchenko <andriy.shevchenko@intel.com>
> Sent: Thursday, October 15, 2026 10:05 PM
...
> > + /* Route a reset-status indicator to its own pin, if reset-gpios is wired
> */
> > + if (device_property_present(dev, "reset-gpios")) {
>
> Huh?! Can we use reset framework instead and do something with help of it?
>
> > + ret = __adis_update_bits_base(adis,
> ADIS16607_REG_USER_GPIO_CFG,
> > + ADIS16607_RESET_GPIO_MASK,
> > +
> FIELD_PREP(ADIS16607_RESET_GPIO_MASK, 1), 2);
> > + if (ret)
> > + return ret;
> > + }
>
Hi Andy, thank you also for taking a look.
This code will be removed from here to probe() as per the possibility of
the device hanging discussion, it make much more sense for it to be there.
This was writing RESET_GPIO_MASK after reset, which pretty much was a
no-op since the device already defaults with that bit set.
Removed the duplicate here entirely - it's not reset-line handling
or reset-framework territory, just an on-chip pin-function register,
and now it will only exist in the one place where it matters.
Thanks,
Radu
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v3 3/6] iio: imu: Add driver for the ADI ADIS16607
2026-10-15 9:33 ` [PATCH v3 3/6] iio: imu: Add driver for the " Radu Sabau via B4 Relay
2026-10-15 9:44 ` sashiko-bot
2026-10-15 19:04 ` Andy Shevchenko
@ 2026-10-17 1:24 ` Jonathan Cameron
2026-10-22 14:37 ` Nuno Sá
3 siblings, 0 replies; 27+ messages in thread
From: Jonathan Cameron @ 2026-10-17 1:24 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
On Thu, 15 Oct 2026 12:33:00 +0300
Radu Sabau via B4 Relay <devnull+radu.sabau.analog.com@kernel.org> wrote:
> 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.
>
> Signed-off-by: Radu Sabau <radu.sabau@analog.com>
Various comments inline.
Thanks,
Jonathan
> diff --git a/drivers/iio/imu/adis16607.c b/drivers/iio/imu/adis16607.c
> new file mode 100644
> index 000000000000..1d471f59f043
> --- /dev/null
> +++ b/drivers/iio/imu/adis16607.c
> +struct adis16607 {
> + const struct adis16607_chip_info *info;
> + struct adis adis;
> + unsigned long clk_freq; /* millihertz */
> + u64 sample_period_ns;
> + /* IIO buffer for reordered data - sized for max channels */
> + u8 data[64] __aligned(IIO_DMA_MINALIGN);
I guess this gets used in a later patch? Add it there instead.
> +};
> +
> +/*
> + * 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;
> + adis->tx[1] = (val >> 8) & 0xFF;
> + adis->tx[2] = val & 0xFF;
put_unaligned_be16()
as that will clearly document what is going on here.
> +
> + return spi_write(adis->spi, adis->tx, 3);
> +}
> +/*
> + * 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 < 6; i++) {
> + ret = __adis_read_reg_16(adis, ADIS16607_REG_SELF_TEST_DATA(i),
> + &st1_val[i]);
> + if (ret)
> + return ret;
> + }
> +
> + /* Force self-test */
> + ret = __adis_write_reg_16(adis, ADIS16607_REG_SELF_TEST,
> + ADIS16607_SNSR_SELF_TEST_MASK | ADIS16607_ST_FORCE_MASK);
> + if (ret)
> + return ret;
> +
> + /* 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 < 6; i++) {
> + ret = __adis_read_reg_16(adis, ADIS16607_REG_SELF_TEST_DATA(i),
> + &st2_val[i]);
> + if (ret)
> + return ret;
> +
> + 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);
> + return -EINVAL;
> + }
> + }
> +
> + /* Clear self-test register */
> + ret = __adis_write_reg_16(adis, ADIS16607_REG_SELF_TEST, 0);
> + if (ret)
> + return ret;
> +
> + return 0;
return __adis_write_reg_16()
> +}
> +static int adis16607_get_freq(struct adis16607 *st, u32 *freq)
> +{
> + u16 dec = 0;
> + int ret;
> +
> + mutex_lock(&st->adis.state_lock);
> +
> + ret = __adis_read_reg_16(&st->adis, ADIS16607_REG_DEC_RATE, &dec);
> + if (ret)
> + goto unlock;
> +
> + *freq = DIV_ROUND_CLOSEST(st->clk_freq, dec + 1);
> +
> +unlock:
> + mutex_unlock(&st->adis.state_lock);
As below. We have guards for this and a special helper
to make them even easier to use for this lock.
> + return ret;
> +}
> +
> +static int adis16607_set_freq(struct adis16607 *st, u32 freq)
> +{
> + u32 dec;
> + int ret;
> +
> + if (!freq)
> + return -EINVAL;
> +
> + mutex_lock(&st->adis.state_lock);
Use guard. However, don't we have more specific helpers for these?
adis_dev_autolock()
> +
> + dec = DIV_ROUND_CLOSEST(st->clk_freq, 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)
With a guard you can have the error path out of line.
Always do that unless it makes the code a lot less redable.
if (ret)
return ret;
st->sample_period_ns = ...
> + st->sample_period_ns = DIV_ROUND_CLOSEST_ULL((u64)NSEC_PER_SEC * 1000ULL * (dec + 1),
> + st->clk_freq);
> +
> + mutex_unlock(&st->adis.state_lock);
> + return ret;
> +}
> +#define ADIS16607_MOD_CHAN(_type, _mod, _address, _si, _bits) { \
> + .type = (_type), \
> + .modified = 1, \
> + .channel2 = (_mod), \
> + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW), \
> + .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE), \
> + .info_mask_shared_by_all = BIT(IIO_CHAN_INFO_SAMP_FREQ), \
> + .address = (_address), \
> + .scan_index = (_si), \
> + .scan_type = { \
> + .sign = 's', \
> + .realbits = (_bits), \
Always 24? If so hard code it.
> + .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, 24)
> +
> +#define ADIS16607_GYRO_CHANNEL(_mod) \
> + ADIS16607_MOD_CHAN(IIO_ANGL_VEL, IIO_MOD_ ## _mod, \
> + ADIS16607_REG_ ## _mod ## _GYRO, \
> + ADIS16607_SCAN_GYRO_ ## _mod, 24)
> +
> +#define ADIS16607_TEMP_CHANNEL() { \
> + .type = IIO_TEMP, \
> + .indexed = 1, \
> + .channel = 0, \
> + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) | \
> + BIT(IIO_CHAN_INFO_SCALE) | \
> + BIT(IIO_CHAN_INFO_OFFSET), \
> + .info_mask_shared_by_all = BIT(IIO_CHAN_INFO_SAMP_FREQ), \
> + .address = ADIS16607_REG_TEMP, \
> + .scan_index = ADIS16607_SCAN_TEMP, \
> + .scan_type = { \
> + .sign = 's', \
> + .realbits = 16, \
> + .storagebits = 16, \
> + .endianness = IIO_BE, \
> + }, \
Only one of these. Just put it inline. The macro isn't adding anything
useful that I can see.
> +}
> +
> +#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),
> + ADIS16607_TEMP_CHANNEL(),
> + IIO_CHAN_SOFT_TIMESTAMP(13),
> +};
> +
> +/*
> + * ADIS16607-2: 450 deg/s gyro, 40g accel
> + * ADIS16607-3: 2000 deg/s gyro, 40g accel
> + */
> +#define ADIS16607_CHIP_INFO_COMMON \
> + .channels = adis16607_channels, \
> + .num_channels = ARRAY_SIZE(adis16607_channels), \
> + .accel_max_val = IIO_G_TO_M_S_2(40), \
> + .accel_max_scale = 31250 << 8, \
> + .temp_scale = 5, \
> + .deltvel_max_val = IIO_G_TO_M_S_2(40), \
> + .int_clk = 8000, \
> + .max_dec = 65535
For only two instances, I'd just duplicate this in the assignments.
> +
> +static const struct adis16607_chip_info adis16607_2_chip_info = {
> + ADIS16607_CHIP_INFO_COMMON,
> + .name = "adis16607-2",
> + .gyro_max_val = IIO_DEGREE_TO_RAD(450),
> + .gyro_max_scale = 30000 << 8,
> + .deltang_max_val = IIO_DEGREE_TO_RAD(450),
> +};
> +
> +static const struct adis16607_chip_info adis16607_3_chip_info = {
> + ADIS16607_CHIP_INFO_COMMON,
> + .name = "adis16607-3",
> + .gyro_max_val = IIO_DEGREE_TO_RAD(2000),
> + .gyro_max_scale = 31250 << 8,
> + .deltang_max_val = IIO_DEGREE_TO_RAD(2000),
> +};
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [PATCH v3 3/6] iio: imu: Add driver for the ADI ADIS16607
2026-10-15 9:33 ` [PATCH v3 3/6] iio: imu: Add driver for the " Radu Sabau via B4 Relay
` (2 preceding siblings ...)
2026-10-17 1:24 ` Jonathan Cameron
@ 2026-10-22 14:37 ` Nuno Sá
3 siblings, 0 replies; 27+ messages in thread
From: Nuno Sá @ 2026-10-22 14:37 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
On Thu, 15 Oct 2026 12:33:00 +0300, Radu Sabau <radu.sabau@analog.com> 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.
>
> Signed-off-by: Radu Sabau <radu.sabau@analog.com>
Hopefully I did not repeated too much feedback.
>
>
> diff --git a/drivers/iio/imu/adis16607.c b/drivers/iio/imu/adis16607.c
> new file mode 100644
> index 000000000000..1d471f59f043
> --- /dev/null
> +++ b/drivers/iio/imu/adis16607.c
> @@ -0,0 +1,855 @@
> [ ... skip 285 lines ... ]
> +
> + 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);
> + return -EINVAL;
Only used in probe no? If so dev_err_probe()
> + }
> + }
> +
> + /* Clear self-test register */
> + ret = __adis_write_reg_16(adis, ADIS16607_REG_SELF_TEST, 0);
> + if (ret)
> + return ret;
> +
Just return __adis_write_reg_16()
> + return 0;
> +}
> +
> +/*
> + * 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, the
> + * bootloader-busy check, and (if wired up) a reset-status GPIO output.
> + */
> +static int adis16607_post_reset_setup(struct adis *adis)
The below API seems a bit over commented!
> [ ... skip 78 lines ... ]
> + if (ret)
> + return ret;
> +
> + ret = __adis_check_status(adis);
> + if (ret)
> + dev_err(dev, "status check after self-test failed: %d\n", ret);
Internally the API already logs the individual errors so the above is
more noise.
> [ ... skip 125 lines ... ]
> + int ret;
> +
> + if (!freq)
> + return -EINVAL;
> +
> + mutex_lock(&st->adis.state_lock);
As mentioned, use the existing helpers.
> +
> + dec = DIV_ROUND_CLOSEST(st->clk_freq, freq);
> + if (dec)
> + dec--;
> +
> + dec = min_t(u32, dec, st->info->max_dec);
min() should be enough no?
> [ ... skip 16 lines ... ]
> + u32 tmp;
> +
> + switch (info) {
> + case IIO_CHAN_INFO_RAW:
> + return adis_single_conversion(indio_dev, chan, 0, val);
> +
No new line.
> [ ... skip 23 lines ... ]
> + }
> +
> + 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;
same
> +
> + 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;
ditto
--
Nuno Sá <nuno.sa@analog.com>
^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH v3 4/6] iio: imu: adis16607: Add FIFO-based buffered/triggered capture
2026-10-15 9:32 [PATCH v3 0/6] iio: imu: Add support for the ADI ADIS16607 Radu Sabau via B4 Relay
` (2 preceding siblings ...)
2026-10-15 9:33 ` [PATCH v3 3/6] iio: imu: Add driver for the " Radu Sabau via B4 Relay
@ 2026-10-15 9:33 ` Radu Sabau via B4 Relay
2026-10-15 9:43 ` sashiko-bot
` (2 more replies)
2026-10-15 9:33 ` [PATCH v3 5/6] iio: imu: adis16607: Add external clock sync support Radu Sabau via B4 Relay
2026-10-15 9:33 ` [PATCH v3 6/6] iio: imu: adis16607: Add calibration bias support for gyro/accel Radu Sabau via B4 Relay
5 siblings, 3 replies; 27+ messages in thread
From: Radu Sabau via B4 Relay @ 2026-10-15 9:33 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, 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 | 420 +++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 418 insertions(+), 2 deletions(-)
diff --git a/drivers/iio/imu/adis16607.c b/drivers/iio/imu/adis16607.c
index 1d471f59f043..dac146645415 100644
--- a/drivers/iio/imu/adis16607.c
+++ b/drivers/iio/imu/adis16607.c
@@ -11,6 +11,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/kernel.h>
#include <linux/math.h>
#include <linux/units.h>
@@ -40,8 +43,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
@@ -52,8 +58,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)
@@ -84,6 +108,9 @@ struct adis16607 {
struct adis adis;
unsigned long clk_freq; /* millihertz */
u64 sample_period_ns;
+ u16 fifo_watermark;
+ /* Scratch buffer used for FIFO pop operations, see adis16607_fifo_pop() */
+ u8 *fifo_buf;
/* IIO buffer for reordered data - sized for max channels */
u8 data[64] __aligned(IIO_DMA_MINALIGN);
};
@@ -224,6 +251,352 @@ 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,
+};
+
+static void adis16607_fifo_buf_cleanup(void *arg)
+{
+ struct adis16607 *st = arg;
+
+ kfree(st->fifo_buf);
+}
+
+/*
+ * Sets up the scratch buffer used for FIFO pop operations (see
+ * adis16607_fifo_pop()). The 32-bit burst mode and DATA_CNTR config bits
+ * are enabled once during startup (adis16607_post_reset_setup()), not
+ * here, since they don't depend on scan_mask.
+ */
+static int adis16607_update_scan_mode(struct iio_dev *indio_dev,
+ const unsigned long *scan_mask)
+{
+ struct adis16607 *st = iio_priv(indio_dev);
+ unsigned int burst_len = ADIS16607_BURST_CMD_SIZE + ADIS16607_FIFO_DATA_LEN;
+
+ kfree(st->fifo_buf);
+
+ st->fifo_buf = kzalloc(burst_len, GFP_KERNEL);
+ if (!st->fifo_buf)
+ return -ENOMEM;
+
+ return 0;
+}
+
+/*
+ * 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, threshold;
+ 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;
+
+ /*
+ * Disable the FIFO before draining: FIFO_THR_MET means it's at the
+ * configured threshold, and leaving it enabled while we pop would
+ * let incoming samples overwrite ones not yet read. No new samples
+ * are enqueued while disabled, so nothing arrives between here and
+ * the flush below - some sample-periods' worth of data not being
+ * captured during this window is inherent to this device's FIFO
+ * design (it has no secondary buffer), not something the driver can
+ * avoid.
+ */
+ ret = __adis_write_reg_16(&st->adis, ADIS16607_REG_USER_FIFO_CFG, 0);
+ if (ret)
+ goto unlock;
+
+ /* Read FIFO word count */
+ ret = __adis_read_reg_16(&st->adis, ADIS16607_REG_FIFO_WORD_CNT, &word_cnt);
+ if (ret)
+ goto reenable_fifo;
+
+ /* Calculate number of complete samples */
+ sample_cnt = word_cnt / ADIS16607_FIFO_WORDS_PER_SAMPLE;
+ if (!sample_cnt)
+ goto reenable_fifo;
+
+ /*
+ * 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 reenable_fifo;
+
+ adis16607_fifo_push_sample(indio_dev, fifo_sample, timestamp);
+ timestamp += st->sample_period_ns;
+ }
+
+reenable_fifo:
+ /* Flush FIFO before re-enabling */
+ ret = __adis_write_reg_16(&st->adis, ADIS16607_REG_USER_FIFO_CFG,
+ ADIS16607_FIFO_FLUSH_MASK);
+ if (ret)
+ dev_warn_ratelimited(&st->adis.spi->dev,
+ "failed to flush FIFO: %d\n", ret);
+
+ /* Re-enable FIFO with watermark threshold */
+ threshold = st->fifo_watermark * ADIS16607_FIFO_WORDS_PER_SAMPLE;
+ ret = __adis_write_reg_16(&st->adis, ADIS16607_REG_USER_FIFO_CFG,
+ threshold & ADIS16607_FIFO_THR_MASK);
+ if (ret)
+ dev_warn_ratelimited(&st->adis.spi->dev,
+ "failed to re-enable FIFO: %d\n", ret);
+
+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;
+
+ mutex_lock(&adis->state_lock);
+
+ /* Flush FIFO to start clean */
+ ret = __adis_write_reg_16(adis, ADIS16607_REG_USER_FIFO_CFG,
+ ADIS16607_FIFO_FLUSH_MASK);
+ if (ret)
+ goto unlock;
+
+ /* Enable FIFO with watermark threshold (in word count) */
+ threshold = st->fifo_watermark * ADIS16607_FIFO_WORDS_PER_SAMPLE;
+ ret = __adis_write_reg_16(adis, ADIS16607_REG_USER_FIFO_CFG,
+ threshold & ADIS16607_FIFO_THR_MASK);
+
+unlock:
+ mutex_unlock(&adis->state_lock);
+ return ret;
+}
+
+static int adis16607_buffer_postdisable(struct iio_dev *indio_dev)
+{
+ struct adis16607 *st = iio_priv(indio_dev);
+ struct adis *adis = &st->adis;
+ int ret;
+
+ mutex_lock(&adis->state_lock);
+
+ /* Disable FIFO */
+ ret = __adis_write_reg_16(adis, ADIS16607_REG_USER_FIFO_CFG, 0);
+
+ mutex_unlock(&adis->state_lock);
+ return ret;
+}
+
+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_t(unsigned int, val, 1, ADIS16607_MAX_FIFO_WM);
+ st->fifo_watermark = val;
+
+ return 0;
+}
+
/*
* Check if the delta between two values is within specified range.
*/
@@ -307,7 +680,8 @@ static int adis16607_sensor_self_test(struct adis16607 *st)
* 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, the
- * bootloader-busy check, and (if wired up) a reset-status GPIO output.
+ * bootloader-busy check, a reset-status GPIO output (if wired up),
+ * enabling 32-bit burst mode + DATA_CNTR, and the rest of USER_GPIO_CFG.
*/
static int adis16607_post_reset_setup(struct adis *adis)
{
@@ -367,7 +741,29 @@ static int adis16607_post_reset_setup(struct adis *adis)
return ret;
}
- 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 possibly
+ * just set above. 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);
}
/*
@@ -705,7 +1101,9 @@ static const struct iio_chan_spec adis16607_channels[] = {
static const struct iio_info adis16607_info = {
.read_raw = adis16607_read_raw,
.write_raw = adis16607_write_raw,
+ .update_scan_mode = adis16607_update_scan_mode,
.debugfs_reg_access = adis_debugfs_reg_access,
+ .hwfifo_set_watermark = adis16607_set_watermark,
};
static const char * const adis16607_status_error_msgs[] = {
@@ -727,6 +1125,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,
@@ -798,6 +1197,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");
@@ -816,6 +1217,21 @@ static int adis16607_probe(struct spi_device *spi)
if (ret)
return ret;
+ /*
+ * Register fifo_buf cleanup before buffer/trigger setup so that on
+ * devm teardown (LIFO order) the IRQ handler is removed first, then
+ * fifo_buf is freed, avoiding a use-after-free in the handler.
+ */
+ ret = devm_add_action_or_reset(dev, adis16607_fifo_buf_cleanup, st);
+ 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 related [flat|nested] 27+ messages in thread* Re: [PATCH v3 4/6] iio: imu: adis16607: Add FIFO-based buffered/triggered capture
2026-10-15 9:33 ` [PATCH v3 4/6] iio: imu: adis16607: Add FIFO-based buffered/triggered capture Radu Sabau via B4 Relay
@ 2026-10-15 9:43 ` sashiko-bot
2026-10-17 1:41 ` Jonathan Cameron
2026-10-22 14:37 ` Nuno Sá
2 siblings, 0 replies; 27+ messages in thread
From: sashiko-bot @ 2026-10-15 9:43 UTC (permalink / raw)
To: Radu Sabau; +Cc: devicetree, robh, conor+dt
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] iio: imu: adis16607: interrupt storm livelock on SPI read failure in trigger handler
--
commit 37b208fdae15ca27441d6faa6832234ed11de06d
Author: Radu Sabau <radu.sabau@analog.com>
iio: imu: adis16607: Add FIFO-based buffered/triggered capture
This commit adds buffered capture via the device's hardware FIFO, which
is the only capture mode this driver supports. It handles the specific
command and payload format for FIFO data directly rather than using generic
adislib helpers.
> diff --git a/drivers/iio/imu/adis16607.c b/drivers/iio/imu/adis16607.c
> index 1d471f59f043d..dac1466454156 100644
> --- a/drivers/iio/imu/adis16607.c
> +++ b/drivers/iio/imu/adis16607.c
[ ... ]
> +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, threshold;
> + 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;
[ ... ]
> +unlock:
> + mutex_unlock(&st->adis.state_lock);
> + iio_trigger_notify_done(indio_dev->trig);
> + return IRQ_HANDLED;
> +}
[Severity: High]
Does this code risk an infinite interrupt storm livelock if the SPI read fails?
If __adis_read_reg_16() fails to read ADIS16607_REG_DIAG_STAT, the handler
jumps to the unlock label and returns IRQ_HANDLED.
Because the register read failed, the hardware interrupt status is not cleared
and the DEV_NOK pin remains asserted. Returning IRQ_HANDLED informs the kernel
IRQ subsystem that the interrupt was successfully serviced, which bypasses
the spurious interrupt detector.
When the threaded handler completes and unmasks the IRQ, won't it immediately
re-trigger and loop forever?
Should the handler return IRQ_NONE on unrecoverable bus errors so the kernel
can eventually disable the screaming interrupt?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261015-adis16607-v3-0-f972d4efee85@analog.com?part=4
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [PATCH v3 4/6] iio: imu: adis16607: Add FIFO-based buffered/triggered capture
2026-10-15 9:33 ` [PATCH v3 4/6] iio: imu: adis16607: Add FIFO-based buffered/triggered capture Radu Sabau via B4 Relay
2026-10-15 9:43 ` sashiko-bot
@ 2026-10-17 1:41 ` Jonathan Cameron
2026-10-19 8:48 ` Sabau, Radu bogdan
2026-10-22 14:37 ` Nuno Sá
2 siblings, 1 reply; 27+ messages in thread
From: Jonathan Cameron @ 2026-10-17 1:41 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
On Thu, 15 Oct 2026 12:33:01 +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>
Various comments inline. Biggest one is I don't follow why we
need to stop the fifo (which is a ring buffer given overwrite can't
happen in a fifo!)
Jonathan
> /* Self-test registers and masks */
> #define ADIS16607_REG_SELF_TEST_DATA(x) ((x) + 0x23)
> #define ADIS16607_SNSR_SELF_TEST_MASK BIT(6)
> @@ -84,6 +108,9 @@ struct adis16607 {
> struct adis adis;
> unsigned long clk_freq; /* millihertz */
> u64 sample_period_ns;
> + u16 fifo_watermark;
> + /* Scratch buffer used for FIFO pop operations, see adis16607_fifo_pop() */
> + u8 *fifo_buf;
As below. I'm not seeing why you don't just put the fifo in here
rather than via a separate allocatoin.
> /* IIO buffer for reordered data - sized for max channels */
> u8 data[64] __aligned(IIO_DMA_MINALIGN);
> };
> @@ -224,6 +251,352 @@ 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,
> +};
> +
> +static void adis16607_fifo_buf_cleanup(void *arg)
> +{
> + struct adis16607 *st = arg;
> +
> + kfree(st->fifo_buf);
> +}
> +
> +/*
> + * Sets up the scratch buffer used for FIFO pop operations (see
> + * adis16607_fifo_pop()). The 32-bit burst mode and DATA_CNTR config bits
> + * are enabled once during startup (adis16607_post_reset_setup()), not
> + * here, since they don't depend on scan_mask.
> + */
> +static int adis16607_update_scan_mode(struct iio_dev *indio_dev,
> + const unsigned long *scan_mask)
> +{
> + struct adis16607 *st = iio_priv(indio_dev);
> + unsigned int burst_len = ADIS16607_BURST_CMD_SIZE + ADIS16607_FIFO_DATA_LEN;
You lost me. That's fixed length, so why this dance? Just put
it in the iio_priv() which is allocated on stack anyway.
> +
> + kfree(st->fifo_buf);
> +
> + st->fifo_buf = kzalloc(burst_len, GFP_KERNEL);
> + if (!st->fifo_buf)
> + return -ENOMEM;
> +
> + return 0;
> +}
> +
> +/*
> + * 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)
A trigger handler that is associated with a fifo is usually a bad
idea. We often only use fifos if there is no trigger associated
because the meaning of one trigger ever scan is lost so they
become just a pointless intermediate layer. Not sure if we can
do that here.
> +{
> + 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, threshold;
> + 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;
> +
> + /*
> + * Disable the FIFO before draining: FIFO_THR_MET means it's at the
> + * configured threshold, and leaving it enabled while we pop would
> + * let incoming samples overwrite ones not yet read. No new samples
It will overwrite without overflowing? That's odd if true. If it's the
overflow case that was true before you started popping so no reason to
care about it more in this case. We generally don't disable fifos for
this reason. The description in the datasheet suggests this is a ring buffer
rather than a fifo as a fifo never overwrites of you'd not be getting
the 'first in' but such is the world of datasheets.
Does it at least guarantee no value tearing so worst that happens is we
get skips of old data if we read to slow?
Overall other than the overwrite not being controllable, this sounds
like most other fifos. We get an overflow if we don't read fast enough.
In that case and only that case we should flush (though I'm not sure
we do it even then when we have overwriting as only option).
In short, I'm not seeing the special bit of this implementation that makes
data loss inevitable. Perhaps some spec pointers and quotes would convince
me?
> + * are enqueued while disabled, so nothing arrives between here and
> + * the flush below - some sample-periods' worth of data not being
> + * captured during this window is inherent to this device's FIFO
> + * design (it has no secondary buffer), not something the driver can
> + * avoid.
> + */
> + ret = __adis_write_reg_16(&st->adis, ADIS16607_REG_USER_FIFO_CFG, 0);
> + if (ret)
> + goto unlock;
> +
> + /* Read FIFO word count */
> + ret = __adis_read_reg_16(&st->adis, ADIS16607_REG_FIFO_WORD_CNT, &word_cnt);
> + if (ret)
> + goto reenable_fifo;
> +
> + /* Calculate number of complete samples */
> + sample_cnt = word_cnt / ADIS16607_FIFO_WORDS_PER_SAMPLE;
> + if (!sample_cnt)
> + goto reenable_fifo;
> +
> + /*
> + * 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 reenable_fifo;
> +
> + adis16607_fifo_push_sample(indio_dev, fifo_sample, timestamp);
> + timestamp += st->sample_period_ns;
> + }
> +
> +reenable_fifo:
> + /* Flush FIFO before re-enabling */
This is also bothering me. The aim of the fifo is smoothly capture
everything. We should only end up flushing if an overflow has
definitely happened.
> + ret = __adis_write_reg_16(&st->adis, ADIS16607_REG_USER_FIFO_CFG,
> + ADIS16607_FIFO_FLUSH_MASK);
> + if (ret)
> + dev_warn_ratelimited(&st->adis.spi->dev,
> + "failed to flush FIFO: %d\n", ret);
> +
> + /* Re-enable FIFO with watermark threshold */
> + threshold = st->fifo_watermark * ADIS16607_FIFO_WORDS_PER_SAMPLE;
> + ret = __adis_write_reg_16(&st->adis, ADIS16607_REG_USER_FIFO_CFG,
> + threshold & ADIS16607_FIFO_THR_MASK);
> + if (ret)
> + dev_warn_ratelimited(&st->adis.spi->dev,
> + "failed to re-enable FIFO: %d\n", ret);
> +
> +unlock:
> + mutex_unlock(&st->adis.state_lock);
> + iio_trigger_notify_done(indio_dev->trig);
> + return IRQ_HANDLED;
> +}
> +
> +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;
> +
> + mutex_lock(&adis->state_lock);
> +
> + /* Flush FIFO to start clean */
> + ret = __adis_write_reg_16(adis, ADIS16607_REG_USER_FIFO_CFG,
> + ADIS16607_FIFO_FLUSH_MASK);
> + if (ret)
> + goto unlock;
> +
> + /* Enable FIFO with watermark threshold (in word count) */
> + threshold = st->fifo_watermark * ADIS16607_FIFO_WORDS_PER_SAMPLE;
> + ret = __adis_write_reg_16(adis, ADIS16607_REG_USER_FIFO_CFG,
> + threshold & ADIS16607_FIFO_THR_MASK);
> +
> +unlock:
As below. Then you can use direct returns on error.
> + mutex_unlock(&adis->state_lock);
> + return ret;
> +}
> +
> +static int adis16607_buffer_postdisable(struct iio_dev *indio_dev)
> +{
> + struct adis16607 *st = iio_priv(indio_dev);
> + struct adis *adis = &st->adis;
> + int ret;
> +
> + mutex_lock(&adis->state_lock);
Use the auto lock helper.
> +
> + /* Disable FIFO */
> + ret = __adis_write_reg_16(adis, ADIS16607_REG_USER_FIFO_CFG, 0);
> +
> + mutex_unlock(&adis->state_lock);
> + return ret;
> +}
> +
> +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_t(unsigned int, val, 1, ADIS16607_MAX_FIFO_WM);
Is the type forcing needed? i.e. why not clamp()?
> + st->fifo_watermark = val;
> +
> + return 0;
> +}
> @@ -798,6 +1197,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");
> @@ -816,6 +1217,21 @@ static int adis16607_probe(struct spi_device *spi)
> if (ret)
> return ret;
>
> + /*
> + * Register fifo_buf cleanup before buffer/trigger setup so that on
> + * devm teardown (LIFO order) the IRQ handler is removed first, then
> + * fifo_buf is freed, avoiding a use-after-free in the handler.
> + */
> + ret = devm_add_action_or_reset(dev, adis16607_fifo_buf_cleanup, st);
See above. I think this will go away when you don't allocate
the buffer in update_scan_mode().
> + 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);
Whilst it's a long line, align these after the (
> + if (ret)
> + return ret;
> +
> ret = devm_iio_device_register(dev, indio_dev);
> if (ret)
> return ret;
>
^ permalink raw reply [flat|nested] 27+ messages in thread* RE: [PATCH v3 4/6] iio: imu: adis16607: Add FIFO-based buffered/triggered capture
2026-10-17 1:41 ` Jonathan Cameron
@ 2026-10-19 8:48 ` Sabau, Radu bogdan
2026-10-21 19:11 ` Jonathan Cameron
0 siblings, 1 reply; 27+ messages in thread
From: Sabau, Radu bogdan @ 2026-10-19 8:48 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@vger.kernel.org,
linux-kernel@vger.kernel.org, devicetree@vger.kernel.org
> -----Original Message-----
> From: Jonathan Cameron <jic23@kernel.org>
> Sent: Saturday, October 17, 2026 4:41 AM
...
> > 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>
> Various comments inline. Biggest one is I don't follow why we
> need to stop the fifo (which is a ring buffer given overwrite can't
> happen in a fifo!)
>
> Jonathan
>
> > /* Self-test registers and masks */
> > #define ADIS16607_REG_SELF_TEST_DATA(x) ((x) + 0x23)
> > #define ADIS16607_SNSR_SELF_TEST_MASK BIT(6)
...
> > +
> > +/*
> > + * 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)
>
> A trigger handler that is associated with a fifo is usually a bad
> idea. We often only use fifos if there is no trigger associated
> because the meaning of one trigger ever scan is lost so they
> become just a pointless intermediate layer. Not sure if we can
> do that here.
Fair concern. I was looking at adis16475.c and noticed it already
does the same thing for its FIFO variants (adis1657x family) -
has_fifo = true, with a trigger handler that pops a variable sample
count per IRQ instead of exactly one. adis_trigger.c backs this up
too: it already special-cases FIFO devices with a level-triggered,
threaded IRQ instead of the edge-triggered one used for non-FIFO
devices, specifically because the watermark condition needs clearing
over SPI before the line deasserts.
So I thought that since this pattern is already established in the
shared library for exactly this kind of device, it made sense to
follow it here, too.
>
> > +{
> > + 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, threshold;
> > + 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;
> > +
> > + /*
> > + * Disable the FIFO before draining: FIFO_THR_MET means it's at the
> > + * configured threshold, and leaving it enabled while we pop would
> > + * let incoming samples overwrite ones not yet read. No new samples
> It will overwrite without overflowing? That's odd if true. If it's the
> overflow case that was true before you started popping so no reason to
> care about it more in this case. We generally don't disable fifos for
> this reason. The description in the datasheet suggests this is a ring buffer
> rather than a fifo as a fifo never overwrites of you'd not be getting
> the 'first in' but such is the world of datasheets.
>
I re-read the datasheet and I agree. I got it all wrong in the first place, as there
was a passage in the FIFO section saying new entries overwrite old ones, but
that is only if the capacity is insufficient. So it's exactly the overflow-only
overwrite you'd expect from a normal ring buffer - not something that happens
just because a pop is in progress.
> Does it at least guarantee no value tearing so worst that happens is we
> get skips of old data if we read to slow?
As per datasheet, "The memory is implemented as 1024 words using a dual-port
architecture so that new data writes and host FIFO reads proceed without
contention."
So no, no tearing - reads and writes are architected not to interfere with each other,
which is also exactly what makes draining without disabling safe.
>
> Overall other than the overwrite not being controllable, this sounds
> like most other fifos. We get an overflow if we don't read fast enough.
> In that case and only that case we should flush (though I'm not sure
> we do it even then when we have overwriting as only option).
>
On point with the flush, too. The flush is completely optional and makes no sense
to exist here anyway. Will be removing this.
> In short, I'm not seeing the special bit of this implementation that makes
> data loss inevitable. Perhaps some spec pointers and quotes would convince
> me?
>
To close the loop: there isn't one. Once FIFO_THR_MET's clear-on-read
behaviour and FIFO_DATA's pointer-advancing pop are accounted for,
this is a standard watermark-triggered ring buffer like any other -
nothing here actually needed the disable step. Dropping it in v4,
along with the flush-and-re-enable that followed it.
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [PATCH v3 4/6] iio: imu: adis16607: Add FIFO-based buffered/triggered capture
2026-10-19 8:48 ` Sabau, Radu bogdan
@ 2026-10-21 19:11 ` Jonathan Cameron
0 siblings, 0 replies; 27+ messages in thread
From: Jonathan Cameron @ 2026-10-21 19:11 UTC (permalink / raw)
To: Sabau, Radu bogdan
Cc: Radu Sabau via B4 Relay, Lars-Peter Clausen, Hennerich, Michael,
Sa, Nuno, David Lechner, Andy Shevchenko, Alexandru Ardelean,
Rob Herring, Krzysztof Kozlowski, Conor Dooley,
linux-iio@vger.kernel.org, linux-kernel@vger.kernel.org,
devicetree@vger.kernel.org
On Mon, 19 Oct 2026 08:48:59 +0000
"Sabau, Radu bogdan" <radu.sabau@analog.com> wrote:
> > -----Original Message-----
> > From: Jonathan Cameron <jic23@kernel.org>
> > Sent: Saturday, October 17, 2026 4:41 AM
>
> ...
>
> > > 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>
> > Various comments inline. Biggest one is I don't follow why we
> > need to stop the fifo (which is a ring buffer given overwrite can't
> > happen in a fifo!)
> >
> > Jonathan
> >
> > > /* Self-test registers and masks */
> > > #define ADIS16607_REG_SELF_TEST_DATA(x) ((x) + 0x23)
> > > #define ADIS16607_SNSR_SELF_TEST_MASK BIT(6)
>
> ...
>
> > > +
> > > +/*
> > > + * 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)
> >
> > A trigger handler that is associated with a fifo is usually a bad
> > idea. We often only use fifos if there is no trigger associated
> > because the meaning of one trigger ever scan is lost so they
> > become just a pointless intermediate layer. Not sure if we can
> > do that here.
>
> Fair concern. I was looking at adis16475.c and noticed it already
> does the same thing for its FIFO variants (adis1657x family) -
> has_fifo = true, with a trigger handler that pops a variable sample
> count per IRQ instead of exactly one. adis_trigger.c backs this up
> too: it already special-cases FIFO devices with a level-triggered,
> threaded IRQ instead of the edge-triggered one used for non-FIFO
> devices, specifically because the watermark condition needs clearing
> over SPI before the line deasserts.
>
> So I thought that since this pattern is already established in the
> shared library for exactly this kind of device, it made sense to
> follow it here, too.
The adis library is old + the fifo was retrofitted much
later - I can't remember if we discussed doing it differently
at the time. We might want to consider making no trigger correspond
to the fifo case just to be more inline with other drivers (whilst
allowing what works today by keeping that path there as well!)
Anyhow, as long as we have the validate callbacks to prevent the trigger
being used by another consumer then it is safe enough to still
have the trigger control being set.
Thanks,
Jonathan
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v3 4/6] iio: imu: adis16607: Add FIFO-based buffered/triggered capture
2026-10-15 9:33 ` [PATCH v3 4/6] iio: imu: adis16607: Add FIFO-based buffered/triggered capture Radu Sabau via B4 Relay
2026-10-15 9:43 ` sashiko-bot
2026-10-17 1:41 ` Jonathan Cameron
@ 2026-10-22 14:37 ` Nuno Sá
2 siblings, 0 replies; 27+ messages in thread
From: Nuno Sá @ 2026-10-22 14:37 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
On Thu, 15 Oct 2026 12:33:01 +0300, Radu Sabau <radu.sabau@analog.com> wrote:
> diff --git a/drivers/iio/imu/adis16607.c b/drivers/iio/imu/adis16607.c
> index 1d471f59f043..dac146645415 100644
> --- a/drivers/iio/imu/adis16607.c
> +++ b/drivers/iio/imu/adis16607.c
> @@ -224,6 +251,352 @@ static const struct adis_ops adis16607_ops = {
> [ ... skip 129 lines ... ]
> + 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) {
Use the proper helper for the above.
--
Nuno Sá <nuno.sa@analog.com>
^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH v3 5/6] iio: imu: adis16607: Add external clock sync support
2026-10-15 9:32 [PATCH v3 0/6] iio: imu: Add support for the ADI ADIS16607 Radu Sabau via B4 Relay
` (3 preceding siblings ...)
2026-10-15 9:33 ` [PATCH v3 4/6] iio: imu: adis16607: Add FIFO-based buffered/triggered capture Radu Sabau via B4 Relay
@ 2026-10-15 9:33 ` Radu Sabau via B4 Relay
2026-10-17 2:02 ` Jonathan Cameron
2026-10-15 9:33 ` [PATCH v3 6/6] iio: imu: adis16607: Add calibration bias support for gyro/accel Radu Sabau via B4 Relay
5 siblings, 1 reply; 27+ messages in thread
From: Radu Sabau via B4 Relay @ 2026-10-15 9:33 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, 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 | 115 +++++++++++++++++++++++++++++++++++++++++---
1 file changed, 108 insertions(+), 7 deletions(-)
diff --git a/drivers/iio/imu/adis16607.c b/drivers/iio/imu/adis16607.c
index dac146645415..c16a33ee773f 100644
--- a/drivers/iio/imu/adis16607.c
+++ b/drivers/iio/imu/adis16607.c
@@ -6,6 +6,7 @@
*/
#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>
@@ -46,6 +47,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
@@ -58,10 +60,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 */
@@ -88,8 +92,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;
@@ -101,6 +119,7 @@ struct adis16607_chip_info {
u32 deltvel_max_val;
u32 int_clk;
u16 max_dec;
+ u8 num_sync;
};
struct adis16607 {
@@ -108,6 +127,7 @@ struct adis16607 {
struct adis adis;
unsigned long clk_freq; /* millihertz */
u64 sample_period_ns;
+ u32 sync_mode;
u16 fifo_watermark;
/* Scratch buffer used for FIFO pop operations, see adis16607_fifo_pop() */
u8 *fifo_buf;
@@ -1106,6 +1126,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",
@@ -1145,7 +1171,9 @@ static const struct adis_data adis16607_data = {
.temp_scale = 5, \
.deltvel_max_val = IIO_G_TO_M_S_2(40), \
.int_clk = 8000, \
- .max_dec = 65535
+ .max_dec = 65535, \
+ .sync_mode = adis16607_sync_modes, \
+ .num_sync = ARRAY_SIZE(adis16607_sync_modes)
static const struct adis16607_chip_info adis16607_2_chip_info = {
ADIS16607_CHIP_INFO_COMMON,
@@ -1163,15 +1191,88 @@ static const struct adis16607_chip_info adis16607_3_chip_info = {
.deltang_max_val = IIO_DEGREE_TO_RAD(2000),
};
-/*
- * No external clock sync support yet: always run on the internal clock.
- * Default dec=0 after reset, so period = 1 / clk_freq.
- */
static int adis16607_config_clk(struct adis16607 *st)
{
- st->clk_freq = (unsigned long)st->info->int_clk * 1000;
+ 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;
+ 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, 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 - use the real value here so sample_period_ns (and
+ * therefore FIFO sample timestamps) don't drift from it.
+ */
+ st->clk_freq = (unsigned long)ext_clk_hz * sync_scale * 1000;
+ break;
+ case ADIS16607_SYNC_DIRECT:
+ /* DIRECT: external clock is the effective internal rate */
+ st->clk_freq = ext_clk_hz * 1000;
+ break;
+ default:
+ st->clk_freq = (unsigned long)st->info->int_clk * 1000;
+ break;
+ }
+
+ /* Default dec=0 after reset: period = 1 / clk_freq */
st->sample_period_ns = DIV_ROUND_CLOSEST_ULL((u64)NSEC_PER_SEC * 1000ULL,
- st->clk_freq);
+ st->clk_freq);
+
return 0;
}
--
2.43.0
^ permalink raw reply related [flat|nested] 27+ messages in thread* Re: [PATCH v3 5/6] iio: imu: adis16607: Add external clock sync support
2026-10-15 9:33 ` [PATCH v3 5/6] iio: imu: adis16607: Add external clock sync support Radu Sabau via B4 Relay
@ 2026-10-17 2:02 ` Jonathan Cameron
2026-10-19 9:08 ` Sabau, Radu bogdan
0 siblings, 1 reply; 27+ messages in thread
From: Jonathan Cameron @ 2026-10-17 2:02 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
On Thu, 15 Oct 2026 12:33:02 +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>
Just a very quick look as I'm out of time for today.
> +
> + /* Default dec=0 after reset: period = 1 / clk_freq */
Why does the comment make sense now but didn't before?
> st->sample_period_ns = DIV_ROUND_CLOSEST_ULL((u64)NSEC_PER_SEC * 1000ULL,
> - st->clk_freq);
> + st->clk_freq);
Tidy this up.
> +
> return 0;
> }
>
>
^ permalink raw reply [flat|nested] 27+ messages in thread
* RE: [PATCH v3 5/6] iio: imu: adis16607: Add external clock sync support
2026-10-17 2:02 ` Jonathan Cameron
@ 2026-10-19 9:08 ` Sabau, Radu bogdan
0 siblings, 0 replies; 27+ messages in thread
From: Sabau, Radu bogdan @ 2026-10-19 9:08 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@vger.kernel.org,
linux-kernel@vger.kernel.org, devicetree@vger.kernel.org
> -----Original Message-----
> From: Jonathan Cameron <jic23@kernel.org>
> Sent: Saturday, October 17, 2026 5:03 AM
...
> > 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>
>
> Just a very quick look as I'm out of time for today.
>
> > +
> > + /* Default dec=0 after reset: period = 1 / clk_freq */
>
> Why does the comment make sense now but didn't before?
Yep, this is a double standard... For USER_SYNC I chose to
write the full state explicitly regardless of its reset default,
specifically so the code doesn't depend on that default holding..
Dropping the comment and the assumption: I'll read DEC_RATE
here instead, same as adis16607_get_freq() already does, so it's
consistent either way rather than picking and choosing.
^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH v3 6/6] iio: imu: adis16607: Add calibration bias support for gyro/accel
2026-10-15 9:32 [PATCH v3 0/6] iio: imu: Add support for the ADI ADIS16607 Radu Sabau via B4 Relay
` (4 preceding siblings ...)
2026-10-15 9:33 ` [PATCH v3 5/6] iio: imu: adis16607: Add external clock sync support Radu Sabau via B4 Relay
@ 2026-10-15 9:33 ` Radu Sabau via B4 Relay
5 siblings, 0 replies; 27+ messages in thread
From: Radu Sabau via B4 Relay @ 2026-10-15 9:33 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, 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 c16a33ee773f..3aeca5dfa882 100644
--- a/drivers/iio/imu/adis16607.c
+++ b/drivers/iio/imu/adis16607.c
@@ -53,6 +53,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 */
@@ -151,6 +157,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:
@@ -962,6 +977,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;
@@ -999,6 +1015,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)
@@ -1017,6 +1043,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) {
@@ -1024,6 +1051,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;
}
@@ -1033,7 +1065,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 related [flat|nested] 27+ messages in thread