On Sun, 3 Apr 2011, Felipe Balbi wrote: > > Felipe, you're not using the word "short" correctly. A short packet is > > one that's smaller than the maxpacket size, whereas a short transfer is > > one that's smaller than the recipient expected. > > Yes, got that much ;-) > > > So, in your example, if the device expects to receive 3 * 512 bytes, > > and the host sends one packet containing 1024 bytes followed by another > > packet containing 512 bytes, it _isn't_ a short transfer -- the device > > receives exactly what it expects! True, the last packet is a short > > packet, but so what? > > If that's the case, we need to remove the "short_not_ok" flag from the > storage gadget. Looking into include/linux/usb/gadget.h that means short > _packets_ are treated as errors: > > 35 * @short_not_ok: When reading data, makes short packets be > 36 * treated as errors (queue stops advancing till cleanup). > > Either remove the flag, or change its sematics. > > $ git grep -e short_not_ok drivers/usb/gadget/*storage*.c > drivers/usb/gadget/f_mass_storage.c: bh->outreq->short_not_ok = 1; > drivers/usb/gadget/f_mass_storage.c: bh->outreq->short_not_ok = 1; > drivers/usb/gadget/f_mass_storage.c: bh->outreq->short_not_ok = 1; > drivers/usb/gadget/file_storage.c: bh->outreq->short_not_ok = 1; > drivers/usb/gadget/file_storage.c: bh->outreq->short_not_ok = 1; > drivers/usb/gadget/file_storage.c: bh->outreq->short_not_ok = 1; You're right; in these gadgets those flags aren't needed and should be removed. (In fact, one of them has been removed already in a patch submitted by Mian Yousaf Kaukab on March 24.) The gadgets do their own checking for short packets and don't need help from the UDC driver. This would have shown up as a bug in testing if a short OUT transfer had ever occurred -- which indicates that it is rare indeed. However other gadget drivers are likely to need the flag. g_file_storage (and g_mass_storage) manages its queues in a rather unusual manner. > > As for short transfers... In theory, if everything is working > > correctly, the Bulk-Only Transport never has short bulk-OUT transfers. > > (Of course, if something goes wrong then one could occur.) Still, I > > don't see why you're so anxious to avoid them. Short transfers are a > > fact of life, explicitly allowed for in the USB spec. Why worry about > > them? > > HW requests OUT transfers to be aligned on wMaxPacketSize. That doesn't seem to be related to my question: Why are you worrying about short transfers? > > > IMO, it's best to either use wMaxPacketSize as block size, or use "the > > > more common value of 2048", to avoid it for a few more USB revisions. > > > Isn't it bad to have the assumption that block size is 512 no matter > > > what ? > > > > What's bad about it? Virtually all the existing disk-like USB mass > > storage devices (i.e., pretty much everything except USB CDROM drives) > > use 512-byte blocks. I bet a lot of BIOSes won't be able to boot from > > a USB disk/flash drive with a block size other than 512. > > I see, then I need to find another way to make this work. > > But in this, then my patch in completely unnecessary and there's no need > to align the transfer on wMaxPacketSize at all. No, you still don't understand. If a particular bulk-OUT request is not the last one in a transfer then its length _must_ be a multiple of wMaxPacketSize. Otherwise the request will get an EOVERFLOW error. For example, let's say the host is going to send 2048 bytes, and the gadget decides to submit two bulk-OUT requests to receive the data: one for 1536 bytes and one for 512 bytes. The first 1024-byte packet will be received okay, but the second 1024-byte packet will overflow the 512 bytes remaining in the first request's transfer buffer. Alan Stern -- 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
