Re: [PATCH 8/11] usbserial: pl2303: Ports tty functions.

[Greg KH <gregkh@suse.de>]

On Sat, Jun 03, 2006 at 07:19:17PM -0300, Luiz Fernando N. Capitulino wrote:
> On Fri, 2 Jun 2006 15:44:35 -0700
> Greg KH <gregkh@suse.de> wrote:
> 
> | On Fri, Jun 02, 2006 at 03:41:21PM -0700, Pete Zaitcev wrote:
> | > On Fri, 2 Jun 2006 13:50:14 -0700, Greg KH <gregkh@suse.de> wrote:
> | > > On Fri, Jun 02, 2006 at 12:03:14AM -0300, Luiz Fernando N.Capitulino wrote:
> | > 
> | > > >   2. The new pl2303's set_termios() can (still) sleep. Serial Core's
> | > > >      documentation says that that method must not sleep, but I couldn't find
> | > > >      where in the Serial Core code it's called in atomic context. So, is this
> | > > >      still true? Isn't the Serial Core's documentation out of date?
> | > > 
> | > > If this is true then we should just stop the port right now, as the USB
> | > > devices can not handle this.  They need to be able to sleep to
> | > > accomplish this functionality.
> | > > 
> | > > Russell, is this a requirement of the serial layer?  Why?
> | > 
> | > Shouldn't it be all right to schedule the change at the moment of
> | > that call and have it happen later? Resisting a temptation to abuse
> | > keventd and schedule_work and using a tasklet may help with latency
> | > enough to make this tolerable.
> | 
> | Some devices require more than one usb message to set all of the proper
> | termios bits in the device.  Creating a way to queue them up and fire
> | them off later, and handle errors if something happened in the middle,
> | after we told userspace the termios change succeeded, might get quite
> | messy :(
> 
>  But set_termios() returns nothing, and look what termios
> man page says about tcsetattr() return value:
> 
> """
> Note that tcsetattr() returns success if any of the requested changes could
> be successfully carried out. Therefore, when making multiple changes it may be
> necessary to follow this call with a further call to tcgetattr() to check that
> all changes have been performed successfully.
> """

Good point, I forgot about that.

>  Also, why do they need to sleep? Did you note that my version of
> set_mctrl() is atomic?

Yes, that looks "atomic" in a way, but when the function returns, the
value is not really set.  It only happens some time in the future when
the urb completes (and hopefully it works, no retry is allowed...)

So it might be a bit "racy" :)

thanks,

greg k-h