Hi All, On 5/5/24 2:32 PM, Hans de Goede wrote: > Add a new trigger which turns LEDs on when there is input > (/dev/input/event*) activity and turns them back off again after there has > been no activity for 5 seconds. > > This is primarily intended to control LED devices which are a backlight for > capacitive touch-buttons, such as e.g. the menu / home / back buttons found > on the bottom bezel of many somewhat older smartphones and tablets. > > This can also be used to turn on the keyboard backlight LED on input > events and turn the keyboard backlight off again when idle. > > Signed-off-by: Hans de Goede <hdegoede@xxxxxxxxxx> Self-nack for this patch. I still believe the basic idea of having such a trigger is good. But the way how this implementation uses the trigger->led_cdevs list is racy and that list is better left as a private implementation detail of drivers/leds/led-trigger.c and not used directly by triggers. So I'm going to rework this for v2 so that it no longer loops over trigger->led_cdevs list. Regards, Hans > --- > drivers/leds/trigger/Kconfig | 16 ++ > drivers/leds/trigger/Makefile | 1 + > drivers/leds/trigger/ledtrig-input-events.c | 252 ++++++++++++++++++++ > 3 files changed, 269 insertions(+) > create mode 100644 drivers/leds/trigger/ledtrig-input-events.c > > diff --git a/drivers/leds/trigger/Kconfig b/drivers/leds/trigger/Kconfig > index d11d80176fc0..809ffba0cd6a 100644 > --- a/drivers/leds/trigger/Kconfig > +++ b/drivers/leds/trigger/Kconfig > @@ -152,4 +152,20 @@ config LEDS_TRIGGER_TTY > > When build as a module this driver will be called ledtrig-tty. > > +config LEDS_TRIGGER_INPUT_EVENTS > + tristate "LED Input events trigger" > + depends on INPUT > + help > + Turn LEDs on when there is input (/dev/input/event*) activity and turn > + them back off again after there has been no activity for 5 seconds. > + > + This is primarily intended to control LEDs which are a backlight for > + capacitive touch-buttons, such as e.g. the menu / home / back buttons > + found on the bottom bezel of many older smartphones and tablets. > + > + This can also be used to turn on the keyboard backlight LED on > + input events and turn the keyboard backlight off again when idle. > + > + When build as a module this driver will be called ledtrig-input-events. > + > endif # LEDS_TRIGGERS > diff --git a/drivers/leds/trigger/Makefile b/drivers/leds/trigger/Makefile > index 25c4db97cdd4..f78a3077efc7 100644 > --- a/drivers/leds/trigger/Makefile > +++ b/drivers/leds/trigger/Makefile > @@ -16,3 +16,4 @@ obj-$(CONFIG_LEDS_TRIGGER_NETDEV) += ledtrig-netdev.o > obj-$(CONFIG_LEDS_TRIGGER_PATTERN) += ledtrig-pattern.o > obj-$(CONFIG_LEDS_TRIGGER_AUDIO) += ledtrig-audio.o > obj-$(CONFIG_LEDS_TRIGGER_TTY) += ledtrig-tty.o > +obj-$(CONFIG_LEDS_TRIGGER_INPUT_EVENTS) += ledtrig-input-events.o > diff --git a/drivers/leds/trigger/ledtrig-input-events.c b/drivers/leds/trigger/ledtrig-input-events.c > new file mode 100644 > index 000000000000..cb86d1afeab4 > --- /dev/null > +++ b/drivers/leds/trigger/ledtrig-input-events.c > @@ -0,0 +1,252 @@ > +// SPDX-License-Identifier: GPL-2.0-only > +/* > + * Input Events LED trigger > + * > + * Copyright (C) 2024 Hans de Goede <hansg@xxxxxxxxxx> > + * Partially based on Atsushi Nemoto's ledtrig-heartbeat.c. > + */ > + > +#include <linux/input.h> > +#include <linux/jiffies.h> > +#include <linux/leds.h> > +#include <linux/module.h> > +#include <linux/slab.h> > +#include <linux/spinlock.h> > +#include <linux/workqueue.h> > +#include "../leds.h" > + > +#define DEFAULT_LED_OFF_DELAY_MS 5000 > + > +struct input_events_data { > + struct delayed_work work; > + spinlock_t lock; > + struct led_classdev *led_cdev; > + int led_cdev_saved_flags; > + /* To avoid repeatedly setting the brightness while there is events */ > + bool led_on; > + unsigned long led_off_time; > + unsigned long led_off_delay; > +}; > + > +static void led_input_events_work(struct work_struct *work) > +{ > + struct input_events_data *data = > + container_of(work, struct input_events_data, work.work); > + > + spin_lock_irq(&data->lock); > + > + /* > + * This time_after_eq() check avoids a race where this work starts > + * running before a new event pushed led_off_time back. > + */ > + if (time_after_eq(jiffies, data->led_off_time)) { > + led_set_brightness_nosleep(data->led_cdev, LED_OFF); > + data->led_on = false; > + } > + > + spin_unlock_irq(&data->lock); > +} > + > +static ssize_t delay_show(struct device *dev, struct device_attribute *attr, char *buf) > +{ > + struct input_events_data *input_events_data = led_trigger_get_drvdata(dev); > + > + return sysfs_emit(buf, "%lu\n", input_events_data->led_off_delay); > +} > + > +static ssize_t delay_store(struct device *dev, struct device_attribute *attr, > + const char *buf, size_t size) > +{ > + struct input_events_data *input_events_data = led_trigger_get_drvdata(dev); > + unsigned long delay; > + int ret; > + > + ret = kstrtoul(buf, 0, &delay); > + if (ret) > + return ret; > + > + /* Clamp between 0.5 and 1000 seconds */ > + delay = clamp_val(delay, 500UL, 1000000UL); > + input_events_data->led_off_delay = msecs_to_jiffies(delay); > + > + return size; > +} > + > +static DEVICE_ATTR_RW(delay); > + > +static struct attribute *input_events_led_attrs[] = { > + &dev_attr_delay.attr, > + NULL > +}; > +ATTRIBUTE_GROUPS(input_events_led); > + > +static int input_events_activate(struct led_classdev *led_cdev) > +{ > + struct input_events_data *data; > + > + data = kzalloc(sizeof(*data), GFP_KERNEL); > + if (!data) > + return -ENOMEM; > + > + INIT_DELAYED_WORK(&data->work, led_input_events_work); > + spin_lock_init(&data->lock); > + data->led_cdev = led_cdev; > + data->led_cdev_saved_flags = led_cdev->flags; > + data->led_off_delay = msecs_to_jiffies(DEFAULT_LED_OFF_DELAY_MS); > + > + led_set_trigger_data(led_cdev, data); > + > + /* > + * Use led_cdev->blink_brightness + LED_BLINK_SW flag so that sysfs > + * brightness writes will change led_cdev->new_blink_brightness for > + * configuring the on state brightness (like ledtrig-heartbeat). > + */ > + if (!led_cdev->blink_brightness) > + led_cdev->blink_brightness = led_cdev->max_brightness; > + > + set_bit(LED_BLINK_SW, &led_cdev->work_flags); > + /* Turn LED off during suspend, original flags are restored on deactivate() */ > + led_cdev->flags |= LED_CORE_SUSPENDRESUME; > + > + /* Start with LED off */ > + led_set_brightness_nosleep(data->led_cdev, LED_OFF); > + return 0; > +} > + > +static void input_events_deactivate(struct led_classdev *led_cdev) > +{ > + struct input_events_data *data = led_get_trigger_data(led_cdev); > + > + led_cdev->flags = data->led_cdev_saved_flags; > + clear_bit(LED_BLINK_SW, &led_cdev->work_flags); > + cancel_delayed_work_sync(&data->work); > + kfree(data); > +} > + > +static struct led_trigger input_events_led_trigger = { > + .name = "input-events", > + .activate = input_events_activate, > + .deactivate = input_events_deactivate, > + .groups = input_events_led_groups, > +}; > + > +static void input_events_event(struct input_handle *handle, unsigned int type, > + unsigned int code, int data) > +{ > + struct led_classdev *led_cdev; > + > + rcu_read_lock(); > + list_for_each_entry_rcu(led_cdev, &input_events_led_trigger.led_cdevs, trig_list) { > + struct input_events_data *data = led_get_trigger_data(led_cdev); > + unsigned long led_off_delay = READ_ONCE(data->led_off_delay); > + unsigned long flags; > + > + if (test_and_clear_bit(LED_BLINK_BRIGHTNESS_CHANGE, &led_cdev->work_flags)) > + led_cdev->blink_brightness = led_cdev->new_blink_brightness; > + > + spin_lock_irqsave(&data->lock, flags); > + > + if (!data->led_on) { > + led_set_brightness_nosleep(led_cdev, led_cdev->blink_brightness); > + data->led_on = true; > + } > + data->led_off_time = jiffies + led_off_delay; > + > + spin_unlock_irqrestore(&data->lock, flags); > + > + mod_delayed_work(system_wq, &data->work, led_off_delay); > + } > + rcu_read_unlock(); > +} > + > +static int input_events_connect(struct input_handler *handler, struct input_dev *dev, > + const struct input_device_id *id) > +{ > + struct input_handle *handle; > + int ret; > + > + handle = kzalloc(sizeof(struct input_handle), GFP_KERNEL); > + if (!handle) > + return -ENOMEM; > + > + handle->dev = dev; > + handle->handler = handler; > + handle->name = "input-events"; > + > + ret = input_register_handle(handle); > + if (ret) > + goto err_free_handle; > + > + ret = input_open_device(handle); > + if (ret) > + goto err_unregister_handle; > + > + return 0; > + > +err_unregister_handle: > + input_unregister_handle(handle); > +err_free_handle: > + kfree(handle); > + return ret; > +} > + > +static void input_events_disconnect(struct input_handle *handle) > +{ > + input_close_device(handle); > + input_unregister_handle(handle); > + kfree(handle); > +} > + > +static const struct input_device_id input_events_ids[] = { > + { > + .flags = INPUT_DEVICE_ID_MATCH_EVBIT, > + .evbit = { BIT_MASK(EV_KEY) }, > + }, > + { > + .flags = INPUT_DEVICE_ID_MATCH_EVBIT, > + .evbit = { BIT_MASK(EV_REL) }, > + }, > + { > + .flags = INPUT_DEVICE_ID_MATCH_EVBIT, > + .evbit = { BIT_MASK(EV_ABS) }, > + }, > + { } > +}; > + > +static struct input_handler input_events_handler = { > + .name = "input-events", > + .event = input_events_event, > + .connect = input_events_connect, > + .disconnect = input_events_disconnect, > + .id_table = input_events_ids, > +}; > + > +static int __init input_events_init(void) > +{ > + int ret; > + > + ret = led_trigger_register(&input_events_led_trigger); > + if (ret) > + return ret; > + > + ret = input_register_handler(&input_events_handler); > + if (ret) { > + led_trigger_unregister(&input_events_led_trigger); > + return ret; > + } > + > + return 0; > +} > + > +static void __exit input_events_exit(void) > +{ > + input_unregister_handler(&input_events_handler); > + led_trigger_unregister(&input_events_led_trigger); > +} > + > +module_init(input_events_init); > +module_exit(input_events_exit); > + > +MODULE_AUTHOR("Hans de Goede <hansg@xxxxxxxxxx>"); > +MODULE_DESCRIPTION("Input Events LED trigger"); > +MODULE_LICENSE("GPL");