From patchwork Tue Jul 14 21:01:54 2009 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Krzysztof Halasa X-Patchwork-Id: 29784 X-Patchwork-Delegate: davem@davemloft.net Return-Path: X-Original-To: patchwork-incoming@bilbo.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from ozlabs.org (ozlabs.org [203.10.76.45]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (Client CN "mx.ozlabs.org", Issuer "CA Cert Signing Authority" (verified OK)) by bilbo.ozlabs.org (Postfix) with ESMTPS id AB3C2B7066 for ; Wed, 15 Jul 2009 07:02:05 +1000 (EST) Received: by ozlabs.org (Postfix) id 8E93DDDD1C; Wed, 15 Jul 2009 07:02:05 +1000 (EST) 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 6DBEDDDDB2 for ; Wed, 15 Jul 2009 07:02:04 +1000 (EST) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755421AbZGNVB6 (ORCPT ); Tue, 14 Jul 2009 17:01:58 -0400 Received: (majordomo@vger.kernel.org) by vger.kernel.org id S1755091AbZGNVB5 (ORCPT ); Tue, 14 Jul 2009 17:01:57 -0400 Received: from khc.piap.pl ([195.187.100.11]:38515 "EHLO khc.piap.pl" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752193AbZGNVB5 (ORCPT ); Tue, 14 Jul 2009 17:01:57 -0400 Received: from intrepid.localdomain (intrepid.localdomain [10.0.0.2]) by khc.piap.pl (Postfix) with ESMTP id 6AB6A9316; Tue, 14 Jul 2009 23:01:54 +0200 (CEST) To: David Miller Cc: netdev@vger.kernel.org, Jeff Kirsher , Jesse Brandeburg , Bruce Allan , PJ Waskiewicz , John Ronciak , e1000-devel@lists.sourceforge.net Subject: Re: Debounce code vs. dma_sync_single_range_for_cpu() and e100 driver. References: <20090713181237.GD31979@n2100.arm.linux.org.uk> <20090714.123848.189674670.davem@davemloft.net> From: Krzysztof Halasa Date: Tue, 14 Jul 2009 23:01:54 +0200 In-Reply-To: <20090714.123848.189674670.davem@davemloft.net> (David Miller's message of "Tue\, 14 Jul 2009 12\:38\:48 -0700 \(PDT\)") Message-ID: MIME-Version: 1.0 Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org (Added Cc:) David Miller writes: > And it's especially buggy if it isn't doing DMA API sync calls before > looking at descriptor fields, as your patch seems to cure. Well, it does that but doesn't "sync for device" _after_ looking at the RX descriptor (when the device is to own the desc again). On IXP4xx the ownership transfer (cache flush/invalidate) happens on "sync for device" only (which IMHO seems a bit fragile, though). So what do we do? Maybe you apply the workaround for 2.6.31 and I (or someone) will convert e100 to coherent allocs for packet descriptors, post-31? This isn't a terrible problem (x86 isn't affected), perhaps we should leave it as is for 2.6.31 making sure it doesn't get out of sight with the workaround in place? Guess it's too risky for me to mess with the coherent allocs conversion for 31, I don't know e100 code at all (and I have to leave for few weeks on Thursday). E100: work around the driver using streaming DMA mapping for RX descriptors. E100 places it's RX packet descriptors inside skb->data and uses them with bidirectional streaming DMA mapping. Unfortunately it fails to transfer skb->data ownership to the device after it reads the descriptor's status, breaking on non-coherent (e.g., ARM) platforms. This have to be converted to use coherent memory for the descriptors. Signed-off-by: Krzysztof Halasa Acked-by: Jeff Kirsher --- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html --- a/drivers/net/e100.c +++ b/drivers/net/e100.c @@ -1762,6 +1762,9 @@ static int e100_rx_indicate(struct nic *nic, struct rx *rx, if (ioread8(&nic->csr->scb.status) & rus_no_res) nic->ru_running = RU_SUSPENDED; + pci_dma_sync_single_for_device(nic->pdev, rx->dma_addr, + sizeof(struct rfd), + PCI_DMA_BIDIRECTIONAL); return -ENODATA; }