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

[Jacek Anaszewski <j.anaszewski@samsung.com>]

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@samsung.com]
> Gesendet: Montag, 16. November 2015 12:20
> An: Hofstätter Markus
> Cc: Bryan Wu; Richard Purdie; linux-leds@vger.kernel.org
> 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@ait.ac.at>
>> Tested-by: Markus Hofstaetter <markus.hofstaetter@ait.ac.at>
>> ---
>>    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@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>