Re: [PATCH] zr36067: Debugging cleanups (updated again)

Hi Trent,

On May 23, 2007, at 11:08 PM, Trent Piepho wrote:
> I've attached my patch to this email.

That doesn't look right. You should do it in this order:
1) make sure we're the actively capturing process. If not, error out.  
Only when you've confirmed that we're the actively capturing process  
and zr-> == fh->, then can you access the zr-> values. Also, you  
appear to poll_wait() if the buf is already DONE:

+		/* Buffer ready to DQBUF? */
+		if (zr->v4l_buffers.buffer[frame].state == BUZ_STATE_DONE)
+			res = POLLIN | POLLRDNORM;
[..]
+		if (res != POLLNVAL)
+			poll_wait(file, &zr->v4l_capq, wait);

That's not correct (I think?). I think the correct sequenc would be  
to keep the old structure, and then do something like this:

if (.. ZORAN_FREE) {
   res = POLLNVAL;
} else {
   if (!DONE)
     poll_wait();
   if (.. ZORAN_DONE)
     res = POLLIN|RDNORM;
}

The only thing that you need to change in the original code is this  
(quasi-patch):

		queue = &zr->v4l_capq;
		frame = zr->v4l_pend[zr->v4l_pend_tail & V4L_MASK_FRAME];
+              if (fh->/zr->v4l_buffers.buffer[frame].state !=  
BUZ_STATE_DONE)
-		poll_wait(file, queue, wait);
+			poll_wait(file, queue, wait);
		if (fh->v4l_buffers.buffer[frame].state == BUZ_STATE_DONE)
?                ^^^^ change to zr->
			res = POLLIN | POLLRDNORM;

Does that make sense?

>> As for the patches, the 0x644 modes look weird to me, since most  
>> of them
>> are copied to the zoran structs on startup and never looked at again.
>
> Many of them are like that, but not all.  I checked how each  
> parameter was
> used, and only set 0644 for those which can be changed and take  
> effect.
> For instance lock_norm is checked every time S_STD is called,  
> v4l_bufsize
> is used on every open, pass_through takes effect on every device  
> close,
> etc.  Is there any option in paticular you think is wrong?

Not specifically, but I guess you're right here, I only glanced  
quickly. Looks ok.

> Not exactly, it says "x contains a bit in the set y and contains no  
> bits
> which are not in the set y." You're assuming y is a single bit,  
> which is
> not the case.  For example, V4L2_STD_PAL is not a single bit, it is  
> a mask
> of multiple bits for all the different PAL standards.  The same for  
> NTSC
> and SECAM.  tvtime will request the standard NTSC-M and the zr36067  
> driver
> will reject it.

Okies, patch is fine then. Didn't know that. :-).

Ronald



-------------------------------------------------------------------------
This SF.net email is sponsored by DB2 Express
Download DB2 Express C - the FREE version of DB2 express and take
control of your XML. No limits. Just data. Click to get it now.
http://sourceforge.net/powerbar/db2/