From mboxrd@z Thu Jan 1 00:00:00 1970 Return-path: Received: from perceval.ideasonboard.com ([95.142.166.194]:34933 "EHLO perceval.ideasonboard.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754099Ab3FKMZF (ORCPT ); Tue, 11 Jun 2013 08:25:05 -0400 From: Laurent Pinchart To: Hans Verkuil Cc: linux-media@vger.kernel.org, Guennadi Liakhovetski , Mike Isely , Hans Verkuil Subject: Re: [REVIEW PATCH 8/9] f_uvc: use v4l2_dev instead of the deprecated parent field. Date: Tue, 11 Jun 2013 14:25:12 +0200 Message-ID: <6690009.papTS4CFX7@avalon> In-Reply-To: <201306111413.34031.hverkuil@xs4all.nl> References: <1370868518-19831-1-git-send-email-hverkuil@xs4all.nl> <2686318.IXVlVQf2B1@avalon> <201306111413.34031.hverkuil@xs4all.nl> MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" Sender: linux-media-owner@vger.kernel.org List-ID: Hi Hans, On Tuesday 11 June 2013 14:13:33 Hans Verkuil wrote: > On Mon 10 June 2013 20:50:42 Laurent Pinchart wrote: > > On Monday 10 June 2013 14:48:37 Hans Verkuil wrote: > > > From: Hans Verkuil > > > > > > Signed-off-by: Hans Verkuil > > > --- > > > > > > drivers/usb/gadget/f_uvc.c | 8 +++++++- > > > drivers/usb/gadget/uvc.h | 2 ++ > > > 2 files changed, 9 insertions(+), 1 deletion(-) > > > > > > diff --git a/drivers/usb/gadget/f_uvc.c b/drivers/usb/gadget/f_uvc.c > > > index 38dcedd..762e82f 100644 > > > --- a/drivers/usb/gadget/f_uvc.c > > > +++ b/drivers/usb/gadget/f_uvc.c > > > @@ -413,7 +413,7 @@ uvc_register_video(struct uvc_device *uvc) > > > > > > if (video == NULL) > > > > > > return -ENOMEM; > > > > > > - video->parent = &cdev->gadget->dev; > > > + video->v4l2_dev = &uvc->v4l2_dev; > > > > > > video->fops = &uvc_v4l2_fops; > > > video->release = video_device_release; > > > strlcpy(video->name, cdev->gadget->name, sizeof(video->name)); > > > > > > @@ -570,6 +570,7 @@ uvc_function_unbind(struct usb_configuration *c, > > > struct > > > usb_function *f) INFO(cdev, "uvc_function_unbind\n"); > > > > > > video_unregister_device(uvc->vdev); > > > > > > + v4l2_device_unregister(&uvc->v4l2_dev); > > > > > > uvc->control_ep->driver_data = NULL; > > > uvc->video.ep->driver_data = NULL; > > > > > > @@ -697,6 +698,11 @@ uvc_function_bind(struct usb_configuration *c, > > > struct > > > usb_function *f) if ((ret = usb_function_deactivate(f)) < 0) > > > > > > goto error; > > > > > > + if (v4l2_device_register(&cdev->gadget->dev, &uvc->v4l2_dev)) { > > > + printk(KERN_INFO "v4l2_device_register failed\n"); > > > + goto error; > > > + } > > > + > > > > > > /* Initialise video. */ > > > ret = uvc_video_init(&uvc->video); > > > if (ret < 0) > > > > Shouldn't you add the corresponding cleanup code in the error path at the > > end of the function ? > > Not really necessary as long as there are no subdevices registered. Still, > it is probably safer to do it anyway. v4l2_device_unregister() calls v4l2_device_disconnect(), which in turn calls put_device() on the underlying device. Even if no subdev is registered I think that's required. > > > diff --git a/drivers/usb/gadget/uvc.h b/drivers/usb/gadget/uvc.h > > > index 817e9e1..7a9111d 100644 > > > --- a/drivers/usb/gadget/uvc.h > > > +++ b/drivers/usb/gadget/uvc.h > > > @@ -57,6 +57,7 @@ struct uvc_event > > > > > > #include > > > #include > > > #include > > > > > > +#include > > > > > > #include "uvc_queue.h" > > > > > > @@ -145,6 +146,7 @@ enum uvc_state > > > > > > struct uvc_device > > > { > > > > > > struct video_device *vdev; > > > > > > + struct v4l2_device v4l2_dev; > > > > > > enum uvc_state state; > > > struct usb_function func; > > > struct uvc_video video; -- Regards, Laurent Pinchart