On Fri, Aug 21, 2009 at 08:50:40PM +0200, ext David Brownell wrote: > On Friday 14 August 2009, Felipe Balbi wrote: > > I think I ran into a bug in your ring buffer handling on u_serial.c > > Not actually "my" ringbuffer code; I avoided changing that > from the original stuff beyond getting rid of needless mem > alloc calls. oops :-p git blame lied to me due to the file renaming :-p > > The problem is then we we try to transfer files of over 31k over an obex > > interface (probably acm and g_serial suffer the same) we get a short > > transfer (511) where we shouldn't get. > > Why "should" that not happen? Seems to me your host-side > read code is expecting N bytes will always pack into the > minimum number of packets; but your peripheral-side write > code is just doing standard TTY stuff, which doesn't try > to provide such guarantees. > > I just skimmed the CDC OBEX docs and don't see any way > to read them as requiring non-TTY packing. To the > contrary it's explicit that "the transport semantics are > as close as possible to a subset of ACM." And it talks > about layering over "TCP-like protocols" which, like > ACM, expose packet boundaries which are not relevant > to transport semantics. > > Which says to me the issue is your host side code. It > should be working with a byte stream model, where packet > boundaries do not matter. But it's treating them as > significant, and thus is wrongly getting upset ... still the 511-byte packet is considered short packet by the host side and thus end of transfer, no ? What should host do in that case ? kick another bulk in transfer ? > > It seems that the problem lies on the gs_buf_space_available(), more > > specifically related to that "-1" you added, I believe, to report > > "correctly" when you have an empty buffer, since: > > > > (buf_size + buf_get - buf_put) % buf_size > > > > would return 0 on that situation. > > That's fairly standard for ring buffers; it avoids an extra > state varible to distinguish whether "both pointers are the > same" means empty or full. That simplifies the logic. I understood that after reading the code more carefully. -- balbi -- 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
