[PATCH 1/3] media/videobuf2-dma-sg: Fix handling of sg_table structure

[PATCH 1/3] media/videobuf2-dma-sg: Fix handling of sg_table structure

[Ricardo Ribalda Delgado]

when sg_alloc_table_from_pages() does not fail it returns a sg_table
structure with nents and nents_orig initialized to the same value.

dma_map_sg returns the dma_map_sg returns the number of areas mapped
by the hardware, which could be different than the areas given as an input.
The output must be saved to nent.
Unfortunately nent differs in sign to the output of dma_map_sg, so an
intermediate value must be used.

The output of dma_map, should be used to transverse the scatter list.

dma_unmap_sg needs the value passed to dma_map_sg (nents_orig).

sg_free_tables uses also orig_nent.

This patch fix the file to follow this paradigm.

Signed-off-by: Ricardo Ribalda Delgado <ricardo.ribalda@gmail.com>
---
 drivers/media/v4l2-core/videobuf2-dma-sg.c | 22 +++++++++++++++-------
 1 file changed, 15 insertions(+), 7 deletions(-)

diff --git a/drivers/media/v4l2-core/videobuf2-dma-sg.c b/drivers/media/v4l2-core/videobuf2-dma-sg.c
index b1838ab..30bac99 100644
--- a/drivers/media/v4l2-core/videobuf2-dma-sg.c
+++ b/drivers/media/v4l2-core/videobuf2-dma-sg.c
@@ -147,9 +147,11 @@ static void *vb2_dma_sg_alloc(void *alloc_ctx, unsigned long size,
 	 * No need to sync to the device, this will happen later when the
 	 * prepare() memop is called.
 	 */
-	if (dma_map_sg_attrs(buf->dev, sgt->sgl, sgt->nents,
-			     buf->dma_dir, &attrs) == 0)
+	ret = dma_map_sg_attrs(buf->dev, sgt->sgl, sgt->orig_nents,
+			buf->dma_dir, &attrs);
+	if (ret <= 0)
 		goto fail_map;
+	sgt->nents = ret;
 
 	buf->handler.refcount = &buf->refcount;
 	buf->handler.put = vb2_dma_sg_put;
@@ -187,7 +189,7 @@ static void vb2_dma_sg_put(void *buf_priv)
 		dma_set_attr(DMA_ATTR_SKIP_CPU_SYNC, &attrs);
 		dprintk(1, "%s: Freeing buffer of %d pages\n", __func__,
 			buf->num_pages);
-		dma_unmap_sg_attrs(buf->dev, sgt->sgl, sgt->nents,
+		dma_unmap_sg_attrs(buf->dev, sgt->sgl, sgt->orig_nents,
 				   buf->dma_dir, &attrs);
 		if (buf->vaddr)
 			vm_unmap_ram(buf->vaddr, buf->num_pages);
@@ -240,6 +242,7 @@ static void *vb2_dma_sg_get_userptr(void *alloc_ctx, unsigned long vaddr,
 	struct vm_area_struct *vma;
 	struct sg_table *sgt;
 	DEFINE_DMA_ATTRS(attrs);
+	int ret;
 
 	dma_set_attr(DMA_ATTR_SKIP_CPU_SYNC, &attrs);
 
@@ -314,9 +317,12 @@ static void *vb2_dma_sg_get_userptr(void *alloc_ctx, unsigned long vaddr,
 	 * No need to sync to the device, this will happen later when the
 	 * prepare() memop is called.
 	 */
-	if (dma_map_sg_attrs(buf->dev, sgt->sgl, sgt->nents,
-			     buf->dma_dir, &attrs) == 0)
+	ret = dma_map_sg_attrs(buf->dev, sgt->sgl, sgt->orig_nents,
+			     buf->dma_dir, &attrs);
+	if (ret <= 0)
 		goto userptr_fail_map;
+	sgt->nents = ret;
+
 	return buf;
 
 userptr_fail_map:
@@ -351,7 +357,8 @@ static void vb2_dma_sg_put_userptr(void *buf_priv)
 
 	dprintk(1, "%s: Releasing userspace buffer of %d pages\n",
 	       __func__, buf->num_pages);
-	dma_unmap_sg_attrs(buf->dev, sgt->sgl, sgt->nents, buf->dma_dir, &attrs);
+	dma_unmap_sg_attrs(buf->dev, sgt->sgl, sgt->orig_nents, buf->dma_dir,
+			&attrs);
 	if (buf->vaddr)
 		vm_unmap_ram(buf->vaddr, buf->num_pages);
 	sg_free_table(buf->dma_sgt);
@@ -463,7 +470,7 @@ static int vb2_dma_sg_dmabuf_ops_attach(struct dma_buf *dbuf, struct device *dev
 
 	rd = buf->dma_sgt->sgl;
 	wr = sgt->sgl;
-	for (i = 0; i < sgt->orig_nents; ++i) {
+	for (i = 0; i < sgt->nents; ++i) {
 		sg_set_page(wr, sg_page(rd), rd->length, rd->offset);
 		rd = sg_next(rd);
 		wr = sg_next(wr);
@@ -527,6 +534,7 @@ static struct sg_table *vb2_dma_sg_dmabuf_ops_map(
 		mutex_unlock(lock);
 		return ERR_PTR(-EIO);
 	}
+	sgt->nents = ret;
 
 	attach->dma_dir = dma_dir;
 
-- 
2.1.4

[PATCH 2/3] media/videobuf2-dma-contig: Fix handling of sg_table structure

[Ricardo Ribalda Delgado]

when sg_alloc_table_from_pages() does not fail it returns a sg_table
structure with nents and nents_orig initialized to the same value.

dma_map_sg returns the dma_map_sg returns the number of areas mapped
by the hardware, which could be different than the areas given as an input.
The output must be saved to nent.
Unfortunately nent differs in sign to the output of dma_map_sg, so an
intermediate value must be used.

The output of dma_map, should be used to transverse the scatter list.

dma_unmap_sg needs the value passed to dma_map_sg (nents_orig).

sg_free_tables uses also orig_nent.

This patch fix the file to follow this paradigm.

Signed-off-by: Ricardo Ribalda Delgado <ricardo.ribalda@gmail.com>
---
 drivers/media/v4l2-core/videobuf2-dma-contig.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/media/v4l2-core/videobuf2-dma-contig.c b/drivers/media/v4l2-core/videobuf2-dma-contig.c
index b481d20..c7e4bdd 100644
--- a/drivers/media/v4l2-core/videobuf2-dma-contig.c
+++ b/drivers/media/v4l2-core/videobuf2-dma-contig.c
@@ -56,7 +56,7 @@ static void vb2_dc_sgt_foreach_page(struct sg_table *sgt,
 	struct scatterlist *s;
 	unsigned int i;
 
-	for_each_sg(sgt->sgl, s, sgt->orig_nents, i) {
+	for_each_sg(sgt->sgl, s, sgt->nents, i) {
 		struct page *page = sg_page(s);
 		unsigned int n_pages = PAGE_ALIGN(s->offset + s->length)
 			>> PAGE_SHIFT;
@@ -260,7 +260,7 @@ static int vb2_dc_dmabuf_ops_attach(struct dma_buf *dbuf, struct device *dev,
 
 	rd = buf->sgt_base->sgl;
 	wr = sgt->sgl;
-	for (i = 0; i < sgt->orig_nents; ++i) {
+	for (i = 0; i < sgt->nents; ++i) {
 		sg_set_page(wr, sg_page(rd), rd->length, rd->offset);
 		rd = sg_next(rd);
 		wr = sg_next(wr);
@@ -324,6 +324,7 @@ static struct sg_table *vb2_dc_dmabuf_ops_map(
 		mutex_unlock(lock);
 		return ERR_PTR(-EIO);
 	}
+	sgt->nents = ret;
 
 	attach->dma_dir = dma_dir;
 
@@ -669,13 +670,14 @@ static void *vb2_dc_get_userptr(void *alloc_ctx, unsigned long vaddr,
 	 * No need to sync to the device, this will happen later when the
 	 * prepare() memop is called.
 	 */
-	sgt->nents = dma_map_sg_attrs(buf->dev, sgt->sgl, sgt->orig_nents,
+	ret = dma_map_sg_attrs(buf->dev, sgt->sgl, sgt->orig_nents,
 				      buf->dma_dir, &attrs);
-	if (sgt->nents <= 0) {
+	if (ret <= 0) {
 		pr_err("failed to map scatterlist\n");
 		ret = -EIO;
 		goto fail_sgt_init;
 	}
+	sgt->nents = ret;
 
 	contig_size = vb2_dc_get_contiguous_size(sgt);
 	if (contig_size < size) {
-- 
2.1.4

[PATCH 3/3] media/videobuf2-dma-vmalloc: Save output from dma_map_sg

[Ricardo Ribalda Delgado]

dma_map_sg returns the actual number of areas mapped. Save it on nents.

Signed-off-by: Ricardo Ribalda Delgado <ricardo.ribalda@gmail.com>
---
 drivers/media/v4l2-core/videobuf2-vmalloc.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/media/v4l2-core/videobuf2-vmalloc.c b/drivers/media/v4l2-core/videobuf2-vmalloc.c
index bcde885..fe18e79 100644
--- a/drivers/media/v4l2-core/videobuf2-vmalloc.c
+++ b/drivers/media/v4l2-core/videobuf2-vmalloc.c
@@ -312,6 +312,7 @@ static struct sg_table *vb2_vmalloc_dmabuf_ops_map(
 		mutex_unlock(lock);
 		return ERR_PTR(-EIO);
 	}
+	sgt->nents = ret;
 
 	attach->dma_dir = dma_dir;
 
-- 
2.1.4

Re: [PATCH 1/3] media/videobuf2-dma-sg: Fix handling of sg_table structure

[Marek Szyprowski]

Hello,

On 2015-02-09 17:14, Ricardo Ribalda Delgado wrote:
> when sg_alloc_table_from_pages() does not fail it returns a sg_table
> structure with nents and nents_orig initialized to the same value.
>
> dma_map_sg returns the dma_map_sg returns the number of areas mapped
> by the hardware, which could be different than the areas given as an input.
> The output must be saved to nent.

Thanks for catching this issue!

> Unfortunately nent differs in sign to the output of dma_map_sg, so an
> intermediate value must be used.

I don't get this part. dma_map_sg() returns the number of scatter list 
entries mapped
to the hardware or zero if anything fails. What is the problem of 
assigning it directly
to nents?

> The output of dma_map, should be used to transverse the scatter list.
>
> dma_unmap_sg needs the value passed to dma_map_sg (nents_orig).
>
> sg_free_tables uses also orig_nent.
>
> This patch fix the file to follow this paradigm.
>
> Signed-off-by: Ricardo Ribalda Delgado <ricardo.ribalda@gmail.com>
> ---
>   drivers/media/v4l2-core/videobuf2-dma-sg.c | 22 +++++++++++++++-------
>   1 file changed, 15 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/media/v4l2-core/videobuf2-dma-sg.c b/drivers/media/v4l2-core/videobuf2-dma-sg.c
> index b1838ab..30bac99 100644
> --- a/drivers/media/v4l2-core/videobuf2-dma-sg.c
> +++ b/drivers/media/v4l2-core/videobuf2-dma-sg.c
> @@ -147,9 +147,11 @@ static void *vb2_dma_sg_alloc(void *alloc_ctx, unsigned long size,
>   	 * No need to sync to the device, this will happen later when the
>   	 * prepare() memop is called.
>   	 */
> -	if (dma_map_sg_attrs(buf->dev, sgt->sgl, sgt->nents,
> -			     buf->dma_dir, &attrs) == 0)
> +	ret = dma_map_sg_attrs(buf->dev, sgt->sgl, sgt->orig_nents,
> +			buf->dma_dir, &attrs);
> +	if (ret <= 0)
>   		goto fail_map;
> +	sgt->nents = ret;
>   
>   	buf->handler.refcount = &buf->refcount;
>   	buf->handler.put = vb2_dma_sg_put;
> @@ -187,7 +189,7 @@ static void vb2_dma_sg_put(void *buf_priv)
>   		dma_set_attr(DMA_ATTR_SKIP_CPU_SYNC, &attrs);
>   		dprintk(1, "%s: Freeing buffer of %d pages\n", __func__,
>   			buf->num_pages);
> -		dma_unmap_sg_attrs(buf->dev, sgt->sgl, sgt->nents,
> +		dma_unmap_sg_attrs(buf->dev, sgt->sgl, sgt->orig_nents,
>   				   buf->dma_dir, &attrs);
>   		if (buf->vaddr)
>   			vm_unmap_ram(buf->vaddr, buf->num_pages);
> @@ -240,6 +242,7 @@ static void *vb2_dma_sg_get_userptr(void *alloc_ctx, unsigned long vaddr,
>   	struct vm_area_struct *vma;
>   	struct sg_table *sgt;
>   	DEFINE_DMA_ATTRS(attrs);
> +	int ret;
>   
>   	dma_set_attr(DMA_ATTR_SKIP_CPU_SYNC, &attrs);
>   
> @@ -314,9 +317,12 @@ static void *vb2_dma_sg_get_userptr(void *alloc_ctx, unsigned long vaddr,
>   	 * No need to sync to the device, this will happen later when the
>   	 * prepare() memop is called.
>   	 */
> -	if (dma_map_sg_attrs(buf->dev, sgt->sgl, sgt->nents,
> -			     buf->dma_dir, &attrs) == 0)
> +	ret = dma_map_sg_attrs(buf->dev, sgt->sgl, sgt->orig_nents,
> +			     buf->dma_dir, &attrs);
> +	if (ret <= 0)
>   		goto userptr_fail_map;

dma_map_sg_attrs() return 0 in case of error, so the check can be 
simplified,
there is no need for temporary variable.

> +	sgt->nents = ret;
> +
>   	return buf;
>   
>   userptr_fail_map:
> @@ -351,7 +357,8 @@ static void vb2_dma_sg_put_userptr(void *buf_priv)
>   
>   	dprintk(1, "%s: Releasing userspace buffer of %d pages\n",
>   	       __func__, buf->num_pages);
> -	dma_unmap_sg_attrs(buf->dev, sgt->sgl, sgt->nents, buf->dma_dir, &attrs);
> +	dma_unmap_sg_attrs(buf->dev, sgt->sgl, sgt->orig_nents, buf->dma_dir,
> +			&attrs);
>   	if (buf->vaddr)
>   		vm_unmap_ram(buf->vaddr, buf->num_pages);
>   	sg_free_table(buf->dma_sgt);
> @@ -463,7 +470,7 @@ static int vb2_dma_sg_dmabuf_ops_attach(struct dma_buf *dbuf, struct device *dev
>   
>   	rd = buf->dma_sgt->sgl;
>   	wr = sgt->sgl;
> -	for (i = 0; i < sgt->orig_nents; ++i) {
> +	for (i = 0; i < sgt->nents; ++i) {

Here the code iterates over every memory page in the scatter list (to create
a copy of it), not the device mapped chunks, so it must use orig_nents
like it was already there.

>   		sg_set_page(wr, sg_page(rd), rd->length, rd->offset);
>   		rd = sg_next(rd);
>   		wr = sg_next(wr);
> @@ -527,6 +534,7 @@ static struct sg_table *vb2_dma_sg_dmabuf_ops_map(
>   		mutex_unlock(lock);
>   		return ERR_PTR(-EIO);
>   	}
> +	sgt->nents = ret;

This one is okay.

>   
>   	attach->dma_dir = dma_dir;
>   

Best regards
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland

Re: [PATCH 2/3] media/videobuf2-dma-contig: Fix handling of sg_table structure

[Marek Szyprowski]

Hello,

On 2015-02-09 17:14, Ricardo Ribalda Delgado wrote:
> when sg_alloc_table_from_pages() does not fail it returns a sg_table
> structure with nents and nents_orig initialized to the same value.
>
> dma_map_sg returns the dma_map_sg returns the number of areas mapped
> by the hardware, which could be different than the areas given as an input.
> The output must be saved to nent.
> Unfortunately nent differs in sign to the output of dma_map_sg, so an
> intermediate value must be used.
>
> The output of dma_map, should be used to transverse the scatter list.
>
> dma_unmap_sg needs the value passed to dma_map_sg (nents_orig).
>
> sg_free_tables uses also orig_nent.
>
> This patch fix the file to follow this paradigm.
>
> Signed-off-by: Ricardo Ribalda Delgado <ricardo.ribalda@gmail.com>
> ---
>   drivers/media/v4l2-core/videobuf2-dma-contig.c | 10 ++++++----
>   1 file changed, 6 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/media/v4l2-core/videobuf2-dma-contig.c b/drivers/media/v4l2-core/videobuf2-dma-contig.c
> index b481d20..c7e4bdd 100644
> --- a/drivers/media/v4l2-core/videobuf2-dma-contig.c
> +++ b/drivers/media/v4l2-core/videobuf2-dma-contig.c
> @@ -56,7 +56,7 @@ static void vb2_dc_sgt_foreach_page(struct sg_table *sgt,
>   	struct scatterlist *s;
>   	unsigned int i;
>   
> -	for_each_sg(sgt->sgl, s, sgt->orig_nents, i) {
> +	for_each_sg(sgt->sgl, s, sgt->nents, i) {
>   		struct page *page = sg_page(s);
>   		unsigned int n_pages = PAGE_ALIGN(s->offset + s->length)
>   			>> PAGE_SHIFT;

This code iterates over memory pages added to the scatter list not the 
dma chunks,
so orig_nents must be used. This change is not needed.

> @@ -260,7 +260,7 @@ static int vb2_dc_dmabuf_ops_attach(struct dma_buf *dbuf, struct device *dev,
>   
>   	rd = buf->sgt_base->sgl;
>   	wr = sgt->sgl;
> -	for (i = 0; i < sgt->orig_nents; ++i) {
> +	for (i = 0; i < sgt->nents; ++i) {
>   		sg_set_page(wr, sg_page(rd), rd->length, rd->offset);
>   		rd = sg_next(rd);
>   		wr = sg_next(wr);

Same comment as for videobuf2-dma-sg.c patch.

> @@ -324,6 +324,7 @@ static struct sg_table *vb2_dc_dmabuf_ops_map(
>   		mutex_unlock(lock);
>   		return ERR_PTR(-EIO);
>   	}
> +	sgt->nents = ret;
>   
>   	attach->dma_dir = dma_dir;
>   
> @@ -669,13 +670,14 @@ static void *vb2_dc_get_userptr(void *alloc_ctx, unsigned long vaddr,
>   	 * No need to sync to the device, this will happen later when the
>   	 * prepare() memop is called.
>   	 */
> -	sgt->nents = dma_map_sg_attrs(buf->dev, sgt->sgl, sgt->orig_nents,
> +	ret = dma_map_sg_attrs(buf->dev, sgt->sgl, sgt->orig_nents,
>   				      buf->dma_dir, &attrs);
> -	if (sgt->nents <= 0) {
> +	if (ret <= 0) {
>   		pr_err("failed to map scatterlist\n");
>   		ret = -EIO;
>   		goto fail_sgt_init;
>   	}
> +	sgt->nents = ret;

This one is okay, although the check for error could be simplified to a 
check
for zero value.

>   
>   	contig_size = vb2_dc_get_contiguous_size(sgt);
>   	if (contig_size < size) {

Best regards
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland

Re: [PATCH 1/3] media/videobuf2-dma-sg: Fix handling of sg_table structure

[Ricardo Ribalda Delgado]

Hello Marek
On Wed, Feb 11, 2015 at 9:06 AM, Marek Szyprowski
<m.szyprowski@samsung.com> wrote:
>> Unfortunately nent differs in sign to the output of dma_map_sg, so an
>> intermediate value must be used.
>
>
> I don't get this part. dma_map_sg() returns the number of scatter list
> entries mapped
> to the hardware or zero if anything fails. What is the problem of assigning
> it directly
> to nents?

Are you sure about that?

The prototype of the function is (from dma-mapping-common.h)
static inline int dma_map_sg_attrs(struct device *dev, struct scatterlist *sg,
  int nents, enum dma_data_direction dir,
  struct dma_attrs *attrs)

which calls map_sg at the struct dma_map_ops (dma-mapping.h)

int (*map_sg)(struct device *dev, struct scatterlist *sg,
     int nents, enum dma_data_direction dir,
     struct dma_attrs *attrs);

Both return int instead of unsigned int....

>
>
> dma_map_sg_attrs() return 0 in case of error, so the check can be
> simplified,
> there is no need for temporary variable.

Check last comment

>>                 vm_unmap_ram(buf->vaddr, buf->num_pages);
>>         sg_free_table(buf->dma_sgt);
>> @@ -463,7 +470,7 @@ static int vb2_dma_sg_dmabuf_ops_attach(struct dma_buf
>> *dbuf, struct device *dev
>>         rd = buf->dma_sgt->sgl;
>>         wr = sgt->sgl;
>> -       for (i = 0; i < sgt->orig_nents; ++i) {
>> +       for (i = 0; i < sgt->nents; ++i) {
>
>
> Here the code iterates over every memory page in the scatter list (to create
> a copy of it), not the device mapped chunks, so it must use orig_nents
> like it was already there.

At that point both have the same value, but you are right, it is more
clear to use orig_nents

>

>
> Best regards


I will resend a version using orig_nents in dmabug_ops attach, but
please take a look the map_sg, I think it can return <0

Best regards!

-- 
Ricardo Ribalda

Re: [PATCH 1/3] media/videobuf2-dma-sg: Fix handling of sg_table structure

[Marek Szyprowski]

Hello,

On 2015-02-11 09:37, Ricardo Ribalda Delgado wrote:
> Hello Marek
> On Wed, Feb 11, 2015 at 9:06 AM, Marek Szyprowski
> <m.szyprowski@samsung.com> wrote:
>>> Unfortunately nent differs in sign to the output of dma_map_sg, so an
>>> intermediate value must be used.
>>
>> I don't get this part. dma_map_sg() returns the number of scatter list
>> entries mapped
>> to the hardware or zero if anything fails. What is the problem of assigning
>> it directly
>> to nents?
> Are you sure about that?
>
> The prototype of the function is (from dma-mapping-common.h)
> static inline int dma_map_sg_attrs(struct device *dev, struct scatterlist *sg,
>    int nents, enum dma_data_direction dir,
>    struct dma_attrs *attrs)
>
> which calls map_sg at the struct dma_map_ops (dma-mapping.h)
>
> int (*map_sg)(struct device *dev, struct scatterlist *sg,
>       int nents, enum dma_data_direction dir,
>       struct dma_attrs *attrs);
>
> Both return int instead of unsigned int....

Well, this int return value seems to be misleading, but according to
Documentation/DMA-API.txt, the only error value is zero:

"As with the other mapping interfaces, dma_map_sg() can fail. When it
does, 0 is returned and a driver must take appropriate action. It is
critical that the driver do something, in the case of a block driver
aborting the request or even oopsing is better than doing nothing and
corrupting the filesystem."

I've also checked various dma-mapping implementation for different
architectures and they follow this convention.

Maybe one should add some comments to include/linux/dma_mapping.h to
clarify this and avoid further confusion.


>
>> dma_map_sg_attrs() return 0 in case of error, so the check can be
>> simplified,
>> there is no need for temporary variable.
> Check last comment
>
>>>                  vm_unmap_ram(buf->vaddr, buf->num_pages);
>>>          sg_free_table(buf->dma_sgt);
>>> @@ -463,7 +470,7 @@ static int vb2_dma_sg_dmabuf_ops_attach(struct dma_buf
>>> *dbuf, struct device *dev
>>>          rd = buf->dma_sgt->sgl;
>>>          wr = sgt->sgl;
>>> -       for (i = 0; i < sgt->orig_nents; ++i) {
>>> +       for (i = 0; i < sgt->nents; ++i) {
>>
>> Here the code iterates over every memory page in the scatter list (to create
>> a copy of it), not the device mapped chunks, so it must use orig_nents
>> like it was already there.
> At that point both have the same value, but you are right, it is more
> clear to use orig_nents
>
>
>
> I will resend a version using orig_nents in dmabug_ops attach, but
> please take a look the map_sg, I think it can return <0
>
>

Best regards
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland

Re: [PATCH 1/3] media/videobuf2-dma-sg: Fix handling of sg_table structure

[Ricardo Ribalda Delgado]

Hello again

On Wed, Feb 11, 2015 at 10:00 AM, Marek Szyprowski
<m.szyprowski@samsung.com> wrote:

> Well, this int return value seems to be misleading, but according to
> Documentation/DMA-API.txt, the only error value is zero:
>
> "As with the other mapping interfaces, dma_map_sg() can fail. When it
> does, 0 is returned and a driver must take appropriate action. It is
> critical that the driver do something, in the case of a block driver
> aborting the request or even oopsing is better than doing nothing and
> corrupting the filesystem."
>
> I've also checked various dma-mapping implementation for different
> architectures and they follow this convention.
>
> Maybe one should add some comments to include/linux/dma_mapping.h to
> clarify this and avoid further confusion.
>
>

Or maybe change it to unsigned int...

Let me redo the patch and resend. I will also try to ping whoever is
the maintainer of dma_mapping

Thanks!



-- 
Ricardo Ribalda