| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20170912110923.0f80c116@archlinux>
Date: Tue, 12 Sep 2017 11:09:23 +0100
From: Jonathan Cameron <jic23@...nel.org>
To: Rob Herring <robh@...nel.org>
Cc: s.abhisit@...il.com, linus.walleij@...aro.org,
lee.jones@...aro.org, Jonathan.Cameron@...wei.com,
pmeerw@...erw.net, jacopo@...ndi.org, linux-kernel@...r.kernel.org,
knaack.h@....de, lars@...afoo.de, fabrice.gasnier@...com,
akinobu.mita@...il.com, marek.vasut+renesas@...il.com,
jacopo+renesas@...ndi.org, mike.looijmans@...ic.nl,
peda@...ntia.se, jeff.dagenais@...il.com,
linux-iio@...r.kernel.org, linux-gpio@...r.kernel.org,
mark.rutland@....com, devicetree@...r.kernel.org, lukas@...ner.de,
adi.reus@...il.com
Subject: Re: [PATCH 5/5] lmp92001: mfd: doc: Add support LMP92001
On Mon, 11 Sep 2017 16:58:41 -0500
Rob Herring <robh@...nel.org> wrote:
> On Thu, Aug 31, 2017 at 01:25:28AM +0700, s.abhisit@...il.com wrote:
> > From: Abhisit Sangjan <s.abhisit@...il.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.
> >
> > Signed-off-by: Abhisit Sangjan <s.abhisit@...il.com>
> > ---
> > 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@...r.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
Powered by blists - more mailing lists