Re: [PATCH 2/3] media: i2c: ov9282: Switch to using the sub-device state lock

[Tarang Raval]

Hi Xiaolei,

> Switch to using the sub-device state lock and properly call
> v4l2_subdev_init_finalize() / v4l2_subdev_cleanup() on probe() /
> remove().
>
> Signed-off-by: Xiaolei Wang <xiaolei.wang@xxxxxxxxxxxxx>

...

>
> @@ -1458,16 +1442,20 @@ static int ov9282_probe(struct i2c_client *client)
>         pm_runtime_enable(ov9282->dev);
>         pm_runtime_idle(ov9282->dev);
>
> +       ret = v4l2_async_register_subdev_sensor(&ov9282->sd);
> +       if (ret < 0)
> +               goto v4l2_subdev_cleanup;
> +
>         return 0;
>
> +v4l2_subdev_cleanup:
> +       v4l2_subdev_cleanup(&ov9282->sd);


If v4l2_async_register_subdev_sensor() fails, runtime PM remains enabled.
Can you please disable runtime PM (and set suspended state) on that error path?


>  error_media_entity:
>         media_entity_cleanup(&ov9282->sd.entity);
>  error_handler_free:
>         v4l2_ctrl_handler_free(ov9282->sd.ctrl_handler);
>  error_power_off:
>         ov9282_power_off(ov9282->dev);
> -error_mutex_destroy:
> -       mutex_destroy(&ov9282->mutex);
>
>         return ret;
>  }
> @@ -1481,9 +1469,9 @@ static int ov9282_probe(struct i2c_client *client)
>  static void ov9282_remove(struct i2c_client *client)
>  {
>         struct v4l2_subdev *sd = i2c_get_clientdata(client);
> -       struct ov9282 *ov9282 = to_ov9282(sd);
>
>         v4l2_async_unregister_subdev(sd);
> +       v4l2_subdev_cleanup(sd);
>         media_entity_cleanup(&sd->entity);
>         v4l2_ctrl_handler_free(sd->ctrl_handler);
>
> @@ -1491,8 +1479,6 @@ static void ov9282_remove(struct i2c_client *client)
>         if (!pm_runtime_status_suspended(&client->dev))
>                 ov9282_power_off(&client->dev);
>         pm_runtime_set_suspended(&client->dev);
> -
> -       mutex_destroy(&ov9282->mutex);
>  }
>
>  static const struct dev_pm_ops ov9282_pm_ops = {
> --
> 2.43.0

with above change

Reviewed-by: Tarang Raval <tarang.raval@xxxxxxxxxxxxxxxxx>

Best Regards,
Tarang