devicetree.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3 4/4] Documentation: add LP3943 DT bindings and document
@ 2013-09-12  1:35 Milo Kim
  2013-09-12 15:24 ` Lee Jones
  0 siblings, 1 reply; 8+ messages in thread
From: Milo Kim @ 2013-09-12  1:35 UTC (permalink / raw)
  To: Lee Jones, Samuel Ortiz
  Cc: linux-kernel, linux-pwm, Linus Walleij, Thierry Reding,
	devicetree, Milo Kim

Bindings for LP3943 MFD, GPIO and PWM controller are added.
And LP3943 driver document is added also.

Cc: devicetree@vger.kernel.org
Cc: Lee Jones <lee.jones@linaro.org>
Cc: Linus Walleij <linus.walleij@linaro.org>
Cc: Samuel Ortiz <sameo@linux.intel.com>
Cc: Thierry Reding <thierry.reding@gmail.com>
Signed-off-by: Milo Kim <milo.kim@ti.com>
---
 Documentation/00-INDEX                             |    2 +
 .../devicetree/bindings/gpio/gpio-lp3943.txt       |   37 ++++++++++++
 Documentation/devicetree/bindings/mfd/lp3943.txt   |   33 +++++++++++
 .../devicetree/bindings/pwm/pwm-lp3943.txt         |   58 ++++++++++++++++++
 Documentation/lp3943.txt                           |   62 ++++++++++++++++++++
 5 files changed, 192 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
 create mode 100644 Documentation/lp3943.txt

diff --git a/Documentation/00-INDEX b/Documentation/00-INDEX
index 0c4cc68..5dd921f 100644
--- a/Documentation/00-INDEX
+++ b/Documentation/00-INDEX
@@ -285,6 +285,8 @@ logo.gif
 	- full colour GIF image of Linux logo (penguin - Tux).
 logo.txt
 	- info on creator of above logo & site to get additional images from.
+lp3943.txt
+	- info on LP3943 MFD driver structure.
 m68k/
 	- directory with info about Linux on Motorola 68k architecture.
 magic-number.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..80fcb7d
--- /dev/null
+++ b/Documentation/devicetree/bindings/gpio/gpio-lp3943.txt
@@ -0,0 +1,37 @@
+TI/National Semiconductor LP3943 GPIO controller
+
+Required properties:
+  - compatible: "ti,lp3943-gpio"
+  - gpio-controller: Marks the device node as a GPIO controller.
+  - #gpio-cells: Should be 2. See gpio.txt in this directory for a
+                 description of the cells format.
+
+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 GPIO_ACTIVE_LOW>;
+	};
+
+	indicator2 {
+		label = "indi2";
+		gpios = <&gpioex 10 GPIO_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..e8591d6
--- /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. From 0x60 to 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 = <2>;
+		ti,pwm0 = <8 9 10>;
+		ti,pwm1 = <15>;
+	};
+};
diff --git a/Documentation/devicetree/bindings/pwm/pwm-lp3943.txt b/Documentation/devicetree/bindings/pwm/pwm-lp3943.txt
new file mode 100644
index 0000000..7bd9d3b
--- /dev/null
+++ b/Documentation/devicetree/bindings/pwm/pwm-lp3943.txt
@@ -0,0 +1,58 @@
+TI/National Semiconductor LP3943 PWM controller
+
+Required properties:
+  - compatible: "ti,lp3943-pwm"
+  - #pwm-cells: Should be 2. See pwm.txt in this directory for a
+                description of the cells format.
+                Note that this hardware limits the period length to the
+                range 6250~1600000.
+  - ti,pwm0 or ti,pwm1: Output pin number(s) for PWM channel 0 or 1.
+    0 = output 0
+    1 = output 1
+    .
+    .
+    15 = output 15
+
+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 8, 9 and 10
+		 * PWM 1 : output 15
+		 */
+		pwm3943: pwm {
+			compatible = "ti,lp3943-pwm";
+			#pwm-cells = <2>;
+			ti,pwm0 = <8 9 10>;
+			ti,pwm1 = <15>;
+		};
+	};
+
+};
+
+/* LEDs control with PWM 0 of LP3943 */
+pwmleds {
+	compatible = "pwm-leds";
+	rgb {
+		label = "indi::rgb";
+		pwms = <&pwm3943 0 10000>;
+		max-brightness = <255>;
+	};
+};
+
+&i2c4 {
+	/* Backlight control with PWM 1 of LP3943 */
+	backlight@2c {
+		compatible = "ti,lp8557";
+		reg = <0x2c>;
+
+		pwms = <&pwm3943 1 10000>;
+		pwm-names = "lp8557";
+	};
+};
diff --git a/Documentation/lp3943.txt b/Documentation/lp3943.txt
new file mode 100644
index 0000000..576ebd0
--- /dev/null
+++ b/Documentation/lp3943.txt
@@ -0,0 +1,62 @@
+TI/National Semiconductor LP3943 MFD driver
+===========================================
+
+LP3943 is an integrated device capable of driving 16 output channels.
+It can be used for GPIO expander and PWM generators.
+LP3493 registers are controlled via the I2C interface.
+
+Driver structure
+----------------
+                                  LED control    General usage for a device
+                                  ___________   ____________________________
+
+  LP3943 MFD ---- GPIO expander    leds-gpio        eg) HW enable pin
+              |
+              --- PWM generator    leds-pwm         eg) PWM input
+
+
+Why do we need GPIO and PWM drivers instead of LED driver?
+To support LED control and general usage, GPIO and PWM drivers are necessary.
+
+According to the datasheet(1), it's just a LED driver which has 16 channels.
+But here is another application, a GPIO expander.(2)
+
+  (1) http://www.ti.com/lit/ds/snvs256b/snvs256b.pdf
+  (2) http://www.ti.com/lit/an/snva287a/snva287a.pdf
+
+Internal two PWM channels are used for LED dimming effect.
+And each output pin can be used as a GPIO as well.
+LED functionality can work with GPIOs or PWMs.
+LEDs can be controlled with legacy leds-gpio(static brightness) or
+leds-pwm drivers(dynamic brightness control).
+Additionally, it can be used for generic GPIO and PWM controller.
+For example, a GPIO is HW enable pin of a device.
+PWM is input pin of a backlight device.
+
+
+LP3943 PWM port map
+-------------------
+Each PWM channel can be mapped to one or multiple output pins.
+
+For example, PWM 0 is used for a backlight device.
+PWM 1 is for RGB LEDs.
+
+  PWM channel    Output pins    PWM consumer
+  ___________    ___________    ____________
+  PWM 0          pin 1          backlight
+  PWM 1          pin 7, 8, 9    RGB LEDs
+
+Then, PWM port map is as below.
+PWM 0: num_outputs = 1, output = pin 1
+PWM 1: num_outputs = 3, output = pin 7, 8, 9
+
+The 'lp3943_pwm_map' structure is used for this feature.
+
+
+Device tree supported
+---------------------
+Please refer to the documents below.
+
+Documentation/devicetree/bindings/mfd/lp3943.txt
+Documentation/devicetree/bindings/gpio/gpio-lp3943.txt
+Documentation/devicetree/bindings/pwm/pwm-lp3943.txt
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH v3 4/4] Documentation: add LP3943 DT bindings and document
  2013-09-12  1:35 [PATCH v3 4/4] Documentation: add LP3943 DT bindings and document Milo Kim
@ 2013-09-12 15:24 ` Lee Jones
  2013-09-15 11:27   ` Thierry Reding
  2013-09-22 14:35   ` Milo Kim
  0 siblings, 2 replies; 8+ messages in thread
From: Lee Jones @ 2013-09-12 15:24 UTC (permalink / raw)
  To: Milo Kim
  Cc: Samuel Ortiz, linux-kernel, linux-pwm, Linus Walleij,
	Thierry Reding, devicetree

On Thu, 12 Sep 2013, Milo Kim wrote:

> Bindings for LP3943 MFD, GPIO and PWM controller are added.
> And LP3943 driver document is added also.
> 
> Cc: devicetree@vger.kernel.org
> Cc: Lee Jones <lee.jones@linaro.org>
> Cc: Linus Walleij <linus.walleij@linaro.org>
> Cc: Samuel Ortiz <sameo@linux.intel.com>
> Cc: Thierry Reding <thierry.reding@gmail.com>
> Signed-off-by: Milo Kim <milo.kim@ti.com>
> ---
>  Documentation/00-INDEX                             |    2 +
>  .../devicetree/bindings/gpio/gpio-lp3943.txt       |   37 ++++++++++++
>  Documentation/devicetree/bindings/mfd/lp3943.txt   |   33 +++++++++++
>  .../devicetree/bindings/pwm/pwm-lp3943.txt         |   58 ++++++++++++++++++
>  Documentation/lp3943.txt                           |   62 ++++++++++++++++++++

I don't think we want driver documentation here.

>  5 files changed, 192 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
>  create mode 100644 Documentation/lp3943.txt
> 
> diff --git a/Documentation/00-INDEX b/Documentation/00-INDEX
> index 0c4cc68..5dd921f 100644
> --- a/Documentation/00-INDEX
> +++ b/Documentation/00-INDEX
> @@ -285,6 +285,8 @@ logo.gif
>  	- full colour GIF image of Linux logo (penguin - Tux).
>  logo.txt
>  	- info on creator of above logo & site to get additional images from.
> +lp3943.txt
> +	- info on LP3943 MFD driver structure.

-- " --

> diff --git a/Documentation/devicetree/bindings/gpio/gpio-lp3943.txt b/Documentation/devicetree/bindings/gpio/gpio-lp3943.txt
> diff --git a/Documentation/devicetree/bindings/mfd/lp3943.txt b/Documentation/devicetree/bindings/mfd/lp3943.txt
> diff --git a/Documentation/devicetree/bindings/pwm/pwm-lp3943.txt b/Documentation/devicetree/bindings/pwm/pwm-lp3943.txt
> diff --git a/Documentation/lp3943.txt b/Documentation/lp3943.txt
> new file mode 100644
> index 0000000..576ebd0
> --- /dev/null
> +++ b/Documentation/lp3943.txt
> @@ -0,0 +1,62 @@
> +TI/National Semiconductor LP3943 MFD driver
> +===========================================
> +
> +LP3943 is an integrated device capable of driving 16 output channels.
> +It can be used for GPIO expander and PWM generators.
> +LP3493 registers are controlled via the I2C interface.
> +
> +Driver structure
> +----------------
> +                                  LED control    General usage for a device
> +                                  ___________   ____________________________
> +
> +  LP3943 MFD ---- GPIO expander    leds-gpio        eg) HW enable pin
> +              |
> +              --- PWM generator    leds-pwm         eg) PWM input
> +
> +
> +Why do we need GPIO and PWM drivers instead of LED driver?
> +To support LED control and general usage, GPIO and PWM drivers are necessary.
> +
> +According to the datasheet(1), it's just a LED driver which has 16 channels.
> +But here is another application, a GPIO expander.(2)
> +
> +  (1) http://www.ti.com/lit/ds/snvs256b/snvs256b.pdf
> +  (2) http://www.ti.com/lit/an/snva287a/snva287a.pdf
> +
> +Internal two PWM channels are used for LED dimming effect.
> +And each output pin can be used as a GPIO as well.
> +LED functionality can work with GPIOs or PWMs.
> +LEDs can be controlled with legacy leds-gpio(static brightness) or
> +leds-pwm drivers(dynamic brightness control).
> +Additionally, it can be used for generic GPIO and PWM controller.
> +For example, a GPIO is HW enable pin of a device.
> +PWM is input pin of a backlight device.
> +
> +
> +LP3943 PWM port map
> +-------------------
> +Each PWM channel can be mapped to one or multiple output pins.
> +
> +For example, PWM 0 is used for a backlight device.
> +PWM 1 is for RGB LEDs.
> +
> +  PWM channel    Output pins    PWM consumer
> +  ___________    ___________    ____________
> +  PWM 0          pin 1          backlight
> +  PWM 1          pin 7, 8, 9    RGB LEDs
> +
> +Then, PWM port map is as below.
> +PWM 0: num_outputs = 1, output = pin 1
> +PWM 1: num_outputs = 3, output = pin 7, 8, 9
> +
> +The 'lp3943_pwm_map' structure is used for this feature.
> +
> +
> +Device tree supported
> +---------------------
> +Please refer to the documents below.
> +
> +Documentation/devicetree/bindings/mfd/lp3943.txt
> +Documentation/devicetree/bindings/gpio/gpio-lp3943.txt
> +Documentation/devicetree/bindings/pwm/pwm-lp3943.txt

Why do you need to document your driver in this way?

If this stuff is really important (and most of it really isn't), then
put it either in the commit log or in the driver.

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

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v3 4/4] Documentation: add LP3943 DT bindings and document
  2013-09-12 15:24 ` Lee Jones
@ 2013-09-15 11:27   ` Thierry Reding
  2013-09-16  7:30     ` Lee Jones
  2013-09-22 14:35   ` Milo Kim
  1 sibling, 1 reply; 8+ messages in thread
From: Thierry Reding @ 2013-09-15 11:27 UTC (permalink / raw)
  To: Lee Jones
  Cc: Milo Kim, Samuel Ortiz, linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-pwm-u79uwXL29TY76Z2rM5mHXA, Linus Walleij,
	devicetree-u79uwXL29TY76Z2rM5mHXA

[-- Attachment #1: Type: text/plain, Size: 844 bytes --]

On Thu, Sep 12, 2013 at 04:24:55PM +0100, Lee Jones wrote:
> On Thu, 12 Sep 2013, Milo Kim wrote:
[...]
> > diff --git a/Documentation/lp3943.txt b/Documentation/lp3943.txt
> > new file mode 100644
> > index 0000000..576ebd0
> > --- /dev/null
> > +++ b/Documentation/lp3943.txt
[...]
> If this stuff is really important (and most of it really isn't), then
> put it either in the commit log or in the driver.

I can only speak for myself, but the initial driver commit message is
about the last place I look for this kind of information. I'd expect it
to be either somewhere in Documentation/ or in one of the source files.
In this case I'd probably look at the MFD core driver source file. More
likely, even, I'd just go google "LP3943" and grab a public datasheet
from TI where all of this can hopefully be found.

Thierry

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v3 4/4] Documentation: add LP3943 DT bindings and document
  2013-09-15 11:27   ` Thierry Reding
@ 2013-09-16  7:30     ` Lee Jones
  2013-09-19 11:25       ` Thierry Reding
  0 siblings, 1 reply; 8+ messages in thread
From: Lee Jones @ 2013-09-16  7:30 UTC (permalink / raw)
  To: Thierry Reding
  Cc: Milo Kim, Samuel Ortiz, linux-kernel, linux-pwm, Linus Walleij,
	devicetree

On Sun, 15 Sep 2013, Thierry Reding wrote:

> On Thu, Sep 12, 2013 at 04:24:55PM +0100, Lee Jones wrote:
> > On Thu, 12 Sep 2013, Milo Kim wrote:
> [...]
> > > diff --git a/Documentation/lp3943.txt b/Documentation/lp3943.txt
> > > new file mode 100644
> > > index 0000000..576ebd0
> > > --- /dev/null
> > > +++ b/Documentation/lp3943.txt
> [...]
> > If this stuff is really important (and most of it really isn't), then
> > put it either in the commit log or in the driver.
> 
> I can only speak for myself, but the initial driver commit message is
> about the last place I look for this kind of information. I'd expect it
> to be either somewhere in Documentation/ or in one of the source files.
> In this case I'd probably look at the MFD core driver source file. More
> likely, even, I'd just go google "LP3943" and grab a public datasheet
> from TI where all of this can hopefully be found.

The latter option is the most common. If the datasheet is publicly
available, then there is no rhyme nor reason to put it in the kernel
source too. /Documentation is already overloaded, it would be even
more so if each and every driver residing in the kernel decided to
bulk it up further.

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

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v3 4/4] Documentation: add LP3943 DT bindings and document
  2013-09-16  7:30     ` Lee Jones
@ 2013-09-19 11:25       ` Thierry Reding
  2013-09-19 11:56         ` Lee Jones
  0 siblings, 1 reply; 8+ messages in thread
From: Thierry Reding @ 2013-09-19 11:25 UTC (permalink / raw)
  To: Lee Jones
  Cc: Milo Kim, Samuel Ortiz, linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-pwm-u79uwXL29TY76Z2rM5mHXA, Linus Walleij,
	devicetree-u79uwXL29TY76Z2rM5mHXA

[-- Attachment #1: Type: text/plain, Size: 1547 bytes --]

On Mon, Sep 16, 2013 at 08:30:00AM +0100, Lee Jones wrote:
> On Sun, 15 Sep 2013, Thierry Reding wrote:
> 
> > On Thu, Sep 12, 2013 at 04:24:55PM +0100, Lee Jones wrote:
> > > On Thu, 12 Sep 2013, Milo Kim wrote:
> > [...]
> > > > diff --git a/Documentation/lp3943.txt b/Documentation/lp3943.txt
> > > > new file mode 100644
> > > > index 0000000..576ebd0
> > > > --- /dev/null
> > > > +++ b/Documentation/lp3943.txt
> > [...]
> > > If this stuff is really important (and most of it really isn't), then
> > > put it either in the commit log or in the driver.
> > 
> > I can only speak for myself, but the initial driver commit message is
> > about the last place I look for this kind of information. I'd expect it
> > to be either somewhere in Documentation/ or in one of the source files.
> > In this case I'd probably look at the MFD core driver source file. More
> > likely, even, I'd just go google "LP3943" and grab a public datasheet
> > from TI where all of this can hopefully be found.
> 
> The latter option is the most common. If the datasheet is publicly
> available, then there is no rhyme nor reason to put it in the kernel
> source too. /Documentation is already overloaded, it would be even
> more so if each and every driver residing in the kernel decided to
> bulk it up further.

Okay. So in this case perhaps Milo should simply drop the documentation
file. Perhaps link to the public datasheet from on of the source files,
although that could be problematic if the URL isn't permanent.

Thierry

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v3 4/4] Documentation: add LP3943 DT bindings and document
  2013-09-19 11:25       ` Thierry Reding
@ 2013-09-19 11:56         ` Lee Jones
  0 siblings, 0 replies; 8+ messages in thread
From: Lee Jones @ 2013-09-19 11:56 UTC (permalink / raw)
  To: Thierry Reding
  Cc: Milo Kim, Samuel Ortiz, linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-pwm-u79uwXL29TY76Z2rM5mHXA, Linus Walleij,
	devicetree-u79uwXL29TY76Z2rM5mHXA

On Thu, 19 Sep 2013, Thierry Reding wrote:

> On Mon, Sep 16, 2013 at 08:30:00AM +0100, Lee Jones wrote:
> > On Sun, 15 Sep 2013, Thierry Reding wrote:
> > 
> > > On Thu, Sep 12, 2013 at 04:24:55PM +0100, Lee Jones wrote:
> > > > On Thu, 12 Sep 2013, Milo Kim wrote:
> > > [...]
> > > > > diff --git a/Documentation/lp3943.txt b/Documentation/lp3943.txt
> > > > > new file mode 100644
> > > > > index 0000000..576ebd0
> > > > > --- /dev/null
> > > > > +++ b/Documentation/lp3943.txt
> > > [...]
> > > > If this stuff is really important (and most of it really isn't), then
> > > > put it either in the commit log or in the driver.
> > > 
> > > I can only speak for myself, but the initial driver commit message is
> > > about the last place I look for this kind of information. I'd expect it
> > > to be either somewhere in Documentation/ or in one of the source files.
> > > In this case I'd probably look at the MFD core driver source file. More
> > > likely, even, I'd just go google "LP3943" and grab a public datasheet
> > > from TI where all of this can hopefully be found.
> > 
> > The latter option is the most common. If the datasheet is publicly
> > available, then there is no rhyme nor reason to put it in the kernel
> > source too. /Documentation is already overloaded, it would be even
> > more so if each and every driver residing in the kernel decided to
> > bulk it up further.
> 
> Okay. So in this case perhaps Milo should simply drop the documentation
> file. Perhaps link to the public datasheet from on of the source files,
> although that could be problematic if the URL isn't permanent.

I'm not sure even that's required.

I just Googled it and found the datasheet on the first hit.

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
--
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

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v3 4/4] Documentation: add LP3943 DT bindings and document
  2013-09-12 15:24 ` Lee Jones
  2013-09-15 11:27   ` Thierry Reding
@ 2013-09-22 14:35   ` Milo Kim
  2013-09-23  7:58     ` Lee Jones
  1 sibling, 1 reply; 8+ messages in thread
From: Milo Kim @ 2013-09-22 14:35 UTC (permalink / raw)
  To: Lee Jones
  Cc: Samuel Ortiz, linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-pwm-u79uwXL29TY76Z2rM5mHXA, Linus Walleij, Thierry Reding,
	devicetree-u79uwXL29TY76Z2rM5mHXA


Hi Lee,

On 09/13/2013 12:24 AM, Lee Jones wrote:
> On Thu, 12 Sep 2013, Milo Kim wrote:

...

>> new file mode 100644
>> index 0000000..576ebd0
>> --- /dev/null
>> +++ b/Documentation/lp3943.txt
>> @@ -0,0 +1,62 @@
>> +TI/National Semiconductor LP3943 MFD driver
>> +===========================================
>> +
>> +LP3943 is an integrated device capable of driving 16 output channels.
>> +It can be used for GPIO expander and PWM generators.
>> +LP3493 registers are controlled via the I2C interface.
>> +
>> +Driver structure
>> +----------------
>> +                                  LED control    General usage for a device
>> +                                  ___________   ____________________________
>> +
>> +  LP3943 MFD ---- GPIO expander    leds-gpio        eg) HW enable pin
>> +              |
>> +              --- PWM generator    leds-pwm         eg) PWM input
>> +
>> +
>> +Why do we need GPIO and PWM drivers instead of LED driver?
>> +To support LED control and general usage, GPIO and PWM drivers are necessary.
>> +
>> +According to the datasheet(1), it's just a LED driver which has 16 channels.
>> +But here is another application, a GPIO expander.(2)
>> +
>> +  (1) http://www.ti.com/lit/ds/snvs256b/snvs256b.pdf
>> +  (2) http://www.ti.com/lit/an/snva287a/snva287a.pdf
>> +
>> +Internal two PWM channels are used for LED dimming effect.
>> +And each output pin can be used as a GPIO as well.
>> +LED functionality can work with GPIOs or PWMs.
>> +LEDs can be controlled with legacy leds-gpio(static brightness) or
>> +leds-pwm drivers(dynamic brightness control).
>> +Additionally, it can be used for generic GPIO and PWM controller.
>> +For example, a GPIO is HW enable pin of a device.
>> +PWM is input pin of a backlight device.
>> +
>> +
>> +LP3943 PWM port map
>> +-------------------
>> +Each PWM channel can be mapped to one or multiple output pins.
>> +
>> +For example, PWM 0 is used for a backlight device.
>> +PWM 1 is for RGB LEDs.
>> +
>> +  PWM channel    Output pins    PWM consumer
>> +  ___________    ___________    ____________
>> +  PWM 0          pin 1          backlight
>> +  PWM 1          pin 7, 8, 9    RGB LEDs
>> +
>> +Then, PWM port map is as below.
>> +PWM 0: num_outputs = 1, output = pin 1
>> +PWM 1: num_outputs = 3, output = pin 7, 8, 9
>> +
>> +The 'lp3943_pwm_map' structure is used for this feature.
>> +
>> +
>> +Device tree supported
>> +---------------------
>> +Please refer to the documents below.
>> +
>> +Documentation/devicetree/bindings/mfd/lp3943.txt
>> +Documentation/devicetree/bindings/gpio/gpio-lp3943.txt
>> +Documentation/devicetree/bindings/pwm/pwm-lp3943.txt
>
> Why do you need to document your driver in this way?
>
> If this stuff is really important (and most of it really isn't), then
> put it either in the commit log or in the driver.

Unnecessary documentation makes it noisy, but I think the LP3943 still 
needs the driver documentation for better understanding.

Many people think LP3943 is just LED driver, but I really want to share 
the application usages - PWM generators and GPIO expanders.
If the driver just supports LED functionality, then it would be created 
as LED class driver. However, this patch-set enables more generic driver 
usages.
So, this documentation would be helpful.

And I want to keep the code and the documentation separate.
If the link address is changed, then only documentation file will be 
modified, not source file.

So, I'd like to create the fifth patch-set with new MFD documentation 
subdirectory.
Additionally, LP3943 platform data example code will be added in the 
'Documentation/mfd/lp3943.txt'. It's for a platform which doesn't 
support the device tree.

Best regards,
Milo
--
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

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v3 4/4] Documentation: add LP3943 DT bindings and document
  2013-09-22 14:35   ` Milo Kim
@ 2013-09-23  7:58     ` Lee Jones
  0 siblings, 0 replies; 8+ messages in thread
From: Lee Jones @ 2013-09-23  7:58 UTC (permalink / raw)
  To: Milo Kim
  Cc: Samuel Ortiz, linux-kernel, linux-pwm, Linus Walleij,
	Thierry Reding, devicetree

> >>+++ b/Documentation/lp3943.txt
> >>@@ -0,0 +1,62 @@
> >>+TI/National Semiconductor LP3943 MFD driver
> >>+===========================================

<snip>

> >Why do you need to document your driver in this way?
> >
> >If this stuff is really important (and most of it really isn't), then
> >put it either in the commit log or in the driver.
> 
> Unnecessary documentation makes it noisy, but I think the LP3943
> still needs the driver documentation for better understanding.
> 
> Many people think LP3943 is just LED driver, but I really want to
> share the application usages - PWM generators and GPIO expanders.
> If the driver just supports LED functionality, then it would be
> created as LED class driver. However, this patch-set enables more
> generic driver usages.
> So, this documentation would be helpful.
> 
> And I want to keep the code and the documentation separate.
> If the link address is changed, then only documentation file will be
> modified, not source file.
> 
> So, I'd like to create the fifth patch-set with new MFD
> documentation subdirectory.
> Additionally, LP3943 platform data example code will be added in the
> 'Documentation/mfd/lp3943.txt'. It's for a platform which doesn't
> support the device tree.

So, I'd actually like to get Sam's opinion on this.

NB: The documentation looks pretty rough at the moment. If we decide
to accept the concept, it will have to be adapted quite a bit before
full acceptance of the document will be granted.

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

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2013-09-23  7:58 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-09-12  1:35 [PATCH v3 4/4] Documentation: add LP3943 DT bindings and document Milo Kim
2013-09-12 15:24 ` Lee Jones
2013-09-15 11:27   ` Thierry Reding
2013-09-16  7:30     ` Lee Jones
2013-09-19 11:25       ` Thierry Reding
2013-09-19 11:56         ` Lee Jones
2013-09-22 14:35   ` Milo Kim
2013-09-23  7:58     ` Lee Jones

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).