Re: [PATCH v2 1/2] dt-bindings: iio: dac: add docs for ad8460

[Krzysztof Kozlowski <krzk@kernel.org>]

On 17/08/2024 13:18, Tinaco, Mariel wrote:
>>> +  clocks:
>>
>> maxItems: 1
>> and drop description (or use items: - description, but then do not
>> repeat redundant parts)
>>
> 
> Simplified the description for this item
> 
>   refio-1p2v-supply:
>     description: Reference voltage to adjust full scale output span
>     maxItems: 1
> 
> Should I put, "maxItems: 1" in the other vrefs as well?

No, why do you change supply? My comment was under clocks. Comments are
always under specific part of code which is being discussed.

> 
>>> +    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.
>>
> 
> You're right. I should have put all the possible values. I populated it with common
> Values found in the datasheet
> 
>   adi,range-microvolt:
>     description: Voltage output range specified as <minimum, maximum>
>     oneOf:
>       - items:
>           - enum: [0, -10000000, -20000000, -30000000, -40000000, -55000000]
>           - enum: [10000000, 20000000, 30000000, 40000000, 55000000]
> 
>>> +      - 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.
>>
> 
> Lifted from gpio-consumer-common yaml. Mapped to corresponding pins
> 
>   wakeup-gpios:
>     description: Corresponds to SDN_RESET pin. To exit shutdown
>       or sleep mode, pulse SDN_RESET HIGH, then leave LOW.

That's not a wakeup-gpio. I think my comment was imprecise. You have
something like three reset/shutdown GPIOs, so pick from
gpio-consumer-common the reset and shutdown. Remaining one could stay as
in your original code.

>     maxItems: 1
> 
>   reset-gpios:
>     description: Manual Power On Reset (POR). Pull this GPIO pin
>       LOW and then HIGH to reset all digital registers to default
>     maxItems: 1
> 
>   shutdown-gpios:
>     description: Corresponds to SDN_IO pin. Shutdown may be
>       initiated by the user, by pulsing SDN_IO high. To exit shutdown,
>       pulse SDN_IO low, then float.
>     maxItems: 1
> 


Best regards,
Krzysztof