| Message ID | 20231123140320.30409-1-user@HYB-hhAwRlzzMZb |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [v5,1/2] dt-bindings: adc: add AD7173 | expand |
| Context | Check | Description |
|---|---|---|
| robh/checkpatch | success | |
| robh/patch-applied | success | |
| robh/dt-meta-schema | fail | build log |
On 23/11/2023 15:02, mitrutzceclan wrote: > From: Dumitru Ceclan <mitrutzceclan@gmail.com> > > The AD7173 family offer a complete integrated Sigma-Delta ADC solution > which can be used in high precision, low noise single channel applications > or higher speed multiplexed applications. The Sigma-Delta ADC is intended > primarily for measurement of signals close to DC but also delivers > outstanding performance with input bandwidths out to ~10kHz. > > Signed-off-by: Dumitru Ceclan <mitrutzceclan@gmail.com> > --- > V4 -> V5 > - Use string enum instead of integers for "adi,reference-select" > - Fix conditional checking in regards to compatible > > .../bindings/iio/adc/adi,ad7173.yaml | 169 ++++++++++++++++++ > 1 file changed, 169 insertions(+) > create mode 100644 Documentation/devicetree/bindings/iio/adc/adi,ad7173.yaml > > diff --git a/Documentation/devicetree/bindings/iio/adc/adi,ad7173.yaml b/Documentation/devicetree/bindings/iio/adc/adi,ad7173.yaml > new file mode 100644 > index 000000000000..32a59a2f2e91 > --- /dev/null > +++ b/Documentation/devicetree/bindings/iio/adc/adi,ad7173.yaml > @@ -0,0 +1,169 @@ > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) > +# Copyright 2023 Analog Devices Inc. > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/iio/adc/adi,ad7173.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Analog Devices AD7173 ADC > + > +maintainers: > + - Ceclan Dumitru <dumitru.ceclan@analog.com> > + > +description: | > + Bindings for the Analog Devices AD717X ADC's. Datasheets for supported chips: Drop "Bindings for" and instead describe hardware. > + https://www.analog.com/media/en/technical-documentation/data-sheets/AD7172-2.pdf > + https://www.analog.com/media/en/technical-documentation/data-sheets/AD7173-8.pdf > + https://www.analog.com/media/en/technical-documentation/data-sheets/AD7175-2.pdf > + https://www.analog.com/media/en/technical-documentation/data-sheets/AD7176-2.pdf > + > +properties: > + compatible: > + enum: > + - adi,ad7172-2 > + - adi,ad7173-8 > + - adi,ad7175-2 > + - adi,ad7176-2 > + > + reg: > + maxItems: 1 > + > + interrupts: > + maxItems: 1 > + > + '#address-cells': > + const: 1 > + > + '#size-cells': > + const: 0 > + > + spi-max-frequency: > + maximum: 20000000 > + > + refin-supply: > + description: external reference supply, can be used as reference for conversion. > + > + refin2-supply: > + description: external reference supply, can be used as reference for conversion. > + > + avdd-supply: > + description: avdd supply, can be used as reference for conversion. > + > + required: Please test your code before sending. You ignored my comment. This has both wrong indentation and wrong placement - should be after all properties and patternProperties. Do not ignore comments but respond to them. Best regards, Krzysztof
On 11/23/23 16:26, Krzysztof Kozlowski wrote: > On 23/11/2023 15:02, mitrutzceclan wrote: >> + Bindings for the Analog Devices AD717X ADC's. Datasheets for supported chips: > > Drop "Bindings for" and instead describe hardware. > Okay ... >> + avdd-supply: >> + description: avdd supply, can be used as reference for conversion. >> + >> + required: > > Please test your code before sending. You ignored my comment. This has > both wrong indentation and wrong placement - should be after all > properties and patternProperties. > > Do not ignore comments but respond to them. > There were no errors while testing the yaml binding (with DT_CHECKER_FLAGS=-m dt_binding_check - to make sure that this is how bindings should be tested). Indeed I did not test the yaml if the required properties are missing from the example. What is indicative in this patch that it was not tested? I did not ignore your comment. I did not have questions about it. I missed the indentation. Sorry about that. But about the placement of 'required': the example-schema does not have the exact case of pattern properties. Also, there are multiple iio/adc (ad4130, ad7124, ad7292) bindings that place required before patternProperties. I assumed that this placement is correct. Will move it in next version. In regards to responding to comments: if there are no questions about a comment and will fix in next version, should there be a response anyway just confirming it?
On Thu, 23 Nov 2023 16:02:55 +0200, mitrutzceclan wrote: > From: Dumitru Ceclan <mitrutzceclan@gmail.com> > > The AD7173 family offer a complete integrated Sigma-Delta ADC solution > which can be used in high precision, low noise single channel applications > or higher speed multiplexed applications. The Sigma-Delta ADC is intended > primarily for measurement of signals close to DC but also delivers > outstanding performance with input bandwidths out to ~10kHz. > > Signed-off-by: Dumitru Ceclan <mitrutzceclan@gmail.com> > --- > V4 -> V5 > - Use string enum instead of integers for "adi,reference-select" > - Fix conditional checking in regards to compatible > > .../bindings/iio/adc/adi,ad7173.yaml | 169 ++++++++++++++++++ > 1 file changed, 169 insertions(+) > create mode 100644 Documentation/devicetree/bindings/iio/adc/adi,ad7173.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: /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/iio/adc/adi,ad7173.yaml: properties:required: ['compatible', 'reg', 'interrupts'] is not of type 'object', 'boolean' from schema $id: http://json-schema.org/draft-07/schema# /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/iio/adc/adi,ad7173.yaml: properties: 'required' should not be valid under {'$ref': '#/definitions/json-schema-prop-names'} hint: A json-schema keyword was found instead of a DT property name. from schema $id: http://devicetree.org/meta-schemas/keywords.yaml# /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/iio/adc/adi,ad7173.yaml: properties:required: ['compatible', 'reg', 'interrupts'] is not of type 'object', 'boolean' from schema $id: http://devicetree.org/meta-schemas/keywords.yaml# doc reference errors (make refcheckdocs): See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20231123140320.30409-1-user@HYB-hhAwRlzzMZb The base for the series is generally the latest rc1. A different dependency should be noted in *this* patch. 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 after running the above command yourself. Note that DT_SCHEMA_FILES can be set to your schema file to speed up checking your schema. However, it must be unset to test all examples with your schema.
On 23/11/2023 16:11, Ceclan Dumitru wrote: > > > On 11/23/23 16:26, Krzysztof Kozlowski wrote: >> On 23/11/2023 15:02, mitrutzceclan wrote: >>> + Bindings for the Analog Devices AD717X ADC's. Datasheets for supported chips: >> >> Drop "Bindings for" and instead describe hardware. >> > > Okay > > ... > >>> + avdd-supply: >>> + description: avdd supply, can be used as reference for conversion. >>> + >>> + required: >> >> Please test your code before sending. You ignored my comment. This has >> both wrong indentation and wrong placement - should be after all >> properties and patternProperties. >> >> Do not ignore comments but respond to them. >> > > There were no errors while testing the yaml binding (with > DT_CHECKER_FLAGS=-m dt_binding_check - to make sure that this is how > bindings should be tested). Indeed I did not test the yaml if the > required properties are missing from the example. What is indicative in > this patch that it was not tested? Then your testing method might miss something, because as you can see - Rob's bot found the issue. > > I did not ignore your comment. I did not have questions about it. I > missed the indentation. Sorry about that. > > But about the placement of 'required': the example-schema does not have > the exact case of pattern properties. Also, there are multiple iio/adc > (ad4130, ad7124, ad7292) bindings that place required before > patternProperties. I assumed that this placement is correct. > > Will move it in next version. > > In regards to responding to comments: if there are no questions about a > comment and will fix in next version, should there be a response anyway > just confirming it? The point is that code did not change here and there was no Ack/Done/something reply. Best regards, Krzysztof
diff --git a/Documentation/devicetree/bindings/iio/adc/adi,ad7173.yaml b/Documentation/devicetree/bindings/iio/adc/adi,ad7173.yaml new file mode 100644 index 000000000000..32a59a2f2e91 --- /dev/null +++ b/Documentation/devicetree/bindings/iio/adc/adi,ad7173.yaml @@ -0,0 +1,169 @@ +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) +# Copyright 2023 Analog Devices Inc. +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/iio/adc/adi,ad7173.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Analog Devices AD7173 ADC + +maintainers: + - Ceclan Dumitru <dumitru.ceclan@analog.com> + +description: | + Bindings for the Analog Devices AD717X ADC's. Datasheets for supported chips: + https://www.analog.com/media/en/technical-documentation/data-sheets/AD7172-2.pdf + https://www.analog.com/media/en/technical-documentation/data-sheets/AD7173-8.pdf + https://www.analog.com/media/en/technical-documentation/data-sheets/AD7175-2.pdf + https://www.analog.com/media/en/technical-documentation/data-sheets/AD7176-2.pdf + +properties: + compatible: + enum: + - adi,ad7172-2 + - adi,ad7173-8 + - adi,ad7175-2 + - adi,ad7176-2 + + reg: + maxItems: 1 + + interrupts: + maxItems: 1 + + '#address-cells': + const: 1 + + '#size-cells': + const: 0 + + spi-max-frequency: + maximum: 20000000 + + refin-supply: + description: external reference supply, can be used as reference for conversion. + + refin2-supply: + description: external reference supply, can be used as reference for conversion. + + avdd-supply: + description: avdd supply, can be used as reference for conversion. + + required: + - compatible + - reg + - interrupts + +patternProperties: + "^channel@[0-9a-f]$": + type: object + $ref: adc.yaml + unevaluatedProperties: false + + properties: + reg: + minimum: 0 + maximum: 15 + + diff-channels: + items: + minimum: 0 + maximum: 31 + + adi,reference-select: + description: | + Select the reference source to use when converting on + the specific channel. Valid values are: + refin : REFIN(+)/REFIN(−). + refin2 : REFIN2(+)/REFIN2(−) + refout-avss: REFOUT/AVSS (Internal reference) + avdd : AVDD + + External reference refin2 only available on ad7173-8. + If not specified, internal reference used. + enum: + - refin + - refin2 + - refout-avss + - avdd + default: refout-avss + + bipolar: + type: boolean + + required: + - reg + - diff-channels + +allOf: + - $ref: /schemas/spi/spi-peripheral-props.yaml# + + - if: + properties: + compatible: + not: + contains: + const: adi,ad7173-8 + then: + properties: + refin2-supply: false + patternProperties: + "^channel@[0-9a-f]$": + properties: + adi,reference-select: + enum: + - refin + - refout-avss + - avdd + +unevaluatedProperties: false + +examples: + - | + #include <dt-bindings/gpio/gpio.h> + #include <dt-bindings/interrupt-controller/irq.h> + + spi { + #address-cells = <1>; + #size-cells = <0>; + + adc@0 { + compatible = "adi,ad7173-8"; + reg = <0>; + + #address-cells = <1>; + #size-cells = <0>; + + interrupts = <25 IRQ_TYPE_EDGE_FALLING>; + interrupt-parent = <&gpio>; + spi-max-frequency = <5000000>; + + channel@0 { + reg = <0>; + bipolar; + diff-channels = <0 1>; + }; + + channel@1 { + reg = <1>; + diff-channels = <2 3>; + }; + + channel@2 { + reg = <2>; + bipolar; + diff-channels = <4 5>; + }; + + channel@3 { + reg = <3>; + bipolar; + diff-channels = <6 7>; + }; + + channel@4 { + reg = <4>; + diff-channels = <8 9>; + }; + }; + };