Re: [PATCH][1/4] usb: dma bounce buffer support using HCD_BOUNCE flag
Alan Stern <[email protected]> Tue, 15 Jan 2008 11:09:47 -0500 (EST)
| Newsgroups | gmane.linux.usb.devel |
|---|---|
| Message-ID | <[email protected]> |
On Mon, 14 Jan 2008, David Brownell wrote:
> > Any comments? Can I just change the name of the flag to HCD_LOCAL_MEM
> > and repost?
>
> OK by me. Maybe a few other folk have opinions though ... Alan?
The core changes look okay, except for some questions of style
preference.
In map_urb_for_dma() and unmap_urb_for_dma(), I think it would look
cleaner if the tests were rearranged along these lines:
if (is_root_hub(urb->dev))
return 0;
if (usb_endpoint_xfer_control(&urb->ep->desc)
&& !(urb->transfer_flags & URB_NO_SETUP_DMA_MAP)) {
if (hcd->self.uses_dma)
urb->setup_dma = dma_map_single...
else if (hcd->driver->flags & HCD_LOCAL_MEM)
status = hcd_alloc_coherent...
}
if (urb->transfer_buffer_length != 0
&& !(urb->transfer_flags & URB_NO_TRANSFER_DMA_MAP)) {
...
Also, in hcd_submit_urb():
> if (unlikely(status)) {
> usbmon_urb_submit_error(&hcd->self, urb, status);
> unmap_urb_for_dma(hcd, urb);
> + }
> + error:
> + if (unlikely(status)) {
> urb->hcpriv = NULL;
> INIT_LIST_HEAD(&urb->urb_list);
> atomic_dec(&urb->use_count);
There's no reason to close the block and start another "if" statement.
Just put the statement label inside the original block.
Alan Stern
-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/
_______________________________________________
[email protected]
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel