[media] gspca: Don't set gspca_dev->dev to NULL before stop0
authorHans de Goede <hdegoede@redhat.com>
Sun, 9 Sep 2012 09:30:02 +0000 (06:30 -0300)
committerMauro Carvalho Chehab <mchehab@redhat.com>
Thu, 13 Sep 2012 20:42:17 +0000 (17:42 -0300)
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 <hdegoede@redhat.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab@redhat.com>
drivers/media/usb/gspca/finepix.c
drivers/media/usb/gspca/gspca.c
drivers/media/usb/gspca/jl2005bcd.c
drivers/media/usb/gspca/sq905.c
drivers/media/usb/gspca/sq905c.c
drivers/media/usb/gspca/vicam.c
drivers/media/usb/gspca/xirlink_cit.c
drivers/media/usb/gspca/zc3xx.c

index c8f2201cc35ad96403230150b999363a7f362a93..04807eee7772a0e9f9a5136a5d6425fcea016a83 100644 (file)
@@ -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 &&
index d4e8343f5b1013b17b1d9541bbaf287c193a6b48..7cce0f201d7001d23555811d460de028438ac0c1 100644 (file)
@@ -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);
index 234777116e5fc5e8cc25a6f4e1940feca8a31412..c4b4a9598db46b62c0ae7b9ca14f743cdebcab07 100644 (file)
@@ -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);
index a8ac97931ad68abe6986ab19fc959bc2596e0d5a..2e05acab304c3e64bd859e6f020b151c1566cfb3 100644 (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_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);
index 70fae6982e967fd24df4cf6623cc297d47a677f4..784620c102b17ebb32193723d70337b0293ec3e7 100644 (file)
@@ -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);
index b1a64b912666a000647ef28af79bf3d8f67475a6..57d88f70c399ad70f979cb8e60ea55bd77047832 100644 (file)
@@ -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);
 }
 
index 13b8d395d21072040c089f13a41090f117d8e55f..d4b23c9bf90c0dad4427091c91a92257d502d23b 100644 (file)
@@ -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) {
index f0bacee33ef9a4e22c7876030c026468336df1d8..234d9eaa8eea31e5edf4b02e02e85c8de01b825d 100644 (file)
@@ -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);
 }