[PATCH v4 0/7] leds: leds-pwm: Device tree support

[PATCH v4 0/7] leds: leds-pwm: Device tree support

[Peter Ujfalusi]

Hello,

Changes since v3:
Addressed comments from Thierry Redding:
- DT binding documentation for leds-pwm updated
- of_pwm_request() renamed as of_pwm_get()
- introduction of devm_of_pwm_get()
- Commit message updates
- Other comments has been also addressed
- Acked-by from Grant is not added to the patches since they were modified since
  v3

Changes since v2:
- rebased on top of linux-next
- DT bindings now alligned with Grant's request
- exporting of_pwm_request() from PWM core to allow clean DT bindings
- DT binding documentation changed to reflect the changes

Changes since v1:
- As suggested by Bryan Wu: the legacy pwm_request() has been removed from
  patch 1
- Device tree bindings added for leds-pwm driver.

When we boot with Device tree we handle one LED per device to be more aligned
with PWM core's DT implementation.
An example of the DT usage is provided in the new DT binding documentation for
leds-pwm.

Tested on OMAP4 Blaze (SDP), BeagleBoard with legacy and DT boot. On Zoom2 with
legacy boot.

Regards,
Peter
---
Peter Ujfalusi (7):
  leds: leds-pwm: Convert to use devm_get_pwm
  leds: leds-pwm: Preparing the driver for device tree support
  pwm: Correct parameter name in header for *pwm_get() functions
  pwm: core: Rename of_pwm_request() to of_pwm_get() and export it
  pwm: Add devm_of_pwm_get() as exported API for users
  leds: leds-pwm: Simplify cleanup code
  leds: leds-pwm: Add device tree bindings

 .../devicetree/bindings/leds/leds-pwm.txt          |  48 +++++++
 drivers/leds/leds-pwm.c                            | 152 +++++++++++++++------
 drivers/pwm/core.c                                 |  38 +++++-
 include/linux/leds_pwm.h                           |   2 +-
 include/linux/pwm.h                                |  20 ++-
 5 files changed, 212 insertions(+), 48 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/leds/leds-pwm.txt

-- 
1.8.0

[PATCH v4 1/7] leds: leds-pwm: Convert to use devm_get_pwm

[Peter Ujfalusi]

Update the driver to use the new API for requesting pwm so we can take
advantage of the pwm_lookup table to find the correct pwm to be used for the
LED functionality.

Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com>
---
 drivers/leds/leds-pwm.c  | 19 ++++++-------------
 include/linux/leds_pwm.h |  2 +-
 2 files changed, 7 insertions(+), 14 deletions(-)

diff --git a/drivers/leds/leds-pwm.c b/drivers/leds/leds-pwm.c
index 2157524..351257c 100644
--- a/drivers/leds/leds-pwm.c
+++ b/drivers/leds/leds-pwm.c
@@ -67,12 +67,11 @@ static int led_pwm_probe(struct platform_device *pdev)
 		cur_led = &pdata->leds[i];
 		led_dat = &leds_data[i];
 
-		led_dat->pwm = pwm_request(cur_led->pwm_id,
-				cur_led->name);
+		led_dat->pwm = devm_pwm_get(&pdev->dev, cur_led->name);
 		if (IS_ERR(led_dat->pwm)) {
 			ret = PTR_ERR(led_dat->pwm);
-			dev_err(&pdev->dev, "unable to request PWM %d\n",
-					cur_led->pwm_id);
+			dev_err(&pdev->dev, "unable to request PWM for %s\n",
+				cur_led->name);
 			goto err;
 		}
 
@@ -86,10 +85,8 @@ static int led_pwm_probe(struct platform_device *pdev)
 		led_dat->cdev.flags |= LED_CORE_SUSPENDRESUME;
 
 		ret = led_classdev_register(&pdev->dev, &led_dat->cdev);
-		if (ret < 0) {
-			pwm_free(led_dat->pwm);
+		if (ret < 0)
 			goto err;
-		}
 	}
 
 	platform_set_drvdata(pdev, leds_data);
@@ -98,10 +95,8 @@ static int led_pwm_probe(struct platform_device *pdev)
 
 err:
 	if (i > 0) {
-		for (i = i - 1; i >= 0; i--) {
+		for (i = i - 1; i >= 0; i--)
 			led_classdev_unregister(&leds_data[i].cdev);
-			pwm_free(leds_data[i].pwm);
-		}
 	}
 
 	return ret;
@@ -115,10 +110,8 @@ static int led_pwm_remove(struct platform_device *pdev)
 
 	leds_data = platform_get_drvdata(pdev);
 
-	for (i = 0; i < pdata->num_leds; i++) {
+	for (i = 0; i < pdata->num_leds; i++)
 		led_classdev_unregister(&leds_data[i].cdev);
-		pwm_free(leds_data[i].pwm);
-	}
 
 	return 0;
 }
diff --git a/include/linux/leds_pwm.h b/include/linux/leds_pwm.h
index 33a0711..a65e964 100644
--- a/include/linux/leds_pwm.h
+++ b/include/linux/leds_pwm.h
@@ -7,7 +7,7 @@
 struct led_pwm {
 	const char	*name;
 	const char	*default_trigger;
-	unsigned	pwm_id;
+	unsigned	pwm_id __deprecated;
 	u8 		active_low;
 	unsigned 	max_brightness;
 	unsigned	pwm_period_ns;
-- 
1.8.0

[PATCH v4 2/7] leds: leds-pwm: Preparing the driver for device tree support

[Peter Ujfalusi]

In order to be able to add device tree support for leds-pwm driver we need
to rearrange the data structures used by the drivers.

Signed-off-by: Peter Ujfalusi <peter.ujfalusi-l0cyMroinI0@public.gmane.org>
---
 drivers/leds/leds-pwm.c | 39 +++++++++++++++++++++++----------------
 1 file changed, 23 insertions(+), 16 deletions(-)

diff --git a/drivers/leds/leds-pwm.c b/drivers/leds/leds-pwm.c
index 351257c..c767837 100644
--- a/drivers/leds/leds-pwm.c
+++ b/drivers/leds/leds-pwm.c
@@ -30,6 +30,11 @@ struct led_pwm_data {
 	unsigned int		period;
 };
 
+struct led_pwm_priv {
+	int num_leds;
+	struct led_pwm_data leds[0];
+};
+
 static void led_pwm_set(struct led_classdev *led_cdev,
 	enum led_brightness brightness)
 {
@@ -47,25 +52,29 @@ static void led_pwm_set(struct led_classdev *led_cdev,
 	}
 }
 
+static inline size_t sizeof_pwm_leds_priv(int num_leds)
+{
+	return sizeof(struct led_pwm_priv) +
+		      (sizeof(struct led_pwm_data) * num_leds);
+}
+
 static int led_pwm_probe(struct platform_device *pdev)
 {
 	struct led_pwm_platform_data *pdata = pdev->dev.platform_data;
-	struct led_pwm *cur_led;
-	struct led_pwm_data *leds_data, *led_dat;
+	struct led_pwm_priv *priv;
 	int i, ret = 0;
 
 	if (!pdata)
 		return -EBUSY;
 
-	leds_data = devm_kzalloc(&pdev->dev,
-			sizeof(struct led_pwm_data) * pdata->num_leds,
-				GFP_KERNEL);
-	if (!leds_data)
+	priv = devm_kzalloc(&pdev->dev, sizeof_pwm_leds_priv(pdata->num_leds),
+			    GFP_KERNEL);
+	if (!priv)
 		return -ENOMEM;
 
 	for (i = 0; i < pdata->num_leds; i++) {
-		cur_led = &pdata->leds[i];
-		led_dat = &leds_data[i];
+		struct led_pwm *cur_led = &pdata->leds[i];
+		struct led_pwm_data *led_dat = &priv->leds[i];
 
 		led_dat->pwm = devm_pwm_get(&pdev->dev, cur_led->name);
 		if (IS_ERR(led_dat->pwm)) {
@@ -88,15 +97,16 @@ static int led_pwm_probe(struct platform_device *pdev)
 		if (ret < 0)
 			goto err;
 	}
+	priv->num_leds = pdata->num_leds;
 
-	platform_set_drvdata(pdev, leds_data);
+	platform_set_drvdata(pdev, priv);
 
 	return 0;
 
 err:
 	if (i > 0) {
 		for (i = i - 1; i >= 0; i--)
-			led_classdev_unregister(&leds_data[i].cdev);
+			led_classdev_unregister(&priv->leds[i].cdev);
 	}
 
 	return ret;
@@ -104,14 +114,11 @@ err:
 
 static int led_pwm_remove(struct platform_device *pdev)
 {
+	struct led_pwm_priv *priv = platform_get_drvdata(pdev);
 	int i;
-	struct led_pwm_platform_data *pdata = pdev->dev.platform_data;
-	struct led_pwm_data *leds_data;
-
-	leds_data = platform_get_drvdata(pdev);
 
-	for (i = 0; i < pdata->num_leds; i++)
-		led_classdev_unregister(&leds_data[i].cdev);
+	for (i = 0; i < priv->num_leds; i++)
+		led_classdev_unregister(&priv->leds[i].cdev);
 
 	return 0;
 }
-- 
1.8.0

[PATCH v4 3/7] pwm: Correct parameter name in header for *pwm_get() functions

[Peter Ujfalusi]

To synchronize the header file definition and the actual code. In the code
the consumer parameter is named as con_id, change the header file and replace
consumer -> con_id in the parameter list.

Signed-off-by: Peter Ujfalusi <peter.ujfalusi-l0cyMroinI0@public.gmane.org>
---
 include/linux/pwm.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/include/linux/pwm.h b/include/linux/pwm.h
index 6d661f3..cc908a5 100644
--- a/include/linux/pwm.h
+++ b/include/linux/pwm.h
@@ -174,10 +174,10 @@ struct pwm_device *pwm_request_from_chip(struct pwm_chip *chip,
 struct pwm_device *of_pwm_xlate_with_flags(struct pwm_chip *pc,
 		const struct of_phandle_args *args);
 
-struct pwm_device *pwm_get(struct device *dev, const char *consumer);
+struct pwm_device *pwm_get(struct device *dev, const char *con_id);
 void pwm_put(struct pwm_device *pwm);
 
-struct pwm_device *devm_pwm_get(struct device *dev, const char *consumer);
+struct pwm_device *devm_pwm_get(struct device *dev, const char *con_id);
 void devm_pwm_put(struct device *dev, struct pwm_device *pwm);
 #else
 static inline int pwm_set_chip_data(struct pwm_device *pwm, void *data)
-- 
1.8.0

[PATCH v4 4/7] pwm: core: Rename of_pwm_request() to of_pwm_get() and export it

[Peter Ujfalusi]

Allow client driver to use of_pwm_get() to get the PWM they need. This
is needed for drivers which handle more than one PWM separately, like
leds-pwm driver, which have:

pwmleds {
	compatible = "pwm-leds";
	kpad {
		label = "omap4::keypad";
		pwms = <&twl_pwm 0 7812500>;
		max-brightness = <127>;
	};

	charging {
		label = "omap4:green:chrg";
		pwms = <&twl_pwmled 0 7812500>;
		max-brightness = <255>;
	};
};

in the dts files.

Signed-off-by: Peter Ujfalusi <peter.ujfalusi-l0cyMroinI0@public.gmane.org>
---
 drivers/pwm/core.c  | 8 ++++----
 include/linux/pwm.h | 7 +++++++
 2 files changed, 11 insertions(+), 4 deletions(-)

diff --git a/drivers/pwm/core.c b/drivers/pwm/core.c
index 903138b..3cb741d 100644
--- a/drivers/pwm/core.c
+++ b/drivers/pwm/core.c
@@ -471,7 +471,7 @@ static struct pwm_chip *of_node_to_pwmchip(struct device_node *np)
 }
 
 /**
- * of_pwm_request() - request a PWM via the PWM framework
+ * of_pwm_get() - request a PWM via the PWM framework
  * @np: device node to get the PWM from
  * @con_id: consumer name
  *
@@ -486,8 +486,7 @@ static struct pwm_chip *of_node_to_pwmchip(struct device_node *np)
  * becomes mandatory for devices that look up the PWM device via the con_id
  * parameter.
  */
-static struct pwm_device *of_pwm_request(struct device_node *np,
-					 const char *con_id)
+struct pwm_device *of_pwm_get(struct device_node *np, const char *con_id)
 {
 	struct pwm_device *pwm = NULL;
 	struct of_phandle_args args;
@@ -545,6 +544,7 @@ put:
 
 	return pwm;
 }
+EXPORT_SYMBOL_GPL(of_pwm_get);
 
 /**
  * pwm_add_table() - register PWM device consumers
@@ -587,7 +587,7 @@ struct pwm_device *pwm_get(struct device *dev, const char *con_id)
 
 	/* look up via DT first */
 	if (IS_ENABLED(CONFIG_OF) && dev && dev->of_node)
-		return of_pwm_request(dev->of_node, con_id);
+		return of_pwm_get(dev->of_node, con_id);
 
 	/*
 	 * We look up the provider in the static table typically provided by
diff --git a/include/linux/pwm.h b/include/linux/pwm.h
index cc908a5..76a1959 100644
--- a/include/linux/pwm.h
+++ b/include/linux/pwm.h
@@ -175,6 +175,7 @@ struct pwm_device *of_pwm_xlate_with_flags(struct pwm_chip *pc,
 		const struct of_phandle_args *args);
 
 struct pwm_device *pwm_get(struct device *dev, const char *con_id);
+struct pwm_device *of_pwm_get(struct device_node *np, const char *con_id);
 void pwm_put(struct pwm_device *pwm);
 
 struct pwm_device *devm_pwm_get(struct device *dev, const char *con_id);
@@ -213,6 +214,12 @@ static inline struct pwm_device *pwm_get(struct device *dev,
 	return ERR_PTR(-ENODEV);
 }
 
+static inline struct pwm_device *of_pwm_get(struct device_node *np,
+					    const char *con_id)
+{
+	return ERR_PTR(-ENODEV);
+}
+
 static inline void pwm_put(struct pwm_device *pwm)
 {
 }
-- 
1.8.0

[PATCH v4 5/7] pwm: Add devm_of_pwm_get() as exported API for users

[Peter Ujfalusi]

When booted with DT users can use devm version of of_pwm_get() to benefit
from automatic resource release.

Signed-off-by: Peter Ujfalusi <peter.ujfalusi-l0cyMroinI0@public.gmane.org>
---
 drivers/pwm/core.c  | 30 ++++++++++++++++++++++++++++++
 include/linux/pwm.h |  9 +++++++++
 2 files changed, 39 insertions(+)

diff --git a/drivers/pwm/core.c b/drivers/pwm/core.c
index 3cb741d..4a13da4 100644
--- a/drivers/pwm/core.c
+++ b/drivers/pwm/core.c
@@ -708,6 +708,36 @@ struct pwm_device *devm_pwm_get(struct device *dev, const char *con_id)
 }
 EXPORT_SYMBOL_GPL(devm_pwm_get);
 
+/**
+ * devm_of_pwm_get() - resource managed of_pwm_get()
+ * @dev: device for PWM consumer
+ * @np: device node to get the PWM from
+ * @con_id: consumer name
+ *
+ * This function performs like of_pwm_get() but the acquired PWM device will
+ * automatically be released on driver detach.
+ */
+struct pwm_device *devm_of_pwm_get(struct device *dev, struct device_node *np,
+				   const char *con_id)
+{
+	struct pwm_device **ptr, *pwm;
+
+	ptr = devres_alloc(devm_pwm_release, sizeof(**ptr), GFP_KERNEL);
+	if (!ptr)
+		return ERR_PTR(-ENOMEM);
+
+	pwm = of_pwm_get(np, con_id);
+	if (!IS_ERR(pwm)) {
+		*ptr = pwm;
+		devres_add(dev, ptr);
+	} else {
+		devres_free(ptr);
+	}
+
+	return pwm;
+}
+EXPORT_SYMBOL_GPL(devm_of_pwm_get);
+
 static int devm_pwm_match(struct device *dev, void *res, void *data)
 {
 	struct pwm_device **p = res;
diff --git a/include/linux/pwm.h b/include/linux/pwm.h
index 76a1959..70655a2 100644
--- a/include/linux/pwm.h
+++ b/include/linux/pwm.h
@@ -179,6 +179,8 @@ struct pwm_device *of_pwm_get(struct device_node *np, const char *con_id);
 void pwm_put(struct pwm_device *pwm);
 
 struct pwm_device *devm_pwm_get(struct device *dev, const char *con_id);
+struct pwm_device *devm_of_pwm_get(struct device *dev, struct device_node *np,
+				   const char *con_id);
 void devm_pwm_put(struct device *dev, struct pwm_device *pwm);
 #else
 static inline int pwm_set_chip_data(struct pwm_device *pwm, void *data)
@@ -230,6 +232,13 @@ static inline struct pwm_device *devm_pwm_get(struct device *dev,
 	return ERR_PTR(-ENODEV);
 }
 
+static inline struct pwm_device *devm_of_pwm_get(struct device *dev,
+						 struct device_node *np,
+						 const char *con_id)
+{
+	return ERR_PTR(-ENODEV);
+}
+
 static inline void devm_pwm_put(struct device *dev, struct pwm_device *pwm)
 {
 }
-- 
1.8.0

[PATCH v4 6/7] leds: leds-pwm: Simplify cleanup code

[Peter Ujfalusi]

The code looks more nicer if we use:

while (i--)

instead:
if (i > 0)
	for (i = i - 1; i >= 0; i--)

Signed-off-by: Peter Ujfalusi <peter.ujfalusi-l0cyMroinI0@public.gmane.org>
---
 drivers/leds/leds-pwm.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/leds/leds-pwm.c b/drivers/leds/leds-pwm.c
index c767837..46f4e44 100644
--- a/drivers/leds/leds-pwm.c
+++ b/drivers/leds/leds-pwm.c
@@ -104,10 +104,8 @@ static int led_pwm_probe(struct platform_device *pdev)
 	return 0;
 
 err:
-	if (i > 0) {
-		for (i = i - 1; i >= 0; i--)
-			led_classdev_unregister(&priv->leds[i].cdev);
-	}
+	while (i--)
+		led_classdev_unregister(&priv->leds[i].cdev);
 
 	return ret;
 }
-- 
1.8.0

[PATCH v4 7/7] leds: leds-pwm: Add device tree bindings

[Peter Ujfalusi]

Support for device tree booted kernel.

For usage see:
Documentation/devicetree/bindings/leds/leds-pwm.txt

Signed-off-by: Peter Ujfalusi <peter.ujfalusi-l0cyMroinI0@public.gmane.org>
---
 .../devicetree/bindings/leds/leds-pwm.txt          |  48 +++++++++
 drivers/leds/leds-pwm.c                            | 112 +++++++++++++++++----
 2 files changed, 140 insertions(+), 20 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/leds/leds-pwm.txt

diff --git a/Documentation/devicetree/bindings/leds/leds-pwm.txt b/Documentation/devicetree/bindings/leds/leds-pwm.txt
new file mode 100644
index 0000000..7297107
--- /dev/null
+++ b/Documentation/devicetree/bindings/leds/leds-pwm.txt
@@ -0,0 +1,48 @@
+LED connected to PWM
+
+Required properties:
+- compatible : should be "pwm-leds".
+
+Each LED is represented as a sub-node of the pwm-leds device.  Each
+node's name represents the name of the corresponding LED.
+
+LED sub-node properties:
+- pwms : PWM property to point to the PWM device (phandle)/port (id) and to
+  specify the period time to be used: <&phandle id period_ns>;
+- pwm-names : (optional) Name to be used by the PWM subsystem for the PWM device
+  For the pwms and pwm-names property please refer to:
+  Documentation/devicetree/bindings/pwm/pwm.txt
+- max-brightness : Maximum brightness possible for the LED
+- label :  (optional)
+  see Documentation/devicetree/bindings/leds/common.txt
+- linux,default-trigger :  (optional)
+  see Documentation/devicetree/bindings/leds/common.txt
+
+Example:
+
+twl_pwm: pwm {
+	/* provides two PWMs (id 0, 1 for PWM1 and PWM2) */
+	compatible = "ti,twl6030-pwm";
+	#pwm-cells = <2>;
+};
+
+twl_pwmled: pwmled {
+	/* provides one PWM (id 0 for Charing indicator LED) */
+	compatible = "ti,twl6030-pwmled";
+	#pwm-cells = <2>;
+};
+
+pwmleds {
+	compatible = "pwm-leds";
+	kpad {
+		label = "omap4::keypad";
+		pwms = <&twl_pwm 0 7812500>;
+		max-brightness = <127>;
+	};
+
+	charging {
+		label = "omap4:green:chrg";
+		pwms = <&twl_pwmled 0 7812500>;
+		max-brightness = <255>;
+	};
+};
diff --git a/drivers/leds/leds-pwm.c b/drivers/leds/leds-pwm.c
index 46f4e44..a1ea5f6 100644
--- a/drivers/leds/leds-pwm.c
+++ b/drivers/leds/leds-pwm.c
@@ -16,6 +16,7 @@
 #include <linux/kernel.h>
 #include <linux/init.h>
 #include <linux/platform_device.h>
+#include <linux/of_platform.h>
 #include <linux/fb.h>
 #include <linux/leds.h>
 #include <linux/err.h>
@@ -58,46 +59,110 @@ static inline size_t sizeof_pwm_leds_priv(int num_leds)
 		      (sizeof(struct led_pwm_data) * num_leds);
 }
 
-static int led_pwm_probe(struct platform_device *pdev)
+static struct led_pwm_priv *led_pwm_create_of(struct platform_device *pdev)
 {
-	struct led_pwm_platform_data *pdata = pdev->dev.platform_data;
+	struct device_node *node = pdev->dev.of_node;
+	struct device_node *child;
 	struct led_pwm_priv *priv;
-	int i, ret = 0;
+	int count, ret;
 
-	if (!pdata)
-		return -EBUSY;
+	/* count LEDs in this device, so we know how much to allocate */
+	count = of_get_child_count(node);
+	if (!count)
+		return NULL;
 
-	priv = devm_kzalloc(&pdev->dev, sizeof_pwm_leds_priv(pdata->num_leds),
+	priv = devm_kzalloc(&pdev->dev, sizeof_pwm_leds_priv(count),
 			    GFP_KERNEL);
 	if (!priv)
-		return -ENOMEM;
+		return NULL;
 
-	for (i = 0; i < pdata->num_leds; i++) {
-		struct led_pwm *cur_led = &pdata->leds[i];
-		struct led_pwm_data *led_dat = &priv->leds[i];
+	for_each_child_of_node(node, child) {
+		struct led_pwm_data *led_dat = &priv->leds[priv->num_leds];
 
-		led_dat->pwm = devm_pwm_get(&pdev->dev, cur_led->name);
+		led_dat->cdev.name = of_get_property(child, "label",
+						     NULL) ? : child->name;
+
+		led_dat->pwm = devm_of_pwm_get(&pdev->dev, child, NULL);
 		if (IS_ERR(led_dat->pwm)) {
-			ret = PTR_ERR(led_dat->pwm);
 			dev_err(&pdev->dev, "unable to request PWM for %s\n",
-				cur_led->name);
+				led_dat->cdev.name);
 			goto err;
 		}
+		/* Get the period from PWM core when n*/
+		led_dat->period = pwm_get_period(led_dat->pwm);
+
+		led_dat->cdev.default_trigger = of_get_property(child,
+						"linux,default-trigger", NULL);
+		of_property_read_u32(child, "max-brightness",
+				     &led_dat->cdev.max_brightness);
 
-		led_dat->cdev.name = cur_led->name;
-		led_dat->cdev.default_trigger = cur_led->default_trigger;
-		led_dat->active_low = cur_led->active_low;
-		led_dat->period = cur_led->pwm_period_ns;
 		led_dat->cdev.brightness_set = led_pwm_set;
 		led_dat->cdev.brightness = LED_OFF;
-		led_dat->cdev.max_brightness = cur_led->max_brightness;
 		led_dat->cdev.flags |= LED_CORE_SUSPENDRESUME;
 
 		ret = led_classdev_register(&pdev->dev, &led_dat->cdev);
-		if (ret < 0)
+		if (ret < 0) {
+			dev_err(&pdev->dev, "failed to register for %s\n",
+				led_dat->cdev.name);
+			of_node_put(child);
 			goto err;
+		}
+		priv->num_leds++;
+	}
+
+	return priv;
+err:
+	while (priv->num_leds--)
+		led_classdev_unregister(&priv->leds[priv->num_leds].cdev);
+
+	return NULL;
+}
+
+static int led_pwm_probe(struct platform_device *pdev)
+{
+	struct led_pwm_platform_data *pdata = pdev->dev.platform_data;
+	struct led_pwm_priv *priv;
+	int i, ret = 0;
+
+	if (pdata && pdata->num_leds) {
+		priv = devm_kzalloc(&pdev->dev,
+				    sizeof_pwm_leds_priv(pdata->num_leds),
+				    GFP_KERNEL);
+		if (!priv)
+			return -ENOMEM;
+
+		for (i = 0; i < pdata->num_leds; i++) {
+			struct led_pwm *cur_led = &pdata->leds[i];
+			struct led_pwm_data *led_dat = &priv->leds[i];
+
+			led_dat->pwm = devm_pwm_get(&pdev->dev, cur_led->name);
+			if (IS_ERR(led_dat->pwm)) {
+				ret = PTR_ERR(led_dat->pwm);
+				dev_err(&pdev->dev,
+					"unable to request PWM for %s\n",
+					cur_led->name);
+				goto err;
+			}
+
+			led_dat->cdev.name = cur_led->name;
+			led_dat->cdev.default_trigger = cur_led->default_trigger;
+			led_dat->active_low = cur_led->active_low;
+			led_dat->period = cur_led->pwm_period_ns;
+			led_dat->cdev.brightness_set = led_pwm_set;
+			led_dat->cdev.brightness = LED_OFF;
+			led_dat->cdev.max_brightness = cur_led->max_brightness;
+			led_dat->cdev.flags |= LED_CORE_SUSPENDRESUME;
+
+			ret = led_classdev_register(&pdev->dev, &led_dat->cdev);
+			if (ret < 0)
+				goto err;
+		}
+		priv->num_leds = pdata->num_leds;
+	} else {
+		priv = led_pwm_create_of(pdev);
+		if (!priv)
+			return -ENODEV;
 	}
-	priv->num_leds = pdata->num_leds;
 
 	platform_set_drvdata(pdev, priv);
 
@@ -121,12 +186,19 @@ static int led_pwm_remove(struct platform_device *pdev)
 	return 0;
 }
 
+static const struct of_device_id of_pwm_leds_match[] = {
+	{ .compatible = "pwm-leds", },
+	{},
+};
+MODULE_DEVICE_TABLE(of, of_pwm_leds_match);
+
 static struct platform_driver led_pwm_driver = {
 	.probe		= led_pwm_probe,
 	.remove		= led_pwm_remove,
 	.driver		= {
 		.name	= "leds_pwm",
 		.owner	= THIS_MODULE,
+		.of_match_table = of_match_ptr(of_pwm_leds_match),
 	},
 };
 
-- 
1.8.0

Re: [PATCH v4 7/7] leds: leds-pwm: Add device tree bindings

[Grant Likely]

Hi Peter,

Thanks for this work. Comments below...

On Wed, 12 Dec 2012 10:04:52 +0100, Peter Ujfalusi <peter.ujfalusi@ti.com> wrote:
> Support for device tree booted kernel.
> 
> For usage see:
> Documentation/devicetree/bindings/leds/leds-pwm.txt
> 
> Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com>

About commit text: Remember that commit text is the first thing people
are going to see when looking over what changed in a given subsystem. It
is also where maintainers like me look first when trying to decide if a
patch makes sense before applying it. When commit text is bare-bones
like the above and the patch is non-trivial, then I end up trying to
reconstruct your though process or rational for a patch.

So, please, if it is anything beyond a trivial patch then tell us more
than simply "support for device tree booted kernel". What platform did
you make the change for? How was it tested? Are there any notable design
decisions that you made when adding this support? Are there any missing
pieces or possible bugs?

I'm picking on you at the moment, but this is a general comment. The
commit text is where you get to convince me that the patch is needed and
tell me about how it was designed. This is really important information;
particularly for poor random user, bisecting his broken kernel and
landing on your commit. In this small way, you can make her Christmas
merrier this year.


> ---
>  .../devicetree/bindings/leds/leds-pwm.txt          |  48 +++++++++
>  drivers/leds/leds-pwm.c                            | 112 +++++++++++++++++----
>  2 files changed, 140 insertions(+), 20 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/leds/leds-pwm.txt
> 
> diff --git a/Documentation/devicetree/bindings/leds/leds-pwm.txt b/Documentation/devicetree/bindings/leds/leds-pwm.txt
> new file mode 100644
> index 0000000..7297107
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/leds/leds-pwm.txt
> @@ -0,0 +1,48 @@
> +LED connected to PWM
> +
> +Required properties:
> +- compatible : should be "pwm-leds".
> +
> +Each LED is represented as a sub-node of the pwm-leds device.  Each
> +node's name represents the name of the corresponding LED.
> +
> +LED sub-node properties:
> +- pwms : PWM property to point to the PWM device (phandle)/port (id) and to
> +  specify the period time to be used: <&phandle id period_ns>;
> +- pwm-names : (optional) Name to be used by the PWM subsystem for the PWM device
> +  For the pwms and pwm-names property please refer to:
> +  Documentation/devicetree/bindings/pwm/pwm.txt
> +- max-brightness : Maximum brightness possible for the LED
> +- label :  (optional)
> +  see Documentation/devicetree/bindings/leds/common.txt
> +- linux,default-trigger :  (optional)
> +  see Documentation/devicetree/bindings/leds/common.txt
> +
> +Example:
> +
> +twl_pwm: pwm {
> +	/* provides two PWMs (id 0, 1 for PWM1 and PWM2) */
> +	compatible = "ti,twl6030-pwm";
> +	#pwm-cells = <2>;
> +};
> +
> +twl_pwmled: pwmled {
> +	/* provides one PWM (id 0 for Charing indicator LED) */
> +	compatible = "ti,twl6030-pwmled";
> +	#pwm-cells = <2>;
> +};
> +
> +pwmleds {
> +	compatible = "pwm-leds";
> +	kpad {
> +		label = "omap4::keypad";
> +		pwms = <&twl_pwm 0 7812500>;
> +		max-brightness = <127>;
> +	};
> +
> +	charging {
> +		label = "omap4:green:chrg";
> +		pwms = <&twl_pwmled 0 7812500>;
> +		max-brightness = <255>;
> +	};

Acked-by: Grant Likely <grant.likely@secretlab.ca>