2023-09-26 10:06:23

by Florian Eckert

[permalink] [raw]
Subject: [PATCH 0/2] ledtrig-tty: add new state evaluation

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

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


2023-09-26 10:07:09

by Florian Eckert

[permalink] [raw]
Subject: [PATCH 1/2] tty: add new helper function tty_get_mget

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

2023-09-26 10:14:41

by Jiri Slaby

[permalink] [raw]
Subject: Re: [PATCH 1/2] tty: add new helper function tty_get_mget

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

2023-09-26 12:07:52

by Florian Eckert

[permalink] [raw]
Subject: Re: [PATCH 1/2] tty: add new helper function tty_get_mget

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!