On 11/20/2016 04:05 PM, Pali Rohár wrote: > On Saturday 19 November 2016 16:44:09 Jacek Anaszewski wrote: >> Hi, >> >> On 11/18/2016 07:47 PM, Hans de Goede wrote: >>> HI, >>> >>> On 18-11-16 17:03, Jacek Anaszewski wrote: >>>> Hi, >>>> >>>> On 11/18/2016 10:07 AM, Hans de Goede wrote: >>>>> Hi, >>>>> >>>>> On 18-11-16 09:55, Jacek Anaszewski wrote: >>>>>> Hi Hans, >>>>>> >>>>>> 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. > > Do you mean me? Or Pavel Machek (CCed) who was involved in LEDs for input devices? I meant Pavel Machek, I haven't known that Pali is an equivalent of Pavel :-) Your opinion is very much appreciated though, thanks. > 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. > Yes, I had also similar doubts, but got almost convinced due to no objections. Now it becomes clear that we need to improve this feature. -- Best regards, Jacek Anaszewski ------------------------------------------------------------------------------ Check out the vibrant tech community on one of the world's most engaging tech sites, SlashDot.org! http://sdm.link/slashdot _______________________________________________ ibm-acpi-devel mailing list ibm-acpi-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.sourceforge.net/lists/listinfo/ibm-acpi-devel