Hi Oleksij,
On Monday 17 February 2014 10:05:41 Oleksij Rempel wrote:
> Am 17.02.2014 01:26, schrieb Laurent Pinchart:
> > 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 ?
>
> It will be great! Thank you :)
>
> >> 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 ?
>
> Suddenly lsusb can't completely decode this descriptor.
The UVC-specific interface descriptors should come before the endpoint
descriptor. The uvcvideo driver supports both orders, lsusb doesn't seem to.
Patches will be appreciated :-)
> >> 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
>
> Are there any thing it specification uvc 1.0? I found only 1.1 and 1.5.
There's a UVC 1.0a.
> Another documentation from Cypress, "AN75779 How to Implement an Image
> Sensor Interface Using EZ-USB ® FX3TM in a USB Video Class (UVC)
> Framework", paragraph "5.9 Terminating the Video Streaming":
>
> "When the application closes, it issues a clear feature request on a
> Windows platform or a Set Interface with alternate setting = 0 request
> on a Mac platform."
>
> It confirms your assumption that there is no clear way to handle bulk
> stream.
So I assume your camera won't work on Mac OS.
> > 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.
>
> I tested this webcam with Windows Xp and 8. It was handled in same way:
> only CLEAR_FEATURE(HALT) was send.
>
> >> 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) ?
>
> yes
>
> > Similarly, with your bulk
> > webcam, to which endpoint is the CLEAR_FEATURE(HALT) request sent ?
>
> EP 1 IN (0x81)
>
> >> 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 ?
>
> Windows XP uses only SET INTERFACE on this webcam. Windows 7 not tested
> for now - should i?
If Windows XP and Windows 8 behave the same way I suppose we can skip Windows
7.
> >> 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.
>
> Yes, Windows 8 knows more about this webcam's then i would like to. For
> example it uses intensively Logitech specific XUs (without installing
> Logitech software): Right Light registers; some think what looks like
> watch dog. May be it knows how to disable LED :) NSA friendly system...
Nice... If you've reversed-engineered RightLight information would be welcome
:-)
> >> 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.
>
> I assume, my cam was produced after XP uvc driver. So it should be
> default behaviour.
What would you think about calling both usb_set_interface() and
usb_clear_halt() for bulk-based devices ? Something like
- usb_set_interface(stream->dev->udev, stream->intfnum, 0);
+ if (stream->intf->num_altsetting == 1)
+ usb_clear_halt(stream->dev->udev,
+ stream->header.bEndpointAddress);
--
Regards,
Laurent Pinchart
Attachment:
signature.asc
Description: This is a digitally signed message part.