diff mbox

Debounce code vs. dma_sync_single_range_for_cpu() and e100 driver.

Message ID m38wirm119.fsf@intrepid.localdomain
State Accepted, archived
Delegated to: David Miller
Headers show

Commit Message

Krzysztof Halasa July 14, 2009, 9:01 p.m. UTC 
(Added Cc:)

David Miller <davem@davemloft.net> 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 <khc@pm.waw.pl>

--
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

Comments

David Miller July 14, 2009, 9:05 p.m. UTC | #1 
From: Krzysztof Halasa <khc@pm.waw.pl>
Date: Tue, 14 Jul 2009 23:01:54 +0200

> Maybe you apply the workaround for 2.6.31 and I (or someone) will
> convert e100 to coherent allocs for packet descriptors, post-31?

I think that's a good plan.

Will give a few days for Intel folks to chime in before applying
your patch for 2.6.31, but otherwise that's my plan.
--
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
Kirsher, Jeffrey T July 16, 2009, 10:27 p.m. UTC | #2 
On Tue, Jul 14, 2009 at 2:05 PM, David Miller<davem@davemloft.net> wrote:
> From: Krzysztof Halasa <khc@pm.waw.pl>
> Date: Tue, 14 Jul 2009 23:01:54 +0200
>
>> Maybe you apply the workaround for 2.6.31 and I (or someone) will
>> convert e100 to coherent allocs for packet descriptors, post-31?
>
> I think that's a good plan.
>
> Will give a few days for Intel folks to chime in before applying
> your patch for 2.6.31, but otherwise that's my plan.

Acked-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
David Miller July 17, 2009, 1:09 a.m. UTC | #3 
From: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
Date: Thu, 16 Jul 2009 15:27:01 -0700

> On Tue, Jul 14, 2009 at 2:05 PM, David Miller<davem@davemloft.net> wrote:
>> From: Krzysztof Halasa <khc@pm.waw.pl>
>> Date: Tue, 14 Jul 2009 23:01:54 +0200
>>
>>> Maybe you apply the workaround for 2.6.31 and I (or someone) will
>>> convert e100 to coherent allocs for packet descriptors, post-31?
>>
>> I think that's a good plan.
>>
>> Will give a few days for Intel folks to chime in before applying
>> your patch for 2.6.31, but otherwise that's my plan.
> 
> Acked-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>

Applied, and queued for -stable, thanks everyone.
--
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
diff mbox

Patch

--- 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;
 	}