Re: [RFC/PATCH 1/2] usb: gadget: u_char: introduce chardev abstraction layer

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tuesday 09 March 2010, Greg KH wrote:
> On Tue, Mar 09, 2010 at 05:16:40PM +0200, Felipe Balbi wrote:
> > On Tue, Mar 09, 2010 at 06:54:30AM -0800, Greg KH wrote:
> > > Why is this needed?  Why not just use the serial driver instead?  That
> > > should provide you with exactly the same end result, right?
> > 
> > we experienced several problems with tty layer. The biggest of them is
> > a race between flush_to_ldisc() and ->receiv_buf() which makes the line
> > discipline (I guess only n_tty) loose bytes in some circumstances.
> 
> That sounds like a bug we need to fix then, right?

That was my reaction too.  (Long time since I looked at this, but I
recall problems there in the past...)


ISTR when writing u_serial that the TTY layer was still in flux, so
it was unclear how to do some things that should have been routine.
Various buffering policies did not seem to be stable.  I recall that
the original serial gadget code had a bunch of cases where it would
lose when trying to push a lot of data ... I fixed quite a few bugs
in such logic, but don't think I got all of them ... many of the
TTY layer policies were evolving at the time.

Locking in the TTY layer was another thing that kept getting redefined.
(I wouldn't say "clarified"; the observed behavior was changing.)


> > On top of that there's also the performance issues with tty. the
> > u_serial.c forces us to push each bMaxPacketSize packet to the
> > controller driver which prevents us from actually getting some good deal
> > out of the DMA engine.

Purely a TX side issue?  ISTR various issues there.  Recall there are a
LOT of layers of buffering here, on both sides.  Weren't there situations
where packetization is visible on the other end?

And at some level, the underlying flow control is always in terms of
USB packets, so trying to use some larger unit would be a lose...


> > With u_char.c we are unloading based on what's in 
> > the kfifo right now (maybe we should use PAGE_SIZE transfers since our
> > kfifo is allocated using vmalloc()), and we can easily optimize that on
> > a per-product basis.
> 
> Then why not just fix u_serial.c to push the same ammount of data?
> 
> > Performance is a key concept here; specially when we need to flash
> > hundreds of thousands of devices in a production line. Every milisecond
> > counts.
> 
> Sure, I understand, but it seems like it is easier to fix the existing
> code for these minor things, than to write a whole new driver, right?
> 
> If you do that, then the existing users will also benifit from those
> changes.

Right ...

- Dave



> 
> thanks,
> 
> greg k-h
> 
> 


--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux