* [PATCH v4 0/6] Add support for ADIS16550 and ADIS16550W
@ 2025-01-10 7:42 Robert Budai
2025-01-10 7:42 ` [PATCH v4 1/6] iio: imu: adis: Add custom ops struct Robert Budai
` (5 more replies)
0 siblings, 6 replies; 21+ messages in thread
From: Robert Budai @ 2025-01-10 7:42 UTC (permalink / raw)
To: Nuno Sa, Ramona Gradinariu, Antoniu Miclaus, Lars-Peter Clausen,
Michael Hennerich, Jonathan Cameron, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Jonathan Corbet, Robert Budai,
Alex Lanzano, linux-iio, devicetree, linux-kernel, linux-doc
Add support for ADIS16550 and ADIS16550W
Robert Budai (6):
iio: imu: adis: Add custom ops struct
iio: imu: adis: Add reset to custom ops
iio: imu: adis: Add DIAG_STAT register size
dt-bindings: iio: Add adis16550 bindings
iio: imu: adis16550: add adis16550 support
docs: iio: add documentation for adis16550 driver
.../bindings/iio/imu/adi,adis16550.yaml | 96 ++
Documentation/iio/adis16550.rst | 378 ++++++
Documentation/iio/index.rst | 1 +
MAINTAINERS | 9 +
drivers/iio/imu/Kconfig | 13 +
drivers/iio/imu/Makefile | 1 +
drivers/iio/imu/adis.c | 30 +-
drivers/iio/imu/adis16550.c | 1202 +++++++++++++++++
include/linux/iio/imu/adis.h | 33 +-
9 files changed, 1749 insertions(+), 14 deletions(-)
create mode 100644 Documentation/devicetree/bindings/iio/imu/adi,adis16550.yaml
create mode 100644 Documentation/iio/adis16550.rst
create mode 100644 drivers/iio/imu/adis16550.c
--
2.34.1
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH v4 1/6] iio: imu: adis: Add custom ops struct
2025-01-10 7:42 [PATCH v4 0/6] Add support for ADIS16550 and ADIS16550W Robert Budai
@ 2025-01-10 7:42 ` Robert Budai
2025-01-10 7:42 ` [PATCH v4 2/6] iio: imu: adis: Add reset to custom ops Robert Budai
` (4 subsequent siblings)
5 siblings, 0 replies; 21+ messages in thread
From: Robert Budai @ 2025-01-10 7:42 UTC (permalink / raw)
To: Nuno Sa, Ramona Gradinariu, Antoniu Miclaus, Lars-Peter Clausen,
Michael Hennerich, Jonathan Cameron, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Jonathan Corbet, Robert Budai,
Alex Lanzano, linux-iio, devicetree, linux-kernel, linux-doc
This patch introduces a custom ops struct letting users define
custom read and write functions. Some adis devices might define
a completely different spi protocol from the one used in the
default implementation.
Co-developed-by: Ramona Gradinariu <ramona.gradinariu@analog.com>
Signed-off-by: Ramona Gradinariu <ramona.gradinariu@analog.com>
Co-developed-by: Antoniu Miclaus <antoniu.miclaus@analog.com>
Signed-off-by: Antoniu Miclaus <antoniu.miclaus@analog.com>
Signed-off-by: Nuno Sá <nuno.sa@analog.com>
Signed-off-by: Robert Budai <robert.budai@analog.com>
---
drivers/iio/imu/adis.c | 16 +++++++++++++---
include/linux/iio/imu/adis.h | 28 +++++++++++++++++++++-------
2 files changed, 34 insertions(+), 10 deletions(-)
diff --git a/drivers/iio/imu/adis.c b/drivers/iio/imu/adis.c
index 494171844812..54915c7a3e76 100644
--- a/drivers/iio/imu/adis.c
+++ b/drivers/iio/imu/adis.c
@@ -223,13 +223,13 @@ int __adis_update_bits_base(struct adis *adis, unsigned int reg, const u32 mask,
int ret;
u32 __val;
- ret = __adis_read_reg(adis, reg, &__val, size);
+ ret = adis->ops->read(adis, reg, &__val, size);
if (ret)
return ret;
__val = (__val & ~mask) | (val & mask);
- return __adis_write_reg(adis, reg, __val, size);
+ return adis->ops->write(adis, reg, __val, size);
}
EXPORT_SYMBOL_NS_GPL(__adis_update_bits_base, "IIO_ADISLIB");
@@ -468,7 +468,7 @@ int adis_single_conversion(struct iio_dev *indio_dev,
guard(mutex)(&adis->state_lock);
- ret = __adis_read_reg(adis, chan->address, &uval,
+ ret = adis->ops->read(adis, chan->address, &uval,
chan->scan_type.storagebits / 8);
if (ret)
return ret;
@@ -488,6 +488,11 @@ 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,
+};
+
/**
* adis_init() - Initialize adis device structure
* @adis: The adis device
@@ -517,6 +522,11 @@ int adis_init(struct adis *adis, struct iio_dev *indio_dev,
adis->spi = spi;
adis->data = data;
+ if (!adis->ops->write && !adis->ops->read)
+ adis->ops = &adis_default_ops;
+ else if (!adis->ops->write || !adis->ops->read)
+ return -EINVAL;
+
iio_device_set_drvdata(indio_dev, adis);
if (data->has_paging) {
diff --git a/include/linux/iio/imu/adis.h b/include/linux/iio/imu/adis.h
index 4bb98d9731de..89cfa75ae9ea 100644
--- a/include/linux/iio/imu/adis.h
+++ b/include/linux/iio/imu/adis.h
@@ -94,6 +94,18 @@ struct adis_data {
unsigned int burst_max_speed_hz;
};
+/**
+ * struct adis_ops: Custom ops for adis devices.
+ * @write: Custom spi write implementation.
+ * @read: Custom spi read implementation.
+ */
+struct adis_ops {
+ int (*write)(struct adis *adis, unsigned int reg, unsigned int value,
+ unsigned int size);
+ int (*read)(struct adis *adis, unsigned int reg, unsigned int *value,
+ unsigned int size);
+};
+
/**
* struct adis - ADIS device instance data
* @spi: Reference to SPI device which owns this ADIS IIO device
@@ -101,6 +113,7 @@ struct adis_data {
* @data: ADIS chip variant specific data
* @burst_extra_len: Burst extra length. Should only be used by devices that can
* dynamically change their burst mode length.
+ * @ops: ops struct for custom read and write functions
* @state_lock: Lock used by the device to protect state
* @msg: SPI message object
* @xfer: SPI transfer objects to be used for a @msg
@@ -116,6 +129,7 @@ struct adis {
const struct adis_data *data;
unsigned int burst_extra_len;
+ const struct adis_ops *ops;
/**
* The state_lock is meant to be used during operations that require
* a sequence of SPI R/W in order to protect the SPI transfer
@@ -168,7 +182,7 @@ int __adis_read_reg(struct adis *adis, unsigned int reg,
static inline int __adis_write_reg_8(struct adis *adis, unsigned int reg,
u8 val)
{
- return __adis_write_reg(adis, reg, val, 1);
+ return adis->ops->write(adis, reg, val, 1);
}
/**
@@ -180,7 +194,7 @@ static inline int __adis_write_reg_8(struct adis *adis, unsigned int reg,
static inline int __adis_write_reg_16(struct adis *adis, unsigned int reg,
u16 val)
{
- return __adis_write_reg(adis, reg, val, 2);
+ return adis->ops->write(adis, reg, val, 2);
}
/**
@@ -192,7 +206,7 @@ static inline int __adis_write_reg_16(struct adis *adis, unsigned int reg,
static inline int __adis_write_reg_32(struct adis *adis, unsigned int reg,
u32 val)
{
- return __adis_write_reg(adis, reg, val, 4);
+ return adis->ops->write(adis, reg, val, 4);
}
/**
@@ -207,7 +221,7 @@ static inline int __adis_read_reg_16(struct adis *adis, unsigned int reg,
unsigned int tmp;
int ret;
- ret = __adis_read_reg(adis, reg, &tmp, 2);
+ ret = adis->ops->read(adis, reg, &tmp, 2);
if (ret == 0)
*val = tmp;
@@ -226,7 +240,7 @@ static inline int __adis_read_reg_32(struct adis *adis, unsigned int reg,
unsigned int tmp;
int ret;
- ret = __adis_read_reg(adis, reg, &tmp, 4);
+ ret = adis->ops->read(adis, reg, &tmp, 4);
if (ret == 0)
*val = tmp;
@@ -244,7 +258,7 @@ static inline int adis_write_reg(struct adis *adis, unsigned int reg,
unsigned int val, unsigned int size)
{
guard(mutex)(&adis->state_lock);
- return __adis_write_reg(adis, reg, val, size);
+ return adis->ops->write(adis, reg, val, size);
}
/**
@@ -258,7 +272,7 @@ static int adis_read_reg(struct adis *adis, unsigned int reg,
unsigned int *val, unsigned int size)
{
guard(mutex)(&adis->state_lock);
- return __adis_read_reg(adis, reg, val, size);
+ return adis->ops->read(adis, reg, val, size);
}
/**
--
2.34.1
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH v4 2/6] iio: imu: adis: Add reset to custom ops
2025-01-10 7:42 [PATCH v4 0/6] Add support for ADIS16550 and ADIS16550W Robert Budai
2025-01-10 7:42 ` [PATCH v4 1/6] iio: imu: adis: Add custom ops struct Robert Budai
@ 2025-01-10 7:42 ` Robert Budai
2025-01-12 15:35 ` Jonathan Cameron
2025-01-10 7:42 ` [PATCH v4 3/6] iio: imu: adis: Add DIAG_STAT register size Robert Budai
` (3 subsequent siblings)
5 siblings, 1 reply; 21+ messages in thread
From: Robert Budai @ 2025-01-10 7:42 UTC (permalink / raw)
To: Nuno Sa, Ramona Gradinariu, Antoniu Miclaus, Lars-Peter Clausen,
Michael Hennerich, Jonathan Cameron, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Jonathan Corbet, Robert Budai,
Alex Lanzano, linux-iio, devicetree, linux-kernel, linux-doc
This patch allows the custom definition of reset functionality
for adis object. It is useful in cases where the driver does not
need to sleep after the reset since it is handled by the library.
Co-developed-by: Ramona Gradinariu <ramona.gradinariu@analog.com>
Signed-off-by: Ramona Gradinariu <ramona.gradinariu@analog.com>
Co-developed-by: Antoniu Miclaus <antoniu.miclaus@analog.com>
Signed-off-by: Antoniu Miclaus <antoniu.miclaus@analog.com>
Signed-off-by: Nuno Sá <nuno.sa@analog.com>
Signed-off-by: Robert Budai <robert.budai@analog.com>
---
drivers/iio/imu/adis.c | 7 ++++---
include/linux/iio/imu/adis.h | 3 +++
2 files changed, 7 insertions(+), 3 deletions(-)
diff --git a/drivers/iio/imu/adis.c b/drivers/iio/imu/adis.c
index 54915c7a3e76..9e4113473dc4 100644
--- a/drivers/iio/imu/adis.c
+++ b/drivers/iio/imu/adis.c
@@ -340,7 +340,7 @@ int __adis_reset(struct adis *adis)
const struct adis_timeout *timeouts = adis->data->timeouts;
ret = __adis_write_reg_8(adis, adis->data->glob_cmd_reg,
- ADIS_GLOB_CMD_SW_RESET);
+ ADIS_GLOB_CMD_SW_RESET);
if (ret) {
dev_err(&adis->spi->dev, "Failed to reset device: %d\n", ret);
return ret;
@@ -491,6 +491,7 @@ 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,
};
/**
@@ -522,9 +523,9 @@ int adis_init(struct adis *adis, struct iio_dev *indio_dev,
adis->spi = spi;
adis->data = data;
- if (!adis->ops->write && !adis->ops->read)
+ if (!adis->ops->write && !adis->ops->read && !adis->ops->reset)
adis->ops = &adis_default_ops;
- else if (!adis->ops->write || !adis->ops->read)
+ else if (!adis->ops->write || !adis->ops->read || !adis->ops->reset)
return -EINVAL;
iio_device_set_drvdata(indio_dev, adis);
diff --git a/include/linux/iio/imu/adis.h b/include/linux/iio/imu/adis.h
index 89cfa75ae9ea..52652f51db2e 100644
--- a/include/linux/iio/imu/adis.h
+++ b/include/linux/iio/imu/adis.h
@@ -98,12 +98,15 @@ struct adis_data {
* struct adis_ops: Custom ops for adis devices.
* @write: Custom spi write implementation.
* @read: Custom spi read implementation.
+ * @reset: Custom sw reset implementation. The custom implementation does not
+ * need to sleep after the reset. It's done by the library already.
*/
struct adis_ops {
int (*write)(struct adis *adis, unsigned int reg, unsigned int value,
unsigned int size);
int (*read)(struct adis *adis, unsigned int reg, unsigned int *value,
unsigned int size);
+ int (*reset)(struct adis *adis);
};
/**
--
2.34.1
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH v4 3/6] iio: imu: adis: Add DIAG_STAT register size
2025-01-10 7:42 [PATCH v4 0/6] Add support for ADIS16550 and ADIS16550W Robert Budai
2025-01-10 7:42 ` [PATCH v4 1/6] iio: imu: adis: Add custom ops struct Robert Budai
2025-01-10 7:42 ` [PATCH v4 2/6] iio: imu: adis: Add reset to custom ops Robert Budai
@ 2025-01-10 7:42 ` Robert Budai
2025-01-12 15:39 ` Jonathan Cameron
2025-01-10 7:42 ` [PATCH v4 4/6] dt-bindings: iio: Add adis16550 bindings Robert Budai
` (2 subsequent siblings)
5 siblings, 1 reply; 21+ messages in thread
From: Robert Budai @ 2025-01-10 7:42 UTC (permalink / raw)
To: Nuno Sa, Ramona Gradinariu, Antoniu Miclaus, Lars-Peter Clausen,
Michael Hennerich, Jonathan Cameron, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Jonathan Corbet, Robert Budai,
Alex Lanzano, linux-iio, devicetree, linux-kernel, linux-doc
Some devices may have more than 16 bits of status. This patch allows the
user to specify the size of the DIAG_STAT register. It defaults to 2 if
not specified. This is mainly for backward compatibility.
Co-developed-by: Ramona Gradinariu <ramona.gradinariu@analog.com>
Signed-off-by: Ramona Gradinariu <ramona.gradinariu@analog.com>
Co-developed-by: Antoniu Miclaus <antoniu.miclaus@analog.com>
Signed-off-by: Antoniu Miclaus <antoniu.miclaus@analog.com>
Signed-off-by: Nuno Sá <nuno.sa@analog.com>
Signed-off-by: Robert Budai <robert.budai@analog.com>
---
drivers/iio/imu/adis.c | 11 ++++++++---
include/linux/iio/imu/adis.h | 2 ++
2 files changed, 10 insertions(+), 3 deletions(-)
diff --git a/drivers/iio/imu/adis.c b/drivers/iio/imu/adis.c
index 9e4113473dc4..a072307bfb6a 100644
--- a/drivers/iio/imu/adis.c
+++ b/drivers/iio/imu/adis.c
@@ -304,11 +304,15 @@ EXPORT_SYMBOL_NS(__adis_enable_irq, "IIO_ADISLIB");
*/
int __adis_check_status(struct adis *adis)
{
- u16 status;
+ unsigned int status;
int ret;
int i;
- ret = __adis_read_reg_16(adis, adis->data->diag_stat_reg, &status);
+ if (adis->data->diag_stat_size)
+ ret = adis->ops->read(adis, adis->data->diag_stat_reg, &status,
+ adis->data->diag_stat_size);
+ else
+ ret = __adis_read_reg_16(adis, adis->data->diag_stat_reg, (u16 *)&status);
if (ret)
return ret;
@@ -317,7 +321,8 @@ int __adis_check_status(struct adis *adis)
if (status == 0)
return 0;
- for (i = 0; i < 16; ++i) {
+ for (i = 0; i < BITS_PER_BYTE * ((adis->data->diag_stat_size) ?
+ adis->data->diag_stat_size : 2); ++i) {
if (status & BIT(i)) {
dev_err(&adis->spi->dev, "%s.\n",
adis->data->status_error_msgs[i]);
diff --git a/include/linux/iio/imu/adis.h b/include/linux/iio/imu/adis.h
index 52652f51db2e..b888b22f5c8c 100644
--- a/include/linux/iio/imu/adis.h
+++ b/include/linux/iio/imu/adis.h
@@ -44,6 +44,7 @@ struct adis_timeout {
* @glob_cmd_reg: Register address of the GLOB_CMD register
* @msc_ctrl_reg: Register address of the MSC_CTRL register
* @diag_stat_reg: Register address of the DIAG_STAT register
+ * @diag_stat_size: Length (in bytes) of the DIAG_STAT register.
* @prod_id_reg: Register address of the PROD_ID register
* @prod_id: Product ID code that should be expected when reading @prod_id_reg
* @self_test_mask: Bitmask of supported self-test operations
@@ -70,6 +71,7 @@ struct adis_data {
unsigned int glob_cmd_reg;
unsigned int msc_ctrl_reg;
unsigned int diag_stat_reg;
+ unsigned int diag_stat_size;
unsigned int prod_id_reg;
unsigned int prod_id;
--
2.34.1
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH v4 4/6] dt-bindings: iio: Add adis16550 bindings
2025-01-10 7:42 [PATCH v4 0/6] Add support for ADIS16550 and ADIS16550W Robert Budai
` (2 preceding siblings ...)
2025-01-10 7:42 ` [PATCH v4 3/6] iio: imu: adis: Add DIAG_STAT register size Robert Budai
@ 2025-01-10 7:42 ` Robert Budai
2025-01-12 15:48 ` Jonathan Cameron
2025-01-13 8:43 ` Krzysztof Kozlowski
2025-01-10 7:42 ` [PATCH v4 5/6] iio: imu: adis16550: add adis16550 support Robert Budai
2025-01-10 7:42 ` [PATCH v4 6/6] docs: iio: add documentation for adis16550 driver Robert Budai
5 siblings, 2 replies; 21+ messages in thread
From: Robert Budai @ 2025-01-10 7:42 UTC (permalink / raw)
To: Nuno Sa, Ramona Gradinariu, Antoniu Miclaus, Lars-Peter Clausen,
Michael Hennerich, Jonathan Cameron, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Jonathan Corbet, Robert Budai,
Alex Lanzano, linux-iio, devicetree, linux-kernel, linux-doc
Document the ADIS16550 device devicetree bindings.
Co-developed-by: Antoniu Miclaus <antoniu.miclaus@analog.com>
Signed-off-by: Antoniu Miclaus <antoniu.miclaus@analog.com>
Signed-off-by: Ramona Gradinariu <ramona.gradinariu@analog.com>
Signed-off-by: Robert Budai <robert.budai@analog.com>
---
4:
- applied styling changes to the bindings file
- restricted sync-mode to intervals 1-2
.../bindings/iio/imu/adi,adis16550.yaml | 96 +++++++++++++++++++
MAINTAINERS | 9 ++
2 files changed, 105 insertions(+)
create mode 100644 Documentation/devicetree/bindings/iio/imu/adi,adis16550.yaml
diff --git a/Documentation/devicetree/bindings/iio/imu/adi,adis16550.yaml b/Documentation/devicetree/bindings/iio/imu/adi,adis16550.yaml
new file mode 100644
index 000000000000..e7ccf3883e55
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/imu/adi,adis16550.yaml
@@ -0,0 +1,96 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/iio/imu/adi,adis16550.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Analog Devices ADIS16550 and similar IMUs
+
+maintainers:
+ - Nuno Sa <nuno.sa@analog.com>
+ - Ramona Gradinariu <ramona.gradinariu@analog.com>
+ - Antoniu Miclaus <antoniu.miclaus@analog.com>
+
+properties:
+ compatible:
+ enum:
+ - adi,adis16550
+ - adi,adis16550w
+
+ reg:
+ maxItems: 1
+
+ spi-cpha: true
+
+ spi-cpol: true
+
+ spi-max-frequency:
+ maximum: 15000000
+
+ vdd-supply: true
+
+ interrupts:
+ maxItems: 1
+
+ reset-gpios:
+ description:
+ RESET active low pin.
+ maxItems: 1
+
+ clocks:
+ maxItems: 1
+ description: If not provided, then the internal clock is used.
+
+ adi,sync-mode:
+ description:
+ Configures the device SYNC pin. The following modes are supported
+ 0 - output_sync
+ 1 - direct_sync
+ 2 - scaled_sync
+ 3 - pulse_sync
+ $ref: /schemas/types.yaml#/definitions/uint32
+ minimum: 1
+ maximum: 2
+
+required:
+ - compatible
+ - reg
+ - interrupts
+ - spi-cpha
+ - spi-cpol
+ - spi-max-frequency
+ - vdd-supply
+
+allOf:
+ - if:
+ properties:
+ adi,sync-mode:
+ const: 2
+
+ then:
+ dependencies:
+ adi,sync-mode: [ clocks ]
+
+ - $ref: /schemas/spi/spi-peripheral-props.yaml#
+
+unevaluatedProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/interrupt-controller/irq.h>
+ spi {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ imu@0 {
+ compatible = "adi,adis16550";
+ reg = <0>;
+ spi-max-frequency = <15000000>;
+ spi-cpol;
+ spi-cpha;
+ vdd-supply = <&vdd>;
+ interrupts = <4 IRQ_TYPE_EDGE_FALLING>;
+ interrupt-parent = <&gpio>;
+ };
+ };
+...
diff --git a/MAINTAINERS b/MAINTAINERS
index 30cbc3d44cd5..89ef9571e025 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1453,6 +1453,15 @@ W: https://ez.analog.com/linux-software-drivers
F: Documentation/devicetree/bindings/iio/imu/adi,adis16475.yaml
F: drivers/iio/imu/adis16475.c
+ANALOG DEVICES INC ADIS16550 DRIVER
+M: Nuno Sa <nuno.sa@analog.com>
+M: Ramona Gradinariu <ramona.gradinariu@analog.com>
+M: Antoniu Miclaus <antoniu.miclaus@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,adis16550.yaml
+
ANALOG DEVICES INC ADM1177 DRIVER
M: Michael Hennerich <Michael.Hennerich@analog.com>
L: linux-hwmon@vger.kernel.org
--
2.34.1
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH v4 5/6] iio: imu: adis16550: add adis16550 support
2025-01-10 7:42 [PATCH v4 0/6] Add support for ADIS16550 and ADIS16550W Robert Budai
` (3 preceding siblings ...)
2025-01-10 7:42 ` [PATCH v4 4/6] dt-bindings: iio: Add adis16550 bindings Robert Budai
@ 2025-01-10 7:42 ` Robert Budai
2025-01-11 12:03 ` kernel test robot
` (2 more replies)
2025-01-10 7:42 ` [PATCH v4 6/6] docs: iio: add documentation for adis16550 driver Robert Budai
5 siblings, 3 replies; 21+ messages in thread
From: Robert Budai @ 2025-01-10 7:42 UTC (permalink / raw)
To: Nuno Sa, Ramona Gradinariu, Antoniu Miclaus, Lars-Peter Clausen,
Michael Hennerich, Jonathan Cameron, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Jonathan Corbet, Robert Budai,
Alex Lanzano, linux-iio, devicetree, linux-kernel, linux-doc
The ADIS16550 is a complete inertial system that includes a triaxis
gyroscope and a triaxis accelerometer. Each inertial sensor in
the ADIS16550 combines industry leading MEMS only technology
with signal conditioning that optimizes dynamic performance. The
factory calibration characterizes each sensor for sensitivity, bias,
and alignment. As a result, each sensor has its own dynamic com-
pensation formulas that provide accurate sensor measurements
Co-developed-by: Ramona Gradinariu <ramona.gradinariu@analog.com>
Signed-off-by: Ramona Gradinariu <ramona.gradinariu@analog.com>
Co-developed-by: Antoniu Miclaus <antoniu.miclaus@analog.com>
Signed-off-by: Antoniu Miclaus <antoniu.miclaus@analog.com>
Signed-off-by: Nuno Sá <nuno.sa@analog.com>
Signed-off-by: Robert Budai <robert.budai@analog.com>
---
4:
- reorganized channels to match the order in the datasheet
- removed extra checks and goto statements
- for buffer memory allocation used only kfree, since adis library already takes care of freeing the buffer
drivers/iio/imu/Kconfig | 13 +
drivers/iio/imu/Makefile | 1 +
drivers/iio/imu/adis16550.c | 1202 +++++++++++++++++++++++++++++++++++
3 files changed, 1216 insertions(+)
create mode 100644 drivers/iio/imu/adis16550.c
diff --git a/drivers/iio/imu/Kconfig b/drivers/iio/imu/Kconfig
index ca0efecb5b5c..c4e06ebbe058 100644
--- a/drivers/iio/imu/Kconfig
+++ b/drivers/iio/imu/Kconfig
@@ -52,6 +52,19 @@ config ADIS16480
Say yes here to build support for Analog Devices ADIS16375, ADIS16480,
ADIS16485, ADIS16488 inertial sensors.
+config ADIS16550
+ tristate "Analog Devices ADIS16550 and similar IMU driver"
+ depends on SPI
+ select IIO_ADIS_LIB
+ select IIO_ADIS_LIB_BUFFER if IIO_BUFFER
+ select CRC32
+ help
+ Say yes here to build support for Analog Devices ADIS16550 inertial
+ sensor.
+
+ To compile this driver as a module, choose M here: the module will be
+ called adis16550.
+
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 04c77c2c4df8..e901aea498d3 100644
--- a/drivers/iio/imu/Makefile
+++ b/drivers/iio/imu/Makefile
@@ -8,6 +8,7 @@ obj-$(CONFIG_ADIS16400) += adis16400.o
obj-$(CONFIG_ADIS16460) += adis16460.o
obj-$(CONFIG_ADIS16475) += adis16475.o
obj-$(CONFIG_ADIS16480) += adis16480.o
+obj-$(CONFIG_ADIS16550) += adis16550.o
adis_lib-y += adis.o
adis_lib-$(CONFIG_IIO_ADIS_LIB_BUFFER) += adis_trigger.o
diff --git a/drivers/iio/imu/adis16550.c b/drivers/iio/imu/adis16550.c
new file mode 100644
index 000000000000..49c3ff9ef1e2
--- /dev/null
+++ b/drivers/iio/imu/adis16550.c
@@ -0,0 +1,1202 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * ADIS16550 IMU driver
+ *
+ * Copyright 2024 Analog Devices Inc.
+ */
+#include <linux/bitfield.h>
+#include <linux/bitops.h>
+#include <linux/clk.h>
+#include <linux/crc32.h>
+#include <linux/debugfs.h>
+#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/lcm.h>
+#include <linux/math.h>
+#include <linux/module.h>
+#include <linux/mod_devicetable.h>
+#include <linux/regulator/consumer.h>
+#include <linux/spi/spi.h>
+#include <linux/swab.h>
+#include <linux/unaligned.h>
+
+#define ADIS16550_REG_BURST_GYRO_ACCEL 0x0a
+#define ADIS16550_REG_BURST_DELTA_ANG_VEL 0x0b
+#define ADIS16550_BURST_DATA_GYRO_ACCEL_MASK GENMASK(6, 1)
+#define ADIS16550_BURST_DATA_DELTA_ANG_VEL_MASK GENMASK(12, 7)
+
+#define ADIS16550_REG_STATUS 0x0e
+#define ADIS16550_REG_TEMP 0x10
+#define ADIS16550_REG_X_GYRO 0x12
+#define ADIS16550_REG_Y_GYRO 0x14
+#define ADIS16550_REG_Z_GYRO 0x16
+#define ADIS16550_REG_X_ACCEL 0x18
+#define ADIS16550_REG_Y_ACCEL 0x1a
+#define ADIS16550_REG_Z_ACCEL 0x1c
+#define ADIS16550_REG_X_DELTANG_L 0x1E
+#define ADIS16550_REG_Y_DELTANG_L 0x20
+#define ADIS16550_REG_Z_DELTANG_L 0x22
+#define ADIS16550_REG_X_DELTVEL_L 0x24
+#define ADIS16550_REG_Y_DELTVEL_L 0x26
+#define ADIS16550_REG_Z_DELTVEL_L 0x28
+#define ADIS16550_REG_X_GYRO_SCALE 0x30
+#define ADIS16550_REG_Y_GYRO_SCALE 0x32
+#define ADIS16550_REG_Z_GYRO_SCALE 0x34
+#define ADIS16550_REG_X_ACCEL_SCALE 0x36
+#define ADIS16550_REG_Y_ACCEL_SCALE 0x38
+#define ADIS16550_REG_Z_ACCEL_SCALE 0x3a
+#define ADIS16550_REG_X_GYRO_BIAS 0x40
+#define ADIS16550_REG_Y_GYRO_BIAS 0x42
+#define ADIS16550_REG_Z_GYRO_BIAS 0x44
+#define ADIS16550_REG_X_ACCEL_BIAS 0x46
+#define ADIS16550_REG_Y_ACCEL_BIAS 0x48
+#define ADIS16550_REG_Z_ACCEL_BIAS 0x4a
+#define ADIS16550_REG_COMMAND 0x50
+#define ADIS16550_REG_CONFIG 0x52
+#define ADIS16550_GYRO_FIR_EN_MASK BIT(3)
+#define ADIS16550_ACCL_FIR_EN_MASK BIT(2)
+#define ADIS16550_SYNC_MASK \
+ (ADIS16550_SYNC_EN_MASK | ADIS16550_SYNC_MODE_MASK)
+#define ADIS16550_SYNC_MODE_MASK BIT(1)
+#define ADIS16550_SYNC_EN_MASK BIT(0)
+/* max of 4000 SPS in scale sync */
+#define ADIS16550_SYNC_SCALE_MAX_RATE (4000 * 1000)
+#define ADIS16550_REG_DEC_RATE 0x54
+#define ADIS16550_REG_SYNC_SCALE 0x56
+#define ADIS16550_REG_SERIAL_NUM 0x76
+#define ADIS16550_REG_FW_REV 0x7A
+#define ADIS16550_REG_FW_DATE 0x7C
+#define ADIS16550_REG_PROD_ID 0x7E
+#define ADIS16550_REG_FLASH_CNT 0x72
+/* spi protocol*/
+#define ADIS16550_SPI_DATA_MASK GENMASK(31, 16)
+#define ADIS16550_SPI_REG_MASK GENMASK(14, 8)
+#define ADIS16550_SPI_R_W_MASK BIT(7)
+#define ADIS16550_SPI_CRC_MASK GENMASK(3, 0)
+#define ADIS16550_SPI_SV_MASK GENMASK(7, 6)
+/* burst read */
+#define ADIS16550_BURST_N_ELEM 12
+#define ADIS16550_BURST_DATA_LEN (ADIS16550_BURST_N_ELEM * 4)
+#define ADIS16550_MAX_SCAN_DATA 12
+
+struct adis16550_sync {
+ u16 sync_mode;
+ u16 min_rate;
+ u16 max_rate;
+};
+
+struct adis16550_chip_info {
+ const struct iio_chan_spec *channels;
+ const struct adis16550_sync *sync_mode;
+ 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;
+ u16 num_sync;
+};
+
+struct adis16550 {
+ const struct adis16550_chip_info *info;
+ struct adis adis;
+ unsigned long clk_freq_hz;
+ u32 sync_mode;
+};
+
+enum {
+ ADIS16550_SV_INIT,
+ ADIS16550_SV_OK,
+ ADIS16550_SV_NOK,
+ ADIS16550_SV_SPI_ERROR,
+};
+
+/*
+ * This is a simplified implementation of lib/crc4.c. It could not be used
+ * directly since the polynomial used is different from the one used by the
+ * 16550 which is 0b10001
+ */
+static u8 spi_crc4(const u32 val)
+{
+ int i;
+ const int bits = 28;
+ u8 crc = 0xa;
+ /* ignore 4lsb */
+ const u32 __val = val >> 4;
+
+ /* Calculate crc4 over four-bit nibbles, starting at the MSbit */
+ for (i = bits - 4; i >= 0; i -= 4)
+ crc = crc ^ ((__val >> i) & 0xf);
+
+ return crc;
+}
+
+static int adis16550_spi_validate(const struct adis *adis, __be32 dout,
+ u16 *data)
+{
+ u32 __dout;
+ u8 crc, crc_rcv, sv;
+
+ __dout = be32_to_cpu(dout);
+
+ /* validate received message */
+ crc_rcv = FIELD_GET(ADIS16550_SPI_CRC_MASK, __dout);
+ crc = spi_crc4(__dout);
+ if (crc_rcv != crc) {
+ dev_err(&adis->spi->dev,
+ "Invalid crc, rcv: 0x%02x, calc: 0x%02x!\n",
+ crc_rcv, crc);
+ return -EIO;
+ }
+ sv = FIELD_GET(ADIS16550_SPI_SV_MASK, __dout);
+ if (sv >= ADIS16550_SV_NOK) {
+ dev_err(&adis->spi->dev,
+ "State vector error detected: %02X", sv);
+ return -EIO;
+ }
+ *data = FIELD_GET(ADIS16550_SPI_DATA_MASK, __dout);
+
+ return 0;
+}
+
+static void adis16550_spi_msg_prepare(const u32 reg, const bool write,
+ const u16 data, __be32 *din)
+{
+ u8 crc;
+ u32 __din;
+
+ __din = FIELD_PREP(ADIS16550_SPI_REG_MASK, reg);
+
+ if (write) {
+ __din |= FIELD_PREP(ADIS16550_SPI_R_W_MASK, true);
+ __din |= FIELD_PREP(ADIS16550_SPI_DATA_MASK, data);
+ }
+
+ crc = spi_crc4(__din);
+ __din |= FIELD_PREP(ADIS16550_SPI_CRC_MASK, crc);
+
+ *din = cpu_to_be32(__din);
+}
+
+static int adis16550_spi_xfer(const struct adis *adis, u32 reg, u32 len,
+ u32 *readval, u32 writeval)
+{
+ int ret;
+ u16 data = 0;
+ struct spi_message msg;
+ __be32 __din[2], __dout[2];
+ bool wr = readval ? false : true;
+ struct spi_device *spi = adis->spi;
+ struct spi_transfer xfers[] = {
+ {
+ .tx_buf = &__din[0],
+ .len = 4,
+ .cs_change = 1,
+ }, {
+ .tx_buf = &__din[1],
+ .len = 4,
+ .cs_change = 1,
+ .rx_buf = __dout,
+ }, {
+ .tx_buf = &__din[1],
+ .rx_buf = &__dout[1],
+ .len = 4,
+ },
+ };
+
+ spi_message_init(&msg);
+
+ switch (len) {
+ case 4:
+ adis16550_spi_msg_prepare(reg + 1, wr, writeval >> 16,
+ &__din[0]);
+ spi_message_add_tail(&xfers[0], &msg);
+ fallthrough;
+ case 2:
+ adis16550_spi_msg_prepare(reg, wr, writeval, &__din[1]);
+ spi_message_add_tail(&xfers[1], &msg);
+ spi_message_add_tail(&xfers[2], &msg);
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ ret = spi_sync(spi, &msg);
+ if (ret) {
+ dev_err(&spi->dev, "Spi failure %d\n", ret);
+ return ret;
+ }
+ /*
+ * When writing a register, the device will reply with a readback on the
+ * transfer so that we can validate if our data was actually written..
+ */
+ switch (len) {
+ case 4:
+ ret = adis16550_spi_validate(adis, __dout[0], &data);
+ if (ret)
+ return ret;
+
+ if (readval) {
+ *readval = data << 16;
+ } else if ((writeval >> 16) != data && reg != ADIS16550_REG_COMMAND) {
+ dev_err(&spi->dev,
+ "Data not written: wr: 0x%04X, rcv: 0x%04X\n",
+ writeval >> 16, data);
+ return -EIO;
+ }
+
+ fallthrough;
+ case 2:
+ ret = adis16550_spi_validate(adis, __dout[1], &data);
+ if (ret)
+ return ret;
+
+ if (readval) {
+ *readval = (*readval & GENMASK(31, 16)) | data;
+ } else if ((writeval & GENMASK(15, 0)) != data && reg != ADIS16550_REG_COMMAND) {
+ dev_err(&spi->dev,
+ "Data not written: wr: 0x%04X, rcv: 0x%04X\n",
+ (u16)writeval, data);
+ return -EIO;
+ }
+ }
+
+ return 0;
+}
+
+static int adis16550_spi_read(struct adis *adis, const u32 reg,
+ u32 *value, const u32 len)
+{
+ return adis16550_spi_xfer(adis, reg, len, value, 0);
+}
+
+static int adis16550_spi_write(struct adis *adis, const u32 reg,
+ const u32 value, const u32 len)
+{
+ return adis16550_spi_xfer(adis, reg, len, NULL, value);
+}
+
+static ssize_t adis16550_show_firmware_revision(struct file *file,
+ char __user *userbuf,
+ size_t count, loff_t *ppos)
+{
+ struct adis16550 *st = file->private_data;
+ char buf[7];
+ size_t len;
+ u16 rev;
+ int ret;
+
+ ret = adis_read_reg_16(&st->adis, ADIS16550_REG_FW_REV, &rev);
+ if (ret)
+ return ret;
+
+ len = scnprintf(buf, sizeof(buf), "%x.%x\n", rev >> 8, rev & 0xff);
+
+ return simple_read_from_buffer(userbuf, count, ppos, buf, len);
+}
+
+static const struct file_operations adis16550_firmware_revision_fops = {
+ .open = simple_open,
+ .read = adis16550_show_firmware_revision,
+ .llseek = default_llseek,
+ .owner = THIS_MODULE,
+};
+
+static ssize_t adis16550_show_firmware_date(struct file *file,
+ char __user *userbuf,
+ size_t count, loff_t *ppos)
+{
+ struct adis16550 *st = file->private_data;
+ u32 date;
+ char buf[12];
+ size_t len;
+ int ret;
+
+ ret = adis_read_reg_32(&st->adis, ADIS16550_REG_FW_DATE, &date);
+ if (ret)
+ return ret;
+
+ len = scnprintf(buf, sizeof(buf), "%.2x-%.2x-%.4x\n", date & 0xff,
+ (date >> 8) & 0xff, date >> 16);
+
+ return simple_read_from_buffer(userbuf, count, ppos, buf, len);
+}
+
+static const struct file_operations adis16550_firmware_date_fops = {
+ .open = simple_open,
+ .read = adis16550_show_firmware_date,
+ .llseek = default_llseek,
+ .owner = THIS_MODULE,
+};
+
+static int adis16550_show_serial_number(void *arg, u64 *val)
+{
+ struct adis16550 *st = arg;
+ u32 serial;
+ int ret;
+
+ ret = adis_read_reg_32(&st->adis, ADIS16550_REG_SERIAL_NUM, &serial);
+ if (ret)
+ return ret;
+
+ *val = serial;
+
+ return 0;
+}
+DEFINE_DEBUGFS_ATTRIBUTE(adis16550_serial_number_fops,
+ adis16550_show_serial_number, NULL, "0x%.8llx\n");
+
+static int adis16550_show_product_id(void *arg, u64 *val)
+{
+ struct adis16550 *st = arg;
+ u16 prod_id;
+ int ret;
+
+ ret = adis_read_reg_16(&st->adis, ADIS16550_REG_PROD_ID, &prod_id);
+ if (ret)
+ return ret;
+
+ *val = prod_id;
+
+ return 0;
+}
+DEFINE_DEBUGFS_ATTRIBUTE(adis16550_product_id_fops,
+ adis16550_show_product_id, NULL, "%llu\n");
+
+static int adis16550_show_flash_count(void *arg, u64 *val)
+{
+ struct adis16550 *st = arg;
+ u16 flash_count;
+ int ret;
+
+ ret = adis_read_reg_16(&st->adis, ADIS16550_REG_FLASH_CNT, &flash_count);
+ if (ret)
+ return ret;
+
+ *val = flash_count;
+
+ return 0;
+}
+DEFINE_DEBUGFS_ATTRIBUTE(adis16550_flash_count_fops,
+ adis16550_show_flash_count, NULL, "%lld\n");
+
+static void adis16550_debugfs_init(struct iio_dev *indio_dev)
+{
+ struct adis16550 *st = iio_priv(indio_dev);
+ struct dentry *d = iio_get_debugfs_dentry(indio_dev);
+
+ debugfs_create_file_unsafe("serial_number", 0400,
+ d, st, &adis16550_serial_number_fops);
+ debugfs_create_file_unsafe("product_id", 0400,
+ d, st, &adis16550_product_id_fops);
+ debugfs_create_file("firmware_revision", 0400,
+ d, st, &adis16550_firmware_revision_fops);
+ debugfs_create_file("firmware_date", 0400, d,
+ st, &adis16550_firmware_date_fops);
+ debugfs_create_file_unsafe("flash_count", 0400,
+ d, st, &adis16550_flash_count_fops);
+}
+
+enum {
+ ADIS16550_SYNC_MODE_DIRECT,
+ ADIS16550_SYNC_MODE_SCALED,
+};
+
+static int adis16550_get_freq(struct adis16550 *st, u32 *freq)
+{
+ int ret;
+ u16 dec = 0;
+ u32 sample_rate = st->clk_freq_hz;
+
+ adis_dev_auto_lock(&st->adis);
+
+ if (st->sync_mode == ADIS16550_SYNC_MODE_SCALED) {
+ u16 sync_scale;
+
+ ret = __adis_read_reg_16(&st->adis, ADIS16550_REG_SYNC_SCALE, &sync_scale);
+ if (ret)
+ return ret;
+
+ sample_rate = st->clk_freq_hz * sync_scale;
+ }
+
+ ret = __adis_read_reg_16(&st->adis, ADIS16550_REG_DEC_RATE, &dec);
+ if (ret)
+ return -EINVAL;
+ *freq = DIV_ROUND_CLOSEST(sample_rate, dec + 1);
+
+ return 0;
+}
+
+static int adis16550_set_freq_hz(struct adis16550 *st, u32 freq_hz)
+{
+ u16 dec;
+ int ret;
+ u32 sample_rate = st->clk_freq_hz;
+ /*
+ * The optimal sample rate for the supported IMUs is between
+ * int_clk - 1000 and int_clk + 500.
+ */
+ u32 max_sample_rate = st->info->int_clk * 1000 + 500000;
+ u32 min_sample_rate = st->info->int_clk * 1000 - 1000000;
+
+ if (!freq_hz)
+ return -EINVAL;
+
+ adis_dev_auto_lock(&st->adis);
+
+ if (st->sync_mode == ADIS16550_SYNC_MODE_SCALED) {
+ unsigned long scaled_rate = lcm(st->clk_freq_hz, freq_hz);
+ int sync_scale;
+
+ if (scaled_rate > max_sample_rate)
+ scaled_rate = max_sample_rate / st->clk_freq_hz * st->clk_freq_hz;
+ else
+ scaled_rate = max_sample_rate / scaled_rate * scaled_rate;
+
+ if (scaled_rate < min_sample_rate)
+ scaled_rate = roundup(min_sample_rate, st->clk_freq_hz);
+
+ sync_scale = scaled_rate / st->clk_freq_hz;
+ ret = __adis_write_reg_16(&st->adis, ADIS16550_REG_SYNC_SCALE,
+ sync_scale);
+ if (ret)
+ return ret;
+
+ sample_rate = scaled_rate;
+ }
+
+ dec = DIV_ROUND_CLOSEST(sample_rate, freq_hz);
+
+ if (dec)
+ dec--;
+
+ dec = min(dec, st->info->max_dec);
+
+ return __adis_write_reg_16(&st->adis, ADIS16550_REG_DEC_RATE, dec);
+}
+
+static int adis16550_get_accl_filter_freq(struct adis16550 *st, int *freq_hz)
+{
+ int ret;
+ u16 config = 0;
+
+ ret = adis_read_reg_16(&st->adis, ADIS16550_REG_CONFIG, &config);
+ if (ret)
+ return -EINVAL;
+
+ if (FIELD_GET(ADIS16550_ACCL_FIR_EN_MASK, config))
+ *freq_hz = 100;
+ else
+ *freq_hz = 0;
+
+ return 0;
+}
+
+static int adis16550_set_accl_filter_freq(struct adis16550 *st, int freq_hz)
+{
+ bool en = false;
+
+ if (freq_hz)
+ en = true;
+
+ return __adis_update_bits(&st->adis, ADIS16550_REG_CONFIG,
+ ADIS16550_ACCL_FIR_EN_MASK,
+ (u32)FIELD_PREP(ADIS16550_ACCL_FIR_EN_MASK, en));
+}
+
+static int adis16550_get_gyro_filter_freq(struct adis16550 *st, int *freq_hz)
+{
+ int ret;
+ u16 config = 0;
+
+ ret = adis_read_reg_16(&st->adis, ADIS16550_REG_CONFIG, &config);
+ if (ret)
+ return -EINVAL;
+
+ if (FIELD_GET(ADIS16550_GYRO_FIR_EN_MASK, config))
+ *freq_hz = 100;
+ else
+ *freq_hz = 0;
+
+ return 0;
+}
+
+static int adis16550_set_gyro_filter_freq(struct adis16550 *st, int freq_hz)
+{
+ bool en = false;
+
+ if (freq_hz)
+ en = true;
+
+ return __adis_update_bits(&st->adis, ADIS16550_REG_CONFIG,
+ ADIS16550_GYRO_FIR_EN_MASK,
+ (u32)FIELD_PREP(ADIS16550_GYRO_FIR_EN_MASK, en));
+}
+
+enum {
+ ADIS16550_SCAN_TEMP,
+ ADIS16550_SCAN_GYRO_X,
+ ADIS16550_SCAN_GYRO_Y,
+ ADIS16550_SCAN_GYRO_Z,
+ ADIS16550_SCAN_ACCEL_X,
+ ADIS16550_SCAN_ACCEL_Y,
+ ADIS16550_SCAN_ACCEL_Z,
+ ADIS16550_SCAN_DELTANG_X,
+ ADIS16550_SCAN_DELTANG_Y,
+ ADIS16550_SCAN_DELTANG_Z,
+ ADIS16550_SCAN_DELTVEL_X,
+ ADIS16550_SCAN_DELTVEL_Y,
+ ADIS16550_SCAN_DELTVEL_Z,
+};
+
+static const u32 adis16550_calib_bias[] = {
+ [ADIS16550_SCAN_GYRO_X] = ADIS16550_REG_X_GYRO_BIAS,
+ [ADIS16550_SCAN_GYRO_Y] = ADIS16550_REG_Y_GYRO_BIAS,
+ [ADIS16550_SCAN_GYRO_Z] = ADIS16550_REG_Z_GYRO_BIAS,
+ [ADIS16550_SCAN_ACCEL_X] = ADIS16550_REG_X_ACCEL_BIAS,
+ [ADIS16550_SCAN_ACCEL_Y] = ADIS16550_REG_Y_ACCEL_BIAS,
+ [ADIS16550_SCAN_ACCEL_Z] = ADIS16550_REG_Z_ACCEL_BIAS,
+
+};
+
+static const u32 adis16550_calib_scale[] = {
+ [ADIS16550_SCAN_GYRO_X] = ADIS16550_REG_X_GYRO_SCALE,
+ [ADIS16550_SCAN_GYRO_Y] = ADIS16550_REG_Y_GYRO_SCALE,
+ [ADIS16550_SCAN_GYRO_Z] = ADIS16550_REG_Z_GYRO_SCALE,
+ [ADIS16550_SCAN_ACCEL_X] = ADIS16550_REG_X_ACCEL_SCALE,
+ [ADIS16550_SCAN_ACCEL_Y] = ADIS16550_REG_Y_ACCEL_SCALE,
+ [ADIS16550_SCAN_ACCEL_Z] = ADIS16550_REG_Z_ACCEL_SCALE,
+};
+
+static int adis16550_read_raw(struct iio_dev *indio_dev,
+ const struct iio_chan_spec *chan,
+ int *val, int *val2, long info)
+{
+ struct adis16550 *st = iio_priv(indio_dev);
+ const int idx = chan->scan_index;
+ u16 scale;
+ 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 = 31;
+ return IIO_VAL_FRACTIONAL_LOG2;
+ case IIO_DELTA_VELOCITY:
+ *val = st->info->deltvel_max_val;
+ *val2 = 31;
+ return IIO_VAL_FRACTIONAL_LOG2;
+ default:
+ return -EINVAL;
+ }
+ case IIO_CHAN_INFO_OFFSET:
+ /* temperature centered at 25°C */
+ *val = DIV_ROUND_CLOSEST(25000, st->info->temp_scale);
+ return IIO_VAL_INT;
+ case IIO_CHAN_INFO_CALIBBIAS:
+ ret = adis_read_reg_32(&st->adis,
+ adis16550_calib_bias[idx], val);
+ if (ret)
+ return ret;
+
+ return IIO_VAL_INT;
+ case IIO_CHAN_INFO_CALIBSCALE:
+ ret = adis_read_reg_16(&st->adis,
+ adis16550_calib_scale[idx], &scale);
+ if (ret)
+ return ret;
+
+ *val = scale;
+ return IIO_VAL_INT;
+ case IIO_CHAN_INFO_SAMP_FREQ:
+ ret = adis16550_get_freq(st, &tmp);
+ if (ret)
+ return ret;
+
+ *val = tmp / 1000;
+ *val2 = (tmp % 1000) * 1000;
+ return IIO_VAL_INT_PLUS_MICRO;
+ case IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY:
+ switch (chan->type) {
+ case IIO_ANGL_VEL:
+ ret = adis16550_get_accl_filter_freq(st, val);
+ if (ret)
+ return ret;
+ return IIO_VAL_INT;
+ case IIO_ACCEL:
+ ret = adis16550_get_gyro_filter_freq(st, val);
+ if (ret)
+ return ret;
+ return IIO_VAL_INT;
+ default:
+ return -EINVAL;
+ }
+ default:
+ return -EINVAL;
+ }
+}
+
+static int adis16550_write_raw(struct iio_dev *indio_dev,
+ const struct iio_chan_spec *chan,
+ int val, int val2, long info)
+{
+ struct adis16550 *st = iio_priv(indio_dev);
+ const int idx = chan->scan_index;
+ u32 tmp;
+
+ switch (info) {
+ case IIO_CHAN_INFO_SAMP_FREQ:
+ tmp = val * 1000 + val2 / 1000;
+ return adis16550_set_freq_hz(st, tmp);
+ case IIO_CHAN_INFO_CALIBBIAS:
+ return adis_write_reg_32(&st->adis, adis16550_calib_bias[idx],
+ val);
+ case IIO_CHAN_INFO_CALIBSCALE:
+ return adis_write_reg_16(&st->adis, adis16550_calib_scale[idx],
+ val);
+ case IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY:
+ switch (chan->type) {
+ case IIO_ANGL_VEL:
+ return adis16550_set_accl_filter_freq(st, val);
+ case IIO_ACCEL:
+ return adis16550_set_gyro_filter_freq(st, val);
+ default:
+ return -EINVAL;
+ }
+ default:
+ return -EINVAL;
+ }
+}
+
+#define ADIS16550_MOD_CHAN(_type, _mod, _address, _si) \
+ { \
+ .type = (_type), \
+ .modified = 1, \
+ .channel2 = (_mod), \
+ .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) | \
+ BIT(IIO_CHAN_INFO_CALIBBIAS) | \
+ BIT(IIO_CHAN_INFO_CALIBSCALE), \
+ .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE) | \
+ BIT(IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY), \
+ .info_mask_shared_by_all = BIT(IIO_CHAN_INFO_SAMP_FREQ), \
+ .address = (_address), \
+ .scan_index = (_si), \
+ .scan_type = { \
+ .sign = 's', \
+ .realbits = 32, \
+ .storagebits = 32, \
+ .endianness = IIO_BE, \
+ }, \
+ }
+
+#define ADIS16550_GYRO_CHANNEL(_mod) \
+ ADIS16550_MOD_CHAN(IIO_ANGL_VEL, IIO_MOD_ ## _mod, \
+ ADIS16550_REG_ ## _mod ## _GYRO, ADIS16550_SCAN_GYRO_ ## _mod)
+
+#define ADIS16550_ACCEL_CHANNEL(_mod) \
+ ADIS16550_MOD_CHAN(IIO_ACCEL, IIO_MOD_ ## _mod, \
+ ADIS16550_REG_ ## _mod ## _ACCEL, ADIS16550_SCAN_ACCEL_ ## _mod)
+
+#define ADIS16550_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 = ADIS16550_REG_TEMP, \
+ .scan_index = ADIS16550_SCAN_TEMP, \
+ .scan_type = { \
+ .sign = 's', \
+ .realbits = 16, \
+ .storagebits = 32, \
+ .endianness = IIO_BE, \
+ }, \
+ }
+
+#define ADIS16550_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 = 32, \
+ .storagebits = 32, \
+ .endianness = IIO_BE, \
+ }, \
+ }
+
+#define ADIS16550_DELTANG_CHAN(_mod) \
+ ADIS16550_MOD_CHAN_DELTA(IIO_DELTA_ANGL, IIO_MOD_ ## _mod, \
+ ADIS16550_REG_ ## _mod ## _DELTANG_L, ADIS16550_SCAN_DELTANG_ ## _mod)
+
+#define ADIS16550_DELTVEL_CHAN(_mod) \
+ ADIS16550_MOD_CHAN_DELTA(IIO_DELTA_VELOCITY, IIO_MOD_ ## _mod, \
+ ADIS16550_REG_ ## _mod ## _DELTVEL_L, ADIS16550_SCAN_DELTVEL_ ## _mod)
+
+#define ADIS16550_DELTANG_CHAN_NO_SCAN(_mod) \
+ ADIS16550_MOD_CHAN_DELTA(IIO_DELTA_ANGL, IIO_MOD_ ## _mod, \
+ ADIS16550_REG_ ## _mod ## _DELTANG_L, -1)
+
+#define ADIS16550_DELTVEL_CHAN_NO_SCAN(_mod) \
+ ADIS16550_MOD_CHAN_DELTA(IIO_DELTA_VELOCITY, IIO_MOD_ ## _mod, \
+ ADIS16550_REG_ ## _mod ## _DELTVEL_L, -1)
+
+static const struct iio_chan_spec adis16550_channels[] = {
+ ADIS16550_TEMP_CHANNEL(),
+ ADIS16550_GYRO_CHANNEL(X),
+ ADIS16550_GYRO_CHANNEL(Y),
+ ADIS16550_GYRO_CHANNEL(Z),
+ ADIS16550_ACCEL_CHANNEL(X),
+ ADIS16550_ACCEL_CHANNEL(Y),
+ ADIS16550_ACCEL_CHANNEL(Z),
+ ADIS16550_DELTANG_CHAN(X),
+ ADIS16550_DELTANG_CHAN(Y),
+ ADIS16550_DELTANG_CHAN(Z),
+ ADIS16550_DELTVEL_CHAN(X),
+ ADIS16550_DELTVEL_CHAN(Y),
+ ADIS16550_DELTVEL_CHAN(Z),
+ IIO_CHAN_SOFT_TIMESTAMP(13),
+};
+
+static const struct adis16550_sync adis16550_sync_modes[] = {
+ { ADIS16550_SYNC_MODE_DIRECT, 3000, 4500 },
+ { ADIS16550_SYNC_MODE_SCALED, 1, 128 },
+};
+
+static const struct adis16550_chip_info adis16550_chip_info = {
+ .num_channels = ARRAY_SIZE(adis16550_channels),
+ .channels = adis16550_channels,
+ .name = "adis16550",
+ .gyro_max_val = 1,
+ .gyro_max_scale = IIO_RAD_TO_DEGREE(80 << 16),
+ .accel_max_val = 1,
+ .accel_max_scale = IIO_M_S_2_TO_G(102400000),
+ .temp_scale = 4,
+ .deltang_max_val = IIO_DEGREE_TO_RAD(720),
+ .deltvel_max_val = 125,
+ .int_clk = 4000,
+ .max_dec = 4095,
+ .sync_mode = adis16550_sync_modes,
+ .num_sync = ARRAY_SIZE(adis16550_sync_modes),
+};
+
+static const struct adis16550_chip_info adis16550w_chip_info = {
+ .num_channels = ARRAY_SIZE(adis16550_channels),
+ .channels = adis16550_channels,
+ .name = "adis16550w",
+ .gyro_max_val = 1,
+ .gyro_max_scale = IIO_RAD_TO_DEGREE(80 << 16),
+ .accel_max_val = 1,
+ .accel_max_scale = IIO_M_S_2_TO_G(102400000),
+ .temp_scale = 4,
+ .deltang_max_val = IIO_DEGREE_TO_RAD(720),
+ .deltvel_max_val = 125,
+ .int_clk = 4000,
+ .max_dec = 4095,
+ .sync_mode = adis16550_sync_modes,
+ .num_sync = ARRAY_SIZE(adis16550_sync_modes),
+};
+
+static u32 adis16550_validate_crc(const u32 *buf, const u8 n_elem,
+ const u32 crc)
+{
+ int i;
+ u32 crc_calc;
+ u32 crc_buf[ADIS16550_BURST_N_ELEM - 2];
+ /*
+ * The crc calculation of the data is done in little endian. Hence, we
+ * always swap the 32bit elements making sure that the data LSB is
+ * always on address 0...
+ */
+ for (i = 0; i < n_elem; i++)
+ crc_buf[i] = swab32(buf[i]);
+
+ crc_calc = crc32(~0, crc_buf, n_elem * 4);
+ crc_calc ^= ~0;
+
+ return (crc_calc == crc);
+}
+
+static irqreturn_t adis16550_trigger_handler(int irq, void *p)
+{
+ u32 crc;
+ u16 dummy;
+ bool valid;
+ int ret, i = 0;
+ u8 data_offset = 3;
+ struct iio_poll_func *pf = p;
+ __be32 data[ADIS16550_MAX_SCAN_DATA];
+ struct iio_dev *indio_dev = pf->indio_dev;
+ struct adis16550 *st = iio_priv(indio_dev);
+ struct adis *adis = iio_device_get_drvdata(indio_dev);
+ __be32 *buffer = adis->buffer;
+
+ ret = spi_sync(adis->spi, &adis->msg);
+ if (ret)
+ goto done;
+ /*
+ * Validate the header. The header is a normal spi reply with state
+ * vector and crc4.
+ */
+ ret = adis16550_spi_validate(&st->adis, buffer[0], &dummy);
+ if (ret)
+ goto done;
+
+ crc = be32_to_cpu(buffer[ADIS16550_BURST_N_ELEM - 1]);
+ /* the header is not included in the crc */
+ valid = adis16550_validate_crc((u32 *)&buffer[1],
+ ADIS16550_BURST_N_ELEM - 2, crc);
+ if (!valid) {
+ dev_err(&adis->spi->dev, "Burst Invalid crc!\n");
+ goto done;
+ }
+
+ memcpy(&data[i], &buffer[data_offset], (ADIS16550_SCAN_ACCEL_Z -
+ ADIS16550_SCAN_GYRO_X + 2) *
+ sizeof(__be32));
+ iio_push_to_buffers_with_timestamp(indio_dev, data, pf->timestamp);
+done:
+ iio_trigger_notify_done(indio_dev->trig);
+ return IRQ_HANDLED;
+}
+
+static const unsigned long adis16550_channel_masks[] = {
+ ADIS16550_BURST_DATA_GYRO_ACCEL_MASK | BIT(ADIS16550_SCAN_TEMP),
+ ADIS16550_BURST_DATA_DELTA_ANG_VEL_MASK | BIT(ADIS16550_SCAN_TEMP),
+};
+
+static int adis16550_update_scan_mode(struct iio_dev *indio_dev,
+ const unsigned long *scan_mask)
+{
+ struct adis *adis = iio_device_get_drvdata(indio_dev);
+ u16 burst_length = ADIS16550_BURST_DATA_LEN;
+ u8 burst_cmd;
+ u8 *tx;
+
+ if (*scan_mask & ADIS16550_BURST_DATA_GYRO_ACCEL_MASK)
+ burst_cmd = ADIS16550_REG_BURST_GYRO_ACCEL;
+ else
+ burst_cmd = ADIS16550_REG_BURST_DELTA_ANG_VEL;
+
+ memset(adis->xfer, 0, 2 * sizeof(*adis->xfer));
+ memset(adis->buffer, 0, burst_length + sizeof(u32));
+
+ tx = adis->buffer + burst_length;
+ tx[0] = 0x00;
+ tx[1] = 0x00;
+ tx[2] = burst_cmd;
+ /* crc4 is 0 on burst command */
+ tx[3] = spi_crc4(get_unaligned_le32(tx));
+
+ adis->xfer[0].tx_buf = tx;
+ adis->xfer[0].len = 4;
+ adis->xfer[0].cs_change = 1;
+ adis->xfer[0].delay.value = 8;
+ adis->xfer[0].delay.unit = SPI_DELAY_UNIT_USECS;
+ adis->xfer[1].rx_buf = adis->buffer;
+ adis->xfer[1].len = burst_length;
+
+ spi_message_init_with_transfers(&adis->msg, adis->xfer, 2);
+
+ return 0;
+}
+
+static int adis16550_reset(struct adis *adis)
+{
+ return __adis_write_reg_16(adis, ADIS16550_REG_COMMAND, BIT(15));
+}
+
+static int adis16550_config_sync(struct adis16550 *st)
+{
+ struct device *dev = &st->adis.spi->dev;
+ const struct adis16550_sync *sync;
+ struct clk *clk;
+ u32 sync_mode;
+ u16 mode;
+ int ret;
+
+ ret = device_property_read_u32(dev, "adi,sync-mode", &sync_mode);
+ if (ret) {
+ st->clk_freq_hz = st->info->int_clk * 1000;
+ return 0;
+ }
+
+ if (sync_mode >= st->info->num_sync) {
+ dev_err(dev, "Invalid sync mode: %u for %s\n", sync_mode,
+ st->info->name);
+ return -EINVAL;
+ }
+
+ sync = &st->info->sync_mode[sync_mode];
+ st->sync_mode = sync->sync_mode;
+
+ clk = devm_clk_get_enabled(dev, NULL);
+ if (IS_ERR(clk))
+ return PTR_ERR(clk);
+
+ st->clk_freq_hz = clk_get_rate(clk);
+ if (st->clk_freq_hz > sync->max_rate || st->clk_freq_hz < sync->min_rate)
+ return dev_err_probe(dev, -EINVAL,
+ "Clk rate: %lu not in a valid range:[%u %u]\n",
+ st->clk_freq_hz, sync->min_rate, sync->max_rate);
+
+ if (sync->sync_mode == ADIS16550_SYNC_MODE_SCALED) {
+ u16 sync_scale;
+ u32 scaled_freq = 0;
+ /*
+ * In pps scaled sync we must scale the input clock to a range
+ * of [3000 45000].
+ */
+ ret = device_property_read_u32(dev, "adi,scaled-output-hz",
+ &scaled_freq);
+ if (ret)
+ return dev_err_probe(dev, ret,
+ "adi,scaled-output-hz property not found");
+
+ if (scaled_freq < 3000 || scaled_freq > 4500)
+ return dev_err_probe(dev, -EINVAL,
+ "Invalid value:%u for adi,scaled-output-hz",
+ scaled_freq);
+
+ sync_scale = DIV_ROUND_CLOSEST(scaled_freq, st->clk_freq_hz);
+
+ ret = adis_write_reg_16(&st->adis, ADIS16550_REG_SYNC_SCALE,
+ sync_scale);
+ if (ret)
+ return ret;
+
+ st->clk_freq_hz = scaled_freq;
+ }
+
+ st->clk_freq_hz *= 1000;
+
+ mode = FIELD_PREP(ADIS16550_SYNC_MODE_MASK, sync->sync_mode) |
+ FIELD_PREP(ADIS16550_SYNC_EN_MASK, true);
+
+ return __adis_update_bits(&st->adis, ADIS16550_REG_CONFIG,
+ ADIS16550_SYNC_MASK, mode);
+}
+
+static const struct iio_info adis16550_info = {
+ .read_raw = &adis16550_read_raw,
+ .write_raw = &adis16550_write_raw,
+ .update_scan_mode = adis16550_update_scan_mode,
+ .debugfs_reg_access = adis_debugfs_reg_access,
+};
+
+enum {
+ ADIS16550_STATUS_CRC_CODE,
+ ADIS16550_STATUS_CRC_CONFIG,
+ ADIS16550_STATUS_FLASH_UPDATE,
+ ADIS16550_STATUS_INERIAL,
+ ADIS16550_STATUS_SENSOR,
+ ADIS16550_STATUS_TEMPERATURE,
+ ADIS16550_STATUS_SPI,
+ ADIS16550_STATUS_PROCESSING,
+ ADIS16550_STATUS_POWER,
+ ADIS16550_STATUS_BOOT,
+ ADIS16550_STATUS_WATCHDOG = 15,
+ ADIS16550_STATUS_REGULATOR = 28,
+ ADIS16550_STATUS_SENSOR_SUPPLY,
+ ADIS16550_STATUS_CPU_SUPPLY,
+ ADIS16550_STATUS_5V_SUPPLY
+};
+
+static const char * const adis16550_status_error_msgs[] = {
+ [ADIS16550_STATUS_CRC_CODE] = "Code CRC Error",
+ [ADIS16550_STATUS_CRC_CONFIG] = "Configuration/Calibration CRC Error",
+ [ADIS16550_STATUS_FLASH_UPDATE] = "Flash Update Error",
+ [ADIS16550_STATUS_INERIAL] = "Overrange for Inertial Signals",
+ [ADIS16550_STATUS_SENSOR] = "Sensor failure",
+ [ADIS16550_STATUS_TEMPERATURE] = "Temperature Error",
+ [ADIS16550_STATUS_SPI] = "SPI Communication Error",
+ [ADIS16550_STATUS_PROCESSING] = "Processing Overrun Error",
+ [ADIS16550_STATUS_POWER] = "Power Supply Failure",
+ [ADIS16550_STATUS_BOOT] = "Boot Memory Failure",
+ [ADIS16550_STATUS_WATCHDOG] = "Watchdog timer flag",
+ [ADIS16550_STATUS_REGULATOR] = "Internal Regulator Error",
+ [ADIS16550_STATUS_SENSOR_SUPPLY] = "Internal Sensor Supply Error.",
+ [ADIS16550_STATUS_CPU_SUPPLY] = "Internal Processor Supply Error.",
+ [ADIS16550_STATUS_5V_SUPPLY] = "External 5V Supply Error",
+};
+
+static const struct adis_timeout adis16550_timeouts = {
+ .reset_ms = 1000,
+ .sw_reset_ms = 1000,
+ .self_test_ms = 1000,
+};
+
+static const struct adis_data adis16550_data = {
+ .diag_stat_reg = ADIS16550_REG_STATUS,
+ .diag_stat_size = 4,
+ .prod_id_reg = ADIS16550_REG_PROD_ID,
+ .prod_id = 16550,
+ .self_test_mask = BIT(1),
+ .self_test_reg = ADIS16550_REG_COMMAND,
+ .cs_change_delay = 5,
+ .unmasked_drdy = true,
+ .status_error_msgs = adis16550_status_error_msgs,
+ .status_error_mask = BIT(ADIS16550_STATUS_CRC_CODE) |
+ BIT(ADIS16550_STATUS_CRC_CONFIG) |
+ BIT(ADIS16550_STATUS_FLASH_UPDATE) |
+ BIT(ADIS16550_STATUS_INERIAL) |
+ BIT(ADIS16550_STATUS_SENSOR) |
+ BIT(ADIS16550_STATUS_TEMPERATURE) |
+ BIT(ADIS16550_STATUS_SPI) |
+ BIT(ADIS16550_STATUS_PROCESSING) |
+ BIT(ADIS16550_STATUS_POWER) |
+ BIT(ADIS16550_STATUS_BOOT) |
+ BIT(ADIS16550_STATUS_WATCHDOG) |
+ BIT(ADIS16550_STATUS_REGULATOR) |
+ BIT(ADIS16550_STATUS_SENSOR_SUPPLY) |
+ BIT(ADIS16550_STATUS_CPU_SUPPLY) |
+ BIT(ADIS16550_STATUS_5V_SUPPLY) |
+ BIT(ADIS16550_STATUS_CRC_CODE),
+ .timeouts = &adis16550_timeouts,
+};
+
+static const struct adis_ops adis16550_ops = {
+ .write = adis16550_spi_write,
+ .read = adis16550_spi_read,
+ .reset = adis16550_reset,
+};
+
+static int adis16550_probe(struct spi_device *spi)
+{
+ struct device *dev = &spi->dev;
+ struct iio_dev *indio_dev;
+ struct adis16550 *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->available_scan_masks = adis16550_channel_masks;
+ indio_dev->info = &adis16550_info;
+ indio_dev->modes = INDIO_DIRECT_MODE;
+
+ st->adis.ops = &adis16550_ops;
+
+ /*
+ * Allocate the xfer and buffer for the burst read operation. The buffer
+ * is used to store the burst data and the xfer is used to send the burst
+ * command and receive the data. On device remove the adis libraary is going
+ * to free the xfer and buffer.
+ */
+ st->adis.xfer = kzalloc(2 * sizeof(*st->adis.xfer),
+ GFP_KERNEL);
+ if (!st->adis.xfer)
+ return -ENOMEM;
+
+ st->adis.buffer = kzalloc(ADIS16550_BURST_DATA_LEN + sizeof(u32),
+ GFP_KERNEL);
+ if (!st->adis.buffer)
+ return -ENOMEM;
+
+ ret = devm_regulator_get_enable(dev, "vdd");
+ if (ret) {
+ dev_err_probe(dev, ret,
+ "Failed to get vdd regulator\n");
+ return ret;
+ }
+
+ ret = adis_init(&st->adis, indio_dev, spi, &adis16550_data);
+ if (ret)
+ return ret;
+
+ ret = __adis_initial_startup(&st->adis);
+ if (ret)
+ return ret;
+
+ ret = adis16550_config_sync(st);
+ if (ret)
+ return ret;
+
+ ret = devm_adis_setup_buffer_and_trigger(&st->adis, indio_dev,
+ adis16550_trigger_handler);
+ if (ret)
+ return ret;
+
+ ret = devm_iio_device_register(dev, indio_dev);
+ if (ret)
+ return ret;
+
+ adis16550_debugfs_init(indio_dev);
+
+ return 0;
+}
+
+static const struct spi_device_id adis16550_id[] = {
+ { "adis16550", (kernel_ulong_t)&adis16550_chip_info},
+ { "adis16550w", (kernel_ulong_t)&adis16550w_chip_info},
+ { }
+};
+MODULE_DEVICE_TABLE(spi, adis16550_id);
+
+static const struct of_device_id adis16550_of_match[] = {
+ { .compatible = "adi,adis16550",
+ .data = &adis16550_chip_info },
+ { .compatible = "adi,adis16550w",
+ .data = &adis16550w_chip_info },
+ { }
+};
+MODULE_DEVICE_TABLE(of, adis16550_of_match);
+
+static struct spi_driver adis16550_driver = {
+ .driver = {
+ .name = "adis16550",
+ .of_match_table = adis16550_of_match,
+ },
+ .probe = adis16550_probe,
+ .id_table = adis16550_id,
+};
+module_spi_driver(adis16550_driver);
+
+MODULE_AUTHOR("Nuno Sa <nuno.sa@analog.com>");
+MODULE_AUTHOR("Ramona Gradinariu <ramona.gradinariu@analog.com>");
+MODULE_AUTHOR("Antoniu Miclaus <antoniu.miclaus@analog.com>");
+MODULE_DESCRIPTION("Analog Devices ADIS16550 IMU driver");
+MODULE_LICENSE("GPL");
+MODULE_IMPORT_NS(IIO_ADISLIB);
--
2.34.1
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH v4 6/6] docs: iio: add documentation for adis16550 driver
2025-01-10 7:42 [PATCH v4 0/6] Add support for ADIS16550 and ADIS16550W Robert Budai
` (4 preceding siblings ...)
2025-01-10 7:42 ` [PATCH v4 5/6] iio: imu: adis16550: add adis16550 support Robert Budai
@ 2025-01-10 7:42 ` Robert Budai
5 siblings, 0 replies; 21+ messages in thread
From: Robert Budai @ 2025-01-10 7:42 UTC (permalink / raw)
To: Nuno Sa, Ramona Gradinariu, Antoniu Miclaus, Lars-Peter Clausen,
Michael Hennerich, Jonathan Cameron, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Jonathan Corbet, Robert Budai,
Alex Lanzano, linux-iio, devicetree, linux-kernel, linux-doc
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="y", Size: 21213 bytes --]
Add documentation for adis16550 driver which describes the driver
device files and shows how the user may use the ABI for various
scenarios (configuration, measurement, etc.).
Co-developed-by: Antoniu Miclaus <antoniu.miclaus@analog.com>
Signed-off-by: Antoniu Miclaus <antoniu.miclaus@analog.com>
Signed-off-by: Ramona Gradinariu <ramona.gradinariu@analog.com>
Signed-off-by: Robert Budai <robert.budai@analog.com>
---
v4:
- removed extra text bits from the documentation
Documentation/iio/adis16550.rst | 378 ++++++++++++++++++++++++++++++++
Documentation/iio/index.rst | 1 +
2 files changed, 379 insertions(+)
create mode 100644 Documentation/iio/adis16550.rst
diff --git a/Documentation/iio/adis16550.rst b/Documentation/iio/adis16550.rst
new file mode 100644
index 000000000000..1ea85fb77bb8
--- /dev/null
+++ b/Documentation/iio/adis16550.rst
@@ -0,0 +1,378 @@
+.. SPDX-License-Identifier: GPL-2.0
+
+================
+ADIS16550 driver
+================
+
+This driver supports Analog Device's IMUs on SPI bus.
+
+1. Supported devices
+====================
+
+* `ADIS16550 <https://www.analog.com/ADIS16550>`_
+
+The ADIS16550 is a complete inertial system that includes a triaxis gyroscope
+and a triaxis accelerometer. The factory calibration characterizes each sensor for
+sensitivity, bias, and alignment. As a result, each sensor has its own dynamic
+compensation formulas that provide accurate sensor measurements.
+
+1. Device attributes
+====================
+
+Accelerometer, gyroscope measurements are always provided. Furthermore, the
+driver offers the capability to retrieve the delta angle and the delta velocity
+measurements computed by the device.
+
+The delta angle measurements represent a calculation of angular displacement
+between each sample update, while the delta velocity measurements represent a
+calculation of linear velocity change between each sample update.
+
+Finally, temperature data are provided which show a coarse measurement of
+the temperature inside of the IMU device. This data is most useful for
+monitoring relative changes in the thermal environment.
+
+Each IIO device, has a device folder under ``/sys/bus/iio/devices/iio:deviceX``,
+where X is the IIO index of the device. Under these folders reside a set of
+device files, depending on the characteristics and features of the hardware
+device in questions. These files are consistently generalized and documented in
+the IIO ABI documentation.
+
+The following tables show the adis16550 related device files, found in the
+specific device folder path ``/sys/bus/iio/devices/iio:deviceX``.
+
++-------------------------------------------+----------------------------------------------------------+
+| 3-Axis Accelerometer related device files | Description |
++-------------------------------------------+----------------------------------------------------------+
+| in_accel_scale | Scale for the accelerometer channels. |
++-------------------------------------------+----------------------------------------------------------+
+| in_accel_filter_low_pass_3db_frequency | Bandwidth for the accelerometer channels. |
++-------------------------------------------+----------------------------------------------------------+
+| in_accel_x_calibbias | Calibration offset for the X-axis accelerometer channel. |
++-------------------------------------------+----------------------------------------------------------+
+| in_accel_x_calibscale | Calibration scale for the X-axis accelerometer channel. |
++-------------------------------------------+----------------------------------------------------------+
+| in_accel_x_raw | Raw X-axis accelerometer channel value. |
++-------------------------------------------+----------------------------------------------------------+
+| in_accel_y_calibbias | Calibration offset for the Y-axis accelerometer channel. |
++-------------------------------------------+----------------------------------------------------------+
+| in_accel_y_calibscale | Calibration scale for the Y-axis accelerometer channel. |
++-------------------------------------------+----------------------------------------------------------+
+| in_accel_y_raw | Raw Y-axis accelerometer channel value. |
++-------------------------------------------+----------------------------------------------------------+
+| in_accel_z_calibbias | Calibration offset for the Z-axis accelerometer channel. |
++-------------------------------------------+----------------------------------------------------------+
+| in_accel_z_calibscale | Calibration scale for the Z-axis accelerometer channel. |
++-------------------------------------------+----------------------------------------------------------+
+| in_accel_z_raw | Raw Z-axis accelerometer channel value. |
++-------------------------------------------+----------------------------------------------------------+
+| in_deltavelocity_scale | Scale for delta velocity channels. |
++-------------------------------------------+----------------------------------------------------------+
+| in_deltavelocity_x_raw | Raw X-axis delta velocity channel value. |
++-------------------------------------------+----------------------------------------------------------+
+| in_deltavelocity_y_raw | Raw Y-axis delta velocity channel value. |
++-------------------------------------------+----------------------------------------------------------+
+| in_deltavelocity_z_raw | Raw Z-axis delta velocity channel value. |
++-------------------------------------------+----------------------------------------------------------+
+
++--------------------------------------------+------------------------------------------------------+
+| 3-Axis Gyroscope related device files | Description |
++--------------------------------------------+------------------------------------------------------+
+| in_anglvel_scale | Scale for the gyroscope channels. |
++--------------------------------------------+------------------------------------------------------+
+| in_anglvel_filter_low_pass_3db_frequency | Scale for the gyroscope channels. |
++--------------------------------------------+------------------------------------------------------+
+| in_anglvel_x_calibbias | Calibration offset for the X-axis gyroscope channel. |
++--------------------------------------------+------------------------------------------------------+
+| in_anglvel_x_calibscale | Calibration scale for the X-axis gyroscope channel. |
++--------------------------------------------+------------------------------------------------------+
+| in_anglvel_x_raw | Raw X-axis gyroscope channel value. |
++--------------------------------------------+------------------------------------------------------+
+| in_anglvel_y_calibbias | Calibration offset for the Y-axis gyroscope channel. |
++--------------------------------------------+------------------------------------------------------+
+| in_anglvel_y_calibscale | Calibration scale for the Y-axis gyroscope channel. |
++--------------------------------------------+------------------------------------------------------+
+| in_anglvel_y_raw | Raw Y-axis gyroscope channel value. |
++--------------------------------------------+------------------------------------------------------+
+| in_anglvel_z_calibbias | Calibration offset for the Z-axis gyroscope channel. |
++--------------------------------------------+------------------------------------------------------+
+| in_anglvel_z_calibscale | Calibration scale for the Z-axis gyroscope channel. |
++--------------------------------------------+------------------------------------------------------+
+| in_anglvel_z_raw | Raw Z-axis gyroscope channel value. |
++--------------------------------------------+------------------------------------------------------+
+| in_deltaangl_scale | Scale for delta angle channels. |
++--------------------------------------------+------------------------------------------------------+
+| in_deltaangl_x_raw | Raw X-axis delta angle channel value. |
++--------------------------------------------+------------------------------------------------------+
+| in_deltaangl_y_raw | Raw Y-axis delta angle channel value. |
++--------------------------------------------+------------------------------------------------------+
+| in_deltaangl_z_raw | Raw Z-axis delta angle channel value. |
++--------------------------------------------+------------------------------------------------------+
+
++----------------------------------+-------------------------------------------+
+| Temperature sensor related files | Description |
++----------------------------------+-------------------------------------------+
+| in_temp0_raw | Raw temperature channel value. |
++----------------------------------+-------------------------------------------+
+| in_temp0_offset | Offset for the temperature sensor channel.|
++----------------------------------+-------------------------------------------+
+| in_temp0_scale | Scale for the temperature sensor channel. |
++----------------------------------+-------------------------------------------+
+
++----------------------------+--------------------------------------------------------------------------------+
+| Miscellaneous device files | Description |
++----------------------------+--------------------------------------------------------------------------------+
+| name | Name of the IIO device. |
++----------------------------+--------------------------------------------------------------------------------+
+| sampling_frequency | Currently selected sample rate. |
++----------------------------+--------------------------------------------------------------------------------+
+| write_lock_enable | Lock the device from further configuration. Only a reset will remove the lock. |
++----------------------------+--------------------------------------------------------------------------------+
+
+The following table shows the adis16550 related device debug files, found in the
+specific device debug folder path ``/sys/kernel/debug/iio/iio:deviceX``.
+
++----------------------+-------------------------------------------------------------------------+
+| Debugfs device files | Description |
++----------------------+-------------------------------------------------------------------------+
+| serial_number | The serial number of the chip in hexadecimal format. |
++----------------------+-------------------------------------------------------------------------+
+| product_id | Chip specific product id (16550). |
++----------------------+-------------------------------------------------------------------------+
+| flash_count | The number of flash writes performed on the device. |
++----------------------+-------------------------------------------------------------------------+
+| firmware_revision | String containing the firmware revision in the following format ##.##. |
++----------------------+-------------------------------------------------------------------------+
+| firmware_date | String containing the firmware date in the following format mm-dd-yyyy. |
++----------------------+-------------------------------------------------------------------------+
+
+Channels processed values
+-------------------------
+
+A channel value can be read from its _raw attribute. The value returned is the
+raw value as reported by the devices. To get the processed value of the channel,
+apply the following formula:
+
+.. code-block:: bash
+
+ processed value = (_raw + _offset) * _scale
+
+Where _offset and _scale are device attributes. If no _offset attribute is
+present, simply assume its value is 0.
+
+The adis16550 driver offers data for 5 types of channels, the table below shows
+the measurement units for the processed value, which are defined by the IIO
+framework:
+
++--------------------------------------+---------------------------+
+| Channel type | Measurement unit |
++--------------------------------------+---------------------------+
+| Acceleration on X, Y, and Z axis | Meters per Second squared |
++--------------------------------------+---------------------------+
+| Angular velocity on X, Y and Z axis | Radians per second |
++--------------------------------------+---------------------------+
+| Delta velocity on X. Y, and Z axis | Meters per Second |
++--------------------------------------+---------------------------+
+| Delta angle on X, Y, and Z axis | Radians |
++--------------------------------------+---------------------------+
+| Temperature | Millidegrees Celsius |
++--------------------------------------+---------------------------+
+
+Usage examples
+--------------
+
+Show device name:
+
+.. code-block:: bash
+
+ root:/sys/bus/iio/devices/iio:device0> cat name
+ adis16550
+
+Show accelerometer channels value:
+
+.. code-block:: bash
+
+ root:/sys/bus/iio/devices/iio:device0> cat in_accel_x_raw
+ 6903851
+ root:/sys/bus/iio/devices/iio:device0> cat in_accel_y_raw
+ 5650550
+ root:/sys/bus/iio/devices/iio:device0> cat in_accel_z_raw
+ 104873530
+ root:/sys/bus/iio/devices/iio:device0> cat in_accel_scale
+ 0.000000095
+
+- X-axis acceleration = in_accel_x_raw * in_accel_scale = 0.655865845 m/s^2
+- Y-axis acceleration = in_accel_y_raw * in_accel_scale = 0.53680225 m/s^2
+- Z-axis acceleration = in_accel_z_raw * in_accel_scale = 9.96298535 m/s^2
+
+Show gyroscope channels value:
+
+.. code-block:: bash
+
+ root:/sys/bus/iio/devices/iio:device0> cat in_anglvel_x_raw
+ 193309
+ root:/sys/bus/iio/devices/iio:device0> cat in_anglvel_y_raw
+ -763676
+ root:/sys/bus/iio/devices/iio:device0> cat in_anglvel_z_raw
+ -358108
+ root:/sys/bus/iio/devices/iio:device0> cat in_anglvel_scale
+ 0.000000003
+
+- X-axis angular velocity = in_anglvel_x_raw * in_anglvel_scale = 0.000579927 rad/s
+- Y-axis angular velocity = in_anglvel_y_raw * in_anglvel_scale = −0.002291028 rad/s
+- Z-axis angular velocity = in_anglvel_z_raw * in_anglvel_scale = −0.001074324 rad/s
+
+Set calibration offset for accelerometer channels:
+
+.. code-block:: bash
+
+ root:/sys/bus/iio/devices/iio:device0> cat in_accel_x_calibbias
+ 0
+
+ root:/sys/bus/iio/devices/iio:device0> echo 5000 > in_accel_x_calibbias
+ root:/sys/bus/iio/devices/iio:device0> cat in_accel_x_calibbias
+ 5000
+
+Set calibration offset for gyroscope channels:
+
+.. code-block:: bash
+
+ root:/sys/bus/iio/devices/iio:device0> cat in_anglvel_y_calibbias
+ 0
+
+ root:/sys/bus/iio/devices/iio:device0> echo -5000 > in_anglvel_y_calibbias
+ root:/sys/bus/iio/devices/iio:device0> cat in_anglvel_y_calibbias
+ -5000
+
+Set sampling frequency:
+
+.. code-block:: bash
+
+ root:/sys/bus/iio/devices/iio:device0> cat sampling_frequency
+ 4000.000000
+
+ root:/sys/bus/iio/devices/iio:device0> echo 1000 > sampling_frequency
+ 1000.000000
+
+Set bandwidth for accelerometer channels:
+
+.. code-block:: bash
+
+ root:/sys/bus/iio/devices/iio:device0> cat in_accel_filter_low_pass_3db_frequency
+ 0
+
+ root:/sys/bus/iio/devices/iio:device0> echo 100 > in_accel_filter_low_pass_3db_frequency
+ root:/sys/bus/iio/devices/iio:device0> cat in_accel_filter_low_pass_3db_frequency
+ 100
+
+Show serial number:
+
+.. code-block:: bash
+
+ root:/sys/kernel/debug/iio/iio:device0> cat serial_number
+ 0x000000b6
+
+Show product id:
+
+.. code-block:: bash
+
+ root:/sys/kernel/debug/iio/iio:device0> cat product_id
+ 16550
+
+Show flash count:
+
+.. code-block:: bash
+
+ root:/sys/kernel/debug/iio/iio:device0> cat flash_count
+ 13
+
+Show firmware revision:
+
+.. code-block:: bash
+
+ root:/sys/kernel/debug/iio/iio:device0> cat firmware_revision
+ 1.5
+
+Show firmware date:
+
+.. code-block:: bash
+
+ root:/sys/kernel/debug/iio/iio:device0> cat firmware_date
+ 28-04-2021
+
+3. Device buffers
+=================
+
+This driver supports IIO buffers.
+
+The device supports retrieving the raw acceleration, gyroscope, delta velocity,
+delta angle and temperature measurements using buffers.
+
+However, when retrieving acceleration or gyroscope data using buffers, delta
+readings will not be available and vice versa. This is because the device only
+allows to read either acceleration and gyroscope data or delta velocity and
+delta angle data at a time and switching between these two burst data selection
+modes is time consuming.
+
+Usage examples
+--------------
+
+Set device trigger in current_trigger, if not already set:
+
+.. code-block:: bash
+
+ root:/sys/bus/iio/devices/iio:device0> cat trigger/current_trigger
+
+ root:/sys/bus/iio/devices/iio:device0> echo adis16550-dev0 > trigger/current_trigger
+ root:/sys/bus/iio/devices/iio:device0> cat trigger/current_trigger
+ adis16550-dev0
+
+Select channels for buffer read:
+
+.. code-block:: bash
+
+ root:/sys/bus/iio/devices/iio:device0> echo 1 > scan_elements/in_deltavelocity_x_en
+ root:/sys/bus/iio/devices/iio:device0> echo 1 > scan_elements/in_deltavelocity_y_en
+ root:/sys/bus/iio/devices/iio:device0> echo 1 > scan_elements/in_deltavelocity_z_en
+ root:/sys/bus/iio/devices/iio:device0> echo 1 > scan_elements/in_temp0_en
+
+Set the number of samples to be stored in the buffer:
+
+.. code-block:: bash
+
+ root:/sys/bus/iio/devices/iio:device0> echo 10 > buffer/length
+
+Enable buffer readings:
+
+.. code-block:: bash
+
+ root:/sys/bus/iio/devices/iio:device0> echo 1 > buffer/enable
+
+Obtain buffered data:
+
+.. code-block:: bash
+
+ root:/sys/bus/iio/devices/iio:device0> hexdump -C /dev/iio\:device0
+ ...
+ 0000cdf0 00 00 0d 2f 00 00 08 43 00 00 09 09 00 00 a4 5f |.../...C......._|
+ 0000ce00 00 00 0d 2f 00 00 07 de 00 00 08 db 00 00 a4 4b |.../...........K|
+ 0000ce10 00 00 0d 2f 00 00 07 58 00 00 08 a3 00 00 a4 55 |.../...X.......U|
+ 0000ce20 00 00 0d 2f 00 00 06 d6 00 00 08 5c 00 00 a4 62 |.../.......\...b|
+ 0000ce30 00 00 0d 2f 00 00 06 45 00 00 08 37 00 00 a4 47 |.../...E...7...G|
+ 0000ce40 00 00 0d 2f 00 00 05 d4 00 00 08 30 00 00 a3 fa |.../.......0....|
+ 0000ce50 00 00 0d 2f 00 00 05 d0 00 00 08 12 00 00 a3 d3 |.../............|
+ 0000ce60 00 00 0d 2f 00 00 05 dd 00 00 08 2e 00 00 a3 e9 |.../............|
+ 0000ce70 00 00 0d 2f 00 00 05 cc 00 00 08 51 00 00 a3 d5 |.../.......Q....|
+ 0000ce80 00 00 0d 2f 00 00 05 ba 00 00 08 22 00 00 a3 9a |.../......."....|
+ 0000ce90 00 00 0d 2f 00 00 05 9c 00 00 07 d9 00 00 a3 40 |.../...........@|
+ 0000cea0 00 00 0d 2f 00 00 05 68 00 00 07 94 00 00 a2 e4 |.../...h........|
+ 0000ceb0 00 00 0d 2f 00 00 05 25 00 00 07 8d 00 00 a2 ce |.../...%........|
+ ...
+
+See ``Documentation/iio/iio_devbuf.rst`` for more information about how buffered
+data is structured.
+
+4. IIO Interfacing Tools
+========================
+
+See ``Documentation/iio/iio_tools.rst`` for the description of the available IIO
+interfacing tools.
diff --git a/Documentation/iio/index.rst b/Documentation/iio/index.rst
index 074dbbf7ba0a..2c3fa1716a69 100644
--- a/Documentation/iio/index.rst
+++ b/Documentation/iio/index.rst
@@ -26,6 +26,7 @@ Industrial I/O Kernel Drivers
ad7944
adis16475
adis16480
+ adis16550
adxl380
bno055
ep93xx_adc
--
2.34.1
^ permalink raw reply related [flat|nested] 21+ messages in thread
* Re: [PATCH v4 5/6] iio: imu: adis16550: add adis16550 support
2025-01-10 7:42 ` [PATCH v4 5/6] iio: imu: adis16550: add adis16550 support Robert Budai
@ 2025-01-11 12:03 ` kernel test robot
2025-01-12 15:33 ` Jonathan Cameron
2025-01-12 4:20 ` kernel test robot
2025-01-12 16:11 ` Jonathan Cameron
2 siblings, 1 reply; 21+ messages in thread
From: kernel test robot @ 2025-01-11 12:03 UTC (permalink / raw)
To: Robert Budai, Nuno Sa, Ramona Gradinariu, Antoniu Miclaus,
Lars-Peter Clausen, Michael Hennerich, Jonathan Cameron,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Jonathan Corbet,
Alex Lanzano, linux-iio, devicetree, linux-kernel, linux-doc
Cc: oe-kbuild-all
Hi Robert,
kernel test robot noticed the following build errors:
[auto build test ERROR on jic23-iio/togreg]
[also build test ERROR on linus/master v6.13-rc6 next-20250110]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Robert-Budai/iio-imu-adis-Add-custom-ops-struct/20250110-154645
base: https://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git togreg
patch link: https://lore.kernel.org/r/20250110074254.38966-6-robert.budai%40analog.com
patch subject: [PATCH v4 5/6] iio: imu: adis16550: add adis16550 support
config: m68k-allmodconfig (https://download.01.org/0day-ci/archive/20250111/202501111951.NHXp98OK-lkp@intel.com/config)
compiler: m68k-linux-gcc (GCC) 14.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250111/202501111951.NHXp98OK-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202501111951.NHXp98OK-lkp@intel.com/
All errors (new ones prefixed by >>):
In file included from include/linux/module.h:22,
from include/linux/device/driver.h:21,
from include/linux/device.h:32,
from include/linux/iio/iio.h:10,
from include/linux/iio/buffer.h:10,
from drivers/iio/imu/adis16550.c:12:
>> drivers/iio/imu/adis16550.c:1202:18: error: expected ',' or ';' before 'IIO_ADISLIB'
1202 | MODULE_IMPORT_NS(IIO_ADISLIB);
| ^~~~~~~~~~~
include/linux/moduleparam.h:26:61: note: in definition of macro '__MODULE_INFO'
26 | = __MODULE_INFO_PREFIX __stringify(tag) "=" info
| ^~~~
include/linux/module.h:299:33: note: in expansion of macro 'MODULE_INFO'
299 | #define MODULE_IMPORT_NS(ns) MODULE_INFO(import_ns, ns)
| ^~~~~~~~~~~
drivers/iio/imu/adis16550.c:1202:1: note: in expansion of macro 'MODULE_IMPORT_NS'
1202 | MODULE_IMPORT_NS(IIO_ADISLIB);
| ^~~~~~~~~~~~~~~~
vim +1202 drivers/iio/imu/adis16550.c
1196
1197 MODULE_AUTHOR("Nuno Sa <nuno.sa@analog.com>");
1198 MODULE_AUTHOR("Ramona Gradinariu <ramona.gradinariu@analog.com>");
1199 MODULE_AUTHOR("Antoniu Miclaus <antoniu.miclaus@analog.com>");
1200 MODULE_DESCRIPTION("Analog Devices ADIS16550 IMU driver");
1201 MODULE_LICENSE("GPL");
> 1202 MODULE_IMPORT_NS(IIO_ADISLIB);
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v4 5/6] iio: imu: adis16550: add adis16550 support
2025-01-10 7:42 ` [PATCH v4 5/6] iio: imu: adis16550: add adis16550 support Robert Budai
2025-01-11 12:03 ` kernel test robot
@ 2025-01-12 4:20 ` kernel test robot
2025-01-12 16:11 ` Jonathan Cameron
2 siblings, 0 replies; 21+ messages in thread
From: kernel test robot @ 2025-01-12 4:20 UTC (permalink / raw)
To: Robert Budai, Nuno Sa, Ramona Gradinariu, Antoniu Miclaus,
Lars-Peter Clausen, Michael Hennerich, Jonathan Cameron,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Jonathan Corbet,
Alex Lanzano, linux-iio, devicetree, linux-kernel, linux-doc
Cc: llvm, oe-kbuild-all
Hi Robert,
kernel test robot noticed the following build errors:
[auto build test ERROR on jic23-iio/togreg]
[also build test ERROR on linus/master v6.13-rc6 next-20250110]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Robert-Budai/iio-imu-adis-Add-custom-ops-struct/20250110-154645
base: https://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git togreg
patch link: https://lore.kernel.org/r/20250110074254.38966-6-robert.budai%40analog.com
patch subject: [PATCH v4 5/6] iio: imu: adis16550: add adis16550 support
config: hexagon-allyesconfig (https://download.01.org/0day-ci/archive/20250112/202501121221.84e3uKFl-lkp@intel.com/config)
compiler: clang version 18.1.8 (https://github.com/llvm/llvm-project 3b5b5c1ec4a3095ab096dd780e84d7ab81f3d7ff)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250112/202501121221.84e3uKFl-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202501121221.84e3uKFl-lkp@intel.com/
All errors (new ones prefixed by >>):
>> drivers/iio/imu/adis16550.c:1202:18: error: expected ';' after top level declarator
1202 | MODULE_IMPORT_NS(IIO_ADISLIB);
| ^
1 error generated.
vim +1202 drivers/iio/imu/adis16550.c
1196
1197 MODULE_AUTHOR("Nuno Sa <nuno.sa@analog.com>");
1198 MODULE_AUTHOR("Ramona Gradinariu <ramona.gradinariu@analog.com>");
1199 MODULE_AUTHOR("Antoniu Miclaus <antoniu.miclaus@analog.com>");
1200 MODULE_DESCRIPTION("Analog Devices ADIS16550 IMU driver");
1201 MODULE_LICENSE("GPL");
> 1202 MODULE_IMPORT_NS(IIO_ADISLIB);
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v4 5/6] iio: imu: adis16550: add adis16550 support
2025-01-11 12:03 ` kernel test robot
@ 2025-01-12 15:33 ` Jonathan Cameron
0 siblings, 0 replies; 21+ messages in thread
From: Jonathan Cameron @ 2025-01-12 15:33 UTC (permalink / raw)
To: kernel test robot
Cc: Robert Budai, Nuno Sa, Ramona Gradinariu, Antoniu Miclaus,
Lars-Peter Clausen, Michael Hennerich, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Jonathan Corbet, Alex Lanzano,
linux-iio, devicetree, linux-kernel, linux-doc, oe-kbuild-all
On Sat, 11 Jan 2025 20:03:22 +0800
kernel test robot <lkp@intel.com> wrote:
> Hi Robert,
>
> kernel test robot noticed the following build errors:
>
> [auto build test ERROR on jic23-iio/togreg]
> [also build test ERROR on linus/master v6.13-rc6 next-20250110]
> [If your patch is applied to the wrong git tree, kindly drop us a note.
> And when submitting patch, we suggest to use '--base' as documented in
> https://git-scm.com/docs/git-format-patch#_base_tree_information]
>
> url: https://github.com/intel-lab-lkp/linux/commits/Robert-Budai/iio-imu-adis-Add-custom-ops-struct/20250110-154645
> base: https://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git togreg
> patch link: https://lore.kernel.org/r/20250110074254.38966-6-robert.budai%40analog.com
> patch subject: [PATCH v4 5/6] iio: imu: adis16550: add adis16550 support
> config: m68k-allmodconfig (https://download.01.org/0day-ci/archive/20250111/202501111951.NHXp98OK-lkp@intel.com/config)
> compiler: m68k-linux-gcc (GCC) 14.2.0
> reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250111/202501111951.NHXp98OK-lkp@intel.com/reproduce)
>
> If you fix the issue in a separate patch/commit (i.e. not just a new version of
> the same patch/commit), kindly add following tags
> | Reported-by: kernel test robot <lkp@intel.com>
> | Closes: https://lore.kernel.org/oe-kbuild-all/202501111951.NHXp98OK-lkp@intel.com/
>
> All errors (new ones prefixed by >>):
>
> In file included from include/linux/module.h:22,
> from include/linux/device/driver.h:21,
> from include/linux/device.h:32,
> from include/linux/iio/iio.h:10,
> from include/linux/iio/buffer.h:10,
> from drivers/iio/imu/adis16550.c:12:
> >> drivers/iio/imu/adis16550.c:1202:18: error: expected ',' or ';' before 'IIO_ADISLIB'
> 1202 | MODULE_IMPORT_NS(IIO_ADISLIB);
> | ^~~~~~~~~~~
> include/linux/moduleparam.h:26:61: note: in definition of macro '__MODULE_INFO'
> 26 | = __MODULE_INFO_PREFIX __stringify(tag) "=" info
> | ^~~~
> include/linux/module.h:299:33: note: in expansion of macro 'MODULE_INFO'
> 299 | #define MODULE_IMPORT_NS(ns) MODULE_INFO(import_ns, ns)
> | ^~~~~~~~~~~
> drivers/iio/imu/adis16550.c:1202:1: note: in expansion of macro 'MODULE_IMPORT_NS'
> 1202 | MODULE_IMPORT_NS(IIO_ADISLIB);
> | ^~~~~~~~~~~~~~~~
>
>
> vim +1202 drivers/iio/imu/adis16550.c
>
> 1196
> 1197 MODULE_AUTHOR("Nuno Sa <nuno.sa@analog.com>");
> 1198 MODULE_AUTHOR("Ramona Gradinariu <ramona.gradinariu@analog.com>");
> 1199 MODULE_AUTHOR("Antoniu Miclaus <antoniu.miclaus@analog.com>");
> 1200 MODULE_DESCRIPTION("Analog Devices ADIS16550 IMU driver");
> 1201 MODULE_LICENSE("GPL");
> > 1202 MODULE_IMPORT_NS(IIO_ADISLIB);
>
You need a base commit from rc2 or above. (technically 2 patches after rc1
but rc2 is probably a better choice!) This will be material for next kernel
release anyway now so this all may become moot!
Jonathan
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v4 2/6] iio: imu: adis: Add reset to custom ops
2025-01-10 7:42 ` [PATCH v4 2/6] iio: imu: adis: Add reset to custom ops Robert Budai
@ 2025-01-12 15:35 ` Jonathan Cameron
0 siblings, 0 replies; 21+ messages in thread
From: Jonathan Cameron @ 2025-01-12 15:35 UTC (permalink / raw)
To: Robert Budai
Cc: Nuno Sa, Ramona Gradinariu, Antoniu Miclaus, Lars-Peter Clausen,
Michael Hennerich, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Jonathan Corbet, Alex Lanzano, linux-iio, devicetree,
linux-kernel, linux-doc
On Fri, 10 Jan 2025 09:42:50 +0200
Robert Budai <robert.budai@analog.com> wrote:
> This patch allows the custom definition of reset functionality
> for adis object. It is useful in cases where the driver does not
> need to sleep after the reset since it is handled by the library.
>
> Co-developed-by: Ramona Gradinariu <ramona.gradinariu@analog.com>
> Signed-off-by: Ramona Gradinariu <ramona.gradinariu@analog.com>
> Co-developed-by: Antoniu Miclaus <antoniu.miclaus@analog.com>
> Signed-off-by: Antoniu Miclaus <antoniu.miclaus@analog.com>
> Signed-off-by: Nuno Sá <nuno.sa@analog.com>
> Signed-off-by: Robert Budai <robert.budai@analog.com>
> ---
> drivers/iio/imu/adis.c | 7 ++++---
> include/linux/iio/imu/adis.h | 3 +++
> 2 files changed, 7 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/iio/imu/adis.c b/drivers/iio/imu/adis.c
> index 54915c7a3e76..9e4113473dc4 100644
> --- a/drivers/iio/imu/adis.c
> +++ b/drivers/iio/imu/adis.c
> @@ -340,7 +340,7 @@ int __adis_reset(struct adis *adis)
> const struct adis_timeout *timeouts = adis->data->timeouts;
>
> ret = __adis_write_reg_8(adis, adis->data->glob_cmd_reg,
> - ADIS_GLOB_CMD_SW_RESET);
> + ADIS_GLOB_CMD_SW_RESET);
Accidental change I guess, but bad anyway. Original looks better aligned.
> if (ret) {
> dev_err(&adis->spi->dev, "Failed to reset device: %d\n", ret);
> return ret;
> @@ -491,6 +491,7 @@ 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,
> };
>
> /**
> @@ -522,9 +523,9 @@ int adis_init(struct adis *adis, struct iio_dev *indio_dev,
>
> adis->spi = spi;
> adis->data = data;
> - if (!adis->ops->write && !adis->ops->read)
> + if (!adis->ops->write && !adis->ops->read && !adis->ops->reset)
> adis->ops = &adis_default_ops;
> - else if (!adis->ops->write || !adis->ops->read)
> + else if (!adis->ops->write || !adis->ops->read || !adis->ops->reset)
> return -EINVAL;
>
> iio_device_set_drvdata(indio_dev, adis);
> diff --git a/include/linux/iio/imu/adis.h b/include/linux/iio/imu/adis.h
> index 89cfa75ae9ea..52652f51db2e 100644
> --- a/include/linux/iio/imu/adis.h
> +++ b/include/linux/iio/imu/adis.h
> @@ -98,12 +98,15 @@ struct adis_data {
> * struct adis_ops: Custom ops for adis devices.
> * @write: Custom spi write implementation.
> * @read: Custom spi read implementation.
> + * @reset: Custom sw reset implementation. The custom implementation does not
> + * need to sleep after the reset. It's done by the library already.
> */
> struct adis_ops {
> int (*write)(struct adis *adis, unsigned int reg, unsigned int value,
> unsigned int size);
> int (*read)(struct adis *adis, unsigned int reg, unsigned int *value,
> unsigned int size);
> + int (*reset)(struct adis *adis);
> };
>
> /**
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v4 3/6] iio: imu: adis: Add DIAG_STAT register size
2025-01-10 7:42 ` [PATCH v4 3/6] iio: imu: adis: Add DIAG_STAT register size Robert Budai
@ 2025-01-12 15:39 ` Jonathan Cameron
0 siblings, 0 replies; 21+ messages in thread
From: Jonathan Cameron @ 2025-01-12 15:39 UTC (permalink / raw)
To: Robert Budai
Cc: Nuno Sa, Ramona Gradinariu, Antoniu Miclaus, Lars-Peter Clausen,
Michael Hennerich, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Jonathan Corbet, Alex Lanzano, linux-iio, devicetree,
linux-kernel, linux-doc
On Fri, 10 Jan 2025 09:42:51 +0200
Robert Budai <robert.budai@analog.com> wrote:
> Some devices may have more than 16 bits of status. This patch allows the
> user to specify the size of the DIAG_STAT register. It defaults to 2 if
> not specified. This is mainly for backward compatibility.
>
> Co-developed-by: Ramona Gradinariu <ramona.gradinariu@analog.com>
> Signed-off-by: Ramona Gradinariu <ramona.gradinariu@analog.com>
> Co-developed-by: Antoniu Miclaus <antoniu.miclaus@analog.com>
> Signed-off-by: Antoniu Miclaus <antoniu.miclaus@analog.com>
> Signed-off-by: Nuno Sá <nuno.sa@analog.com>
> Signed-off-by: Robert Budai <robert.budai@analog.com>
> ---
> drivers/iio/imu/adis.c | 11 ++++++++---
> include/linux/iio/imu/adis.h | 2 ++
> 2 files changed, 10 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/iio/imu/adis.c b/drivers/iio/imu/adis.c
> index 9e4113473dc4..a072307bfb6a 100644
> --- a/drivers/iio/imu/adis.c
> +++ b/drivers/iio/imu/adis.c
> @@ -304,11 +304,15 @@ EXPORT_SYMBOL_NS(__adis_enable_irq, "IIO_ADISLIB");
> */
> int __adis_check_status(struct adis *adis)
> {
> - u16 status;
> + unsigned int status;
> int ret;
> int i;
>
> - ret = __adis_read_reg_16(adis, adis->data->diag_stat_reg, &status);
> + if (adis->data->diag_stat_size)
> + ret = adis->ops->read(adis, adis->data->diag_stat_reg, &status,
> + adis->data->diag_stat_size);
> + else
> + ret = __adis_read_reg_16(adis, adis->data->diag_stat_reg, (u16 *)&status);
> if (ret)
> return ret;
>
> @@ -317,7 +321,8 @@ int __adis_check_status(struct adis *adis)
> if (status == 0)
> return 0;
>
> - for (i = 0; i < 16; ++i) {
> + for (i = 0; i < BITS_PER_BYTE * ((adis->data->diag_stat_size) ?
> + adis->data->diag_stat_size : 2); ++i) {
This is getting hard to read. I think a local variable is appropriate.
> if (status & BIT(i)) {
> dev_err(&adis->spi->dev, "%s.\n",
> adis->data->status_error_msgs[i]);
> diff --git a/include/linux/iio/imu/adis.h b/include/linux/iio/imu/adis.h
> index 52652f51db2e..b888b22f5c8c 100644
> --- a/include/linux/iio/imu/adis.h
> +++ b/include/linux/iio/imu/adis.h
> @@ -44,6 +44,7 @@ struct adis_timeout {
> * @glob_cmd_reg: Register address of the GLOB_CMD register
> * @msc_ctrl_reg: Register address of the MSC_CTRL register
> * @diag_stat_reg: Register address of the DIAG_STAT register
> + * @diag_stat_size: Length (in bytes) of the DIAG_STAT register.
Add something about 0 being treated as meaning 2. Or add this to
all instances of this structure so we don't need that 0 case.
Whilst there are quite a lot instances, they are mostly filled
by a couple of macros so it shouldn't be too bad to update them all.
> * @prod_id_reg: Register address of the PROD_ID register
> * @prod_id: Product ID code that should be expected when reading @prod_id_reg
> * @self_test_mask: Bitmask of supported self-test operations
> @@ -70,6 +71,7 @@ struct adis_data {
> unsigned int glob_cmd_reg;
> unsigned int msc_ctrl_reg;
> unsigned int diag_stat_reg;
> + unsigned int diag_stat_size;
> unsigned int prod_id_reg;
>
> unsigned int prod_id;
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v4 4/6] dt-bindings: iio: Add adis16550 bindings
2025-01-10 7:42 ` [PATCH v4 4/6] dt-bindings: iio: Add adis16550 bindings Robert Budai
@ 2025-01-12 15:48 ` Jonathan Cameron
2025-01-13 9:29 ` Nuno Sá
2025-01-13 8:43 ` Krzysztof Kozlowski
1 sibling, 1 reply; 21+ messages in thread
From: Jonathan Cameron @ 2025-01-12 15:48 UTC (permalink / raw)
To: Robert Budai
Cc: Nuno Sa, Ramona Gradinariu, Antoniu Miclaus, Lars-Peter Clausen,
Michael Hennerich, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Jonathan Corbet, Alex Lanzano, linux-iio, devicetree,
linux-kernel, linux-doc
On Fri, 10 Jan 2025 09:42:52 +0200
Robert Budai <robert.budai@analog.com> wrote:
> Document the ADIS16550 device devicetree bindings.
>
> Co-developed-by: Antoniu Miclaus <antoniu.miclaus@analog.com>
> Signed-off-by: Antoniu Miclaus <antoniu.miclaus@analog.com>
> Signed-off-by: Ramona Gradinariu <ramona.gradinariu@analog.com>
> Signed-off-by: Robert Budai <robert.budai@analog.com>
> ---
>
> 4:
> - applied styling changes to the bindings file
> - restricted sync-mode to intervals 1-2
>
> .../bindings/iio/imu/adi,adis16550.yaml | 96 +++++++++++++++++++
> MAINTAINERS | 9 ++
> 2 files changed, 105 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/iio/imu/adi,adis16550.yaml
>
> diff --git a/Documentation/devicetree/bindings/iio/imu/adi,adis16550.yaml b/Documentation/devicetree/bindings/iio/imu/adi,adis16550.yaml
> new file mode 100644
> index 000000000000..e7ccf3883e55
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/imu/adi,adis16550.yaml
> @@ -0,0 +1,96 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/iio/imu/adi,adis16550.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Analog Devices ADIS16550 and similar IMUs
> +
> +maintainers:
> + - Nuno Sa <nuno.sa@analog.com>
> + - Ramona Gradinariu <ramona.gradinariu@analog.com>
> + - Antoniu Miclaus <antoniu.miclaus@analog.com>
> +
> +properties:
> + compatible:
> + enum:
> + - adi,adis16550
> + - adi,adis16550w
> +
> + reg:
> + maxItems: 1
> +
> + spi-cpha: true
> +
> + spi-cpol: true
> +
> + spi-max-frequency:
> + maximum: 15000000
> +
> + vdd-supply: true
> +
> + interrupts:
> + maxItems: 1
> +
> + reset-gpios:
> + description:
> + RESET active low pin.
> + maxItems: 1
> +
> + clocks:
> + maxItems: 1
> + description: If not provided, then the internal clock is used.
> +
> + adi,sync-mode:
> + description:
> + Configures the device SYNC pin. The following modes are supported
> + 0 - output_sync
> + 1 - direct_sync
> + 2 - scaled_sync
A little more on these would be good. They are 'weird' options
that are not commonly seen so help the reader out.
For scaled_sync don't we need information on the scale for it to be useful?
If we had that then a value of 1 would mean direct sync and wouldn't need
another control.
I'm not fully understanding the usecases for this.
If we have a say a pulse per second input, the control of the scale should
be userspace anyway. So maybe this maps to the input clock that we can elect to
use and control the effective frequency of by using scaled sync?
I'm not sure what pulse sync is. Grepping the datasheet didn't give me
anything that seemed related. The sync pin is input only so I'm also
not sure on output sync.
> + 3 - pulse_sync
> + $ref: /schemas/types.yaml#/definitions/uint32
> + minimum: 1
> + maximum: 2
You describe 0 to 3 but only allow 1 or 2?
> +
> +required:
> + - compatible
> + - reg
> + - interrupts
> + - spi-cpha
> + - spi-cpol
> + - spi-max-frequency
> + - vdd-supply
> +
> +allOf:
> + - if:
> + properties:
> + adi,sync-mode:
> + const: 2
> +
> + then:
> + dependencies:
> + adi,sync-mode: [ clocks ]
> +
> + - $ref: /schemas/spi/spi-peripheral-props.yaml#
> +
> +unevaluatedProperties: false
> +
> +examples:
> + - |
> + #include <dt-bindings/interrupt-controller/irq.h>
> + spi {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + imu@0 {
> + compatible = "adi,adis16550";
> + reg = <0>;
> + spi-max-frequency = <15000000>;
> + spi-cpol;
> + spi-cpha;
> + vdd-supply = <&vdd>;
> + interrupts = <4 IRQ_TYPE_EDGE_FALLING>;
> + interrupt-parent = <&gpio>;
> + };
> + };
> +...
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v4 5/6] iio: imu: adis16550: add adis16550 support
2025-01-10 7:42 ` [PATCH v4 5/6] iio: imu: adis16550: add adis16550 support Robert Budai
2025-01-11 12:03 ` kernel test robot
2025-01-12 4:20 ` kernel test robot
@ 2025-01-12 16:11 ` Jonathan Cameron
2025-01-13 9:36 ` Nuno Sá
2025-01-13 9:48 ` Nuno Sá
2 siblings, 2 replies; 21+ messages in thread
From: Jonathan Cameron @ 2025-01-12 16:11 UTC (permalink / raw)
To: Robert Budai
Cc: Nuno Sa, Ramona Gradinariu, Antoniu Miclaus, Lars-Peter Clausen,
Michael Hennerich, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Jonathan Corbet, Alex Lanzano, linux-iio, devicetree,
linux-kernel, linux-doc
On Fri, 10 Jan 2025 09:42:53 +0200
Robert Budai <robert.budai@analog.com> wrote:
> The ADIS16550 is a complete inertial system that includes a triaxis
> gyroscope and a triaxis accelerometer. Each inertial sensor in
> the ADIS16550 combines industry leading MEMS only technology
> with signal conditioning that optimizes dynamic performance. The
> factory calibration characterizes each sensor for sensitivity, bias,
> and alignment. As a result, each sensor has its own dynamic com-
> pensation formulas that provide accurate sensor measurements
>
> Co-developed-by: Ramona Gradinariu <ramona.gradinariu@analog.com>
> Signed-off-by: Ramona Gradinariu <ramona.gradinariu@analog.com>
> Co-developed-by: Antoniu Miclaus <antoniu.miclaus@analog.com>
> Signed-off-by: Antoniu Miclaus <antoniu.miclaus@analog.com>
> Signed-off-by: Nuno Sá <nuno.sa@analog.com>
> Signed-off-by: Robert Budai <robert.budai@analog.com>
> ---
>
> 4:
> - reorganized channels to match the order in the datasheet
> - removed extra checks and goto statements
> - for buffer memory allocation used only kfree, since adis library already takes care of freeing the buffer
That last bit makes for a mess wrt to who owns the buffer and lifetime
management. Suggestions inline.
Jonathan
> diff --git a/drivers/iio/imu/adis16550.c b/drivers/iio/imu/adis16550.c
> new file mode 100644
> index 000000000000..49c3ff9ef1e2
> --- /dev/null
> +++ b/drivers/iio/imu/adis16550.c
> @@ -0,0 +1,1202 @@
...
> +static int adis16550_set_accl_filter_freq(struct adis16550 *st, int freq_hz)
> +{
> + bool en = false;
> +
> + if (freq_hz)
> + en = true;
> +
> + return __adis_update_bits(&st->adis, ADIS16550_REG_CONFIG,
> + ADIS16550_ACCL_FIR_EN_MASK,
> + (u32)FIELD_PREP(ADIS16550_ACCL_FIR_EN_MASK, en));
Why is the cast needed? Only bit 3 is set.
> +}
> +static irqreturn_t adis16550_trigger_handler(int irq, void *p)
> +{
> + u32 crc;
> + u16 dummy;
> + bool valid;
> + int ret, i = 0;
> + u8 data_offset = 3;
> + struct iio_poll_func *pf = p;
> + __be32 data[ADIS16550_MAX_SCAN_DATA];
> + struct iio_dev *indio_dev = pf->indio_dev;
> + struct adis16550 *st = iio_priv(indio_dev);
> + struct adis *adis = iio_device_get_drvdata(indio_dev);
> + __be32 *buffer = adis->buffer;
Related to below lifetime comment. Move this buffer into iio_priv()
given you aren't using the lifetime management in the adis library code.
> +
> + ret = spi_sync(adis->spi, &adis->msg);
> + if (ret)
> + goto done;
> + /*
> + * Validate the header. The header is a normal spi reply with state
> + * vector and crc4.
> + */
> + ret = adis16550_spi_validate(&st->adis, buffer[0], &dummy);
> + if (ret)
> + goto done;
> +
> + crc = be32_to_cpu(buffer[ADIS16550_BURST_N_ELEM - 1]);
> + /* the header is not included in the crc */
> + valid = adis16550_validate_crc((u32 *)&buffer[1],
> + ADIS16550_BURST_N_ELEM - 2, crc);
> + if (!valid) {
> + dev_err(&adis->spi->dev, "Burst Invalid crc!\n");
> + goto done;
> + }
> +
> + memcpy(&data[i], &buffer[data_offset], (ADIS16550_SCAN_ACCEL_Z -
i is always 0? If so just use data here and get rid of i.
If data_offset is always 3, encode that directly here as well instead
of confusing local variable.
> + ADIS16550_SCAN_GYRO_X + 2) *
> + sizeof(__be32));
> + iio_push_to_buffers_with_timestamp(indio_dev, data, pf->timestamp);
> +done:
> + iio_trigger_notify_done(indio_dev->trig);
> + return IRQ_HANDLED;
> +}
> +static int adis16550_update_scan_mode(struct iio_dev *indio_dev,
> + const unsigned long *scan_mask)
> +{
> + struct adis *adis = iio_device_get_drvdata(indio_dev);
> + u16 burst_length = ADIS16550_BURST_DATA_LEN;
> + u8 burst_cmd;
> + u8 *tx;
> +
> + if (*scan_mask & ADIS16550_BURST_DATA_GYRO_ACCEL_MASK)
> + burst_cmd = ADIS16550_REG_BURST_GYRO_ACCEL;
> + else
> + burst_cmd = ADIS16550_REG_BURST_DELTA_ANG_VEL;
> +
> + memset(adis->xfer, 0, 2 * sizeof(*adis->xfer));
> + memset(adis->buffer, 0, burst_length + sizeof(u32));
This overlaps with the comment on lifetime management below.
I would move the xfer and buffer pointers into iio_priv leaving
adis->xfer and adis->buffer == NULL so that the core frees nothing.
> +
> + tx = adis->buffer + burst_length;
> + tx[0] = 0x00;
> + tx[1] = 0x00;
> + tx[2] = burst_cmd;
> + /* crc4 is 0 on burst command */
> + tx[3] = spi_crc4(get_unaligned_le32(tx));
> +
> + adis->xfer[0].tx_buf = tx;
> + adis->xfer[0].len = 4;
> + adis->xfer[0].cs_change = 1;
> + adis->xfer[0].delay.value = 8;
> + adis->xfer[0].delay.unit = SPI_DELAY_UNIT_USECS;
> + adis->xfer[1].rx_buf = adis->buffer;
> + adis->xfer[1].len = burst_length;
> +
> + spi_message_init_with_transfers(&adis->msg, adis->xfer, 2);
> +
> + return 0;
> +}
> +static int adis16550_config_sync(struct adis16550 *st)
> +{
> + struct device *dev = &st->adis.spi->dev;
> + const struct adis16550_sync *sync;
> + struct clk *clk;
> + u32 sync_mode;
> + u16 mode;
> + int ret;
> +
> + ret = device_property_read_u32(dev, "adi,sync-mode", &sync_mode);
> + if (ret) {
> + st->clk_freq_hz = st->info->int_clk * 1000;
> + return 0;
> + }
> +
> + if (sync_mode >= st->info->num_sync) {
> + dev_err(dev, "Invalid sync mode: %u for %s\n", sync_mode,
> + st->info->name);
> + return -EINVAL;
> + }
> +
> + sync = &st->info->sync_mode[sync_mode];
> + st->sync_mode = sync->sync_mode;
> +
> + clk = devm_clk_get_enabled(dev, NULL);
> + if (IS_ERR(clk))
> + return PTR_ERR(clk);
> +
> + st->clk_freq_hz = clk_get_rate(clk);
> + if (st->clk_freq_hz > sync->max_rate || st->clk_freq_hz < sync->min_rate)
> + return dev_err_probe(dev, -EINVAL,
> + "Clk rate: %lu not in a valid range:[%u %u]\n",
> + st->clk_freq_hz, sync->min_rate, sync->max_rate);
> +
> + if (sync->sync_mode == ADIS16550_SYNC_MODE_SCALED) {
Similar to DT binding discussion. You can see from the clock frequency if you
need to need to use sync_mode_scaled or not. I don't see a need for a binding
to control that.
> + u16 sync_scale;
> + u32 scaled_freq = 0;
> + /*
> + * In pps scaled sync we must scale the input clock to a range
> + * of [3000 45000].
> + */
> + ret = device_property_read_u32(dev, "adi,scaled-output-hz",
> + &scaled_freq);
> + if (ret)
> + return dev_err_probe(dev, ret,
> + "adi,scaled-output-hz property not found");
> +
> + if (scaled_freq < 3000 || scaled_freq > 4500)
> + return dev_err_probe(dev, -EINVAL,
> + "Invalid value:%u for adi,scaled-output-hz",
> + scaled_freq);
> +
> + sync_scale = DIV_ROUND_CLOSEST(scaled_freq, st->clk_freq_hz);
> +
> + ret = adis_write_reg_16(&st->adis, ADIS16550_REG_SYNC_SCALE,
> + sync_scale);
> + if (ret)
> + return ret;
> +
> + st->clk_freq_hz = scaled_freq;
> + }
> +
> + st->clk_freq_hz *= 1000;
> +
> + mode = FIELD_PREP(ADIS16550_SYNC_MODE_MASK, sync->sync_mode) |
> + FIELD_PREP(ADIS16550_SYNC_EN_MASK, true);
> +
> + return __adis_update_bits(&st->adis, ADIS16550_REG_CONFIG,
> + ADIS16550_SYNC_MASK, mode);
> +}
> +
> +static int adis16550_probe(struct spi_device *spi)
> +{
> + struct device *dev = &spi->dev;
> + struct iio_dev *indio_dev;
> + struct adis16550 *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->available_scan_masks = adis16550_channel_masks;
> + indio_dev->info = &adis16550_info;
> + indio_dev->modes = INDIO_DIRECT_MODE;
> +
> + st->adis.ops = &adis16550_ops;
> +
> + /*
> + * Allocate the xfer and buffer for the burst read operation. The buffer
> + * is used to store the burst data and the xfer is used to send the burst
> + * command and receive the data. On device remove the adis libraary is going
library.
> + * to free the xfer and buffer.
That is a horrible lifetime control. They should either be allocated in the library
and freed in the library or allocated and freed in the driver. Mixing that doesn't
make sense.
Can you store them somewhere else so that the ownership is never in the adis
library?
> + */
> + st->adis.xfer = kzalloc(2 * sizeof(*st->adis.xfer),
> + if (!st->adis.xfer)
> + return -ENOMEM;
> +
> + st->adis.buffer = kzalloc(ADIS16550_BURST_DATA_LEN + sizeof(u32),
> + GFP_KERNEL);
> + if (!st->adis.buffer)
> + return -ENOMEM;
> +
> + ret = devm_regulator_get_enable(dev, "vdd");
> + if (ret) {
> + dev_err_probe(dev, ret,
> + "Failed to get vdd regulator\n");
> + return ret;
return dev_err_probe()
and drop the brackets.
> + }
> +
> + ret = adis_init(&st->adis, indio_dev, spi, &adis16550_data);
> + if (ret)
> + return ret;
> +
> + ret = __adis_initial_startup(&st->adis);
> + if (ret)
> + return ret;
> +
> + ret = adis16550_config_sync(st);
> + if (ret)
> + return ret;
> +
> + ret = devm_adis_setup_buffer_and_trigger(&st->adis, indio_dev,
> + adis16550_trigger_handler);
> + if (ret)
> + return ret;
> +
> + ret = devm_iio_device_register(dev, indio_dev);
> + if (ret)
> + return ret;
> +
> + adis16550_debugfs_init(indio_dev);
> +
> + return 0;
> +}
> +
> +static const struct spi_device_id adis16550_id[] = {
> + { "adis16550", (kernel_ulong_t)&adis16550_chip_info},
> + { "adis16550w", (kernel_ulong_t)&adis16550w_chip_info},
> + { }
> +};
> +MODULE_DEVICE_TABLE(spi, adis16550_id);
> +
> +static const struct of_device_id adis16550_of_match[] = {
> + { .compatible = "adi,adis16550",
> + .data = &adis16550_chip_info },
> + { .compatible = "adi,adis16550w",
> + .data = &adis16550w_chip_info },
Under 80 chars on one line, so don't wrap these.
> + { }
> +};
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v4 4/6] dt-bindings: iio: Add adis16550 bindings
2025-01-10 7:42 ` [PATCH v4 4/6] dt-bindings: iio: Add adis16550 bindings Robert Budai
2025-01-12 15:48 ` Jonathan Cameron
@ 2025-01-13 8:43 ` Krzysztof Kozlowski
2025-01-15 13:35 ` Budai, Robert
1 sibling, 1 reply; 21+ messages in thread
From: Krzysztof Kozlowski @ 2025-01-13 8:43 UTC (permalink / raw)
To: Robert Budai
Cc: Nuno Sa, Ramona Gradinariu, Antoniu Miclaus, Lars-Peter Clausen,
Michael Hennerich, Jonathan Cameron, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Jonathan Corbet, Alex Lanzano,
linux-iio, devicetree, linux-kernel, linux-doc
On Fri, Jan 10, 2025 at 09:42:52AM +0200, Robert Budai wrote:
> +maintainers:
> + - Nuno Sa <nuno.sa@analog.com>
> + - Ramona Gradinariu <ramona.gradinariu@analog.com>
> + - Antoniu Miclaus <antoniu.miclaus@analog.com>
> +
> +properties:
> + compatible:
> + enum:
> + - adi,adis16550
> + - adi,adis16550w
Where is the answer for my questions at v1? No responses on email,
nothing improved in the patchset. Go back to my comments and respond to
them or implement them.
> +
> + reg:
> + maxItems: 1
> +
> + spi-cpha: true
> +
> + spi-cpol: true
> +
> + spi-max-frequency:
> + maximum: 15000000
> +
> + vdd-supply: true
> +
> + interrupts:
> + maxItems: 1
> +
> + reset-gpios:
> + description:
> + RESET active low pin.
> + maxItems: 1
> +
> + clocks:
> + maxItems: 1
> + description: If not provided, then the internal clock is used.
> +
> + adi,sync-mode:
> + description:
> + Configures the device SYNC pin. The following modes are supported
> + 0 - output_sync
> + 1 - direct_sync
> + 2 - scaled_sync
> + 3 - pulse_sync
> + $ref: /schemas/types.yaml#/definitions/uint32
> + minimum: 1
> + maximum: 2
So what does the "3" mean? Why documenting something impossible?
Anyway, use strings for these and drop "_sync" suffixes.
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v4 4/6] dt-bindings: iio: Add adis16550 bindings
2025-01-12 15:48 ` Jonathan Cameron
@ 2025-01-13 9:29 ` Nuno Sá
2025-01-13 14:22 ` Budai, Robert
0 siblings, 1 reply; 21+ messages in thread
From: Nuno Sá @ 2025-01-13 9:29 UTC (permalink / raw)
To: Jonathan Cameron, Robert Budai
Cc: Nuno Sa, Ramona Gradinariu, Antoniu Miclaus, Lars-Peter Clausen,
Michael Hennerich, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Jonathan Corbet, Alex Lanzano, linux-iio, devicetree,
linux-kernel, linux-doc
On Sun, 2025-01-12 at 15:48 +0000, Jonathan Cameron wrote:
> On Fri, 10 Jan 2025 09:42:52 +0200
> Robert Budai <robert.budai@analog.com> wrote:
>
> > Document the ADIS16550 device devicetree bindings.
> >
> > Co-developed-by: Antoniu Miclaus <antoniu.miclaus@analog.com>
> > Signed-off-by: Antoniu Miclaus <antoniu.miclaus@analog.com>
> > Signed-off-by: Ramona Gradinariu <ramona.gradinariu@analog.com>
> > Signed-off-by: Robert Budai <robert.budai@analog.com>
> > ---
> >
> > 4:
> > - applied styling changes to the bindings file
> > - restricted sync-mode to intervals 1-2
> >
> > .../bindings/iio/imu/adi,adis16550.yaml | 96 +++++++++++++++++++
> > MAINTAINERS | 9 ++
> > 2 files changed, 105 insertions(+)
> > create mode 100644
> > Documentation/devicetree/bindings/iio/imu/adi,adis16550.yaml
> >
> > diff --git a/Documentation/devicetree/bindings/iio/imu/adi,adis16550.yaml
> > b/Documentation/devicetree/bindings/iio/imu/adi,adis16550.yaml
> > new file mode 100644
> > index 000000000000..e7ccf3883e55
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/iio/imu/adi,adis16550.yaml
> > @@ -0,0 +1,96 @@
> > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/iio/imu/adi,adis16550.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Analog Devices ADIS16550 and similar IMUs
> > +
> > +maintainers:
> > + - Nuno Sa <nuno.sa@analog.com>
> > + - Ramona Gradinariu <ramona.gradinariu@analog.com>
> > + - Antoniu Miclaus <antoniu.miclaus@analog.com>
> > +
> > +properties:
> > + compatible:
> > + enum:
> > + - adi,adis16550
> > + - adi,adis16550w
> > +
> > + reg:
> > + maxItems: 1
> > +
> > + spi-cpha: true
> > +
> > + spi-cpol: true
> > +
> > + spi-max-frequency:
> > + maximum: 15000000
> > +
> > + vdd-supply: true
> > +
> > + interrupts:
> > + maxItems: 1
> > +
> > + reset-gpios:
> > + description:
> > + RESET active low pin.
> > + maxItems: 1
> > +
> > + clocks:
> > + maxItems: 1
> > + description: If not provided, then the internal clock is used.
> > +
> > + adi,sync-mode:
> > + description:
> > + Configures the device SYNC pin. The following modes are supported
> > + 0 - output_sync
> > + 1 - direct_sync
> > + 2 - scaled_sync
>
> A little more on these would be good. They are 'weird' options
> that are not commonly seen so help the reader out.
>
> For scaled_sync don't we need information on the scale for it to be useful?
> If we had that then a value of 1 would mean direct sync and wouldn't need
> another control.
>
> I'm not fully understanding the usecases for this.
>
> If we have a say a pulse per second input, the control of the scale should
> be userspace anyway. So maybe this maps to the input clock that we can elect
> to
> use and control the effective frequency of by using scaled sync?
I guess you likely already saw it in the driver. The scale value is
automatically set by the driver depending on the desired ODR (sampling
frequency).
>
> I'm not sure what pulse sync is. Grepping the datasheet didn't give me
> anything that seemed related. The sync pin is input only so I'm also
> not sure on output sync.
I think this is a copy paste from the adis16475 bindings. For this device, it
seems we only have:
* internal clock;
* external:
* direct mode
* scaled mode
But yeah, as you pointed out I think we do not need the binding. The presence of
an optional input clock plus the frequency should be all we need in order to set
the desired configuration. It should also be possible to add the allowed ranges
to the external input clock in the bindings...
- Nuno Sá
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v4 5/6] iio: imu: adis16550: add adis16550 support
2025-01-12 16:11 ` Jonathan Cameron
@ 2025-01-13 9:36 ` Nuno Sá
2025-01-13 9:48 ` Nuno Sá
1 sibling, 0 replies; 21+ messages in thread
From: Nuno Sá @ 2025-01-13 9:36 UTC (permalink / raw)
To: Jonathan Cameron, Robert Budai
Cc: Nuno Sa, Ramona Gradinariu, Antoniu Miclaus, Lars-Peter Clausen,
Michael Hennerich, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Jonathan Corbet, Alex Lanzano, linux-iio, devicetree,
linux-kernel, linux-doc
On Sun, 2025-01-12 at 16:11 +0000, Jonathan Cameron wrote:
> On Fri, 10 Jan 2025 09:42:53 +0200
> Robert Budai <robert.budai@analog.com> wrote:
>
> > The ADIS16550 is a complete inertial system that includes a triaxis
> > gyroscope and a triaxis accelerometer. Each inertial sensor in
> > the ADIS16550 combines industry leading MEMS only technology
> > with signal conditioning that optimizes dynamic performance. The
> > factory calibration characterizes each sensor for sensitivity, bias,
> > and alignment. As a result, each sensor has its own dynamic com-
> > pensation formulas that provide accurate sensor measurements
> >
> > Co-developed-by: Ramona Gradinariu <ramona.gradinariu@analog.com>
> > Signed-off-by: Ramona Gradinariu <ramona.gradinariu@analog.com>
> > Co-developed-by: Antoniu Miclaus <antoniu.miclaus@analog.com>
> > Signed-off-by: Antoniu Miclaus <antoniu.miclaus@analog.com>
> > Signed-off-by: Nuno Sá <nuno.sa@analog.com>
> > Signed-off-by: Robert Budai <robert.budai@analog.com>
> > ---
> >
> > 4:
> > - reorganized channels to match the order in the datasheet
> > - removed extra checks and goto statements
> > - for buffer memory allocation used only kfree, since adis library already
> > takes care of freeing the buffer
>
> That last bit makes for a mess wrt to who owns the buffer and lifetime
> management. Suggestions inline.
>
>
...
> > +
> > + /*
> > + * Allocate the xfer and buffer for the burst read operation. The
> > buffer
> > + * is used to store the burst data and the xfer is used to send the
> > burst
> > + * command and receive the data. On device remove the adis libraary
> > is going
>
> library.
>
> > + * to free the xfer and buffer.
>
> That is a horrible lifetime control. They should either be allocated in the
> library
> and freed in the library or allocated and freed in the driver. Mixing that
> doesn't
> make sense.
Agreed. TBH, I did not remembered at all to suggest ignoring the library and let
it call kfree() on NULL. I also don't like that much of how things are done in
the lib but I don't think it would be that straight to change things for the non
'burst mode' cases. For the devices implementing 'burst mode' doing the free +
allocation on every update_scan_modes seems unnecessary. Anyways, maybe
something worth looking at in the future.
- Nuno Sá
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v4 5/6] iio: imu: adis16550: add adis16550 support
2025-01-12 16:11 ` Jonathan Cameron
2025-01-13 9:36 ` Nuno Sá
@ 2025-01-13 9:48 ` Nuno Sá
1 sibling, 0 replies; 21+ messages in thread
From: Nuno Sá @ 2025-01-13 9:48 UTC (permalink / raw)
To: Jonathan Cameron, Robert Budai
Cc: Nuno Sa, Ramona Gradinariu, Antoniu Miclaus, Lars-Peter Clausen,
Michael Hennerich, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Jonathan Corbet, Alex Lanzano, linux-iio, devicetree,
linux-kernel, linux-doc
On Sun, 2025-01-12 at 16:11 +0000, Jonathan Cameron wrote:
> On Fri, 10 Jan 2025 09:42:53 +0200
> Robert Budai <robert.budai@analog.com> wrote:
>
> > The ADIS16550 is a complete inertial system that includes a triaxis
> > gyroscope and a triaxis accelerometer. Each inertial sensor in
> > the ADIS16550 combines industry leading MEMS only technology
> > with signal conditioning that optimizes dynamic performance. The
> > factory calibration characterizes each sensor for sensitivity, bias,
> > and alignment. As a result, each sensor has its own dynamic com-
> > pensation formulas that provide accurate sensor measurements
> >
> > Co-developed-by: Ramona Gradinariu <ramona.gradinariu@analog.com>
> > Signed-off-by: Ramona Gradinariu <ramona.gradinariu@analog.com>
> > Co-developed-by: Antoniu Miclaus <antoniu.miclaus@analog.com>
> > Signed-off-by: Antoniu Miclaus <antoniu.miclaus@analog.com>
> > Signed-off-by: Nuno Sá <nuno.sa@analog.com>
> > Signed-off-by: Robert Budai <robert.budai@analog.com>
> > ---
> >
> > 4:
> > - reorganized channels to match the order in the datasheet
> > - removed extra checks and goto statements
> > - for buffer memory allocation used only kfree, since adis library already
> > takes care of freeing the buffer
>
> That last bit makes for a mess wrt to who owns the buffer and lifetime
> management. Suggestions inline.
>
> Jonathan
>
> > diff --git a/drivers/iio/imu/adis16550.c b/drivers/iio/imu/adis16550.c
> > new file mode 100644
> > index 000000000000..49c3ff9ef1e2
> > --- /dev/null
> > +++ b/drivers/iio/imu/adis16550.c
> > @@ -0,0 +1,1202 @@
> ...
>
>
> > +static int adis16550_set_accl_filter_freq(struct adis16550 *st, int
> > freq_hz)
> > +{
> > + bool en = false;
> > +
> > + if (freq_hz)
> > + en = true;
> > +
> > + return __adis_update_bits(&st->adis, ADIS16550_REG_CONFIG,
> > + ADIS16550_ACCL_FIR_EN_MASK,
> > +
> > (u32)FIELD_PREP(ADIS16550_ACCL_FIR_EN_MASK, en));
>
> Why is the cast needed? Only bit 3 is set.
Typically this is needed for the cases where we want to write in 2 byte
registers and we want to make sure sizeof(val) (on the macro evaluation) gives
us the proper size. But yes, for this case as we want 4 bytes, it should not be
needed. Hmm but I think we might get 'unsigned long' from FIELD_PREP() since
mask is also of that type?
- Nuno Sá
^ permalink raw reply [flat|nested] 21+ messages in thread
* RE: [PATCH v4 4/6] dt-bindings: iio: Add adis16550 bindings
2025-01-13 9:29 ` Nuno Sá
@ 2025-01-13 14:22 ` Budai, Robert
2025-01-14 8:19 ` Nuno Sá
0 siblings, 1 reply; 21+ messages in thread
From: Budai, Robert @ 2025-01-13 14:22 UTC (permalink / raw)
To: Nuno Sá, Jonathan Cameron
Cc: Sa, Nuno, Gradinariu, Ramona, Miclaus, Antoniu,
Lars-Peter Clausen, Hennerich, Michael, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Jonathan Corbet, Alex Lanzano,
linux-iio@vger.kernel.org, devicetree@vger.kernel.org,
linux-kernel@vger.kernel.org, linux-doc@vger.kernel.org
> -----Original Message-----
> From: Nuno Sá <noname.nuno@gmail.com>
> Sent: Monday, January 13, 2025 11:30 AM
> To: Jonathan Cameron <jic23@kernel.org>; Budai, Robert
> <Robert.Budai@analog.com>
> Cc: Sa, Nuno <Nuno.Sa@analog.com>; Gradinariu, Ramona
> <Ramona.Gradinariu@analog.com>; Miclaus, Antoniu
> <Antoniu.Miclaus@analog.com>; Lars-Peter Clausen <lars@metafoo.de>;
> Hennerich, Michael <Michael.Hennerich@analog.com>; Rob Herring
> <robh@kernel.org>; Krzysztof Kozlowski <krzk+dt@kernel.org>; Conor Dooley
> <conor+dt@kernel.org>; Jonathan Corbet <corbet@lwn.net>; Alex Lanzano
> <lanzano.alex@gmail.com>; linux-iio@vger.kernel.org;
> devicetree@vger.kernel.org; linux-kernel@vger.kernel.org; linux-
> doc@vger.kernel.org
> Subject: Re: [PATCH v4 4/6] dt-bindings: iio: Add adis16550 bindings
>
> [External]
>
> On Sun, 2025-01-12 at 15:48 +0000, Jonathan Cameron wrote:
> > On Fri, 10 Jan 2025 09:42:52 +0200
> > Robert Budai <robert.budai@analog.com> wrote:
> >
> > > Document the ADIS16550 device devicetree bindings.
> > >
> > > Co-developed-by: Antoniu Miclaus <antoniu.miclaus@analog.com>
> > > Signed-off-by: Antoniu Miclaus <antoniu.miclaus@analog.com>
> > > Signed-off-by: Ramona Gradinariu <ramona.gradinariu@analog.com>
> > > Signed-off-by: Robert Budai <robert.budai@analog.com>
> > > ---
> > >
> > > 4:
> > > - applied styling changes to the bindings file
> > > - restricted sync-mode to intervals 1-2
> > >
> > > .../bindings/iio/imu/adi,adis16550.yaml | 96 +++++++++++++++++++
> > > MAINTAINERS | 9 ++
> > > 2 files changed, 105 insertions(+)
> > > create mode 100644
> > > Documentation/devicetree/bindings/iio/imu/adi,adis16550.yaml
> > >
> > > diff --git
> a/Documentation/devicetree/bindings/iio/imu/adi,adis16550.yaml
> > > b/Documentation/devicetree/bindings/iio/imu/adi,adis16550.yaml
> > > new file mode 100644
> > > index 000000000000..e7ccf3883e55
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/iio/imu/adi,adis16550.yaml
> > > @@ -0,0 +1,96 @@
> > > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> > > +%YAML 1.2
> > > +---
> > > +$id:
> https://urldefense.com/v3/__http://devicetree.org/schemas/iio/imu/adi,adis
> 16550.yaml*__;Iw!!A3Ni8CS0y2Y!74KHajr7iKZQ7Ld5deb4LytVFckO_Og8tIG
> Ukf233OLregM6AqtN-v-IBRfAn-4Z1tC0bwbcEpNO7Glv8YjiXWI$
> > > +$schema: https://urldefense.com/v3/__http://devicetree.org/meta-
> schemas/core.yaml*__;Iw!!A3Ni8CS0y2Y!74KHajr7iKZQ7Ld5deb4LytVFckO_
> Og8tIGUkf233OLregM6AqtN-v-IBRfAn-4Z1tC0bwbcEpNO7GlvNAV5ERI$
> > > +
> > > +title: Analog Devices ADIS16550 and similar IMUs
> > > +
> > > +maintainers:
> > > + - Nuno Sa <nuno.sa@analog.com>
> > > + - Ramona Gradinariu <ramona.gradinariu@analog.com>
> > > + - Antoniu Miclaus <antoniu.miclaus@analog.com>
> > > +
> > > +properties:
> > > + compatible:
> > > + enum:
> > > + - adi,adis16550
> > > + - adi,adis16550w
> > > +
> > > + reg:
> > > + maxItems: 1
> > > +
> > > + spi-cpha: true
> > > +
> > > + spi-cpol: true
> > > +
> > > + spi-max-frequency:
> > > + maximum: 15000000
> > > +
> > > + vdd-supply: true
> > > +
> > > + interrupts:
> > > + maxItems: 1
> > > +
> > > + reset-gpios:
> > > + description:
> > > + RESET active low pin.
> > > + maxItems: 1
> > > +
> > > + clocks:
> > > + maxItems: 1
> > > + description: If not provided, then the internal clock is used.
> > > +
> > > + adi,sync-mode:
> > > + description:
> > > + Configures the device SYNC pin. The following modes are supported
> > > + 0 - output_sync
> > > + 1 - direct_sync
> > > + 2 - scaled_sync
> >
> > A little more on these would be good. They are 'weird' options
> > that are not commonly seen so help the reader out.
> >
> > For scaled_sync don't we need information on the scale for it to be useful?
> > If we had that then a value of 1 would mean direct sync and wouldn't need
> > another control.
> >
> > I'm not fully understanding the usecases for this.
> >
> > If we have a say a pulse per second input, the control of the scale should
> > be userspace anyway. So maybe this maps to the input clock that we can
> elect
> > to
> > use and control the effective frequency of by using scaled sync?
>
> I guess you likely already saw it in the driver. The scale value is
> automatically set by the driver depending on the desired ODR (sampling
> frequency).
>
> >
> > I'm not sure what pulse sync is. Grepping the datasheet didn't give me
> > anything that seemed related. The sync pin is input only so I'm also
> > not sure on output sync.
>
> I think this is a copy paste from the adis16475 bindings. For this device, it
> seems we only have:
> * internal clock;
> * external:
> * direct mode
> * scaled mode
>
> But yeah, as you pointed out I think we do not need the binding. The presence
> of
> an optional input clock plus the frequency should be all we need in order to set
> the desired configuration. It should also be possible to add the allowed ranges
> to the external input clock in the bindings...
>
>
> - Nuno Sá
>
Will drop this binding than and add a frequency one that is dependent on clock
with specified ranges.
- Robert Budai
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v4 4/6] dt-bindings: iio: Add adis16550 bindings
2025-01-13 14:22 ` Budai, Robert
@ 2025-01-14 8:19 ` Nuno Sá
0 siblings, 0 replies; 21+ messages in thread
From: Nuno Sá @ 2025-01-14 8:19 UTC (permalink / raw)
To: Budai, Robert, Jonathan Cameron
Cc: Sa, Nuno, Gradinariu, Ramona, Miclaus, Antoniu,
Lars-Peter Clausen, Hennerich, Michael, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Jonathan Corbet, Alex Lanzano,
linux-iio@vger.kernel.org, devicetree@vger.kernel.org,
linux-kernel@vger.kernel.org, linux-doc@vger.kernel.org
On Mon, 2025-01-13 at 14:22 +0000, Budai, Robert wrote:
>
>
> > -----Original Message-----
> > From: Nuno Sá <noname.nuno@gmail.com>
> > Sent: Monday, January 13, 2025 11:30 AM
> > To: Jonathan Cameron <jic23@kernel.org>; Budai, Robert
> > <Robert.Budai@analog.com>
> > Cc: Sa, Nuno <Nuno.Sa@analog.com>; Gradinariu, Ramona
> > <Ramona.Gradinariu@analog.com>; Miclaus, Antoniu
> > <Antoniu.Miclaus@analog.com>; Lars-Peter Clausen <lars@metafoo.de>;
> > Hennerich, Michael <Michael.Hennerich@analog.com>; Rob Herring
> > <robh@kernel.org>; Krzysztof Kozlowski <krzk+dt@kernel.org>; Conor Dooley
> > <conor+dt@kernel.org>; Jonathan Corbet <corbet@lwn.net>; Alex Lanzano
> > <lanzano.alex@gmail.com>; linux-iio@vger.kernel.org;
> > devicetree@vger.kernel.org; linux-kernel@vger.kernel.org; linux-
> > doc@vger.kernel.org
> > Subject: Re: [PATCH v4 4/6] dt-bindings: iio: Add adis16550 bindings
> >
> > [External]
> >
> > On Sun, 2025-01-12 at 15:48 +0000, Jonathan Cameron wrote:
> > > On Fri, 10 Jan 2025 09:42:52 +0200
> > > Robert Budai <robert.budai@analog.com> wrote:
> > >
> > > > Document the ADIS16550 device devicetree bindings.
> > > >
> > > > Co-developed-by: Antoniu Miclaus <antoniu.miclaus@analog.com>
> > > > Signed-off-by: Antoniu Miclaus <antoniu.miclaus@analog.com>
> > > > Signed-off-by: Ramona Gradinariu <ramona.gradinariu@analog.com>
> > > > Signed-off-by: Robert Budai <robert.budai@analog.com>
> > > > ---
> > > >
> > > > 4:
> > > > - applied styling changes to the bindings file
> > > > - restricted sync-mode to intervals 1-2
> > > >
> > > > .../bindings/iio/imu/adi,adis16550.yaml | 96 +++++++++++++++++++
> > > > MAINTAINERS | 9 ++
> > > > 2 files changed, 105 insertions(+)
> > > > create mode 100644
> > > > Documentation/devicetree/bindings/iio/imu/adi,adis16550.yaml
> > > >
> > > > diff --git
> > a/Documentation/devicetree/bindings/iio/imu/adi,adis16550.yaml
> > > > b/Documentation/devicetree/bindings/iio/imu/adi,adis16550.yaml
> > > > new file mode 100644
> > > > index 000000000000..e7ccf3883e55
> > > > --- /dev/null
> > > > +++ b/Documentation/devicetree/bindings/iio/imu/adi,adis16550.yaml
> > > > @@ -0,0 +1,96 @@
> > > > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> > > > +%YAML 1.2
> > > > +---
> > > > +$id:
> > https://urldefense.com/v3/__http://devicetree.org/schemas/iio/imu/adi,adis
> > 16550.yaml*__;Iw!!A3Ni8CS0y2Y!74KHajr7iKZQ7Ld5deb4LytVFckO_Og8tIG
> > Ukf233OLregM6AqtN-v-IBRfAn-4Z1tC0bwbcEpNO7Glv8YjiXWI$
> > > > +$schema: https://urldefense.com/v3/__http://devicetree.org/meta-
> > schemas/core.yaml*__;Iw!!A3Ni8CS0y2Y!74KHajr7iKZQ7Ld5deb4LytVFckO_
> > Og8tIGUkf233OLregM6AqtN-v-IBRfAn-4Z1tC0bwbcEpNO7GlvNAV5ERI$
> > > > +
> > > > +title: Analog Devices ADIS16550 and similar IMUs
> > > > +
> > > > +maintainers:
> > > > + - Nuno Sa <nuno.sa@analog.com>
> > > > + - Ramona Gradinariu <ramona.gradinariu@analog.com>
> > > > + - Antoniu Miclaus <antoniu.miclaus@analog.com>
> > > > +
> > > > +properties:
> > > > + compatible:
> > > > + enum:
> > > > + - adi,adis16550
> > > > + - adi,adis16550w
> > > > +
> > > > + reg:
> > > > + maxItems: 1
> > > > +
> > > > + spi-cpha: true
> > > > +
> > > > + spi-cpol: true
> > > > +
> > > > + spi-max-frequency:
> > > > + maximum: 15000000
> > > > +
> > > > + vdd-supply: true
> > > > +
> > > > + interrupts:
> > > > + maxItems: 1
> > > > +
> > > > + reset-gpios:
> > > > + description:
> > > > + RESET active low pin.
> > > > + maxItems: 1
> > > > +
> > > > + clocks:
> > > > + maxItems: 1
> > > > + description: If not provided, then the internal clock is used.
> > > > +
> > > > + adi,sync-mode:
> > > > + description:
> > > > + Configures the device SYNC pin. The following modes are supported
> > > > + 0 - output_sync
> > > > + 1 - direct_sync
> > > > + 2 - scaled_sync
> > >
> > > A little more on these would be good. They are 'weird' options
> > > that are not commonly seen so help the reader out.
> > >
> > > For scaled_sync don't we need information on the scale for it to be
> > > useful?
> > > If we had that then a value of 1 would mean direct sync and wouldn't need
> > > another control.
> > >
> > > I'm not fully understanding the usecases for this.
> > >
> > > If we have a say a pulse per second input, the control of the scale should
> > > be userspace anyway. So maybe this maps to the input clock that we can
> > elect
> > > to
> > > use and control the effective frequency of by using scaled sync?
> >
> > I guess you likely already saw it in the driver. The scale value is
> > automatically set by the driver depending on the desired ODR (sampling
> > frequency).
> >
> > >
> > > I'm not sure what pulse sync is. Grepping the datasheet didn't give me
> > > anything that seemed related. The sync pin is input only so I'm also
> > > not sure on output sync.
> >
> > I think this is a copy paste from the adis16475 bindings. For this device,
> > it
> > seems we only have:
> > * internal clock;
> > * external:
> > * direct mode
> > * scaled mode
> >
> > But yeah, as you pointed out I think we do not need the binding. The
> > presence
> > of
> > an optional input clock plus the frequency should be all we need in order to
> > set
> > the desired configuration. It should also be possible to add the allowed
> > ranges
> > to the external input clock in the bindings...
> >
> >
> > - Nuno Sá
> >
> Will drop this binding than and add a frequency one that is dependent on clock
> with specified ranges.
>
Note I would not add a new binding if there's no standard one. My previous
comment can be misleading... I see there is a 'clock-frequency' property in the
clock schema but descriptions says it's legacy and apparently for fixed-clocks.
So maybe not suitable for this case?
- Nuno Sá
^ permalink raw reply [flat|nested] 21+ messages in thread
* RE: [PATCH v4 4/6] dt-bindings: iio: Add adis16550 bindings
2025-01-13 8:43 ` Krzysztof Kozlowski
@ 2025-01-15 13:35 ` Budai, Robert
0 siblings, 0 replies; 21+ messages in thread
From: Budai, Robert @ 2025-01-15 13:35 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: Sa, Nuno, Gradinariu, Ramona, Miclaus, Antoniu,
Lars-Peter Clausen, Hennerich, Michael, Jonathan Cameron,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Jonathan Corbet,
Alex Lanzano, linux-iio@vger.kernel.org,
devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
linux-doc@vger.kernel.org
> -----Original Message-----
> From: Krzysztof Kozlowski <krzk@kernel.org>
> Sent: Monday, January 13, 2025 10:44 AM
> To: Budai, Robert <Robert.Budai@analog.com>
> Cc: Sa, Nuno <Nuno.Sa@analog.com>; Gradinariu, Ramona
> <Ramona.Gradinariu@analog.com>; Miclaus, Antoniu
> <Antoniu.Miclaus@analog.com>; Lars-Peter Clausen <lars@metafoo.de>;
> Hennerich, Michael <Michael.Hennerich@analog.com>; Jonathan Cameron
> <jic23@kernel.org>; Rob Herring <robh@kernel.org>; Krzysztof Kozlowski
> <krzk+dt@kernel.org>; Conor Dooley <conor+dt@kernel.org>; Jonathan
> Corbet <corbet@lwn.net>; Alex Lanzano <lanzano.alex@gmail.com>; linux-
> iio@vger.kernel.org; devicetree@vger.kernel.org; linux-kernel@vger.kernel.org;
> linux-doc@vger.kernel.org
> Subject: Re: [PATCH v4 4/6] dt-bindings: iio: Add adis16550 bindings
>
> [External]
>
> On Fri, Jan 10, 2025 at 09:42:52AM +0200, Robert Budai wrote:
> > +maintainers:
> > + - Nuno Sa <nuno.sa@analog.com>
> > + - Ramona Gradinariu <ramona.gradinariu@analog.com>
> > + - Antoniu Miclaus <antoniu.miclaus@analog.com>
> > +
> > +properties:
> > + compatible:
> > + enum:
> > + - adi,adis16550
> > + - adi,adis16550w
>
> Where is the answer for my questions at v1? No responses on email,
> nothing improved in the patchset. Go back to my comments and respond to
> them or implement them.
>
[Robert Budai
^ permalink raw reply [flat|nested] 21+ messages in thread
end of thread, other threads:[~2025-01-15 13:35 UTC | newest]
Thread overview: 21+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-01-10 7:42 [PATCH v4 0/6] Add support for ADIS16550 and ADIS16550W Robert Budai
2025-01-10 7:42 ` [PATCH v4 1/6] iio: imu: adis: Add custom ops struct Robert Budai
2025-01-10 7:42 ` [PATCH v4 2/6] iio: imu: adis: Add reset to custom ops Robert Budai
2025-01-12 15:35 ` Jonathan Cameron
2025-01-10 7:42 ` [PATCH v4 3/6] iio: imu: adis: Add DIAG_STAT register size Robert Budai
2025-01-12 15:39 ` Jonathan Cameron
2025-01-10 7:42 ` [PATCH v4 4/6] dt-bindings: iio: Add adis16550 bindings Robert Budai
2025-01-12 15:48 ` Jonathan Cameron
2025-01-13 9:29 ` Nuno Sá
2025-01-13 14:22 ` Budai, Robert
2025-01-14 8:19 ` Nuno Sá
2025-01-13 8:43 ` Krzysztof Kozlowski
2025-01-15 13:35 ` Budai, Robert
2025-01-10 7:42 ` [PATCH v4 5/6] iio: imu: adis16550: add adis16550 support Robert Budai
2025-01-11 12:03 ` kernel test robot
2025-01-12 15:33 ` Jonathan Cameron
2025-01-12 4:20 ` kernel test robot
2025-01-12 16:11 ` Jonathan Cameron
2025-01-13 9:36 ` Nuno Sá
2025-01-13 9:48 ` Nuno Sá
2025-01-10 7:42 ` [PATCH v4 6/6] docs: iio: add documentation for adis16550 driver Robert Budai
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).