Am 26.02.2014 00:00, schrieb Laurent Pinchart: > Hi Oleksij, > > On Thursday 20 February 2014 08:25:31 Oleksij Rempel wrote: >> Am 20.02.2014 01:36, schrieb Laurent Pinchart: >>> On Monday 17 February 2014 10:40:35 Oleksij Rempel wrote: >>>> Am 17.02.2014 10:28, schrieb Laurent Pinchart: >>>>> 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: >>> >>> [snip] >>> >>>>>>> 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 :-) >>>> >>>> That was easy: >>>> https://wikidevi.com/wiki/82066163-7050-ab49-b8cc-b3855e8d2252 >>>> See attachment with a testprogram. >>> >>> That's really interesting. Do you think we could integrate that in libv4l >>> ? >> >> May be, we should retest if we really can get better exposure. What was >> your experience with it? > > None :-) I've never tested it. > >> I'll collect more information about other XUs used by win driver. Right >> now i have fallowing information: >> - on stream start and stream end windows set LED in automatic mode. >> - clear_halt is probably workaround for some bug which i get if i stop >> the stream from virtual machine. >> >>>>> 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); >>>> >>>> It works too. I was just afraid to be too different form MS >>>> implementation. Manufactures usually test it against MS driver so it sort >>>> of dictate specification. >>> >>> I think I agree with you. Could you please test the following patch ? >> >> It is not working. Your patch is setting Interface to 0x80, but it >> should set it to 0x81. > > OK, next try :-) Please see below. > >>> From 04f3b43ba80ef5f3fbc26f81b8309dc166226b75 Mon Sep 17 00:00:00 2001 >>> From: Oleksij Rempel <linux@xxxxxxxxxxxxxxxx> >>> Date: Sun, 16 Feb 2014 10:59:32 +0100 >>> Subject: [PATCH] uvcvideo: Do not use usb_set_interface on bulk EP >> >> [snip] >> >>> Replace selection of alternate setting 0 with clearing of the endpoint >>> halt feature at video stream stop for bulk-based devices. Let's refrain >>> from blaming Microsoft this time, as it's not clear whether this >>> Windows-specific but USB-compliant behaviour was specifically developed >>> to handle bulkd-based UVC devices, or if the camera just took advantage >>> of it. >>> >>> CC: stable@xxxxxxxxxxxxxxx >>> Signed-off-by: Oleksij Rempel <linux@xxxxxxxxxxxxxxxx> >>> Signed-off-by: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx> >>> --- >>> >>> drivers/media/usb/uvc/uvc_video.c | 14 +++++++++++++- >>> 1 file changed, 13 insertions(+), 1 deletion(-) >>> >>> diff --git a/drivers/media/usb/uvc/uvc_video.c >>> b/drivers/media/usb/uvc/uvc_video.c index 103cd4e..29147fd 100644 >>> --- a/drivers/media/usb/uvc/uvc_video.c >>> +++ b/drivers/media/usb/uvc/uvc_video.c >>> @@ -1850,7 +1850,19 @@ 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 { >>> + /* UVC doesn't specify how to inform a bulk-based device >>> + * when the video stream is stopped. Windows sends a >>> + * CLEAR_FEATURE(HALT) request to the video streaming >>> + * bulk endpoint, mimic the same behaviour. >>> + */ >>> + usb_clear_halt(stream->dev->udev, >>> + stream->header.bEndpointAddress); > > Could you please replace that with > > unsigned int epnum = stream->header.bEndpointAddress > & USB_ENDPOINT_NUMBER_MASK; > unsigned int dir = stream->header.bEndpointAddress > & USB_ENDPOINT_DIR_MASK; > unsigned int pipe; > > pipe = usb_sndbulkpipe(stream->dev->udev, epnum) | dir; > usb_clear_halt(stream->dev->udev, pipe); > > and retest ? It works. -- Regards, Oleksij
Attachment:
signature.asc
Description: OpenPGP digital signature