diff options
author | Hans Verkuil <hans.verkuil@cisco.com> | 2012-05-06 08:28:22 -0400 |
---|---|---|
committer | Mauro Carvalho Chehab <mchehab@redhat.com> | 2012-05-14 08:28:39 -0400 |
commit | 254902b01d2acc6aced99ec17caa4c6cd890cdea (patch) | |
tree | 2e33317c4e142f73ddca8e24b6c9968325cbc8bc /drivers/media/video/gspca | |
parent | cc7b6f257d42eb9829b38e3a8807943426a89a87 (diff) |
[media] gspca: Fix locking issues related to suspend/resume
There are two bugs here: first the calls to stop0 (in gspca_suspend) and
gspca_init_transfer (in gspca_resume) need to be called with the usb_lock held.
That's true for the other places they are called and it is what subdrivers
expect. Quite a few will unlock the usb_lock in stop0 while waiting for a
worker thread to finish, and if usb_lock isn't held then that can cause a
kernel oops.
The other problem is that a worker thread needs to detect that it has to
halt due to a suspend. Otherwise it will just go on looping. So add tests
against gspca_dev->frozen in the worker threads that need it.
Hdg, 2 minor changes:
1) The finepix device is ok with stopping reading a frame halfway through,
so add frozen checks in all places where we also check if we're still
streaming
2) Use gspca_dev->dev instead of gspca_dev->present to check for disconnect
in all touched drivers. I plan to do this everywhere in the future, and
most relevant lines in the touched drivers are already modified by this
patch.
Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab@redhat.com>
Diffstat (limited to 'drivers/media/video/gspca')
-rw-r--r-- | drivers/media/video/gspca/finepix.c | 8 | ||||
-rw-r--r-- | drivers/media/video/gspca/gspca.c | 16 | ||||
-rw-r--r-- | drivers/media/video/gspca/jl2005bcd.c | 6 | ||||
-rw-r--r-- | drivers/media/video/gspca/sq905.c | 8 | ||||
-rw-r--r-- | drivers/media/video/gspca/sq905c.c | 6 | ||||
-rw-r--r-- | drivers/media/video/gspca/vicam.c | 4 | ||||
-rw-r--r-- | drivers/media/video/gspca/zc3xx.c | 5 |
7 files changed, 31 insertions, 22 deletions
diff --git a/drivers/media/video/gspca/finepix.c b/drivers/media/video/gspca/finepix.c index 0107513cd728..d0befe981098 100644 --- a/drivers/media/video/gspca/finepix.c +++ b/drivers/media/video/gspca/finepix.c | |||
@@ -94,7 +94,7 @@ static void dostream(struct work_struct *work) | |||
94 | 94 | ||
95 | /* loop reading a frame */ | 95 | /* loop reading a frame */ |
96 | again: | 96 | again: |
97 | while (gspca_dev->present && gspca_dev->streaming) { | 97 | while (!gspca_dev->frozen && gspca_dev->dev && gspca_dev->streaming) { |
98 | 98 | ||
99 | /* request a frame */ | 99 | /* request a frame */ |
100 | mutex_lock(&gspca_dev->usb_lock); | 100 | mutex_lock(&gspca_dev->usb_lock); |
@@ -102,7 +102,8 @@ again: | |||
102 | mutex_unlock(&gspca_dev->usb_lock); | 102 | mutex_unlock(&gspca_dev->usb_lock); |
103 | if (ret < 0) | 103 | if (ret < 0) |
104 | break; | 104 | break; |
105 | if (!gspca_dev->present || !gspca_dev->streaming) | 105 | if (gspca_dev->frozen || !gspca_dev->dev || |
106 | !gspca_dev->streaming) | ||
106 | break; | 107 | break; |
107 | 108 | ||
108 | /* the frame comes in parts */ | 109 | /* the frame comes in parts */ |
@@ -117,7 +118,8 @@ again: | |||
117 | * error. Just restart. */ | 118 | * error. Just restart. */ |
118 | goto again; | 119 | goto again; |
119 | } | 120 | } |
120 | if (!gspca_dev->present || !gspca_dev->streaming) | 121 | if (gspca_dev->frozen || !gspca_dev->dev || |
122 | !gspca_dev->streaming) | ||
121 | goto out; | 123 | goto out; |
122 | if (len < FPIX_MAX_TRANSFER || | 124 | if (len < FPIX_MAX_TRANSFER || |
123 | (data[len - 2] == 0xff && | 125 | (data[len - 2] == 0xff && |
diff --git a/drivers/media/video/gspca/gspca.c b/drivers/media/video/gspca/gspca.c index 7669f27238c3..a14c8f71d48b 100644 --- a/drivers/media/video/gspca/gspca.c +++ b/drivers/media/video/gspca/gspca.c | |||
@@ -2499,8 +2499,11 @@ int gspca_suspend(struct usb_interface *intf, pm_message_t message) | |||
2499 | destroy_urbs(gspca_dev); | 2499 | destroy_urbs(gspca_dev); |
2500 | gspca_input_destroy_urb(gspca_dev); | 2500 | gspca_input_destroy_urb(gspca_dev); |
2501 | gspca_set_alt0(gspca_dev); | 2501 | gspca_set_alt0(gspca_dev); |
2502 | if (gspca_dev->sd_desc->stop0) | 2502 | if (gspca_dev->sd_desc->stop0) { |
2503 | mutex_lock(&gspca_dev->usb_lock); | ||
2503 | gspca_dev->sd_desc->stop0(gspca_dev); | 2504 | gspca_dev->sd_desc->stop0(gspca_dev); |
2505 | mutex_unlock(&gspca_dev->usb_lock); | ||
2506 | } | ||
2504 | return 0; | 2507 | return 0; |
2505 | } | 2508 | } |
2506 | EXPORT_SYMBOL(gspca_suspend); | 2509 | EXPORT_SYMBOL(gspca_suspend); |
@@ -2508,7 +2511,7 @@ EXPORT_SYMBOL(gspca_suspend); | |||
2508 | int gspca_resume(struct usb_interface *intf) | 2511 | int gspca_resume(struct usb_interface *intf) |
2509 | { | 2512 | { |
2510 | struct gspca_dev *gspca_dev = usb_get_intfdata(intf); | 2513 | struct gspca_dev *gspca_dev = usb_get_intfdata(intf); |
2511 | int streaming; | 2514 | int streaming, ret = 0; |
2512 | 2515 | ||
2513 | gspca_dev->frozen = 0; | 2516 | gspca_dev->frozen = 0; |
2514 | gspca_dev->sd_desc->init(gspca_dev); | 2517 | gspca_dev->sd_desc->init(gspca_dev); |
@@ -2521,9 +2524,12 @@ int gspca_resume(struct usb_interface *intf) | |||
2521 | streaming = gspca_dev->streaming; | 2524 | streaming = gspca_dev->streaming; |
2522 | gspca_dev->streaming = 0; | 2525 | gspca_dev->streaming = 0; |
2523 | v4l2_ctrl_handler_setup(gspca_dev->vdev.ctrl_handler); | 2526 | v4l2_ctrl_handler_setup(gspca_dev->vdev.ctrl_handler); |
2524 | if (streaming) | 2527 | if (streaming) { |
2525 | return gspca_init_transfer(gspca_dev); | 2528 | mutex_lock(&gspca_dev->queue_lock); |
2526 | return 0; | 2529 | ret = gspca_init_transfer(gspca_dev); |
2530 | mutex_unlock(&gspca_dev->queue_lock); | ||
2531 | } | ||
2532 | return ret; | ||
2527 | } | 2533 | } |
2528 | EXPORT_SYMBOL(gspca_resume); | 2534 | EXPORT_SYMBOL(gspca_resume); |
2529 | #endif | 2535 | #endif |
diff --git a/drivers/media/video/gspca/jl2005bcd.c b/drivers/media/video/gspca/jl2005bcd.c index 53f58ef367cf..e1fc2561e4bc 100644 --- a/drivers/media/video/gspca/jl2005bcd.c +++ b/drivers/media/video/gspca/jl2005bcd.c | |||
@@ -335,7 +335,7 @@ static void jl2005c_dostream(struct work_struct *work) | |||
335 | goto quit_stream; | 335 | goto quit_stream; |
336 | } | 336 | } |
337 | 337 | ||
338 | while (gspca_dev->present && gspca_dev->streaming) { | 338 | while (!gspca_dev->frozen && gspca_dev->dev && gspca_dev->streaming) { |
339 | /* Check if this is a new frame. If so, start the frame first */ | 339 | /* Check if this is a new frame. If so, start the frame first */ |
340 | if (!header_read) { | 340 | if (!header_read) { |
341 | mutex_lock(&gspca_dev->usb_lock); | 341 | mutex_lock(&gspca_dev->usb_lock); |
@@ -367,7 +367,7 @@ static void jl2005c_dostream(struct work_struct *work) | |||
367 | buffer, act_len); | 367 | buffer, act_len); |
368 | header_read = 1; | 368 | header_read = 1; |
369 | } | 369 | } |
370 | while (bytes_left > 0 && gspca_dev->present) { | 370 | while (bytes_left > 0 && gspca_dev->dev) { |
371 | data_len = bytes_left > JL2005C_MAX_TRANSFER ? | 371 | data_len = bytes_left > JL2005C_MAX_TRANSFER ? |
372 | JL2005C_MAX_TRANSFER : bytes_left; | 372 | JL2005C_MAX_TRANSFER : bytes_left; |
373 | ret = usb_bulk_msg(gspca_dev->dev, | 373 | ret = usb_bulk_msg(gspca_dev->dev, |
@@ -390,7 +390,7 @@ static void jl2005c_dostream(struct work_struct *work) | |||
390 | } | 390 | } |
391 | } | 391 | } |
392 | quit_stream: | 392 | quit_stream: |
393 | if (gspca_dev->present) { | 393 | if (gspca_dev->dev) { |
394 | mutex_lock(&gspca_dev->usb_lock); | 394 | mutex_lock(&gspca_dev->usb_lock); |
395 | jl2005c_stop(gspca_dev); | 395 | jl2005c_stop(gspca_dev); |
396 | mutex_unlock(&gspca_dev->usb_lock); | 396 | mutex_unlock(&gspca_dev->usb_lock); |
diff --git a/drivers/media/video/gspca/sq905.c b/drivers/media/video/gspca/sq905.c index 2fe3c29bd6b7..a144ce759b66 100644 --- a/drivers/media/video/gspca/sq905.c +++ b/drivers/media/video/gspca/sq905.c | |||
@@ -232,7 +232,7 @@ static void sq905_dostream(struct work_struct *work) | |||
232 | frame_sz = gspca_dev->cam.cam_mode[gspca_dev->curr_mode].sizeimage | 232 | frame_sz = gspca_dev->cam.cam_mode[gspca_dev->curr_mode].sizeimage |
233 | + FRAME_HEADER_LEN; | 233 | + FRAME_HEADER_LEN; |
234 | 234 | ||
235 | while (gspca_dev->present && gspca_dev->streaming) { | 235 | while (!gspca_dev->frozen && gspca_dev->dev && gspca_dev->streaming) { |
236 | /* request some data and then read it until we have | 236 | /* request some data and then read it until we have |
237 | * a complete frame. */ | 237 | * a complete frame. */ |
238 | bytes_left = frame_sz; | 238 | bytes_left = frame_sz; |
@@ -242,7 +242,7 @@ static void sq905_dostream(struct work_struct *work) | |||
242 | we must finish reading an entire frame, otherwise the | 242 | we must finish reading an entire frame, otherwise the |
243 | next time we stream we start reading in the middle of a | 243 | next time we stream we start reading in the middle of a |
244 | frame. */ | 244 | frame. */ |
245 | while (bytes_left > 0 && gspca_dev->present) { | 245 | while (bytes_left > 0 && gspca_dev->dev) { |
246 | data_len = bytes_left > SQ905_MAX_TRANSFER ? | 246 | data_len = bytes_left > SQ905_MAX_TRANSFER ? |
247 | SQ905_MAX_TRANSFER : bytes_left; | 247 | SQ905_MAX_TRANSFER : bytes_left; |
248 | ret = sq905_read_data(gspca_dev, buffer, data_len, 1); | 248 | ret = sq905_read_data(gspca_dev, buffer, data_len, 1); |
@@ -274,7 +274,7 @@ static void sq905_dostream(struct work_struct *work) | |||
274 | gspca_frame_add(gspca_dev, LAST_PACKET, | 274 | gspca_frame_add(gspca_dev, LAST_PACKET, |
275 | NULL, 0); | 275 | NULL, 0); |
276 | } | 276 | } |
277 | if (gspca_dev->present) { | 277 | if (gspca_dev->dev) { |
278 | /* acknowledge the frame */ | 278 | /* acknowledge the frame */ |
279 | mutex_lock(&gspca_dev->usb_lock); | 279 | mutex_lock(&gspca_dev->usb_lock); |
280 | ret = sq905_ack_frame(gspca_dev); | 280 | ret = sq905_ack_frame(gspca_dev); |
@@ -284,7 +284,7 @@ static void sq905_dostream(struct work_struct *work) | |||
284 | } | 284 | } |
285 | } | 285 | } |
286 | quit_stream: | 286 | quit_stream: |
287 | if (gspca_dev->present) { | 287 | if (gspca_dev->dev) { |
288 | mutex_lock(&gspca_dev->usb_lock); | 288 | mutex_lock(&gspca_dev->usb_lock); |
289 | sq905_command(gspca_dev, SQ905_CLEAR); | 289 | sq905_command(gspca_dev, SQ905_CLEAR); |
290 | mutex_unlock(&gspca_dev->usb_lock); | 290 | mutex_unlock(&gspca_dev->usb_lock); |
diff --git a/drivers/media/video/gspca/sq905c.c b/drivers/media/video/gspca/sq905c.c index ae783634712f..720c187f6ec7 100644 --- a/drivers/media/video/gspca/sq905c.c +++ b/drivers/media/video/gspca/sq905c.c | |||
@@ -150,7 +150,7 @@ static void sq905c_dostream(struct work_struct *work) | |||
150 | goto quit_stream; | 150 | goto quit_stream; |
151 | } | 151 | } |
152 | 152 | ||
153 | while (gspca_dev->present && gspca_dev->streaming) { | 153 | while (!gspca_dev->frozen && gspca_dev->dev && gspca_dev->streaming) { |
154 | /* Request the header, which tells the size to download */ | 154 | /* Request the header, which tells the size to download */ |
155 | ret = usb_bulk_msg(gspca_dev->dev, | 155 | ret = usb_bulk_msg(gspca_dev->dev, |
156 | usb_rcvbulkpipe(gspca_dev->dev, 0x81), | 156 | usb_rcvbulkpipe(gspca_dev->dev, 0x81), |
@@ -169,7 +169,7 @@ static void sq905c_dostream(struct work_struct *work) | |||
169 | packet_type = FIRST_PACKET; | 169 | packet_type = FIRST_PACKET; |
170 | gspca_frame_add(gspca_dev, packet_type, | 170 | gspca_frame_add(gspca_dev, packet_type, |
171 | buffer, FRAME_HEADER_LEN); | 171 | buffer, FRAME_HEADER_LEN); |
172 | while (bytes_left > 0 && gspca_dev->present) { | 172 | while (bytes_left > 0 && gspca_dev->dev) { |
173 | data_len = bytes_left > SQ905C_MAX_TRANSFER ? | 173 | data_len = bytes_left > SQ905C_MAX_TRANSFER ? |
174 | SQ905C_MAX_TRANSFER : bytes_left; | 174 | SQ905C_MAX_TRANSFER : bytes_left; |
175 | ret = usb_bulk_msg(gspca_dev->dev, | 175 | ret = usb_bulk_msg(gspca_dev->dev, |
@@ -191,7 +191,7 @@ static void sq905c_dostream(struct work_struct *work) | |||
191 | } | 191 | } |
192 | } | 192 | } |
193 | quit_stream: | 193 | quit_stream: |
194 | if (gspca_dev->present) { | 194 | if (gspca_dev->dev) { |
195 | mutex_lock(&gspca_dev->usb_lock); | 195 | mutex_lock(&gspca_dev->usb_lock); |
196 | sq905c_command(gspca_dev, SQ905C_CLEAR, 0); | 196 | sq905c_command(gspca_dev, SQ905C_CLEAR, 0); |
197 | mutex_unlock(&gspca_dev->usb_lock); | 197 | mutex_unlock(&gspca_dev->usb_lock); |
diff --git a/drivers/media/video/gspca/vicam.c b/drivers/media/video/gspca/vicam.c index e48ec4db6da4..432d6cd99cd6 100644 --- a/drivers/media/video/gspca/vicam.c +++ b/drivers/media/video/gspca/vicam.c | |||
@@ -225,7 +225,7 @@ static void vicam_dostream(struct work_struct *work) | |||
225 | goto exit; | 225 | goto exit; |
226 | } | 226 | } |
227 | 227 | ||
228 | while (gspca_dev->present && gspca_dev->streaming) { | 228 | while (!gspca_dev->frozen && gspca_dev->dev && gspca_dev->streaming) { |
229 | ret = vicam_read_frame(gspca_dev, buffer, frame_sz); | 229 | ret = vicam_read_frame(gspca_dev, buffer, frame_sz); |
230 | if (ret < 0) | 230 | if (ret < 0) |
231 | break; | 231 | break; |
@@ -327,7 +327,7 @@ static void sd_stop0(struct gspca_dev *gspca_dev) | |||
327 | dev->work_thread = NULL; | 327 | dev->work_thread = NULL; |
328 | mutex_lock(&gspca_dev->usb_lock); | 328 | mutex_lock(&gspca_dev->usb_lock); |
329 | 329 | ||
330 | if (gspca_dev->present) | 330 | if (gspca_dev->dev) |
331 | vicam_set_camera_power(gspca_dev, 0); | 331 | vicam_set_camera_power(gspca_dev, 0); |
332 | } | 332 | } |
333 | 333 | ||
diff --git a/drivers/media/video/gspca/zc3xx.c b/drivers/media/video/gspca/zc3xx.c index 7d9a4f1be9dc..8f21bae46ef8 100644 --- a/drivers/media/video/gspca/zc3xx.c +++ b/drivers/media/video/gspca/zc3xx.c | |||
@@ -6093,7 +6093,8 @@ static void transfer_update(struct work_struct *work) | |||
6093 | /* get the transfer status */ | 6093 | /* get the transfer status */ |
6094 | /* the bit 0 of the bridge register 11 indicates overflow */ | 6094 | /* the bit 0 of the bridge register 11 indicates overflow */ |
6095 | mutex_lock(&gspca_dev->usb_lock); | 6095 | mutex_lock(&gspca_dev->usb_lock); |
6096 | if (!gspca_dev->present || !gspca_dev->streaming) | 6096 | if (gspca_dev->frozen || !gspca_dev->dev || |
6097 | !gspca_dev->streaming) | ||
6097 | goto err; | 6098 | goto err; |
6098 | reg11 = reg_r(gspca_dev, 0x0011); | 6099 | reg11 = reg_r(gspca_dev, 0x0011); |
6099 | if (gspca_dev->usb_err < 0 | 6100 | if (gspca_dev->usb_err < 0 |
@@ -6949,7 +6950,7 @@ static void sd_stop0(struct gspca_dev *gspca_dev) | |||
6949 | mutex_lock(&gspca_dev->usb_lock); | 6950 | mutex_lock(&gspca_dev->usb_lock); |
6950 | sd->work_thread = NULL; | 6951 | sd->work_thread = NULL; |
6951 | } | 6952 | } |
6952 | if (!gspca_dev->present) | 6953 | if (!gspca_dev->dev) |
6953 | return; | 6954 | return; |
6954 | send_unknown(gspca_dev, sd->sensor); | 6955 | send_unknown(gspca_dev, sd->sensor); |
6955 | } | 6956 | } |