HI, On 20-11-16 17:21, Pavel Machek wrote: > Hi! > >>>>>>> Thanks for the patch. >>>>>>> >>>>>>> I think we need less generic trigger name. >>>>>>> With present name we pretend that all kbd-backlight controllers >>>>>>> can change LED brightness autonomously. >>>>>>> >>>>>>> How about kbd-backlight-pollable ? >>>>>> >>>>>> This is a trigger to control kbd-backlights, in the >>>>>> current use-case the brightness change is already done >>>>>> by the firmware, hence the set_brightness argument to >>>>>> ledtrig_kbd_backlight(), so that we can avoid setting >>>>>> it again. >>>>>> >>>>>> But I can see future cases where we do want to have some >>>>>> event (e.g. a wmi hotkey event on a laptop where the firmware >>>>>> does not do the adjustment automatically) which does >>>>>> lead to actually updating the brightness. >>>>>> >>>>>> So I decided to go with a generic kbd-backlight trigger, >>>>>> which in the future can also be used to directly control >>>>>> kbd-backlight brightness; and not just to make ot >>>>>> poll-able. >>>>> >>>>> I thought that kbd-backlight stands for "keyboard backlight", >>>> >>>> It does. >>>> >>>>> that's why I assessed it is too generic. >>>> >>>> The whole purpose of the trigger as implemented is to be >>>> generic, as it seems senseless to implement a one off >>>> trigger for just the dell / thinkpad case. >>>> >>>>> It seems however to be >>>>> the other way round - if kbd-backlight means that this is >>>>> a trigger only for use with dell-laptop keyboard driver >>>>> (I see kbd namespacing prefix in the driver functions) than it is >>>>> not generic at all. >>>> >>>> The trigger as implemented is generic, if you think >>>> otherwise, please let me know which part is not generic >>>> according to you. >>> >>> I think I was too meticulous here. In the end of the previous >>> message I mentioned that we cannot guarantee that all keyboard >>> backlight controllers can adjust brightness autonomously. >>> Nonetheless, for the ones that cannot do that it would make no sense >>> to have a trigger. In view of that this trigger is generic enough. >>> >>> I'll wait for Pavel's opinion before merging the patch set >>> as he was also involved in this whole thread. > > If we have a keyboard backlight that may be changed automatically, I'd > go for trigger. If we know for sure that hardware will not change > brightnes "on its own", I'd not put a trigger there (and polling makes > no sense). If we don't know... I guess I'd go for trigger. > > We can do various white/blacklists if we really want to.. > >> As pointed in other email, we do not know if HW really controls keyboard backlight, >> so adding "fake" trigger on machines without HW control is not a good idea. > > Well, if we know that hardware will not change the brightness on its > own, yes, I'd avoid the trigger. If we don't know (as is common on > ACPI machines, I'd keep the trigger). Right, this is taken care of by the default_trigger field of the led_classdev, we only set the "kbd-backlight" default_trigger on (kbd-backlight) LED devices where we know we have a hotkey directly controlling the brightness. Regards, Hans ------------------------------------------------------------------------------ _______________________________________________ ibm-acpi-devel mailing list ibm-acpi-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.sourceforge.net/lists/listinfo/ibm-acpi-devel
