2013-07-30 00:42:40

by Kim, Milo

[permalink] [raw]
Subject: [PATCH v2 4/4] Documentation: dt: bindings: add LP3943 bindings

Bindings for LP3943 MFD, GPIO and PWM controller are added.

Cc: [email protected]
Cc: Lee Jones <[email protected]>
Cc: Linus Walleij <[email protected]>
Cc: Samuel Ortiz <[email protected]>
Cc: Thierry Reding <[email protected]>
Signed-off-by: Milo Kim <[email protected]>
---
.../devicetree/bindings/gpio/gpio-lp3943.txt | 42 +++++++++++++
Documentation/devicetree/bindings/mfd/lp3943.txt | 33 ++++++++++
.../devicetree/bindings/pwm/pwm-lp3943.txt | 65 ++++++++++++++++++++
3 files changed, 140 insertions(+)
create mode 100644 Documentation/devicetree/bindings/gpio/gpio-lp3943.txt
create mode 100644 Documentation/devicetree/bindings/mfd/lp3943.txt
create mode 100644 Documentation/devicetree/bindings/pwm/pwm-lp3943.txt

diff --git a/Documentation/devicetree/bindings/gpio/gpio-lp3943.txt b/Documentation/devicetree/bindings/gpio/gpio-lp3943.txt
new file mode 100644
index 0000000..952fefd
--- /dev/null
+++ b/Documentation/devicetree/bindings/gpio/gpio-lp3943.txt
@@ -0,0 +1,42 @@
+TI/National Semiconductor LP3943 GPIO controller
+
+Required properties:
+ - compatible: "ti,lp3943-gpio"
+ - gpio-controller: device node as a GPIO controller
+ - #gpio-cells: Should be two.
+ 1) pin number offset
+ 2) GPIO polarity
+ 0 = active high, 1 = active low
+
+For the GPIO properties please refer to:
+Documentation/devicetree/bindings/gpio/gpio.txt
+
+Example:
+Simple LED controls with LP3943 GPIO controller
+
+&i2c4 {
+ lp3943@60 {
+ compatible = "ti,lp3943";
+ reg = <0x60>;
+
+ gpioex: gpio {
+ compatible = "ti,lp3943-gpio";
+ gpio-controller;
+ #gpio-cells = <2>;
+ };
+ };
+};
+
+leds {
+ compatible = "gpio-leds";
+ indicator1 {
+ label = "indi1";
+ gpios = <&gpioex 9 1>; /* active-low */
+ };
+
+ indicator2 {
+ label = "indi2";
+ gpios = <&gpioex 10 1>; /* active-low */
+ default-state = "off";
+ };
+};
diff --git a/Documentation/devicetree/bindings/mfd/lp3943.txt b/Documentation/devicetree/bindings/mfd/lp3943.txt
new file mode 100644
index 0000000..ef62784
--- /dev/null
+++ b/Documentation/devicetree/bindings/mfd/lp3943.txt
@@ -0,0 +1,33 @@
+TI/National Semiconductor LP3943 MFD driver
+
+Required properties:
+ - compatible: "ti,lp3943"
+ - reg: I2C slave address (u8). 0x60 ~ 0x67
+
+LP3943 consists of two sub-devices, lp3943-gpio and lp3943-pwm.
+
+For the LP3943 GPIO properties please refer to:
+Documentation/devicetree/bindings/gpio/gpio-lp3943.txt
+
+For the LP3943 PWM properties please refer to:
+Documentation/devicetree/bindings/pwm/pwm-lp3943.txt
+
+Example:
+
+lp3943@60 {
+ compatible = "ti,lp3943";
+ reg = <0x60>;
+
+ gpioex: gpio {
+ compatible = "ti,lp3943-gpio";
+ gpio-controller;
+ #gpio-cells = <2>;
+ };
+
+ pwm3943: pwm {
+ compatible = "ti,lp3943-pwm";
+ #pwm-cells = <3>;
+ ti,pwm0 = < 8 9 10 >;
+ ti,pwm1 = <16>;
+ };
+};
diff --git a/Documentation/devicetree/bindings/pwm/pwm-lp3943.txt b/Documentation/devicetree/bindings/pwm/pwm-lp3943.txt
new file mode 100644
index 0000000..3a6c7fc
--- /dev/null
+++ b/Documentation/devicetree/bindings/pwm/pwm-lp3943.txt
@@ -0,0 +1,65 @@
+TI/National Semiconductor LP3943 PWM controller
+
+Required properties:
+ - compatible: "ti,lp3943-pwm"
+ - #pwm-cells: Should be three.
+ 1) index of PWM controller
+ 0 = PWM 0, 1 = PWM 1
+ 2) PWM signal period in nanoseconds
+ min = 6250, max = 1600000
+ 3) bitmask of optional PWM polarity flags
+ 0 = normal, 1 = inversed
+ - ti,pwm0 or ti,pwm1: Output pin number(s) for PWM port 0 or 1.
+ 0 = invalid
+ 1 = output 0
+ 2 = output 1
+ .
+ .
+ 16 = output 15
+
+For the PWM properties please refer to:
+Documentation/devicetree/bindings/pwm/pwm.txt
+
+Example:
+PWM 0 is for RGB LED brightness control
+PWM 1 is for brightness control of LP8557 backlight device
+
+&i2c3 {
+ lp3943@60 {
+ compatible = "ti,lp3943";
+ reg = <0x60>;
+
+ /*
+ * PWM 0 : output 7, 8 and 9
+ * PWM 1 : output 15
+ */
+ pwm3943: pwm {
+ compatible = "ti,lp3943-pwm";
+ #pwm-cells = <3>;
+ ti,pwm0 = < 8 9 10 >;
+ ti,pwm1 = <16>;
+ };
+ };
+
+};
+
+/* LEDs control with PWM 0 of LP3943 */
+pwmleds {
+ compatible = "pwm-leds";
+ rgb {
+ label = "indi::rgb";
+ pwms = <&pwm3943 0 10000 0>;
+ max-brightness = <255>;
+ };
+};
+
+&i2c4 {
+ /* Backlight control with PWM 1 of LP3943 */
+ backlight@2c {
+ compatible = "ti,lp8557";
+ reg = <0x2c>;
+
+ pwms = <&pwm3943 1 10000 1>; /* inversed */
+ pwm-names = "lp8557";
+ };
+};
--
1.7.9.5


Best Regards,
Milo


????{.n?+???????+%?????ݶ??w??{.n?+????{??G?????{ay?ʇڙ?,j??f???h?????????z_??(?階?ݢj"???m??????G????????????&???~???iO???z??v?^?m???? ????????I?


2013-07-31 11:09:37

by Lee Jones

[permalink] [raw]
Subject: Re: [PATCH v2 4/4] Documentation: dt: bindings: add LP3943 bindings

On Tue, 30 Jul 2013, Kim, Milo wrote:

> Bindings for LP3943 MFD, GPIO and PWM controller are added.
>
> Cc: [email protected]
> Cc: Lee Jones <[email protected]>
> Cc: Linus Walleij <[email protected]>
> Cc: Samuel Ortiz <[email protected]>
> Cc: Thierry Reding <[email protected]>
> Signed-off-by: Milo Kim <[email protected]>
> ---
> .../devicetree/bindings/gpio/gpio-lp3943.txt | 42 +++++++++++++
> Documentation/devicetree/bindings/mfd/lp3943.txt | 33 ++++++++++
> .../devicetree/bindings/pwm/pwm-lp3943.txt | 65 ++++++++++++++++++++
> 3 files changed, 140 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/gpio/gpio-lp3943.txt
> create mode 100644 Documentation/devicetree/bindings/mfd/lp3943.txt
> create mode 100644 Documentation/devicetree/bindings/pwm/pwm-lp3943.txt
>
> diff --git a/Documentation/devicetree/bindings/gpio/gpio-lp3943.txt b/Documentation/devicetree/bindings/gpio/gpio-lp3943.txt
> new file mode 100644
> index 0000000..952fefd
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/gpio/gpio-lp3943.txt
> @@ -0,0 +1,42 @@
> +TI/National Semiconductor LP3943 GPIO controller
> +
> +Required properties:
> + - compatible: "ti,lp3943-gpio"
> + - gpio-controller: device node as a GPIO controller
> + - #gpio-cells: Should be two.
> + 1) pin number offset
> + 2) GPIO polarity
> + 0 = active high, 1 = active low

Please use the defines instead on '0' and '1':
include/dt-bindings/gpio/gpio.h

> + gpios = <&gpioex 9 1>; /* active-low */

> + gpios = <&gpioex 10 1>; /* active-low */

--
Lee Jones
Linaro ST-Ericsson Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

2013-08-14 10:58:46

by Thierry Reding

[permalink] [raw]
Subject: Re: [PATCH v2 4/4] Documentation: dt: bindings: add LP3943 bindings

On Tue, Jul 30, 2013 at 12:42:29AM +0000, Kim, Milo wrote:
[...]
> diff --git a/Documentation/devicetree/bindings/gpio/gpio-lp3943.txt b/Documentation/devicetree/bindings/gpio/gpio-lp3943.txt
[...]
> +Required properties:
> + - compatible: "ti,lp3943-gpio"
> + - gpio-controller: device node as a GPIO controller

"Marks the device node as a GPIO controller"?

> + - #gpio-cells: Should be two.

"Should be 2"?

[...]
> + gpios = <&gpioex 9 1>; /* active-low */
[...]
> + gpios = <&gpioex 10 1>; /* active-low */

If you use the defines as Lee suggested, then you don't need these
comments either.

> diff --git a/Documentation/devicetree/bindings/mfd/lp3943.txt b/Documentation/devicetree/bindings/mfd/lp3943.txt
> new file mode 100644
> index 0000000..ef62784
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mfd/lp3943.txt
> @@ -0,0 +1,33 @@
> +TI/National Semiconductor LP3943 MFD driver
> +
> +Required properties:
> + - compatible: "ti,lp3943"
> + - reg: I2C slave address (u8). 0x60 ~ 0x67

u8 is a Linux specific and implementation detail. I think it's safe to
assume that everybody knows that I2C slave addresses are 7 bits (and
you even provide a valid range), so just drop the "(u8)".

> +lp3943@60 {
> + compatible = "ti,lp3943";
> + reg = <0x60>;
> +
> + gpioex: gpio {
> + compatible = "ti,lp3943-gpio";
> + gpio-controller;
> + #gpio-cells = <2>;
> + };
> +
> + pwm3943: pwm {
> + compatible = "ti,lp3943-pwm";
> + #pwm-cells = <3>;
> + ti,pwm0 = < 8 9 10 >;

This should probably be written as "<8 9 10>" for consistency.

> diff --git a/Documentation/devicetree/bindings/pwm/pwm-lp3943.txt b/Documentation/devicetree/bindings/pwm/pwm-lp3943.txt
[...]
> +Required properties:
> + - compatible: "ti,lp3943-pwm"
> + - #pwm-cells: Should be three.

"Should be three"? Although since as I mentioned the chip doesn't
support polarity inversion, this should now be "2".

> + 1) index of PWM controller
> + 0 = PWM 0, 1 = PWM 1
> + 2) PWM signal period in nanoseconds
> + min = 6250, max = 1600000
> + 3) bitmask of optional PWM polarity flags
> + 0 = normal, 1 = inversed

All of this can probably go away. The new way of doing this is to
reference the standard bindings document using something like the below:

- #pwm-cells: Should be 2. See pwm.txt in this directory for a
description of the cells format.

Since you have a restriction on the minimum and maximum period lengths,
you could add something like:

Note that this hardware limits the period length to the
range 6250-1600000.

> + - ti,pwm0 or ti,pwm1: Output pin number(s) for PWM port 0 or 1.

"PWM port" -> "PWM channel"

> + 0 = invalid
> + 1 = output 0
> + 2 = output 1
> + .
> + .
> + 16 = output 15

I thought I read that this is now a 1:1 mapping?

> +Example:
> +PWM 0 is for RGB LED brightness control
> +PWM 1 is for brightness control of LP8557 backlight device
> +
> +&i2c3 {
> + lp3943@60 {
> + compatible = "ti,lp3943";
> + reg = <0x60>;
> +
> + /*
> + * PWM 0 : output 7, 8 and 9
> + * PWM 1 : output 15
> + */
> + pwm3943: pwm {
> + compatible = "ti,lp3943-pwm";
> + #pwm-cells = <3>;
> + ti,pwm0 = < 8 9 10 >;

And here as well: "< 8 9 10 >" -> "<8 9 10>".

Thierry


Attachments:
(No filename) (3.22 kB)
(No filename) (836.00 B)
Download all attachments