Linux-IIO Archive on lore.kernel.org
 help / color / Atom feed
From: Jonathan Cameron <jic23@kernel.org>
To: Adam Michaelis <adam.michaelis@rockwellcollins.com>
Cc: linux-iio@vger.kernel.org, lars@metafoo.de,
	michael.hennerich@analog.com, knaack.h@gmx.de, pmeerw@pmeerw.net,
	robh+dt@kernel.org, mark.rutland@arm.com,
	charles-antoine.couret@essensium.com, devicetree@vger.kernel.org,
	brandon.maier@rockwellcollins.com,
	clayton.shotwell@rockwellcollins.com
Subject: Re: [PATCH 2/6] dt-bindings: iio: ad7949: Add adi,reference-select
Date: Sun, 5 May 2019 13:22:34 +0100
Message-ID: <20190505132234.313b78e9@archlinux> (raw)
In-Reply-To: <1556745423-11368-2-git-send-email-adam.michaelis@rockwellcollins.com>

On Wed,  1 May 2019 16:16:59 -0500
Adam Michaelis <adam.michaelis@rockwellcollins.com> wrote:

> Adding optional parameter to AD7949 to specify the source for the
> reference voltage signal. Default value is maintaned with option '6' to
> match previous version of driver.
> 
> Signed-off-by: Adam Michaelis <adam.michaelis@rockwellcollins.com>
> ---
>  .../devicetree/bindings/iio/adc/ad7949.txt         | 22 ++++++++++++++++++++--
>  1 file changed, 20 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/iio/adc/ad7949.txt b/Documentation/devicetree/bindings/iio/adc/ad7949.txt
> index c7f5057356b1..14ee9a2cb2a5 100644
> --- a/Documentation/devicetree/bindings/iio/adc/ad7949.txt
> +++ b/Documentation/devicetree/bindings/iio/adc/ad7949.txt
> @@ -6,11 +6,29 @@ Required properties:
>  	* "adi,ad7682"
>  	* "adi,ad7689"
>   - reg: spi chip select number for the device
> - - vref-supply: The regulator supply for ADC reference voltage
>  
> -Example:
> +Optional properties:
> + - adi,reference-select: Select the reference voltage source to use
> + when converting the input voltages. Valid values are:
So my immediate thought here is we are mapping one binding to several
different things. Some of which are definitely better described in other
ways.

So let us break it down:

Internal vs external.
- External should require a regulator.  If the regulator is there, normal
assumption would be you want to use it.

Which internal reference?  Hmm. This would be incompatible with the external
regulator and I'd expect the presence of such a regulator to override this.
That does need a new binding.
adi,internal-reference-milivolts = 2500 or 4096.   Much nicer to have
real numbers for someone wondering how it is configured than an enum.

Temperature sensor enabled: Why is this a devicetree question rather than
a runtime decision?

Buffer enabled: This needs a custom binding
adi,external-reference-buffer-enable or something like that?

Makes for a more consistent binding where some elements can be common
across similar devices.  It would be good to see if similar bindings
already exist.  Potentially tings like the reference-buffer enable
may be worth making standard ADC properties rather than device
specific.

Thanks,

Jonathan

	
> +   0: Internal 2.5V reference; temperature sensor enabled
> +   1: Internal 4.096V reference; temperature sensor enabled
> +   2: External reference, temperature sensor enabled, no buffer
> +   3: External reference, temperature sensor enabled, buffer enabled
> +   6: External reference, temperature sensor disabled, no buffer
> +   7: External reference, temperature sensor disabled, buffer enabled
> + - vref-supply: The regulator supply for ADC reference voltage. Required
> + if external reference selected by 'adi,reference-select'.
> +
> +Examples:
>  adc@0 {
>  	compatible = "adi,ad7949";
>  	reg = <0>;
> +	adi,reference-select = <2>;
>  	vref-supply = <&vdd_supply>;
>  };
> +
> +adc@0 {
> +	compatible = "adi,ad7949";
> +	reg = <0>;
> +	adi,reference-select = <0>;
> +};


  reply index

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-05-01 21:16 [PATCH 1/6] iio: ad7949: Support internal Vref Adam Michaelis
2019-05-01 21:16 ` [PATCH 2/6] dt-bindings: iio: ad7949: Add adi,reference-select Adam Michaelis
2019-05-05 12:22   ` Jonathan Cameron [this message]
2019-05-06 19:28     ` Adam Michaelis
2019-05-07 18:21     ` Adam Michaelis
2019-05-11 10:38       ` Jonathan Cameron
2019-05-13 14:51         ` [External] " Adam Michaelis
2019-05-01 21:17 ` [PATCH 3/6] iio: ad7949: Support configuration read-back Adam Michaelis
2019-05-01 21:17 ` [PATCH 4/6] dt-bindings: iio: ad7949: Add cfg-readback option Adam Michaelis
2019-05-01 21:17 ` [PATCH 5/6] iio: ad7949: Fix SPI interfacing for 14-bit messages Adam Michaelis
2019-05-01 21:17 ` [PATCH 6/6] iio: ad7949: Fix dummy read cycle placement Adam Michaelis
2019-05-02  6:30 ` [PATCH 1/6] iio: ad7949: Support internal Vref Couret Charles-Antoine
  -- strict thread matches above, loose matches on Subject: below --
2019-05-01 19:30 [PATCH 2/6] dt-bindings: iio: ad7949: Add adi,reference-select Adam Michaelis

Reply instructions:

You may reply publically 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=20190505132234.313b78e9@archlinux \
    --to=jic23@kernel.org \
    --cc=adam.michaelis@rockwellcollins.com \
    --cc=brandon.maier@rockwellcollins.com \
    --cc=charles-antoine.couret@essensium.com \
    --cc=clayton.shotwell@rockwellcollins.com \
    --cc=devicetree@vger.kernel.org \
    --cc=knaack.h@gmx.de \
    --cc=lars@metafoo.de \
    --cc=linux-iio@vger.kernel.org \
    --cc=mark.rutland@arm.com \
    --cc=michael.hennerich@analog.com \
    --cc=pmeerw@pmeerw.net \
    --cc=robh+dt@kernel.org \
    /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

Linux-IIO Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-iio/0 linux-iio/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-iio linux-iio/ https://lore.kernel.org/linux-iio \
		linux-iio@vger.kernel.org linux-iio@archiver.kernel.org
	public-inbox-index linux-iio

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-iio


AGPL code for this site: git clone https://public-inbox.org/ public-inbox