On Mon, 10 Apr 2006, Paul Fulghum wrote:
> Try this all-in-one patch.
>
> Included:
> * usb serial_open fix (fix ENODEV)
> * usb_console_write fix (fix CR after LF)
> * ftdi patch (fix oops on tty struct == NULL)
> * ftdi uninitialized tmp_termios fix
> * remove __init from usb_console_setup (oops on late device install)
> * add usb_serial_console_disconnect (write errors after device removal)
Interestingly, as I received this email I was debugging a patch, where I
introduced a function with the same name:-) So, at least we agree on the
name:-)
Unfortunately, your patch doesn't work quite well.
1. It has a typo:
> void usb_serial_console_exit (void)
> {
> - unregister_console(&usbcons);
> + if (usbcons_info.port) {
> + unregister_console(&usbcons);
> + usbcons_info.port = NULL;
> + if (usbcons_info.port->open_count)
> + usbcons_info.port->open_count--;
> + }
You wanted to put the "= NULL" a bit later:-)
2. It looks like it introduces some race - during the boot the output on
the ttyUSB is interrupted for some time - about 1 second, then it goes on.
If you boot without the dongle and plug it in later, the system locks up
also within about a second.
3. If you pull the dongle out after booting with it, the same "ftdi_write
- failed submitting write urb, error -19" errors come, so, this is not
fixed either.
4. The whole design of the usb-console is not quite clear to me. In
principle, you can specify multiple "console=" parameters, like
"console=ttyS0,9600 console=tty1", but I don't know if one can do
"console=ttyS0,9600 console=ttyS1,38400"? For usb-serial console port[0]
is hard coded in. More than 1 console is impossible. But is it really the
right thing to unregister_console() on dongle-disconnect, as in:
> @@ -257,8 +278,23 @@ void usb_serial_console_init (int serial
> }
> }
>
> +/*
> + * called when disconnecting a port
> + * unregister console if necessary
> + */
> +void usb_serial_console_disconnect(struct usb_serial_port *port)
> +{
> + if (usbcons_info.port == port)
> + usb_serial_console_exit();
> +}
> +
> void usb_serial_console_exit (void)
> {
> - unregister_console(&usbcons);
> + if (usbcons_info.port) {
> + unregister_console(&usbcons);
> + usbcons_info.port = NULL;
> + if (usbcons_info.port->open_count)
> + usbcons_info.port->open_count--;
> + }
> }
>
? The whole concept of hot-(un)plugging of consoles is unclear to me:-)
Below is the patch I am currently testing. I can plug in the dongle after
boot without problem, but unplugging it still doesn't work:-( And I don't
understand why. It is clear, that it hangs because of recursion - it
prints to console and it fails and it prints an error message... I limited
the error messages to 5 and then I see just 5 of them and then I can debug
further. I implemented a usb_serial_console_disconnect() (a bit different
from yours - I didn't want to unregister_console()) and you can see where
and how I call it in usb_serial_disconnect(). And how I am trying to bail
out early from serial_write(), but somehow it doesn't work... This is the
output I am getting from my extended error printk: "ftdi_write - failed
submitting write urb, error -19, state 0, bus c7c39800", so, the sate is
indeed USB_STATE_NOTATTACHED, so, no idea why it still recurses... I'll
debug further.
Thanks
Guennadi
---
Guennadi Liakhovetski
Index: drivers/usb/serial/console.c
===================================================================
RCS file: /usr/src/cvs/linux-2_6/drivers/usb/serial/console.c,v
retrieving revision 1.1.1.3
diff -u -p -r1.1.1.3 console.c
--- drivers/usb/serial/console.c 13 Jan 2005 21:09:08 -0000 1.1.1.3
+++ drivers/usb/serial/console.c 10 Apr 2006 21:47:06 -0000
@@ -54,7 +54,7 @@ static struct console usbcons;
* serial.c code, except that the specifier is "ttyUSB" instead
* of "ttyS".
*/
-static int __init usb_console_setup(struct console *co, char *options)
+static int usb_console_setup(struct console *co, char *options)
{
struct usbcons_info *info = &usbcons_info;
int baud = 9600;
@@ -187,6 +187,7 @@ static int __init usb_console_setup(stru
/* set up the initial termios settings */
serial->type->set_termios(port, NULL);
+// serial->type->set_termios(port, NULL);
port->tty = NULL;
kfree (termios);
kfree (tty);
@@ -213,17 +214,38 @@ static void usb_console_write(struct con
if (!port->open_count) {
dbg ("%s - port not opened", __FUNCTION__);
- goto exit;
+ return;
}
- /* pass on to the driver specific version of this function if it is
available */
- if (serial->type->write)
- retval = serial->type->write(port, buf, count);
- else
- retval = usb_serial_generic_write(port, buf, count);
-
-exit:
- dbg("%s - return value (if we had one): %d", __FUNCTION__, retval);
+ while (count) {
+ unsigned int i;
+ unsigned int lf;
+ /* search for LF so we can insert CR if necessary */
+ for (i=0, lf=0 ; i < count ; i++) {
+ if (*(buf + i) == 10) {
+ lf = 1;
+ i++;
+ break;
+ }
+ }
+ /* pass on to the driver specific version of this function if
it is available */
+ if (serial->type->write)
+ retval = serial->type->write(port, buf, i);
+ else
+ retval = usb_serial_generic_write(port, buf, i);
+ dbg("%s - return value : %d", __FUNCTION__, retval);
+ if (lf) {
+ /* append CR after LF */
+ unsigned char cr = 13;
+ if (serial->type->write)
+ retval = serial->type->write(port, &cr, 1);
+ else
+ retval = usb_serial_generic_write(port, &cr, 1);
+ dbg("%s - return value : %d", __FUNCTION__, retval);
+ }
+ buf += i;
+ count -= i;
+ }
}
static struct console usbcons = {
@@ -234,6 +256,14 @@ static struct console usbcons = {
.index = -1,
};
+void usb_serial_console_disconnect(struct usb_serial *serial)
+{
+ if (serial && serial->port && serial->port[0] && serial->port[0] ==
usbcons_info.port) {
+ usbcons_info.port->open_count--;
+ usbcons_info.port = NULL;
+ }
+}
+
void usb_serial_console_init (int serial_debug, int minor)
{
debug = serial_debug;
Index: drivers/usb/serial/ftdi_sio.c
===================================================================
RCS file: /usr/src/cvs/linux-2_6/drivers/usb/serial/ftdi_sio.c,v
retrieving revision 1.1.1.16
diff -u -p -r1.1.1.16 ftdi_sio.c
--- drivers/usb/serial/ftdi_sio.c 30 Mar 2006 19:18:07 -0000 1.1.1.16
+++ drivers/usb/serial/ftdi_sio.c 10 Apr 2006 21:20:45 -0000
@@ -1254,7 +1254,6 @@ static void ftdi_shutdown (struct usb_se
static int ftdi_open (struct usb_serial_port *port, struct file *filp)
{ /* ftdi_open */
- struct termios tmp_termios;
struct usb_device *dev = port->serial->dev;
struct ftdi_private *priv = usb_get_serial_port_data(port);
unsigned long flags;
@@ -1264,8 +1263,8 @@ static int ftdi_open (struct usb_serial
dbg("%s", __FUNCTION__);
-
- port->tty->low_latency = (priv->flags & ASYNC_LOW_LATENCY) ? 1 : 0;
+ if (port->tty)
+ port->tty->low_latency = (priv->flags & ASYNC_LOW_LATENCY) ? 1
: 0;
/* No error checking for this (will get errors later anyway) */
/* See ftdi_sio.h for description of what is reset */
@@ -1279,7 +1278,8 @@ static int ftdi_open (struct usb_serial
This is same behaviour as serial.c/rs_open() - Kuba */
/* ftdi_set_termios will send usb control messages */
- ftdi_set_termios(port, &tmp_termios);
+ if (port->tty)
+ ftdi_set_termios(port, NULL);
/* FIXME: Flow control might be enabled, so it should be checked -
we have no control of defaults! */
@@ -1365,6 +1365,7 @@ static int ftdi_write (struct usb_serial
int data_offset ; /* will be 1 for the SIO and 0 otherwise */
int status;
int transfer_size;
+ static int err_cnt;
dbg("%s port %d, %d bytes", __FUNCTION__, port->number, count);
@@ -1435,7 +1436,11 @@ static int ftdi_write (struct usb_serial
status = usb_submit_urb(urb, GFP_ATOMIC);
if (status) {
- err("%s - failed submitting write urb, error %d", __FUNCTION__,
status);
+ if (err_cnt++ < 5) {
+ struct usb_device *dev = urb->dev;
+ err("%s - failed submitting write urb, error %d, state
%d, bus %p",
+ __FUNCTION__, status, dev ? dev->state : "-1000",
dev ? dev->bus : NULL);
+ }
count = status;
kfree (buffer);
}
@@ -1860,7 +1865,7 @@ static void ftdi_set_termios (struct usb
err("%s urb failed to set baudrate", __FUNCTION__);
}
/* Ensure RTS and DTR are raised when baudrate changed from 0 */
- if ((old_termios->c_cflag & CBAUD) == B0) {
+ if (!old_termios || (old_termios->c_cflag & CBAUD) == B0) {
set_mctrl(port, TIOCM_DTR | TIOCM_RTS);
}
}
Index: drivers/usb/serial/usb-serial.c
===================================================================
RCS file: /usr/src/cvs/linux-2_6/drivers/usb/serial/usb-serial.c,v
retrieving revision 1.1.1.12
diff -u -p -r1.1.1.12 usb-serial.c
--- drivers/usb/serial/usb-serial.c 30 Mar 2006 19:18:07 -0000 1.1.1.12
+++ drivers/usb/serial/usb-serial.c 10 Apr 2006 21:20:06 -0000
@@ -197,12 +197,12 @@ static int serial_open (struct tty_struc
++port->open_count;
- if (port->open_count == 1) {
+ /* set up our port structure making the tty driver
+ * remember our port object, and us it */
+ tty->driver_data = port;
+ port->tty = tty;
- /* set up our port structure making the tty driver
- * remember our port object, and us it */
- tty->driver_data = port;
- port->tty = tty;
+ if (port->open_count == 1) {
/* lock this module before we call it
* this may fail, which means we must bail out,
@@ -271,7 +271,7 @@ static int serial_write (struct tty_stru
struct usb_serial_port *port = tty->driver_data;
int retval = -EINVAL;
- if (!port)
+ if (!port || port->serial->dev->state == USB_STATE_NOTATTACHED)
goto exit;
dbg("%s - port %d, %d byte(s)", __FUNCTION__, port->number, count);
@@ -982,6 +982,7 @@ void usb_serial_disconnect(struct usb_in
struct device *dev = &interface->dev;
struct usb_serial_port *port;
+ usb_serial_console_disconnect(serial);
dbg ("%s", __FUNCTION__);
usb_set_intfdata (interface, NULL);
Index: drivers/usb/serial/usb-serial.h
===================================================================
RCS file: /usr/src/cvs/linux-2_6/drivers/usb/serial/usb-serial.h,v
retrieving revision 1.1.1.9
diff -u -p -r1.1.1.9 usb-serial.h
--- drivers/usb/serial/usb-serial.h 30 Mar 2006 19:18:07 -0000 1.1.1.9
+++ drivers/usb/serial/usb-serial.h 10 Apr 2006 21:14:44 -0000
@@ -248,9 +248,11 @@ extern int ezusb_set_reset (struct usb_s
#ifdef CONFIG_USB_SERIAL_CONSOLE
extern void usb_serial_console_init (int debug, int minor);
extern void usb_serial_console_exit (void);
+extern void usb_serial_console_disconnect(struct usb_serial *serial);
#else
static inline void usb_serial_console_init (int debug, int minor) { }
static inline void usb_serial_console_exit (void) { }
+static inline void usb_serial_console_disconnect(struct usb_serial *serial) {}
#endif
/* Functions needed by other parts of the usbserial core */
-------------------------------------------------------
This SF.Net email is sponsored by xPML, a groundbreaking scripting language
that extends applications into web and mobile media. Attend the live webcast
and join the prime developer group breaking into this new coding territory!
http://sel.as-us.falkag.net/sel?cmd=lnk&kid=110944&bid=241720&dat=121642
_______________________________________________
linux-usb-devel@lists.sourceforge.net
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
