From patchwork Fri Nov 14 01:21:50 2008 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Jesse Brandeburg X-Patchwork-Id: 8683 X-Patchwork-Delegate: davem@davemloft.net Return-Path: X-Original-To: patchwork-incoming@ozlabs.org Delivered-To: patchwork-incoming@ozlabs.org Received: from vger.kernel.org (vger.kernel.org [209.132.176.167]) by ozlabs.org (Postfix) with ESMTP id 2CD23DDE04 for ; Fri, 14 Nov 2008 12:22:39 +1100 (EST) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755567AbYKNBWU (ORCPT ); Thu, 13 Nov 2008 20:22:20 -0500 Received: (majordomo@vger.kernel.org) by vger.kernel.org id S1756010AbYKNBWT (ORCPT ); Thu, 13 Nov 2008 20:22:19 -0500 Received: from mga09.intel.com ([134.134.136.24]:12207 "EHLO mga09.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1756422AbYKNBWO (ORCPT ); Thu, 13 Nov 2008 20:22:14 -0500 Received: from orsmga002.jf.intel.com ([10.7.209.21]) by orsmga102.jf.intel.com with ESMTP; 13 Nov 2008 17:16:21 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="4.33,599,1220252400"; d="scan'208";a="359918493" Received: from plxs0284.pdx.intel.com ([10.7.76.148]) by orsmga002.jf.intel.com with ESMTP; 13 Nov 2008 17:21:01 -0800 Received: from jbrandeb-desk.amr.corp.intel.com (jbrandeb-desk.amr.corp.intel.com [134.134.3.128]) by plxs0284.pdx.intel.com (8.12.11.20060308/8.12.10/MailSET/Hub) with ESMTP id mAE1Lo06026835; Thu, 13 Nov 2008 17:21:51 -0800 Date: Thu, 13 Nov 2008 17:21:50 -0800 (Pacific Standard Time) From: "Brandeburg, Jesse" To: Russell King - ARM Linux cc: =?X-UNKNOWN?Q?Anders_Grafstr=F6m?= , "linux-arm-kernel@lists.arm.linux.org.uk" , "netdev@vger.kernel.org" , e1000-devel@lists.sourceforge.net Subject: Re: Validation of DMA params breaks e100 driver (2.6.28-rc2) In-Reply-To: <20081031210131.GA18015@flint.arm.linux.org.uk> Message-ID: References: <4909E08F.9040306@users.sourceforge.net> <20081031210131.GA18015@flint.arm.linux.org.uk> ReplyTo: "Brandeburg, Jesse" X-X-Sender: amrjbrandeb@imapmail.glb.intel.com MIME-Version: 1.0 Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org On Fri, 31 Oct 2008, Russell King - ARM Linux wrote: > On Thu, Oct 30, 2008 at 05:27:59PM +0100, Anders Grafström wrote: > > The e100 driver triggers BUG_ON(buf->direction != dir) > > by doing pci_map_single(..., PCI_DMA_BIDIRECTIONAL) > > and pci_dma_sync_single_for_device(..., PCI_DMA_TODEVICE). > > > > I'm guessing it's allowed to do that and that something like > > the patch below is called for? > > No, it is not allowed to do that - that's why it's called "BUG_ON". > Changing the DMA direction, especially with dmabounce will result > in unexpected behaviour. okay, how about this patch... only compile tested as I couldn't get net-next-2.6 to boot on my test machine. I'll get some testing done on this, but in the meantime.... e100: fix dma error in direction for mapping From: Jesse Brandeburg Russell King - ARM Linux wrote: > On Thu, Oct 30, 2008 at 05:27:59PM +0100, Anders Grafström wrote: >> The e100 driver triggers BUG_ON(buf->direction != dir) >> by doing pci_map_single(..., PCI_DMA_BIDIRECTIONAL) >> and pci_dma_sync_single_for_device(..., PCI_DMA_TODEVICE). >> >> I'm guessing it's allowed to do that and that something like >> the patch below is called for? > > No, it is not allowed to do that - that's why it's called "BUG_ON". > Changing the DMA direction, especially with dmabounce will result > in unexpected behaviour. This is my attempt to fix this issue: Reported by: Anders Grafstrom CC: Russell King Signed-off-by: Jesse Brandeburg Acked-by: David S. Miller --- drivers/net/e100.c | 20 ++++++++++---------- 1 files changed, 10 insertions(+), 10 deletions(-) diff --git a/drivers/net/e100.c b/drivers/net/e100.c index 62cdefa..c9c7079 100644 --- a/drivers/net/e100.c +++ b/drivers/net/e100.c @@ -166,7 +166,7 @@ #define DRV_NAME "e100" #define DRV_EXT "-NAPI" -#define DRV_VERSION "3.5.23-k4"DRV_EXT +#define DRV_VERSION "3.5.23-k6"DRV_EXT #define DRV_DESCRIPTION "Intel(R) PRO/100 Network Driver" #define DRV_COPYRIGHT "Copyright(c) 1999-2006 Intel Corporation" #define PFX DRV_NAME ": " @@ -1804,7 +1804,7 @@ static int e100_rx_alloc_skb(struct nic *nic, struct rx *rx) struct rfd *prev_rfd = (struct rfd *)rx->prev->skb->data; put_unaligned_le32(rx->dma_addr, &prev_rfd->link); pci_dma_sync_single_for_device(nic->pdev, rx->prev->dma_addr, - sizeof(struct rfd), PCI_DMA_TODEVICE); + sizeof(struct rfd), PCI_DMA_BIDIRECTIONAL); } return 0; @@ -1823,7 +1823,7 @@ static int e100_rx_indicate(struct nic *nic, struct rx *rx, /* Need to sync before taking a peek at cb_complete bit */ pci_dma_sync_single_for_cpu(nic->pdev, rx->dma_addr, - sizeof(struct rfd), PCI_DMA_FROMDEVICE); + sizeof(struct rfd), PCI_DMA_BIDIRECTIONAL); rfd_status = le16_to_cpu(rfd->status); DPRINTK(RX_STATUS, DEBUG, "status=0x%04X\n", rfd_status); @@ -1850,7 +1850,7 @@ static int e100_rx_indicate(struct nic *nic, struct rx *rx, /* Get data */ pci_unmap_single(nic->pdev, rx->dma_addr, - RFD_BUF_LEN, PCI_DMA_FROMDEVICE); + RFD_BUF_LEN, PCI_DMA_BIDIRECTIONAL); /* If this buffer has the el bit, but we think the receiver * is still running, check to see if it really stopped while @@ -1942,7 +1942,7 @@ static void e100_rx_clean(struct nic *nic, unsigned int *work_done, new_before_last_rfd->command |= cpu_to_le16(cb_el); pci_dma_sync_single_for_device(nic->pdev, new_before_last_rx->dma_addr, sizeof(struct rfd), - PCI_DMA_TODEVICE); + PCI_DMA_BIDIRECTIONAL); /* Now that we have a new stopping point, we can clear the old * stopping point. We must sync twice to get the proper @@ -1950,11 +1950,11 @@ static void e100_rx_clean(struct nic *nic, unsigned int *work_done, old_before_last_rfd->command &= ~cpu_to_le16(cb_el); pci_dma_sync_single_for_device(nic->pdev, old_before_last_rx->dma_addr, sizeof(struct rfd), - PCI_DMA_TODEVICE); + PCI_DMA_BIDIRECTIONAL); old_before_last_rfd->size = cpu_to_le16(VLAN_ETH_FRAME_LEN); pci_dma_sync_single_for_device(nic->pdev, old_before_last_rx->dma_addr, sizeof(struct rfd), - PCI_DMA_TODEVICE); + PCI_DMA_BIDIRECTIONAL); } if(restart_required) { @@ -1977,7 +1977,7 @@ static void e100_rx_clean_list(struct nic *nic) for(rx = nic->rxs, i = 0; i < count; rx++, i++) { if(rx->skb) { pci_unmap_single(nic->pdev, rx->dma_addr, - RFD_BUF_LEN, PCI_DMA_FROMDEVICE); + RFD_BUF_LEN, PCI_DMA_BIDIRECTIONAL); dev_kfree_skb(rx->skb); } } @@ -2020,7 +2020,7 @@ static int e100_rx_alloc_list(struct nic *nic) before_last->command |= cpu_to_le16(cb_el); before_last->size = 0; pci_dma_sync_single_for_device(nic->pdev, rx->dma_addr, - sizeof(struct rfd), PCI_DMA_TODEVICE); + sizeof(struct rfd), PCI_DMA_BIDIRECTIONAL); nic->rx_to_use = nic->rx_to_clean = nic->rxs; nic->ru_running = RU_SUSPENDED; @@ -2221,7 +2221,7 @@ static int e100_loopback_test(struct nic *nic, enum loopback loopback_mode) msleep(10); pci_dma_sync_single_for_cpu(nic->pdev, nic->rx_to_clean->dma_addr, - RFD_BUF_LEN, PCI_DMA_FROMDEVICE); + RFD_BUF_LEN, PCI_DMA_BIDIRECTIONAL); if(memcmp(nic->rx_to_clean->skb->data + sizeof(struct rfd), skb->data, ETH_DATA_LEN))