[PATCH 0/2] leds: ledtrig-tty: mode xtension

[PATCH 0/2] leds: ledtrig-tty: mode xtension

[Florian Eckert]

The following patch set adds the possibility to change the tty trigger
modes. Until now, the LED is flashed when data is transmitted. With a
legacy serial interface there are additional signal lines.
If this line changes then the LED is turned on/off.

CTS: Clear To Send       -> DCE is ready to accept data from the DTE
DSR: Data Set Ready      -> DCE is ready to receive and send data.
CAR: Data Carrier Detect -> DCE is receiving a carrier from a remote DCE.
RNG: Ring Indicator      -> DCE has detected an incoming ring on the line.

Florian Eckert (2):
  tty: new helper function tty_get_mget
  leds: trigger: ledtrig-tty: add additional modes

 .../ABI/testing/sysfs-class-led-trigger-tty   |  16 ++
 drivers/leds/trigger/ledtrig-tty.c            | 148 ++++++++++++++++--
 drivers/tty/tty_io.c                          |  32 +++-
 include/linux/tty.h                           |   3 +-
 4 files changed, 176 insertions(+), 23 deletions(-)

-- 
2.30.2

[PATCH 1/2] tty: new helper function tty_get_mget

[Florian Eckert]

For a given struct tty_struct, this provides the appropriate tty line
state flags needed to add more modes to the ledtrig-tty trigger.

The new function is then used to get via tty_tiocmget() the different tty
line states.

Signed-off-by: Florian Eckert <fe@dev.tdt.de>
---
 drivers/tty/tty_io.c | 32 +++++++++++++++++++++++++-------
 include/linux/tty.h  |  3 ++-
 2 files changed, 27 insertions(+), 8 deletions(-)

diff --git a/drivers/tty/tty_io.c b/drivers/tty/tty_io.c
index 3149114bf130..e4dc59eda43f 100644
--- a/drivers/tty/tty_io.c
+++ b/drivers/tty/tty_io.c
@@ -2493,26 +2493,44 @@ static int send_break(struct tty_struct *tty, unsigned int duration)
 	return retval;
 }
 
+/*
+ * tty_get_mget		-	get modem status
+ * @tty: tty device
+ *
+ * Obtain the modem status bits from the tty driver if the feature
+ * is supported. Return -ENOTTY if it is not available.
+ *
+ */
+
+int tty_get_mget(struct tty_struct *tty)
+{
+	int retval = -ENOTTY;
+
+	if (tty->ops->tiocmget)
+		retval = tty->ops->tiocmget(tty);
+
+	return retval;
+}
+EXPORT_SYMBOL_GPL(tty_get_mget);
+
 /**
  * tty_tiocmget		-	get modem status
  * @tty: tty device
  * @p: pointer to result
  *
  * Obtain the modem status bits from the tty driver if the feature is
- * supported. Return -%ENOTTY if it is not available.
+ * supported.
  *
  * Locking: none (up to the driver)
  */
 static int tty_tiocmget(struct tty_struct *tty, int __user *p)
 {
-	int retval = -ENOTTY;
+	int retval;
 
-	if (tty->ops->tiocmget) {
-		retval = tty->ops->tiocmget(tty);
+	retval = tty_get_mget(tty);
+	if (retval >= 0)
+		retval = put_user(retval, p);
 
-		if (retval >= 0)
-			retval = put_user(retval, p);
-	}
 	return retval;
 }
 
diff --git a/include/linux/tty.h b/include/linux/tty.h
index 730c3301d710..9c1485c31081 100644
--- a/include/linux/tty.h
+++ b/include/linux/tty.h
@@ -419,8 +419,9 @@ void tty_unthrottle(struct tty_struct *tty);
 int tty_throttle_safe(struct tty_struct *tty);
 int tty_unthrottle_safe(struct tty_struct *tty);
 int tty_do_resize(struct tty_struct *tty, struct winsize *ws);
-int tty_get_icount(struct tty_struct *tty,
+int tty_get_icount(struct tty_struct *tty
 		struct serial_icounter_struct *icount);
+extern int tty_get_mget(struct tty_struct *tty);
 int is_current_pgrp_orphaned(void);
 void tty_hangup(struct tty_struct *tty);
 void tty_vhangup(struct tty_struct *tty);
-- 
2.30.2

[PATCH 2/2] leds: trigger: ledtrig-tty: add additional modes

[Florian Eckert]

Add additional modes to trigger the selected LED.
The following modes are supported:

TD/RD:	Flash LED on data transmission (default)
CTS:	DCE Ready to accept data from the DTE.
DSR:	DCE is ready to receive and send data.
CAR:	DCE is receiving a carrier from a remote DTE.
RNG:	DCE has detected an incoming ring signal.

The mode can be changed for example with the following command:
echo "CTS" /sys/class/leds/<led>/mode

This would turn on the LED, when the DTE(modem) signals the DCE that it
is ready to accept data.

Signed-off-by: Florian Eckert <fe@dev.tdt.de>
---
 .../ABI/testing/sysfs-class-led-trigger-tty   |  16 ++
 drivers/leds/trigger/ledtrig-tty.c            | 148 ++++++++++++++++--
 2 files changed, 149 insertions(+), 15 deletions(-)

diff --git a/Documentation/ABI/testing/sysfs-class-led-trigger-tty b/Documentation/ABI/testing/sysfs-class-led-trigger-tty
index 2bf6b24e781b..bb6333ed7028 100644
--- a/Documentation/ABI/testing/sysfs-class-led-trigger-tty
+++ b/Documentation/ABI/testing/sysfs-class-led-trigger-tty
@@ -4,3 +4,19 @@ KernelVersion:	5.10
 Contact:	linux-leds@vger.kernel.org
 Description:
 		Specifies the tty device name of the triggering tty
+
+What:		/sys/class/leds/<led>/mode
+Date:		January 2023
+KernelVersion:	6.3
+Description:
+		Specifies the operating to trigger the LED.
+		The following operating modes are supported:
+		TD/RD: Flash LED on data transmission (default)
+		CTS:   DCE Ready to accept data from the DTE.
+		       LED on if line is high.
+		DSR:   DCE is ready to receive and send data.
+		       LED on if line is high.
+		CAR:   DCE is receiving a carrier from a remote DTE.
+		       LED on if line is high.
+		RNG:   DCE has detected an incoming ring signal.
+		       LED on if line is high.
diff --git a/drivers/leds/trigger/ledtrig-tty.c b/drivers/leds/trigger/ledtrig-tty.c
index f62db7e520b5..152b3020998e 100644
--- a/drivers/leds/trigger/ledtrig-tty.c
+++ b/drivers/leds/trigger/ledtrig-tty.c
@@ -7,6 +7,14 @@
 #include <linux/tty.h>
 #include <uapi/linux/serial.h>
 
+enum tty_led_mode {
+	TTY_LED_CNT,
+	TTY_LED_CTS,
+	TTY_LED_DSR,
+	TTY_LED_CAR,
+	TTY_LED_RNG,
+};
+
 struct ledtrig_tty_data {
 	struct led_classdev *led_cdev;
 	struct delayed_work dwork;
@@ -14,6 +22,15 @@ struct ledtrig_tty_data {
 	const char *ttyname;
 	struct tty_struct *tty;
 	int rx, tx;
+	unsigned int mode;
+};
+
+static const char * const mode[] = {
+	[TTY_LED_CNT] = "TD/RD", // Trasmit Data / Receive Data
+	[TTY_LED_CTS] = "CTS", // CTS Clear To Send
+	[TTY_LED_DSR] = "DSR", // DSR Data Set Ready
+	[TTY_LED_CAR] = "CAR", // CAR Data Carrier Detect (DCD)
+	[TTY_LED_RNG] = "RNG", // RNG Ring Indicator (RI)
 };
 
 static void ledtrig_tty_restart(struct ledtrig_tty_data *trigger_data)
@@ -21,6 +38,74 @@ static void ledtrig_tty_restart(struct ledtrig_tty_data *trigger_data)
 	schedule_delayed_work(&trigger_data->dwork, 0);
 }
 
+static ssize_t mode_show(struct device *dev,
+			 struct device_attribute *attr, char *buf)
+{
+	struct ledtrig_tty_data *trigger_data = led_trigger_get_drvdata(dev);
+	enum tty_led_mode tty_mode;
+
+	mutex_lock(&trigger_data->mutex);
+	tty_mode = trigger_data->mode;
+	mutex_unlock(&trigger_data->mutex);
+
+	switch (tty_mode) {
+	case TTY_LED_CTS:
+		return sprintf(buf, "%s [%s] %s %s %s\n", mode[TTY_LED_CNT],
+				mode[TTY_LED_CTS], mode[TTY_LED_DSR],
+				mode[TTY_LED_CAR], mode[TTY_LED_RNG]);
+	case TTY_LED_DSR:
+		return sprintf(buf, "%s %s [%s] %s %s\n", mode[TTY_LED_CNT],
+				mode[TTY_LED_CTS], mode[TTY_LED_DSR],
+				mode[TTY_LED_CAR], mode[TTY_LED_RNG]);
+	case TTY_LED_CAR:
+		return sprintf(buf, "%s %s %s [%s] %s\n", mode[TTY_LED_CNT],
+				mode[TTY_LED_CTS], mode[TTY_LED_DSR],
+				mode[TTY_LED_CAR], mode[TTY_LED_RNG]);
+	case TTY_LED_RNG:
+		return sprintf(buf, "%s %s %s %s [%s]\n", mode[TTY_LED_CNT],
+				mode[TTY_LED_CTS], mode[TTY_LED_DSR],
+				mode[TTY_LED_CAR], mode[TTY_LED_RNG]);
+	case TTY_LED_CNT:
+	default:
+		return sprintf(buf, "[%s] %s %s %s %s\n", mode[TTY_LED_CNT],
+				mode[TTY_LED_CTS], mode[TTY_LED_DSR],
+				mode[TTY_LED_CAR], mode[TTY_LED_RNG]);
+	}
+}
+
+static ssize_t mode_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);
+	ssize_t ret = size;
+	enum tty_led_mode tty_mode;
+
+	/* Check for new line in string*/
+	if (size > 0 && buf[size - 1] == '\n')
+		size -= 1;
+
+	if (strncmp(buf, mode[TTY_LED_CTS], size) == 0)
+		tty_mode = TTY_LED_CTS;
+	else if (strncmp(buf, mode[TTY_LED_DSR], size) == 0)
+		tty_mode = TTY_LED_DSR;
+	else if (strncmp(buf, mode[TTY_LED_CAR], size) == 0)
+		tty_mode = TTY_LED_CAR;
+	else if (strncmp(buf, mode[TTY_LED_RNG], size) == 0)
+		tty_mode = TTY_LED_RNG;
+	else if (strncmp(buf, mode[TTY_LED_CNT], size) == 0)
+		tty_mode = TTY_LED_CNT;
+	else
+		return -EINVAL;
+
+	mutex_lock(&trigger_data->mutex);
+	trigger_data->mode = tty_mode;
+	mutex_unlock(&trigger_data->mutex);
+
+	return ret;
+}
+static DEVICE_ATTR_RW(mode);
+
 static ssize_t ttyname_show(struct device *dev,
 			    struct device_attribute *attr, char *buf)
 {
@@ -76,6 +161,18 @@ static ssize_t ttyname_store(struct device *dev,
 }
 static DEVICE_ATTR_RW(ttyname);
 
+static void ledtrig_tty_flags(struct ledtrig_tty_data *trigger_data,
+		unsigned int flag)
+{
+	unsigned int status;
+
+	status = tty_get_mget(trigger_data->tty);
+	if (status & flag)
+		led_set_brightness_sync(trigger_data->led_cdev, LED_ON);
+	else
+		led_set_brightness_sync(trigger_data->led_cdev, LED_OFF);
+}
+
 static void ledtrig_tty_work(struct work_struct *work)
 {
 	struct ledtrig_tty_data *trigger_data =
@@ -113,21 +210,38 @@ static void ledtrig_tty_work(struct work_struct *work)
 		trigger_data->tty = tty;
 	}
 
-	ret = tty_get_icount(trigger_data->tty, &icount);
-	if (ret) {
-		dev_info(trigger_data->tty->dev, "Failed to get icount, stopped polling\n");
-		mutex_unlock(&trigger_data->mutex);
-		return;
-	}
-
-	if (icount.rx != trigger_data->rx ||
-	    icount.tx != trigger_data->tx) {
-		led_set_brightness_sync(trigger_data->led_cdev, LED_ON);
-
-		trigger_data->rx = icount.rx;
-		trigger_data->tx = icount.tx;
-	} else {
-		led_set_brightness_sync(trigger_data->led_cdev, LED_OFF);
+	switch (trigger_data->mode) {
+	case TTY_LED_CTS:
+		ledtrig_tty_flags(trigger_data, TIOCM_CTS);
+		break;
+	case TTY_LED_DSR:
+		ledtrig_tty_flags(trigger_data, TIOCM_DSR);
+		break;
+	case TTY_LED_CAR:
+		ledtrig_tty_flags(trigger_data, TIOCM_CAR);
+		break;
+	case TTY_LED_RNG:
+		ledtrig_tty_flags(trigger_data, TIOCM_RNG);
+		break;
+	case TTY_LED_CNT:
+	default:
+		ret = tty_get_icount(trigger_data->tty, &icount);
+		if (ret) {
+			dev_info(trigger_data->tty->dev, "Failed to get icount, stopped polling\n");
+			mutex_unlock(&trigger_data->mutex);
+			return;
+		}
+
+		if (icount.rx != trigger_data->rx ||
+		    icount.tx != trigger_data->tx) {
+			led_set_brightness_sync(trigger_data->led_cdev, LED_ON);
+
+			trigger_data->rx = icount.rx;
+			trigger_data->tx = icount.tx;
+		} else {
+			led_set_brightness_sync(trigger_data->led_cdev, LED_OFF);
+		}
+		break;
 	}
 
 out:
@@ -137,6 +251,7 @@ static void ledtrig_tty_work(struct work_struct *work)
 
 static struct attribute *ledtrig_tty_attrs[] = {
 	&dev_attr_ttyname.attr,
+	&dev_attr_mode.attr,
 	NULL
 };
 ATTRIBUTE_GROUPS(ledtrig_tty);
@@ -149,6 +264,9 @@ static int ledtrig_tty_activate(struct led_classdev *led_cdev)
 	if (!trigger_data)
 		return -ENOMEM;
 
+	/* set default mode */
+	trigger_data->mode = TTY_LED_CNT;
+
 	led_set_trigger_data(led_cdev, trigger_data);
 
 	INIT_DELAYED_WORK(&trigger_data->dwork, ledtrig_tty_work);
-- 
2.30.2

Re: [PATCH 1/2] tty: new helper function tty_get_mget

[kernel test robot]

Hi Florian,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on tty/tty-testing]
[also build test WARNING on tty/tty-next tty/tty-linus pavel-leds/for-next staging/staging-testing staging/staging-next staging/staging-linus linus/master v6.2-rc8 next-20230213]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Florian-Eckert/tty-new-helper-function-tty_get_mget/20230213-223413
base:   https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/tty.git tty-testing
patch link:    https://lore.kernel.org/r/20230213140638.620206-2-fe%40dev.tdt.de
patch subject: [PATCH 1/2] tty: new helper function tty_get_mget
config: x86_64-randconfig-a002-20230213 (https://download.01.org/0day-ci/archive/20230214/202302140339.1vFSjZ1M-lkp@intel.com/config)
compiler: clang version 14.0.6 (https://github.com/llvm/llvm-project f28c006a5895fc0e329fe15fead81e37457cb1d1)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/intel-lab-lkp/linux/commit/096ca9c9428ec2a8ebab4af0798461e66585cdde
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Florian-Eckert/tty-new-helper-function-tty_get_mget/20230213-223413
        git checkout 096ca9c9428ec2a8ebab4af0798461e66585cdde
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=x86_64 olddefconfig
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=x86_64 SHELL=/bin/bash drivers/misc/bcm-vk/ drivers/tty/ fs/proc/ init/

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>
| Link: https://lore.kernel.org/oe-kbuild-all/202302140339.1vFSjZ1M-lkp@intel.com/

All warnings (new ones prefixed by >>):

   In file included from drivers/tty/tty_port.c:8:
   include/linux/tty.h:423:3: error: expected ')'
                   struct serial_icounter_struct *icount);
                   ^
   include/linux/tty.h:422:19: note: to match this '('
   int tty_get_icount(struct tty_struct *tty
                     ^
>> drivers/tty/tty_port.c:266:2: warning: implicit conversion from 'unsigned long' to 'unsigned int' changes value from 18446744073709551615 to 4294967295 [-Wconstant-conversion]
           INIT_KFIFO(port->xmit_fifo);
           ^~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/kfifo.h:130:69: note: expanded from macro 'INIT_KFIFO'
           __kfifo->mask = __is_kfifo_ptr(__tmp) ? 0 : ARRAY_SIZE(__tmp->buf) - 1;\
                         ~                             ~~~~~~~~~~~~~~~~~~~~~~~^~~
   1 warning and 1 error generated.
--
   In file included from fs/proc/array.c:62:
   include/linux/tty.h:423:3: error: expected ')'
                   struct serial_icounter_struct *icount);
                   ^
   include/linux/tty.h:422:19: note: to match this '('
   int tty_get_icount(struct tty_struct *tty
                     ^
   In file included from fs/proc/array.c:64:
>> include/linux/mman.h:154:9: warning: division by zero is undefined [-Wdivision-by-zero]
                  _calc_vm_trans(flags, MAP_SYNC,       VM_SYNC      ) |
                  ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/mman.h:132:21: note: expanded from macro '_calc_vm_trans'
      : ((x) & (bit1)) / ((bit1) / (bit2))))
                       ^ ~~~~~~~~~~~~~~~~~
   1 warning and 1 error generated.
--
   In file included from drivers/misc/bcm-vk/bcm_vk_dev.c:20:
   In file included from drivers/misc/bcm-vk/bcm_vk.h:18:
   include/linux/tty.h:423:3: error: expected ')'
                   struct serial_icounter_struct *icount);
                   ^
   include/linux/tty.h:422:19: note: to match this '('
   int tty_get_icount(struct tty_struct *tty
                     ^
>> drivers/misc/bcm-vk/bcm_vk_dev.c:1320:6: warning: shift count >= width of type [-Wshift-count-overflow]
                                           DMA_BIT_MASK(BCM_VK_DMA_BITS));
                                           ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/dma-mapping.h:76:54: note: expanded from macro 'DMA_BIT_MASK'
   #define DMA_BIT_MASK(n) (((n) == 64) ? ~0ULL : ((1ULL<<(n))-1))
                                                        ^ ~~~
   1 warning and 1 error generated.


vim +266 drivers/tty/tty_port.c

9e48565d217a8a drivers/char/tty_port.c Alan Cox   2008-10-13  260  
9e48565d217a8a drivers/char/tty_port.c Alan Cox   2008-10-13  261  void tty_port_free_xmit_buf(struct tty_port *port)
9e48565d217a8a drivers/char/tty_port.c Alan Cox   2008-10-13  262  {
44e4909e453eaa drivers/char/tty_port.c Alan Cox   2009-11-30  263  	mutex_lock(&port->buf_mutex);
9e48565d217a8a drivers/char/tty_port.c Alan Cox   2008-10-13  264  	free_page((unsigned long)port->xmit_buf);
9e48565d217a8a drivers/char/tty_port.c Alan Cox   2008-10-13  265  	port->xmit_buf = NULL;
4e2a44c1408b6a drivers/tty/tty_port.c  Jiri Slaby 2022-01-24 @266  	INIT_KFIFO(port->xmit_fifo);
44e4909e453eaa drivers/char/tty_port.c Alan Cox   2009-11-30  267  	mutex_unlock(&port->buf_mutex);
9e48565d217a8a drivers/char/tty_port.c Alan Cox   2008-10-13  268  }
9e48565d217a8a drivers/char/tty_port.c Alan Cox   2008-10-13  269  EXPORT_SYMBOL(tty_port_free_xmit_buf);
9e48565d217a8a drivers/char/tty_port.c Alan Cox   2008-10-13  270  

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests

Re: [PATCH 1/2] tty: new helper function tty_get_mget

[kernel test robot]

Hi Florian,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on tty/tty-testing]
[also build test WARNING on tty/tty-next tty/tty-linus pavel-leds/for-next staging/staging-testing staging/staging-next staging/staging-linus linus/master v6.2-rc8 next-20230213]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Florian-Eckert/tty-new-helper-function-tty_get_mget/20230213-223413
base:   https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/tty.git tty-testing
patch link:    https://lore.kernel.org/r/20230213140638.620206-2-fe%40dev.tdt.de
patch subject: [PATCH 1/2] tty: new helper function tty_get_mget
config: m68k-allyesconfig (https://download.01.org/0day-ci/archive/20230214/202302140320.YmmWP6V8-lkp@intel.com/config)
compiler: m68k-linux-gcc (GCC) 12.1.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/intel-lab-lkp/linux/commit/096ca9c9428ec2a8ebab4af0798461e66585cdde
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Florian-Eckert/tty-new-helper-function-tty_get_mget/20230213-223413
        git checkout 096ca9c9428ec2a8ebab4af0798461e66585cdde
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=m68k olddefconfig
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=m68k SHELL=/bin/bash drivers/leds/trigger/ drivers/tty/

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>
| Link: https://lore.kernel.org/oe-kbuild-all/202302140320.YmmWP6V8-lkp@intel.com/

All warnings (new ones prefixed by >>):

   In file included from drivers/tty/tty_io.c:76:
   include/linux/tty.h:423:17: error: expected ';', ',' or ')' before 'struct'
     423 |                 struct serial_icounter_struct *icount);
         |                 ^~~~~~
>> drivers/tty/tty_io.c:2589:5: warning: no previous prototype for 'tty_get_icount' [-Wmissing-prototypes]
    2589 | int tty_get_icount(struct tty_struct *tty,
         |     ^~~~~~~~~~~~~~


vim +/tty_get_icount +2589 drivers/tty/tty_io.c

^1da177e4c3f41 drivers/char/tty_io.c Linus Torvalds   2005-04-16  2579  
d20c219c731784 drivers/tty/tty_io.c  Uwe Kleine-König 2020-12-18  2580  /**
d20c219c731784 drivers/tty/tty_io.c  Uwe Kleine-König 2020-12-18  2581   * tty_get_icount	-	get tty statistics
d20c219c731784 drivers/tty/tty_io.c  Uwe Kleine-König 2020-12-18  2582   * @tty: tty device
d20c219c731784 drivers/tty/tty_io.c  Uwe Kleine-König 2020-12-18  2583   * @icount: output parameter
d20c219c731784 drivers/tty/tty_io.c  Uwe Kleine-König 2020-12-18  2584   *
796a75a98762f1 drivers/tty/tty_io.c  Jiri Slaby       2021-11-26  2585   * Gets a copy of the @tty's icount statistics.
d20c219c731784 drivers/tty/tty_io.c  Uwe Kleine-König 2020-12-18  2586   *
d20c219c731784 drivers/tty/tty_io.c  Uwe Kleine-König 2020-12-18  2587   * Locking: none (up to the driver)
d20c219c731784 drivers/tty/tty_io.c  Uwe Kleine-König 2020-12-18  2588   */
d20c219c731784 drivers/tty/tty_io.c  Uwe Kleine-König 2020-12-18 @2589  int tty_get_icount(struct tty_struct *tty,
d20c219c731784 drivers/tty/tty_io.c  Uwe Kleine-König 2020-12-18  2590  		   struct serial_icounter_struct *icount)
d20c219c731784 drivers/tty/tty_io.c  Uwe Kleine-König 2020-12-18  2591  {
d20c219c731784 drivers/tty/tty_io.c  Uwe Kleine-König 2020-12-18  2592  	memset(icount, 0, sizeof(*icount));
d20c219c731784 drivers/tty/tty_io.c  Uwe Kleine-König 2020-12-18  2593  
d20c219c731784 drivers/tty/tty_io.c  Uwe Kleine-König 2020-12-18  2594  	if (tty->ops->get_icount)
d20c219c731784 drivers/tty/tty_io.c  Uwe Kleine-König 2020-12-18  2595  		return tty->ops->get_icount(tty, icount);
d20c219c731784 drivers/tty/tty_io.c  Uwe Kleine-König 2020-12-18  2596  	else
1b8b20868a6d64 drivers/tty/tty_io.c  Johan Hovold     2021-04-07  2597  		return -ENOTTY;
d20c219c731784 drivers/tty/tty_io.c  Uwe Kleine-König 2020-12-18  2598  }
d20c219c731784 drivers/tty/tty_io.c  Uwe Kleine-König 2020-12-18  2599  EXPORT_SYMBOL_GPL(tty_get_icount);
d20c219c731784 drivers/tty/tty_io.c  Uwe Kleine-König 2020-12-18  2600  

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests

Re: [PATCH 1/2] tty: new helper function tty_get_mget

[kernel test robot]

Hi Florian,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on tty/tty-testing]
[also build test ERROR on tty/tty-next tty/tty-linus staging/staging-testing staging/staging-next staging/staging-linus linus/master v6.2-rc8 next-20230213]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Florian-Eckert/tty-new-helper-function-tty_get_mget/20230213-223413
base:   https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/tty.git tty-testing
patch link:    https://lore.kernel.org/r/20230213140638.620206-2-fe%40dev.tdt.de
patch subject: [PATCH 1/2] tty: new helper function tty_get_mget
config: riscv-buildonly-randconfig-r005-20230212 (https://download.01.org/0day-ci/archive/20230214/202302140516.F9Riaass-lkp@intel.com/config)
compiler: clang version 17.0.0 (https://github.com/llvm/llvm-project db0e6591612b53910a1b366863348bdb9d7d2fb1)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # install riscv cross compiling tool for clang build
        # apt-get install binutils-riscv64-linux-gnu
        # https://github.com/intel-lab-lkp/linux/commit/096ca9c9428ec2a8ebab4af0798461e66585cdde
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Florian-Eckert/tty-new-helper-function-tty_get_mget/20230213-223413
        git checkout 096ca9c9428ec2a8ebab4af0798461e66585cdde
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=riscv olddefconfig
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=riscv SHELL=/bin/bash drivers/misc/bcm-vk/ drivers/tty/

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>
| Link: https://lore.kernel.org/oe-kbuild-all/202302140516.F9Riaass-lkp@intel.com/

All error/warnings (new ones prefixed by >>):

   In file included from drivers/tty/tty_io.c:76:
   include/linux/tty.h:423:3: error: expected ')'
                   struct serial_icounter_struct *icount);
                   ^
   include/linux/tty.h:422:19: note: to match this '('
   int tty_get_icount(struct tty_struct *tty
                     ^
>> drivers/tty/tty_io.c:2589:5: error: conflicting types for 'tty_get_icount'
   int tty_get_icount(struct tty_struct *tty,
       ^
   include/linux/tty.h:422:5: note: previous declaration is here
   int tty_get_icount(struct tty_struct *tty
       ^
>> drivers/tty/tty_io.c:2606:31: error: too many arguments to function call, expected 1, have 2
           retval = tty_get_icount(tty, &icount);
                    ~~~~~~~~~~~~~~      ^~~~~~~
   drivers/tty/tty_io.c:2599:19: note: 'tty_get_icount' declared here
   EXPORT_SYMBOL_GPL(tty_get_icount);
                     ^
   3 errors generated.
--
   In file included from drivers/tty/tty_port.c:8:
   include/linux/tty.h:423:3: error: expected ')'
                   struct serial_icounter_struct *icount);
                   ^
   include/linux/tty.h:422:19: note: to match this '('
   int tty_get_icount(struct tty_struct *tty
                     ^
>> drivers/tty/tty_port.c:266:2: warning: implicit conversion from 'unsigned long' to 'unsigned int' changes value from 18446744073709551615 to 4294967295 [-Wconstant-conversion]
           INIT_KFIFO(port->xmit_fifo);
           ^~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/kfifo.h:130:69: note: expanded from macro 'INIT_KFIFO'
           __kfifo->mask = __is_kfifo_ptr(__tmp) ? 0 : ARRAY_SIZE(__tmp->buf) - 1;\
                         ~                             ~~~~~~~~~~~~~~~~~~~~~~~^~~
   1 warning and 1 error generated.
--
   In file included from drivers/misc/bcm-vk/bcm_vk_dev.c:20:
   In file included from drivers/misc/bcm-vk/bcm_vk.h:18:
   include/linux/tty.h:423:3: error: expected ')'
                   struct serial_icounter_struct *icount);
                   ^
   include/linux/tty.h:422:19: note: to match this '('
   int tty_get_icount(struct tty_struct *tty
                     ^
>> drivers/misc/bcm-vk/bcm_vk_dev.c:1320:6: warning: shift count >= width of type [-Wshift-count-overflow]
                                           DMA_BIT_MASK(BCM_VK_DMA_BITS));
                                           ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/dma-mapping.h:76:54: note: expanded from macro 'DMA_BIT_MASK'
   #define DMA_BIT_MASK(n) (((n) == 64) ? ~0ULL : ((1ULL<<(n))-1))
                                                        ^ ~~~
   1 warning and 1 error generated.


vim +/tty_get_icount +2589 drivers/tty/tty_io.c

^1da177e4c3f41 drivers/char/tty_io.c Linus Torvalds   2005-04-16  2579  
d20c219c731784 drivers/tty/tty_io.c  Uwe Kleine-König 2020-12-18  2580  /**
d20c219c731784 drivers/tty/tty_io.c  Uwe Kleine-König 2020-12-18  2581   * tty_get_icount	-	get tty statistics
d20c219c731784 drivers/tty/tty_io.c  Uwe Kleine-König 2020-12-18  2582   * @tty: tty device
d20c219c731784 drivers/tty/tty_io.c  Uwe Kleine-König 2020-12-18  2583   * @icount: output parameter
d20c219c731784 drivers/tty/tty_io.c  Uwe Kleine-König 2020-12-18  2584   *
796a75a98762f1 drivers/tty/tty_io.c  Jiri Slaby       2021-11-26  2585   * Gets a copy of the @tty's icount statistics.
d20c219c731784 drivers/tty/tty_io.c  Uwe Kleine-König 2020-12-18  2586   *
d20c219c731784 drivers/tty/tty_io.c  Uwe Kleine-König 2020-12-18  2587   * Locking: none (up to the driver)
d20c219c731784 drivers/tty/tty_io.c  Uwe Kleine-König 2020-12-18  2588   */
d20c219c731784 drivers/tty/tty_io.c  Uwe Kleine-König 2020-12-18 @2589  int tty_get_icount(struct tty_struct *tty,
d20c219c731784 drivers/tty/tty_io.c  Uwe Kleine-König 2020-12-18  2590  		   struct serial_icounter_struct *icount)
d20c219c731784 drivers/tty/tty_io.c  Uwe Kleine-König 2020-12-18  2591  {
d20c219c731784 drivers/tty/tty_io.c  Uwe Kleine-König 2020-12-18  2592  	memset(icount, 0, sizeof(*icount));
d20c219c731784 drivers/tty/tty_io.c  Uwe Kleine-König 2020-12-18  2593  
d20c219c731784 drivers/tty/tty_io.c  Uwe Kleine-König 2020-12-18  2594  	if (tty->ops->get_icount)
d20c219c731784 drivers/tty/tty_io.c  Uwe Kleine-König 2020-12-18  2595  		return tty->ops->get_icount(tty, icount);
d20c219c731784 drivers/tty/tty_io.c  Uwe Kleine-König 2020-12-18  2596  	else
1b8b20868a6d64 drivers/tty/tty_io.c  Johan Hovold     2021-04-07  2597  		return -ENOTTY;
d20c219c731784 drivers/tty/tty_io.c  Uwe Kleine-König 2020-12-18  2598  }
d20c219c731784 drivers/tty/tty_io.c  Uwe Kleine-König 2020-12-18  2599  EXPORT_SYMBOL_GPL(tty_get_icount);
d20c219c731784 drivers/tty/tty_io.c  Uwe Kleine-König 2020-12-18  2600  
d281da7ff6f70e drivers/char/tty_io.c Alan Cox         2010-09-16  2601  static int tty_tiocgicount(struct tty_struct *tty, void __user *arg)
d281da7ff6f70e drivers/char/tty_io.c Alan Cox         2010-09-16  2602  {
d281da7ff6f70e drivers/char/tty_io.c Alan Cox         2010-09-16  2603  	struct serial_icounter_struct icount;
d20c219c731784 drivers/tty/tty_io.c  Uwe Kleine-König 2020-12-18  2604  	int retval;
d20c219c731784 drivers/tty/tty_io.c  Uwe Kleine-König 2020-12-18  2605  
d20c219c731784 drivers/tty/tty_io.c  Uwe Kleine-König 2020-12-18 @2606  	retval = tty_get_icount(tty, &icount);
d281da7ff6f70e drivers/char/tty_io.c Alan Cox         2010-09-16  2607  	if (retval != 0)
d281da7ff6f70e drivers/char/tty_io.c Alan Cox         2010-09-16  2608  		return retval;
d20c219c731784 drivers/tty/tty_io.c  Uwe Kleine-König 2020-12-18  2609  
d281da7ff6f70e drivers/char/tty_io.c Alan Cox         2010-09-16  2610  	if (copy_to_user(arg, &icount, sizeof(icount)))
d281da7ff6f70e drivers/char/tty_io.c Alan Cox         2010-09-16  2611  		return -EFAULT;
d281da7ff6f70e drivers/char/tty_io.c Alan Cox         2010-09-16  2612  	return 0;
d281da7ff6f70e drivers/char/tty_io.c Alan Cox         2010-09-16  2613  }
d281da7ff6f70e drivers/char/tty_io.c Alan Cox         2010-09-16  2614  

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests

Re: [PATCH 1/2] tty: new helper function tty_get_mget

[kernel test robot]

Hi Florian,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on tty/tty-testing]
[also build test ERROR on tty/tty-next tty/tty-linus pavel-leds/for-next staging/staging-testing staging/staging-next staging/staging-linus linus/master v6.2-rc8 next-20230213]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Florian-Eckert/tty-new-helper-function-tty_get_mget/20230213-223413
base:   https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/tty.git tty-testing
patch link:    https://lore.kernel.org/r/20230213140638.620206-2-fe%40dev.tdt.de
patch subject: [PATCH 1/2] tty: new helper function tty_get_mget
config: x86_64-randconfig-a005-20230213 (https://download.01.org/0day-ci/archive/20230214/202302140536.fks6kujh-lkp@intel.com/config)
compiler: clang version 14.0.6 (https://github.com/llvm/llvm-project f28c006a5895fc0e329fe15fead81e37457cb1d1)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/intel-lab-lkp/linux/commit/096ca9c9428ec2a8ebab4af0798461e66585cdde
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Florian-Eckert/tty-new-helper-function-tty_get_mget/20230213-223413
        git checkout 096ca9c9428ec2a8ebab4af0798461e66585cdde
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=x86_64 olddefconfig
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=x86_64 SHELL=/bin/bash

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>
| Link: https://lore.kernel.org/oe-kbuild-all/202302140536.fks6kujh-lkp@intel.com/

All errors (new ones prefixed by >>):

   In file included from drivers/leds/trigger/ledtrig-tty.c:7:
   include/linux/tty.h:423:3: error: expected ')'
                   struct serial_icounter_struct *icount);
                   ^
   include/linux/tty.h:422:19: note: to match this '('
   int tty_get_icount(struct tty_struct *tty
                     ^
>> drivers/leds/trigger/ledtrig-tty.c:116:42: error: too many arguments to function call, expected single argument 'tty', have 2 arguments
           ret = tty_get_icount(trigger_data->tty, &icount);
                 ~~~~~~~~~~~~~~                    ^~~~~~~
   include/linux/tty.h:422:5: note: 'tty_get_icount' declared here
   int tty_get_icount(struct tty_struct *tty
       ^
   2 errors generated.


vim +/tty +116 drivers/leds/trigger/ledtrig-tty.c

fd4a641ac88fbb Uwe Kleine-König 2021-01-13   78  
fd4a641ac88fbb Uwe Kleine-König 2021-01-13   79  static void ledtrig_tty_work(struct work_struct *work)
fd4a641ac88fbb Uwe Kleine-König 2021-01-13   80  {
fd4a641ac88fbb Uwe Kleine-König 2021-01-13   81  	struct ledtrig_tty_data *trigger_data =
fd4a641ac88fbb Uwe Kleine-König 2021-01-13   82  		container_of(work, struct ledtrig_tty_data, dwork.work);
fd4a641ac88fbb Uwe Kleine-König 2021-01-13   83  	struct serial_icounter_struct icount;
fd4a641ac88fbb Uwe Kleine-König 2021-01-13   84  	int ret;
fd4a641ac88fbb Uwe Kleine-König 2021-01-13   85  
fd4a641ac88fbb Uwe Kleine-König 2021-01-13   86  	mutex_lock(&trigger_data->mutex);
fd4a641ac88fbb Uwe Kleine-König 2021-01-13   87  
fd4a641ac88fbb Uwe Kleine-König 2021-01-13   88  	if (!trigger_data->ttyname) {
fd4a641ac88fbb Uwe Kleine-König 2021-01-13   89  		/* exit without rescheduling */
fd4a641ac88fbb Uwe Kleine-König 2021-01-13   90  		mutex_unlock(&trigger_data->mutex);
fd4a641ac88fbb Uwe Kleine-König 2021-01-13   91  		return;
fd4a641ac88fbb Uwe Kleine-König 2021-01-13   92  	}
fd4a641ac88fbb Uwe Kleine-König 2021-01-13   93  
fd4a641ac88fbb Uwe Kleine-König 2021-01-13   94  	/* try to get the tty corresponding to $ttyname */
fd4a641ac88fbb Uwe Kleine-König 2021-01-13   95  	if (!trigger_data->tty) {
fd4a641ac88fbb Uwe Kleine-König 2021-01-13   96  		dev_t devno;
fd4a641ac88fbb Uwe Kleine-König 2021-01-13   97  		struct tty_struct *tty;
fd4a641ac88fbb Uwe Kleine-König 2021-01-13   98  		int ret;
fd4a641ac88fbb Uwe Kleine-König 2021-01-13   99  
fd4a641ac88fbb Uwe Kleine-König 2021-01-13  100  		ret = tty_dev_name_to_number(trigger_data->ttyname, &devno);
fd4a641ac88fbb Uwe Kleine-König 2021-01-13  101  		if (ret < 0)
fd4a641ac88fbb Uwe Kleine-König 2021-01-13  102  			/*
fd4a641ac88fbb Uwe Kleine-König 2021-01-13  103  			 * A device with this name might appear later, so keep
fd4a641ac88fbb Uwe Kleine-König 2021-01-13  104  			 * retrying.
fd4a641ac88fbb Uwe Kleine-König 2021-01-13  105  			 */
fd4a641ac88fbb Uwe Kleine-König 2021-01-13  106  			goto out;
fd4a641ac88fbb Uwe Kleine-König 2021-01-13  107  
fd4a641ac88fbb Uwe Kleine-König 2021-01-13  108  		tty = tty_kopen_shared(devno);
fd4a641ac88fbb Uwe Kleine-König 2021-01-13  109  		if (IS_ERR(tty) || !tty)
fd4a641ac88fbb Uwe Kleine-König 2021-01-13  110  			/* What to do? retry or abort */
fd4a641ac88fbb Uwe Kleine-König 2021-01-13  111  			goto out;
fd4a641ac88fbb Uwe Kleine-König 2021-01-13  112  
fd4a641ac88fbb Uwe Kleine-König 2021-01-13  113  		trigger_data->tty = tty;
fd4a641ac88fbb Uwe Kleine-König 2021-01-13  114  	}
fd4a641ac88fbb Uwe Kleine-König 2021-01-13  115  
fd4a641ac88fbb Uwe Kleine-König 2021-01-13 @116  	ret = tty_get_icount(trigger_data->tty, &icount);
fd4a641ac88fbb Uwe Kleine-König 2021-01-13  117  	if (ret) {
fd4a641ac88fbb Uwe Kleine-König 2021-01-13  118  		dev_info(trigger_data->tty->dev, "Failed to get icount, stopped polling\n");
fd4a641ac88fbb Uwe Kleine-König 2021-01-13  119  		mutex_unlock(&trigger_data->mutex);
fd4a641ac88fbb Uwe Kleine-König 2021-01-13  120  		return;
fd4a641ac88fbb Uwe Kleine-König 2021-01-13  121  	}
fd4a641ac88fbb Uwe Kleine-König 2021-01-13  122  
fd4a641ac88fbb Uwe Kleine-König 2021-01-13  123  	if (icount.rx != trigger_data->rx ||
fd4a641ac88fbb Uwe Kleine-König 2021-01-13  124  	    icount.tx != trigger_data->tx) {
ba8a86e4dadb33 Uwe Kleine-König 2021-02-19  125  		led_set_brightness_sync(trigger_data->led_cdev, LED_ON);
fd4a641ac88fbb Uwe Kleine-König 2021-01-13  126  
fd4a641ac88fbb Uwe Kleine-König 2021-01-13  127  		trigger_data->rx = icount.rx;
fd4a641ac88fbb Uwe Kleine-König 2021-01-13  128  		trigger_data->tx = icount.tx;
fd4a641ac88fbb Uwe Kleine-König 2021-01-13  129  	} else {
ba8a86e4dadb33 Uwe Kleine-König 2021-02-19  130  		led_set_brightness_sync(trigger_data->led_cdev, LED_OFF);
fd4a641ac88fbb Uwe Kleine-König 2021-01-13  131  	}
fd4a641ac88fbb Uwe Kleine-König 2021-01-13  132  
fd4a641ac88fbb Uwe Kleine-König 2021-01-13  133  out:
fd4a641ac88fbb Uwe Kleine-König 2021-01-13  134  	mutex_unlock(&trigger_data->mutex);
fd4a641ac88fbb Uwe Kleine-König 2021-01-13  135  	schedule_delayed_work(&trigger_data->dwork, msecs_to_jiffies(100));
fd4a641ac88fbb Uwe Kleine-König 2021-01-13  136  }
fd4a641ac88fbb Uwe Kleine-König 2021-01-13  137  

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests

Re: [PATCH 1/2] tty: new helper function tty_get_mget

[Jiri Slaby]

On 13. 02. 23, 15:06, Florian Eckert wrote:
> For a given struct tty_struct, this provides the appropriate tty line
> state flags needed to add more modes to the ledtrig-tty trigger.
> 
> The new function is then used to get via tty_tiocmget() the different tty
> line states.
> 
> Signed-off-by: Florian Eckert <fe@dev.tdt.de>
> ---
>   drivers/tty/tty_io.c | 32 +++++++++++++++++++++++++-------
>   include/linux/tty.h  |  3 ++-
>   2 files changed, 27 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/tty/tty_io.c b/drivers/tty/tty_io.c
> index 3149114bf130..e4dc59eda43f 100644
> --- a/drivers/tty/tty_io.c
> +++ b/drivers/tty/tty_io.c
> @@ -2493,26 +2493,44 @@ static int send_break(struct tty_struct *tty, unsigned int duration)
>   	return retval;
>   }
>   
> +/*
> + * tty_get_mget		-	get modem status
> + * @tty: tty device
> + *
> + * Obtain the modem status bits from the tty driver if the feature
> + * is supported. Return -ENOTTY if it is not available.
> + *

Superfluous empty line.

> + */
> +

This empty line is even wrong wrt kernel-doc!

> +int tty_get_mget(struct tty_struct *tty)
> +{
> +	int retval = -ENOTTY;
> +
> +	if (tty->ops->tiocmget)
> +		retval = tty->ops->tiocmget(tty);
> +
> +	return retval;
> +}
...
> --- a/include/linux/tty.h
> +++ b/include/linux/tty.h
> @@ -419,8 +419,9 @@ void tty_unthrottle(struct tty_struct *tty);
>   int tty_throttle_safe(struct tty_struct *tty);
>   int tty_unthrottle_safe(struct tty_struct *tty);
>   int tty_do_resize(struct tty_struct *tty, struct winsize *ws);
> -int tty_get_icount(struct tty_struct *tty,
> +int tty_get_icount(struct tty_struct *tty

The robot already complained about this ^^^.

Have you even tried to compile-test this?

>   		struct serial_icounter_struct *icount);
> +extern int tty_get_mget(struct tty_struct *tty);

No need for extern. Didn't you notice the surrounding declarations?

>   int is_current_pgrp_orphaned(void);
>   void tty_hangup(struct tty_struct *tty);
>   void tty_vhangup(struct tty_struct *tty);

thanks,
-- 
js
suse labs

Re: [PATCH 2/2] leds: trigger: ledtrig-tty: add additional modes

[Jiri Slaby]

On 13. 02. 23, 15:06, Florian Eckert wrote:
> Add additional modes to trigger the selected LED.
> The following modes are supported:
> 
> TD/RD:	Flash LED on data transmission (default)
> CTS:	DCE Ready to accept data from the DTE.
> DSR:	DCE is ready to receive and send data.
> CAR:	DCE is receiving a carrier from a remote DTE.
> RNG:	DCE has detected an incoming ring signal.
> 
> The mode can be changed for example with the following command:
> echo "CTS" /sys/class/leds/<led>/mode

This will emit only:
CTS /sys/class/leds/<led>/mode

> This would turn on the LED, when the DTE(modem) signals the DCE that it
> is ready to accept data.
> 
> Signed-off-by: Florian Eckert <fe@dev.tdt.de>
...
> --- a/drivers/leds/trigger/ledtrig-tty.c
> +++ b/drivers/leds/trigger/ledtrig-tty.c
> @@ -7,6 +7,14 @@
>   #include <linux/tty.h>
>   #include <uapi/linux/serial.h>
>   
> +enum tty_led_mode {
> +	TTY_LED_CNT,
> +	TTY_LED_CTS,
> +	TTY_LED_DSR,
> +	TTY_LED_CAR,
> +	TTY_LED_RNG,
> +};
> +
>   struct ledtrig_tty_data {
>   	struct led_classdev *led_cdev;
>   	struct delayed_work dwork;
> @@ -14,6 +22,15 @@ struct ledtrig_tty_data {
>   	const char *ttyname;
>   	struct tty_struct *tty;
>   	int rx, tx;
> +	unsigned int mode;

Why not the enum then?

> +};
> +
> +static const char * const mode[] = {

This is not a wise name.

> +	[TTY_LED_CNT] = "TD/RD", // Trasmit Data / Receive Data
> +	[TTY_LED_CTS] = "CTS", // CTS Clear To Send
> +	[TTY_LED_DSR] = "DSR", // DSR Data Set Ready
> +	[TTY_LED_CAR] = "CAR", // CAR Data Carrier Detect (DCD)
> +	[TTY_LED_RNG] = "RNG", // RNG Ring Indicator (RI)
>   };
>   
>   static void ledtrig_tty_restart(struct ledtrig_tty_data *trigger_data)
> @@ -21,6 +38,74 @@ static void ledtrig_tty_restart(struct ledtrig_tty_data *trigger_data)
>   	schedule_delayed_work(&trigger_data->dwork, 0);
>   }
>   
> +static ssize_t mode_show(struct device *dev,
> +			 struct device_attribute *attr, char *buf)
> +{
> +	struct ledtrig_tty_data *trigger_data = led_trigger_get_drvdata(dev);
> +	enum tty_led_mode tty_mode;
> +
> +	mutex_lock(&trigger_data->mutex);
> +	tty_mode = trigger_data->mode;
> +	mutex_unlock(&trigger_data->mutex);
> +
> +	switch (tty_mode) {
> +	case TTY_LED_CTS:
> +		return sprintf(buf, "%s [%s] %s %s %s\n", mode[TTY_LED_CNT],
> +				mode[TTY_LED_CTS], mode[TTY_LED_DSR],
> +				mode[TTY_LED_CAR], mode[TTY_LED_RNG]);
> +	case TTY_LED_DSR:
> +		return sprintf(buf, "%s %s [%s] %s %s\n", mode[TTY_LED_CNT],
> +				mode[TTY_LED_CTS], mode[TTY_LED_DSR],
> +				mode[TTY_LED_CAR], mode[TTY_LED_RNG]);
> +	case TTY_LED_CAR:
> +		return sprintf(buf, "%s %s %s [%s] %s\n", mode[TTY_LED_CNT],
> +				mode[TTY_LED_CTS], mode[TTY_LED_DSR],
> +				mode[TTY_LED_CAR], mode[TTY_LED_RNG]);
> +	case TTY_LED_RNG:
> +		return sprintf(buf, "%s %s %s %s [%s]\n", mode[TTY_LED_CNT],
> +				mode[TTY_LED_CTS], mode[TTY_LED_DSR],
> +				mode[TTY_LED_CAR], mode[TTY_LED_RNG]);
> +	case TTY_LED_CNT:
> +	default:
> +		return sprintf(buf, "[%s] %s %s %s %s\n", mode[TTY_LED_CNT],
> +				mode[TTY_LED_CTS], mode[TTY_LED_DSR],
> +				mode[TTY_LED_CAR], mode[TTY_LED_RNG]);

Can't we do the above in a loop easier?

> +static ssize_t mode_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);
> +	ssize_t ret = size;
> +	enum tty_led_mode tty_mode;
> +
> +	/* Check for new line in string*/
> +	if (size > 0 && buf[size - 1] == '\n')
> +		size -= 1;
> +
> +	if (strncmp(buf, mode[TTY_LED_CTS], size) == 0)
> +		tty_mode = TTY_LED_CTS;
> +	else if (strncmp(buf, mode[TTY_LED_DSR], size) == 0)
> +		tty_mode = TTY_LED_DSR;
> +	else if (strncmp(buf, mode[TTY_LED_CAR], size) == 0)
> +		tty_mode = TTY_LED_CAR;
> +	else if (strncmp(buf, mode[TTY_LED_RNG], size) == 0)
> +		tty_mode = TTY_LED_RNG;
> +	else if (strncmp(buf, mode[TTY_LED_CNT], size) == 0)
> +		tty_mode = TTY_LED_CNT;
> +	else
> +		return -EINVAL;

Again, a loop?

> +
> +	mutex_lock(&trigger_data->mutex);
> +	trigger_data->mode = tty_mode;
> +	mutex_unlock(&trigger_data->mutex);
> +
> +	return ret;
> +}
> +static DEVICE_ATTR_RW(mode);
> +
>   static ssize_t ttyname_show(struct device *dev,
>   			    struct device_attribute *attr, char *buf)
>   {
> @@ -76,6 +161,18 @@ static ssize_t ttyname_store(struct device *dev,
>   }
>   static DEVICE_ATTR_RW(ttyname);
>   
> +static void ledtrig_tty_flags(struct ledtrig_tty_data *trigger_data,
> +		unsigned int flag)
> +{
> +	unsigned int status;
> +
> +	status = tty_get_mget(trigger_data->tty);

So what about negative values = errors?

> +	if (status & flag)

They really might hit here.

> +		led_set_brightness_sync(trigger_data->led_cdev, LED_ON);
> +	else
> +		led_set_brightness_sync(trigger_data->led_cdev, LED_OFF);
> +}
> +


thanks,
-- 
js
suse labs

Re: [PATCH 1/2] tty: new helper function tty_get_mget

[Florian Eckert]

>> --- a/include/linux/tty.h
>> +++ b/include/linux/tty.h
>> @@ -419,8 +419,9 @@ void tty_unthrottle(struct tty_struct *tty);
>>   int tty_throttle_safe(struct tty_struct *tty);
>>   int tty_unthrottle_safe(struct tty_struct *tty);
>>   int tty_do_resize(struct tty_struct *tty, struct winsize *ws);
>> -int tty_get_icount(struct tty_struct *tty,
>> +int tty_get_icount(struct tty_struct *tty
> 
> The robot already complained about this ^^^.
> 
> Have you even tried to compile-test this?

Sorry I made a rebase of the patch set. My mistake!
I compiled and tested my changes against kernel 5.15 (OpenWrt).
And it worked as expected.

> 
>>   		struct serial_icounter_struct *icount);
>> +extern int tty_get_mget(struct tty_struct *tty);
> 

Re: [PATCH 2/2] leds: trigger: ledtrig-tty: add additional modes

[Uwe Kleine-König]

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

On Mon, Feb 13, 2023 at 03:06:38PM +0100, Florian Eckert wrote:
> Add additional modes to trigger the selected LED.
> The following modes are supported:
> 
> TD/RD:	Flash LED on data transmission (default)
> CTS:	DCE Ready to accept data from the DTE.
> DSR:	DCE is ready to receive and send data.
> CAR:	DCE is receiving a carrier from a remote DTE.
> RNG:	DCE has detected an incoming ring signal.
> 
> The mode can be changed for example with the following command:
> echo "CTS" /sys/class/leds/<led>/mode

I wonder if the abstraction is better be done such that you can also
configure the mode to trigger on (for example) TD and RNG. Then you'd
need one property per signal and then something like the following would
be possible:

	p=/sys/class/leds/<led>
	echo 1 > $p/tx
	echo 0 > $p/rx
	echo 1 > $p/rng

Best regards
Uwe

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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH 2/2] leds: trigger: ledtrig-tty: add additional modes

[Florian Eckert]

Hello Uwe,

>> 
>> TD/RD:	Flash LED on data transmission (default)
>> CTS:	DCE Ready to accept data from the DTE.
>> DSR:	DCE is ready to receive and send data.
>> CAR:	DCE is receiving a carrier from a remote DTE.
>> RNG:	DCE has detected an incoming ring signal.
>> 
>> The mode can be changed for example with the following command:
>> echo "CTS" /sys/class/leds/<led>/mode
> 
> I wonder if the abstraction is better be done such that you can also
> configure the mode to trigger on (for example) TD and RNG. Then you'd
> need one property per signal and then something like the following 
> would
> be possible:
> 
> 	p=/sys/class/leds/<led>
> 	echo 1 > $p/tx
> 	echo 0 > $p/rx
> 	echo 1 > $p/rng

I thought about that before implementing this patch set, but then I 
discarded it.
Coding several states would then become confusing for someone who look 
at the LED.
I have now consciously decided that I want only display one state.

> 
> Best regards
> Uwe

Re: [PATCH 2/2] leds: trigger: ledtrig-tty: add additional modes

[Florian Eckert]

Hello Jiri,

> 
>> +};
>> +
>> +static const char * const mode[] = {
> 

> This is not a wise name.

What do you suggest? For me, the 'mode' here was the most accurate.
It describes how the trigger should behave.

How about
* action
* switch
* tty_trigger


> 
>> +	[TTY_LED_CNT] = "TD/RD", // Trasmit Data / Receive Data
>> +	[TTY_LED_CTS] = "CTS", // CTS Clear To Send
>> +	[TTY_LED_DSR] = "DSR", // DSR Data Set Ready
>> +	[TTY_LED_CAR] = "CAR", // CAR Data Carrier Detect (DCD)
>> +	[TTY_LED_RNG] = "RNG", // RNG Ring Indicator (RI)
>>   };
>>     static void ledtrig_tty_restart(struct ledtrig_tty_data 
>> *trigger_data)
>> @@ -21,6 +38,74 @@ static void ledtrig_tty_restart(struct 
>> ledtrig_tty_data *trigger_data)
>>   	schedule_delayed_work(&trigger_data->dwork, 0);
>>   }

Best regards
-- Florian

Re: [PATCH 2/2] leds: trigger: ledtrig-tty: add additional modes

[Jiri Slaby]

Hi,

On 14. 02. 23, 12:13, Florian Eckert wrote:
>>> +static const char * const mode[] = {
>>
> 
>> This is not a wise name.
> 
> What do you suggest? For me, the 'mode' here was the most accurate.
> It describes how the trigger should behave.

mode is fine but with a context. Like tty_led_mode.

thanks,
-- 
js