On Tue, 2021-01-05 at 17:27 +0200, Laurent Pinchart wrote:
> Overwriting the whole video_device isn't future-proof as it would
> overwrite any field initialized by video_device_alloc(). Furthermore,
> the current implementation modifies the global template video_device as
> if it were a per-instance structure, which is bad practice. To fix all
> this, initialize the video device programmatically in
> imx_media_capture_device_init().
>
> Signed-off-by: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx>
> ---
> drivers/staging/media/imx/imx-media-capture.c | 23 ++++++++-----------
> 1 file changed, 9 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/staging/media/imx/imx-media-capture.c b/drivers/staging/media/imx/imx-media-capture.c
> index d7cc1423b71e..e22d98ce5d1e 100644
> --- a/drivers/staging/media/imx/imx-media-capture.c
> +++ b/drivers/staging/media/imx/imx-media-capture.c
> @@ -672,16 +672,6 @@ static const struct v4l2_file_operations capture_fops = {
> .mmap = vb2_fop_mmap,
> };
>
> -static struct video_device capture_videodev = {
> - .fops = &capture_fops,
> - .ioctl_ops = &capture_ioctl_ops,
> - .minor = -1,
> - .release = video_device_release,
> - .vfl_dir = VFL_DIR_RX,
> - .tvnorms = V4L2_STD_NTSC | V4L2_STD_PAL | V4L2_STD_SECAM,
> - .device_caps = V4L2_CAP_VIDEO_CAPTURE | V4L2_CAP_STREAMING,
> -};
> -
> struct imx_media_buffer *
> imx_media_capture_device_next_buf(struct imx_media_video_dev *vdev)
> {
> @@ -815,17 +805,22 @@ imx_media_capture_device_init(struct device *dev, struct v4l2_subdev *src_sd,
> spin_lock_init(&priv->q_lock);
>
> /* Allocate and initialize the video device. */
> - snprintf(capture_videodev.name, sizeof(capture_videodev.name),
> - "%s capture", src_sd->name);
> -
> vfd = video_device_alloc();
> if (!vfd)
> return ERR_PTR(-ENOMEM);
>
> - *vfd = capture_videodev;
> + vfd->fops = &capture_fops;
> + vfd->ioctl_ops = &capture_ioctl_ops;
> + vfd->minor = -1;
> + vfd->release = video_device_release;
> + vfd->vfl_dir = VFL_DIR_RX;
> + vfd->tvnorms = V4L2_STD_NTSC | V4L2_STD_PAL | V4L2_STD_SECAM;
> + vfd->device_caps = V4L2_CAP_VIDEO_CAPTURE | V4L2_CAP_STREAMING;
> vfd->lock = &priv->mutex;
> vfd->queue = &priv->q;
>
> + snprintf(vfd->name, sizeof(vfd->name), "%s capture", src_sd->name);
> +
> video_set_drvdata(vfd, priv);
> priv->vdev.vfd = vfd;
> INIT_LIST_HEAD(&priv->vdev.list);
Reviewed-by: Philipp Zabel <p.zabel@xxxxxxxxxxxxxx>
regards
Philipp
