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