* [PATCH v3 1/4] leds: leds-pwm: Convert to use devm_get_pwm
2012-12-10 10:00 [PATCH v3 0/4] leds: leds-pwm: Device tree support Peter Ujfalusi
@ 2012-12-10 10:00 ` Peter Ujfalusi
2012-12-11 7:03 ` Thierry Reding
2012-12-10 10:00 ` [PATCH v3 2/4] leds: leds-pwm: Preparing the driver for device tree support Peter Ujfalusi
` (2 subsequent siblings)
3 siblings, 1 reply; 18+ 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] 18+ messages in thread* Re: [PATCH v3 1/4] leds: leds-pwm: Convert to use devm_get_pwm
2012-12-10 10:00 ` [PATCH v3 1/4] leds: leds-pwm: Convert to use devm_get_pwm Peter Ujfalusi
@ 2012-12-11 7:03 ` Thierry Reding
[not found] ` <3193669.Hv54bBklsP@barack>
0 siblings, 1 reply; 18+ 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] 18+ messages in thread
* [PATCH v3 2/4] leds: leds-pwm: Preparing the driver for device tree support
2012-12-10 10:00 [PATCH v3 0/4] leds: leds-pwm: Device tree support Peter Ujfalusi
2012-12-10 10:00 ` [PATCH v3 1/4] leds: leds-pwm: Convert to use devm_get_pwm Peter Ujfalusi
@ 2012-12-10 10:00 ` Peter Ujfalusi
2012-12-11 7:12 ` Thierry Reding
2012-12-10 10:00 ` [PATCH v3 3/4] pwm: core: Export of_pwm_request() so client drivers can also use it Peter Ujfalusi
2012-12-10 10:00 ` [PATCH v3 4/4] leds: leds-pwm: Add device tree bindings Peter Ujfalusi
3 siblings, 1 reply; 18+ 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] 18+ messages in thread* Re: [PATCH v3 2/4] leds: leds-pwm: Preparing the driver for device tree support
2012-12-10 10:00 ` [PATCH v3 2/4] leds: leds-pwm: Preparing the driver for device tree support Peter Ujfalusi
@ 2012-12-11 7:12 ` Thierry Reding
0 siblings, 0 replies; 18+ 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] 18+ 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 [PATCH v3 0/4] leds: leds-pwm: Device tree support Peter Ujfalusi
2012-12-10 10:00 ` [PATCH v3 1/4] leds: leds-pwm: Convert to use devm_get_pwm Peter Ujfalusi
2012-12-10 10:00 ` [PATCH v3 2/4] leds: leds-pwm: Preparing the driver for device tree support Peter Ujfalusi
@ 2012-12-10 10:00 ` Peter Ujfalusi
2012-12-10 23:22 ` Grant Likely
2012-12-11 7:00 ` Thierry Reding
2012-12-10 10:00 ` [PATCH v3 4/4] leds: leds-pwm: Add device tree bindings Peter Ujfalusi
3 siblings, 2 replies; 18+ 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] 18+ 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 ` [PATCH v3 3/4] pwm: core: Export of_pwm_request() so client drivers can also use it Peter Ujfalusi
@ 2012-12-10 23:22 ` Grant Likely
2012-12-11 7:00 ` Thierry Reding
1 sibling, 0 replies; 18+ 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] 18+ 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 ` [PATCH v3 3/4] pwm: core: Export of_pwm_request() so client drivers can also use it Peter Ujfalusi
2012-12-10 23:22 ` Grant Likely
@ 2012-12-11 7:00 ` Thierry Reding
1 sibling, 0 replies; 18+ 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] 18+ messages in thread
* [PATCH v3 4/4] leds: leds-pwm: Add device tree bindings
2012-12-10 10:00 [PATCH v3 0/4] leds: leds-pwm: Device tree support Peter Ujfalusi
` (2 preceding siblings ...)
2012-12-10 10:00 ` [PATCH v3 3/4] pwm: core: Export of_pwm_request() so client drivers can also use it Peter Ujfalusi
@ 2012-12-10 10:00 ` Peter Ujfalusi
2012-12-10 23:23 ` Grant Likely
2012-12-11 7:25 ` Thierry Reding
3 siblings, 2 replies; 18+ 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] 18+ messages in thread* Re: [PATCH v3 4/4] leds: leds-pwm: Add device tree bindings
2012-12-10 10:00 ` [PATCH v3 4/4] leds: leds-pwm: Add device tree bindings Peter Ujfalusi
@ 2012-12-10 23:23 ` Grant Likely
2012-12-11 7:25 ` Thierry Reding
1 sibling, 0 replies; 18+ 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] 18+ messages in thread
* Re: [PATCH v3 4/4] leds: leds-pwm: Add device tree bindings
2012-12-10 10:00 ` [PATCH v3 4/4] leds: leds-pwm: Add device tree bindings Peter Ujfalusi
2012-12-10 23:23 ` Grant Likely
@ 2012-12-11 7:25 ` Thierry Reding
2012-12-12 9:00 ` Peter Ujfalusi
1 sibling, 1 reply; 18+ 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] 18+ messages in thread* Re: [PATCH v3 4/4] leds: leds-pwm: Add device tree bindings
2012-12-11 7:25 ` Thierry Reding
@ 2012-12-12 9:00 ` Peter Ujfalusi
0 siblings, 0 replies; 18+ messages in thread
From: Peter Ujfalusi @ 2012-12-12 9:00 UTC (permalink / raw)
To: Thierry Reding
Cc: Bryan Wu, Richard Purdie, Grant Likely, linux-kernel,
devicetree-discuss, linux-doc, linux-leds
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
^ permalink raw reply [flat|nested] 18+ messages in thread