On 11/5/07, Laurent Pinchart <[EMAIL PROTECTED]> wrote:
> 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.
>
It would be possible but there is one complication that imho makes it
pointless. The problem is that I do not know how to verify the
extracted firmware, but I know how to verify the non-extracted via the
sha1 sums. So the only way I can think of is to hardcode the the
hashes of the extracted firmware into the firmware loading, but that's
a bit pointless because it's already done. I am not familiar with
bcm43xx's code base and I do not know if they have such a problem or
not and if and how they solve it. I am assuming that uploading random
firmware data to the device might damage it.
> Best regards,
>
> Laurent Pinchart
>
--
Ivan N. Zlatev
Web: http://www.i-nZ.net
"It's all some kind of whacked out conspiracy."
_______________________________________________
Linux-uvc-devel mailing list
[email protected]
https://lists.berlios.de/mailman/listinfo/linux-uvc-devel
