From 844db450e6e2cf710752af1a019a877af390b541 Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Sun, 9 Sep 2012 07:30:02 -0300 Subject: [PATCH] [media] gspca: Update / fix various comments wrt workqueue usb_lock usage Signed-off-by: Hans de Goede Signed-off-by: Mauro Carvalho Chehab --- drivers/media/usb/gspca/finepix.c | 9 ++++++++- drivers/media/usb/gspca/jl2005bcd.c | 12 +++++------- drivers/media/usb/gspca/sn9c20x.c | 2 ++ drivers/media/usb/gspca/sonixj.c | 2 ++ drivers/media/usb/gspca/sq905.c | 11 +++++------ drivers/media/usb/gspca/sq905c.c | 12 +++++------- drivers/media/usb/gspca/vicam.c | 13 ++++++------- drivers/media/usb/gspca/zc3xx.c | 4 +++- 8 files changed, 36 insertions(+), 29 deletions(-) diff --git a/drivers/media/usb/gspca/finepix.c b/drivers/media/usb/gspca/finepix.c index fb68a2934255..52bdb569760b 100644 --- a/drivers/media/usb/gspca/finepix.c +++ b/drivers/media/usb/gspca/finepix.c @@ -77,7 +77,14 @@ static int command(struct gspca_dev *gspca_dev, 12, FPIX_TIMEOUT); } -/* workqueue */ +/* + * This function is called as a workqueue function and runs whenever the camera + * is streaming data. Because it is a workqueue function it is allowed to sleep + * so we can use synchronous USB calls. To avoid possible collisions with other + * threads attempting to use gspca_dev->usb_buf we take the usb_lock when + * performing USB operations using it. In practice we don't really need this + * as the camera doesn't provide any controls. + */ static void dostream(struct work_struct *work) { struct usb_fpix *dev = container_of(work, struct usb_fpix, work_struct); diff --git a/drivers/media/usb/gspca/jl2005bcd.c b/drivers/media/usb/gspca/jl2005bcd.c index c4b4a9598db4..62ba80d9b998 100644 --- a/drivers/media/usb/gspca/jl2005bcd.c +++ b/drivers/media/usb/gspca/jl2005bcd.c @@ -306,15 +306,13 @@ static int jl2005c_stop(struct gspca_dev *gspca_dev) return retval; } -/* This function is called as a workqueue function and runs whenever the camera +/* + * This function is called as a workqueue function and runs whenever the camera * is streaming data. Because it is a workqueue function it is allowed to sleep * so we can use synchronous USB calls. To avoid possible collisions with other - * threads attempting to use the camera's USB interface the gspca usb_lock is - * used when performing the one USB control operation inside the workqueue, - * which tells the camera to close the stream. In practice the only thing - * which needs to be protected against is the usb_set_interface call that - * gspca makes during stream_off. Otherwise the camera doesn't provide any - * controls that the user could try to change. + * threads attempting to use gspca_dev->usb_buf we take the usb_lock when + * performing USB operations using it. In practice we don't really need this + * as the camera doesn't provide any controls. */ static void jl2005c_dostream(struct work_struct *work) { diff --git a/drivers/media/usb/gspca/sn9c20x.c b/drivers/media/usb/gspca/sn9c20x.c index b9c6f17eabb2..41f769fe340c 100644 --- a/drivers/media/usb/gspca/sn9c20x.c +++ b/drivers/media/usb/gspca/sn9c20x.c @@ -2197,8 +2197,10 @@ static void qual_upd(struct work_struct *work) struct gspca_dev *gspca_dev = &sd->gspca_dev; s32 qual = v4l2_ctrl_g_ctrl(sd->jpegqual); + /* To protect gspca_dev->usb_buf and gspca_dev->usb_err */ mutex_lock(&gspca_dev->usb_lock); PDEBUG(D_STREAM, "qual_upd %d%%", qual); + gspca_dev->usb_err = 0; set_quality(gspca_dev, qual); mutex_unlock(&gspca_dev->usb_lock); } diff --git a/drivers/media/usb/gspca/sonixj.c b/drivers/media/usb/gspca/sonixj.c index 150b2df40f7f..5a86047b846f 100644 --- a/drivers/media/usb/gspca/sonixj.c +++ b/drivers/media/usb/gspca/sonixj.c @@ -2380,8 +2380,10 @@ static void qual_upd(struct work_struct *work) struct sd *sd = container_of(work, struct sd, work); struct gspca_dev *gspca_dev = &sd->gspca_dev; + /* To protect gspca_dev->usb_buf and gspca_dev->usb_err */ mutex_lock(&gspca_dev->usb_lock); PDEBUG(D_STREAM, "qual_upd %d%%", sd->quality); + gspca_dev->usb_err = 0; setjpegqual(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 2e05acab304c..1d99f10a3e19 100644 --- a/drivers/media/usb/gspca/sq905.c +++ b/drivers/media/usb/gspca/sq905.c @@ -201,14 +201,13 @@ sq905_read_data(struct gspca_dev *gspca_dev, u8 *data, int size, int need_lock) return 0; } -/* This function is called as a workqueue function and runs whenever the camera +/* + * This function is called as a workqueue function and runs whenever the camera * is streaming data. Because it is a workqueue function it is allowed to sleep * so we can use synchronous USB calls. To avoid possible collisions with other - * threads attempting to use the camera's USB interface we take the gspca - * usb_lock when performing USB operations. In practice the only thing we need - * to protect against is the usb_set_interface call that gspca makes during - * stream_off as the camera doesn't provide any controls that the user could try - * to change. + * threads attempting to use gspca_dev->usb_buf we take the usb_lock when + * performing USB operations using it. In practice we don't really need this + * as the camera doesn't provide any controls. */ static void sq905_dostream(struct work_struct *work) { diff --git a/drivers/media/usb/gspca/sq905c.c b/drivers/media/usb/gspca/sq905c.c index 784620c102b1..410cdcbb55d4 100644 --- a/drivers/media/usb/gspca/sq905c.c +++ b/drivers/media/usb/gspca/sq905c.c @@ -123,15 +123,13 @@ static int sq905c_read(struct gspca_dev *gspca_dev, u16 command, u16 index, return 0; } -/* This function is called as a workqueue function and runs whenever the camera +/* + * This function is called as a workqueue function and runs whenever the camera * is streaming data. Because it is a workqueue function it is allowed to sleep * so we can use synchronous USB calls. To avoid possible collisions with other - * threads attempting to use the camera's USB interface the gspca usb_lock is - * used when performing the one USB control operation inside the workqueue, - * which tells the camera to close the stream. In practice the only thing - * which needs to be protected against is the usb_set_interface call that - * gspca makes during stream_off. Otherwise the camera doesn't provide any - * controls that the user could try to change. + * threads attempting to use gspca_dev->usb_buf we take the usb_lock when + * performing USB operations using it. In practice we don't really need this + * as the camera doesn't provide any controls. */ static void sq905c_dostream(struct work_struct *work) { diff --git a/drivers/media/usb/gspca/vicam.c b/drivers/media/usb/gspca/vicam.c index 57d88f70c399..d6890bc37198 100644 --- a/drivers/media/usb/gspca/vicam.c +++ b/drivers/media/usb/gspca/vicam.c @@ -110,7 +110,7 @@ static int vicam_set_camera_power(struct gspca_dev *gspca_dev, int state) } /* - * request and read a block of data - see warning on vicam_command. + * request and read a block of data */ static int vicam_read_frame(struct gspca_dev *gspca_dev, u8 *data, int size) { @@ -170,14 +170,13 @@ static int vicam_read_frame(struct gspca_dev *gspca_dev, u8 *data, int size) return 0; } -/* This function is called as a workqueue function and runs whenever the camera +/* + * This function is called as a workqueue function and runs whenever the camera * is streaming data. Because it is a workqueue function it is allowed to sleep * so we can use synchronous USB calls. To avoid possible collisions with other - * threads attempting to use the camera's USB interface we take the gspca - * usb_lock when performing USB operations. In practice the only thing we need - * to protect against is the usb_set_interface call that gspca makes during - * stream_off as the camera doesn't provide any controls that the user could try - * to change. + * threads attempting to use gspca_dev->usb_buf we take the usb_lock when + * performing USB operations using it. In practice we don't really need this + * as the cameras controls are only written from the workqueue. */ static void vicam_dostream(struct work_struct *work) { diff --git a/drivers/media/usb/gspca/zc3xx.c b/drivers/media/usb/gspca/zc3xx.c index 234d9eaa8eea..c47ba14c7619 100644 --- a/drivers/media/usb/gspca/zc3xx.c +++ b/drivers/media/usb/gspca/zc3xx.c @@ -5945,6 +5945,7 @@ static void transfer_update(struct work_struct *work) for (;;) { msleep(100); + /* To protect gspca_dev->usb_buf and gspca_dev->usb_err */ mutex_lock(&gspca_dev->usb_lock); #ifdef CONFIG_PM if (gspca_dev->frozen) @@ -6831,7 +6832,8 @@ static int sd_start(struct gspca_dev *gspca_dev) return 0; } -/* called on streamoff with alt 0 and on disconnect */ +/* called on streamoff with alt==0 and on disconnect */ +/* the usb_lock is held at entry - restore on exit */ static void sd_stop0(struct gspca_dev *gspca_dev) { struct sd *sd = (struct sd *) gspca_dev; -- 2.39.5