Re: [PATCH 1/2] usb: gadget/uvc: remove connect/disconnect calls on open/release

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

 



Hi,

On 05/04/13 21:22, Bhupesh SHARMA 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.

thank you for the explanation, however my original question is about
avoiding critical hardware errors attended to the current gadget's design.
If you think that UDC should be controlled every time on open()/close()
syscalls, how do you see an optimal way to mitigate problems described
above?

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.

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




[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux