On 02/12/2012 01:24 AM, Richard Weinberger wrote:
>> @@ -228,92 +238,6 @@ void line_set_termios(struct tty_struct *tty, struct 
>> ktermios * old)
>>      /* nothing */
>>  }
>>  
>> -static const struct {
>> -    int  cmd;
>> -    char *level;
>> -    char *name;
>> -} tty_ioctls[] = {
>> -    /* don't print these, they flood the log ... */
>> -    { TCGETS,      NULL,       "TCGETS"      },
>> -    { TCSETS,      NULL,       "TCSETS"      },
>> -    { TCSETSW,     NULL,       "TCSETSW"     },
>> -    { TCFLSH,      NULL,       "TCFLSH"      },
>> -    { TCSBRK,      NULL,       "TCSBRK"      },
>> -
>> -    /* general tty stuff */
>> -    { TCSETSF,     KERN_DEBUG, "TCSETSF"     },
>> -    { TCGETA,      KERN_DEBUG, "TCGETA"      },
>> -    { TIOCMGET,    KERN_DEBUG, "TIOCMGET"    },
>> -    { TCSBRKP,     KERN_DEBUG, "TCSBRKP"     },
>> -    { TIOCMSET,    KERN_DEBUG, "TIOCMSET"    },
>> -
>> -    /* linux-specific ones */
>> -    { TIOCLINUX,   KERN_INFO,  "TIOCLINUX"   },
>> -    { KDGKBMODE,   KERN_INFO,  "KDGKBMODE"   },
>> -    { KDGKBTYPE,   KERN_INFO,  "KDGKBTYPE"   },
>> -    { KDSIGACCEPT, KERN_INFO,  "KDSIGACCEPT" },
>> -};
>> -
>> -int line_ioctl(struct tty_struct *tty, unsigned int cmd,
>> -                            unsigned long arg)
>> -{
>> -    int ret;
>> -    int i;
>> -
>> -    ret = 0;
>> -    switch(cmd) {
>> -#ifdef TIOCGETP
>> -    case TIOCGETP:
>> -    case TIOCSETP:
>> -    case TIOCSETN:
>> -#endif
>> -#ifdef TIOCGETC
>> -    case TIOCGETC:
>> -    case TIOCSETC:
>> -#endif
>> -#ifdef TIOCGLTC
>> -    case TIOCGLTC:
>> -    case TIOCSLTC:
>> -#endif
>> -    /* Note: these are out of date as we now have TCGETS2 etc but this
>> -       whole lot should probably go away */
>> -    case TCGETS:
>> -    case TCSETSF:
>> -    case TCSETSW:
>> -    case TCSETS:
>> -    case TCGETA:
>> -    case TCSETAF:
>> -    case TCSETAW:
>> -    case TCSETA:
>> -    case TCXONC:
>> -    case TCFLSH:
>> -    case TIOCOUTQ:
>> -    case TIOCINQ:
>> -    case TIOCGLCKTRMIOS:
>> -    case TIOCSLCKTRMIOS:
>> -    case TIOCPKT:
>> -    case TIOCGSOFTCAR:
>> -    case TIOCSSOFTCAR:
>> -            return -ENOIOCTLCMD;
>> -#if 0
>> -    case TCwhatever:
>> -            /* do something */
>> -            break;
>> -#endif
>> -    default:
>> -            for (i = 0; i < ARRAY_SIZE(tty_ioctls); i++)
>> -                    if (cmd == tty_ioctls[i].cmd)
>> -                            break;
>> -            if (i == ARRAY_SIZE(tty_ioctls)) {
>> -                    printk(KERN_ERR "%s: %s: unknown ioctl: 0x%x\n",
>> -                           __func__, tty->name, cmd);
>> -            }
>> -            ret = -ENOIOCTLCMD;
>> -            break;
>> -    }
>> -    return ret;
>> -}
>> -
>>  void line_throttle(struct tty_struct *tty)
>>  {
>>      struct line *line = tty->driver_data;
>> @@ -343,7 +267,7 @@ static irqreturn_t line_write_interrupt(int irq, void 
>> *data)
>>  {
>>      struct chan *chan = data;
>>      struct line *line = chan->line;
>> -    struct tty_struct *tty = line->tty;
>> +    struct tty_struct *tty = tty_port_tty_get(&line->port);
>>      int err;
>>  
>>      /*
>> @@ -354,6 +278,9 @@ static irqreturn_t line_write_interrupt(int irq, void 
>> *data)
>>      spin_lock(&line->lock);
>>      err = flush_buffer(line);
>>      if (err == 0) {
>> +            tty_kref_put(tty);
>> +
>> +            spin_unlock(&line->lock);

This and the ioctl change above fullfils the subject of the patch in no
way. Do this fix and the cleanup above separately, please.

>> @@ -404,27 +334,27 @@ int line_setup_irq(int fd, int input, int output, 
>> struct line *line, void *data)
>>   * first open or last close.  Otherwise, open and close just return.
>>   */
>>  
>> -int line_open(struct line *lines, struct tty_struct *tty)
>> +int line_open(struct tty_struct *tty, struct file *filp)
>>  {
>> -    struct line *line = &lines[tty->index];
>> -    int err = -ENODEV;
>> +    struct line *line = tty->driver_data;
>> +    return tty_port_open(&line->port, tty, filp);
>> +}
>>  
>> -    spin_lock(&line->count_lock);
>> -    if (!line->valid)
>> -            goto out_unlock;
>> +int line_install(struct tty_driver *driver, struct tty_struct *tty, struct 
>> line *line)

As it stands, it should be tty_port->ops->activate, not
tty->ops->install. Yes, you can still set driver_data and check for
multiple opens in install. Then use also tty_standard_install.

>> +{
>> +    int ret = tty_init_termios(tty);
>>  
>> -    err = 0;
>> -    if (line->count++)
>> -            goto out_unlock;
>> +    if (ret)
>> +            return ret;
>>  
>> -    BUG_ON(tty->driver_data);
>> +    tty_driver_kref_get(driver);
>> +    tty->count++;
>>      tty->driver_data = line;
>> -    line->tty = tty;
>> +    driver->ttys[tty->index] = tty;
>>  
>> -    spin_unlock(&line->count_lock);
>> -    err = enable_chan(line);
>> -    if (err) /* line_close() will be called by our caller */
>> -            return err;
>> +    ret = enable_chan(line);
>> +    if (ret)
>> +            return ret;
>>  
>>      INIT_DELAYED_WORK(&line->task, line_timer_cb);
>>  
>> @@ -437,48 +367,36 @@ int line_open(struct line *lines, struct tty_struct 
>> *tty)
>>                       &tty->winsize.ws_col);
>>  
>>      return 0;
>> -
>> -out_unlock:
>> -    spin_unlock(&line->count_lock);
>> -    return err;
>>  }
...
>> +void line_close(struct tty_struct *tty, struct file * filp)
>> +{
>> +    struct line *line = tty->driver_data;
>> +
>> +    if (!line)
>> +            return;

Unless you set tty->driver_data to NULL somewhere, this can never
happen. If you do, why -- this tends to be racy?

>> +    tty_port_close(&line->port, tty, filp);
>> +}
...
>> --- a/arch/um/drivers/ssl.c
>> +++ b/arch/um/drivers/ssl.c
>> @@ -92,6 +92,7 @@ static int ssl_remove(int n, char **error_out)
>>                         error_out);
>>  }
>>  
>> +#if 0

Hmm, remove unused code instead.

>>  static int ssl_open(struct tty_struct *tty, struct file *filp)
>>  {
>>      int err = line_open(serial_lines, tty);
...
>> @@ -124,8 +124,16 @@ void ssl_hangup(struct tty_struct *tty)
>>  }
>>  #endif
>>  
>> +static int ssl_install(struct tty_driver *driver, struct tty_struct *tty)
>> +{
>> +    if (tty->index < NR_PORTS)

Do you register more than NR_PORTS when allocating tty_driver? If not,
this test is always true. But presumably you won't need this hook anyway.

>> +            return line_install(driver, tty, &serial_lines[tty->index]);
>> +    else
>> +            return -ENODEV;
>> +}

thanks,
-- 
js
suse labs

------------------------------------------------------------------------------
Virtualization & Cloud Management Using Capacity Planning
Cloud computing makes use of virtualization - but cloud computing 
also focuses on allowing computing to be delivered as a service.
http://www.accelacomm.com/jaw/sfnl/114/51521223/
_______________________________________________
User-mode-linux-devel mailing list
User-mode-linux-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/user-mode-linux-devel

Reply via email to