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 <[email protected]>
---
.../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: [email protected]
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
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 <[email protected]>
...
> --- 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
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/ |
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
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
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