Thread (3 messages) 3 messages, 3 authors, 2013-08-14

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

From: Thierry Reding <hidden>
Date: 2013-08-14 10:58:46
Also in: linux-pwm, lkml

On Tue, Jul 30, 2013 at 12:42:29AM +0000, Kim, Milo wrote:
[...]
quoted hunk ↗ jump to hunk
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.
quoted hunk ↗ jump to hunk
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.
quoted hunk ↗ jump to hunk
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

  • (unnamed) [application/pgp-signature] 836 bytes
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help