|
From: Jacek A. <j.a...@sa...> - 2016-11-18 16:04:00
|
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",
that's why I assessed it is too generic. 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.
Current LED subsystem triggers are generic - e.g. disk, mtd,
backlight (it registers on video fb notifications).
Driver specific trigger should be implemented inside the driver.
Last but not least - generic keyboard backlight trigger can't assume
that all devices of this type can adjust backlight brightness.
Best Regards,
Jacek Anaszewski
>> On 11/17/2016 11:24 PM, Hans de Goede wrote:
>>> Add a trigger to control keyboard backlight LED devices. Note that in
>>> some cases the keyboard backlight control is hardwired (taken care of
>>> in firmware outside of the kernels control), in that case this triggers
>>> main purpose is to allow userspace to monitor these changes.
>>>
>>> The ledtrig_kbd_backlight function has a set_brightness parameter to
>>> differentiate between full backlight control through the trigger
>>> (set_brightness set to true) or change notification only (false).
>>>
>>> Note the Kconfig option for this is a bool because the code is so
>>> small that it is not worth the overhead of being a module.
>>>
>>> Signed-off-by: Hans de Goede <hde...@re...>
>>> ---
>>> Changes in v5:
>>> -This is a new patch in v5 of this patch-set (replacing earlier attempts
>>> at similar functionality)
>>> ---
>>> drivers/leds/trigger/Kconfig | 10 ++++++++
>>> drivers/leds/trigger/Makefile | 1 +
>>> drivers/leds/trigger/ledtrig-kbd-backlight.c | 38
>>> ++++++++++++++++++++++++++++
>>> include/linux/leds.h | 8 ++++++
>>> 4 files changed, 57 insertions(+)
>>> create mode 100644 drivers/leds/trigger/ledtrig-kbd-backlight.c
>>>
>>> diff --git a/drivers/leds/trigger/Kconfig b/drivers/leds/trigger/Kconfig
>>> index 3f9ddb9..350e2c7 100644
>>> --- a/drivers/leds/trigger/Kconfig
>>> +++ b/drivers/leds/trigger/Kconfig
>>> @@ -67,6 +67,16 @@ config LEDS_TRIGGER_BACKLIGHT
>>>
>>> If unsure, say N.
>>>
>>> +config LEDS_TRIGGER_KBD_BACKLIGHT
>>> + bool "LED keyboard backlight Trigger"
>>> + depends on LEDS_TRIGGERS
>>> + help
>>> + This trigger can control keyboard backlight LED devices,
>>> + it also allows user-space to monitor keyboard backlight
>>> brightness
>>> + changes done through e.g. hotkeys on some laptops.
>>> +
>>> + If unsure, say Y.
>>> +
>>> config LEDS_TRIGGER_CPU
>>> bool "LED CPU Trigger"
>>> depends on LEDS_TRIGGERS
>>> diff --git a/drivers/leds/trigger/Makefile
>>> b/drivers/leds/trigger/Makefile
>>> index a72c43c..be6b249 100644
>>> --- a/drivers/leds/trigger/Makefile
>>> +++ b/drivers/leds/trigger/Makefile
>>> @@ -4,6 +4,7 @@ obj-$(CONFIG_LEDS_TRIGGER_DISK) += ledtrig-disk.o
>>> obj-$(CONFIG_LEDS_TRIGGER_MTD) += ledtrig-mtd.o
>>> obj-$(CONFIG_LEDS_TRIGGER_HEARTBEAT) += ledtrig-heartbeat.o
>>> obj-$(CONFIG_LEDS_TRIGGER_BACKLIGHT) += ledtrig-backlight.o
>>> +obj-$(CONFIG_LEDS_TRIGGER_KBD_BACKLIGHT) += ledtrig-kbd-backlight.o
>>> obj-$(CONFIG_LEDS_TRIGGER_GPIO) += ledtrig-gpio.o
>>> obj-$(CONFIG_LEDS_TRIGGER_CPU) += ledtrig-cpu.o
>>> obj-$(CONFIG_LEDS_TRIGGER_DEFAULT_ON) += ledtrig-default-on.o
>>> diff --git a/drivers/leds/trigger/ledtrig-kbd-backlight.c
>>> b/drivers/leds/trigger/ledtrig-kbd-backlight.c
>>> new file mode 100644
>>> index 0000000..353ee92
>>> --- /dev/null
>>> +++ b/drivers/leds/trigger/ledtrig-kbd-backlight.c
>>> @@ -0,0 +1,38 @@
>>> +/*
>>> + * LED Trigger for keyboard backlight control
>>> + *
>>> + * Copyright 2016, Hans de Goede <hde...@re...>
>>> + *
>>> + * This program is free software; you can redistribute it and/or modify
>>> + * it under the terms of the GNU General Public License version 2 as
>>> + * published by the Free Software Foundation.
>>> + *
>>> + */
>>> +
>>> +#include <linux/module.h>
>>> +#include <linux/kernel.h>
>>> +#include <linux/init.h>
>>> +#include <linux/device.h>
>>> +#include <linux/leds.h>
>>> +#include "../leds.h"
>>> +
>>> +static struct led_trigger kbd_backlight_trigger = {
>>> + .name = "kbd-backlight",
>>> + .activate = led_trigger_add_current_brightness,
>>> + .deactivate = led_trigger_remove_current_brightness,
>>> +};
>>> +
>>> +void ledtrig_kbd_backlight(bool set_brightness, enum led_brightness
>>> brightness)
>>> +{
>>> + if (set_brightness)
>>> + led_trigger_event(&kbd_backlight_trigger, brightness);
>>> +
>>> +
>>> led_trigger_notify_current_brightness_change(&kbd_backlight_trigger);
>>> +}
>>> +EXPORT_SYMBOL_GPL(ledtrig_kbd_backlight);
>>> +
>>> +static int __init kbd_backlight_trig_init(void)
>>> +{
>>> + return led_trigger_register(&kbd_backlight_trigger);
>>> +}
>>> +device_initcall(kbd_backlight_trig_init);
>>> diff --git a/include/linux/leds.h b/include/linux/leds.h
>>> index d3eb992..870b8c2 100644
>>> --- a/include/linux/leds.h
>>> +++ b/include/linux/leds.h
>>> @@ -353,6 +353,14 @@ static inline void ledtrig_flash_ctrl(bool on) {}
>>> static inline void ledtrig_torch_ctrl(bool on) {}
>>> #endif
>>>
>>> +#ifdef CONFIG_LEDS_TRIGGER_KBD_BACKLIGHT
>>> +extern void ledtrig_kbd_backlight(bool set_brightness,
>>> + enum led_brightness brightness);
>>> +#else
>>> +static inline void ledtrig_kbd_backlight(bool set_brightness,
>>> + enum led_brightness brightness) {}
>>> +#endif
>>> +
>>> /*
>>> * Generic LED platform data for describing LED names and default
>>> triggers.
>>> */
>>>
>>
>>
>
>
>
|
