* [PATCH v3 0/4] leds: leds-pwm: Device tree support
@ 2012-12-10 10:00 ` Peter Ujfalusi
0 siblings, 0 replies; 31+ messages in thread
From: Peter Ujfalusi @ 2012-12-10 10:00 UTC (permalink / raw)
To: Bryan Wu, Richard Purdie, Grant Likely, Thierry Reding
Cc: linux-kernel, devicetree-discuss, linux-doc, linux-leds
Hello,
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) with legacy and DT boot.
Regards,
Peter
---
Peter Ujfalusi (4):
leds: leds-pwm: Convert to use devm_get_pwm
leds: leds-pwm: Preparing the driver for device tree support
pwm: core: Export of_pwm_request() so client drivers can also use it
leds: leds-pwm: Add device tree bindings
.../devicetree/bindings/leds/leds-pwm.txt | 46 ++++++
drivers/leds/leds-pwm.c | 158 ++++++++++++++++-----
drivers/pwm/core.c | 2 +-
include/linux/leds_pwm.h | 2 +-
include/linux/pwm.h | 7 +
5 files changed, 175 insertions(+), 40 deletions(-)
create mode 100644 Documentation/devicetree/bindings/leds/leds-pwm.txt
--
1.8.0
^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH v3 1/4] leds: leds-pwm: Convert to use devm_get_pwm
2012-12-10 10:00 ` Peter Ujfalusi
@ 2012-12-10 10:00 ` Peter Ujfalusi
-1 siblings, 0 replies; 31+ messages in thread
From: Peter Ujfalusi @ 2012-12-10 10:00 UTC (permalink / raw)
To: Bryan Wu, Richard Purdie, Grant Likely, Thierry Reding
Cc: linux-kernel, devicetree-discuss, linux-doc, linux-leds
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 <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
^ permalink raw reply related [flat|nested] 31+ messages in thread* [PATCH v3 1/4] leds: leds-pwm: Convert to use devm_get_pwm
@ 2012-12-10 10:00 ` Peter Ujfalusi
0 siblings, 0 replies; 31+ messages in thread
From: Peter Ujfalusi @ 2012-12-10 10:00 UTC (permalink / raw)
To: Bryan Wu, Richard Purdie, Grant Likely, Thierry Reding
Cc: linux-kernel, devicetree-discuss, linux-doc, linux-leds
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 <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
^ permalink raw reply related [flat|nested] 31+ messages in thread* Re: [PATCH v3 1/4] leds: leds-pwm: Convert to use devm_get_pwm
2012-12-10 10:00 ` Peter Ujfalusi
(?)
@ 2012-12-11 7:03 ` Thierry Reding
[not found] ` <20121211070307.GB8294-RM9K5IK7kjIyiCvfTdI0JKcOhU4Rzj621B7CTYaBSLdn68oJJulU0Q@public.gmane.org>
-1 siblings, 1 reply; 31+ messages in thread
From: Thierry Reding @ 2012-12-11 7:03 UTC (permalink / raw)
To: Peter Ujfalusi
Cc: Bryan Wu, Richard Purdie, Grant Likely, linux-kernel,
devicetree-discuss, linux-doc, linux-leds
[-- Attachment #1: Type: text/plain, Size: 1441 bytes --]
On Mon, Dec 10, 2012 at 11:00:34AM +0100, Peter Ujfalusi 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 <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;
> }
The commit message says that legacy mode is used as fallback if
devm_get_pwm() (that should really be devm_pwm_get() btw) fails but I
don't see where pwm_request() is called.
Thierry
[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply [flat|nested] 31+ messages in thread
[parent not found: <1355133637-2784-1-git-send-email-peter.ujfalusi-l0cyMroinI0@public.gmane.org>]
* [PATCH v3 2/4] leds: leds-pwm: Preparing the driver for device tree support
2012-12-10 10:00 ` Peter Ujfalusi
@ 2012-12-10 10:00 ` Peter Ujfalusi
-1 siblings, 0 replies; 31+ messages in thread
From: Peter Ujfalusi @ 2012-12-10 10:00 UTC (permalink / raw)
To: Bryan Wu, Richard Purdie, Grant Likely, Thierry Reding
Cc: devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-leds-u79uwXL29TY76Z2rM5mHXA,
linux-doc-u79uwXL29TY76Z2rM5mHXA
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..02f0c0c 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 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
^ permalink raw reply related [flat|nested] 31+ messages in thread* [PATCH v3 2/4] leds: leds-pwm: Preparing the driver for device tree support
@ 2012-12-10 10:00 ` Peter Ujfalusi
0 siblings, 0 replies; 31+ messages in thread
From: Peter Ujfalusi @ 2012-12-10 10:00 UTC (permalink / raw)
To: Bryan Wu, Richard Purdie, Grant Likely, Thierry Reding
Cc: linux-kernel, devicetree-discuss, linux-doc, linux-leds
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@ti.com>
---
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..02f0c0c 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 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
^ permalink raw reply related [flat|nested] 31+ messages in thread[parent not found: <1355133637-2784-3-git-send-email-peter.ujfalusi-l0cyMroinI0@public.gmane.org>]
* Re: [PATCH v3 2/4] leds: leds-pwm: Preparing the driver for device tree support
2012-12-10 10:00 ` Peter Ujfalusi
@ 2012-12-11 7:12 ` Thierry Reding
-1 siblings, 0 replies; 31+ messages in thread
From: Thierry Reding @ 2012-12-11 7:12 UTC (permalink / raw)
To: Peter Ujfalusi
Cc: linux-doc-u79uwXL29TY76Z2rM5mHXA,
devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ, Bryan Wu,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, Richard Purdie,
linux-leds-u79uwXL29TY76Z2rM5mHXA
[-- Attachment #1.1: Type: text/plain, Size: 2205 bytes --]
On Mon, Dec 10, 2012 at 11:00:35AM +0100, Peter Ujfalusi wrote:
> 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..02f0c0c 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[];
> +};
I think you want leds[0] here. Otherwise your structure is too large by
sizeof(struct led_pwm_data *).
> +
> 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)
Perhaps this should return size_t?
> +{
> + 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);
I'm not sure if sizeof_pwm_leds_priv() requires to be a separate
function. You could make it shorter by doing something like:
size_t extra = sizeof(*led_dat) * pdata->num_leds;
priv = devm_kzalloc(&pdev->dev, sizeof(*priv) + extra, GFP_KERNEL);
But that's really just a matter of taste, so no further objections if
you want to keep the inline function.
Thierry
[-- Attachment #1.2: Type: application/pgp-signature, Size: 836 bytes --]
[-- Attachment #2: Type: text/plain, Size: 192 bytes --]
_______________________________________________
devicetree-discuss mailing list
devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org
https://lists.ozlabs.org/listinfo/devicetree-discuss
^ permalink raw reply [flat|nested] 31+ messages in thread* Re: [PATCH v3 2/4] leds: leds-pwm: Preparing the driver for device tree support
@ 2012-12-11 7:12 ` Thierry Reding
0 siblings, 0 replies; 31+ messages in thread
From: Thierry Reding @ 2012-12-11 7:12 UTC (permalink / raw)
To: Peter Ujfalusi
Cc: Bryan Wu, Richard Purdie, Grant Likely, linux-kernel,
devicetree-discuss, linux-doc, linux-leds
[-- Attachment #1: Type: text/plain, Size: 2183 bytes --]
On Mon, Dec 10, 2012 at 11:00:35AM +0100, Peter Ujfalusi wrote:
> 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@ti.com>
> ---
> 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..02f0c0c 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[];
> +};
I think you want leds[0] here. Otherwise your structure is too large by
sizeof(struct led_pwm_data *).
> +
> 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)
Perhaps this should return size_t?
> +{
> + 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);
I'm not sure if sizeof_pwm_leds_priv() requires to be a separate
function. You could make it shorter by doing something like:
size_t extra = sizeof(*led_dat) * pdata->num_leds;
priv = devm_kzalloc(&pdev->dev, sizeof(*priv) + extra, GFP_KERNEL);
But that's really just a matter of taste, so no further objections if
you want to keep the inline function.
Thierry
[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH v3 3/4] pwm: core: Export of_pwm_request() so client drivers can also use it
2012-12-10 10:00 ` Peter Ujfalusi
@ 2012-12-10 10:00 ` Peter Ujfalusi
-1 siblings, 0 replies; 31+ messages in thread
From: Peter Ujfalusi @ 2012-12-10 10:00 UTC (permalink / raw)
To: Bryan Wu, Richard Purdie, Grant Likely, Thierry Reding
Cc: linux-kernel, devicetree-discuss, linux-doc, linux-leds
Allow client driver to use of_pwm_request() 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@ti.com>
---
drivers/pwm/core.c | 2 +-
include/linux/pwm.h | 7 +++++++
2 files changed, 8 insertions(+), 1 deletion(-)
diff --git a/drivers/pwm/core.c b/drivers/pwm/core.c
index 903138b..3a7ebcc 100644
--- a/drivers/pwm/core.c
+++ b/drivers/pwm/core.c
@@ -486,7 +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,
+struct pwm_device *of_pwm_request(struct device_node *np,
const char *con_id)
{
struct pwm_device *pwm = NULL;
diff --git a/include/linux/pwm.h b/include/linux/pwm.h
index 6d661f3..d70ffe3 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 *consumer);
+struct pwm_device *of_pwm_request(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 *consumer);
@@ -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_request(struct device_node *np,
+ const char *con_id)
+{
+ return ERR_PTR(-ENODEV);
+}
+
static inline void pwm_put(struct pwm_device *pwm)
{
}
--
1.8.0
^ permalink raw reply related [flat|nested] 31+ messages in thread* [PATCH v3 3/4] pwm: core: Export of_pwm_request() so client drivers can also use it
@ 2012-12-10 10:00 ` Peter Ujfalusi
0 siblings, 0 replies; 31+ messages in thread
From: Peter Ujfalusi @ 2012-12-10 10:00 UTC (permalink / raw)
To: Bryan Wu, Richard Purdie, Grant Likely, Thierry Reding
Cc: linux-kernel, devicetree-discuss, linux-doc, linux-leds
Allow client driver to use of_pwm_request() 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@ti.com>
---
drivers/pwm/core.c | 2 +-
include/linux/pwm.h | 7 +++++++
2 files changed, 8 insertions(+), 1 deletion(-)
diff --git a/drivers/pwm/core.c b/drivers/pwm/core.c
index 903138b..3a7ebcc 100644
--- a/drivers/pwm/core.c
+++ b/drivers/pwm/core.c
@@ -486,7 +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,
+struct pwm_device *of_pwm_request(struct device_node *np,
const char *con_id)
{
struct pwm_device *pwm = NULL;
diff --git a/include/linux/pwm.h b/include/linux/pwm.h
index 6d661f3..d70ffe3 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 *consumer);
+struct pwm_device *of_pwm_request(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 *consumer);
@@ -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_request(struct device_node *np,
+ const char *con_id)
+{
+ return ERR_PTR(-ENODEV);
+}
+
static inline void pwm_put(struct pwm_device *pwm)
{
}
--
1.8.0
^ permalink raw reply related [flat|nested] 31+ messages in thread[parent not found: <1355133637-2784-4-git-send-email-peter.ujfalusi-l0cyMroinI0@public.gmane.org>]
* Re: [PATCH v3 3/4] pwm: core: Export of_pwm_request() so client drivers can also use it
2012-12-10 10:00 ` Peter Ujfalusi
@ 2012-12-10 23:22 ` Grant Likely
-1 siblings, 0 replies; 31+ messages in thread
From: Grant Likely @ 2012-12-10 23:22 UTC (permalink / raw)
To: Peter Ujfalusi, Bryan Wu, Richard Purdie, Thierry Reding
Cc: devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-leds-u79uwXL29TY76Z2rM5mHXA,
linux-doc-u79uwXL29TY76Z2rM5mHXA
On Mon, 10 Dec 2012 11:00:36 +0100, Peter Ujfalusi <peter.ujfalusi-l0cyMroinI0@public.gmane.org> wrote:
> Allow client driver to use of_pwm_request() 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>
Acked-by: Grant Likely <grant.likely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org>
> ---
> drivers/pwm/core.c | 2 +-
> include/linux/pwm.h | 7 +++++++
> 2 files changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/pwm/core.c b/drivers/pwm/core.c
> index 903138b..3a7ebcc 100644
> --- a/drivers/pwm/core.c
> +++ b/drivers/pwm/core.c
> @@ -486,7 +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,
> +struct pwm_device *of_pwm_request(struct device_node *np,
> const char *con_id)
> {
> struct pwm_device *pwm = NULL;
> diff --git a/include/linux/pwm.h b/include/linux/pwm.h
> index 6d661f3..d70ffe3 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 *consumer);
> +struct pwm_device *of_pwm_request(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 *consumer);
> @@ -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_request(struct device_node *np,
> + const char *con_id)
> +{
> + return ERR_PTR(-ENODEV);
> +}
> +
> static inline void pwm_put(struct pwm_device *pwm)
> {
> }
> --
> 1.8.0
>
--
Grant Likely, B.Sc, P.Eng.
Secret Lab Technologies, Ltd.
^ permalink raw reply [flat|nested] 31+ messages in thread* Re: [PATCH v3 3/4] pwm: core: Export of_pwm_request() so client drivers can also use it
@ 2012-12-10 23:22 ` Grant Likely
0 siblings, 0 replies; 31+ messages in thread
From: Grant Likely @ 2012-12-10 23:22 UTC (permalink / raw)
To: Peter Ujfalusi, Bryan Wu, Richard Purdie, Thierry Reding
Cc: linux-kernel, devicetree-discuss, linux-doc, linux-leds
On Mon, 10 Dec 2012 11:00:36 +0100, Peter Ujfalusi <peter.ujfalusi@ti.com> wrote:
> Allow client driver to use of_pwm_request() 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@ti.com>
Acked-by: Grant Likely <grant.likely@secretlab.ca>
> ---
> drivers/pwm/core.c | 2 +-
> include/linux/pwm.h | 7 +++++++
> 2 files changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/pwm/core.c b/drivers/pwm/core.c
> index 903138b..3a7ebcc 100644
> --- a/drivers/pwm/core.c
> +++ b/drivers/pwm/core.c
> @@ -486,7 +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,
> +struct pwm_device *of_pwm_request(struct device_node *np,
> const char *con_id)
> {
> struct pwm_device *pwm = NULL;
> diff --git a/include/linux/pwm.h b/include/linux/pwm.h
> index 6d661f3..d70ffe3 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 *consumer);
> +struct pwm_device *of_pwm_request(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 *consumer);
> @@ -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_request(struct device_node *np,
> + const char *con_id)
> +{
> + return ERR_PTR(-ENODEV);
> +}
> +
> static inline void pwm_put(struct pwm_device *pwm)
> {
> }
> --
> 1.8.0
>
--
Grant Likely, B.Sc, P.Eng.
Secret Lab Technologies, Ltd.
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH v3 3/4] pwm: core: Export of_pwm_request() so client drivers can also use it
2012-12-10 10:00 ` Peter Ujfalusi
(?)
(?)
@ 2012-12-11 7:00 ` Thierry Reding
-1 siblings, 0 replies; 31+ messages in thread
From: Thierry Reding @ 2012-12-11 7:00 UTC (permalink / raw)
To: Peter Ujfalusi
Cc: Bryan Wu, Richard Purdie, Grant Likely, linux-kernel,
devicetree-discuss, linux-doc, linux-leds
[-- Attachment #1: Type: text/plain, Size: 2319 bytes --]
On Mon, Dec 10, 2012 at 11:00:36AM +0100, Peter Ujfalusi wrote:
> Allow client driver to use of_pwm_request() to get the pwm they need. This
> is needed for drivers which handle more than one pwm separately, like
> leds-pwm driver which have:
Hi Peter,
I really was hoping that we didn't have to export this function, but I
can't think of any other way to solve the problem at hand either. I'd
prefer to rename the function to of_pwm_get() at the same time to keep
consistent with other subsystems that provide similar functionality.
Also, please use all-caps for PWM in prose. And while at it, you can
drop the "core:" and "so client drivers can also use it" from the
subject line.
> 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@ti.com>
> ---
> drivers/pwm/core.c | 2 +-
> include/linux/pwm.h | 7 +++++++
> 2 files changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/pwm/core.c b/drivers/pwm/core.c
> index 903138b..3a7ebcc 100644
> --- a/drivers/pwm/core.c
> +++ b/drivers/pwm/core.c
> @@ -486,7 +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,
> +struct pwm_device *of_pwm_request(struct device_node *np,
> const char *con_id)
> {
> struct pwm_device *pwm = NULL;
This is missing an EXPORT_SYMBOL_GPL.
> diff --git a/include/linux/pwm.h b/include/linux/pwm.h
> index 6d661f3..d70ffe3 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 *consumer);
> +struct pwm_device *of_pwm_request(struct device_node *np, const char *con_id);
While at it, maybe rename the con_id parameter as well to match
pwm_get().
Thierry
[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH v3 4/4] leds: leds-pwm: Add device tree bindings
2012-12-10 10:00 ` Peter Ujfalusi
@ 2012-12-10 10:00 ` Peter Ujfalusi
-1 siblings, 0 replies; 31+ messages in thread
From: Peter Ujfalusi @ 2012-12-10 10:00 UTC (permalink / raw)
To: Bryan Wu, Richard Purdie, Grant Likely, Thierry Reding
Cc: linux-kernel, devicetree-discuss, linux-doc, linux-leds
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>
---
.../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 0000000..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;
- 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);
+ pwm_put(led_dat->pwm);
goto err;
+ }
+ priv->num_leds++;
+ }
+
+ return priv;
+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);
+ }
+ }
+
+ 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);
@@ -114,21 +184,33 @@ err:
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);
+ }
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
^ permalink raw reply related [flat|nested] 31+ messages in thread* [PATCH v3 4/4] leds: leds-pwm: Add device tree bindings
@ 2012-12-10 10:00 ` Peter Ujfalusi
0 siblings, 0 replies; 31+ messages in thread
From: Peter Ujfalusi @ 2012-12-10 10:00 UTC (permalink / raw)
To: Bryan Wu, Richard Purdie, Grant Likely, Thierry Reding
Cc: linux-kernel, devicetree-discuss, linux-doc, linux-leds
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>
---
.../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 0000000..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;
- 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);
+ pwm_put(led_dat->pwm);
goto err;
+ }
+ priv->num_leds++;
+ }
+
+ return priv;
+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);
+ }
+ }
+
+ 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);
@@ -114,21 +184,33 @@ err:
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);
+ }
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
^ permalink raw reply related [flat|nested] 31+ messages in thread* Re: [PATCH v3 4/4] leds: leds-pwm: Add device tree bindings
2012-12-10 10:00 ` Peter Ujfalusi
(?)
@ 2012-12-10 23:23 ` Grant Likely
-1 siblings, 0 replies; 31+ messages in thread
From: Grant Likely @ 2012-12-10 23:23 UTC (permalink / raw)
To: Peter Ujfalusi, Bryan Wu, Richard Purdie, Thierry Reding
Cc: linux-kernel, devicetree-discuss, linux-doc, linux-leds
On Mon, 10 Dec 2012 11:00:37 +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>
Acked-by: Grant Likely <grant.likely@secretlab.ca>
The other patches in this series look good to.
g.
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH v3 4/4] leds: leds-pwm: Add device tree bindings
2012-12-10 10:00 ` Peter Ujfalusi
(?)
(?)
@ 2012-12-11 7:25 ` Thierry Reding
[not found] ` <20121211072545.GD8294-RM9K5IK7kjIyiCvfTdI0JKcOhU4Rzj621B7CTYaBSLdn68oJJulU0Q@public.gmane.org>
-1 siblings, 1 reply; 31+ messages in thread
From: Thierry Reding @ 2012-12-11 7:25 UTC (permalink / raw)
To: Peter Ujfalusi
Cc: Bryan Wu, Richard Purdie, Grant Likely, linux-kernel,
devicetree-discuss, linux-doc, linux-leds
[-- Attachment #1: Type: text/plain, Size: 1362 bytes --]
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
[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply [flat|nested] 31+ messages in thread