Am 19.02.2016 um 14:59 schrieb Jacek Anaszewski: > 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. > Ah, now I get you. Yes, at a first glance it seems like the semantics were changed. However I don't see this as a bad thing if we interpret LED_FULL as "maximum brightness of a device". -- 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
