Am 12.02.2012 14:01, schrieb Jiri Slaby: > This and the ioctl change above fullfils the subject of the patch in no > way. Do this fix and the cleanup above separately, please.
Fair point.
>
>>> @@ -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.
Okay.
>>> +{
>>> + 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?
The old driver set tty->driver_data to NULL.
I guess we can remove it.
>>> + 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.
Will do.
>>> 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.
Okay.
>>> + return line_install(driver, tty, &serial_lines[tty->index]);
>>> + else
>>> + return -ENODEV;
>>> +}
>
> thanks,
Thanks,
//richard
signature.asc
Description: OpenPGP digital signature
------------------------------------------------------------------------------ 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
