Re: [PATCH v2 1/3] media: i2c: ov5647: Convert to CCI register access helpers

[Tarang Raval]

>  static int ov5647_set_mode(struct v4l2_subdev *sd)
>  {
>         struct i2c_client *client = v4l2_get_subdevdata(sd);
>         struct ov5647 *sensor = to_sensor(sd);
> -       u8 resetval, rdval;
> +       u64 resetval, rdval;
>         int ret;
> 
> -       ret = ov5647_read(sd, OV5647_SW_STANDBY, &rdval);
> +       ret = cci_read(sensor->regmap, OV5647_SW_STANDBY, &rdval, NULL);
>         if (ret < 0)
>                 return ret;
> 
> -       ret = ov5647_write_array(sd, sensor->mode->reg_list,
> -                                sensor->mode->num_regs);
> +       ret = cci_multi_reg_write(sensor->regmap, sensor->mode->reg_list,
> +                                 sensor->mode->num_regs, NULL);
>         if (ret < 0) {
>                 dev_err(&client->dev, "write sensor default regs error\n");
>                 return ret;
> @@ -706,13 +624,13 @@ static int ov5647_set_mode(struct v4l2_subdev *sd)
>         if (ret < 0)
>                 return ret;
> 
> -       ret = ov5647_read(sd, OV5647_SW_STANDBY, &resetval);
> +       ret = cci_read(sensor->regmap, OV5647_SW_STANDBY, &resetval, NULL);
>         if (ret < 0)
>                 return ret;

Are both read operations in ov5647_set_mode() really needed?

Every mode table already ends with writing 0x0100 => 0x01.

Does anyone know the purpose of these reads, or are they unused?

Best Regards,
Tarang

>         if (!(resetval & 0x01)) {
>                 dev_err(&client->dev, "Device was in SW standby");
> -               ret = ov5647_write(sd, OV5647_SW_STANDBY, 0x01);
> +               ret = cci_write(sensor->regmap, OV5647_SW_STANDBY, 0x01, NULL);
>                 if (ret < 0)
>                         return ret;
>         }