linux-kernel - Re: [PATCH v2 1/2] dt-bindings: iio: adc: Add TI ADS1120 binding

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: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20251110-curly-ultra-coucal-d7cc32@kuoka>
Date: Mon, 10 Nov 2025 08:59:36 +0100
From: Krzysztof Kozlowski <krzk@...nel.org>
To: Ajith Anandhan <ajithanandhan0406@...il.com>
Cc: jic23@...nel.org, dlechner@...libre.com, nuno.sa@...log.com, 
	andy@...nel.org, robh@...nel.org, krzk+dt@...nel.org, conor+dt@...nel.org, 
	linux-iio@...r.kernel.org, devicetree@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 1/2] dt-bindings: iio: adc: Add TI ADS1120 binding

On Sun, Nov 09, 2025 at 07:41:18PM +0530, Ajith Anandhan wrote:
> Add device tree binding documentation for the Texas Instruments
> ADS1120.

A nit, subject: drop second/last, redundant "binding". The
"dt-bindings" prefix is already stating that these are bindings.
See also:
https://elixir.bootlin.com/linux/v6.17-rc3/source/Documentation/devicetree/bindings/submitting-patches.rst#L18

> 
> The binding defines required properties like compatible, reg, and
> SPI configuration parameters.

Drop sentence, completely redundant. We can read the diff.

> 
> Signed-off-by: Ajith Anandhan <ajithanandhan0406@...il.com>
> ---
>  .../bindings/iio/adc/ti,ads1120.yaml          | 109 ++++++++++++++++++
>  1 file changed, 109 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/iio/adc/ti,ads1120.yaml
> 
> diff --git a/Documentation/devicetree/bindings/iio/adc/ti,ads1120.yaml b/Documentation/devicetree/bindings/iio/adc/ti,ads1120.yaml
> new file mode 100644
> index 000000000..2449094af
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/adc/ti,ads1120.yaml
> @@ -0,0 +1,109 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/iio/adc/ti,ads1120.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Texas Instruments ADS1120 4-channel, 16-bit, 2kSPS ADC
> +
> +maintainers:
> +  - Ajith Anandhan <ajithanandhan0406@...il.com>
> +
> +description: |
> +  The ADS1120 is a precision, 16-bit, analog-to-digital converter (ADC)
> +  that features two differential or four single-ended inputs through a
> +  flexible input multiplexer.
> +
> +  Datasheet: https://www.ti.com/lit/gpn/ads1120
> +
> +properties:
> +  compatible:
> +    const: ti,ads1120
> +
> +  reg:
> +    maxItems: 1
> +
> +  interrupts:
> +    minItems: 1
> +    maxItems: 2
> +    description: |

Do not need '|' unless you need to preserve formatting.

> +      Interrupts for the DRDY (data ready) pin(s). The device can output
> +      DRDY on a dedicated pin or multiplex it with DOUT. If both pins are
> +      wired, both interrupts can be specified.
> +
> +  interrupt-names:
> +    minItems: 1
> +    maxItems: 2
> +    items:
> +      enum:
> +        - drdy
> +        - dout

No, this cannot be flexible so much. Look at existing examples how this
is supposed to be written.

minItems: 1
items:
 - enum
 - const

> +
> +  avdd-supply:
> +    description: |


Do not need '|' unless you need to preserve formatting.


> +      Analog power supply, typically 2.3V to 5.5V.
> +
> +  vref-supply:
> +    description: |

Do not need '|' unless you need to preserve formatting.

> +      Optional external voltage reference. Can be connected to either
> +      REFP0/REFN0 or REFP1/REFN1 pins. If not supplied, the internal
> +      2.048V reference is used.
> +
> +  ti,avdd-is-ref:
> +    type: boolean
> +    description: |

Do not need '|' unless you need to preserve formatting.

> +      If present, indicates that the AVDD supply voltage is of sufficient
> +      quality and stability to be used as the voltage reference instead of
> +      the internal reference. This allows the driver to select AVDD as the
> +      reference source for potentially better performance.

Driver? Aren't you just describing the case when AVDD is connected to
REF pins?

> +
> +  clocks:
> +    maxItems: 1
> +    description: |

Do not need '|' unless you need to preserve formatting.

> +      Optional external clock input. If not specified, the internal

Drop first sentence, completely redundant. Schema defines what is
optional, not your description. Can it be something else than external
clock input? No, it cannot. Please write informative descriptions, not
just inflate the text.

> +      oscillator is used.
> +
> +  spi-max-frequency:
> +    maximum: 4000000
> +
> +  spi-cpha: true
> +
> +  "#io-channel-cells":
> +    const: 1
> +
> +required:
> +  - compatible
> +  - reg

avdd-suply

> +
> +allOf:
> +  - $ref: /schemas/spi/spi-peripheral-props.yaml#
> +
> +unevaluatedProperties: false

Best regards,
Krzysztof