| Date: Mon, 28 Mar 2005 10:51:23 +0200
| From: Michiel <[EMAIL PROTECTED]>
| I then find these messages in /var/log/messages:
| Mar 28 09:55:59 mediabox kernel: ivtv: 1000 ms time out waiting for firmware
Note: I don't see how the bugs I am about to describe cause the
problem described in the original posting.
This diagnostic is coming from function ivtv_api_getresult in
ivtv-mailbox.c. I think that the diagnostic is inaccurate. As you
will see, I think that the driver actually waited 10000ms -- ten times
as long as was reported.
At the bottom of this message, you will see the definition of
ivtv_api_getresult, extracted from ivtv-0.3.2p, with line numbers.
When and if the loop (lines 14 through 31) is being executed for the
api_timeout + 1st time, a diagnostic is printed by line 19. This
would be after waiting for api_timeout / 100 seconds or api_timeout *
10 ms. Yet the diagnostic says api_timeout ms. So the diagnostic is
wrong by a factor of ten.
==> BUG 1: the time printed in the diagnostic is 10 times too small.
An ugly but correct fix would be print api_timeout * 10 rather than
just api_timeout (change line 21). Why is this ugly? Because it is
not obvious how this 10 (and the one in line 16) relates to the 100 in
line 28. Good coding style would create a manifest constant and use
it these three places to tie these uses together.
BIGGER QUESTION: what timeout value was intended? The diagnostic
quoted above says 1000 ms, but should have said 10000 ms. Could it be
that the calling code expects the timeout to actually be the smaller
value? This seems quite likely to me. One approach to accomplish
this would be to have the counter incremented by 10 each time through
the loop, not just by 1. The idea is that counter would then count
milliseconds. The hard-wired 10 would no longer belong in line line
21.
==> possible BUG 2: the routine waits to timeout 10 times longer than
the callers expect.
Nit picking: it is conventional to write "10ms" not "10 ms". I
recommend getting rid of that space from the two formats (lines 16 and
20).
Small bug: lines 15-16 print a message about waiting even the loop is
about to give up and not wait. I think that lines 15-16 should be
moved after line 23 to fix this.
==> BUG 3: message in lines 15-16 sometimes printed when not true.
As usual, my comments come from reading the source code. I haven't
been able to run it yet. You should check if what I say seems
correct.
Hugh Redelmeier
[EMAIL PROTECTED] voice: +1 416 482-8253
1 static int ivtv_api_getresult(struct ivtv_mailbox *mbox, u32 * result,
2 u32 data[], int api_timeout, int cmd)
3 {
4 u32 readdata;
5 int count = 0;
6
7 if (NULL == mbox) {
8 IVTV_DEBUG(IVTV_DEBUG_ERR, "invalid api mailbox\n");
9 return -ENODEV;
10 }
11
12 readdata = ivtv_read_reg((unsigned char *)&mbox->flags);
13
14 while (!(readdata & IVTV_MBOX_FIRMWARE_DONE)) {
15 IVTV_DEBUG(IVTV_DEBUG_API,
16 "[%d]result not ready, waiting 10 ms\n",
count);
17
18 if (count++ > api_timeout) {
19 IVTV_DEBUG(IVTV_DEBUG_ERR,
20 "%d ms time out waiting for
firmware\n",
21 api_timeout);
22 return -EBUSY;
23 }
24
25 /* we want to finish this api call and not break for
26 any pending signals. */
27 set_current_state(TASK_UNINTERRUPTIBLE);
28 schedule_timeout(HZ / 100);
29
30 readdata = ivtv_read_reg((unsigned char *)&mbox->flags);
31 }
32
33 *result = ivtv_read_reg((unsigned char *)&mbox->retval);
34
35 /* Failed, data must be bad */
36 if (ivtv_read_reg((unsigned char *)&mbox->retval) != 0x00)
37 return -EBUSY;
38
39 for (count = 0; count < IVTV_MBOX_MAX_DATA; count++)
40 data[count] =
41 ivtv_read_reg((unsigned char *)&mbox->data[count]);
42
43 return 0;
44 }
-------------------------------------------------------
SF email is sponsored by - The IT Product Guide
Read honest & candid reviews on hundreds of IT Products from real users.
Discover which products truly live up to the hype. Start reading now.
http://ads.osdn.com/?ad_id=6595&alloc_id=14396&op=click
_______________________________________________
ivtv-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/ivtv-devel
