I was testing under qemu, which doesn't support isochroneous
transfers. It crashed and pointed out a bug and here is the fix.
1st patch
When urb submit fails, m5602_start_transfer failed to set
cam->isoc_in_buffer to NULL and caused a double free in
m5602_stop_transfer.
2nd patch
m5602_release_buffers, redid the error out logic.
When a usb_submit_urb fails it will now kill any succsfully submitted
URBs, previously it wouldn't kill any. I'm not sure if this is
required, but I assume it is.
The previous logic is more efficient by jumping to and only freeing
the required buffers, but it is an error condition and wasn't ever
being hit (or at least not without crashing in the second open, see
first patch) so it might as well have one simpler loop and iterator
over all buffers.
m5602.c, set cam->control_buffer to NULL when freed
m5602_v4l2.c, set cam->canvas, cam->isoc_in_buffer[i] to NULL when freed
Signed-off-by: David Fries <[EMAIL PROTECTED]>
---
branches/m5602-s5k83a/m5602_v4l2.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/branches/m5602-s5k83a/m5602_v4l2.c
b/branches/m5602-s5k83a/m5602_v4l2.c
index cc90392..dec6f8e 100644
--- a/branches/m5602-s5k83a/m5602_v4l2.c
+++ b/branches/m5602-s5k83a/m5602_v4l2.c
@@ -509,7 +509,7 @@ free_urbs:
free_buffers:
for (i = 0; (i < M5602_URBS) && cam->isoc_in_buffer[i]; i++) {
kfree(cam->isoc_in_buffer[i]);
- cam->urb[i] = NULL;
+ cam->isoc_in_buffer[i] = NULL;
}
PDEBUG(DBG_V4L2, "failed to start transfer");
--
1.4.4.4
---
branches/m5602-s5k83a/m5602.c | 1 +
branches/m5602-s5k83a/m5602_v4l2.c | 18 ++++++++++--------
2 files changed, 11 insertions(+), 8 deletions(-)
diff --git a/branches/m5602-s5k83a/m5602.c b/branches/m5602-s5k83a/m5602.c
index 275c820..a617050 100644
--- a/branches/m5602-s5k83a/m5602.c
+++ b/branches/m5602-s5k83a/m5602.c
@@ -339,6 +339,7 @@ static void m5602_release_resources(struct m5602_camera
*cam)
/* up(&m5602_sysfs_lock); */
kfree(cam->control_buffer);
+ cam->control_buffer = NULL;
}
struct file_operations v4l_m5602_fops = {
diff --git a/branches/m5602-s5k83a/m5602_v4l2.c
b/branches/m5602-s5k83a/m5602_v4l2.c
index dec6f8e..cd884bf 100644
--- a/branches/m5602-s5k83a/m5602_v4l2.c
+++ b/branches/m5602-s5k83a/m5602_v4l2.c
@@ -166,6 +166,7 @@ static void m5602_release_buffers(struct m5602_camera *cam)
/* Free the canvas */
kfree(cam->canvas);
+ cam->canvas = NULL;
mutex_unlock(&cam->canvas_mutex);
@@ -449,7 +450,7 @@ static int m5602_start_transfer(struct m5602_camera *cam)
if (!urb) {
PDEBUG(DBG_DATA, "No enough memory for URB");
retval = -ENOMEM;
- goto free_urbs;
+ goto free_buffers;
}
cam->urb[i] = urb;
@@ -474,7 +475,7 @@ static int m5602_start_transfer(struct m5602_camera *cam)
retval = usb_set_interface(cam->udev, 0, 2);
if (retval)
- goto free_urbs;
+ goto free_buffers;
for (i = 0; i < M5602_URBS; i++) {
if (cam->urb[i]) {
@@ -482,7 +483,10 @@ static int m5602_start_transfer(struct m5602_camera *cam)
if (retval) {
PDEBUG(DBG_V4L2, "usb_submit_urb failed! %d",
retval);
- goto free_urbs;
+ /* Start killing the previous URB. */
+ for (i--; i>=0; i--)
+ usb_kill_urb(cam->urb[i]);
+ goto free_buffers;
}
}
}
@@ -500,14 +504,11 @@ static int m5602_start_transfer(struct m5602_camera *cam)
return retval;
-free_urbs:
- for (i = 0; (i < M5602_URBS) && cam->urb[i]; i++) {
+free_buffers:
+ for (i = 0; i < M5602_URBS; i++) {
usb_free_urb(cam->urb[i]);
cam->urb[i] = NULL;
- }
-free_buffers:
- for (i = 0; (i < M5602_URBS) && cam->isoc_in_buffer[i]; i++) {
kfree(cam->isoc_in_buffer[i]);
cam->isoc_in_buffer[i] = NULL;
}
@@ -534,6 +535,7 @@ static int m5602_stop_transfer(struct m5602_camera *cam)
}
kfree(cam->isoc_in_buffer[i]);
+ cam->isoc_in_buffer[i] = NULL;
}
PDEBUG(DBG_V4L2, "isochronous buffers and urbs freed");
--
1.4.4.4
--
David Fries <[EMAIL PROTECTED]>
http://fries.net/~david/ (PGP encryption key available)
-------------------------------------------------------------------------
Sponsored by: SourceForge.net Community Choice Awards: VOTE NOW!
Studies have shown that voting for your favorite open source project,
along with a healthy diet, reduces your potential for chronic lameness
and boredom. Vote Now at http://www.sourceforge.net/community/cca08
_______________________________________________
M560x-driver-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/m560x-driver-devel
