diff mbox series

[v8,5/6] leds: ledtrig-tty: make rx tx activitate configurable

Message ID 20231109085038.371977-6-fe@dev.tdt.de
State Superseded
Headers show
Series ledtrig-tty: add additional tty state evaluation | expand

Commit Message

Florian Eckert Nov. 9, 2023, 8:50 a.m. UTC
Until now, the LED blinks when data is sent via the tty (rx/tx).
This is not configurable.

This change adds the possibility to make the indication for the direction
of the transmitted data independently controllable via the new rx and
tx sysfs entries.

- 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.

This new sysfs entry are on by default. Thus the trigger behaves as
before this change.

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

Comments

Greg Kroah-Hartman Nov. 23, 2023, 2:12 p.m. UTC | #1
On Thu, Nov 09, 2023 at 09:50:37AM +0100, Florian Eckert wrote:
> Until now, the LED blinks when data is sent via the tty (rx/tx).
> This is not configurable.
> 
> This change adds the possibility to make the indication for the direction
> of the transmitted data independently controllable via the new rx and
> tx sysfs entries.
> 
> - 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.
> 
> This new sysfs entry are on by default. Thus the trigger behaves as
> before this change.
> 
> Signed-off-by: Florian Eckert <fe@dev.tdt.de>
> ---
>  .../ABI/testing/sysfs-class-led-trigger-tty   |  16 +++
>  drivers/leds/trigger/ledtrig-tty.c            | 124 ++++++++++++++++--
>  2 files changed, 130 insertions(+), 10 deletions(-)
> 
> diff --git a/Documentation/ABI/testing/sysfs-class-led-trigger-tty b/Documentation/ABI/testing/sysfs-class-led-trigger-tty
> index 2bf6b24e781b..504dece151b8 100644
> --- a/Documentation/ABI/testing/sysfs-class-led-trigger-tty
> +++ b/Documentation/ABI/testing/sysfs-class-led-trigger-tty
> @@ -4,3 +4,19 @@ KernelVersion:	5.10
>  Contact:	linux-leds@vger.kernel.org
>  Description:
>  		Specifies the tty device name of the triggering tty
> +
> +What:		/sys/class/leds/<led>/rx
> +Date:		February 2024
> +KernelVersion:	6.8
> +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:		February 2024
> +KernelVersion:	6.8
> +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.
> diff --git a/drivers/leds/trigger/ledtrig-tty.c b/drivers/leds/trigger/ledtrig-tty.c
> index 3badf74fa420..1a40a78bf1ee 100644
> --- a/drivers/leds/trigger/ledtrig-tty.c
> +++ b/drivers/leds/trigger/ledtrig-tty.c
> @@ -17,6 +17,19 @@ struct ledtrig_tty_data {
>  	const char *ttyname;
>  	struct tty_struct *tty;
>  	int rx, tx;
> +	bool mode_rx;
> +	bool mode_tx;
> +};
> +
> +/* Indicates which state the LED should now display */
> +enum led_trigger_tty_state {
> +	TTY_LED_BLINK,
> +	TTY_LED_DISABLE,
> +};
> +
> +enum led_trigger_tty_modes {
> +	TRIGGER_TTY_RX = 0,
> +	TRIGGER_TTY_TX,
>  };
>  
>  static int ledtrig_tty_waitforcompletion(struct device *dev)
> @@ -86,12 +99,82 @@ 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 completion;
> +	bool state;
> +
> +	reinit_completion(&trigger_data->sysfs);
> +	completion = ledtrig_tty_waitforcompletion(dev);
> +	if (completion < 0)
> +		return completion;

Why do you need to wait for anything just to read the sysfs file?  What
does that sync up with?  And why would it matter?

> +
> +	switch (attr) {
> +	case TRIGGER_TTY_RX:
> +		state = trigger_data->mode_rx;
> +		break;
> +	case TRIGGER_TTY_TX:
> +		state = trigger_data->mode_tx;
> +		break;
> +	}
> +
> +	return sysfs_emit(buf, "%u\n", state);
> +}
> +
> +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);
> +	int completion;
> +	bool state;
> +	int ret;
> +
> +	ret = kstrtobool(buf, &state);
> +	if (ret)
> +		return ret;
> +
> +	reinit_completion(&trigger_data->sysfs);
> +	completion = ledtrig_tty_waitforcompletion(dev);
> +	if (completion < 0)
> +		return completion;

Same here, why sync anything?

What am I missing as to why a completion is needed?

thanks,

greg k-h
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..504dece151b8 100644
--- a/Documentation/ABI/testing/sysfs-class-led-trigger-tty
+++ b/Documentation/ABI/testing/sysfs-class-led-trigger-tty
@@ -4,3 +4,19 @@  KernelVersion:	5.10
 Contact:	linux-leds@vger.kernel.org
 Description:
 		Specifies the tty device name of the triggering tty
+
+What:		/sys/class/leds/<led>/rx
+Date:		February 2024
+KernelVersion:	6.8
+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:		February 2024
+KernelVersion:	6.8
+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.
diff --git a/drivers/leds/trigger/ledtrig-tty.c b/drivers/leds/trigger/ledtrig-tty.c
index 3badf74fa420..1a40a78bf1ee 100644
--- a/drivers/leds/trigger/ledtrig-tty.c
+++ b/drivers/leds/trigger/ledtrig-tty.c
@@ -17,6 +17,19 @@  struct ledtrig_tty_data {
 	const char *ttyname;
 	struct tty_struct *tty;
 	int rx, tx;
+	bool mode_rx;
+	bool mode_tx;
+};
+
+/* Indicates which state the LED should now display */
+enum led_trigger_tty_state {
+	TTY_LED_BLINK,
+	TTY_LED_DISABLE,
+};
+
+enum led_trigger_tty_modes {
+	TRIGGER_TTY_RX = 0,
+	TRIGGER_TTY_TX,
 };
 
 static int ledtrig_tty_waitforcompletion(struct device *dev)
@@ -86,12 +99,82 @@  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 completion;
+	bool state;
+
+	reinit_completion(&trigger_data->sysfs);
+	completion = ledtrig_tty_waitforcompletion(dev);
+	if (completion < 0)
+		return completion;
+
+	switch (attr) {
+	case TRIGGER_TTY_RX:
+		state = trigger_data->mode_rx;
+		break;
+	case TRIGGER_TTY_TX:
+		state = trigger_data->mode_tx;
+		break;
+	}
+
+	return sysfs_emit(buf, "%u\n", state);
+}
+
+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);
+	int completion;
+	bool state;
+	int ret;
+
+	ret = kstrtobool(buf, &state);
+	if (ret)
+		return ret;
+
+	reinit_completion(&trigger_data->sysfs);
+	completion = ledtrig_tty_waitforcompletion(dev);
+	if (completion < 0)
+		return completion;
+
+	switch (attr) {
+	case TRIGGER_TTY_RX:
+		trigger_data->mode_rx = state;
+		break;
+	case TRIGGER_TTY_TX:
+		trigger_data->mode_tx = state;
+		break;
+	}
+
+	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);
+
 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 serial_icounter_struct icount;
 	struct led_classdev *led_cdev = trigger_data->led_cdev;
+	enum led_trigger_tty_state state = TTY_LED_DISABLE;
+	unsigned long interval = LEDTRIG_TTY_INTERVAL;
 	int ret;
 
 	if (!trigger_data->ttyname)
@@ -119,21 +202,36 @@  static void ledtrig_tty_work(struct work_struct *work)
 		trigger_data->tty = tty;
 	}
 
-	ret = tty_get_icount(trigger_data->tty, &icount);
-	if (ret)
-		goto out;
+	if (trigger_data->mode_rx || trigger_data->mode_tx) {
+		struct serial_icounter_struct icount;
 
-	if (icount.rx != trigger_data->rx ||
-	    icount.tx != trigger_data->tx) {
-		unsigned long interval = LEDTRIG_TTY_INTERVAL;
+		ret = tty_get_icount(trigger_data->tty, &icount);
+		if (ret)
+			goto out;
 
-		led_blink_set_oneshot(led_cdev, &interval, &interval, 0);
+		if (trigger_data->mode_tx && (icount.tx != trigger_data->tx)) {
+			trigger_data->tx = icount.tx;
+			state = TTY_LED_BLINK;
+		}
 
-		trigger_data->rx = icount.rx;
-		trigger_data->tx = icount.tx;
+		if (trigger_data->mode_rx && (icount.rx != trigger_data->rx)) {
+			trigger_data->rx = icount.rx;
+			state = TTY_LED_BLINK;
+		}
 	}
 
 out:
+	switch (state) {
+	case TTY_LED_BLINK:
+		led_blink_set_oneshot(led_cdev, &interval, &interval, 0);
+		break;
+	case TTY_LED_DISABLE:
+		fallthrough;
+	default:
+		led_set_brightness(led_cdev, LED_OFF);
+		break;
+	}
+
 	complete_all(&trigger_data->sysfs);
 	schedule_delayed_work(&trigger_data->dwork,
 			      msecs_to_jiffies(LEDTRIG_TTY_INTERVAL * 2));
@@ -141,6 +239,8 @@  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,
 	NULL
 };
 ATTRIBUTE_GROUPS(ledtrig_tty);
@@ -153,6 +253,10 @@  static int ledtrig_tty_activate(struct led_classdev *led_cdev)
 	if (!trigger_data)
 		return -ENOMEM;
 
+	/* Enable default rx/tx mode */
+	trigger_data->mode_rx = true;
+	trigger_data->mode_tx = true;
+
 	led_set_trigger_data(led_cdev, trigger_data);
 
 	INIT_DELAYED_WORK(&trigger_data->dwork, ledtrig_tty_work);