Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756426Ab0LQWfQ (ORCPT ); Fri, 17 Dec 2010 17:35:16 -0500 Received: from cantor.suse.de ([195.135.220.2]:45144 "EHLO mx1.suse.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1756083Ab0LQWfO (ORCPT ); Fri, 17 Dec 2010 17:35:14 -0500 Date: Fri, 17 Dec 2010 14:35:02 -0800 From: Greg KH To: Robert Morell Cc: David Brownell , Benoit Goby , Alan Stern , Sarah Sharp , Matthew Wilcox , Ming Lei , Jacob Pan , Olof Johansson , Erik Gilling , Colin Cross , linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH 2/2] USB: ehci: tegra: Align DMA transfers to 32 bytes Message-ID: <20101217223502.GB19418@suse.de> References: <1292623129-26361-1-git-send-email-rmorell@nvidia.com> <1292623129-26361-3-git-send-email-rmorell@nvidia.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1292623129-26361-3-git-send-email-rmorell@nvidia.com> User-Agent: Mutt/1.5.20 (2009-06-14) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 5292 Lines: 162 On Fri, Dec 17, 2010 at 01:58:49PM -0800, Robert Morell wrote: > The Tegra2 USB controller doesn't properly deal with misaligned DMA > buffers, causing corruption. This is especially prevalent with USB > network adapters, where skbuff alignment is often in the middle of a > 4-byte dword. > > To avoid this, allocate a temporary buffer for the DMA if the provided > buffer isn't sufficiently aligned. > > Signed-off-by: Robert Morell > Reviewed-by: Scott Williams > Reviewed-by: Janne Hellsten > --- > drivers/usb/host/ehci-tegra.c | 94 +++++++++++++++++++++++++++++++++++++++++ > include/linux/usb.h | 1 + > 2 files changed, 95 insertions(+), 0 deletions(-) > > diff --git a/drivers/usb/host/ehci-tegra.c b/drivers/usb/host/ehci-tegra.c > index d990c1c..a75e4db 100644 > --- a/drivers/usb/host/ehci-tegra.c > +++ b/drivers/usb/host/ehci-tegra.c > @@ -32,6 +32,10 @@ > #define TEGRA_USB_USBMODE_HOST (3 << 0) > #define TEGRA_USB_PORTSC1_PTC(x) (((x) & 0xf) << 16) > > +#define TEGRA_USB_DMA_ALIGN 32 > + > +#define URB_ALIGNED_TEMP_BUFFER 0x80000000 > + > struct tegra_ehci_context { > bool valid; > u32 command; > @@ -457,6 +461,94 @@ static int tegra_ehci_bus_resume(struct usb_hcd *hcd) > } > #endif > > +struct temp_buffer { > + void *kmalloc_ptr; > + void *old_xfer_buffer; > + u8 data[0]; > +}; > + > +static void free_temp_buffer(struct urb *urb) > +{ > + enum dma_data_direction dir; > + struct temp_buffer *temp; > + > + if (!(urb->transfer_flags & URB_ALIGNED_TEMP_BUFFER)) > + return; > + > + dir = usb_urb_dir_in(urb) ? DMA_FROM_DEVICE : DMA_TO_DEVICE; > + > + temp = container_of(urb->transfer_buffer, struct temp_buffer, > + data); > + > + if (dir == DMA_FROM_DEVICE) > + memcpy(temp->old_xfer_buffer, temp->data, > + urb->transfer_buffer_length); > + urb->transfer_buffer = temp->old_xfer_buffer; > + kfree(temp->kmalloc_ptr); > + > + urb->transfer_flags &= ~URB_ALIGNED_TEMP_BUFFER; > +} > + > +static int alloc_temp_buffer(struct urb *urb, gfp_t mem_flags) > +{ > + enum dma_data_direction dir; > + struct temp_buffer *temp, *kmalloc_ptr; > + size_t kmalloc_size; > + void *data; > + > + if (urb->num_sgs || urb->sg || > + urb->transfer_buffer_length == 0 || > + !((uintptr_t)urb->transfer_buffer & (TEGRA_USB_DMA_ALIGN - 1))) > + return 0; > + > + dir = usb_urb_dir_in(urb) ? DMA_FROM_DEVICE : DMA_TO_DEVICE; > + > + /* Allocate a buffer with enough padding for alignment */ > + kmalloc_size = urb->transfer_buffer_length + > + sizeof(struct temp_buffer) + TEGRA_USB_DMA_ALIGN - 1; > + > + kmalloc_ptr = kmalloc(kmalloc_size, mem_flags); > + if (!kmalloc_ptr) > + return -ENOMEM; > + > + /* Position our struct temp_buffer such that data is aligned */ > + temp = PTR_ALIGN(kmalloc_ptr + 1, TEGRA_USB_DMA_ALIGN) - 1; > + > + temp->kmalloc_ptr = kmalloc_ptr; > + temp->old_xfer_buffer = urb->transfer_buffer; > + if (dir == DMA_TO_DEVICE) > + memcpy(temp->data, urb->transfer_buffer, > + urb->transfer_buffer_length); > + urb->transfer_buffer = temp->data; > + > + BUILD_BUG_ON(!(URB_ALIGNED_TEMP_BUFFER & URB_DRIVER_PRIVATE)); See below why this should be removed > + urb->transfer_flags |= URB_ALIGNED_TEMP_BUFFER; > + > + return 0; > +} > + > +static int tegra_ehci_map_urb_for_dma(struct usb_hcd *hcd, struct urb *urb, > + gfp_t mem_flags) > +{ > + int ret; > + > + ret = alloc_temp_buffer(urb, mem_flags); > + if (ret) > + return ret; > + > + ret = usb_hcd_map_urb_for_dma(hcd, urb, mem_flags); > + if (ret) > + free_temp_buffer(urb); > + > + return ret; > +} > + > +static void tegra_ehci_unmap_urb_for_dma(struct usb_hcd *hcd, struct urb *urb) > +{ > + usb_hcd_unmap_urb_for_dma(hcd, urb); > + free_temp_buffer(urb); > +} > + > static const struct hc_driver tegra_ehci_hc_driver = { > .description = hcd_name, > .product_desc = "Tegra EHCI Host Controller", > @@ -472,6 +564,8 @@ static const struct hc_driver tegra_ehci_hc_driver = { > .shutdown = tegra_ehci_shutdown, > .urb_enqueue = ehci_urb_enqueue, > .urb_dequeue = ehci_urb_dequeue, > + .map_urb_for_dma = tegra_ehci_map_urb_for_dma, > + .unmap_urb_for_dma = tegra_ehci_unmap_urb_for_dma, > .endpoint_disable = ehci_endpoint_disable, > .endpoint_reset = ehci_endpoint_reset, > .get_frame_number = ehci_get_frame, > diff --git a/include/linux/usb.h b/include/linux/usb.h > index 35fe6ab..cd07173 100644 > --- a/include/linux/usb.h > +++ b/include/linux/usb.h > @@ -975,6 +975,7 @@ extern int usb_disabled(void); > #define URB_SETUP_MAP_SINGLE 0x00100000 /* Setup packet DMA mapped */ > #define URB_SETUP_MAP_LOCAL 0x00200000 /* HCD-local setup packet */ > #define URB_DMA_SG_COMBINED 0x00400000 /* S-G entries were combined */ > +#define URB_DRIVER_PRIVATE 0x80000000 /* For driver-private use */ Um, what? You are using this for a build check, which seems pointless as you already modified the code to not need this. So it doesn't look like this is needed, right? thanks, greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/