Re: [PATCH v3 4/4] leds: leds-pwm: Add device tree bindings

2012-12-12 Thread Peter Ujfalusi 
On 12/11/2012 08:25 AM, Thierry Reding wrote:
>> +static const struct of_device_id of_pwm_leds_match[] = {
>> +{ .compatible = "pwm-leds", },
>> +{},
>> +};
> 
> Doesn't this cause a compiler warning for !OF builds?

This is not causing any compiler warnings.

-- 
Péter
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v3 4/4] leds: leds-pwm: Add device tree bindings

2012-12-12 Thread Peter Ujfalusi 
On 12/11/2012 08:25 AM, Thierry Reding wrote:
 +static const struct of_device_id of_pwm_leds_match[] = {
 +{ .compatible = pwm-leds, },
 +{},
 +};
 
 Doesn't this cause a compiler warning for !OF builds?

This is not causing any compiler warnings.

-- 
Péter
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v3 4/4] leds: leds-pwm: Add device tree bindings

2012-12-10 Thread Thierry Reding 
On Mon, Dec 10, 2012 at 11:00:37AM +0100, Peter Ujfalusi wrote:
[...]
> +LED sub-node properties:
> +- pwms : PWM property, please refer to: 
> +  Documentation/devicetree/bindings/pwm/pwm.txt

Instead of only referring to the generic PWM binding document, this
should probably explain what the PWM device is used for.

> +err:
> + if (priv->num_leds > 0) {
> + for (count = priv->num_leds - 1; count >= 0; count--) {
> + led_classdev_unregister(>leds[count].cdev);
> + pwm_put(priv->leds[count].pwm);
> + }
> + }

Can this not be written more simply as follows?

while (priv->num_leds--) {
...
}

>  static int led_pwm_remove(struct platform_device *pdev)
>  {
> + struct led_pwm_platform_data *pdata = pdev->dev.platform_data;
>   struct led_pwm_priv *priv = platform_get_drvdata(pdev);
>   int i;
>  
> - for (i = 0; i < priv->num_leds; i++)
> + for (i = 0; i < priv->num_leds; i++) {
>   led_classdev_unregister(>leds[i].cdev);
> + if (!pdata)
> + pwm_put(priv->leds[i].pwm);
> + }

Perhaps while at it we can add devm_of_pwm_get() along with exporting
of_pwm_get() so that you don't have to special-case this?

> +static const struct of_device_id of_pwm_leds_match[] = {
> + { .compatible = "pwm-leds", },
> + {},
> +};

Doesn't this cause a compiler warning for !OF builds?

Thierry


pgpu79iP0dP5J.pgp
Description: PGP signature

Re: [PATCH v3 4/4] leds: leds-pwm: Add device tree bindings

2012-12-10 Thread Grant Likely 
On Mon, 10 Dec 2012 11:00:37 +0100, Peter Ujfalusi  
wrote:
> Support for device tree booted kernel.
> 
> For usage see:
> Documentation/devicetree/bindings/leds/leds-pwm.txt
> 
> Signed-off-by: Peter Ujfalusi 

Acked-by: Grant Likely 

The other patches in this series look good to.

g.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH v3 4/4] leds: leds-pwm: Add device tree bindings

2012-12-10 Thread Peter Ujfalusi 
Support for device tree booted kernel.

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

Signed-off-by: Peter Ujfalusi 
---
 .../devicetree/bindings/leds/leds-pwm.txt  |  46 
 drivers/leds/leds-pwm.c| 124 +
 2 files changed, 149 insertions(+), 21 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 000..abdff60
--- /dev/null
+++ b/Documentation/devicetree/bindings/leds/leds-pwm.txt
@@ -0,0 +1,46 @@
+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, please refer to: 
+  Documentation/devicetree/bindings/pwm/pwm.txt
+- pwm-names : (optional) Name to be used by the PWM subsystem for the PWM 
device
+- 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 = <_pwm 0 7812500>;
+   max-brightness = <127>;
+   };
+
+   charging {
+   label = "omap4:green:chrg";
+   pwms = <_pwmled 0 7812500>;
+   max-brightness = <255>;
+   };
+};
diff --git a/drivers/leds/leds-pwm.c b/drivers/leds/leds-pwm.c
index 02f0c0c..a57ac27 100644
--- a/drivers/leds/leds-pwm.c
+++ b/drivers/leds/leds-pwm.c
@@ -16,6 +16,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -58,46 +59,115 @@ static inline int 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(>dev, sizeof_pwm_leds_priv(pdata->num_leds),
+   priv = devm_kzalloc(>dev, sizeof_pwm_leds_priv(count),
GFP_KERNEL);
if (!priv)
-   return -ENOMEM;
+   return NULL;
+
+   for_each_child_of_node(node, child) {
+   struct led_pwm_data *led_dat = >leds[priv->num_leds];
 
-   for (i = 0; i < pdata->num_leds; i++) {
-   struct led_pwm *cur_led = >leds[i];
-   struct led_pwm_data *led_dat = >leds[i];
+   led_dat->cdev.name = of_get_property(child, "label",
+NULL) ? : child->name;
 
-   led_dat->pwm = devm_pwm_get(>dev, cur_led->name);
+   led_dat->pwm = of_pwm_request(child, NULL);
if (IS_ERR(led_dat->pwm)) {
-   ret = PTR_ERR(led_dat->pwm);
dev_err(>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",
+_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 |=

[PATCH v3 4/4] leds: leds-pwm: Add device tree bindings

2012-12-10 Thread Peter Ujfalusi 
Support for device tree booted kernel.

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

Signed-off-by: Peter Ujfalusi peter.ujfal...@ti.com
---
 .../devicetree/bindings/leds/leds-pwm.txt  |  46 
 drivers/leds/leds-pwm.c| 124 +
 2 files changed, 149 insertions(+), 21 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 000..abdff60
--- /dev/null
+++ b/Documentation/devicetree/bindings/leds/leds-pwm.txt
@@ -0,0 +1,46 @@
+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, please refer to: 
+  Documentation/devicetree/bindings/pwm/pwm.txt
+- pwm-names : (optional) Name to be used by the PWM subsystem for the PWM 
device
+- 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 02f0c0c..a57ac27 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,115 @@ static inline int 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_each_child_of_node(node, child) {
+   struct led_pwm_data *led_dat = priv-leds[priv-num_leds];
 
-   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-cdev.name = of_get_property(child, label,
+NULL) ? : child-name;
 
-   led_dat-pwm = devm_pwm_get(pdev-dev, cur_led-name);
+   led_dat-pwm = of_pwm_request(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;
-

Re: [PATCH v3 4/4] leds: leds-pwm: Add device tree bindings

2012-12-10 Thread Grant Likely 
On Mon, 10 Dec 2012 11:00:37 +0100, Peter Ujfalusi peter.ujfal...@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.ujfal...@ti.com

Acked-by: Grant Likely grant.lik...@secretlab.ca

The other patches in this series look good to.

g.

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v3 4/4] leds: leds-pwm: Add device tree bindings

2012-12-10 Thread Thierry Reding 
On Mon, Dec 10, 2012 at 11:00:37AM +0100, Peter Ujfalusi wrote:
[...]
 +LED sub-node properties:
 +- pwms : PWM property, please refer to: 
 +  Documentation/devicetree/bindings/pwm/pwm.txt

Instead of only referring to the generic PWM binding document, this
should probably explain what the PWM device is used for.

 +err:
 + if (priv-num_leds  0) {
 + for (count = priv-num_leds - 1; count = 0; count--) {
 + led_classdev_unregister(priv-leds[count].cdev);
 + pwm_put(priv-leds[count].pwm);
 + }
 + }

Can this not be written more simply as follows?

while (priv-num_leds--) {
...
}

  static int led_pwm_remove(struct platform_device *pdev)
  {
 + struct led_pwm_platform_data *pdata = pdev-dev.platform_data;
   struct led_pwm_priv *priv = platform_get_drvdata(pdev);
   int i;
  
 - for (i = 0; i  priv-num_leds; i++)
 + for (i = 0; i  priv-num_leds; i++) {
   led_classdev_unregister(priv-leds[i].cdev);
 + if (!pdata)
 + pwm_put(priv-leds[i].pwm);
 + }

Perhaps while at it we can add devm_of_pwm_get() along with exporting
of_pwm_get() so that you don't have to special-case this?

 +static const struct of_device_id of_pwm_leds_match[] = {
 + { .compatible = pwm-leds, },
 + {},
 +};

Doesn't this cause a compiler warning for !OF builds?

Thierry


pgpu79iP0dP5J.pgp
Description: PGP signature