Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1664512 > unrolled thread

[PATCH v2 1/8] tty: add a poll() callback in struct tty_operations

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [PATCH v2 1/8] tty: add a poll() callback in struct tty_operations Tal Shorer <tal.shorer@gmail.com> - 2017-06-13 09:00 +0200
    Re: [PATCH v2 1/8] tty: add a poll() callback in struct  tty_operations Alan Cox <gnomes@lxorguk.ukuu.org.uk> - 2017-06-14 03:20 +0200
      Re: [PATCH v2 1/8] tty: add a poll() callback in struct tty_operations Tal Shorer <tal.shorer@gmail.com> - 2017-06-14 10:30 +0200
        Re: [PATCH v2 1/8] tty: add a poll() callback in struct tty_operations Tal Shorer <tal.shorer@gmail.com> - 2017-06-14 10:30 +0200
        Re: [PATCH v2 1/8] tty: add a poll() callback in struct  tty_operations Alan Cox <gnomes@lxorguk.ukuu.org.uk> - 2017-06-14 15:40 +0200
          Re: [PATCH v2 1/8] tty: add a poll() callback in struct tty_operations Tal Shorer <tal.shorer@gmail.com> - 2017-06-15 16:50 +0200

#1664512 — [PATCH v2 1/8] tty: add a poll() callback in struct tty_operations

If a tty driver wants to notify the user of some exceptional event,
such as a usb cdc acm device set_line_coding event, it needs a way to
modify the mask returned by poll() and possible also add wait queues.
In order to do that, we allow the driver to supply a poll() callback
of its own, which will be called in n_tty_poll().

Signed-off-by: Tal Shorer <tal.shorer@gmail.com>
---
 drivers/tty/n_tty.c        | 2 ++
 include/linux/tty_driver.h | 3 +++
 2 files changed, 5 insertions(+)

diff --git a/drivers/tty/n_tty.c b/drivers/tty/n_tty.c
index bdf0e6e..7af8c29 100644
--- a/drivers/tty/n_tty.c
+++ b/drivers/tty/n_tty.c
@@ -2394,6 +2394,8 @@ static unsigned int n_tty_poll(struct tty_struct *tty, struct file *file,
 			tty_chars_in_buffer(tty) < WAKEUP_CHARS &&
 			tty_write_room(tty) > 0)
 		mask |= POLLOUT | POLLWRNORM;
+	if (tty->ops->poll)
+		mask |= tty->ops->poll(tty, file, wait);
 	return mask;
 }
 
diff --git a/include/linux/tty_driver.h b/include/linux/tty_driver.h
index b742b5e..630ef03 100644
--- a/include/linux/tty_driver.h
+++ b/include/linux/tty_driver.h
@@ -243,6 +243,7 @@
 #include <linux/list.h>
 #include <linux/cdev.h>
 #include <linux/termios.h>
+#include <linux/poll.h>
 
 struct tty_struct;
 struct tty_driver;
@@ -285,6 +286,8 @@ struct tty_operations {
 	int (*set_termiox)(struct tty_struct *tty, struct termiox *tnew);
 	int (*get_icount)(struct tty_struct *tty,
 				struct serial_icounter_struct *icount);
+	unsigned int (*poll)(struct tty_struct *tty, struct file *file,
+				poll_table *wait);
 #ifdef CONFIG_CONSOLE_POLL
 	int (*poll_init)(struct tty_driver *driver, int line, char *options);
 	int (*poll_get_char)(struct tty_driver *driver, int line);
-- 
2.7.4

[toc] | [next] | [standalone]

#1665351 — Re: [PATCH v2 1/8] tty: add a poll() callback in struct tty_operations

On Tue, 13 Jun 2017 09:52:07 +0300
Tal Shorer <tal.shorer@gmail.com> wrote:

> If a tty driver wants to notify the user of some exceptional event,
> such as a usb cdc acm device set_line_coding event, it needs a way to
> modify the mask returned by poll() and possible also add wait queues.
> In order to do that, we allow the driver to supply a poll() callback
> of its own, which will be called in n_tty_poll().
> 
> Signed-off-by: Tal Shorer <tal.shorer@gmail.com>

You might be in any line discipline so you need to support that for each
ldisc that supports poll(). Also what do I do when I get an exception
event - what does it mean, how do I understand that, are you proposing a
standardized meaning ? Have you checked whether that conflicts with SuS
v3 or POSIX ? What will it do with existing code.

At the very least it probably has to be something you turn on, and you
might then want to model it on the pty/tty interface logic and extend
TIOCPKT a shade.

Alan

[toc] | [prev] | [next] | [standalone]

#1665553

On Wed, Jun 14, 2017 at 4:15 AM, Alan Cox <gnomes@lxorguk.ukuu.org.uk> wrote:
> On Tue, 13 Jun 2017 09:52:07 +0300
> Tal Shorer <tal.shorer@gmail.com> wrote:
>
>> If a tty driver wants to notify the user of some exceptional event,
>> such as a usb cdc acm device set_line_coding event, it needs a way to
>> modify the mask returned by poll() and possible also add wait queues.
>> In order to do that, we allow the driver to supply a poll() callback
>> of its own, which will be called in n_tty_poll().
>>
>> Signed-off-by: Tal Shorer <tal.shorer@gmail.com>
>
> You might be in any line discipline so you need to support that for each
> ldisc that supports poll(). Also what do I do when I get an exception
> event - what does it mean, how do I understand that, are you proposing a
> standardized meaning ? Have you checked whether that conflicts with SuS
> v3 or POSIX ? What will it do with existing code.
>
> At the very least it probably has to be something you turn on, and you
> might then want to model it on the pty/tty interface logic and extend
> TIOCPKT a shade.
That would cut it, but TIOCPKT is too coupled with having a linked tty.
I could make acm behave like a pty (accept TIOCPKT and issue the
ctrl_status bits), but for that I need n_tty to know that packet does
not always mean a linked tty is present, and that in case it isn't we
take our own ctrl_status bits instead of the link's. I could write a
small (inline?) function to fetch the correct ctrl_status bits and put
that in n_tty. Does that make sense?

[toc] | [prev] | [next] | [standalone]

#1665574

On Wed, Jun 14, 2017 at 11:20 AM, Tal Shorer <tal.shorer@gmail.com> wrote:
> On Wed, Jun 14, 2017 at 4:15 AM, Alan Cox <gnomes@lxorguk.ukuu.org.uk> wrote:
>> On Tue, 13 Jun 2017 09:52:07 +0300
>> Tal Shorer <tal.shorer@gmail.com> wrote:
>>
>>> If a tty driver wants to notify the user of some exceptional event,
>>> such as a usb cdc acm device set_line_coding event, it needs a way to
>>> modify the mask returned by poll() and possible also add wait queues.
>>> In order to do that, we allow the driver to supply a poll() callback
>>> of its own, which will be called in n_tty_poll().
>>>
>>> Signed-off-by: Tal Shorer <tal.shorer@gmail.com>
>>
>> You might be in any line discipline so you need to support that for each
>> ldisc that supports poll(). Also what do I do when I get an exception
>> event - what does it mean, how do I understand that, are you proposing a
>> standardized meaning ? Have you checked whether that conflicts with SuS
>> v3 or POSIX ? What will it do with existing code.
>>
>> At the very least it probably has to be something you turn on, and you
>> might then want to model it on the pty/tty interface logic and extend
>> TIOCPKT a shade.
> That would cut it, but TIOCPKT is too coupled with having a linked tty.
> I could make acm behave like a pty (accept TIOCPKT and issue the
> ctrl_status bits), but for that I need n_tty to know that packet does
> not always mean a linked tty is present, and that in case it isn't we
> take our own ctrl_status bits instead of the link's. I could write a
> small (inline?) function to fetch the correct ctrl_status bits and put
> that in n_tty. Does that make sense?
Or (maybe) better yet, I can do a hack and have the acm tty point to
itself as the link, which means n_tty doesn't have to change at all.
Any thoughts?

[toc] | [prev] | [next] | [standalone]

#1665837 — Re: [PATCH v2 1/8] tty: add a poll() callback in struct tty_operations

> That would cut it, but TIOCPKT is too coupled with having a linked tty.
> I could make acm behave like a pty (accept TIOCPKT and issue the
> ctrl_status bits), but for that I need n_tty to know that packet does
> not always mean a linked tty is present, and that in case it isn't we
> take our own ctrl_status bits instead of the link's. I could write a
> small (inline?) function to fetch the correct ctrl_status bits and put
> that in n_tty. Does that make sense?

I think that makes sense, and I would do the job properly rather than do
a hack with tty->link. Those hacks in the long term never work out the
best approach.

Alan

[toc] | [prev] | [next] | [standalone]

#1666767

On Wed, Jun 14, 2017 at 4:33 PM, Alan Cox <gnomes@lxorguk.ukuu.org.uk> wrote:
>> That would cut it, but TIOCPKT is too coupled with having a linked tty.
>> I could make acm behave like a pty (accept TIOCPKT and issue the
>> ctrl_status bits), but for that I need n_tty to know that packet does
>> not always mean a linked tty is present, and that in case it isn't we
>> take our own ctrl_status bits instead of the link's. I could write a
>> small (inline?) function to fetch the correct ctrl_status bits and put
>> that in n_tty. Does that make sense?
>
> I think that makes sense, and I would do the job properly rather than do
> a hack with tty->link. Those hacks in the long term never work out the
> best approach.
>
> Alan
Ok, so I'm doing that and everything is great until I got to actually
modifying tty->termios.
I need to modify it from interrupt context (the usb_request's
complete() callback), but modifying the termios requires a
rw_semaphore I can't take.
I could queue_work() to do it, but then I'd have to flush the work
from another non-sleepable context in acm_disconnect() (which runs
under a spinlock).
I can't change the semaphore to a spinlock because some drivers that
use it actually wanna sleep while holding it.
Any ideas?

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web