[PATCH] media: ov2740: Move pm-runtime cleanup on probe-errors to proper place

[PATCH] media: ov2740: Move pm-runtime cleanup on probe-errors to proper place

[Hans de Goede]

When v4l2_subdev_init_finalize() fails no changes have been made to
the runtime-pm device state yet, so the probe_error_media_entity_cleanup
rollback path should not touch the runtime-pm device state.

Instead this should be done from the probe_error_v4l2_subdev_cleanup
rollback path. Note the pm_runtime_xxx() calls are put above
the v4l2_subdev_cleanup() call to have the reverse call order of probe().

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/media/i2c/ov2740.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/media/i2c/ov2740.c b/drivers/media/i2c/ov2740.c
index 80d151e8ae29..6cf461e3373c 100644
--- a/drivers/media/i2c/ov2740.c
+++ b/drivers/media/i2c/ov2740.c
@@ -1456,12 +1456,12 @@ static int ov2740_probe(struct i2c_client *client)
 	return 0;
 
 probe_error_v4l2_subdev_cleanup:
+	pm_runtime_disable(&client->dev);
+	pm_runtime_set_suspended(&client->dev);
 	v4l2_subdev_cleanup(&ov2740->sd);
 
 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);
-- 
2.49.0

Re: [PATCH] media: ov2740: Move pm-runtime cleanup on probe-errors to proper place

[Bingbu Cao]

Hans,

Thank you for the patch.

Reviewed-by: Bingbu Cao <bingbu.cao@intel.com>

On 3/24/25 9:01 PM, Hans de Goede wrote:
> When v4l2_subdev_init_finalize() fails no changes have been made to
> the runtime-pm device state yet, so the probe_error_media_entity_cleanup
> rollback path should not touch the runtime-pm device state.
> 
> Instead this should be done from the probe_error_v4l2_subdev_cleanup
> rollback path. Note the pm_runtime_xxx() calls are put above
> the v4l2_subdev_cleanup() call to have the reverse call order of probe().
> 
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
>  drivers/media/i2c/ov2740.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/media/i2c/ov2740.c b/drivers/media/i2c/ov2740.c
> index 80d151e8ae29..6cf461e3373c 100644
> --- a/drivers/media/i2c/ov2740.c
> +++ b/drivers/media/i2c/ov2740.c
> @@ -1456,12 +1456,12 @@ static int ov2740_probe(struct i2c_client *client)
>  	return 0;
>  
>  probe_error_v4l2_subdev_cleanup:
> +	pm_runtime_disable(&client->dev);
> +	pm_runtime_set_suspended(&client->dev);
>  	v4l2_subdev_cleanup(&ov2740->sd);
>  
>  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);
> 

-- 
Best regards,
Bingbu Cao

Re: [PATCH] media: ov2740: Move pm-runtime cleanup on probe-errors to proper place

[Sakari Ailus]

Hi Hans,

On Mon, Mar 24, 2025 at 02:01:09PM +0100, Hans de Goede wrote:
> When v4l2_subdev_init_finalize() fails no changes have been made to
> the runtime-pm device state yet, so the probe_error_media_entity_cleanup
> rollback path should not touch the runtime-pm device state.
> 
> Instead this should be done from the probe_error_v4l2_subdev_cleanup
> rollback path. Note the pm_runtime_xxx() calls are put above
> the v4l2_subdev_cleanup() call to have the reverse call order of probe().
> 
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>


Thanks for the fix!

This also deserves:

Fixes: 289c25923ecd ("media: ov2740: Use sub-device active state")
Cc: stable@vger.kernel.org

> ---
>  drivers/media/i2c/ov2740.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/media/i2c/ov2740.c b/drivers/media/i2c/ov2740.c
> index 80d151e8ae29..6cf461e3373c 100644
> --- a/drivers/media/i2c/ov2740.c
> +++ b/drivers/media/i2c/ov2740.c
> @@ -1456,12 +1456,12 @@ static int ov2740_probe(struct i2c_client *client)
>  	return 0;
>  
>  probe_error_v4l2_subdev_cleanup:
> +	pm_runtime_disable(&client->dev);
> +	pm_runtime_set_suspended(&client->dev);
>  	v4l2_subdev_cleanup(&ov2740->sd);
>  
>  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);

-- 
Kind regards,

Sakari Ailus