media: atomisp: fix querycap initialization logic
authorMauro Carvalho Chehab <mchehab+huawei@kernel.org>
Sat, 2 May 2020 16:15:48 +0000 (18:15 +0200)
committerMauro Carvalho Chehab <mchehab+huawei@kernel.org>
Wed, 20 May 2020 10:32:20 +0000 (12:32 +0200)
Some recent changes at V4L2 core changed the way querycap is handled.

Due to that, this warning is generated:

WARNING: CPU: 1 PID: 503 at drivers/media/v4l2-core/v4l2-dev.c:885 __video_register_device+0x93e/0x1120 [videodev]

as introduced by this commit:

commit 3c1350501c21db8e3b1a38d9e97db29694305c3b
Author: Hans Verkuil <hverkuil-cisco@xs4all.nl>
Date:   Tue Jul 23 04:21:25 2019 -0400

    media: v4l2-dev/ioctl: require non-zero device_caps, verify sane querycap results

    Now that all V4L2 drivers set device_caps in struct video_device, we can add
    a check for this to ensure all future drivers fill this in.

The fix is simple: we just need to initialize dev_caps before
registering the V4L2 dev.

While here, solve other problems at VIDIOC_QUERYCAP ioctl.

Reported-by: Patrik Gfeller <patrik.gfeller@gmail.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
drivers/staging/media/atomisp/pci/atomisp_ioctl.c
drivers/staging/media/atomisp/pci/atomisp_subdev.c
drivers/staging/media/atomisp/pci/atomisp_v4l2.c
drivers/staging/media/atomisp/pci/atomisp_v4l2.h

index 3417cd5..a5e71e5 100644 (file)
 
 #include "hrt/hive_isp_css_mm_hrt.h"
 
-/* for v4l2_capability */
 static const char *DRIVER = "atomisp"; /* max size 15 */
 static const char *CARD = "ATOM ISP";  /* max size 31 */
-static const char *BUS_INFO = "PCI-3"; /* max size 31 */
 
 /*
  * FIXME: ISP should not know beforehand all CIDs supported by sensor.
@@ -543,25 +541,18 @@ const struct atomisp_format_bridge *atomisp_get_format_bridge_from_mbus(
 /*
  * v4l2 ioctls
  * return ISP capabilities
- *
- * FIXME: capabilities should be different for video0/video2/video3
  */
 static int atomisp_querycap(struct file *file, void *fh,
                            struct v4l2_capability *cap)
 {
-       memset(cap, 0, sizeof(struct v4l2_capability));
-
-       WARN_ON(sizeof(DRIVER) > sizeof(cap->driver) ||
-               sizeof(CARD) > sizeof(cap->card) ||
-               sizeof(BUS_INFO) > sizeof(cap->bus_info));
+       struct video_device *vdev = video_devdata(file);
+       struct atomisp_device *isp = video_get_drvdata(vdev);
 
-       strncpy(cap->driver, DRIVER, sizeof(cap->driver) - 1);
-       strncpy(cap->card, CARD, sizeof(cap->card) - 1);
-       strncpy(cap->bus_info, BUS_INFO, sizeof(cap->card) - 1);
+       strscpy(cap->driver, DRIVER, sizeof(cap->driver) - 1);
+       strscpy(cap->card, CARD, sizeof(cap->card) - 1);
+       snprintf(cap->bus_info, sizeof(cap->bus_info), "PCI:%s",
+                pci_name(isp->pdev));
 
-       cap->device_caps = V4L2_CAP_VIDEO_CAPTURE |
-                          V4L2_CAP_STREAMING | V4L2_CAP_VIDEO_OUTPUT;
-       cap->capabilities = cap->device_caps | V4L2_CAP_DEVICE_CAPS;
        return 0;
 }
 
index 8f95afc..4659012 100644 (file)
@@ -1341,29 +1341,57 @@ int atomisp_subdev_register_entities(struct atomisp_sub_device *asd,
                                     struct v4l2_device *vdev)
 {
        int ret;
+       u32 device_caps;
+
+       /*
+        * FIXME: check if all device caps are properly initialized.
+        * Should any of those use V4L2_CAP_META_OUTPUT? Probably yes.
+        */
+
+       device_caps = V4L2_CAP_IO_MC |
+                     V4L2_CAP_VIDEO_CAPTURE |
+                     V4L2_CAP_STREAMING;
 
        /* Register the subdev and video node. */
+
        ret = v4l2_device_register_subdev(vdev, &asd->subdev);
        if (ret < 0)
                goto error;
 
-       ret = atomisp_video_register(&asd->video_out_capture, vdev);
+       asd->video_out_capture.vdev.v4l2_dev = vdev;
+       asd->video_out_capture.vdev.device_caps = device_caps |
+                                                 V4L2_CAP_VIDEO_OUTPUT;
+       ret = video_register_device(&asd->video_out_capture.vdev,
+                                   VFL_TYPE_VIDEO, -1);
        if (ret < 0)
                goto error;
 
-       ret = atomisp_video_register(&asd->video_out_vf, vdev);
+       asd->video_out_vf.vdev.v4l2_dev = vdev;
+       asd->video_out_vf.vdev.device_caps = device_caps |
+                                            V4L2_CAP_VIDEO_OUTPUT;
+       ret = video_register_device(&asd->video_out_vf.vdev,
+                                   VFL_TYPE_VIDEO, -1);
        if (ret < 0)
                goto error;
-
-       ret = atomisp_video_register(&asd->video_out_preview, vdev);
+       asd->video_out_preview.vdev.v4l2_dev = vdev;
+       asd->video_out_preview.vdev.device_caps = device_caps |
+                                                 V4L2_CAP_VIDEO_OUTPUT;
+       ret = video_register_device(&asd->video_out_preview.vdev,
+                                   VFL_TYPE_VIDEO, -1);
        if (ret < 0)
                goto error;
-
-       ret = atomisp_video_register(&asd->video_out_video_capture, vdev);
+       asd->video_out_video_capture.vdev.v4l2_dev = vdev;
+       asd->video_out_video_capture.vdev.device_caps = device_caps |
+                                                       V4L2_CAP_VIDEO_OUTPUT;
+       ret = video_register_device(&asd->video_out_video_capture.vdev,
+                                   VFL_TYPE_VIDEO, -1);
        if (ret < 0)
                goto error;
-
-       ret = atomisp_acc_register(&asd->video_acc, vdev);
+       asd->video_acc.vdev.v4l2_dev = vdev;
+       asd->video_acc.vdev.device_caps = device_caps |
+                                         V4L2_CAP_VIDEO_OUTPUT;
+       ret = video_register_device(&asd->video_acc.vdev,
+                                   VFL_TYPE_VIDEO, -1);
        if (ret < 0)
                goto error;
 
@@ -1373,7 +1401,12 @@ int atomisp_subdev_register_entities(struct atomisp_sub_device *asd,
         */
        if (asd->index)
                return 0;
-       ret = atomisp_video_register(&asd->video_in, vdev);
+
+       asd->video_in.vdev.v4l2_dev = vdev;
+       asd->video_in.vdev.device_caps = device_caps |
+                                         V4L2_CAP_VIDEO_CAPTURE;
+       ret = video_register_device(&asd->video_in.vdev,
+                                   VFL_TYPE_VIDEO, -1);
        if (ret < 0)
                goto error;
 
index f1bae97..fa919ac 100644 (file)
@@ -541,36 +541,6 @@ void atomisp_acc_init(struct atomisp_acc_pipe *video, const char *name)
        video_set_drvdata(&video->vdev, video->isp);
 }
 
-int atomisp_video_register(struct atomisp_video_pipe *video,
-                          struct v4l2_device *vdev)
-{
-       int ret;
-
-       video->vdev.v4l2_dev = vdev;
-
-       ret = video_register_device(&video->vdev, VFL_TYPE_VIDEO, -1);
-       if (ret < 0)
-               dev_err(vdev->dev, "%s: could not register video device (%d)\n",
-                       __func__, ret);
-
-       return ret;
-}
-
-int atomisp_acc_register(struct atomisp_acc_pipe *video,
-                        struct v4l2_device *vdev)
-{
-       int ret;
-
-       video->vdev.v4l2_dev = vdev;
-
-       ret = video_register_device(&video->vdev, VFL_TYPE_VIDEO, -1);
-       if (ret < 0)
-               dev_err(vdev->dev, "%s: could not register video device (%d)\n",
-                       __func__, ret);
-
-       return ret;
-}
-
 void atomisp_video_unregister(struct atomisp_video_pipe *video)
 {
        if (video_is_registered(&video->vdev)) {
index 37cdb98..87881fa 100644 (file)
@@ -29,11 +29,7 @@ struct firmware;
 int atomisp_video_init(struct atomisp_video_pipe *video, const char *name);
 void atomisp_acc_init(struct atomisp_acc_pipe *video, const char *name);
 void atomisp_video_unregister(struct atomisp_video_pipe *video);
-int atomisp_video_register(struct atomisp_video_pipe *video,
-                          struct v4l2_device *vdev);
 void atomisp_acc_unregister(struct atomisp_acc_pipe *video);
-int atomisp_acc_register(struct atomisp_acc_pipe *video,
-                        struct v4l2_device *vdev);
 const struct firmware *atomisp_load_firmware(struct atomisp_device *isp);
 int atomisp_csi_lane_config(struct atomisp_device *isp);