Commit 7cfe749f authored by Tony Makkiel's avatar Tony Makkiel Committed by Jacek Anaszewski
Browse files

leds: core: Fix brightness setting upon hardware blinking enabled 



Commit 76931edd ("leds: fix brightness changing when software blinking
is active") changed the semantics of led_set_brightness() which according
to the documentation should disable blinking upon any brightness setting.
Moreover it made it different for soft blink case, where it was possible
to change blink brightness, and for hardware blink case, where setting
any brightness greater than 0 was ignored.

While the change itself is against the documentation claims, it was driven
also by the fact that timer trigger remained active after turning blinking
off. Fixing that would have required major refactoring in the led-core,
led-class, and led-triggers because of cyclic dependencies.

Finally, it has been decided that allowing for brightness change during
blinking is beneficial as it can be accomplished without disturbing
blink rhythm.

The change in brightness setting semantics will not affect existing
LED class drivers that implement blink_set op thanks to the LED_BLINK_SW
flag introduced by this patch. The flag state will be from now on checked
in led_set_brightness() which will allow to distinguish between software
and hardware blink mode. In the latter case the control will be passed
directly to the drivers which apply their semantics on brightness set,
which is disable the blinking in case of most such drivers. New drivers
will apply new semantics and just change the brightness while hardware
blinking is on, if possible.

The issue was smuggled by subsequent LED core improvements, which modified
the code that originally introduced the problem.

Fixes: f1e80c07 ("leds: core: Add two new LED_BLINK_ flags")
Signed-off-by: default avatarTony Makkiel <tony.makkiel@daqri.com>
Signed-off-by: default avatarJacek Anaszewski <j.anaszewski@samsung.com>
parent af8c34ce

Documentation/leds/leds-class.txt

+2 −2
Original line number Diff line number Diff line
@@ -74,8 +74,8 @@ blink_set() function (see <linux/leds.h>). To set an LED to blinking, 
however, it is better to use the API function led_blink_set(), as it

will check and implement software fallback if necessary.



To turn off blinking again, use the API function led_brightness_set()

as that will not just set the LED brightness but also stop any software

To turn off blinking, use the API function led_brightness_set()

with brightness value LED_OFF, which should stop any software

timers that may have been required for blinking.



The blink_set() function should choose a user friendly blinking value

drivers/leds/led-core.c

+6 −3
Original line number Diff line number Diff line
@@ -53,11 +53,12 @@ static void led_timer_function(unsigned long data) 


	if (!led_cdev->blink_delay_on || !led_cdev->blink_delay_off) {

		led_set_brightness_nosleep(led_cdev, LED_OFF);

		led_cdev->flags &= ~LED_BLINK_SW;

		return;

	}



	if (led_cdev->flags & LED_BLINK_ONESHOT_STOP) {

		led_cdev->flags &= ~LED_BLINK_ONESHOT_STOP;

		led_cdev->flags &=  ~(LED_BLINK_ONESHOT_STOP | LED_BLINK_SW);

		return;

	}
@@ -151,6 +152,7 @@ static void led_set_software_blink(struct led_classdev *led_cdev, 
		return;

	}



	led_cdev->flags |= LED_BLINK_SW;

	mod_timer(&led_cdev->blink_timer, jiffies + 1);

}
@@ -219,6 +221,7 @@ void led_stop_software_blink(struct led_classdev *led_cdev) 
	del_timer_sync(&led_cdev->blink_timer);

	led_cdev->blink_delay_on = 0;

	led_cdev->blink_delay_off = 0;

	led_cdev->flags &= ~LED_BLINK_SW;

}

EXPORT_SYMBOL_GPL(led_stop_software_blink);
@@ -226,10 +229,10 @@ void led_set_brightness(struct led_classdev *led_cdev, 
			enum led_brightness brightness)

{

	/*

	 * In case blinking is on delay brightness setting

	 * If software blink is active, delay brightness setting

	 * until the next timer tick.

	 */

	if (led_cdev->blink_delay_on || led_cdev->blink_delay_off) {

	if (led_cdev->flags & LED_BLINK_SW) {

		/*

		 * If we need to disable soft blinking delegate this to the

		 * work queue task to avoid problems in case we are called

include/linux/leds.h

+12 −11
Original line number Diff line number Diff line
@@ -42,15 +42,16 @@ struct led_classdev { 
#define LED_UNREGISTERING	(1 << 1)

	/* Upper 16 bits reflect control information */

#define LED_CORE_SUSPENDRESUME	(1 << 16)

#define LED_BLINK_ONESHOT	(1 << 17)

#define LED_BLINK_ONESHOT_STOP	(1 << 18)

#define LED_BLINK_INVERT	(1 << 19)

#define LED_BLINK_BRIGHTNESS_CHANGE (1 << 20)

#define LED_BLINK_DISABLE	(1 << 21)

#define LED_SYSFS_DISABLE	(1 << 22)

#define LED_DEV_CAP_FLASH	(1 << 23)

#define LED_HW_PLUGGABLE	(1 << 24)

#define LED_PANIC_INDICATOR	(1 << 25)

#define LED_BLINK_SW		(1 << 17)

#define LED_BLINK_ONESHOT	(1 << 18)

#define LED_BLINK_ONESHOT_STOP	(1 << 19)

#define LED_BLINK_INVERT	(1 << 20)

#define LED_BLINK_BRIGHTNESS_CHANGE (1 << 21)

#define LED_BLINK_DISABLE	(1 << 22)

#define LED_SYSFS_DISABLE	(1 << 23)

#define LED_DEV_CAP_FLASH	(1 << 24)

#define LED_HW_PLUGGABLE	(1 << 25)

#define LED_PANIC_INDICATOR	(1 << 26)



	/* Set LED brightness level

	 * Must not sleep. Use brightness_set_blocking for drivers
@@ -72,8 +73,8 @@ struct led_classdev { 
	 * and if both are zero then a sensible default should be chosen.

	 * The call should adjust the timings in that case and if it can't

	 * match the values specified exactly.

	 * Deactivate blinking again when the brightness is set to a fixed

	 * value via the brightness_set() callback.

	 * Deactivate blinking again when the brightness is set to LED_OFF

	 * via the brightness_set() callback.

	 */

	int		(*blink_set)(struct led_classdev *led_cdev,

				     unsigned long *delay_on,

CRA Git | Maintained and supported by SUSTech CRA and CCSE