[RFT/RFC PATCH 0/8] em28xx: move more v4l/dvb specific code to the extension modules and fix the resources handling

[RFT/RFC PATCH 0/8] em28xx: move more v4l/dvb specific code to the extension modules and fix the resources handling

[Frank Schäfer]

Patch 1 is fix for the analog device initialization on the first open() call.
Patches 2-7 move some dvb and v4l specific code from the core module to the 
extension modules and fix the resources releasing logic/order in the core and
the v4l2 module. This will also fix the various sysfs group remove warnings 
which we can see since kernel 3.13.
Patch 8 finally removes some dead code lines and fixes leaking the memory of 
video, vbi and radio video_device structs.

I've tested all patches carefully with the devices I have, but because this is
really critical stuff, they should be reviewed and tested by others, too.


Frank Schäfer (8):
  em28xx-v4l: fix device initialization in em28xx_v4l2_open() for radio
    and VBI mode
  em28xx: move usb buffer pre-allocation and transfer uninit from the
    core to the dvb extension
  em28xx: move usb transfer uninit on device disconnect from the core to
    the v4l-extension
  em28xx: move v4l2_device_disconnect() call from the core to the v4l
    extension
  em28xx-v4l: move v4l2_ctrl_handler freeing and v4l2_device
    unregistration to em28xx_v4l2_fini
  em28xx: move v4l2 dummy clock deregistration from the core to the v4l
    extension
  em28xx: always call em28xx_release_resources() in the usb disconnect
    handler
  em28xx-v4l: fix the freeing of the video devices memory

 drivers/media/usb/em28xx/em28xx-cards.c |   48 ++--------------
 drivers/media/usb/em28xx/em28xx-dvb.c   |   23 ++++++++
 drivers/media/usb/em28xx/em28xx-video.c |   91 +++++++++++++++++++------------
 3 Dateien geändert, 85 Zeilen hinzugefügt(+), 77 Zeilen entfernt(-)

-- 
1.7.10.4

[RFT/RFC PATCH 1/8] em28xx-v4l: fix device initialization in em28xx_v4l2_open() for radio and VBI mode

[Frank Schäfer]

- bail out on unsupported VFL_TYPE
- em28xx_set_mode() needs to be called for VBI and radio mode, too
- em28xx_wake_i2c() needs to be called for VBI and radio mode, too
- em28xx_resolution_set() also needs to be called for VBI

Signed-off-by: Frank Schäfer <fschaefer.oss@googlemail.com>
---
 drivers/media/usb/em28xx/em28xx-video.c |   16 +++++++++++-----
 1 Datei geändert, 11 Zeilen hinzugefügt(+), 5 Zeilen entfernt(-)

diff --git a/drivers/media/usb/em28xx/em28xx-video.c b/drivers/media/usb/em28xx/em28xx-video.c
index b65d13a..83c99e6 100644
--- a/drivers/media/usb/em28xx/em28xx-video.c
+++ b/drivers/media/usb/em28xx/em28xx-video.c
@@ -1826,6 +1826,10 @@ static int em28xx_v4l2_open(struct file *filp)
 	case VFL_TYPE_VBI:
 		fh_type = V4L2_BUF_TYPE_VBI_CAPTURE;
 		break;
+	case VFL_TYPE_RADIO:
+		break;
+	default:
+		return -EINVAL;
 	}
 
 	em28xx_videodbg("open dev=%s type=%s users=%d\n",
@@ -1846,15 +1850,17 @@ static int em28xx_v4l2_open(struct file *filp)
 	fh->type = fh_type;
 	filp->private_data = fh;
 
-	if (fh->type == V4L2_BUF_TYPE_VIDEO_CAPTURE && dev->users == 0) {
+	if (dev->users == 0) {
 		em28xx_set_mode(dev, EM28XX_ANALOG_MODE);
-		em28xx_resolution_set(dev);
 
-		/* Needed, since GPIO might have disabled power of
-		   some i2c device
+		if (vdev->vfl_type != VFL_TYPE_RADIO)
+			em28xx_resolution_set(dev);
+
+		/*
+		 * Needed, since GPIO might have disabled power
+		 * of some i2c devices
 		 */
 		em28xx_wake_i2c(dev);
-
 	}
 
 	if (vdev->vfl_type == VFL_TYPE_RADIO) {
-- 
1.7.10.4

[RFT/RFC PATCH 2/8] em28xx: move usb buffer pre-allocation and transfer uninit from the core to the dvb extension

[Frank Schäfer]

Signed-off-by: Frank Schäfer <fschaefer.oss@googlemail.com>
---
 drivers/media/usb/em28xx/em28xx-cards.c |   21 ---------------------
 drivers/media/usb/em28xx/em28xx-dvb.c   |   23 +++++++++++++++++++++++
 2 Dateien geändert, 23 Zeilen hinzugefügt(+), 21 Zeilen entfernt(-)

diff --git a/drivers/media/usb/em28xx/em28xx-cards.c b/drivers/media/usb/em28xx/em28xx-cards.c
index b2cfd5d..4d89df9 100644
--- a/drivers/media/usb/em28xx/em28xx-cards.c
+++ b/drivers/media/usb/em28xx/em28xx-cards.c
@@ -3290,26 +3290,6 @@ static int em28xx_usb_probe(struct usb_interface *interface,
 
 		em28xx_info("dvb set to %s mode.\n",
 			    dev->dvb_xfer_bulk ? "bulk" : "isoc");
-
-		/* pre-allocate DVB usb transfer buffers */
-		if (dev->dvb_xfer_bulk) {
-			retval = em28xx_alloc_urbs(dev, EM28XX_DIGITAL_MODE,
-					    dev->dvb_xfer_bulk,
-					    EM28XX_DVB_NUM_BUFS,
-					    512,
-					    EM28XX_DVB_BULK_PACKET_MULTIPLIER);
-		} else {
-			retval = em28xx_alloc_urbs(dev, EM28XX_DIGITAL_MODE,
-					    dev->dvb_xfer_bulk,
-					    EM28XX_DVB_NUM_BUFS,
-					    dev->dvb_max_pkt_size_isoc,
-					    EM28XX_DVB_NUM_ISOC_PACKETS);
-		}
-		if (retval) {
-			printk(DRIVER_NAME
-			       ": Failed to pre-allocate USB transfer buffers for DVB.\n");
-			goto err_free;
-		}
 	}
 
 	request_modules(dev);
@@ -3367,7 +3347,6 @@ static void em28xx_usb_disconnect(struct usb_interface *interface)
 			    video_device_node_name(dev->vdev));
 
 		em28xx_uninit_usb_xfer(dev, EM28XX_ANALOG_MODE);
-		em28xx_uninit_usb_xfer(dev, EM28XX_DIGITAL_MODE);
 	}
 	mutex_unlock(&dev->lock);
 
diff --git a/drivers/media/usb/em28xx/em28xx-dvb.c b/drivers/media/usb/em28xx/em28xx-dvb.c
index 9d0fcc8..60510f0 100644
--- a/drivers/media/usb/em28xx/em28xx-dvb.c
+++ b/drivers/media/usb/em28xx/em28xx-dvb.c
@@ -1013,6 +1013,27 @@ static int em28xx_dvb_init(struct em28xx *dev)
 	dev->dvb = dvb;
 	dvb->fe[0] = dvb->fe[1] = NULL;
 
+	/* pre-allocate DVB usb transfer buffers */
+	if (dev->dvb_xfer_bulk) {
+		result = em28xx_alloc_urbs(dev, EM28XX_DIGITAL_MODE,
+					   dev->dvb_xfer_bulk,
+					   EM28XX_DVB_NUM_BUFS,
+					   512,
+					   EM28XX_DVB_BULK_PACKET_MULTIPLIER);
+	} else {
+		result = em28xx_alloc_urbs(dev, EM28XX_DIGITAL_MODE,
+					   dev->dvb_xfer_bulk,
+					   EM28XX_DVB_NUM_BUFS,
+					   dev->dvb_max_pkt_size_isoc,
+					   EM28XX_DVB_NUM_ISOC_PACKETS);
+	}
+	if (result) {
+		em28xx_errdev("em28xx_dvb: failed to pre-allocate USB transfer buffers for DVB.\n");
+		kfree(dvb);
+		dev->dvb = NULL;
+		return result;
+	}
+
 	mutex_lock(&dev->lock);
 	em28xx_set_mode(dev, EM28XX_DIGITAL_MODE);
 	/* init frontend */
@@ -1449,6 +1470,8 @@ static int em28xx_dvb_fini(struct em28xx *dev)
 	if (dev->dvb) {
 		struct em28xx_dvb *dvb = dev->dvb;
 
+		em28xx_uninit_usb_xfer(dev, EM28XX_DIGITAL_MODE);
+
 		if (dev->disconnected) {
 			/* We cannot tell the device to sleep
 			 * once it has been unplugged. */
-- 
1.7.10.4

[RFT/RFC PATCH 3/8] em28xx: move usb transfer uninit on device disconnect from the core to the v4l-extension

[Frank Schäfer]

Signed-off-by: Frank Schäfer <fschaefer.oss@googlemail.com>
---
 drivers/media/usb/em28xx/em28xx-cards.c |    4 +---
 drivers/media/usb/em28xx/em28xx-video.c |    2 ++
 2 Dateien geändert, 3 Zeilen hinzugefügt(+), 3 Zeilen entfernt(-)

diff --git a/drivers/media/usb/em28xx/em28xx-cards.c b/drivers/media/usb/em28xx/em28xx-cards.c
index 4d89df9..e0040f8 100644
--- a/drivers/media/usb/em28xx/em28xx-cards.c
+++ b/drivers/media/usb/em28xx/em28xx-cards.c
@@ -3342,12 +3342,10 @@ static void em28xx_usb_disconnect(struct usb_interface *interface)
 
 	v4l2_device_disconnect(&dev->v4l2_dev);
 
-	if (dev->users) {
+	if (dev->users)
 		em28xx_warn("device %s is open! Deregistration and memory deallocation are deferred on close.\n",
 			    video_device_node_name(dev->vdev));
 
-		em28xx_uninit_usb_xfer(dev, EM28XX_ANALOG_MODE);
-	}
 	mutex_unlock(&dev->lock);
 
 	em28xx_close_extension(dev);
diff --git a/drivers/media/usb/em28xx/em28xx-video.c b/drivers/media/usb/em28xx/em28xx-video.c
index 83c99e6..634e88a 100644
--- a/drivers/media/usb/em28xx/em28xx-video.c
+++ b/drivers/media/usb/em28xx/em28xx-video.c
@@ -1893,6 +1893,8 @@ static int em28xx_v4l2_fini(struct em28xx *dev)
 		return 0;
 	}
 
+	em28xx_uninit_usb_xfer(dev, EM28XX_ANALOG_MODE);
+
 	if (dev->radio_dev) {
 		if (video_is_registered(dev->radio_dev))
 			video_unregister_device(dev->radio_dev);
-- 
1.7.10.4

[RFT/RFC PATCH 4/8] em28xx: move v4l2_device_disconnect() call from the core to the v4l extension

[Frank Schäfer]

Signed-off-by: Frank Schäfer <fschaefer.oss@googlemail.com>
---
 drivers/media/usb/em28xx/em28xx-cards.c |   10 ----------
 drivers/media/usb/em28xx/em28xx-video.c |    5 +++++
 2 Dateien geändert, 5 Zeilen hinzugefügt(+), 10 Zeilen entfernt(-)

diff --git a/drivers/media/usb/em28xx/em28xx-cards.c b/drivers/media/usb/em28xx/em28xx-cards.c
index e0040f8..34ff918b 100644
--- a/drivers/media/usb/em28xx/em28xx-cards.c
+++ b/drivers/media/usb/em28xx/em28xx-cards.c
@@ -3338,16 +3338,6 @@ static void em28xx_usb_disconnect(struct usb_interface *interface)
 
 	flush_request_modules(dev);
 
-	mutex_lock(&dev->lock);
-
-	v4l2_device_disconnect(&dev->v4l2_dev);
-
-	if (dev->users)
-		em28xx_warn("device %s is open! Deregistration and memory deallocation are deferred on close.\n",
-			    video_device_node_name(dev->vdev));
-
-	mutex_unlock(&dev->lock);
-
 	em28xx_close_extension(dev);
 
 	/* NOTE: must be called BEFORE the resources are released */
diff --git a/drivers/media/usb/em28xx/em28xx-video.c b/drivers/media/usb/em28xx/em28xx-video.c
index 634e88a..7535762 100644
--- a/drivers/media/usb/em28xx/em28xx-video.c
+++ b/drivers/media/usb/em28xx/em28xx-video.c
@@ -1893,6 +1893,8 @@ static int em28xx_v4l2_fini(struct em28xx *dev)
 		return 0;
 	}
 
+	v4l2_device_disconnect(&dev->v4l2_dev);
+
 	em28xx_uninit_usb_xfer(dev, EM28XX_ANALOG_MODE);
 
 	if (dev->radio_dev) {
@@ -1921,6 +1923,9 @@ static int em28xx_v4l2_fini(struct em28xx *dev)
 		dev->vdev = NULL;
 	}
 
+	if (dev->users)
+		em28xx_warn("Device is open ! Deregistration and memory deallocation are deferred on close.\n");
+
 	return 0;
 }
 
-- 
1.7.10.4

[RFT/RFC PATCH 5/8] em28xx-v4l: move v4l2_ctrl_handler freeing and v4l2_device unregistration to em28xx_v4l2_fini

[Frank Schäfer]

v4l2_ctrl_handler_free() and v4l2_device_unregister() are currently only called
when the last user closes the device and the device is already disconnected.
But that's wrong, we need to call these functions whenever the em28xx-v4l
extension is closed and we can already do this if the device is still opened
by some users.

Signed-off-by: Frank Schäfer <fschaefer.oss@googlemail.com>
---
 drivers/media/usb/em28xx/em28xx-video.c |    7 ++++---
 1 Datei geändert, 4 Zeilen hinzugefügt(+), 3 Zeilen entfernt(-)

diff --git a/drivers/media/usb/em28xx/em28xx-video.c b/drivers/media/usb/em28xx/em28xx-video.c
index 7535762..3ac8700 100644
--- a/drivers/media/usb/em28xx/em28xx-video.c
+++ b/drivers/media/usb/em28xx/em28xx-video.c
@@ -1923,8 +1923,11 @@ static int em28xx_v4l2_fini(struct em28xx *dev)
 		dev->vdev = NULL;
 	}
 
+	v4l2_ctrl_handler_free(&dev->ctrl_handler);
+	v4l2_device_unregister(&dev->v4l2_dev);
+
 	if (dev->users)
-		em28xx_warn("Device is open ! Deregistration and memory deallocation are deferred on close.\n");
+		em28xx_warn("Device is open ! Memory deallocation is deferred on last close.\n");
 
 	return 0;
 }
@@ -1951,8 +1954,6 @@ static int em28xx_v4l2_close(struct file *filp)
 
 		if (dev->disconnected) {
 			em28xx_release_resources(dev);
-			v4l2_ctrl_handler_free(&dev->ctrl_handler);
-			v4l2_device_unregister(&dev->v4l2_dev);
 			kfree(dev->alt_max_pkt_size_isoc);
 			goto exit;
 		}
-- 
1.7.10.4

[RFT/RFC PATCH 6/8] em28xx: move v4l2 dummy clock deregistration from the core to the v4l extension

[Frank Schäfer]

Signed-off-by: Frank Schäfer <fschaefer.oss@googlemail.com>
---
 drivers/media/usb/em28xx/em28xx-cards.c |    3 ---
 drivers/media/usb/em28xx/em28xx-video.c |    6 ++++++
 2 Dateien geändert, 6 Zeilen hinzugefügt(+), 3 Zeilen entfernt(-)

diff --git a/drivers/media/usb/em28xx/em28xx-cards.c b/drivers/media/usb/em28xx/em28xx-cards.c
index 34ff918b..cc7677a 100644
--- a/drivers/media/usb/em28xx/em28xx-cards.c
+++ b/drivers/media/usb/em28xx/em28xx-cards.c
@@ -36,7 +36,6 @@
 #include <media/tvaudio.h>
 #include <media/i2c-addr.h>
 #include <media/tveeprom.h>
-#include <media/v4l2-clk.h>
 #include <media/v4l2-common.h>
 
 #include "em28xx.h"
@@ -2831,8 +2830,6 @@ void em28xx_release_resources(struct em28xx *dev)
 	if (dev->def_i2c_bus)
 		em28xx_i2c_unregister(dev, 1);
 	em28xx_i2c_unregister(dev, 0);
-	if (dev->clk)
-		v4l2_clk_unregister_fixed(dev->clk);
 
 	usb_put_dev(dev->udev);
 
diff --git a/drivers/media/usb/em28xx/em28xx-video.c b/drivers/media/usb/em28xx/em28xx-video.c
index 3ac8700..f209f95 100644
--- a/drivers/media/usb/em28xx/em28xx-video.c
+++ b/drivers/media/usb/em28xx/em28xx-video.c
@@ -42,6 +42,7 @@
 #include <media/v4l2-common.h>
 #include <media/v4l2-ioctl.h>
 #include <media/v4l2-event.h>
+#include <media/v4l2-clk.h>
 #include <media/msp3400.h>
 #include <media/tuner.h>
 
@@ -1926,6 +1927,11 @@ static int em28xx_v4l2_fini(struct em28xx *dev)
 	v4l2_ctrl_handler_free(&dev->ctrl_handler);
 	v4l2_device_unregister(&dev->v4l2_dev);
 
+	if (dev->clk) {
+		v4l2_clk_unregister_fixed(dev->clk);
+		dev->clk = NULL;
+	}
+
 	if (dev->users)
 		em28xx_warn("Device is open ! Memory deallocation is deferred on last close.\n");
 
-- 
1.7.10.4

[RFT/RFC PATCH 7/8] em28xx: always call em28xx_release_resources() in the usb disconnect handler

[Frank Schäfer]

When the usb device is disconnected, the resources are no longer available,
so there is no reason to keep them registered.

This will also fix the various sysfs group removal warnings which we can see
since kernel 3.13.

Signed-off-by: Frank Schäfer <fschaefer.oss@googlemail.com>
---
 drivers/media/usb/em28xx/em28xx-cards.c |   12 +++++-------
 drivers/media/usb/em28xx/em28xx-video.c |    1 -
 2 Dateien geändert, 5 Zeilen hinzugefügt(+), 8 Zeilen entfernt(-)

diff --git a/drivers/media/usb/em28xx/em28xx-cards.c b/drivers/media/usb/em28xx/em28xx-cards.c
index cc7677a..4150829 100644
--- a/drivers/media/usb/em28xx/em28xx-cards.c
+++ b/drivers/media/usb/em28xx/em28xx-cards.c
@@ -2827,6 +2827,8 @@ void em28xx_release_resources(struct em28xx *dev)
 {
 	/*FIXME: I2C IR should be disconnected */
 
+	mutex_lock(&dev->lock);
+
 	if (dev->def_i2c_bus)
 		em28xx_i2c_unregister(dev, 1);
 	em28xx_i2c_unregister(dev, 0);
@@ -2835,6 +2837,8 @@ void em28xx_release_resources(struct em28xx *dev)
 
 	/* Mark device as unused */
 	clear_bit(dev->devno, &em28xx_devused);
+
+	mutex_unlock(&dev->lock);
 };
 EXPORT_SYMBOL_GPL(em28xx_release_resources);
 
@@ -3337,13 +3341,7 @@ static void em28xx_usb_disconnect(struct usb_interface *interface)
 
 	em28xx_close_extension(dev);
 
-	/* NOTE: must be called BEFORE the resources are released */
-
-	mutex_lock(&dev->lock);
-	if (!dev->users)
-		em28xx_release_resources(dev);
-
-	mutex_unlock(&dev->lock);
+	em28xx_release_resources(dev);
 
 	if (!dev->users) {
 		kfree(dev->alt_max_pkt_size_isoc);
diff --git a/drivers/media/usb/em28xx/em28xx-video.c b/drivers/media/usb/em28xx/em28xx-video.c
index f209f95..87b140f 100644
--- a/drivers/media/usb/em28xx/em28xx-video.c
+++ b/drivers/media/usb/em28xx/em28xx-video.c
@@ -1959,7 +1959,6 @@ static int em28xx_v4l2_close(struct file *filp)
 		   free the remaining resources */
 
 		if (dev->disconnected) {
-			em28xx_release_resources(dev);
 			kfree(dev->alt_max_pkt_size_isoc);
 			goto exit;
 		}
-- 
1.7.10.4

[RFT/RFC PATCH 8/8] em28xx-v4l: fix the freeing of the video devices memory

[Frank Schäfer]

Remove some dead code from em28xx_v4l2_fini() and fix the leaking of the video, 
vbi and radio video_device struct memories.

Signed-off-by: Frank Schäfer <fschaefer.oss@googlemail.com>
---
 drivers/media/usb/em28xx/em28xx-video.c |   56 +++++++++++++++++--------------
 1 Datei geändert, 30 Zeilen hinzugefügt(+), 26 Zeilen entfernt(-)

diff --git a/drivers/media/usb/em28xx/em28xx-video.c b/drivers/media/usb/em28xx/em28xx-video.c
index 87b140f..a643adda 100644
--- a/drivers/media/usb/em28xx/em28xx-video.c
+++ b/drivers/media/usb/em28xx/em28xx-video.c
@@ -1899,29 +1899,19 @@ static int em28xx_v4l2_fini(struct em28xx *dev)
 	em28xx_uninit_usb_xfer(dev, EM28XX_ANALOG_MODE);
 
 	if (dev->radio_dev) {
-		if (video_is_registered(dev->radio_dev))
-			video_unregister_device(dev->radio_dev);
-		else
-			video_device_release(dev->radio_dev);
-		dev->radio_dev = NULL;
+		em28xx_info("V4L2 device %s deregistered\n",
+			    video_device_node_name(dev->radio_dev));
+		video_unregister_device(dev->radio_dev);
 	}
 	if (dev->vbi_dev) {
 		em28xx_info("V4L2 device %s deregistered\n",
 			    video_device_node_name(dev->vbi_dev));
-		if (video_is_registered(dev->vbi_dev))
-			video_unregister_device(dev->vbi_dev);
-		else
-			video_device_release(dev->vbi_dev);
-		dev->vbi_dev = NULL;
+		video_unregister_device(dev->vbi_dev);
 	}
 	if (dev->vdev) {
 		em28xx_info("V4L2 device %s deregistered\n",
 			    video_device_node_name(dev->vdev));
-		if (video_is_registered(dev->vdev))
-			video_unregister_device(dev->vdev);
-		else
-			video_device_release(dev->vdev);
-		dev->vdev = NULL;
+		video_unregister_device(dev->vdev);
 	}
 
 	v4l2_ctrl_handler_free(&dev->ctrl_handler);
@@ -1955,9 +1945,7 @@ static int em28xx_v4l2_close(struct file *filp)
 	mutex_lock(&dev->lock);
 
 	if (dev->users == 1) {
-		/* the device is already disconnect,
-		   free the remaining resources */
-
+		/* free the remaining resources if device is disconnected */
 		if (dev->disconnected) {
 			kfree(dev->alt_max_pkt_size_isoc);
 			goto exit;
@@ -1985,6 +1973,23 @@ exit:
 	return 0;
 }
 
+/*
+ * em28xx_videodevice_release()
+ * called when the last user of the video device exits and frees the memeory
+ */
+static void em28xx_videodevice_release(struct video_device *vdev)
+{
+	struct em28xx *dev = video_get_drvdata(vdev);
+
+	video_device_release(vdev);
+	if (vdev == dev->vdev)
+		dev->vdev = NULL;
+	else if (vdev == dev->vbi_dev)
+		dev->vbi_dev = NULL;
+	else if (vdev == dev->radio_dev)
+		dev->radio_dev = NULL;
+}
+
 static const struct v4l2_file_operations em28xx_v4l_fops = {
 	.owner         = THIS_MODULE,
 	.open          = em28xx_v4l2_open,
@@ -2039,11 +2044,10 @@ static const struct v4l2_ioctl_ops video_ioctl_ops = {
 };
 
 static const struct video_device em28xx_video_template = {
-	.fops                       = &em28xx_v4l_fops,
-	.release                    = video_device_release_empty,
-	.ioctl_ops 		    = &video_ioctl_ops,
-
-	.tvnorms                    = V4L2_STD_ALL,
+	.fops		= &em28xx_v4l_fops,
+	.ioctl_ops	= &video_ioctl_ops,
+	.release	= em28xx_videodevice_release,
+	.tvnorms	= V4L2_STD_ALL,
 };
 
 static const struct v4l2_file_operations radio_fops = {
@@ -2069,9 +2073,9 @@ static const struct v4l2_ioctl_ops radio_ioctl_ops = {
 };
 
 static struct video_device em28xx_radio_template = {
-	.name                 = "em28xx-radio",
-	.fops                 = &radio_fops,
-	.ioctl_ops 	      = &radio_ioctl_ops,
+	.fops		= &radio_fops,
+	.ioctl_ops	= &radio_ioctl_ops,
+	.release	= em28xx_videodevice_release,
 };
 
 /* I2C possible address to saa7115, tvp5150, msp3400, tvaudio */
-- 
1.7.10.4

Re: [RFT/RFC PATCH 0/8] em28xx: move more v4l/dvb specific code to the extension modules and fix the resources handling

[Mauro Carvalho Chehab]

Em Sun, 12 Jan 2014 17:24:17 +0100
Frank Schäfer <fschaefer.oss@googlemail.com> escreveu:

> Patch 1 is fix for the analog device initialization on the first open() call.
> Patches 2-7 move some dvb and v4l specific code from the core module to the 
> extension modules and fix the resources releasing logic/order in the core and
> the v4l2 module. This will also fix the various sysfs group remove warnings 
> which we can see since kernel 3.13.
> Patch 8 finally removes some dead code lines and fixes leaking the memory of 
> video, vbi and radio video_device structs.
> 
> I've tested all patches carefully with the devices I have, but because this is
> really critical stuff, they should be reviewed and tested by others, too.

Those seem to improve it. But there are some bugs still:

1) If the device is inserted/removed quickly several times, the machine deadly
crashes. I tested it with alsa module, and also without it.

2) If the device got removed while streaming, bad things happen.

This is what happens with both audio and video streaming (analog mode):

	[  233.084393] usb 3-2: USB disconnect, device number 2
	[  233.084936] em28xx_audio_isocirq, status -108, 64 packets (size 4416)
	[  233.085024] em2882/3 #0: disconnecting em2882/3 #0 video
	[  233.085092] em2882/3 #0: V4L2 device vbi0 deregistered
	[  233.085630] em2882/3 #0: V4L2 device video1 deregistered
	[  233.086402] em2882/3 #0: Device is open ! Memory deallocation is deferred on last close.

Logs are ok, but the application (xawtv) refuses to die.

I manually removed em28xx-alsa from /lib/modules/.../usb/em28xx. After
such change, device removal while streaming worked.

3) DVB removal while streaming worked relatively well:

[  513.709376] usb 3-2: USB disconnect, device number 5
[  513.709666] em2882/3 #0: disconnecting (null)

	Note that the device name is "(null)" here.

[  514.173191] em2882/3 #0: writing to i2c device at 0x1c failed (error=-19)
[  514.173277] lgdt330x: i2c_write_demod_bytes error (addr 02 <- 00, err = -19)
[  514.173375] em2882/3 #0: writing to i2c device at 0x1c failed (error=-19)
[  514.173428] lgdt330x: i2c_read_demod_bytes: addr 0x0e select 0x6e error (ret == -19)
[  514.173453] em2882/3 #0: writing to i2c device at 0x1c failed (error=-19)
[  514.173484] lgdt330x: i2c_read_demod_bytes: addr 0x0e select 0x6e error (ret == -19)
[  514.173507] em2882/3 #0: writing to i2c device at 0x1c failed (error=-19)
[  514.173536] lgdt330x: i2c_read_demod_bytes: addr 0x0e select 0x8b error (ret == -19)
[  514.273789] em2882/3 #0: writing to i2c device at 0x1c failed (error=-19)
[  514.273823] lgdt330x: i2c_read_demod_bytes: addr 0x0e select 0x58 error (ret == -19)
[  514.374050] em2882/3 #0: writing to i2c device at 0x1c failed (error=-19)
[  514.374089] lgdt330x: i2c_read_demod_bytes: addr 0x0e select 0x58 error (ret == -19)
[  514.474290] em2882/3 #0: writing to i2c device at 0x1c failed (error=-19)
[  514.474328] lgdt330x: i2c_read_demod_bytes: addr 0x0e select 0x58 error (ret == -19)
[  514.574553] em2882/3 #0: writing to i2c device at 0x1c failed (error=-19)
[  514.574590] lgdt330x: i2c_read_demod_bytes: addr 0x0e select 0x58 error (ret == -19)
[  514.673537] em2882/3 #0: writing to i2c device at 0x1c failed (error=-19)
[  514.673567] lgdt330x: i2c_read_demod_bytes: addr 0x0e select 0x58 error (ret == -19)
[  514.673594] em2882/3 #0: writing to i2c device at 0x1c failed (error=-19)
[  514.673611] lgdt330x: i2c_write_demod_bytes error (addr 02 <- 00, err = -19)
[  514.674788] em2882/3 #0: writing to i2c device at 0x1c failed (error=-19)
[  514.674817] lgdt330x: i2c_read_demod_bytes: addr 0x0e select 0x58 error (ret == -19)
[  514.775030] em2882/3 #0: writing to i2c device at 0x1c failed (error=-19)
[  514.775070] lgdt330x: i2c_read_demod_bytes: addr 0x0e select 0x58 error (ret == -19)
[  514.875268] em2882/3 #0: writing to i2c device at 0x1c failed (error=-19)
[  514.875295] lgdt330x: i2c_read_demod_bytes: addr 0x0e select 0x58 error (ret == -19)
[  514.975500] em2882/3 #0: writing to i2c device at 0x1c failed (error=-19)
[  514.975537] lgdt330x: i2c_read_demod_bytes: addr 0x0e select 0x58 error (ret == -19)
[  515.075739] em2882/3 #0: writing to i2c device at 0x1c failed (error=-19)
[  515.075776] lgdt330x: i2c_read_demod_bytes: addr 0x0e select 0x58 error (ret == -19)
[  515.173956] em2882/3 #0: writing to i2c device at 0x1c failed (error=-19)
[  515.173994] lgdt330x: i2c_read_demod_bytes: addr 0x0e select 0x58 error (ret == -19)
[  515.174008] em2882/3 #0: writing to i2c device at 0x1c failed (error=-19)
[  515.174026] lgdt330x: i2c_write_demod_bytes error (addr 02 <- 00, err = -19)
[  515.175970] em2882/3 #0: writing to i2c device at 0x1c failed (error=-19)
[  515.176007] lgdt330x: i2c_read_demod_bytes: addr 0x0e select 0x58 error (ret == -19)
[  515.276203] em2882/3 #0: writing to i2c device at 0x1c failed (error=-19)
[  515.276229] lgdt330x: i2c_read_demod_bytes: addr 0x0e select 0x58 error (ret == -19)
[  515.376440] em2882/3 #0: writing to i2c device at 0x1c failed (error=-19)
[  515.376477] lgdt330x: i2c_read_demod_bytes: addr 0x0e select 0x58 error (ret == -19)
[  515.476674] em2882/3 #0: writing to i2c device at 0x1c failed (error=-19)
[  515.476703] lgdt330x: i2c_read_demod_bytes: addr 0x0e select 0x58 error (ret == -19)
[  515.576912] em2882/3 #0: writing to i2c device at 0x1c failed (error=-19)
[  515.576941] lgdt330x: i2c_read_demod_bytes: addr 0x0e select 0x58 error (ret == -19)
[  515.674383] em2882/3 #0: writing to i2c device at 0x1c failed (error=-19)
[  515.674413] lgdt330x: i2c_read_demod_bytes: addr 0x0e select 0x58 error (ret == -19)
[  515.674440] em2882/3 #0: writing to i2c device at 0x1c failed (error=-19)
[  515.674458] lgdt330x: i2c_write_demod_bytes error (addr 02 <- 00, err = -19)
[  515.677155] em2882/3 #0: writing to i2c device at 0x1c failed (error=-19)
[  515.677194] lgdt330x: i2c_read_demod_bytes: addr 0x0e select 0x58 error (ret == -19)
[  515.777398] em2882/3 #0: writing to i2c device at 0x1c failed (error=-19)
[  515.777425] lgdt330x: i2c_read_demod_bytes: addr 0x0e select 0x58 error (ret == -19)
[  515.877637] em2882/3 #0: writing to i2c device at 0x1c failed (error=-19)
[  515.877675] lgdt330x: i2c_read_demod_bytes: addr 0x0e select 0x58 error (ret == -19)
[  515.977877] em2882/3 #0: writing to i2c device at 0x1c failed (error=-19)
[  515.977913] lgdt330x: i2c_read_demod_bytes: addr 0x0e select 0x58 error (ret == -19)
[  516.078110] em2882/3 #0: writing to i2c device at 0x1c failed (error=-19)
[  516.078148] lgdt330x: i2c_read_demod_bytes: addr 0x0e select 0x58 error (ret == -19)
[  516.174802] em2882/3 #0: writing to i2c device at 0x1c failed (error=-19)
[  516.174841] lgdt330x: i2c_read_demod_bytes: addr 0x0e select 0x58 error (ret == -19)
[  516.174854] em2882/3 #0: writing to i2c device at 0x1c failed (error=-19)
[  516.174872] lgdt330x: i2c_write_demod_bytes error (addr 02 <- 00, err = -19)
[  516.178347] em2882/3 #0: writing to i2c device at 0x1c failed (error=-19)
[  516.178383] lgdt330x: i2c_read_demod_bytes: addr 0x0e select 0x58 error (ret == -19)
[  516.278587] em2882/3 #0: writing to i2c device at 0x1c failed (error=-19)
[  516.278616] lgdt330x: i2c_read_demod_bytes: addr 0x0e select 0x58 error (ret == -19)
[  516.378830] em2882/3 #0: writing to i2c device at 0x1c failed (error=-19)
[  516.378869] lgdt330x: i2c_read_demod_bytes: addr 0x0e select 0x58 error (ret == -19)
[  516.479074] em2882/3 #0: writing to i2c device at 0x1c failed (error=-19)
[  516.479113] lgdt330x: i2c_read_demod_bytes: addr 0x0e select 0x58 error (ret == -19)
[  516.579364] em2882/3 #0: writing to i2c device at 0x1c failed (error=-19)
[  516.579390] lgdt330x: i2c_read_demod_bytes: addr 0x0e select 0x58 error (ret == -19)
[  516.675223] em2882/3 #0: writing to i2c device at 0x1c failed (error=-19)
[  516.675261] lgdt330x: i2c_read_demod_bytes: addr 0x0e select 0x58 error (ret == -19)
[  516.675275] em2882/3 #0: writing to i2c device at 0x1c failed (error=-19)
[  516.675292] lgdt330x: i2c_write_demod_bytes error (addr 02 <- 00, err = -19)
[  516.679632] em2882/3 #0: writing to i2c device at 0x1c failed (error=-19)
[  516.679668] lgdt330x: i2c_read_demod_bytes: addr 0x0e select 0x58 error (ret == -19)
[  516.779918] em2882/3 #0: writing to i2c device at 0x1c failed (error=-19)
[  516.779956] lgdt330x: i2c_read_demod_bytes: addr 0x0e select 0x58 error (ret == -19)
[  516.880195] em2882/3 #0: writing to i2c device at 0x1c failed (error=-19)
[  516.880232] lgdt330x: i2c_read_demod_bytes: addr 0x0e select 0x58 error (ret == -19)
[  516.980476] em2882/3 #0: writing to i2c device at 0x1c failed (error=-19)
[  516.980513] lgdt330x: i2c_read_demod_bytes: addr 0x0e select 0x58 error (ret == -19)
[  517.080761] em2882/3 #0: writing to i2c device at 0x1c failed (error=-19)
[  517.080790] lgdt330x: i2c_read_demod_bytes: addr 0x0e select 0x58 error (ret == -19)
[  517.175648] em2882/3 #0: writing to i2c device at 0x1c failed (error=-19)
[  517.175686] lgdt330x: i2c_read_demod_bytes: addr 0x0e select 0x58 error (ret == -19)
[  517.175699] em2882/3 #0: writing to i2c device at 0x1c failed (error=-19)
[  517.175717] lgdt330x: i2c_write_demod_bytes error (addr 02 <- 00, err = -19)
[  517.181043] em2882/3 #0: writing to i2c device at 0x1c failed (error=-19)
[  517.181072] lgdt330x: i2c_read_demod_bytes: addr 0x0e select 0x58 error (ret == -19)
[  517.281324] em2882/3 #0: writing to i2c device at 0x1c failed (error=-19)
[  517.281353] lgdt330x: i2c_read_demod_bytes: addr 0x0e select 0x58 error (ret == -19)
[  517.381600] em2882/3 #0: writing to i2c device at 0x1c failed (error=-19)
[  517.381637] lgdt330x: i2c_read_demod_bytes: addr 0x0e select 0x58 error (ret == -19)
[  517.481894] em2882/3 #0: writing to i2c device at 0x1c failed (error=-19)
[  517.481930] lgdt330x: i2c_read_demod_bytes: addr 0x0e select 0x58 error (ret == -19)
[  517.582177] em2882/3 #0: writing to i2c device at 0x1c failed (error=-19)
[  517.582214] lgdt330x: i2c_read_demod_bytes: addr 0x0e select 0x58 error (ret == -19)
[  517.676069] em2882/3 #0: writing to i2c device at 0x1c failed (error=-19)
[  517.676107] lgdt330x: i2c_read_demod_bytes: addr 0x0e select 0x58 error (ret == -19)
[  517.676121] em2882/3 #0: writing to i2c device at 0x1c failed (error=-19)
[  517.676138] lgdt330x: i2c_write_demod_bytes error (addr 02 <- 00, err = -19)
[  517.682459] em2882/3 #0: writing to i2c device at 0x1c failed (error=-19)
[  517.682495] lgdt330x: i2c_read_demod_bytes: addr 0x0e select 0x58 error (ret == -19)
[  517.782742] em2882/3 #0: writing to i2c device at 0x1c failed (error=-19)
[  517.782779] lgdt330x: i2c_read_demod_bytes: addr 0x0e select 0x58 error (ret == -19)
[  518.176592] em2882/3 #0: writing to i2c device at 0x1c failed (error=-19)
[  518.176683] lgdt330x: i2c_read_demod_bytes: addr 0x0e select 0x58 error (ret == -19)
[  518.176716] em2882/3 #0: writing to i2c device at 0x1c failed (error=-19)
[  518.176759] lgdt330x: i2c_write_demod_bytes error (addr 02 <- 00, err = -19)
[  518.677002] em2882/3 #0: writing to i2c device at 0x1c failed (error=-19)
[  518.677089] lgdt330x: i2c_read_demod_bytes: addr 0x0e select 0x58 error (ret == -19)
[  518.677119] em2882/3 #0: writing to i2c device at 0x1c failed (error=-19)
[  518.677160] lgdt330x: i2c_write_demod_bytes error (addr 02 <- 00, err = -19)
[  519.177419] em2882/3 #0: writing to i2c device at 0x1c failed (error=-19)
[  519.177506] lgdt330x: i2c_read_demod_bytes: addr 0x0e select 0x58 error (ret == -19)
[  519.177535] em2882/3 #0: writing to i2c device at 0x1c failed (error=-19)
[  519.177576] lgdt330x: i2c_write_demod_bytes error (addr 02 <- 00, err = -19)
[  519.677827] em2882/3 #0: writing to i2c device at 0x1c failed (error=-19)
[  519.677917] lgdt330x: i2c_read_demod_bytes: addr 0x0e select 0x58 error (ret == -19)
[  519.677950] em2882/3 #0: writing to i2c device at 0x1c failed (error=-19)
[  519.677994] lgdt330x: i2c_write_demod_bytes error (addr 02 <- 00, err = -19)
[  519.850303] xc2028 19-0061: destroying instance

In summary, it seems that there are still two bugs:

a) the error patch during device initialization has likely bugs;

b) there are still some things missing on em28xx-alsa, in order to
properly handle device removal.

Yet, there are some improvements on this series. If nobody complains, I'll
likely merge this series in a few days.

> 
> 
> Frank Schäfer (8):
>   em28xx-v4l: fix device initialization in em28xx_v4l2_open() for radio
>     and VBI mode
>   em28xx: move usb buffer pre-allocation and transfer uninit from the
>     core to the dvb extension
>   em28xx: move usb transfer uninit on device disconnect from the core to
>     the v4l-extension
>   em28xx: move v4l2_device_disconnect() call from the core to the v4l
>     extension
>   em28xx-v4l: move v4l2_ctrl_handler freeing and v4l2_device
>     unregistration to em28xx_v4l2_fini
>   em28xx: move v4l2 dummy clock deregistration from the core to the v4l
>     extension
>   em28xx: always call em28xx_release_resources() in the usb disconnect
>     handler
>   em28xx-v4l: fix the freeing of the video devices memory
> 
>  drivers/media/usb/em28xx/em28xx-cards.c |   48 ++--------------
>  drivers/media/usb/em28xx/em28xx-dvb.c   |   23 ++++++++
>  drivers/media/usb/em28xx/em28xx-video.c |   91 +++++++++++++++++++------------
>  3 Dateien geändert, 85 Zeilen hinzugefügt(+), 77 Zeilen entfernt(-)
> 


-- 

Cheers,
Mauro