[{"id":4359,"web_url":"https://patchwork.libcamera.org/comment/4359/","msgid":"","date":"2020-04-01T11:28:08","subject":"Re: [libcamera-devel] [v2 3/3] media: v4l2-dev: Add\n\tv4l2_device_register_ro_subdev_node()","submitter":{"id":43,"url":"https://patchwork.libcamera.org/api/people/43/","name":"Hans Verkuil","email":"hverkuil-cisco@xs4all.nl"},"content":"On 3/27/20 11:35 PM, Jacopo Mondi wrote:\n> Add to the V4L2 core a function to register device nodes for video\n> subdevices in read-only mode.\n> \n> Registering a device node in read-only mode is useful to expose to\n> userspace the current sub-device configuration, without allowing\n> application to change it by using the V4L2 subdevice ioctls.\n> \n> Signed-off-by: Jacopo Mondi \n> ---\n> drivers/media/v4l2-core/v4l2-device.c | 7 +++--\n> drivers/media/v4l2-core/v4l2-subdev.c | 19 ++++++++++++\n> include/media/v4l2-dev.h | 7 +++++\n> include/media/v4l2-device.h | 42 ++++++++++++++++++++++++---\n> 4 files changed, 69 insertions(+), 6 deletions(-)\n> \n> diff --git a/drivers/media/v4l2-core/v4l2-device.c b/drivers/media/v4l2-core/v4l2-device.c\n> index c69941214bb2..4517a70379ce 100644\n> --- a/drivers/media/v4l2-core/v4l2-device.c\n> +++ b/drivers/media/v4l2-core/v4l2-device.c\n> @@ -186,7 +186,8 @@ static void v4l2_device_release_subdev_node(struct video_device *vdev)\n> \tkfree(vdev);\n> }\n> \n> -int v4l2_device_register_subdev_nodes(struct v4l2_device *v4l2_dev)\n> +int __v4l2_device_register_subdev_nodes(struct v4l2_device *v4l2_dev,\n> +\t\t\t\t\tbool read_only)\n> {\n> \tstruct video_device *vdev;\n> \tstruct v4l2_subdev *sd;\n> @@ -215,6 +216,8 @@ int v4l2_device_register_subdev_nodes(struct v4l2_device *v4l2_dev)\n> \t\tvdev->fops = &v4l2_subdev_fops;\n> \t\tvdev->release = v4l2_device_release_subdev_node;\n> \t\tvdev->ctrl_handler = sd->ctrl_handler;\n> +\t\tif (read_only)\n> +\t\t\tset_bit(V4L2_FL_RO_DEVNODE, &vdev->flags);\n\nI noticed the discussion about set_bit vs BIT: if memory serves, then at some point\nin time setting/testing/clearing the V4L2_FL_REGISTERED had to be atomic. However,\nlooking at it today this no longer appears to be needed, so it can probably all be\nreplaced by normal bit operations. But that should be done in a separate patch if\nanyone is interested in making that change.\n\n> \t\terr = __video_register_device(vdev, VFL_TYPE_SUBDEV, -1, 1,\n> \t\t\t\t\t sd->owner);\n> \t\tif (err < 0) {\n> @@ -252,7 +255,7 @@ int v4l2_device_register_subdev_nodes(struct v4l2_device *v4l2_dev)\n> \n> \treturn err;\n> }\n> -EXPORT_SYMBOL_GPL(v4l2_device_register_subdev_nodes);\n> +EXPORT_SYMBOL_GPL(__v4l2_device_register_subdev_nodes);\n> \n> void v4l2_device_unregister_subdev(struct v4l2_subdev *sd)\n> {\n> diff --git a/drivers/media/v4l2-core/v4l2-subdev.c b/drivers/media/v4l2-core/v4l2-subdev.c\n> index a376b351135f..87fea21919fc 100644\n> --- a/drivers/media/v4l2-core/v4l2-subdev.c\n> +++ b/drivers/media/v4l2-core/v4l2-subdev.c\n> @@ -331,6 +331,7 @@ static long subdev_do_ioctl(struct file *file, unsigned int cmd, void *arg)\n> \tstruct v4l2_fh *vfh = file->private_data;\n> #if defined(CONFIG_VIDEO_V4L2_SUBDEV_API)\n> \tstruct v4l2_subdev_fh *subdev_fh = to_v4l2_subdev_fh(vfh);\n> +\tbool ro_subdev = test_bit(V4L2_FL_RO_DEVNODE, &vdev->flags);\n> #endif\n> \tint rval;\n> \n> @@ -477,6 +478,9 @@ static long subdev_do_ioctl(struct file *file, unsigned int cmd, void *arg)\n> \tcase VIDIOC_SUBDEV_S_FMT: {\n> \t\tstruct v4l2_subdev_format *format = arg;\n> \n> +\t\tif (format->which != V4L2_SUBDEV_FORMAT_TRY && ro_subdev)\n> +\t\t\treturn -EPERM;\n> +\n> \t\tmemset(format->reserved, 0, sizeof(format->reserved));\n> \t\tmemset(format->format.reserved, 0, sizeof(format->format.reserved));\n> \t\treturn v4l2_subdev_call(sd, pad, set_fmt, subdev_fh->pad, format);\n> @@ -504,6 +508,9 @@ static long subdev_do_ioctl(struct file *file, unsigned int cmd, void *arg)\n> \t\tstruct v4l2_subdev_crop *crop = arg;\n> \t\tstruct v4l2_subdev_selection sel;\n> \n> +\t\tif (crop->which != V4L2_SUBDEV_FORMAT_TRY && ro_subdev)\n> +\t\t\treturn -EPERM;\n> +\n> \t\tmemset(crop->reserved, 0, sizeof(crop->reserved));\n> \t\tmemset(&sel, 0, sizeof(sel));\n> \t\tsel.which = crop->which;\n> @@ -545,6 +552,9 @@ static long subdev_do_ioctl(struct file *file, unsigned int cmd, void *arg)\n> \tcase VIDIOC_SUBDEV_S_FRAME_INTERVAL: {\n> \t\tstruct v4l2_subdev_frame_interval *fi = arg;\n> \n> +\t\tif (ro_subdev)\n> +\t\t\treturn -EPERM;\n> +\n> \t\tmemset(fi->reserved, 0, sizeof(fi->reserved));\n> \t\treturn v4l2_subdev_call(sd, video, s_frame_interval, arg);\n> \t}\n> @@ -568,6 +578,9 @@ static long subdev_do_ioctl(struct file *file, unsigned int cmd, void *arg)\n> \tcase VIDIOC_SUBDEV_S_SELECTION: {\n> \t\tstruct v4l2_subdev_selection *sel = arg;\n> \n> +\t\tif (sel->which != V4L2_SUBDEV_FORMAT_TRY && ro_subdev)\n> +\t\t\treturn -EPERM;\n> +\n> \t\tmemset(sel->reserved, 0, sizeof(sel->reserved));\n> \t\treturn v4l2_subdev_call(\n> \t\t\tsd, pad, set_selection, subdev_fh->pad, sel);\n> @@ -604,6 +617,9 @@ static long subdev_do_ioctl(struct file *file, unsigned int cmd, void *arg)\n> \t\treturn v4l2_subdev_call(sd, video, g_dv_timings, arg);\n> \n> \tcase VIDIOC_SUBDEV_S_DV_TIMINGS:\n> +\t\tif (ro_subdev)\n> +\t\t\treturn -EPERM;\n> +\n> \t\treturn v4l2_subdev_call(sd, video, s_dv_timings, arg);\n> \n> \tcase VIDIOC_SUBDEV_G_STD:\n> @@ -612,6 +628,9 @@ static long subdev_do_ioctl(struct file *file, unsigned int cmd, void *arg)\n> \tcase VIDIOC_SUBDEV_S_STD: {\n> \t\tv4l2_std_id *std = arg;\n> \n> +\t\tif (ro_subdev)\n> +\t\t\treturn -EPERM;\n> +\n> \t\treturn v4l2_subdev_call(sd, video, s_std, *std);\n> \t}\n> \n> diff --git a/include/media/v4l2-dev.h b/include/media/v4l2-dev.h\n> index 4602c15ff878..bdbd953a976c 100644\n> --- a/include/media/v4l2-dev.h\n> +++ b/include/media/v4l2-dev.h\n> @@ -82,11 +82,18 @@ struct v4l2_ctrl_handler;\n> *\tbut the old crop API will still work as expected in order to preserve\n> *\tbackwards compatibility.\n> *\tNever set this flag for new drivers.\n> + * @V4L2_FL_RO_DEVNODE:\n> + *\tindicates that the video device node is registered in read-only mode.\n> + *\tThe flag only applies to device nodes registered for sub-devices, it is\n> + *\tset by the core when the sub-devices device nodes are registered with\n> + *\tv4l2_device_register_ro_subdev_nodes() and used by the sub-device ioctl\n> + *\thandler to restrict access to some ioctl calls.\n> */\n> enum v4l2_video_device_flags {\n> \tV4L2_FL_REGISTERED\t\t= 0,\n> \tV4L2_FL_USES_V4L2_FH\t\t= 1,\n> \tV4L2_FL_QUIRK_INVERTED_CROP\t= 2,\n> +\tV4L2_FL_RO_DEVNODE\t\t= 3,\n\nI think this should be renamed to V4L2_FL_SUBDEV_RO_DEVNODE since this is subdev\nspecific and not for general device node usage. 'SUBDEV' should definitely be\npart of the flag name.\n\n> };\n> \n> /* Priority helper functions */\n> diff --git a/include/media/v4l2-device.h b/include/media/v4l2-device.h\n> index 7c912b7d2870..c01aa9df70b5 100644\n> --- a/include/media/v4l2-device.h\n> +++ b/include/media/v4l2-device.h\n> @@ -174,14 +174,48 @@ int __must_check v4l2_device_register_subdev(struct v4l2_device *v4l2_dev,\n> void v4l2_device_unregister_subdev(struct v4l2_subdev *sd);\n> \n> /**\n> - * v4l2_device_register_subdev_nodes - Registers device nodes for all subdevs\n> - *\tof the v4l2 device that are marked with\n> - *\tthe %V4L2_SUBDEV_FL_HAS_DEVNODE flag.\n> + * __v4l2_device_register_ro_subdev_nodes - Registers device nodes for\n> + * all subdevs of the v4l2 device that are marked with the\n> + * %V4L2_SUBDEV_FL_HAS_DEVNODE flag.\n> *\n> * @v4l2_dev: pointer to struct v4l2_device\n> + * @read_only: subdevices read-only flag. True to register the subdevices\n> + *\tdevice nodes in read-only mode, false to allow full access to the\n> + *\tsubdevice userspace API.\n> */\n> int __must_check\n> -v4l2_device_register_subdev_nodes(struct v4l2_device *v4l2_dev);\n> +__v4l2_device_register_subdev_nodes(struct v4l2_device *v4l2_dev,\n> +\t\t\t\t bool read_only);\n> +\n> +/**\n> + * v4l2_device_register_subdev_nodes - Registers subdevices device nodes with\n> + *\tunrestricted access to the subdevice userspace operations\n> + *\n> + * Internally calls __v4l2_device_register_subdev_nodes(). See its documentation\n> + * for more details.\n> + *\n> + * @v4l2_dev: pointer to struct v4l2_device\n> + */\n> +static inline int __must_check\n> +v4l2_device_register_subdev_nodes(struct v4l2_device *v4l2_dev)\n> +{\n> +\treturn __v4l2_device_register_subdev_nodes(v4l2_dev, false);\n> +}\n> +\n> +/**\n> + * v4l2_device_register_ro_subdev_nodes - Registers subdevices device nodes\n> + *\tin read-only mode\n> + *\n> + * Internally calls __v4l2_device_register_subdev_nodes(). See its documentation\n> + * for more details.\n> + *\n> + * @v4l2_dev: pointer to struct v4l2_device\n> + */\n> +static inline int __must_check\n> +v4l2_device_register_ro_subdev_nodes(struct v4l2_device *v4l2_dev)\n> +{\n> +\treturn __v4l2_device_register_subdev_nodes(v4l2_dev, true);\n> +}\n> \n> /**\n> * v4l2_subdev_notify - Sends a notification to v4l2_device.\n> --\n> 2.25.1\n> \n\nRegards,\n\n\tHans","headers":{"Return-Path":"","Received":["from lb3-smtp-cloud9.xs4all.net (lb3-smtp-cloud9.xs4all.net\n\t[194.109.24.30])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id CDD8260105\n\tfor ;\n\tWed, 1 Apr 2020 13:28:12 +0200 (CEST)","from [192.168.2.10] ([46.9.234.233])\n\tby smtp-cloud9.xs4all.net with ESMTPA\n\tid JbXUjEeArfHuvJbXYjEBl6; Wed, 01 Apr 2020 13:28:12 +0200"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key; \n\tunprotected) header.d=xs4all.nl header.i=@xs4all.nl\n\theader.b=\"g/xuxrTp\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=xs4all.nl; s=s1;\n\tt=1585740492; bh=k2sluWsDcHocHINMSfPv2wA4fx44wfVTZLvnm3m7s00=;\n\th=Subject:To:From:Message-ID:Date:MIME-Version:Content-Type:From:\n\tSubject;\n\tb=g/xuxrTpwTsW/s3i2C3OBaDduWKue6ckAdJ5DvJ6WQ091AOjLs8pXHawwKAbKKFxW\n\togukDqJSPr3g9SisQdbaLggwSg/6sjByM/ivPI6XkXLpFfqhIEkn52FawW2MwnROlV\n\t6ZjOj2xKJGQ/LYPSHJ0JILvZE7fXZ2048fWYpOYBLU4RIS+vfkIEWFF1493oU3QExx\n\tGe7XEhifI+Zz92hLiV1RH0fXIJm1+GEewhXUyhV10CyZv8YC1XdNAXLdiyes5OCq82\n\tqloTFFjDqVZvGf7wC1ncoTMwA80GvaLoOZSO8gUGM8ehI7Ux8po7IKsTB1dX5uReBD\n\tlcHbpyWBEga6w==","To":"Jacopo Mondi , 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","References":"<20200327223522.506832-1-jacopo@jmondi.org>\n\t<20200327223522.506832-4-jacopo@jmondi.org>","From":"Hans Verkuil ","Message-ID":"","Date":"Wed, 1 Apr 2020 13:28:08 +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":"<20200327223522.506832-4-jacopo@jmondi.org>","Content-Type":"text/plain; charset=utf-8","Content-Language":"en-US","Content-Transfer-Encoding":"7bit","X-CMAE-Envelope":"MS4wfKBKC9BUDxKaSgBk77tkZQ/r6grc2s92jgqcDof2spP6ZjV+2M5IU47MBvg5oOANOPPlF1obBTXPdvf3gLSZNk4tENaIbuefq3czLWRTUcjgbZSPlD6k\n\tqdmUzqieTPjv/10zphlyE3k9nxM//LBdJgu9LXJeTz5/hU4Gfbw78e9XFifIOchfzzI4vaDRCeW3I3oSxgp8SfMjLtUaE/DM8WzCDPKtMgd88F1wbj3QH1F1\n\tAcVU29jK0JvSnApZTiZLu/jLz2ndgH//nHkD7V4k0afYyTOW3+NJAm448P/VWjuYx9n3TU4Y5WspGPv+mfetIQWiy0KljKilVs67sufsAPQk+xI/Eefpx9j/\n\t+6rkIQ1f8dzpWlXKsgBIAhv9s87O3zYEUNdao4p6wnU5W0+zWGo5UdBmA3V+nW1c2pGy0Z00","Subject":"Re: [libcamera-devel] [v2 3/3] media: v4l2-dev: Add\n\tv4l2_device_register_ro_subdev_node()","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","X-List-Received-Date":"Wed, 01 Apr 2020 11:28:13 -0000"}},{"id":4363,"web_url":"https://patchwork.libcamera.org/comment/4363/","msgid":"<20200401122604.6pixnazmzskzzidp@uno.localdomain>","date":"2020-04-01T12:26:04","subject":"Re: [libcamera-devel] [v2 3/3] media: v4l2-dev: Add\n\tv4l2_device_register_ro_subdev_node()","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Hans,\n thanks for review\n `\nOn Wed, Apr 01, 2020 at 01:28:08PM +0200, Hans Verkuil wrote:\n> On 3/27/20 11:35 PM, Jacopo Mondi wrote:\n> > Add to the V4L2 core a function to register device nodes for video\n> > subdevices in read-only mode.\n> >\n> > Registering a device node in read-only mode is useful to expose to\n> > userspace the current sub-device configuration, without allowing\n> > application to change it by using the V4L2 subdevice ioctls.\n> >\n> > Signed-off-by: Jacopo Mondi \n> > ---\n> > drivers/media/v4l2-core/v4l2-device.c | 7 +++--\n> > drivers/media/v4l2-core/v4l2-subdev.c | 19 ++++++++++++\n> > include/media/v4l2-dev.h | 7 +++++\n> > include/media/v4l2-device.h | 42 ++++++++++++++++++++++++---\n> > 4 files changed, 69 insertions(+), 6 deletions(-)\n> >\n> > diff --git a/drivers/media/v4l2-core/v4l2-device.c b/drivers/media/v4l2-core/v4l2-device.c\n> > index c69941214bb2..4517a70379ce 100644\n> > --- a/drivers/media/v4l2-core/v4l2-device.c\n> > +++ b/drivers/media/v4l2-core/v4l2-device.c\n> > @@ -186,7 +186,8 @@ static void v4l2_device_release_subdev_node(struct video_device *vdev)\n> > \tkfree(vdev);\n> > }\n> >\n> > -int v4l2_device_register_subdev_nodes(struct v4l2_device *v4l2_dev)\n> > +int __v4l2_device_register_subdev_nodes(struct v4l2_device *v4l2_dev,\n> > +\t\t\t\t\tbool read_only)\n> > {\n> > \tstruct video_device *vdev;\n> > \tstruct v4l2_subdev *sd;\n> > @@ -215,6 +216,8 @@ int v4l2_device_register_subdev_nodes(struct v4l2_device *v4l2_dev)\n> > \t\tvdev->fops = &v4l2_subdev_fops;\n> > \t\tvdev->release = v4l2_device_release_subdev_node;\n> > \t\tvdev->ctrl_handler = sd->ctrl_handler;\n> > +\t\tif (read_only)\n> > +\t\t\tset_bit(V4L2_FL_RO_DEVNODE, &vdev->flags);\n>\n> I noticed the discussion about set_bit vs BIT: if memory serves, then at some point\n> in time setting/testing/clearing the V4L2_FL_REGISTERED had to be atomic. However,\n> looking at it today this no longer appears to be needed, so it can probably all be\n> replaced by normal bit operations. But that should be done in a separate patch if\n> anyone is interested in making that change.\n>\n\nAck, I agree it's an overkill and that it could be changed on top\n\n> > \t\terr = __video_register_device(vdev, VFL_TYPE_SUBDEV, -1, 1,\n> > \t\t\t\t\t sd->owner);\n> > \t\tif (err < 0) {\n> > @@ -252,7 +255,7 @@ int v4l2_device_register_subdev_nodes(struct v4l2_device *v4l2_dev)\n> >\n> > \treturn err;\n> > }\n> > -EXPORT_SYMBOL_GPL(v4l2_device_register_subdev_nodes);\n> > +EXPORT_SYMBOL_GPL(__v4l2_device_register_subdev_nodes);\n> >\n> > void v4l2_device_unregister_subdev(struct v4l2_subdev *sd)\n> > {\n> > diff --git a/drivers/media/v4l2-core/v4l2-subdev.c b/drivers/media/v4l2-core/v4l2-subdev.c\n> > index a376b351135f..87fea21919fc 100644\n> > --- a/drivers/media/v4l2-core/v4l2-subdev.c\n> > +++ b/drivers/media/v4l2-core/v4l2-subdev.c\n> > @@ -331,6 +331,7 @@ static long subdev_do_ioctl(struct file *file, unsigned int cmd, void *arg)\n> > \tstruct v4l2_fh *vfh = file->private_data;\n> > #if defined(CONFIG_VIDEO_V4L2_SUBDEV_API)\n> > \tstruct v4l2_subdev_fh *subdev_fh = to_v4l2_subdev_fh(vfh);\n> > +\tbool ro_subdev = test_bit(V4L2_FL_RO_DEVNODE, &vdev->flags);\n> > #endif\n> > \tint rval;\n> >\n> > @@ -477,6 +478,9 @@ static long subdev_do_ioctl(struct file *file, unsigned int cmd, void *arg)\n> > \tcase VIDIOC_SUBDEV_S_FMT: {\n> > \t\tstruct v4l2_subdev_format *format = arg;\n> >\n> > +\t\tif (format->which != V4L2_SUBDEV_FORMAT_TRY && ro_subdev)\n> > +\t\t\treturn -EPERM;\n> > +\n> > \t\tmemset(format->reserved, 0, sizeof(format->reserved));\n> > \t\tmemset(format->format.reserved, 0, sizeof(format->format.reserved));\n> > \t\treturn v4l2_subdev_call(sd, pad, set_fmt, subdev_fh->pad, format);\n> > @@ -504,6 +508,9 @@ static long subdev_do_ioctl(struct file *file, unsigned int cmd, void *arg)\n> > \t\tstruct v4l2_subdev_crop *crop = arg;\n> > \t\tstruct v4l2_subdev_selection sel;\n> >\n> > +\t\tif (crop->which != V4L2_SUBDEV_FORMAT_TRY && ro_subdev)\n> > +\t\t\treturn -EPERM;\n> > +\n> > \t\tmemset(crop->reserved, 0, sizeof(crop->reserved));\n> > \t\tmemset(&sel, 0, sizeof(sel));\n> > \t\tsel.which = crop->which;\n> > @@ -545,6 +552,9 @@ static long subdev_do_ioctl(struct file *file, unsigned int cmd, void *arg)\n> > \tcase VIDIOC_SUBDEV_S_FRAME_INTERVAL: {\n> > \t\tstruct v4l2_subdev_frame_interval *fi = arg;\n> >\n> > +\t\tif (ro_subdev)\n> > +\t\t\treturn -EPERM;\n> > +\n> > \t\tmemset(fi->reserved, 0, sizeof(fi->reserved));\n> > \t\treturn v4l2_subdev_call(sd, video, s_frame_interval, arg);\n> > \t}\n> > @@ -568,6 +578,9 @@ static long subdev_do_ioctl(struct file *file, unsigned int cmd, void *arg)\n> > \tcase VIDIOC_SUBDEV_S_SELECTION: {\n> > \t\tstruct v4l2_subdev_selection *sel = arg;\n> >\n> > +\t\tif (sel->which != V4L2_SUBDEV_FORMAT_TRY && ro_subdev)\n> > +\t\t\treturn -EPERM;\n> > +\n> > \t\tmemset(sel->reserved, 0, sizeof(sel->reserved));\n> > \t\treturn v4l2_subdev_call(\n> > \t\t\tsd, pad, set_selection, subdev_fh->pad, sel);\n> > @@ -604,6 +617,9 @@ static long subdev_do_ioctl(struct file *file, unsigned int cmd, void *arg)\n> > \t\treturn v4l2_subdev_call(sd, video, g_dv_timings, arg);\n> >\n> > \tcase VIDIOC_SUBDEV_S_DV_TIMINGS:\n> > +\t\tif (ro_subdev)\n> > +\t\t\treturn -EPERM;\n> > +\n> > \t\treturn v4l2_subdev_call(sd, video, s_dv_timings, arg);\n> >\n> > \tcase VIDIOC_SUBDEV_G_STD:\n> > @@ -612,6 +628,9 @@ static long subdev_do_ioctl(struct file *file, unsigned int cmd, void *arg)\n> > \tcase VIDIOC_SUBDEV_S_STD: {\n> > \t\tv4l2_std_id *std = arg;\n> >\n> > +\t\tif (ro_subdev)\n> > +\t\t\treturn -EPERM;\n> > +\n> > \t\treturn v4l2_subdev_call(sd, video, s_std, *std);\n> > \t}\n> >\n> > diff --git a/include/media/v4l2-dev.h b/include/media/v4l2-dev.h\n> > index 4602c15ff878..bdbd953a976c 100644\n> > --- a/include/media/v4l2-dev.h\n> > +++ b/include/media/v4l2-dev.h\n> > @@ -82,11 +82,18 @@ struct v4l2_ctrl_handler;\n> > *\tbut the old crop API will still work as expected in order to preserve\n> > *\tbackwards compatibility.\n> > *\tNever set this flag for new drivers.\n> > + * @V4L2_FL_RO_DEVNODE:\n> > + *\tindicates that the video device node is registered in read-only mode.\n> > + *\tThe flag only applies to device nodes registered for sub-devices, it is\n> > + *\tset by the core when the sub-devices device nodes are registered with\n> > + *\tv4l2_device_register_ro_subdev_nodes() and used by the sub-device ioctl\n> > + *\thandler to restrict access to some ioctl calls.\n> > */\n> > enum v4l2_video_device_flags {\n> > \tV4L2_FL_REGISTERED\t\t= 0,\n> > \tV4L2_FL_USES_V4L2_FH\t\t= 1,\n> > \tV4L2_FL_QUIRK_INVERTED_CROP\t= 2,\n> > +\tV4L2_FL_RO_DEVNODE\t\t= 3,\n>\n> I think this should be renamed to V4L2_FL_SUBDEV_RO_DEVNODE since this is subdev\n> specific and not for general device node usage. 'SUBDEV' should definitely be\n> part of the flag name.\n>\n\nTrue indeed! I'll rename the flag\n\nThanks\n j\n\n> > };\n> >\n> > /* Priority helper functions */\n> > diff --git a/include/media/v4l2-device.h b/include/media/v4l2-device.h\n> > index 7c912b7d2870..c01aa9df70b5 100644\n> > --- a/include/media/v4l2-device.h\n> > +++ b/include/media/v4l2-device.h\n> > @@ -174,14 +174,48 @@ int __must_check v4l2_device_register_subdev(struct v4l2_device *v4l2_dev,\n> > void v4l2_device_unregister_subdev(struct v4l2_subdev *sd);\n> >\n> > /**\n> > - * v4l2_device_register_subdev_nodes - Registers device nodes for all subdevs\n> > - *\tof the v4l2 device that are marked with\n> > - *\tthe %V4L2_SUBDEV_FL_HAS_DEVNODE flag.\n> > + * __v4l2_device_register_ro_subdev_nodes - Registers device nodes for\n> > + * all subdevs of the v4l2 device that are marked with the\n> > + * %V4L2_SUBDEV_FL_HAS_DEVNODE flag.\n> > *\n> > * @v4l2_dev: pointer to struct v4l2_device\n> > + * @read_only: subdevices read-only flag. True to register the subdevices\n> > + *\tdevice nodes in read-only mode, false to allow full access to the\n> > + *\tsubdevice userspace API.\n> > */\n> > int __must_check\n> > -v4l2_device_register_subdev_nodes(struct v4l2_device *v4l2_dev);\n> > +__v4l2_device_register_subdev_nodes(struct v4l2_device *v4l2_dev,\n> > +\t\t\t\t bool read_only);\n> > +\n> > +/**\n> > + * v4l2_device_register_subdev_nodes - Registers subdevices device nodes with\n> > + *\tunrestricted access to the subdevice userspace operations\n> > + *\n> > + * Internally calls __v4l2_device_register_subdev_nodes(). See its documentation\n> > + * for more details.\n> > + *\n> > + * @v4l2_dev: pointer to struct v4l2_device\n> > + */\n> > +static inline int __must_check\n> > +v4l2_device_register_subdev_nodes(struct v4l2_device *v4l2_dev)\n> > +{\n> > +\treturn __v4l2_device_register_subdev_nodes(v4l2_dev, false);\n> > +}\n> > +\n> > +/**\n> > + * v4l2_device_register_ro_subdev_nodes - Registers subdevices device nodes\n> > + *\tin read-only mode\n> > + *\n> > + * Internally calls __v4l2_device_register_subdev_nodes(). See its documentation\n> > + * for more details.\n> > + *\n> > + * @v4l2_dev: pointer to struct v4l2_device\n> > + */\n> > +static inline int __must_check\n> > +v4l2_device_register_ro_subdev_nodes(struct v4l2_device *v4l2_dev)\n> > +{\n> > +\treturn __v4l2_device_register_subdev_nodes(v4l2_dev, true);\n> > +}\n> >\n> > /**\n> > * v4l2_subdev_notify - Sends a notification to v4l2_device.\n> > --\n> > 2.25.1\n> >\n>\n> Regards,\n>\n> \tHans","headers":{"Return-Path":"","Received":["from relay11.mail.gandi.net (relay11.mail.gandi.net\n\t[217.70.178.231])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 20B5660105\n\tfor ;\n\tWed, 1 Apr 2020 14:23:06 +0200 (CEST)","from uno.localdomain (93-34-114-233.ip49.fastwebnet.it\n\t[93.34.114.233]) (Authenticated sender: jacopo@jmondi.org)\n\tby relay11.mail.gandi.net (Postfix) with ESMTPSA id 7DE3D10000A;\n\tWed, 1 Apr 2020 12:23:03 +0000 (UTC)"],"Date":"Wed, 1 Apr 2020 14:26:04 +0200","From":"Jacopo Mondi ","To":"Hans Verkuil ","Cc":"linux-media@vger.kernel.org, libcamera-devel@lists.libcamera.org,\n\tmchehab@kernel.org, sakari.ailus@linux.intel.com,\n\tandrey.konovalov@linaro.org, laurent.pinchart@ideasonboard.com","Message-ID":"<20200401122604.6pixnazmzskzzidp@uno.localdomain>","References":"<20200327223522.506832-1-jacopo@jmondi.org>\n\t<20200327223522.506832-4-jacopo@jmondi.org>\n\t","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"","Subject":"Re: [libcamera-devel] [v2 3/3] media: v4l2-dev: Add\n\tv4l2_device_register_ro_subdev_node()","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","X-List-Received-Date":"Wed, 01 Apr 2020 12:23:06 -0000"}}]