Re: [PATCH 2/3] media: i2c: ov8856: Enable runtime PM before registering the sub-device

[Sakari Ailus]

Hi Babis,

On Wed, Nov 04, 2026 at 11:42:48AM +0200, Babis Danias via B4 Relay wrote:
> From: Babis Danias <b.danias@xxxxxxxxxxxx>
> 
> ov8856_probe() registers the async sub-device before it enables runtime
> PM. But the sub-device can be used as soon as it is registered. On ACPI
> systems where ipu-bridge instantiates a VCM for the sensor, the bridge's
> .bound() callback queues a work item that runtime-resumes the sensor.
> If the work runs before probe reaches pm_runtime_enable(),
> pm_runtime_get_sync() fails with -EACCES and the VCM is never created:
> 
>   [  213.265894] ov8856 i2c-OVTI8856:00: Error -13 runtime-resuming sensor, cannot instantiate VCM
> 
> On a Dell Latitude 7320 Detachable this happened on every bind of the
> rear sensor after the IPU6 ISYS driver had probed, so the dw9714 focus
> motor never showed up. Documentation/driver-api/media/v4l2-subdev.rst
> asks drivers to enable runtime PM before registering the sub-device.
> 
> Enable runtime PM first, and hold a usage reference until registration
> is done, so that the sensor can't be suspended halfway through. Drop the
> reference with pm_runtime_put(), which also lets the sensor suspend once
> probe is done, as the old pm_runtime_idle() call did. If registration
> fails, undo the runtime PM setup. The probe_error_media_entity_cleanup
> label then has no users left, so remove it.
> 
> The problem was found, and this patch written, with the help of an AI
> coding assistant (Claude Opus 5.5). See the cover letter for details.
> 
> Fixes: 879347f0c258 ("media: ov8856: Add support for OV8856 sensor")
> Cc: stable@xxxxxxxxxxxxxxx
> Signed-off-by: Babis Danias <b.danias@xxxxxxxxxxxx>
> ---
>  drivers/media/i2c/ov8856.c | 23 ++++++++++++++++-------
>  1 file changed, 16 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/media/i2c/ov8856.c b/drivers/media/i2c/ov8856.c
> index 988ce0310..f9bcdd412 100644
> --- a/drivers/media/i2c/ov8856.c
> +++ b/drivers/media/i2c/ov8856.c
> @@ -2425,22 +2425,31 @@ static int ov8856_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(ov8856->dev);
> +	/*
> +	 * The sub-device can be used as soon as it is registered, so enable
> +	 * runtime PM first, and hold a reference until registration is done.
> +	 */
> +	pm_runtime_get_noresume(ov8856->dev);

Don't do pm_runtime_get_noresume() here, instead call pm_runtime_idle()
after enabling Runtime PM, perhaps still after registering the async
sub-device.

> +	pm_runtime_enable(ov8856->dev);
> +
>  	ret = v4l2_async_register_subdev_sensor(&ov8856->sd);
>  	if (ret < 0) {
>  		dev_err(ov8856->dev, "failed to register V4L2 subdev: %d",
>  			ret);
> -		goto probe_error_media_entity_cleanup;
> +		goto probe_error_pm_runtime_disable;
>  	}
>  
> -	/* Set the device's state to active if it's in D0 state. */
> -	if (full_power)
> -		pm_runtime_set_active(ov8856->dev);
> -	pm_runtime_enable(ov8856->dev);
> -	pm_runtime_idle(ov8856->dev);
> +	pm_runtime_put(ov8856->dev);
>  
>  	return 0;
>  
> -probe_error_media_entity_cleanup:
> +probe_error_pm_runtime_disable:
> +	pm_runtime_disable(ov8856->dev);
> +	pm_runtime_set_suspended(ov8856->dev);
> +	pm_runtime_put_noidle(ov8856->dev);
>  	media_entity_cleanup(&ov8856->sd.entity);
>  
>  probe_error_v4l2_ctrl_handler_free:
> 

-- 
Regards,

Sakari Ailus