Hi Rob,

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

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'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>;
+};