Re: [PATCH v11 1/2] dt-bindings: adc: add AD7173

[Ceclan Dumitru <mitrutzceclan@xxxxxxxxx>]

On 1/15/24 23:53, David Lechner wrote:
> On Wed, Dec 20, 2023 at 4:48 AM Dumitru Ceclan <mitrutzceclan@xxxxxxxxx> wrote:

...
> 
> According to the timing diagram in the datasheet, SCLK is high during
> idle, so don't we need `spi-cpol: true` here?
> 
> Likewise, data is valid on the trailing SCLK edge, so don't we need
> `spi-cpha: true` here?
> 
> 
V1 Rob Herring suggested that if device is not configurable, driver
should set the spi mode
>> +  gpio-controller:
>> +    description: Marks the device node as a GPIO controller.
>> +
>> +  "#gpio-cells":
>> +    const: 2
>> +    description:
>> +      The first cell is the GPIO number and the second cell specifies
>> +      GPIO flags, as defined in <dt-bindings/gpio/gpio.h>.
>> +
>> +  refin-supply:
>> +    description: external reference supply, can be used as reference for conversion.
> 
> If I'm understanding correctly, this represents both voltage inputs
> REF+ and REF-, correct? The datasheet says "Reference Input Negative
> Terminal. REF− can span from AVSS to AVDD1 − 1 V". It seems like they
> should be separate supplies in case REF- is non-zero. Otherwise, how
> can we know what voltage it is? (same comment applies to refin2.)
> Yes, but in that case, the value of the referenced supply should reflect
that and be equal to (REF+)-(REF-). I'll add to the description this.

...

>> +required:
>> +  - compatible
>> +  - reg
>> +  - interrupts
> 
> Why are interrupts required? What if the pin is not connected?
> 
>From the datasheet, the reading of the conversions seem to be only
interrupt based: "As soon as the next conversion is complete,
the data register is updated; therefore, the period in which to
read the conversion is limited." this paragraph suggests to me that
interrupts are required