[PATCH 0/2] ledtrig-tty: add new state evaluation

[PATCH 0/2] ledtrig-tty: add new state evaluation

[Florian Eckert]

This is a follow-up patchset, based on the mailing list discussion from
March 2023 based on the old patchset v7 [1]. I have changed, the LED trigger
handling via the sysfs interfaces as suggested by Uwe Kleine-König.

[1] https://lore.kernel.org/linux-leds/20230306093524.amm7o4ppa7gon4ew@pengutronix.de/

Florian Eckert (2):
  tty: add new helper function tty_get_mget
  trigger: ledtrig-tty: add new line mode to triggers

 .../ABI/testing/sysfs-class-led-trigger-tty   |  54 ++++
 drivers/leds/trigger/ledtrig-tty.c            | 272 +++++++++++++++++-
 drivers/tty/tty_io.c                          |  29 +-
 include/linux/tty.h                           |   1 +
 4 files changed, 339 insertions(+), 17 deletions(-)

-- 
2.30.2

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

[Florian Eckert]

The struct 'tty_struct' has a callback to read the status flags of the tty
if the tty driver provides them. So fare, the data is transferred directly
to userspace with the function 'tty_tiocmget'. This function cannot be
used to evaluate the status line of the tty interface in the ledtrig-tty
trigger. To make this possible, a new function must be added that does
not immediately pass the data on to userspace.

The new function 'tty_get_mget' only returns the status register.
This information can then be processed further in the ledtrig-tty
trigger.

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

diff --git a/drivers/tty/tty_io.c b/drivers/tty/tty_io.c
index 8a94e5a43c6d..8070ed0ce41f 100644
--- a/drivers/tty/tty_io.c
+++ b/drivers/tty/tty_io.c
@@ -2494,6 +2494,25 @@ 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.
+ *
+ */
+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
@@ -2506,14 +2525,12 @@ static int send_break(struct tty_struct *tty, unsigned int duration)
  */
 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 f002d0f25db7..7b9edd4a7007 100644
--- a/include/linux/tty.h
+++ b/include/linux/tty.h
@@ -421,6 +421,7 @@ 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,
 		struct serial_icounter_struct *icount);
+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

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

[Jiri Slaby]

On 26. 09. 23, 11:36, Florian Eckert wrote:
> The struct 'tty_struct' has a callback to read the status flags of the tty
> if the tty driver provides them. So fare, the data is transferred directly
> to userspace with the function 'tty_tiocmget'. This function cannot be
> used to evaluate the status line of the tty interface in the ledtrig-tty
> trigger. To make this possible, a new function must be added that does
> not immediately pass the data on to userspace.
> 
> The new function 'tty_get_mget' only returns the status register.
> This information can then be processed further in the ledtrig-tty
> trigger.
> 
> Signed-off-by: Florian Eckert <fe@dev.tdt.de>
> ---
>   drivers/tty/tty_io.c | 29 +++++++++++++++++++++++------
>   include/linux/tty.h  |  1 +
>   2 files changed, 24 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/tty/tty_io.c b/drivers/tty/tty_io.c
> index 8a94e5a43c6d..8070ed0ce41f 100644
> --- a/drivers/tty/tty_io.c
> +++ b/drivers/tty/tty_io.c
> @@ -2494,6 +2494,25 @@ static int send_break(struct tty_struct *tty, unsigned int duration)
>   	return retval;
>   }
>   
> +/**
> + * tty_get_mget		-	get modem status

Heh, the naming is funny. It apparently comes from tiocmget. But that 
comes from:
tty ioctl modem get (TIOCMGET)
tty ioctl modem set (TIOCMSET)

So you should name it like tty_get_modem() not get_mget().

Also those extra spaces around "-" caused some issues in the generated 
output and should be removed (everywhere).

> + * @tty: tty device
> + *
> + * Obtain the modem status bits from the tty driver if the feature
> + * is supported.
> + *

Superfluous empty line here.

> + */
> +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);


thanks,
-- 
js
suse labs

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

[Florian Eckert]

Thanks for your fast respond!

>> @@ -2494,6 +2494,25 @@ static int send_break(struct tty_struct *tty, 
>> unsigned int duration)
>>   	return retval;
>>   }
>>   +/**
>> + * tty_get_mget		-	get modem status
> 
> Heh, the naming is funny. It apparently comes from tiocmget. But that
> comes from:
> tty ioctl modem get (TIOCMGET)
> tty ioctl modem set (TIOCMSET)
> 
> So you should name it like tty_get_modem() not get_mget().

I didn't like the name too, but I couldn't think of another.
The function is returning the state of serial control and status 
signals.

 From your suggestion for the name, however, you can not deduce that at 
all.
How would it be then with the following name?

tty_tioctl_state() ?

> 
> Also those extra spaces around "-" caused some issues in the generated
> output and should be removed (everywhere).

Ok, I will change this in an own commit throughout the file.


Thanks

Florian

@jirislaby: Forgot to send this message to the mailing list as well!

[PATCH 2/2] trigger: ledtrig-tty: add new line mode to triggers

[Florian Eckert]

Until now, the LED blinks when data is sent via the tty (rx/tx).
The serial tty interface also supports additional input signals, that can
also be evaluated within this trigger. This change is adding the following
additional input sources, which could be controlled
via the '/sys/class/<leds>/' sysfs interface.

- line_cts:
  DCE is ready to accept data from the DTE (Clear To  Send). If the line
  state is detected, the LED is switched on.
  If set to 0 (default), the LED will not evaluate CTS.
  If set to 1, the LED will evaluate CTS.

- line_dsr:
  DCE is ready to receive and send data (Data Set Ready). If the line state
  is detected, the LED is switched on.
  If set to 0 (default), the LED will not evaluate DSR.
  If set to 1, the LED will evaluate DSR.

- line_car:
  DTE is receiving a carrier from the DCE (Data Carrier Detect). If the
  line state is detected, the LED is switched on.
  If set to 0 (default), the LED will not evaluate CAR (DCD).
  If set to 1, the LED will evaluate CAR (DCD).

- line_rng:
  DCE has detected an incoming ring signal on the telephone line
  (Ring Indicator). If the line state is detected, the LED is switched on.
  If set to 0 (default), the LED will not evaluate RNG (RI).
  If set to 1, the LED will evaluate RNG (RI).

In addition to the new line_* entries in sysfs, the indication for the
direction of the transmitted data is independently controllable via the
new rx and tx sysfs entrie now too. These are on by default. Thus the
trigger behaves as before this change.

- rx:
  Signal reception (rx) of data on the named tty device.
  If set to 0, the LED will not blink on reception.
  If set to 1 (default), the LED will blink on reception.

- tx:
  Signal transmission (tx) of data on the named tty device.
  If set to 0, the LED will not blink on transmission.
  If set to 1 (default), the LED will blink on transmission.

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

diff --git a/Documentation/ABI/testing/sysfs-class-led-trigger-tty b/Documentation/ABI/testing/sysfs-class-led-trigger-tty
index 2bf6b24e781b..14c38a2370ea 100644
--- a/Documentation/ABI/testing/sysfs-class-led-trigger-tty
+++ b/Documentation/ABI/testing/sysfs-class-led-trigger-tty
@@ -4,3 +4,57 @@ KernelVersion:	5.10
 Contact:	linux-leds@vger.kernel.org
 Description:
 		Specifies the tty device name of the triggering tty
+
+What:		/sys/class/leds/<led>/rx
+Date:		September 2023
+KernelVersion:	6.7
+Description:
+		Signal reception (rx) of data on the named tty device.
+		If set to 0, the LED will not blink on reception.
+		If set to 1 (default), the LED will blink on reception.
+
+What:		/sys/class/leds/<led>/tx
+Date:		September 2023
+KernelVersion:	6.7
+Description:
+		Signal transmission (tx) of data on the named tty device.
+		If set to 0, the LED will not blink on transmission.
+		If set to 1 (default), the LED will blink on transmission.
+
+car rng
+What:		/sys/class/leds/<led>/line_cts
+Date:		September 2023
+KernelVersion:	6.7
+Description:
+		DCE is ready to accept data from the DTE (Clear To Send). If
+		the line state is detected, the LED is switched on.
+		If set to 0 (default), the LED will not evaluate CTS.
+		If set to 1, the LED will evaluate CTS.
+
+What:		/sys/class/leds/<led>/line_dsr
+Date:		September 2023
+KernelVersion:	6.7
+Description:
+		DCE is ready to receive and send data (Data Set Ready). If
+		the line state is detected, the LED is switched on.
+		If set to 0 (default), the LED will not evaluate DSR.
+		If set to 1, the LED will evaluate DSR.
+
+What:		/sys/class/leds/<led>/line_car
+Date:		September 2023
+KernelVersion:	6.7
+Description:
+		DTE is receiving a carrier from the DCE (Data Carrier Detect).
+		If the line state is detected, the LED is switched on.
+		If set to 0 (default), the LED will not evaluate CAR (DCD).
+		If set to 1, the LED will evaluate CAR (DCD).
+
+What:		/sys/class/leds/<led>/line_cts
+Date:		September 2023
+KernelVersion:	6.7
+Description:
+		DCE has detected an incoming ring signal on the telephone
+		line (Ring Indicator). If the line state is detected, the
+		LED is switched on.
+		If set to 0 (default), the LED will not evaluate RNG (RI).
+		If set to 1, the LED will evaluate RNG (RI).
diff --git a/drivers/leds/trigger/ledtrig-tty.c b/drivers/leds/trigger/ledtrig-tty.c
index 8ae0d2d284af..0ea6d2a2599d 100644
--- a/drivers/leds/trigger/ledtrig-tty.c
+++ b/drivers/leds/trigger/ledtrig-tty.c
@@ -16,6 +16,28 @@ struct ledtrig_tty_data {
 	const char *ttyname;
 	struct tty_struct *tty;
 	int rx, tx;
+	unsigned long mode;
+#define LEDTRIG_TTY_MODE_TX	0
+#define LEDTRIG_TTY_MODE_RX	1
+#define LEDTRIG_TTY_MODE_CTS	2
+#define LEDTRIG_TTY_MODE_DSR	3
+#define LEDTRIG_TTY_MODE_CAR	4
+#define LEDTRIG_TTY_MODE_RNG	5
+};
+
+enum tty_led_state {
+	TTY_LED_BLINK,
+	TTY_LED_ENABLE,
+	TTY_LED_DISABLE,
+};
+
+enum ledtrig_tty_attr {
+	LEDTRIG_TTY_ATTR_TX,
+	LEDTRIG_TTY_ATTR_RX,
+	LEDTRIG_TTY_ATTR_CTS,
+	LEDTRIG_TTY_ATTR_DSR,
+	LEDTRIG_TTY_ATTR_CAR,
+	LEDTRIG_TTY_ATTR_RNG,
 };
 
 static void ledtrig_tty_restart(struct ledtrig_tty_data *trigger_data)
@@ -78,13 +100,184 @@ static ssize_t ttyname_store(struct device *dev,
 }
 static DEVICE_ATTR_RW(ttyname);
 
+static ssize_t ledtrig_tty_attr_show(struct device *dev, char *buf,
+	enum ledtrig_tty_attr attr)
+{
+	struct ledtrig_tty_data *trigger_data = led_trigger_get_drvdata(dev);
+	int bit;
+
+	switch (attr) {
+	case LEDTRIG_TTY_ATTR_TX:
+		bit = LEDTRIG_TTY_MODE_TX;
+		break;
+	case LEDTRIG_TTY_ATTR_RX:
+		bit = LEDTRIG_TTY_MODE_RX;
+		break;
+	case LEDTRIG_TTY_ATTR_CTS:
+		bit = LEDTRIG_TTY_MODE_CTS;
+		break;
+	case LEDTRIG_TTY_ATTR_DSR:
+		bit = LEDTRIG_TTY_MODE_DSR;
+		break;
+	case LEDTRIG_TTY_ATTR_CAR:
+		bit = LEDTRIG_TTY_MODE_CAR;
+		break;
+	case LEDTRIG_TTY_ATTR_RNG:
+		bit = LEDTRIG_TTY_MODE_RNG;
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	return sprintf(buf, "%u\n", test_bit(bit, &trigger_data->mode));
+}
+
+static ssize_t ledtrig_tty_attr_store(struct device *dev, const char *buf,
+	size_t size, enum ledtrig_tty_attr attr)
+{
+	struct ledtrig_tty_data *trigger_data = led_trigger_get_drvdata(dev);
+	unsigned long state;
+	int ret;
+	int bit;
+
+	ret = kstrtoul(buf, 0, &state);
+	if (ret)
+		return ret;
+
+	switch (attr) {
+	case LEDTRIG_TTY_ATTR_TX:
+		bit = LEDTRIG_TTY_MODE_TX;
+		break;
+	case LEDTRIG_TTY_ATTR_RX:
+		bit = LEDTRIG_TTY_MODE_RX;
+		break;
+	case LEDTRIG_TTY_ATTR_CTS:
+		bit = LEDTRIG_TTY_MODE_CTS;
+		break;
+	case LEDTRIG_TTY_ATTR_DSR:
+		bit = LEDTRIG_TTY_MODE_DSR;
+		break;
+	case LEDTRIG_TTY_ATTR_CAR:
+		bit = LEDTRIG_TTY_MODE_CAR;
+		break;
+	case LEDTRIG_TTY_ATTR_RNG:
+		bit = LEDTRIG_TTY_MODE_RNG;
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	if (state)
+		set_bit(bit, &trigger_data->mode);
+	else
+		clear_bit(bit, &trigger_data->mode);
+
+	return size;
+}
+
+static ssize_t tx_show(struct device *dev,
+	struct device_attribute *attr, char *buf)
+{
+	return ledtrig_tty_attr_show(dev, buf, LEDTRIG_TTY_ATTR_TX);
+}
+
+static ssize_t tx_store(struct device *dev,
+	struct device_attribute *attr, const char *buf, size_t size)
+{
+	return ledtrig_tty_attr_store(dev, buf, size, LEDTRIG_TTY_ATTR_TX);
+}
+static DEVICE_ATTR_RW(tx);
+
+static ssize_t rx_show(struct device *dev,
+	struct device_attribute *attr, char *buf)
+{
+	return ledtrig_tty_attr_show(dev, buf, LEDTRIG_TTY_ATTR_RX);
+}
+
+static ssize_t rx_store(struct device *dev,
+	struct device_attribute *attr, const char *buf, size_t size)
+{
+	return ledtrig_tty_attr_store(dev, buf, size, LEDTRIG_TTY_ATTR_RX);
+}
+static DEVICE_ATTR_RW(rx);
+
+static ssize_t line_cts_show(struct device *dev,
+	struct device_attribute *attr, char *buf)
+{
+	return ledtrig_tty_attr_show(dev, buf, LEDTRIG_TTY_ATTR_CTS);
+}
+
+static ssize_t line_cts_store(struct device *dev,
+	struct device_attribute *attr, const char *buf, size_t size)
+{
+	return ledtrig_tty_attr_store(dev, buf, size, LEDTRIG_TTY_ATTR_CTS);
+}
+static DEVICE_ATTR_RW(line_cts);
+
+static ssize_t line_dsr_show(struct device *dev,
+	struct device_attribute *attr, char *buf)
+{
+	return ledtrig_tty_attr_show(dev, buf, LEDTRIG_TTY_ATTR_DSR);
+}
+
+static ssize_t line_dsr_store(struct device *dev,
+	struct device_attribute *attr, const char *buf, size_t size)
+{
+	return ledtrig_tty_attr_store(dev, buf, size, LEDTRIG_TTY_ATTR_DSR);
+}
+static DEVICE_ATTR_RW(line_dsr);
+
+static ssize_t line_car_show(struct device *dev,
+	struct device_attribute *attr, char *buf)
+{
+	return ledtrig_tty_attr_show(dev, buf, LEDTRIG_TTY_ATTR_CAR);
+}
+
+static ssize_t line_car_store(struct device *dev,
+	struct device_attribute *attr, const char *buf, size_t size)
+{
+	return ledtrig_tty_attr_store(dev, buf, size, LEDTRIG_TTY_ATTR_CAR);
+}
+static DEVICE_ATTR_RW(line_car);
+
+static ssize_t line_rng_show(struct device *dev,
+	struct device_attribute *attr, char *buf)
+{
+	return ledtrig_tty_attr_show(dev, buf, LEDTRIG_TTY_ATTR_RNG);
+}
+
+static ssize_t line_rng_store(struct device *dev,
+	struct device_attribute *attr, const char *buf, size_t size)
+{
+	return ledtrig_tty_attr_store(dev, buf, size, LEDTRIG_TTY_ATTR_RNG);
+}
+static DEVICE_ATTR_RW(line_rng);
+
+
+static int ledtrig_tty_flag(struct ledtrig_tty_data *trigger_data, unsigned int flag)
+{
+	unsigned int status;
+	int ret;
+
+	status = tty_get_mget(trigger_data->tty);
+	if (status & flag)
+		ret = 1;
+	else
+		ret = 0;
+
+	return ret;
+}
+
 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 led_classdev *led_cdev = trigger_data->led_cdev;
 	struct serial_icounter_struct icount;
+	enum tty_led_state state;
 	int ret;
 
+	state = TTY_LED_DISABLE;
 	mutex_lock(&trigger_data->mutex);
 
 	if (!trigger_data->ttyname) {
@@ -115,22 +308,69 @@ 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 (test_bit(LEDTRIG_TTY_MODE_CTS, &trigger_data->mode)) {
+		ret = ledtrig_tty_flag(trigger_data, TIOCM_CTS);
+		if (ret)
+			state = TTY_LED_ENABLE;
 	}
 
-	if (icount.rx != trigger_data->rx ||
-	    icount.tx != trigger_data->tx) {
-		unsigned long interval = LEDTRIG_TTY_INTERVAL;
+	if (test_bit(LEDTRIG_TTY_MODE_DSR, &trigger_data->mode)) {
+		ret = ledtrig_tty_flag(trigger_data, TIOCM_DSR);
+		if (ret)
+			state = TTY_LED_ENABLE;
+	}
 
+	if (test_bit(LEDTRIG_TTY_MODE_CAR, &trigger_data->mode)) {
+		ret = ledtrig_tty_flag(trigger_data, TIOCM_CAR);
+		if (ret)
+			state = TTY_LED_ENABLE;
+	}
+
+	if (test_bit(LEDTRIG_TTY_MODE_RNG, &trigger_data->mode)) {
+		ret = ledtrig_tty_flag(trigger_data, TIOCM_RNG);
+		if (ret)
+			state = TTY_LED_ENABLE;
+	}
+
+	/* The rx/tx handling must come after the evaluation of TIOCM_*,
+	 * since the display for rx/tx has priority
+	 */
+	if (test_bit(LEDTRIG_TTY_MODE_TX, &trigger_data->mode) ||
+	    test_bit(LEDTRIG_TTY_MODE_RX, &trigger_data->mode)) {
+		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 (test_bit(LEDTRIG_TTY_MODE_TX, &trigger_data->mode) &&
+		    (icount.tx != trigger_data->tx)) {
+			trigger_data->tx = icount.tx;
+			state = TTY_LED_BLINK;
+		}
+
+		if (test_bit(LEDTRIG_TTY_MODE_RX, &trigger_data->mode) &&
+		    (icount.rx != trigger_data->rx)) {
+			trigger_data->rx = icount.rx;
+			state = TTY_LED_BLINK;
+		}
+	}
+
+	switch (state) {
+	case TTY_LED_BLINK:
+		unsigned long interval = LEDTRIG_TTY_INTERVAL;
 		led_blink_set_oneshot(trigger_data->led_cdev, &interval,
 				      &interval, 0);
-
-		trigger_data->rx = icount.rx;
-		trigger_data->tx = icount.tx;
+		break;
+	case TTY_LED_ENABLE:
+		led_set_brightness(led_cdev, led_cdev->blink_brightness);
+		break;
+	case TTY_LED_DISABLE:
+		fallthrough;
+	default:
+		led_set_brightness(led_cdev, 0);
+		break;
 	}
 
 out:
@@ -141,6 +381,12 @@ static void ledtrig_tty_work(struct work_struct *work)
 
 static struct attribute *ledtrig_tty_attrs[] = {
 	&dev_attr_ttyname.attr,
+	&dev_attr_rx.attr,
+	&dev_attr_tx.attr,
+	&dev_attr_line_cts.attr,
+	&dev_attr_line_dsr.attr,
+	&dev_attr_line_car.attr,
+	&dev_attr_line_rng.attr,
 	NULL
 };
 ATTRIBUTE_GROUPS(ledtrig_tty);
@@ -153,6 +399,10 @@ static int ledtrig_tty_activate(struct led_classdev *led_cdev)
 	if (!trigger_data)
 		return -ENOMEM;
 
+	/* Enable default rx/tx LED blink */
+	set_bit(LEDTRIG_TTY_MODE_TX, &trigger_data->mode);
+	set_bit(LEDTRIG_TTY_MODE_RX, &trigger_data->mode);
+
 	led_set_trigger_data(led_cdev, trigger_data);
 
 	INIT_DELAYED_WORK(&trigger_data->dwork, ledtrig_tty_work);
-- 
2.30.2

Re: [PATCH 2/2] trigger: ledtrig-tty: add new line mode to triggers

[kernel test robot]

Hi Florian,

kernel test robot noticed the following build errors:

[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.6-rc3 next-20230926]
[cannot apply to pavel-leds/for-next]
[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-add-new-helper-function-tty_get_mget/20230926-180154
base:   https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/tty.git tty-testing
patch link:    https://lore.kernel.org/r/20230926093607.59536-3-fe%40dev.tdt.de
patch subject: [PATCH 2/2] trigger: ledtrig-tty: add new line mode to triggers
config: x86_64-randconfig-161-20230927 (https://download.01.org/0day-ci/archive/20230927/202309270440.IJB24Xap-lkp@intel.com/config)
compiler: gcc-7 (Ubuntu 7.5.0-6ubuntu2) 7.5.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20230927/202309270440.IJB24Xap-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202309270440.IJB24Xap-lkp@intel.com/

All errors (new ones prefixed by >>):

   drivers/leds/trigger/ledtrig-tty.c: In function 'ledtrig_tty_work':
>> drivers/leds/trigger/ledtrig-tty.c:362:3: error: a label can only be part of a statement and a declaration is not a statement
      unsigned long interval = LEDTRIG_TTY_INTERVAL;
      ^~~~~~~~


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

   270	
   271	static void ledtrig_tty_work(struct work_struct *work)
   272	{
   273		struct ledtrig_tty_data *trigger_data =
   274			container_of(work, struct ledtrig_tty_data, dwork.work);
   275		struct led_classdev *led_cdev = trigger_data->led_cdev;
   276		struct serial_icounter_struct icount;
   277		enum tty_led_state state;
   278		int ret;
   279	
   280		state = TTY_LED_DISABLE;
   281		mutex_lock(&trigger_data->mutex);
   282	
   283		if (!trigger_data->ttyname) {
   284			/* exit without rescheduling */
   285			mutex_unlock(&trigger_data->mutex);
   286			return;
   287		}
   288	
   289		/* try to get the tty corresponding to $ttyname */
   290		if (!trigger_data->tty) {
   291			dev_t devno;
   292			struct tty_struct *tty;
   293			int ret;
   294	
   295			ret = tty_dev_name_to_number(trigger_data->ttyname, &devno);
   296			if (ret < 0)
   297				/*
   298				 * A device with this name might appear later, so keep
   299				 * retrying.
   300				 */
   301				goto out;
   302	
   303			tty = tty_kopen_shared(devno);
   304			if (IS_ERR(tty) || !tty)
   305				/* What to do? retry or abort */
   306				goto out;
   307	
   308			trigger_data->tty = tty;
   309		}
   310	
   311		if (test_bit(LEDTRIG_TTY_MODE_CTS, &trigger_data->mode)) {
   312			ret = ledtrig_tty_flag(trigger_data, TIOCM_CTS);
   313			if (ret)
   314				state = TTY_LED_ENABLE;
   315		}
   316	
   317		if (test_bit(LEDTRIG_TTY_MODE_DSR, &trigger_data->mode)) {
   318			ret = ledtrig_tty_flag(trigger_data, TIOCM_DSR);
   319			if (ret)
   320				state = TTY_LED_ENABLE;
   321		}
   322	
   323		if (test_bit(LEDTRIG_TTY_MODE_CAR, &trigger_data->mode)) {
   324			ret = ledtrig_tty_flag(trigger_data, TIOCM_CAR);
   325			if (ret)
   326				state = TTY_LED_ENABLE;
   327		}
   328	
   329		if (test_bit(LEDTRIG_TTY_MODE_RNG, &trigger_data->mode)) {
   330			ret = ledtrig_tty_flag(trigger_data, TIOCM_RNG);
   331			if (ret)
   332				state = TTY_LED_ENABLE;
   333		}
   334	
   335		/* The rx/tx handling must come after the evaluation of TIOCM_*,
   336		 * since the display for rx/tx has priority
   337		 */
   338		if (test_bit(LEDTRIG_TTY_MODE_TX, &trigger_data->mode) ||
   339		    test_bit(LEDTRIG_TTY_MODE_RX, &trigger_data->mode)) {
   340			ret = tty_get_icount(trigger_data->tty, &icount);
   341			if (ret) {
   342				dev_info(trigger_data->tty->dev, "Failed to get icount, stopped polling\n");
   343				mutex_unlock(&trigger_data->mutex);
   344				return;
   345			}
   346	
   347			if (test_bit(LEDTRIG_TTY_MODE_TX, &trigger_data->mode) &&
   348			    (icount.tx != trigger_data->tx)) {
   349				trigger_data->tx = icount.tx;
   350				state = TTY_LED_BLINK;
   351			}
   352	
   353			if (test_bit(LEDTRIG_TTY_MODE_RX, &trigger_data->mode) &&
   354			    (icount.rx != trigger_data->rx)) {
   355				trigger_data->rx = icount.rx;
   356				state = TTY_LED_BLINK;
   357			}
   358		}
   359	
   360		switch (state) {
   361		case TTY_LED_BLINK:
 > 362			unsigned long interval = LEDTRIG_TTY_INTERVAL;
   363			led_blink_set_oneshot(trigger_data->led_cdev, &interval,
   364					      &interval, 0);
   365			break;
   366		case TTY_LED_ENABLE:
   367			led_set_brightness(led_cdev, led_cdev->blink_brightness);
   368			break;
   369		case TTY_LED_DISABLE:
   370			fallthrough;
   371		default:
   372			led_set_brightness(led_cdev, 0);
   373			break;
   374		}
   375	
   376	out:
   377		mutex_unlock(&trigger_data->mutex);
   378		schedule_delayed_work(&trigger_data->dwork,
   379				      msecs_to_jiffies(LEDTRIG_TTY_INTERVAL * 2));
   380	}
   381	

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

Re: [PATCH 2/2] trigger: ledtrig-tty: add new line mode to triggers

[kernel test robot]

:::::: 
:::::: Manual check reason: "low confidence static check warning: drivers/leds/trigger/ledtrig-tty.c:362:17: sparse: sparse: typename in expression"
:::::: 

BCC: lkp@intel.com
CC: oe-kbuild-all@lists.linux.dev
In-Reply-To: <20230926093607.59536-3-fe@dev.tdt.de>
References: <20230926093607.59536-3-fe@dev.tdt.de>
TO: Florian Eckert <fe@dev.tdt.de>
TO: Eckert.Florian@googlemail.com
TO: gregkh@linuxfoundation.org
TO: jirislaby@kernel.org
TO: pavel@ucw.cz
TO: lee@kernel.org
TO: kabel@kernel.org
TO: u.kleine-koenig@pengutronix.de
CC: linux-kernel@vger.kernel.org
CC: linux-serial@vger.kernel.org
CC: linux-leds@vger.kernel.org

Hi Florian,

kernel test robot noticed the following build warnings:

[auto build test WARNING on tty/tty-testing]
[also build test WARNING on tty/tty-next tty/tty-linus staging/staging-testing staging/staging-next staging/staging-linus linus/master v6.6-rc3 next-20230926]
[cannot apply to pavel-leds/for-next]
[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-add-new-helper-function-tty_get_mget/20230926-180154
base:   https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/tty.git tty-testing
patch link:    https://lore.kernel.org/r/20230926093607.59536-3-fe%40dev.tdt.de
patch subject: [PATCH 2/2] trigger: ledtrig-tty: add new line mode to triggers
:::::: branch date: 16 hours ago
:::::: commit date: 16 hours ago
config: x86_64-randconfig-123-20230927 (https://download.01.org/0day-ci/archive/20230927/202309270921.wW6DyJDu-lkp@intel.com/config)
compiler: gcc-12 (Debian 12.2.0-14) 12.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20230927/202309270921.wW6DyJDu-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/r/202309270921.wW6DyJDu-lkp@intel.com/

sparse warnings: (new ones prefixed by >>)
>> drivers/leds/trigger/ledtrig-tty.c:362:17: sparse: sparse: typename in expression
   drivers/leds/trigger/ledtrig-tty.c:362:26: sparse: sparse: Expected ; at end of statement
   drivers/leds/trigger/ledtrig-tty.c:362:26: sparse: sparse: got long
   drivers/leds/trigger/ledtrig-tty.c:362:17: sparse: sparse: undefined identifier 'unsigned'
   drivers/leds/trigger/ledtrig-tty.c:363:64: sparse: sparse: undefined identifier 'interval'

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

7c8a6442409e77b Florian Eckert   2023-09-26  270  
fd4a641ac88fbba Uwe Kleine-König 2021-01-13  271  static void ledtrig_tty_work(struct work_struct *work)
fd4a641ac88fbba Uwe Kleine-König 2021-01-13  272  {
fd4a641ac88fbba Uwe Kleine-König 2021-01-13  273  	struct ledtrig_tty_data *trigger_data =
fd4a641ac88fbba Uwe Kleine-König 2021-01-13  274  		container_of(work, struct ledtrig_tty_data, dwork.work);
7c8a6442409e77b Florian Eckert   2023-09-26  275  	struct led_classdev *led_cdev = trigger_data->led_cdev;
fd4a641ac88fbba Uwe Kleine-König 2021-01-13  276  	struct serial_icounter_struct icount;
7c8a6442409e77b Florian Eckert   2023-09-26  277  	enum tty_led_state state;
fd4a641ac88fbba Uwe Kleine-König 2021-01-13  278  	int ret;
fd4a641ac88fbba Uwe Kleine-König 2021-01-13  279  
7c8a6442409e77b Florian Eckert   2023-09-26  280  	state = TTY_LED_DISABLE;
fd4a641ac88fbba Uwe Kleine-König 2021-01-13  281  	mutex_lock(&trigger_data->mutex);
fd4a641ac88fbba Uwe Kleine-König 2021-01-13  282  
fd4a641ac88fbba Uwe Kleine-König 2021-01-13  283  	if (!trigger_data->ttyname) {
fd4a641ac88fbba Uwe Kleine-König 2021-01-13  284  		/* exit without rescheduling */
fd4a641ac88fbba Uwe Kleine-König 2021-01-13  285  		mutex_unlock(&trigger_data->mutex);
fd4a641ac88fbba Uwe Kleine-König 2021-01-13  286  		return;
fd4a641ac88fbba Uwe Kleine-König 2021-01-13  287  	}
fd4a641ac88fbba Uwe Kleine-König 2021-01-13  288  
fd4a641ac88fbba Uwe Kleine-König 2021-01-13  289  	/* try to get the tty corresponding to $ttyname */
fd4a641ac88fbba Uwe Kleine-König 2021-01-13  290  	if (!trigger_data->tty) {
fd4a641ac88fbba Uwe Kleine-König 2021-01-13  291  		dev_t devno;
fd4a641ac88fbba Uwe Kleine-König 2021-01-13  292  		struct tty_struct *tty;
fd4a641ac88fbba Uwe Kleine-König 2021-01-13  293  		int ret;
fd4a641ac88fbba Uwe Kleine-König 2021-01-13  294  
fd4a641ac88fbba Uwe Kleine-König 2021-01-13  295  		ret = tty_dev_name_to_number(trigger_data->ttyname, &devno);
fd4a641ac88fbba Uwe Kleine-König 2021-01-13  296  		if (ret < 0)
fd4a641ac88fbba Uwe Kleine-König 2021-01-13  297  			/*
fd4a641ac88fbba Uwe Kleine-König 2021-01-13  298  			 * A device with this name might appear later, so keep
fd4a641ac88fbba Uwe Kleine-König 2021-01-13  299  			 * retrying.
fd4a641ac88fbba Uwe Kleine-König 2021-01-13  300  			 */
fd4a641ac88fbba Uwe Kleine-König 2021-01-13  301  			goto out;
fd4a641ac88fbba Uwe Kleine-König 2021-01-13  302  
fd4a641ac88fbba Uwe Kleine-König 2021-01-13  303  		tty = tty_kopen_shared(devno);
fd4a641ac88fbba Uwe Kleine-König 2021-01-13  304  		if (IS_ERR(tty) || !tty)
fd4a641ac88fbba Uwe Kleine-König 2021-01-13  305  			/* What to do? retry or abort */
fd4a641ac88fbba Uwe Kleine-König 2021-01-13  306  			goto out;
fd4a641ac88fbba Uwe Kleine-König 2021-01-13  307  
fd4a641ac88fbba Uwe Kleine-König 2021-01-13  308  		trigger_data->tty = tty;
fd4a641ac88fbba Uwe Kleine-König 2021-01-13  309  	}
fd4a641ac88fbba Uwe Kleine-König 2021-01-13  310  
7c8a6442409e77b Florian Eckert   2023-09-26  311  	if (test_bit(LEDTRIG_TTY_MODE_CTS, &trigger_data->mode)) {
7c8a6442409e77b Florian Eckert   2023-09-26  312  		ret = ledtrig_tty_flag(trigger_data, TIOCM_CTS);
7c8a6442409e77b Florian Eckert   2023-09-26  313  		if (ret)
7c8a6442409e77b Florian Eckert   2023-09-26  314  			state = TTY_LED_ENABLE;
7c8a6442409e77b Florian Eckert   2023-09-26  315  	}
7c8a6442409e77b Florian Eckert   2023-09-26  316  
7c8a6442409e77b Florian Eckert   2023-09-26  317  	if (test_bit(LEDTRIG_TTY_MODE_DSR, &trigger_data->mode)) {
7c8a6442409e77b Florian Eckert   2023-09-26  318  		ret = ledtrig_tty_flag(trigger_data, TIOCM_DSR);
7c8a6442409e77b Florian Eckert   2023-09-26  319  		if (ret)
7c8a6442409e77b Florian Eckert   2023-09-26  320  			state = TTY_LED_ENABLE;
7c8a6442409e77b Florian Eckert   2023-09-26  321  	}
7c8a6442409e77b Florian Eckert   2023-09-26  322  
7c8a6442409e77b Florian Eckert   2023-09-26  323  	if (test_bit(LEDTRIG_TTY_MODE_CAR, &trigger_data->mode)) {
7c8a6442409e77b Florian Eckert   2023-09-26  324  		ret = ledtrig_tty_flag(trigger_data, TIOCM_CAR);
7c8a6442409e77b Florian Eckert   2023-09-26  325  		if (ret)
7c8a6442409e77b Florian Eckert   2023-09-26  326  			state = TTY_LED_ENABLE;
7c8a6442409e77b Florian Eckert   2023-09-26  327  	}
7c8a6442409e77b Florian Eckert   2023-09-26  328  
7c8a6442409e77b Florian Eckert   2023-09-26  329  	if (test_bit(LEDTRIG_TTY_MODE_RNG, &trigger_data->mode)) {
7c8a6442409e77b Florian Eckert   2023-09-26  330  		ret = ledtrig_tty_flag(trigger_data, TIOCM_RNG);
7c8a6442409e77b Florian Eckert   2023-09-26  331  		if (ret)
7c8a6442409e77b Florian Eckert   2023-09-26  332  			state = TTY_LED_ENABLE;
7c8a6442409e77b Florian Eckert   2023-09-26  333  	}
7c8a6442409e77b Florian Eckert   2023-09-26  334  
7c8a6442409e77b Florian Eckert   2023-09-26  335  	/* The rx/tx handling must come after the evaluation of TIOCM_*,
7c8a6442409e77b Florian Eckert   2023-09-26  336  	 * since the display for rx/tx has priority
7c8a6442409e77b Florian Eckert   2023-09-26  337  	 */
7c8a6442409e77b Florian Eckert   2023-09-26  338  	if (test_bit(LEDTRIG_TTY_MODE_TX, &trigger_data->mode) ||
7c8a6442409e77b Florian Eckert   2023-09-26  339  	    test_bit(LEDTRIG_TTY_MODE_RX, &trigger_data->mode)) {
fd4a641ac88fbba Uwe Kleine-König 2021-01-13  340  		ret = tty_get_icount(trigger_data->tty, &icount);
fd4a641ac88fbba Uwe Kleine-König 2021-01-13  341  		if (ret) {
fd4a641ac88fbba Uwe Kleine-König 2021-01-13  342  			dev_info(trigger_data->tty->dev, "Failed to get icount, stopped polling\n");
fd4a641ac88fbba Uwe Kleine-König 2021-01-13  343  			mutex_unlock(&trigger_data->mutex);
fd4a641ac88fbba Uwe Kleine-König 2021-01-13  344  			return;
fd4a641ac88fbba Uwe Kleine-König 2021-01-13  345  		}
fd4a641ac88fbba Uwe Kleine-König 2021-01-13  346  
7c8a6442409e77b Florian Eckert   2023-09-26  347  		if (test_bit(LEDTRIG_TTY_MODE_TX, &trigger_data->mode) &&
7c8a6442409e77b Florian Eckert   2023-09-26  348  		    (icount.tx != trigger_data->tx)) {
7c8a6442409e77b Florian Eckert   2023-09-26  349  			trigger_data->tx = icount.tx;
7c8a6442409e77b Florian Eckert   2023-09-26  350  			state = TTY_LED_BLINK;
7c8a6442409e77b Florian Eckert   2023-09-26  351  		}
7c8a6442409e77b Florian Eckert   2023-09-26  352  
7c8a6442409e77b Florian Eckert   2023-09-26  353  		if (test_bit(LEDTRIG_TTY_MODE_RX, &trigger_data->mode) &&
7c8a6442409e77b Florian Eckert   2023-09-26  354  		    (icount.rx != trigger_data->rx)) {
7c8a6442409e77b Florian Eckert   2023-09-26  355  			trigger_data->rx = icount.rx;
7c8a6442409e77b Florian Eckert   2023-09-26  356  			state = TTY_LED_BLINK;
7c8a6442409e77b Florian Eckert   2023-09-26  357  		}
7c8a6442409e77b Florian Eckert   2023-09-26  358  	}
730094577e0c37e Marek Behún      2023-08-02  359  
7c8a6442409e77b Florian Eckert   2023-09-26  360  	switch (state) {
7c8a6442409e77b Florian Eckert   2023-09-26  361  	case TTY_LED_BLINK:
7c8a6442409e77b Florian Eckert   2023-09-26 @362  		unsigned long interval = LEDTRIG_TTY_INTERVAL;
730094577e0c37e Marek Behún      2023-08-02  363  		led_blink_set_oneshot(trigger_data->led_cdev, &interval,
730094577e0c37e Marek Behún      2023-08-02  364  				      &interval, 0);
7c8a6442409e77b Florian Eckert   2023-09-26  365  		break;
7c8a6442409e77b Florian Eckert   2023-09-26  366  	case TTY_LED_ENABLE:
7c8a6442409e77b Florian Eckert   2023-09-26  367  		led_set_brightness(led_cdev, led_cdev->blink_brightness);
7c8a6442409e77b Florian Eckert   2023-09-26  368  		break;
7c8a6442409e77b Florian Eckert   2023-09-26  369  	case TTY_LED_DISABLE:
7c8a6442409e77b Florian Eckert   2023-09-26  370  		fallthrough;
7c8a6442409e77b Florian Eckert   2023-09-26  371  	default:
7c8a6442409e77b Florian Eckert   2023-09-26  372  		led_set_brightness(led_cdev, 0);
7c8a6442409e77b Florian Eckert   2023-09-26  373  		break;
fd4a641ac88fbba Uwe Kleine-König 2021-01-13  374  	}
fd4a641ac88fbba Uwe Kleine-König 2021-01-13  375  
fd4a641ac88fbba Uwe Kleine-König 2021-01-13  376  out:
fd4a641ac88fbba Uwe Kleine-König 2021-01-13  377  	mutex_unlock(&trigger_data->mutex);
730094577e0c37e Marek Behún      2023-08-02  378  	schedule_delayed_work(&trigger_data->dwork,
730094577e0c37e Marek Behún      2023-08-02  379  			      msecs_to_jiffies(LEDTRIG_TTY_INTERVAL * 2));
fd4a641ac88fbba Uwe Kleine-König 2021-01-13  380  }
fd4a641ac88fbba Uwe Kleine-König 2021-01-13  381  

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