diff mbox series

[v4,3/4] dt-bindings: iio: adc: ad7192: Add AD7194 support

Message ID 20240228122617.185814-4-alisa.roman@analog.com (mailing list archive)
State Changes Requested
Headers show
Series iio: adc: ad7192: Add support for AD7194 | expand

Commit Message

Alisa-Dariana Roman Feb. 28, 2024, 12:26 p.m. UTC
Unlike the other AD719Xs, AD7194 has configurable differential
channels. The default configuration for these channels can be changed
from the devicetree.

Also add an example for AD7194 devicetree.

Signed-off-by: Alisa-Dariana Roman <alisa.roman@analog.com>
Signed-off-by: romandariana <alisa.roman@analog.com>
---
 .../bindings/iio/adc/adi,ad7192.yaml          | 75 +++++++++++++++++++
 1 file changed, 75 insertions(+)

Comments

Krzysztof Kozlowski Feb. 28, 2024, 2:06 p.m. UTC | #1
On 28/02/2024 13:26, Alisa-Dariana Roman wrote:
> Unlike the other AD719Xs, AD7194 has configurable differential
> channels. The default configuration for these channels can be changed
> from the devicetree.
> 
> Also add an example for AD7194 devicetree.
> 
> Signed-off-by: Alisa-Dariana Roman <alisa.roman@analog.com>
> Signed-off-by: romandariana <alisa.roman@analog.com>

Something is not right here...

> ---
>  .../bindings/iio/adc/adi,ad7192.yaml          | 75 +++++++++++++++++++
>  1 file changed, 75 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/iio/adc/adi,ad7192.yaml b/Documentation/devicetree/bindings/iio/adc/adi,ad7192.yaml
> index 16def2985ab4..c62862760311 100644
> --- a/Documentation/devicetree/bindings/iio/adc/adi,ad7192.yaml
> +++ b/Documentation/devicetree/bindings/iio/adc/adi,ad7192.yaml
> @@ -21,8 +21,15 @@ properties:
>        - adi,ad7190
>        - adi,ad7192
>        - adi,ad7193
> +      - adi,ad7194
>        - adi,ad7195
>  
> +  "#address-cells":
> +    const: 1
> +
> +  "#size-cells":
> +    const: 0
> +
>    reg:
>      maxItems: 1
>  
> @@ -96,8 +103,44 @@ required:
>    - spi-cpol
>    - spi-cpha
>  
> +patternProperties:
> +  "^channel@([0-9]+)$":

No need for ()

> +    type: object
> +    $ref: adc.yaml
> +    unevaluatedProperties: false
> +
> +    properties:
> +      reg:
> +        description: The channel index.
> +        minimum: 1
> +        maximum: 16
> +
> +      diff-channels:
> +        description: |
> +          Both inputs can be connected to pins AIN1 to AIN16 by choosing the
> +          appropriate value from 1 to 16. The negative input can also be
> +          connected to AINCOM by choosing 0.
> +        items:
> +          minimum: 0
> +          maximum: 16
> +
> +    required:
> +      - reg
> +      - diff-channels
> +
>  allOf:
>    - $ref: /schemas/spi/spi-peripheral-props.yaml#
> +  - if:
> +      properties:
> +        compatible:
> +          enum:
> +            - adi,ad7190
> +            - adi,ad7192
> +            - adi,ad7193
> +            - adi,ad7195
> +    then:
> +      patternProperties:
> +        "^channel@([0-9]+)$": false

No need for ()

>  
>  unevaluatedProperties: false
>  
> @@ -127,3 +170,35 @@ examples:


Best regards,
Krzysztof
David Lechner Feb. 28, 2024, 7:31 p.m. UTC | #2
On Wed, Feb 28, 2024 at 6:26 AM Alisa-Dariana Roman
<alisadariana@gmail.com> wrote:
>
> Unlike the other AD719Xs, AD7194 has configurable differential
> channels. The default configuration for these channels can be changed
> from the devicetree.
>
> Also add an example for AD7194 devicetree.
>
> Signed-off-by: Alisa-Dariana Roman <alisa.roman@analog.com>
> Signed-off-by: romandariana <alisa.roman@analog.com>

Why two SOBs with the same email?

> ---
>  .../bindings/iio/adc/adi,ad7192.yaml          | 75 +++++++++++++++++++
>  1 file changed, 75 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/iio/adc/adi,ad7192.yaml b/Documentation/devicetree/bindings/iio/adc/adi,ad7192.yaml
> index 16def2985ab4..c62862760311 100644
> --- a/Documentation/devicetree/bindings/iio/adc/adi,ad7192.yaml
> +++ b/Documentation/devicetree/bindings/iio/adc/adi,ad7192.yaml
> @@ -21,8 +21,15 @@ properties:
>        - adi,ad7190
>        - adi,ad7192
>        - adi,ad7193
> +      - adi,ad7194
>        - adi,ad7195
>
> +  "#address-cells":
> +    const: 1
> +
> +  "#size-cells":
> +    const: 0
> +
>    reg:
>      maxItems: 1
>

Based on the discussion in v3, I was expecting to see an aincom-supply
property added. (Although that probably belongs in a separate patch
since it applies to all existing chips, not just the one being added
here.)


> @@ -96,8 +103,44 @@ required:
>    - spi-cpol
>    - spi-cpha
>
> +patternProperties:
> +  "^channel@([0-9]+)$":
> +    type: object
> +    $ref: adc.yaml
> +    unevaluatedProperties: false
> +
> +    properties:
> +      reg:
> +        description: The channel index.
> +        minimum: 1
> +        maximum: 16

I guess this is OK, but max of 16 is a bit artificial since there
could be unusual use cases where inputs are reused on multiple
channels. Technically, there are 16 * 17 = 272 possible combinations
that could appear.
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/iio/adc/adi,ad7192.yaml b/Documentation/devicetree/bindings/iio/adc/adi,ad7192.yaml
index 16def2985ab4..c62862760311 100644
--- a/Documentation/devicetree/bindings/iio/adc/adi,ad7192.yaml
+++ b/Documentation/devicetree/bindings/iio/adc/adi,ad7192.yaml
@@ -21,8 +21,15 @@  properties:
       - adi,ad7190
       - adi,ad7192
       - adi,ad7193
+      - adi,ad7194
       - adi,ad7195
 
+  "#address-cells":
+    const: 1
+
+  "#size-cells":
+    const: 0
+
   reg:
     maxItems: 1
 
@@ -96,8 +103,44 @@  required:
   - spi-cpol
   - spi-cpha
 
+patternProperties:
+  "^channel@([0-9]+)$":
+    type: object
+    $ref: adc.yaml
+    unevaluatedProperties: false
+
+    properties:
+      reg:
+        description: The channel index.
+        minimum: 1
+        maximum: 16
+
+      diff-channels:
+        description: |
+          Both inputs can be connected to pins AIN1 to AIN16 by choosing the
+          appropriate value from 1 to 16. The negative input can also be
+          connected to AINCOM by choosing 0.
+        items:
+          minimum: 0
+          maximum: 16
+
+    required:
+      - reg
+      - diff-channels
+
 allOf:
   - $ref: /schemas/spi/spi-peripheral-props.yaml#
+  - if:
+      properties:
+        compatible:
+          enum:
+            - adi,ad7190
+            - adi,ad7192
+            - adi,ad7193
+            - adi,ad7195
+    then:
+      patternProperties:
+        "^channel@([0-9]+)$": false
 
 unevaluatedProperties: false
 
@@ -127,3 +170,35 @@  examples:
             adi,burnout-currents-enable;
         };
     };
+  - |
+    spi {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        adc@0 {
+            #address-cells = <1>;
+            #size-cells = <0>;
+            compatible = "adi,ad7194";
+            reg = <0>;
+            spi-max-frequency = <1000000>;
+            spi-cpol;
+            spi-cpha;
+            clocks = <&ad7192_mclk>;
+            clock-names = "mclk";
+            interrupts = <25 0x2>;
+            interrupt-parent = <&gpio>;
+            dvdd-supply = <&dvdd>;
+            avdd-supply = <&avdd>;
+            vref-supply = <&vref>;
+
+            channel@1 {
+                reg = <1>;
+                diff-channels = <1 6>;
+            };
+
+            channel@16 {
+                reg = <16>;
+                diff-channels = <16 5>;
+            };
+        };
+    };