Re: [PATCH 1/2] dt-bindings:iio:adc: add max14001 bindings

[Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>]

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