Hi Ivan,
On Monday 05 November 2007, Ivan N. Zlatev wrote:
> 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.
What do you want to protect against exactly ? Once the firmware has been
extracted from the Mac OS X driver and has been verified using it's sha1 sum,
what are the possible reasons for firmware corruption ?
> 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.
I don't know either. The Cypress chip used in the iSight doesn't validate the
firmware.
Random firmwares might damage the hardware, but the odds are extremely small.
You would have to upload random data that make a valid 8051 program, and that
program would have to setup I/Os that would conflict with the hardware. Even
then it wouldn't necessarily damage the hardware.
If firmware loading were to be carried from userspace there wouldn't be any
issue on the driver side :-) The UVC driver will require a userspace tool to
configure extension unit controls anyway, so I think we could distribute a
userspace firmware handling tool along with it.
Best regards,
Laurent Pinchart
_______________________________________________
Linux-uvc-devel mailing list
[email protected]
https://lists.berlios.de/mailman/listinfo/linux-uvc-devel
