Hi Prabhakar,
One more comment.
On Fri, Oct 14, 2022 at 07:34:56PM +0100, Prabhakar wrote:
> @@ -1209,12 +1190,16 @@ static int ov5645_probe(struct i2c_client *client)
>
> dev_info(dev, "OV5645 detected at address 0x%02x\n", client->addr);
>
> + pm_runtime_set_active(dev);
> + pm_runtime_get_noresume(dev);
> + pm_runtime_enable(dev);
You won't gain anything by eanbling runtime PM here. Just move it to the
end of the function before the rest of the calls. Error handling becomes
more simple.
> +
> ret = ov5645_read_reg(ov5645, OV5645_AEC_PK_MANUAL,
> &ov5645->aec_pk_manual);
> if (ret < 0) {
> dev_err(dev, "could not read AEC/AGC mode\n");
> ret = -ENODEV;
> - goto power_down;
> + goto err_pm_runtime;
> }
>
> ret = ov5645_read_reg(ov5645, OV5645_TIMING_TC_REG20,
> @@ -1222,7 +1207,7 @@ static int ov5645_probe(struct i2c_client *client)
> if (ret < 0) {
> dev_err(dev, "could not read vflip value\n");
> ret = -ENODEV;
> - goto power_down;
> + goto err_pm_runtime;
> }
>
> ret = ov5645_read_reg(ov5645, OV5645_TIMING_TC_REG21,
> @@ -1230,23 +1215,30 @@ static int ov5645_probe(struct i2c_client *client)
> if (ret < 0) {
> dev_err(dev, "could not read hflip value\n");
> ret = -ENODEV;
> - goto power_down;
> + goto err_pm_runtime;
> }
>
> - ov5645_s_power(&ov5645->sd, false);
> -
> ret = v4l2_async_register_subdev(&ov5645->sd);
> if (ret < 0) {
> dev_err(dev, "could not register v4l2 device\n");
> + pm_runtime_disable(dev);
> + pm_runtime_set_suspended(dev);
> goto free_entity;
> }
>
> + pm_runtime_set_autosuspend_delay(dev, 1000);
> + pm_runtime_use_autosuspend(dev);
> + pm_runtime_put_autosuspend(dev);
> +
> ov5645_entity_init_cfg(&ov5645->sd, NULL);
>
> return 0;
>
> +err_pm_runtime:
> + pm_runtime_disable(dev);
> + pm_runtime_put_noidle(dev);
> power_down:
> - ov5645_s_power(&ov5645->sd, false);
> + ov5645_set_power_off(dev);
> free_entity:
> media_entity_cleanup(&ov5645->sd.entity);
> free_ctrl:
--
Kind regards,
Sakari Ailus
