Hi Jonathan,

On Sat, 1 Oct 2016 12:21:57 +0100, Jonathan Cameron [off-list ref] wrote:

On 01/10/16 12:15, Jonathan Cameron wrote:

quoted

On 30/09/16 14:19, Tomas Novotny wrote:

quoted

Signed-off-by: Tomas Novotny <redacted>
---
 .../devicetree/bindings/iio/dac/mcp4725.txt        | 28 ++++++++++++++++++++++
 1 file changed, 28 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/iio/dac/mcp4725.txt

diff --git a/Documentation/devicetree/bindings/iio/dac/mcp4725.txt b/Documentation/devicetree/bindings/iio/dac/mcp4725.txt
new file mode 100644
index 0000000..69c9462
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/dac/mcp4725.txt

@@ -0,0 +1,28 @@
+Microchpip mcp4725 and mcp4726 DAC device driver
+
+Required properties:
+	- compatible: Must be "microchip,mcp4725" or "microchip,mcp4726"
+	- reg: Should contain the DAC I2C address
+	- vref-millivolt: Value of the reference voltage

Sorry dozing this morning so missed this until looking at the code.

Use the regulator framework to supply two regulators:

vref-suppy and vdd-supply
first one optional.

Then query them in the driver.  Much more flexible than just hard coding
a value here.  Entirely possible to have a variable regulator used for
the reference.

I see now that this is a pretty standard approach in the iio/dac. I will do
it in the regulator way. I will send v2 then.

Thanks for the review,

Tomas

quoted

quoted

+
+Optional properties:
+	- vref-mode: Reference voltage selection. It is available only on

Not a generic attribute, so wants to have a vendor prefix on it.

quoted

+	  mcp4726. Valid values are:
+		- 0, 1: Vdd pin voltage (unbuffered)

This should not be a direct mapping of the register values but rather
a means to control the setting.  Having two values mapping to the
same thing makes no sense.

Would also be odd to specify a vref and then not use it.  So you could
infer this first option.

quoted

+		- 2: Vref pin voltage unbuffered
+		- 3: Vref pin voltage internally buffered

Having inferred the first option then this becomes control of whether it
is buffered  or not. So could be named appropriately to cover that.
Might be nice to have a little note here on why you might want the
buffer or not..

quoted

+
+Examples:
+
+        mcp4725@60 {
+                compatible = "microchip,mcp4725";
+                reg = <0x60>;
+                vref-millivolt = <3300>;
+        };
+
+        mcp4726@60 {
+                compatible = "microchip,mcp4726";
+                reg = <0x60>;
+                vref-mode = <2>;
+                vref-millivolt = <2500>;

quoted

quoted

+        };

--
To unsubscribe from this list: send the line "unsubscribe linux-iio" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

--
To unsubscribe from this list: send the line "unsubscribe linux-iio" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

--
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