On 20/08/16 03:54, Akinobu Mita wrote:

Hi Rob,

2016-08-16 1:42 GMT+09:00 Jonathan Cameron [off-list ref]:

quoted

On 07/08/16 16:22, Akinobu Mita wrote:

quoted

This adds Texas Instruments' ADC12130/ADC12132/ADC12138 12-bit plus
sign ADC driver.  I have tested with the ADC12138.  The ADC12130 and
ADC12132 are not tested but these are similar to ADC12138 except that
the mode programming instruction is a bit different.

Signed-off-by: Akinobu Mita <redacted>
Acked-by: Rob Herring <redacted>

Hi,

Now had time for a closer look.  A few minor bits and bobs to fix
up highlighted inline.

Looks pretty good.

Jonathan

quoted

Cc: Jonathan Cameron <redacted>
Cc: Hartmut Knaack <redacted>
Cc: Lars-Peter Clausen <lars-Qo5EllUWu/uELgA04lAiVw@public.gmane.org>
Cc: Peter Meerwald <redacted>
---
* Changes from v2
- improve error label names

 .../devicetree/bindings/iio/adc/ti-adc12138.txt    |  32 ++
 drivers/iio/adc/Kconfig                            |  12 +
 drivers/iio/adc/Makefile                           |   1 +
 drivers/iio/adc/ti-adc12138.c                      | 542 +++++++++++++++++++++
 4 files changed, 587 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/iio/adc/ti-adc12138.txt
 create mode 100644 drivers/iio/adc/ti-adc12138.c

diff --git a/Documentation/devicetree/bindings/iio/adc/ti-adc12138.txt b/Documentation/devicetree/bindings/iio/adc/ti-adc12138.txt
new file mode 100644
index 0000000..3a11d2a
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/adc/ti-adc12138.txt

@@ -0,0 +1,32 @@
+* Texas Instruments' ADC12130/ADC12132/ADC12138
+
+Required properties:
+ - compatible: Should be one of
+     * "ti,adc12130"
+     * "ti,adc12132"
+     * "ti,adc12138"
+ - reg: SPI chip select number for the device
+ - interrupts: Should contain interrupt for EOC (end of conversion)
+ - clocks: phandle to conversion clock input
+ - spi-max-frequency: Definision as per
+     Documentation/devicetree/bindings/spi/spi-bus.txt
+ - vref-p-supply: The regulator supply for positive analog voltage reference
+
+Optional properties:
+ - vref-n-supply: The regulator supply for negative analog voltage reference
+     If not specified, this is assumed to be analog ground.

This is novel. Documented in the datasheet as a negative reference which
must be positive (greater than 0) for it to work well.

Intersting question on whether this should be optional. Easy enough to
provided a fixed regulator at 0V afterall.

quoted

+ - acquisition-time: The number of conversion clock periods for the S/H's
+     acquisition time.  Should be one of 6, 10, 18, 34.  If not specified,
+     default value of 10 is used.

Should this be a ti specific parameter.  Not sure.

Is it better to change this parameter to "ti,acquisition-time" ?

I think so. If not it needs to go in generic docs rather than this file then
be appropriately cross referenced from here. I'd keep it as ti specific for now.
We can generalize later if it makes sense (though will have to support this
as well which isn't too difficult).

quoted

I'd also add a note here about why one would set this to other than the
default (source impedance etc).

quoted

+
+Example:
+adc@0 {
+     compatible = "ti,adc12138";
+     reg = <0>;
+     interrupts = <28 IRQ_TYPE_EDGE_RISING>;
+     interrupt-parent = <&gpio1>;
+     clocks = <&cclk>;
+     vref-p-supply = <&ldo4_reg>;
+     spi-max-frequency = <5000000>;
+     acquisition-time = <6>;
+};

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