| Message ID | 20210105152852.5733-16-laurent.pinchart@ideasonboard.com |
|---|---|
| State | Accepted |
| Commit | 8e574216979ec74283262938f1708d40152cc3a6 |
| Headers | show |
| Series | [01/75] media: imx: Drop dependency on I2C | expand |
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 --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);
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(-)