| Message ID | 20230414102844.21579-1-kimseer.paller@analog.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | [1/2] dt-bindings:iio:adc: add max14001 bindings | expand |
On 14/04/2023 12:28, Kim Seer Paller wrote: > Add bindings for MAX14001. > > The MAX14001 is configurable, isolated 10-bit ADCs for multi-range > binary inputs. Subject: missing spaces between prefixes. Subject: drop second/last, redundant "bindings". The "dt-bindings" prefix is already stating that these are bindings. > > Signed-off-by: Kim Seer Paller <kimseer.paller@analog.com> > --- > .../bindings/iio/adc/adi,max14001.yaml | 83 +++++++++++++++++++ > MAINTAINERS | 7 ++ > 2 files changed, 90 insertions(+) > create mode 100644 Documentation/devicetree/bindings/iio/adc/adi,max14001.yaml > > diff --git a/Documentation/devicetree/bindings/iio/adc/adi,max14001.yaml b/Documentation/devicetree/bindings/iio/adc/adi,max14001.yaml > new file mode 100644 > index 000000000..4546bf595 > --- /dev/null > +++ b/Documentation/devicetree/bindings/iio/adc/adi,max14001.yaml > @@ -0,0 +1,83 @@ > +# 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,max14001.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: MAX14001 ADC device driver Drop device driver. Bindings are for hardware, not Linux drivers. > + > +maintainers: > + - Kim Seer Paller <kimseer.paller@analog.com> > + > +description: | > + Single channel 10 bit ADC with SPI interface. Datasheet > + can be found here: > + https://www.analog.com/media/en/technical-documentation/data-sheets/MAX14001-MAX14002.pdf > + > +properties: > + compatible: > + enum: > + - adi,max14001 > + > + reg: > + maxItems: 1 > + > + spi-max-frequency: > + maximum: 5000000 > + > + vref-supply: > + description: Voltage reference to establish input scaling. > + > + adi,use-fadc: > + $ref: /schemas/types.yaml#/definitions/flag > + type: boolean Does not look like you tested the bindings. Please run `make dt_binding_check` (see Documentation/devicetree/bindings/writing-schema.rst for instructions). Keep one. > + description: If set, the filtered ADC data (FADC register) will be read, > + otherwise the unfiltered ADC data (ADC register) will be read. Hmmmm, looks familiar. Don't we have existing property for this? > + > + adi,inrush-mode: > + $ref: /schemas/types.yaml#/definitions/flag > + type: boolean > + description: If set, the device will use FAST inrush mode, > + otherwise the device will use ADC controlled inrush mode. > + > + adi,filter: > + $ref: /schemas/types.yaml#/definitions/uint32 > + enum: [ 0, 1, 2, 3 ] > + description: | Do not need '|' unless you need to preserve formatting. > + 0: Filtering off > + 1: Average 2 readings > + 2: Average 4 readings > + 3: Average 8 readings Isn't this also matching existing property for number of sample averaging? > + > + adi,current-source: > + $ref: /schemas/types.yaml#/definitions/flag > + type: boolean > + description: If set, the 70uA current source will be connected to the REFIN pin, > + otherwise the current source will be turned off. > + > +allOf: > + - $ref: /schemas/spi/spi-peripheral-props.yaml# > + > +required: > + - compatible > + - reg > + > +unevaluatedProperties: false > + > +examples: > + - | > + spi { > + #address-cells = <1>; > + #size-cells = <0>; > + status = "okay"; Drop status > + > + adc@0 { > + compatible = "adi,max14001"; > + reg = <0>; > + spi-max-frequency = <5000000>; > + vref-supply = <&vref_reg>; > + adi,use-fadc; > + }; > + }; > +... > diff --git a/MAINTAINERS b/MAINTAINERS > index 0e64787aa..766847ad2 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -12573,6 +12573,13 @@ S: Maintained > F: Documentation/devicetree/bindings/sound/max9860.txt > F: sound/soc/codecs/max9860.* > > +MAX14001 IIO ADC DRIVER Are you sure you ordered it correctly? Best regards, Krzysztof
Hi Kim,
kernel test robot noticed the following build warnings:
[auto build test WARNING on jic23-iio/togreg]
[also build test WARNING on linus/master v6.3-rc6]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Kim-Seer-Paller/iio-adc-add-max14001-support/20230414-183416
base: https://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git togreg
patch link: https://lore.kernel.org/r/20230414102844.21579-1-kimseer.paller%40analog.com
patch subject: [PATCH 1/2] dt-bindings:iio:adc: add max14001 bindings
reproduce:
# https://github.com/intel-lab-lkp/linux/commit/8e4267ba9a592dc820ad029c5e602098ec981159
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Kim-Seer-Paller/iio-adc-add-max14001-support/20230414-183416
git checkout 8e4267ba9a592dc820ad029c5e602098ec981159
make menuconfig
# enable CONFIG_COMPILE_TEST, CONFIG_WARN_MISSING_DOCUMENTS, CONFIG_WARN_ABI_ERRORS
make htmldocs
If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>
| Link: https://lore.kernel.org/oe-kbuild-all/202304151615.k0j79iDf-lkp@intel.com/
All warnings (new ones prefixed by >>):
>> Warning: MAINTAINERS references a file that doesn't exist: Documentation/devicetree/bindings/iio/dac/adi,max14001.yaml
On Fri, 14 Apr 2023 23:15:07 +0200 Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote: > On 14/04/2023 12:28, Kim Seer Paller wrote: > > Add bindings for MAX14001. > > > > The MAX14001 is configurable, isolated 10-bit ADCs for multi-range > > binary inputs. > > Subject: missing spaces between prefixes. > > Subject: drop second/last, redundant "bindings". The "dt-bindings" > prefix is already stating that these are bindings. A few follow up comments inline, Thanks, Jonathan > > > > > Signed-off-by: Kim Seer Paller <kimseer.paller@analog.com> > > --- > > .../bindings/iio/adc/adi,max14001.yaml | 83 +++++++++++++++++++ > > MAINTAINERS | 7 ++ > > 2 files changed, 90 insertions(+) > > create mode 100644 Documentation/devicetree/bindings/iio/adc/adi,max14001.yaml > > > > diff --git a/Documentation/devicetree/bindings/iio/adc/adi,max14001.yaml b/Documentation/devicetree/bindings/iio/adc/adi,max14001.yaml > > new file mode 100644 > > index 000000000..4546bf595 > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/iio/adc/adi,max14001.yaml > > @@ -0,0 +1,83 @@ > > +# 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,max14001.yaml# > > +$schema: http://devicetree.org/meta-schemas/core.yaml# > > + > > +title: MAX14001 ADC device driver > > Drop device driver. Bindings are for hardware, not Linux drivers. > > > + > > +maintainers: > > + - Kim Seer Paller <kimseer.paller@analog.com> > > + > > +description: | > > + Single channel 10 bit ADC with SPI interface. Datasheet > > + can be found here: > > + https://www.analog.com/media/en/technical-documentation/data-sheets/MAX14001-MAX14002.pdf > > + > > +properties: > > + compatible: > > + enum: > > + - adi,max14001 > > + > > + reg: > > + maxItems: 1 > > + > > + spi-max-frequency: > > + maximum: 5000000 > > + > > + vref-supply: > > + description: Voltage reference to establish input scaling. > > + > > + adi,use-fadc: > > + $ref: /schemas/types.yaml#/definitions/flag > > + type: boolean > > Does not look like you tested the bindings. Please run `make > dt_binding_check` (see > Documentation/devicetree/bindings/writing-schema.rst for instructions). > > Keep one. > > > + description: If set, the filtered ADC data (FADC register) will be read, > > + otherwise the unfiltered ADC data (ADC register) will be read. > > Hmmmm, looks familiar. Don't we have existing property for this? That should be a userspace decision, not a DT provided one. We have a bunch of controls defined for controlling filters. Ideal is probably to map it to that ABI. It's possible that isn't flexible enough though for this case (I haven't dived into datasheet to find out1). If so propose ABI additions with documentation in Documentation/ABI/testing/sysfs-bus-iio > > > + > > + adi,inrush-mode: > > + $ref: /schemas/types.yaml#/definitions/flag > > + type: boolean > > + description: If set, the device will use FAST inrush mode, > > + otherwise the device will use ADC controlled inrush mode. > > + > > + adi,filter: > > + $ref: /schemas/types.yaml#/definitions/uint32 > > + enum: [ 0, 1, 2, 3 ] > > + description: | > > Do not need '|' unless you need to preserve formatting. > > > + 0: Filtering off > > + 1: Average 2 readings > > + 2: Average 4 readings > > + 3: Average 8 readings > > Isn't this also matching existing property for number of sample averaging? Another one that belongs in the userspace ABI with driver picking a reaonable default. In IIO terms it's either a low pass filter, or oversampling depending on exactly what the maths is. Low pass filter if next reading include elements of the previous one (so a moving window). Oversampling if frequency of available readings drops such that each real reading contributes to only one output. If you need this stuff in the DT binding we need a strong argument for why this is a feature of the analog signals being sampled, rather than trade offs being made over noise etc. > > > + > > + adi,current-source: > > + $ref: /schemas/types.yaml#/definitions/flag > > + type: boolean > > + description: If set, the 70uA current source will be connected to the REFIN pin, > > + otherwise the current source will be turned off. This is unusual enough that I wonder if a more specific name is needed. adi,current-source-to-refin or maybe adi,current-source-for-shunt-volt-ref I'm not an expert in these, so perhaps others have better suggestions for how to describe this in a compact form. adi,current-source alone could mean any number of things that aren't what we have here. Jonathan
diff --git a/Documentation/devicetree/bindings/iio/adc/adi,max14001.yaml b/Documentation/devicetree/bindings/iio/adc/adi,max14001.yaml new file mode 100644 index 000000000..4546bf595 --- /dev/null +++ b/Documentation/devicetree/bindings/iio/adc/adi,max14001.yaml @@ -0,0 +1,83 @@ +# 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,max14001.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: MAX14001 ADC device driver + +maintainers: + - Kim Seer Paller <kimseer.paller@analog.com> + +description: | + Single channel 10 bit ADC with SPI interface. Datasheet + can be found here: + https://www.analog.com/media/en/technical-documentation/data-sheets/MAX14001-MAX14002.pdf + +properties: + compatible: + enum: + - adi,max14001 + + reg: + maxItems: 1 + + spi-max-frequency: + maximum: 5000000 + + vref-supply: + description: Voltage reference to establish input scaling. + + adi,use-fadc: + $ref: /schemas/types.yaml#/definitions/flag + type: boolean + description: If set, the filtered ADC data (FADC register) will be read, + otherwise the unfiltered ADC data (ADC register) will be read. + + adi,inrush-mode: + $ref: /schemas/types.yaml#/definitions/flag + type: boolean + description: If set, the device will use FAST inrush mode, + otherwise the device will use ADC controlled inrush mode. + + adi,filter: + $ref: /schemas/types.yaml#/definitions/uint32 + enum: [ 0, 1, 2, 3 ] + description: | + 0: Filtering off + 1: Average 2 readings + 2: Average 4 readings + 3: Average 8 readings + + adi,current-source: + $ref: /schemas/types.yaml#/definitions/flag + type: boolean + description: If set, the 70uA current source will be connected to the REFIN pin, + otherwise the current source will be turned off. + +allOf: + - $ref: /schemas/spi/spi-peripheral-props.yaml# + +required: + - compatible + - reg + +unevaluatedProperties: false + +examples: + - | + spi { + #address-cells = <1>; + #size-cells = <0>; + status = "okay"; + + adc@0 { + compatible = "adi,max14001"; + reg = <0>; + spi-max-frequency = <5000000>; + vref-supply = <&vref_reg>; + adi,use-fadc; + }; + }; +... diff --git a/MAINTAINERS b/MAINTAINERS index 0e64787aa..766847ad2 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -12573,6 +12573,13 @@ S: Maintained F: Documentation/devicetree/bindings/sound/max9860.txt F: sound/soc/codecs/max9860.* +MAX14001 IIO ADC DRIVER +M: Kim Seer Paller <kimseer.paller@analog.com> +L: linux-iio@vger.kernel.org +S: Supported +W: https://ez.analog.com/linux-software-drivers +F: Documentation/devicetree/bindings/iio/dac/adi,max14001.yaml + MAXBOTIX ULTRASONIC RANGER IIO DRIVER M: Andreas Klinger <ak@it-klinger.de> L: linux-iio@vger.kernel.org
Add bindings for MAX14001. The MAX14001 is configurable, isolated 10-bit ADCs for multi-range binary inputs. Signed-off-by: Kim Seer Paller <kimseer.paller@analog.com> --- .../bindings/iio/adc/adi,max14001.yaml | 83 +++++++++++++++++++ MAINTAINERS | 7 ++ 2 files changed, 90 insertions(+) create mode 100644 Documentation/devicetree/bindings/iio/adc/adi,max14001.yaml