Re: [PATCH 5/6] dt-bindings: iio: adc: mcp320x: Update for mcp3550/1/3

2 messages, 2 authors, 2017-08-27 · open the first message on its own page

Re: [PATCH 5/6] dt-bindings: iio: adc: mcp320x: Update for mcp3550/1/3

From: Rob Herring <hidden>
Date: 2017-08-25 19:59:41

On Tue, Aug 22, 2017 at 03:33:00PM +0200, Lukas Wunner wrote:
quoted hunk
All chips supported by this driver clock data out on the falling edge
and latch data in on the rising edge, hence SPI mode (0,0) or (1,1)
must be used.

Furthermore, none of the chips has an internal reference voltage
regulator, so an external supply is always required and needs to be
specified in the device tree lest the IIO "scale" in sysfs cannot be
calculated.

Document these requirements in the device tree binding, together with
the new "microchip,continuous-conversion" property added specifically
for the mcp3550/1/3.

Cc: Mathias Duckeck <m.duckeck-XB/JSsFECOqzQB+pC5nmwQ@public.gmane.org>
Signed-off-by: Lukas Wunner <lukas-JFq808J9C/izQB+pC5nmwQ@public.gmane.org>
---
 .../devicetree/bindings/iio/adc/mcp320x.txt        | 23 ++++++++++++++++++++++
 1 file changed, 23 insertions(+)
diff --git a/Documentation/devicetree/bindings/iio/adc/mcp320x.txt b/Documentation/devicetree/bindings/iio/adc/mcp320x.txt
index bcd3ac8e6e0c..cf28af9ec0ac 100644
--- a/Documentation/devicetree/bindings/iio/adc/mcp320x.txt
+++ b/Documentation/devicetree/bindings/iio/adc/mcp320x.txt
@@ -29,15 +29,38 @@ Required properties:
 				"microchip,mcp3204"
 				"microchip,mcp3208"
 				"microchip,mcp3301"
+				"microchip,mcp3550-50"
+				"microchip,mcp3550-60"
+				"microchip,mcp3551"
+				"microchip,mcp3553"
 
 			NOTE: The use of the compatibles with no vendor prefix
 			is deprecated and only listed because old DT use them.
 
+	- spi-cpha, spi-cpol (boolean):
+			Either SPI mode (0,0) or (1,1) must be used, so specify
+			none or both of spi-cpha, spi-cpol.  The MCP3550/1/3
+			is more efficient in mode (1,1) as only 3 instead of
+			4 bytes need to be read from the ADC, but not all SPI
+			masters support it.
+
+	- vref-supply:	Phandle to the external reference voltage supply.
+
+Optional properties:
+	- microchip,continuous-conversion (boolean):
Second binding I have seen today with a continuous property. Make this 
common (or maybe we already have one).
+			Only applicable to MCP3550/1/3:  These ADCs have long
+			conversion times and therefore support "continuous
+			conversion mode" to allow retrieval of conversions
+			at any time without observing a delay.  The mode is
+			enabled by permanently driving CS low, e.g. by wiring
+			it to ground.
+
 Examples:
 spi_controller {
 	mcp3x0x@0 {
 		compatible = "mcp3002";
 		reg = <0>;
 		spi-max-frequency = <1000000>;
+		vref-supply = <&vref_reg>;
 	};
 };
-- 
2.11.0

Re: [PATCH 5/6] dt-bindings: iio: adc: mcp320x: Update for mcp3550/1/3

From: Lukas Wunner <hidden>
Date: 2017-08-27 15:34:05

On Fri, Aug 25, 2017 at 02:59:41PM -0500, Rob Herring wrote:
On Tue, Aug 22, 2017 at 03:33:00PM +0200, Lukas Wunner wrote:
quoted
--- a/Documentation/devicetree/bindings/iio/adc/mcp320x.txt
+++ b/Documentation/devicetree/bindings/iio/adc/mcp320x.txt
+Optional properties:
+	- microchip,continuous-conversion (boolean):
+			Only applicable to MCP3550/1/3:  These ADCs have long
+			conversion times and therefore support "continuous
+			conversion mode" to allow retrieval of conversions
+			at any time without observing a delay.  The mode is
+			enabled by permanently driving CS low, e.g. by wiring
+			it to ground.
Second binding I have seen today with a continuous property. Make this 
common (or maybe we already have one).
The other one was the TI LMP92001 ADC driver posted by Abhisit Sangjan
(cc), however looking at the datasheet of that chip reveals that
continuous versus one-shot mode is selected by flipping a bit in the
chip's register map.

So it is configurable at run-time.  It's not something that's hardwired.
(Which is the case with the MCP3550 in my patch.)

My understanding was that run-time configurable options should not be
listed in the device-tree at all, only hardware features.  If that is
correct then that device-tree property should be dropped from Abhisit
Sangjan's patch.  Configuring the feature via sysfs is fine I guess.

However we do have another driver supporting continuous versus one-shot
mode and that is drivers/iio/light/us5182d.c by Adriana Reus (cc).
The feature was added with commit c3304c212326.  I'm not sure if it's
hardwired or runtime-configurable, the datasheet is gone from the
manufacturer's website.

I agree that a common "continuous" property makes sense.  We haven't
defined any common IIO properties so far and that has already led to
inconsistencies.  E.g. most ADC/DAC drivers name the reference regulator
"vref-supply", but e.g. drivers/iio/dac/ad7303.c calls it "REF-supply".

What do you think of this:

-- >8 --
Subject: [PATCH] dt-bindings: iio: Document common properties

It's about time we standardize on common names for frequently used IIO
properties.  For starters, document "vref-supply" and "continuous".

Suggested-by: Rob Herring <redacted>
Signed-off-by: Lukas Wunner <lukas-JFq808J9C/izQB+pC5nmwQ@public.gmane.org>
---
 Documentation/devicetree/bindings/iio/iio-bindings.txt | 15 +++++++++++++++
 1 file changed, 15 insertions(+)
diff --git a/Documentation/devicetree/bindings/iio/iio-bindings.txt b/Documentation/devicetree/bindings/iio/iio-bindings.txt
index 68d6f8c..c3e87e15 100644
--- a/Documentation/devicetree/bindings/iio/iio-bindings.txt
+++ b/Documentation/devicetree/bindings/iio/iio-bindings.txt
@@ -95,3 +95,18 @@ vdd channel is connected to output 0 of the &ref device.
 		io-channels = <&adc 10>, <&adc 11>;
 		io-channel-names = "adc1", "adc2";
 	};
+
+==Common IIO properties==
+
+Reference voltage:
+ADCs, DACs and several other IIO devices require a reference voltage.
+By convention the property specifying this regulator is named "vref-supply".
+If the chip lacks a dedicated Vref pin and instead uses its own power supply
+as reference, the property specifying the regulator is commonly named
+"vdd-supply" or "vcc-supply".
+
+Continuous mode:
+Some sensors can be configured to perform continuous (versus one-shot)
+measurements.  Continuous mode may require more energy in return for faster
+or more reliable measurements.  A boolean property named "continuous"
+signifies that the device is configured for this mode.
-- 
2.11.0
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help