lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite for Android: free password hash cracker in your pocket
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:   Sun, 6 Nov 2022 15:46:34 +0000
From:   Jonathan Cameron <jic23@...nel.org>
To:     Cosmin Tanislav <demonsingur@...il.com>
Cc:     Cosmin Tanislav <cosmin.tanislav@...log.com>,
        Lars-Peter Clausen <lars@...afoo.de>,
        Michael Hennerich <Michael.Hennerich@...log.com>,
        Rob Herring <robh+dt@...nel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
        Linus Walleij <linus.walleij@...aro.org>,
        William Breathitt Gray <william.gray@...aro.org>,
        linux-iio@...r.kernel.org, devicetree@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 1/2] dt-bindings: iio: addac: add AD74115

On Thu,  3 Nov 2022 11:44:35 +0200
Cosmin Tanislav <demonsingur@...il.com> wrote:

> From: Cosmin Tanislav <cosmin.tanislav@...log.com>
> 
> The AD74115H is a single-channel, software-configurable, input and
> output device for industrial control applications. The AD74115H
> provides a wide range of use cases, integrated on a single chip.
> 
> These use cases include analog output, analog input, digital output,
> digital input, resistance temperature detector (RTD), and thermocouple
> measurement capability. The AD74115H also has an integrated HART modem.
> 
> A serial peripheral interface (SPI) is used to handle all communications
> to the device, including communications with the HART modem. The digital
> input and digital outputs can be accessed via the SPI or the
> general-purpose input and output (GPIO) pins to support higher
> speed data rates.
> 
> The device features a 16-bit, sigma-delta analog-to-digital converter
> (ADC) and a 14-bit digital-to-analog converter (DAC).
> The AD74115H contains a high accuracy 2.5 V on-chip reference that can
> be used as the DAC and ADC reference.
> 
> Signed-off-by: Cosmin Tanislav <cosmin.tanislav@...log.com>

Hi Cosmin,

A few questions inline.  Complex device so I'll doubt we'll ever get this
binding to be as tidy as for simpler devices.  Hence most of the below are
suggestions rather than requirements from me.

Jonathan

> ---
>  .../bindings/iio/addac/adi,ad74115.yaml       | 370 ++++++++++++++++++
>  MAINTAINERS                                   |   7 +
>  2 files changed, 377 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/iio/addac/adi,ad74115.yaml
> 
> diff --git a/Documentation/devicetree/bindings/iio/addac/adi,ad74115.yaml b/Documentation/devicetree/bindings/iio/addac/adi,ad74115.yaml
> new file mode 100644
> index 000000000000..621f11d5c1f3
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/addac/adi,ad74115.yaml
> @@ -0,0 +1,370 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/iio/addac/adi,ad74115.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Analog Devices AD74115H device
> +
> +maintainers:
> +  - Cosmin Tanislav <cosmin.tanislav@...log.com>
> +
> +description: |
> +  The AD74115H is a single-channel software configurable input/output
> +  device for industrial control applications. It contains functionality for
> +  analog output, analog input, digital output, digital input, resistance
> +  temperature detector, and thermocouple measurements integrated into a single
> +  chip solution with an SPI interface. The device features a 16-bit ADC and a
> +  14-bit DAC.
> +
> +    https://www.analog.com/en/products/ad74115h.html
> +
> +properties:
> +  compatible:
> +    enum:
> +      - adi,ad74115h
> +
> +  reg:
> +    maxItems: 1
> +
> +  '#address-cells':
> +    const: 1
> +
> +  '#size-cells':
> +    const: 0

I'm not seeing any child nodes, so why do we need these two?

> +
> +  avdd-supply: true
> +  avcc-supply: true
> +  dvcc-supply: true
> +  aldo1v8-supply: true

aldo1v8 is an output pin. "1.8 V Analog LDO Output. Do not use ALDO1V8 externally."
The associated input is avcc.  Given we shouldn't connect anything to the pin,
we don't want it in the binding docs

> +  dovdd-supply: true
> +  refin-supply: true
> +

...

> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    description: |
> +      Conversion range for ADC conversion 2.
> +      0 - 0V to 12V
> +      1 - -12V to +12V
> +      2 - -2.5V to +2.5V
> +      3 - -2.5V to 0V
> +      4 - 0V to 2.5V
> +      5 - 0V to 0.625V
> +      6 - -104mV to +104mV
> +      7 - 0V to 12V

For a lot of similar cases we handle these numerically to give
a human readable dts.  Is there a strong reason not to do so here (in mv)


> +    minimum: 0
> +    maximum: 7
> +    default: 0
> +
> +  adi,sense-agnd-buffer-lp:
lp is a little ambiguous, given we have a habit of using it for low pass
in filters etc. Perhaps worth spelling these out?
     adi,sens-agnd-buffer-low-power etc?

> +    type: boolean
> +    description: |
> +      Whether to enable low-power buffered mode for the AGND sense pin.
> +
> +  adi,lf-buffer-lp:
> +    type: boolean
> +    description: |
> +      Whether to enable low-power buffered mode for the low-side filtered
> +      sense pin.
> +
> +  adi,hf-buffer-lp:
> +    type: boolean
> +    description: |
> +      Whether to enable low-power buffered mode for the high-side filtered
> +      sense pin.
> +
> +  adi,ext2-buffer-lp:
> +    type: boolean
> +    description: Whether to enable low-power buffered mode for the EXT2 pin.
> +
> +  adi,ext1-buffer-lp:
> +    type: boolean
> +    description: Whether to enable low-power buffered mode for the EXT1 pin.

> +
> +required:
> +  - compatible
> +  - reg
> +  - spi-cpol
> +  - avdd-supply
> +
> +allOf:
> +  - $ref: /schemas/spi/spi-peripheral-props.yaml#
> +  - if:
> +      properties:
> +        adi,digital-input-sink-range-high: true
> +    then:
> +      properties:
> +        adi,digital-input-sink-microamp:
> +          maximum: 7400
> +
> +additionalProperties: false

Does this need to be unevalutatedProperties to allow
for the extra ones in spi-periphera-props.yaml?

> +

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ