V4L/DVB (11220): gspca - finepix: Use a workqueue for streaming.

Tested-by: Frank Zago <frank@zago.net>
Acked-by: Frank Zago <frank@zago.net>
Signed-off-by: Jean-Francois Moine <moinejf@free.fr>
Signed-off-by: Mauro Carvalho Chehab <mchehab@redhat.com>

drivers/media/video/gspca/finepix.c

@@ -27,7 +27,7 @@ MODULE_DESCRIPTION("Fujifilm FinePix USB V4L2 driver");
 MODULE_LICENSE("GPL");
 
 /* Default timeout, in ms */
-#define FPIX_TIMEOUT (HZ / 10)
+#define FPIX_TIMEOUT 250
 
 /* Maximum transfer size to use. The windows driver reads by chunks of
  * 0x2000 bytes, so do the same. Note: reading more seems to work
@@ -38,38 +38,15 @@ MODULE_LICENSE("GPL");
 struct usb_fpix {
 	struct gspca_dev gspca_dev;	/* !! must be the first item */
 
-	/*
-	 * USB stuff
-	 */
-	struct usb_ctrlrequest ctrlreq;
-	struct urb *control_urb;
-	struct timer_list bulk_timer;
-
-	enum {
-		FPIX_NOP,	/* inactive, else streaming */
-		FPIX_RESET,	/* must reset */
-		FPIX_REQ_FRAME,	/* requesting a frame */
-		FPIX_READ_FRAME,	/* reading frame */
-	} state;
-
-	/*
-	 * Driver stuff
-	 */
-	struct delayed_work wqe;
-	struct completion can_close;
-	int streaming;
+	struct work_struct work_struct;
+	struct workqueue_struct *work_thread;
 };
 
 /* Delay after which claim the next frame. If the delay is too small,
  * the camera will return old frames. On the 4800Z, 20ms is bad, 25ms
- * will fail every 4 or 5 frames, but 30ms is perfect. */
-#define NEXT_FRAME_DELAY  (((HZ * 30) + 999) / 1000)
-
-#define dev_new_state(new_state) {				\
-		PDEBUG(D_STREAM, "new state from %d to %d at %s:%d",	\
-			dev->state, new_state, __func__, __LINE__);	\
-		dev->state = new_state;					\
-}
+ * will fail every 4 or 5 frames, but 30ms is perfect. On the A210,
+ * 30ms is bad while 35ms is perfect. */
+#define NEXT_FRAME_DELAY 35
 
 /* These cameras only support 320x200. */
 static const struct v4l2_pix_format fpix_mode[1] = {
@@ -80,314 +57,183 @@ static const struct v4l2_pix_format fpix_mode[1] = {
 		.priv = 0}
 };
 
-/* Reads part of a frame */
-static void read_frame_part(struct usb_fpix *dev)
+/* send a command to the webcam */
+static int command(struct gspca_dev *gspca_dev,
+		int order)	/* 0: reset, 1: frame request */
 {
-	int ret;
+	static u8 order_values[2][12] = {
+		{0xc6, 0, 0, 0, 0, 0, 0,    0, 0x20, 0, 0, 0},	/* reset */
+		{0xd3, 0, 0, 0, 0, 0, 0, 0x01,    0, 0, 0, 0},	/* fr req */
+	};
 
-	PDEBUG(D_STREAM, "read_frame_part");
-
-	/* Reads part of a frame */
-	ret = usb_submit_urb(dev->gspca_dev.urb[0], GFP_ATOMIC);
-	if (ret) {
-		dev_new_state(FPIX_RESET);
-		schedule_delayed_work(&dev->wqe, 1);
-		PDEBUG(D_STREAM, "usb_submit_urb failed with %d",
-			ret);
-	} else {
-		/* Sometimes we never get a callback, so use a timer.
-		 * Is this masking a bug somewhere else? */
-		dev->bulk_timer.expires = jiffies + msecs_to_jiffies(150);
-		add_timer(&dev->bulk_timer);
-	}
+	memcpy(gspca_dev->usb_buf, order_values[order], 12);
+	return usb_control_msg(gspca_dev->dev,
+			usb_sndctrlpipe(gspca_dev->dev, 0),
+			USB_REQ_GET_STATUS,
+			USB_DIR_OUT | USB_TYPE_CLASS |
+			USB_RECIP_INTERFACE, 0, 0, gspca_dev->usb_buf,
+			12, FPIX_TIMEOUT);
 }
 
-/* Callback for URBs. */
-static void urb_callback(struct urb *urb)
+/* workqueue */
+static void dostream(struct work_struct *work)
 {
-	struct gspca_dev *gspca_dev = urb->context;
-	struct usb_fpix *dev = (struct usb_fpix *) gspca_dev;
-
-	PDEBUG(D_PACK,
-		"enter urb_callback - status=%d, length=%d",
-		urb->status, urb->actual_length);
-
-	if (dev->state == FPIX_READ_FRAME)
-		del_timer(&dev->bulk_timer);
-
-	if (urb->status != 0) {
-		/* We kill a stuck urb every 50 frames on average, so don't
-		 * display a log message for that. */
-		if (urb->status != -ECONNRESET)
-			PDEBUG(D_STREAM, "bad URB status %d", urb->status);
-		dev_new_state(FPIX_RESET);
-		schedule_delayed_work(&dev->wqe, 1);
-	}
-
-	switch (dev->state) {
-	case FPIX_REQ_FRAME:
-		dev_new_state(FPIX_READ_FRAME);
-		read_frame_part(dev);
-		break;
-
-	case FPIX_READ_FRAME: {
-		unsigned char *data = urb->transfer_buffer;
-		struct gspca_frame *frame;
-
-		frame = gspca_get_i_frame(&dev->gspca_dev);
-		if (frame == NULL)
-			gspca_dev->last_packet_type = DISCARD_PACKET;
-		if (urb->actual_length < FPIX_MAX_TRANSFER ||
-			(data[urb->actual_length-2] == 0xff &&
-				data[urb->actual_length-1] == 0xd9)) {
-
-			/* If the result is less than what was asked
-			 * for, then it's the end of the
-			 * frame. Sometime the jpeg is not complete,
-			 * but there's nothing we can do. We also end
-			 * here if the the jpeg ends right at the end
-			 * of the frame. */
-			if (frame)
-				gspca_frame_add(gspca_dev, LAST_PACKET,
-						frame,
-						data, urb->actual_length);
-			dev_new_state(FPIX_REQ_FRAME);
-			schedule_delayed_work(&dev->wqe, NEXT_FRAME_DELAY);
-		} else {
+	struct usb_fpix *dev = container_of(work, struct usb_fpix, work_struct);
+	struct gspca_dev *gspca_dev = &dev->gspca_dev;
+	struct urb *urb = gspca_dev->urb[0];
+	u8 *data = urb->transfer_buffer;
+	struct gspca_frame *frame;
+	int ret = 0;
+	int len;
+
+	/* synchronize with the main driver */
+	mutex_lock(&gspca_dev->usb_lock);
+	mutex_unlock(&gspca_dev->usb_lock);
+	PDEBUG(D_STREAM, "dostream started");
+
+	/* loop reading a frame */
+again:
+	while (gspca_dev->present && gspca_dev->streaming) {
+
+		/* request a frame */
+		mutex_lock(&gspca_dev->usb_lock);
+		ret = command(gspca_dev, 1);
+		mutex_unlock(&gspca_dev->usb_lock);
+		if (ret < 0)
+			break;
+		if (!gspca_dev->present || !gspca_dev->streaming)
+			break;
+
+		/* the frame comes in parts */
+		for (;;) {
+			ret = usb_bulk_msg(gspca_dev->dev,
+					urb->pipe,
+					data,
+					FPIX_MAX_TRANSFER,
+					&len, FPIX_TIMEOUT);
+			if (ret < 0) {
+				/* Most of the time we get a timeout
+				 * error. Just restart. */
+				goto again;
+			}
+			if (!gspca_dev->present || !gspca_dev->streaming)
+				goto out;
+			frame = gspca_get_i_frame(&dev->gspca_dev);
+			if (frame == NULL)
+				gspca_dev->last_packet_type = DISCARD_PACKET;
+
+			if (len < FPIX_MAX_TRANSFER ||
+				(data[len - 2] == 0xff &&
+					data[len - 1] == 0xd9)) {
+
+				/* If the result is less than what was asked
+				 * for, then it's the end of the
+				 * frame. Sometimes the jpeg is not complete,
+				 * but there's nothing we can do. We also end
+				 * here if the the jpeg ends right at the end
+				 * of the frame. */
+				if (frame)
+					frame = gspca_frame_add(gspca_dev,
+							LAST_PACKET,
+							frame,
+							data, len);
+				break;
+			}
 
 			/* got a partial image */
 			if (frame)
 				gspca_frame_add(gspca_dev,
 						gspca_dev->last_packet_type
-								== LAST_PACKET
+							== LAST_PACKET
 						? FIRST_PACKET : INTER_PACKET,
-						frame,
-					data, urb->actual_length);
-			read_frame_part(dev);
+						frame, data, len);
 		}
-		break;
-	    }
-
-	case FPIX_NOP:
-	case FPIX_RESET:
-		PDEBUG(D_STREAM, "invalid state %d", dev->state);
-		break;
-	}
-}
-
-/* Request a new frame */
-static void request_frame(struct usb_fpix *dev)
-{
-	int ret;
-	struct gspca_dev *gspca_dev = &dev->gspca_dev;
 
-	/* Setup command packet */
-	memset(gspca_dev->usb_buf, 0, 12);
-	gspca_dev->usb_buf[0] = 0xd3;
-	gspca_dev->usb_buf[7] = 0x01;
-
-	/* Request a frame */
-	dev->ctrlreq.bRequestType =
-		USB_DIR_OUT | USB_TYPE_CLASS | USB_RECIP_INTERFACE;
-	dev->ctrlreq.bRequest = USB_REQ_GET_STATUS;
-	dev->ctrlreq.wValue = 0;
-	dev->ctrlreq.wIndex = 0;
-	dev->ctrlreq.wLength = cpu_to_le16(12);
-
-	usb_fill_control_urb(dev->control_urb,
-			     gspca_dev->dev,
-			     usb_sndctrlpipe(gspca_dev->dev, 0),
-			     (unsigned char *) &dev->ctrlreq,
-			     gspca_dev->usb_buf,
-			     12, urb_callback, gspca_dev);
-
-	ret = usb_submit_urb(dev->control_urb, GFP_ATOMIC);
-	if (ret) {
-		dev_new_state(FPIX_RESET);
-		schedule_delayed_work(&dev->wqe, 1);
-		PDEBUG(D_STREAM, "usb_submit_urb failed with %d", ret);
+		/* We must wait before trying reading the next
+		 * frame. If we don't, or if the delay is too short,
+		 * the camera will disconnect. */
+		msleep(NEXT_FRAME_DELAY);
 	}
-}
-
-/*--------------------------------------------------------------------------*/
 
-/* State machine. */
-static void fpix_sm(struct work_struct *work)
-{
-	struct usb_fpix *dev = container_of(work, struct usb_fpix, wqe.work);
-
-	PDEBUG(D_STREAM, "fpix_sm state %d", dev->state);
-
-	/* verify that the device wasn't unplugged */
-	if (!dev->gspca_dev.present) {
-		PDEBUG(D_STREAM, "device is gone");
-		dev_new_state(FPIX_NOP);
-		complete(&dev->can_close);
-		return;
-	}
-
-	if (!dev->streaming) {
-		PDEBUG(D_STREAM, "stopping state machine");
-		dev_new_state(FPIX_NOP);
-		complete(&dev->can_close);
-		return;
-	}
-
-	switch (dev->state) {
-	case FPIX_RESET:
-		dev_new_state(FPIX_REQ_FRAME);
-		schedule_delayed_work(&dev->wqe, HZ / 10);
-		break;
-
-	case FPIX_REQ_FRAME:
-		/* get an image */
-		request_frame(dev);
-		break;
-
-	case FPIX_NOP:
-	case FPIX_READ_FRAME:
-		PDEBUG(D_STREAM, "invalid state %d", dev->state);
-		break;
-	}
+out:
+	PDEBUG(D_STREAM, "dostream stopped");
 }
 
 /* this function is called at probe time */
 static int sd_config(struct gspca_dev *gspca_dev,
 		const struct usb_device_id *id)
 {
+	struct usb_fpix *dev = (struct usb_fpix *) gspca_dev;
 	struct cam *cam = &gspca_dev->cam;
 
 	cam->cam_mode = fpix_mode;
 	cam->nmodes = 1;
 	cam->bulk_size = FPIX_MAX_TRANSFER;
 
-/*	gspca_dev->nbalt = 1;	 * use bulk transfer */
-	return 0;
-}
-
-/* Stop streaming and free the ressources allocated by sd_start. */
-static void sd_stopN(struct gspca_dev *gspca_dev)
-{
-	struct usb_fpix *dev = (struct usb_fpix *) gspca_dev;
+	INIT_WORK(&dev->work_struct, dostream);
 
-	dev->streaming = 0;
-
-	/* Stop the state machine */
-	if (dev->state != FPIX_NOP)
-		wait_for_completion(&dev->can_close);
-}
-
-/* called on streamoff with alt 0 and disconnect */
-static void sd_stop0(struct gspca_dev *gspca_dev)
-{
-	struct usb_fpix *dev = (struct usb_fpix *) gspca_dev;
-
-	usb_free_urb(dev->control_urb);
-	dev->control_urb = NULL;
-}
-
-/* Kill an URB that hasn't completed. */
-static void timeout_kill(unsigned long data)
-{
-	struct urb *urb = (struct urb *) data;
-
-	usb_unlink_urb(urb);
+	return 0;
 }
 
 /* this function is called at probe and resume time */
 static int sd_init(struct gspca_dev *gspca_dev)
 {
-	struct usb_fpix *dev = (struct usb_fpix *) gspca_dev;
-
-	INIT_DELAYED_WORK(&dev->wqe, fpix_sm);
-
-	init_timer(&dev->bulk_timer);
-	dev->bulk_timer.function = timeout_kill;
-
 	return 0;
 }
 
+/* start the camera */
 static int sd_start(struct gspca_dev *gspca_dev)
 {
 	struct usb_fpix *dev = (struct usb_fpix *) gspca_dev;
-	int ret;
-	int size_ret;
+	int ret, len;
 
 	/* Init the device */
-	memset(gspca_dev->usb_buf, 0, 12);
-	gspca_dev->usb_buf[0] = 0xc6;
-	gspca_dev->usb_buf[8] = 0x20;
-
-	ret = usb_control_msg(gspca_dev->dev,
-			usb_sndctrlpipe(gspca_dev->dev, 0),
-			USB_REQ_GET_STATUS,
-			USB_DIR_OUT | USB_TYPE_CLASS |
-			USB_RECIP_INTERFACE, 0, 0, gspca_dev->usb_buf,
-			12, FPIX_TIMEOUT);
-
-	if (ret != 12) {
-		PDEBUG(D_STREAM, "usb_control_msg failed (%d)", ret);
-		ret = -EIO;
-		goto error;
+	ret = command(gspca_dev, 0);
+	if (ret < 0) {
+		PDEBUG(D_STREAM, "init failed %d", ret);
+		return ret;
 	}
 
 	/* Read the result of the command. Ignore the result, for it
 	 * varies with the device. */
 	ret = usb_bulk_msg(gspca_dev->dev,
 			gspca_dev->urb[0]->pipe,
-			gspca_dev->usb_buf, FPIX_MAX_TRANSFER, &size_ret,
+			gspca_dev->urb[0]->transfer_buffer,
+			FPIX_MAX_TRANSFER, &len,
 			FPIX_TIMEOUT);
-	if (ret != 0) {
-		PDEBUG(D_STREAM, "usb_bulk_msg failed (%d)", ret);
-		ret = -EIO;
-		goto error;
+	if (ret < 0) {
+		PDEBUG(D_STREAM, "usb_bulk_msg failed %d", ret);
+		return ret;
 	}
 
 	/* Request a frame, but don't read it */
-	memset(gspca_dev->usb_buf, 0, 12);
-	gspca_dev->usb_buf[0] = 0xd3;
-	gspca_dev->usb_buf[7] = 0x01;
-
-	ret = usb_control_msg(gspca_dev->dev,
-			usb_sndctrlpipe(gspca_dev->dev, 0),
-			USB_REQ_GET_STATUS,
-			USB_DIR_OUT | USB_TYPE_CLASS |
-			USB_RECIP_INTERFACE, 0, 0, gspca_dev->usb_buf,
-			12, FPIX_TIMEOUT);
-	if (ret != 12) {
-		PDEBUG(D_STREAM, "usb_control_msg failed (%d)", ret);
-		ret = -EIO;
-		goto error;
+	ret = command(gspca_dev, 1);
+	if (ret < 0) {
+		PDEBUG(D_STREAM, "frame request failed %d", ret);
+		return ret;
 	}
 
 	/* Again, reset bulk in endpoint */
 	usb_clear_halt(gspca_dev->dev, gspca_dev->urb[0]->pipe);
 
-	/* Allocate a control URB */
-	dev->control_urb = usb_alloc_urb(0, GFP_KERNEL);
-	if (!dev->control_urb) {
-		PDEBUG(D_STREAM, "No free urbs available");
-		ret = -EIO;
-		goto error;
-	}
-
-	/* Various initializations. */
-	init_completion(&dev->can_close);
-	dev->bulk_timer.data = (unsigned long)dev->gspca_dev.urb[0];
-	dev->gspca_dev.urb[0]->complete = urb_callback;
-	dev->streaming = 1;
-
-	/* Schedule a frame request. */
-	dev_new_state(FPIX_REQ_FRAME);
-	schedule_delayed_work(&dev->wqe, 1);
+	/* Start the workqueue function to do the streaming */
+	dev->work_thread = create_singlethread_workqueue(MODULE_NAME);
+	queue_work(dev->work_thread, &dev->work_struct);
 
 	return 0;
+}
+
+/* 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 usb_fpix *dev = (struct usb_fpix *) gspca_dev;
 
-error:
-	/* Free the ressources */
-	sd_stopN(gspca_dev);
-	sd_stop0(gspca_dev);
-	return ret;
+	/* wait for the work queue to terminate */
+	mutex_unlock(&gspca_dev->usb_lock);
+	destroy_workqueue(dev->work_thread);
+	mutex_lock(&gspca_dev->usb_lock);
+	dev->work_thread = NULL;
 }
 
 /* Table of supported USB devices */
@@ -422,12 +268,11 @@ MODULE_DEVICE_TABLE(usb, device_table);
 
 /* sub-driver description */
 static const struct sd_desc sd_desc = {
-	.name = MODULE_NAME,
+	.name   = MODULE_NAME,
 	.config = sd_config,
-	.init = sd_init,
-	.start = sd_start,
-	.stopN = sd_stopN,
-	.stop0 = sd_stop0,
+	.init   = sd_init,
+	.start  = sd_start,
+	.stop0  = sd_stop0,
 };
 
 /* -- device connect -- */
@@ -441,13 +286,13 @@ static int sd_probe(struct usb_interface *intf,
 }
 
 static struct usb_driver sd_driver = {
-	.name = MODULE_NAME,
-	.id_table = device_table,
-	.probe = sd_probe,
+	.name       = MODULE_NAME,
+	.id_table   = device_table,
+	.probe      = sd_probe,
 	.disconnect = gspca_disconnect,
 #ifdef CONFIG_PM
 	.suspend = gspca_suspend,
-	.resume = gspca_resume,
+	.resume  = gspca_resume,
 #endif
 };
 
@@ -455,12 +300,14 @@ static struct usb_driver sd_driver = {
 static int __init sd_mod_init(void)
 {
 	int ret;
+
 	ret = usb_register(&sd_driver);
 	if (ret < 0)
 		return ret;
 	PDEBUG(D_PROBE, "registered");
 	return 0;
 }
+
 static void __exit sd_mod_exit(void)
 {
 	usb_deregister(&sd_driver);