Hi Hans, On Tue, Jul 23, 2019 at 10:21:25AM +0200, Hans Verkuil wrote: > 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. > > Also verify that when the querycap ioctl is called the driver didn't mess > with the device_caps value and that capabilities is a superset of device_caps. > > Signed-off-by: Hans Verkuil <hverkuil-cisco@xxxxxxxxx> > --- > diff --git a/drivers/media/v4l2-core/v4l2-dev.c b/drivers/media/v4l2-core/v4l2-dev.c > index cbb74f748555..3af72e3bbd65 100644 > --- a/drivers/media/v4l2-core/v4l2-dev.c > +++ b/drivers/media/v4l2-core/v4l2-dev.c > @@ -859,6 +859,9 @@ int __video_register_device(struct video_device *vdev, > /* the v4l2_dev pointer MUST be present */ > if (WARN_ON(!vdev->v4l2_dev)) > return -EINVAL; > + /* the device_caps field MUST be set */ > + if (WARN_ON(!vdev->device_caps)) > + return -EINVAL; > > /* v4l2_fh support */ > spin_lock_init(&vdev->fh_lock); > diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c b/drivers/media/v4l2-core/v4l2-ioctl.c > index e36629ae2203..61846c355627 100644 > --- a/drivers/media/v4l2-core/v4l2-ioctl.c > +++ b/drivers/media/v4l2-core/v4l2-ioctl.c > @@ -1057,14 +1057,18 @@ static int v4l_querycap(const struct v4l2_ioctl_ops *ops, > > ret = ops->vidioc_querycap(file, fh, cap); > > - cap->capabilities |= V4L2_CAP_EXT_PIX_FORMAT; > /* > - * Drivers MUST fill in device_caps, so check for this and > - * warn if it was forgotten. > + * Drivers must not change device_caps, so check for this and > + * warn if this happened. > + */ > + WARN_ON(cap->device_caps != vfd->device_caps); > + /* > + * Check that capabilities is a superset of > + * vfd->device_caps | V4L2_CAP_DEVICE_CAPS > */ > - WARN(!(cap->capabilities & V4L2_CAP_DEVICE_CAPS) || > - !cap->device_caps, "Bad caps for driver %s, %x %x", > - cap->driver, cap->capabilities, cap->device_caps); > + WARN_ON((cap->capabilities & (vfd->device_caps | V4L2_CAP_DEVICE_CAPS)) != This is over 80, it'd be nicer it it was wrapped. Either way, Reviewed-by: Sakari Ailus <sakari.ailus@xxxxxxxxxxxxxxx> > + (vfd->device_caps | V4L2_CAP_DEVICE_CAPS)); > + cap->capabilities |= V4L2_CAP_EXT_PIX_FORMAT; > cap->device_caps |= V4L2_CAP_EXT_PIX_FORMAT; > > return ret; -- Kind regards, Sakari Ailus
