2024-02-28 12:27:41

by Alisa-Dariana Roman

[permalink] [raw]
Subject: [PATCH v4 3/4] dt-bindings: iio: adc: ad7192: Add AD7194 support

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 <[email protected]>
Signed-off-by: romandariana <[email protected]>
---
.../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]+)$":
+ 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>;
+ };
+ };
+ };
--
2.34.1



2024-02-28 14:06:30

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH v4 3/4] dt-bindings: iio: adc: ad7192: Add AD7194 support

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 <[email protected]>
> Signed-off-by: romandariana <[email protected]>

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


2024-02-28 19:32:07

by David Lechner

[permalink] [raw]
Subject: Re: [PATCH v4 3/4] dt-bindings: iio: adc: ad7192: Add AD7194 support

On Wed, Feb 28, 2024 at 6:26 AM Alisa-Dariana Roman
<[email protected]> 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 <[email protected]>
> Signed-off-by: romandariana <[email protected]>

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.