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@xxxxxxxxxx>
> ---
> .../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@xxxxxxxxxxxxxxx
> 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
