On 02/18/2016 09:34 PM, Heiner Kallweit wrote:
[...]
/* Time to switch the LED on. */
brightness = led_cdev->blink_brightness;
delay = led_cdev->blink_delay_on;
@@ -133,8 +133,8 @@ static void led_set_software_blink(struct led_classdev *led_cdev,
if (current_brightness)
led_cdev->blink_brightness = current_brightness;
if (!led_cdev->blink_brightness)
- led_cdev->blink_brightness = led_cdev->max_brightness;
-
+ led_cdev->blink_brightness =
+ led_validate_brightness(led_cdev, LED_FULL);
This function name doesn't fit here, since term "validation" usually
refers to validating user provided data.
led_confine_brightness() ?
And why LED_FULL and not led_cdev->max_brightness?
LED_FULL is reduced to max_brightness anyway by led_validate_brightness.
IMHO LED_FULL makes clearer that the intention is: switch it on.
max_brightness is a device-specific property that I'd like to hide
as far as possible in the generic core code.
Without checking led_validate_brightness() internals it looks like this
patch was changing led_set_software_blink() semantics.
As far as I can see it doesn't. In monochrome mode
led_validate_brightness(led_cdev, LED_FULL) evaluates to
led_cdev->max_brightness.
I meant that looking at this change alone makes such an impression.
Besides, if we now have led_confine_brightness(), then it implies that
brightness will be limited up to LED_FULL level, whereas in fact
the upper limit will be max_brightness for monochrome LEDs.
--
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
