LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: David Newall <davidn@davidnewall.com>
To: Greg KH <greg@kroah.com>
Cc: linux-usb@vger.kernel.org,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>
Subject: Re: Handshaking on USB serial devices
Date: Thu, 14 Feb 2008 19:55:44 +1030	[thread overview]
Message-ID: <47B40918.20206@davidnewall.com> (raw)
In-Reply-To: <20080214050211.GB1432@kroah.com>

Greg KH wrote:
> On Thu, Feb 14, 2008 at 01:15:37AM +1030, David Newall wrote:
>   
>> Consider a USB-attached serial port that is set to do RTS/CTS (or
>> DSR/DTR) handshaking: What stops the kernel sending more data to it when
>> the remote end lowers CTS (or DTR)?
>>     
>
> The tty layer should look at the proper flags and not send data on to
> the driver in this kind of instance.
>
> Is this not happening properly for you?  If so, which USB serial driver?
>   

It's not happening properly, for pl2303 (and, I admit it! on kernel
2.4.)  I think there's a widespread problem (i.e. with other drivers.) 
None of the drivers that I've examined check CTS or DTR; or, if they do,
I can't see where.  Likewise, I can't see it being done in n_tty nor
tty_io; not even in usbserial.  The best answer I can see is write_room,
but that doesn't seem quite right.  It seems that data written (via
*_write) will merrily be transmitted to the converter, even while the
remote end is signalling to stop, even if local hardware buffers fill.

I have a working solution for the 2.4 driver, but am looking towards
something for 2.6 and beyond.  The 2.4 solution is in two parts: first,
implement a write_room function, which returns 0 when transmission is
required to stop, which apparently gives a hint to the tty layer.  (NB,
doesn't help when using a different line discipline.)  The second part
is to return 0 (or maybe -EAGAIN?) in the write function when
transmission is required to stop.  This appears sound.  (Note that the
generic putchar does no error checking, and fails when the write URB is
busy.  That's a problem with a fairly obvious solution.)

The current 2.6 driver maintains it's own buffer.  I think that's a bad
thing: usbserial already buffers writes, and the extra buffer copy seems
unnecessary, however it does solve the putchar problem.  Buffered (i.e.
by the 2.6 series pl2303 driver) data is written as soon as practicable,
regardless of CTS/DTR.  The same general workaround, but placed in
pl2303_send seems correct to me; that is, stop submitting write urbs
when the remote end lowers CTS/DTR, and trigger the resume from the
interrupt callback (specifically in update_line_status.)

As I say, this appears to be a widespread problem.  I've looked at a
number of drivers and in none of them can I see anything to prevent
transmission when the remote end demands a stop.  I'm not sure that the
tty layer, which I think I said attempts to address the problem, albeit
it does a half-arsed job, is the right place to do hardware
handshaking.  Equally, I'm not sure that the wheel should be re-invented
for every driver, but that might be unavoidable.

To make it clear: Even aside from the buffer in 2.6's pl2303.c, there's
a race: An in-flight write URB can fill all hardware buffers, making
unsafe what previously appeared to be a safe write.  I think it's
essential to delay submission of the URB on a stop-transmit condition.

  reply	other threads:[~2008-02-14  9:25 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-02-13 14:45 David Newall
2008-02-14  5:02 ` Greg KH
2008-02-14  9:25   ` David Newall [this message]
2008-02-14 12:10     ` Alan Cox
2008-02-14 16:16       ` Gene Heskett
2008-02-14 16:31         ` Alan Cox
2008-02-14 17:55           ` Gene Heskett
2008-02-14 19:37             ` Alan Cox
2008-02-14 20:04               ` Gene Heskett
2008-02-14 20:52                 ` Greg KH
2008-02-14 21:32                   ` Gene Heskett
2008-02-15  5:08               ` David Newall
2008-02-14 22:39         ` Krzysztof Halasa
2008-02-14 23:09           ` Gene Heskett
2008-02-14 18:04       ` David Newall
2008-02-14 18:53         ` David Brownell
2008-02-14 19:36         ` Alan Cox
2008-02-21 15:22           ` David Newall
2008-02-21 15:15             ` Alan Cox
2008-02-21 19:35               ` David Newall
2008-02-21 20:58                 ` Greg KH
2008-02-14 20:47     ` Greg KH
2008-02-15  5:19       ` David Newall
2008-02-14 11:55   ` Alan Cox
     [not found] <9Wr5Z-7cw-1@gated-at.bofh.it>
     [not found] ` <9WSJ4-222-21@gated-at.bofh.it>
     [not found]   ` <9WTlN-2T0-7@gated-at.bofh.it>
     [not found]     ` <9WTYn-3Zb-29@gated-at.bofh.it>
2008-02-15 12:00       ` Bodo Eggert

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=47B40918.20206@davidnewall.com \
    --to=davidn@davidnewall.com \
    --cc=greg@kroah.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-usb@vger.kernel.org \
    --subject='Re: Handshaking on USB serial devices' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).