[PATCH v2] tty: implement led triggers
From: johan@kernel.org (Johan Hovold)
Date: 2018-05-07 08:02:59
Also in:
linux-serial, lkml
On Thu, May 03, 2018 at 10:19:52PM +0200, Uwe Kleine-K?nig wrote:
The rx trigger fires when data is pushed to the ldisc. This is a bit later than the actual receiving of data but has the nice benefit that it doesn't need adaption for each driver and isn't in the hot path. Similarily the tx trigger fires when data taken from the ldisc.
You meant copied from user space, or written to the ldisc, here. Note that the rx-path is shared with serdev, but the write path is not. So with this series, serdev devices would only trigger the rx-led.
Signed-off-by: Uwe Kleine-K?nig <redacted> --- Changes since v1, sent with Message-Id: 20180503100448.1350-1-u.kleine-koenig at pengutronix.de: - implement tx trigger; - introduce Kconfig symbol for conditional compilation; - set trigger to NULL if allocating the name failed to not free random pointers in case the port struct wasn't zeroed; - use if/else instead of goto
quoted hunk ↗ jump to hunk
@@ -499,6 +500,7 @@ static void flush_to_ldisc(struct work_struct *work) struct tty_buffer *head = buf->head; struct tty_buffer *next; int count; + unsigned long delay = 50 /* ms */;
Comment after the semicolon?
/* Ldisc or user is trying to gain exclusive access */ if (atomic_read(&buf->priority))
quoted hunk ↗ jump to hunk
diff --git a/drivers/tty/tty_port.c b/drivers/tty/tty_port.c index 25d736880013..f042879a597c 100644 --- a/drivers/tty/tty_port.c +++ b/drivers/tty/tty_port.c@@ -16,6 +16,7 @@ #include <linux/wait.h> #include <linux/bitops.h> #include <linux/delay.h> +#include <linux/leds.h> #include <linux/module.h> #include <linux/serdev.h>@@ -157,6 +158,30 @@ struct device *tty_port_register_device_attr_serdev(struct tty_port *port, tty_port_link_device(port, driver, index); +#ifdef CONFIG_TTY_LEDS_TRIGGER + port->led_trigger_rx_name = kasprintf(GFP_KERNEL, "%s%d-rx", + driver->name, index); + if (port->led_trigger_rx_name) { + led_trigger_register_simple(port->led_trigger_rx_name, + &port->led_trigger_rx); + } else { + port->led_trigger_rx = NULL; + pr_err("Failed to allocate trigger name for %s%d\n", + driver->name, index); + } + + port->led_trigger_tx_name = kasprintf(GFP_KERNEL, "%s%d-tx", + driver->name, index); + if (port->led_trigger_tx_name) { + led_trigger_register_simple(port->led_trigger_tx_name, + &port->led_trigger_tx); + } else { + port->led_trigger_tx = NULL; + pr_err("Failed to allocate trigger name for %s%d\n", + driver->name, index); + } +#endif
Besides the ugly ifdefs here, you're leaking the above LED resources in the error paths below (e.g. on probe deferrals).
dev = serdev_tty_port_register(port, device, driver, index);
if (PTR_ERR(dev) != -ENODEV) {
/* Skip creating cdev if we registered a serdev device */quoted hunk ↗ jump to hunk
@@ -249,6 +249,13 @@ struct tty_port { set to size of fifo */ struct kref kref; /* Ref counter */ void *client_data; + +#ifdef CONFIG_TTY_LEDS_TRIGGER + struct led_trigger *led_trigger_rx; + char *led_trigger_rx_name;
const?
+ struct led_trigger *led_trigger_tx; + char *led_trigger_tx_name;
ditto.
+#endif
Johan