diff options
author | Mika Westerberg <mika.westerberg@iki.fi> | 2011-06-11 04:39:58 -0400 |
---|---|---|
committer | David S. Miller <davem@davemloft.net> | 2011-06-11 19:25:16 -0400 |
commit | f1c089e3192f1afdfa76226dc38ef81b08ac810d (patch) | |
tree | e3968efef6b98608e5a49edf6c84097ff066badd /drivers/net/arm | |
parent | 1f758a4341ac83289a549e6ba2d29a08cf639717 (diff) |
net: ep93xx_eth: fix DMA API violations
Russell King said:
>
> So, to summarize what its doing:
>
> 1. It allocates buffers for rx and tx.
> 2. It maps them with dma_map_single().
> This transfers ownership of the buffer to the DMA device.
> 3. In ep93xx_xmit,
> 3a. It copies the data into the buffer with skb_copy_and_csum_dev()
> This violates the DMA buffer ownership rules - the CPU should
> not be writing to this buffer while it is (in principle) owned
> by the DMA device.
> 3b. It then calls dma_sync_single_for_cpu() for the buffer.
> This transfers ownership of the buffer to the CPU, which surely
> is the wrong direction.
> 4. In ep93xx_rx,
> 4a. It calls dma_sync_single_for_cpu() for the buffer.
> This at least transfers the DMA buffer ownership to the CPU
> before the CPU reads the buffer
> 4b. It then uses skb_copy_to_linear_data() to copy the data out.
> At no point does it transfer ownership back to the DMA device.
> 5. When the driver is removed, it dma_unmap_single()'s the buffer.
> This transfers ownership of the buffer to the CPU.
> 6. It frees the buffer.
>
> While it may work on ep93xx, it's not respecting the DMA API rules,
> and with DMA debugging enabled it will probably encounter quite a few
> warnings.
This patch fixes these violations.
Signed-off-by: Mika Westerberg <mika.westerberg@iki.fi>
Acked-by: Russell King <rmk+kernel@arm.linux.org.uk>
Acked-by: H Hartley Sweeten <hsweeten@visionengravers.com>
Tested-by: Petr Stetiar <ynezz@true.cz>
Signed-off-by: David S. Miller <davem@davemloft.net>
Diffstat (limited to 'drivers/net/arm')
-rw-r--r-- | drivers/net/arm/ep93xx_eth.c | 18 |
1 files changed, 13 insertions, 5 deletions
diff --git a/drivers/net/arm/ep93xx_eth.c b/drivers/net/arm/ep93xx_eth.c index bef38116abaf..0b46b8ea0e80 100644 --- a/drivers/net/arm/ep93xx_eth.c +++ b/drivers/net/arm/ep93xx_eth.c | |||
@@ -283,10 +283,14 @@ static int ep93xx_rx(struct net_device *dev, int processed, int budget) | |||
283 | 283 | ||
284 | skb = dev_alloc_skb(length + 2); | 284 | skb = dev_alloc_skb(length + 2); |
285 | if (likely(skb != NULL)) { | 285 | if (likely(skb != NULL)) { |
286 | struct ep93xx_rdesc *rxd = &ep->descs->rdesc[entry]; | ||
286 | skb_reserve(skb, 2); | 287 | skb_reserve(skb, 2); |
287 | dma_sync_single_for_cpu(dev->dev.parent, ep->descs->rdesc[entry].buf_addr, | 288 | dma_sync_single_for_cpu(dev->dev.parent, rxd->buf_addr, |
288 | length, DMA_FROM_DEVICE); | 289 | length, DMA_FROM_DEVICE); |
289 | skb_copy_to_linear_data(skb, ep->rx_buf[entry], length); | 290 | skb_copy_to_linear_data(skb, ep->rx_buf[entry], length); |
291 | dma_sync_single_for_device(dev->dev.parent, | ||
292 | rxd->buf_addr, length, | ||
293 | DMA_FROM_DEVICE); | ||
290 | skb_put(skb, length); | 294 | skb_put(skb, length); |
291 | skb->protocol = eth_type_trans(skb, dev); | 295 | skb->protocol = eth_type_trans(skb, dev); |
292 | 296 | ||
@@ -348,6 +352,7 @@ poll_some_more: | |||
348 | static int ep93xx_xmit(struct sk_buff *skb, struct net_device *dev) | 352 | static int ep93xx_xmit(struct sk_buff *skb, struct net_device *dev) |
349 | { | 353 | { |
350 | struct ep93xx_priv *ep = netdev_priv(dev); | 354 | struct ep93xx_priv *ep = netdev_priv(dev); |
355 | struct ep93xx_tdesc *txd; | ||
351 | int entry; | 356 | int entry; |
352 | 357 | ||
353 | if (unlikely(skb->len > MAX_PKT_SIZE)) { | 358 | if (unlikely(skb->len > MAX_PKT_SIZE)) { |
@@ -359,11 +364,14 @@ static int ep93xx_xmit(struct sk_buff *skb, struct net_device *dev) | |||
359 | entry = ep->tx_pointer; | 364 | entry = ep->tx_pointer; |
360 | ep->tx_pointer = (ep->tx_pointer + 1) & (TX_QUEUE_ENTRIES - 1); | 365 | ep->tx_pointer = (ep->tx_pointer + 1) & (TX_QUEUE_ENTRIES - 1); |
361 | 366 | ||
362 | ep->descs->tdesc[entry].tdesc1 = | 367 | txd = &ep->descs->tdesc[entry]; |
363 | TDESC1_EOF | (entry << 16) | (skb->len & 0xfff); | 368 | |
369 | txd->tdesc1 = TDESC1_EOF | (entry << 16) | (skb->len & 0xfff); | ||
370 | dma_sync_single_for_cpu(dev->dev.parent, txd->buf_addr, skb->len, | ||
371 | DMA_TO_DEVICE); | ||
364 | skb_copy_and_csum_dev(skb, ep->tx_buf[entry]); | 372 | skb_copy_and_csum_dev(skb, ep->tx_buf[entry]); |
365 | dma_sync_single_for_cpu(dev->dev.parent, ep->descs->tdesc[entry].buf_addr, | 373 | dma_sync_single_for_device(dev->dev.parent, txd->buf_addr, skb->len, |
366 | skb->len, DMA_TO_DEVICE); | 374 | DMA_TO_DEVICE); |
367 | dev_kfree_skb(skb); | 375 | dev_kfree_skb(skb); |
368 | 376 | ||
369 | spin_lock_irq(&ep->tx_pending_lock); | 377 | spin_lock_irq(&ep->tx_pending_lock); |