Re: [PATCH 1/1] leds: trigger: Add new LED Input events trigger

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

 



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");





[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux OMAP]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux