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

[Jonathan Cameron <Jonathan.Cameron@xxxxxxxxxx>]

On Wed, 17 Jan 2024 14:30:33 +0200
Ceclan Dumitru <mitrutzceclan@xxxxxxxxx> wrote:

> 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

I'm not sure on that given it's fairly common to do evil things with
Not Gates to implement cheap level converters.  That tends to break
doing this entirely in driver.

However, I'm fine with whatever the current convention on this is.
We can always deal with inverters when hardware turns up that is
doing that.

> >> +  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.

See other thread - I don't think that works for single ended where we should
probably provide an offset.
> 
> ...
> 
> >> +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

Nasty.  However, a valid use case would be to use a single channel
at a time, which case that statement doesn't matter.  We have other
devices that only support onehot channel sequencing - that would work
here.

>