On Sun, May 5, 2013 at 2:22 AM, Bhupesh SHARMA <bhupesh.linux@xxxxxxxxx> wrote: > Hi, > > > On 5/3/2013 6:00 PM, Vladimir Zapolskiy wrote: >> >> Hi Laurent, >> >> thank you for the comment. >> >> On 05/03/13 02:05, Laurent Pinchart wrote: >>> >>> Hi Vladimir, >>> >>> On Friday 03 May 2013 02:00:29 Vladimir Zapolskiy wrote: >>>> >>>> On 05/03/13 01:18, Laurent Pinchart wrote: >>>>> >>>>> On Friday 03 May 2013 01:13:48 Vladimir Zapolskiy wrote: >>>>>> >>>>>> This change removes redundant calls to uvc_function_connect() and >>>>>> uvc_function_disconnect() on V4L2 device node open and release. >>>>>> >>>>>> These two functions attemp to control pull-up on D+ line directly, >>>>>> however such an action should be performed by an UDC iteself, and >>>>>> within >>>>>> the gadget there is no information about current mode of the >>>>>> controller. >>>>>> >>>>>> The UDC may be in suspended state, or an OTG controller may be in host >>>>>> mode, therefore it seems better not to try to forcibly pull-up D+ line >>>>>> on open() syscall. >>>>> >>>>> >>>>> OK, but we then need to fix the problem properly. The UVC gadget >>>>> must not >>>>> appear connected until an application opens the corresponding device. >>>>> Likewise, it must disconnect from the bus when the application >>>>> closes the >>>>> device. How can this be guaranteed properly ? >>>> >>>> >>>> For better understanding of the issue, could you explain briefly why >>>> do you >>>> prefer to have the gadget not connected to the bus, if device node is >>>> not >>>> opened? >>> >>> >>> As soon as the gadget will connect to the bus the device will be >>> enumerated by >>> the host and bound to a host driver that will query the device using UVC- >>> specific requests. The userspace application is involved in replying >>> to those >>> requests, so it needs to be bound to the device on the gadget side or the >>> initialization process on the host side will fail. >> >> >> It might be a flaw in design, if a kernel space component depends on a >> user >> space application to be operable. Also the same scenario seems to be >> invalid, >> if an application unawared of specific to UVC features of /dev/videoX >> opens >> the device node, e.g. >> http://git.kernel.org/cgit/linux/hotplug/udev.git/tree/src/v4l_id/v4l_id.c >> or yavta etc. I presume a host should dictate behaviour of device and >> gadget >> in particular, and not a target's user space application, please correct >> me. >> >> About this particular change, as I mentioned in a cover letter an >> alternative >> approach may be to add sanity checks to .pullup operations for every UDC >> driver >> (or probably to usb_gadget_connect()), but in this case it is not clear >> how UVC >> gadget is going to be notified about changes of UDC state, e.g. assume a >> test >> that /dev/videoX is opened, when OTG is in Host mode, device registration >> doesn't happen on open(), and then USB B cable is inserted to the port. >> >> I would appreciate your thoughts. > > > The whole point of having a user-space application governing the behavior of > UVC webcam gadget as per commands from a UVC host is to plug the same with a > real video capture source driver to provide the video frames captured from > say a camera sensor and route the UVC specific control requests to a real > video capture device by converting the same to equivalent V4L2 commands. > > Let's take an example. Let's say you have a camera sensor that supports > capture in two modes: > > 1. 640*480, YUV422 (default mode) > 2. 640*480, JPEG > > Lets say the UVC webcam gadget suggest to the host that it supports 640*480, > YUV422 output by default (to be in sync with the characteristics of the > actual capture source). > > Now on the Host, some user tries to open the UVC webcam appearing to him as > one /dev/videoX node using the CHEESE application, and he > tries to capture frames in MJPEG format as it is also supported by the UVC > webcam, no before streaming frames this control command should be routed as > a V4L2_S_FMT(640*480, JPEG) command to the V4L2 video capture driver. This > is where the user-space application comes into picture. > > Also note that the format is selected from the Host side before the > streaming is started. So, you should not connect to the UDC, unless you have > the user-space application up and running and is present to pass > relevant UVC commands (after converting them to equivalent V4L2 commands) to > the video capture driver. > I am curious is it possible the above two things done by kernel layer eg, usb camera driver layer. > The present design is an interface between two kernel subsystems which are > aware of each other and works fine for most use-cases. > > Regards, > Bhupesh > > >> >>>>>> Signed-off-by: Vladimir Zapolskiy<vladimir_zapolskiy@xxxxxxxxxx> >>>>>> Cc: Laurent Pinchart<laurent.pinchart@xxxxxxxxxxxxxxxx> >>>>>> --- >>>>>> >>>>>> drivers/usb/gadget/uvc_v4l2.c | 5 ----- >>>>>> 1 file changed, 5 deletions(-) >>>>>> >>>>>> diff --git a/drivers/usb/gadget/uvc_v4l2.c >>>>>> b/drivers/usb/gadget/uvc_v4l2.c >>>>>> index ad48e81..e2b66e1 100644 >>>>>> --- a/drivers/usb/gadget/uvc_v4l2.c >>>>>> +++ b/drivers/usb/gadget/uvc_v4l2.c >>>>>> @@ -132,20 +132,15 @@ uvc_v4l2_open(struct file *file) >>>>>> >>>>>> handle->device =&uvc->video; >>>>>> file->private_data =&handle->vfh; >>>>>> >>>>>> - uvc_function_connect(uvc); >>>>>> >>>>>> return 0; >>>>>> >>>>>> } >>>>>> >>>>>> static int >>>>>> uvc_v4l2_release(struct file *file) >>>>>> { >>>>>> >>>>>> - struct video_device *vdev = video_devdata(file); >>>>>> - struct uvc_device *uvc = video_get_drvdata(vdev); >>>>>> >>>>>> struct uvc_file_handle *handle = >>>>>> to_uvc_file_handle(file->private_data); >>>>>> struct uvc_video *video = handle->device; >>>>>> >>>>>> - uvc_function_disconnect(uvc); >>>>>> - >>>>>> >>>>>> uvc_video_enable(video, 0); >>>>>> uvc_free_buffers(&video->queue); >> >> -- >> To unsubscribe from this list: send the line "unsubscribe linux-usb" in >> the body of a message to majordomo@xxxxxxxxxxxxxxx >> More majordomo info at http://vger.kernel.org/majordomo-info.html > > > -- > To unsubscribe from this list: send the line "unsubscribe linux-usb" in > the body of a message to majordomo@xxxxxxxxxxxxxxx > More majordomo info at http://vger.kernel.org/majordomo-info.html -- BR, Peter Chen -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
