[PATCH 4/5] tm6000: add frontend tuner xc5000

[PATCH 4/5] tm6000: add frontend tuner xc5000

[stefan.ringel]

From: Stefan Ringel <stefan.ringel@arcor.de>

Signed-off-by: Stefan Ringel <stefan.ringel@arcor.de>
---
 drivers/staging/tm6000/tm6000-dvb.c |   66 ++++++++++++++++++++++++----------
 1 files changed, 46 insertions(+), 20 deletions(-)

diff --git a/drivers/staging/tm6000/tm6000-dvb.c b/drivers/staging/tm6000/tm6000-dvb.c
index e6a802e..b504a90 100644
--- a/drivers/staging/tm6000/tm6000-dvb.c
+++ b/drivers/staging/tm6000/tm6000-dvb.c
@@ -257,27 +257,53 @@ int tm6000_dvb_register(struct tm6000_core *dev)
 	dvb->adapter.priv = dev;
 
 	if (dvb->frontend) {
-		struct xc2028_config cfg = {
-			.i2c_adap = &dev->i2c_adap,
-			.i2c_addr = dev->tuner_addr,
-		};
-
-		dvb->frontend->callback = tm6000_tuner_callback;
-		ret = dvb_register_frontend(&dvb->adapter, dvb->frontend);
-		if (ret < 0) {
-			printk(KERN_ERR
-				"tm6000: couldn't register frontend\n");
-			goto adapter_err;
+		switch (dev->tuner_type) {
+		case TUNER_XC2028:
+			struct xc2028_config cfg = {
+				.i2c_adap = &dev->i2c_adap,
+				.i2c_addr = dev->tuner_addr,
+			};
+
+			dvb->frontend->callback = tm6000_tuner_callback;
+			ret = dvb_register_frontend(&dvb->adapter, dvb->frontend);
+			if (ret < 0) {
+				printk(KERN_ERR
+					"tm6000: couldn't register frontend\n");
+				goto adapter_err;
+			}
+
+			if (!dvb_attach(xc2028_attach, dvb->frontend, &cfg)) {
+				printk(KERN_ERR "tm6000: couldn't register "
+						"frontend (xc3028)\n");
+				ret = -EINVAL;
+				goto frontend_err;
+			}
+			printk(KERN_INFO "tm6000: XC2028/3028 asked to be "
+					 "attached to frontend!\n");
+			break;
+		case TUNER_XC5000:
+			struct xc5000_config cfg = {
+				.i2c_address = dev->tuner_addr,
+			};
+
+			dvb->frontend->callback = tm6000_xc5000_callback;
+			ret = dvb_register_frontend(&dvb->adapter, dvb->frontend);
+			if (ret < 0) {
+				printk(KERN_ERR
+					"tm6000: couldn't register frontend\n");
+				goto adapter_err;
+			}
+
+			if (!dvb_attach(xc5000_attach, dvb->frontend, &dev->i2c_adap, &cfg)) {
+				printk(KERN_ERR "tm6000: couldn't register "
+						"frontend (xc5000)\n");
+				ret = -EINVAL;
+				goto frontend_err;
+			}
+			printk(KERN_INFO "tm6000: XC5000 asked to be "
+					 "attached to frontend!\n");
+			break;
 		}
-
-		if (!dvb_attach(xc2028_attach, dvb->frontend, &cfg)) {
-			printk(KERN_ERR "tm6000: couldn't register "
-					"frontend (xc3028)\n");
-			ret = -EINVAL;
-			goto frontend_err;
-		}
-		printk(KERN_INFO "tm6000: XC2028/3028 asked to be "
-				 "attached to frontend!\n");
 	} else {
 		printk(KERN_ERR "tm6000: no frontend found\n");
 	}
-- 
1.7.0.3

[PATCH 5/5] tm6000: rewrite copy_streams

[stefan.ringel]

From: Stefan Ringel <stefan.ringel@arcor.de>

fusion function copy streams and copy_packets to new function copy_streams.

Signed-off-by: Stefan Ringel <stefan.ringel@arcor.de>
---
 drivers/staging/tm6000/tm6000-usb-isoc.h |    5 +-
 drivers/staging/tm6000/tm6000-video.c    |  337 +++++++++++-------------------
 2 files changed, 127 insertions(+), 215 deletions(-)

diff --git a/drivers/staging/tm6000/tm6000-usb-isoc.h b/drivers/staging/tm6000/tm6000-usb-isoc.h
index 5a5049a..138716a 100644
--- a/drivers/staging/tm6000/tm6000-usb-isoc.h
+++ b/drivers/staging/tm6000/tm6000-usb-isoc.h
@@ -39,7 +39,7 @@ struct usb_isoc_ctl {
 	int				pos, size, pktsize;
 
 		/* Last field: ODD or EVEN? */
-	int				field;
+	int				vfield;
 
 		/* Stores incomplete commands */
 	u32				tmp_buf;
@@ -47,7 +47,4 @@ struct usb_isoc_ctl {
 
 		/* Stores already requested buffers */
 	struct tm6000_buffer    	*buf;
-
-		/* Stores the number of received fields */
-	int				nfields;
 };
diff --git a/drivers/staging/tm6000/tm6000-video.c b/drivers/staging/tm6000/tm6000-video.c
index 2a61cc3..31c574f 100644
--- a/drivers/staging/tm6000/tm6000-video.c
+++ b/drivers/staging/tm6000/tm6000-video.c
@@ -186,234 +186,153 @@ const char *tm6000_msg_type[] = {
 /*
  * Identify the tm5600/6000 buffer header type and properly handles
  */
-static int copy_packet(struct urb *urb, u32 header, u8 **ptr, u8 *endp,
-			u8 *out_p, struct tm6000_buffer **buf)
-{
-	struct tm6000_dmaqueue  *dma_q = urb->context;
-	struct tm6000_core *dev = container_of(dma_q, struct tm6000_core, vidq);
-	u8 c;
-	unsigned int cmd, cpysize, pktsize, size, field, block, line, pos = 0;
-	int rc = 0;
-	/* FIXME: move to tm6000-isoc */
-	static int last_line = -2, start_line = -2, last_field = -2;
-
-	/* FIXME: this is the hardcoded window size
-	 */
-	unsigned int linewidth = (*buf)->vb.width << 1;
-
-	if (!dev->isoc_ctl.cmd) {
-		c = (header >> 24) & 0xff;
-
-		/* split the header fields */
-		size  = ((header & 0x7e) << 1);
-
-		if (size > 0)
-			size -= 4;
-
-		block = (header >> 7) & 0xf;
-		field = (header >> 11) & 0x1;
-		line  = (header >> 12) & 0x1ff;
-		cmd   = (header >> 21) & 0x7;
-
-		/* Validates header fields */
-		if(size > TM6000_URB_MSG_LEN)
-			size = TM6000_URB_MSG_LEN;
-
-		if (cmd == TM6000_URB_MSG_VIDEO) {
-			if ((block+1)*TM6000_URB_MSG_LEN>linewidth)
-				cmd = TM6000_URB_MSG_ERR;
-
-			/* FIXME: Mounts the image as field0+field1
-			 * It should, instead, check if the user selected
-			 * entrelaced or non-entrelaced mode
-			 */
-			pos= ((line<<1)+field)*linewidth +
-				block*TM6000_URB_MSG_LEN;
-
-			/* Don't allow to write out of the buffer */
-			if (pos+TM6000_URB_MSG_LEN > (*buf)->vb.size) {
-				dprintk(dev, V4L2_DEBUG_ISOC,
-					"ERR: size=%d, num=%d, line=%d, "
-					"field=%d\n",
-					size, block, line, field);
-
-				cmd = TM6000_URB_MSG_ERR;
-			}
-		} else {
-			pos=0;
-		}
-
-		/* Prints debug info */
-		dprintk(dev, V4L2_DEBUG_ISOC, "size=%d, num=%d, "
-				" line=%d, field=%d\n",
-				size, block, line, field);
-
-		if ((last_line!=line)&&(last_line+1!=line) &&
-		    (cmd != TM6000_URB_MSG_ERR) )  {
-			if (cmd != TM6000_URB_MSG_VIDEO)  {
-				dprintk(dev, V4L2_DEBUG_ISOC,  "cmd=%d, "
-					"size=%d, num=%d, line=%d, field=%d\n",
-					cmd, size, block, line, field);
-			}
-			if (start_line<0)
-				start_line=last_line;
-			/* Prints debug info */
-			dprintk(dev, V4L2_DEBUG_ISOC, "lines= %d-%d, "
-					"field=%d\n",
-					start_line, last_line, field);
-
-			if ((start_line<6 && last_line>200) &&
-				(last_field != field) ) {
-
-				dev->isoc_ctl.nfields++;
-				if (dev->isoc_ctl.nfields>=2) {
-					dev->isoc_ctl.nfields=0;
-
-					/* Announces that a new buffer were filled */
-					buffer_filled (dev, dma_q, *buf);
-					dprintk(dev, V4L2_DEBUG_ISOC,
-							"new buffer filled\n");
-					get_next_buf (dma_q, buf);
-					if (!*buf)
-						return rc;
-					out_p = videobuf_to_vmalloc(&((*buf)->vb));
-					if (!out_p)
-						return rc;
-
-					pos = dev->isoc_ctl.pos = 0;
-				}
-			}
-
-			start_line=line;
-			last_field=field;
-		}
-		if (cmd == TM6000_URB_MSG_VIDEO)
-			last_line = line;
-
-		pktsize = TM6000_URB_MSG_LEN;
-	} else {
-		/* Continue the last copy */
-		cmd = dev->isoc_ctl.cmd;
-		size= dev->isoc_ctl.size;
-		pos = dev->isoc_ctl.pos;
-		pktsize = dev->isoc_ctl.pktsize;
-	}
-
-	cpysize = (endp-(*ptr) > size) ? size : endp - *ptr;
-
-	if (cpysize) {
-		/* handles each different URB message */
-		switch(cmd) {
-		case TM6000_URB_MSG_VIDEO:
-			/* Fills video buffer */
-			memcpy(&out_p[pos], *ptr, cpysize);
-			break;
-		case TM6000_URB_MSG_PTS:
-			break;
-		case TM6000_URB_MSG_AUDIO:
-			/* Need some code to process audio */
-			printk ("%ld: cmd=%s, size=%d\n", jiffies,
-				tm6000_msg_type[cmd],size);
-			break;
-		case TM6000_URB_MSG_VBI:
-			break;
-		default:
-			dprintk (dev, V4L2_DEBUG_ISOC, "cmd=%s, size=%d\n",
-						tm6000_msg_type[cmd],size);
-		}
-	}
-	if (cpysize<size) {
-		/* End of URB packet, but cmd processing is not
-		 * complete. Preserve the state for a next packet
-		 */
-		dev->isoc_ctl.pos = pos+cpysize;
-		dev->isoc_ctl.size= size-cpysize;
-		dev->isoc_ctl.cmd = cmd;
-		dev->isoc_ctl.pktsize = pktsize-cpysize;
-		(*ptr)+=cpysize;
-	} else {
-		dev->isoc_ctl.cmd = 0;
-		(*ptr)+=pktsize;
-	}
-
-	return rc;
-}
-
 static int copy_streams(u8 *data, unsigned long len,
 			struct urb *urb)
 {
 	struct tm6000_dmaqueue  *dma_q = urb->context;
 	struct tm6000_core *dev= container_of(dma_q,struct tm6000_core,vidq);
-	u8 *ptr=data, *endp=data+len;
+	u8 *ptr=data, *endp=data+len, c;
 	unsigned long header=0;
 	int rc=0;
-	struct tm6000_buffer *buf;
-	char *outp = NULL;
-
-	get_next_buf(dma_q, &buf);
-	if (buf)
-		outp = videobuf_to_vmalloc(&buf->vb);
+	unsigned int cmd, cpysize, pktsize, size, field, block, line, pos = 0;
+	struct tm6000_buffer *vbuf;
+	char *voutp = NULL;
+	unsigned int linewidth;
 
-	if (!outp)
+	/* get video buffer */
+	get_next_buf (dma_q, &vbuf);
+	if (!vbuf)
+		return rc;
+	voutp = videobuf_to_vmalloc(&vbuf->vb);
+	if (!voutp)
 		return 0;
 
-	for (ptr=data; ptr<endp;) {
+	for (ptr = data; ptr < endp;) {
 		if (!dev->isoc_ctl.cmd) {
-			u8 *p=(u8 *)&dev->isoc_ctl.tmp_buf;
-			/* FIXME: This seems very complex
-			 * It just recovers up to 3 bytes of the header that
-			 * might be at the previous packet
-			 */
-			if (dev->isoc_ctl.tmp_buf_len) {
-				while (dev->isoc_ctl.tmp_buf_len) {
-					if ( *(ptr+3-dev->isoc_ctl.tmp_buf_len) == 0x47) {
-						break;
-					}
-					p++;
-					dev->isoc_ctl.tmp_buf_len--;
-				}
-				if (dev->isoc_ctl.tmp_buf_len) {
-					memcpy(&header, p,
-						dev->isoc_ctl.tmp_buf_len);
-					memcpy((u8 *)&header +
+			/* Header */
+			if (dev->isoc_ctl.tmp_buf_len > 0) {
+				/* from last urb or packet */
+				header = dev->isoc_ctl.tmp_buf;
+				if (4 - dev->isoc_ctl.tmp_buf_len > 0)
+					memcpy ((u8 *)&header +
 						dev->isoc_ctl.tmp_buf_len,
 						ptr,
 						4 - dev->isoc_ctl.tmp_buf_len);
 					ptr += 4 - dev->isoc_ctl.tmp_buf_len;
-					goto HEADER;
 				}
+				dev->isoc_ctl.tmp_buf_len = 0;
+			} else {
+				if (ptr + 3 >= endp) {
+					/* have incomplete header */
+					dev->isoc_ctl.tmp_buf_len = endp - ptr;
+					memcpy (&dev->isoc_ctl.tmp_buf, ptr,
+						dev->isoc_ctl.tmp_buf_len);
+					return rc;
+				}
+				/* Seek for sync */
+				for (; ptr < endp - 3; ptr++) {
+					if (ptr < endp - 187) {
+						if (*(ptr + 3) == 0x47 &&
+							(*(ptr + 187) == 0x47)
+							break;
+					} else {
+						if (*(ptr + 3) == 0x47)
+							break;
+					}
+					if (ptr + 3 >= endp)
+						return rc;
+				}
+				/* Get message header */
+				header = *(unsigned long *)ptr;
+				ptr += 4;
 			}
-			/* Seek for sync */
-			for (;ptr<endp-3;ptr++) {
-				if (*(ptr+3)==0x47)
-					break;
+			/* split the header fields */
+			c = (header >> 24) & 0xff;
+			size = ((header & 0x7e) << 1);
+			if (size > 0)
+				size -= 4;
+			block = (header >> 7) & 0xf;
+			field = (header >> 11) & 0x1;
+			line  = (header >> 12) & 0x1ff;
+			cmd   = (header >> 21) & 0x7;
+			/* Validates haeder fields */
+			if (size > TM6000_URB_MSG_LEN)
+				size = TM6000_URB_MSG_LEN;
+			pktsize = TM6000_URB_MSG_LEN;
+			/* calculate position in buffer 
+			 * and change the buffer
+			 */
+			switch (cmd) {
+			case TM6000_URB_MSG_VIDEO:
+				if ((dev->isoc_ctl.vfield != field) && 
+					(field == 1) {
+					/* Announces that a new buffer 
+					 * were filled 
+					 */
+					buffer_filled (dev, dma_q, vbuf);
+					dprintk (dev, V4L2_DEBUG_ISOC,
+							"new buffer filled\n");
+					get_next_buf (dma_q, &vbuf);
+					if (!vbuf)
+						return rc;
+					voutp = videobuf_to_vmalloc (&vbuf->vb);
+					if (!voutp)
+						return rc;
+				}
+				linewidth = vbuf->vb.width << 1;
+				pos = ((line << 1) - field - 1) * linewidth +
+					block * TM6000_URB_MSG_LEN;
+				/* Don't allow to write out of the buffer */
+				if (pos + size > vbuf->vb.size)
+					cmd = TM6000_URB_MSG_ERR;
+				dev->isoc_ctl.vfield = field;
+				break;
+			case TM6000_URB_MSG_AUDIO:
+			case TM6000_URB_MSG_VBI:
+			case TM6000_URB_MSG_PTS:
+				break;
 			}
-
-			if (ptr+3>=endp) {
-				dev->isoc_ctl.tmp_buf_len=endp-ptr;
-				memcpy (&dev->isoc_ctl.tmp_buf,ptr,
-					dev->isoc_ctl.tmp_buf_len);
-				dev->isoc_ctl.cmd=0;
-				return rc;
+		} else {
+			/* Continue the last copy */
+			cmd = dev->isoc_ctl.cmd;
+			size = dev->isoc_ctl.size;
+			pos = dev->isoc_ctl.pos;
+			pktsize = dev->isoc_ctl-pktsize;
+		}
+		cpysize = (endp - ptr > size) ? size : endp - ptr;
+		if (cpysize) {
+			/* copy data in different buffers */
+			switch (cmd) {
+			case TM6000_URB_MSG_VIDEO:
+				/* Fills video buffer */
+				if (vbuf)
+					memcpy (&voutp[pos], ptr, cpysize);
+				break;
+			case TM6000_URB_MSG_AUDIO:
+				/* Need some code to copy audio buffer */
+				break;
+			case TM6000_URB_MSG_VBI:
+				/* Need some code to copy vbi buffer */
+				break;
+			case TM6000_URB_MSG_PTS:
+				/* Need some code to copy pts */
+				break;
 			}
-
-			/* Get message header */
-			header=*(unsigned long *)ptr;
-			ptr+=4;
 		}
-HEADER:
-		/* Copy or continue last copy */
-		rc=copy_packet(urb,header,&ptr,endp,outp,&buf);
-		if (rc<0) {
-			buf=NULL;
-			printk(KERN_ERR "tm6000: buffer underrun at %ld\n",
-					jiffies);
-			return rc;
+		if (cpysize < size) {
+			/* End of URB packet, but cmd processing is not
+			 * complete. Preserve the state for a next packet
+			 */
+			dev->isoc_ctl.pos = pos + cpysize;
+			dev->isoc_ctl.size = size - cpysize;
+			dev->isoc_ctl.cmd = cmd;
+			dev->isoc_ctl.pktsize = pktsize - cpysize;
+			ptr += cpysize;
+		} else {
+			dev->isoc_ctl.cmd = 0;
+			ptr += pktsize;
 		}
-		if (!*buf)
-			return 0;
 	}
-
 	return 0;
 }
 /*
@@ -510,7 +429,6 @@ static inline int tm6000_isoc_copy(struct urb *urb)
 {
 	struct tm6000_dmaqueue  *dma_q = urb->context;
 	struct tm6000_core *dev= container_of(dma_q,struct tm6000_core,vidq);
-	struct tm6000_buffer *buf;
 	int i, len=0, rc=1, status;
 	char *p;
 
@@ -585,7 +503,6 @@ static void tm6000_uninit_isoc(struct tm6000_core *dev)
 	struct urb *urb;
 	int i;
 
-	dev->isoc_ctl.nfields = -1;
 	dev->isoc_ctl.buf = NULL;
 	for (i = 0; i < dev->isoc_ctl.num_bufs; i++) {
 		urb=dev->isoc_ctl.urb[i];
@@ -610,8 +527,6 @@ static void tm6000_uninit_isoc(struct tm6000_core *dev)
 	dev->isoc_ctl.urb=NULL;
 	dev->isoc_ctl.transfer_buffer=NULL;
 	dev->isoc_ctl.num_bufs = 0;
-
-	dev->isoc_ctl.num_bufs=0;
 }
 
 /*
-- 
1.7.0.3

Re: [PATCH 5/5] tm6000: rewrite copy_streams

[Mauro Carvalho Chehab]

Em Sun, 23 May 2010 20:31:45 +0200
stefan.ringel@arcor.de escreveu:

> From: Stefan Ringel <stefan.ringel@arcor.de>
> 
> fusion function copy streams and copy_packets to new function copy_streams.
> 
> Signed-off-by: Stefan Ringel <stefan.ringel@arcor.de>
> ---
>  drivers/staging/tm6000/tm6000-usb-isoc.h |    5 +-
>  drivers/staging/tm6000/tm6000-video.c    |  337 +++++++++++-------------------
>  2 files changed, 127 insertions(+), 215 deletions(-)
> 
> diff --git a/drivers/staging/tm6000/tm6000-usb-isoc.h b/drivers/staging/tm6000/tm6000-usb-isoc.h
> index 5a5049a..138716a 100644
> --- a/drivers/staging/tm6000/tm6000-usb-isoc.h
> +++ b/drivers/staging/tm6000/tm6000-usb-isoc.h
> @@ -39,7 +39,7 @@ struct usb_isoc_ctl {
>  	int				pos, size, pktsize;
>  
>  		/* Last field: ODD or EVEN? */
> -	int				field;
> +	int				vfield;
>  
>  		/* Stores incomplete commands */
>  	u32				tmp_buf;
> @@ -47,7 +47,4 @@ struct usb_isoc_ctl {
>  
>  		/* Stores already requested buffers */
>  	struct tm6000_buffer    	*buf;
> -
> -		/* Stores the number of received fields */
> -	int				nfields;
>  };
> diff --git a/drivers/staging/tm6000/tm6000-video.c b/drivers/staging/tm6000/tm6000-video.c
> index 2a61cc3..31c574f 100644
> --- a/drivers/staging/tm6000/tm6000-video.c
> +++ b/drivers/staging/tm6000/tm6000-video.c
> @@ -186,234 +186,153 @@ const char *tm6000_msg_type[] = {
>  /*
>   * Identify the tm5600/6000 buffer header type and properly handles
>   */
> -static int copy_packet(struct urb *urb, u32 header, u8 **ptr, u8 *endp,
> -			u8 *out_p, struct tm6000_buffer **buf)
> -{
> -	struct tm6000_dmaqueue  *dma_q = urb->context;
> -	struct tm6000_core *dev = container_of(dma_q, struct tm6000_core, vidq);
> -	u8 c;
> -	unsigned int cmd, cpysize, pktsize, size, field, block, line, pos = 0;
> -	int rc = 0;
> -	/* FIXME: move to tm6000-isoc */
> -	static int last_line = -2, start_line = -2, last_field = -2;
> -
> -	/* FIXME: this is the hardcoded window size
> -	 */
> -	unsigned int linewidth = (*buf)->vb.width << 1;
> -
> -	if (!dev->isoc_ctl.cmd) {
> -		c = (header >> 24) & 0xff;
> -
> -		/* split the header fields */
> -		size  = ((header & 0x7e) << 1);
> -
> -		if (size > 0)
> -			size -= 4;
> -
> -		block = (header >> 7) & 0xf;
> -		field = (header >> 11) & 0x1;
> -		line  = (header >> 12) & 0x1ff;
> -		cmd   = (header >> 21) & 0x7;
> -
> -		/* Validates header fields */
> -		if(size > TM6000_URB_MSG_LEN)
> -			size = TM6000_URB_MSG_LEN;
> -
> -		if (cmd == TM6000_URB_MSG_VIDEO) {
> -			if ((block+1)*TM6000_URB_MSG_LEN>linewidth)
> -				cmd = TM6000_URB_MSG_ERR;
> -
> -			/* FIXME: Mounts the image as field0+field1
> -			 * It should, instead, check if the user selected
> -			 * entrelaced or non-entrelaced mode
> -			 */
> -			pos= ((line<<1)+field)*linewidth +
> -				block*TM6000_URB_MSG_LEN;
> -
> -			/* Don't allow to write out of the buffer */
> -			if (pos+TM6000_URB_MSG_LEN > (*buf)->vb.size) {
> -				dprintk(dev, V4L2_DEBUG_ISOC,
> -					"ERR: size=%d, num=%d, line=%d, "
> -					"field=%d\n",
> -					size, block, line, field);
> -
> -				cmd = TM6000_URB_MSG_ERR;
> -			}
> -		} else {
> -			pos=0;
> -		}
> -
> -		/* Prints debug info */
> -		dprintk(dev, V4L2_DEBUG_ISOC, "size=%d, num=%d, "
> -				" line=%d, field=%d\n",
> -				size, block, line, field);
> -
> -		if ((last_line!=line)&&(last_line+1!=line) &&
> -		    (cmd != TM6000_URB_MSG_ERR) )  {
> -			if (cmd != TM6000_URB_MSG_VIDEO)  {
> -				dprintk(dev, V4L2_DEBUG_ISOC,  "cmd=%d, "
> -					"size=%d, num=%d, line=%d, field=%d\n",
> -					cmd, size, block, line, field);
> -			}
> -			if (start_line<0)
> -				start_line=last_line;
> -			/* Prints debug info */
> -			dprintk(dev, V4L2_DEBUG_ISOC, "lines= %d-%d, "
> -					"field=%d\n",
> -					start_line, last_line, field);
> -
> -			if ((start_line<6 && last_line>200) &&
> -				(last_field != field) ) {
> -
> -				dev->isoc_ctl.nfields++;
> -				if (dev->isoc_ctl.nfields>=2) {
> -					dev->isoc_ctl.nfields=0;
> -
> -					/* Announces that a new buffer were filled */
> -					buffer_filled (dev, dma_q, *buf);
> -					dprintk(dev, V4L2_DEBUG_ISOC,
> -							"new buffer filled\n");
> -					get_next_buf (dma_q, buf);
> -					if (!*buf)
> -						return rc;
> -					out_p = videobuf_to_vmalloc(&((*buf)->vb));
> -					if (!out_p)
> -						return rc;
> -
> -					pos = dev->isoc_ctl.pos = 0;
> -				}
> -			}
> -
> -			start_line=line;
> -			last_field=field;
> -		}
> -		if (cmd == TM6000_URB_MSG_VIDEO)
> -			last_line = line;
> -
> -		pktsize = TM6000_URB_MSG_LEN;
> -	} else {
> -		/* Continue the last copy */
> -		cmd = dev->isoc_ctl.cmd;
> -		size= dev->isoc_ctl.size;
> -		pos = dev->isoc_ctl.pos;
> -		pktsize = dev->isoc_ctl.pktsize;
> -	}
> -
> -	cpysize = (endp-(*ptr) > size) ? size : endp - *ptr;
> -
> -	if (cpysize) {
> -		/* handles each different URB message */
> -		switch(cmd) {
> -		case TM6000_URB_MSG_VIDEO:
> -			/* Fills video buffer */
> -			memcpy(&out_p[pos], *ptr, cpysize);
> -			break;
> -		case TM6000_URB_MSG_PTS:
> -			break;
> -		case TM6000_URB_MSG_AUDIO:
> -			/* Need some code to process audio */
> -			printk ("%ld: cmd=%s, size=%d\n", jiffies,
> -				tm6000_msg_type[cmd],size);
> -			break;
> -		case TM6000_URB_MSG_VBI:
> -			break;
> -		default:
> -			dprintk (dev, V4L2_DEBUG_ISOC, "cmd=%s, size=%d\n",
> -						tm6000_msg_type[cmd],size);
> -		}
> -	}
> -	if (cpysize<size) {
> -		/* End of URB packet, but cmd processing is not
> -		 * complete. Preserve the state for a next packet
> -		 */
> -		dev->isoc_ctl.pos = pos+cpysize;
> -		dev->isoc_ctl.size= size-cpysize;
> -		dev->isoc_ctl.cmd = cmd;
> -		dev->isoc_ctl.pktsize = pktsize-cpysize;
> -		(*ptr)+=cpysize;
> -	} else {
> -		dev->isoc_ctl.cmd = 0;
> -		(*ptr)+=pktsize;
> -	}
> -
> -	return rc;
> -}
> -
>  static int copy_streams(u8 *data, unsigned long len,
>  			struct urb *urb)
>  {
>  	struct tm6000_dmaqueue  *dma_q = urb->context;
>  	struct tm6000_core *dev= container_of(dma_q,struct tm6000_core,vidq);
> -	u8 *ptr=data, *endp=data+len;
> +	u8 *ptr=data, *endp=data+len, c;
>  	unsigned long header=0;
>  	int rc=0;
> -	struct tm6000_buffer *buf;
> -	char *outp = NULL;
> -
> -	get_next_buf(dma_q, &buf);
> -	if (buf)
> -		outp = videobuf_to_vmalloc(&buf->vb);
> +	unsigned int cmd, cpysize, pktsize, size, field, block, line, pos = 0;
> +	struct tm6000_buffer *vbuf;
> +	char *voutp = NULL;
> +	unsigned int linewidth;
>  
> -	if (!outp)
> +	/* get video buffer */
> +	get_next_buf (dma_q, &vbuf);
> +	if (!vbuf)
> +		return rc;
> +	voutp = videobuf_to_vmalloc(&vbuf->vb);
> +	if (!voutp)
>  		return 0;
>  
> -	for (ptr=data; ptr<endp;) {
> +	for (ptr = data; ptr < endp;) {
>  		if (!dev->isoc_ctl.cmd) {
> -			u8 *p=(u8 *)&dev->isoc_ctl.tmp_buf;
> -			/* FIXME: This seems very complex
> -			 * It just recovers up to 3 bytes of the header that
> -			 * might be at the previous packet
> -			 */
> -			if (dev->isoc_ctl.tmp_buf_len) {
> -				while (dev->isoc_ctl.tmp_buf_len) {
> -					if ( *(ptr+3-dev->isoc_ctl.tmp_buf_len) == 0x47) {
> -						break;
> -					}
> -					p++;
> -					dev->isoc_ctl.tmp_buf_len--;
> -				}
> -				if (dev->isoc_ctl.tmp_buf_len) {
> -					memcpy(&header, p,
> -						dev->isoc_ctl.tmp_buf_len);
> -					memcpy((u8 *)&header +
> +			/* Header */
> +			if (dev->isoc_ctl.tmp_buf_len > 0) {
> +				/* from last urb or packet */
> +				header = dev->isoc_ctl.tmp_buf;
> +				if (4 - dev->isoc_ctl.tmp_buf_len > 0)
> +					memcpy ((u8 *)&header +
>  						dev->isoc_ctl.tmp_buf_len,
>  						ptr,
>  						4 - dev->isoc_ctl.tmp_buf_len);
>  					ptr += 4 - dev->isoc_ctl.tmp_buf_len;
> -					goto HEADER;
>  				}
> +				dev->isoc_ctl.tmp_buf_len = 0;
> +			} else {
> +				if (ptr + 3 >= endp) {
> +					/* have incomplete header */
> +					dev->isoc_ctl.tmp_buf_len = endp - ptr;
> +					memcpy (&dev->isoc_ctl.tmp_buf, ptr,
> +						dev->isoc_ctl.tmp_buf_len);
> +					return rc;
> +				}
> +				/* Seek for sync */
> +				for (; ptr < endp - 3; ptr++) {
> +					if (ptr < endp - 187) {
> +						if (*(ptr + 3) == 0x47 &&
> +							(*(ptr + 187) == 0x47)
> +							break;

Hmm... are you sure you need to do this? Just checking for the current sync seems
to be enough, except if the URB is corrupted. In the latter case, it would be better
to do the opposite: test for the sync at either ptr or ptr + 187:

		if (*(ptr + 3) == 0x47 || (*(ptr + 187) == 0x47)

I don't like the above neither, but this device requires so many workarounds to work
with a bad hardware/firmware that one more hack wouldn't hurt, but, if this is really
needed, a proper comment explaining the reason for the hack should be added.

> +					} else {
> +						if (*(ptr + 3) == 0x47)
> +							break;
> +					}
> +					if (ptr + 3 >= endp)
> +						return rc;

Huh? This test look strange: if ptr + 3 >= endp, the loop would be end before
calling this code. So, it seems to be a dead code.

> +				}
> +				/* Get message header */
> +				header = *(unsigned long *)ptr;
> +				ptr += 4;
>  			}

-- 

Cheers,
Mauro

Re: [PATCH 5/5] tm6000: rewrite copy_streams

[Stefan Ringel]

[-- Attachment #1: Type: text/plain, Size: 10212 bytes --]

Am 27.05.2010 23:23, schrieb Mauro Carvalho Chehab:
> Em Sun, 23 May 2010 20:31:45 +0200
> stefan.ringel@arcor.de escreveu:
>
>   
>> From: Stefan Ringel <stefan.ringel@arcor.de>
>>
>> fusion function copy streams and copy_packets to new function copy_streams.
>>
>> Signed-off-by: Stefan Ringel <stefan.ringel@arcor.de>
>> ---
>>  drivers/staging/tm6000/tm6000-usb-isoc.h |    5 +-
>>  drivers/staging/tm6000/tm6000-video.c    |  337 +++++++++++-------------------
>>  2 files changed, 127 insertions(+), 215 deletions(-)
>>
>> diff --git a/drivers/staging/tm6000/tm6000-usb-isoc.h b/drivers/staging/tm6000/tm6000-usb-isoc.h
>> index 5a5049a..138716a 100644
>> --- a/drivers/staging/tm6000/tm6000-usb-isoc.h
>> +++ b/drivers/staging/tm6000/tm6000-usb-isoc.h
>> @@ -39,7 +39,7 @@ struct usb_isoc_ctl {
>>  	int				pos, size, pktsize;
>>  
>>  		/* Last field: ODD or EVEN? */
>> -	int				field;
>> +	int				vfield;
>>  
>>  		/* Stores incomplete commands */
>>  	u32				tmp_buf;
>> @@ -47,7 +47,4 @@ struct usb_isoc_ctl {
>>  
>>  		/* Stores already requested buffers */
>>  	struct tm6000_buffer    	*buf;
>> -
>> -		/* Stores the number of received fields */
>> -	int				nfields;
>>  };
>> diff --git a/drivers/staging/tm6000/tm6000-video.c b/drivers/staging/tm6000/tm6000-video.c
>> index 2a61cc3..31c574f 100644
>> --- a/drivers/staging/tm6000/tm6000-video.c
>> +++ b/drivers/staging/tm6000/tm6000-video.c
>> @@ -186,234 +186,153 @@ const char *tm6000_msg_type[] = {
>>  /*
>>   * Identify the tm5600/6000 buffer header type and properly handles
>>   */
>> -static int copy_packet(struct urb *urb, u32 header, u8 **ptr, u8 *endp,
>> -			u8 *out_p, struct tm6000_buffer **buf)
>> -{
>> -	struct tm6000_dmaqueue  *dma_q = urb->context;
>> -	struct tm6000_core *dev = container_of(dma_q, struct tm6000_core, vidq);
>> -	u8 c;
>> -	unsigned int cmd, cpysize, pktsize, size, field, block, line, pos = 0;
>> -	int rc = 0;
>> -	/* FIXME: move to tm6000-isoc */
>> -	static int last_line = -2, start_line = -2, last_field = -2;
>> -
>> -	/* FIXME: this is the hardcoded window size
>> -	 */
>> -	unsigned int linewidth = (*buf)->vb.width << 1;
>> -
>> -	if (!dev->isoc_ctl.cmd) {
>> -		c = (header >> 24) & 0xff;
>> -
>> -		/* split the header fields */
>> -		size  = ((header & 0x7e) << 1);
>> -
>> -		if (size > 0)
>> -			size -= 4;
>> -
>> -		block = (header >> 7) & 0xf;
>> -		field = (header >> 11) & 0x1;
>> -		line  = (header >> 12) & 0x1ff;
>> -		cmd   = (header >> 21) & 0x7;
>> -
>> -		/* Validates header fields */
>> -		if(size > TM6000_URB_MSG_LEN)
>> -			size = TM6000_URB_MSG_LEN;
>> -
>> -		if (cmd == TM6000_URB_MSG_VIDEO) {
>> -			if ((block+1)*TM6000_URB_MSG_LEN>linewidth)
>> -				cmd = TM6000_URB_MSG_ERR;
>> -
>> -			/* FIXME: Mounts the image as field0+field1
>> -			 * It should, instead, check if the user selected
>> -			 * entrelaced or non-entrelaced mode
>> -			 */
>> -			pos= ((line<<1)+field)*linewidth +
>> -				block*TM6000_URB_MSG_LEN;
>> -
>> -			/* Don't allow to write out of the buffer */
>> -			if (pos+TM6000_URB_MSG_LEN > (*buf)->vb.size) {
>> -				dprintk(dev, V4L2_DEBUG_ISOC,
>> -					"ERR: size=%d, num=%d, line=%d, "
>> -					"field=%d\n",
>> -					size, block, line, field);
>> -
>> -				cmd = TM6000_URB_MSG_ERR;
>> -			}
>> -		} else {
>> -			pos=0;
>> -		}
>> -
>> -		/* Prints debug info */
>> -		dprintk(dev, V4L2_DEBUG_ISOC, "size=%d, num=%d, "
>> -				" line=%d, field=%d\n",
>> -				size, block, line, field);
>> -
>> -		if ((last_line!=line)&&(last_line+1!=line) &&
>> -		    (cmd != TM6000_URB_MSG_ERR) )  {
>> -			if (cmd != TM6000_URB_MSG_VIDEO)  {
>> -				dprintk(dev, V4L2_DEBUG_ISOC,  "cmd=%d, "
>> -					"size=%d, num=%d, line=%d, field=%d\n",
>> -					cmd, size, block, line, field);
>> -			}
>> -			if (start_line<0)
>> -				start_line=last_line;
>> -			/* Prints debug info */
>> -			dprintk(dev, V4L2_DEBUG_ISOC, "lines= %d-%d, "
>> -					"field=%d\n",
>> -					start_line, last_line, field);
>> -
>> -			if ((start_line<6 && last_line>200) &&
>> -				(last_field != field) ) {
>> -
>> -				dev->isoc_ctl.nfields++;
>> -				if (dev->isoc_ctl.nfields>=2) {
>> -					dev->isoc_ctl.nfields=0;
>> -
>> -					/* Announces that a new buffer were filled */
>> -					buffer_filled (dev, dma_q, *buf);
>> -					dprintk(dev, V4L2_DEBUG_ISOC,
>> -							"new buffer filled\n");
>> -					get_next_buf (dma_q, buf);
>> -					if (!*buf)
>> -						return rc;
>> -					out_p = videobuf_to_vmalloc(&((*buf)->vb));
>> -					if (!out_p)
>> -						return rc;
>> -
>> -					pos = dev->isoc_ctl.pos = 0;
>> -				}
>> -			}
>> -
>> -			start_line=line;
>> -			last_field=field;
>> -		}
>> -		if (cmd == TM6000_URB_MSG_VIDEO)
>> -			last_line = line;
>> -
>> -		pktsize = TM6000_URB_MSG_LEN;
>> -	} else {
>> -		/* Continue the last copy */
>> -		cmd = dev->isoc_ctl.cmd;
>> -		size= dev->isoc_ctl.size;
>> -		pos = dev->isoc_ctl.pos;
>> -		pktsize = dev->isoc_ctl.pktsize;
>> -	}
>> -
>> -	cpysize = (endp-(*ptr) > size) ? size : endp - *ptr;
>> -
>> -	if (cpysize) {
>> -		/* handles each different URB message */
>> -		switch(cmd) {
>> -		case TM6000_URB_MSG_VIDEO:
>> -			/* Fills video buffer */
>> -			memcpy(&out_p[pos], *ptr, cpysize);
>> -			break;
>> -		case TM6000_URB_MSG_PTS:
>> -			break;
>> -		case TM6000_URB_MSG_AUDIO:
>> -			/* Need some code to process audio */
>> -			printk ("%ld: cmd=%s, size=%d\n", jiffies,
>> -				tm6000_msg_type[cmd],size);
>> -			break;
>> -		case TM6000_URB_MSG_VBI:
>> -			break;
>> -		default:
>> -			dprintk (dev, V4L2_DEBUG_ISOC, "cmd=%s, size=%d\n",
>> -						tm6000_msg_type[cmd],size);
>> -		}
>> -	}
>> -	if (cpysize<size) {
>> -		/* End of URB packet, but cmd processing is not
>> -		 * complete. Preserve the state for a next packet
>> -		 */
>> -		dev->isoc_ctl.pos = pos+cpysize;
>> -		dev->isoc_ctl.size= size-cpysize;
>> -		dev->isoc_ctl.cmd = cmd;
>> -		dev->isoc_ctl.pktsize = pktsize-cpysize;
>> -		(*ptr)+=cpysize;
>> -	} else {
>> -		dev->isoc_ctl.cmd = 0;
>> -		(*ptr)+=pktsize;
>> -	}
>> -
>> -	return rc;
>> -}
>> -
>>  static int copy_streams(u8 *data, unsigned long len,
>>  			struct urb *urb)
>>  {
>>  	struct tm6000_dmaqueue  *dma_q = urb->context;
>>  	struct tm6000_core *dev= container_of(dma_q,struct tm6000_core,vidq);
>> -	u8 *ptr=data, *endp=data+len;
>> +	u8 *ptr=data, *endp=data+len, c;
>>  	unsigned long header=0;
>>  	int rc=0;
>> -	struct tm6000_buffer *buf;
>> -	char *outp = NULL;
>> -
>> -	get_next_buf(dma_q, &buf);
>> -	if (buf)
>> -		outp = videobuf_to_vmalloc(&buf->vb);
>> +	unsigned int cmd, cpysize, pktsize, size, field, block, line, pos = 0;
>> +	struct tm6000_buffer *vbuf;
>> +	char *voutp = NULL;
>> +	unsigned int linewidth;
>>  
>> -	if (!outp)
>> +	/* get video buffer */
>> +	get_next_buf (dma_q, &vbuf);
>> +	if (!vbuf)
>> +		return rc;
>> +	voutp = videobuf_to_vmalloc(&vbuf->vb);
>> +	if (!voutp)
>>  		return 0;
>>  
>> -	for (ptr=data; ptr<endp;) {
>> +	for (ptr = data; ptr < endp;) {
>>  		if (!dev->isoc_ctl.cmd) {
>> -			u8 *p=(u8 *)&dev->isoc_ctl.tmp_buf;
>> -			/* FIXME: This seems very complex
>> -			 * It just recovers up to 3 bytes of the header that
>> -			 * might be at the previous packet
>> -			 */
>> -			if (dev->isoc_ctl.tmp_buf_len) {
>> -				while (dev->isoc_ctl.tmp_buf_len) {
>> -					if ( *(ptr+3-dev->isoc_ctl.tmp_buf_len) == 0x47) {
>> -						break;
>> -					}
>> -					p++;
>> -					dev->isoc_ctl.tmp_buf_len--;
>> -				}
>> -				if (dev->isoc_ctl.tmp_buf_len) {
>> -					memcpy(&header, p,
>> -						dev->isoc_ctl.tmp_buf_len);
>> -					memcpy((u8 *)&header +
>> +			/* Header */
>> +			if (dev->isoc_ctl.tmp_buf_len > 0) {
>> +				/* from last urb or packet */
>> +				header = dev->isoc_ctl.tmp_buf;
>> +				if (4 - dev->isoc_ctl.tmp_buf_len > 0)
>> +					memcpy ((u8 *)&header +
>>  						dev->isoc_ctl.tmp_buf_len,
>>  						ptr,
>>  						4 - dev->isoc_ctl.tmp_buf_len);
>>  					ptr += 4 - dev->isoc_ctl.tmp_buf_len;
>> -					goto HEADER;
>>  				}
>> +				dev->isoc_ctl.tmp_buf_len = 0;
>> +			} else {
>> +				if (ptr + 3 >= endp) {
>> +					/* have incomplete header */
>> +					dev->isoc_ctl.tmp_buf_len = endp - ptr;
>> +					memcpy (&dev->isoc_ctl.tmp_buf, ptr,
>> +						dev->isoc_ctl.tmp_buf_len);
>> +					return rc;
>> +				}
>> +				/* Seek for sync */
>> +				for (; ptr < endp - 3; ptr++) {
>> +					if (ptr < endp - 187) {
>> +						if (*(ptr + 3) == 0x47 &&
>> +							(*(ptr + 187) == 0x47)
>> +							break;
>>     
> Hmm... are you sure you need to do this? Just checking for the current sync seems
> to be enough, except if the URB is corrupted. In the latter case, it would be better
> to do the opposite: test for the sync at either ptr or ptr + 187:
>
> 		if (*(ptr + 3) == 0x47 || (*(ptr + 187) == 0x47)
>   
Yes 0x47 can be both
1. data
2. sync byte
or will you that it sync into data? It can ignored copy data and lose
data (2 blocks)

for example:

                                    block 1                    block 2
                                 it are cmd 1            it are cmd 1
 norm.          :              header data             header data
                                |          |                       |   
    |
 0x47 is data :                            header  data
                                                    |        |
                                                  
> I don't like the above neither, but this device requires so many workarounds to work
> with a bad hardware/firmware that one more hack wouldn't hurt, but, if this is really
> needed, a proper comment explaining the reason for the hack should be added.
>
>   
>> +					} else {
>> +						if (*(ptr + 3) == 0x47)
>> +							break;
>> +					}
>> +					if (ptr + 3 >= endp)
>> +						return rc;
>>     
> Huh? This test look strange: if ptr + 3 >= endp, the loop would be end before
> calling this code. So, it seems to be a dead code.
>
>   
for example:
it goes with 6 into for ... and doesn't found a sync byte so it must
return from this function and not go out from for ... -> it's not sync
and going to lose data blocks.
>> +				}
>> +				/* Get message header */
>> +				header = *(unsigned long *)ptr;
>> +				ptr += 4;
>>  			}
>>     
>   


-- 
Stefan Ringel <stefan.ringel@arcor.de>


[-- Attachment #2: stefan_ringel.vcf --]
[-- Type: text/x-vcard, Size: 157 bytes --]

begin:vcard
fn:Stefan Ringel
n:Ringel;Stefan
email;internet:stefan.ringel@arcor.de
note:web: www.stefanringel.de
x-mozilla-html:FALSE
version:2.1
end:vcard

Re: [PATCH 5/5] tm6000: rewrite copy_streams

[Stefan Ringel]

[-- Attachment #1: Type: text/plain, Size: 9516 bytes --]

Am 27.05.2010 23:23, schrieb Mauro Carvalho Chehab:
> Em Sun, 23 May 2010 20:31:45 +0200
> stefan.ringel@arcor.de escreveu:
>
>   
>> From: Stefan Ringel <stefan.ringel@arcor.de>
>>
>> fusion function copy streams and copy_packets to new function copy_streams.
>>
>> Signed-off-by: Stefan Ringel <stefan.ringel@arcor.de>
>> ---
>>  drivers/staging/tm6000/tm6000-usb-isoc.h |    5 +-
>>  drivers/staging/tm6000/tm6000-video.c    |  337 +++++++++++-------------------
>>  2 files changed, 127 insertions(+), 215 deletions(-)
>>
>> diff --git a/drivers/staging/tm6000/tm6000-usb-isoc.h b/drivers/staging/tm6000/tm6000-usb-isoc.h
>> index 5a5049a..138716a 100644
>> --- a/drivers/staging/tm6000/tm6000-usb-isoc.h
>> +++ b/drivers/staging/tm6000/tm6000-usb-isoc.h
>> @@ -39,7 +39,7 @@ struct usb_isoc_ctl {
>>  	int				pos, size, pktsize;
>>  
>>  		/* Last field: ODD or EVEN? */
>> -	int				field;
>> +	int				vfield;
>>  
>>  		/* Stores incomplete commands */
>>  	u32				tmp_buf;
>> @@ -47,7 +47,4 @@ struct usb_isoc_ctl {
>>  
>>  		/* Stores already requested buffers */
>>  	struct tm6000_buffer    	*buf;
>> -
>> -		/* Stores the number of received fields */
>> -	int				nfields;
>>  };
>> diff --git a/drivers/staging/tm6000/tm6000-video.c b/drivers/staging/tm6000/tm6000-video.c
>> index 2a61cc3..31c574f 100644
>> --- a/drivers/staging/tm6000/tm6000-video.c
>> +++ b/drivers/staging/tm6000/tm6000-video.c
>> @@ -186,234 +186,153 @@ const char *tm6000_msg_type[] = {
>>  /*
>>   * Identify the tm5600/6000 buffer header type and properly handles
>>   */
>> -static int copy_packet(struct urb *urb, u32 header, u8 **ptr, u8 *endp,
>> -			u8 *out_p, struct tm6000_buffer **buf)
>> -{
>> -	struct tm6000_dmaqueue  *dma_q = urb->context;
>> -	struct tm6000_core *dev = container_of(dma_q, struct tm6000_core, vidq);
>> -	u8 c;
>> -	unsigned int cmd, cpysize, pktsize, size, field, block, line, pos = 0;
>> -	int rc = 0;
>> -	/* FIXME: move to tm6000-isoc */
>> -	static int last_line = -2, start_line = -2, last_field = -2;
>> -
>> -	/* FIXME: this is the hardcoded window size
>> -	 */
>> -	unsigned int linewidth = (*buf)->vb.width << 1;
>> -
>> -	if (!dev->isoc_ctl.cmd) {
>> -		c = (header >> 24) & 0xff;
>> -
>> -		/* split the header fields */
>> -		size  = ((header & 0x7e) << 1);
>> -
>> -		if (size > 0)
>> -			size -= 4;
>> -
>> -		block = (header >> 7) & 0xf;
>> -		field = (header >> 11) & 0x1;
>> -		line  = (header >> 12) & 0x1ff;
>> -		cmd   = (header >> 21) & 0x7;
>> -
>> -		/* Validates header fields */
>> -		if(size > TM6000_URB_MSG_LEN)
>> -			size = TM6000_URB_MSG_LEN;
>> -
>> -		if (cmd == TM6000_URB_MSG_VIDEO) {
>> -			if ((block+1)*TM6000_URB_MSG_LEN>linewidth)
>> -				cmd = TM6000_URB_MSG_ERR;
>> -
>> -			/* FIXME: Mounts the image as field0+field1
>> -			 * It should, instead, check if the user selected
>> -			 * entrelaced or non-entrelaced mode
>> -			 */
>> -			pos= ((line<<1)+field)*linewidth +
>> -				block*TM6000_URB_MSG_LEN;
>> -
>> -			/* Don't allow to write out of the buffer */
>> -			if (pos+TM6000_URB_MSG_LEN > (*buf)->vb.size) {
>> -				dprintk(dev, V4L2_DEBUG_ISOC,
>> -					"ERR: size=%d, num=%d, line=%d, "
>> -					"field=%d\n",
>> -					size, block, line, field);
>> -
>> -				cmd = TM6000_URB_MSG_ERR;
>> -			}
>> -		} else {
>> -			pos=0;
>> -		}
>> -
>> -		/* Prints debug info */
>> -		dprintk(dev, V4L2_DEBUG_ISOC, "size=%d, num=%d, "
>> -				" line=%d, field=%d\n",
>> -				size, block, line, field);
>> -
>> -		if ((last_line!=line)&&(last_line+1!=line) &&
>> -		    (cmd != TM6000_URB_MSG_ERR) )  {
>> -			if (cmd != TM6000_URB_MSG_VIDEO)  {
>> -				dprintk(dev, V4L2_DEBUG_ISOC,  "cmd=%d, "
>> -					"size=%d, num=%d, line=%d, field=%d\n",
>> -					cmd, size, block, line, field);
>> -			}
>> -			if (start_line<0)
>> -				start_line=last_line;
>> -			/* Prints debug info */
>> -			dprintk(dev, V4L2_DEBUG_ISOC, "lines= %d-%d, "
>> -					"field=%d\n",
>> -					start_line, last_line, field);
>> -
>> -			if ((start_line<6 && last_line>200) &&
>> -				(last_field != field) ) {
>> -
>> -				dev->isoc_ctl.nfields++;
>> -				if (dev->isoc_ctl.nfields>=2) {
>> -					dev->isoc_ctl.nfields=0;
>> -
>> -					/* Announces that a new buffer were filled */
>> -					buffer_filled (dev, dma_q, *buf);
>> -					dprintk(dev, V4L2_DEBUG_ISOC,
>> -							"new buffer filled\n");
>> -					get_next_buf (dma_q, buf);
>> -					if (!*buf)
>> -						return rc;
>> -					out_p = videobuf_to_vmalloc(&((*buf)->vb));
>> -					if (!out_p)
>> -						return rc;
>> -
>> -					pos = dev->isoc_ctl.pos = 0;
>> -				}
>> -			}
>> -
>> -			start_line=line;
>> -			last_field=field;
>> -		}
>> -		if (cmd == TM6000_URB_MSG_VIDEO)
>> -			last_line = line;
>> -
>> -		pktsize = TM6000_URB_MSG_LEN;
>> -	} else {
>> -		/* Continue the last copy */
>> -		cmd = dev->isoc_ctl.cmd;
>> -		size= dev->isoc_ctl.size;
>> -		pos = dev->isoc_ctl.pos;
>> -		pktsize = dev->isoc_ctl.pktsize;
>> -	}
>> -
>> -	cpysize = (endp-(*ptr) > size) ? size : endp - *ptr;
>> -
>> -	if (cpysize) {
>> -		/* handles each different URB message */
>> -		switch(cmd) {
>> -		case TM6000_URB_MSG_VIDEO:
>> -			/* Fills video buffer */
>> -			memcpy(&out_p[pos], *ptr, cpysize);
>> -			break;
>> -		case TM6000_URB_MSG_PTS:
>> -			break;
>> -		case TM6000_URB_MSG_AUDIO:
>> -			/* Need some code to process audio */
>> -			printk ("%ld: cmd=%s, size=%d\n", jiffies,
>> -				tm6000_msg_type[cmd],size);
>> -			break;
>> -		case TM6000_URB_MSG_VBI:
>> -			break;
>> -		default:
>> -			dprintk (dev, V4L2_DEBUG_ISOC, "cmd=%s, size=%d\n",
>> -						tm6000_msg_type[cmd],size);
>> -		}
>> -	}
>> -	if (cpysize<size) {
>> -		/* End of URB packet, but cmd processing is not
>> -		 * complete. Preserve the state for a next packet
>> -		 */
>> -		dev->isoc_ctl.pos = pos+cpysize;
>> -		dev->isoc_ctl.size= size-cpysize;
>> -		dev->isoc_ctl.cmd = cmd;
>> -		dev->isoc_ctl.pktsize = pktsize-cpysize;
>> -		(*ptr)+=cpysize;
>> -	} else {
>> -		dev->isoc_ctl.cmd = 0;
>> -		(*ptr)+=pktsize;
>> -	}
>> -
>> -	return rc;
>> -}
>> -
>>  static int copy_streams(u8 *data, unsigned long len,
>>  			struct urb *urb)
>>  {
>>  	struct tm6000_dmaqueue  *dma_q = urb->context;
>>  	struct tm6000_core *dev= container_of(dma_q,struct tm6000_core,vidq);
>> -	u8 *ptr=data, *endp=data+len;
>> +	u8 *ptr=data, *endp=data+len, c;
>>  	unsigned long header=0;
>>  	int rc=0;
>> -	struct tm6000_buffer *buf;
>> -	char *outp = NULL;
>> -
>> -	get_next_buf(dma_q, &buf);
>> -	if (buf)
>> -		outp = videobuf_to_vmalloc(&buf->vb);
>> +	unsigned int cmd, cpysize, pktsize, size, field, block, line, pos = 0;
>> +	struct tm6000_buffer *vbuf;
>> +	char *voutp = NULL;
>> +	unsigned int linewidth;
>>  
>> -	if (!outp)
>> +	/* get video buffer */
>> +	get_next_buf (dma_q, &vbuf);
>> +	if (!vbuf)
>> +		return rc;
>> +	voutp = videobuf_to_vmalloc(&vbuf->vb);
>> +	if (!voutp)
>>  		return 0;
>>  
>> -	for (ptr=data; ptr<endp;) {
>> +	for (ptr = data; ptr < endp;) {
>>  		if (!dev->isoc_ctl.cmd) {
>> -			u8 *p=(u8 *)&dev->isoc_ctl.tmp_buf;
>> -			/* FIXME: This seems very complex
>> -			 * It just recovers up to 3 bytes of the header that
>> -			 * might be at the previous packet
>> -			 */
>> -			if (dev->isoc_ctl.tmp_buf_len) {
>> -				while (dev->isoc_ctl.tmp_buf_len) {
>> -					if ( *(ptr+3-dev->isoc_ctl.tmp_buf_len) == 0x47) {
>> -						break;
>> -					}
>> -					p++;
>> -					dev->isoc_ctl.tmp_buf_len--;
>> -				}
>> -				if (dev->isoc_ctl.tmp_buf_len) {
>> -					memcpy(&header, p,
>> -						dev->isoc_ctl.tmp_buf_len);
>> -					memcpy((u8 *)&header +
>> +			/* Header */
>> +			if (dev->isoc_ctl.tmp_buf_len > 0) {
>> +				/* from last urb or packet */
>> +				header = dev->isoc_ctl.tmp_buf;
>> +				if (4 - dev->isoc_ctl.tmp_buf_len > 0)
>> +					memcpy ((u8 *)&header +
>>  						dev->isoc_ctl.tmp_buf_len,
>>  						ptr,
>>  						4 - dev->isoc_ctl.tmp_buf_len);
>>  					ptr += 4 - dev->isoc_ctl.tmp_buf_len;
>> -					goto HEADER;
>>  				}
>> +				dev->isoc_ctl.tmp_buf_len = 0;
>> +			} else {
>> +				if (ptr + 3 >= endp) {
>> +					/* have incomplete header */
>> +					dev->isoc_ctl.tmp_buf_len = endp - ptr;
>> +					memcpy (&dev->isoc_ctl.tmp_buf, ptr,
>> +						dev->isoc_ctl.tmp_buf_len);
>> +					return rc;
>> +				}
>> +				/* Seek for sync */
>> +				for (; ptr < endp - 3; ptr++) {
>> +					if (ptr < endp - 187) {
>> +						if (*(ptr + 3) == 0x47 &&
>> +							(*(ptr + 187) == 0x47)
>> +							break;
>>     
> Hmm... are you sure you need to do this? Just checking for the current sync seems
> to be enough, except if the URB is corrupted. In the latter case, it would be better
> to do the opposite: test for the sync at either ptr or ptr + 187:
>
> 		if (*(ptr + 3) == 0x47 || (*(ptr + 187) == 0x47)
>
> I don't like the above neither, but this device requires so many workarounds to work
> with a bad hardware/firmware that one more hack wouldn't hurt, but, if this is really
> needed, a proper comment explaining the reason for the hack should be added.
>
>   
>> +					} else {
>> +						if (*(ptr + 3) == 0x47)
>> +							break;
>> +					}
>> +					if (ptr + 3 >= endp)
>> +						return rc;
>>     
> Huh? This test look strange: if ptr + 3 >= endp, the loop would be end before
> calling this code. So, it seems to be a dead code.
>   
Ah, I have a idea I remove this check and move if... from before this
for... after it.
>   
>> +				}
>> +				/* Get message header */
>> +				header = *(unsigned long *)ptr;
>> +				ptr += 4;
>>  			}
>>     
>   


-- 
Stefan Ringel <stefan.ringel@arcor.de>


[-- Attachment #2: stefan_ringel.vcf --]
[-- Type: text/x-vcard, Size: 157 bytes --]

begin:vcard
fn:Stefan Ringel
n:Ringel;Stefan
email;internet:stefan.ringel@arcor.de
note:web: www.stefanringel.de
x-mozilla-html:FALSE
version:2.1
end:vcard

Re: [PATCH 5/5] tm6000: rewrite copy_streams

[Stefan Ringel]

[-- Attachment #1: Type: text/plain, Size: 9476 bytes --]

Am 27.05.2010 23:23, schrieb Mauro Carvalho Chehab:
> Em Sun, 23 May 2010 20:31:45 +0200
> stefan.ringel@arcor.de escreveu:
>
>   
>> From: Stefan Ringel <stefan.ringel@arcor.de>
>>
>> fusion function copy streams and copy_packets to new function copy_streams.
>>
>> Signed-off-by: Stefan Ringel <stefan.ringel@arcor.de>
>> ---
>>  drivers/staging/tm6000/tm6000-usb-isoc.h |    5 +-
>>  drivers/staging/tm6000/tm6000-video.c    |  337 +++++++++++-------------------
>>  2 files changed, 127 insertions(+), 215 deletions(-)
>>
>> diff --git a/drivers/staging/tm6000/tm6000-usb-isoc.h b/drivers/staging/tm6000/tm6000-usb-isoc.h
>> index 5a5049a..138716a 100644
>> --- a/drivers/staging/tm6000/tm6000-usb-isoc.h
>> +++ b/drivers/staging/tm6000/tm6000-usb-isoc.h
>> @@ -39,7 +39,7 @@ struct usb_isoc_ctl {
>>  	int				pos, size, pktsize;
>>  
>>  		/* Last field: ODD or EVEN? */
>> -	int				field;
>> +	int				vfield;
>>  
>>  		/* Stores incomplete commands */
>>  	u32				tmp_buf;
>> @@ -47,7 +47,4 @@ struct usb_isoc_ctl {
>>  
>>  		/* Stores already requested buffers */
>>  	struct tm6000_buffer    	*buf;
>> -
>> -		/* Stores the number of received fields */
>> -	int				nfields;
>>  };
>> diff --git a/drivers/staging/tm6000/tm6000-video.c b/drivers/staging/tm6000/tm6000-video.c
>> index 2a61cc3..31c574f 100644
>> --- a/drivers/staging/tm6000/tm6000-video.c
>> +++ b/drivers/staging/tm6000/tm6000-video.c
>> @@ -186,234 +186,153 @@ const char *tm6000_msg_type[] = {
>>  /*
>>   * Identify the tm5600/6000 buffer header type and properly handles
>>   */
>> -static int copy_packet(struct urb *urb, u32 header, u8 **ptr, u8 *endp,
>> -			u8 *out_p, struct tm6000_buffer **buf)
>> -{
>> -	struct tm6000_dmaqueue  *dma_q = urb->context;
>> -	struct tm6000_core *dev = container_of(dma_q, struct tm6000_core, vidq);
>> -	u8 c;
>> -	unsigned int cmd, cpysize, pktsize, size, field, block, line, pos = 0;
>> -	int rc = 0;
>> -	/* FIXME: move to tm6000-isoc */
>> -	static int last_line = -2, start_line = -2, last_field = -2;
>> -
>> -	/* FIXME: this is the hardcoded window size
>> -	 */
>> -	unsigned int linewidth = (*buf)->vb.width << 1;
>> -
>> -	if (!dev->isoc_ctl.cmd) {
>> -		c = (header >> 24) & 0xff;
>> -
>> -		/* split the header fields */
>> -		size  = ((header & 0x7e) << 1);
>> -
>> -		if (size > 0)
>> -			size -= 4;
>> -
>> -		block = (header >> 7) & 0xf;
>> -		field = (header >> 11) & 0x1;
>> -		line  = (header >> 12) & 0x1ff;
>> -		cmd   = (header >> 21) & 0x7;
>> -
>> -		/* Validates header fields */
>> -		if(size > TM6000_URB_MSG_LEN)
>> -			size = TM6000_URB_MSG_LEN;
>> -
>> -		if (cmd == TM6000_URB_MSG_VIDEO) {
>> -			if ((block+1)*TM6000_URB_MSG_LEN>linewidth)
>> -				cmd = TM6000_URB_MSG_ERR;
>> -
>> -			/* FIXME: Mounts the image as field0+field1
>> -			 * It should, instead, check if the user selected
>> -			 * entrelaced or non-entrelaced mode
>> -			 */
>> -			pos= ((line<<1)+field)*linewidth +
>> -				block*TM6000_URB_MSG_LEN;
>> -
>> -			/* Don't allow to write out of the buffer */
>> -			if (pos+TM6000_URB_MSG_LEN > (*buf)->vb.size) {
>> -				dprintk(dev, V4L2_DEBUG_ISOC,
>> -					"ERR: size=%d, num=%d, line=%d, "
>> -					"field=%d\n",
>> -					size, block, line, field);
>> -
>> -				cmd = TM6000_URB_MSG_ERR;
>> -			}
>> -		} else {
>> -			pos=0;
>> -		}
>> -
>> -		/* Prints debug info */
>> -		dprintk(dev, V4L2_DEBUG_ISOC, "size=%d, num=%d, "
>> -				" line=%d, field=%d\n",
>> -				size, block, line, field);
>> -
>> -		if ((last_line!=line)&&(last_line+1!=line) &&
>> -		    (cmd != TM6000_URB_MSG_ERR) )  {
>> -			if (cmd != TM6000_URB_MSG_VIDEO)  {
>> -				dprintk(dev, V4L2_DEBUG_ISOC,  "cmd=%d, "
>> -					"size=%d, num=%d, line=%d, field=%d\n",
>> -					cmd, size, block, line, field);
>> -			}
>> -			if (start_line<0)
>> -				start_line=last_line;
>> -			/* Prints debug info */
>> -			dprintk(dev, V4L2_DEBUG_ISOC, "lines= %d-%d, "
>> -					"field=%d\n",
>> -					start_line, last_line, field);
>> -
>> -			if ((start_line<6 && last_line>200) &&
>> -				(last_field != field) ) {
>> -
>> -				dev->isoc_ctl.nfields++;
>> -				if (dev->isoc_ctl.nfields>=2) {
>> -					dev->isoc_ctl.nfields=0;
>> -
>> -					/* Announces that a new buffer were filled */
>> -					buffer_filled (dev, dma_q, *buf);
>> -					dprintk(dev, V4L2_DEBUG_ISOC,
>> -							"new buffer filled\n");
>> -					get_next_buf (dma_q, buf);
>> -					if (!*buf)
>> -						return rc;
>> -					out_p = videobuf_to_vmalloc(&((*buf)->vb));
>> -					if (!out_p)
>> -						return rc;
>> -
>> -					pos = dev->isoc_ctl.pos = 0;
>> -				}
>> -			}
>> -
>> -			start_line=line;
>> -			last_field=field;
>> -		}
>> -		if (cmd == TM6000_URB_MSG_VIDEO)
>> -			last_line = line;
>> -
>> -		pktsize = TM6000_URB_MSG_LEN;
>> -	} else {
>> -		/* Continue the last copy */
>> -		cmd = dev->isoc_ctl.cmd;
>> -		size= dev->isoc_ctl.size;
>> -		pos = dev->isoc_ctl.pos;
>> -		pktsize = dev->isoc_ctl.pktsize;
>> -	}
>> -
>> -	cpysize = (endp-(*ptr) > size) ? size : endp - *ptr;
>> -
>> -	if (cpysize) {
>> -		/* handles each different URB message */
>> -		switch(cmd) {
>> -		case TM6000_URB_MSG_VIDEO:
>> -			/* Fills video buffer */
>> -			memcpy(&out_p[pos], *ptr, cpysize);
>> -			break;
>> -		case TM6000_URB_MSG_PTS:
>> -			break;
>> -		case TM6000_URB_MSG_AUDIO:
>> -			/* Need some code to process audio */
>> -			printk ("%ld: cmd=%s, size=%d\n", jiffies,
>> -				tm6000_msg_type[cmd],size);
>> -			break;
>> -		case TM6000_URB_MSG_VBI:
>> -			break;
>> -		default:
>> -			dprintk (dev, V4L2_DEBUG_ISOC, "cmd=%s, size=%d\n",
>> -						tm6000_msg_type[cmd],size);
>> -		}
>> -	}
>> -	if (cpysize<size) {
>> -		/* End of URB packet, but cmd processing is not
>> -		 * complete. Preserve the state for a next packet
>> -		 */
>> -		dev->isoc_ctl.pos = pos+cpysize;
>> -		dev->isoc_ctl.size= size-cpysize;
>> -		dev->isoc_ctl.cmd = cmd;
>> -		dev->isoc_ctl.pktsize = pktsize-cpysize;
>> -		(*ptr)+=cpysize;
>> -	} else {
>> -		dev->isoc_ctl.cmd = 0;
>> -		(*ptr)+=pktsize;
>> -	}
>> -
>> -	return rc;
>> -}
>> -
>>  static int copy_streams(u8 *data, unsigned long len,
>>  			struct urb *urb)
>>  {
>>  	struct tm6000_dmaqueue  *dma_q = urb->context;
>>  	struct tm6000_core *dev= container_of(dma_q,struct tm6000_core,vidq);
>> -	u8 *ptr=data, *endp=data+len;
>> +	u8 *ptr=data, *endp=data+len, c;
>>  	unsigned long header=0;
>>  	int rc=0;
>> -	struct tm6000_buffer *buf;
>> -	char *outp = NULL;
>> -
>> -	get_next_buf(dma_q, &buf);
>> -	if (buf)
>> -		outp = videobuf_to_vmalloc(&buf->vb);
>> +	unsigned int cmd, cpysize, pktsize, size, field, block, line, pos = 0;
>> +	struct tm6000_buffer *vbuf;
>> +	char *voutp = NULL;
>> +	unsigned int linewidth;
>>  
>> -	if (!outp)
>> +	/* get video buffer */
>> +	get_next_buf (dma_q, &vbuf);
>> +	if (!vbuf)
>> +		return rc;
>> +	voutp = videobuf_to_vmalloc(&vbuf->vb);
>> +	if (!voutp)
>>  		return 0;
>>  
>> -	for (ptr=data; ptr<endp;) {
>> +	for (ptr = data; ptr < endp;) {
>>  		if (!dev->isoc_ctl.cmd) {
>> -			u8 *p=(u8 *)&dev->isoc_ctl.tmp_buf;
>> -			/* FIXME: This seems very complex
>> -			 * It just recovers up to 3 bytes of the header that
>> -			 * might be at the previous packet
>> -			 */
>> -			if (dev->isoc_ctl.tmp_buf_len) {
>> -				while (dev->isoc_ctl.tmp_buf_len) {
>> -					if ( *(ptr+3-dev->isoc_ctl.tmp_buf_len) == 0x47) {
>> -						break;
>> -					}
>> -					p++;
>> -					dev->isoc_ctl.tmp_buf_len--;
>> -				}
>> -				if (dev->isoc_ctl.tmp_buf_len) {
>> -					memcpy(&header, p,
>> -						dev->isoc_ctl.tmp_buf_len);
>> -					memcpy((u8 *)&header +
>> +			/* Header */
>> +			if (dev->isoc_ctl.tmp_buf_len > 0) {
>> +				/* from last urb or packet */
>> +				header = dev->isoc_ctl.tmp_buf;
>> +				if (4 - dev->isoc_ctl.tmp_buf_len > 0)
>> +					memcpy ((u8 *)&header +
>>  						dev->isoc_ctl.tmp_buf_len,
>>  						ptr,
>>  						4 - dev->isoc_ctl.tmp_buf_len);
>>  					ptr += 4 - dev->isoc_ctl.tmp_buf_len;
>> -					goto HEADER;
>>  				}
>> +				dev->isoc_ctl.tmp_buf_len = 0;
>> +			} else {
>> +				if (ptr + 3 >= endp) {
>> +					/* have incomplete header */
>> +					dev->isoc_ctl.tmp_buf_len = endp - ptr;
>> +					memcpy (&dev->isoc_ctl.tmp_buf, ptr,
>> +						dev->isoc_ctl.tmp_buf_len);
>> +					return rc;
>> +				}
>> +				/* Seek for sync */
>> +				for (; ptr < endp - 3; ptr++) {
>> +					if (ptr < endp - 187) {
>> +						if (*(ptr + 3) == 0x47 &&
>> +							(*(ptr + 187) == 0x47)
>> +							break;
>>     
> Hmm... are you sure you need to do this? Just checking for the current sync seems
> to be enough, except if the URB is corrupted. In the latter case, it would be better
> to do the opposite: test for the sync at either ptr or ptr + 187:
>
> 		if (*(ptr + 3) == 0x47 || (*(ptr + 187) == 0x47)
>
>   
Mauro that doesn't work. It lost any blocks.
> I don't like the above neither, but this device requires so many workarounds to work
> with a bad hardware/firmware that one more hack wouldn't hurt, but, if this is really
> needed, a proper comment explaining the reason for the hack should be added.
>
>   
>> +					} else {
>> +						if (*(ptr + 3) == 0x47)
>> +							break;
>> +					}
>> +					if (ptr + 3 >= endp)
>> +						return rc;
>>     
> Huh? This test look strange: if ptr + 3 >= endp, the loop would be end before
> calling this code. So, it seems to be a dead code.
>
>   
>> +				}
>> +				/* Get message header */
>> +				header = *(unsigned long *)ptr;
>> +				ptr += 4;
>>  			}
>>     
>   


-- 
Stefan Ringel <stefan.ringel@arcor.de>


[-- Attachment #2: stefan_ringel.vcf --]
[-- Type: text/x-vcard, Size: 157 bytes --]

begin:vcard
fn:Stefan Ringel
n:Ringel;Stefan
email;internet:stefan.ringel@arcor.de
note:web: www.stefanringel.de
x-mozilla-html:FALSE
version:2.1
end:vcard