Anyways I've reviewed your patch and in general I like it, I only see
one problem:
@@ -99,7 +130,8 @@ static void video_release(struct device
{
struct video_device *vfd = container_of(cd, struct video_device,
dev); -#if 1 /* keep */
+ return;
+#if 1 /* keep */
/* needed until all drivers are fixed */
if (!vfd->release)
return;
@@ -107,6 +139,7 @@ static void video_release(struct device
vfd->release(vfd);
}
+
static struct class video_class = {
.name = VIDEO_NAME,
#if LINUX_VERSION_CODE < KERNEL_VERSION(2, 6, 19)
Here you basicly make the release callback of the video class device
a no-op. First of all I think it would be better to just delete it
then to add a return, which sort of hides its an empty function now.
I thought so as well, but without a release function the low-level class code in the kernel starts complaining about the missing release.
More importantly, its wrong to make this a no-op. When a driver
unregisters a v4l device, and all cdev usage has stopped there can
still be open references to sysfs files of the video class device,
but in this case currently the video_unregister_device call will lead
to the vfd->release callback getting called freeing the vfd struct,
which contains the class device.
You might have gotten confused here: video_release() didn't call the main release callback: there was a return in front. I'd forgotten to remove that test code.
I've also tested what happens when you keep a sysfs file open: it seems to work OK in that video_release is called even though the sysfs file is still open.
That said, I've moved the cdev_del call from video_unregister_device() to the video_release() function. It makes sense not to delete the char device until that callback is called.
This patch is here: http://linuxtv.org/hg/~hverkuil/v4l-dvb-cdev2/rev/575997018499
I believe the way to fix this is todo a get on the kobj contained in
the cdev in video_register_device before registering the class
device, and then in the class device release callback do a put on
this kobj.
There is no need to do an additional get: cdev_init does that already, so the char dev stays alive at least until the cdev_del (longer if apps still keep it open).
I would be very curious to hear how well it works with the gspca driver. In particular if you can indeed reconnect while an application still has a char device open from before the disconnect. Currently the gspca own locking seems to disallow that (the new device doesn't appear until all applications stopped using the old one).