linux-kernel - Re: [PATCH 3/9] dt-bindings: iio: adc: adi,ad7606: improve descriptions

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]

Date: Sun, 23 Jun 2024 16:28:27 +0100
From: Jonathan Cameron <jic23@...nel.org>
To: Guillaume Stols <gstols@...libre.com>
Cc: Lars-Peter Clausen <lars@...afoo.de>, Michael Hennerich
 <Michael.Hennerich@...log.com>, Rob Herring <robh@...nel.org>, Krzysztof
 Kozlowski <krzk+dt@...nel.org>, Conor Dooley <conor+dt@...nel.org>,
 Beniamin Bia <beniamin.bia@...log.com>, Stefan Popa
 <stefan.popa@...log.com>, linux-iio@...r.kernel.org,
 linux-kernel@...r.kernel.org, linux-fbdev@...r.kernel.org,
 devicetree@...r.kernel.org, Jonathan Cameron <Jonathan.Cameron@...wei.com>,
 jstephan@...libre.com, dlechner@...libre.com
Subject: Re: [PATCH 3/9] dt-bindings: iio: adc: adi,ad7606: improve
 descriptions

On Tue, 18 Jun 2024 14:02:35 +0000
Guillaume Stols <gstols@...libre.com> wrote:

> Reword a few descriptions, and normalize the text width to 80 characters.

Don't rewrap text in a patch that does anything else as those real
changes are hidden.

Even if you are changing the text, minimise rewraps to those necessary
to avoid lines getting too long and fix them up in a follow up patch
that just rewraps.

I think most of the changes are fine, but it's really hard to spot
the real changes in here!

> 
> Signed-off-by: Guillaume Stols <gstols@...libre.com>
> ---
>  .../devicetree/bindings/iio/adc/adi,ad7606.yaml    | 61 ++++++++++++----------
>  1 file changed, 34 insertions(+), 27 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/iio/adc/adi,ad7606.yaml b/Documentation/devicetree/bindings/iio/adc/adi,ad7606.yaml
> index 00fdaed11cbd..80866940123c 100644
> --- a/Documentation/devicetree/bindings/iio/adc/adi,ad7606.yaml
> +++ b/Documentation/devicetree/bindings/iio/adc/adi,ad7606.yaml
> @@ -36,64 +36,71 @@ properties:
>    avcc-supply: true
>  
>    interrupts:
> +    description:
> +      The BUSY pin falling edge indicates that the conversion is over, and thus
> +      new data is available.
>      maxItems: 1
>  
>    adi,conversion-start-gpios:
>      description:
> -      Must be the device tree identifier of the CONVST pin.
> -      This logic input is used to initiate conversions on the analog
> -      input channels. As the line is active high, it should be marked
> -      GPIO_ACTIVE_HIGH.
> +      Must be the device tree identifier of the CONVST pin(s). This logic input

Why plural when it has maxitems 1?

> +      is used to initiate conversions on the analog input channels. As the line
> +      is active high, it should be marked GPIO_ACTIVE_HIGH.

If you drop the plural change don't rewrap this in v2, it is unnecessary noise
that takes away from the real improvements.
>      maxItems: 1
>  
>    reset-gpios:
>      description:
> -      Must be the device tree identifier of the RESET pin. If specified,
> -      it will be asserted during driver probe. As the line is active high,
> -      it should be marked GPIO_ACTIVE_HIGH.
> +      Must be the device tree identifier of the RESET pin. If specified, it will
> +      be asserted during driver probe. On the AD7606x, as the line is active
> +      high, it should be marked GPIO_ACTIVE_HIGH. On the AD7616, as the line is
> +      active low, it should be marked GPIO_ACTIVE_LOW.
>      maxItems: 1


>  
>    adi,range-gpios:
>      description:
> -      Must be the device tree identifier of the RANGE pin. The polarity on
> -      this pin determines the input range of the analog input channels. If
> -      this pin is tied to a logic high, the analog input range is ±10V for
> -      all channels. If this pin is tied to a logic low, the analog input range
> +      Must be the device tree identifier of the RANGE pin. The state on this
> +      pin determines the input range of the analog input channels. If this pin
> +      is tied to a logic high, the analog input range is ±10V for all channels.
> +      On the AD760X, if this pin is tied to a logic low, the analog input range
>        is ±5V for all channels. As the line is active high, it should be marked
> -      GPIO_ACTIVE_HIGH.
> +      GPIO_ACTIVE_HIGH. On the AD7616, there are 2 pins, and if the 2 pins are
> +      tied to a logic high, software mode is enabled, otherwise one of the 3
> +      possible range values is selected.

With max items 1 how do we have 2?

>      maxItems: 1
>  
>    adi,oversampling-ratio-gpios:
>      description:
> -      Must be the device tree identifier of the over-sampling
> -      mode pins. As the line is active high, it should be marked
> -      GPIO_ACTIVE_HIGH.
> +      Must be the device tree identifier of the over-sampling mode pins. As the
> +      line is active high, it should be marked GPIO_ACTIVE_HIGH. On the AD7606X
> +      parts that support it, if all 3 pins are tied to a logic high, software
> +      mode is enabled.
>      maxItems: 3