Re: [PATCH RFC] usb: musb: do not use dma for control transfers

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, 4 Aug 2010, Anand Gadiyar wrote:

> The Inventra DMA engine in many instances of the MUSB controller
> cannot use DMA for control transfers on EP0, but can use DMA for
> all other transfers.
> 
> The USB core maps urbs for DMA if hcd->self.uses_dma is true.
> (This is true for MUSB as well).
> 
> One way to solve this issue is to split the uses_dma flag into
> two, one for control transfers and one for "ordinary" transfers.
> The patch below implements this.
> 
> The original discussion thread for this issue was here [1].
> A suggestion by Oliver Neukum to not use a flag describing
> exceptions [2] was not tested, as it would mean touching all
> the other HCD drivers.
> 
> An alternative approach Alan suggested is to have the MUSB HCD
> driver unmap an urb if it cannot carry out any transfer using DMA.
> We're yet to try this out, and I'll try and put this together in a bit.

This approach is more efficient because it skips the unnecessary
map/unmap steps.

> [1] http://marc.info/?t=126477582800002&r=1&w=2
> [2] http://marc.info/?l=linux-kernel&m=126639787526097&w=2
> 
> NYET-Signed-off-by: Anand Gadiyar <gadiyar@xxxxxx>
> Cc: Oliver Neukum <oliver@xxxxxxxxxx>
> Cc: Alan Stern <stern@xxxxxxxxxxxxxxxxxxx>
> Cc: Praveena NADAHALLY <praveen.nadahally@xxxxxxxxxxxxxx>
> ---
>  drivers/usb/core/hcd.c         |    8 +++++---
>  drivers/usb/musb/musb_core.c   |    3 +++
>  drivers/usb/musb/musb_gadget.c |    4 ++++
>  include/linux/usb.h            |    6 +++++-
>  4 files changed, 17 insertions(+), 4 deletions(-)
> 
> Index: linux-2.6/drivers/usb/core/hcd.c
> ===================================================================
> --- linux-2.6.orig/drivers/usb/core/hcd.c
> +++ linux-2.6/drivers/usb/core/hcd.c
> @@ -1321,7 +1321,9 @@ static int map_urb_for_dma(struct usb_hc
>  	 */
>  
>  	if (usb_endpoint_xfer_control(&urb->ep->desc)) {
> -		if (hcd->self.uses_dma) {
> +		if (hcd->self.uses_pio_for_control_xfer)
> +			urb->transfer_flags = URB_NO_TRANSFER_DMA_MAP;

You should "or" in the new flag value; don't overwrite other flags.

> +		if (!hcd->self.uses_pio_for_control_xfer) {

This should be

		else if (hcd->self.uses_dma) {

> Index: linux-2.6/include/linux/usb.h
> ===================================================================
> --- linux-2.6.orig/include/linux/usb.h
> +++ linux-2.6/include/linux/usb.h
> @@ -310,7 +310,11 @@ struct usb_bus {
>  	struct device *controller;	/* host/master side hardware */
>  	int busnum;			/* Bus number (in order of reg) */
>  	const char *bus_name;		/* stable id (PCI slot_name etc) */
> -	u8 uses_dma;			/* Does the host controller use DMA? */
> +	u8 uses_dma_for_ordinary_xfer;	/* Does the host controller use DMA? */

I don't see any reason to change the existing name.  Especially since 
your new name is so much more unwieldy.

> +	u8 uses_pio_for_control_xfer;	/*
> +					 * Does the host controller use PIO
> +					 * for control transfers?
> +					 */

And this could be shortened to uses_pio_for_control.

The rest of the patch looks okay.

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


[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux