Re: [PATCH 2/2] dma: mv_xor: do not sync the DMA buffer after being deallocated

2013-01-17 Thread Lubomir Rintel 
On Sun, 2013-01-13 at 14:18 +0100, Lubomir Rintel wrote:
> On Fri, 2013-01-04 at 17:10 +0100, Thomas Petazzoni wrote:
> > Dear Lubomir Rintel,
> > 
> > On Thu, 27 Dec 2012 20:23:48 +0100, Lubomir Rintel wrote:
> > 
> > >   dma_sync_single_for_cpu(dma_chan->device->dev, dest_dma,
> > >   MV_XOR_TEST_SIZE, DMA_FROM_DEVICE);
> > > + dma_unmap_single(dma_chan->device->dev, dest_dma, MV_XOR_TEST_SIZE,
> > > +  DMA_FROM_DEVICE);
> > 
> > I would assume that dma_unmap_single() implies a
> > dma_sync_single_for_cpu() since you're unmapping the DMA buffer. So if
> > you use dma_unmap_single(), I think you can remove the call to
> > dma_sync_single_for_cpu().
> 
> Yes, it indeed seems to be the case (comment for arm_dma_unmap_page():
> "After this call, reads by the CPU to the buffer are guaranteed to see
> whatever the device wrote there.".
> 
> > >   dma_sync_single_for_cpu(dma_chan->device->dev, dest_dma,
> > >   PAGE_SIZE, DMA_FROM_DEVICE);
> > > + dma_unmap_page(dma_chan->device->dev, dest_dma, PAGE_SIZE,
> > > +DMA_FROM_DEVICE);
> > 
> > Ditto.
> > 
> > Also, the mv_xor_memcpy_self_test() function not only dma_map_single()
> > the destination buffer, but also the source buffer. So presumably, the
> > source buffer should also be dma_unmap_single()'d.
> > 
> > And for the mv_xor_xor_self_test() function, multiple source buffers
> > are dma_map_page()'d, so they should all be dma_unmap_page()'d I guess,
> > not only the destination buffer.
> 
> Those get released by the mv_xor_run_tx_complete_actions() callback.
> 
> I will follow up with an updated patch.

Actually, after cleaning up the sync a couple more selfcheck warnings
popped up. I'd be very thankful if you could take a look (patches
chained to this message).

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 2/2] dma: mv_xor: do not sync the DMA buffer after being deallocated

2013-01-17 Thread Lubomir Rintel 
On Sun, 2013-01-13 at 14:18 +0100, Lubomir Rintel wrote:
 On Fri, 2013-01-04 at 17:10 +0100, Thomas Petazzoni wrote:
  Dear Lubomir Rintel,
  
  On Thu, 27 Dec 2012 20:23:48 +0100, Lubomir Rintel wrote:
  
 dma_sync_single_for_cpu(dma_chan-device-dev, dest_dma,
 MV_XOR_TEST_SIZE, DMA_FROM_DEVICE);
   + dma_unmap_single(dma_chan-device-dev, dest_dma, MV_XOR_TEST_SIZE,
   +  DMA_FROM_DEVICE);
  
  I would assume that dma_unmap_single() implies a
  dma_sync_single_for_cpu() since you're unmapping the DMA buffer. So if
  you use dma_unmap_single(), I think you can remove the call to
  dma_sync_single_for_cpu().
 
 Yes, it indeed seems to be the case (comment for arm_dma_unmap_page():
 After this call, reads by the CPU to the buffer are guaranteed to see
 whatever the device wrote there..
 
 dma_sync_single_for_cpu(dma_chan-device-dev, dest_dma,
 PAGE_SIZE, DMA_FROM_DEVICE);
   + dma_unmap_page(dma_chan-device-dev, dest_dma, PAGE_SIZE,
   +DMA_FROM_DEVICE);
  
  Ditto.
  
  Also, the mv_xor_memcpy_self_test() function not only dma_map_single()
  the destination buffer, but also the source buffer. So presumably, the
  source buffer should also be dma_unmap_single()'d.
  
  And for the mv_xor_xor_self_test() function, multiple source buffers
  are dma_map_page()'d, so they should all be dma_unmap_page()'d I guess,
  not only the destination buffer.
 
 Those get released by the mv_xor_run_tx_complete_actions() callback.
 
 I will follow up with an updated patch.

Actually, after cleaning up the sync a couple more selfcheck warnings
popped up. I'd be very thankful if you could take a look (patches
chained to this message).

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 2/2] dma: mv_xor: do not sync the DMA buffer after being deallocated

2013-01-13 Thread Lubomir Rintel 
On Fri, 2013-01-04 at 17:10 +0100, Thomas Petazzoni wrote:
> Dear Lubomir Rintel,
> 
> On Thu, 27 Dec 2012 20:23:48 +0100, Lubomir Rintel wrote:
> 
> > dma_sync_single_for_cpu(dma_chan->device->dev, dest_dma,
> > MV_XOR_TEST_SIZE, DMA_FROM_DEVICE);
> > +   dma_unmap_single(dma_chan->device->dev, dest_dma, MV_XOR_TEST_SIZE,
> > +DMA_FROM_DEVICE);
> 
> I would assume that dma_unmap_single() implies a
> dma_sync_single_for_cpu() since you're unmapping the DMA buffer. So if
> you use dma_unmap_single(), I think you can remove the call to
> dma_sync_single_for_cpu().

Yes, it indeed seems to be the case (comment for arm_dma_unmap_page():
"After this call, reads by the CPU to the buffer are guaranteed to see
whatever the device wrote there.".

> > dma_sync_single_for_cpu(dma_chan->device->dev, dest_dma,
> > PAGE_SIZE, DMA_FROM_DEVICE);
> > +   dma_unmap_page(dma_chan->device->dev, dest_dma, PAGE_SIZE,
> > +  DMA_FROM_DEVICE);
> 
> Ditto.
> 
> Also, the mv_xor_memcpy_self_test() function not only dma_map_single()
> the destination buffer, but also the source buffer. So presumably, the
> source buffer should also be dma_unmap_single()'d.
> 
> And for the mv_xor_xor_self_test() function, multiple source buffers
> are dma_map_page()'d, so they should all be dma_unmap_page()'d I guess,
> not only the destination buffer.

Those get released by the mv_xor_run_tx_complete_actions() callback.

I will follow up with an updated patch.

Regards,
Lubo

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 2/2] dma: mv_xor: do not sync the DMA buffer after being deallocated

2013-01-13 Thread Lubomir Rintel 
On Fri, 2013-01-04 at 17:10 +0100, Thomas Petazzoni wrote:
 Dear Lubomir Rintel,
 
 On Thu, 27 Dec 2012 20:23:48 +0100, Lubomir Rintel wrote:
 
  dma_sync_single_for_cpu(dma_chan-device-dev, dest_dma,
  MV_XOR_TEST_SIZE, DMA_FROM_DEVICE);
  +   dma_unmap_single(dma_chan-device-dev, dest_dma, MV_XOR_TEST_SIZE,
  +DMA_FROM_DEVICE);
 
 I would assume that dma_unmap_single() implies a
 dma_sync_single_for_cpu() since you're unmapping the DMA buffer. So if
 you use dma_unmap_single(), I think you can remove the call to
 dma_sync_single_for_cpu().

Yes, it indeed seems to be the case (comment for arm_dma_unmap_page():
After this call, reads by the CPU to the buffer are guaranteed to see
whatever the device wrote there..

  dma_sync_single_for_cpu(dma_chan-device-dev, dest_dma,
  PAGE_SIZE, DMA_FROM_DEVICE);
  +   dma_unmap_page(dma_chan-device-dev, dest_dma, PAGE_SIZE,
  +  DMA_FROM_DEVICE);
 
 Ditto.
 
 Also, the mv_xor_memcpy_self_test() function not only dma_map_single()
 the destination buffer, but also the source buffer. So presumably, the
 source buffer should also be dma_unmap_single()'d.
 
 And for the mv_xor_xor_self_test() function, multiple source buffers
 are dma_map_page()'d, so they should all be dma_unmap_page()'d I guess,
 not only the destination buffer.

Those get released by the mv_xor_run_tx_complete_actions() callback.

I will follow up with an updated patch.

Regards,
Lubo

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 2/2] dma: mv_xor: do not sync the DMA buffer after being deallocated

2013-01-04 Thread Thomas Petazzoni 
Dear Lubomir Rintel,

On Thu, 27 Dec 2012 20:23:48 +0100, Lubomir Rintel wrote:

>   dma_sync_single_for_cpu(dma_chan->device->dev, dest_dma,
>   MV_XOR_TEST_SIZE, DMA_FROM_DEVICE);
> + dma_unmap_single(dma_chan->device->dev, dest_dma, MV_XOR_TEST_SIZE,
> +  DMA_FROM_DEVICE);

I would assume that dma_unmap_single() implies a
dma_sync_single_for_cpu() since you're unmapping the DMA buffer. So if
you use dma_unmap_single(), I think you can remove the call to
dma_sync_single_for_cpu().


>   dma_sync_single_for_cpu(dma_chan->device->dev, dest_dma,
>   PAGE_SIZE, DMA_FROM_DEVICE);
> + dma_unmap_page(dma_chan->device->dev, dest_dma, PAGE_SIZE,
> +DMA_FROM_DEVICE);

Ditto.

Also, the mv_xor_memcpy_self_test() function not only dma_map_single()
the destination buffer, but also the source buffer. So presumably, the
source buffer should also be dma_unmap_single()'d.

And for the mv_xor_xor_self_test() function, multiple source buffers
are dma_map_page()'d, so they should all be dma_unmap_page()'d I guess,
not only the destination buffer.

Does that make sense?

Thanks,

Thomas
-- 
Thomas Petazzoni, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 2/2] dma: mv_xor: do not sync the DMA buffer after being deallocated

2013-01-04 Thread Thomas Petazzoni 
Dear Lubomir Rintel,

On Thu, 27 Dec 2012 20:23:48 +0100, Lubomir Rintel wrote:

   dma_sync_single_for_cpu(dma_chan-device-dev, dest_dma,
   MV_XOR_TEST_SIZE, DMA_FROM_DEVICE);
 + dma_unmap_single(dma_chan-device-dev, dest_dma, MV_XOR_TEST_SIZE,
 +  DMA_FROM_DEVICE);

I would assume that dma_unmap_single() implies a
dma_sync_single_for_cpu() since you're unmapping the DMA buffer. So if
you use dma_unmap_single(), I think you can remove the call to
dma_sync_single_for_cpu().


   dma_sync_single_for_cpu(dma_chan-device-dev, dest_dma,
   PAGE_SIZE, DMA_FROM_DEVICE);
 + dma_unmap_page(dma_chan-device-dev, dest_dma, PAGE_SIZE,
 +DMA_FROM_DEVICE);

Ditto.

Also, the mv_xor_memcpy_self_test() function not only dma_map_single()
the destination buffer, but also the source buffer. So presumably, the
source buffer should also be dma_unmap_single()'d.

And for the mv_xor_xor_self_test() function, multiple source buffers
are dma_map_page()'d, so they should all be dma_unmap_page()'d I guess,
not only the destination buffer.

Does that make sense?

Thanks,

Thomas
-- 
Thomas Petazzoni, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH 2/2] dma: mv_xor: do not sync the DMA buffer after being deallocated

2012-12-27 Thread Lubomir Rintel 
DMA_CHECK was unhappy:
mv_xor mv_xor.0: DMA-API: device driver tries to sync DMA memory it has not

Also, the xor test was causing the mv_xor_slot_cleanup() to incorrectly
dealocate the destination buffer. This is fixed as well, since the deallocation
is done in mv_xor_probe() (with correct flags) now:
mv_xor mv_xor.0: DMA-API: device driver frees DMA memory with different 
direction [device address=0x1dea4000] [size=4096 bytes] [mapped with 
DMA_FROM_DEVICE] [unmapped with DMA_BIDIRECTIONAL]

Signed-off-by: Lubomir Rintel 
---
 drivers/dma/mv_xor.c |9 +++--
 1 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/dma/mv_xor.c b/drivers/dma/mv_xor.c
index 39387df..f642d8b 100644
--- a/drivers/dma/mv_xor.c
+++ b/drivers/dma/mv_xor.c
@@ -946,7 +946,7 @@ static int mv_xor_memcpy_self_test(struct mv_xor_chan 
*mv_chan)
tx = mv_xor_prep_dma_memcpy(dma_chan, dest_dma, src_dma,
MV_XOR_TEST_SIZE,
DMA_COMPL_SRC_UNMAP_SINGLE |
-   DMA_COMPL_DEST_UNMAP_SINGLE);
+   DMA_COMPL_SKIP_DEST_UNMAP);
cookie = mv_xor_tx_submit(tx);
mv_xor_issue_pending(dma_chan);
async_tx_ack(tx);
@@ -962,6 +962,8 @@ static int mv_xor_memcpy_self_test(struct mv_xor_chan 
*mv_chan)
 
dma_sync_single_for_cpu(dma_chan->device->dev, dest_dma,
MV_XOR_TEST_SIZE, DMA_FROM_DEVICE);
+   dma_unmap_single(dma_chan->device->dev, dest_dma, MV_XOR_TEST_SIZE,
+DMA_FROM_DEVICE);
if (memcmp(src, dest, MV_XOR_TEST_SIZE)) {
dev_err(dma_chan->device->dev,
"Self-test copy failed compare, disabling\n");
@@ -1039,7 +1041,8 @@ mv_xor_xor_self_test(struct mv_xor_chan *mv_chan)
   0, PAGE_SIZE, DMA_TO_DEVICE);
 
tx = mv_xor_prep_dma_xor(dma_chan, dest_dma, dma_srcs,
-MV_XOR_NUM_SRC_TEST, PAGE_SIZE, 0);
+MV_XOR_NUM_SRC_TEST, PAGE_SIZE,
+DMA_COMPL_SKIP_DEST_UNMAP);
 
cookie = mv_xor_tx_submit(tx);
mv_xor_issue_pending(dma_chan);
@@ -1056,6 +1059,8 @@ mv_xor_xor_self_test(struct mv_xor_chan *mv_chan)
 
dma_sync_single_for_cpu(dma_chan->device->dev, dest_dma,
PAGE_SIZE, DMA_FROM_DEVICE);
+   dma_unmap_page(dma_chan->device->dev, dest_dma, PAGE_SIZE,
+  DMA_FROM_DEVICE);
for (i = 0; i < (PAGE_SIZE / sizeof(u32)); i++) {
u32 *ptr = page_address(dest);
if (ptr[i] != cmp_word) {
-- 
1.7.1

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH 2/2] dma: mv_xor: do not sync the DMA buffer after being deallocated

2012-12-27 Thread Lubomir Rintel 
DMA_CHECK was unhappy:
mv_xor mv_xor.0: DMA-API: device driver tries to sync DMA memory it has not

Also, the xor test was causing the mv_xor_slot_cleanup() to incorrectly
dealocate the destination buffer. This is fixed as well, since the deallocation
is done in mv_xor_probe() (with correct flags) now:
mv_xor mv_xor.0: DMA-API: device driver frees DMA memory with different 
direction [device address=0x1dea4000] [size=4096 bytes] [mapped with 
DMA_FROM_DEVICE] [unmapped with DMA_BIDIRECTIONAL]

Signed-off-by: Lubomir Rintel lkund...@v3.sk
---
 drivers/dma/mv_xor.c |9 +++--
 1 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/dma/mv_xor.c b/drivers/dma/mv_xor.c
index 39387df..f642d8b 100644
--- a/drivers/dma/mv_xor.c
+++ b/drivers/dma/mv_xor.c
@@ -946,7 +946,7 @@ static int mv_xor_memcpy_self_test(struct mv_xor_chan 
*mv_chan)
tx = mv_xor_prep_dma_memcpy(dma_chan, dest_dma, src_dma,
MV_XOR_TEST_SIZE,
DMA_COMPL_SRC_UNMAP_SINGLE |
-   DMA_COMPL_DEST_UNMAP_SINGLE);
+   DMA_COMPL_SKIP_DEST_UNMAP);
cookie = mv_xor_tx_submit(tx);
mv_xor_issue_pending(dma_chan);
async_tx_ack(tx);
@@ -962,6 +962,8 @@ static int mv_xor_memcpy_self_test(struct mv_xor_chan 
*mv_chan)
 
dma_sync_single_for_cpu(dma_chan-device-dev, dest_dma,
MV_XOR_TEST_SIZE, DMA_FROM_DEVICE);
+   dma_unmap_single(dma_chan-device-dev, dest_dma, MV_XOR_TEST_SIZE,
+DMA_FROM_DEVICE);
if (memcmp(src, dest, MV_XOR_TEST_SIZE)) {
dev_err(dma_chan-device-dev,
Self-test copy failed compare, disabling\n);
@@ -1039,7 +1041,8 @@ mv_xor_xor_self_test(struct mv_xor_chan *mv_chan)
   0, PAGE_SIZE, DMA_TO_DEVICE);
 
tx = mv_xor_prep_dma_xor(dma_chan, dest_dma, dma_srcs,
-MV_XOR_NUM_SRC_TEST, PAGE_SIZE, 0);
+MV_XOR_NUM_SRC_TEST, PAGE_SIZE,
+DMA_COMPL_SKIP_DEST_UNMAP);
 
cookie = mv_xor_tx_submit(tx);
mv_xor_issue_pending(dma_chan);
@@ -1056,6 +1059,8 @@ mv_xor_xor_self_test(struct mv_xor_chan *mv_chan)
 
dma_sync_single_for_cpu(dma_chan-device-dev, dest_dma,
PAGE_SIZE, DMA_FROM_DEVICE);
+   dma_unmap_page(dma_chan-device-dev, dest_dma, PAGE_SIZE,
+  DMA_FROM_DEVICE);
for (i = 0; i  (PAGE_SIZE / sizeof(u32)); i++) {
u32 *ptr = page_address(dest);
if (ptr[i] != cmp_word) {
-- 
1.7.1

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/