On Wednesday 18 November 2009 01:38:48 Laurent Pinchart wrote:
> Fix all device drivers to use the video_drvdata function instead of
> maintaining a local list of minor to private data mappings. Call
> video_set_drvdata to register the driver private pointer when not
> already done.
>
> Where applicable, the local list of mappings is completely removed when
> it becomes unused.
>
> Signed-off-by: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx>
Very nice cleanup!
But you need to check the lock_kernel calls carefully, I think one is now
unbalanced:
> Index: v4l-dvb-mc-uvc/linux/drivers/media/video/cx88/cx88-video.c
> ===================================================================
> --- v4l-dvb-mc-uvc.orig/linux/drivers/media/video/cx88/cx88-video.c
> +++ v4l-dvb-mc-uvc/linux/drivers/media/video/cx88/cx88-video.c
> @@ -76,10 +76,6 @@ MODULE_PARM_DESC(vid_limit,"capture memo
> #define dprintk(level,fmt, arg...) if (video_debug >= level) \
> printk(KERN_DEBUG "%s/0: " fmt, core->name , ## arg)
>
> -/* ------------------------------------------------------------------ */
> -
> -static LIST_HEAD(cx8800_devlist);
> -
> /* ------------------------------------------------------------------- */
> /* static data */
>
> @@ -980,31 +976,23 @@ static int get_ressource(struct cx8800_f
> static int video_open(struct file *file)
> {
> int minor = video_devdata(file)->minor;
> - struct cx8800_dev *h,*dev = NULL;
> + struct video_device *vdev = video_devdata(file);
> + struct cx8800_dev *dev = video_drvdata(file);
> struct cx88_core *core;
> struct cx8800_fh *fh;
> enum v4l2_buf_type type = 0;
> int radio = 0;
>
> - lock_kernel();
Lock is removed.
> - list_for_each_entry(h, &cx8800_devlist, devlist) {
> - if (h->video_dev->minor == minor) {
> - dev = h;
> - type = V4L2_BUF_TYPE_VIDEO_CAPTURE;
> - }
> - if (h->vbi_dev->minor == minor) {
> - dev = h;
> - type = V4L2_BUF_TYPE_VBI_CAPTURE;
> - }
> - if (h->radio_dev &&
> - h->radio_dev->minor == minor) {
> - radio = 1;
> - dev = h;
> - }
> - }
> - if (NULL == dev) {
> - unlock_kernel();
> - return -ENODEV;
> + switch (vdev->vfl_type) {
> + case VFL_TYPE_GRABBER:
> + type = V4L2_BUF_TYPE_VIDEO_CAPTURE;
> + break;
> + case VFL_TYPE_VBI:
> + type = V4L2_BUF_TYPE_VBI_CAPTURE;
> + break;
> + case VFL_TYPE_RADIO:
> + radio = 1;
> + break;
> }
But not added here. And I assume there is an unlock at the end of this
function as well.
Regards,
Hans
--
Hans Verkuil - video4linux developer - sponsored by TANDBERG Telecom
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
