From 345321dc9c52b774f42c934339f9b3e2f0a39395 Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Sun, 9 Sep 2012 06:30:02 -0300 Subject: [PATCH] [media] gspca: Don't set gspca_dev->dev to NULL before stop0 In commit a3d6e8cc0e6ddc8b3cfdeb3c979f07ed1aa528b3 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 254902b01d2acc6aced99ec17caa4c6cd890cdea, so this patch also reverts those parts of that commit. Signed-off-by: Hans de Goede Signed-off-by: Mauro Carvalho Chehab --- drivers/media/usb/gspca/finepix.c | 6 +++--- drivers/media/usb/gspca/gspca.c | 3 +-- drivers/media/usb/gspca/jl2005bcd.c | 6 +++--- drivers/media/usb/gspca/sq905.c | 8 ++++---- drivers/media/usb/gspca/sq905c.c | 6 +++--- drivers/media/usb/gspca/vicam.c | 4 ++-- drivers/media/usb/gspca/xirlink_cit.c | 4 +--- drivers/media/usb/gspca/zc3xx.c | 4 ++-- 8 files changed, 19 insertions(+), 22 deletions(-) diff --git a/drivers/media/usb/gspca/finepix.c b/drivers/media/usb/gspca/finepix.c index c8f2201cc35a..04807eee7772 100644 --- a/drivers/media/usb/gspca/finepix.c +++ b/drivers/media/usb/gspca/finepix.c @@ -94,7 +94,7 @@ static void dostream(struct work_struct *work) /* loop reading a frame */ again: - while (gspca_dev->dev && gspca_dev->streaming) { + while (gspca_dev->present && gspca_dev->streaming) { #ifdef CONFIG_PM if (gspca_dev->frozen) break; @@ -110,7 +110,7 @@ again: if (gspca_dev->frozen) break; #endif - if (!gspca_dev->dev || !gspca_dev->streaming) + if (!gspca_dev->present || !gspca_dev->streaming) break; /* the frame comes in parts */ @@ -129,7 +129,7 @@ again: if (gspca_dev->frozen) goto out; #endif - if (!gspca_dev->dev || !gspca_dev->streaming) + if (!gspca_dev->present || !gspca_dev->streaming) goto out; if (len < FPIX_MAX_TRANSFER || (data[len - 2] == 0xff && diff --git a/drivers/media/usb/gspca/gspca.c b/drivers/media/usb/gspca/gspca.c index d4e8343f5b10..7cce0f201d70 100644 --- a/drivers/media/usb/gspca/gspca.c +++ b/drivers/media/usb/gspca/gspca.c @@ -2358,8 +2358,6 @@ void gspca_disconnect(struct usb_interface *intf) mutex_lock(&gspca_dev->usb_lock); - usb_set_intfdata(intf, NULL); - gspca_dev->dev = NULL; gspca_dev->present = 0; destroy_urbs(gspca_dev); @@ -2375,6 +2373,7 @@ void gspca_disconnect(struct usb_interface *intf) if (gspca_dev->sd_desc->stop0 && gspca_dev->streaming) gspca_dev->sd_desc->stop0(gspca_dev); gspca_dev->streaming = 0; + gspca_dev->dev = NULL; wake_up_interruptible(&gspca_dev->wq); v4l2_device_disconnect(&gspca_dev->v4l2_dev); diff --git a/drivers/media/usb/gspca/jl2005bcd.c b/drivers/media/usb/gspca/jl2005bcd.c index 234777116e5f..c4b4a9598db4 100644 --- a/drivers/media/usb/gspca/jl2005bcd.c +++ b/drivers/media/usb/gspca/jl2005bcd.c @@ -335,7 +335,7 @@ static void jl2005c_dostream(struct work_struct *work) goto quit_stream; } - while (gspca_dev->dev && gspca_dev->streaming) { + while (gspca_dev->present && gspca_dev->streaming) { #ifdef CONFIG_PM if (gspca_dev->frozen) break; @@ -371,7 +371,7 @@ static void jl2005c_dostream(struct work_struct *work) buffer, act_len); header_read = 1; } - while (bytes_left > 0 && gspca_dev->dev) { + while (bytes_left > 0 && gspca_dev->present) { data_len = bytes_left > JL2005C_MAX_TRANSFER ? JL2005C_MAX_TRANSFER : bytes_left; ret = usb_bulk_msg(gspca_dev->dev, @@ -394,7 +394,7 @@ static void jl2005c_dostream(struct work_struct *work) } } quit_stream: - if (gspca_dev->dev) { + if (gspca_dev->present) { mutex_lock(&gspca_dev->usb_lock); jl2005c_stop(gspca_dev); mutex_unlock(&gspca_dev->usb_lock); diff --git a/drivers/media/usb/gspca/sq905.c b/drivers/media/usb/gspca/sq905.c index a8ac97931ad6..2e05acab304c 100644 --- a/drivers/media/usb/gspca/sq905.c +++ b/drivers/media/usb/gspca/sq905.c @@ -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_HEADER_LEN; - while (gspca_dev->dev && gspca_dev->streaming) { + while (gspca_dev->present && gspca_dev->streaming) { #ifdef CONFIG_PM if (gspca_dev->frozen) break; @@ -246,7 +246,7 @@ static void sq905_dostream(struct work_struct *work) we must finish reading an entire frame, otherwise the next time we stream we start reading in the middle of a frame. */ - while (bytes_left > 0 && gspca_dev->dev) { + while (bytes_left > 0 && gspca_dev->present) { data_len = bytes_left > SQ905_MAX_TRANSFER ? SQ905_MAX_TRANSFER : bytes_left; 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, NULL, 0); } - if (gspca_dev->dev) { + if (gspca_dev->present) { /* acknowledge the frame */ mutex_lock(&gspca_dev->usb_lock); ret = sq905_ack_frame(gspca_dev); @@ -288,7 +288,7 @@ static void sq905_dostream(struct work_struct *work) } } quit_stream: - if (gspca_dev->dev) { + if (gspca_dev->present) { mutex_lock(&gspca_dev->usb_lock); sq905_command(gspca_dev, SQ905_CLEAR); mutex_unlock(&gspca_dev->usb_lock); diff --git a/drivers/media/usb/gspca/sq905c.c b/drivers/media/usb/gspca/sq905c.c index 70fae6982e96..784620c102b1 100644 --- a/drivers/media/usb/gspca/sq905c.c +++ b/drivers/media/usb/gspca/sq905c.c @@ -150,7 +150,7 @@ static void sq905c_dostream(struct work_struct *work) goto quit_stream; } - while (gspca_dev->dev && gspca_dev->streaming) { + while (gspca_dev->present && gspca_dev->streaming) { #ifdef CONFIG_PM if (gspca_dev->frozen) break; @@ -173,7 +173,7 @@ static void sq905c_dostream(struct work_struct *work) packet_type = FIRST_PACKET; gspca_frame_add(gspca_dev, packet_type, 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 ? SQ905C_MAX_TRANSFER : bytes_left; ret = usb_bulk_msg(gspca_dev->dev, @@ -195,7 +195,7 @@ static void sq905c_dostream(struct work_struct *work) } } quit_stream: - if (gspca_dev->dev) { + if (gspca_dev->present) { mutex_lock(&gspca_dev->usb_lock); sq905c_command(gspca_dev, SQ905C_CLEAR, 0); mutex_unlock(&gspca_dev->usb_lock); diff --git a/drivers/media/usb/gspca/vicam.c b/drivers/media/usb/gspca/vicam.c index b1a64b912666..57d88f70c399 100644 --- a/drivers/media/usb/gspca/vicam.c +++ b/drivers/media/usb/gspca/vicam.c @@ -194,7 +194,7 @@ static void vicam_dostream(struct work_struct *work) goto exit; } - while (gspca_dev->dev && gspca_dev->streaming) { + while (gspca_dev->present && gspca_dev->streaming) { #ifdef CONFIG_PM if (gspca_dev->frozen) break; @@ -299,7 +299,7 @@ static void sd_stop0(struct gspca_dev *gspca_dev) dev->work_thread = NULL; mutex_lock(&gspca_dev->usb_lock); - if (gspca_dev->dev) + if (gspca_dev->present) vicam_set_camera_power(gspca_dev, 0); } diff --git a/drivers/media/usb/gspca/xirlink_cit.c b/drivers/media/usb/gspca/xirlink_cit.c index 13b8d395d210..d4b23c9bf90c 100644 --- a/drivers/media/usb/gspca/xirlink_cit.c +++ b/drivers/media/usb/gspca/xirlink_cit.c @@ -2697,9 +2697,7 @@ static void sd_stop0(struct gspca_dev *gspca_dev) { struct sd *sd = (struct sd *) gspca_dev; - /* We cannot use gspca_dev->present here as that is not set when - sd_init gets called and we get called from sd_init */ - if (!gspca_dev->dev) + if (!gspca_dev->present) return; switch (sd->model) { diff --git a/drivers/media/usb/gspca/zc3xx.c b/drivers/media/usb/gspca/zc3xx.c index f0bacee33ef9..234d9eaa8eea 100644 --- a/drivers/media/usb/gspca/zc3xx.c +++ b/drivers/media/usb/gspca/zc3xx.c @@ -5950,7 +5950,7 @@ static void transfer_update(struct work_struct *work) if (gspca_dev->frozen) goto err; #endif - if (!gspca_dev->dev || !gspca_dev->streaming) + if (!gspca_dev->present || !gspca_dev->streaming) goto err; /* 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); sd->work_thread = NULL; } - if (!gspca_dev->dev) + if (!gspca_dev->present) return; send_unknown(gspca_dev, sd->sensor); } -- 2.39.5