Re: Handling of bounce buffers by rh_call_control

[David Brownell <david-b@pacbell.net>]

Hi,

Richard Curnow wrote:
> The following patch
> 
> ===== drivers/usb/hcd.c 1.10 vs edited =====
> --- 1.10/drivers/usb/hcd.c      Mon Mar 31 14:22:42 2003
> +++ edited/drivers/usb/hcd.c    Wed Dec 17 11:26:53 2003
> @@ -323,7 +323,7 @@
>         struct usb_ctrlrequest *cmd = (struct usb_ctrlrequest *) urb->setup_packet;
>         u16             typeReq, wValue, wIndex, wLength;
>         const u8        *bufp = 0;
> -       u8              *ubuf = urb->transfer_buffer;
> +       u8              *ubuf = (u8 *) urb->transfer_dma;
>         int             len = 0;
>  
>         typeReq  = (cmd->bRequestType << 8) | cmd->bRequest;
> 
> seems to be needed to make the following code later in the function work
> 
> 	if (bufp) {
> 		if (urb->transfer_buffer_length < len)
> 			len = urb->transfer_buffer_length;
> 		urb->actual_length = len;
> 		// always USB_DIR_IN, toward host
> 		memcpy (ubuf, bufp, len);

Which is why that particular patch is wrong:  "ubuf" is a dma address,
not expected to work for memcpy().


> 	}
> 
> in the case where bounce buffers are being used to implement PCI DMA
> operations.  Without the patch, the subsequent pci_unmap_single copies
> the contents of the bounce buffer over the top of urb->transfer_buffer,
> destroying what the memcpy() put there.

A similar bug was fixed in 2.6 by a patch from Russell King.  I think
the 2.4 version has lingered since it only affect EHCI, and most folk
using EHCI won't use bounce buffers.

Could you look at that patch (bk revtool or bkweb will show it, early
September 2002) and provide a 2.4 version?


> My USB knowledge is pretty limited, so I've no idea whether this patch
> adversely affects anything else in rh_call_control.
> 
> The patch is against 2.4.23-pre-something, but I see the code's the same
> in 2.6.

What changed in 2.6 was that hcd_submit_urb() has a special case for
the root hub.  That's always PIO, so the code now bypasses all the
DMA mapping code when an URB is destined for a root hub.  The original
code accidentally assumed no bounce buffering would go on.

- Dave