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: <71cbe674-b232-4327-929b-351630907540@kernel.org>
Date: Tue, 30 Jul 2024 08:15:01 +0200
From: Krzysztof Kozlowski <krzk@...nel.org>
To: Mariel Tinaco <Mariel.Tinaco@...log.com>, linux-iio@...r.kernel.org,
 devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
 Jonathan Cameron <jic23@...nel.org>, Lars-Peter Clausen <lars@...afoo.de>,
 Rob Herring <robh@...nel.org>, Krzysztof Kozlowski <krzk+dt@...nel.org>,
 Michael Hennerich <Michael.Hennerich@...log.com>,
 Conor Dooley <conor+dt@...nel.org>,
 Marcelo Schmitt <marcelo.schmitt1@...il.com>,
 Dimitri Fedrau <dima.fedrau@...il.com>, David Lechner
 <dlechner@...libre.com>, Nuno Sá <noname.nuno@...il.com>
Subject: Re: [PATCH v2 1/2] dt-bindings: iio: dac: add docs for ad8460

On 30/07/2024 05:05, Mariel Tinaco wrote:
> This adds the bindings documentation for the 14-bit
> High Voltage, High Current, Waveform Generator
> Digital-to-Analog converter.
> 
> Signed-off-by: Mariel Tinaco <Mariel.Tinaco@...log.com>

> +
> +  refio-1p2v-supply:
> +    description: Drive voltage in the range of 1.2V maximum to as low as
> +      low as 0.12V through the REF_IO pin to adjust full scale output span
> +
> +  clocks:

maxItems: 1
and drop description (or use items: - description, but then do not
repeat redundant parts)

> +    description: The clock for the DAC. This is the sync clock
> +
> +  adi,rset-ohms:
> +    description: Specify value of external resistor connected to FS_ADJ pin
> +      to establish internal HVDAC's reference current I_REF
> +    default: 2000
> +    minimum: 2000
> +    maximum: 20000
> +
> +  adi,range-microvolt:
> +    description: |
> +      Voltage output range specified as <minimum, maximum>
> +    oneOf:

Not an oneOf.

> +      - items:
> +          - const: -40000000
> +          - const: 40000000

Why do you need this property if this cannot be anything else? Drop.

> +
> +  adi,range-microamp:
> +    description: |

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

> +      Current output range specified as <minimum, maximum>
> +    oneOf:
> +      - items:
> +          - const: 0
> +          - const: 50000
> +      - items:
> +          - const: -50000
> +          - const: 50000
> +
> +  adi,temp-max-millicelsius:
> +    description: Overtemperature threshold
> +    default: 50000
> +    minimum: 20000
> +    maximum: 150000
> +
> +  sdn-reset-gpios:

reset-gpios or shutdown-gpios or anything from gpio-consumer-common
which is not deprecated.

> +    description: GPIO spec for the SHUTDOWN RESET pin. As the line is active high,

Do not repeat the obvious or redundant parts. There is no point in
saying that "GPIO spec is a GPIO spec for ...". It cannot be anything
else than GPIO spec. Instead say something useful. It's confusing to
have two reset pins, so explain what is the purpose of this pin.

> +      it should be marked GPIO_ACTIVE_HIGH.

Drop last part "it should be marked", because it is clearly incorrect.
Different board designs can have it differently.


> +    maxItems: 1
> +
> +  reset-gpios:
> +    description: GPIO spec for the RESET pin. As the line is active low, it
> +      should be marked GPIO_ACTIVE_LOW.

Again, marking it always as active low is not correct. It is enough to
say that line is active low.

> +    maxItems: 1
> +
> +  sdn-io-gpios:
> +    description: GPIO spec for the SHUTDOWN INPUT/OUTPUT pin. As the line is
> +      active high, it should be marked GPIO_ACTIVE_HIGH.

What's the purpose?

> +    maxItems: 1
> +
> +required:
> +  - compatible
> +  - reg
> +  - clocks
> +
> +allOf:
> +  - $ref: /schemas/spi/spi-peripheral-props.yaml#
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/gpio/gpio.h>
> +
> +    spi {
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +
> +        dac@0 {
> +            compatible = "adi,ad8460";
> +            reg = <0>;
> +            spi-max-frequency = <8000000>;
> +            adi,rset-ohms = <2000>;
> +            adi,range-microvolt = <(-40000000) 40000000>;
> +            adi,range-microamp = <0 50000>;
> +            adi,temp-max-millicelsius = <50000>;

Custom properties go to the end. See DTS coding style.

> +
> +            dmas = <&tx_dma 0>;
> +            dma-names = "tx";


Best regards,
Krzysztof


Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ