Re: [PATCH 3/3] usb console,usb-serial: pass initial console baudon to first tty open

From: Jason Wessel
Date: Tue Sep 22 2009 - 16:55:00 EST


Alan Cox wrote:
>
> I think the console should probably have a ktermios structure and that is
> something which in future could become more generic (arguably in a saner
> world the termios would belong to the tty_port in the end as and when
> everything is a tty port - which would make a whole ton of things a lot
> lot more logical)


What about something like the patch shown below?

I did come up with a question. I was curious where you envisioned the kfree() for the ktermios allocated in the usb serial?

Perhaps we don't care because it will never go away but I don't want to add a memory leak. Another option would to make the termios in the struct console not be a pointer, but that obviously makes the structure a bit bigger. Did you have a suggestion?

Thanks,
Jason.


---
drivers/char/tty_io.c | 2 ++
drivers/usb/serial/console.c | 25 +++++++++++++------------
drivers/usb/serial/usb-serial.c | 15 +++++++++++++--
include/linux/console.h | 1 +
include/linux/tty_driver.h | 1 +
include/linux/usb/serial.h | 2 +-
6 files changed, 31 insertions(+), 15 deletions(-)

--- a/drivers/usb/serial/console.c
+++ b/drivers/usb/serial/console.c
@@ -66,7 +66,7 @@ static int usb_console_setup(struct cons
struct usb_serial_port *port;
int retval;
struct tty_struct *tty = NULL;
- struct ktermios *termios = NULL, dummy;
+ struct ktermios dummy;

dbg("%s", __func__);

@@ -141,14 +141,17 @@ static int usb_console_setup(struct cons
goto reset_open_count;
}
kref_init(&tty->kref);
- termios = kzalloc(sizeof(*termios), GFP_KERNEL);
- if (!termios) {
+
+ if (!co->termios)
+ co->termios = kzalloc(sizeof(struct ktermios),
+ GFP_KERNEL);
+ if (!co->termios) {
retval = -ENOMEM;
err("no more memory");
goto free_tty;
}
memset(&dummy, 0, sizeof(struct ktermios));
- tty->termios = termios;
+ tty->termios = co->termios;
tty_port_tty_set(&port->port, tty);
}

@@ -161,16 +164,15 @@ static int usb_console_setup(struct cons

if (retval) {
err("could not open USB console port");
- goto free_termios;
+ goto free_port;
}

if (serial->type->set_termios) {
- termios->c_cflag = cflag;
- tty_termios_encode_baud_rate(termios, baud, baud);
+ co->termios->c_cflag = cflag;
+ tty_termios_encode_baud_rate(co->termios, baud, baud);
serial->type->set_termios(tty, port, &dummy);

tty_port_tty_set(&port->port, NULL);
- kfree(termios);
kfree(tty);
}
set_bit(ASYNCB_INITIALIZED, &port->port.flags);
@@ -180,13 +182,12 @@ static int usb_console_setup(struct cons
--port->port.count;
/* The console is special in terms of closing the device so
* indicate this port is now acting as a system console. */
- port->console = 1;
+ port->console = co;

mutex_unlock(&serial->disc_mutex);
return retval;

- free_termios:
- kfree(termios);
+ free_port:
tty_port_tty_set(&port->port, NULL);
free_tty:
kfree(tty);
@@ -312,7 +313,7 @@ void usb_serial_console_exit(void)
{
if (usbcons_info.port) {
unregister_console(&usbcons);
- usbcons_info.port->console = 0;
+ usbcons_info.port->console = NULL;
usbcons_info.port = NULL;
}
}
--- a/include/linux/console.h
+++ b/include/linux/console.h
@@ -104,6 +104,7 @@ struct console {
short flags;
short index;
int cflag;
+ struct ktermios *termios;
void *data;
struct console *next;
};
--- a/drivers/char/tty_io.c
+++ b/drivers/char/tty_io.c
@@ -1175,6 +1175,8 @@ int tty_init_termios(struct tty_struct *
memcpy(tp, &tty->driver->init_termios,
sizeof(struct ktermios));
tty->driver->termios[idx] = tp;
+ if (tty->ops->init_termios)
+ tty->ops->init_termios(tty);
}
tty->termios = tp;
tty->termios_locked = tp + 1;
--- a/include/linux/tty_driver.h
+++ b/include/linux/tty_driver.h
@@ -259,6 +259,7 @@ struct tty_operations {
unsigned int set, unsigned int clear);
int (*resize)(struct tty_struct *tty, struct winsize *ws);
int (*set_termiox)(struct tty_struct *tty, struct termiox *tnew);
+ void (*init_termios)(struct tty_struct *tty);
#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);
--- a/drivers/usb/serial/usb-serial.c
+++ b/drivers/usb/serial/usb-serial.c
@@ -35,6 +35,7 @@
#include <linux/serial.h>
#include <linux/usb.h>
#include <linux/usb/serial.h>
+#include <linux/console.h>
#include "pl2303.h"

/*
@@ -212,6 +213,7 @@ static int serial_install(struct tty_dri
if (!try_module_get(serial->type->driver.owner))
goto error_module_get;

+ tty->driver_data = port;
/* perform the standard setup */
retval = tty_init_termios(tty);
if (retval)
@@ -227,8 +229,6 @@ static int serial_install(struct tty_dri
if (serial->type->init_termios)
serial->type->init_termios(tty);

- tty->driver_data = port;
-
/* Final install (we use the default method) */
tty_driver_kref_get(driver);
tty->count++;
@@ -237,6 +237,7 @@ static int serial_install(struct tty_dri

error_get_interface:
error_init_termios:
+ tty->driver_data = NULL;
module_put(serial->type->driver.owner);
error_module_get:
error_no_port:
@@ -245,6 +246,15 @@ static int serial_install(struct tty_dri
return retval;
}

+static void serial_init_termios(struct tty_struct *tty)
+{
+ struct usb_serial_port *port = tty->driver_data;
+
+ if (port->console && port->console->termios)
+ memcpy(tty->driver->termios[tty->index],
+ port->console->termios, sizeof(struct ktermios));
+}
+
static int serial_open(struct tty_struct *tty, struct file *filp)
{
struct usb_serial_port *port = tty->driver_data;
@@ -1202,6 +1212,7 @@ static const struct tty_operations seria
.shutdown = serial_release,
.install = serial_install,
.proc_fops = &serial_proc_fops,
+ .init_termios = serial_init_termios,
};


--- a/include/linux/usb/serial.h
+++ b/include/linux/usb/serial.h
@@ -106,7 +106,7 @@ struct usb_serial_port {
struct work_struct work;
char throttled;
char throttle_req;
- char console;
+ struct console *console;
unsigned long sysrq; /* sysrq timeout */
struct device dev;
enum port_dev_state dev_state;
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/