Hi Oleksij,
Thank you for the patch.
On Sunday 16 February 2014 10:59:32 Oleksij Rempel wrote:
> Current uvcvideo driver do not correctly disables stream on
> Bulk VideoStream EP. This couse problems on a webcam buildin
> to Asus Zenbook UX302LA. For example it will be not disabled
> and can't be strated after usb port was suspended.
s/srated/started/
> Since usb_set_interface can be used only on Iso EP. We need some
> way to handle Bulk VS EP.
> Widnwos (xp - 8) uvcvideo driver use usb_clear_halt
s/Widnwos/Windows/
> in this case. It seems to be correct way to hadle Bulk and Int EPs.
s/hadle/handle/
Do you mind if I fix the typos and grammar when applying the patch to my tree
?
> And it solves this problem on UX302LA ultrabook.
>
> CC: stable@xxxxxxxxxxxxxxx
> Signed-off-by: Oleksij Rempel <linux@xxxxxxxxxxxxxxxx>
> ---
> drivers/media/usb/uvc/uvc_video.c | 10 +++++++++-
> 1 file changed, 9 insertions(+), 1 deletion(-)
I've copied the content of your other e-mail below to keep all discussions in
one place.
> Hello all,
>
> i have Asus UX302LA with buildin uvc webcam: 1bcf:2987 Sunplus
> Innovation Technology Inc.
Could you please send me the lsusb -v output for that camera ?
> This cam uses bulk stream and has some troubles with current linux uvc
> driver. It is driver bug since it works on plain uvcvideo windows driver
> (even XP SP2 :)).
>
> The symptom is fallowing:
> - the cam will work first time with all linux apps
> - after stream off it will take about 4 seconds untill webcam LED will
> go off too. The time is equal to usbautosuspend settings.
> - it will be impossible to start webcam second time. Even no reboot to
> windows will help. Only complete power off will restore functionality.
> - if i disable asbautosuspend, webcam will work, but LED will never go
> off even if it is not used.
>
> It looks like VS interface is not turned off on Linux and in combination
> with usb autosuspend, webcam or usb link will go to nirvana.
I agree with your analysis. Your camera seems to never stop the stream and to
crash when suspended while the stream is active.
> After comparing usb log from windows and linux i found that this webcam
> need CLEAR HALT of VideoStreaming interface. Linux driver will do only
> SET INTERFACE command. Suddenly i don't have other bulk webcams to see
> if windows do it with all of them.
>
> Here is comparison of different webcams and operations which do windows8
> on stream end:
> Asus UX302LA (UVC 1.00; Bulk) - 1bcf:2987 Sunplus Innovation Technology Inc.
> • CLEAR HALT (Interface=0x1) VS interface
The UVC specification doesn't mention clearing the streaming interface control
endpoint halt status. The specification actually doesn't state how the host
should control stream start/stop for devices that use bulk endpoints. That's a
shortcoming of UVC in my opinion
I don't know whether Microsoft has decided to address this issue in their own
way by sending a CLEAR_FEATURE(HALT) request when stopping the video stream,
or if that's a standard procedure in their UVC driver that hasn't been added
specifically for bulk endpoints. Neither the UVC specification nor the USB
specification forbid sending a CLEAR_FEATURE(HALT) request to the streaming
interface bulk endpoint in this case.
It should be noted that the USB specification allows devices to stall a
SET_INTERFACE(0) request for an interface that has a single alternate setting
(section 9.4.10). In that case the Linux USB core sends a CLEAR_FEATURE(HALT)
request to every endpoint of that interface.
> Logitech C905: (UVC - 1.00; ISO)
> • SET INTERFACE (Interface=1)
> • CLEAR HALT (Interface=0x7) Halt interrupt EP
The CLEAR_FEATURE(HALT) request is targeted at endpoints, not interfaces. I
suppose you mean endpoint 0x87 here (EP 7 IN) ? Similarly, with your bulk
webcam, to which endpoint is the CLEAR_FEATURE(HALT) request sent ?
> Logitech C600: (UVC - 1.00; ISO)
> • SET INTERFACE (Interface=1)
> • CLEAR HALT (Interface=0x7) Halt interrupt EP
>
> Logitech E3500: (UVC - 1.00; ISO)
> • CLEAR HALT (Interface=0x7) Halt interrupt EP
> • SET INTERFACE (Interface=1) (different order then in c600 and c905 )
>
> Logitech C920: (UVC 1.00; ISO)
> • SET INTERFACE (Interface=1)
> • Interrupt is EP 3, why it is not halted?
>
> Asus UX31A (UVC - 1.00; ISO) - 04f2:b330 Chicony Electronics Co., Ltd
> Asus 720p CMOS webcam
> • SET INTERFACE (Interface=1)
> • Interrupt is EP 3, why it is not halted?
>
> So far it looks like Windows8 use CLEAR HALT for Bulk and SET INTERFACE
> for Iso.
What about Windows XP and Windows 7 ? Do they use the same scheme ?
> But i don't understand why Windows8 trying to halt Interrupt if EP=7 and not
> doing that if EP=3?
No idea... I wonder if the Windows UVC driver includes device quirks.
> diff --git a/drivers/media/usb/uvc/uvc_video.c
> b/drivers/media/usb/uvc/uvc_video.c index 898c208..9d335c0 100644
> --- a/drivers/media/usb/uvc/uvc_video.c
> +++ b/drivers/media/usb/uvc/uvc_video.c
> @@ -1847,7 +1847,15 @@ int uvc_video_enable(struct uvc_streaming *stream,
> int enable)
>
> if (!enable) {
> uvc_uninit_video(stream, 1);
> - usb_set_interface(stream->dev->udev, stream->intfnum, 0);
> + if (stream->intf->num_altsetting > 1) {
> + usb_set_interface(stream->dev->udev,
> + stream->intfnum, 0);
> + } else {
> + usb_clear_halt(stream->dev->udev,
> + usb_rcvctrlpipe(stream->dev->udev,
> + stream->intfnum));
usb_rcvctrlpipe() expects an endpoint number as its second argument, not an
interface number. The following call should do.
usb_clear_halt(stream->dev->udev, stream->header.bEndpointAddress);
We probably need to validate the streaming header bEndpointAddress field when
probing the device to ensure that we won't call usb_clear_halt() on an
unrelated endpoint.
> + }
> +
This might cause a regression with other bulk devices that expect a
SET_INTERFACE(0), or don't expect a CLEAR_FEATURE(HALT), although I suppose
those devices wouldn't work with Windows then. I've tested you patch (with the
above modification) with the only bulk device I own and it doesn't seem to
cause any regression.
> uvc_queue_enable(&stream->queue, 0);
> uvc_video_clock_cleanup(stream);
> return 0;
--
Regards,
Laurent Pinchart
--
To unsubscribe from this list: send the line "unsubscribe stable" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html