[PATCH 1/9] staging: easycap: Split device struct alloc and retrieval code

[PATCH 1/9] staging: easycap: Split device struct alloc and retrieval code

[Ezequiel Garcia]

When the device is probed a driver struct is either
allocated or retrieved.
This operation is logically splitted in several functions.

Signed-off-by: Ezequiel Garcia <elezegarcia@gmail.com>
---
 drivers/staging/media/easycap/easycap_main.c |  384 +++++++++++++++-----------
 1 files changed, 216 insertions(+), 168 deletions(-)

diff --git a/drivers/staging/media/easycap/easycap_main.c b/drivers/staging/media/easycap/easycap_main.c
index d0fe34a..c4198be 100644
--- a/drivers/staging/media/easycap/easycap_main.c
+++ b/drivers/staging/media/easycap/easycap_main.c
@@ -2842,6 +2842,209 @@ static void easycap_complete(struct urb *purb)
 	return;
 }
 
+static struct easycap *alloc_easycap(u8 bInterfaceNumber)
+{
+	struct easycap *peasycap;
+	int i;
+
+	peasycap = kzalloc(sizeof(struct easycap), GFP_KERNEL);
+	if (!peasycap) {
+		SAY("ERROR: Could not allocate peasycap\n");
+		return NULL;
+	}
+
+	if (mutex_lock_interruptible(&mutex_dongle)) {
+		SAY("ERROR: cannot lock mutex_dongle\n");
+		kfree(peasycap);
+		return NULL;
+	}
+
+	/* Find a free dongle in easycapdc60_dongle array */
+	for (i = 0; i < DONGLE_MANY; i++) {
+
+		if ((!easycapdc60_dongle[i].peasycap) &&
+		    (!mutex_is_locked(&easycapdc60_dongle[i].mutex_video)) &&
+		    (!mutex_is_locked(&easycapdc60_dongle[i].mutex_audio))) {
+
+			easycapdc60_dongle[i].peasycap = peasycap;
+			peasycap->isdongle = i;
+			JOM(8, "intf[%i]: peasycap-->easycap"
+				"_dongle[%i].peasycap\n",
+				bInterfaceNumber, i);
+			break;
+		}
+	}
+
+	mutex_unlock(&mutex_dongle);
+
+	if (i >= DONGLE_MANY) {
+		SAM("ERROR: too many dongles\n");
+		kfree(peasycap);
+		return NULL;
+	}
+
+	return peasycap;
+}
+
+/*
+ * FIXME: Identify the appropriate pointer peasycap for interfaces
+ * 1 and 2. The address of peasycap->pusb_device is reluctantly used
+ * for this purpose.
+ */
+static struct easycap *get_easycap(struct usb_device *usbdev,
+				   u8 bInterfaceNumber)
+{
+	int i;
+	struct easycap *peasycap;
+
+	for (i = 0; i < DONGLE_MANY; i++) {
+		if (easycapdc60_dongle[i].peasycap->pusb_device == usbdev) {
+			peasycap = easycapdc60_dongle[i].peasycap;
+			JOT(8, "intf[%i]: dongle[%i].peasycap\n",
+					bInterfaceNumber, i);
+			break;
+		}
+	}
+	if (i >= DONGLE_MANY) {
+		SAY("ERROR: peasycap is unknown when probing interface %i\n",
+			bInterfaceNumber);
+		return NULL;
+	}
+	if (!peasycap) {
+		SAY("ERROR: peasycap is NULL when probing interface %i\n",
+			bInterfaceNumber);
+		return NULL;
+	}
+
+	return peasycap;
+}
+
+static void init_easycap(struct easycap *peasycap,
+			 struct usb_device *usbdev,
+			 struct usb_interface *intf,
+			 u8 bInterfaceNumber)
+{
+	/* Save usb_device and usb_interface */
+	peasycap->pusb_device = usbdev;
+	peasycap->pusb_interface = intf;
+
+	peasycap->minor = -1;
+	kref_init(&peasycap->kref);
+	JOM(8, "intf[%i]: after kref_init(..._video) "
+		"%i=peasycap->kref.refcount.counter\n",
+		bInterfaceNumber, peasycap->kref.refcount.counter);
+
+	/* module params */
+	peasycap->gain = (s8)clamp(easycap_gain, 0, 31);
+
+	init_waitqueue_head(&peasycap->wq_video);
+	init_waitqueue_head(&peasycap->wq_audio);
+	init_waitqueue_head(&peasycap->wq_trigger);
+
+	peasycap->allocation_video_struct = sizeof(struct easycap);
+
+	peasycap->microphone = false;
+
+	peasycap->video_interface = -1;
+	peasycap->video_altsetting_on = -1;
+	peasycap->video_altsetting_off = -1;
+	peasycap->video_endpointnumber = -1;
+	peasycap->video_isoc_maxframesize = -1;
+	peasycap->video_isoc_buffer_size = -1;
+
+	peasycap->audio_interface = -1;
+	peasycap->audio_altsetting_on = -1;
+	peasycap->audio_altsetting_off = -1;
+	peasycap->audio_endpointnumber = -1;
+	peasycap->audio_isoc_maxframesize = -1;
+	peasycap->audio_isoc_buffer_size = -1;
+
+	peasycap->frame_buffer_many = FRAME_BUFFER_MANY;
+}
+
+static int populate_inputset(struct easycap *peasycap)
+{
+	struct inputset *inputset;
+	struct easycap_format *peasycap_format;
+	struct v4l2_pix_format *pix;
+	int m, i, k, mask, fmtidx;
+	s32 value;
+
+	inputset = peasycap->inputset;
+
+	/* FIXME: peasycap->ntsc is not yet initialized */
+	fmtidx = peasycap->ntsc ? NTSC_M : PAL_BGHIN;
+
+	m = 0;
+	mask = 0;
+	for (i = 0; easycap_standard[i].mask != 0xffff; i++) {
+		if (fmtidx == easycap_standard[i].v4l2_standard.index) {
+			m++;
+			for (k = 0; k < INPUT_MANY; k++)
+				inputset[k].standard_offset = i;
+			mask = easycap_standard[i].mask;
+		}
+	}
+
+	if (m != 1) {
+		SAM("ERROR: inputset->standard_offset unpopulated, %i=m\n", m);
+		return -ENOENT;
+	}
+
+	peasycap_format = &easycap_format[0];
+	m = 0;
+	for (i = 0; peasycap_format->v4l2_format.fmt.pix.width; i++) {
+		pix = &peasycap_format->v4l2_format.fmt.pix;
+		if (((peasycap_format->mask & 0x0F) == (mask & 0x0F))
+			&& pix->field == V4L2_FIELD_NONE
+			&& pix->pixelformat == V4L2_PIX_FMT_UYVY
+			&& pix->width  == 640 && pix->height == 480) {
+			m++;
+			for (k = 0; k < INPUT_MANY; k++)
+				inputset[k].format_offset = i;
+			break;
+		}
+		peasycap_format++;
+	}
+	if (m != 1) {
+		SAM("ERROR: inputset[]->format_offset unpopulated\n");
+		return -ENOENT;
+	}
+
+	m = 0;
+	for (i = 0; easycap_control[i].id != 0xffffffff; i++) {
+		value = easycap_control[i].default_value;
+		if (V4L2_CID_BRIGHTNESS == easycap_control[i].id) {
+			m++;
+			for (k = 0; k < INPUT_MANY; k++)
+				inputset[k].brightness = value;
+		} else if (V4L2_CID_CONTRAST == easycap_control[i].id) {
+			m++;
+			for (k = 0; k < INPUT_MANY; k++)
+				inputset[k].contrast = value;
+		} else if (V4L2_CID_SATURATION == easycap_control[i].id) {
+			m++;
+			for (k = 0; k < INPUT_MANY; k++)
+				inputset[k].saturation = value;
+		} else if (V4L2_CID_HUE == easycap_control[i].id) {
+			m++;
+			for (k = 0; k < INPUT_MANY; k++)
+				inputset[k].hue = value;
+		}
+	}
+
+	if (m != 4) {
+		SAM("ERROR: inputset[]->brightness underpopulated\n");
+		return -ENOENT;
+	}
+
+	for (k = 0; k < INPUT_MANY; k++)
+		inputset[k].input = k;
+	JOM(4, "populated inputset[]\n");
+
+	return 0;
+}
+
 static const struct v4l2_file_operations v4l2_fops = {
 	.owner		= THIS_MODULE,
 	.open		= easycap_open_noinode,
@@ -2863,7 +3066,6 @@ static int easycap_usb_probe(struct usb_interface *intf,
 	struct usb_interface_descriptor *interface;
 	struct urb *purb;
 	struct easycap *peasycap;
-	int ndong;
 	struct data_urb *pdata_urb;
 	int i, j, k, m, rc;
 	u8 bInterfaceNumber;
@@ -2874,11 +3076,6 @@ static int easycap_usb_probe(struct usb_interface *intf,
 	int okepn[8];
 	int okmps[8];
 	int maxpacketsize;
-	u16 mask;
-	s32 value;
-	struct easycap_format *peasycap_format;
-	int fmtidx;
-	struct inputset *inputset;
 
 	usbdev = interface_to_usbdev(intf);
 
@@ -2916,76 +3113,16 @@ static int easycap_usb_probe(struct usb_interface *intf,
 	 * interfaces 1 and 2 are probed.
 	 */
 	if (0 == bInterfaceNumber) {
-		peasycap = kzalloc(sizeof(struct easycap), GFP_KERNEL);
-		if (!peasycap) {
-			SAY("ERROR: Could not allocate peasycap\n");
-			return -ENOMEM;
-		}
-
-		/* Perform urgent initializations */
-		peasycap->minor = -1;
-		kref_init(&peasycap->kref);
-		JOM(8, "intf[%i]: after kref_init(..._video) "
-				"%i=peasycap->kref.refcount.counter\n",
-				bInterfaceNumber, peasycap->kref.refcount.counter);
-
-		/* module params */
-		peasycap->gain = (s8)clamp(easycap_gain, 0, 31);
-
-		init_waitqueue_head(&peasycap->wq_video);
-		init_waitqueue_head(&peasycap->wq_audio);
-		init_waitqueue_head(&peasycap->wq_trigger);
-
-		if (mutex_lock_interruptible(&mutex_dongle)) {
-			SAY("ERROR: cannot down mutex_dongle\n");
-			return -ERESTARTSYS;
-		}
-
-		for (ndong = 0; ndong < DONGLE_MANY; ndong++) {
-			if ((!easycapdc60_dongle[ndong].peasycap) &&
-					(!mutex_is_locked(&easycapdc60_dongle
-						[ndong].mutex_video)) &&
-					(!mutex_is_locked(&easycapdc60_dongle
-						[ndong].mutex_audio))) {
-				easycapdc60_dongle[ndong].peasycap = peasycap;
-				peasycap->isdongle = ndong;
-				JOM(8, "intf[%i]: peasycap-->easycap"
-						"_dongle[%i].peasycap\n",
-						bInterfaceNumber, ndong);
-				break;
-			}
-		}
-
-		if (DONGLE_MANY <= ndong) {
-			SAM("ERROR: too many dongles\n");
-			mutex_unlock(&mutex_dongle);
+		/*
+		 * Alloc structure and save it in a free slot in
+		 * easycapdc60_dongle array
+		 */
+		peasycap = alloc_easycap(bInterfaceNumber);
+		if (!peasycap)
 			return -ENOMEM;
-		}
-		mutex_unlock(&mutex_dongle);
 
-		peasycap->allocation_video_struct = sizeof(struct easycap);
-
-		/* and further initialize the structure */
-		peasycap->pusb_device = usbdev;
-		peasycap->pusb_interface = intf;
-
-		peasycap->microphone = false;
-
-		peasycap->video_interface = -1;
-		peasycap->video_altsetting_on = -1;
-		peasycap->video_altsetting_off = -1;
-		peasycap->video_endpointnumber = -1;
-		peasycap->video_isoc_maxframesize = -1;
-		peasycap->video_isoc_buffer_size = -1;
-
-		peasycap->audio_interface = -1;
-		peasycap->audio_altsetting_on = -1;
-		peasycap->audio_altsetting_off = -1;
-		peasycap->audio_endpointnumber = -1;
-		peasycap->audio_isoc_maxframesize = -1;
-		peasycap->audio_isoc_buffer_size = -1;
-
-		peasycap->frame_buffer_many = FRAME_BUFFER_MANY;
+		/* Perform basic struct initialization */
+		init_easycap(peasycap, usbdev, intf, bInterfaceNumber);
 
 		/* Dynamically fill in the available formats */
 		rc = easycap_video_fillin_formats();
@@ -2996,103 +3133,14 @@ static int easycap_usb_probe(struct usb_interface *intf,
 		JOM(4, "%i formats available\n", rc);
 
 		/* Populate easycap.inputset[] */
-		inputset = peasycap->inputset;
-		fmtidx = peasycap->ntsc ? NTSC_M : PAL_BGHIN;
-		m = 0;
-		mask = 0;
-		for (i = 0; 0xFFFF != easycap_standard[i].mask; i++) {
-			if (fmtidx == easycap_standard[i].v4l2_standard.index) {
-				m++;
-				for (k = 0; k < INPUT_MANY; k++)
-					inputset[k].standard_offset = i;
-
-				mask = easycap_standard[i].mask;
-			}
-		}
-		if (1 != m) {
-			SAM("ERROR: "
-			    "inputset->standard_offset unpopulated, %i=m\n", m);
-			return -ENOENT;
-		}
-
-		peasycap_format = &easycap_format[0];
-		m = 0;
-		for (i = 0; peasycap_format->v4l2_format.fmt.pix.width; i++) {
-			struct v4l2_pix_format *pix =
-				&peasycap_format->v4l2_format.fmt.pix;
-			if (((peasycap_format->mask & 0x0F) == (mask & 0x0F)) &&
-			    pix->field == V4L2_FIELD_NONE &&
-			    pix->pixelformat == V4L2_PIX_FMT_UYVY &&
-			    pix->width  == 640 && pix->height == 480) {
-				m++;
-				for (k = 0; k < INPUT_MANY; k++)
-					inputset[k].format_offset = i;
-				break;
-			}
-			peasycap_format++;
-		}
-		if (1 != m) {
-			SAM("ERROR: inputset[]->format_offset unpopulated\n");
-			return -ENOENT;
-		}
-
-		m = 0;
-		for (i = 0; 0xFFFFFFFF != easycap_control[i].id; i++) {
-			value = easycap_control[i].default_value;
-			if (V4L2_CID_BRIGHTNESS == easycap_control[i].id) {
-				m++;
-				for (k = 0; k < INPUT_MANY; k++)
-					inputset[k].brightness = value;
-			} else if (V4L2_CID_CONTRAST == easycap_control[i].id) {
-				m++;
-				for (k = 0; k < INPUT_MANY; k++)
-					inputset[k].contrast = value;
-			} else if (V4L2_CID_SATURATION == easycap_control[i].id) {
-				m++;
-				for (k = 0; k < INPUT_MANY; k++)
-					inputset[k].saturation = value;
-			} else if (V4L2_CID_HUE == easycap_control[i].id) {
-				m++;
-				for (k = 0; k < INPUT_MANY; k++)
-					inputset[k].hue = value;
-			}
-		}
-
-		if (4 != m) {
-			SAM("ERROR: inputset[]->brightness underpopulated\n");
-			return -ENOENT;
-		}
-		for (k = 0; k < INPUT_MANY; k++)
-			inputset[k].input = k;
-		JOM(4, "populated inputset[]\n");
+		rc = populate_inputset(peasycap);
+		if (rc < 0)
+			return rc;
 		JOM(4, "finished initialization\n");
 	} else {
-
-		/*
-		 * FIXME: Identify the appropriate pointer
-		 * peasycap for interfaces 1 and 2.
-		 * The address of peasycap->pusb_device
-		 * is reluctantly used for this purpose.
-		 */
-		for (ndong = 0; ndong < DONGLE_MANY; ndong++) {
-			if (usbdev == easycapdc60_dongle[ndong].peasycap->
-									pusb_device) {
-				peasycap = easycapdc60_dongle[ndong].peasycap;
-				JOT(8, "intf[%i]: dongle[%i].peasycap\n",
-						bInterfaceNumber, ndong);
-				break;
-			}
-		}
-		if (DONGLE_MANY <= ndong) {
-			SAY("ERROR: peasycap is unknown when probing interface %i\n",
-								bInterfaceNumber);
+		peasycap = get_easycap(usbdev, bInterfaceNumber);
+		if (!peasycap)
 			return -ENODEV;
-		}
-		if (!peasycap) {
-			SAY("ERROR: peasycap is NULL when probing interface %i\n",
-								bInterfaceNumber);
-			return -ENODEV;
-		}
 	}
 
 	if ((USB_CLASS_VIDEO == bInterfaceClass) ||
-- 
1.7.3.4

[PATCH 2/9] staging: easycap: Split buffer and video urb allocation

[Ezequiel Garcia]

When the device is probed, this driver allocates
frame buffers, field buffers, isoc buffers and urbs.
This patch just split this into separate functions,
which helps clearing the currently gigantic probe function.

Signed-off-by: Ezequiel Garcia <elezegarcia@gmail.com>
---
 drivers/staging/media/easycap/easycap_main.c |  375 +++++++++++++++-----------
 1 files changed, 211 insertions(+), 164 deletions(-)

diff --git a/drivers/staging/media/easycap/easycap_main.c b/drivers/staging/media/easycap/easycap_main.c
index c4198be..6d7cdef 100644
--- a/drivers/staging/media/easycap/easycap_main.c
+++ b/drivers/staging/media/easycap/easycap_main.c
@@ -3045,6 +3045,203 @@ static int populate_inputset(struct easycap *peasycap)
 	return 0;
 }
 
+static int alloc_framebuffers(struct easycap *peasycap)
+{
+	int i, j;
+	void *pbuf;
+
+	JOM(4, "allocating %i frame buffers of size %li\n",
+			FRAME_BUFFER_MANY, (long int)FRAME_BUFFER_SIZE);
+	JOM(4, ".... each scattered over %li pages\n",
+			FRAME_BUFFER_SIZE/PAGE_SIZE);
+
+	for (i = 0; i < FRAME_BUFFER_MANY; i++) {
+		for (j = 0; j < FRAME_BUFFER_SIZE/PAGE_SIZE; j++) {
+			if (peasycap->frame_buffer[i][j].pgo)
+				SAM("attempting to reallocate framebuffers\n");
+			else {
+				pbuf = (void *)__get_free_page(GFP_KERNEL);
+				if (!pbuf) {
+					SAM("ERROR: Could not allocate "
+					"framebuffer %i page %i\n", i, j);
+					return -ENOMEM;
+				}
+				peasycap->allocation_video_page += 1;
+				peasycap->frame_buffer[i][j].pgo = pbuf;
+			}
+			peasycap->frame_buffer[i][j].pto =
+			    peasycap->frame_buffer[i][j].pgo;
+		}
+	}
+
+	peasycap->frame_fill = 0;
+	peasycap->frame_read = 0;
+	JOM(4, "allocation of frame buffers done: %i pages\n", i*j);
+
+	return 0;
+}
+
+static int alloc_fieldbuffers(struct easycap *peasycap)
+{
+	int i, j;
+	void *pbuf;
+
+	JOM(4, "allocating %i field buffers of size %li\n",
+			FIELD_BUFFER_MANY, (long int)FIELD_BUFFER_SIZE);
+	JOM(4, ".... each scattered over %li pages\n",
+			FIELD_BUFFER_SIZE/PAGE_SIZE);
+
+	for (i = 0; i < FIELD_BUFFER_MANY; i++) {
+		for (j = 0; j < FIELD_BUFFER_SIZE/PAGE_SIZE; j++) {
+			if (peasycap->field_buffer[i][j].pgo) {
+				SAM("ERROR: attempting to reallocate "
+					"fieldbuffers\n");
+			} else {
+				pbuf = (void *) __get_free_page(GFP_KERNEL);
+				if (!pbuf) {
+					SAM("ERROR: Could not allocate "
+					"fieldbuffer %i page %i\n", i, j);
+					return -ENOMEM;
+				}
+				peasycap->allocation_video_page += 1;
+				peasycap->field_buffer[i][j].pgo = pbuf;
+			}
+			peasycap->field_buffer[i][j].pto =
+				peasycap->field_buffer[i][j].pgo;
+		}
+		/* TODO: Hardcoded 0x0200 meaning? */
+		peasycap->field_buffer[i][0].kount = 0x0200;
+	}
+	peasycap->field_fill = 0;
+	peasycap->field_page = 0;
+	peasycap->field_read = 0;
+	JOM(4, "allocation of field buffers done:  %i pages\n", i*j);
+
+	return 0;
+}
+
+static int alloc_isocbuffers(struct easycap *peasycap)
+{
+	int i;
+	void *pbuf;
+
+	JOM(4, "allocating %i isoc video buffers of size %i\n",
+			VIDEO_ISOC_BUFFER_MANY,
+			peasycap->video_isoc_buffer_size);
+	JOM(4, ".... each occupying contiguous memory pages\n");
+
+	for (i = 0; i < VIDEO_ISOC_BUFFER_MANY; i++) {
+		pbuf = (void *)__get_free_pages(GFP_KERNEL,
+				VIDEO_ISOC_ORDER);
+		if (!pbuf) {
+			SAM("ERROR: Could not allocate isoc "
+				"video buffer %i\n", i);
+			return -ENOMEM;
+		}
+		peasycap->allocation_video_page += BIT(VIDEO_ISOC_ORDER);
+
+		peasycap->video_isoc_buffer[i].pgo = pbuf;
+		peasycap->video_isoc_buffer[i].pto =
+			pbuf + peasycap->video_isoc_buffer_size;
+		peasycap->video_isoc_buffer[i].kount = i;
+	}
+	JOM(4, "allocation of isoc video buffers done: %i pages\n",
+			i * (0x01 << VIDEO_ISOC_ORDER));
+	return 0;
+}
+
+static int create_video_urbs(struct easycap *peasycap)
+{
+	struct urb *purb;
+	struct data_urb *pdata_urb;
+	int i, j;
+
+	JOM(4, "allocating %i struct urb.\n", VIDEO_ISOC_BUFFER_MANY);
+	JOM(4, "using %i=peasycap->video_isoc_framesperdesc\n",
+			peasycap->video_isoc_framesperdesc);
+	JOM(4, "using %i=peasycap->video_isoc_maxframesize\n",
+			peasycap->video_isoc_maxframesize);
+	JOM(4, "using %i=peasycap->video_isoc_buffer_sizen",
+			peasycap->video_isoc_buffer_size);
+
+	for (i = 0; i < VIDEO_ISOC_BUFFER_MANY; i++) {
+		purb = usb_alloc_urb(peasycap->video_isoc_framesperdesc,
+				GFP_KERNEL);
+		if (!purb) {
+			SAM("ERROR: usb_alloc_urb returned NULL for buffer "
+				"%i\n", i);
+			return -ENOMEM;
+		}
+
+		peasycap->allocation_video_urb += 1;
+		pdata_urb = kzalloc(sizeof(struct data_urb), GFP_KERNEL);
+		if (!pdata_urb) {
+			SAM("ERROR: Could not allocate struct data_urb.\n");
+			return -ENOMEM;
+		}
+
+		peasycap->allocation_video_struct +=
+			sizeof(struct data_urb);
+
+		pdata_urb->purb = purb;
+		pdata_urb->isbuf = i;
+		pdata_urb->length = 0;
+		list_add_tail(&(pdata_urb->list_head),
+				peasycap->purb_video_head);
+
+		if (!i) {
+			JOM(4, "initializing video urbs thus:\n");
+			JOM(4, "  purb->interval = 1;\n");
+			JOM(4, "  purb->dev = peasycap->pusb_device;\n");
+			JOM(4, "  purb->pipe = usb_rcvisocpipe"
+					"(peasycap->pusb_device,%i);\n",
+					peasycap->video_endpointnumber);
+			JOM(4, "  purb->transfer_flags = URB_ISO_ASAP;\n");
+			JOM(4, "  purb->transfer_buffer = peasycap->"
+					"video_isoc_buffer[.].pgo;\n");
+			JOM(4, "  purb->transfer_buffer_length = %i;\n",
+					peasycap->video_isoc_buffer_size);
+			JOM(4, "  purb->complete = easycap_complete;\n");
+			JOM(4, "  purb->context = peasycap;\n");
+			JOM(4, "  purb->start_frame = 0;\n");
+			JOM(4, "  purb->number_of_packets = %i;\n",
+					peasycap->video_isoc_framesperdesc);
+			JOM(4, "  for (j = 0; j < %i; j++)\n",
+					peasycap->video_isoc_framesperdesc);
+			JOM(4, "    {\n");
+			JOM(4, "    purb->iso_frame_desc[j].offset = j*%i;\n",
+					peasycap->video_isoc_maxframesize);
+			JOM(4, "    purb->iso_frame_desc[j].length = %i;\n",
+					peasycap->video_isoc_maxframesize);
+			JOM(4, "    }\n");
+		}
+
+		purb->interval = 1;
+		purb->dev = peasycap->pusb_device;
+		purb->pipe = usb_rcvisocpipe(peasycap->pusb_device,
+				peasycap->video_endpointnumber);
+
+		purb->transfer_flags = URB_ISO_ASAP;
+		purb->transfer_buffer = peasycap->video_isoc_buffer[i].pgo;
+		purb->transfer_buffer_length =
+			peasycap->video_isoc_buffer_size;
+
+		purb->complete = easycap_complete;
+		purb->context = peasycap;
+		purb->start_frame = 0;
+		purb->number_of_packets = peasycap->video_isoc_framesperdesc;
+
+		for (j = 0; j < peasycap->video_isoc_framesperdesc; j++) {
+			purb->iso_frame_desc[j].offset =
+				j * peasycap->video_isoc_maxframesize;
+			purb->iso_frame_desc[j].length =
+				peasycap->video_isoc_maxframesize;
+		}
+	}
+	JOM(4, "allocation of %i struct urb done.\n", i);
+	return 0;
+}
+
 static const struct v4l2_file_operations v4l2_fops = {
 	.owner		= THIS_MODULE,
 	.open		= easycap_open_noinode,
@@ -3067,7 +3264,7 @@ static int easycap_usb_probe(struct usb_interface *intf,
 	struct urb *purb;
 	struct easycap *peasycap;
 	struct data_urb *pdata_urb;
-	int i, j, k, m, rc;
+	int i, j, k, rc;
 	u8 bInterfaceNumber;
 	u8 bInterfaceClass;
 	u8 bInterfaceSubClass;
@@ -3416,173 +3613,23 @@ static int easycap_usb_probe(struct usb_interface *intf,
 		 */
 		INIT_LIST_HEAD(&(peasycap->urb_video_head));
 		peasycap->purb_video_head = &(peasycap->urb_video_head);
-		JOM(4, "allocating %i frame buffers of size %li\n",
-				FRAME_BUFFER_MANY, (long int)FRAME_BUFFER_SIZE);
-		JOM(4, ".... each scattered over %li pages\n",
-							FRAME_BUFFER_SIZE/PAGE_SIZE);
-
-		for (k = 0;  k < FRAME_BUFFER_MANY;  k++) {
-			for (m = 0;  m < FRAME_BUFFER_SIZE/PAGE_SIZE;  m++) {
-				if (peasycap->frame_buffer[k][m].pgo)
-					SAM("attempting to reallocate frame "
-									" buffers\n");
-				else {
-					pbuf = (void *)__get_free_page(GFP_KERNEL);
-					if (!pbuf) {
-						SAM("ERROR: Could not allocate frame "
-							"buffer %i page %i\n", k, m);
-						return -ENOMEM;
-					}
-
-					peasycap->allocation_video_page += 1;
-					peasycap->frame_buffer[k][m].pgo = pbuf;
-				}
-				peasycap->frame_buffer[k][m].pto =
-						peasycap->frame_buffer[k][m].pgo;
-			}
-		}
 
-		peasycap->frame_fill = 0;
-		peasycap->frame_read = 0;
-		JOM(4, "allocation of frame buffers done:  %i pages\n", k *
-									m);
-		JOM(4, "allocating %i field buffers of size %li\n",
-				FIELD_BUFFER_MANY, (long int)FIELD_BUFFER_SIZE);
-		JOM(4, ".... each scattered over %li pages\n",
-						FIELD_BUFFER_SIZE/PAGE_SIZE);
-
-		for (k = 0;  k < FIELD_BUFFER_MANY;  k++) {
-			for (m = 0;  m < FIELD_BUFFER_SIZE/PAGE_SIZE;  m++) {
-				if (peasycap->field_buffer[k][m].pgo) {
-					SAM("ERROR: attempting to reallocate "
-								"field buffers\n");
-				} else {
-					pbuf = (void *) __get_free_page(GFP_KERNEL);
-					if (!pbuf) {
-						SAM("ERROR: Could not allocate field"
-							" buffer %i page %i\n", k, m);
-						return -ENOMEM;
-					}
-
-					peasycap->allocation_video_page += 1;
-					peasycap->field_buffer[k][m].pgo = pbuf;
-				}
-				peasycap->field_buffer[k][m].pto =
-						peasycap->field_buffer[k][m].pgo;
-			}
-			peasycap->field_buffer[k][0].kount = 0x0200;
-		}
-		peasycap->field_fill = 0;
-		peasycap->field_page = 0;
-		peasycap->field_read = 0;
-		JOM(4, "allocation of field buffers done:  %i pages\n", k *
-									m);
-		JOM(4, "allocating %i isoc video buffers of size %i\n",
-						VIDEO_ISOC_BUFFER_MANY,
-						peasycap->video_isoc_buffer_size);
-		JOM(4, ".... each occupying contiguous memory pages\n");
-
-		for (k = 0;  k < VIDEO_ISOC_BUFFER_MANY; k++) {
-			pbuf = (void *)__get_free_pages(GFP_KERNEL,
-							VIDEO_ISOC_ORDER);
-			if (!pbuf) {
-				SAM("ERROR: Could not allocate isoc video buffer "
-									"%i\n", k);
-				return -ENOMEM;
-			}
-			peasycap->allocation_video_page +=
-						BIT(VIDEO_ISOC_ORDER);
-
-			peasycap->video_isoc_buffer[k].pgo = pbuf;
-			peasycap->video_isoc_buffer[k].pto =
-				pbuf + peasycap->video_isoc_buffer_size;
-			peasycap->video_isoc_buffer[k].kount = k;
-		}
-		JOM(4, "allocation of isoc video buffers done: %i pages\n",
-						k * (0x01 << VIDEO_ISOC_ORDER));
-
-		/* Allocate and initialize multiple struct usb */
-		JOM(4, "allocating %i struct urb.\n", VIDEO_ISOC_BUFFER_MANY);
-		JOM(4, "using %i=peasycap->video_isoc_framesperdesc\n",
-						peasycap->video_isoc_framesperdesc);
-		JOM(4, "using %i=peasycap->video_isoc_maxframesize\n",
-						peasycap->video_isoc_maxframesize);
-		JOM(4, "using %i=peasycap->video_isoc_buffer_sizen",
-						peasycap->video_isoc_buffer_size);
-
-		for (k = 0;  k < VIDEO_ISOC_BUFFER_MANY; k++) {
-			purb = usb_alloc_urb(peasycap->video_isoc_framesperdesc,
-									GFP_KERNEL);
-			if (!purb) {
-				SAM("ERROR: usb_alloc_urb returned NULL for buffer "
-									"%i\n", k);
-				return -ENOMEM;
-			}
-
-			peasycap->allocation_video_urb += 1;
-			pdata_urb = kzalloc(sizeof(struct data_urb), GFP_KERNEL);
-			if (!pdata_urb) {
-				SAM("ERROR: Could not allocate struct data_urb.\n");
-				return -ENOMEM;
-			}
-
-			peasycap->allocation_video_struct +=
-							sizeof(struct data_urb);
+		rc = alloc_framebuffers(peasycap);
+		if (rc < 0)
+			return rc;
 
-			pdata_urb->purb = purb;
-			pdata_urb->isbuf = k;
-			pdata_urb->length = 0;
-			list_add_tail(&(pdata_urb->list_head),
-							peasycap->purb_video_head);
+		rc = alloc_fieldbuffers(peasycap);
+		if (rc < 0)
+			return rc;
 
-			/* Initialize allocated urbs */
-			if (!k) {
-				JOM(4, "initializing video urbs thus:\n");
-				JOM(4, "  purb->interval = 1;\n");
-				JOM(4, "  purb->dev = peasycap->pusb_device;\n");
-				JOM(4, "  purb->pipe = usb_rcvisocpipe"
-						"(peasycap->pusb_device,%i);\n",
-						peasycap->video_endpointnumber);
-				JOM(4, "  purb->transfer_flags = URB_ISO_ASAP;\n");
-				JOM(4, "  purb->transfer_buffer = peasycap->"
-						"video_isoc_buffer[.].pgo;\n");
-				JOM(4, "  purb->transfer_buffer_length = %i;\n",
-						peasycap->video_isoc_buffer_size);
-				JOM(4, "  purb->complete = easycap_complete;\n");
-				JOM(4, "  purb->context = peasycap;\n");
-				JOM(4, "  purb->start_frame = 0;\n");
-				JOM(4, "  purb->number_of_packets = %i;\n",
-						peasycap->video_isoc_framesperdesc);
-				JOM(4, "  for (j = 0; j < %i; j++)\n",
-						peasycap->video_isoc_framesperdesc);
-				JOM(4, "    {\n");
-				JOM(4, "    purb->iso_frame_desc[j].offset = j*%i;\n",
-						peasycap->video_isoc_maxframesize);
-				JOM(4, "    purb->iso_frame_desc[j].length = %i;\n",
-						peasycap->video_isoc_maxframesize);
-				JOM(4, "    }\n");
-			}
+		rc = alloc_isocbuffers(peasycap);
+		if (rc < 0)
+			return rc;
 
-			purb->interval = 1;
-			purb->dev = peasycap->pusb_device;
-			purb->pipe = usb_rcvisocpipe(peasycap->pusb_device,
-						peasycap->video_endpointnumber);
-			purb->transfer_flags = URB_ISO_ASAP;
-			purb->transfer_buffer = peasycap->video_isoc_buffer[k].pgo;
-			purb->transfer_buffer_length =
-						peasycap->video_isoc_buffer_size;
-			purb->complete = easycap_complete;
-			purb->context = peasycap;
-			purb->start_frame = 0;
-			purb->number_of_packets = peasycap->video_isoc_framesperdesc;
-			for (j = 0;  j < peasycap->video_isoc_framesperdesc; j++) {
-				purb->iso_frame_desc[j].offset = j *
-						peasycap->video_isoc_maxframesize;
-				purb->iso_frame_desc[j].length =
-						peasycap->video_isoc_maxframesize;
-			}
-		}
-		JOM(4, "allocation of %i struct urb done.\n", k);
+		/* Allocate and initialize video urbs */
+		rc = create_video_urbs(peasycap);
+		if (rc < 0)
+			return rc;
 
 		/* Save pointer peasycap in this interface */
 		usb_set_intfdata(intf, peasycap);
-- 
1.7.3.4

[PATCH 3/9] staging: easycap: Push bInterfaceNumber saving to config_easycap()

[Ezequiel Garcia]

Signed-off-by: Ezequiel Garcia <elezegarcia@gmail.com>
---
 drivers/staging/media/easycap/easycap_main.c |   72 +++++++++++++++-----------
 1 files changed, 41 insertions(+), 31 deletions(-)

diff --git a/drivers/staging/media/easycap/easycap_main.c b/drivers/staging/media/easycap/easycap_main.c
index 6d7cdef..9d6dc09 100644
--- a/drivers/staging/media/easycap/easycap_main.c
+++ b/drivers/staging/media/easycap/easycap_main.c
@@ -3242,6 +3242,44 @@ static int create_video_urbs(struct easycap *peasycap)
 	return 0;
 }
 
+static void config_easycap(struct easycap *peasycap,
+			   u8 bInterfaceNumber,
+			   u8 bInterfaceClass,
+			   u8 bInterfaceSubClass)
+{
+	if ((USB_CLASS_VIDEO == bInterfaceClass) ||
+	    (USB_CLASS_VENDOR_SPEC == bInterfaceClass)) {
+		if (-1 == peasycap->video_interface) {
+			peasycap->video_interface = bInterfaceNumber;
+			JOM(4, "setting peasycap->video_interface=%i\n",
+				peasycap->video_interface);
+		} else {
+			if (peasycap->video_interface != bInterfaceNumber) {
+				SAM("ERROR: attempting to reset "
+				    "peasycap->video_interface\n");
+				SAM("...... continuing with "
+				    "%i=peasycap->video_interface\n",
+				    peasycap->video_interface);
+			}
+		}
+	} else if ((USB_CLASS_AUDIO == bInterfaceClass) &&
+		   (USB_SUBCLASS_AUDIOSTREAMING == bInterfaceSubClass)) {
+		if (-1 == peasycap->audio_interface) {
+			peasycap->audio_interface = bInterfaceNumber;
+			JOM(4, "setting peasycap->audio_interface=%i\n",
+				peasycap->audio_interface);
+		} else {
+			if (peasycap->audio_interface != bInterfaceNumber) {
+				SAM("ERROR: attempting to reset "
+				    "peasycap->audio_interface\n");
+				SAM("...... continuing with "
+				    "%i=peasycap->audio_interface\n",
+				    peasycap->audio_interface);
+			}
+		}
+	}
+}
+
 static const struct v4l2_file_operations v4l2_fops = {
 	.owner		= THIS_MODULE,
 	.open		= easycap_open_noinode,
@@ -3340,37 +3378,9 @@ static int easycap_usb_probe(struct usb_interface *intf,
 			return -ENODEV;
 	}
 
-	if ((USB_CLASS_VIDEO == bInterfaceClass) ||
-	    (USB_CLASS_VENDOR_SPEC == bInterfaceClass)) {
-		if (-1 == peasycap->video_interface) {
-			peasycap->video_interface = bInterfaceNumber;
-			JOM(4, "setting peasycap->video_interface=%i\n",
-							peasycap->video_interface);
-		} else {
-			if (peasycap->video_interface != bInterfaceNumber) {
-				SAM("ERROR: attempting to reset "
-						"peasycap->video_interface\n");
-				SAM("...... continuing with "
-						"%i=peasycap->video_interface\n",
-						peasycap->video_interface);
-			}
-		}
-	} else if ((USB_CLASS_AUDIO == bInterfaceClass) &&
-		   (USB_SUBCLASS_AUDIOSTREAMING == bInterfaceSubClass)) {
-		if (-1 == peasycap->audio_interface) {
-			peasycap->audio_interface = bInterfaceNumber;
-			JOM(4, "setting peasycap->audio_interface=%i\n",
-							 peasycap->audio_interface);
-		} else {
-			if (peasycap->audio_interface != bInterfaceNumber) {
-				SAM("ERROR: attempting to reset "
-						"peasycap->audio_interface\n");
-				SAM("...... continuing with "
-						"%i=peasycap->audio_interface\n",
-						peasycap->audio_interface);
-			}
-		}
-	}
+	config_easycap(peasycap, bInterfaceNumber,
+				 bInterfaceClass,
+				 bInterfaceSubClass);
 
 	/*
 	 * Investigate all altsettings. This is done in detail
-- 
1.7.3.4

[PATCH 4/9] staging: easycap: Initialize 'ntsc' parameter before usage

[Ezequiel Garcia]

This parameter is now initialized at init_easycap(),
this way we assure it won't be used uninitialized.

Signed-off-by: Ezequiel Garcia <elezegarcia@gmail.com>
---
 drivers/staging/media/easycap/easycap_main.c |    8 ++++----
 1 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/staging/media/easycap/easycap_main.c b/drivers/staging/media/easycap/easycap_main.c
index 9d6dc09..480164d 100644
--- a/drivers/staging/media/easycap/easycap_main.c
+++ b/drivers/staging/media/easycap/easycap_main.c
@@ -2960,6 +2960,10 @@ static void init_easycap(struct easycap *peasycap,
 	peasycap->audio_isoc_buffer_size = -1;
 
 	peasycap->frame_buffer_many = FRAME_BUFFER_MANY;
+
+	peasycap->ntsc = easycap_ntsc;
+	JOM(8, "defaulting initially to %s\n",
+		easycap_ntsc ? "NTSC" : "PAL");
 }
 
 static int populate_inputset(struct easycap *peasycap)
@@ -2972,7 +2976,6 @@ static int populate_inputset(struct easycap *peasycap)
 
 	inputset = peasycap->inputset;
 
-	/* FIXME: peasycap->ntsc is not yet initialized */
 	fmtidx = peasycap->ntsc ? NTSC_M : PAL_BGHIN;
 
 	m = 0;
@@ -3650,9 +3653,6 @@ static int easycap_usb_probe(struct usb_interface *intf,
 		 * because some udev rules triggers easycap_open()
 		 * immediately after registration, causing a clash.
 		 */
-		peasycap->ntsc = easycap_ntsc;
-		JOM(8, "defaulting initially to %s\n",
-			easycap_ntsc ? "NTSC" : "PAL");
 		rc = reset(peasycap);
 		if (rc) {
 			SAM("ERROR: reset() rc = %i\n", rc);
-- 
1.7.3.4

[PATCH 5/9] staging: easycap: Push video registration to easycap_register_video()

[Ezequiel Garcia]

Signed-off-by: Ezequiel Garcia <elezegarcia@gmail.com>
---
 drivers/staging/media/easycap/easycap_main.c |   58 +++++++++++++++-----------
 1 files changed, 33 insertions(+), 25 deletions(-)

diff --git a/drivers/staging/media/easycap/easycap_main.c b/drivers/staging/media/easycap/easycap_main.c
index 480164d..68af1a2 100644
--- a/drivers/staging/media/easycap/easycap_main.c
+++ b/drivers/staging/media/easycap/easycap_main.c
@@ -3291,6 +3291,37 @@ static const struct v4l2_file_operations v4l2_fops = {
 	.mmap		= easycap_mmap,
 };
 
+static int easycap_register_video(struct easycap *peasycap)
+{
+	/*
+	 * FIXME: This is believed to be harmless,
+	 * but may well be unnecessary or wrong.
+	 */
+	peasycap->video_device.v4l2_dev = NULL;
+
+	strcpy(&peasycap->video_device.name[0], "easycapdc60");
+	peasycap->video_device.fops = &v4l2_fops;
+	peasycap->video_device.minor = -1;
+	peasycap->video_device.release = (void *)(&videodev_release);
+
+	video_set_drvdata(&(peasycap->video_device), (void *)peasycap);
+
+	if (0 != (video_register_device(&(peasycap->video_device),
+					VFL_TYPE_GRABBER, -1))) {
+		err("Not able to register with videodev");
+		videodev_release(&(peasycap->video_device));
+		return -ENODEV;
+	}
+
+	peasycap->registered_video++;
+
+	SAM("registered with videodev: %i=minor\n",
+	    peasycap->video_device.minor);
+	    peasycap->minor = peasycap->video_device.minor;
+
+	return 0;
+}
+
 /*
  * When the device is plugged, this function is called three times,
  * one for each interface.
@@ -3667,32 +3698,9 @@ static int easycap_usb_probe(struct usb_interface *intf,
 		JOM(4, "registered device instance: %s\n",
 			peasycap->v4l2_device.name);
 
-		/*
-		 * FIXME: This is believed to be harmless,
-		 * but may well be unnecessary or wrong.
-		 */
-		peasycap->video_device.v4l2_dev = NULL;
-
-
-		strcpy(&peasycap->video_device.name[0], "easycapdc60");
-		peasycap->video_device.fops = &v4l2_fops;
-		peasycap->video_device.minor = -1;
-		peasycap->video_device.release = (void *)(&videodev_release);
-
-		video_set_drvdata(&(peasycap->video_device), (void *)peasycap);
-
-		if (0 != (video_register_device(&(peasycap->video_device),
-							VFL_TYPE_GRABBER, -1))) {
-			err("Not able to register with videodev");
-			videodev_release(&(peasycap->video_device));
+		rc = easycap_register_video(peasycap);
+		if (rc < 0)
 			return -ENODEV;
-		}
-
-		peasycap->registered_video++;
-		SAM("registered with videodev: %i=minor\n",
-						peasycap->video_device.minor);
-		peasycap->minor = peasycap->video_device.minor;
-
 		break;
 	}
 	/* 1: Audio control */
-- 
1.7.3.4

[PATCH 6/9] staging: easycap: Split audio buffer and urb allocation

[Ezequiel Garcia]

When the device is probed, this driver allocates
audio buffers, and audio urbs.
This patch just split this into separate functions,
which helps clearing the currently gigantic probe function.

Signed-off-by: Ezequiel Garcia <elezegarcia@gmail.com>
---
 drivers/staging/media/easycap/easycap_main.c |  229 ++++++++++++++------------
 1 files changed, 124 insertions(+), 105 deletions(-)

diff --git a/drivers/staging/media/easycap/easycap_main.c b/drivers/staging/media/easycap/easycap_main.c
index 68af1a2..6e1734d 100644
--- a/drivers/staging/media/easycap/easycap_main.c
+++ b/drivers/staging/media/easycap/easycap_main.c
@@ -3245,6 +3245,123 @@ static int create_video_urbs(struct easycap *peasycap)
 	return 0;
 }
 
+static int alloc_audio_buffers(struct easycap *peasycap)
+{
+	void *pbuf;
+	int k;
+
+	JOM(4, "allocating %i isoc audio buffers of size %i\n",
+		AUDIO_ISOC_BUFFER_MANY,
+		peasycap->audio_isoc_buffer_size);
+	JOM(4, ".... each occupying contiguous memory pages\n");
+
+	for (k = 0;  k < AUDIO_ISOC_BUFFER_MANY;  k++) {
+		pbuf = (void *)__get_free_pages(GFP_KERNEL, AUDIO_ISOC_ORDER);
+		if (!pbuf) {
+			SAM("ERROR: Could not allocate isoc audio buffer %i\n",
+			    k);
+				return -ENOMEM;
+		}
+		peasycap->allocation_audio_page += BIT(AUDIO_ISOC_ORDER);
+
+		peasycap->audio_isoc_buffer[k].pgo = pbuf;
+		peasycap->audio_isoc_buffer[k].pto =
+			pbuf + peasycap->audio_isoc_buffer_size;
+		peasycap->audio_isoc_buffer[k].kount = k;
+	}
+
+	JOM(4, "allocation of isoc audio buffers done.\n");
+	return 0;
+}
+
+static int create_audio_urbs(struct easycap *peasycap)
+{
+	struct urb *purb;
+	struct data_urb *pdata_urb;
+	int k, j;
+
+	JOM(4, "allocating %i struct urb.\n", AUDIO_ISOC_BUFFER_MANY);
+	JOM(4, "using %i=peasycap->audio_isoc_framesperdesc\n",
+		peasycap->audio_isoc_framesperdesc);
+	JOM(4, "using %i=peasycap->audio_isoc_maxframesize\n",
+		peasycap->audio_isoc_maxframesize);
+	JOM(4, "using %i=peasycap->audio_isoc_buffer_size\n",
+		peasycap->audio_isoc_buffer_size);
+
+	for (k = 0;  k < AUDIO_ISOC_BUFFER_MANY; k++) {
+		purb = usb_alloc_urb(peasycap->audio_isoc_framesperdesc,
+				     GFP_KERNEL);
+		if (!purb) {
+			SAM("ERROR: usb_alloc_urb returned NULL for buffer "
+			     "%i\n", k);
+			return -ENOMEM;
+		}
+		peasycap->allocation_audio_urb += 1 ;
+		pdata_urb = kzalloc(sizeof(struct data_urb), GFP_KERNEL);
+		if (!pdata_urb) {
+			usb_free_urb(purb);
+			SAM("ERROR: Could not allocate struct data_urb.\n");
+			return -ENOMEM;
+		}
+		peasycap->allocation_audio_struct +=
+			sizeof(struct data_urb);
+
+		pdata_urb->purb = purb;
+		pdata_urb->isbuf = k;
+		pdata_urb->length = 0;
+		list_add_tail(&(pdata_urb->list_head),
+				peasycap->purb_audio_head);
+
+		if (!k) {
+			JOM(4, "initializing audio urbs thus:\n");
+			JOM(4, "  purb->interval = 1;\n");
+			JOM(4, "  purb->dev = peasycap->pusb_device;\n");
+			JOM(4, "  purb->pipe = usb_rcvisocpipe(peasycap->"
+				"pusb_device,%i);\n",
+				peasycap->audio_endpointnumber);
+			JOM(4, "  purb->transfer_flags = URB_ISO_ASAP;\n");
+			JOM(4, "  purb->transfer_buffer = "
+				"peasycap->audio_isoc_buffer[.].pgo;\n");
+			JOM(4, "  purb->transfer_buffer_length = %i;\n",
+				peasycap->audio_isoc_buffer_size);
+			JOM(4, "  purb->complete = easycap_alsa_complete;\n");
+			JOM(4, "  purb->context = peasycap;\n");
+			JOM(4, "  purb->start_frame = 0;\n");
+			JOM(4, "  purb->number_of_packets = %i;\n",
+				peasycap->audio_isoc_framesperdesc);
+			JOM(4, "  for (j = 0; j < %i; j++)\n",
+				peasycap->audio_isoc_framesperdesc);
+			JOM(4, "    {\n");
+			JOM(4, "    purb->iso_frame_desc[j].offset = j*%i;\n",
+				peasycap->audio_isoc_maxframesize);
+			JOM(4, "    purb->iso_frame_desc[j].length = %i;\n",
+				peasycap->audio_isoc_maxframesize);
+			JOM(4, "    }\n");
+		}
+
+		purb->interval = 1;
+		purb->dev = peasycap->pusb_device;
+		purb->pipe = usb_rcvisocpipe(peasycap->pusb_device,
+					     peasycap->audio_endpointnumber);
+		purb->transfer_flags = URB_ISO_ASAP;
+		purb->transfer_buffer = peasycap->audio_isoc_buffer[k].pgo;
+		purb->transfer_buffer_length =
+			peasycap->audio_isoc_buffer_size;
+		purb->complete = easycap_alsa_complete;
+		purb->context = peasycap;
+		purb->start_frame = 0;
+		purb->number_of_packets = peasycap->audio_isoc_framesperdesc;
+		for (j = 0;  j < peasycap->audio_isoc_framesperdesc; j++) {
+			purb->iso_frame_desc[j].offset =
+				j * peasycap->audio_isoc_maxframesize;
+			purb->iso_frame_desc[j].length =
+				peasycap->audio_isoc_maxframesize;
+		}
+	}
+	JOM(4, "allocation of %i struct urb done.\n", k);
+	return 0;
+}
+
 static void config_easycap(struct easycap *peasycap,
 			   u8 bInterfaceNumber,
 			   u8 bInterfaceClass,
@@ -3333,14 +3450,11 @@ static int easycap_usb_probe(struct usb_interface *intf,
 	struct usb_host_interface *alt;
 	struct usb_endpoint_descriptor *ep;
 	struct usb_interface_descriptor *interface;
-	struct urb *purb;
 	struct easycap *peasycap;
-	struct data_urb *pdata_urb;
-	int i, j, k, rc;
+	int i, j, rc;
 	u8 bInterfaceNumber;
 	u8 bInterfaceClass;
 	u8 bInterfaceSubClass;
-	void *pbuf;
 	int okalt[8], isokalt;
 	int okepn[8];
 	int okmps[8];
@@ -3823,109 +3937,14 @@ static int easycap_usb_probe(struct usb_interface *intf,
 		INIT_LIST_HEAD(&(peasycap->urb_audio_head));
 		peasycap->purb_audio_head = &(peasycap->urb_audio_head);
 
-		JOM(4, "allocating %i isoc audio buffers of size %i\n",
-			AUDIO_ISOC_BUFFER_MANY,
-			peasycap->audio_isoc_buffer_size);
-		JOM(4, ".... each occupying contiguous memory pages\n");
-
-		for (k = 0;  k < AUDIO_ISOC_BUFFER_MANY;  k++) {
-			pbuf = (void *)__get_free_pages(GFP_KERNEL,
-							AUDIO_ISOC_ORDER);
-			if (!pbuf) {
-				SAM("ERROR: Could not allocate isoc audio buffer "
-								"%i\n", k);
-				return -ENOMEM;
-			}
-			peasycap->allocation_audio_page +=
-						BIT(AUDIO_ISOC_ORDER);
-
-			peasycap->audio_isoc_buffer[k].pgo = pbuf;
-			peasycap->audio_isoc_buffer[k].pto = pbuf +
-			peasycap->audio_isoc_buffer_size;
-			peasycap->audio_isoc_buffer[k].kount = k;
-		}
-		JOM(4, "allocation of isoc audio buffers done.\n");
+		alloc_audio_buffers(peasycap);
+		if (rc < 0)
+			return rc;
 
 		/* Allocate and initialize urbs */
-		JOM(4, "allocating %i struct urb.\n", AUDIO_ISOC_BUFFER_MANY);
-		JOM(4, "using %i=peasycap->audio_isoc_framesperdesc\n",
-					peasycap->audio_isoc_framesperdesc);
-		JOM(4, "using %i=peasycap->audio_isoc_maxframesize\n",
-					peasycap->audio_isoc_maxframesize);
-		JOM(4, "using %i=peasycap->audio_isoc_buffer_size\n",
-					peasycap->audio_isoc_buffer_size);
-
-		for (k = 0;  k < AUDIO_ISOC_BUFFER_MANY; k++) {
-			purb = usb_alloc_urb(peasycap->audio_isoc_framesperdesc,
-								GFP_KERNEL);
-			if (!purb) {
-				SAM("ERROR: usb_alloc_urb returned NULL for buffer "
-								"%i\n", k);
-				return -ENOMEM;
-			}
-			peasycap->allocation_audio_urb += 1 ;
-			pdata_urb = kzalloc(sizeof(struct data_urb), GFP_KERNEL);
-			if (!pdata_urb) {
-				usb_free_urb(purb);
-				SAM("ERROR: Could not allocate struct data_urb.\n");
-				return -ENOMEM;
-			}
-			peasycap->allocation_audio_struct +=
-						sizeof(struct data_urb);
-
-			pdata_urb->purb = purb;
-			pdata_urb->isbuf = k;
-			pdata_urb->length = 0;
-			list_add_tail(&(pdata_urb->list_head),
-							peasycap->purb_audio_head);
-
-			if (!k) {
-				JOM(4, "initializing audio urbs thus:\n");
-				JOM(4, "  purb->interval = 1;\n");
-				JOM(4, "  purb->dev = peasycap->pusb_device;\n");
-				JOM(4, "  purb->pipe = usb_rcvisocpipe(peasycap->"
-						"pusb_device,%i);\n",
-						peasycap->audio_endpointnumber);
-				JOM(4, "  purb->transfer_flags = URB_ISO_ASAP;\n");
-				JOM(4, "  purb->transfer_buffer = "
-					"peasycap->audio_isoc_buffer[.].pgo;\n");
-				JOM(4, "  purb->transfer_buffer_length = %i;\n",
-					peasycap->audio_isoc_buffer_size);
-				JOM(4, "  purb->complete = easycap_alsa_complete;\n");
-				JOM(4, "  purb->context = peasycap;\n");
-				JOM(4, "  purb->start_frame = 0;\n");
-				JOM(4, "  purb->number_of_packets = %i;\n",
-						peasycap->audio_isoc_framesperdesc);
-				JOM(4, "  for (j = 0; j < %i; j++)\n",
-						peasycap->audio_isoc_framesperdesc);
-				JOM(4, "    {\n");
-				JOM(4, "    purb->iso_frame_desc[j].offset = j*%i;\n",
-					peasycap->audio_isoc_maxframesize);
-				JOM(4, "    purb->iso_frame_desc[j].length = %i;\n",
-					peasycap->audio_isoc_maxframesize);
-				JOM(4, "    }\n");
-			}
-
-			purb->interval = 1;
-			purb->dev = peasycap->pusb_device;
-			purb->pipe = usb_rcvisocpipe(peasycap->pusb_device,
-						peasycap->audio_endpointnumber);
-			purb->transfer_flags = URB_ISO_ASAP;
-			purb->transfer_buffer = peasycap->audio_isoc_buffer[k].pgo;
-			purb->transfer_buffer_length =
-						peasycap->audio_isoc_buffer_size;
-			purb->complete = easycap_alsa_complete;
-			purb->context = peasycap;
-			purb->start_frame = 0;
-			purb->number_of_packets = peasycap->audio_isoc_framesperdesc;
-			for (j = 0;  j < peasycap->audio_isoc_framesperdesc; j++) {
-				purb->iso_frame_desc[j].offset = j *
-						peasycap->audio_isoc_maxframesize;
-				purb->iso_frame_desc[j].length =
-						peasycap->audio_isoc_maxframesize;
-			}
-		}
-		JOM(4, "allocation of %i struct urb done.\n", k);
+		rc = create_audio_urbs(peasycap);
+		if (rc < 0)
+			return rc;
 
 		/* Save pointer peasycap in this interface */
 		usb_set_intfdata(intf, peasycap);
-- 
1.7.3.4

[PATCH 7/9] staging: easycap: Clean comment style in easycap_usb_disconnect()

[Ezequiel Garcia]

Signed-off-by: Ezequiel Garcia <elezegarcia@gmail.com>
---
 drivers/staging/media/easycap/easycap_main.c |   64 ++++++++++----------------
 1 files changed, 24 insertions(+), 40 deletions(-)

diff --git a/drivers/staging/media/easycap/easycap_main.c b/drivers/staging/media/easycap/easycap_main.c
index 6e1734d..de53ed8 100644
--- a/drivers/staging/media/easycap/easycap_main.c
+++ b/drivers/staging/media/easycap/easycap_main.c
@@ -3973,15 +3973,13 @@ static int easycap_usb_probe(struct usb_interface *intf,
 	SAM("ends successfully for interface %i\n", bInterfaceNumber);
 	return 0;
 }
-/*****************************************************************************/
-/*---------------------------------------------------------------------------*/
+
 /*
- *  WHEN THIS FUNCTION IS CALLED THE EasyCAP HAS ALREADY BEEN PHYSICALLY
- *  UNPLUGGED.  HENCE peasycap->pusb_device IS NO LONGER VALID.
- *
- *  THIS FUNCTION AFFECTS ALSA.  BEWARE.
+ * When this function is called the device has already been
+ * physically unplugged.
+ * Hence, peasycap->pusb_device is no longer valid.
+ * This function affects alsa.
  */
-/*---------------------------------------------------------------------------*/
 static void easycap_usb_disconnect(struct usb_interface *pusb_interface)
 {
 	struct usb_host_interface *pusb_host_interface;
@@ -4006,6 +4004,7 @@ static void easycap_usb_disconnect(struct usb_interface *pusb_interface)
 	minor = pusb_interface->minor;
 	JOT(4, "intf[%i]: minor=%i\n", bInterfaceNumber, minor);
 
+	/* There is nothing to do for Interface Number 1 */
 	if (1 == bInterfaceNumber)
 		return;
 
@@ -4014,11 +4013,8 @@ static void easycap_usb_disconnect(struct usb_interface *pusb_interface)
 		SAY("ERROR: peasycap is NULL\n");
 		return;
 	}
-/*---------------------------------------------------------------------------*/
-/*
- *  IF THE WAIT QUEUES ARE NOT CLEARED A DEADLOCK IS POSSIBLE.  BEWARE.
-*/
-/*---------------------------------------------------------------------------*/
+
+	/* If the waitqueues are not cleared a deadlock is possible */
 	peasycap->video_eof = 1;
 	peasycap->audio_eof = 1;
 	wake_up_interruptible(&(peasycap->wq_video));
@@ -4034,15 +4030,14 @@ static void easycap_usb_disconnect(struct usb_interface *pusb_interface)
 	default:
 		break;
 	}
-/*--------------------------------------------------------------------------*/
-/*
- *  DEREGISTER
- *
- *  THIS PROCEDURE WILL BLOCK UNTIL easycap_poll(), VIDEO IOCTL AND AUDIO
- *  IOCTL ARE ALL UNLOCKED.  IF THIS IS NOT DONE AN Oops CAN OCCUR WHEN
- *  AN EasyCAP IS UNPLUGGED WHILE THE URBS ARE RUNNING.  BEWARE.
- */
-/*--------------------------------------------------------------------------*/
+
+	/*
+	 * Deregister
+	 * This procedure will block until easycap_poll(),
+	 * video and audio ioctl are all unlocked.
+	 * If this is not done an oops can occur when an easycap
+	 * is unplugged while the urbs are running.
+	 */
 	kd = easycap_isdongle(peasycap);
 	switch (bInterfaceNumber) {
 	case 0: {
@@ -4059,7 +4054,6 @@ static void easycap_usb_disconnect(struct usb_interface *pusb_interface)
 		} else {
 			SAY("ERROR: %i=kd is bad: cannot lock dongle\n", kd);
 		}
-/*---------------------------------------------------------------------------*/
 		if (!peasycap->v4l2_device.name[0]) {
 			SAM("ERROR: peasycap->v4l2_device.name is empty\n");
 			if (0 <= kd && DONGLE_MANY > kd)
@@ -4075,7 +4069,6 @@ static void easycap_usb_disconnect(struct usb_interface *pusb_interface)
 		JOM(4, "intf[%i]: video_unregister_device() minor=%i\n",
 				bInterfaceNumber, minor);
 		peasycap->registered_video--;
-/*^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^*/
 
 		if (0 <= kd && DONGLE_MANY > kd) {
 			mutex_unlock(&easycapdc60_dongle[kd].mutex_video);
@@ -4111,12 +4104,12 @@ static void easycap_usb_disconnect(struct usb_interface *pusb_interface)
 	default:
 		break;
 	}
-/*---------------------------------------------------------------------------*/
-/*
- *  CALL easycap_delete() IF NO REMAINING REFERENCES TO peasycap
- *  (ALSO WHEN ALSA HAS BEEN IN USE)
- */
-/*---------------------------------------------------------------------------*/
+
+	/*
+	 * If no remaining references to peasycap,
+	 * call easycap_delete.
+	 * (Also when alsa has been in use)
+	 */
 	if (!peasycap->kref.refcount.counter) {
 		SAM("ERROR: peasycap->kref.refcount.counter is zero "
 							"so cannot call kref_put()\n");
@@ -4151,17 +4144,11 @@ static void easycap_usb_disconnect(struct usb_interface *pusb_interface)
 		mutex_unlock(&easycapdc60_dongle[kd].mutex_video);
 		JOT(4, "unlocked dongle[%i].mutex_video\n", kd);
 	}
-/*---------------------------------------------------------------------------*/
 	JOM(4, "ends\n");
 	return;
 }
-/*****************************************************************************/
 
-/*---------------------------------------------------------------------------*/
-/*
- *  PARAMETERS APPLICABLE TO ENTIRE DRIVER, I.E. BOTH VIDEO AND AUDIO
- */
-/*---------------------------------------------------------------------------*/
+/* Devices supported by this driver */
 static struct usb_device_id easycap_usb_device_id_table[] = {
 	{USB_DEVICE(USB_EASYCAP_VENDOR_ID, USB_EASYCAP_PRODUCT_ID)},
 	{ }
@@ -4196,14 +4183,11 @@ static int __init easycap_module_init(void)
 
 	return rc;
 }
-/*****************************************************************************/
+
 static void __exit easycap_module_exit(void)
 {
 	usb_deregister(&easycap_usb_driver);
 }
-/*****************************************************************************/
 
 module_init(easycap_module_init);
 module_exit(easycap_module_exit);
-
-/*****************************************************************************/
-- 
1.7.3.4

[PATCH 8/9] staging: easycap: Clean comment style in easycap_delete()

[Ezequiel Garcia]

Signed-off-by: Ezequiel Garcia <elezegarcia@gmail.com>
---
 drivers/staging/media/easycap/easycap_main.c |   43 +++++++++----------------
 1 files changed, 16 insertions(+), 27 deletions(-)

diff --git a/drivers/staging/media/easycap/easycap_main.c b/drivers/staging/media/easycap/easycap_main.c
index de53ed8..76a2c5b 100644
--- a/drivers/staging/media/easycap/easycap_main.c
+++ b/drivers/staging/media/easycap/easycap_main.c
@@ -700,17 +700,13 @@ static int videodev_release(struct video_device *pvideo_device)
 	JOM(4, "ending successfully\n");
 	return 0;
 }
-/*^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^*/
-/*****************************************************************************/
-/*--------------------------------------------------------------------------*/
+
 /*
- *  THIS FUNCTION IS CALLED FROM WITHIN easycap_usb_disconnect() AND IS
- *  PROTECTED BY SEMAPHORES SET AND CLEARED BY easycap_usb_disconnect().
- *
- *  BY THIS STAGE THE DEVICE HAS ALREADY BEEN PHYSICALLY UNPLUGGED, SO
- *  peasycap->pusb_device IS NO LONGER VALID.
+ * This function is called from within easycap_usb_disconnect() and is
+ * protected by semaphores set and cleared by easycap_usb_disconnect().
+ * By this stage the device has already been physically unplugged,
+ * so peasycap->pusb_device is no longer valid.
  */
-/*---------------------------------------------------------------------------*/
 static void easycap_delete(struct kref *pkref)
 {
 	struct easycap *peasycap;
@@ -731,11 +727,8 @@ static void easycap_delete(struct kref *pkref)
 		return;
 	}
 	kd = easycap_isdongle(peasycap);
-/*---------------------------------------------------------------------------*/
-/*
- *  FREE VIDEO.
- */
-/*---------------------------------------------------------------------------*/
+
+	/* Free video urbs */
 	if (peasycap->purb_video_head) {
 		m = 0;
 		list_for_each(plist_head, peasycap->purb_video_head) {
@@ -750,7 +743,6 @@ static void easycap_delete(struct kref *pkref)
 		}
 
 		JOM(4, "%i video urbs freed\n", m);
-/*---------------------------------------------------------------------------*/
 		JOM(4, "freeing video data_urb structures.\n");
 		m = 0;
 		list_for_each_safe(plist_head, plist_next,
@@ -768,7 +760,8 @@ static void easycap_delete(struct kref *pkref)
 		JOM(4, "setting peasycap->purb_video_head=NULL\n");
 		peasycap->purb_video_head = NULL;
 	}
-/*---------------------------------------------------------------------------*/
+
+	/* Free video isoc buffers */
 	JOM(4, "freeing video isoc buffers.\n");
 	m = 0;
 	for (k = 0;  k < VIDEO_ISOC_BUFFER_MANY;  k++) {
@@ -784,7 +777,7 @@ static void easycap_delete(struct kref *pkref)
 	}
 	JOM(4, "isoc video buffers freed: %i pages\n",
 			m * (0x01 << VIDEO_ISOC_ORDER));
-/*---------------------------------------------------------------------------*/
+	/* Free video field buffers */
 	JOM(4, "freeing video field buffers.\n");
 	gone = 0;
 	for (k = 0;  k < FIELD_BUFFER_MANY;  k++) {
@@ -799,7 +792,8 @@ static void easycap_delete(struct kref *pkref)
 		}
 	}
 	JOM(4, "video field buffers freed: %i pages\n", gone);
-/*---------------------------------------------------------------------------*/
+
+	/* Free video frame buffers */
 	JOM(4, "freeing video frame buffers.\n");
 	gone = 0;
 	for (k = 0;  k < FRAME_BUFFER_MANY;  k++) {
@@ -814,11 +808,8 @@ static void easycap_delete(struct kref *pkref)
 		}
 	}
 	JOM(4, "video frame buffers freed: %i pages\n", gone);
-/*---------------------------------------------------------------------------*/
-/*
- *  FREE AUDIO.
- */
-/*---------------------------------------------------------------------------*/
+
+	/* Free audio urbs */
 	if (peasycap->purb_audio_head) {
 		JOM(4, "freeing audio urbs\n");
 		m = 0;
@@ -833,7 +824,6 @@ static void easycap_delete(struct kref *pkref)
 			}
 		}
 		JOM(4, "%i audio urbs freed\n", m);
-/*---------------------------------------------------------------------------*/
 		JOM(4, "freeing audio data_urb structures.\n");
 		m = 0;
 		list_for_each_safe(plist_head, plist_next,
@@ -851,7 +841,8 @@ static void easycap_delete(struct kref *pkref)
 		JOM(4, "setting peasycap->purb_audio_head=NULL\n");
 		peasycap->purb_audio_head = NULL;
 	}
-/*---------------------------------------------------------------------------*/
+
+	/* Free audio isoc buffers */
 	JOM(4, "freeing audio isoc buffers.\n");
 	m = 0;
 	for (k = 0;  k < AUDIO_ISOC_BUFFER_MANY;  k++) {
@@ -867,7 +858,6 @@ static void easycap_delete(struct kref *pkref)
 	}
 	JOM(4, "easyoss_delete(): isoc audio buffers freed: %i pages\n",
 					m * (0x01 << AUDIO_ISOC_ORDER));
-/*---------------------------------------------------------------------------*/
 	JOM(4, "freeing easycap structure.\n");
 	allocation_video_urb    = peasycap->allocation_video_urb;
 	allocation_video_page   = peasycap->allocation_video_page;
@@ -895,7 +885,6 @@ static void easycap_delete(struct kref *pkref)
 
 	kfree(peasycap);
 
-/*---------------------------------------------------------------------------*/
 	SAY("%8i=video urbs    after all deletions\n", allocation_video_urb);
 	SAY("%8i=video pages   after all deletions\n", allocation_video_page);
 	SAY("%8i=video structs after all deletions\n", allocation_video_struct);
-- 
1.7.3.4

[PATCH 9/9] staging: easycap: Split easycap_delete() into several pieces

[Ezequiel Garcia]

The patch splits easycap_delete(), which is in charge of
buffer deallocation, into smaller functions each
deallocating a specific kind of buffer.

Signed-off-by: Ezequiel Garcia <elezegarcia@gmail.com>
---
 drivers/staging/media/easycap/easycap_main.c |  445 ++++++++++++++-----------
 1 files changed, 249 insertions(+), 196 deletions(-)

diff --git a/drivers/staging/media/easycap/easycap_main.c b/drivers/staging/media/easycap/easycap_main.c
index 76a2c5b..aed9537 100644
--- a/drivers/staging/media/easycap/easycap_main.c
+++ b/drivers/staging/media/easycap/easycap_main.c
@@ -701,202 +701,6 @@ static int videodev_release(struct video_device *pvideo_device)
 	return 0;
 }
 
-/*
- * This function is called from within easycap_usb_disconnect() and is
- * protected by semaphores set and cleared by easycap_usb_disconnect().
- * By this stage the device has already been physically unplugged,
- * so peasycap->pusb_device is no longer valid.
- */
-static void easycap_delete(struct kref *pkref)
-{
-	struct easycap *peasycap;
-	struct data_urb *pdata_urb;
-	struct list_head *plist_head, *plist_next;
-	int k, m, gone, kd;
-	int allocation_video_urb;
-	int allocation_video_page;
-	int allocation_video_struct;
-	int allocation_audio_urb;
-	int allocation_audio_page;
-	int allocation_audio_struct;
-	int registered_video, registered_audio;
-
-	peasycap = container_of(pkref, struct easycap, kref);
-	if (!peasycap) {
-		SAM("ERROR: peasycap is NULL: cannot perform deletions\n");
-		return;
-	}
-	kd = easycap_isdongle(peasycap);
-
-	/* Free video urbs */
-	if (peasycap->purb_video_head) {
-		m = 0;
-		list_for_each(plist_head, peasycap->purb_video_head) {
-			pdata_urb = list_entry(plist_head,
-						struct data_urb, list_head);
-			if (pdata_urb && pdata_urb->purb) {
-				usb_free_urb(pdata_urb->purb);
-				pdata_urb->purb = NULL;
-				peasycap->allocation_video_urb--;
-				m++;
-			}
-		}
-
-		JOM(4, "%i video urbs freed\n", m);
-		JOM(4, "freeing video data_urb structures.\n");
-		m = 0;
-		list_for_each_safe(plist_head, plist_next,
-					peasycap->purb_video_head) {
-			pdata_urb = list_entry(plist_head,
-						struct data_urb, list_head);
-			if (pdata_urb) {
-				peasycap->allocation_video_struct -=
-						sizeof(struct data_urb);
-				kfree(pdata_urb);
-				m++;
-			}
-		}
-		JOM(4, "%i video data_urb structures freed\n", m);
-		JOM(4, "setting peasycap->purb_video_head=NULL\n");
-		peasycap->purb_video_head = NULL;
-	}
-
-	/* Free video isoc buffers */
-	JOM(4, "freeing video isoc buffers.\n");
-	m = 0;
-	for (k = 0;  k < VIDEO_ISOC_BUFFER_MANY;  k++) {
-		if (peasycap->video_isoc_buffer[k].pgo) {
-			free_pages((unsigned long)
-				   peasycap->video_isoc_buffer[k].pgo,
-					VIDEO_ISOC_ORDER);
-			peasycap->video_isoc_buffer[k].pgo = NULL;
-			peasycap->allocation_video_page -=
-						BIT(VIDEO_ISOC_ORDER);
-			m++;
-		}
-	}
-	JOM(4, "isoc video buffers freed: %i pages\n",
-			m * (0x01 << VIDEO_ISOC_ORDER));
-	/* Free video field buffers */
-	JOM(4, "freeing video field buffers.\n");
-	gone = 0;
-	for (k = 0;  k < FIELD_BUFFER_MANY;  k++) {
-		for (m = 0;  m < FIELD_BUFFER_SIZE/PAGE_SIZE;  m++) {
-			if (peasycap->field_buffer[k][m].pgo) {
-				free_page((unsigned long)
-					  peasycap->field_buffer[k][m].pgo);
-				peasycap->field_buffer[k][m].pgo = NULL;
-				peasycap->allocation_video_page -= 1;
-				gone++;
-			}
-		}
-	}
-	JOM(4, "video field buffers freed: %i pages\n", gone);
-
-	/* Free video frame buffers */
-	JOM(4, "freeing video frame buffers.\n");
-	gone = 0;
-	for (k = 0;  k < FRAME_BUFFER_MANY;  k++) {
-		for (m = 0;  m < FRAME_BUFFER_SIZE/PAGE_SIZE;  m++) {
-			if (peasycap->frame_buffer[k][m].pgo) {
-				free_page((unsigned long)
-					  peasycap->frame_buffer[k][m].pgo);
-				peasycap->frame_buffer[k][m].pgo = NULL;
-				peasycap->allocation_video_page -= 1;
-				gone++;
-			}
-		}
-	}
-	JOM(4, "video frame buffers freed: %i pages\n", gone);
-
-	/* Free audio urbs */
-	if (peasycap->purb_audio_head) {
-		JOM(4, "freeing audio urbs\n");
-		m = 0;
-		list_for_each(plist_head, (peasycap->purb_audio_head)) {
-			pdata_urb = list_entry(plist_head,
-					struct data_urb, list_head);
-			if (pdata_urb && pdata_urb->purb) {
-				usb_free_urb(pdata_urb->purb);
-				pdata_urb->purb = NULL;
-				peasycap->allocation_audio_urb--;
-				m++;
-			}
-		}
-		JOM(4, "%i audio urbs freed\n", m);
-		JOM(4, "freeing audio data_urb structures.\n");
-		m = 0;
-		list_for_each_safe(plist_head, plist_next,
-					peasycap->purb_audio_head) {
-			pdata_urb = list_entry(plist_head,
-					struct data_urb, list_head);
-			if (pdata_urb) {
-				peasycap->allocation_audio_struct -=
-							sizeof(struct data_urb);
-				kfree(pdata_urb);
-				m++;
-			}
-		}
-		JOM(4, "%i audio data_urb structures freed\n", m);
-		JOM(4, "setting peasycap->purb_audio_head=NULL\n");
-		peasycap->purb_audio_head = NULL;
-	}
-
-	/* Free audio isoc buffers */
-	JOM(4, "freeing audio isoc buffers.\n");
-	m = 0;
-	for (k = 0;  k < AUDIO_ISOC_BUFFER_MANY;  k++) {
-		if (peasycap->audio_isoc_buffer[k].pgo) {
-			free_pages((unsigned long)
-					(peasycap->audio_isoc_buffer[k].pgo),
-					AUDIO_ISOC_ORDER);
-			peasycap->audio_isoc_buffer[k].pgo = NULL;
-			peasycap->allocation_audio_page -=
-					BIT(AUDIO_ISOC_ORDER);
-			m++;
-		}
-	}
-	JOM(4, "easyoss_delete(): isoc audio buffers freed: %i pages\n",
-					m * (0x01 << AUDIO_ISOC_ORDER));
-	JOM(4, "freeing easycap structure.\n");
-	allocation_video_urb    = peasycap->allocation_video_urb;
-	allocation_video_page   = peasycap->allocation_video_page;
-	allocation_video_struct = peasycap->allocation_video_struct;
-	registered_video        = peasycap->registered_video;
-	allocation_audio_urb    = peasycap->allocation_audio_urb;
-	allocation_audio_page   = peasycap->allocation_audio_page;
-	allocation_audio_struct = peasycap->allocation_audio_struct;
-	registered_audio        = peasycap->registered_audio;
-
-	if (0 <= kd && DONGLE_MANY > kd) {
-		if (mutex_lock_interruptible(&mutex_dongle)) {
-			SAY("ERROR: cannot down mutex_dongle\n");
-		} else {
-			JOM(4, "locked mutex_dongle\n");
-			easycapdc60_dongle[kd].peasycap = NULL;
-			mutex_unlock(&mutex_dongle);
-			JOM(4, "unlocked mutex_dongle\n");
-			JOT(4, "   null-->dongle[%i].peasycap\n", kd);
-			allocation_video_struct -= sizeof(struct easycap);
-		}
-	} else {
-		SAY("ERROR: cannot purge dongle[].peasycap");
-	}
-
-	kfree(peasycap);
-
-	SAY("%8i=video urbs    after all deletions\n", allocation_video_urb);
-	SAY("%8i=video pages   after all deletions\n", allocation_video_page);
-	SAY("%8i=video structs after all deletions\n", allocation_video_struct);
-	SAY("%8i=video devices after all deletions\n", registered_video);
-	SAY("%8i=audio urbs    after all deletions\n", allocation_audio_urb);
-	SAY("%8i=audio pages   after all deletions\n", allocation_audio_page);
-	SAY("%8i=audio structs after all deletions\n", allocation_audio_struct);
-	SAY("%8i=audio devices after all deletions\n", registered_audio);
-
-	JOT(4, "ending.\n");
-	return;
-}
 /*****************************************************************************/
 static unsigned int easycap_poll(struct file *file, poll_table *wait)
 {
@@ -2875,6 +2679,56 @@ static struct easycap *alloc_easycap(u8 bInterfaceNumber)
 	return peasycap;
 }
 
+static void free_easycap(struct easycap *peasycap)
+{
+	int allocation_video_urb;
+	int allocation_video_page;
+	int allocation_video_struct;
+	int allocation_audio_urb;
+	int allocation_audio_page;
+	int allocation_audio_struct;
+	int registered_video, registered_audio;
+	int kd;
+
+	JOM(4, "freeing easycap structure.\n");
+	allocation_video_urb    = peasycap->allocation_video_urb;
+	allocation_video_page   = peasycap->allocation_video_page;
+	allocation_video_struct = peasycap->allocation_video_struct;
+	registered_video        = peasycap->registered_video;
+	allocation_audio_urb    = peasycap->allocation_audio_urb;
+	allocation_audio_page   = peasycap->allocation_audio_page;
+	allocation_audio_struct = peasycap->allocation_audio_struct;
+	registered_audio        = peasycap->registered_audio;
+
+	kd = easycap_isdongle(peasycap);
+	if (0 <= kd && DONGLE_MANY > kd) {
+		if (mutex_lock_interruptible(&mutex_dongle)) {
+			SAY("ERROR: cannot down mutex_dongle\n");
+		} else {
+			JOM(4, "locked mutex_dongle\n");
+			easycapdc60_dongle[kd].peasycap = NULL;
+			mutex_unlock(&mutex_dongle);
+			JOM(4, "unlocked mutex_dongle\n");
+			JOT(4, "   null-->dongle[%i].peasycap\n", kd);
+			allocation_video_struct -= sizeof(struct easycap);
+		}
+	} else {
+		SAY("ERROR: cannot purge dongle[].peasycap");
+	}
+
+	/* Free device structure */
+	kfree(peasycap);
+
+	SAY("%8i=video urbs    after all deletions\n", allocation_video_urb);
+	SAY("%8i=video pages   after all deletions\n", allocation_video_page);
+	SAY("%8i=video structs after all deletions\n", allocation_video_struct);
+	SAY("%8i=video devices after all deletions\n", registered_video);
+	SAY("%8i=audio urbs    after all deletions\n", allocation_audio_urb);
+	SAY("%8i=audio pages   after all deletions\n", allocation_audio_page);
+	SAY("%8i=audio structs after all deletions\n", allocation_audio_struct);
+	SAY("%8i=audio devices after all deletions\n", registered_audio);
+}
+
 /*
  * FIXME: Identify the appropriate pointer peasycap for interfaces
  * 1 and 2. The address of peasycap->pusb_device is reluctantly used
@@ -3073,6 +2927,26 @@ static int alloc_framebuffers(struct easycap *peasycap)
 	return 0;
 }
 
+static void free_framebuffers(struct easycap *peasycap)
+{
+	int k, m, gone;
+
+	JOM(4, "freeing video frame buffers.\n");
+	gone = 0;
+	for (k = 0;  k < FRAME_BUFFER_MANY;  k++) {
+		for (m = 0;  m < FRAME_BUFFER_SIZE/PAGE_SIZE;  m++) {
+			if (peasycap->frame_buffer[k][m].pgo) {
+				free_page((unsigned long)
+					peasycap->frame_buffer[k][m].pgo);
+				peasycap->frame_buffer[k][m].pgo = NULL;
+				peasycap->allocation_video_page -= 1;
+				gone++;
+			}
+		}
+	}
+	JOM(4, "video frame buffers freed: %i pages\n", gone);
+}
+
 static int alloc_fieldbuffers(struct easycap *peasycap)
 {
 	int i, j;
@@ -3112,6 +2986,26 @@ static int alloc_fieldbuffers(struct easycap *peasycap)
 	return 0;
 }
 
+static void free_fieldbuffers(struct easycap *peasycap)
+{
+	int k, m, gone;
+
+	JOM(4, "freeing video field buffers.\n");
+	gone = 0;
+	for (k = 0;  k < FIELD_BUFFER_MANY;  k++) {
+		for (m = 0;  m < FIELD_BUFFER_SIZE/PAGE_SIZE;  m++) {
+			if (peasycap->field_buffer[k][m].pgo) {
+				free_page((unsigned long)
+					  peasycap->field_buffer[k][m].pgo);
+				peasycap->field_buffer[k][m].pgo = NULL;
+				peasycap->allocation_video_page -= 1;
+				gone++;
+			}
+		}
+	}
+	JOM(4, "video field buffers freed: %i pages\n", gone);
+}
+
 static int alloc_isocbuffers(struct easycap *peasycap)
 {
 	int i;
@@ -3142,6 +3036,27 @@ static int alloc_isocbuffers(struct easycap *peasycap)
 	return 0;
 }
 
+static void free_isocbuffers(struct easycap *peasycap)
+{
+	int k, m;
+
+	JOM(4, "freeing video isoc buffers.\n");
+	m = 0;
+	for (k = 0;  k < VIDEO_ISOC_BUFFER_MANY;  k++) {
+		if (peasycap->video_isoc_buffer[k].pgo) {
+			free_pages((unsigned long)
+				   peasycap->video_isoc_buffer[k].pgo,
+					VIDEO_ISOC_ORDER);
+			peasycap->video_isoc_buffer[k].pgo = NULL;
+			peasycap->allocation_video_page -=
+						BIT(VIDEO_ISOC_ORDER);
+			m++;
+		}
+	}
+	JOM(4, "isoc video buffers freed: %i pages\n",
+			m * (0x01 << VIDEO_ISOC_ORDER));
+}
+
 static int create_video_urbs(struct easycap *peasycap)
 {
 	struct urb *purb;
@@ -3234,6 +3149,45 @@ static int create_video_urbs(struct easycap *peasycap)
 	return 0;
 }
 
+static void free_video_urbs(struct easycap *peasycap)
+{
+	struct list_head *plist_head, *plist_next;
+	struct data_urb *pdata_urb;
+	int m;
+
+	if (peasycap->purb_video_head) {
+		m = 0;
+		list_for_each(plist_head, peasycap->purb_video_head) {
+			pdata_urb = list_entry(plist_head,
+					struct data_urb, list_head);
+			if (pdata_urb && pdata_urb->purb) {
+				usb_free_urb(pdata_urb->purb);
+				pdata_urb->purb = NULL;
+				peasycap->allocation_video_urb--;
+				m++;
+			}
+		}
+
+		JOM(4, "%i video urbs freed\n", m);
+		JOM(4, "freeing video data_urb structures.\n");
+		m = 0;
+		list_for_each_safe(plist_head, plist_next,
+					peasycap->purb_video_head) {
+			pdata_urb = list_entry(plist_head,
+					struct data_urb, list_head);
+			if (pdata_urb) {
+				peasycap->allocation_video_struct -=
+					sizeof(struct data_urb);
+				kfree(pdata_urb);
+				m++;
+			}
+		}
+		JOM(4, "%i video data_urb structures freed\n", m);
+		JOM(4, "setting peasycap->purb_video_head=NULL\n");
+		peasycap->purb_video_head = NULL;
+	}
+}
+
 static int alloc_audio_buffers(struct easycap *peasycap)
 {
 	void *pbuf;
@@ -3263,6 +3217,27 @@ static int alloc_audio_buffers(struct easycap *peasycap)
 	return 0;
 }
 
+static void free_audio_buffers(struct easycap *peasycap)
+{
+	int k, m;
+
+	JOM(4, "freeing audio isoc buffers.\n");
+	m = 0;
+	for (k = 0;  k < AUDIO_ISOC_BUFFER_MANY;  k++) {
+		if (peasycap->audio_isoc_buffer[k].pgo) {
+			free_pages((unsigned long)
+					(peasycap->audio_isoc_buffer[k].pgo),
+					AUDIO_ISOC_ORDER);
+			peasycap->audio_isoc_buffer[k].pgo = NULL;
+			peasycap->allocation_audio_page -=
+					BIT(AUDIO_ISOC_ORDER);
+			m++;
+		}
+	}
+	JOM(4, "easyoss_delete(): isoc audio buffers freed: %i pages\n",
+					m * (0x01 << AUDIO_ISOC_ORDER));
+}
+
 static int create_audio_urbs(struct easycap *peasycap)
 {
 	struct urb *purb;
@@ -3351,6 +3326,45 @@ static int create_audio_urbs(struct easycap *peasycap)
 	return 0;
 }
 
+static void free_audio_urbs(struct easycap *peasycap)
+{
+	struct list_head *plist_head, *plist_next;
+	struct data_urb *pdata_urb;
+	int m;
+
+	if (peasycap->purb_audio_head) {
+		JOM(4, "freeing audio urbs\n");
+		m = 0;
+		list_for_each(plist_head, (peasycap->purb_audio_head)) {
+			pdata_urb = list_entry(plist_head,
+					struct data_urb, list_head);
+			if (pdata_urb && pdata_urb->purb) {
+				usb_free_urb(pdata_urb->purb);
+				pdata_urb->purb = NULL;
+				peasycap->allocation_audio_urb--;
+				m++;
+			}
+		}
+		JOM(4, "%i audio urbs freed\n", m);
+		JOM(4, "freeing audio data_urb structures.\n");
+		m = 0;
+		list_for_each_safe(plist_head, plist_next,
+					peasycap->purb_audio_head) {
+			pdata_urb = list_entry(plist_head,
+					struct data_urb, list_head);
+			if (pdata_urb) {
+				peasycap->allocation_audio_struct -=
+							sizeof(struct data_urb);
+				kfree(pdata_urb);
+				m++;
+			}
+		}
+		JOM(4, "%i audio data_urb structures freed\n", m);
+		JOM(4, "setting peasycap->purb_audio_head=NULL\n");
+		peasycap->purb_audio_head = NULL;
+	}
+}
+
 static void config_easycap(struct easycap *peasycap,
 			   u8 bInterfaceNumber,
 			   u8 bInterfaceClass,
@@ -3389,6 +3403,45 @@ static void config_easycap(struct easycap *peasycap,
 	}
 }
 
+/*
+ * This function is called from within easycap_usb_disconnect() and is
+ * protected by semaphores set and cleared by easycap_usb_disconnect().
+ * By this stage the device has already been physically unplugged,
+ * so peasycap->pusb_device is no longer valid.
+ */
+static void easycap_delete(struct kref *pkref)
+{
+	struct easycap *peasycap;
+
+	peasycap = container_of(pkref, struct easycap, kref);
+	if (!peasycap) {
+		SAM("ERROR: peasycap is NULL: cannot perform deletions\n");
+		return;
+	}
+
+	/* Free video urbs */
+	free_video_urbs(peasycap);
+
+	/* Free video isoc buffers */
+	free_isocbuffers(peasycap);
+
+	/* Free video field buffers */
+	free_fieldbuffers(peasycap);
+
+	/* Free video frame buffers */
+	free_framebuffers(peasycap);
+
+	/* Free audio urbs */
+	free_audio_urbs(peasycap);
+
+	/* Free audio isoc buffers */
+	free_audio_buffers(peasycap);
+
+	free_easycap(peasycap);
+
+	JOT(4, "ending.\n");
+}
+
 static const struct v4l2_file_operations v4l2_fops = {
 	.owner		= THIS_MODULE,
 	.open		= easycap_open_noinode,
-- 
1.7.3.4

Re: [PATCH 5/9] staging: easycap: Push video registration to easycap_register_video()

[Dan Carpenter]

[-- Attachment #1: Type: text/plain, Size: 321 bytes --]

On Fri, Feb 24, 2012 at 12:24:18PM -0300, Ezequiel Garcia wrote:
> +		rc = easycap_register_video(peasycap);
> +		if (rc < 0)
>  			return -ENODEV;

Don't resend.  These are beautiful patches you are sending and I
wouldn't want to slow you down.  But it would have been better to
return rc here.

regards,
dan carpenter


[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH 5/9] staging: easycap: Push video registration to easycap_register_video()

[Ezequiel García]

Hi Dan,

On Fri, Feb 24, 2012 at 3:39 PM, Dan Carpenter <dan.carpenter@oracle.com> wrote:
> On Fri, Feb 24, 2012 at 12:24:18PM -0300, Ezequiel Garcia wrote:
>> +             rc = easycap_register_video(peasycap);
>> +             if (rc < 0)
>>                       return -ENODEV;
>
> Don't resend.  These are beautiful patches you are sending and I
> wouldn't want to slow you down.  But it would have been better to
> return rc here.

Ok, I'll do that in the future. Thanks for the feedback, and feel free
to keep it coming :)
Thanks for taking the time to review.

I have to say I'm thrilled with having time (and knowledge) to work on
the linux kernel.
It was 12 years ago when I first installed a linux distro and I had no
idea what to do with it
back then! ;)

Re: [PATCH 9/9] staging: easycap: Split easycap_delete() into several pieces

[Dan Carpenter]

[-- Attachment #1: Type: text/plain, Size: 222 bytes --]

On Fri, Feb 24, 2012 at 12:24:22PM -0300, Ezequiel Garcia wrote:
> +	/* Free video urbs */
> +	free_video_urbs(peasycap);

These comments aren't redundant.  Could you remove them in a later
patch?

regards,
dan carpenter


[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]