Re: [PATCH 11/19] media: nxp: imx7-media-csi: use _vb2_fop_release/vb2_video_unregister_device

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

On Fri, Jul 17, 2026 at 10:19:34AM +0200, Hans Verkuil wrote:
> Use _vb2_fop_release instead of manually calling vb2_queue_release
> and v4l2_fh_release.
> 
> Use vb2_video_unregister_device instead of video_unregister_device
> to ensure vb2_queue_release is called inside vb2_video_unregister_device
> if needed.

Please expand on the "if needed" to explain why this is needed.

> Signed-off-by: Hans Verkuil <hverkuil+cisco@kernel.org>
> Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Cc: Frank Li <Frank.Li@nxp.com>
> ---
>  drivers/media/platform/nxp/imx7-media-csi.c | 10 +++-------
>  1 file changed, 3 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/media/platform/nxp/imx7-media-csi.c b/drivers/media/platform/nxp/imx7-media-csi.c
> index 7ddc7ba06e3d..eb2b54944ed1 100644
> --- a/drivers/media/platform/nxp/imx7-media-csi.c
> +++ b/drivers/media/platform/nxp/imx7-media-csi.c
> @@ -1545,14 +1545,10 @@ static int imx7_csi_video_release(struct file *file)
>  
>  	mutex_lock(&csi->vdev_mutex);
>  
> -	if (file->private_data == vq->owner) {
> -		vb2_queue_release(vq);
> -		vq->owner = NULL;
> -	}
> +	_vb2_fop_release(file, NULL);
>  
>  	v4l2_pipeline_pm_put(&csi->vdev->entity);
>  
> -	v4l2_fh_release(file);

You're effectively inverting the order of v4l2_pipeline_pm_put() and
v4l2_fh_release(). Please explain in the commit message why this is
safe.

Commit messages need first and foremost to explain *why* a change is
needed. The *what* and *how* are secondary.

>  	mutex_unlock(&csi->vdev_mutex);
>  	return 0;
>  }
> @@ -1627,7 +1623,7 @@ static int imx7_csi_video_register(struct imx7_csi *csi)
>  				    MEDIA_LNK_FL_ENABLED);
>  	if (ret) {
>  		dev_err(csi->dev, "failed to create link to device node\n");
> -		video_unregister_device(vdev);
> +		vb2_video_unregister_device(vdev);
>  		return ret;
>  	}
>  
> @@ -1637,7 +1633,7 @@ static int imx7_csi_video_register(struct imx7_csi *csi)
>  static void imx7_csi_video_unregister(struct imx7_csi *csi)
>  {
>  	media_entity_cleanup(&csi->vdev->entity);
> -	video_unregister_device(csi->vdev);
> +	vb2_video_unregister_device(csi->vdev);
>  }
>  
>  static int imx7_csi_video_init(struct imx7_csi *csi)

-- 
Regards,

Laurent Pinchart