lkml.org 
[lkml]   [2021]   [Jul]   [10]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH v2 4/4] dt-bindings: iio: adc: ad7949: add adi,reference-source
    On Fri,  9 Jul 2021 11:58:56 -0400
    Liam Beguin <liambeguin@gmail.com> wrote:

    > From: Liam Beguin <lvb@xiphos.com>
    >
    > Add bindings documentation for the adi,reference-source property.
    > This property is required to properly configure the ADC sample request
    > based on which reference source should be used for the calculation.

    Should this be per channel? That will effect some of what I say below...

    >
    > Signed-off-by: Liam Beguin <lvb@xiphos.com>
    > ---
    > .../bindings/iio/adc/adi,ad7949.yaml | 21 +++++++++++++++++++
    > 1 file changed, 21 insertions(+)
    >
    > diff --git a/Documentation/devicetree/bindings/iio/adc/adi,ad7949.yaml b/Documentation/devicetree/bindings/iio/adc/adi,ad7949.yaml
    > index 9b56bd4d5510..eae3121cad01 100644
    > --- a/Documentation/devicetree/bindings/iio/adc/adi,ad7949.yaml
    > +++ b/Documentation/devicetree/bindings/iio/adc/adi,ad7949.yaml
    > @@ -35,6 +35,27 @@ properties:
    > "#io-channel-cells":
    > const: 1
    >
    > + adi,reference-select:

    This is one field in the register, but it's not one thing, so lets break it up
    in DT. We should do this both to make for more readable dts files and to
    enforce the requirements on regulators...

    > + description: |
    > + Select the reference voltage source to use when converting samples.
    > + Acceptable values are:
    > + - 0: Internal reference and temperature sensor enabled.
    > + Vref=2.5V, buffered output
    > + - 1: Internal reference and temperature sensor enabled.
    > + Vref=4.096V, buffered output
    > + - 2: Use external reference, temperature sensor enabled.
    > + Internal buffer disabled
    > + - 3: Use external reference, internal buffer and temperature sensor
    > + enabled.
    > + - 6: Use external reference, internal buffer and temperature sensor
    > + disabled.
    > + - 7: Use external reference, internal buffer enabled.
    > + Internal reference and temperature sensor disabled.

    So question 1 is whether to use an external or internal reference.
    Normally we'd make the coarse decision of whether to use an external reference
    by whether there is a regulator provided. That won't work so well if we make
    this per channel.

    Question 2, assuming internal reference, what voltage? Those should take
    an actual voltage (probably in mV and match against an enum of the two possible values).
    Binding should check to make sure this isn't specified as well as saying we
    are using an external refernce.

    Question 3, assuming external reference, is temperature sensor enabled?
    - actually dumb question, but why would anyone not want this enabled? Maybe turn it
    off in runtime pm, but in general if you've fitted a chip with a temperature sensor
    you at least sometimes want to measure temperature! So my gut feeling is don't
    allow this to be controlled (effectively drop cases 6 and 7 above as being
    unlikely to be of interest to anyone)

    Question 4, Is the internal buffer enabled when using and external reference.
    This one is interesting. We could just expose it in general, but I wonder
    if we can do something that reflects how it is used. From the various figures in
    the datasheet this seems to be coupled to whether the external reference is on
    pin REF_IN or pin REF. If that's the case can we have two optional regs only
    one of which should be supplied? However, this gets more fiddly because
    the default right now is vref-supply actually being connected to the vrefin connection.
    That's annoying as it stops us using the obvious naming...
    Hence I think we can have
    vref-supply (actually connected to vrefin) and vref-unbuffered-supply



    > +
    > + $ref: /schemas/types.yaml#/definitions/uint32
    > + enum: [0, 1, 2, 3, 6, 7]
    > + default: 7
    > +
    > required:
    > - compatible
    > - reg

    \
     
     \ /
      Last update: 2021-07-10 18:58    [W:2.334 / U:0.560 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site