[{"id":4271,"web_url":"https://patchwork.libcamera.org/comment/4271/","msgid":"<313fcb7e-6612-9cf5-a4eb-ba6edb39f754@linaro.org>","date":"2020-03-25T08:42:27","subject":"Re: [libcamera-devel] [PATCH 2/4] media: v4l2-dev: Add\n\tv4l2_device_register_ro_subdev_node()","submitter":{"id":25,"url":"https://patchwork.libcamera.org/api/people/25/","name":"Andrey Konovalov","email":"andrey.konovalov@linaro.org"},"content":"Hi Jacopo,\n\nThank you for your patch set!\n\nOn 24.03.2020 23:28, Jacopo Mondi wrote:\n> Add to the V4L2 code 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 | 16 +++++++++++++++-\n> drivers/media/v4l2-core/v4l2-subdev.c | 19 +++++++++++++++++++\n> include/media/v4l2-dev.h | 7 +++++++\n> include/media/v4l2-device.h | 10 ++++++++++\n> 4 files changed, 51 insertions(+), 1 deletion(-)\n> \n> diff --git a/drivers/media/v4l2-core/v4l2-device.c b/drivers/media/v4l2-core/v4l2-device.c\n> index 63d6b147b21e..6f9dba36eda1 100644\n> --- a/drivers/media/v4l2-core/v4l2-device.c\n> +++ b/drivers/media/v4l2-core/v4l2-device.c\n> @@ -188,7 +188,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> @@ -217,6 +218,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\tvdev->flags |= V4L2_FL_RO_DEVNODE;\n\n\n\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_devnode = !!(vdev->flags & V4L2_FL_RO_DEVNODE);\n\nSo V4L2_FL_RO_DEVNODE is a bit mask, ...\n\n\n\n> diff --git a/include/media/v4l2-dev.h b/include/media/v4l2-dev.h\n> index 48531e57cc5a..029873a338f2 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... then V4L2_FL_RO_DEVNODE should rather be equal to 4, than to (V4L2_FL_USES_V4L2_FH | V4L2_FL_QUIRK_INVERTED_CROP)\n\nThanks,\nAndrey\n\n> };\n> \n> /* Priority helper functions */\n> diff --git a/include/media/v4l2-device.h b/include/media/v4l2-device.h\n> index e0b8f2602670..0df667ba9938 100644\n> --- a/include/media/v4l2-device.h\n> +++ b/include/media/v4l2-device.h\n> @@ -183,6 +183,16 @@ void v4l2_device_unregister_subdev(struct v4l2_subdev *sd);\n> int __must_check\n> v4l2_device_register_subdev_nodes(struct v4l2_device *v4l2_dev);\n> \n> +/**\n> + * v4l2_device_register_ro_subdev_nodes - Registers read-only 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> + */\n> +int __must_check\n> +v4l2_device_register_ro_subdev_nodes(struct v4l2_device *v4l2_dev);\n> +\n> /**\n> * v4l2_subdev_notify - Sends a notification to v4l2_device.\n> *\n>","headers":{"Return-Path":"","Received":["from mail-lj1-x242.google.com (mail-lj1-x242.google.com\n\t[IPv6:2a00:1450:4864:20::242])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id C6F5F60410\n\tfor ;\n\tWed, 25 Mar 2020 09:42:30 +0100 (CET)","by mail-lj1-x242.google.com with SMTP id f20so1567111ljm.0\n\tfor ;\n\tWed, 25 Mar 2020 01:42:30 -0700 (PDT)","from [192.168.118.216] (37-144-159-139.broadband.corbina.ru.\n\t[37.144.159.139]) by smtp.gmail.com with ESMTPSA id\n\tu7sm1525603ljo.1.2020.03.25.01.42.28\n\t(version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128);\n\tWed, 25 Mar 2020 01:42:29 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key; \n\tunprotected) header.d=linaro.org\n\theader.i=@linaro.org header.b=\"r/6AJPQe\"; \n\tdkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google;\n\th=subject:to:cc:references:from:message-id:date:user-agent\n\t:mime-version:in-reply-to:content-language:content-transfer-encoding; \n\tbh=6ZInQ7P2kWfW5JLJarR/YZgcnvg7Lm/+5nKFHObLkXk=;\n\tb=r/6AJPQeqEYeuT/qWLoTXhttgDIWmYy6htJ0cV9xtJo6lz88VHvfnetIpXArPjyQ2D\n\tlgMsgjII6O1Ge+PhBT640YeF4jl3cp+auq85e6m1bnEHqnAY1kMODoNTvLbzkVCwvrpW\n\tAj9NMW6ypBunR7qC6IuS0pflwlcr6nMXsT1t8MG8/vs00ycZci8B5bFNj3z7/iAHx5+5\n\t/8ax1sM+8NO6BuODqnL+IXYZfnOrqrOFuE8fh7kHi0qW7WP96+FQiiJaADmw2/9wGyYK\n\tcd5K4VTdQwoXp+IB6M/yWWApb5BixoY3O9m0bOxIvGr2hwAH/uu5hV6SYNDsN2efKWRD\n\tatMA==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:subject:to:cc:references:from:message-id:date\n\t:user-agent:mime-version:in-reply-to:content-language\n\t:content-transfer-encoding;\n\tbh=6ZInQ7P2kWfW5JLJarR/YZgcnvg7Lm/+5nKFHObLkXk=;\n\tb=lNW+nhg2WDEtFu/ZavgVf+vq7WaPSu0kYmDrW6ImyOiimB4xjgzWoZ5dZsh88DsUE8\n\tycMUtiQFYMLb33vYW8TyjbDmpm8mvcsMQL3OILimkF1UhFjVyzbzxGitzAKcdQEJbicq\n\tN9Jy9Fx5xoC6wEqgy9l2dMBR6y1OfBkXARiXkM4J87Per3WviX/8faxu0wpoahf5/O8/\n\tOohlu1pOwx3oXVwzXuCoO4eMzuOUSjiI9hnRcjw/GkDggoWxz52bq8B1heEQUOJ4U1LH\n\toZ5HUvdOTF2br52JuGNcWFUFCfj142yQxDIPyXDv+EL69RLjUwdU7QQkT7cImSL2CkKO\n\tCk2Q==","X-Gm-Message-State":"AGi0PubrlvkGVm0WKzvToYItb1ZbuQzKYUtaRJYr9BvnAb2CVF3jy+Rc\n\tqdV0tMgaYevnlhUlWg0wyqlB5Q==","X-Google-Smtp-Source":"ADFU+vv+Q2CS2Rvp0B5xrEvQl0g8ZdyXuPwfxVYwHnNBuuvOmT2ruc/0R4aVA+tJ9pUs4Z6xP6jmUg==","X-Received":"by 2002:a2e:b801:: with SMTP id u1mr1281617ljo.84.1585125749909; \n\tWed, 25 Mar 2020 01:42:29 -0700 (PDT)","To":"Jacopo Mondi , linux-media@vger.kernel.org,\n\tlibcamera-devel@lists.libcamera.org","Cc":"hverkuil-cisco@xs4all.nl, mchehab@kernel.org,\n\tsakari.ailus@linux.intel.com","References":"<20200324202844.1518292-1-jacopo@jmondi.org>\n\t<20200324202844.1518292-3-jacopo@jmondi.org>","From":"Andrey Konovalov ","Message-ID":"<313fcb7e-6612-9cf5-a4eb-ba6edb39f754@linaro.org>","Date":"Wed, 25 Mar 2020 11:42:27 +0300","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101\n\tThunderbird/68.4.1","MIME-Version":"1.0","In-Reply-To":"<20200324202844.1518292-3-jacopo@jmondi.org>","Content-Type":"text/plain; charset=utf-8; format=flowed","Content-Language":"en-US","Content-Transfer-Encoding":"7bit","Subject":"Re: [libcamera-devel] [PATCH 2/4] 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, 25 Mar 2020 08:42:31 -0000"}},{"id":4281,"web_url":"https://patchwork.libcamera.org/comment/4281/","msgid":"<20200325112342.andqi2uognizd4uq@uno.localdomain>","date":"2020-03-25T11:23:42","subject":"Re: [libcamera-devel] [PATCH 2/4] 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 Andrey,\n thanks for the review\n\nOn Wed, Mar 25, 2020 at 11:42:27AM +0300, Andrey Konovalov wrote:\n> Hi Jacopo,\n>\n> Thank you for your patch set!\n>\n> On 24.03.2020 23:28, Jacopo Mondi wrote:\n> > Add to the V4L2 code 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 | 16 +++++++++++++++-\n> > drivers/media/v4l2-core/v4l2-subdev.c | 19 +++++++++++++++++++\n> > include/media/v4l2-dev.h | 7 +++++++\n> > include/media/v4l2-device.h | 10 ++++++++++\n> > 4 files changed, 51 insertions(+), 1 deletion(-)\n> >\n> > diff --git a/drivers/media/v4l2-core/v4l2-device.c b/drivers/media/v4l2-core/v4l2-device.c\n> > index 63d6b147b21e..6f9dba36eda1 100644\n> > --- a/drivers/media/v4l2-core/v4l2-device.c\n> > +++ b/drivers/media/v4l2-core/v4l2-device.c\n> > @@ -188,7 +188,8 @@ static void v4l2_device_release_subdev_node(struct video_device *vdev)\n> > \tkfree(vdev);\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> > @@ -217,6 +218,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\tvdev->flags |= V4L2_FL_RO_DEVNODE;\n>\n> \n>\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_devnode = !!(vdev->flags & V4L2_FL_RO_DEVNODE);\n>\n> So V4L2_FL_RO_DEVNODE is a bit mask, ...\n>\n> \n>\n> > diff --git a/include/media/v4l2-dev.h b/include/media/v4l2-dev.h\n> > index 48531e57cc5a..029873a338f2 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> ... then V4L2_FL_RO_DEVNODE should rather be equal to 4, than to (V4L2_FL_USES_V4L2_FH | V4L2_FL_QUIRK_INVERTED_CROP)\n\nUps!\n\nI should have noticed looking at V4L2_FL_REGISTERED=0 that these\nflags are actually meant to be used as bit shifts. I can't say I like\nthe idea, but I should have known better than this.\n\nI'll resend using set_bit() and test_bit().\n\nThanks for your help!\n\n>\n> Thanks,\n> Andrey\n>\n> > };\n> > /* Priority helper functions */\n> > diff --git a/include/media/v4l2-device.h b/include/media/v4l2-device.h\n> > index e0b8f2602670..0df667ba9938 100644\n> > --- a/include/media/v4l2-device.h\n> > +++ b/include/media/v4l2-device.h\n> > @@ -183,6 +183,16 @@ void v4l2_device_unregister_subdev(struct v4l2_subdev *sd);\n> > int __must_check\n> > v4l2_device_register_subdev_nodes(struct v4l2_device *v4l2_dev);\n> > +/**\n> > + * v4l2_device_register_ro_subdev_nodes - Registers read-only 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> > + */\n> > +int __must_check\n> > +v4l2_device_register_ro_subdev_nodes(struct v4l2_device *v4l2_dev);\n> > +\n> > /**\n> > * v4l2_subdev_notify - Sends a notification to v4l2_device.\n> > *\n> >","headers":{"Return-Path":"","Received":["from relay5-d.mail.gandi.net (relay5-d.mail.gandi.net\n\t[217.70.183.197])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 141AD62BD6\n\tfor ;\n\tWed, 25 Mar 2020 12:20:46 +0100 (CET)","from uno.localdomain (2-224-242-101.ip172.fastwebnet.it\n\t[2.224.242.101]) (Authenticated sender: jacopo@jmondi.org)\n\tby relay5-d.mail.gandi.net (Postfix) with ESMTPSA id 302ED1C0010;\n\tWed, 25 Mar 2020 11:20:43 +0000 (UTC)"],"X-Originating-IP":"2.224.242.101","Date":"Wed, 25 Mar 2020 12:23:42 +0100","From":"Jacopo Mondi ","To":"Andrey Konovalov ","Cc":"linux-media@vger.kernel.org, libcamera-devel@lists.libcamera.org,\n\thverkuil-cisco@xs4all.nl, mchehab@kernel.org,\n\tsakari.ailus@linux.intel.com","Message-ID":"<20200325112342.andqi2uognizd4uq@uno.localdomain>","References":"<20200324202844.1518292-1-jacopo@jmondi.org>\n\t<20200324202844.1518292-3-jacopo@jmondi.org>\n\t<313fcb7e-6612-9cf5-a4eb-ba6edb39f754@linaro.org>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<313fcb7e-6612-9cf5-a4eb-ba6edb39f754@linaro.org>","Subject":"Re: [libcamera-devel] [PATCH 2/4] 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, 25 Mar 2020 11:20:46 -0000"}},{"id":4290,"web_url":"https://patchwork.libcamera.org/comment/4290/","msgid":"<20200325214536.GV19171@pendragon.ideasonboard.com>","date":"2020-03-25T21:45:36","subject":"Re: [libcamera-devel] [PATCH 2/4] media: v4l2-dev: Add\n\tv4l2_device_register_ro_subdev_node()","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Jacopo,\n\nThank you for the patch.\n\nOn Tue, Mar 24, 2020 at 09:28:42PM +0100, Jacopo Mondi wrote:\n> Add to the V4L2 code 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 | 16 +++++++++++++++-\n> drivers/media/v4l2-core/v4l2-subdev.c | 19 +++++++++++++++++++\n> include/media/v4l2-dev.h | 7 +++++++\n> include/media/v4l2-device.h | 10 ++++++++++\n> 4 files changed, 51 insertions(+), 1 deletion(-)\n> \n> diff --git a/drivers/media/v4l2-core/v4l2-device.c b/drivers/media/v4l2-core/v4l2-device.c\n> index 63d6b147b21e..6f9dba36eda1 100644\n> --- a/drivers/media/v4l2-core/v4l2-device.c\n> +++ b/drivers/media/v4l2-core/v4l2-device.c\n> @@ -188,7 +188,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> @@ -217,6 +218,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\tvdev->flags |= V4L2_FL_RO_DEVNODE;\n\nAs Andrey pointed out, this should be BIT(V4L2_FL_RO_DEVNODE).\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> @@ -254,8 +257,19 @@ int v4l2_device_register_subdev_nodes(struct v4l2_device *v4l2_dev)\n> \n> \treturn err;\n> }\n> +\n> +int v4l2_device_register_subdev_nodes(struct v4l2_device *v4l2_dev)\n> +{\n> +\treturn __v4l2_device_register_subdev_nodes(v4l2_dev, false);\n> +}\n> EXPORT_SYMBOL_GPL(v4l2_device_register_subdev_nodes);\n> \n> +int v4l2_device_register_ro_subdev_nodes(struct v4l2_device *v4l2_dev)\n> +{\n> +\treturn __v4l2_device_register_subdev_nodes(v4l2_dev, true);\n> +}\n> +EXPORT_SYMBOL_GPL(v4l2_device_register_ro_subdev_nodes);\n\nI would export __v4l2_device_register_subdev_nodes and implement these\ntwo functions as static inline in include/media/v4l2-device.h.\n\n> +\n> void v4l2_device_unregister_subdev(struct v4l2_subdev *sd)\n> {\n> \tstruct v4l2_device *v4l2_dev;\n> diff --git a/drivers/media/v4l2-core/v4l2-subdev.c b/drivers/media/v4l2-core/v4l2-subdev.c\n> index f725cd9b66b9..9247ee6c293f 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_devnode = !!(vdev->flags & V4L2_FL_RO_DEVNODE);\n\nSame here, BIT(V4L2_FL_RO_DEVNODE).\n\nI would name the variable ro_api to emphasize this is not about the\ndevice node being read-only (in the sense of POSIX file permissions).\n\n> \tint rval;\n> #endif\n> \n> @@ -453,6 +454,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_devnode)\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> @@ -480,6 +484,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_devnode)\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> @@ -521,6 +528,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_devnode)\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> @@ -544,6 +554,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_devnode)\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> @@ -580,6 +593,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_devnode)\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> @@ -588,6 +604,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_devnode)\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 48531e57cc5a..029873a338f2 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> \n> /* Priority helper functions */\n> diff --git a/include/media/v4l2-device.h b/include/media/v4l2-device.h\n> index e0b8f2602670..0df667ba9938 100644\n> --- a/include/media/v4l2-device.h\n> +++ b/include/media/v4l2-device.h\n> @@ -183,6 +183,16 @@ void v4l2_device_unregister_subdev(struct v4l2_subdev *sd);\n> int __must_check\n> v4l2_device_register_subdev_nodes(struct v4l2_device *v4l2_dev);\n> \n> +/**\n> + * v4l2_device_register_ro_subdev_nodes - Registers read-only 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> + */\n> +int __must_check\n> +v4l2_device_register_ro_subdev_nodes(struct v4l2_device *v4l2_dev);\n> +\n> /**\n> * v4l2_subdev_notify - Sends a notification to v4l2_device.\n> *","headers":{"Return-Path":"","Received":["from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id D0C8860413\n\tfor ;\n\tWed, 25 Mar 2020 22:45:41 +0100 (CET)","from pendragon.ideasonboard.com (81-175-216-236.bb.dnainternet.fi\n\t[81.175.216.236])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 0B83B80C;\n\tWed, 25 Mar 2020 22:45:40 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"Hvn6vLqt\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1585172741;\n\tbh=p2NAutUcAb1Netd+amhiFxsbPOHvCWxQhkU4ke1ABdI=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=Hvn6vLqt7XdJE4odA5Vm3XBJv2bFQ8rHApYoKKU2ZVrknQLCAH8rp3VBWkrRYcvbs\n\tmV3pCvRPrFkijYZDBEOdQBnse/EVVzcvAfxmTFUEdRhNXiVMBNRpydXJN6Z9uAShZS\n\tznQCKEdleYI5WzB/PdkaF+o3ZdZynKfE9BYqQmjY=","Date":"Wed, 25 Mar 2020 23:45:36 +0200","From":"Laurent Pinchart ","To":"Jacopo Mondi ","Cc":"linux-media@vger.kernel.org, libcamera-devel@lists.libcamera.org,\n\thverkuil-cisco@xs4all.nl, mchehab@kernel.org,\n\tsakari.ailus@linux.intel.com","Message-ID":"<20200325214536.GV19171@pendragon.ideasonboard.com>","References":"<20200324202844.1518292-1-jacopo@jmondi.org>\n\t<20200324202844.1518292-3-jacopo@jmondi.org>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20200324202844.1518292-3-jacopo@jmondi.org>","User-Agent":"Mutt/1.10.1 (2018-07-13)","Subject":"Re: [libcamera-devel] [PATCH 2/4] 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, 25 Mar 2020 21:45:42 -0000"}},{"id":4294,"web_url":"https://patchwork.libcamera.org/comment/4294/","msgid":"<20200325225710.nwycq7lqw5kzodi5@uno.localdomain>","date":"2020-03-25T22:57:10","subject":"Re: [libcamera-devel] [PATCH 2/4] 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 Laurent,\n\nOn Wed, Mar 25, 2020 at 11:45:36PM +0200, Laurent Pinchart wrote:\n> Hi Jacopo,\n>\n> Thank you for the patch.\n>\n> On Tue, Mar 24, 2020 at 09:28:42PM +0100, Jacopo Mondi wrote:\n> > Add to the V4L2 code 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 | 16 +++++++++++++++-\n> > drivers/media/v4l2-core/v4l2-subdev.c | 19 +++++++++++++++++++\n> > include/media/v4l2-dev.h | 7 +++++++\n> > include/media/v4l2-device.h | 10 ++++++++++\n> > 4 files changed, 51 insertions(+), 1 deletion(-)\n> >\n> > diff --git a/drivers/media/v4l2-core/v4l2-device.c b/drivers/media/v4l2-core/v4l2-device.c\n> > index 63d6b147b21e..6f9dba36eda1 100644\n> > --- a/drivers/media/v4l2-core/v4l2-device.c\n> > +++ b/drivers/media/v4l2-core/v4l2-device.c\n> > @@ -188,7 +188,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> > @@ -217,6 +218,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\tvdev->flags |= V4L2_FL_RO_DEVNODE;\n>\n> As Andrey pointed out, this should be BIT(V4L2_FL_RO_DEVNODE).\n>\n\nthe rest of the v4l2 core seems to be use set_bit()/test_bit() when\nhandling V4L2_FL_ (which, my bad, I rarely encoutered compared to the\nto me more canonical BIT() combined with bitops).\n\nI would then go for them then\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> > @@ -254,8 +257,19 @@ int v4l2_device_register_subdev_nodes(struct v4l2_device *v4l2_dev)\n> >\n> > \treturn err;\n> > }\n> > +\n> > +int v4l2_device_register_subdev_nodes(struct v4l2_device *v4l2_dev)\n> > +{\n> > +\treturn __v4l2_device_register_subdev_nodes(v4l2_dev, false);\n> > +}\n> > EXPORT_SYMBOL_GPL(v4l2_device_register_subdev_nodes);\n> >\n> > +int v4l2_device_register_ro_subdev_nodes(struct v4l2_device *v4l2_dev)\n> > +{\n> > +\treturn __v4l2_device_register_subdev_nodes(v4l2_dev, true);\n> > +}\n> > +EXPORT_SYMBOL_GPL(v4l2_device_register_ro_subdev_nodes);\n>\n> I would export __v4l2_device_register_subdev_nodes and implement these\n> two functions as static inline in include/media/v4l2-device.h.\n>\n\nGood point we could save a function call.\n\n> > +\n> > void v4l2_device_unregister_subdev(struct v4l2_subdev *sd)\n> > {\n> > \tstruct v4l2_device *v4l2_dev;\n> > diff --git a/drivers/media/v4l2-core/v4l2-subdev.c b/drivers/media/v4l2-core/v4l2-subdev.c\n> > index f725cd9b66b9..9247ee6c293f 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_devnode = !!(vdev->flags & V4L2_FL_RO_DEVNODE);\n>\n> Same here, BIT(V4L2_FL_RO_DEVNODE).\n>\n> I would name the variable ro_api to emphasize this is not about the\n> device node being read-only (in the sense of POSIX file permissions).\n>\n\nAck, even if I would like ro_subdev a bit more. Anyway bikesheeding on\nvariable names, ro_api is fine.\n\nThanks\n j\n\n\n> > \tint rval;\n> > #endif\n> >\n> > @@ -453,6 +454,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_devnode)\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> > @@ -480,6 +484,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_devnode)\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> > @@ -521,6 +528,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_devnode)\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> > @@ -544,6 +554,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_devnode)\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> > @@ -580,6 +593,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_devnode)\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> > @@ -588,6 +604,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_devnode)\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 48531e57cc5a..029873a338f2 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> >\n> > /* Priority helper functions */\n> > diff --git a/include/media/v4l2-device.h b/include/media/v4l2-device.h\n> > index e0b8f2602670..0df667ba9938 100644\n> > --- a/include/media/v4l2-device.h\n> > +++ b/include/media/v4l2-device.h\n> > @@ -183,6 +183,16 @@ void v4l2_device_unregister_subdev(struct v4l2_subdev *sd);\n> > int __must_check\n> > v4l2_device_register_subdev_nodes(struct v4l2_device *v4l2_dev);\n> >\n> > +/**\n> > + * v4l2_device_register_ro_subdev_nodes - Registers read-only 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> > + */\n> > +int __must_check\n> > +v4l2_device_register_ro_subdev_nodes(struct v4l2_device *v4l2_dev);\n> > +\n> > /**\n> > * v4l2_subdev_notify - Sends a notification to v4l2_device.\n> > *\n>\n> --\n> Regards,\n>\n> Laurent Pinchart","headers":{"Return-Path":"","Received":["from relay2-d.mail.gandi.net (relay2-d.mail.gandi.net\n\t[217.70.183.194])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id C33B060413\n\tfor ;\n\tWed, 25 Mar 2020 23:54:13 +0100 (CET)","from uno.localdomain (2-224-242-101.ip172.fastwebnet.it\n\t[2.224.242.101]) (Authenticated sender: jacopo@jmondi.org)\n\tby relay2-d.mail.gandi.net (Postfix) with ESMTPSA id DFAAF40003;\n\tWed, 25 Mar 2020 22:54:11 +0000 (UTC)"],"X-Originating-IP":"2.224.242.101","Date":"Wed, 25 Mar 2020 23:57:10 +0100","From":"Jacopo Mondi ","To":"Laurent Pinchart ","Cc":"linux-media@vger.kernel.org, libcamera-devel@lists.libcamera.org,\n\thverkuil-cisco@xs4all.nl, mchehab@kernel.org,\n\tsakari.ailus@linux.intel.com","Message-ID":"<20200325225710.nwycq7lqw5kzodi5@uno.localdomain>","References":"<20200324202844.1518292-1-jacopo@jmondi.org>\n\t<20200324202844.1518292-3-jacopo@jmondi.org>\n\t<20200325214536.GV19171@pendragon.ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20200325214536.GV19171@pendragon.ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH 2/4] 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, 25 Mar 2020 22:54:13 -0000"}},{"id":4296,"web_url":"https://patchwork.libcamera.org/comment/4296/","msgid":"<20200326000808.GB3709@mara.localdomain>","date":"2020-03-26T00:08:08","subject":"Re: [libcamera-devel] [PATCH 2/4] media: v4l2-dev: Add\n\tv4l2_device_register_ro_subdev_node()","submitter":{"id":37,"url":"https://patchwork.libcamera.org/api/people/37/","name":"Sakari Ailus","email":"sakari.ailus@linux.intel.com"},"content":"Hi Jacopo,\n\nOn Wed, Mar 25, 2020 at 12:23:42PM +0100, Jacopo Mondi wrote:\n> Hi Andrey,\n> thanks for the review\n> \n> On Wed, Mar 25, 2020 at 11:42:27AM +0300, Andrey Konovalov wrote:\n> > Hi Jacopo,\n> >\n> > Thank you for your patch set!\n> >\n> > On 24.03.2020 23:28, Jacopo Mondi wrote:\n> > > Add to the V4L2 code 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 | 16 +++++++++++++++-\n> > > drivers/media/v4l2-core/v4l2-subdev.c | 19 +++++++++++++++++++\n> > > include/media/v4l2-dev.h | 7 +++++++\n> > > include/media/v4l2-device.h | 10 ++++++++++\n> > > 4 files changed, 51 insertions(+), 1 deletion(-)\n> > >\n> > > diff --git a/drivers/media/v4l2-core/v4l2-device.c b/drivers/media/v4l2-core/v4l2-device.c\n> > > index 63d6b147b21e..6f9dba36eda1 100644\n> > > --- a/drivers/media/v4l2-core/v4l2-device.c\n> > > +++ b/drivers/media/v4l2-core/v4l2-device.c\n> > > @@ -188,7 +188,8 @@ static void v4l2_device_release_subdev_node(struct video_device *vdev)\n> > > \tkfree(vdev);\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> > > @@ -217,6 +218,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\tvdev->flags |= V4L2_FL_RO_DEVNODE;\n> >\n> > \n> >\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_devnode = !!(vdev->flags & V4L2_FL_RO_DEVNODE);\n\nNo need to shout, i.e.\n\n\tbool ro_devnode = ... & ...;\n\n> >\n> > So V4L2_FL_RO_DEVNODE is a bit mask, ...\n> >\n> > \n> >\n> > > diff --git a/include/media/v4l2-dev.h b/include/media/v4l2-dev.h\n> > > index 48531e57cc5a..029873a338f2 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> > ... then V4L2_FL_RO_DEVNODE should rather be equal to 4, than to (V4L2_FL_USES_V4L2_FH | V4L2_FL_QUIRK_INVERTED_CROP)\n> \n> Ups!\n> \n> I should have noticed looking at V4L2_FL_REGISTERED=0 that these\n> flags are actually meant to be used as bit shifts. I can't say I like\n> the idea, but I should have known better than this.\n> \n> I'll resend using set_bit() and test_bit().\n\nYou can also use BIT(flag name).\n\nEither works, but IMO test_bit() is a bit of overkill.","headers":{"Return-Path":"","Received":["from mga12.intel.com (mga12.intel.com [192.55.52.136])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 5989E60411\n\tfor ;\n\tThu, 26 Mar 2020 01:08:15 +0100 (CET)","from fmsmga007.fm.intel.com ([10.253.24.52])\n\tby fmsmga106.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; \n\t25 Mar 2020 17:08:13 -0700","from jamendoz-mobl1.amr.corp.intel.com (HELO mara.localdomain)\n\t([10.249.35.12])\n\tby fmsmga007.fm.intel.com with ESMTP; 25 Mar 2020 17:08:11 -0700","from sailus by mara.localdomain with local (Exim 4.92)\n\t(envelope-from )\n\tid 1jHG49-00011Q-Da; Thu, 26 Mar 2020 02:08:10 +0200"],"IronPort-SDR":["C4SY+ooHcr/FRSvCsoPGph7unG2ilgrXGOAzFnbAEzlcxdUpd0mSuwIAnhV9NEBqx3LU6YwXC8\n\tZoFGH0u5EOuQ==","2QVkHT2EOC+Vu02ewkEqzFuRBU2RudMixVdt8W5ETqk3wfNFhLHdQHQ+//Bm0dAef2tdPb6roS\n\tXV46VFPBPQXg=="],"X-Amp-Result":"SKIPPED(no attachment in message)","X-Amp-File-Uploaded":"False","X-ExtLoop1":"1","X-IronPort-AV":"E=Sophos;i=\"5.72,306,1580803200\"; d=\"scan'208\";a=\"238655773\"","Date":"Thu, 26 Mar 2020 02:08:08 +0200","From":"Sakari Ailus ","To":"Jacopo Mondi ","Cc":"Andrey Konovalov ,\n\tlinux-media@vger.kernel.org, libcamera-devel@lists.libcamera.org,\n\thverkuil-cisco@xs4all.nl, mchehab@kernel.org","Message-ID":"<20200326000808.GB3709@mara.localdomain>","References":"<20200324202844.1518292-1-jacopo@jmondi.org>\n\t<20200324202844.1518292-3-jacopo@jmondi.org>\n\t<313fcb7e-6612-9cf5-a4eb-ba6edb39f754@linaro.org>\n\t<20200325112342.andqi2uognizd4uq@uno.localdomain>","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"<20200325112342.andqi2uognizd4uq@uno.localdomain>","User-Agent":"Mutt/1.10.1 (2018-07-13)","Subject":"Re: [libcamera-devel] [PATCH 2/4] 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":"Thu, 26 Mar 2020 00:08:16 -0000"}}]