Re: V4L2: switch to register_chrdev_region: needs testing/review of release() handling
- From: Laurent Pinchart <laurent.pinchart@xxxxxxxxx>
- Date: Sat, 23 Aug 2008 16:01:32 +0200
Hi Hans,
On Sunday 17 August 2008, Hans Verkuil wrote:
Hi all,
As part of my ongoing cleanup of the v4l subsystem I've been looking
into converting v4l from register_chrdev to register_chrdev_region. The
latter is more flexible and allows for a larger range of minor numbers.
In addition it allows us to intercept the release callback when the
char device's refcount reaches 0.
This is very useful for hotpluggable devices like USB webcams. Currently
usb video drivers need to do the refcounting themselves, but with this
patch they can rely on the release callback since it will only be
called when the last user has closed the device. Since current usb
drivers do the refcounting in varying degrees of competency (from 'not'
to 'if you're lucky' to 'buggy' to 'perfect') it would be nice to have
the v4l framework take care of this.
So on a disconnect the driver can call video_unregister_device() even if
an application still has the device open. Only when the application
closes as well will the release be called and the driver can do the
final cleanup.
In fact, I think with this change it should even be possible to
reconnect the webcam even while some application is still using the old
char device. In that case a new minor number will be chosen since the
old one is still in use, but otherwise the webcam should just work as
usual. This is untested, though.
Note that right now I basically copy the old release callback as
installed by cdev_init() and install our own v4l callback instead (to
be precise, I replace the ktype pointer with our own kobj_type).
It would be much cleaner if chardev.c would allow one to set a callback
explicitly. It's not difficult to do that, but before doing that I
first have to know whether my approach is working correctly.
The v4l-dvb repository with my changes is here:
http://linuxtv.org/hg/~hverkuil/v4l-dvb-cdev2/
To see the diff in question:
http://linuxtv.org/hg/~hverkuil/v4l-dvb-cdev2/rev/98acd2c1dea1
I have tested myself with the quickcam_messenger webcam. For this driver
this change actually fixed a bug: disconnecting while a capture was in
progress and then trying to use /dev/video0 would lock that second
application.
I also tested with gspca: I could find no differences here, it all
worked as before.
There are a lot more USB video devices and it would be great if people
could test with their devices to see if this doesn't break anything.
Having a release callback that is called when it is really safe to free
everything should make life a lot easier I think.
I've given your patch a try on 2.6.27-rc4 with the uvcvideo driver. Nothing
seems to have been broken so far, and the uvcvideo driver got a bit simpler
as I've been able to get rid of the refcounting logic. Good job.
Do we really need the double refcounting (class device and character device) ?
As far as I can tell, the class device is only used for the name and index
sysfs attributes. Its release callback, video_release, is called as soon as
video_unregister_device drops its reference to the class device, even when
sysfs files are still opened.
Best regards,
Laurent Pinchart
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
- Follow-Ups:
- References:
- Prev by Date: Re: 2.6.25 DMA: Out of SW-IOMMU space - Asus M2N32 AMD 8GB memory
- Next by Date: Random crashes with 2.6.27-rc3 on PPC
- Previous by thread: Re: V4L2: switch to register_chrdev_region: needs testing/review of release() handling
- Next by thread: Re: V4L2: switch to register_chrdev_region: needs testing/review of release() handling
- Index(es):
Relevant Pages
|
