Re: [PATCH v5 2/6] leds: triggers: Add a keyboard backlight trigger

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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




[Index of Archives]     [Linux ACPI]     [Linux Kernel]     [Linux Laptop]     [Kernel Newbies]     [Share Photos]     [Security]     [Netfilter]     [Bugtraq]     [Photo]     [Yosemite Photos]     [Yosemite Advice]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Samba]     [Device Mapper]

  Powered by Linux