Re: [PATCH 5/5] dt-bindings: iio: Add adis16475 documentation

Lars-Peter Clausen <lars@xxxxxxxxxx> · Thu, 5 Mar 2020 11:34:04 +0100

On 3/4/20 7:00 PM, Sa, Nuno wrote:
On Tue, 2020-03-03 at 21:10 +0000, Jonathan Cameron wrote:
[External]

On Tue, 25 Feb 2020 13:41:52 +0100
Nuno Sá <nuno.sa@xxxxxxxxxx> wrote:

Document the ADIS16475 device devicetree bindings.

Signed-off-by: Nuno Sá <nuno.sa@xxxxxxxxxx>
One thing inline on the burst mode stuff.

Thanks,

Jonathan

---
  .../bindings/iio/imu/adi,adis16475.yaml       | 130
++++++++++++++++++
  MAINTAINERS                                   |   1 +
  2 files changed, 131 insertions(+)
  create mode 100644
Documentation/devicetree/bindings/iio/imu/adi,adis16475.yaml

diff --git
a/Documentation/devicetree/bindings/iio/imu/adi,adis16475.yaml
b/Documentation/devicetree/bindings/iio/imu/adi,adis16475.yaml
new file mode 100644
index 000000000000..c0f2146e000c
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/imu/adi,adis16475.yaml
@@ -0,0 +1,130 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/iio/imu/adi,adis16475.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Analog Devices ADIS16475 and similar IMUs
+
+maintainers:
+  - Nuno Sá <nuno.sa@xxxxxxxxxx>
+
+description: |
+  Analog Devices ADIS16475 and similar IMUs
+
https://www.analog.com/media/en/technical-documentation/data-sheets/ADIS16475.pdf
+
+properties:
+  compatible:
+    enum:
+      - adi,adis16475-1
+      - adi,adis16475-2
+      - adi,adis16475-3
+      - adi,adis16477-1
+      - adi,adis16477-2
+      - adi,adis16477-3
+      - adi,adis16470
+      - adi,adis16465-1
+      - adi,adis16465-2
+      - adi,adis16465-3
+      - adi,adis16467-1
+      - adi,adis16467-2
+      - adi,adis16467-3
+      - adi,adis16500
+      - adi,adis16505-1
+      - adi,adis16505-2
+      - adi,adis16505-3
+      - adi,adis16507-1
+      - adi,adis16507-2
+      - adi,adis16507-3
+
+  reg:
+    maxItems: 1
+
+  spi-cpha: true
+
+  spi-cpol: true
+
+  spi-max-frequency:
+    maximum: 2000000
+
+  interrupts:
+    maxItems: 1
+
+  clocks:
+    maxItems: 1
+
+  clock-names:
+    oneOf:
+      - const: sync
+      - const: direct-sync
+      - const: pulse-sync
+      - const: scaled-sync
+
+  reset-gpios:
+    description:
+      Must be the device tree identifier of the RESET pin. If
specified,
+      it will be asserted during driver probe. As the line is
active low,
+      it should be marked GPIO_ACTIVE_LOW.
+    maxItems: 1
+
+  adi,scaled-output-hz:
+    description:
+      This property must be present if the clock mode is scaled-
sync through
+      clock-names property. In this mode, the input clock can have
a range
+      of 1Hz to 128HZ which must be scaled to originate an
allowable sample
+      rate. This property specifies that rate.
+    minimum: 1900
+    maximum: 2100
+
+required:
+  - compatible
+  - reg
+  - interrupts
+  - spi-cpha
+  - spi-cpol
+
+if:
+  properties:
+    compatible:
+      contains:
+        enum:
+          - adi,adis16500
+          - adi,adis16505-1
+          - adi,adis16505-2
+          - adi,adis16505-3
+          - adi,adis16507-1
+          - adi,adis16507-2
+          - adi,adis16507-3
+
+then:
+  properties:
+    clock-names:
+      oneOf:
+        - const: sync
+        - const: direct-sync
+        - const: scaled-sync
+
+    adi,burst32-enable:
+      description:
+        Enable burst32 mode. In this mode, a burst reading
contains calibrated
+        gyroscope and accelerometer data in 32-bit format.
Why is this in DT?  Is it not a runtime decision
(ideally automatically selected)
So, you mean just have this mode by default on parts that support it?

Maybe lets start with explaining what burst32 mode is, so everybody is 
on the same page.

The way registers are usually accessed for this chip is that you first 
write the address you want to read on the SPI bus and then read the 
selected register. This can be quite slow though if you want to read 
multiple registers and is too slow to be able to read all the data at 
full data rate. So there is a special burst mode which allows to read 
all the data registers in one go.

Now by default the data registers are 16-bit. But there is an internal 
decimation filter and the extra bits produced by the decimation filter 
go into additional data registers making the data 32-bit wide. The chip 
allows to configure whether to read the only 16-bit MSBs or the full 
32-bit register.

So the decision whether a user wants to use 32-bit or 16-bit depends on 
whether the extra 16-bit are needed or if they are even available. E.g. 
if the decimation filter is off there wont be any extra bits.

This means ideally it would be user configurable whether to use 16-bit 
or 32-bit burst mode, since it is application specific. The problem is 
we don't have an interface for changing the bit width of a buffer 
channel. Adding such an interface would require quite a bit of effort 
since the assumption currently is that the bit width does not chance. 
E.g. libiio assumes this and would stop working if it did change.

Maybe as a compromise for now. Use 32-bit burst when there is actually 
meaningful data is the LSBs, i.e. the decimation filter is used and 
disable it otherwise. And then think about how to make it configurable 
as a follow up action.

In my opinion there is should not be a difference in the userspace 
interface for chips that do support 32-bit burst and those that don't. 
For devices that don't support 32-bit burst it should be emulated by 
reading the LSB bits registers manually.

- Lars







