Re: [PATCH v2 1/9] dt-bindings: adc: ad7173: add support for ad411x

[Jonathan Cameron <jic23@kernel.org>]

> >  
> > >> +
> > >> +          There are special values that can be selected besides the voltage
> > >> +          analog inputs:
> > >> +            21: REF+
> > >> +            22: REF−
> > >> +          Supported only by AD7172-2, AD7172-4, AD7175-2, AD7175-8, AD7177-2:
> > >> +            19: ((AVDD1 − AVSS)/5)+
> > >> +            20: ((AVDD1 − AVSS)/5)−
> > >> +          Supported only by AD4111, AD4112:
> > >> +            12: IIN3+
> > >> +            11: IIN3−
> > >> +            13: IIN2+
> > >> +            10: IIN2−
> > >> +            14: IIN1+
> > >> +             9: IIN1−
> > >> +            15: IIN0+
> > >> +             8: IIN0−  
> > >
> > > I just made a late reply on v1 where Jonathan suggested that the
> > > current inputs are differential with a similar comment to this:
> > >
> > > It doesn't seem to me like current inputs are differential if they are
> > > only measuring one current. They take 2 pins because you need a way
> > > for current to come in and go back out, but the datasheet calls them
> > > single-ended inputs.
> > >  
> > It seemed to me that the conclusion that we arrived to was to expose the
> > precise pins that are used in the conversion and document the selection.
> >
> > Yes, it is a single-ended channel. So revert to the way it was in V1 using
> > single-channel?  
> 
> I'd like to hear Jonathan's opinion on this one.

Yes.  I think we rather went off on a false tangent on this.  Any current input
using a shunt is of this form and so far we've treated them as single ended :(
e.g. pac1934 does this for it's sense inputs where there is an external
'sense resitor'.  Similar for the stand alone afe driver for a current sense shunt
which is used with a differential voltage input, but presents a single ended
current measurement.

Sorry for misguiding things :( 

At the end of this, I don't suppose anyone fancies writing up some notes
on how to describe different types of channel?

> 
> >  
> > >> +
> > >>          items:
> > >>            minimum: 0
> > >>            maximum: 31
> > >> @@ -154,6 +195,23 @@ patternProperties:
> > >>            - avdd
> > >>          default: refout-avss
> > >>
> > >> +      adi,current-channel:
> > >> +        description: |
> > >> +          Signal that the selected inputs are current channels.
> > >> +          Only available on AD4111 and AD4112.
> > >> +        type: boolean

I'm lost. Why do we need this one?  Is the channel selection not sufficient
to tell us this?

> > >> +
> > >> +      adi,channel-type:
> > >> +        description:
> > >> +          Used to differentiate between different channel types as the device
> > >> +           register configurations are the same for all usage types.
> > >> +        $ref: /schemas/types.yaml#/definitions/string
> > >> +        enum:
> > >> +          - single-ended
> > >> +          - pseudo-differential
> > >> +          - differential
> > >> +        default: differential
> > >> +  
> > >
> > > As suggested above, we should soon have diff-channels and
> > > single-channel to differentiate between (fully) differential and
> > > single-ended. Do we actually need to differentiate between
> > > single-ended and pseudo-differential though?
> > >  
> > Not really, so just a bool differential flag? (this seems weird as we have diff-channels)  
> 
> Or we need to change the proposed single-channel property to allow two
> inputs. I guess we'll see what Johnathan has to say.

I think single ended fits better for the current channels with just one
parameter.