| Message ID | 20210204113551.68744-3-alexandru.tachici@analog.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | iio: adc: ad7124: allow 16 channels | expand |
On Thu, 04 Feb 2021 13:35:51 +0200, alexandru.tachici@analog.com wrote: > From: Alexandru Tachici <alexandru.tachici@analog.com> > > Document use of configurations in device-tree bindings. > > Signed-off-by: Alexandru Tachici <alexandru.tachici@analog.com> > --- > .../bindings/iio/adc/adi,ad7124.yaml | 72 +++++++++++++++---- > 1 file changed, 57 insertions(+), 15 deletions(-) > My bot found errors running 'make dt_binding_check' on your patch: yamllint warnings/errors: ./Documentation/devicetree/bindings/iio/adc/adi,ad7124.yaml:76:10: [warning] wrong indentation: expected 10 but found 9 (indentation) ./Documentation/devicetree/bindings/iio/adc/adi,ad7124.yaml:114:10: [warning] wrong indentation: expected 10 but found 9 (indentation) dtschema/dtc warnings/errors: See https://patchwork.ozlabs.org/patch/1435965 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.
On Thu, 4 Feb 2021 13:35:51 +0200 <alexandru.tachici@analog.com> wrote: > From: Alexandru Tachici <alexandru.tachici@analog.com> > > Document use of configurations in device-tree bindings. > > Signed-off-by: Alexandru Tachici <alexandru.tachici@analog.com> 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>; > }; > }; > };
On Sat, Feb 6, 2021 at 9:26 AM Jonathan Cameron <jic23@kernel.org> wrote: > > On Thu, 4 Feb 2021 13:35:51 +0200 > <alexandru.tachici@analog.com> wrote: > > > From: Alexandru Tachici <alexandru.tachici@analog.com> > > > > Document use of configurations in device-tree bindings. > > > > Signed-off-by: Alexandru Tachici <alexandru.tachici@analog.com> > > 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
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>; }; }; };