[{"id":4762,"web_url":"https://patchwork.libcamera.org/comment/4762/","msgid":"<5cd4a741-68af-9276-a822-f478385188d2@xs4all.nl>","date":"2020-05-07T14:46:59","subject":"Re: [libcamera-devel] [PATCH v6 5/6] v4l2-subdev: add\n\tVIDIOC_SUBDEV_QUERYCAP ioctl","submitter":{"id":43,"url":"https://patchwork.libcamera.org/api/people/43/","name":"Hans Verkuil","email":"hverkuil-cisco@xs4all.nl"},"content":"On 07/05/2020 16:35, Jacopo Mondi wrote:\n> From: Hans Verkuil <hans.verkuil@cisco.com>\n> \n> While normal video/radio/vbi/swradio nodes have a proper QUERYCAP ioctl\n> that apps can call to determine that it is indeed a V4L2 device, there\n> is currently no equivalent for v4l-subdev nodes. Adding this ioctl will\n> solve that, and it will allow utilities like v4l2-compliance to be used\n> with these devices as well.\n> \n> SUBDEV_QUERYCAP currently returns the version and subdev_caps of the\n> subdevice. Define as the initial set of subdev_caps the read-only or\n> read/write flags, to signal to userspace which set of IOCTLs are\n> available on the subdevice.\n> \n> Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>\n> Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>\n> ---\n>  drivers/media/v4l2-core/v4l2-subdev.c | 12 ++++++++++++\n>  include/uapi/linux/v4l2-subdev.h      | 18 ++++++++++++++++++\n>  2 files changed, 30 insertions(+)\n> \n> diff --git a/drivers/media/v4l2-core/v4l2-subdev.c b/drivers/media/v4l2-core/v4l2-subdev.c\n> index 174778f9c0bc4..6ae617a1e7e78 100644\n> --- a/drivers/media/v4l2-core/v4l2-subdev.c\n> +++ b/drivers/media/v4l2-core/v4l2-subdev.c\n> @@ -15,6 +15,7 @@\n>  #include <linux/types.h>\n>  #include <linux/videodev2.h>\n>  #include <linux/export.h>\n> +#include <linux/version.h>\n> \n>  #include <media/v4l2-ctrls.h>\n>  #include <media/v4l2-device.h>\n> @@ -344,6 +345,17 @@ static long subdev_do_ioctl(struct file *file, unsigned int cmd, void *arg)\n>  \tint rval;\n> \n>  \tswitch (cmd) {\n> +\tcase VIDIOC_SUBDEV_QUERYCAP: {\n> +\t\tstruct v4l2_subdev_capability *cap = arg;\n> +\n> +\t\tmemset(cap->reserved, 0, sizeof(cap->reserved));\n> +\t\tcap->version = LINUX_VERSION_CODE;\n> +\t\tcap->capabilities = ro_subdev ? V4L2_SUBDEV_CAP_RO_SUBDEV\n> +\t\t\t\t\t      : V4L2_SUBDEV_CAP_RW_SUBDEV;\n> +\n> +\t\treturn 0;\n> +\t}\n> +\n>  \tcase VIDIOC_QUERYCTRL:\n>  \t\t/*\n>  \t\t * TODO: this really should be folded into v4l2_queryctrl (this\n> diff --git a/include/uapi/linux/v4l2-subdev.h b/include/uapi/linux/v4l2-subdev.h\n> index 03970ce307419..29d1874cb5e95 100644\n> --- a/include/uapi/linux/v4l2-subdev.h\n> +++ b/include/uapi/linux/v4l2-subdev.h\n> @@ -155,9 +155,27 @@ struct v4l2_subdev_selection {\n>  \t__u32 reserved[8];\n>  };\n> \n> +/**\n> + * struct v4l2_subdev_capability - subdev capabilities\n> + * @version: the driver versioning number\n> + * @capabilities: the subdev capabilities, see V4L2_SUBDEV_CAP_*\n> + * @reserved: for future use, set to zero for now\n> + */\n> +struct v4l2_subdev_capability {\n> +\t__u32 version;\n> +\t__u32 capabilities;\n> +\t__u32 reserved[14];\n> +};\n> +\n> +/* The v4l2 sub-device video device node is registered in read-only mode. */\n> +#define V4L2_SUBDEV_CAP_RO_SUBDEV\t\tBIT(0)\n> +/* The v4l2 sub-device video device node is registered in read/write mode. */\n> +#define V4L2_SUBDEV_CAP_RW_SUBDEV\t\tBIT(1)\n\nHuh? Read-write is the default, so why create a capability for that? Just drop\nthis cap. If bit RO_SUBDEV is set, then this is read-only, otherwise it is read-write.\nMakes perfect sense.\n\nOtherwise this series looks good, so a v7 that removes V4L2_SUBDEV_CAP_RW_SUBDEV\nshould be ready for a pull request.\n\nRegards,\n\n\tHans\n\n> +\n>  /* Backwards compatibility define --- to be removed */\n>  #define v4l2_subdev_edid v4l2_edid\n> \n> +#define VIDIOC_SUBDEV_QUERYCAP\t\t\t_IOR('V',  0, struct v4l2_subdev_capability)\n>  #define VIDIOC_SUBDEV_G_FMT\t\t\t_IOWR('V',  4, struct v4l2_subdev_format)\n>  #define VIDIOC_SUBDEV_S_FMT\t\t\t_IOWR('V',  5, struct v4l2_subdev_format)\n>  #define VIDIOC_SUBDEV_G_FRAME_INTERVAL\t\t_IOWR('V', 21, struct v4l2_subdev_frame_interval)\n> --\n> 2.26.1\n>","headers":{"Return-Path":"<hverkuil-cisco@xs4all.nl>","Received":["from lb2-smtp-cloud9.xs4all.net (lb2-smtp-cloud9.xs4all.net\n\t[194.109.24.26])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 01F16603EA\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu,  7 May 2020 16:47:02 +0200 (CEST)","from cust-b5b5937f ([IPv6:fc0c:c16d:66b8:757f:c639:739b:9d66:799d])\n\tby smtp-cloud9.xs4all.net with ESMTPA\n\tid WhnfjvQNU8hmdWhnijFrvc; Thu, 07 May 2020 16:47:02 +0200"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key; \n\tunprotected) header.d=xs4all.nl header.i=@xs4all.nl\n\theader.b=\"eErFTK8i\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=xs4all.nl; s=s1;\n\tt=1588862822; bh=xhbR5zjpUF3DY1TQnuMQP3VjlnOa3Vl/AUt6VyxHOsc=;\n\th=Subject:To:From:Message-ID:Date:MIME-Version:Content-Type:From:\n\tSubject;\n\tb=eErFTK8iJPfhRwLylknY9wjwaecw57F5avixznyw0+TrY+GQI+0WIUvftiGOTkj81\n\tVi/nW0q8qHXdv5nzOe90OxjYPmxtbBu8BBuxfxH4qQtZvSerpvG3wc/a+qzAeob3LS\n\tK00njXTDwqA4voxKqKa9mm7+TcW15zi1TicErR3ISbVayS41zsm/EqmnBHTdc4KFmK\n\t59tuQ/cdTW39CzJub9c75PxiWlTHmIq74DiUp01s1GJCBaSmp1hc5eJK+yS+1xZYqG\n\tE87ahiP7hJT6JmhOxthcocwoAS0rA1B7k/a879FzvN4X/SbKLoCgYxNTXDhgMTHh+S\n\ta0a5I+SVmrkXg==","To":"Jacopo Mondi <jacopo@jmondi.org>, linux-media@vger.kernel.org,\n\tlibcamera-devel@lists.libcamera.org","Cc":"mchehab@kernel.org, sakari.ailus@linux.intel.com,\n\tandrey.konovalov@linaro.org, laurent.pinchart@ideasonboard.com,\n\tHans Verkuil <hans.verkuil@cisco.com>","References":"<20200507143537.2945672-1-jacopo@jmondi.org>\n\t<20200507143537.2945672-6-jacopo@jmondi.org>","From":"Hans Verkuil <hverkuil-cisco@xs4all.nl>","Message-ID":"<5cd4a741-68af-9276-a822-f478385188d2@xs4all.nl>","Date":"Thu, 7 May 2020 16:46:59 +0200","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101\n\tThunderbird/68.5.0","MIME-Version":"1.0","In-Reply-To":"<20200507143537.2945672-6-jacopo@jmondi.org>","Content-Type":"text/plain; charset=utf-8","Content-Language":"en-US","Content-Transfer-Encoding":"7bit","X-CMAE-Envelope":"MS4wfEgTwCEM8NCcQbqDfPCC9ay4SeJNlmXsWgOsIqo8GfX5r+JhMtvVxbJX/Qrjb19FImg9c+u7/XBQhWnZDLoVJr0HXxM8zsEQ1XU8fPABK/xTEVOCJAv3\n\tMBQcNtN05KoVXKB/J7gx0xWQLfFXlGTb3D2ccgVTPzJFlAJ9OGi4RUTdcwOQN+FNhJdkVOLqGwcKbN1+vqvar/152E10sPmusQGAN6Uh4QgBNfvBnXKIghsC\n\tsXQQq5qXmvmT3dz75xXbpQPjCslBUOZ7lUSrVBIw/0RZA3ZMDqtgD8cjZtPIOut/CvJtmUxx+1s6u6CoOGtuQp/OG+jH5evl29eh+0rZ2M0LUIaev2hv3i6S\n\tcqq6h7QzomcnCQx/f2HXKtxu9y2erV3psl3snyXyiBZRV0zj/RTdDcI22y0IzY/y8E73tYKy00GYtUVQVDF+GVmUNbiMtGHlqKiLGgVkLap+9l+TnrE=","Subject":"Re: [libcamera-devel] [PATCH v6 5/6] v4l2-subdev: add\n\tVIDIOC_SUBDEV_QUERYCAP ioctl","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","X-List-Received-Date":"Thu, 07 May 2020 14:47:03 -0000"}}]