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

[Krzysztof Kozlowski <krzk@kernel.org>]

On 09/09/2024 08:22, Tinaco, Mariel wrote:
> 
> 
>> -----Original Message-----
>> From: Jonathan Cameron <jic23@kernel.org>
>> Sent: Sunday, September 8, 2024 1:02 AM
>> To: Krzysztof Kozlowski <krzk@kernel.org>
>> Cc: Tinaco, Mariel <Mariel.Tinaco@analog.com>; linux-iio@vger.kernel.org;
>> devicetree@vger.kernel.org; linux-kernel@vger.kernel.org; Lars-Peter Clausen
>> <lars@metafoo.de>; Rob Herring <robh@kernel.org>; Krzysztof Kozlowski
>> <krzk+dt@kernel.org>; Hennerich, Michael <Michael.Hennerich@analog.com>;
>> Conor Dooley <conor+dt@kernel.org>; Marcelo Schmitt
>> <marcelo.schmitt1@gmail.com>; Dimitri Fedrau <dima.fedrau@gmail.com>;
>> David Lechner <dlechner@baylibre.com>; Nuno Sá
>> <noname.nuno@gmail.com>
>> Subject: Re: [PATCH v3 1/2] dt-bindings: iio: dac: add docs for ad8460
>>
>> [External]
>>
>> On Wed, 4 Sep 2024 08:20:53 +0200
>> Krzysztof Kozlowski <krzk@kernel.org> wrote:
>>
>>> On Wed, Sep 04, 2024 at 10:30:39AM +0800, Mariel Tinaco wrote:
>>>> This adds the bindings documentation for the 14-bit
>>>
>>> Please do not use "This commit/patch/change", but imperative mood. See
>>> longer explanation here:
>>> https://urldefense.com/v3/__https://elixir.bootlin.com/linux/v5.17.1/s
>>> ource/Documentation/process/submitting-
>> patches.rst*L95__;Iw!!A3Ni8CS0y
>>> 2Y!7lj0hq-U2ClkGNYfHqjR3-k-
>> ea6TFUFsgEYQokkU95K6TXPHIPU33VxQcl_iH_etJ4k
>>> pbPEV39dP1oAd$
>>>
>>>> High Voltage, High Current, Waveform Generator Digital-to-Analog
>>>> converter.
>>>>
>>>> Signed-off-by: Mariel Tinaco <Mariel.Tinaco@analog.com>
>>>> ---
>>>>  .../bindings/iio/dac/adi,ad8460.yaml          | 154 ++++++++++++++++++
>>>>  MAINTAINERS                                   |   7 +
>>>>  2 files changed, 161 insertions(+)
>>>>  create mode 100644
>>>> Documentation/devicetree/bindings/iio/dac/adi,ad8460.yaml
>>>
>>>> +  adi,range-microvolt:
>>>> +    description: Voltage output range specified as <minimum, maximum>
>>>> +    oneOf:
>>>
>>> This oneOf does not make sense. There is only one condition. Drop.
>>>
>>>> +      - items:
>>>> +          - enum: [0, -10000000, -20000000, -30000000, -40000000, -
>> 55000000]
>>>> +          - enum: [10000000, 20000000, 30000000, 40000000,
>>>> + 55000000]
>>>
>>> What's the default? It's not a required property.
>>>
>>>> +
>>>> +  adi,range-microamp:
>>>> +    description: Current output range specified as <minimum, maximum>
>>>> +    oneOf:
>>>> +      - items:
>>>> +          - enum: [-50000, -100000, -300000, -500000, -1000000]
>>>
>>> I don't understand why 0 is not listed here.
>>
>> I'm not sure why it is a list at all. Seems like the hardware allows a continuous
>> value so this should just specify max and min.
>>
> 
> That's right, the values can be flexible but only at a certain range. 
> The first element of the array should only be in the negative range, while
> The second element of the array should only be in the positive range.
> 
> Is there a way to do this with the max and min attribute?
> 
> Items:
> 	Item 1
> 		min: -10000
> 		max: 0
> 	item 2
> 		min: 0
> 		max: 10000 

items:
  - minimum: -10000
    maximum: 0
  - minimum: 0
    maximum: 10000

should work, try.

Best regards,
Krzysztof