Re: [PATCH 5/7] media: ov2740: Enable runtime PM before registering the async subdev

[Laurent Pinchart <laurent.pinchart@ideasonboard.com>]

Hi Sakari,

Thank you for the patch.

On Fri, Sep 15, 2023 at 10:28:07AM +0300, Sakari Ailus wrote:
> Enable runtime PM before registering the async sub-device as the ipu
> bridge may try to resume the device immediately after the async sub-device

I wouldn't mention ipu bridge there, as this driver is not specific to a
particular CSI-2 receiver.

> has been registered. If runtime PM is still disabled, this will fail.
> 
> Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> ---
>  drivers/media/i2c/ov2740.c | 14 ++++++++------
>  1 file changed, 8 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/media/i2c/ov2740.c b/drivers/media/i2c/ov2740.c
> index 41d4f85470fd..319dc00e47b4 100644
> --- a/drivers/media/i2c/ov2740.c
> +++ b/drivers/media/i2c/ov2740.c
> @@ -1172,6 +1172,12 @@ static int ov2740_probe(struct i2c_client *client)
>  		goto probe_error_v4l2_ctrl_handler_free;
>  	}
>  
> +	/* Set the device's state to active if it's in D0 state. */
> +	if (full_power)
> +		pm_runtime_set_active(&client->dev);

I wonder why we need this in drivers. If ACPI has powered the device on
prior to calling probe(), couldn't it also set the PM runtime state
accordingly ?

> +	pm_runtime_enable(&client->dev);
> +	pm_runtime_idle(&client->dev);
> +

With the commit message fixed,

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

>  	ret = v4l2_async_register_subdev_sensor(&ov2740->sd);
>  	if (ret < 0) {
>  		dev_err_probe(dev, ret, "failed to register V4L2 subdev\n");
> @@ -1182,16 +1188,12 @@ static int ov2740_probe(struct i2c_client *client)
>  	if (ret)
>  		dev_warn(&client->dev, "register nvmem failed, ret %d\n", ret);
>  
> -	/* Set the device's state to active if it's in D0 state. */
> -	if (full_power)
> -		pm_runtime_set_active(&client->dev);
> -	pm_runtime_enable(&client->dev);
> -	pm_runtime_idle(&client->dev);
> -
>  	return 0;
>  
>  probe_error_media_entity_cleanup:
>  	media_entity_cleanup(&ov2740->sd.entity);
> +	pm_runtime_disable(&client->dev);
> +	pm_runtime_set_suspended(&client->dev);
>  
>  probe_error_v4l2_ctrl_handler_free:
>  	v4l2_ctrl_handler_free(ov2740->sd.ctrl_handler);

-- 
Regards,

Laurent Pinchart