* Re: [PATCH v4 1/2] Add binding for ti,adc1018. It allows selection of channel as a Device Tree property
2021-12-31 13:19 [PATCH v4 1/2] Add binding for ti,adc1018. It allows selection of channel as a Device Tree property Iain Hunter
@ 2022-01-01 22:01 ` Rob Herring
2022-01-04 15:53 ` Rob Herring
2022-01-09 11:17 ` Jonathan Cameron
2 siblings, 0 replies; 6+ messages in thread
From: Rob Herring @ 2022-01-01 22:01 UTC (permalink / raw)
To: Iain Hunter
Cc: iain, Rob Herring, Jonathan Cameron, linux-iio, devicetree,
Lars-Peter Clausen, linux-kernel
On Fri, 31 Dec 2021 13:19:15 +0000, Iain Hunter wrote:
> New binding file uses the adc.yaml to define channel selection
>
> Signed-off-by: Iain Hunter <drhunter95@gmail.com>
> ---
> .../bindings/iio/adc/ti,ads1018.yaml | 126 ++++++++++++++++++
> 1 file changed, 126 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/iio/adc/ti,ads1018.yaml
>
My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
on your patch (DT_CHECKER_FLAGS is new in v5.13):
yamllint warnings/errors:
dtschema/dtc warnings/errors:
Documentation/devicetree/bindings/iio/adc/ti,ads1018.example.dts:24.19-31.15: Warning (spi_bus_reg): /example-0/spi/adc@1: SPI bus unit address format error, expected "0"
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/iio/adc/ti,ads1018.example.dt.yaml: adc@1: 'ti,adc-channels' does not match any of the regexes: '^channel@([0-3])$', 'pinctrl-[0-9]+'
From schema: /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/iio/adc/ti,ads1018.yaml
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/iio/adc/ti,ads1018.example.dt.yaml: adc@0: 'ti,adc-diff-channels' does not match any of the regexes: '^channel@([0-3])$', 'pinctrl-[0-9]+'
From schema: /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/iio/adc/ti,ads1018.yaml
doc reference errors (make refcheckdocs):
See https://patchwork.ozlabs.org/patch/1574373
This check can fail if there are any dependencies. The base for a patch
series is generally the most recent rc1.
If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:
pip3 install dtschema --upgrade
Please check and re-submit.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v4 1/2] Add binding for ti,adc1018. It allows selection of channel as a Device Tree property
2021-12-31 13:19 [PATCH v4 1/2] Add binding for ti,adc1018. It allows selection of channel as a Device Tree property Iain Hunter
2022-01-01 22:01 ` Rob Herring
@ 2022-01-04 15:53 ` Rob Herring
2022-01-09 11:17 ` Jonathan Cameron
2 siblings, 0 replies; 6+ messages in thread
From: Rob Herring @ 2022-01-04 15:53 UTC (permalink / raw)
To: Iain Hunter
Cc: iain, Jonathan Cameron, Lars-Peter Clausen, linux-iio,
devicetree, linux-kernel
On Fri, Dec 31, 2021 at 01:19:15PM +0000, Iain Hunter wrote:
> New binding file uses the adc.yaml to define channel selection
For the subject, follow the conventions used by other files in the
subsystem. Run 'git log --oneline directory/for/file' and it should be
evident.
>
> Signed-off-by: Iain Hunter <drhunter95@gmail.com>
> ---
> .../bindings/iio/adc/ti,ads1018.yaml | 126 ++++++++++++++++++
> 1 file changed, 126 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/iio/adc/ti,ads1018.yaml
>
> diff --git a/Documentation/devicetree/bindings/iio/adc/ti,ads1018.yaml b/Documentation/devicetree/bindings/iio/adc/ti,ads1018.yaml
> new file mode 100644
> index 000000000000..a65fee9d83dd
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/adc/ti,ads1018.yaml
> @@ -0,0 +1,126 @@
> +# 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 I2C analog to digital converter
> +
> +maintainers:
> + - Iain Hunter <iain@hunterembedded.co.uk>
> +
> +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
> +
> + spi-max-frequency: true
> + spi-cpol: true
> + spi-cpha: true
> +
> +required:
> + - compatible
> + - reg
> + - "#address-cells"
> + - "#size-cells"
> + - spi-cpha
> +
> +additionalProperties: false
> +
> +patternProperties:
> + "^channel@([0-3])$":
0-3 doesn't match the allowed 'reg' values.
> + $ref: "adc.yaml"
> + type: object
> +
> + properties:
> + reg:
> + description: |
> + Must be 0, actual channel selected in ti,adc-channels for single ended
> + or ti-adc-channels-diff for differential
> + $ref: /schemas/types.yaml#/definitions/uint32
> + enum: [0]
> +
> + ti,adc-channels:
> + description: |
> + List of single-ended channels muxed for this ADC. It can have up to 4
> + channels numbered 0-3
> + $ref: /schemas/types.yaml#/definitions/uint32-array
> + deprecated: true
> +
> + ti,adc-diff-channels:
> + description: |
> + List of differential channels muxed for this ADC between the pins vinp
> + and vinn. The 4 possible options are:
> + vinp=0, vinn=1
> + vinp=0, vinn=3
> + vinp=1, vinn=3
> + vinp=2, vinn=3
> +
> + They are listed in a pair <vinp vinn>.
> +
> + Note: At least one of "ti,adc-channels" or "ti,adc-diff-channels" is
> + required.
> + $ref: /schemas/types.yaml#/definitions/uint32-matrix
> + items:
No bounds for the number of tuples?
> + items:
> + - description: |
> + "vinp" indicates positive input number
> + minimum: 0
> + maximum: 2
> + - description: |
> + "vinn" indicates negative input number
> + minimum: 1
> + maximum: 3
> +
> +
> + required:
> + - reg
> +
> +examples:
> + - |
> + // example on SPI1 with single ended channel 1
> + spi {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + adc@1 {
> + compatible = "ti,ads1018";
> + reg = <0x0>;
> + #address-cells = <1>;
> + #size-cells = <0>;
> + spi-cpha;
> + ti,adc-channels = <1>;
> + };
> + };
> + - |
> + // example on SPI0 with differential between inputs 0 and 3
> + spi {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + adc@0 {
> + compatible = "ti,ads1018";
> + reg = <0x0>;
> + #address-cells = <1>;
> + #size-cells = <0>;
> + spi-cpha;
> + ti,adc-diff-channels = <0 3>;
> + };
> + };
> +
> +...
> --
> 2.25.1
>
>
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH v4 1/2] Add binding for ti,adc1018. It allows selection of channel as a Device Tree property
2021-12-31 13:19 [PATCH v4 1/2] Add binding for ti,adc1018. It allows selection of channel as a Device Tree property Iain Hunter
2022-01-01 22:01 ` Rob Herring
2022-01-04 15:53 ` Rob Herring
@ 2022-01-09 11:17 ` Jonathan Cameron
2022-01-09 14:29 ` iain
2 siblings, 1 reply; 6+ messages in thread
From: Jonathan Cameron @ 2022-01-09 11:17 UTC (permalink / raw)
To: Iain Hunter
Cc: iain, Lars-Peter Clausen, Rob Herring, linux-iio, devicetree,
linux-kernel
On Fri, 31 Dec 2021 13:19:15 +0000
Iain Hunter <drhunter95@gmail.com> wrote:
> New binding file uses the adc.yaml to define channel selection
>
> Signed-off-by: Iain Hunter <drhunter95@gmail.com>
Hi Iain,
A few comments in addition to those Rob sent.
It's worth noting that there is a lot of 'history' in IIO bindings so
sometimes copying stuff from an existing binding is no longer the way
things should be done.
Jonathan
> ---
> .../bindings/iio/adc/ti,ads1018.yaml | 126 ++++++++++++++++++
> 1 file changed, 126 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/iio/adc/ti,ads1018.yaml
>
> diff --git a/Documentation/devicetree/bindings/iio/adc/ti,ads1018.yaml b/Documentation/devicetree/bindings/iio/adc/ti,ads1018.yaml
> new file mode 100644
> index 000000000000..a65fee9d83dd
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/adc/ti,ads1018.yaml
> @@ -0,0 +1,126 @@
> +# 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 I2C analog to digital converter
> +
> +maintainers:
> + - Iain Hunter <iain@hunterembedded.co.uk>
> +
> +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
> +
> + spi-max-frequency: true
> + spi-cpol: true
> + spi-cpha: true
> +
> +required:
> + - compatible
> + - reg
> + - "#address-cells"
> + - "#size-cells"
> + - spi-cpha
> +
> +additionalProperties: false
> +
> +patternProperties:
> + "^channel@([0-3])$":
> + $ref: "adc.yaml"
> + type: object
> +
> + properties:
> + reg:
> + description: |
> + Must be 0, actual channel selected in ti,adc-channels for single ended
> + or ti-adc-channels-diff for differential
> + $ref: /schemas/types.yaml#/definitions/uint32
> + enum: [0]
No. Should be some sort of index value. If I recall correctly, existing use is reg == channel
number when single ended and more loosely defined for differential. In many cases first of the
pair, but that's not always guaranteed to be unique (e.g. 0-1 and 0-3 in this case).
> +
> + ti,adc-channels:
> + description: |
> + List of single-ended channels muxed for this ADC. It can have up to 4
> + channels numbered 0-3
This is a new binding, so how can we have deprecated properties?
Also seems very odd indeed to have a list of channels defined inside a per channel node.
> + $ref: /schemas/types.yaml#/definitions/uint32-array
> + deprecated: true
> +
> + ti,adc-diff-channels:
Can this use diff-channels in the standard adc binding:
Documentation/devicetree/bindings/iio/adc/adc.yaml
> + description: |
> + List of differential channels muxed for this ADC between the pins vinp
> + and vinn. The 4 possible options are:
> + vinp=0, vinn=1
> + vinp=0, vinn=3
> + vinp=1, vinn=3
> + vinp=2, vinn=3
> +
> + They are listed in a pair <vinp vinn>.
> +
> + Note: At least one of "ti,adc-channels" or "ti,adc-diff-channels" is
> + required.
> + $ref: /schemas/types.yaml#/definitions/uint32-matrix
> + items:
> + items:
> + - description: |
> + "vinp" indicates positive input number
> + minimum: 0
> + maximum: 2
> + - description: |
> + "vinn" indicates negative input number
> + minimum: 1
> + maximum: 3
This should be a pair based constraint as not all options possible. Something like
oneOf:
- items:
- const: 0
- const: 1
- items:
- enum: [0, 1, 2]
- const: 3
> +
> +
> + required:
> + - reg
> +
> +examples:
> + - |
> + // example on SPI1 with single ended channel 1
> + spi {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + adc@1 {
> + compatible = "ti,ads1018";
> + reg = <0x0>;
> + #address-cells = <1>;
> + #size-cells = <0>;
> + spi-cpha;
> + ti,adc-channels = <1>;
More recent approach to this is the one you've used for differential channels
- 1 child node per channel.
> + };
> + };
> + - |
> + // example on SPI0 with differential between inputs 0 and 3
The SPI0 vs 1 is correctly not part of this example, so drop that from
the comment.
> + spi {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + adc@0 {
> + compatible = "ti,ads1018";
> + reg = <0x0>;
> + #address-cells = <1>;
> + #size-cells = <0>;
> + spi-cpha;
> + ti,adc-diff-channels = <0 3>;
This doesn't obey the schema you have above at all. Would looks something like
channel@0 {
diff-channels = <0 3>;
}
> + };
> + };
> +
> +...
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH v4 1/2] Add binding for ti,adc1018. It allows selection of channel as a Device Tree property
2022-01-09 11:17 ` Jonathan Cameron
@ 2022-01-09 14:29 ` iain
2022-01-09 16:49 ` Jonathan Cameron
0 siblings, 1 reply; 6+ messages in thread
From: iain @ 2022-01-09 14:29 UTC (permalink / raw)
To: Iain Hunter, Jonathan Cameron
Cc: iain, Lars-Peter Clausen, Rob Herring, linux-iio, devicetree,
linux-kernel
On Sunday, 9 January 2022 11:17:18 GMT Jonathan Cameron wrote:
> On Fri, 31 Dec 2021 13:19:15 +0000
>
> Iain Hunter <drhunter95@gmail.com> wrote:
> > New binding file uses the adc.yaml to define channel selection
> >
> > Signed-off-by: Iain Hunter <drhunter95@gmail.com>
>
> Hi Iain,
>
> A few comments in addition to those Rob sent.
> It's worth noting that there is a lot of 'history' in IIO bindings so
> sometimes copying stuff from an existing binding is no longer the way
> things should be done.
>
> Jonathan
Hi Jonathan and Rob,
Thanks for your comments. I'd say my fundamental problem is that I am
stumbling about in the dark. To be honest I haven't even worked out the benefit
of the yaml bindings.
I identified the stm32adc binding as the most up to date file to use as a
reference. If there is a better one then can you let me know.
I will work through the comments to try to understand and then implement them.
Thanks, Iain
>
> > ---
> >
> > .../bindings/iio/adc/ti,ads1018.yaml | 126 ++++++++++++++++++
> > 1 file changed, 126 insertions(+)
> > create mode 100644
> > Documentation/devicetree/bindings/iio/adc/ti,ads1018.yaml>
> > diff --git a/Documentation/devicetree/bindings/iio/adc/ti,ads1018.yaml
> > b/Documentation/devicetree/bindings/iio/adc/ti,ads1018.yaml new file mode
> > 100644
> > index 000000000000..a65fee9d83dd
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/iio/adc/ti,ads1018.yaml
> > @@ -0,0 +1,126 @@
> > +# 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 I2C analog to digital converter
> > +
> > +maintainers:
> > + - Iain Hunter <iain@hunterembedded.co.uk>
> > +
> > +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
> > +
> > + spi-max-frequency: true
> > + spi-cpol: true
> > + spi-cpha: true
> > +
> > +required:
> > + - compatible
> > + - reg
> > + - "#address-cells"
> > + - "#size-cells"
> > + - spi-cpha
> > +
> > +additionalProperties: false
> > +
> > +patternProperties:
> > + "^channel@([0-3])$":
> > + $ref: "adc.yaml"
> > + type: object
> > +
> > + properties:
> > + reg:
> > + description: |
> > + Must be 0, actual channel selected in ti,adc-channels for
> > single ended + or ti-adc-channels-diff for differential
> > + $ref: /schemas/types.yaml#/definitions/uint32
> > + enum: [0]
>
> No. Should be some sort of index value. If I recall correctly, existing use
> is reg == channel number when single ended and more loosely defined for
> differential. In many cases first of the pair, but that's not always
> guaranteed to be unique (e.g. 0-1 and 0-3 in this case).
> > +
> > + ti,adc-channels:
> > + description: |
> > + List of single-ended channels muxed for this ADC. It can have
> > up to 4 + channels numbered 0-3
>
> This is a new binding, so how can we have deprecated properties?
> Also seems very odd indeed to have a list of channels defined inside a per
> channel node.
> > + $ref: /schemas/types.yaml#/definitions/uint32-array
> > + deprecated: true
> > +
As you can guess, it's because I don't understand it properly :)
>
> > + ti,adc-diff-channels:
> Can this use diff-channels in the standard adc binding:
> Documentation/devicetree/bindings/iio/adc/adc.yaml
>
> > + description: |
> > + List of differential channels muxed for this ADC between the
> > pins vinp + and vinn. The 4 possible options are:
> > + vinp=0, vinn=1
> > + vinp=0, vinn=3
> > + vinp=1, vinn=3
> > + vinp=2, vinn=3
> > +
> > + They are listed in a pair <vinp vinn>.
> > +
> > + Note: At least one of "ti,adc-channels" or
> > "ti,adc-diff-channels" is + required.
> > + $ref: /schemas/types.yaml#/definitions/uint32-matrix
> > + items:
> > + items:
> > + - description: |
> > + "vinp" indicates positive input number
> > + minimum: 0
> > + maximum: 2
> > + - description: |
> > + "vinn" indicates negative input number
> > + minimum: 1
> > + maximum: 3
>
> This should be a pair based constraint as not all options possible.
> Something like oneOf:
> - items:
> - const: 0
> - const: 1
> - items:
> - enum: [0, 1, 2]
> - const: 3
>
> > +
> > +
> > + required:
> > + - reg
> > +
> > +examples:
> > + - |
> > + // example on SPI1 with single ended channel 1
> > + spi {
> > + #address-cells = <1>;
> > + #size-cells = <0>;
> > +
> > + adc@1 {
> > + compatible = "ti,ads1018";
> > + reg = <0x0>;
> > + #address-cells = <1>;
> > + #size-cells = <0>;
> > + spi-cpha;
> > + ti,adc-channels = <1>;
>
> More recent approach to this is the one you've used for differential
> channels - 1 child node per channel.
>
> > + };
> > + };
> > + - |
> > + // example on SPI0 with differential between inputs 0 and 3
>
> The SPI0 vs 1 is correctly not part of this example, so drop that from
> the comment.
>
> > + spi {
> > + #address-cells = <1>;
> > + #size-cells = <0>;
> > +
> > + adc@0 {
> > + compatible = "ti,ads1018";
> > + reg = <0x0>;
> > + #address-cells = <1>;
> > + #size-cells = <0>;
> > + spi-cpha;
> > + ti,adc-diff-channels = <0 3>;
>
> This doesn't obey the schema you have above at all. Would looks something
> like channel@0 {
> diff-channels = <0 3>;
> }
>
> > + };
> > + };
> > +
> > +...
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH v4 1/2] Add binding for ti,adc1018. It allows selection of channel as a Device Tree property
2022-01-09 14:29 ` iain
@ 2022-01-09 16:49 ` Jonathan Cameron
0 siblings, 0 replies; 6+ messages in thread
From: Jonathan Cameron @ 2022-01-09 16:49 UTC (permalink / raw)
To: iain
Cc: Iain Hunter, Lars-Peter Clausen, Rob Herring, linux-iio,
devicetree, linux-kernel
On Sun, 09 Jan 2022 14:29:34 +0000
iain@hunterembedded.co.uk wrote:
> On Sunday, 9 January 2022 11:17:18 GMT Jonathan Cameron wrote:
> > On Fri, 31 Dec 2021 13:19:15 +0000
> >
> > Iain Hunter <drhunter95@gmail.com> wrote:
> > > New binding file uses the adc.yaml to define channel selection
> > >
> > > Signed-off-by: Iain Hunter <drhunter95@gmail.com>
> >
> > Hi Iain,
> >
> > A few comments in addition to those Rob sent.
> > It's worth noting that there is a lot of 'history' in IIO bindings so
> > sometimes copying stuff from an existing binding is no longer the way
> > things should be done.
> >
> > Jonathan
>
> Hi Jonathan and Rob,
>
> Thanks for your comments. I'd say my fundamental problem is that I am
> stumbling about in the dark. To be honest I haven't even worked out the benefit
> of the yaml bindings.
CI is in many ways the biggest one + formality of definition vs the older
text files where to put it likely things were often rather vague.
yaml has it's 'interesting corners' and a rather steep learning curve but
I'm not aware of anything better and we are stuck with it now anyway!
>
> I identified the stm32adc binding as the most up to date file to use as a
> reference. If there is a better one then can you let me know.
It's very much a work in progress so best practice is still evolving.
The stm32-adc ones are now quite old and reflect a very complex bit of hardware.
adc/ti-tsc2076.yaml is fairly similar and up to date.
Problem with bindings vs other code is we can't update everything to the
current view on how to do things because of backwards compatibility. Over time
we'll eventually get there as the parts people use in designs are replaced but
that's a lot longer game than for almost anything else.
Sad truth with bindings is they almost always go through a couple of revisions
as a result. Ideally I'd write some docs on what we consider best practice
for IIO drivers as that could at least be kept up to date, but lots of other
things on the todo list :(
+ today patches are coming in slightly quicker than I review them. *sigh* :)
Jonathan
>
> I will work through the comments to try to understand and then implement them.
> Thanks, Iain
> >
> > > ---
> > >
> > > .../bindings/iio/adc/ti,ads1018.yaml | 126 ++++++++++++++++++
> > > 1 file changed, 126 insertions(+)
> > > create mode 100644
> > > Documentation/devicetree/bindings/iio/adc/ti,ads1018.yaml>
> > > diff --git a/Documentation/devicetree/bindings/iio/adc/ti,ads1018.yaml
> > > b/Documentation/devicetree/bindings/iio/adc/ti,ads1018.yaml new file mode
> > > 100644
> > > index 000000000000..a65fee9d83dd
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/iio/adc/ti,ads1018.yaml
> > > @@ -0,0 +1,126 @@
> > > +# 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 I2C analog to digital converter
> > > +
> > > +maintainers:
> > > + - Iain Hunter <iain@hunterembedded.co.uk>
> > > +
> > > +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
> > > +
> > > + spi-max-frequency: true
> > > + spi-cpol: true
> > > + spi-cpha: true
> > > +
> > > +required:
> > > + - compatible
> > > + - reg
> > > + - "#address-cells"
> > > + - "#size-cells"
> > > + - spi-cpha
> > > +
> > > +additionalProperties: false
> > > +
> > > +patternProperties:
> > > + "^channel@([0-3])$":
> > > + $ref: "adc.yaml"
> > > + type: object
> > > +
> > > + properties:
> > > + reg:
> > > + description: |
> > > + Must be 0, actual channel selected in ti,adc-channels for
> > > single ended + or ti-adc-channels-diff for differential
> > > + $ref: /schemas/types.yaml#/definitions/uint32
> > > + enum: [0]
> >
> > No. Should be some sort of index value. If I recall correctly, existing use
> > is reg == channel number when single ended and more loosely defined for
> > differential. In many cases first of the pair, but that's not always
> > guaranteed to be unique (e.g. 0-1 and 0-3 in this case).
> > > +
> > > + ti,adc-channels:
> > > + description: |
> > > + List of single-ended channels muxed for this ADC. It can have
> > > up to 4 + channels numbered 0-3
> >
> > This is a new binding, so how can we have deprecated properties?
> > Also seems very odd indeed to have a list of channels defined inside a per
> > channel node.
> > > + $ref: /schemas/types.yaml#/definitions/uint32-array
> > > + deprecated: true
> > > +
>
> As you can guess, it's because I don't understand it properly :)
>
> >
> > > + ti,adc-diff-channels:
> > Can this use diff-channels in the standard adc binding:
> > Documentation/devicetree/bindings/iio/adc/adc.yaml
> >
> > > + description: |
> > > + List of differential channels muxed for this ADC between the
> > > pins vinp + and vinn. The 4 possible options are:
> > > + vinp=0, vinn=1
> > > + vinp=0, vinn=3
> > > + vinp=1, vinn=3
> > > + vinp=2, vinn=3
> > > +
> > > + They are listed in a pair <vinp vinn>.
> > > +
> > > + Note: At least one of "ti,adc-channels" or
> > > "ti,adc-diff-channels" is + required.
> > > + $ref: /schemas/types.yaml#/definitions/uint32-matrix
> > > + items:
> > > + items:
> > > + - description: |
> > > + "vinp" indicates positive input number
> > > + minimum: 0
> > > + maximum: 2
> > > + - description: |
> > > + "vinn" indicates negative input number
> > > + minimum: 1
> > > + maximum: 3
> >
> > This should be a pair based constraint as not all options possible.
> > Something like oneOf:
> > - items:
> > - const: 0
> > - const: 1
> > - items:
> > - enum: [0, 1, 2]
> > - const: 3
> >
> > > +
> > > +
> > > + required:
> > > + - reg
> > > +
> > > +examples:
> > > + - |
> > > + // example on SPI1 with single ended channel 1
> > > + spi {
> > > + #address-cells = <1>;
> > > + #size-cells = <0>;
> > > +
> > > + adc@1 {
> > > + compatible = "ti,ads1018";
> > > + reg = <0x0>;
> > > + #address-cells = <1>;
> > > + #size-cells = <0>;
> > > + spi-cpha;
> > > + ti,adc-channels = <1>;
> >
> > More recent approach to this is the one you've used for differential
> > channels - 1 child node per channel.
> >
> > > + };
> > > + };
> > > + - |
> > > + // example on SPI0 with differential between inputs 0 and 3
> >
> > The SPI0 vs 1 is correctly not part of this example, so drop that from
> > the comment.
> >
> > > + spi {
> > > + #address-cells = <1>;
> > > + #size-cells = <0>;
> > > +
> > > + adc@0 {
> > > + compatible = "ti,ads1018";
> > > + reg = <0x0>;
> > > + #address-cells = <1>;
> > > + #size-cells = <0>;
> > > + spi-cpha;
> > > + ti,adc-diff-channels = <0 3>;
> >
> > This doesn't obey the schema you have above at all. Would looks something
> > like channel@0 {
> > diff-channels = <0 3>;
> > }
> >
> > > + };
> > > + };
> > > +
> > > +...
>
>
>
>
^ permalink raw reply [flat|nested] 6+ messages in thread