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
