[media] gspca: Don't set gspca_dev->dev to NULL before stop0

In commit a3d6e8cc0e gspca_dev->dev is set
to NULL on disconnect, before calling stop0. The plan was to get rid of
gspca_dev->present and instead simply check for gspca_dev->dev everywhere
where we were checking for present. This should be race free since all users
of gspca_dev->dev hold the usb_lock, or so I thought.

But I was wrong, drivers which use a work-queue + synchronous bulk transfers
to get the video data don't hold the usb_lock while doing so, their stop0
callbacks stop the workqueue, so they won't be using gspca_dev->dev anymore
after the stop0 call, but they might be dereferincing it before, so we should
not set gspca_dev->dev to NULL on disconnect before calling stop0.

This also means that the workqueue functions in these drivers cannot
use gspca_dev->dev to check if they need to stop because of disconnection,
so we will need to keep gspca_dev->present around, and set that to 0 on
disconnect, before calling stop0. Unfortunately as part of the plan to remove
gspca_dev->present, these workqueues where already moved over to checking
for gspca_dev->dev instead of gspca_dev->present as part of commit
254902b01d, so this patch also reverts those
parts of that commit.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab@redhat.com>
This commit is contained in:
Hans de Goede 2012-09-09 06:30:02 -03:00 committed by Mauro Carvalho Chehab
parent 8c96f0a207
commit 345321dc9c
8 changed files with 19 additions and 22 deletions

View File

@ -94,7 +94,7 @@ static void dostream(struct work_struct *work)
/* loop reading a frame */ /* loop reading a frame */
again: again:
while (gspca_dev->dev && gspca_dev->streaming) { while (gspca_dev->present && gspca_dev->streaming) {
#ifdef CONFIG_PM #ifdef CONFIG_PM
if (gspca_dev->frozen) if (gspca_dev->frozen)
break; break;
@ -110,7 +110,7 @@ static void dostream(struct work_struct *work)
if (gspca_dev->frozen) if (gspca_dev->frozen)
break; break;
#endif #endif
if (!gspca_dev->dev || !gspca_dev->streaming) if (!gspca_dev->present || !gspca_dev->streaming)
break; break;
/* the frame comes in parts */ /* the frame comes in parts */
@ -129,7 +129,7 @@ static void dostream(struct work_struct *work)
if (gspca_dev->frozen) if (gspca_dev->frozen)
goto out; goto out;
#endif #endif
if (!gspca_dev->dev || !gspca_dev->streaming) if (!gspca_dev->present || !gspca_dev->streaming)
goto out; goto out;
if (len < FPIX_MAX_TRANSFER || if (len < FPIX_MAX_TRANSFER ||
(data[len - 2] == 0xff && (data[len - 2] == 0xff &&

View File

@ -2358,8 +2358,6 @@ void gspca_disconnect(struct usb_interface *intf)
mutex_lock(&gspca_dev->usb_lock); mutex_lock(&gspca_dev->usb_lock);
usb_set_intfdata(intf, NULL);
gspca_dev->dev = NULL;
gspca_dev->present = 0; gspca_dev->present = 0;
destroy_urbs(gspca_dev); destroy_urbs(gspca_dev);
@ -2375,6 +2373,7 @@ void gspca_disconnect(struct usb_interface *intf)
if (gspca_dev->sd_desc->stop0 && gspca_dev->streaming) if (gspca_dev->sd_desc->stop0 && gspca_dev->streaming)
gspca_dev->sd_desc->stop0(gspca_dev); gspca_dev->sd_desc->stop0(gspca_dev);
gspca_dev->streaming = 0; gspca_dev->streaming = 0;
gspca_dev->dev = NULL;
wake_up_interruptible(&gspca_dev->wq); wake_up_interruptible(&gspca_dev->wq);
v4l2_device_disconnect(&gspca_dev->v4l2_dev); v4l2_device_disconnect(&gspca_dev->v4l2_dev);

View File

@ -335,7 +335,7 @@ static void jl2005c_dostream(struct work_struct *work)
goto quit_stream; goto quit_stream;
} }
while (gspca_dev->dev && gspca_dev->streaming) { while (gspca_dev->present && gspca_dev->streaming) {
#ifdef CONFIG_PM #ifdef CONFIG_PM
if (gspca_dev->frozen) if (gspca_dev->frozen)
break; break;
@ -371,7 +371,7 @@ static void jl2005c_dostream(struct work_struct *work)
buffer, act_len); buffer, act_len);
header_read = 1; header_read = 1;
} }
while (bytes_left > 0 && gspca_dev->dev) { while (bytes_left > 0 && gspca_dev->present) {
data_len = bytes_left > JL2005C_MAX_TRANSFER ? data_len = bytes_left > JL2005C_MAX_TRANSFER ?
JL2005C_MAX_TRANSFER : bytes_left; JL2005C_MAX_TRANSFER : bytes_left;
ret = usb_bulk_msg(gspca_dev->dev, ret = usb_bulk_msg(gspca_dev->dev,
@ -394,7 +394,7 @@ static void jl2005c_dostream(struct work_struct *work)
} }
} }
quit_stream: quit_stream:
if (gspca_dev->dev) { if (gspca_dev->present) {
mutex_lock(&gspca_dev->usb_lock); mutex_lock(&gspca_dev->usb_lock);
jl2005c_stop(gspca_dev); jl2005c_stop(gspca_dev);
mutex_unlock(&gspca_dev->usb_lock); mutex_unlock(&gspca_dev->usb_lock);

View File

@ -232,7 +232,7 @@ static void sq905_dostream(struct work_struct *work)
frame_sz = gspca_dev->cam.cam_mode[gspca_dev->curr_mode].sizeimage frame_sz = gspca_dev->cam.cam_mode[gspca_dev->curr_mode].sizeimage
+ FRAME_HEADER_LEN; + FRAME_HEADER_LEN;
while (gspca_dev->dev && gspca_dev->streaming) { while (gspca_dev->present && gspca_dev->streaming) {
#ifdef CONFIG_PM #ifdef CONFIG_PM
if (gspca_dev->frozen) if (gspca_dev->frozen)
break; break;
@ -246,7 +246,7 @@ static void sq905_dostream(struct work_struct *work)
we must finish reading an entire frame, otherwise the we must finish reading an entire frame, otherwise the
next time we stream we start reading in the middle of a next time we stream we start reading in the middle of a
frame. */ frame. */
while (bytes_left > 0 && gspca_dev->dev) { while (bytes_left > 0 && gspca_dev->present) {
data_len = bytes_left > SQ905_MAX_TRANSFER ? data_len = bytes_left > SQ905_MAX_TRANSFER ?
SQ905_MAX_TRANSFER : bytes_left; SQ905_MAX_TRANSFER : bytes_left;
ret = sq905_read_data(gspca_dev, buffer, data_len, 1); ret = sq905_read_data(gspca_dev, buffer, data_len, 1);
@ -278,7 +278,7 @@ static void sq905_dostream(struct work_struct *work)
gspca_frame_add(gspca_dev, LAST_PACKET, gspca_frame_add(gspca_dev, LAST_PACKET,
NULL, 0); NULL, 0);
} }
if (gspca_dev->dev) { if (gspca_dev->present) {
/* acknowledge the frame */ /* acknowledge the frame */
mutex_lock(&gspca_dev->usb_lock); mutex_lock(&gspca_dev->usb_lock);
ret = sq905_ack_frame(gspca_dev); ret = sq905_ack_frame(gspca_dev);
@ -288,7 +288,7 @@ static void sq905_dostream(struct work_struct *work)
} }
} }
quit_stream: quit_stream:
if (gspca_dev->dev) { if (gspca_dev->present) {
mutex_lock(&gspca_dev->usb_lock); mutex_lock(&gspca_dev->usb_lock);
sq905_command(gspca_dev, SQ905_CLEAR); sq905_command(gspca_dev, SQ905_CLEAR);
mutex_unlock(&gspca_dev->usb_lock); mutex_unlock(&gspca_dev->usb_lock);

View File

@ -150,7 +150,7 @@ static void sq905c_dostream(struct work_struct *work)
goto quit_stream; goto quit_stream;
} }
while (gspca_dev->dev && gspca_dev->streaming) { while (gspca_dev->present && gspca_dev->streaming) {
#ifdef CONFIG_PM #ifdef CONFIG_PM
if (gspca_dev->frozen) if (gspca_dev->frozen)
break; break;
@ -173,7 +173,7 @@ static void sq905c_dostream(struct work_struct *work)
packet_type = FIRST_PACKET; packet_type = FIRST_PACKET;
gspca_frame_add(gspca_dev, packet_type, gspca_frame_add(gspca_dev, packet_type,
buffer, FRAME_HEADER_LEN); buffer, FRAME_HEADER_LEN);
while (bytes_left > 0 && gspca_dev->dev) { while (bytes_left > 0 && gspca_dev->present) {
data_len = bytes_left > SQ905C_MAX_TRANSFER ? data_len = bytes_left > SQ905C_MAX_TRANSFER ?
SQ905C_MAX_TRANSFER : bytes_left; SQ905C_MAX_TRANSFER : bytes_left;
ret = usb_bulk_msg(gspca_dev->dev, ret = usb_bulk_msg(gspca_dev->dev,
@ -195,7 +195,7 @@ static void sq905c_dostream(struct work_struct *work)
} }
} }
quit_stream: quit_stream:
if (gspca_dev->dev) { if (gspca_dev->present) {
mutex_lock(&gspca_dev->usb_lock); mutex_lock(&gspca_dev->usb_lock);
sq905c_command(gspca_dev, SQ905C_CLEAR, 0); sq905c_command(gspca_dev, SQ905C_CLEAR, 0);
mutex_unlock(&gspca_dev->usb_lock); mutex_unlock(&gspca_dev->usb_lock);

View File

@ -194,7 +194,7 @@ static void vicam_dostream(struct work_struct *work)
goto exit; goto exit;
} }
while (gspca_dev->dev && gspca_dev->streaming) { while (gspca_dev->present && gspca_dev->streaming) {
#ifdef CONFIG_PM #ifdef CONFIG_PM
if (gspca_dev->frozen) if (gspca_dev->frozen)
break; break;
@ -299,7 +299,7 @@ static void sd_stop0(struct gspca_dev *gspca_dev)
dev->work_thread = NULL; dev->work_thread = NULL;
mutex_lock(&gspca_dev->usb_lock); mutex_lock(&gspca_dev->usb_lock);
if (gspca_dev->dev) if (gspca_dev->present)
vicam_set_camera_power(gspca_dev, 0); vicam_set_camera_power(gspca_dev, 0);
} }

View File

@ -2697,9 +2697,7 @@ static void sd_stop0(struct gspca_dev *gspca_dev)
{ {
struct sd *sd = (struct sd *) gspca_dev; struct sd *sd = (struct sd *) gspca_dev;
/* We cannot use gspca_dev->present here as that is not set when if (!gspca_dev->present)
sd_init gets called and we get called from sd_init */
if (!gspca_dev->dev)
return; return;
switch (sd->model) { switch (sd->model) {

View File

@ -5950,7 +5950,7 @@ static void transfer_update(struct work_struct *work)
if (gspca_dev->frozen) if (gspca_dev->frozen)
goto err; goto err;
#endif #endif
if (!gspca_dev->dev || !gspca_dev->streaming) if (!gspca_dev->present || !gspca_dev->streaming)
goto err; goto err;
/* Bit 0 of register 11 indicates FIFO overflow */ /* Bit 0 of register 11 indicates FIFO overflow */
@ -6842,7 +6842,7 @@ static void sd_stop0(struct gspca_dev *gspca_dev)
mutex_lock(&gspca_dev->usb_lock); mutex_lock(&gspca_dev->usb_lock);
sd->work_thread = NULL; sd->work_thread = NULL;
} }
if (!gspca_dev->dev) if (!gspca_dev->present)
return; return;
send_unknown(gspca_dev, sd->sensor); send_unknown(gspca_dev, sd->sensor);
} }