On Wed, Jul 13, 2005 at 03:21:34AM +0400, Alexey Dobriyan wrote:
> On Tuesday 12 July 2005 22:56, Greg KH wrote:
> > drivers/usb/media/sn9c102_ov7630.c | 394 ++++++++++++++++
>
> > + Add support for OV7630 image sensors
>
> > --- /dev/null
> > +++ b/drivers/usb/media/sn9c102_ov7630.c
>
> > +static int ov7630_init(struct sn9c102_device* cam)
> > +{
>
> > + err += sn9c102_write_reg(cam, 0x00, 0x14);
> > + err += sn9c102_write_reg(cam, 0x60, 0x17);
> > + err += sn9c102_write_reg(cam, 0x0f, 0x18);
> > + err += sn9c102_write_reg(cam, 0x50, 0x19);
> > +
>
> Does this empty line make sense? Does it logically separates 2 stages of
> initialization or it's just a typo?
It separates 2 stages of initialization.
>
> > + err += sn9c102_i2c_write(cam, 0x12, 0x8d);
> > + err += sn9c102_i2c_write(cam, 0x11, 0x00);
> > + err += sn9c102_i2c_write(cam, 0x15, 0x34);
> > + err += sn9c102_i2c_write(cam, 0x16, 0x03);
> > + err += sn9c102_i2c_write(cam, 0x17, 0x1c);
> > + err += sn9c102_i2c_write(cam, 0x18, 0xbd);
> > + err += sn9c102_i2c_write(cam, 0x19, 0x06);
> > + err += sn9c102_i2c_write(cam, 0x1a, 0xf6);
> > + err += sn9c102_i2c_write(cam, 0x1b, 0x04);
> > + err += sn9c102_i2c_write(cam, 0x20, 0x44);
> > + err += sn9c102_i2c_write(cam, 0x23, 0xee);
> > + err += sn9c102_i2c_write(cam, 0x26, 0xa0);
> > + err += sn9c102_i2c_write(cam, 0x27, 0x9a);
> > + err += sn9c102_i2c_write(cam, 0x28, 0x20);
> > + err += sn9c102_i2c_write(cam, 0x29, 0x30);
> > + err += sn9c102_i2c_write(cam, 0x2f, 0x3d);
> > + err += sn9c102_i2c_write(cam, 0x30, 0x24);
> > + err += sn9c102_i2c_write(cam, 0x32, 0x86);
> > + err += sn9c102_i2c_write(cam, 0x60, 0xa9);
> > + err += sn9c102_i2c_write(cam, 0x61, 0x42);
> > + err += sn9c102_i2c_write(cam, 0x65, 0x00);
> > + err += sn9c102_i2c_write(cam, 0x69, 0x38);
> > + err += sn9c102_i2c_write(cam, 0x6f, 0x88);
> > + err += sn9c102_i2c_write(cam, 0x70, 0x0b);
> > + err += sn9c102_i2c_write(cam, 0x71, 0x00);
> > + err += sn9c102_i2c_write(cam, 0x74, 0x21);
> > + err += sn9c102_i2c_write(cam, 0x7d, 0xf7);
>
> Perhaps, a little array with these magic constants won't hurt. I'm not
> insisting, though.
I prefer the current style.
>
> > +static struct sn9c102_sensor ov7630 = {
>
> > + .qctrl = {
>
> > + {
> > + .id = V4L2_CID_DO_WHITE_BALANCE,
> > + .type = V4L2_CTRL_TYPE_INTEGER,
> > + .name = "white balance background: blue",
> > + .minimum = 0x00,
> > + .maximum = 0x3f,
> > + .step = 0x01,
> > + .default_value = 0x20,
> > + .flags = 0,
> > + },
> > + {
> > + .id = V4L2_CID_WHITENESS,
> > + .type = V4L2_CTRL_TYPE_INTEGER,
> > + .name = "white balance background: red",
> > + .minimum = 0x00,
> > + .maximum = 0x3f,
> > + .step = 0x01,
> > + .default_value = 0x20,
> > + .flags = 0,
> > + },
>
> Looks suspicious. In other places .id has the same meaning as .name. Is this
> correct?
Yes
>
> > +int sn9c102_probe_ov7630(struct sn9c102_device* cam)
> > +{
> >
> > + if (le16_to_cpu(ov7630.usbdev->descriptor.idProduct) != 0x608f &&
> > + le16_to_cpu(ov7630.usbdev->descriptor.idProduct) != 0x602c)
>
> You can switch to cpu_to_le16(). Those on the right side are constants.
Yes, correct.
-------------------------------------------------------
This SF.Net email is sponsored by the 'Do More With Dual!' webinar happening
July 14 at 8am PDT/11am EDT. We invite you to explore the latest in dual
core and dual graphics technology at this free one hour event hosted by HP,
AMD, and NVIDIA. To register visit http://www.hp.com/go/dualwebinar
_______________________________________________
linux-usb-devel@lists.sourceforge.net
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
