* [PATCH 0/2] ledtrig-tty: add new state evaluation
@ 2023-09-26 9:36 Florian Eckert
2023-09-26 9:36 ` [PATCH 1/2] tty: add new helper function tty_get_mget Florian Eckert
2023-09-26 9:36 ` [PATCH 2/2] trigger: ledtrig-tty: add new line mode to triggers Florian Eckert
0 siblings, 2 replies; 6+ messages in thread
From: Florian Eckert @ 2023-09-26 9:36 UTC (permalink / raw)
To: Eckert.Florian, gregkh, jirislaby, pavel, lee, kabel,
u.kleine-koenig
Cc: linux-kernel, linux-serial, linux-leds
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 1/2] tty: add new helper function tty_get_mget
2023-09-26 9:36 [PATCH 0/2] ledtrig-tty: add new state evaluation Florian Eckert
@ 2023-09-26 9:36 ` Florian Eckert
2023-09-26 9:48 ` Jiri Slaby
2023-09-26 9:36 ` [PATCH 2/2] trigger: ledtrig-tty: add new line mode to triggers Florian Eckert
1 sibling, 1 reply; 6+ messages in thread
From: Florian Eckert @ 2023-09-26 9:36 UTC (permalink / raw)
To: Eckert.Florian, gregkh, jirislaby, pavel, lee, kabel,
u.kleine-koenig
Cc: linux-kernel, linux-serial, linux-leds
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
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 2/2] trigger: ledtrig-tty: add new line mode to triggers
2023-09-26 9:36 [PATCH 0/2] ledtrig-tty: add new state evaluation Florian Eckert
2023-09-26 9:36 ` [PATCH 1/2] tty: add new helper function tty_get_mget Florian Eckert
@ 2023-09-26 9:36 ` Florian Eckert
2023-09-26 20:33 ` kernel test robot
1 sibling, 1 reply; 6+ messages in thread
From: Florian Eckert @ 2023-09-26 9:36 UTC (permalink / raw)
To: Eckert.Florian, gregkh, jirislaby, pavel, lee, kabel,
u.kleine-koenig
Cc: linux-kernel, linux-serial, linux-leds
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
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] tty: add new helper function tty_get_mget
2023-09-26 9:36 ` [PATCH 1/2] tty: add new helper function tty_get_mget Florian Eckert
@ 2023-09-26 9:48 ` Jiri Slaby
2023-09-26 12:03 ` Florian Eckert
0 siblings, 1 reply; 6+ messages in thread
From: Jiri Slaby @ 2023-09-26 9:48 UTC (permalink / raw)
To: Florian Eckert, Eckert.Florian, gregkh, pavel, lee, kabel,
u.kleine-koenig
Cc: linux-kernel, linux-serial, linux-leds
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] tty: add new helper function tty_get_mget
2023-09-26 9:48 ` Jiri Slaby
@ 2023-09-26 12:03 ` Florian Eckert
0 siblings, 0 replies; 6+ messages in thread
From: Florian Eckert @ 2023-09-26 12:03 UTC (permalink / raw)
To: Jiri Slaby
Cc: Eckert.Florian, gregkh, pavel, lee, kabel, u.kleine-koenig,
linux-kernel, linux-serial, linux-leds
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!
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] trigger: ledtrig-tty: add new line mode to triggers
2023-09-26 9:36 ` [PATCH 2/2] trigger: ledtrig-tty: add new line mode to triggers Florian Eckert
@ 2023-09-26 20:33 ` kernel test robot
0 siblings, 0 replies; 6+ messages in thread
From: kernel test robot @ 2023-09-26 20:33 UTC (permalink / raw)
To: Florian Eckert, Eckert.Florian, gregkh, jirislaby, pavel, lee,
kabel, u.kleine-koenig
Cc: oe-kbuild-all, linux-kernel, linux-serial, linux-leds
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
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2023-09-26 20:34 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-09-26 9:36 [PATCH 0/2] ledtrig-tty: add new state evaluation Florian Eckert
2023-09-26 9:36 ` [PATCH 1/2] tty: add new helper function tty_get_mget Florian Eckert
2023-09-26 9:48 ` Jiri Slaby
2023-09-26 12:03 ` Florian Eckert
2023-09-26 9:36 ` [PATCH 2/2] trigger: ledtrig-tty: add new line mode to triggers Florian Eckert
2023-09-26 20:33 ` kernel test robot
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).