On Fri, 6 Aug 2021 21:33:44 -0300
Lucas Stankus [off-list ref] wrote:

On Fri, Aug 6, 2021 at 3:10 PM Rob Herring [off-list ref] wrote:

quoted

On Thu, Aug 05, 2021 at 03:29:37AM -0300, Lucas Stankus wrote:  

quoted

Add device tree binding documentation for ADXL313 3-axis accelerometer.

Signed-off-by: Lucas Stankus <lucas.p.stankus@gmail.com>
---
 .../bindings/iio/accel/adi,adxl313.yaml       | 90 +++++++++++++++++++
 1 file changed, 90 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/iio/accel/adi,adxl313.yaml

diff --git a/Documentation/devicetree/bindings/iio/accel/adi,adxl313.yaml b/Documentation/devicetree/bindings/iio/accel/adi,adxl313.yaml
new file mode 100644
index 000000000000..fea03b6790f3
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/accel/adi,adxl313.yaml

@@ -0,0 +1,90 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/iio/accel/adi,adxl313.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Analog Devices ADXL313 3-Axis Digital Accelerometer
+
+maintainers:
+  - Lucas Stankus <lucas.p.stankus@gmail.com>
+
+description: |
+  Analog Devices ADXL313 3-Axis Digital Accelerometer that supports
+  both I2C & SPI interfaces.
+    https://www.analog.com/en/products/adxl313.html
+
+properties:
+  compatible:
+    enum:
+      - adi,adxl313
+
+  reg:
+    maxItems: 1
+
+  spi-3wire: true
+
+  spi-cpha: true
+
+  spi-cpol: true  

These 3 generally shouldn't be needed, but can be set from the driver.
If they are valid, is any combination of them really valid?
 

Only the 3wire is optional, both cpha and cpol are required for proper
spi connection.

We've been round this one a few time, and last time we discussed the
cases where you'd need these in DT (because of inverters on the bus)
https://lore.kernel.org/linux-iio/20191204111231.GO1998@sirena.org.uk/ (local)
conclusion was, that we don't want to put the burden on the dt files
for those odd cases.  The equivalent for interrupt lines is interestingly
different because in those cases the two-cell version includes the
type of interrupt, so it makes little sense to push that down into the
drivers as well.

Mind you I'm not 100% sure how we would retrofit a binding if necessary
for the inverted cases.  Hope we don't hit one here :)

As you note, 3wire is needed in the binding because it's optional.

Jonathan

quoted

quoted

+
+  spi-max-frequency: true
+
+  vs-supply:
+    description: Regulator that supplies power to the accelerometer
+
+  vdd-supply:
+    description: Regulator that supplies the digital interface supply voltage
+
+  interrupts:
+    maxItems: 2  

This means there must be 2 entries. If 1 is valid, you need 'minItems'.
 

I'll add 'minItems' for the v3 then, thanks!

quoted

quoted

+
+  interrupt-names:
+    maxItems: 2  

You need 'minItems' too to fix the error.
 

Thank you again and sorry for not catching that error before submitting.

quoted

quoted

+    items:
+      enum:
+        - INT1
+        - INT2
+
+required:
+  - compatible
+  - reg
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/gpio/gpio.h>
+    #include <dt-bindings/interrupt-controller/irq.h>
+    i2c0 {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        /* Example for a I2C device node */
+        accelerometer@53 {
+            compatible = "adi,adxl313";
+            reg = <0x53>;
+            interrupt-parent = <&gpio0>;
+            interrupts = <0 IRQ_TYPE_LEVEL_HIGH>;
+            interrupt-names = "INT1";
+        };
+    };
+  - |
+    #include <dt-bindings/gpio/gpio.h>
+    #include <dt-bindings/interrupt-controller/irq.h>
+    spi {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        /* Example for a SPI device node */
+        accelerometer@0 {
+            compatible = "adi,adxl313";
+            reg = <0>;
+            spi-max-frequency = <5000000>;
+            spi-cpol;
+            spi-cpha;
+            interrupt-parent = <&gpio0>;
+            interrupts = <0 IRQ_TYPE_LEVEL_HIGH>;
+            interrupt-names = "INT1";
+        };
+    };
--
2.32.0