[Spca50x-devs] PATCH: gspcav2: pac207 support: gspca_main fixes
- Date: Sat, 19 Apr 2008 21:47:03 +0200
- From: Hans de Goede <j.w.r.degoede@xxxxxx>
- Subject: [Spca50x-devs] PATCH: gspcav2: pac207 support: gspca_main fixes
Hi all,
I've finally managed to make some time to work on gspcav2. I've been working on
fixing the pac207 to work and to incorporate some improvements I've added to my
own pac207 v4l2 driver (better autogain support).
During this I've hit 3 issues in gspca_main.ko from gspcav2-0.0.28, for which
I've attached the following patches, listed in order in which I encountered the
issues during development:
gspcav2-0.0.28-avoid-soft-lockup.patch:
---------------------------------------
Due to a bug in pac207.c (which I introduced while working on it) the cam
crashed, causing the entire ISOC urb flow to stop, so no more iso_irq's. This
caused gspca_frame_wait, to wait undefinitely on the waitqueue causing a
softlock up of the kernel (yeah lots of reboots to debug that one).
This patch fixes this by adding a timeout, and it also adds a check for device
disconnects and some other thread / user stopping the stream (yes this can
happen with our current design, which imho warrant a rethink, more on that later).
gspcav2-0.0.28-only-allow-one-user.patch:
-----------------------------------------
ekiga, for some unknown reason does the following:
1) open the device
2) request buffers
3) open the device again
4) close the fd returned in 3)
5) queue buffers
5) fails because the close in 4) will free all buffers / frames causing any
index passed in 5) to be invalid.
Now one may claim this is an ekiga bug, but it is not, the current
gspca_main.ko wrongly claims to support multiple opens and multiple readers
even. This clearly shows it doesn't, with multiple (possibly independent)
readers each reader will want to be in charge of the frame/buffer queue, this
will only work if there is a queue _per_ user and then some ugliness in the
isoirq handling to feed received data to all users with available buffers. But
currently we only have one queue per device, not per user. Since the v4l2 spec
does not ask for support for multiple readers, the easiest (and cleanest /
sanest) fix is to not allow multiple opens at all. This is ok with ekiga as its
fine with its step 3) failing.
gspcav2-0.0.28-frame-add-no-null-deref.patch
--------------------------------------------
gspca_frame_add() wrongly assumes (in a PDEBUG call) that it may dereference
its data argument even if the len argument is 0. Since the pac207 code does a:
gspca_frame_add(gspca_dev, FIRST_PACKET, frame, NULL, 0); on a sof (and the
drops back to its normal routine of adding INTERMEDIATE packets with actual
data), this causes a kernel panic (null deref in irq context) when debugging is
enabled. This was another fun one to debug.
With these 3 patches applied (and a heavily modified pac207.c) I now have
gspcav2 working with a itworks PCW03 (093a:2460) pac207 webcam.
I still need todo some cleanups to my pac207 code and then I'll send a patch
for that too.
Regards,
Hans
--- gspcav2-0.0.28/gspca.c 2008-04-12 11:55:47.000000000 +0200
+++ gspcav2-0.0.28.new/gspca.c 2008-04-19 21:19:42.000000000 +0200
@@ -1346,10 +1345,13 @@
/* wait till a frame is ready */
for (;;) {
- ret = wait_event_interruptible(gspca_dev->wq,
- atomic_read(&gspca_dev->nevent) > 0);
- if (ret != 0)
- return ret;
+ ret = wait_event_interruptible_timeout(gspca_dev->wq,
+ atomic_read(&gspca_dev->nevent) > 0,
+ msecs_to_jiffies(3000));
+ if (ret <= 0)
+ return (ret < 0)? ret : -EIO;
+ if (!gspca_dev->streaming || !gspca_dev->present)
+ return -EIO;
i = gspca_dev->fr_o;
j = gspca_dev->fr_queue[i];
frame = &gspca_dev->frame[j];
--- gspcav2-0.0.28/gspca.c 2008-04-12 11:55:47.000000000 +0200
+++ gspcav2-0.0.28.new/gspca.c 2008-04-19 21:19:42.000000000 +0200
@@ -172,8 +172,7 @@
{
int i, j;
- PDEBUG(D_PACK, "add t:%d l:%d %02x %02x %02x %02x...",
- packet_type, len, data[0], data[1], data[2], data[3]);
+ PDEBUG(D_PACK, "add t:%d l:%d", packet_type, len);
/* when start of a new frame, if the current frame buffer
* is not queued, discard the whole frame */
--- gspcav2-0.0.28/gspca.c 2008-04-12 11:55:47.000000000 +0200
+++ gspcav2-0.0.28.new/gspca.c 2008-04-19 21:19:42.000000000 +0200
@@ -859,7 +858,7 @@
PDEBUG(D_ERR|D_CONF, "init device failed %d", ret);
goto out;
}
- } else if (gspca_dev->users > 8) { /* (arbitrary value) */
+ } else if (gspca_dev->users >= 1) { /* should never be > 1 */
ret = -EBUSY;
goto out;
}
-------------------------------------------------------------------------
This SF.net email is sponsored by the 2008 JavaOne(SM) Conference
Don't miss this year's exciting event. There's still time to save $100.
Use priority code J8TL2D2.
http://ad.doubleclick.net/clk;198757673;13503038;p?http://java.sun.com/javaone
_______________________________________________
Spca50x-devs mailing list
Spca50x-devs@xxxxxxxxxxxxxxxxxxxxx
https://lists.sourceforge.net/lists/listinfo/spca50x-devs