diff mbox series

[v4,3/3] leds: ledtrig-tty: add new line mode evaluation

Message ID 20231019112809.881730-4-fe@dev.tdt.de
State New
Headers show
Series ledtrig-tty: add additional tty state evaluation | expand

Commit Message

Florian Eckert Oct. 19, 2023, 11:28 a.m. UTC
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 (CTS = 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 (DSR = 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 (DCD = 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
  (RI = 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).

Explanation:
DCE = Data Communication Equipment (Modem)
DTE = Data Terminal Equipment (Computer)

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            | 192 +++++++++++++++++-
 2 files changed, 235 insertions(+), 11 deletions(-)

Comments

Greg KH Oct. 21, 2023, 4:07 p.m. UTC | #1
On Thu, Oct 19, 2023 at 01:28:09PM +0200, Florian Eckert wrote:
> 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 (CTS = 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 (DSR = 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 (DCD = 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
>   (RI = 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).
> 
> Explanation:
> DCE = Data Communication Equipment (Modem)
> DTE = Data Terminal Equipment (Computer)
> 
> 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            | 192 +++++++++++++++++-
>  2 files changed, 235 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..08127b1a4602 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:		October 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:		October 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:		October 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:		October 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:		October 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:		October 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..6a96439a7e55 100644
> --- a/drivers/leds/trigger/ledtrig-tty.c
> +++ b/drivers/leds/trigger/ledtrig-tty.c
> @@ -16,6 +16,24 @@ struct ledtrig_tty_data {
>  	const char *ttyname;
>  	struct tty_struct *tty;
>  	int rx, tx;
> +	unsigned long mode;

Why is mode "unsigned long" when the tty layer treats it as an int?  And
really, this should be set to an explit size, u32 perhaps?  Or am I
confused as to exactly what this is?

> +};
> +
> +enum led_trigger_tty_state {
> +	TTY_LED_BLINK,
> +	TTY_LED_ENABLE,
> +	TTY_LED_DISABLE,
> +};
> +
> +enum led_trigger_tty_modes {
> +	TRIGGER_TTY_RX = 0,
> +	TRIGGER_TTY_TX,
> +	TRIGGER_TTY_CTS,
> +	TRIGGER_TTY_DSR,
> +	TRIGGER_TTY_CAR,
> +	TRIGGER_TTY_RNG,
> +	/* Keep last */
> +	__TRIGGER_TTY_MAX,
>  };
>

Oh wait, is "mode" this?  If so, why not define it as an enum?  Or if
not, I'm totally confused as to what is going on here, sorry.


>  static void ledtrig_tty_restart(struct ledtrig_tty_data *trigger_data)
> @@ -78,13 +96,106 @@ 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 led_trigger_tty_modes attr)
> +{
> +	struct ledtrig_tty_data *trigger_data = led_trigger_get_drvdata(dev);
> +	int bit;
> +
> +	switch (attr) {
> +	case TRIGGER_TTY_RX:
> +	case TRIGGER_TTY_TX:
> +	case TRIGGER_TTY_CTS:
> +	case TRIGGER_TTY_DSR:
> +	case TRIGGER_TTY_CAR:
> +	case TRIGGER_TTY_RNG:
> +		bit = attr;
> +		break;
> +	default:
> +		return -EINVAL;
> +	}
> +
> +	return sprintf(buf, "%u\n", test_bit(bit, &trigger_data->mode));

sysfs_emit() for all new sysfs attributes please.

> +}
> +
> +static ssize_t ledtrig_tty_attr_store(struct device *dev, const char *buf,
> +	size_t size, enum led_trigger_tty_modes 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 TRIGGER_TTY_RX:
> +	case TRIGGER_TTY_TX:
> +	case TRIGGER_TTY_CTS:
> +	case TRIGGER_TTY_DSR:
> +	case TRIGGER_TTY_CAR:
> +	case TRIGGER_TTY_RNG:
> +		bit = attr;
> +		break;
> +	default:
> +		return -EINVAL;
> +	}
> +
> +	if (state)
> +		set_bit(bit, &trigger_data->mode);
> +	else
> +		clear_bit(bit, &trigger_data->mode);

I think your test of "state" here is wrong, if you write in "40000" you
are treating it as "1", which I don't think you want, right?

thanks,

greg k-h
Florian Eckert Oct. 22, 2023, 10:24 a.m. UTC | #2
On 2023-10-21 18:07, Greg KH wrote:
>> diff --git a/drivers/leds/trigger/ledtrig-tty.c 
>> b/drivers/leds/trigger/ledtrig-tty.c
>> index 8ae0d2d284af..6a96439a7e55 100644
>> --- a/drivers/leds/trigger/ledtrig-tty.c
>> +++ b/drivers/leds/trigger/ledtrig-tty.c
>> @@ -16,6 +16,24 @@ struct ledtrig_tty_data {
>>  	const char *ttyname;
>>  	struct tty_struct *tty;
>>  	int rx, tx;
>> +	unsigned long mode;
> 
> Why is mode "unsigned long" when the tty layer treats it as an int?  
> And
> really, this should be set to an explit size, u32 perhaps?  Or am I
> confused as to exactly what this is?

This is about the line state that the LED should show "altogether".
All states that the LED is to display are stored here.

For example:
Via the sysfs of the LED I can set the flags rx, tx and line_cts to
a "not" zero value. That means that the led is enable if the CTS of the
tty ist set, and the LED flashes if rx/tx data are transmitted via
this tty.

Therefore, the bits 0 (TRIGGER_TTY_RX), 1 (TRIGGER_TTY_TX) and
2 (TRIGGER_TTY_CTS) are set in the variable. As defined in the
enum led_trigger_tty_modes

I think I have not chosen the correct name for the variable there.
Maybe line_state, would be a better choice?

>> +};
>> +
>> +enum led_trigger_tty_state {
>> +	TTY_LED_BLINK,
>> +	TTY_LED_ENABLE,
>> +	TTY_LED_DISABLE,
>> +};
>> +
>> +enum led_trigger_tty_modes {
>> +	TRIGGER_TTY_RX = 0,
>> +	TRIGGER_TTY_TX,
>> +	TRIGGER_TTY_CTS,
>> +	TRIGGER_TTY_DSR,
>> +	TRIGGER_TTY_CAR,
>> +	TRIGGER_TTY_RNG,
>> +	/* Keep last */
>> +	__TRIGGER_TTY_MAX,
>>  };
>> 
> 
> Oh wait, is "mode" this?  If so, why not define it as an enum?  Or if
> not, I'm totally confused as to what is going on here, sorry.

See explanation above. I can not set this to an enum because I could
set more then one Flag via the sysfs.

> 
>>  static void ledtrig_tty_restart(struct ledtrig_tty_data 
>> *trigger_data)
>> @@ -78,13 +96,106 @@ 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 led_trigger_tty_modes attr)
>> +{
>> +	struct ledtrig_tty_data *trigger_data = 
>> led_trigger_get_drvdata(dev);
>> +	int bit;
>> +
>> +	switch (attr) {
>> +	case TRIGGER_TTY_RX:
>> +	case TRIGGER_TTY_TX:
>> +	case TRIGGER_TTY_CTS:
>> +	case TRIGGER_TTY_DSR:
>> +	case TRIGGER_TTY_CAR:
>> +	case TRIGGER_TTY_RNG:
>> +		bit = attr;
>> +		break;
>> +	default:
>> +		return -EINVAL;
>> +	}
>> +
>> +	return sprintf(buf, "%u\n", test_bit(bit, &trigger_data->mode));
> 
> sysfs_emit() for all new sysfs attributes please.

Correct. Thanks for the hint will use sysf_emit() function in the next
patchset round.

> 
>> +}
>> +
>> +static ssize_t ledtrig_tty_attr_store(struct device *dev, const char 
>> *buf,
>> +	size_t size, enum led_trigger_tty_modes 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 TRIGGER_TTY_RX:
>> +	case TRIGGER_TTY_TX:
>> +	case TRIGGER_TTY_CTS:
>> +	case TRIGGER_TTY_DSR:
>> +	case TRIGGER_TTY_CAR:
>> +	case TRIGGER_TTY_RNG:
>> +		bit = attr;
>> +		break;
>> +	default:
>> +		return -EINVAL;
>> +	}
>> +
>> +	if (state)
>> +		set_bit(bit, &trigger_data->mode);
>> +	else
>> +		clear_bit(bit, &trigger_data->mode);
> 
> I think your test of "state" here is wrong, if you write in "40000" you
> are treating it as "1", which I don't think you want, right?

If I have understood your question correctly, then I would say that your
assumption is not correct. I just want to check here whether it is a 
number
greater than zero or not. If the number is greater than zero then the 
bit
should be set in the 'mode' variable of the struct and if it is zero 
then
it should be cleared.

The LED could indicate more then one state there. As described above.
This was requested by Uwe Kleine-König in the old v7 patch series [1].

Thanks for your review!

---
Florian

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

Patch

diff --git a/Documentation/ABI/testing/sysfs-class-led-trigger-tty b/Documentation/ABI/testing/sysfs-class-led-trigger-tty
index 2bf6b24e781b..08127b1a4602 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:		October 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:		October 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:		October 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:		October 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:		October 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:		October 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..6a96439a7e55 100644
--- a/drivers/leds/trigger/ledtrig-tty.c
+++ b/drivers/leds/trigger/ledtrig-tty.c
@@ -16,6 +16,24 @@  struct ledtrig_tty_data {
 	const char *ttyname;
 	struct tty_struct *tty;
 	int rx, tx;
+	unsigned long mode;
+};
+
+enum led_trigger_tty_state {
+	TTY_LED_BLINK,
+	TTY_LED_ENABLE,
+	TTY_LED_DISABLE,
+};
+
+enum led_trigger_tty_modes {
+	TRIGGER_TTY_RX = 0,
+	TRIGGER_TTY_TX,
+	TRIGGER_TTY_CTS,
+	TRIGGER_TTY_DSR,
+	TRIGGER_TTY_CAR,
+	TRIGGER_TTY_RNG,
+	/* Keep last */
+	__TRIGGER_TTY_MAX,
 };
 
 static void ledtrig_tty_restart(struct ledtrig_tty_data *trigger_data)
@@ -78,13 +96,106 @@  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 led_trigger_tty_modes attr)
+{
+	struct ledtrig_tty_data *trigger_data = led_trigger_get_drvdata(dev);
+	int bit;
+
+	switch (attr) {
+	case TRIGGER_TTY_RX:
+	case TRIGGER_TTY_TX:
+	case TRIGGER_TTY_CTS:
+	case TRIGGER_TTY_DSR:
+	case TRIGGER_TTY_CAR:
+	case TRIGGER_TTY_RNG:
+		bit = attr;
+		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 led_trigger_tty_modes 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 TRIGGER_TTY_RX:
+	case TRIGGER_TTY_TX:
+	case TRIGGER_TTY_CTS:
+	case TRIGGER_TTY_DSR:
+	case TRIGGER_TTY_CAR:
+	case TRIGGER_TTY_RNG:
+		bit = attr;
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	if (state)
+		set_bit(bit, &trigger_data->mode);
+	else
+		clear_bit(bit, &trigger_data->mode);
+
+	return size;
+}
+
+#define DEFINE_TTY_TRIGGER(trigger_name, trigger) \
+	static ssize_t trigger_name##_show(struct device *dev, \
+		struct device_attribute *attr, char *buf) \
+	{ \
+		return ledtrig_tty_attr_show(dev, buf, trigger); \
+	} \
+	static ssize_t trigger_name##_store(struct device *dev, \
+		struct device_attribute *attr, const char *buf, size_t size) \
+	{ \
+		return ledtrig_tty_attr_store(dev, buf, size, trigger); \
+	} \
+	static DEVICE_ATTR_RW(trigger_name)
+
+DEFINE_TTY_TRIGGER(rx, TRIGGER_TTY_RX);
+DEFINE_TTY_TRIGGER(tx, TRIGGER_TTY_TX);
+DEFINE_TTY_TRIGGER(line_cts, TRIGGER_TTY_CTS);
+DEFINE_TTY_TRIGGER(line_dsr, TRIGGER_TTY_DSR);
+DEFINE_TTY_TRIGGER(line_car, TRIGGER_TTY_CAR);
+DEFINE_TTY_TRIGGER(line_rng, TRIGGER_TTY_RNG);
+
+static bool ledtrig_tty_evaluate(struct ledtrig_tty_data *trigger_data,
+		int flag)
+{
+	int status;
+
+	status = tty_get_tiocm(trigger_data->tty);
+
+	if (status >= 0)
+		return (status & flag);
+
+	return false;
+}
+
 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;
+	unsigned long interval = LEDTRIG_TTY_INTERVAL;
 	struct serial_icounter_struct icount;
+	enum led_trigger_tty_state state;
+	int current_brightness;
 	int ret;
 
+	state = TTY_LED_DISABLE;
 	mutex_lock(&trigger_data->mutex);
 
 	if (!trigger_data->ttyname) {
@@ -115,22 +226,71 @@  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(TRIGGER_TTY_CTS, &trigger_data->mode)) {
+		if (ledtrig_tty_evaluate(trigger_data, TIOCM_CTS))
+			state = TTY_LED_ENABLE;
+	}
+
+	if (test_bit(TRIGGER_TTY_DSR, &trigger_data->mode)) {
+		if (ledtrig_tty_evaluate(trigger_data, TIOCM_DSR))
+			state = TTY_LED_ENABLE;
 	}
 
-	if (icount.rx != trigger_data->rx ||
-	    icount.tx != trigger_data->tx) {
-		unsigned long interval = LEDTRIG_TTY_INTERVAL;
+	if (test_bit(TRIGGER_TTY_CAR, &trigger_data->mode)) {
+		if (ledtrig_tty_evaluate(trigger_data, TIOCM_CAR))
+			state = TTY_LED_ENABLE;
+	}
+
+	if (test_bit(TRIGGER_TTY_RNG, &trigger_data->mode)) {
+		if (ledtrig_tty_evaluate(trigger_data, TIOCM_RNG))
+			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(TRIGGER_TTY_RX, &trigger_data->mode) ||
+	    test_bit(TRIGGER_TTY_TX, &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(TRIGGER_TTY_RX, &trigger_data->mode) &&
+		    (icount.tx != trigger_data->tx)) {
+			trigger_data->tx = icount.tx;
+			state = TTY_LED_BLINK;
+		}
+
+		if (test_bit(TRIGGER_TTY_TX, &trigger_data->mode) &&
+		    (icount.rx != trigger_data->rx)) {
+			trigger_data->rx = icount.rx;
+			state = TTY_LED_BLINK;
+		}
+	}
 
+	current_brightness = led_cdev->brightness;
+	if (current_brightness)
+		led_cdev->blink_brightness = current_brightness;
+
+	if (!led_cdev->blink_brightness)
+		led_cdev->blink_brightness = led_cdev->max_brightness;
+
+	switch (state) {
+	case TTY_LED_BLINK:
 		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, LED_OFF);
+		break;
 	}
 
 out:
@@ -141,6 +301,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 +319,10 @@  static int ledtrig_tty_activate(struct led_classdev *led_cdev)
 	if (!trigger_data)
 		return -ENOMEM;
 
+	/* Enable default rx/tx LED blink */
+	set_bit(TRIGGER_TTY_TX, &trigger_data->mode);
+	set_bit(TRIGGER_TTY_RX, &trigger_data->mode);
+
 	led_set_trigger_data(led_cdev, trigger_data);
 
 	INIT_DELAYED_WORK(&trigger_data->dwork, ledtrig_tty_work);