Re: [PATCH] media: v4l2: Fix the _MPLANE format check in v4l_enum_fmt()

From: Tomasz Figa <tfiga@chromium.org>
To: Boris Brezillon <boris.brezillon@collabora.com>
Cc: Mauro Carvalho Chehab <mchehab@kernel.org>,
	Hans Verkuil <hans.verkuil@cisco.com>,
	Laurent Pinchart <laurent.pinchart@ideasonboard.com>,
	Sakari Ailus <sakari.ailus@iki.fi>,
	Linux Media Mailing List <linux-media@vger.kernel.org>,
	kernel@collabora.com, Hirokazu Honda <hiroh@chromium.org>,
	Nicolas Dufresne <nicolas@ndufresne.ca>,
	Sylwester Nawrocki <s.nawrocki@samsung.com>,
	Maxime Jourdan <mjourdan@baylibre.com>
Subject: Re: [PATCH] media: v4l2: Fix the _MPLANE format check in v4l_enum_fmt()
Date: Thu, 6 Jun 2019 12:53:57 +0900	[thread overview]
Message-ID: <CAAFQd5Aji4v7fJZ+at_W58L_0+wC5E9wvSW=gCPLWswFECBMJA@mail.gmail.com> (raw)
In-Reply-To: <20190605164625.5109-1-boris.brezillon@collabora.com>

On Thu, Jun 6, 2019 at 1:46 AM Boris Brezillon
<boris.brezillon@collabora.com> wrote:
>
> CAP_M2M_MPLANE means the device supports _MPLANE formats for both
> capture and output. Adjust the check to avoid EINVAL errors on
> such devices.
>
> Fixes: 366c719d6479 ("media: v4l2: Get rid of ->vidioc_enum_fmt_vid_{cap,out}_mplane")
> Reported-by: Maxime Jourdan <mjourdan@baylibre.com>
> Signed-off-by: Boris Brezillon <boris.brezillon@collabora.com>
> ---
>  drivers/media/v4l2-core/v4l2-ioctl.c | 9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c b/drivers/media/v4l2-core/v4l2-ioctl.c
> index b4c73e8f23c5..ace9b9761bed 100644
> --- a/drivers/media/v4l2-core/v4l2-ioctl.c
> +++ b/drivers/media/v4l2-core/v4l2-ioctl.c
> @@ -1385,6 +1385,7 @@ static int v4l_enum_fmt(const struct v4l2_ioctl_ops *ops,
>         struct video_device *vdev = video_devdata(file);
>         struct v4l2_fmtdesc *p = arg;
>         int ret = check_fmt(file, p->type);
> +       u32 cap_mask;
>
>         if (ret)
>                 return ret;
> @@ -1393,7 +1394,9 @@ static int v4l_enum_fmt(const struct v4l2_ioctl_ops *ops,
>         switch (p->type) {
>         case V4L2_BUF_TYPE_VIDEO_CAPTURE:
>         case V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE:
> -               if (!!(vdev->device_caps & V4L2_CAP_VIDEO_CAPTURE_MPLANE) !=
> +               cap_mask = V4L2_CAP_VIDEO_CAPTURE_MPLANE |
> +                          V4L2_CAP_VIDEO_M2M_MPLANE;
> +               if (!!(vdev->device_caps & cap_mask) !=

Shouldn't devices that report V4L2_CAP_VIDEO_M2M_MPLANE have also
V4L2_CAP_VIDEO_CAPTURE_MPLANE and V4L2_CAP_VIDEO_OUTPUT_MPLANE
reported anyway?

Best regards,
Tomasz