diff mbox series

[15/75] media: imx: capture: Initialize video_device programmatically

Message ID 20210105152852.5733-16-laurent.pinchart@ideasonboard.com (mailing list archive)
State New
Headers show
Series media: imx: Miscellaneous fixes and cleanups for i.MX7 | expand

Commit Message

Laurent Pinchart Jan. 5, 2021, 3:27 p.m. UTC
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@ideasonboard.com>
---
 drivers/staging/media/imx/imx-media-capture.c | 23 ++++++++-----------
 1 file changed, 9 insertions(+), 14 deletions(-)

Comments

Philipp Zabel Jan. 6, 2021, 2:31 p.m. UTC | #1
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@ideasonboard.com>
> ---
>  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@pengutronix.de>

regards
Philipp
diff mbox series

Patch

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);