linux-media.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH for v3.18] media: use sg = sg_next(sg) instead of sg++
@ 2014-11-21  9:20 Hans Verkuil
  2014-11-21 10:05 ` Hans Verkuil
  2014-11-24 18:24 ` Mauro Carvalho Chehab
  0 siblings, 2 replies; 7+ messages in thread
From: Hans Verkuil @ 2014-11-21  9:20 UTC (permalink / raw)
  To: Linux Media Mailing List

Several drivers (mostly copy-and-paste) still used sg++ instead of
sg = sg_next(sg). Fix them since sg++ won't work if contiguous scatter
entries where combined into one larger entry.

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
Cc: stable@vger.kernel.org      # for v3.7 and up
---
 drivers/media/pci/bt8xx/bttv-risc.c      | 12 ++++++------
 drivers/media/pci/cx23885/cx23885-core.c |  6 +++---
 drivers/media/pci/cx25821/cx25821-core.c | 12 ++++++------
 drivers/media/pci/cx88/cx88-core.c       |  6 +++---
 drivers/media/pci/ivtv/ivtv-udma.c       |  2 +-
 5 files changed, 19 insertions(+), 19 deletions(-)

diff --git a/drivers/media/pci/bt8xx/bttv-risc.c b/drivers/media/pci/bt8xx/bttv-risc.c
index 82cc47d..4d3f05a 100644
--- a/drivers/media/pci/bt8xx/bttv-risc.c
+++ b/drivers/media/pci/bt8xx/bttv-risc.c
@@ -84,7 +84,7 @@ bttv_risc_packed(struct bttv *btv, struct btcx_riscmem *risc,
 			continue;
 		while (offset && offset >= sg_dma_len(sg)) {
 			offset -= sg_dma_len(sg);
-			sg++;
+			sg = sg_next(sg);
 		}
 		if (bpl <= sg_dma_len(sg)-offset) {
 			/* fits into current chunk */
@@ -100,13 +100,13 @@ bttv_risc_packed(struct bttv *btv, struct btcx_riscmem *risc,
 			*(rp++)=cpu_to_le32(sg_dma_address(sg)+offset);
 			todo -= (sg_dma_len(sg)-offset);
 			offset = 0;
-			sg++;
+			sg = sg_next(sg);
 			while (todo > sg_dma_len(sg)) {
 				*(rp++)=cpu_to_le32(BT848_RISC_WRITE|
 						    sg_dma_len(sg));
 				*(rp++)=cpu_to_le32(sg_dma_address(sg));
 				todo -= sg_dma_len(sg);
-				sg++;
+				sg = sg_next(sg);
 			}
 			*(rp++)=cpu_to_le32(BT848_RISC_WRITE|BT848_RISC_EOL|
 					    todo);
@@ -187,15 +187,15 @@ bttv_risc_planar(struct bttv *btv, struct btcx_riscmem *risc,
 			/* go to next sg entry if needed */
 			while (yoffset && yoffset >= sg_dma_len(ysg)) {
 				yoffset -= sg_dma_len(ysg);
-				ysg++;
+				ysg = sg_next(ysg);
 			}
 			while (uoffset && uoffset >= sg_dma_len(usg)) {
 				uoffset -= sg_dma_len(usg);
-				usg++;
+				usg = sg_next(usg);
 			}
 			while (voffset && voffset >= sg_dma_len(vsg)) {
 				voffset -= sg_dma_len(vsg);
-				vsg++;
+				vsg = sg_next(vsg);
 			}
 
 			/* calculate max number of bytes we can write */
diff --git a/drivers/media/pci/cx23885/cx23885-core.c b/drivers/media/pci/cx23885/cx23885-core.c
index 331edda..3bd386c 100644
--- a/drivers/media/pci/cx23885/cx23885-core.c
+++ b/drivers/media/pci/cx23885/cx23885-core.c
@@ -1078,7 +1078,7 @@ static __le32 *cx23885_risc_field(__le32 *rp, struct scatterlist *sglist,
 	for (line = 0; line < lines; line++) {
 		while (offset && offset >= sg_dma_len(sg)) {
 			offset -= sg_dma_len(sg);
-			sg++;
+			sg = sg_next(sg);
 		}
 
 		if (lpi && line > 0 && !(line % lpi))
@@ -1101,14 +1101,14 @@ static __le32 *cx23885_risc_field(__le32 *rp, struct scatterlist *sglist,
 			*(rp++) = cpu_to_le32(0); /* bits 63-32 */
 			todo -= (sg_dma_len(sg)-offset);
 			offset = 0;
-			sg++;
+			sg = sg_next(sg);
 			while (todo > sg_dma_len(sg)) {
 				*(rp++) = cpu_to_le32(RISC_WRITE|
 						    sg_dma_len(sg));
 				*(rp++) = cpu_to_le32(sg_dma_address(sg));
 				*(rp++) = cpu_to_le32(0); /* bits 63-32 */
 				todo -= sg_dma_len(sg);
-				sg++;
+				sg = sg_next(sg);
 			}
 			*(rp++) = cpu_to_le32(RISC_WRITE|RISC_EOL|todo);
 			*(rp++) = cpu_to_le32(sg_dma_address(sg));
diff --git a/drivers/media/pci/cx25821/cx25821-core.c b/drivers/media/pci/cx25821/cx25821-core.c
index e81173c..389fffd 100644
--- a/drivers/media/pci/cx25821/cx25821-core.c
+++ b/drivers/media/pci/cx25821/cx25821-core.c
@@ -996,7 +996,7 @@ static __le32 *cx25821_risc_field(__le32 * rp, struct scatterlist *sglist,
 	for (line = 0; line < lines; line++) {
 		while (offset && offset >= sg_dma_len(sg)) {
 			offset -= sg_dma_len(sg);
-			sg++;
+			sg = sg_next(sg);
 		}
 		if (bpl <= sg_dma_len(sg) - offset) {
 			/* fits into current chunk */
@@ -1014,14 +1014,14 @@ static __le32 *cx25821_risc_field(__le32 * rp, struct scatterlist *sglist,
 			*(rp++) = cpu_to_le32(0);	/* bits 63-32 */
 			todo -= (sg_dma_len(sg) - offset);
 			offset = 0;
-			sg++;
+			sg = sg_next(sg);
 			while (todo > sg_dma_len(sg)) {
 				*(rp++) = cpu_to_le32(RISC_WRITE |
 						sg_dma_len(sg));
 				*(rp++) = cpu_to_le32(sg_dma_address(sg));
 				*(rp++) = cpu_to_le32(0);	/* bits 63-32 */
 				todo -= sg_dma_len(sg);
-				sg++;
+				sg = sg_next(sg);
 			}
 			*(rp++) = cpu_to_le32(RISC_WRITE | RISC_EOL | todo);
 			*(rp++) = cpu_to_le32(sg_dma_address(sg));
@@ -1101,7 +1101,7 @@ static __le32 *cx25821_risc_field_audio(__le32 * rp, struct scatterlist *sglist,
 	for (line = 0; line < lines; line++) {
 		while (offset && offset >= sg_dma_len(sg)) {
 			offset -= sg_dma_len(sg);
-			sg++;
+			sg = sg_next(sg);
 		}
 
 		if (lpi && line > 0 && !(line % lpi))
@@ -1125,14 +1125,14 @@ static __le32 *cx25821_risc_field_audio(__le32 * rp, struct scatterlist *sglist,
 			*(rp++) = cpu_to_le32(0);	/* bits 63-32 */
 			todo -= (sg_dma_len(sg) - offset);
 			offset = 0;
-			sg++;
+			sg = sg_next(sg);
 			while (todo > sg_dma_len(sg)) {
 				*(rp++) = cpu_to_le32(RISC_WRITE |
 						sg_dma_len(sg));
 				*(rp++) = cpu_to_le32(sg_dma_address(sg));
 				*(rp++) = cpu_to_le32(0);	/* bits 63-32 */
 				todo -= sg_dma_len(sg);
-				sg++;
+				sg = sg_next(sg);
 			}
 			*(rp++) = cpu_to_le32(RISC_WRITE | RISC_EOL | todo);
 			*(rp++) = cpu_to_le32(sg_dma_address(sg));
diff --git a/drivers/media/pci/cx88/cx88-core.c b/drivers/media/pci/cx88/cx88-core.c
index 9fa4acb..dee177e 100644
--- a/drivers/media/pci/cx88/cx88-core.c
+++ b/drivers/media/pci/cx88/cx88-core.c
@@ -95,7 +95,7 @@ static __le32* cx88_risc_field(__le32 *rp, struct scatterlist *sglist,
 	for (line = 0; line < lines; line++) {
 		while (offset && offset >= sg_dma_len(sg)) {
 			offset -= sg_dma_len(sg);
-			sg++;
+			sg = sg_next(sg);
 		}
 		if (lpi && line>0 && !(line % lpi))
 			sol = RISC_SOL | RISC_IRQ1 | RISC_CNT_INC;
@@ -114,13 +114,13 @@ static __le32* cx88_risc_field(__le32 *rp, struct scatterlist *sglist,
 			*(rp++)=cpu_to_le32(sg_dma_address(sg)+offset);
 			todo -= (sg_dma_len(sg)-offset);
 			offset = 0;
-			sg++;
+			sg = sg_next(sg);
 			while (todo > sg_dma_len(sg)) {
 				*(rp++)=cpu_to_le32(RISC_WRITE|
 						    sg_dma_len(sg));
 				*(rp++)=cpu_to_le32(sg_dma_address(sg));
 				todo -= sg_dma_len(sg);
-				sg++;
+				sg = sg_next(sg);
 			}
 			*(rp++)=cpu_to_le32(RISC_WRITE|RISC_EOL|todo);
 			*(rp++)=cpu_to_le32(sg_dma_address(sg));
diff --git a/drivers/media/pci/ivtv/ivtv-udma.c b/drivers/media/pci/ivtv/ivtv-udma.c
index 7338cb2..bee2329 100644
--- a/drivers/media/pci/ivtv/ivtv-udma.c
+++ b/drivers/media/pci/ivtv/ivtv-udma.c
@@ -76,7 +76,7 @@ void ivtv_udma_fill_sg_array (struct ivtv_user_dma *dma, u32 buffer_offset, u32
 	int i;
 	struct scatterlist *sg;
 
-	for (i = 0, sg = dma->SGlist; i < dma->SG_length; i++, sg++) {
+	for (i = 0, sg = dma->SGlist; i < dma->SG_length; i++, sg = sg_next(sg)) {
 		dma->SGarray[i].size = cpu_to_le32(sg_dma_len(sg));
 		dma->SGarray[i].src = cpu_to_le32(sg_dma_address(sg));
 		dma->SGarray[i].dst = cpu_to_le32(buffer_offset);
-- 
2.1.1


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH for v3.18] media: use sg = sg_next(sg) instead of sg++
  2014-11-21  9:20 [PATCH for v3.18] media: use sg = sg_next(sg) instead of sg++ Hans Verkuil
@ 2014-11-21 10:05 ` Hans Verkuil
  2014-11-21 10:50   ` Hans Verkuil
  2014-11-24 18:24 ` Mauro Carvalho Chehab
  1 sibling, 1 reply; 7+ messages in thread
From: Hans Verkuil @ 2014-11-21 10:05 UTC (permalink / raw)
  To: Linux Media Mailing List; +Cc: Mariusz Bialonczyk

Mariusz,

Thanks for reporting and testing this so quickly!

On 11/21/2014 10:20 AM, Hans Verkuil wrote:
> Several drivers (mostly copy-and-paste) still used sg++ instead of
> sg = sg_next(sg). Fix them since sg++ won't work if contiguous scatter
> entries where combined into one larger entry.
> 
> Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>

Reported-by: Mariusz Bialonczyk <manio@skyboo.net>
Tested-by: Mariusz Bialonczyk <manio@skyboo.net>

Regards,

	Hans

> Cc: stable@vger.kernel.org      # for v3.7 and up
> ---
>  drivers/media/pci/bt8xx/bttv-risc.c      | 12 ++++++------
>  drivers/media/pci/cx23885/cx23885-core.c |  6 +++---
>  drivers/media/pci/cx25821/cx25821-core.c | 12 ++++++------
>  drivers/media/pci/cx88/cx88-core.c       |  6 +++---
>  drivers/media/pci/ivtv/ivtv-udma.c       |  2 +-
>  5 files changed, 19 insertions(+), 19 deletions(-)
> 
> diff --git a/drivers/media/pci/bt8xx/bttv-risc.c b/drivers/media/pci/bt8xx/bttv-risc.c
> index 82cc47d..4d3f05a 100644
> --- a/drivers/media/pci/bt8xx/bttv-risc.c
> +++ b/drivers/media/pci/bt8xx/bttv-risc.c
> @@ -84,7 +84,7 @@ bttv_risc_packed(struct bttv *btv, struct btcx_riscmem *risc,
>  			continue;
>  		while (offset && offset >= sg_dma_len(sg)) {
>  			offset -= sg_dma_len(sg);
> -			sg++;
> +			sg = sg_next(sg);
>  		}
>  		if (bpl <= sg_dma_len(sg)-offset) {
>  			/* fits into current chunk */
> @@ -100,13 +100,13 @@ bttv_risc_packed(struct bttv *btv, struct btcx_riscmem *risc,
>  			*(rp++)=cpu_to_le32(sg_dma_address(sg)+offset);
>  			todo -= (sg_dma_len(sg)-offset);
>  			offset = 0;
> -			sg++;
> +			sg = sg_next(sg);
>  			while (todo > sg_dma_len(sg)) {
>  				*(rp++)=cpu_to_le32(BT848_RISC_WRITE|
>  						    sg_dma_len(sg));
>  				*(rp++)=cpu_to_le32(sg_dma_address(sg));
>  				todo -= sg_dma_len(sg);
> -				sg++;
> +				sg = sg_next(sg);
>  			}
>  			*(rp++)=cpu_to_le32(BT848_RISC_WRITE|BT848_RISC_EOL|
>  					    todo);
> @@ -187,15 +187,15 @@ bttv_risc_planar(struct bttv *btv, struct btcx_riscmem *risc,
>  			/* go to next sg entry if needed */
>  			while (yoffset && yoffset >= sg_dma_len(ysg)) {
>  				yoffset -= sg_dma_len(ysg);
> -				ysg++;
> +				ysg = sg_next(ysg);
>  			}
>  			while (uoffset && uoffset >= sg_dma_len(usg)) {
>  				uoffset -= sg_dma_len(usg);
> -				usg++;
> +				usg = sg_next(usg);
>  			}
>  			while (voffset && voffset >= sg_dma_len(vsg)) {
>  				voffset -= sg_dma_len(vsg);
> -				vsg++;
> +				vsg = sg_next(vsg);
>  			}
>  
>  			/* calculate max number of bytes we can write */
> diff --git a/drivers/media/pci/cx23885/cx23885-core.c b/drivers/media/pci/cx23885/cx23885-core.c
> index 331edda..3bd386c 100644
> --- a/drivers/media/pci/cx23885/cx23885-core.c
> +++ b/drivers/media/pci/cx23885/cx23885-core.c
> @@ -1078,7 +1078,7 @@ static __le32 *cx23885_risc_field(__le32 *rp, struct scatterlist *sglist,
>  	for (line = 0; line < lines; line++) {
>  		while (offset && offset >= sg_dma_len(sg)) {
>  			offset -= sg_dma_len(sg);
> -			sg++;
> +			sg = sg_next(sg);
>  		}
>  
>  		if (lpi && line > 0 && !(line % lpi))
> @@ -1101,14 +1101,14 @@ static __le32 *cx23885_risc_field(__le32 *rp, struct scatterlist *sglist,
>  			*(rp++) = cpu_to_le32(0); /* bits 63-32 */
>  			todo -= (sg_dma_len(sg)-offset);
>  			offset = 0;
> -			sg++;
> +			sg = sg_next(sg);
>  			while (todo > sg_dma_len(sg)) {
>  				*(rp++) = cpu_to_le32(RISC_WRITE|
>  						    sg_dma_len(sg));
>  				*(rp++) = cpu_to_le32(sg_dma_address(sg));
>  				*(rp++) = cpu_to_le32(0); /* bits 63-32 */
>  				todo -= sg_dma_len(sg);
> -				sg++;
> +				sg = sg_next(sg);
>  			}
>  			*(rp++) = cpu_to_le32(RISC_WRITE|RISC_EOL|todo);
>  			*(rp++) = cpu_to_le32(sg_dma_address(sg));
> diff --git a/drivers/media/pci/cx25821/cx25821-core.c b/drivers/media/pci/cx25821/cx25821-core.c
> index e81173c..389fffd 100644
> --- a/drivers/media/pci/cx25821/cx25821-core.c
> +++ b/drivers/media/pci/cx25821/cx25821-core.c
> @@ -996,7 +996,7 @@ static __le32 *cx25821_risc_field(__le32 * rp, struct scatterlist *sglist,
>  	for (line = 0; line < lines; line++) {
>  		while (offset && offset >= sg_dma_len(sg)) {
>  			offset -= sg_dma_len(sg);
> -			sg++;
> +			sg = sg_next(sg);
>  		}
>  		if (bpl <= sg_dma_len(sg) - offset) {
>  			/* fits into current chunk */
> @@ -1014,14 +1014,14 @@ static __le32 *cx25821_risc_field(__le32 * rp, struct scatterlist *sglist,
>  			*(rp++) = cpu_to_le32(0);	/* bits 63-32 */
>  			todo -= (sg_dma_len(sg) - offset);
>  			offset = 0;
> -			sg++;
> +			sg = sg_next(sg);
>  			while (todo > sg_dma_len(sg)) {
>  				*(rp++) = cpu_to_le32(RISC_WRITE |
>  						sg_dma_len(sg));
>  				*(rp++) = cpu_to_le32(sg_dma_address(sg));
>  				*(rp++) = cpu_to_le32(0);	/* bits 63-32 */
>  				todo -= sg_dma_len(sg);
> -				sg++;
> +				sg = sg_next(sg);
>  			}
>  			*(rp++) = cpu_to_le32(RISC_WRITE | RISC_EOL | todo);
>  			*(rp++) = cpu_to_le32(sg_dma_address(sg));
> @@ -1101,7 +1101,7 @@ static __le32 *cx25821_risc_field_audio(__le32 * rp, struct scatterlist *sglist,
>  	for (line = 0; line < lines; line++) {
>  		while (offset && offset >= sg_dma_len(sg)) {
>  			offset -= sg_dma_len(sg);
> -			sg++;
> +			sg = sg_next(sg);
>  		}
>  
>  		if (lpi && line > 0 && !(line % lpi))
> @@ -1125,14 +1125,14 @@ static __le32 *cx25821_risc_field_audio(__le32 * rp, struct scatterlist *sglist,
>  			*(rp++) = cpu_to_le32(0);	/* bits 63-32 */
>  			todo -= (sg_dma_len(sg) - offset);
>  			offset = 0;
> -			sg++;
> +			sg = sg_next(sg);
>  			while (todo > sg_dma_len(sg)) {
>  				*(rp++) = cpu_to_le32(RISC_WRITE |
>  						sg_dma_len(sg));
>  				*(rp++) = cpu_to_le32(sg_dma_address(sg));
>  				*(rp++) = cpu_to_le32(0);	/* bits 63-32 */
>  				todo -= sg_dma_len(sg);
> -				sg++;
> +				sg = sg_next(sg);
>  			}
>  			*(rp++) = cpu_to_le32(RISC_WRITE | RISC_EOL | todo);
>  			*(rp++) = cpu_to_le32(sg_dma_address(sg));
> diff --git a/drivers/media/pci/cx88/cx88-core.c b/drivers/media/pci/cx88/cx88-core.c
> index 9fa4acb..dee177e 100644
> --- a/drivers/media/pci/cx88/cx88-core.c
> +++ b/drivers/media/pci/cx88/cx88-core.c
> @@ -95,7 +95,7 @@ static __le32* cx88_risc_field(__le32 *rp, struct scatterlist *sglist,
>  	for (line = 0; line < lines; line++) {
>  		while (offset && offset >= sg_dma_len(sg)) {
>  			offset -= sg_dma_len(sg);
> -			sg++;
> +			sg = sg_next(sg);
>  		}
>  		if (lpi && line>0 && !(line % lpi))
>  			sol = RISC_SOL | RISC_IRQ1 | RISC_CNT_INC;
> @@ -114,13 +114,13 @@ static __le32* cx88_risc_field(__le32 *rp, struct scatterlist *sglist,
>  			*(rp++)=cpu_to_le32(sg_dma_address(sg)+offset);
>  			todo -= (sg_dma_len(sg)-offset);
>  			offset = 0;
> -			sg++;
> +			sg = sg_next(sg);
>  			while (todo > sg_dma_len(sg)) {
>  				*(rp++)=cpu_to_le32(RISC_WRITE|
>  						    sg_dma_len(sg));
>  				*(rp++)=cpu_to_le32(sg_dma_address(sg));
>  				todo -= sg_dma_len(sg);
> -				sg++;
> +				sg = sg_next(sg);
>  			}
>  			*(rp++)=cpu_to_le32(RISC_WRITE|RISC_EOL|todo);
>  			*(rp++)=cpu_to_le32(sg_dma_address(sg));
> diff --git a/drivers/media/pci/ivtv/ivtv-udma.c b/drivers/media/pci/ivtv/ivtv-udma.c
> index 7338cb2..bee2329 100644
> --- a/drivers/media/pci/ivtv/ivtv-udma.c
> +++ b/drivers/media/pci/ivtv/ivtv-udma.c
> @@ -76,7 +76,7 @@ void ivtv_udma_fill_sg_array (struct ivtv_user_dma *dma, u32 buffer_offset, u32
>  	int i;
>  	struct scatterlist *sg;
>  
> -	for (i = 0, sg = dma->SGlist; i < dma->SG_length; i++, sg++) {
> +	for (i = 0, sg = dma->SGlist; i < dma->SG_length; i++, sg = sg_next(sg)) {
>  		dma->SGarray[i].size = cpu_to_le32(sg_dma_len(sg));
>  		dma->SGarray[i].src = cpu_to_le32(sg_dma_address(sg));
>  		dma->SGarray[i].dst = cpu_to_le32(buffer_offset);
> 


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH for v3.18] media: use sg = sg_next(sg) instead of sg++
  2014-11-21 10:05 ` Hans Verkuil
@ 2014-11-21 10:50   ` Hans Verkuil
  0 siblings, 0 replies; 7+ messages in thread
From: Hans Verkuil @ 2014-11-21 10:50 UTC (permalink / raw)
  To: Linux Media Mailing List; +Cc: Mariusz Bialonczyk

On 11/21/2014 11:05 AM, Hans Verkuil wrote:
> Mariusz,
> 
> Thanks for reporting and testing this so quickly!
> 
> On 11/21/2014 10:20 AM, Hans Verkuil wrote:
>> Several drivers (mostly copy-and-paste) still used sg++ instead of
>> sg = sg_next(sg). Fix them since sg++ won't work if contiguous scatter
>> entries where combined into one larger entry.

I did a bit more research and I found out why this hasn't been a problem
before: the vb1 code will not combine scatterlist entries since it fills
in the entries manually instead of relying on the core code as vb2 does.
So this is a result of switching cx23885 to vb2 after all, although the
driver code really should have been using sg_next regardless.

So I will post only the cx23885 fix to 3.18, the fixes for the other
drivers will be for 3.19 which is crucial for cx88 since that is moved
to vb2 in 3.19 as well, so it will result in a similar bug.

Regards,

	Hans

>>
>> Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
> 
> Reported-by: Mariusz Bialonczyk <manio@skyboo.net>
> Tested-by: Mariusz Bialonczyk <manio@skyboo.net>
> 
> Regards,
> 
> 	Hans
> 
>> Cc: stable@vger.kernel.org      # for v3.7 and up
>> ---
>>  drivers/media/pci/bt8xx/bttv-risc.c      | 12 ++++++------
>>  drivers/media/pci/cx23885/cx23885-core.c |  6 +++---
>>  drivers/media/pci/cx25821/cx25821-core.c | 12 ++++++------
>>  drivers/media/pci/cx88/cx88-core.c       |  6 +++---
>>  drivers/media/pci/ivtv/ivtv-udma.c       |  2 +-
>>  5 files changed, 19 insertions(+), 19 deletions(-)
>>
>> diff --git a/drivers/media/pci/bt8xx/bttv-risc.c b/drivers/media/pci/bt8xx/bttv-risc.c
>> index 82cc47d..4d3f05a 100644
>> --- a/drivers/media/pci/bt8xx/bttv-risc.c
>> +++ b/drivers/media/pci/bt8xx/bttv-risc.c
>> @@ -84,7 +84,7 @@ bttv_risc_packed(struct bttv *btv, struct btcx_riscmem *risc,
>>  			continue;
>>  		while (offset && offset >= sg_dma_len(sg)) {
>>  			offset -= sg_dma_len(sg);
>> -			sg++;
>> +			sg = sg_next(sg);
>>  		}
>>  		if (bpl <= sg_dma_len(sg)-offset) {
>>  			/* fits into current chunk */
>> @@ -100,13 +100,13 @@ bttv_risc_packed(struct bttv *btv, struct btcx_riscmem *risc,
>>  			*(rp++)=cpu_to_le32(sg_dma_address(sg)+offset);
>>  			todo -= (sg_dma_len(sg)-offset);
>>  			offset = 0;
>> -			sg++;
>> +			sg = sg_next(sg);
>>  			while (todo > sg_dma_len(sg)) {
>>  				*(rp++)=cpu_to_le32(BT848_RISC_WRITE|
>>  						    sg_dma_len(sg));
>>  				*(rp++)=cpu_to_le32(sg_dma_address(sg));
>>  				todo -= sg_dma_len(sg);
>> -				sg++;
>> +				sg = sg_next(sg);
>>  			}
>>  			*(rp++)=cpu_to_le32(BT848_RISC_WRITE|BT848_RISC_EOL|
>>  					    todo);
>> @@ -187,15 +187,15 @@ bttv_risc_planar(struct bttv *btv, struct btcx_riscmem *risc,
>>  			/* go to next sg entry if needed */
>>  			while (yoffset && yoffset >= sg_dma_len(ysg)) {
>>  				yoffset -= sg_dma_len(ysg);
>> -				ysg++;
>> +				ysg = sg_next(ysg);
>>  			}
>>  			while (uoffset && uoffset >= sg_dma_len(usg)) {
>>  				uoffset -= sg_dma_len(usg);
>> -				usg++;
>> +				usg = sg_next(usg);
>>  			}
>>  			while (voffset && voffset >= sg_dma_len(vsg)) {
>>  				voffset -= sg_dma_len(vsg);
>> -				vsg++;
>> +				vsg = sg_next(vsg);
>>  			}
>>  
>>  			/* calculate max number of bytes we can write */
>> diff --git a/drivers/media/pci/cx23885/cx23885-core.c b/drivers/media/pci/cx23885/cx23885-core.c
>> index 331edda..3bd386c 100644
>> --- a/drivers/media/pci/cx23885/cx23885-core.c
>> +++ b/drivers/media/pci/cx23885/cx23885-core.c
>> @@ -1078,7 +1078,7 @@ static __le32 *cx23885_risc_field(__le32 *rp, struct scatterlist *sglist,
>>  	for (line = 0; line < lines; line++) {
>>  		while (offset && offset >= sg_dma_len(sg)) {
>>  			offset -= sg_dma_len(sg);
>> -			sg++;
>> +			sg = sg_next(sg);
>>  		}
>>  
>>  		if (lpi && line > 0 && !(line % lpi))
>> @@ -1101,14 +1101,14 @@ static __le32 *cx23885_risc_field(__le32 *rp, struct scatterlist *sglist,
>>  			*(rp++) = cpu_to_le32(0); /* bits 63-32 */
>>  			todo -= (sg_dma_len(sg)-offset);
>>  			offset = 0;
>> -			sg++;
>> +			sg = sg_next(sg);
>>  			while (todo > sg_dma_len(sg)) {
>>  				*(rp++) = cpu_to_le32(RISC_WRITE|
>>  						    sg_dma_len(sg));
>>  				*(rp++) = cpu_to_le32(sg_dma_address(sg));
>>  				*(rp++) = cpu_to_le32(0); /* bits 63-32 */
>>  				todo -= sg_dma_len(sg);
>> -				sg++;
>> +				sg = sg_next(sg);
>>  			}
>>  			*(rp++) = cpu_to_le32(RISC_WRITE|RISC_EOL|todo);
>>  			*(rp++) = cpu_to_le32(sg_dma_address(sg));
>> diff --git a/drivers/media/pci/cx25821/cx25821-core.c b/drivers/media/pci/cx25821/cx25821-core.c
>> index e81173c..389fffd 100644
>> --- a/drivers/media/pci/cx25821/cx25821-core.c
>> +++ b/drivers/media/pci/cx25821/cx25821-core.c
>> @@ -996,7 +996,7 @@ static __le32 *cx25821_risc_field(__le32 * rp, struct scatterlist *sglist,
>>  	for (line = 0; line < lines; line++) {
>>  		while (offset && offset >= sg_dma_len(sg)) {
>>  			offset -= sg_dma_len(sg);
>> -			sg++;
>> +			sg = sg_next(sg);
>>  		}
>>  		if (bpl <= sg_dma_len(sg) - offset) {
>>  			/* fits into current chunk */
>> @@ -1014,14 +1014,14 @@ static __le32 *cx25821_risc_field(__le32 * rp, struct scatterlist *sglist,
>>  			*(rp++) = cpu_to_le32(0);	/* bits 63-32 */
>>  			todo -= (sg_dma_len(sg) - offset);
>>  			offset = 0;
>> -			sg++;
>> +			sg = sg_next(sg);
>>  			while (todo > sg_dma_len(sg)) {
>>  				*(rp++) = cpu_to_le32(RISC_WRITE |
>>  						sg_dma_len(sg));
>>  				*(rp++) = cpu_to_le32(sg_dma_address(sg));
>>  				*(rp++) = cpu_to_le32(0);	/* bits 63-32 */
>>  				todo -= sg_dma_len(sg);
>> -				sg++;
>> +				sg = sg_next(sg);
>>  			}
>>  			*(rp++) = cpu_to_le32(RISC_WRITE | RISC_EOL | todo);
>>  			*(rp++) = cpu_to_le32(sg_dma_address(sg));
>> @@ -1101,7 +1101,7 @@ static __le32 *cx25821_risc_field_audio(__le32 * rp, struct scatterlist *sglist,
>>  	for (line = 0; line < lines; line++) {
>>  		while (offset && offset >= sg_dma_len(sg)) {
>>  			offset -= sg_dma_len(sg);
>> -			sg++;
>> +			sg = sg_next(sg);
>>  		}
>>  
>>  		if (lpi && line > 0 && !(line % lpi))
>> @@ -1125,14 +1125,14 @@ static __le32 *cx25821_risc_field_audio(__le32 * rp, struct scatterlist *sglist,
>>  			*(rp++) = cpu_to_le32(0);	/* bits 63-32 */
>>  			todo -= (sg_dma_len(sg) - offset);
>>  			offset = 0;
>> -			sg++;
>> +			sg = sg_next(sg);
>>  			while (todo > sg_dma_len(sg)) {
>>  				*(rp++) = cpu_to_le32(RISC_WRITE |
>>  						sg_dma_len(sg));
>>  				*(rp++) = cpu_to_le32(sg_dma_address(sg));
>>  				*(rp++) = cpu_to_le32(0);	/* bits 63-32 */
>>  				todo -= sg_dma_len(sg);
>> -				sg++;
>> +				sg = sg_next(sg);
>>  			}
>>  			*(rp++) = cpu_to_le32(RISC_WRITE | RISC_EOL | todo);
>>  			*(rp++) = cpu_to_le32(sg_dma_address(sg));
>> diff --git a/drivers/media/pci/cx88/cx88-core.c b/drivers/media/pci/cx88/cx88-core.c
>> index 9fa4acb..dee177e 100644
>> --- a/drivers/media/pci/cx88/cx88-core.c
>> +++ b/drivers/media/pci/cx88/cx88-core.c
>> @@ -95,7 +95,7 @@ static __le32* cx88_risc_field(__le32 *rp, struct scatterlist *sglist,
>>  	for (line = 0; line < lines; line++) {
>>  		while (offset && offset >= sg_dma_len(sg)) {
>>  			offset -= sg_dma_len(sg);
>> -			sg++;
>> +			sg = sg_next(sg);
>>  		}
>>  		if (lpi && line>0 && !(line % lpi))
>>  			sol = RISC_SOL | RISC_IRQ1 | RISC_CNT_INC;
>> @@ -114,13 +114,13 @@ static __le32* cx88_risc_field(__le32 *rp, struct scatterlist *sglist,
>>  			*(rp++)=cpu_to_le32(sg_dma_address(sg)+offset);
>>  			todo -= (sg_dma_len(sg)-offset);
>>  			offset = 0;
>> -			sg++;
>> +			sg = sg_next(sg);
>>  			while (todo > sg_dma_len(sg)) {
>>  				*(rp++)=cpu_to_le32(RISC_WRITE|
>>  						    sg_dma_len(sg));
>>  				*(rp++)=cpu_to_le32(sg_dma_address(sg));
>>  				todo -= sg_dma_len(sg);
>> -				sg++;
>> +				sg = sg_next(sg);
>>  			}
>>  			*(rp++)=cpu_to_le32(RISC_WRITE|RISC_EOL|todo);
>>  			*(rp++)=cpu_to_le32(sg_dma_address(sg));
>> diff --git a/drivers/media/pci/ivtv/ivtv-udma.c b/drivers/media/pci/ivtv/ivtv-udma.c
>> index 7338cb2..bee2329 100644
>> --- a/drivers/media/pci/ivtv/ivtv-udma.c
>> +++ b/drivers/media/pci/ivtv/ivtv-udma.c
>> @@ -76,7 +76,7 @@ void ivtv_udma_fill_sg_array (struct ivtv_user_dma *dma, u32 buffer_offset, u32
>>  	int i;
>>  	struct scatterlist *sg;
>>  
>> -	for (i = 0, sg = dma->SGlist; i < dma->SG_length; i++, sg++) {
>> +	for (i = 0, sg = dma->SGlist; i < dma->SG_length; i++, sg = sg_next(sg)) {
>>  		dma->SGarray[i].size = cpu_to_le32(sg_dma_len(sg));
>>  		dma->SGarray[i].src = cpu_to_le32(sg_dma_address(sg));
>>  		dma->SGarray[i].dst = cpu_to_le32(buffer_offset);
>>
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-media" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH for v3.18] media: use sg = sg_next(sg) instead of sg++
  2014-11-21  9:20 [PATCH for v3.18] media: use sg = sg_next(sg) instead of sg++ Hans Verkuil
  2014-11-21 10:05 ` Hans Verkuil
@ 2014-11-24 18:24 ` Mauro Carvalho Chehab
  2014-11-24 18:37   ` Hans Verkuil
  1 sibling, 1 reply; 7+ messages in thread
From: Mauro Carvalho Chehab @ 2014-11-24 18:24 UTC (permalink / raw)
  To: Hans Verkuil; +Cc: Linux Media Mailing List

Em Fri, 21 Nov 2014 10:20:56 +0100
Hans Verkuil <hverkuil@xs4all.nl> escreveu:

> Several drivers (mostly copy-and-paste) still used sg++ instead of
> sg = sg_next(sg). Fix them since sg++ won't work if contiguous scatter
> entries where combined into one larger entry.
> 
> Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
> Cc: stable@vger.kernel.org      # for v3.7 and up
> ---
>  drivers/media/pci/bt8xx/bttv-risc.c      | 12 ++++++------
>  drivers/media/pci/cx23885/cx23885-core.c |  6 +++---
>  drivers/media/pci/cx25821/cx25821-core.c | 12 ++++++------
>  drivers/media/pci/cx88/cx88-core.c       |  6 +++---
>  drivers/media/pci/ivtv/ivtv-udma.c       |  2 +-
>  5 files changed, 19 insertions(+), 19 deletions(-)
> 
> diff --git a/drivers/media/pci/bt8xx/bttv-risc.c b/drivers/media/pci/bt8xx/bttv-risc.c
> index 82cc47d..4d3f05a 100644
> --- a/drivers/media/pci/bt8xx/bttv-risc.c
> +++ b/drivers/media/pci/bt8xx/bttv-risc.c
> @@ -84,7 +84,7 @@ bttv_risc_packed(struct bttv *btv, struct btcx_riscmem *risc,
>  			continue;
>  		while (offset && offset >= sg_dma_len(sg)) {
>  			offset -= sg_dma_len(sg);
> -			sg++;
> +			sg = sg_next(sg);
>  		}
>  		if (bpl <= sg_dma_len(sg)-offset) {
>  			/* fits into current chunk */
> @@ -100,13 +100,13 @@ bttv_risc_packed(struct bttv *btv, struct btcx_riscmem *risc,
>  			*(rp++)=cpu_to_le32(sg_dma_address(sg)+offset);
>  			todo -= (sg_dma_len(sg)-offset);
>  			offset = 0;
> -			sg++;
> +			sg = sg_next(sg);
>  			while (todo > sg_dma_len(sg)) {
>  				*(rp++)=cpu_to_le32(BT848_RISC_WRITE|
>  						    sg_dma_len(sg));
>  				*(rp++)=cpu_to_le32(sg_dma_address(sg));
>  				todo -= sg_dma_len(sg);
> -				sg++;
> +				sg = sg_next(sg);
>  			}
>  			*(rp++)=cpu_to_le32(BT848_RISC_WRITE|BT848_RISC_EOL|
>  					    todo);
> @@ -187,15 +187,15 @@ bttv_risc_planar(struct bttv *btv, struct btcx_riscmem *risc,
>  			/* go to next sg entry if needed */
>  			while (yoffset && yoffset >= sg_dma_len(ysg)) {
>  				yoffset -= sg_dma_len(ysg);
> -				ysg++;
> +				ysg = sg_next(ysg);
>  			}
>  			while (uoffset && uoffset >= sg_dma_len(usg)) {
>  				uoffset -= sg_dma_len(usg);
> -				usg++;
> +				usg = sg_next(usg);
>  			}
>  			while (voffset && voffset >= sg_dma_len(vsg)) {
>  				voffset -= sg_dma_len(vsg);
> -				vsg++;
> +				vsg = sg_next(vsg);
>  			}
>  
>  			/* calculate max number of bytes we can write */
> diff --git a/drivers/media/pci/cx23885/cx23885-core.c b/drivers/media/pci/cx23885/cx23885-core.c
> index 331edda..3bd386c 100644
> --- a/drivers/media/pci/cx23885/cx23885-core.c
> +++ b/drivers/media/pci/cx23885/cx23885-core.c
> @@ -1078,7 +1078,7 @@ static __le32 *cx23885_risc_field(__le32 *rp, struct scatterlist *sglist,
>  	for (line = 0; line < lines; line++) {
>  		while (offset && offset >= sg_dma_len(sg)) {
>  			offset -= sg_dma_len(sg);
> -			sg++;
> +			sg = sg_next(sg);
>  		}
>  
>  		if (lpi && line > 0 && !(line % lpi))
> @@ -1101,14 +1101,14 @@ static __le32 *cx23885_risc_field(__le32 *rp, struct scatterlist *sglist,
>  			*(rp++) = cpu_to_le32(0); /* bits 63-32 */
>  			todo -= (sg_dma_len(sg)-offset);
>  			offset = 0;
> -			sg++;
> +			sg = sg_next(sg);
>  			while (todo > sg_dma_len(sg)) {
>  				*(rp++) = cpu_to_le32(RISC_WRITE|
>  						    sg_dma_len(sg));
>  				*(rp++) = cpu_to_le32(sg_dma_address(sg));
>  				*(rp++) = cpu_to_le32(0); /* bits 63-32 */
>  				todo -= sg_dma_len(sg);
> -				sg++;
> +				sg = sg_next(sg);
>  			}
>  			*(rp++) = cpu_to_le32(RISC_WRITE|RISC_EOL|todo);
>  			*(rp++) = cpu_to_le32(sg_dma_address(sg));
> diff --git a/drivers/media/pci/cx25821/cx25821-core.c b/drivers/media/pci/cx25821/cx25821-core.c
> index e81173c..389fffd 100644
> --- a/drivers/media/pci/cx25821/cx25821-core.c
> +++ b/drivers/media/pci/cx25821/cx25821-core.c
> @@ -996,7 +996,7 @@ static __le32 *cx25821_risc_field(__le32 * rp, struct scatterlist *sglist,
>  	for (line = 0; line < lines; line++) {
>  		while (offset && offset >= sg_dma_len(sg)) {
>  			offset -= sg_dma_len(sg);
> -			sg++;
> +			sg = sg_next(sg);
>  		}
>  		if (bpl <= sg_dma_len(sg) - offset) {
>  			/* fits into current chunk */
> @@ -1014,14 +1014,14 @@ static __le32 *cx25821_risc_field(__le32 * rp, struct scatterlist *sglist,
>  			*(rp++) = cpu_to_le32(0);	/* bits 63-32 */
>  			todo -= (sg_dma_len(sg) - offset);
>  			offset = 0;
> -			sg++;
> +			sg = sg_next(sg);
>  			while (todo > sg_dma_len(sg)) {
>  				*(rp++) = cpu_to_le32(RISC_WRITE |
>  						sg_dma_len(sg));
>  				*(rp++) = cpu_to_le32(sg_dma_address(sg));
>  				*(rp++) = cpu_to_le32(0);	/* bits 63-32 */
>  				todo -= sg_dma_len(sg);
> -				sg++;
> +				sg = sg_next(sg);
>  			}
>  			*(rp++) = cpu_to_le32(RISC_WRITE | RISC_EOL | todo);
>  			*(rp++) = cpu_to_le32(sg_dma_address(sg));
> @@ -1101,7 +1101,7 @@ static __le32 *cx25821_risc_field_audio(__le32 * rp, struct scatterlist *sglist,
>  	for (line = 0; line < lines; line++) {
>  		while (offset && offset >= sg_dma_len(sg)) {
>  			offset -= sg_dma_len(sg);
> -			sg++;
> +			sg = sg_next(sg);
>  		}
>  
>  		if (lpi && line > 0 && !(line % lpi))
> @@ -1125,14 +1125,14 @@ static __le32 *cx25821_risc_field_audio(__le32 * rp, struct scatterlist *sglist,
>  			*(rp++) = cpu_to_le32(0);	/* bits 63-32 */
>  			todo -= (sg_dma_len(sg) - offset);
>  			offset = 0;
> -			sg++;
> +			sg = sg_next(sg);
>  			while (todo > sg_dma_len(sg)) {
>  				*(rp++) = cpu_to_le32(RISC_WRITE |
>  						sg_dma_len(sg));
>  				*(rp++) = cpu_to_le32(sg_dma_address(sg));
>  				*(rp++) = cpu_to_le32(0);	/* bits 63-32 */
>  				todo -= sg_dma_len(sg);
> -				sg++;
> +				sg = sg_next(sg);
>  			}
>  			*(rp++) = cpu_to_le32(RISC_WRITE | RISC_EOL | todo);
>  			*(rp++) = cpu_to_le32(sg_dma_address(sg));
> diff --git a/drivers/media/pci/cx88/cx88-core.c b/drivers/media/pci/cx88/cx88-core.c
> index 9fa4acb..dee177e 100644
> --- a/drivers/media/pci/cx88/cx88-core.c
> +++ b/drivers/media/pci/cx88/cx88-core.c
> @@ -95,7 +95,7 @@ static __le32* cx88_risc_field(__le32 *rp, struct scatterlist *sglist,
>  	for (line = 0; line < lines; line++) {
>  		while (offset && offset >= sg_dma_len(sg)) {
>  			offset -= sg_dma_len(sg);
> -			sg++;
> +			sg = sg_next(sg);
>  		}
>  		if (lpi && line>0 && !(line % lpi))
>  			sol = RISC_SOL | RISC_IRQ1 | RISC_CNT_INC;
> @@ -114,13 +114,13 @@ static __le32* cx88_risc_field(__le32 *rp, struct scatterlist *sglist,
>  			*(rp++)=cpu_to_le32(sg_dma_address(sg)+offset);
>  			todo -= (sg_dma_len(sg)-offset);
>  			offset = 0;
> -			sg++;
> +			sg = sg_next(sg);
>  			while (todo > sg_dma_len(sg)) {
>  				*(rp++)=cpu_to_le32(RISC_WRITE|
>  						    sg_dma_len(sg));
>  				*(rp++)=cpu_to_le32(sg_dma_address(sg));
>  				todo -= sg_dma_len(sg);
> -				sg++;
> +				sg = sg_next(sg);
>  			}
>  			*(rp++)=cpu_to_le32(RISC_WRITE|RISC_EOL|todo);
>  			*(rp++)=cpu_to_le32(sg_dma_address(sg));
> diff --git a/drivers/media/pci/ivtv/ivtv-udma.c b/drivers/media/pci/ivtv/ivtv-udma.c
> index 7338cb2..bee2329 100644
> --- a/drivers/media/pci/ivtv/ivtv-udma.c
> +++ b/drivers/media/pci/ivtv/ivtv-udma.c
> @@ -76,7 +76,7 @@ void ivtv_udma_fill_sg_array (struct ivtv_user_dma *dma, u32 buffer_offset, u32
>  	int i;
>  	struct scatterlist *sg;
>  
> -	for (i = 0, sg = dma->SGlist; i < dma->SG_length; i++, sg++) {
> +	for (i = 0, sg = dma->SGlist; i < dma->SG_length; i++, sg = sg_next(sg)) {

This hunk seems awkward, at least on my eyes. 

As you've pointed at the description, S/G can be grouped into bigger
segments. So, the maximum number of S/G can actually be less than
dma->SGlist.

The proper fix here seems to convert it into a while that will stop
if the accumulated size matches the transfer size, just like the loops
at the other similar drivers.

>  		dma->SGarray[i].size = cpu_to_le32(sg_dma_len(sg));
>  		dma->SGarray[i].src = cpu_to_le32(sg_dma_address(sg));
>  		dma->SGarray[i].dst = cpu_to_le32(buffer_offset);

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH for v3.18] media: use sg = sg_next(sg) instead of sg++
  2014-11-24 18:24 ` Mauro Carvalho Chehab
@ 2014-11-24 18:37   ` Hans Verkuil
  2014-11-24 18:51     ` Mauro Carvalho Chehab
  0 siblings, 1 reply; 7+ messages in thread
From: Hans Verkuil @ 2014-11-24 18:37 UTC (permalink / raw)
  To: Mauro Carvalho Chehab; +Cc: Linux Media Mailing List

On 11/24/2014 07:24 PM, Mauro Carvalho Chehab wrote:
> Em Fri, 21 Nov 2014 10:20:56 +0100
> Hans Verkuil <hverkuil@xs4all.nl> escreveu:
> 
>> Several drivers (mostly copy-and-paste) still used sg++ instead of
>> sg = sg_next(sg). Fix them since sg++ won't work if contiguous scatter
>> entries where combined into one larger entry.
>>
>> Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
>> Cc: stable@vger.kernel.org      # for v3.7 and up
>> ---
>>  drivers/media/pci/bt8xx/bttv-risc.c      | 12 ++++++------
>>  drivers/media/pci/cx23885/cx23885-core.c |  6 +++---
>>  drivers/media/pci/cx25821/cx25821-core.c | 12 ++++++------
>>  drivers/media/pci/cx88/cx88-core.c       |  6 +++---
>>  drivers/media/pci/ivtv/ivtv-udma.c       |  2 +-
>>  5 files changed, 19 insertions(+), 19 deletions(-)
>>
>> diff --git a/drivers/media/pci/bt8xx/bttv-risc.c b/drivers/media/pci/bt8xx/bttv-risc.c
>> index 82cc47d..4d3f05a 100644
>> --- a/drivers/media/pci/bt8xx/bttv-risc.c
>> +++ b/drivers/media/pci/bt8xx/bttv-risc.c
>> @@ -84,7 +84,7 @@ bttv_risc_packed(struct bttv *btv, struct btcx_riscmem *risc,
>>  			continue;
>>  		while (offset && offset >= sg_dma_len(sg)) {
>>  			offset -= sg_dma_len(sg);
>> -			sg++;
>> +			sg = sg_next(sg);
>>  		}
>>  		if (bpl <= sg_dma_len(sg)-offset) {
>>  			/* fits into current chunk */
>> @@ -100,13 +100,13 @@ bttv_risc_packed(struct bttv *btv, struct btcx_riscmem *risc,
>>  			*(rp++)=cpu_to_le32(sg_dma_address(sg)+offset);
>>  			todo -= (sg_dma_len(sg)-offset);
>>  			offset = 0;
>> -			sg++;
>> +			sg = sg_next(sg);
>>  			while (todo > sg_dma_len(sg)) {
>>  				*(rp++)=cpu_to_le32(BT848_RISC_WRITE|
>>  						    sg_dma_len(sg));
>>  				*(rp++)=cpu_to_le32(sg_dma_address(sg));
>>  				todo -= sg_dma_len(sg);
>> -				sg++;
>> +				sg = sg_next(sg);
>>  			}
>>  			*(rp++)=cpu_to_le32(BT848_RISC_WRITE|BT848_RISC_EOL|
>>  					    todo);
>> @@ -187,15 +187,15 @@ bttv_risc_planar(struct bttv *btv, struct btcx_riscmem *risc,
>>  			/* go to next sg entry if needed */
>>  			while (yoffset && yoffset >= sg_dma_len(ysg)) {
>>  				yoffset -= sg_dma_len(ysg);
>> -				ysg++;
>> +				ysg = sg_next(ysg);
>>  			}
>>  			while (uoffset && uoffset >= sg_dma_len(usg)) {
>>  				uoffset -= sg_dma_len(usg);
>> -				usg++;
>> +				usg = sg_next(usg);
>>  			}
>>  			while (voffset && voffset >= sg_dma_len(vsg)) {
>>  				voffset -= sg_dma_len(vsg);
>> -				vsg++;
>> +				vsg = sg_next(vsg);
>>  			}
>>  
>>  			/* calculate max number of bytes we can write */
>> diff --git a/drivers/media/pci/cx23885/cx23885-core.c b/drivers/media/pci/cx23885/cx23885-core.c
>> index 331edda..3bd386c 100644
>> --- a/drivers/media/pci/cx23885/cx23885-core.c
>> +++ b/drivers/media/pci/cx23885/cx23885-core.c
>> @@ -1078,7 +1078,7 @@ static __le32 *cx23885_risc_field(__le32 *rp, struct scatterlist *sglist,
>>  	for (line = 0; line < lines; line++) {
>>  		while (offset && offset >= sg_dma_len(sg)) {
>>  			offset -= sg_dma_len(sg);
>> -			sg++;
>> +			sg = sg_next(sg);
>>  		}
>>  
>>  		if (lpi && line > 0 && !(line % lpi))
>> @@ -1101,14 +1101,14 @@ static __le32 *cx23885_risc_field(__le32 *rp, struct scatterlist *sglist,
>>  			*(rp++) = cpu_to_le32(0); /* bits 63-32 */
>>  			todo -= (sg_dma_len(sg)-offset);
>>  			offset = 0;
>> -			sg++;
>> +			sg = sg_next(sg);
>>  			while (todo > sg_dma_len(sg)) {
>>  				*(rp++) = cpu_to_le32(RISC_WRITE|
>>  						    sg_dma_len(sg));
>>  				*(rp++) = cpu_to_le32(sg_dma_address(sg));
>>  				*(rp++) = cpu_to_le32(0); /* bits 63-32 */
>>  				todo -= sg_dma_len(sg);
>> -				sg++;
>> +				sg = sg_next(sg);
>>  			}
>>  			*(rp++) = cpu_to_le32(RISC_WRITE|RISC_EOL|todo);
>>  			*(rp++) = cpu_to_le32(sg_dma_address(sg));
>> diff --git a/drivers/media/pci/cx25821/cx25821-core.c b/drivers/media/pci/cx25821/cx25821-core.c
>> index e81173c..389fffd 100644
>> --- a/drivers/media/pci/cx25821/cx25821-core.c
>> +++ b/drivers/media/pci/cx25821/cx25821-core.c
>> @@ -996,7 +996,7 @@ static __le32 *cx25821_risc_field(__le32 * rp, struct scatterlist *sglist,
>>  	for (line = 0; line < lines; line++) {
>>  		while (offset && offset >= sg_dma_len(sg)) {
>>  			offset -= sg_dma_len(sg);
>> -			sg++;
>> +			sg = sg_next(sg);
>>  		}
>>  		if (bpl <= sg_dma_len(sg) - offset) {
>>  			/* fits into current chunk */
>> @@ -1014,14 +1014,14 @@ static __le32 *cx25821_risc_field(__le32 * rp, struct scatterlist *sglist,
>>  			*(rp++) = cpu_to_le32(0);	/* bits 63-32 */
>>  			todo -= (sg_dma_len(sg) - offset);
>>  			offset = 0;
>> -			sg++;
>> +			sg = sg_next(sg);
>>  			while (todo > sg_dma_len(sg)) {
>>  				*(rp++) = cpu_to_le32(RISC_WRITE |
>>  						sg_dma_len(sg));
>>  				*(rp++) = cpu_to_le32(sg_dma_address(sg));
>>  				*(rp++) = cpu_to_le32(0);	/* bits 63-32 */
>>  				todo -= sg_dma_len(sg);
>> -				sg++;
>> +				sg = sg_next(sg);
>>  			}
>>  			*(rp++) = cpu_to_le32(RISC_WRITE | RISC_EOL | todo);
>>  			*(rp++) = cpu_to_le32(sg_dma_address(sg));
>> @@ -1101,7 +1101,7 @@ static __le32 *cx25821_risc_field_audio(__le32 * rp, struct scatterlist *sglist,
>>  	for (line = 0; line < lines; line++) {
>>  		while (offset && offset >= sg_dma_len(sg)) {
>>  			offset -= sg_dma_len(sg);
>> -			sg++;
>> +			sg = sg_next(sg);
>>  		}
>>  
>>  		if (lpi && line > 0 && !(line % lpi))
>> @@ -1125,14 +1125,14 @@ static __le32 *cx25821_risc_field_audio(__le32 * rp, struct scatterlist *sglist,
>>  			*(rp++) = cpu_to_le32(0);	/* bits 63-32 */
>>  			todo -= (sg_dma_len(sg) - offset);
>>  			offset = 0;
>> -			sg++;
>> +			sg = sg_next(sg);
>>  			while (todo > sg_dma_len(sg)) {
>>  				*(rp++) = cpu_to_le32(RISC_WRITE |
>>  						sg_dma_len(sg));
>>  				*(rp++) = cpu_to_le32(sg_dma_address(sg));
>>  				*(rp++) = cpu_to_le32(0);	/* bits 63-32 */
>>  				todo -= sg_dma_len(sg);
>> -				sg++;
>> +				sg = sg_next(sg);
>>  			}
>>  			*(rp++) = cpu_to_le32(RISC_WRITE | RISC_EOL | todo);
>>  			*(rp++) = cpu_to_le32(sg_dma_address(sg));
>> diff --git a/drivers/media/pci/cx88/cx88-core.c b/drivers/media/pci/cx88/cx88-core.c
>> index 9fa4acb..dee177e 100644
>> --- a/drivers/media/pci/cx88/cx88-core.c
>> +++ b/drivers/media/pci/cx88/cx88-core.c
>> @@ -95,7 +95,7 @@ static __le32* cx88_risc_field(__le32 *rp, struct scatterlist *sglist,
>>  	for (line = 0; line < lines; line++) {
>>  		while (offset && offset >= sg_dma_len(sg)) {
>>  			offset -= sg_dma_len(sg);
>> -			sg++;
>> +			sg = sg_next(sg);
>>  		}
>>  		if (lpi && line>0 && !(line % lpi))
>>  			sol = RISC_SOL | RISC_IRQ1 | RISC_CNT_INC;
>> @@ -114,13 +114,13 @@ static __le32* cx88_risc_field(__le32 *rp, struct scatterlist *sglist,
>>  			*(rp++)=cpu_to_le32(sg_dma_address(sg)+offset);
>>  			todo -= (sg_dma_len(sg)-offset);
>>  			offset = 0;
>> -			sg++;
>> +			sg = sg_next(sg);
>>  			while (todo > sg_dma_len(sg)) {
>>  				*(rp++)=cpu_to_le32(RISC_WRITE|
>>  						    sg_dma_len(sg));
>>  				*(rp++)=cpu_to_le32(sg_dma_address(sg));
>>  				todo -= sg_dma_len(sg);
>> -				sg++;
>> +				sg = sg_next(sg);
>>  			}
>>  			*(rp++)=cpu_to_le32(RISC_WRITE|RISC_EOL|todo);
>>  			*(rp++)=cpu_to_le32(sg_dma_address(sg));
>> diff --git a/drivers/media/pci/ivtv/ivtv-udma.c b/drivers/media/pci/ivtv/ivtv-udma.c
>> index 7338cb2..bee2329 100644
>> --- a/drivers/media/pci/ivtv/ivtv-udma.c
>> +++ b/drivers/media/pci/ivtv/ivtv-udma.c
>> @@ -76,7 +76,7 @@ void ivtv_udma_fill_sg_array (struct ivtv_user_dma *dma, u32 buffer_offset, u32
>>  	int i;
>>  	struct scatterlist *sg;
>>  
>> -	for (i = 0, sg = dma->SGlist; i < dma->SG_length; i++, sg++) {
>> +	for (i = 0, sg = dma->SGlist; i < dma->SG_length; i++, sg = sg_next(sg)) {
> 
> This hunk seems awkward, at least on my eyes. 
> 
> As you've pointed at the description, S/G can be grouped into bigger
> segments. So, the maximum number of S/G can actually be less than
> dma->SGlist.

That's correct, and dma->SG_length is equal to that number. SG_length is assigned
with the result of dma_map_sg() which returns "the number of bus address segments
mapped (this may be shorter than <nents> passed in if some elements of the
scatter/gather list are physically or virtually adjacent and an IOMMU maps them
with a single entry)."

So the patch is correct.

Regards,

	Hans

> 
> The proper fix here seems to convert it into a while that will stop
> if the accumulated size matches the transfer size, just like the loops
> at the other similar drivers.
> 
>>  		dma->SGarray[i].size = cpu_to_le32(sg_dma_len(sg));
>>  		dma->SGarray[i].src = cpu_to_le32(sg_dma_address(sg));
>>  		dma->SGarray[i].dst = cpu_to_le32(buffer_offset);


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH for v3.18] media: use sg = sg_next(sg) instead of sg++
  2014-11-24 18:37   ` Hans Verkuil
@ 2014-11-24 18:51     ` Mauro Carvalho Chehab
  2014-11-24 19:16       ` Hans Verkuil
  0 siblings, 1 reply; 7+ messages in thread
From: Mauro Carvalho Chehab @ 2014-11-24 18:51 UTC (permalink / raw)
  To: Hans Verkuil; +Cc: Linux Media Mailing List

Em Mon, 24 Nov 2014 19:37:48 +0100
Hans Verkuil <hverkuil@xs4all.nl> escreveu:

> On 11/24/2014 07:24 PM, Mauro Carvalho Chehab wrote:
> > Em Fri, 21 Nov 2014 10:20:56 +0100
> > Hans Verkuil <hverkuil@xs4all.nl> escreveu:
> > 
> >> Several drivers (mostly copy-and-paste) still used sg++ instead of
> >> sg = sg_next(sg). Fix them since sg++ won't work if contiguous scatter
> >> entries where combined into one larger entry.
> >>
> >> Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
> >> Cc: stable@vger.kernel.org      # for v3.7 and up
> >> ---
> >>  drivers/media/pci/bt8xx/bttv-risc.c      | 12 ++++++------
> >>  drivers/media/pci/cx23885/cx23885-core.c |  6 +++---
> >>  drivers/media/pci/cx25821/cx25821-core.c | 12 ++++++------
> >>  drivers/media/pci/cx88/cx88-core.c       |  6 +++---
> >>  drivers/media/pci/ivtv/ivtv-udma.c       |  2 +-
> >>  5 files changed, 19 insertions(+), 19 deletions(-)
> >>
> >> diff --git a/drivers/media/pci/bt8xx/bttv-risc.c b/drivers/media/pci/bt8xx/bttv-risc.c
> >> index 82cc47d..4d3f05a 100644
> >> --- a/drivers/media/pci/bt8xx/bttv-risc.c
> >> +++ b/drivers/media/pci/bt8xx/bttv-risc.c
> >> @@ -84,7 +84,7 @@ bttv_risc_packed(struct bttv *btv, struct btcx_riscmem *risc,
> >>  			continue;
> >>  		while (offset && offset >= sg_dma_len(sg)) {
> >>  			offset -= sg_dma_len(sg);
> >> -			sg++;
> >> +			sg = sg_next(sg);
> >>  		}
> >>  		if (bpl <= sg_dma_len(sg)-offset) {
> >>  			/* fits into current chunk */
> >> @@ -100,13 +100,13 @@ bttv_risc_packed(struct bttv *btv, struct btcx_riscmem *risc,
> >>  			*(rp++)=cpu_to_le32(sg_dma_address(sg)+offset);
> >>  			todo -= (sg_dma_len(sg)-offset);
> >>  			offset = 0;
> >> -			sg++;
> >> +			sg = sg_next(sg);
> >>  			while (todo > sg_dma_len(sg)) {
> >>  				*(rp++)=cpu_to_le32(BT848_RISC_WRITE|
> >>  						    sg_dma_len(sg));
> >>  				*(rp++)=cpu_to_le32(sg_dma_address(sg));
> >>  				todo -= sg_dma_len(sg);
> >> -				sg++;
> >> +				sg = sg_next(sg);
> >>  			}
> >>  			*(rp++)=cpu_to_le32(BT848_RISC_WRITE|BT848_RISC_EOL|
> >>  					    todo);
> >> @@ -187,15 +187,15 @@ bttv_risc_planar(struct bttv *btv, struct btcx_riscmem *risc,
> >>  			/* go to next sg entry if needed */
> >>  			while (yoffset && yoffset >= sg_dma_len(ysg)) {
> >>  				yoffset -= sg_dma_len(ysg);
> >> -				ysg++;
> >> +				ysg = sg_next(ysg);
> >>  			}
> >>  			while (uoffset && uoffset >= sg_dma_len(usg)) {
> >>  				uoffset -= sg_dma_len(usg);
> >> -				usg++;
> >> +				usg = sg_next(usg);
> >>  			}
> >>  			while (voffset && voffset >= sg_dma_len(vsg)) {
> >>  				voffset -= sg_dma_len(vsg);
> >> -				vsg++;
> >> +				vsg = sg_next(vsg);
> >>  			}
> >>  
> >>  			/* calculate max number of bytes we can write */
> >> diff --git a/drivers/media/pci/cx23885/cx23885-core.c b/drivers/media/pci/cx23885/cx23885-core.c
> >> index 331edda..3bd386c 100644
> >> --- a/drivers/media/pci/cx23885/cx23885-core.c
> >> +++ b/drivers/media/pci/cx23885/cx23885-core.c
> >> @@ -1078,7 +1078,7 @@ static __le32 *cx23885_risc_field(__le32 *rp, struct scatterlist *sglist,
> >>  	for (line = 0; line < lines; line++) {
> >>  		while (offset && offset >= sg_dma_len(sg)) {
> >>  			offset -= sg_dma_len(sg);
> >> -			sg++;
> >> +			sg = sg_next(sg);
> >>  		}
> >>  
> >>  		if (lpi && line > 0 && !(line % lpi))
> >> @@ -1101,14 +1101,14 @@ static __le32 *cx23885_risc_field(__le32 *rp, struct scatterlist *sglist,
> >>  			*(rp++) = cpu_to_le32(0); /* bits 63-32 */
> >>  			todo -= (sg_dma_len(sg)-offset);
> >>  			offset = 0;
> >> -			sg++;
> >> +			sg = sg_next(sg);
> >>  			while (todo > sg_dma_len(sg)) {
> >>  				*(rp++) = cpu_to_le32(RISC_WRITE|
> >>  						    sg_dma_len(sg));
> >>  				*(rp++) = cpu_to_le32(sg_dma_address(sg));
> >>  				*(rp++) = cpu_to_le32(0); /* bits 63-32 */
> >>  				todo -= sg_dma_len(sg);
> >> -				sg++;
> >> +				sg = sg_next(sg);
> >>  			}
> >>  			*(rp++) = cpu_to_le32(RISC_WRITE|RISC_EOL|todo);
> >>  			*(rp++) = cpu_to_le32(sg_dma_address(sg));
> >> diff --git a/drivers/media/pci/cx25821/cx25821-core.c b/drivers/media/pci/cx25821/cx25821-core.c
> >> index e81173c..389fffd 100644
> >> --- a/drivers/media/pci/cx25821/cx25821-core.c
> >> +++ b/drivers/media/pci/cx25821/cx25821-core.c
> >> @@ -996,7 +996,7 @@ static __le32 *cx25821_risc_field(__le32 * rp, struct scatterlist *sglist,
> >>  	for (line = 0; line < lines; line++) {
> >>  		while (offset && offset >= sg_dma_len(sg)) {
> >>  			offset -= sg_dma_len(sg);
> >> -			sg++;
> >> +			sg = sg_next(sg);
> >>  		}
> >>  		if (bpl <= sg_dma_len(sg) - offset) {
> >>  			/* fits into current chunk */
> >> @@ -1014,14 +1014,14 @@ static __le32 *cx25821_risc_field(__le32 * rp, struct scatterlist *sglist,
> >>  			*(rp++) = cpu_to_le32(0);	/* bits 63-32 */
> >>  			todo -= (sg_dma_len(sg) - offset);
> >>  			offset = 0;
> >> -			sg++;
> >> +			sg = sg_next(sg);
> >>  			while (todo > sg_dma_len(sg)) {
> >>  				*(rp++) = cpu_to_le32(RISC_WRITE |
> >>  						sg_dma_len(sg));
> >>  				*(rp++) = cpu_to_le32(sg_dma_address(sg));
> >>  				*(rp++) = cpu_to_le32(0);	/* bits 63-32 */
> >>  				todo -= sg_dma_len(sg);
> >> -				sg++;
> >> +				sg = sg_next(sg);
> >>  			}
> >>  			*(rp++) = cpu_to_le32(RISC_WRITE | RISC_EOL | todo);
> >>  			*(rp++) = cpu_to_le32(sg_dma_address(sg));
> >> @@ -1101,7 +1101,7 @@ static __le32 *cx25821_risc_field_audio(__le32 * rp, struct scatterlist *sglist,
> >>  	for (line = 0; line < lines; line++) {
> >>  		while (offset && offset >= sg_dma_len(sg)) {
> >>  			offset -= sg_dma_len(sg);
> >> -			sg++;
> >> +			sg = sg_next(sg);
> >>  		}
> >>  
> >>  		if (lpi && line > 0 && !(line % lpi))
> >> @@ -1125,14 +1125,14 @@ static __le32 *cx25821_risc_field_audio(__le32 * rp, struct scatterlist *sglist,
> >>  			*(rp++) = cpu_to_le32(0);	/* bits 63-32 */
> >>  			todo -= (sg_dma_len(sg) - offset);
> >>  			offset = 0;
> >> -			sg++;
> >> +			sg = sg_next(sg);
> >>  			while (todo > sg_dma_len(sg)) {
> >>  				*(rp++) = cpu_to_le32(RISC_WRITE |
> >>  						sg_dma_len(sg));
> >>  				*(rp++) = cpu_to_le32(sg_dma_address(sg));
> >>  				*(rp++) = cpu_to_le32(0);	/* bits 63-32 */
> >>  				todo -= sg_dma_len(sg);
> >> -				sg++;
> >> +				sg = sg_next(sg);
> >>  			}
> >>  			*(rp++) = cpu_to_le32(RISC_WRITE | RISC_EOL | todo);
> >>  			*(rp++) = cpu_to_le32(sg_dma_address(sg));
> >> diff --git a/drivers/media/pci/cx88/cx88-core.c b/drivers/media/pci/cx88/cx88-core.c
> >> index 9fa4acb..dee177e 100644
> >> --- a/drivers/media/pci/cx88/cx88-core.c
> >> +++ b/drivers/media/pci/cx88/cx88-core.c
> >> @@ -95,7 +95,7 @@ static __le32* cx88_risc_field(__le32 *rp, struct scatterlist *sglist,
> >>  	for (line = 0; line < lines; line++) {
> >>  		while (offset && offset >= sg_dma_len(sg)) {
> >>  			offset -= sg_dma_len(sg);
> >> -			sg++;
> >> +			sg = sg_next(sg);
> >>  		}
> >>  		if (lpi && line>0 && !(line % lpi))
> >>  			sol = RISC_SOL | RISC_IRQ1 | RISC_CNT_INC;
> >> @@ -114,13 +114,13 @@ static __le32* cx88_risc_field(__le32 *rp, struct scatterlist *sglist,
> >>  			*(rp++)=cpu_to_le32(sg_dma_address(sg)+offset);
> >>  			todo -= (sg_dma_len(sg)-offset);
> >>  			offset = 0;
> >> -			sg++;
> >> +			sg = sg_next(sg);
> >>  			while (todo > sg_dma_len(sg)) {
> >>  				*(rp++)=cpu_to_le32(RISC_WRITE|
> >>  						    sg_dma_len(sg));
> >>  				*(rp++)=cpu_to_le32(sg_dma_address(sg));
> >>  				todo -= sg_dma_len(sg);
> >> -				sg++;
> >> +				sg = sg_next(sg);
> >>  			}
> >>  			*(rp++)=cpu_to_le32(RISC_WRITE|RISC_EOL|todo);
> >>  			*(rp++)=cpu_to_le32(sg_dma_address(sg));
> >> diff --git a/drivers/media/pci/ivtv/ivtv-udma.c b/drivers/media/pci/ivtv/ivtv-udma.c
> >> index 7338cb2..bee2329 100644
> >> --- a/drivers/media/pci/ivtv/ivtv-udma.c
> >> +++ b/drivers/media/pci/ivtv/ivtv-udma.c
> >> @@ -76,7 +76,7 @@ void ivtv_udma_fill_sg_array (struct ivtv_user_dma *dma, u32 buffer_offset, u32
> >>  	int i;
> >>  	struct scatterlist *sg;
> >>  
> >> -	for (i = 0, sg = dma->SGlist; i < dma->SG_length; i++, sg++) {
> >> +	for (i = 0, sg = dma->SGlist; i < dma->SG_length; i++, sg = sg_next(sg)) {
> > 
> > This hunk seems awkward, at least on my eyes. 
> > 
> > As you've pointed at the description, S/G can be grouped into bigger
> > segments. So, the maximum number of S/G can actually be less than
> > dma->SGlist.
> 
> That's correct, and dma->SG_length is equal to that number. SG_length is assigned
> with the result of dma_map_sg() which returns "the number of bus address segments
> mapped (this may be shorter than <nents> passed in if some elements of the
> scatter/gather list are physically or virtually adjacent and an IOMMU maps them
> with a single entry)."
> 
> So the patch is correct.

OK.

Btw, you added this patch at a pull request for Kernel 3.19. Shouldn't it
be, instead, be sent to 3.18 and c/c stable?
> 
> Regards,
> 
> 	Hans
> 
> > 
> > The proper fix here seems to convert it into a while that will stop
> > if the accumulated size matches the transfer size, just like the loops
> > at the other similar drivers.
> > 
> >>  		dma->SGarray[i].size = cpu_to_le32(sg_dma_len(sg));
> >>  		dma->SGarray[i].src = cpu_to_le32(sg_dma_address(sg));
> >>  		dma->SGarray[i].dst = cpu_to_le32(buffer_offset);
> 

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH for v3.18] media: use sg = sg_next(sg) instead of sg++
  2014-11-24 18:51     ` Mauro Carvalho Chehab
@ 2014-11-24 19:16       ` Hans Verkuil
  0 siblings, 0 replies; 7+ messages in thread
From: Hans Verkuil @ 2014-11-24 19:16 UTC (permalink / raw)
  To: Mauro Carvalho Chehab; +Cc: Linux Media Mailing List

On 11/24/2014 07:51 PM, Mauro Carvalho Chehab wrote:
> Em Mon, 24 Nov 2014 19:37:48 +0100
> Hans Verkuil <hverkuil@xs4all.nl> escreveu:
> 
>> On 11/24/2014 07:24 PM, Mauro Carvalho Chehab wrote:
>>> Em Fri, 21 Nov 2014 10:20:56 +0100
>>> Hans Verkuil <hverkuil@xs4all.nl> escreveu:
>>>
>>>> Several drivers (mostly copy-and-paste) still used sg++ instead of
>>>> sg = sg_next(sg). Fix them since sg++ won't work if contiguous scatter
>>>> entries where combined into one larger entry.
>>>>
>>>> Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
>>>> Cc: stable@vger.kernel.org      # for v3.7 and up
>>>> ---
>>>>  drivers/media/pci/bt8xx/bttv-risc.c      | 12 ++++++------
>>>>  drivers/media/pci/cx23885/cx23885-core.c |  6 +++---
>>>>  drivers/media/pci/cx25821/cx25821-core.c | 12 ++++++------
>>>>  drivers/media/pci/cx88/cx88-core.c       |  6 +++---
>>>>  drivers/media/pci/ivtv/ivtv-udma.c       |  2 +-
>>>>  5 files changed, 19 insertions(+), 19 deletions(-)
>>>>
>>>> diff --git a/drivers/media/pci/bt8xx/bttv-risc.c b/drivers/media/pci/bt8xx/bttv-risc.c
>>>> index 82cc47d..4d3f05a 100644
>>>> --- a/drivers/media/pci/bt8xx/bttv-risc.c
>>>> +++ b/drivers/media/pci/bt8xx/bttv-risc.c
>>>> @@ -84,7 +84,7 @@ bttv_risc_packed(struct bttv *btv, struct btcx_riscmem *risc,
>>>>  			continue;
>>>>  		while (offset && offset >= sg_dma_len(sg)) {
>>>>  			offset -= sg_dma_len(sg);
>>>> -			sg++;
>>>> +			sg = sg_next(sg);
>>>>  		}
>>>>  		if (bpl <= sg_dma_len(sg)-offset) {
>>>>  			/* fits into current chunk */
>>>> @@ -100,13 +100,13 @@ bttv_risc_packed(struct bttv *btv, struct btcx_riscmem *risc,
>>>>  			*(rp++)=cpu_to_le32(sg_dma_address(sg)+offset);
>>>>  			todo -= (sg_dma_len(sg)-offset);
>>>>  			offset = 0;
>>>> -			sg++;
>>>> +			sg = sg_next(sg);
>>>>  			while (todo > sg_dma_len(sg)) {
>>>>  				*(rp++)=cpu_to_le32(BT848_RISC_WRITE|
>>>>  						    sg_dma_len(sg));
>>>>  				*(rp++)=cpu_to_le32(sg_dma_address(sg));
>>>>  				todo -= sg_dma_len(sg);
>>>> -				sg++;
>>>> +				sg = sg_next(sg);
>>>>  			}
>>>>  			*(rp++)=cpu_to_le32(BT848_RISC_WRITE|BT848_RISC_EOL|
>>>>  					    todo);
>>>> @@ -187,15 +187,15 @@ bttv_risc_planar(struct bttv *btv, struct btcx_riscmem *risc,
>>>>  			/* go to next sg entry if needed */
>>>>  			while (yoffset && yoffset >= sg_dma_len(ysg)) {
>>>>  				yoffset -= sg_dma_len(ysg);
>>>> -				ysg++;
>>>> +				ysg = sg_next(ysg);
>>>>  			}
>>>>  			while (uoffset && uoffset >= sg_dma_len(usg)) {
>>>>  				uoffset -= sg_dma_len(usg);
>>>> -				usg++;
>>>> +				usg = sg_next(usg);
>>>>  			}
>>>>  			while (voffset && voffset >= sg_dma_len(vsg)) {
>>>>  				voffset -= sg_dma_len(vsg);
>>>> -				vsg++;
>>>> +				vsg = sg_next(vsg);
>>>>  			}
>>>>  
>>>>  			/* calculate max number of bytes we can write */
>>>> diff --git a/drivers/media/pci/cx23885/cx23885-core.c b/drivers/media/pci/cx23885/cx23885-core.c
>>>> index 331edda..3bd386c 100644
>>>> --- a/drivers/media/pci/cx23885/cx23885-core.c
>>>> +++ b/drivers/media/pci/cx23885/cx23885-core.c
>>>> @@ -1078,7 +1078,7 @@ static __le32 *cx23885_risc_field(__le32 *rp, struct scatterlist *sglist,
>>>>  	for (line = 0; line < lines; line++) {
>>>>  		while (offset && offset >= sg_dma_len(sg)) {
>>>>  			offset -= sg_dma_len(sg);
>>>> -			sg++;
>>>> +			sg = sg_next(sg);
>>>>  		}
>>>>  
>>>>  		if (lpi && line > 0 && !(line % lpi))
>>>> @@ -1101,14 +1101,14 @@ static __le32 *cx23885_risc_field(__le32 *rp, struct scatterlist *sglist,
>>>>  			*(rp++) = cpu_to_le32(0); /* bits 63-32 */
>>>>  			todo -= (sg_dma_len(sg)-offset);
>>>>  			offset = 0;
>>>> -			sg++;
>>>> +			sg = sg_next(sg);
>>>>  			while (todo > sg_dma_len(sg)) {
>>>>  				*(rp++) = cpu_to_le32(RISC_WRITE|
>>>>  						    sg_dma_len(sg));
>>>>  				*(rp++) = cpu_to_le32(sg_dma_address(sg));
>>>>  				*(rp++) = cpu_to_le32(0); /* bits 63-32 */
>>>>  				todo -= sg_dma_len(sg);
>>>> -				sg++;
>>>> +				sg = sg_next(sg);
>>>>  			}
>>>>  			*(rp++) = cpu_to_le32(RISC_WRITE|RISC_EOL|todo);
>>>>  			*(rp++) = cpu_to_le32(sg_dma_address(sg));
>>>> diff --git a/drivers/media/pci/cx25821/cx25821-core.c b/drivers/media/pci/cx25821/cx25821-core.c
>>>> index e81173c..389fffd 100644
>>>> --- a/drivers/media/pci/cx25821/cx25821-core.c
>>>> +++ b/drivers/media/pci/cx25821/cx25821-core.c
>>>> @@ -996,7 +996,7 @@ static __le32 *cx25821_risc_field(__le32 * rp, struct scatterlist *sglist,
>>>>  	for (line = 0; line < lines; line++) {
>>>>  		while (offset && offset >= sg_dma_len(sg)) {
>>>>  			offset -= sg_dma_len(sg);
>>>> -			sg++;
>>>> +			sg = sg_next(sg);
>>>>  		}
>>>>  		if (bpl <= sg_dma_len(sg) - offset) {
>>>>  			/* fits into current chunk */
>>>> @@ -1014,14 +1014,14 @@ static __le32 *cx25821_risc_field(__le32 * rp, struct scatterlist *sglist,
>>>>  			*(rp++) = cpu_to_le32(0);	/* bits 63-32 */
>>>>  			todo -= (sg_dma_len(sg) - offset);
>>>>  			offset = 0;
>>>> -			sg++;
>>>> +			sg = sg_next(sg);
>>>>  			while (todo > sg_dma_len(sg)) {
>>>>  				*(rp++) = cpu_to_le32(RISC_WRITE |
>>>>  						sg_dma_len(sg));
>>>>  				*(rp++) = cpu_to_le32(sg_dma_address(sg));
>>>>  				*(rp++) = cpu_to_le32(0);	/* bits 63-32 */
>>>>  				todo -= sg_dma_len(sg);
>>>> -				sg++;
>>>> +				sg = sg_next(sg);
>>>>  			}
>>>>  			*(rp++) = cpu_to_le32(RISC_WRITE | RISC_EOL | todo);
>>>>  			*(rp++) = cpu_to_le32(sg_dma_address(sg));
>>>> @@ -1101,7 +1101,7 @@ static __le32 *cx25821_risc_field_audio(__le32 * rp, struct scatterlist *sglist,
>>>>  	for (line = 0; line < lines; line++) {
>>>>  		while (offset && offset >= sg_dma_len(sg)) {
>>>>  			offset -= sg_dma_len(sg);
>>>> -			sg++;
>>>> +			sg = sg_next(sg);
>>>>  		}
>>>>  
>>>>  		if (lpi && line > 0 && !(line % lpi))
>>>> @@ -1125,14 +1125,14 @@ static __le32 *cx25821_risc_field_audio(__le32 * rp, struct scatterlist *sglist,
>>>>  			*(rp++) = cpu_to_le32(0);	/* bits 63-32 */
>>>>  			todo -= (sg_dma_len(sg) - offset);
>>>>  			offset = 0;
>>>> -			sg++;
>>>> +			sg = sg_next(sg);
>>>>  			while (todo > sg_dma_len(sg)) {
>>>>  				*(rp++) = cpu_to_le32(RISC_WRITE |
>>>>  						sg_dma_len(sg));
>>>>  				*(rp++) = cpu_to_le32(sg_dma_address(sg));
>>>>  				*(rp++) = cpu_to_le32(0);	/* bits 63-32 */
>>>>  				todo -= sg_dma_len(sg);
>>>> -				sg++;
>>>> +				sg = sg_next(sg);
>>>>  			}
>>>>  			*(rp++) = cpu_to_le32(RISC_WRITE | RISC_EOL | todo);
>>>>  			*(rp++) = cpu_to_le32(sg_dma_address(sg));
>>>> diff --git a/drivers/media/pci/cx88/cx88-core.c b/drivers/media/pci/cx88/cx88-core.c
>>>> index 9fa4acb..dee177e 100644
>>>> --- a/drivers/media/pci/cx88/cx88-core.c
>>>> +++ b/drivers/media/pci/cx88/cx88-core.c
>>>> @@ -95,7 +95,7 @@ static __le32* cx88_risc_field(__le32 *rp, struct scatterlist *sglist,
>>>>  	for (line = 0; line < lines; line++) {
>>>>  		while (offset && offset >= sg_dma_len(sg)) {
>>>>  			offset -= sg_dma_len(sg);
>>>> -			sg++;
>>>> +			sg = sg_next(sg);
>>>>  		}
>>>>  		if (lpi && line>0 && !(line % lpi))
>>>>  			sol = RISC_SOL | RISC_IRQ1 | RISC_CNT_INC;
>>>> @@ -114,13 +114,13 @@ static __le32* cx88_risc_field(__le32 *rp, struct scatterlist *sglist,
>>>>  			*(rp++)=cpu_to_le32(sg_dma_address(sg)+offset);
>>>>  			todo -= (sg_dma_len(sg)-offset);
>>>>  			offset = 0;
>>>> -			sg++;
>>>> +			sg = sg_next(sg);
>>>>  			while (todo > sg_dma_len(sg)) {
>>>>  				*(rp++)=cpu_to_le32(RISC_WRITE|
>>>>  						    sg_dma_len(sg));
>>>>  				*(rp++)=cpu_to_le32(sg_dma_address(sg));
>>>>  				todo -= sg_dma_len(sg);
>>>> -				sg++;
>>>> +				sg = sg_next(sg);
>>>>  			}
>>>>  			*(rp++)=cpu_to_le32(RISC_WRITE|RISC_EOL|todo);
>>>>  			*(rp++)=cpu_to_le32(sg_dma_address(sg));
>>>> diff --git a/drivers/media/pci/ivtv/ivtv-udma.c b/drivers/media/pci/ivtv/ivtv-udma.c
>>>> index 7338cb2..bee2329 100644
>>>> --- a/drivers/media/pci/ivtv/ivtv-udma.c
>>>> +++ b/drivers/media/pci/ivtv/ivtv-udma.c
>>>> @@ -76,7 +76,7 @@ void ivtv_udma_fill_sg_array (struct ivtv_user_dma *dma, u32 buffer_offset, u32
>>>>  	int i;
>>>>  	struct scatterlist *sg;
>>>>  
>>>> -	for (i = 0, sg = dma->SGlist; i < dma->SG_length; i++, sg++) {
>>>> +	for (i = 0, sg = dma->SGlist; i < dma->SG_length; i++, sg = sg_next(sg)) {
>>>
>>> This hunk seems awkward, at least on my eyes. 
>>>
>>> As you've pointed at the description, S/G can be grouped into bigger
>>> segments. So, the maximum number of S/G can actually be less than
>>> dma->SGlist.
>>
>> That's correct, and dma->SG_length is equal to that number. SG_length is assigned
>> with the result of dma_map_sg() which returns "the number of bus address segments
>> mapped (this may be shorter than <nents> passed in if some elements of the
>> scatter/gather list are physically or virtually adjacent and an IOMMU maps them
>> with a single entry)."
>>
>> So the patch is correct.
> 
> OK.
> 
> Btw, you added this patch at a pull request for Kernel 3.19. Shouldn't it
> be, instead, be sent to 3.18 and c/c stable?

The cx23885 fix is already queued for 3.18. The bttv, cx25821 and cx88 are not
affected since they use vb1 and that won't combine scatterlists. The ivtv patch
could go to 3.18, I guess, but it is in a rarely used part of the driver and it
has been broken for quite some time. I do not feel that it is necessary to fix
for 3.18. IMHO.

Regards,

	Hans

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2014-11-24 19:16 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-11-21  9:20 [PATCH for v3.18] media: use sg = sg_next(sg) instead of sg++ Hans Verkuil
2014-11-21 10:05 ` Hans Verkuil
2014-11-21 10:50   ` Hans Verkuil
2014-11-24 18:24 ` Mauro Carvalho Chehab
2014-11-24 18:37   ` Hans Verkuil
2014-11-24 18:51     ` Mauro Carvalho Chehab
2014-11-24 19:16       ` Hans Verkuil

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).