LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Jonathan Cameron <jic23@kernel.org>
To: Liam Beguin <liambeguin@gmail.com>
Cc: lars@metafoo.de, Michael.Hennerich@analog.com,
	charles-antoine.couret@essensium.com, Nuno.Sa@analog.com,
	linux-kernel@vger.kernel.org, linux-iio@vger.kernel.org,
	devicetree@vger.kernel.org, robh+dt@kernel.org
Subject: Re: [PATCH v2 4/4] dt-bindings: iio: adc: ad7949: add adi,reference-source
Date: Sat, 10 Jul 2021 18:00:01 +0100	[thread overview]
Message-ID: <20210710180001.051f7367@jic23-huawei> (raw)
In-Reply-To: <20210709155856.1732245-5-liambeguin@gmail.com>

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


  reply	other threads:[~2021-07-10 16:57 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-07-09 15:58 [PATCH v1 0/4] AD7949 Fixes Liam Beguin
2021-07-09 15:58 ` [PATCH v2 1/4] iio: adc: ad7949: define and use bitfield names Liam Beguin
2021-07-10 16:08   ` Jonathan Cameron
2021-07-10 18:17     ` Liam Beguin
2021-07-09 15:58 ` [PATCH v2 2/4] iio: adc: ad7949: fix spi messages on non 14-bit controllers Liam Beguin
2021-07-10 16:39   ` Jonathan Cameron
2021-07-11 19:58     ` Liam Beguin
2021-07-12 10:12       ` Jonathan Cameron
2021-07-09 15:58 ` [PATCH v2 3/4] iio: adc: ad7949: add support for internal vref Liam Beguin
2021-07-10 17:03   ` Jonathan Cameron
2021-07-09 15:58 ` [PATCH v2 4/4] dt-bindings: iio: adc: ad7949: add adi,reference-source Liam Beguin
2021-07-10 17:00   ` Jonathan Cameron [this message]
2021-07-12 17:05     ` Liam Beguin
2021-07-13  9:16       ` Jonathan Cameron

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210710180001.051f7367@jic23-huawei \
    --to=jic23@kernel.org \
    --cc=Michael.Hennerich@analog.com \
    --cc=Nuno.Sa@analog.com \
    --cc=charles-antoine.couret@essensium.com \
    --cc=devicetree@vger.kernel.org \
    --cc=lars@metafoo.de \
    --cc=liambeguin@gmail.com \
    --cc=linux-iio@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=robh+dt@kernel.org \
    --subject='Re: [PATCH v2 4/4] dt-bindings: iio: adc: ad7949: add adi,reference-source' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).