Skip to contentSkip to conversation

leds: add DT support for LEDs wired to supply and default brightness

PATCH New Russell King 3 participants 4 messages Feb 28, 2014 – Mar 27, 2014

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

PATCH 1/2 leds: add DT support for LEDs wired to supply and default brightness

Some PWM outputs are wired such that the LED they're controlling is connected to supply rather than ground. Therefore, the duty cycle needs to be inverted to make the LED behave as it should do.

We also provide a way to specify the default brightness when a trigger is not specified.

---
 Documentation/devicetree/bindings/leds/leds-pwm.txt |  3 +++
 drivers/leds/leds-pwm.c                             | 15 +++++++++++----
 2 files changed, 14 insertions(+), 4 deletions(-)
Documentation/devicetree/bindings/leds/leds-pwm.txt +3 -0
index 7297107cf832..ffcb74ab61fd 100644
@@ -13,6 +13,9 @@ node's name represents the name of the corresponding LED.
   For the pwms and pwm-names property please refer to:
   Documentation/devicetree/bindings/pwm/pwm.txt
 - max-brightness : Maximum brightness possible for the LED
+- default-brightness : (optional) Default brightness for the LED
+- active-low : (optional) For PWMs where the LED is wired to supply
+  rather than ground.
 - label :  (optional)
   see Documentation/devicetree/bindings/leds/common.txt
 - linux,default-trigger :  (optional)
drivers/leds/leds-pwm.c +11 -4
index 605047428b5a..fe2cd849b593 100644
@@ -70,6 +70,10 @@ static void led_pwm_set(struct led_classdev *led_cdev,
 
 	duty *= brightness;
 	do_div(duty, max);
+
+	if (led_dat->active_low)
+		duty = led_dat->period - duty;
+
 	led_dat->duty = duty;
 
 	if (led_dat->can_sleep)
@@ -93,6 +97,10 @@ static int led_pwm_create_of(struct platform_device *pdev,
 	for_each_child_of_node(pdev->dev.of_node, child) {
 		struct led_pwm_data *led_dat = &priv->leds[priv->num_leds];
 
+		led_dat->cdev.brightness_set = led_pwm_set;
+		led_dat->cdev.brightness = LED_OFF;
+		led_dat->cdev.flags |= LED_CORE_SUSPENDRESUME;
+
 		led_dat->cdev.name = of_get_property(child, "label",
 						     NULL) ? : child->name;
 
@@ -110,10 +118,9 @@ static int led_pwm_create_of(struct platform_device *pdev,
 						"linux,default-trigger", NULL);
 		of_property_read_u32(child, "max-brightness",
 				     &led_dat->cdev.max_brightness);
-
-		led_dat->cdev.brightness_set = led_pwm_set;
-		led_dat->cdev.brightness = LED_OFF;
-		led_dat->cdev.flags |= LED_CORE_SUSPENDRESUME;
+		of_property_read_u32(child, "default-brightness",
+				     &led_dat->cdev.brightness);
+		led_dat->active_low = of_property_read_bool(child, "active-low");
 
 		led_dat->can_sleep = pwm_can_sleep(led_dat->pwm);
 		if (led_dat->can_sleep)
Signature · 1 line
1.8.3.1
#1 lore raw

PATCH 1/2 leds: add DT support for LEDs wired to supply and default brightness

Quoted 18 lines · Russell King

Some PWM outputs are wired such that the LED they're controlling is connected to supply rather than ground. Therefore, the duty cycle needs to be inverted to make the LED behave as it should do.

We also provide a way to specify the default brightness when a trigger is not specified.

---
 Documentation/devicetree/bindings/leds/leds-pwm.txt |  3 +++
 drivers/leds/leds-pwm.c                             | 15 +++++++++++----
 2 files changed, 14 insertions(+), 4 deletions(-)
Documentation/devicetree/bindings/leds/leds-pwm.txt +0 -0
index 7297107cf832..ffcb74ab61fd 100644

Documentation/devicetree/bindings/leds/leds-pwm.txt node's name represents the name of the corresponding LED. · line 21

Quoted 9 lines
Documentation/devicetree/bindings/leds/leds-pwm.txt +3 -0
@@ -13,6 +13,9 @@ node's name represents the name of the corresponding LED.
   For the pwms and pwm-names property please refer to:
   Documentation/devicetree/bindings/pwm/pwm.txt
 - max-brightness : Maximum brightness possible for the LED
+- default-brightness : (optional) Default brightness for the LED
+- active-low : (optional) For PWMs where the LED is wired to supply
+  rather than ground.
 - label :  (optional)
   see Documentation/devicetree/bindings/leds/common.txt
 - linux,default-trigger :  (optional)

Why remove "max-brightness", "label" and "linux,default-trigger" here? I believe you're still using them in your second patch like

+ front {

+                       active-low;
+                       default-brightness = <128>;
+                       label = "imx6:red:front";
+                       max-brightness = <248>;
+                       pwms = <&pwm1 0 50000>;

+ };

Others are good to me.

Thierry, could you please review this patchset?

Thanks,
-Bryan

Quoted 43 lines
drivers/leds/leds-pwm.c +11 -5
index 605047428b5a..fe2cd849b593 100644
@@ -70,6 +70,10 @@ static void led_pwm_set(struct led_classdev *led_cdev,
 
        duty *= brightness;
        do_div(duty, max);
+
+       if (led_dat->active_low)
+               duty = led_dat->period - duty;
+
        led_dat->duty = duty;
 
        if (led_dat->can_sleep)
@@ -93,6 +97,10 @@ static int led_pwm_create_of(struct platform_device *pdev,
        for_each_child_of_node(pdev->dev.of_node, child) {
                struct led_pwm_data *led_dat = &priv->leds[priv->num_leds];
 
+               led_dat->cdev.brightness_set = led_pwm_set;
+               led_dat->cdev.brightness = LED_OFF;
+               led_dat->cdev.flags |= LED_CORE_SUSPENDRESUME;
+
                led_dat->cdev.name = of_get_property(child, "label",
                                                     NULL) ? : child->name;
 
@@ -110,10 +118,9 @@ static int led_pwm_create_of(struct platform_device *pdev,
                                                "linux,default-trigger", NULL);
                of_property_read_u32(child, "max-brightness",
                                     &led_dat->cdev.max_brightness);
-
-               led_dat->cdev.brightness_set = led_pwm_set;
-               led_dat->cdev.brightness = LED_OFF;
-               led_dat->cdev.flags |= LED_CORE_SUSPENDRESUME;
+               of_property_read_u32(child, "default-brightness",
+                                    &led_dat->cdev.brightness);
+               led_dat->active_low = of_property_read_bool(child, "active-low");
 
                led_dat->can_sleep = pwm_can_sleep(led_dat->pwm);
                if (led_dat->can_sleep)
--

1.8.3.1

#2 lore raw

PATCH 1/2 leds: add DT support for LEDs wired to supply and default brightness

Quoted 15 lines · Bryan Wu
Quoted 13 lines · Russell King

Some PWM outputs are wired such that the LED they're controlling is connected to supply rather than ground. Therefore, the duty cycle needs to be inverted to make the LED behave as it should do.

We also provide a way to specify the default brightness when a trigger is not specified.

---
 Documentation/devicetree/bindings/leds/leds-pwm.txt |  3 +++
 drivers/leds/leds-pwm.c                             | 15 +++++++++++----
 2 files changed, 14 insertions(+), 4 deletions(-)
Quoted 16 lines
Quoted 5 lines
Documentation/devicetree/bindings/leds/leds-pwm.txt +0 -0
index 7297107cf832..ffcb74ab61fd 100644

Documentation/devicetree/bindings/leds/leds-pwm.txt node's name represents the name of the corresponding LED. · line 21

Quoted 9 lines
Documentation/devicetree/bindings/leds/leds-pwm.txt +3 -0
@@ -13,6 +13,9 @@ node's name represents the name of the corresponding LED.
   For the pwms and pwm-names property please refer to:
   Documentation/devicetree/bindings/pwm/pwm.txt
 - max-brightness : Maximum brightness possible for the LED
+- default-brightness : (optional) Default brightness for the LED
+- active-low : (optional) For PWMs where the LED is wired to supply
+  rather than ground.
 - label :  (optional)
   see Documentation/devicetree/bindings/leds/common.txt
 - linux,default-trigger :  (optional)

Why remove "max-brightness", "label" and "linux,default-trigger" here?

They are not removed, they are just unchanged list items after literal '-'s in the context of the patch.

Best regards
Uwe

Signature · 2 lines
Pengutronix e.K.                           | Uwe Kleine-K?nig            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
#3 lore raw

PATCH 1/2 leds: add DT support for LEDs wired to supply and default brightness

Quoted 32 lines · Uwe Kleine-K?nig
Quoted 15 lines · Bryan Wu
Quoted 13 lines · Russell King

Some PWM outputs are wired such that the LED they're controlling is connected to supply rather than ground. Therefore, the duty cycle needs to be inverted to make the LED behave as it should do.

We also provide a way to specify the default brightness when a trigger is not specified.

---
 Documentation/devicetree/bindings/leds/leds-pwm.txt |  3 +++
 drivers/leds/leds-pwm.c                             | 15 +++++++++++----
 2 files changed, 14 insertions(+), 4 deletions(-)
Quoted 16 lines
Quoted 5 lines
Documentation/devicetree/bindings/leds/leds-pwm.txt +0 -0
index 7297107cf832..ffcb74ab61fd 100644

Documentation/devicetree/bindings/leds/leds-pwm.txt node's name represents the name of the corresponding LED. · line 21

Quoted 9 lines
Documentation/devicetree/bindings/leds/leds-pwm.txt +3 -0
@@ -13,6 +13,9 @@ node's name represents the name of the corresponding LED.
   For the pwms and pwm-names property please refer to:
   Documentation/devicetree/bindings/pwm/pwm.txt
 - max-brightness : Maximum brightness possible for the LED
+- default-brightness : (optional) Default brightness for the LED
+- active-low : (optional) For PWMs where the LED is wired to supply
+  rather than ground.
 - label :  (optional)
   see Documentation/devicetree/bindings/leds/common.txt
 - linux,default-trigger :  (optional)

Why remove "max-brightness", "label" and "linux,default-trigger" here?

Quoted 3 lines

They are not removed, they are just unchanged list items after literal '-'s in the context of the patch.

Sorry, my bad. please ignore my question then.

Thanks,
-Bryan