leds: pwm: Simplify with resource-managed devm_led_classdev_register()
Subsystems (2)
PATCH 1/4 leds: pwm: Simplify with resource-managed devm_led_classdev_register()
Simplify the exit path with resource-managed version of registering LED class device. The code should be functionally the same, except that on device removal the led_pwm_priv->num_leds is not decremented to zero (which should not have any effect as device is going away).
--- drivers/leds/leds-pwm.c | 22 ++-------------------- 1 file changed, 2 insertions(+), 20 deletions(-)
drivers/leds/leds-pwm.c +2 -20
@@ -74,12 +74,6 @@ static inline size_t sizeof_pwm_leds_priv(int num_leds) (sizeof (struct led_pwm_data ) * num_leds); } -static void led_pwm_cleanup (struct led_pwm_priv *priv) -{ -while (priv->num_leds--) -led_classdev_unregister (&priv->leds[priv->num_leds].cdev); -} -static int led_pwm_add (struct device *dev,struct led_pwm_priv *priv,struct led_pwm *led,struct device_node *child) {
@@ -120,7 +114,7 @@ static int led_pwm_add(struct device *dev, struct led_pwm_priv *priv,if (!led_data->period && (led->pwm_period_ns >0 )) led_data->period = led->pwm_period_ns; - ret =led_classdev_register (dev, &led_data->cdev); + ret =devm_led_classdev_register (dev, &led_data->cdev);if (ret ==0 ) { priv->num_leds++;led_pwm_set (&led_data->cdev, led_data->cdev.brightness);
@@ -191,25 +185,14 @@ static int led_pwm_probe(struct platform_device *pdev) ret =led_pwm_create_of (&pdev->dev, priv); } -if (ret) { -led_pwm_cleanup (priv); +if (ret)return ret; - }platform_set_drvdata (pdev, priv);return 0 ; } -static int led_pwm_remove (struct platform_device *pdev) -{ -struct led_pwm_priv *priv =platform_get_drvdata (pdev); - -led_pwm_cleanup (priv); - -return 0 ; -} -static const struct of_device_id of_pwm_leds_match[] = { { .compatible ="pwm-leds" , }, {},
@@ -218,7 +201,6 @@ 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" , .of_match_table = of_pwm_leds_match,
Signature · 1 line
2.7.4
PATCH 2/4 leds: pwm: Use OF variant of LED registering function
The PWM leds can be instantiated from Device Tree so pass the respective device node to LED core. This provides the LED system with proper device node and exposes it through uevent.
--- drivers/leds/leds-pwm.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/leds/leds-pwm.c +1 -1
@@ -114,7 +114,7 @@ static int led_pwm_add(struct device *dev, struct led_pwm_priv *priv,if (!led_data->period && (led->pwm_period_ns >0 )) led_data->period = led->pwm_period_ns; - ret =devm_led_classdev_register (dev, &led_data->cdev); + ret =devm_of_led_classdev_register (dev, child, &led_data->cdev);if (ret ==0 ) { priv->num_leds++;led_pwm_set (&led_data->cdev, led_data->cdev.brightness);
Signature · 1 line
2.7.4
PATCH 3/4 dt-bindings: leds: Add pattern initialization from Device Tree
In reply to Krzysztof KozlowskiDocument new linux,trigger-pattern property for initialization of LED pattern trigger.
--- Documentation/devicetree/bindings/leds/common.txt | 7 +++++++ 1 file changed, 7 insertions(+)
Documentation/devicetree/bindings/leds/common.txt +7 -0
@@ -37,6 +37,13 @@ Optional properties for child nodes: "ide-disk" - LED indicates IDE disk activity (deprecated), in new implementations use "disk-activity" "timer" - LED flashes at a fixed, configurable rate + "pattern" - LED alters the brightness for the specified duration with one + software timer (see + Documentation/ABI/testing/sysfs-class-led-trigger-pattern) + +- linux,trigger-pattern : Default pattern for "pattern" trigger (see + Documentation/ABI/testing/sysfs-class-led-trigger-pattern + for description of format) - led-max-microamp : Maximum LED supply current in microamperes. This property can be made mandatory for the board configurations
Signature · 1 line
2.7.4
PATCH 4/4 leds: trigger: Add pattern initialization from Device Tree
In reply to Krzysztof KozlowskiAllow initialization of pattern used in pattern trigger from Device Tree property.
This is especially useful for embedded systems where the pattern trigger would be used to indicate the process of boot status in a nice, user-friendly blinking way. This initialization pattern will be used till user-space is brought up and sets its own pattern, indicating the boot status is for example finished.
--- drivers/leds/trigger/ledtrig-pattern.c | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+)
drivers/leds/trigger/ledtrig-pattern.c +18 -0
@@ -11,6 +11,7 @@#include <linux/leds.h> #include <linux/module.h> #include <linux/mutex.h> +#include <linux/of.h> #include <linux/slab.h> #include <linux/timer.h>
@@ -331,6 +332,21 @@ static const struct attribute_group *pattern_trig_groups[] = { NULL, }; +static void pattern_init (struct led_classdev *led_cdev) +{ +struct device_node *np =dev_of_node (led_cdev->dev); +const char *pattern; + +if (!np) +return ; + +if (!of_property_read_string (np,"linux,trigger-pattern" , &pattern)) { +if (strlen (pattern)) +pattern_trig_store_patterns (led_cdev, pattern, +strlen (pattern), false); + } +} +static int pattern_trig_activate (struct led_classdev *led_cdev) {struct pattern_trig_data *data;
@@ -354,6 +370,8 @@ static int pattern_trig_activate(struct led_classdev *led_cdev)timer_setup (&data->timer, pattern_trig_timer_function,0 ); led_cdev->activated = true; +pattern_init (led_cdev); +return 0 ; }
Signature · 1 line
2.7.4
Re: [PATCH 4/4] leds: trigger: Add pattern initialization from Device Tree
Hi Krzysztof,
Thank you for the patch set.
Applied 1/4 and 2/4.
I'll hold on merging 3/4 until we sort out the issues I have with this one. Please refer to my comment below.
Quoted 49 lines · Krzysztof Kozlowski
Allow initialization of pattern used in pattern trigger from Device Tree property.
This is especially useful for embedded systems where the pattern trigger would be used to indicate the process of boot status in a nice, user-friendly blinking way. This initialization pattern will be used till user-space is brought up and sets its own pattern, indicating the boot status is for example finished.
Signed-off-by Krzysztof Kozlowski <[email protected]>--- drivers/leds/trigger/ledtrig-pattern.c | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+)drivers/leds/trigger/ledtrig-pattern.c +16
-0@@ -11,6 +11,7 @@#include <linux/leds.h> #include <linux/module.h> #include <linux/mutex.h> +#include <linux/of.h> #include <linux/slab.h> #include <linux/timer.h> @@ -331,6 +332,21 @@ static const struct attribute_group *pattern_trig_groups[] = { NULL, }; +static void pattern_init (struct led_classdev *led_cdev) +{ +struct device_node *np =dev_of_node (led_cdev->dev); +const char *pattern; + +if (!np) +return ; + +if (!of_property_read_string (np,"linux,trigger-pattern" , &pattern)) { +if (strlen (pattern)) +pattern_trig_store_patterns (led_cdev, pattern, +strlen (pattern), false); + } +} +static int pattern_trig_activate (struct led_classdev *led_cdev) {struct pattern_trig_data *data;
drivers/leds/trigger/ledtrig-pattern.c static int pattern_trig_activate(struct led_classdev *led_cdev) · line 373
Quoted 4 lines
drivers/leds/trigger/ledtrig-pattern.c +1
-0@@ -354,6 +370,8 @@ static int pattern_trig_activate(struct led_classdev *led_cdev)timer_setup (&data->timer, pattern_trig_timer_function,0 ); led_cdev->activated = true; +pattern_init (led_cdev);
It means that the pattern defined in DT would be applied whenever the pattern trigger is set for a LED class device, whereas it should happen only on LED class device initialization, if linux,default-trigger is set to "pattern".
What we would need for making that work is a generic support for parsing trigger specific initialization data in the of_led_classdev_register().
Quoted 5 lines
drivers/leds/trigger/ledtrig-pattern.c +1
-0+return 0 ; }
Signature · 2 lines
Best regards, Jacek Anaszewski
Re: [PATCH 4/4] leds: trigger: Add pattern initialization from Device Tree
Quoted 55 lines · Jacek Anaszewski
Hi Krzysztof,
Thank you for the patch set.
Applied 1/4 and 2/4.
I'll hold on merging 3/4 until we sort out the issues I have with this one. Please refer to my comment below.
Quoted 45 lines · Krzysztof Kozlowski
Allow initialization of pattern used in pattern trigger from Device Tree property.
This is especially useful for embedded systems where the pattern trigger would be used to indicate the process of boot status in a nice, user-friendly blinking way. This initialization pattern will be used till user-space is brought up and sets its own pattern, indicating the boot status is for example finished.
Signed-off-by Krzysztof Kozlowski <[email protected]>---
drivers/leds/trigger/ledtrig-pattern.c| 18 ++++++++++++++++++1 file changed, 18 insertions(+)drivers/leds/trigger/ledtrig-pattern.c +16
-0@@ -11,6 +11,7 @@ #include <linux/leds.h> #include <linux/module.h> #include <linux/mutex.h> +#include <linux/of.h> #include <linux/slab.h> #include <linux/timer.h>@@ -331,6 +332,21 @@ static const struct attribute_group *pattern_trig_groups[] = { NULL, }; +static void pattern_init (struct led_classdev *led_cdev) +{ + struct device_node *np = dev_of_node(led_cdev->dev); + const char *pattern; + + if (!np) + return; + + if (!of_property_read_string(np, "linux,trigger-pattern", &pattern)) { + if (strlen(pattern)) + pattern_trig_store_patterns(led_cdev, pattern, + strlen(pattern), false); + } +} +
Quoted 8 lines
drivers/leds/trigger/ledtrig-pattern.c static int pattern_trig_activate(struct · line 373
Quoted 8 lines
drivers/leds/trigger/ledtrig-pattern.c +0
-0static int pattern_trig_activate(struct led_classdev *led_cdev) {struct
pattern_trig_data*data;drivers/leds/trigger/ledtrig-pattern.c +1
-0@@ -354,6 +370,8 @@ static int pattern_trig_activate(struct led_classdev *led_cdev) timer_setup(&data->timer, pattern_trig_timer_function, 0); led_cdev->activated = true; + pattern_init(led_cdev);
With my recent patches it would suffice to replace above line with:
if (led_cdev->flags & LED_INIT_DEFAULT_TRIGGER) {
pattern_init(led_cdev);
led_cdev->flags &= ~LED_INIT_DEFAULT_TRIGGER;
}Quoted 14 lines
It means that the pattern defined in DT would be applied whenever the pattern trigger is set for a LED class device, whereas it should happen only on LED class device initialization, if linux,default-trigger is set to "pattern".
What we would need for making that work is a generic support for parsing trigger specific initialization data in the
of_led_classdev_register().Quoted 4 lines
drivers/leds/trigger/ledtrig-pattern.c +1
-0+ return 0; }
Signature · 2 lines
Best regards, Jacek Anaszewski
Re: [PATCH 4/4] leds: trigger: Add pattern initialization from Device Tree
Quoted 69 lines · Jacek Anaszewski
Quoted 56 lines · Jacek Anaszewski
Hi Krzysztof,
Thank you for the patch set.
Applied 1/4 and 2/4.
I'll hold on merging 3/4 until we sort out the issues I have with this one. Please refer to my comment below.
Quoted 46 lines · Krzysztof Kozlowski
Allow initialization of pattern used in pattern trigger from Device Tree property.
This is especially useful for embedded systems where the pattern trigger would be used to indicate the process of boot status in a nice, user-friendly blinking way. This initialization pattern will be used till user-space is brought up and sets its own pattern, indicating the boot status is for example finished.
Signed-off-by Krzysztof Kozlowski <[email protected]>--- drivers/leds/trigger/ledtrig-pattern.c | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+)drivers/leds/trigger/ledtrig-pattern.c +16
-0@@ -11,6 +11,7 @@#include <linux/leds.h> #include <linux/module.h> #include <linux/mutex.h> +#include <linux/of.h> #include <linux/slab.h> #include <linux/timer.h> @@ -331,6 +332,21 @@ static const struct attribute_group *pattern_trig_groups[] = { NULL, }; +static void pattern_init (struct led_classdev *led_cdev) +{ +struct device_node *np =dev_of_node (led_cdev->dev); +const char *pattern; + +if (!np) +return ; + +if (!of_property_read_string (np,"linux,trigger-pattern" , &pattern)) { +if (strlen (pattern)) +pattern_trig_store_patterns (led_cdev, pattern, +strlen (pattern), false); + } +} +Quoted 8 lines
drivers/leds/trigger/ledtrig-pattern.c static int pattern_trig_activate(struct · line 373
Quoted 8 lines
drivers/leds/trigger/ledtrig-pattern.c +0
-0static int pattern_trig_activate (struct led_classdev *led_cdev) {struct pattern_trig_data *data;drivers/leds/trigger/ledtrig-pattern.c +1
-0@@ -354,6 +370,8 @@ static int pattern_trig_activate(struct led_classdev *led_cdev) timer_setup(&data->timer, pattern_trig_timer_function, 0); led_cdev->activated = true; +pattern_init (led_cdev);With my recent patches it would suffice to replace above line with:
Quoted 4 lines
if (led_cdev->flags & LED_INIT_DEFAULT_TRIGGER) {
pattern_init(led_cdev);
led_cdev->flags &= ~LED_INIT_DEFAULT_TRIGGER;
}Right, I see the problem. I'll rebase on top of it.
Best regards,
Krzysztof
Re: [PATCH 4/4] leds: trigger: Add pattern initialization from Device Tree
In reply to Krzysztof KozlowskiQuoted 34 lines · Krzysztof Kozlowski
Allow initialization of pattern used in pattern trigger from Device Tree property.
This is especially useful for embedded systems where the pattern trigger would be used to indicate the process of boot status in a nice, user-friendly blinking way. This initialization pattern will be used till user-space is brought up and sets its own pattern, indicating the boot status is for example finished.
Signed-off-by Krzysztof Kozlowski <[email protected]>--- drivers/leds/trigger/ledtrig-pattern.c | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+)drivers/leds/trigger/ledtrig-pattern.c +5
-0@@ -11,6 +11,7 @@#include <linux/leds.h> #include <linux/module.h> #include <linux/mutex.h> +#include <linux/of.h> #include <linux/slab.h> #include <linux/timer.h> @@ -331,6 +332,21 @@ static const struct attribute_group *pattern_trig_groups[] = { NULL, }; +static void pattern_init (struct led_classdev *led_cdev) +{ +struct device_node *np =dev_of_node (led_cdev->dev); +const char *pattern;
drivers/leds/trigger/ledtrig-pattern.c static const struct attribute_group *pattern_trig_groups[] = { · line 348
Quoted 10 lines
drivers/leds/trigger/ledtrig-pattern.c +10
-0+ +if (!np) +return ; + +if (!of_property_read_string (np,"linux,trigger-pattern" , &pattern)) { +if (strlen (pattern)) +pattern_trig_store_patterns (led_cdev, pattern, +strlen (pattern), false); + } +}
" if (of_...())
return;
if (!strlen())
return"
Check return value of pattern_trig_store_patterns().
Pavel
Signature · 2 lines
(english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
Re: [PATCH 4/4] leds: trigger: Add pattern initialization from Device Tree
Quoted 48 lines · Pavel Machek
Quoted 34 lines · Krzysztof Kozlowski
Allow initialization of pattern used in pattern trigger from Device Tree property.
This is especially useful for embedded systems where the pattern trigger would be used to indicate the process of boot status in a nice, user-friendly blinking way. This initialization pattern will be used till user-space is brought up and sets its own pattern, indicating the boot status is for example finished.
Signed-off-by Krzysztof Kozlowski <[email protected]>--- drivers/leds/trigger/ledtrig-pattern.c | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+)drivers/leds/trigger/ledtrig-pattern.c +5
-0@@ -11,6 +11,7 @@#include <linux/leds.h> #include <linux/module.h> #include <linux/mutex.h> +#include <linux/of.h> #include <linux/slab.h> #include <linux/timer.h> @@ -331,6 +332,21 @@ static const struct attribute_group *pattern_trig_groups[] = { NULL, }; +static void pattern_init (struct led_classdev *led_cdev) +{ +struct device_node *np =dev_of_node (led_cdev->dev); +const char *pattern;drivers/leds/trigger/ledtrig-pattern.c static const struct attribute_group *pattern_trig_groups[] = { · line 348
Quoted 10 lines
drivers/leds/trigger/ledtrig-pattern.c +10
-0+ +if (!np) +return ; + +if (!of_property_read_string (np,"linux,trigger-pattern" , &pattern)) { +if (strlen (pattern)) +pattern_trig_store_patterns (led_cdev, pattern, +strlen (pattern), false); + } +}" if (of_...())
Quoted 4 lines
return;if (!strlen()) return"
Sure.
Quoted 2 lines
Check return value of
pattern_trig_store_patterns().
i can add printing of error in such case but I am not sure whether it is worth to fail entire pattern_trig_activate().
Best regards,
Krzysztof