Re: [PATCH 1/2] V4L/DVB: buf-dma-sg.c: don't assume nr_pages == sglen

2010-03-30 Thread Arnout Vandecappelle 

On Wednesday 24 March 2010 06:43:22, Sakari Ailus wrote:
 Hi Arnout,
 
 Thanks for the patch.
 
 Arnout Vandecappelle wrote:
[snip]
  -   dma_sync_sg_for_cpu(q-dev, dma-sglist, dma-nr_pages, 
  dma-direction); 
  +   dma_sync_sg_for_cpu(q-dev, dma-sglist, dma-sglen, dma-direction);
 
 I think the same problem still exists --- dma-sglen is not initialised
 anywhere, is it?

 Yes it is.
 In videobuf_dma_map (where dma-sglist is set), there are two conditions:

if (dma-bus_addr) {
dma-sglist = kmalloc(sizeof(struct scatterlist), GFP_KERNEL);
if (NULL != dma-sglist) {
dma-sglen  = 1;
...
}
}
...
if (!dma-bus_addr) {
dma-sglen = dma_map_sg(q-dev, dma-sglist,
dma-nr_pages, dma-direction);
...
}


 Regards,
 Arnout
-- 
Arnout Vandecappelle   arnout at mind be
Senior Embedded Software Architect +32-16-286540
Essensium/Mind http://www.mind.be
G.Geenslaan 9, 3001 Leuven, BelgiumBE 872 984 063 RPR Leuven
LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle
GPG fingerprint:  31BB CF53 8660 6F88 345D  54CC A836 5879 20D7 CF43
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/2] V4L/DVB: buf-dma-sg.c: don't assume nr_pages == sglen

2010-03-17 Thread Sakari Ailus 
Hi Arnout,

Arnout Vandecappelle wrote:
 videobuf_pages_to_sg() and videobuf_vmalloc_to_sg() happen to create
 a scatterlist element for every page.  However, this is not true for
 bus addresses, so other functions shouldn't rely on the length of the
 scatter list being equal to nr_pages.
 ---
  drivers/media/video/videobuf-dma-sg.c |6 +++---
  1 files changed, 3 insertions(+), 3 deletions(-)
 
 diff --git a/drivers/media/video/videobuf-dma-sg.c 
 b/drivers/media/video/videobuf-dma-sg.c
 index da1790e..3b6f1b8 100644
 --- a/drivers/media/video/videobuf-dma-sg.c
 +++ b/drivers/media/video/videobuf-dma-sg.c
 @@ -244,7 +244,7 @@ int videobuf_dma_map(struct videobuf_queue* q, struct 
 videobuf_dmabuf *dma)
   }
   if (!dma-bus_addr) {
   dma-sglen = dma_map_sg(q-dev, dma-sglist,
 - dma-nr_pages, dma-direction);
 + dma-sglen, dma-direction);
   if (0 == dma-sglen) {
   printk(KERN_WARNING
  %s: videobuf_map_sg failed\n,__func__);

Where is dma-sglen actually set?

videobuf_dma_map() is used in __videobuf_iolock
(drivers/media/video/videobuf-dma-sg.c) but neither
videobuf_dma_init_kernel() nor videobuf_dma_init_user() seem to set it.
This apparently leaves the value uninitialised.

I definitely think it should be assigned somewhere. :-)

-- 
Sakari Ailus
sakari.ai...@maxwell.research.nokia.com
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/2] V4L/DVB: buf-dma-sg.c: don't assume nr_pages == sglen

2010-03-17 Thread Arnout Vandecappelle 

On Wednesday 17 March 2010 21:57:18, Sakari Ailus wrote:
 Arnout Vandecappelle wrote:
  diff --git a/drivers/media/video/videobuf-dma-sg.c
  b/drivers/media/video/videobuf-dma-sg.c index da1790e..3b6f1b8 100644
  --- a/drivers/media/video/videobuf-dma-sg.c
  +++ b/drivers/media/video/videobuf-dma-sg.c
  @@ -244,7 +244,7 @@ int videobuf_dma_map(struct videobuf_queue* q,
  struct videobuf_dmabuf *dma)
  
  }
  if (!dma-bus_addr) {
  
  dma-sglen = dma_map_sg(q-dev, dma-sglist,
  
  -   dma-nr_pages, dma-direction);
  +   dma-sglen, dma-direction);
  
  if (0 == dma-sglen) {
  
  printk(KERN_WARNING
  
 %s: videobuf_map_sg failed\n,__func__);
 
 Where is dma-sglen actually set?
 
 videobuf_dma_map() is used in __videobuf_iolock
 (drivers/media/video/videobuf-dma-sg.c) but neither
 videobuf_dma_init_kernel() nor videobuf_dma_init_user() seem to set it.
 This apparently leaves the value uninitialised.
 
 I definitely think it should be assigned somewhere. :-)

 It's assigned there exactly - nr_pages shouldn't have been replaced there.

 Updated patches follow.

 Regards,
 Arnout

-- 
Arnout Vandecappelle   arnout at mind be
Senior Embedded Software Architect +32-16-286540
Essensium/Mind http://www.mind.be
G.Geenslaan 9, 3001 Leuven, BelgiumBE 872 984 063 RPR Leuven
LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle
GPG fingerprint:  31BB CF53 8660 6F88 345D  54CC A836 5879 20D7 CF43
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 1/2] V4L/DVB: buf-dma-sg.c: don't assume nr_pages == sglen

2010-03-17 Thread Arnout Vandecappelle 
videobuf_pages_to_sg() and videobuf_vmalloc_to_sg() happen to create
a scatterlist element for every page.  However, this is not true for
bus addresses, so other functions shouldn't rely on the length of the
scatter list being equal to nr_pages.
---
 drivers/media/video/videobuf-dma-sg.c |4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/media/video/videobuf-dma-sg.c 
b/drivers/media/video/videobuf-dma-sg.c
index da1790e..18aaf54 100644
--- a/drivers/media/video/videobuf-dma-sg.c
+++ b/drivers/media/video/videobuf-dma-sg.c
@@ -262,7 +262,7 @@ int videobuf_dma_sync(struct videobuf_queue *q, struct 
videobuf_dmabuf *dma)
MAGIC_CHECK(dma-magic, MAGIC_DMABUF);
BUG_ON(!dma-sglen);
 
-   dma_sync_sg_for_cpu(q-dev, dma-sglist, dma-nr_pages, dma-direction);
+   dma_sync_sg_for_cpu(q-dev, dma-sglist, dma-sglen, dma-direction);
return 0;
 }
 
@@ -272,7 +272,7 @@ int videobuf_dma_unmap(struct videobuf_queue* q,struct 
videobuf_dmabuf *dma)
if (!dma-sglen)
return 0;
 
-   dma_unmap_sg(q-dev, dma-sglist, dma-nr_pages, dma-direction);
+   dma_unmap_sg(q-dev, dma-sglist, dma-sglen, dma-direction);
 
kfree(dma-sglist);
dma-sglist = NULL;
-- 
1.6.3.3

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

[PATCH 1/2] V4L/DVB: buf-dma-sg.c: don't assume nr_pages == sglen

2010-03-04 Thread Arnout Vandecappelle 
videobuf_pages_to_sg() and videobuf_vmalloc_to_sg() happen to create
a scatterlist element for every page.  However, this is not true for
bus addresses, so other functions shouldn't rely on the length of the
scatter list being equal to nr_pages.
---
 drivers/media/video/videobuf-dma-sg.c |6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/media/video/videobuf-dma-sg.c 
b/drivers/media/video/videobuf-dma-sg.c
index da1790e..3b6f1b8 100644
--- a/drivers/media/video/videobuf-dma-sg.c
+++ b/drivers/media/video/videobuf-dma-sg.c
@@ -244,7 +244,7 @@ int videobuf_dma_map(struct videobuf_queue* q, struct 
videobuf_dmabuf *dma)
}
if (!dma-bus_addr) {
dma-sglen = dma_map_sg(q-dev, dma-sglist,
-   dma-nr_pages, dma-direction);
+   dma-sglen, dma-direction);
if (0 == dma-sglen) {
printk(KERN_WARNING
   %s: videobuf_map_sg failed\n,__func__);
@@ -262,7 +262,7 @@ int videobuf_dma_sync(struct videobuf_queue *q, struct 
videobuf_dmabuf *dma)
MAGIC_CHECK(dma-magic, MAGIC_DMABUF);
BUG_ON(!dma-sglen);
 
-   dma_sync_sg_for_cpu(q-dev, dma-sglist, dma-nr_pages, dma-direction);
+   dma_sync_sg_for_cpu(q-dev, dma-sglist, dma-sglen, dma-direction);
return 0;
 }
 
@@ -272,7 +272,7 @@ int videobuf_dma_unmap(struct videobuf_queue* q,struct 
videobuf_dmabuf *dma)
if (!dma-sglen)
return 0;
 
-   dma_unmap_sg(q-dev, dma-sglist, dma-nr_pages, dma-direction);
+   dma_unmap_sg(q-dev, dma-sglist, dma-sglen, dma-direction);
 
kfree(dma-sglist);
dma-sglist = NULL;
-- 
1.6.3.3

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