Re: [PATCH 1/1] leds: call led_pwm_set() in leds-pwm to enforce default LED_OFF

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Markus,

Thanks for your explanation. I missed that led_pwm_set behaves
differently when active-low DT property is set.
I've just applied that patch to the LED tree, for-next branch.

Thanks,
Jacek Anaszewski

On 11/16/2015 02:48 PM, Hofstätter Markus wrote:
Hi Jacek,

thx for looking into the patch.

AFAICS default brightness is hard coded to LED_OFF, so this
will turn active-low LEDs on.

LED_OFF will turn a active led on only if you don't set active-low in the dt. In latter case  the behaviour is inverted to match the same brightness meanings (for pwms that don't support inversion in the driver).
As you saw the LED_OFF setting is hard coded and set it in the device structure by the probe/pwm add routine.

The issue here is that this will only report a brightness of 0 (e.g., cat ..led/brightness is LED_OFF), but it never configures the associated pwm to that state. So the PIN will be left at its default resp. previous state until you set a new brightness value.

E.g., in my case on the i.MX6Q, the default PIN setting outputs low --> led is at full brightness and cat brightness reports 0. Setting 0 brightness again, turns the light off.
Also, I had looked at the PWM register and saw that it has not been configured until I have set the brightness again.
Anyhow, this would also be an issue for active-high leds, if  the default/previous PIN state were active-high, as it would not be set to low. This however, is not the case in most scenarios.

Best regards,

Markus Hofstätter

________________________________________
Von: Jacek Anaszewski [j.anaszewski@xxxxxxxxxxx]
Gesendet: Montag, 16. November 2015 12:20
An: Hofstätter Markus
Cc: Bryan Wu; Richard Purdie; linux-leds@xxxxxxxxxxxxxxx
Betreff: Re: [PATCH 1/1] leds: call led_pwm_set() in leds-pwm to enforce default LED_OFF

Hi Markus,

On 11/11/2015 12:40 PM, Markus Hofstaetter wrote:
Some PWMs are disabled by default or the default pin setting
does not match the LED_OFF state (e.g., active-low leds).
Hence, the driver may end up reporting 0 brightness, but
the leds are actually on using full brightness, because
it never enforces its default configuration.

AFAICS default brightness is hard coded to LED_OFF, so this
will turn active-low LEDs on.

So enforce it by calling led_pwm_set() after successfully
registering the device.

Tested on a Phytec phyFLEX i.MX6Q board based on kernel
v3.19.5.

Signed-off-by: Markus Hofstaetter <markus.hofstaetter@xxxxxxxxx>
Tested-by: Markus Hofstaetter <markus.hofstaetter@xxxxxxxxx>
---
   drivers/leds/leds-pwm.c | 1 +
   1 file changed, 1 insertion(+)

diff --git a/drivers/leds/leds-pwm.c b/drivers/leds/leds-pwm.c
index 1d07e3e..3149dbe 100644
--- a/drivers/leds/leds-pwm.c
+++ b/drivers/leds/leds-pwm.c
@@ -132,6 +132,7 @@ static int led_pwm_add(struct device *dev, struct led_pwm_priv *priv,
       ret = led_classdev_register(dev, &led_data->cdev);
       if (ret == 0) {
               priv->num_leds++;
+             led_pwm_set(&led_data->cdev, led_data->cdev.brightness);
       } else {
               dev_err(dev, "failed to register PWM led for %s: %d\n",
                       led->name, ret);



--
Best Regards,
Jacek Anaszewski
--
To unsubscribe from this list: send the line "unsubscribe linux-leds" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


--
To unsubscribe from this list: send the line "unsubscribe linux-leds" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux OMAP]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux