Hello,
The patch:
https://lkml.org/lkml/2012/11/7/187
grown to be a series to add DeviceTree support for the leds-pwm driver.
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) with legacy and DT boot.
Regards,
Peter
---
Peter Ujfalusi (3):
leds: leds-pwm: Convert to use devm_get_pwm
leds: leds-pwm: Preparing the driver for device tree support
leds: leds-pwm: Add device tree bindings
.../devicetree/bindings/leds/leds-pwm.txt | 34 +++++
drivers/leds/leds-pwm.c | 161 ++++++++++++++-------
include/linux/leds_pwm.h | 2 +-
3 files changed, 146 insertions(+), 51 deletions(-)
create mode 100644 Documentation/devicetree/bindings/leds/leds-pwm.txt
--
1.8.0
Support for device tree booted kernel.
When the kernel is booted with DeviceTree blob we support one led per
leds-pwm device to have cleaner integration with the PWM subsystem.
For usage see:
Documentation/devicetree/bindings/leds/leds-pwm.txt
Signed-off-by: Peter Ujfalusi <[email protected]>
---
.../devicetree/bindings/leds/leds-pwm.txt | 34 ++++++
drivers/leds/leds-pwm.c | 125 +++++++++++++++------
2 files changed, 127 insertions(+), 32 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..9fe3040
--- /dev/null
+++ b/Documentation/devicetree/bindings/leds/leds-pwm.txt
@@ -0,0 +1,34 @@
+LED connected to PWM
+
+Required properties:
+- compatible : should be "pwm-leds".
+- 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
+- label : (optional) The label for this LED. If omitted, the label is
+ taken from the node name (excluding the unit address).
+- max-brightness : Maximum brightness possible for the LED
+- linux,default-trigger : (optional) This parameter, if present, is a
+ string defining the trigger assigned to the LED. Current triggers are:
+ "backlight" - LED will act as a back-light, controlled by the framebuffer
+ system
+ "default-on" - LED will turn on, but see "default-state" below
+ "heartbeat" - LED "double" flashes at a load average based rate
+ "ide-disk" - LED indicates disk activity
+ "timer" - LED flashes at a fixed, configurable rate
+
+Example:
+
+twl_pwm: pwm {
+ compatible = "ti,twl6030-pwm";
+ #pwm-cells = <2>;
+};
+
+pwmled_keypad {
+ compatible = "pwm-leds";
+ pwms = <&twl_pwm 0 7812500>;
+ pwm-names = "Keypad LED";
+
+ label = "omap4::keypad";
+ max-brightness = <127>;
+};
diff --git a/drivers/leds/leds-pwm.c b/drivers/leds/leds-pwm.c
index b219ea9..333c942 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,98 @@ static inline int sizeof_pwm_leds_priv(int num_leds)
(sizeof(struct led_pwm_data) * num_leds);
}
+static struct led_pwm_priv *led_pwm_create_of(struct platform_device *pdev)
+{
+ struct device_node *node = pdev->dev.of_node;
+ struct led_pwm_priv *priv;
+ struct led_pwm_data *led_dat;
+ int ret;
+
+ /* With DT boot we support one LED per leds-pwm device */
+ priv = devm_kzalloc(&pdev->dev, sizeof_pwm_leds_priv(1),
+ GFP_KERNEL);
+ if (!priv)
+ return NULL;
+
+ led_dat = &priv->leds[priv->num_leds];
+ led_dat->cdev.name = of_get_property(node, "label", NULL) ? : node->name;
+ led_dat->pwm = devm_pwm_get(&pdev->dev, NULL);
+ if (IS_ERR(led_dat->pwm)) {
+ dev_err(&pdev->dev, "unable to request PWM for %s\n",
+ led_dat->cdev.name);
+ return NULL;
+ }
+ /* 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(node,
+ "linux,default-trigger",
+ NULL);
+ of_property_read_u32(node, "max-brightness",
+ &led_dat->cdev.max_brightness);
+
+ led_dat->cdev.brightness_set = led_pwm_set;
+ led_dat->cdev.brightness = LED_OFF;
+ led_dat->cdev.flags |= LED_CORE_SUSPENDRESUME;
+
+ ret = led_classdev_register(&pdev->dev, &led_dat->cdev);
+ if (ret < 0) {
+ dev_err(&pdev->dev, "failed to register for %s\n",
+ led_dat->cdev.name);
+ of_node_put(node);
+ return NULL;
+ }
+
+ priv->num_leds = 1;
+
+ return priv;
+}
+
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)
- return -EBUSY;
-
- 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;
+ 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;
}
-
- 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);
@@ -123,12 +176,20 @@ static int __devexit 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 = __devexit_p(led_pwm_remove),
.driver = {
.name = "leds_pwm",
.owner = THIS_MODULE,
+ .of_match_table = of_match_ptr(of_pwm_leds_match),
},
};
--
1.8.0
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.
If the devm_get_pwm fails we fall back to legacy mode to try to get the pwm.
Signed-off-by: Peter Ujfalusi <[email protected]>
---
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 f2e44c7..c953c75 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 __devexit 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
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 <[email protected]>
---
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 c953c75..b219ea9 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[];
+};
+
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 int 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 __devexit 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
On Mon, Nov 12, 2012 at 6:41 AM, Peter Ujfalusi <[email protected]> wrote:
> 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.
> If the devm_get_pwm fails we fall back to legacy mode to try to get the pwm.
>
> Signed-off-by: Peter Ujfalusi <[email protected]>
> ---
> 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 f2e44c7..c953c75 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 __devexit 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;
I suggest we remove this later, we can provide patches from this from
platform data of board file. And I think this patch is good for me to
merge, will do it soon.
Thanks,
-Bryan
> u8 active_low;
> unsigned max_brightness;
> unsigned pwm_period_ns;
> --
> 1.8.0
>
Hi Bryan,
On 11/14/2012 02:14 AM, Bryan Wu wrote:
>> 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;
>
> I suggest we remove this later, we can provide patches from this from
> platform data of board file. And I think this patch is good for me to
> merge, will do it soon.
Yes, we can remove it in 3.9. I marked it as deprecated for now to avoid
breakage - if any - in 3.8.
Thank you,
P?ter