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?
> + 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.
> +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?
> +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.
-------------------------------------------------------
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
