Re: [PATCH 1/5] go7007: Add struct v4l2_device.

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Peter,

Thanks for this patch series! It's looking much better now.

I did find one thing though in this patch:

On Tuesday 10 November 2009 20:21:27 Pete Eberlein wrote:
> From: Pete Eberlein <pete@xxxxxxxxxxxx>
> 
> This adds a struct v4l2_device to the go7007 device struct and registers
> it during v4l2 initialization.  The v4l2_device registration overwrites
> the go->dev device_data, which is a struct usb_interface with intfdata set
> to the struct go7007.  This changes intfdata to point to the struct
> v4l2_device inside struct go7007, which is what v4l2_device_register will
> also set it to (and warn about non-null drvdata on register.)  Since usb
> disconnect can happen any time, this intfdata should always be present.
> 
> Priority: normal
> 
> Signed-off-by: Pete Eberlein <pete@xxxxxxxxxxxx>
> 

...

> diff -r 19c0469c02c3 -r a603ad1e6a1c linux/drivers/staging/go7007/go7007-v4l2.c
> --- a/linux/drivers/staging/go7007/go7007-v4l2.c	Sat Nov 07 15:51:01 2009 -0200
> +++ b/linux/drivers/staging/go7007/go7007-v4l2.c	Tue Nov 10 10:41:56 2009 -0800
> @@ -1827,7 +1827,7 @@
>  	go->video_dev = video_device_alloc();
>  	if (go->video_dev == NULL)
>  		return -ENOMEM;
> -	memcpy(go->video_dev, &go7007_template, sizeof(go7007_template));
> +	*go->video_dev = go7007_template;
>  	go->video_dev->parent = go->dev;
>  	rv = video_register_device(go->video_dev, VFL_TYPE_GRABBER, -1);
>  	if (rv < 0) {
> @@ -1837,6 +1837,8 @@
>  	}
>  	video_set_drvdata(go->video_dev, go);
>  	++go->ref_count;
> +	v4l2_device_register(go->dev, &go->v4l2_dev);

Please check the return code here! 

You should have seen this warning when you compiled:

v4l/go7007-v4l2.c: In function 'go7007_v4l2_init':
v4l/go7007-v4l2.c:1840: warning: ignoring return value of 'v4l2_device_register', declared with attribute warn_unused_result

> +
>  	printk(KERN_INFO "%s: registered device video%d [v4l2]\n",
>  	       go->video_dev->name, go->video_dev->num);
>  
> @@ -1858,4 +1860,5 @@
>  	mutex_unlock(&go->hw_lock);
>  	if (go->video_dev)
>  		video_unregister_device(go->video_dev);
> +	v4l2_device_unregister(&go->v4l2_dev);
>  }
> 

Other than that single warning it looks good!

Can you fix and repost this patch? Then I can prepare a pull request containing
all these changes.

Thanks,

	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

[Index of Archives]     [Linux Input]     [Video for Linux]     [Gstreamer Embedded]     [Mplayer Users]     [Linux USB Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]
  Powered by Linux