On Thu, Apr 27, 2000 at 01:00:03PM -0500, Al Borchers wrote:
> We have been developing a driver for a USB serial box, using
> the Keyspan code and the generic serial drivers as examples.
> Thanks for this great work!
You're very welcome. What device are you working on?
> In the process, we found a problem with our driver and it is also
> a problem with the Keyspan driver. There is a related problem
> in the generic serial driver. This is in 2.3.99-pre6.
>
> The keyspan_pda_write function sleeps if usb->status is -EINPROGRESS.
> However, keyspan_pda_write can be called on interrupt time from the
> n_tty.c line discipline code and then sleep causes a panic.
>
> To see the problem connect the Keyspan to an external terminal. Run
> "cat < /dev/ttyUSB0". On the external terminal type "hello" and
> press RETURN. You will see the hello and a carriage return
> echo to the external terminal, but no linefeed will be echoed. The
> system running the Keyspan driver will panic when RETURN is sent from
> the external terminal.
>
> Note if you change stty settings so that CARRIAGE RETURN/LINEFEED
> is not echoed there will not be a panic.
You are getting lucky. This is dependent on your device transmit buffer
size.
> When keyspan_pda_rx_interrupt gets the RETURN from the external
> terminal it calls tty_flip_buffer_push which queues up flush_to_ldisc.
> When this is run (on interrupt time) it calls tty->driver.put_char
> twice to echo CARRIAGE RETURN and LINEFEED. put_char calls
> keyspan_pda_write. The CARRIAGE RETURN gets through, but when
> keyspan_pda_write is called again to write the LINEFEED, urb->status
> is -EINPROGRESS and so it sleeps. Panic.
Eeek. Not good.
I personally don't think that the wait queue needs to be in the
keyspan_pda_write function, but since I didn't write that part of the
driver...
Patches to fix this are welcome, but I think the true answer to the
problem would be to do as the other usb-serial drivers do. More
reasoning behind this below:
> The generic serial driver simply returns 0 when it sees -EINPROGRESS;
> this means, in this situation, that the put_char for the LINEFEED will
> simply fail and no LINEFEED will be echoed.
This is true, BUT I will argue that echo is not a true test. I have some
scripts and other test programs here that I (and others) have developed
to test out the usb-serial drivers. You are welcome to them.
The thing is that the generic serial driver SHOULD return -EINPROGRESS
when it can't write the data out. Later, when the write finishes, the
tty layer is called again, and it attempts the write again, IF the device
isn't closed. The problem is that echo closes the device after it
finishes (at least I think this is the problem).
If you shove a ton of data through the port, using the -EINPROGRESS
code, no data gets dropped. It's up to the calling program to resend any
data that doesn't get written to the port (by looking at the actual
amount of data that was written during the write.) Maybe echo does not
do this. Try using minicom for a better tool that pays attention to the
serial flow control and other stuff.
Since the visor, generic, and another driver that I have written all use
this, and I have not had any problems with the data being sent, I don't
think it is an issue.
> It seems to me that the best possible solution is to have a write
> buffer in the driver and hope there is room to stuff characters in
> the buffer when -EINPROGRESS is set and in_interrupt() is true.
> However, this could still fail when the buffer is full.
>
> Scheduling the write for later might write data out of order.
Just adding another write queue to the usb-serial driver is adding
unneeded complexity to the driver (the tty layer already has a write
queue for this very purpose.)
> Any suggestions for other ways around this problem?
Change your test tools, or increase the transmit buffer on your usb to
serial converter :)
Did this help?
greg k-h
greg@(kroah|wirex).com
---------------------------------------------------------------------
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
