Alan Stern wrote: > 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. > Will do. > > + if (!hcd->self.uses_pio_for_control_xfer) { > > This should be > > else if (hcd->self.uses_dma) { > I don't get this. The lines below will call dma_map_single() for the setup part of the control transfer. Right? > > 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. Agreed for both points. I did think this was not a good name to choose, but couldn't come up with a better one. Will change. > > The rest of the patch looks okay. > > Alan Stern Thanks! Will test again on a board tomorrow and repost.-- 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
