Hi Ivan,

On Monday 05 November 2007, Ivan N. Zlatev wrote:
> On 11/5/07, Laurent Pinchart <[EMAIL PROTECTED]> wrote:
> > Hi Ivan,
> >
> > On Sunday 04 November 2007, Ivan N. Zlatev wrote:
> > > Hello,
> > >
> > > Attached you will find a patch that adds iSight support to uvcvideo
> > > and applies to HEAD (revision 140). This patch has existed for some
> > > time now. Initially it was very instrusive to the uvcvideo code, but
> > > this has been fixed.
> >
> > I can see that, it got better :-)
> >
> > > Please, lets get this reviewed and merged with uvcvideo. It's very
> > > time consuming for me to keep it in sync with uvcvideo head. openSUSE
> > > patches uvcvideo with this patch, Gentoo too, possibly others. It
> > > would make our life much easier if we could get this upstream.
> >
> > Ok, let's try. I agree keeping patches in sync isn't exactly fun.
> >
> > > This patch enables the isight camera found in all apple macs fully,
> > > including suspend and resume support.
> > >
> > > iSight is an uvc device, but not fully. I am not an expert but it
> > > seems to use a custom packet format on top of the uvc, has different
> > > guid, has partially broken video descriptors and requires firmware.
> > > This patch deals with those in the most intrusive way I could.
> >
> > I suppose you mean the less intrusive way.
>
> Yes indeed. :-)
>
> > > Inside the uvcvideo code it will only change the decoding queue if the
> > > device is recognized as an isight device. In addition it will call
> > > uvc_load_firmware during the uvc initialization.
> > >
> > > Outside in the self-contained isight.c|h it will handle the firmware
> > > loading, patching the usb descriptors and the decoding.
> > >
> > > I would like to express my gratitude to Martin Szulecki who worked
> > > with me on getting the patch in sync with latest uvcvideo and testing
> > > it for the past two days.
> >
> > There are two issues left that bother me.
> >
> > 1. I'm still not convinced firmware loading belongs in the kernel. Let's
> > discuss the second issue first.
> >
> > 2. If I'm not mistaken, some iSight firmware revisions set the
> > bInterfaceClass and bInterfaceSubClass fields to 0xff. This won't be
> > compatible with various tests in uvc_driver.c that check for
> > SC_VIDEOSTREAMING or SC_VIDEOCONTROL in the bInterfaceSubClass field.
> >
> > Are you aware of the second problem ?
>
> Yes. There are two workarounds we thought of. One is to introduce an
> ISIGHT_BROKEN_INTF quirk and the other one is to patch the descriptors
> as far as see. Currently we do the second as it is less intrusive to
> the uvcvideo code. After the firmware is loaded (and the device
> reinitialized) isight_patch_usb_descriptor[1] from isight.c is called
> by the firmware loading just before the control is passed back to
> uvcvideo. I am not convinced that this is a 100% reliable way to do
> it, but me and a couple of other people tested on different machines
> and it seems to work quite well.
>
> [1]
> static void isight_patch_usb_descriptor (struct usb_device *dev)
> {
>       struct usb_interface *intf = NULL;
>       struct usb_host_interface *setting = NULL;
>
>       isight_printk ("Patching broken iSight USB interface descriptors\n");
>
>       intf = usb_ifnum_to_if (dev, 0);
>     if (intf != NULL) {
>         setting = &intf->altsetting[0];
>         setting->desc.bInterfaceClass         = USB_CLASS_VIDEO;
>         setting->desc.bInterfaceSubClass      = SC_VIDEOCONTROL;
>         setting->desc.bInterfaceProtocol      = PC_PROTOCOL_UNDEFINED;
>     }
>
>       intf = usb_ifnum_to_if (dev, 1);
>     if (intf != NULL) {
>         setting = &intf->altsetting[0];
>         setting->desc.bInterfaceClass         = USB_CLASS_VIDEO;
>         setting->desc.bInterfaceSubClass      = SC_VIDEOSTREAMING;
>         setting->desc.bInterfaceProtocol      = PC_PROTOCOL_UNDEFINED;
>         setting = &intf->altsetting[1];
>         setting->desc.bInterfaceClass         = USB_CLASS_VIDEO;
>         setting->desc.bInterfaceSubClass      = SC_VIDEOSTREAMING;
>         setting->desc.bInterfaceProtocol      = PC_PROTOCOL_UNDEFINED;
>     }
> }

I like the second solution better as well. I should have read isight.c in 
details before asking sorry.

Wouldn't it be possible to extract the firmware and patch it in userspace, 
even if we keep firmware loading in the driver ? That's how the bcm43xx 
driver works for instance. It would then be easier to update the firmware 
extraction code. Descriptor patching would happen in userspace as well during 
the extraction phase.

Best regards,

Laurent Pinchart
_______________________________________________
Linux-uvc-devel mailing list
[email protected]
https://lists.berlios.de/mailman/listinfo/linux-uvc-devel

Reply via email to