On Mon, Apr 04, 2011 at 11:39:49AM -0400, Alan Stern wrote:
> On Mon, 4 Apr 2011, Felipe Balbi wrote:
>
> > Like I said before: HW requests me to give lengths which are divisible
> > by wMaxPacketSize. I can't quote parts of the HW docs here, but I *must*
> > give the HW sizes which are divisible by wMaxPacketSize in case of OUT
> > transfers.
>
> Ah, I didn't realize this. It's ironic then that Mian's patch reducing
> the size of the CBW request length was just submitted. Well, we can
heh.
> retract that patch easily enough if necessary...
ok. I think it's better to revert that and apply the patch using
set_bulk_out_req_length() which I sent earlier, see below...
> This hardware restriction doesn't affect only g_file_storage; you will
> have to take it into account with _every_ gadget driver. The only
> general solution is to use a bounce buffer. Collect all but the last
> partial packet in the gdaget's transfer buffer as usual, then use a
> private maxpacket-sized buffer for the remainder, which you then copy
> to the end of the transfer buffer.
A bounce buffer isn't good enough. Remember I'm talking USB3, with the
current 16K buffer size in g_file_storage, we will get one IRQ every:
(16 << 10) / (5 << 30) = ~3us
If every 3us I need to do a memcpy() of the remaining 1..1023 bytes that
won't fly. The internal DMA, of course, can handle many requests and
interrupt only at the end, which would decrease the amount of
interrupts, but that would mean I would have to:
list_for_each_entry(req, &dev->req_list, list) {
memcpy(req->request.buf + req->request.actual,
req->bounce_buffer, req->remaining);
req->request.complete(&req->ep, &req->request);
}
depending on the amount of requests which I have, that'll mean a lot of
time spent handling one IRQ. IMHO, it's much easier - and much better
for most of the other UDC controllers, to have req->length for OUT
transfers to be aligned on ep->maxpacket.
That way, I don't need bounce buffers, I don't need to do any memcpy()
and I only need to iterate over the list of requests and call the
complete() method like it should be.
What I actually need from gadget drivers, for USB3 at least, is for them
to queue as many requests as possible, so the storm of interrupts can
be decreased and we have many buffers to play with.
If we have, e.g. 8 requests for a particular gadget driver, we could
interrupt after every 4 requests so that while HW is handling 4, gadget
driver is processing and recycling the other 4.
BTW, from what I could see, Storage gadget is the only one doesn't
ensure OUT transfers to be aligned on ep->maxpacket, see gadget ether
for instance:
/* Padding up to RX_EXTRA handles minor disagreements with host.
* Normally we use the USB "terminate on short read" convention;
* so allow up to (N*maxpacket), since that memory is normally
* already allocated. Some hardware doesn't deal well with short
* reads (e.g. DMA must be N*maxpacket), so for now don't trim a
* byte off the end (to force hardware errors on overflow).
*
* RNDIS uses internal framing, and explicitly allows senders to
* pad to end-of-packet. That's potentially nice for speed, but
* means receivers can't recover lost synch on their own (because
* new packets don't only start after a short RX).
*/
size += sizeof(struct ethhdr) + dev->net->mtu + RX_EXTRA;
size += dev->port_usb->header_len;
size += out->maxpacket - 1;
size -= size % out->maxpacket;
the same goes for serial gadget, but that thing queues requests which
are exactly ep->maxpacket:
for (i = 0; i < n; i++) {
req = gs_alloc_req(ep, ep->maxpacket, GFP_ATOMIC);
if (!req)
return list_empty(head) ? -ENOMEM : 0;
req->complete = fn;
list_add_tail(&req->list, head);
if (allocated)
(*allocated)++;
}
same goes for EHCI debug driver:
#define DBGP_REQ_LEN 512
...
req->length = DBGP_REQ_LEN;
so IMHO, it's better to keep request->length for OUT transfers aligned
on ep->maxpacket. What do you say ?
--
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
