Hi!

> Add support for OV5647 sensor.
> 

> +static int ov5647_write(struct v4l2_subdev *sd, u16 reg, u8 val)
> +{
> +     int ret;
> +     unsigned char data[3] = { reg >> 8, reg & 0xff, val};
> +     struct i2c_client *client = v4l2_get_subdevdata(sd);
> +
> +     ret = i2c_master_send(client, data, 3);
> +     if (ret != 3) {
> +             dev_dbg(&client->dev, "%s: i2c write error, reg: %x\n",
> +                             __func__, reg);
> +             return ret < 0 ? ret : -EIO;
> +     }
> +     return 0;
> +}

Sorry, this is wrong. It should something <0 any time error is detected.

> +static int ov5647_write_array(struct v4l2_subdev *sd,
> +                             struct regval_list *regs, int array_size)
> +{
> +     int i = 0;
> +     int ret = 0;
> +
> +     if (!regs)
> +             return -EINVAL;
> +
> +     while (i < array_size) {
> +             ret = ov5647_write(sd, regs->addr, regs->data);
> +             if (ret < 0)
> +                     return ret;
> +             i++;
> +             regs++;
> +     }
> +     return 0;
> +}

For example this expects <0 on error.

> +static int set_sw_standby(struct v4l2_subdev *sd, bool standby)
> +{
> +     int ret;
> +     unsigned char rdval;
> +
> +     ret = ov5647_read(sd, 0x0100, &rdval);
> +     if (ret != 0)
> +             return ret;
> +
> +     if (standby)
> +             ret = ov5647_write(sd, 0x0100, rdval&0xfe);
> +     else
> +             ret = ov5647_write(sd, 0x0100, rdval|0x01);
> +
> +     return ret;

if (standby)
     rdval &= 0xfe;
else
     rdval |= 0x01;

ret = ov5647_write(sd, 0x0100, rdval);

?


> +/**
> + * @short Store information about the video data format.
> + */
> +static struct sensor_format_struct {
> +     __u8 *desc;
> +     u32 mbus_code;

u8 is suitable here.


> +     ov5647_read(sd, 0x0100, &resetval);
> +             if (!resetval&0x01) {

add ()s here.

> +static int sensor_power(struct v4l2_subdev *sd, int on)
> +{
> +     int ret;
> +     struct ov5647 *ov5647 = to_state(sd);
> +     struct i2c_client *client = v4l2_get_subdevdata(sd);
> +
> +     ret = 0;
> +     mutex_lock(&ov5647->lock);
> +
> +     if (on) {
> +             dev_dbg(&client->dev, "OV5647 power on!\n");
> +
> +             ret = ov5647_write_array(sd, sensor_oe_enable_regs,
> +                             ARRAY_SIZE(sensor_oe_enable_regs));
> +
> +             ret = __sensor_init(sd);
> +
> +             if (ret < 0)
> +                     dev_err(&client->dev,
> +                             "Camera not available! Check Power!\n");
> +     } else {
> +             dev_dbg(&client->dev, "OV5647 power off!\n");
> +
> +             dev_dbg(&client->dev, "disable oe\n");
> +             ret = ov5647_write_array(sd, sensor_oe_disable_regs,
> +                             ARRAY_SIZE(sensor_oe_disable_regs));
> +
> +             if (ret < 0)
> +                     dev_dbg(&client->dev, "disable oe failed!\n");
> +
> +             ret = set_sw_standby(sd, true);
> +
> +             if (ret < 0)
> +                     dev_dbg(&client->dev, "soft stby failed!\n");

dev_err for errors? Little less "!"s in the output?

> +static int sensor_get_register(struct v4l2_subdev *sd,
> +                             struct v4l2_dbg_register *reg)
> +{
> +     unsigned char val = 0;
> +     int ret;
> +
> +     ret = ov5647_read(sd, reg->reg & 0xff, &val);
> +     reg->val = val;
> +     reg->size = 1;
> +     return ret;
> +}

Filling reg->* when read failed is strange.

> +static int sensor_set_register(struct v4l2_subdev *sd,
> +                             const struct v4l2_dbg_register *reg)
> +{
> +     ov5647_write(sd, reg->reg & 0xff, reg->val & 0xff);
> +     return 0;
> +}

error handling?

Best regards,
                                                                        Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Attachment: signature.asc
Description: Digital signature

Reply via email to