linux-leds.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v4 0/3] tty/leds: implement a trigger for ttys
@ 2019-12-17 16:58 Uwe Kleine-König
  2019-12-17 16:58 ` [PATCH v4 1/3] tty: rename tty_kopen() and add new function tty_kopen_shared() Uwe Kleine-König
                   ` (3 more replies)
  0 siblings, 4 replies; 12+ messages in thread
From: Uwe Kleine-König @ 2019-12-17 16:58 UTC (permalink / raw)
  To: Jacek Anaszewski, Pavel Machek, Dan Murphy, Greg Kroah-Hartman,
	Jiri Slaby
  Cc: kernel, linux-kernel, linux-leds, linux-serial

Hello,

v3 of this series was sent earlier today starting with Message-Id:
20191217150736.1479-1-u.kleine-koenig@pengutronix.de.

v4 only changes patch 3 dropping a few printks, fixing the show callback
for the dev attribute to match its store function. And I moved
ledtrig_tty_restart() into the already existing if (tty) {...} block in
dev_store and dropped the same check from the former function.

Best regards
Uwe

Uwe Kleine-König (3):
  tty: rename tty_kopen() and add new function tty_kopen_shared()
  tty: new helper function tty_get_icount()
  leds: trigger: implement a tty trigger

 drivers/leds/trigger/Kconfig        |   7 ++
 drivers/leds/trigger/Makefile       |   1 +
 drivers/leds/trigger/ledtrig-tty.c  | 153 ++++++++++++++++++++++++++++
 drivers/staging/speakup/spk_ttyio.c |   2 +-
 drivers/tty/tty_io.c                |  87 ++++++++++++----
 include/linux/tty.h                 |   7 +-
 6 files changed, 232 insertions(+), 25 deletions(-)
 create mode 100644 drivers/leds/trigger/ledtrig-tty.c

-- 
2.24.0


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH v4 1/3] tty: rename tty_kopen() and add new function tty_kopen_shared()
  2019-12-17 16:58 [PATCH v4 0/3] tty/leds: implement a trigger for ttys Uwe Kleine-König
@ 2019-12-17 16:58 ` Uwe Kleine-König
  2019-12-17 18:08   ` Jacek Anaszewski
  2019-12-17 16:58 ` [PATCH v4 2/3] tty: new helper function tty_get_icount() Uwe Kleine-König
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 12+ messages in thread
From: Uwe Kleine-König @ 2019-12-17 16:58 UTC (permalink / raw)
  To: Jacek Anaszewski, Pavel Machek, Dan Murphy, Greg Kroah-Hartman,
	Jiri Slaby
  Cc: kernel, linux-kernel, linux-leds, linux-serial

Introduce a new function tty_kopen_shared() that yields a struct
tty_struct. The semantic difference to tty_kopen() is that the tty is
expected to be used already. So rename tty_kopen() to
tty_kopen_exclusive() for clearness, adapt the single user and put the
common code in a new static helper function.

tty_kopen_shared is to be used to implement an LED trigger for tty
devices in one of the next patches.

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
 drivers/staging/speakup/spk_ttyio.c |  2 +-
 drivers/tty/tty_io.c                | 58 ++++++++++++++++++++---------
 include/linux/tty.h                 |  5 ++-
 3 files changed, 44 insertions(+), 21 deletions(-)

diff --git a/drivers/staging/speakup/spk_ttyio.c b/drivers/staging/speakup/spk_ttyio.c
index 5a9eff08cb96..e9db06eae875 100644
--- a/drivers/staging/speakup/spk_ttyio.c
+++ b/drivers/staging/speakup/spk_ttyio.c
@@ -147,7 +147,7 @@ static int spk_ttyio_initialise_ldisc(struct spk_synth *synth)
 	if (ret)
 		return ret;
 
-	tty = tty_kopen(dev);
+	tty = tty_kopen_exclusive(dev);
 	if (IS_ERR(tty))
 		return PTR_ERR(tty);
 
diff --git a/drivers/tty/tty_io.c b/drivers/tty/tty_io.c
index a1453fe10862..b718666ce73c 100644
--- a/drivers/tty/tty_io.c
+++ b/drivers/tty/tty_io.c
@@ -1875,22 +1875,7 @@ static struct tty_driver *tty_lookup_driver(dev_t device, struct file *filp,
 	return driver;
 }
 
-/**
- *	tty_kopen	-	open a tty device for kernel
- *	@device: dev_t of device to open
- *
- *	Opens tty exclusively for kernel. Performs the driver lookup,
- *	makes sure it's not already opened and performs the first-time
- *	tty initialization.
- *
- *	Returns the locked initialized &tty_struct
- *
- *	Claims the global tty_mutex to serialize:
- *	  - concurrent first-time tty initialization
- *	  - concurrent tty driver removal w/ lookup
- *	  - concurrent tty removal from driver table
- */
-struct tty_struct *tty_kopen(dev_t device)
+static struct tty_struct *tty_kopen(dev_t device, int shared)
 {
 	struct tty_struct *tty;
 	struct tty_driver *driver;
@@ -1905,7 +1890,7 @@ struct tty_struct *tty_kopen(dev_t device)
 
 	/* check whether we're reopening an existing tty */
 	tty = tty_driver_lookup_tty(driver, NULL, index);
-	if (IS_ERR(tty))
+	if (IS_ERR(tty) || shared)
 		goto out;
 
 	if (tty) {
@@ -1923,7 +1908,44 @@ struct tty_struct *tty_kopen(dev_t device)
 	tty_driver_kref_put(driver);
 	return tty;
 }
-EXPORT_SYMBOL_GPL(tty_kopen);
+
+/**
+ *	tty_kopen_exclusive	-	open a tty device for kernel
+ *	@device: dev_t of device to open
+ *
+ *	Opens tty exclusively for kernel. Performs the driver lookup,
+ *	makes sure it's not already opened and performs the first-time
+ *	tty initialization.
+ *
+ *	Returns the locked initialized &tty_struct
+ *
+ *	Claims the global tty_mutex to serialize:
+ *	  - concurrent first-time tty initialization
+ *	  - concurrent tty driver removal w/ lookup
+ *	  - concurrent tty removal from driver table
+ */
+struct tty_struct *tty_kopen_exclusive(dev_t device)
+{
+	return tty_kopen(device, 0);
+}
+EXPORT_SYMBOL_GPL(tty_kopen_exclusive);
+
+/**
+ *	tty_kopen_shared	-	open a tty device for shared in-kernel use
+ *	@device: dev_t of device to open
+ *
+ *	Opens an already existing tty
+ *	rnel. Performs the driver lookup,
+ *	makes sure it's not already opened and performs the first-time
+ *	tty initialization.
+ *
+ *	Locking is identical to tty_kopen() above.
+ */
+struct tty_struct *tty_kopen_shared(dev_t device)
+{
+	return tty_kopen(device, 1);
+}
+EXPORT_SYMBOL_GPL(tty_kopen_shared);
 
 /**
  *	tty_open_by_driver	-	open a tty device
diff --git a/include/linux/tty.h b/include/linux/tty.h
index bfa4e2ee94a9..d0bcf3226fb2 100644
--- a/include/linux/tty.h
+++ b/include/linux/tty.h
@@ -411,7 +411,8 @@ extern struct tty_struct *get_current_tty(void);
 /* tty_io.c */
 extern int __init tty_init(void);
 extern const char *tty_name(const struct tty_struct *tty);
-extern struct tty_struct *tty_kopen(dev_t device);
+extern struct tty_struct *tty_kopen_exclusive(dev_t device);
+extern struct tty_struct *tty_kopen_shared(dev_t device);
 extern void tty_kclose(struct tty_struct *tty);
 extern int tty_dev_name_to_number(const char *name, dev_t *number);
 extern int tty_ldisc_lock(struct tty_struct *tty, unsigned long timeout);
@@ -436,7 +437,7 @@ static inline int __init tty_init(void)
 { return 0; }
 static inline const char *tty_name(const struct tty_struct *tty)
 { return "(none)"; }
-static inline struct tty_struct *tty_kopen(dev_t device)
+static inline struct tty_struct *tty_kopen_exclusive(dev_t device)
 { return ERR_PTR(-ENODEV); }
 static inline void tty_kclose(struct tty_struct *tty)
 { }
-- 
2.24.0


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v4 2/3] tty: new helper function tty_get_icount()
  2019-12-17 16:58 [PATCH v4 0/3] tty/leds: implement a trigger for ttys Uwe Kleine-König
  2019-12-17 16:58 ` [PATCH v4 1/3] tty: rename tty_kopen() and add new function tty_kopen_shared() Uwe Kleine-König
@ 2019-12-17 16:58 ` Uwe Kleine-König
  2019-12-17 16:58 ` [PATCH v4 3/3] leds: trigger: implement a tty trigger Uwe Kleine-König
  2019-12-17 17:22 ` [PATCH v4 0/3] tty/leds: implement a trigger for ttys Greg Kroah-Hartman
  3 siblings, 0 replies; 12+ messages in thread
From: Uwe Kleine-König @ 2019-12-17 16:58 UTC (permalink / raw)
  To: Jacek Anaszewski, Pavel Machek, Dan Murphy, Greg Kroah-Hartman,
	Jiri Slaby
  Cc: kernel, linux-kernel, linux-leds, linux-serial

For a given struct tty_struct this yields the corresponding statistics
about sent and received characters (and some more) which is needed to
implement an LED trigger for tty devices.

The new function is then used to simplify tty_tiocgicount().

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
 drivers/tty/tty_io.c | 29 +++++++++++++++++++++++++----
 include/linux/tty.h  |  2 ++
 2 files changed, 27 insertions(+), 4 deletions(-)

diff --git a/drivers/tty/tty_io.c b/drivers/tty/tty_io.c
index b718666ce73c..3e74e6cec1a6 100644
--- a/drivers/tty/tty_io.c
+++ b/drivers/tty/tty_io.c
@@ -2492,15 +2492,36 @@ static int tty_tiocmset(struct tty_struct *tty, unsigned int cmd,
 	return tty->ops->tiocmset(tty, set, clear);
 }
 
+/**
+ *	tty_get_icount		-	get tty statistics
+ *	@tty: tty device
+ *	@icount: output parameter
+ *
+ *	Gets a copy of the tty's icount statistics.
+ *
+ *	Locking: none (up to the driver)
+ */
+int tty_get_icount(struct tty_struct *tty,
+		   struct serial_icounter_struct *icount)
+{
+	memset(icount, 0, sizeof(*icount));
+
+	if (tty->ops->get_icount)
+		return tty->ops->get_icount(tty, icount);
+	else
+		return -EINVAL;
+}
+EXPORT_SYMBOL_GPL(tty_get_icount);
+
 static int tty_tiocgicount(struct tty_struct *tty, void __user *arg)
 {
-	int retval = -EINVAL;
 	struct serial_icounter_struct icount;
-	memset(&icount, 0, sizeof(icount));
-	if (tty->ops->get_icount)
-		retval = tty->ops->get_icount(tty, &icount);
+	int retval;
+
+	retval = tty_get_icount(tty, &icount);
 	if (retval != 0)
 		return retval;
+
 	if (copy_to_user(arg, &icount, sizeof(icount)))
 		return -EFAULT;
 	return 0;
diff --git a/include/linux/tty.h b/include/linux/tty.h
index d0bcf3226fb2..40ea5c409619 100644
--- a/include/linux/tty.h
+++ b/include/linux/tty.h
@@ -495,6 +495,8 @@ extern void tty_unthrottle(struct tty_struct *tty);
 extern int tty_throttle_safe(struct tty_struct *tty);
 extern int tty_unthrottle_safe(struct tty_struct *tty);
 extern int tty_do_resize(struct tty_struct *tty, struct winsize *ws);
+extern int tty_get_icount(struct tty_struct *tty,
+			  struct serial_icounter_struct *icount);
 extern int is_current_pgrp_orphaned(void);
 extern void tty_hangup(struct tty_struct *tty);
 extern void tty_vhangup(struct tty_struct *tty);
-- 
2.24.0


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v4 3/3] leds: trigger: implement a tty trigger
  2019-12-17 16:58 [PATCH v4 0/3] tty/leds: implement a trigger for ttys Uwe Kleine-König
  2019-12-17 16:58 ` [PATCH v4 1/3] tty: rename tty_kopen() and add new function tty_kopen_shared() Uwe Kleine-König
  2019-12-17 16:58 ` [PATCH v4 2/3] tty: new helper function tty_get_icount() Uwe Kleine-König
@ 2019-12-17 16:58 ` Uwe Kleine-König
  2019-12-17 17:22 ` [PATCH v4 0/3] tty/leds: implement a trigger for ttys Greg Kroah-Hartman
  3 siblings, 0 replies; 12+ messages in thread
From: Uwe Kleine-König @ 2019-12-17 16:58 UTC (permalink / raw)
  To: Jacek Anaszewski, Pavel Machek, Dan Murphy, Greg Kroah-Hartman,
	Jiri Slaby
  Cc: kernel, linux-kernel, linux-leds, linux-serial

Usage is as follows:

	myled=ledname
	tty=ttyS0

	echo tty > /sys/class/leds/$myled/trigger
	cat /sys/class/tty/$tty/dev > /sys/class/leds/$myled/dev

. When this new trigger is active it periodically checks the tty's
statistics and when it changed since the last check the led is flashed
once.

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
 drivers/leds/trigger/Kconfig       |   7 ++
 drivers/leds/trigger/Makefile      |   1 +
 drivers/leds/trigger/ledtrig-tty.c | 153 +++++++++++++++++++++++++++++
 3 files changed, 161 insertions(+)
 create mode 100644 drivers/leds/trigger/ledtrig-tty.c

diff --git a/drivers/leds/trigger/Kconfig b/drivers/leds/trigger/Kconfig
index ce9429ca6dde..40ff08c93f56 100644
--- a/drivers/leds/trigger/Kconfig
+++ b/drivers/leds/trigger/Kconfig
@@ -144,4 +144,11 @@ config LEDS_TRIGGER_AUDIO
 	  the audio mute and mic-mute changes.
 	  If unsure, say N
 
+config LEDS_TRIGGER_TTY
+	tristate "LED Trigger for TTY devices"
+	depends on TTY
+	help
+	  This allows LEDs to be controlled by activity on ttys which includes
+	  serial devices like /dev/ttyS0.
+
 endif # LEDS_TRIGGERS
diff --git a/drivers/leds/trigger/Makefile b/drivers/leds/trigger/Makefile
index 733a83e2a718..25c4db97cdd4 100644
--- a/drivers/leds/trigger/Makefile
+++ b/drivers/leds/trigger/Makefile
@@ -15,3 +15,4 @@ obj-$(CONFIG_LEDS_TRIGGER_PANIC)	+= ledtrig-panic.o
 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
diff --git a/drivers/leds/trigger/ledtrig-tty.c b/drivers/leds/trigger/ledtrig-tty.c
new file mode 100644
index 000000000000..e9bbad52cc3f
--- /dev/null
+++ b/drivers/leds/trigger/ledtrig-tty.c
@@ -0,0 +1,153 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#include <linux/leds.h>
+#include <linux/module.h>
+#include <linux/slab.h>
+#include <linux/tty.h>
+#include <uapi/linux/serial.h>
+
+struct ledtrig_tty_data {
+	struct led_classdev *led_cdev;
+	struct delayed_work dwork;
+	struct tty_struct *tty;
+	dev_t device;
+	int rx, tx;
+};
+
+static void ledtrig_tty_halt(struct ledtrig_tty_data *trigger_data)
+{
+	cancel_delayed_work_sync(&trigger_data->dwork);
+}
+
+static void ledtrig_tty_restart(struct ledtrig_tty_data *trigger_data)
+{
+	schedule_delayed_work(&trigger_data->dwork, 0);
+}
+
+static ssize_t dev_show(struct device *dev,
+			struct device_attribute *attr, char *buf)
+{
+	struct ledtrig_tty_data *trigger_data = led_trigger_get_drvdata(dev);
+	ssize_t len = 0;
+
+	if (trigger_data->tty)
+		len = sprintf(buf, "%d:%d\n", MAJOR(trigger_data->device),
+			      MINOR(trigger_data->device));
+
+	return len;
+}
+
+static ssize_t dev_store(struct device *dev,
+			 struct device_attribute *attr, const char *buf,
+			 size_t size)
+{
+	struct ledtrig_tty_data *trigger_data = led_trigger_get_drvdata(dev);
+	struct tty_struct *tty;
+	unsigned major, minor;
+	int ret;
+
+	if (size == 0 || (size == 1 && buf[0] == '\n')) {
+		tty = NULL;
+	} else {
+		ret = sscanf(buf, "%u:%u", &major, &minor);
+		if (ret < 2)
+			return -EINVAL;
+
+		tty = tty_kopen_shared(MKDEV(major, minor));
+		if (IS_ERR(tty))
+			return PTR_ERR(tty);
+	}
+
+	ledtrig_tty_halt(trigger_data);
+
+	tty_kref_put(trigger_data->tty);
+	trigger_data->tty = tty;
+	trigger_data->device = MKDEV(major, minor);
+
+	if (tty) {
+		struct serial_icounter_struct icount;
+		ret = tty_get_icount(trigger_data->tty, &icount);
+		if (!ret) {
+			trigger_data->tx = icount.tx;
+			trigger_data->rx = icount.rx;
+		}
+
+		ledtrig_tty_restart(trigger_data);
+	}
+
+	return size;
+}
+static DEVICE_ATTR_RW(dev);
+
+static void ledtrig_tty_work(struct work_struct *work)
+{
+	struct ledtrig_tty_data *trigger_data =
+		container_of(work, struct ledtrig_tty_data, dwork.work);
+	struct serial_icounter_struct icount;
+	int ret;
+
+	if (!trigger_data->tty) {
+		led_set_brightness(trigger_data->led_cdev, LED_OFF);
+		return;
+	}
+
+	ret = tty_get_icount(trigger_data->tty, &icount);
+	if (ret)
+		return;
+
+	if (icount.rx != trigger_data->rx ||
+	    icount.tx != trigger_data->tx) {
+		unsigned long delay_on = 100, delay_off = 100;
+
+		led_blink_set_oneshot(trigger_data->led_cdev,
+				      &delay_on, &delay_off, 0);
+
+		trigger_data->rx = icount.rx;
+		trigger_data->tx = icount.tx;
+	}
+
+	schedule_delayed_work(&trigger_data->dwork, msecs_to_jiffies(100));
+}
+
+static struct attribute *ledtrig_tty_attrs[] = {
+	&dev_attr_dev.attr,
+	NULL
+};
+ATTRIBUTE_GROUPS(ledtrig_tty);
+
+static int ledtrig_tty_activate(struct led_classdev *led_cdev)
+{
+	struct ledtrig_tty_data *trigger_data;
+
+	trigger_data = kzalloc(sizeof(*trigger_data), GFP_KERNEL);
+	if (!trigger_data)
+		return -ENOMEM;
+
+	led_set_trigger_data(led_cdev, trigger_data);
+
+	INIT_DELAYED_WORK(&trigger_data->dwork, ledtrig_tty_work);
+	trigger_data->led_cdev = led_cdev;
+
+	return 0;
+}
+
+static void ledtrig_tty_deactivate(struct led_classdev *led_cdev)
+{
+	struct ledtrig_tty_data *trigger_data = led_get_trigger_data(led_cdev);
+
+	cancel_delayed_work_sync(&trigger_data->dwork);
+
+	kfree(trigger_data);
+}
+
+struct led_trigger ledtrig_tty = {
+	.name = "tty",
+	.activate = ledtrig_tty_activate,
+	.deactivate = ledtrig_tty_deactivate,
+	.groups = ledtrig_tty_groups,
+};
+module_led_trigger(ledtrig_tty);
+
+MODULE_AUTHOR("Uwe Kleine-König <u.kleine-koenig@pengutronix.de>");
+MODULE_DESCRIPTION("UART LED trigger");
+MODULE_LICENSE("GPL v2");
-- 
2.24.0


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH v4 0/3] tty/leds: implement a trigger for ttys
  2019-12-17 16:58 [PATCH v4 0/3] tty/leds: implement a trigger for ttys Uwe Kleine-König
                   ` (2 preceding siblings ...)
  2019-12-17 16:58 ` [PATCH v4 3/3] leds: trigger: implement a tty trigger Uwe Kleine-König
@ 2019-12-17 17:22 ` Greg Kroah-Hartman
  2019-12-21 18:36   ` Pavel Machek
  3 siblings, 1 reply; 12+ messages in thread
From: Greg Kroah-Hartman @ 2019-12-17 17:22 UTC (permalink / raw)
  To: Uwe Kleine-König
  Cc: Jacek Anaszewski, Pavel Machek, Dan Murphy, Jiri Slaby, kernel,
	linux-kernel, linux-leds, linux-serial

On Tue, Dec 17, 2019 at 05:58:13PM +0100, Uwe Kleine-König wrote:
> Hello,
> 
> v3 of this series was sent earlier today starting with Message-Id:
> 20191217150736.1479-1-u.kleine-koenig@pengutronix.de.
> 
> v4 only changes patch 3 dropping a few printks, fixing the show callback
> for the dev attribute to match its store function. And I moved
> ledtrig_tty_restart() into the already existing if (tty) {...} block in
> dev_store and dropped the same check from the former function.
> 

All looks good to me, so if the LED developers give their ack, I can
take it in my tty tree.

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v4 1/3] tty: rename tty_kopen() and add new function tty_kopen_shared()
  2019-12-17 16:58 ` [PATCH v4 1/3] tty: rename tty_kopen() and add new function tty_kopen_shared() Uwe Kleine-König
@ 2019-12-17 18:08   ` Jacek Anaszewski
  2019-12-17 18:20     ` Greg Kroah-Hartman
  2019-12-17 18:27     ` Uwe Kleine-König
  0 siblings, 2 replies; 12+ messages in thread
From: Jacek Anaszewski @ 2019-12-17 18:08 UTC (permalink / raw)
  To: Uwe Kleine-König, Pavel Machek, Dan Murphy,
	Greg Kroah-Hartman, Jiri Slaby
  Cc: kernel, linux-kernel, linux-leds, linux-serial

Hi Uwe,

I wanted to test the set but unfortunately this
patch does not apply. See below for the apparent reason.

On 12/17/19 5:58 PM, Uwe Kleine-König wrote:
> Introduce a new function tty_kopen_shared() that yields a struct
> tty_struct. The semantic difference to tty_kopen() is that the tty is
> expected to be used already. So rename tty_kopen() to
> tty_kopen_exclusive() for clearness, adapt the single user and put the
> common code in a new static helper function.
> 
> tty_kopen_shared is to be used to implement an LED trigger for tty
> devices in one of the next patches.
> 
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> ---
>  drivers/staging/speakup/spk_ttyio.c |  2 +-
>  drivers/tty/tty_io.c                | 58 ++++++++++++++++++++---------
>  include/linux/tty.h                 |  5 ++-
>  3 files changed, 44 insertions(+), 21 deletions(-)
> 
> diff --git a/drivers/staging/speakup/spk_ttyio.c b/drivers/staging/speakup/spk_ttyio.c
> index 5a9eff08cb96..e9db06eae875 100644
> --- a/drivers/staging/speakup/spk_ttyio.c
> +++ b/drivers/staging/speakup/spk_ttyio.c
> @@ -147,7 +147,7 @@ static int spk_ttyio_initialise_ldisc(struct spk_synth *synth)
>  	if (ret)
>  		return ret;
>  
> -	tty = tty_kopen(dev);
> +	tty = tty_kopen_exclusive(dev);
>  	if (IS_ERR(tty))
>  		return PTR_ERR(tty);
>  
> diff --git a/drivers/tty/tty_io.c b/drivers/tty/tty_io.c
> index a1453fe10862..b718666ce73c 100644
> --- a/drivers/tty/tty_io.c
> +++ b/drivers/tty/tty_io.c
> @@ -1875,22 +1875,7 @@ static struct tty_driver *tty_lookup_driver(dev_t device, struct file *filp,
>  	return driver;
>  }
>  
> -/**
> - *	tty_kopen	-	open a tty device for kernel
> - *	@device: dev_t of device to open
> - *
> - *	Opens tty exclusively for kernel. Performs the driver lookup,
> - *	makes sure it's not already opened and performs the first-time
> - *	tty initialization.
> - *
> - *	Returns the locked initialized &tty_struct
> - *
> - *	Claims the global tty_mutex to serialize:
> - *	  - concurrent first-time tty initialization
> - *	  - concurrent tty driver removal w/ lookup
> - *	  - concurrent tty removal from driver table
> - */
> -struct tty_struct *tty_kopen(dev_t device)
> +static struct tty_struct *tty_kopen(dev_t device, int shared)
>  {
>  	struct tty_struct *tty;
>  	struct tty_driver *driver;

In mainline, even in v5.5-rc2 we have here NULL assignment:

struct tty_driver *driver = NULL;

> @@ -1905,7 +1890,7 @@ struct tty_struct *tty_kopen(dev_t device)
>  


-- 
Best regards,
Jacek Anaszewski

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v4 1/3] tty: rename tty_kopen() and add new function tty_kopen_shared()
  2019-12-17 18:08   ` Jacek Anaszewski
@ 2019-12-17 18:20     ` Greg Kroah-Hartman
  2019-12-17 18:27     ` Uwe Kleine-König
  1 sibling, 0 replies; 12+ messages in thread
From: Greg Kroah-Hartman @ 2019-12-17 18:20 UTC (permalink / raw)
  To: Jacek Anaszewski
  Cc: Uwe Kleine-König, Pavel Machek, Dan Murphy, Jiri Slaby,
	kernel, linux-kernel, linux-leds, linux-serial

On Tue, Dec 17, 2019 at 07:08:47PM +0100, Jacek Anaszewski wrote:
> Hi Uwe,
> 
> I wanted to test the set but unfortunately this
> patch does not apply. See below for the apparent reason.
> 
> On 12/17/19 5:58 PM, Uwe Kleine-König wrote:
> > Introduce a new function tty_kopen_shared() that yields a struct
> > tty_struct. The semantic difference to tty_kopen() is that the tty is
> > expected to be used already. So rename tty_kopen() to
> > tty_kopen_exclusive() for clearness, adapt the single user and put the
> > common code in a new static helper function.
> > 
> > tty_kopen_shared is to be used to implement an LED trigger for tty
> > devices in one of the next patches.
> > 
> > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> > ---
> >  drivers/staging/speakup/spk_ttyio.c |  2 +-
> >  drivers/tty/tty_io.c                | 58 ++++++++++++++++++++---------
> >  include/linux/tty.h                 |  5 ++-
> >  3 files changed, 44 insertions(+), 21 deletions(-)
> > 
> > diff --git a/drivers/staging/speakup/spk_ttyio.c b/drivers/staging/speakup/spk_ttyio.c
> > index 5a9eff08cb96..e9db06eae875 100644
> > --- a/drivers/staging/speakup/spk_ttyio.c
> > +++ b/drivers/staging/speakup/spk_ttyio.c
> > @@ -147,7 +147,7 @@ static int spk_ttyio_initialise_ldisc(struct spk_synth *synth)
> >  	if (ret)
> >  		return ret;
> >  
> > -	tty = tty_kopen(dev);
> > +	tty = tty_kopen_exclusive(dev);
> >  	if (IS_ERR(tty))
> >  		return PTR_ERR(tty);
> >  
> > diff --git a/drivers/tty/tty_io.c b/drivers/tty/tty_io.c
> > index a1453fe10862..b718666ce73c 100644
> > --- a/drivers/tty/tty_io.c
> > +++ b/drivers/tty/tty_io.c
> > @@ -1875,22 +1875,7 @@ static struct tty_driver *tty_lookup_driver(dev_t device, struct file *filp,
> >  	return driver;
> >  }
> >  
> > -/**
> > - *	tty_kopen	-	open a tty device for kernel
> > - *	@device: dev_t of device to open
> > - *
> > - *	Opens tty exclusively for kernel. Performs the driver lookup,
> > - *	makes sure it's not already opened and performs the first-time
> > - *	tty initialization.
> > - *
> > - *	Returns the locked initialized &tty_struct
> > - *
> > - *	Claims the global tty_mutex to serialize:
> > - *	  - concurrent first-time tty initialization
> > - *	  - concurrent tty driver removal w/ lookup
> > - *	  - concurrent tty removal from driver table
> > - */
> > -struct tty_struct *tty_kopen(dev_t device)
> > +static struct tty_struct *tty_kopen(dev_t device, int shared)
> >  {
> >  	struct tty_struct *tty;
> >  	struct tty_driver *driver;
> 
> In mainline, even in v5.5-rc2 we have here NULL assignment:
> 
> struct tty_driver *driver = NULL;

This is based on a patch that is already in my tty-testing tree and that
was posted earlier.  It should show up in linux-next in a day or so.

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v4 1/3] tty: rename tty_kopen() and add new function tty_kopen_shared()
  2019-12-17 18:08   ` Jacek Anaszewski
  2019-12-17 18:20     ` Greg Kroah-Hartman
@ 2019-12-17 18:27     ` Uwe Kleine-König
  2019-12-17 20:19       ` Jacek Anaszewski
  1 sibling, 1 reply; 12+ messages in thread
From: Uwe Kleine-König @ 2019-12-17 18:27 UTC (permalink / raw)
  To: Jacek Anaszewski
  Cc: Pavel Machek, Dan Murphy, Greg Kroah-Hartman, Jiri Slaby,
	linux-leds, linux-kernel, kernel, linux-serial

Hello Jacek,

On Tue, Dec 17, 2019 at 07:08:47PM +0100, Jacek Anaszewski wrote:
> I wanted to test the set but unfortunately this
> patch does not apply. See below for the apparent reason.
>
> > [...]
> > -struct tty_struct *tty_kopen(dev_t device)
> > +static struct tty_struct *tty_kopen(dev_t device, int shared)
> >  {
> >  	struct tty_struct *tty;
> >  	struct tty_driver *driver;
> 
> In mainline, even in v5.5-rc2 we have here NULL assignment:
> 
> struct tty_driver *driver = NULL;

Yeah, if you don't want to wait for Greg's tree to appear in next, this
is the patch you're missing:

	https://lkml.org/lkml/2019/12/17/101

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | https://www.pengutronix.de/ |

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v4 1/3] tty: rename tty_kopen() and add new function tty_kopen_shared()
  2019-12-17 18:27     ` Uwe Kleine-König
@ 2019-12-17 20:19       ` Jacek Anaszewski
  2019-12-17 20:23         ` Greg Kroah-Hartman
  0 siblings, 1 reply; 12+ messages in thread
From: Jacek Anaszewski @ 2019-12-17 20:19 UTC (permalink / raw)
  To: Uwe Kleine-König
  Cc: Pavel Machek, Dan Murphy, Greg Kroah-Hartman, Jiri Slaby,
	linux-leds, linux-kernel, kernel, linux-serial

On 12/17/19 7:27 PM, Uwe Kleine-König wrote:
> Hello Jacek,
> 
> On Tue, Dec 17, 2019 at 07:08:47PM +0100, Jacek Anaszewski wrote:
>> I wanted to test the set but unfortunately this
>> patch does not apply. See below for the apparent reason.
>>
>>> [...]
>>> -struct tty_struct *tty_kopen(dev_t device)
>>> +static struct tty_struct *tty_kopen(dev_t device, int shared)
>>>  {
>>>  	struct tty_struct *tty;
>>>  	struct tty_driver *driver;
>>
>> In mainline, even in v5.5-rc2 we have here NULL assignment:
>>
>> struct tty_driver *driver = NULL;
> 
> Yeah, if you don't want to wait for Greg's tree to appear in next, this
> is the patch you're missing:
> 
> 	https://lkml.org/lkml/2019/12/17/101

Thanks for the pointer.

And regarding the trigger - I've tried below use case
on my desktop Debian:

Ctrl+Alt+F3 to change the console

Then logged in and executed tty command:

$ tty
$ /dev/tty3

$ cd /sys/class/leds/input5\:\:capslock
note: this is LED on my USB keyboard and it works correct
with e.g. timer trigger

/sys/class/leds/input5::capslock$ echo tty > trigger

$ cat /sys/class/tty/tty3/dev > dev

Type some characters.

LED does not blink.

$ echo "aaa" > /dev/tty3
$ aaa

LED also does not blink.

I assume this is correct test case for tty trigger?

-- 
Best regards,
Jacek Anaszewski



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v4 1/3] tty: rename tty_kopen() and add new function tty_kopen_shared()
  2019-12-17 20:19       ` Jacek Anaszewski
@ 2019-12-17 20:23         ` Greg Kroah-Hartman
  0 siblings, 0 replies; 12+ messages in thread
From: Greg Kroah-Hartman @ 2019-12-17 20:23 UTC (permalink / raw)
  To: Jacek Anaszewski
  Cc: Uwe Kleine-König, Pavel Machek, Dan Murphy, Jiri Slaby,
	linux-leds, linux-kernel, kernel, linux-serial

On Tue, Dec 17, 2019 at 09:19:38PM +0100, Jacek Anaszewski wrote:
> On 12/17/19 7:27 PM, Uwe Kleine-König wrote:
> > Hello Jacek,
> > 
> > On Tue, Dec 17, 2019 at 07:08:47PM +0100, Jacek Anaszewski wrote:
> >> I wanted to test the set but unfortunately this
> >> patch does not apply. See below for the apparent reason.
> >>
> >>> [...]
> >>> -struct tty_struct *tty_kopen(dev_t device)
> >>> +static struct tty_struct *tty_kopen(dev_t device, int shared)
> >>>  {
> >>>  	struct tty_struct *tty;
> >>>  	struct tty_driver *driver;
> >>
> >> In mainline, even in v5.5-rc2 we have here NULL assignment:
> >>
> >> struct tty_driver *driver = NULL;
> > 
> > Yeah, if you don't want to wait for Greg's tree to appear in next, this
> > is the patch you're missing:
> > 
> > 	https://lkml.org/lkml/2019/12/17/101
> 
> Thanks for the pointer.
> 
> And regarding the trigger - I've tried below use case
> on my desktop Debian:
> 
> Ctrl+Alt+F3 to change the console
> 
> Then logged in and executed tty command:
> 
> $ tty
> $ /dev/tty3
> 
> $ cd /sys/class/leds/input5\:\:capslock
> note: this is LED on my USB keyboard and it works correct
> with e.g. timer trigger
> 
> /sys/class/leds/input5::capslock$ echo tty > trigger
> 
> $ cat /sys/class/tty/tty3/dev > dev
> 
> Type some characters.
> 
> LED does not blink.
> 
> $ echo "aaa" > /dev/tty3
> $ aaa
> 
> LED also does not blink.
> 
> I assume this is correct test case for tty trigger?

Try it on a real serial port, tty3 is a virtual one and I do not think
the "transmit/receive" statistics are every updated on a virtual one as
it does not make any sense to do so.

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v4 0/3] tty/leds: implement a trigger for ttys
  2019-12-17 17:22 ` [PATCH v4 0/3] tty/leds: implement a trigger for ttys Greg Kroah-Hartman
@ 2019-12-21 18:36   ` Pavel Machek
  2019-12-23  9:58     ` Uwe Kleine-König
  0 siblings, 1 reply; 12+ messages in thread
From: Pavel Machek @ 2019-12-21 18:36 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Uwe Kleine-König, Jacek Anaszewski, Dan Murphy, Jiri Slaby,
	kernel, linux-kernel, linux-leds, linux-serial

[-- Attachment #1: Type: text/plain, Size: 1065 bytes --]

On Tue 2019-12-17 18:22:32, Greg Kroah-Hartman wrote:
> On Tue, Dec 17, 2019 at 05:58:13PM +0100, Uwe Kleine-König wrote:
> > Hello,
> > 
> > v3 of this series was sent earlier today starting with Message-Id:
> > 20191217150736.1479-1-u.kleine-koenig@pengutronix.de.
> > 
> > v4 only changes patch 3 dropping a few printks, fixing the show callback
> > for the dev attribute to match its store function. And I moved
> > ledtrig_tty_restart() into the already existing if (tty) {...} block in
> > dev_store and dropped the same check from the former function.
> > 
> 
> All looks good to me, so if the LED developers give their ack, I can
> take it in my tty tree.

Nothing obviously wrong in 1 and 2 of the series. Feel free to take
them.

I fear conflicts if you take 3/ through your tree, so perhaps I can
take it through the LEDs?

But I'll have some comments there.

Best regards,
									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v4 0/3] tty/leds: implement a trigger for ttys
  2019-12-21 18:36   ` Pavel Machek
@ 2019-12-23  9:58     ` Uwe Kleine-König
  0 siblings, 0 replies; 12+ messages in thread
From: Uwe Kleine-König @ 2019-12-23  9:58 UTC (permalink / raw)
  To: Pavel Machek
  Cc: Greg Kroah-Hartman, Jacek Anaszewski, Dan Murphy, Jiri Slaby,
	kernel, linux-kernel, linux-leds, linux-serial

On Sat, Dec 21, 2019 at 07:36:11PM +0100, Pavel Machek wrote:
> On Tue 2019-12-17 18:22:32, Greg Kroah-Hartman wrote:
> > On Tue, Dec 17, 2019 at 05:58:13PM +0100, Uwe Kleine-König wrote:
> > > Hello,
> > > 
> > > v3 of this series was sent earlier today starting with Message-Id:
> > > 20191217150736.1479-1-u.kleine-koenig@pengutronix.de.
> > > 
> > > v4 only changes patch 3 dropping a few printks, fixing the show callback
> > > for the dev attribute to match its store function. And I moved
> > > ledtrig_tty_restart() into the already existing if (tty) {...} block in
> > > dev_store and dropped the same check from the former function.
> > > 
> > 
> > All looks good to me, so if the LED developers give their ack, I can
> > take it in my tty tree.
> 
> Nothing obviously wrong in 1 and 2 of the series. Feel free to take
> them.
> 
> I fear conflicts if you take 3/ through your tree, so perhaps I can
> take it through the LEDs?
> 
> But I'll have some comments there.

If the feedback implies that using statistics is not the right thing,
the tty patches are not needed however, so I suggest to wait until the
discussion is closed before taking them.

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | https://www.pengutronix.de/ |

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2019-12-23  9:59 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-12-17 16:58 [PATCH v4 0/3] tty/leds: implement a trigger for ttys Uwe Kleine-König
2019-12-17 16:58 ` [PATCH v4 1/3] tty: rename tty_kopen() and add new function tty_kopen_shared() Uwe Kleine-König
2019-12-17 18:08   ` Jacek Anaszewski
2019-12-17 18:20     ` Greg Kroah-Hartman
2019-12-17 18:27     ` Uwe Kleine-König
2019-12-17 20:19       ` Jacek Anaszewski
2019-12-17 20:23         ` Greg Kroah-Hartman
2019-12-17 16:58 ` [PATCH v4 2/3] tty: new helper function tty_get_icount() Uwe Kleine-König
2019-12-17 16:58 ` [PATCH v4 3/3] leds: trigger: implement a tty trigger Uwe Kleine-König
2019-12-17 17:22 ` [PATCH v4 0/3] tty/leds: implement a trigger for ttys Greg Kroah-Hartman
2019-12-21 18:36   ` Pavel Machek
2019-12-23  9:58     ` Uwe Kleine-König

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).