2021-02-04 23:56:27

by Alexandru Tachici

[permalink] [raw]
Subject: [PATCH v2 2/2] dt-bindings: iio: adc: ad7124: add config nodes

From: Alexandru Tachici <[email protected]>

Document use of configurations in device-tree bindings.

Signed-off-by: Alexandru Tachici <[email protected]>
---
.../bindings/iio/adc/adi,ad7124.yaml | 72 +++++++++++++++----
1 file changed, 57 insertions(+), 15 deletions(-)

diff --git a/Documentation/devicetree/bindings/iio/adc/adi,ad7124.yaml b/Documentation/devicetree/bindings/iio/adc/adi,ad7124.yaml
index fb3d0dae9bae..330064461d0a 100644
--- a/Documentation/devicetree/bindings/iio/adc/adi,ad7124.yaml
+++ b/Documentation/devicetree/bindings/iio/adc/adi,ad7124.yaml
@@ -62,20 +62,19 @@ required:
- interrupts

patternProperties:
- "^channel@([0-9]|1[0-5])$":
- $ref: "adc.yaml"
+ "^config@(2[0-7])$":
type: object
description: |
- Represents the external channels which are connected to the ADC.
+ Represents a channel configuration.
+ See Documentation/devicetree/bindings/iio/adc/adc.txt.

properties:
reg:
description: |
- The channel number. It can have up to 8 channels on ad7124-4
- and 16 channels on ad7124-8, numbered from 0 to 15.
+ The config number. It can have up to 8 configuration.
items:
- minimum: 0
- maximum: 15
+ minimum: 20
+ maximum: 27

adi,reference-select:
description: |
@@ -88,8 +87,6 @@ patternProperties:
$ref: /schemas/types.yaml#/definitions/uint32
enum: [0, 1, 3]

- diff-channels: true
-
bipolar: true

adi,buffered-positive:
@@ -100,6 +97,35 @@ patternProperties:
description: Enable buffered mode for negative input.
type: boolean

+ additionalProperties: false
+
+ "^channel@([0-9]|1[0-5])$":
+ type: object
+ description: |
+ Represents the external channels which are connected to the ADC.
+ See Documentation/devicetree/bindings/iio/adc/adc.txt.
+
+ properties:
+ reg:
+ description: |
+ The channel number. It can have up to 8 channels on ad7124-4
+ and 16 channels on ad7124-8, numbered from 0 to 15.
+ items:
+ minimum: 0
+ maximum: 15
+
+ diff-channels: true
+
+ adi,configuration:
+ description: |
+ The devices has 8 configuration and ad7124-8 support up to 16 unipolar channels.
+ Each channel can be assigned one configuration. Some channels will be sharing the
+ same configuration.
+ allOf:
+ - $ref: /schemas/types.yaml#/definitions/uint32
+ minimum: 20
+ maximum: 27
+
required:
- reg
- diff-channels
@@ -127,30 +153,46 @@ examples:
#address-cells = <1>;
#size-cells = <0>;

- channel@0 {
- reg = <0>;
- diff-channels = <0 1>;
+ config@20 {
+ reg = <20>;
adi,reference-select = <0>;
adi,buffered-positive;
};

- channel@1 {
- reg = <1>;
+ config@21 {
+ reg = <21>;
bipolar;
- diff-channels = <2 3>;
adi,reference-select = <0>;
adi,buffered-positive;
adi,buffered-negative;
};

+ config@22 {
+ reg = <22>;
+ };
+
+ channel@0 {
+ reg = <0>;
+ diff-channels = <0 1>;
+ adi,configuration = <20>;
+ };
+
+ channel@1 {
+ reg = <1>;
+ diff-channels = <2 3>;
+ adi,configuration = <21>;
+ };
+
channel@2 {
reg = <2>;
diff-channels = <4 5>;
+ adi,configuration = <22>;
};

channel@3 {
reg = <3>;
diff-channels = <6 7>;
+ adi,configuration = <22>;
};
};
};
--
2.20.1


2021-02-06 15:29:58

by Jonathan Cameron

[permalink] [raw]
Subject: Re: [PATCH v2 2/2] dt-bindings: iio: adc: ad7124: add config nodes

On Thu, 4 Feb 2021 13:35:51 +0200
<[email protected]> wrote:

> From: Alexandru Tachici <[email protected]>
>
> Document use of configurations in device-tree bindings.
>
> Signed-off-by: Alexandru Tachici <[email protected]>

Ignoring discussing in my reply to the cover letter...

This is a breaking change as described. We can't move properties
around without some sort of fullback for them being in the old
location.

> ---
> .../bindings/iio/adc/adi,ad7124.yaml | 72 +++++++++++++++----
> 1 file changed, 57 insertions(+), 15 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/iio/adc/adi,ad7124.yaml b/Documentation/devicetree/bindings/iio/adc/adi,ad7124.yaml
> index fb3d0dae9bae..330064461d0a 100644
> --- a/Documentation/devicetree/bindings/iio/adc/adi,ad7124.yaml
> +++ b/Documentation/devicetree/bindings/iio/adc/adi,ad7124.yaml
> @@ -62,20 +62,19 @@ required:
> - interrupts
>
> patternProperties:
> - "^channel@([0-9]|1[0-5])$":
> - $ref: "adc.yaml"
> + "^config@(2[0-7])$":
> type: object
> description: |
> - Represents the external channels which are connected to the ADC.
> + Represents a channel configuration.
> + See Documentation/devicetree/bindings/iio/adc/adc.txt.

adc.yaml now.


>
> properties:
> reg:
> description: |
> - The channel number. It can have up to 8 channels on ad7124-4
> - and 16 channels on ad7124-8, numbered from 0 to 15.
> + The config number. It can have up to 8 configuration.
> items:
> - minimum: 0
> - maximum: 15
> + minimum: 20
> + maximum: 27

Number then 0-7 please rather than 20-27.

>
> adi,reference-select:
> description: |
> @@ -88,8 +87,6 @@ patternProperties:
> $ref: /schemas/types.yaml#/definitions/uint32
> enum: [0, 1, 3]
>
> - diff-channels: true
> -
> bipolar: true
>
> adi,buffered-positive:
> @@ -100,6 +97,35 @@ patternProperties:
> description: Enable buffered mode for negative input.
> type: boolean
>
> + additionalProperties: false
> +
> + "^channel@([0-9]|1[0-5])$":
> + type: object
> + description: |
> + Represents the external channels which are connected to the ADC.
> + See Documentation/devicetree/bindings/iio/adc/adc.txt.
> +
> + properties:
> + reg:
> + description: |
> + The channel number. It can have up to 8 channels on ad7124-4
> + and 16 channels on ad7124-8, numbered from 0 to 15.
> + items:
> + minimum: 0
> + maximum: 15
> +
> + diff-channels: true
> +
> + adi,configuration:
> + description: |
> + The devices has 8 configuration and ad7124-8 support up to 16 unipolar channels.
> + Each channel can be assigned one configuration. Some channels will be sharing the
> + same configuration.
> + allOf:
> + - $ref: /schemas/types.yaml#/definitions/uint32
> + minimum: 20
> + maximum: 27
> +
> required:
> - reg
> - diff-channels
> @@ -127,30 +153,46 @@ examples:
> #address-cells = <1>;
> #size-cells = <0>;
>
> - channel@0 {
> - reg = <0>;
> - diff-channels = <0 1>;
> + config@20 {
> + reg = <20>;
> adi,reference-select = <0>;
> adi,buffered-positive;
> };
>
> - channel@1 {
> - reg = <1>;
> + config@21 {
> + reg = <21>;
> bipolar;
> - diff-channels = <2 3>;
> adi,reference-select = <0>;
> adi,buffered-positive;
> adi,buffered-negative;
> };
>
> + config@22 {
> + reg = <22>;
> + };
> +
> + channel@0 {
> + reg = <0>;
> + diff-channels = <0 1>;
> + adi,configuration = <20>;
> + };
> +
> + channel@1 {
> + reg = <1>;
> + diff-channels = <2 3>;
> + adi,configuration = <21>;
> + };
> +
> channel@2 {
> reg = <2>;
> diff-channels = <4 5>;
> + adi,configuration = <22>;
> };
>
> channel@3 {
> reg = <3>;
> diff-channels = <6 7>;
> + adi,configuration = <22>;
> };
> };
> };

2021-02-08 18:30:48

by Rob Herring

[permalink] [raw]
Subject: Re: [PATCH v2 2/2] dt-bindings: iio: adc: ad7124: add config nodes

On Sat, Feb 6, 2021 at 9:26 AM Jonathan Cameron <[email protected]> wrote:
>
> On Thu, 4 Feb 2021 13:35:51 +0200
> <[email protected]> wrote:
>
> > From: Alexandru Tachici <[email protected]>
> >
> > Document use of configurations in device-tree bindings.
> >
> > Signed-off-by: Alexandru Tachici <[email protected]>
>
> Ignoring discussing in my reply to the cover letter...
>
> This is a breaking change as described. We can't move properties
> around without some sort of fullback for them being in the old
> location.
>
> > ---
> > .../bindings/iio/adc/adi,ad7124.yaml | 72 +++++++++++++++----
> > 1 file changed, 57 insertions(+), 15 deletions(-)
> >
> > diff --git a/Documentation/devicetree/bindings/iio/adc/adi,ad7124.yaml b/Documentation/devicetree/bindings/iio/adc/adi,ad7124.yaml
> > index fb3d0dae9bae..330064461d0a 100644
> > --- a/Documentation/devicetree/bindings/iio/adc/adi,ad7124.yaml
> > +++ b/Documentation/devicetree/bindings/iio/adc/adi,ad7124.yaml
> > @@ -62,20 +62,19 @@ required:
> > - interrupts
> >
> > patternProperties:
> > - "^channel@([0-9]|1[0-5])$":
> > - $ref: "adc.yaml"
> > + "^config@(2[0-7])$":
> > type: object
> > description: |
> > - Represents the external channels which are connected to the ADC.
> > + Represents a channel configuration.
> > + See Documentation/devicetree/bindings/iio/adc/adc.txt.
>
> adc.yaml now.
>
>
> >
> > properties:
> > reg:
> > description: |
> > - The channel number. It can have up to 8 channels on ad7124-4
> > - and 16 channels on ad7124-8, numbered from 0 to 15.
> > + The config number. It can have up to 8 configuration.
> > items:
> > - minimum: 0
> > - maximum: 15
> > + minimum: 20
> > + maximum: 27
>
> Number then 0-7 please rather than 20-27.

That doesn't work. It would be creating 2 address spaces at one level
with channel@0 and config@0. The way to address this is add a
'configs' node with config@N children.

My question here though is where does 20-27 come from. I suspect it's
made up which isn't good either. Addresses should also be rooted in
something in the h/w.

Rob