[PATCH 5/5] lmp92001: mfd: doc: Add support LMP92001

[PATCH 5/5] lmp92001: mfd: doc: Add support LMP92001

[s.abhisit-Re5JQEeQqe8AvxtiuMwx3w]

From: Abhisit Sangjan <s.abhisit-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>

TI LMP92001 Analog System Monitor and Controller

8-bit GPIOs.
12 DACs with 12-bit resolution.

The GPIOs and DACs are shared port function with Cy function pin to
take control the pin suddenly from external hardware.
DAC's referance voltage selectable for Internal/External.

16 + 1 ADCs with 12-bit resolution.

Built-in internal Temperature Sensor on channel 17.
Window Comparator Function is supported on channel 1-3 and 9-11 for
monitoring with interrupt signal (pending to implement for interrupt).
ADC's referance voltage selectable for Internal/External.

Signed-off-by: Abhisit Sangjan <s.abhisit-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
 Documentation/ABI/testing/sysfs-bus-iio-lmp920001  | 65 ++++++++++++++++++++++
 .../devicetree/bindings/gpio/gpio-lmp92001.txt     | 22 ++++++++
 .../bindings/iio/adc/ti-lmp92001-adc.txt           | 20 +++++++
 .../bindings/iio/dac/ti-lmp92001-dac.txt           | 35 ++++++++++++
 4 files changed, 142 insertions(+)
 create mode 100644 Documentation/ABI/testing/sysfs-bus-iio-lmp920001
 create mode 100644 Documentation/devicetree/bindings/gpio/gpio-lmp92001.txt
 create mode 100644 Documentation/devicetree/bindings/iio/adc/ti-lmp92001-adc.txt
 create mode 100644 Documentation/devicetree/bindings/iio/dac/ti-lmp92001-dac.txt

diff --git a/Documentation/ABI/testing/sysfs-bus-iio-lmp920001 b/Documentation/ABI/testing/sysfs-bus-iio-lmp920001
new file mode 100644
index 000000000000..f0965e7d823f
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-bus-iio-lmp920001
@@ -0,0 +1,65 @@
+What:           /sys/bus/iio/devices/iio:deviceX/gang
+Date:           August 2016
+KernelVersion:  4.12
+Contact:        Abhisit Sangjan <s.abhisit-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
+Description:
+                Controls the association of analog output channels OUTx with
+                asynchronous control inputs Cy for DAC.
+                Can be either:
+                - "0"
+                - "1"
+
+                        Cy to OUTx Assignment
+                ----------------------------------
+                | Cy | CDAC:GANG=0 | CDAC:GANG=1 |
+                ----------------------------------
+                | C1 | OUT[1:4]    | OUT[1:3]    |
+                ----------------------------------
+                | C2 | OUT[5:6]    | OUT[4:6]    |
+                ----------------------------------
+                | C3 | OUT[7:8]    | OUT[7:9]    |
+                ----------------------------------
+                | C4 | OUT[9:12]   | OUT[10:12]  |
+                ----------------------------------
+
+What:           /sys/bus/iio/devices/iio:deviceX/outx
+Date:           August 2016
+KernelVersion:  4.12
+Contact:        Abhisit Sangjan <s.abhisit-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
+Description:
+                The pin output mode for DAC.
+                Can be either:
+                - "hiz" = High impedance state.
+                - "dac" = DAC output.
+                - "0" = Drive it to low.
+                - "1" = Drive it to high.
+
+What:           /sys/bus/iio/devices/iio:deviceX/vref
+Date:           August 2016
+KernelVersion:  4.12
+Contact:        Abhisit Sangjan <s.abhisit-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
+Description:
+                This is voltage reference source for DACs.
+                Can be either:
+                - "external"
+                - "internal"
+
+What:           /sys/devices/.../iio:deviceX/en
+Date:           August 2016
+KernelVersion:  4.12
+Contact:        Abhisit Sangjan <s.abhisit-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
+Description:
+                This is ADC Conversion Enable for each channel.
+                Can be either:
+                - "enable"
+                - "disable"
+
+What:           /sys/devices/.../iio:deviceX/vref
+Date:           August 2016
+KernelVersion:  4.12
+Contact:        Abhisit Sangjan <s.abhisit-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
+Description:
+                This is voltage reference source for ADCs.
+                Can be either:
+                - "external"
+                - "internal"
diff --git a/Documentation/devicetree/bindings/gpio/gpio-lmp92001.txt b/Documentation/devicetree/bindings/gpio/gpio-lmp92001.txt
new file mode 100644
index 000000000000..f9a18c492145
--- /dev/null
+++ b/Documentation/devicetree/bindings/gpio/gpio-lmp92001.txt
@@ -0,0 +1,22 @@
+* Texas Instruments' LMP92001 GPIOs
+
+Required properties:
+ - compatible: Must be set to "ti,lmp92001-gpio".
+ - reg: i2c chip address for the device.
+ - gpio-controller: Marks the device node as a gpio controller.
+ - #gpio-cells : Should be two.  The first cell is the pin number and the
+  second cell is used to specify the gpio polarity:
+        0 = Active high
+        1 = Active low
+
+Example:
+lmp92001@20 {
+        compatible = "ti,lmp92001";
+        reg = <0x20>;
+
+        gpio-controller {
+                compatible = "ti,lmp92001-gpio";
+                gpio-controller;
+                #gpio-cells = <2>;
+        };
+};
diff --git a/Documentation/devicetree/bindings/iio/adc/ti-lmp92001-adc.txt b/Documentation/devicetree/bindings/iio/adc/ti-lmp92001-adc.txt
new file mode 100644
index 000000000000..4565961bf511
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/adc/ti-lmp92001-adc.txt
@@ -0,0 +1,20 @@
+* Texas Instruments' LMP92001 ADCs
+
+Required properties:
+ - compatible: Must be set to "ti,lmp92001-adc".
+ - reg: i2c chip address for the device.
+ - ti,lmp92001-adc-mask: bit mask for which channel is enable.
+        0 = Off
+        1 = On
+
+Example:
+lmp92001@20 {
+        compatible = "ti,lmp92001";
+        reg = <0x20>;
+
+        lmp92001-adc {
+                compatible = "ti,lmp92001-adc";
+                ti,lmp92001-adc-mode = "continuous";
+                ti,lmp92001-adc-mask = <0x00000079>;
+        };
+};
diff --git a/Documentation/devicetree/bindings/iio/dac/ti-lmp92001-dac.txt b/Documentation/devicetree/bindings/iio/dac/ti-lmp92001-dac.txt
new file mode 100644
index 000000000000..882db9ca92f5
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/dac/ti-lmp92001-dac.txt
@@ -0,0 +1,35 @@
+* Texas Instruments' LMP92001 DACs
+
+Required properties:
+ - compatible: Must be set to "ti,lmp92001-dac".
+ - reg: i2c chip address for the device.
+ - ti,lmp92001-dac-hiz: hi-impedance control,
+        1 = Forces all OUT[1:12] outputs to hi-z, 0 = normal
+ - ti,lmp92001-dac-outx:
+        Cy = 0, 1 = will force associated OUTx outputs to VDD
+        Cy = 0, 0 = will force associated OUTx outputs to GND
+ - ti,lmp92001-dac-gang: What group of Cy is governed to.
+        -----------------------------------------
+        |  Cy   | CDAC:GANG = 0 | CDAC:GANG = 1 |
+        -----------------------------------------
+        |  C1   |   OUT[1:4]    |    OUT[1:3]   |
+        -----------------------------------------
+        |  C2   |   OUT[5:6]    |    OUT[4:6]   |
+        -----------------------------------------
+        |  C3   |   OUT[7:8]    |    OUT[7:9]   |
+        -----------------------------------------
+        |  C4   |   OUT[9:12]   |    OUT[10:12] |
+        -----------------------------------------
+
+Example:
+lmp92001@20 {
+        compatible = "ti,lmp92001";
+        reg = <0x20>;
+
+        lmp92001-dac {
+                compatible = "ti,lmp92001-dac";
+                ti,lmp92001-dac-hiz  = /bits/ 8 <0>;
+                ti,lmp92001-dac-outx = /bits/ 8 <0>;
+                ti,lmp92001-dac-gang = /bits/ 8 <0>;
+        };
+};
-- 
2.13.0

Re: [PATCH 5/5] lmp92001: mfd: doc: Add support LMP92001

[Jonathan Cameron]

On Thu, 31 Aug 2017 01:25:28 +0700
s.abhisit@gmail.com wrote:

> From: Abhisit Sangjan <s.abhisit@gmail.com>
> 
> TI LMP92001 Analog System Monitor and Controller
> 
> 8-bit GPIOs.
> 12 DACs with 12-bit resolution.
> 
> The GPIOs and DACs are shared port function with Cy function pin to
> take control the pin suddenly from external hardware.
> DAC's referance voltage selectable for Internal/External.
> 
> 16 + 1 ADCs with 12-bit resolution.
> 
> Built-in internal Temperature Sensor on channel 17.
> Window Comparator Function is supported on channel 1-3 and 9-11 for
> monitoring with interrupt signal (pending to implement for interrupt).
> ADC's referance voltage selectable for Internal/External.
> 

I would break this patch up somewhat.  Have a set of DT patches separate
from any sysfs attribute documentation.

> Signed-off-by: Abhisit Sangjan <s.abhisit@gmail.com>
> ---
>  Documentation/ABI/testing/sysfs-bus-iio-lmp920001  | 65 ++++++++++++++++++++++
>  .../devicetree/bindings/gpio/gpio-lmp92001.txt     | 22 ++++++++
>  .../bindings/iio/adc/ti-lmp92001-adc.txt           | 20 +++++++
>  .../bindings/iio/dac/ti-lmp92001-dac.txt           | 35 ++++++++++++
>  4 files changed, 142 insertions(+)
>  create mode 100644 Documentation/ABI/testing/sysfs-bus-iio-lmp920001
>  create mode 100644 Documentation/devicetree/bindings/gpio/gpio-lmp92001.txt
>  create mode 100644 Documentation/devicetree/bindings/iio/adc/ti-lmp92001-adc.txt
>  create mode 100644 Documentation/devicetree/bindings/iio/dac/ti-lmp92001-dac.txt
> 
> diff --git a/Documentation/ABI/testing/sysfs-bus-iio-lmp920001 b/Documentation/ABI/testing/sysfs-bus-iio-lmp920001
> new file mode 100644
> index 000000000000..f0965e7d823f
> --- /dev/null
> +++ b/Documentation/ABI/testing/sysfs-bus-iio-lmp920001
> @@ -0,0 +1,65 @@
> +What:           /sys/bus/iio/devices/iio:deviceX/gang
> +Date:           August 2016
> +KernelVersion:  4.12
> +Contact:        Abhisit Sangjan <s.abhisit@gmail.com>
> +Description:
> +                Controls the association of analog output channels OUTx with
> +                asynchronous control inputs Cy for DAC.
> +                Can be either:
> +                - "0"
> +                - "1"
> +
> +                        Cy to OUTx Assignment
> +                ----------------------------------
> +                | Cy | CDAC:GANG=0 | CDAC:GANG=1 |
> +                ----------------------------------
> +                | C1 | OUT[1:4]    | OUT[1:3]    |
> +                ----------------------------------
> +                | C2 | OUT[5:6]    | OUT[4:6]    |
> +                ----------------------------------
> +                | C3 | OUT[7:8]    | OUT[7:9]    |
> +                ----------------------------------
> +                | C4 | OUT[9:12]   | OUT[10:12]  |
> +                ----------------------------------
> +
> +What:           /sys/bus/iio/devices/iio:deviceX/outx
> +Date:           August 2016
> +KernelVersion:  4.12
> +Contact:        Abhisit Sangjan <s.abhisit@gmail.com>
> +Description:
> +                The pin output mode for DAC.
> +                Can be either:
> +                - "hiz" = High impedance state.
> +                - "dac" = DAC output.
> +                - "0" = Drive it to low.
> +                - "1" = Drive it to high.

Two options here - consider the hiz 0 and 1 to be powerdown
modes in which case use the standard interfaces for them, or
take the view the 0 1 choice is to allow this device to
act as a gpio chip and support that explicitly.

> +
> +What:           /sys/bus/iio/devices/iio:deviceX/vref
> +Date:           August 2016
> +KernelVersion:  4.12
> +Contact:        Abhisit Sangjan <s.abhisit@gmail.com>
> +Description:
> +                This is voltage reference source for DACs.
> +                Can be either:
> +                - "external"
> +                - "internal"

It is rare for userspace control of this to make sense.  Tends to be
the case that there is only one right answer for a given piece of
hardware.

> +
> +What:           /sys/devices/.../iio:deviceX/en
> +Date:           August 2016
> +KernelVersion:  4.12
> +Contact:        Abhisit Sangjan <s.abhisit@gmail.com>
> +Description:
> +                This is ADC Conversion Enable for each channel.
> +                Can be either:
> +                - "enable"
> +                - "disable"

If you want this fine control use the buffered interfaces rather
than sysfs.  I don't want to add this complexity to the sysfs
interface.  For sysfs if nothing outside sysfs is going on (e.g.
no buffered capture) it should be read an attribute get the answer
without having to configure other elements manually.

Also as currently documented, you only have one such file per
iio:device which seems unlikely to be what you were aiming for.


> +
> +What:           /sys/devices/.../iio:deviceX/vref

Here twice...

> +Date:           August 2016
> +KernelVersion:  4.12
> +Contact:        Abhisit Sangjan <s.abhisit@gmail.com>
> +Description:
> +                This is voltage reference source for ADCs.
> +                Can be either:
> +                - "external"
> +                - "internal"
> diff --git a/Documentation/devicetree/bindings/gpio/gpio-lmp92001.txt b/Documentation/devicetree/bindings/gpio/gpio-lmp92001.txt
> new file mode 100644
> index 000000000000..f9a18c492145
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/gpio/gpio-lmp92001.txt
> @@ -0,0 +1,22 @@
> +* Texas Instruments' LMP92001 GPIOs
> +
> +Required properties:
> + - compatible: Must be set to "ti,lmp92001-gpio".
> + - reg: i2c chip address for the device.
> + - gpio-controller: Marks the device node as a gpio controller.
> + - #gpio-cells : Should be two.  The first cell is the pin number and the
> +  second cell is used to specify the gpio polarity:
> +        0 = Active high
> +        1 = Active low
> +
> +Example:
> +lmp92001@20 {
> +        compatible = "ti,lmp92001";
> +        reg = <0x20>;
> +
> +        gpio-controller {
> +                compatible = "ti,lmp92001-gpio";
> +                gpio-controller;
> +                #gpio-cells = <2>;
> +        };
> +};
> diff --git a/Documentation/devicetree/bindings/iio/adc/ti-lmp92001-adc.txt b/Documentation/devicetree/bindings/iio/adc/ti-lmp92001-adc.txt
> new file mode 100644
> index 000000000000..4565961bf511
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/adc/ti-lmp92001-adc.txt
> @@ -0,0 +1,20 @@
> +* Texas Instruments' LMP92001 ADCs
> +
> +Required properties:
> + - compatible: Must be set to "ti,lmp92001-adc".
> + - reg: i2c chip address for the device.
> + - ti,lmp92001-adc-mask: bit mask for which channel is enable.
> +        0 = Off
> +        1 = On
> +
> +Example:
> +lmp92001@20 {
> +        compatible = "ti,lmp92001";
> +        reg = <0x20>;
> +
> +        lmp92001-adc {
> +                compatible = "ti,lmp92001-adc";
> +                ti,lmp92001-adc-mode = "continuous";
> +                ti,lmp92001-adc-mask = <0x00000079>;
> +        };
> +};
> diff --git a/Documentation/devicetree/bindings/iio/dac/ti-lmp92001-dac.txt b/Documentation/devicetree/bindings/iio/dac/ti-lmp92001-dac.txt
> new file mode 100644
> index 000000000000..882db9ca92f5
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/dac/ti-lmp92001-dac.txt
> @@ -0,0 +1,35 @@
> +* Texas Instruments' LMP92001 DACs
> +
> +Required properties:
> + - compatible: Must be set to "ti,lmp92001-dac".
> + - reg: i2c chip address for the device.
> + - ti,lmp92001-dac-hiz: hi-impedance control,
> +        1 = Forces all OUT[1:12] outputs to hi-z, 0 = normal

Why?

> + - ti,lmp92001-dac-outx:
> +        Cy = 0, 1 = will force associated OUTx outputs to VDD
> +        Cy = 0, 0 = will force associated OUTx outputs to GND

Clashes with the above?  I'm not sure why you would want these to
be device tree bindings at all.

> + - ti,lmp92001-dac-gang: What group of Cy is governed to.
> +        -----------------------------------------
> +        |  Cy   | CDAC:GANG = 0 | CDAC:GANG = 1 |
> +        -----------------------------------------
> +        |  C1   |   OUT[1:4]    |    OUT[1:3]   |
> +        -----------------------------------------
> +        |  C2   |   OUT[5:6]    |    OUT[4:6]   |
> +        -----------------------------------------
> +        |  C3   |   OUT[7:8]    |    OUT[7:9]   |
> +        -----------------------------------------
> +        |  C4   |   OUT[9:12]   |    OUT[10:12] |
> +        -----------------------------------------

This last one is an ugly bit of hardware, so probably fine
to have in DT.
> +
> +Example:
> +lmp92001@20 {
> +        compatible = "ti,lmp92001";
> +        reg = <0x20>;
> +
> +        lmp92001-dac {
> +                compatible = "ti,lmp92001-dac";
> +                ti,lmp92001-dac-hiz  = /bits/ 8 <0>;
> +                ti,lmp92001-dac-outx = /bits/ 8 <0>;
> +                ti,lmp92001-dac-gang = /bits/ 8 <0>;
> +        };
> +};

Re: [PATCH 5/5] lmp92001: mfd: doc: Add support LMP92001

[Rob Herring]

On Thu, Aug 31, 2017 at 01:25:28AM +0700, s.abhisit-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org wrote:
> From: Abhisit Sangjan <s.abhisit-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> 
> TI LMP92001 Analog System Monitor and Controller
> 
> 8-bit GPIOs.
> 12 DACs with 12-bit resolution.
> 
> The GPIOs and DACs are shared port function with Cy function pin to
> take control the pin suddenly from external hardware.
> DAC's referance voltage selectable for Internal/External.
> 
> 16 + 1 ADCs with 12-bit resolution.
> 
> Built-in internal Temperature Sensor on channel 17.
> Window Comparator Function is supported on channel 1-3 and 9-11 for
> monitoring with interrupt signal (pending to implement for interrupt).
> ADC's referance voltage selectable for Internal/External.
> 
> Signed-off-by: Abhisit Sangjan <s.abhisit-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> ---
>  Documentation/ABI/testing/sysfs-bus-iio-lmp920001  | 65 ++++++++++++++++++++++
>  .../devicetree/bindings/gpio/gpio-lmp92001.txt     | 22 ++++++++

As Jonathan said, please split.

>  .../bindings/iio/adc/ti-lmp92001-adc.txt           | 20 +++++++
>  .../bindings/iio/dac/ti-lmp92001-dac.txt           | 35 ++++++++++++
>  4 files changed, 142 insertions(+)
>  create mode 100644 Documentation/ABI/testing/sysfs-bus-iio-lmp920001
>  create mode 100644 Documentation/devicetree/bindings/gpio/gpio-lmp92001.txt
>  create mode 100644 Documentation/devicetree/bindings/iio/adc/ti-lmp92001-adc.txt
>  create mode 100644 Documentation/devicetree/bindings/iio/dac/ti-lmp92001-dac.txt

[...]

> diff --git a/Documentation/devicetree/bindings/gpio/gpio-lmp92001.txt b/Documentation/devicetree/bindings/gpio/gpio-lmp92001.txt
> new file mode 100644
> index 000000000000..f9a18c492145
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/gpio/gpio-lmp92001.txt
> @@ -0,0 +1,22 @@
> +* Texas Instruments' LMP92001 GPIOs
> +

Need to state what this is a child of and refer to that binding doc. 
Same for the others.

> +Required properties:
> + - compatible: Must be set to "ti,lmp92001-gpio".
> + - reg: i2c chip address for the device.

? That's for the parent.

> + - gpio-controller: Marks the device node as a gpio controller.
> + - #gpio-cells : Should be two.  The first cell is the pin number and the
> +  second cell is used to specify the gpio polarity:
> +        0 = Active high
> +        1 = Active low
> +
> +Example:
> +lmp92001@20 {
> +        compatible = "ti,lmp92001";
> +        reg = <0x20>;
> +
> +        gpio-controller {

Should be "gpio {"

> +                compatible = "ti,lmp92001-gpio";
> +                gpio-controller;
> +                #gpio-cells = <2>;
> +        };
> +};
> diff --git a/Documentation/devicetree/bindings/iio/adc/ti-lmp92001-adc.txt b/Documentation/devicetree/bindings/iio/adc/ti-lmp92001-adc.txt
> new file mode 100644
> index 000000000000..4565961bf511
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/adc/ti-lmp92001-adc.txt
> @@ -0,0 +1,20 @@
> +* Texas Instruments' LMP92001 ADCs
> +
> +Required properties:
> + - compatible: Must be set to "ti,lmp92001-adc".
> + - reg: i2c chip address for the device.
> + - ti,lmp92001-adc-mask: bit mask for which channel is enable.
> +        0 = Off
> +        1 = On
> +
> +Example:
> +lmp92001@20 {
> +        compatible = "ti,lmp92001";
> +        reg = <0x20>;
> +
> +        lmp92001-adc {

adc {

> +                compatible = "ti,lmp92001-adc";
> +                ti,lmp92001-adc-mode = "continuous";

Not documented and I believe we're going to add a common property for 
this.

> +                ti,lmp92001-adc-mask = <0x00000079>;
> +        };
> +};
> diff --git a/Documentation/devicetree/bindings/iio/dac/ti-lmp92001-dac.txt b/Documentation/devicetree/bindings/iio/dac/ti-lmp92001-dac.txt
> new file mode 100644
> index 000000000000..882db9ca92f5
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/dac/ti-lmp92001-dac.txt
> @@ -0,0 +1,35 @@
> +* Texas Instruments' LMP92001 DACs
> +
> +Required properties:
> + - compatible: Must be set to "ti,lmp92001-dac".
> + - reg: i2c chip address for the device.
> + - ti,lmp92001-dac-hiz: hi-impedance control,
> +        1 = Forces all OUT[1:12] outputs to hi-z, 0 = normal
> + - ti,lmp92001-dac-outx:
> +        Cy = 0, 1 = will force associated OUTx outputs to VDD
> +        Cy = 0, 0 = will force associated OUTx outputs to GND
> + - ti,lmp92001-dac-gang: What group of Cy is governed to.
> +        -----------------------------------------
> +        |  Cy   | CDAC:GANG = 0 | CDAC:GANG = 1 |
> +        -----------------------------------------
> +        |  C1   |   OUT[1:4]    |    OUT[1:3]   |
> +        -----------------------------------------
> +        |  C2   |   OUT[5:6]    |    OUT[4:6]   |
> +        -----------------------------------------
> +        |  C3   |   OUT[7:8]    |    OUT[7:9]   |
> +        -----------------------------------------
> +        |  C4   |   OUT[9:12]   |    OUT[10:12] |
> +        -----------------------------------------
> +
> +Example:
> +lmp92001@20 {
> +        compatible = "ti,lmp92001";
> +        reg = <0x20>;
> +
> +        lmp92001-dac {

dac {

> +                compatible = "ti,lmp92001-dac";
> +                ti,lmp92001-dac-hiz  = /bits/ 8 <0>;
> +                ti,lmp92001-dac-outx = /bits/ 8 <0>;
> +                ti,lmp92001-dac-gang = /bits/ 8 <0>;
> +        };
> +};
> -- 
> 2.13.0
> 

Re: [PATCH 5/5] lmp92001: mfd: doc: Add support LMP92001

[Jonathan Cameron]

On Mon, 11 Sep 2017 16:58:41 -0500
Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:

> On Thu, Aug 31, 2017 at 01:25:28AM +0700, s.abhisit-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org wrote:
> > From: Abhisit Sangjan <s.abhisit-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> > 
> > TI LMP92001 Analog System Monitor and Controller
> > 
> > 8-bit GPIOs.
> > 12 DACs with 12-bit resolution.
> > 
> > The GPIOs and DACs are shared port function with Cy function pin to
> > take control the pin suddenly from external hardware.
> > DAC's referance voltage selectable for Internal/External.
> > 
> > 16 + 1 ADCs with 12-bit resolution.
> > 
> > Built-in internal Temperature Sensor on channel 17.
> > Window Comparator Function is supported on channel 1-3 and 9-11 for
> > monitoring with interrupt signal (pending to implement for interrupt).
> > ADC's referance voltage selectable for Internal/External.
> > 
> > Signed-off-by: Abhisit Sangjan <s.abhisit-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> > ---
> >  Documentation/ABI/testing/sysfs-bus-iio-lmp920001  | 65 ++++++++++++++++++++++
> >  .../devicetree/bindings/gpio/gpio-lmp92001.txt     | 22 ++++++++  
> 
> As Jonathan said, please split.
> 
> >  .../bindings/iio/adc/ti-lmp92001-adc.txt           | 20 +++++++
> >  .../bindings/iio/dac/ti-lmp92001-dac.txt           | 35 ++++++++++++
> >  4 files changed, 142 insertions(+)
> >  create mode 100644 Documentation/ABI/testing/sysfs-bus-iio-lmp920001
> >  create mode 100644 Documentation/devicetree/bindings/gpio/gpio-lmp92001.txt
> >  create mode 100644 Documentation/devicetree/bindings/iio/adc/ti-lmp92001-adc.txt
> >  create mode 100644 Documentation/devicetree/bindings/iio/dac/ti-lmp92001-dac.txt  
> 
> [...]
> 
> > diff --git a/Documentation/devicetree/bindings/gpio/gpio-lmp92001.txt b/Documentation/devicetree/bindings/gpio/gpio-lmp92001.txt
> > new file mode 100644
> > index 000000000000..f9a18c492145
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/gpio/gpio-lmp92001.txt
> > @@ -0,0 +1,22 @@
> > +* Texas Instruments' LMP92001 GPIOs
> > +  
> 
> Need to state what this is a child of and refer to that binding doc. 
> Same for the others.
> 
> > +Required properties:
> > + - compatible: Must be set to "ti,lmp92001-gpio".
> > + - reg: i2c chip address for the device.  
> 
> ? That's for the parent.
> 
> > + - gpio-controller: Marks the device node as a gpio controller.
> > + - #gpio-cells : Should be two.  The first cell is the pin number and the
> > +  second cell is used to specify the gpio polarity:
> > +        0 = Active high
> > +        1 = Active low
> > +
> > +Example:
> > +lmp92001@20 {
> > +        compatible = "ti,lmp92001";
> > +        reg = <0x20>;
> > +
> > +        gpio-controller {  
> 
> Should be "gpio {"
> 
> > +                compatible = "ti,lmp92001-gpio";
> > +                gpio-controller;
> > +                #gpio-cells = <2>;
> > +        };
> > +};
> > diff --git a/Documentation/devicetree/bindings/iio/adc/ti-lmp92001-adc.txt b/Documentation/devicetree/bindings/iio/adc/ti-lmp92001-adc.txt
> > new file mode 100644
> > index 000000000000..4565961bf511
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/iio/adc/ti-lmp92001-adc.txt
> > @@ -0,0 +1,20 @@
> > +* Texas Instruments' LMP92001 ADCs
> > +
> > +Required properties:
> > + - compatible: Must be set to "ti,lmp92001-adc".
> > + - reg: i2c chip address for the device.
> > + - ti,lmp92001-adc-mask: bit mask for which channel is enable.
> > +        0 = Off
> > +        1 = On
> > +
> > +Example:
> > +lmp92001@20 {
> > +        compatible = "ti,lmp92001";
> > +        reg = <0x20>;
> > +
> > +        lmp92001-adc {  
> 
> adc {
> 
> > +                compatible = "ti,lmp92001-adc";
> > +                ti,lmp92001-adc-mode = "continuous";  
> 
> Not documented and I believe we're going to add a common property for 
> this.

Possibly.  I'm not keen on it being explicitly controlled in general.
It isn't a feature of the hardware in this case (I think) and as userspace
control it is very hard for userspace to know what to do with it -
normally it is better to put sufficient smarts in the driver to handle
this transparently from either dt or userspace abi point of view.

At a rough guess 10-25% of the hardware in IIO has a similar mode (possibly more)
We expose it precisely 1 other driver at the moment (and to be honest
I can't recall why we allowed it in there)  There are all sorts of optimizations
that can be done in the driver if we don't make this a manual control.

Jonathan

> 
> > +                ti,lmp92001-adc-mask = <0x00000079>;
> > +        };
> > +};
> > diff --git a/Documentation/devicetree/bindings/iio/dac/ti-lmp92001-dac.txt b/Documentation/devicetree/bindings/iio/dac/ti-lmp92001-dac.txt
> > new file mode 100644
> > index 000000000000..882db9ca92f5
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/iio/dac/ti-lmp92001-dac.txt
> > @@ -0,0 +1,35 @@
> > +* Texas Instruments' LMP92001 DACs
> > +
> > +Required properties:
> > + - compatible: Must be set to "ti,lmp92001-dac".
> > + - reg: i2c chip address for the device.
> > + - ti,lmp92001-dac-hiz: hi-impedance control,
> > +        1 = Forces all OUT[1:12] outputs to hi-z, 0 = normal
> > + - ti,lmp92001-dac-outx:
> > +        Cy = 0, 1 = will force associated OUTx outputs to VDD
> > +        Cy = 0, 0 = will force associated OUTx outputs to GND
> > + - ti,lmp92001-dac-gang: What group of Cy is governed to.
> > +        -----------------------------------------
> > +        |  Cy   | CDAC:GANG = 0 | CDAC:GANG = 1 |
> > +        -----------------------------------------
> > +        |  C1   |   OUT[1:4]    |    OUT[1:3]   |
> > +        -----------------------------------------
> > +        |  C2   |   OUT[5:6]    |    OUT[4:6]   |
> > +        -----------------------------------------
> > +        |  C3   |   OUT[7:8]    |    OUT[7:9]   |
> > +        -----------------------------------------
> > +        |  C4   |   OUT[9:12]   |    OUT[10:12] |
> > +        -----------------------------------------
> > +
> > +Example:
> > +lmp92001@20 {
> > +        compatible = "ti,lmp92001";
> > +        reg = <0x20>;
> > +
> > +        lmp92001-dac {  
> 
> dac {
> 
> > +                compatible = "ti,lmp92001-dac";
> > +                ti,lmp92001-dac-hiz  = /bits/ 8 <0>;
> > +                ti,lmp92001-dac-outx = /bits/ 8 <0>;
> > +                ti,lmp92001-dac-gang = /bits/ 8 <0>;
> > +        };
> > +};
> > -- 
> > 2.13.0
> >   
> --
> 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

Re: [PATCH 5/5] lmp92001: mfd: doc: Add support LMP92001

[Rob Herring]

On Tue, Sep 12, 2017 at 5:09 AM, Jonathan Cameron <jic23-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
> On Mon, 11 Sep 2017 16:58:41 -0500
> Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
>
>> On Thu, Aug 31, 2017 at 01:25:28AM +0700, s.abhisit-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org wrote:
>> > From: Abhisit Sangjan <s.abhisit-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>> >
>> > TI LMP92001 Analog System Monitor and Controller
>> >
>> > 8-bit GPIOs.
>> > 12 DACs with 12-bit resolution.
>> >
>> > The GPIOs and DACs are shared port function with Cy function pin to
>> > take control the pin suddenly from external hardware.
>> > DAC's referance voltage selectable for Internal/External.
>> >
>> > 16 + 1 ADCs with 12-bit resolution.
>> >
>> > Built-in internal Temperature Sensor on channel 17.
>> > Window Comparator Function is supported on channel 1-3 and 9-11 for
>> > monitoring with interrupt signal (pending to implement for interrupt).
>> > ADC's referance voltage selectable for Internal/External.
>> >
>> > Signed-off-by: Abhisit Sangjan <s.abhisit-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>> > ---
>> >  Documentation/ABI/testing/sysfs-bus-iio-lmp920001  | 65 ++++++++++++++++++++++
>> >  .../devicetree/bindings/gpio/gpio-lmp92001.txt     | 22 ++++++++
>>
>> As Jonathan said, please split.
>>
>> >  .../bindings/iio/adc/ti-lmp92001-adc.txt           | 20 +++++++
>> >  .../bindings/iio/dac/ti-lmp92001-dac.txt           | 35 ++++++++++++
>> >  4 files changed, 142 insertions(+)
>> >  create mode 100644 Documentation/ABI/testing/sysfs-bus-iio-lmp920001
>> >  create mode 100644 Documentation/devicetree/bindings/gpio/gpio-lmp92001.txt
>> >  create mode 100644 Documentation/devicetree/bindings/iio/adc/ti-lmp92001-adc.txt
>> >  create mode 100644 Documentation/devicetree/bindings/iio/dac/ti-lmp92001-dac.txt
>>
>> [...]
>>
>> > diff --git a/Documentation/devicetree/bindings/gpio/gpio-lmp92001.txt b/Documentation/devicetree/bindings/gpio/gpio-lmp92001.txt
>> > new file mode 100644
>> > index 000000000000..f9a18c492145
>> > --- /dev/null
>> > +++ b/Documentation/devicetree/bindings/gpio/gpio-lmp92001.txt
>> > @@ -0,0 +1,22 @@
>> > +* Texas Instruments' LMP92001 GPIOs
>> > +
>>
>> Need to state what this is a child of and refer to that binding doc.
>> Same for the others.
>>
>> > +Required properties:
>> > + - compatible: Must be set to "ti,lmp92001-gpio".
>> > + - reg: i2c chip address for the device.
>>
>> ? That's for the parent.
>>
>> > + - gpio-controller: Marks the device node as a gpio controller.
>> > + - #gpio-cells : Should be two.  The first cell is the pin number and the
>> > +  second cell is used to specify the gpio polarity:
>> > +        0 = Active high
>> > +        1 = Active low
>> > +
>> > +Example:
>> > +lmp92001@20 {
>> > +        compatible = "ti,lmp92001";
>> > +        reg = <0x20>;
>> > +
>> > +        gpio-controller {
>>
>> Should be "gpio {"
>>
>> > +                compatible = "ti,lmp92001-gpio";
>> > +                gpio-controller;
>> > +                #gpio-cells = <2>;
>> > +        };
>> > +};
>> > diff --git a/Documentation/devicetree/bindings/iio/adc/ti-lmp92001-adc.txt b/Documentation/devicetree/bindings/iio/adc/ti-lmp92001-adc.txt
>> > new file mode 100644
>> > index 000000000000..4565961bf511
>> > --- /dev/null
>> > +++ b/Documentation/devicetree/bindings/iio/adc/ti-lmp92001-adc.txt
>> > @@ -0,0 +1,20 @@
>> > +* Texas Instruments' LMP92001 ADCs
>> > +
>> > +Required properties:
>> > + - compatible: Must be set to "ti,lmp92001-adc".
>> > + - reg: i2c chip address for the device.
>> > + - ti,lmp92001-adc-mask: bit mask for which channel is enable.
>> > +        0 = Off
>> > +        1 = On
>> > +
>> > +Example:
>> > +lmp92001@20 {
>> > +        compatible = "ti,lmp92001";
>> > +        reg = <0x20>;
>> > +
>> > +        lmp92001-adc {
>>
>> adc {
>>
>> > +                compatible = "ti,lmp92001-adc";
>> > +                ti,lmp92001-adc-mode = "continuous";
>>
>> Not documented and I believe we're going to add a common property for
>> this.
>
> Possibly.  I'm not keen on it being explicitly controlled in general.
> It isn't a feature of the hardware in this case (I think) and as userspace
> control it is very hard for userspace to know what to do with it -
> normally it is better to put sufficient smarts in the driver to handle
> this transparently from either dt or userspace abi point of view.

That certainly makes sense.

No property or common property. Your choice. :)

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