[PATCH 00/80] TTY updates for 2.6.28 - Kernel

This is a discussion on [PATCH 00/80] TTY updates for 2.6.28 - Kernel ; From: Alan Cox get_current_tty now does internal locking and returns a referenced object, thus our use of tty_mutex here can go away. Signed-off-by: Alan Cox --- drivers/s390/char/fs3270.c | 6 +----- 1 files changed, 1 insertions(+), 5 deletions(-) diff --git a/drivers/s390/char/fs3270.c ...

+ Reply to Thread
Page 4 of 4 FirstFirst ... 2 3 4
Results 61 to 70 of 70

Thread: [PATCH 00/80] TTY updates for 2.6.28

  1. [PATCH 78/80] fs3270: remove extra locks

    From: Alan Cox

    get_current_tty now does internal locking and returns a referenced object,
    thus our use of tty_mutex here can go away.

    Signed-off-by: Alan Cox
    ---

    drivers/s390/char/fs3270.c | 6 +-----
    1 files changed, 1 insertions(+), 5 deletions(-)


    diff --git a/drivers/s390/char/fs3270.c b/drivers/s390/char/fs3270.c
    index 84fbc90..1227f45 100644
    --- a/drivers/s390/char/fs3270.c
    +++ b/drivers/s390/char/fs3270.c
    @@ -426,18 +426,14 @@ fs3270_open(struct inode *inode, struct file *filp)
    minor = iminor(filp->f_path.dentry->d_inode);
    /* Check for minor 0 multiplexer. */
    if (minor == 0) {
    - struct tty_struct *tty;
    - mutex_lock(&tty_mutex);
    - tty = get_current_tty();
    + struct tty_struct *tty = get_current_tty();
    if (!tty || tty->driver->major != IBM_TTY3270_MAJOR) {
    tty_kref_put(tty);
    - mutex_unlock(&tty_mutex);
    rc = -ENODEV;
    goto out;
    }
    minor = tty->index + RAW3270_FIRSTMINOR;
    tty_kref_put(tty);
    - mutex_unlock(&tty_mutex);
    }
    /* Check if some other program is already using fullscreen mode. */
    fp = (struct fs3270 *) raw3270_find_view(&fs3270_fn, minor);

    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  2. [PATCH 70/80] tty: some ICANON magic is in the wrong places

    From: Alan Cox

    Move the set up on ldisc change into the ldisc
    Move the INQ/OUTQ cases into the driver not in shared ioctl code where it
    gives bogus answers for other ldisc values

    Signed-off-by: Alan Cox
    ---

    drivers/bluetooth/hci_ldisc.c | 2 +-
    drivers/char/n_hdlc.c | 2 +-
    drivers/char/n_tty.c | 53 +++++++++++++++++++++++++++++++++++++++--
    drivers/char/tty_ioctl.c | 42 ++------------------------------
    include/linux/tty.h | 2 +-
    5 files changed, 56 insertions(+), 45 deletions(-)


    diff --git a/drivers/bluetooth/hci_ldisc.c b/drivers/bluetooth/hci_ldisc.c
    index 8dfcf77..4426bb5 100644
    --- a/drivers/bluetooth/hci_ldisc.c
    +++ b/drivers/bluetooth/hci_ldisc.c
    @@ -484,7 +484,7 @@ static int hci_uart_tty_ioctl(struct tty_struct *tty, struct file * file,
    return -EUNATCH;

    default:
    - err = n_tty_ioctl(tty, file, cmd, arg);
    + err = n_tty_ioctl_helper(tty, file, cmd, arg);
    break;
    };

    diff --git a/drivers/char/n_hdlc.c b/drivers/char/n_hdlc.c
    index 69ec639..bacb3e2 100644
    --- a/drivers/char/n_hdlc.c
    +++ b/drivers/char/n_hdlc.c
    @@ -764,7 +764,7 @@ static int n_hdlc_tty_ioctl(struct tty_struct *tty, struct file *file,
    break;

    default:
    - error = n_tty_ioctl (tty, file, cmd, arg);
    + error = n_tty_ioctl_helper(tty, file, cmd, arg);
    break;
    }
    return error;
    diff --git a/drivers/char/n_tty.c b/drivers/char/n_tty.c
    index 708c2b1..b4f5dcc 100644
    --- a/drivers/char/n_tty.c
    +++ b/drivers/char/n_tty.c
    @@ -1011,8 +1011,20 @@ int is_ignored(int sig)

    static void n_tty_set_termios(struct tty_struct *tty, struct ktermios *old)
    {
    - if (!tty)
    - return;
    + int canon_change = 1;
    + BUG_ON(!tty);
    +
    + if (old)
    + canon_change = (old->c_lflag ^ tty->termios->c_lflag) & ICANON;
    + if (canon_change) {
    + memset(&tty->read_flags, 0, sizeof tty->read_flags);
    + tty->canon_head = tty->read_tail;
    + tty->canon_data = 0;
    + tty->erasing = 0;
    + }
    +
    + if (canon_change && !L_ICANON(tty) && tty->read_cnt)
    + wake_up_interruptible(&tty->read_wait);

    tty->icanon = (L_ICANON(tty) != 0);
    if (test_bit(TTY_HW_COOK_IN, &tty->flags)) {
    @@ -1573,6 +1585,43 @@ static unsigned int normal_poll(struct tty_struct *tty, struct file *file,
    return mask;
    }

    +static unsigned long inq_canon(struct tty_struct *tty)
    +{
    + int nr, head, tail;
    +
    + if (!tty->canon_data || !tty->read_buf)
    + return 0;
    + head = tty->canon_head;
    + tail = tty->read_tail;
    + nr = (head - tail) & (N_TTY_BUF_SIZE-1);
    + /* Skip EOF-chars.. */
    + while (head != tail) {
    + if (test_bit(tail, tty->read_flags) &&
    + tty->read_buf[tail] == __DISABLED_CHAR)
    + nr--;
    + tail = (tail+1) & (N_TTY_BUF_SIZE-1);
    + }
    + return nr;
    +}
    +
    +static int n_tty_ioctl(struct tty_struct *tty, struct file *file,
    + unsigned int cmd, unsigned long arg)
    +{
    + int retval;
    +
    + switch (cmd) {
    + case TIOCOUTQ:
    + return put_user(tty_chars_in_buffer(tty), (int __user *) arg);
    + case TIOCINQ:
    + retval = tty->read_cnt;
    + if (L_ICANON(tty))
    + retval = inq_canon(tty);
    + return put_user(retval, (unsigned int __user *) arg);
    + default:
    + return n_tty_ioctl_helper(tty, file, cmd, arg);
    + }
    +}
    +
    struct tty_ldisc_ops tty_ldisc_N_TTY = {
    .magic = TTY_LDISC_MAGIC,
    .name = "n_tty",
    diff --git a/drivers/char/tty_ioctl.c b/drivers/char/tty_ioctl.c
    index 14cc19c..a408c8e 100644
    --- a/drivers/char/tty_ioctl.c
    +++ b/drivers/char/tty_ioctl.c
    @@ -489,7 +489,6 @@ EXPORT_SYMBOL(tty_termios_hw_change);

    static void change_termios(struct tty_struct *tty, struct ktermios *new_termios)
    {
    - int canon_change;
    struct ktermios old_termios;
    struct tty_ldisc *ld;
    unsigned long flags;
    @@ -505,18 +504,6 @@ static void change_termios(struct tty_struct *tty, struct ktermios *new_termios)
    old_termios = *tty->termios;
    *tty->termios = *new_termios;
    unset_locked_termios(tty->termios, &old_termios, tty->termios_locked);
    - canon_change = (old_termios.c_lflag ^ tty->termios->c_lflag) & ICANON;
    - if (canon_change) {
    - memset(&tty->read_flags, 0, sizeof tty->read_flags);
    - tty->canon_head = tty->read_tail;
    - tty->canon_data = 0;
    - tty->erasing = 0;
    - }
    -
    - /* This bit should be in the ldisc code */
    - if (canon_change && !L_ICANON(tty) && tty->read_cnt)
    - /* Get characters left over from canonical mode. */
    - wake_up_interruptible(&tty->read_wait);

    /* See if packet mode change of state. */
    if (tty->link && tty->link->packet) {
    @@ -677,24 +664,6 @@ static int set_termiox(struct tty_struct *tty, void __user *arg, int opt)

    #endif

    -static unsigned long inq_canon(struct tty_struct *tty)
    -{
    - int nr, head, tail;
    -
    - if (!tty->canon_data || !tty->read_buf)
    - return 0;
    - head = tty->canon_head;
    - tail = tty->read_tail;
    - nr = (head - tail) & (N_TTY_BUF_SIZE-1);
    - /* Skip EOF-chars.. */
    - while (head != tail) {
    - if (test_bit(tail, tty->read_flags) &&
    - tty->read_buf[tail] == __DISABLED_CHAR)
    - nr--;
    - tail = (tail+1) & (N_TTY_BUF_SIZE-1);
    - }
    - return nr;
    -}

    #ifdef TIOCGETP
    /*
    @@ -1110,7 +1079,7 @@ int tty_perform_flush(struct tty_struct *tty, unsigned long arg)
    }
    EXPORT_SYMBOL_GPL(tty_perform_flush);

    -int n_tty_ioctl(struct tty_struct *tty, struct file *file,
    +int n_tty_ioctl_helper(struct tty_struct *tty, struct file *file,
    unsigned int cmd, unsigned long arg)
    {
    unsigned long flags;
    @@ -1148,13 +1117,6 @@ int n_tty_ioctl(struct tty_struct *tty, struct file *file,
    return 0;
    case TCFLSH:
    return tty_perform_flush(tty, arg);
    - case TIOCOUTQ:
    - return put_user(tty_chars_in_buffer(tty), (int __user *) arg);
    - case TIOCINQ:
    - retval = tty->read_cnt;
    - if (L_ICANON(tty))
    - retval = inq_canon(tty);
    - return put_user(retval, (unsigned int __user *) arg);
    case TIOCPKT:
    {
    int pktmode;
    @@ -1180,4 +1142,4 @@ int n_tty_ioctl(struct tty_struct *tty, struct file *file,
    return tty_mode_ioctl(tty, file, cmd, arg);
    }
    }
    -EXPORT_SYMBOL(n_tty_ioctl);
    +EXPORT_SYMBOL(n_tty_ioctl_helper);
    diff --git a/include/linux/tty.h b/include/linux/tty.h
    index 3c7c757..3b8121d 100644
    --- a/include/linux/tty.h
    +++ b/include/linux/tty.h
    @@ -466,7 +466,7 @@ static inline void tty_audit_push_task(struct task_struct *tsk,
    #endif

    /* tty_ioctl.c */
    -extern int n_tty_ioctl(struct tty_struct *tty, struct file *file,
    +extern int n_tty_ioctl_helper(struct tty_struct *tty, struct file *file,
    unsigned int cmd, unsigned long arg);

    /* serial.c */

    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  3. [PATCH 68/80] pty: simplify unix98 allocation

    From: Alan Cox

    We need both termios and termios_locked so allocate them as one

    Signed-off-by: Alan Cox
    ---

    drivers/char/pty.c | 18 +++++++-----------
    1 files changed, 7 insertions(+), 11 deletions(-)


    diff --git a/drivers/char/pty.c b/drivers/char/pty.c
    index 3c6b791..6d45827 100644
    --- a/drivers/char/pty.c
    +++ b/drivers/char/pty.c
    @@ -488,7 +488,6 @@ static void pty_unix98_shutdown(struct tty_struct *tty)
    {
    /* We have our own method as we don't use the tty index */
    kfree(tty->termios);
    - kfree(tty->termios_locked);
    }

    /* We have no need to install and remove our tty objects as devpts does all
    @@ -509,20 +508,17 @@ static int pty_unix98_install(struct tty_driver *driver, struct tty_struct *tty)
    }
    initialize_tty_struct(o_tty, driver->other, idx);

    - tty->termios = kmalloc(sizeof(struct ktermios), GFP_KERNEL);
    + tty->termios = kzalloc(sizeof(struct ktermios[2]), GFP_KERNEL);
    if (tty->termios == NULL)
    goto free_mem_out;
    *tty->termios = driver->init_termios;
    - tty->termios_locked = kzalloc(sizeof(struct ktermios), GFP_KERNEL);
    - if (tty->termios_locked == NULL)
    - goto free_mem_out;
    - o_tty->termios = kmalloc(sizeof(struct ktermios), GFP_KERNEL);
    + tty->termios_locked = tty->termios + 1;
    +
    + o_tty->termios = kzalloc(sizeof(struct ktermios[2]), GFP_KERNEL);
    if (o_tty->termios == NULL)
    goto free_mem_out;
    *o_tty->termios = driver->other->init_termios;
    - o_tty->termios_locked = kzalloc(sizeof(struct ktermios), GFP_KERNEL);
    - if (o_tty->termios_locked == NULL)
    - goto free_mem_out;
    + o_tty->termios_locked = o_tty->termios + 1;

    tty_driver_kref_get(driver->other);
    if (driver->subtype == PTY_TYPE_MASTER)
    @@ -540,10 +536,10 @@ static int pty_unix98_install(struct tty_driver *driver, struct tty_struct *tty)
    pty_count++;
    return 0;
    free_mem_out:
    - pty_unix98_shutdown(o_tty);
    + kfree(o_tty->termios);
    module_put(o_tty->driver->owner);
    free_tty_struct(o_tty);
    - pty_unix98_shutdown(tty);
    + kfree(tty->termios);
    return -ENOMEM;
    }


    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  4. [PATCH 79/80] fs3270: Correct error returns

    From: Alan Cox

    Drop the kernel lock further and also correct cases where we set rc to an
    error code, and then return 0

    Signed-off-by: Alan Cox
    ---

    drivers/s390/char/fs3270.c | 9 ++++-----
    1 files changed, 4 insertions(+), 5 deletions(-)


    diff --git a/drivers/s390/char/fs3270.c b/drivers/s390/char/fs3270.c
    index 1227f45..40759c3 100644
    --- a/drivers/s390/char/fs3270.c
    +++ b/drivers/s390/char/fs3270.c
    @@ -418,23 +418,22 @@ fs3270_open(struct inode *inode, struct file *filp)
    {
    struct fs3270 *fp;
    struct idal_buffer *ib;
    - int minor, rc;
    + int minor, rc = 0;

    if (imajor(filp->f_path.dentry->d_inode) != IBM_FS3270_MAJOR)
    return -ENODEV;
    - lock_kernel();
    minor = iminor(filp->f_path.dentry->d_inode);
    /* Check for minor 0 multiplexer. */
    if (minor == 0) {
    struct tty_struct *tty = get_current_tty();
    if (!tty || tty->driver->major != IBM_TTY3270_MAJOR) {
    tty_kref_put(tty);
    - rc = -ENODEV;
    - goto out;
    + return -ENODEV;
    }
    minor = tty->index + RAW3270_FIRSTMINOR;
    tty_kref_put(tty);
    }
    + lock_kernel();
    /* Check if some other program is already using fullscreen mode. */
    fp = (struct fs3270 *) raw3270_find_view(&fs3270_fn, minor);
    if (!IS_ERR(fp)) {
    @@ -476,7 +475,7 @@ fs3270_open(struct inode *inode, struct file *filp)
    filp->private_data = fp;
    out:
    unlock_kernel();
    - return 0;
    + return rc;
    }

    /*

    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  5. [PATCH 80/80] tty: rename the remaining oddly named n_tty functions

    From: Alan Cox

    Original idea for this from a patch by Rodolfo Giometti which merges various
    bits of PPS support

    Signed-off-by: Alan Cox
    ---

    drivers/char/n_tty.c | 26 +++++++++++++-------------
    1 files changed, 13 insertions(+), 13 deletions(-)


    diff --git a/drivers/char/n_tty.c b/drivers/char/n_tty.c
    index c0285b9..efbfe96 100644
    --- a/drivers/char/n_tty.c
    +++ b/drivers/char/n_tty.c
    @@ -26,7 +26,7 @@
    *
    * 2002/03/18 Implemented n_tty_wakeup to send SIGIO POLL_OUTs to
    * waiting writing processes-Sapan Bhatia .
    - * Also fixed a bug in BLOCKING mode where write_chan returns
    + * Also fixed a bug in BLOCKING mode where n_tty_write returns
    * EAGAIN
    */

    @@ -358,7 +358,7 @@ static int opost(unsigned char c, struct tty_struct *tty)
    * the simple cases normally found and helps to generate blocks of
    * symbols for the console driver and thus improve performance.
    *
    - * Called from write_chan under the tty layer write lock. Relies
    + * Called from n_tty_write under the tty layer write lock. Relies
    * on lock_kernel for the tty->column state.
    */

    @@ -1183,7 +1183,7 @@ static inline int input_available_p(struct tty_struct *tty, int amt)
    * @b: user data
    * @nr: size of data
    *
    - * Helper function to speed up read_chan. It is only called when
    + * Helper function to speed up n_tty_read. It is only called when
    * ICANON is off; it copies characters straight from the tty queue to
    * user space directly. It can be profitably called twice; once to
    * drain the space from the tail pointer to the (physical) end of the
    @@ -1250,7 +1250,7 @@ static int job_control(struct tty_struct *tty, struct file *file)
    if (file->f_op->write != redirected_tty_write &&
    current->signal->tty == tty) {
    if (!tty->pgrp)
    - printk(KERN_ERR "read_chan: no tty->pgrp!\n");
    + printk(KERN_ERR "n_tty_read: no tty->pgrp!\n");
    else if (task_pgrp(current) != tty->pgrp) {
    if (is_ignored(SIGTTIN) ||
    is_current_pgrp_orphaned())
    @@ -1265,7 +1265,7 @@ static int job_control(struct tty_struct *tty, struct file *file)


    /**
    - * read_chan - read function for tty
    + * n_tty_read - read function for tty
    * @tty: tty device
    * @file: file object
    * @buf: userspace buffer pointer
    @@ -1279,7 +1279,7 @@ static int job_control(struct tty_struct *tty, struct file *file)
    * This code must be sure never to sleep through a hangup.
    */

    -static ssize_t read_chan(struct tty_struct *tty, struct file *file,
    +static ssize_t n_tty_read(struct tty_struct *tty, struct file *file,
    unsigned char __user *buf, size_t nr)
    {
    unsigned char __user *b = buf;
    @@ -1481,7 +1481,7 @@ do_it_again:
    }

    /**
    - * write_chan - write function for tty
    + * n_tty_write - write function for tty
    * @tty: tty device
    * @file: file object
    * @buf: userspace buffer pointer
    @@ -1495,7 +1495,7 @@ do_it_again:
    * This code must be sure never to sleep through a hangup.
    */

    -static ssize_t write_chan(struct tty_struct *tty, struct file *file,
    +static ssize_t n_tty_write(struct tty_struct *tty, struct file *file,
    const unsigned char *buf, size_t nr)
    {
    const unsigned char *b = buf;
    @@ -1569,7 +1569,7 @@ break_out:
    }

    /**
    - * normal_poll - poll method for N_TTY
    + * n_tty_poll - poll method for N_TTY
    * @tty: terminal device
    * @file: file accessing it
    * @wait: poll table
    @@ -1582,7 +1582,7 @@ break_out:
    * Called without the kernel lock held - fine
    */

    -static unsigned int normal_poll(struct tty_struct *tty, struct file *file,
    +static unsigned int n_tty_poll(struct tty_struct *tty, struct file *file,
    poll_table *wait)
    {
    unsigned int mask = 0;
    @@ -1655,11 +1655,11 @@ struct tty_ldisc_ops tty_ldisc_N_TTY = {
    .close = n_tty_close,
    .flush_buffer = n_tty_flush_buffer,
    .chars_in_buffer = n_tty_chars_in_buffer,
    - .read = read_chan,
    - .write = write_chan,
    + .read = n_tty_read,
    + .write = n_tty_write,
    .ioctl = n_tty_ioctl,
    .set_termios = n_tty_set_termios,
    - .poll = normal_poll,
    + .poll = n_tty_poll,
    .receive_buf = n_tty_receive_buf,
    .write_wakeup = n_tty_write_wakeup
    };

    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  6. Re: [PATCH 72/80] tty: fix up gigaset a bit

    Am 13.10.2008 11:44 schrieb Alan Cox:
    > From: Alan Cox
    >
    > Stephen's fixes reminded me that gigaset is still rather broken so fix it up
    > a bit
    >
    > Signed-off-by: Alan Cox


    Acked-by: Tilman Schmidt

    Thanks, Alan, for these improvements.
    I'm now looking into the FIXME comments you added.

    > @@ -571,6 +571,7 @@ gigaset_tty_close(struct tty_struct *tty)
    > }
    >
    > /* prevent other callers from entering ldisc methods */
    > + /* FIXME: should use the tty state flags */
    > tty->disc_data = NULL;
    >
    > if (!cs->hw.ser)


    Do you know of an example line discipline that has got this right?
    My model for this code was drivers/net/ppp_async.c but now it seems
    that this was not as exemplary as I had hoped.

    > @@ -680,6 +675,8 @@ gigaset_tty_ioctl(struct tty_struct *tty, struct file *file,
    > /*
    > * Poll on the tty.
    > * Unused, always return zero.
    > + *
    > + * FIXME: should probably return an exception - especially on hangup
    > */
    > static unsigned int
    > gigaset_tty_poll(struct tty_struct *tty, struct file *file, poll_table*wait)


    Looking around, I see that many LDs don't even provide a poll method.
    So I'm thinking of just dropping this one. Would that be ok?

    Thanks,
    Tilman

    --
    Tilman Schmidt E-Mail: tilman@imap.cc
    Bonn, Germany
    Diese Nachricht besteht zu 100% aus wiederverwerteten Bits.
    Ungeöffnet mindestens haltbar bis: (siehe Rückseite)


    -----BEGIN PGP SIGNATURE-----
    Version: GnuPG v1.4.4 (MingW32)
    Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

    iD8DBQFI92LSQ3+did9BuFsRAsU5AJwJJxBS/7Jt2Taad78R6fmM7rpE6gCeIXyK
    2Ri88TzX1AhtLjY7uFj4jHA=
    =1lOU
    -----END PGP SIGNATURE-----


  7. Re: [PATCH 72/80] tty: fix up gigaset a bit

    > > /* prevent other callers from entering ldisc methods */
    > > + /* FIXME: should use the tty state flags */
    > > tty->disc_data = NULL;
    > >
    > > if (!cs->hw.ser)

    >
    > Do you know of an example line discipline that has got this right?
    > My model for this code was drivers/net/ppp_async.c but now it seems
    > that this was not as exemplary as I had hoped.


    If you want to know if the tty has been closed for further I/O then you
    can use test_bit(TTY_IO_ERROR, &tty->flags). This gets set at the right
    points on close/hangup/etc.

    > > @@ -680,6 +675,8 @@ gigaset_tty_ioctl(struct tty_struct *tty, struct file *file,
    > > /*
    > > * Poll on the tty.
    > > * Unused, always return zero.
    > > + *
    > > + * FIXME: should probably return an exception - especially on hangup
    > > */
    > > static unsigned int
    > > gigaset_tty_poll(struct tty_struct *tty, struct file *file, poll_table *wait)

    >
    > Looking around, I see that many LDs don't even provide a poll method.
    > So I'm thinking of just dropping this one. Would that be ok?


    PPP doesn't route characters via the tty interface while you seem to do
    so for the AT emulation so you do need it. In the hangup case you want to
    return an exception event too: probably you want

    POLLIN|POLLOUT|POLLERR|POLLHUP|POLLRDNORM|POLLWRNO RM

    for that case.

    Alan
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  8. Re: [PATCH 72/80] tty: fix up gigaset a bit

    Scripsit Alan Cox die 17.10.2008 13:40:
    >>> /* prevent other callers from entering ldisc methods */
    >>> + /* FIXME: should use the tty state flags */
    >>> tty->disc_data = NULL;
    >>>
    >>> if (!cs->hw.ser)

    >>
    >> Do you know of an example line discipline that has got this right?
    >> My model for this code was drivers/net/ppp_async.c but now it seems
    >> that this was not as exemplary as I had hoped.

    >
    > If you want to know if the tty has been closed for further I/O then you
    > can use test_bit(TTY_IO_ERROR, &tty->flags). This gets set at the right
    > points on close/hangup/etc.


    Ok, but where do you see a need for such a test? AFAICS I have covered
    all the eventualities without resorting to an explicit tty->flags test,
    and looking around among my LD brethren it looks like they came to the
    same conclusion. (Not that I'd want to pull the "a million flies can't
    be wrong" card. ;-) Can you point me to a place or situation where an
    additional test_bit(TTY_IO_ERROR, &tty->flags) would catch a case that
    would otherwise be handled incorrectly?

    >> Looking around, I see that many LDs don't even provide a poll method.
    >> So I'm thinking of just dropping this one. Would that be ok?

    >
    > PPP doesn't route characters via the tty interface while you seem to do
    > so for the AT emulation so you do need it.


    Actually I don't. The AT command interface is accessed through a
    separate tty interface created by the main module of the Gigaset driver
    in drivers/isdn/gigaset/interface.c.

    (The reason for that is that ser_gigaset is just one of three hardware
    backends for accessing Gigaset devices, and the only one with an
    existing tty device. I have to create my own AT command interface
    device for the other two, anyway. Specialcasing ser_gigaset in order to
    route that back to the tty device of the serial port would just
    complicate things needlessly.)

    So I think removing the poll method is the right thing to do. If you
    have no objections, I'll submit a patch doing just that. (Hoping to
    still make it for the .28 merge window.)

    Thanks,
    Tilman


    -----BEGIN PGP SIGNATURE-----
    Version: GnuPG v2.0.4-svn0 (GNU/Linux)
    Comment: Using GnuPG with SUSE - http://enigmail.mozdev.org

    iD8DBQFI+yf8Q3+did9BuFsRAuVGAJ9sRNSxr+IMCH9/d644MkpE7jcW5ACgm0zf
    /wLANUAzwjSVMtO31xu77LY=
    =20rt
    -----END PGP SIGNATURE-----


  9. Re: [PATCH 72/80] tty: fix up gigaset a bit

    > additional test_bit(TTY_IO_ERROR, &tty->flags) would catch a case that
    > would otherwise be handled incorrectly?


    Fiddling with tty->disc_data by hand probably currently does the right
    thing. I don't guarantee it will continue to do so after the BKL removal
    gets to ldisc setting.

    > So I think removing the poll method is the right thing to do. If you
    > have no objections, I'll submit a patch doing just that. (Hoping to
    > still make it for the .28 merge window.)


    If you really don't need the poll then it can go - for 2.6.29. The
    current code isn't wrong on a functional level it just returns unexpected
    results in the unplug case.
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  10. Re: [PATCH 72/80] tty: fix up gigaset a bit

    On 22.10.2008 11:00 Alan Cox wrote:
    > Fiddling with tty->disc_data by hand probably currently does the right
    > thing. I don't guarantee it will continue to do so after the BKL removal
    > gets to ldisc setting.


    I never expected you to guarantee anything. :-)
    Anyway, I'll continue keeping my eyes open for changes in the tty area.
    Should you come across a specific problem with the driver, feel free
    to drop me a line.

    > If you really don't need the poll then it can go - for 2.6.29. The
    > current code isn't wrong on a functional level it just returns unexpected
    > results in the unplug case.


    Ok, if it won't make it for 2.6.28 anyway then I don't have to rush
    and can take my time submitting this through the proper channels.

    Thanks,
    Tilman

    --
    Tilman Schmidt E-Mail: tilman@imap.cc
    Bonn, Germany
    Diese Nachricht besteht zu 100% aus wiederverwerteten Bits.
    Ungeffnet mindestens haltbar bis: (siehe Rckseite)


    -----BEGIN PGP SIGNATURE-----
    Version: GnuPG v1.4.4 (MingW32)
    Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

    iD8DBQFJAa+5Q3+did9BuFsRAgjDAJ9kUQnm2yvUwjOfvTEyeX v85ODW6gCcCGnZ
    AAcUcYXLRAw/4ByP3mSkSmo=
    =lz0p
    -----END PGP SIGNATURE-----


+ Reply to Thread
Page 4 of 4 FirstFirst ... 2 3 4