* [PATCH for-2.6.38] p54pci: update receive dma buffers before and after processing
@ 2011-02-11 0:48 Christian Lamparter
2011-02-11 0:52 ` John W. Linville
2011-02-11 1:37 ` Larry Finger
0 siblings, 2 replies; 4+ messages in thread
From: Christian Lamparter @ 2011-02-11 0:48 UTC (permalink / raw)
To: linux-wireless; +Cc: linville
Documentation/DMA-API-HOWTO.txt states:
"DMA transfers need to be synced properly in order for
the cpu and device to see the most uptodate and correct
copy of the DMA buffer."
Cc: <stable@kernel.org>
Signed-off-by: Christian Lamparter <chunkeey@googlemail.com>
---
Embarrassingly, this bug is unique to p54 and has been around
since 2.6.25. This tells us something interesting about our
userbase.
---
diff --git a/drivers/net/wireless/p54/p54pci.c b/drivers/net/wireless/p54/p54pci.c
index 1eacba4..0494d7b 100644
--- a/drivers/net/wireless/p54/p54pci.c
+++ b/drivers/net/wireless/p54/p54pci.c
@@ -199,6 +199,7 @@ static void p54p_check_rx_ring(struct ieee80211_hw *dev, u32 *index,
while (i != idx) {
u16 len;
struct sk_buff *skb;
+ dma_addr_t dma_addr;
desc = &ring[i];
len = le16_to_cpu(desc->len);
skb = rx_buf[i];
@@ -216,17 +217,20 @@ static void p54p_check_rx_ring(struct ieee80211_hw *dev, u32 *index,
len = priv->common.rx_mtu;
}
+ dma_addr = le32_to_cpu(desc->host_addr);
+ pci_dma_sync_single_for_cpu(priv->pdev, dma_addr,
+ priv->common.rx_mtu + 32, PCI_DMA_FROMDEVICE);
skb_put(skb, len);
if (p54_rx(dev, skb)) {
- pci_unmap_single(priv->pdev,
- le32_to_cpu(desc->host_addr),
- priv->common.rx_mtu + 32,
- PCI_DMA_FROMDEVICE);
+ pci_unmap_single(priv->pdev, dma_addr,
+ priv->common.rx_mtu + 32, PCI_DMA_FROMDEVICE);
rx_buf[i] = NULL;
- desc->host_addr = 0;
+ desc->host_addr = cpu_to_le32(0);
} else {
skb_trim(skb, 0);
+ pci_dma_sync_single_for_device(priv->pdev, dma_addr,
+ priv->common.rx_mtu + 32, PCI_DMA_FROMDEVICE);
desc->len = cpu_to_le16(priv->common.rx_mtu + 32);
}
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH for-2.6.38] p54pci: update receive dma buffers before and after processing
2011-02-11 0:48 [PATCH for-2.6.38] p54pci: update receive dma buffers before and after processing Christian Lamparter
@ 2011-02-11 0:52 ` John W. Linville
2011-02-11 1:37 ` Larry Finger
1 sibling, 0 replies; 4+ messages in thread
From: John W. Linville @ 2011-02-11 0:52 UTC (permalink / raw)
To: Christian Lamparter; +Cc: linux-wireless
On Fri, Feb 11, 2011 at 01:48:42AM +0100, Christian Lamparter wrote:
> Documentation/DMA-API-HOWTO.txt states:
>
> "DMA transfers need to be synced properly in order for
> the cpu and device to see the most uptodate and correct
> copy of the DMA buffer."
>
> Cc: <stable@kernel.org>
> Signed-off-by: Christian Lamparter <chunkeey@googlemail.com>
> ---
> Embarrassingly, this bug is unique to p54 and has been around
> since 2.6.25. This tells us something interesting about our
> userbase.
Most likely it tells us that most people using p54 are using little
endian boxes with coherent DMA... :-)
John
--
John W. Linville Someday the world will need a hero, and you
linville@tuxdriver.com might be all we have. Be ready.
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH for-2.6.38] p54pci: update receive dma buffers before and after processing
2011-02-11 0:48 [PATCH for-2.6.38] p54pci: update receive dma buffers before and after processing Christian Lamparter
2011-02-11 0:52 ` John W. Linville
@ 2011-02-11 1:37 ` Larry Finger
2011-02-11 12:05 ` Christian Lamparter
1 sibling, 1 reply; 4+ messages in thread
From: Larry Finger @ 2011-02-11 1:37 UTC (permalink / raw)
To: Christian Lamparter; +Cc: linux-wireless, linville
On 02/10/2011 06:48 PM, Christian Lamparter wrote:
> Documentation/DMA-API-HOWTO.txt states:
>
> "DMA transfers need to be synced properly in order for
> the cpu and device to see the most uptodate and correct
> copy of the DMA buffer."
>
> Cc: <stable@kernel.org>
> Signed-off-by: Christian Lamparter <chunkeey@googlemail.com>
> ---
> Embarrassingly, this bug is unique to p54 and has been around
> since 2.6.25. This tells us something interesting about our
> userbase.
I do use it on a LE system that probably has coherent DMA. Did someone run into
this bug?
> ---
> diff --git a/drivers/net/wireless/p54/p54pci.c b/drivers/net/wireless/p54/p54pci.c
> index 1eacba4..0494d7b 100644
> --- a/drivers/net/wireless/p54/p54pci.c
> +++ b/drivers/net/wireless/p54/p54pci.c
> @@ -199,6 +199,7 @@ static void p54p_check_rx_ring(struct ieee80211_hw *dev, u32 *index,
> while (i != idx) {
> u16 len;
> struct sk_buff *skb;
> + dma_addr_t dma_addr;
> desc = &ring[i];
> len = le16_to_cpu(desc->len);
> skb = rx_buf[i];
> @@ -216,17 +217,20 @@ static void p54p_check_rx_ring(struct ieee80211_hw *dev, u32 *index,
>
> len = priv->common.rx_mtu;
> }
> + dma_addr = le32_to_cpu(desc->host_addr);
> + pci_dma_sync_single_for_cpu(priv->pdev, dma_addr,
> + priv->common.rx_mtu + 32, PCI_DMA_FROMDEVICE);
> skb_put(skb, len);
>
> if (p54_rx(dev, skb)) {
> - pci_unmap_single(priv->pdev,
> - le32_to_cpu(desc->host_addr),
> - priv->common.rx_mtu + 32,
> - PCI_DMA_FROMDEVICE);
> + pci_unmap_single(priv->pdev, dma_addr,
> + priv->common.rx_mtu + 32, PCI_DMA_FROMDEVICE);
> rx_buf[i] = NULL;
> - desc->host_addr = 0;
> + desc->host_addr = cpu_to_le32(0);
How does a BE zero differ from a LE zero? Using it this way emphasizes that it
needs to be translated - I'm not objecting, just curious.
Larry
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH for-2.6.38] p54pci: update receive dma buffers before and after processing
2011-02-11 1:37 ` Larry Finger
@ 2011-02-11 12:05 ` Christian Lamparter
0 siblings, 0 replies; 4+ messages in thread
From: Christian Lamparter @ 2011-02-11 12:05 UTC (permalink / raw)
To: Larry Finger; +Cc: linux-wireless, linville
On Friday 11 February 2011 02:37:00 Larry Finger wrote:
> On 02/10/2011 06:48 PM, Christian Lamparter wrote:
> > ---
> > diff --git a/drivers/net/wireless/p54/p54pci.c b/drivers/net/wireless/p54/p54pci.c
> > index 1eacba4..0494d7b 100644
> > --- a/drivers/net/wireless/p54/p54pci.c
> > +++ b/drivers/net/wireless/p54/p54pci.c
> > @@ -216,17 +217,20 @@ static void p54p_check_rx_ring(struct ieee80211_hw *dev, u32 *index,
> > rx_buf[i] = NULL;
> > - desc->host_addr = 0;
> > + desc->host_addr = cpu_to_le32(0);
>
> How does a BE zero differ from a LE zero?
As far as I can know "0" is something like an endian palindrome.
Otherwise [non-NULL] we would have fixed this long ago, because
then a warning is generated by "sparse":
drivers/net/wireless/p54/p54pci.c:229:20: warning: incorrect type in assignment (different base types)
drivers/net/wireless/p54/p54pci.c:229:20: expected restricted __le32 [usertype] host_addr
drivers/net/wireless/p54/p54pci.c:229:20: got int
> Using it this way emphasizes that it needs to be translated -
> I'm not objecting, just curious.
Furthermore the compiler optimizes *constant* cpu_to_le
[and vice versa] conversions, so there's no hidden cost
from using them.
Regards,
Chr
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2011-02-11 12:05 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-02-11 0:48 [PATCH for-2.6.38] p54pci: update receive dma buffers before and after processing Christian Lamparter
2011-02-11 0:52 ` John W. Linville
2011-02-11 1:37 ` Larry Finger
2011-02-11 12:05 ` Christian Lamparter
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).