Re: [PATCH 1/2] add binding for iio adc driver for TI ADS1018

On Tue, 9 Nov 2021 10:00:14 +0000
Iain Hunter [off-list ref] wrote:

Hi Daniel,
I'm happy to add myself. I assume that is just a case of re-submitting the
patch with my name instead of yours. Is that correct?

That would be perfect.  Thanks for taking this on,

A few comments below from me.

Jonathan

Iain

On Mon, Nov 8, 2021 at 6:45 PM Daniel Baluta [off-list ref] wrote:

quoted

This looks good to me with the only mention that I don't actually have
time to maintain this :(.

Perhaps you can add yourself?

On 11/8/21 8:43 PM, Iain Hunter wrote:  

quoted

Add a new binding for new IIO ADS1018 driver.

Signed-off-by: Iain Hunter <redacted>
---
  .../bindings/iio/adc/ti,ads1018.yaml          | 109 ++++++++++++++++++
  1 file changed, 109 insertions(+)
  create mode 100644  

Documentation/devicetree/bindings/iio/adc/ti,ads1018.yaml  

quoted

diff --git a/Documentation/devicetree/bindings/iio/adc/ti,ads1018.yaml  

b/Documentation/devicetree/bindings/iio/adc/ti,ads1018.yaml  

quoted

new file mode 100644
index 000000000000..ba0fdfba2c45

--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/adc/ti,ads1018.yaml

@@ -0,0 +1,109 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/iio/adc/ti,ads1018.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: TI ADS1018 4 channel SPI analog to digital converter
+
+maintainers:
+  - Daniel Baluta <daniel.baluta@nxp.com>
+
+description: |
+  Datasheet at: https://www.ti.com/lit/gpn/ads1018
+  Supports both single ended and differential channels.
+
+properties:
+  compatible:
+    const: ti,ads1018
+
+  reg:
+    maxItems: 1
+
+  "#address-cells":
+    const: 1
+
+  "#size-cells":
+    const: 0
+
+  "#io-channel-cells":
+    const: 1
+
+required:
+  - compatible
+  - reg
+  - "#address-cells"
+  - "#size-cells"
+
+additionalProperties: false
+
+patternProperties:
+  "^channel@[0-7]+$":
+    type: object
+    description:
+      Child nodes needed for each channel that the platform uses.
+
+    properties:
+      reg:
+        description: |
+          0: Voltage over AIN0 and AIN1.
+          1: Voltage over AIN0 and AIN3.
+          2: Voltage over AIN1 and AIN3.
+          3: Voltage over AIN2 and AIN3.
+          4: Voltage over AIN0 and GND.
+          5: Voltage over AIN1 and GND.
+          6: Voltage over AIN2 and GND.
+          7: Voltage over AIN3 and GND.

This should map to the generic channel description in
adc.yaml.  If we need to extend that then we can do that rather than a
separate device specific definition.

Note that is relatively new, so there are lots of drivers with older
bindings in tree that do things more closely to what you have here.

quoted

quoted

+        items:
+          - minimum: 0
+            maximum: 7
+
+      ti,gain:
+        $ref: /schemas/types.yaml#/definitions/uint32
+        minimum: 0
+        maximum: 5
+        description: |
+          pga is the programmable gain amplifier (values are full scale)
+          0: +/- 6.144 V
+          1: +/- 4.096 V
+          2: +/- 2.048 V (default)
+          3: +/- 1.024 V
+          4: +/- 0.512 V
+          5: +/- 0.256 V

Why a DT property? Normally we'd leave this to userspace control.

quoted

quoted

+
+      ti,datarate:
+        $ref: /schemas/types.yaml#/definitions/uint32
+        minimum: 0
+        maximum: 6
+        description: |
+          Data acquisition rate in samples per second

Why is this a DT property?

If we do keep it as such use the actual values (via an enum)
and rename it to add -hz which will then remove the nee for the
$ref

quoted

quoted

+          0: 128
+          1: 250
+          2: 490
+          3: 920
+          4: 1600 (default)
+          5: 2400
+          6: 3300
+
+    required:
+      - reg
+
+examples:
+  - |
+    spi {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        adc@1 {
+            compatible = "ti,ads1018";
+            reg = <0x1>;
+            #address-cells = <1>;
+            #size-cells = <0>;
+            channel@4 {
+              reg = <4>;
+              ti,gain = <3>;
+              ti,datarate = <5>;
+            };
+        };
+    };
+...