[{"id":4643,"web_url":"https://patchwork.libcamera.org/comment/4643/","msgid":"<20200429094949.GD9190@paasikivi.fi.intel.com>","date":"2020-04-29T09:49:49","subject":"Re: [libcamera-devel] [PATCH v5.1] media: v4l2-subdev: Guard whole\n\tfops and ioctl hdlr","submitter":{"id":37,"url":"https://patchwork.libcamera.org/api/people/37/","name":"Sakari Ailus","email":"sakari.ailus@linux.intel.com"},"content":"Hi Jacopo,\n\nThanks for the update.\n\nOn Wed, Apr 29, 2020 at 10:58:55AM +0200, Jacopo Mondi wrote:\n> A sub-device device node can be registered in user space only if the\n> CONFIG_V4L2_SUBDEV_API Kconfig option is selected. Currently the\n> open/close file operations and the ioctl handler have some parts of their\n> implementations guarded by #if defined(CONFIG_V4L2_SUBDEV_API), while\n> they are actually not accessible without a video device node registered\n> to user space.\n> \n> Guard the whole open, close and ioctl handler and provide stubs if the\n> V4L2_SUBDEV_API Kconfig option is not selected.\n> \n> This slightly reduces the kernel size when the option is not selected\n> and simplifies the file ops and ioctl implementations.\n> \n> Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>\n> ---\n> A different approach compared to v5, which was anyway buggy and not a proper\n> solution.\n> \n> Sending out for comments, while waiting for consensus on v5 [5/6] (reserved\n> space in the ioctl argument vs versioning based on structure size)\n> \n> Compile tested with and without V4L2_SUBDEV_API Kconfig option enabled and\n> with drivers that depends on it built-in or as modules.\n> \n> ---\n>  drivers/media/v4l2-core/v4l2-subdev.c | 39 +++++++++++++++++++++------\n>  1 file changed, 31 insertions(+), 8 deletions(-)\n> \n> diff --git a/drivers/media/v4l2-core/v4l2-subdev.c b/drivers/media/v4l2-core/v4l2-subdev.c\n> index 1dc263c2ca0a..6fef52880c99 100644\n> --- a/drivers/media/v4l2-core/v4l2-subdev.c\n> +++ b/drivers/media/v4l2-core/v4l2-subdev.c\n> @@ -22,24 +22,22 @@\n>  #include <media/v4l2-fh.h>\n>  #include <media/v4l2-event.h>\n> \n> +#if defined(CONFIG_V4L2_SUBDEV_API)\n>  static int subdev_fh_init(struct v4l2_subdev_fh *fh, struct v4l2_subdev *sd)\n>  {\n> -#if defined(CONFIG_VIDEO_V4L2_SUBDEV_API)\n>  \tif (sd->entity.num_pads) {\n>  \t\tfh->pad = v4l2_subdev_alloc_pad_config(sd);\n>  \t\tif (fh->pad == NULL)\n>  \t\t\treturn -ENOMEM;\n>  \t}\n> -#endif\n> +\n>  \treturn 0;\n>  }\n> \n>  static void subdev_fh_free(struct v4l2_subdev_fh *fh)\n>  {\n> -#if defined(CONFIG_VIDEO_V4L2_SUBDEV_API)\n>  \tv4l2_subdev_free_pad_config(fh->pad);\n>  \tfh->pad = NULL;\n> -#endif\n>  }\n> \n>  static int subdev_open(struct file *file)\n> @@ -111,6 +109,17 @@ static int subdev_close(struct file *file)\n> \n>  \treturn 0;\n>  }\n> +#else /* CONFIG_V4L2_SUBDEV_API */\n> +static int subdev_open(struct file *file)\n> +{\n> +\treturn 0;\n\nPerhaps:\n\nreturn -ENODEV;\n\nAnd I'd use inline functions in the header.\n\n> +}\n> +\n> +static int subdev_close(struct file *file)\n> +{\n> +\treturn 0;\n> +}\n> +#endif /* CONFIG_V4L2_SUBDEV_API */\n> \n>  static inline int check_which(u32 which)\n>  {\n> @@ -324,15 +333,14 @@ const struct v4l2_subdev_ops v4l2_subdev_call_wrappers = {\n>  };\n>  EXPORT_SYMBOL(v4l2_subdev_call_wrappers);\n> \n> +#if defined(CONFIG_V4L2_SUBDEV_API)\n>  static long subdev_do_ioctl(struct file *file, unsigned int cmd, void *arg)\n>  {\n>  \tstruct video_device *vdev = video_devdata(file);\n>  \tstruct v4l2_subdev *sd = vdev_to_v4l2_subdev(vdev);\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_SUBDEV_RO_DEVNODE, &vdev->flags);\n> -#endif\n>  \tint rval;\n> \n>  \tswitch (cmd) {\n> @@ -466,7 +474,6 @@ static long subdev_do_ioctl(struct file *file, unsigned int cmd, void *arg)\n>  \t\treturn ret;\n>  \t}\n> \n> -#if defined(CONFIG_VIDEO_V4L2_SUBDEV_API)\n>  \tcase VIDIOC_SUBDEV_G_FMT: {\n>  \t\tstruct v4l2_subdev_format *format = arg;\n> \n> @@ -646,7 +653,7 @@ static long subdev_do_ioctl(struct file *file, unsigned int cmd, void *arg)\n> \n>  \tcase VIDIOC_SUBDEV_QUERYSTD:\n>  \t\treturn v4l2_subdev_call(sd, video, querystd, arg);\n> -#endif\n> +\n>  \tdefault:\n>  \t\treturn v4l2_subdev_call(sd, core, ioctl, cmd, arg);\n>  \t}\n> @@ -686,6 +693,22 @@ static long subdev_compat_ioctl32(struct file *file, unsigned int cmd,\n>  }\n>  #endif\n> \n> +#else /* CONFIG_V4L2_SUBDEV_API */\n> +static long subdev_ioctl(struct file *file, unsigned int cmd,\n> +\tunsigned long arg)\n> +{\n> +\treturn 0;\n\nreturn -ENOTTY;\n\n> +}\n> +\n> +#ifdef CONFIG_COMPAT\n> +static long subdev_compat_ioctl32(struct file *file, unsigned int cmd,\n> +\tunsigned long arg)\n> +{\n> +\treturn 0;\n\nDitto.\n\n> +}\n> +#endif\n> +#endif /* CONFIG_V4L2_SUBDEV_API */\n> +\n>  static __poll_t subdev_poll(struct file *file, poll_table *wait)\n>  {\n>  \tstruct video_device *vdev = video_devdata(file);","headers":{"Return-Path":"<sakari.ailus@linux.intel.com>","Received":["from mga18.intel.com (mga18.intel.com [134.134.136.126])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 53A3E601B8\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tWed, 29 Apr 2020 11:49:56 +0200 (CEST)","from fmsmga004.fm.intel.com ([10.253.24.48])\n\tby orsmga106.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; \n\t29 Apr 2020 02:49:54 -0700","from paasikivi.fi.intel.com ([10.237.72.42])\n\tby fmsmga004-auth.fm.intel.com with\n\tESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 29 Apr 2020 02:49:52 -0700","by paasikivi.fi.intel.com (Postfix, from userid 1000)\n\tid 0661B202AE; Wed, 29 Apr 2020 12:49:50 +0300 (EEST)"],"IronPort-SDR":["kA6AYDgpD2EmyVJyqck0gcGVAV3++T9qlJbYmh7TpgzV+tDvdEo9t3pgGE1grsRbMhTp5HFb4T\n\teQhHkzcgZxxg==","JEygnl9yaEVQsCszSJ2YUNIeGk7D7OJ5qRcpn7IdBsgxubgyF8Akbscx6buZNflAkYtGOb46jv\n\tC9xTEnY1B5Zw=="],"X-Amp-Result":"SKIPPED(no attachment in message)","X-Amp-File-Uploaded":"False","X-IronPort-AV":"E=Sophos;i=\"5.73,331,1583222400\"; d=\"scan'208\";a=\"282447760\"","Date":"Wed, 29 Apr 2020 12:49:49 +0300","From":"Sakari Ailus <sakari.ailus@linux.intel.com>","To":"Jacopo Mondi <jacopo@jmondi.org>","Cc":"linux-media@vger.kernel.org, libcamera-devel@lists.libcamera.org,\n\tmchehab@kernel.org, hverkuil-cisco@xs4all.nl,\n\tandrey.konovalov@linaro.org, laurent.pinchart@ideasonboard.com","Message-ID":"<20200429094949.GD9190@paasikivi.fi.intel.com>","References":"<20200428210609.6793-5-jacopo@jmondi.org>\n\t<20200429085855.186273-1-jacopo@jmondi.org>","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"<20200429085855.186273-1-jacopo@jmondi.org>","User-Agent":"Mutt/1.10.1 (2018-07-13)","Subject":"Re: [libcamera-devel] [PATCH v5.1] media: v4l2-subdev: Guard whole\n\tfops and ioctl hdlr","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":"Wed, 29 Apr 2020 09:49:57 -0000"}},{"id":4644,"web_url":"https://patchwork.libcamera.org/comment/4644/","msgid":"<20200429101600.pkw3i5igq5lla5ev@uno.localdomain>","date":"2020-04-29T10:16:00","subject":"Re: [libcamera-devel] [PATCH v5.1] media: v4l2-subdev: Guard whole\n\tfops and ioctl hdlr","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Sakari,\n\nOn Wed, Apr 29, 2020 at 12:49:49PM +0300, Sakari Ailus wrote:\n> Hi Jacopo,\n>\n> Thanks for the update.\n>\n> On Wed, Apr 29, 2020 at 10:58:55AM +0200, Jacopo Mondi wrote:\n> > A sub-device device node can be registered in user space only if the\n> > CONFIG_V4L2_SUBDEV_API Kconfig option is selected. Currently the\n> > open/close file operations and the ioctl handler have some parts of their\n> > implementations guarded by #if defined(CONFIG_V4L2_SUBDEV_API), while\n> > they are actually not accessible without a video device node registered\n> > to user space.\n> >\n> > Guard the whole open, close and ioctl handler and provide stubs if the\n> > V4L2_SUBDEV_API Kconfig option is not selected.\n> >\n> > This slightly reduces the kernel size when the option is not selected\n> > and simplifies the file ops and ioctl implementations.\n> >\n> > Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>\n> > ---\n> > A different approach compared to v5, which was anyway buggy and not a proper\n> > solution.\n> >\n> > Sending out for comments, while waiting for consensus on v5 [5/6] (reserved\n> > space in the ioctl argument vs versioning based on structure size)\n> >\n> > Compile tested with and without V4L2_SUBDEV_API Kconfig option enabled and\n> > with drivers that depends on it built-in or as modules.\n> >\n> > ---\n> >  drivers/media/v4l2-core/v4l2-subdev.c | 39 +++++++++++++++++++++------\n> >  1 file changed, 31 insertions(+), 8 deletions(-)\n> >\n> > diff --git a/drivers/media/v4l2-core/v4l2-subdev.c b/drivers/media/v4l2-core/v4l2-subdev.c\n> > index 1dc263c2ca0a..6fef52880c99 100644\n> > --- a/drivers/media/v4l2-core/v4l2-subdev.c\n> > +++ b/drivers/media/v4l2-core/v4l2-subdev.c\n> > @@ -22,24 +22,22 @@\n> >  #include <media/v4l2-fh.h>\n> >  #include <media/v4l2-event.h>\n> >\n> > +#if defined(CONFIG_V4L2_SUBDEV_API)\n> >  static int subdev_fh_init(struct v4l2_subdev_fh *fh, struct v4l2_subdev *sd)\n> >  {\n> > -#if defined(CONFIG_VIDEO_V4L2_SUBDEV_API)\n> >  \tif (sd->entity.num_pads) {\n> >  \t\tfh->pad = v4l2_subdev_alloc_pad_config(sd);\n> >  \t\tif (fh->pad == NULL)\n> >  \t\t\treturn -ENOMEM;\n> >  \t}\n> > -#endif\n> > +\n> >  \treturn 0;\n> >  }\n> >\n> >  static void subdev_fh_free(struct v4l2_subdev_fh *fh)\n> >  {\n> > -#if defined(CONFIG_VIDEO_V4L2_SUBDEV_API)\n> >  \tv4l2_subdev_free_pad_config(fh->pad);\n> >  \tfh->pad = NULL;\n> > -#endif\n> >  }\n> >\n> >  static int subdev_open(struct file *file)\n> > @@ -111,6 +109,17 @@ static int subdev_close(struct file *file)\n> >\n> >  \treturn 0;\n> >  }\n> > +#else /* CONFIG_V4L2_SUBDEV_API */\n> > +static int subdev_open(struct file *file)\n> > +{\n> > +\treturn 0;\n>\n> Perhaps:\n>\n> return -ENODEV;\n>\n> And I'd use inline functions in the header.\n\nThere is no way this functions gets called if no device node is\nregistered to userspace, I think. These are only here to please the\ncompiler. Am I mistaken ?\n\nAlso, these function are not exported by any headers, but only the\nfile_operations structure that contains them is:\n\ninclude/media/v4l2-subdev.h\n        extern const struct v4l2_file_operations v4l2_subdev_fops;\n\n>\n> > +}\n> > +\n> > +static int subdev_close(struct file *file)\n> > +{\n> > +\treturn 0;\n> > +}\n> > +#endif /* CONFIG_V4L2_SUBDEV_API */\n> >\n> >  static inline int check_which(u32 which)\n> >  {\n> > @@ -324,15 +333,14 @@ const struct v4l2_subdev_ops v4l2_subdev_call_wrappers = {\n> >  };\n> >  EXPORT_SYMBOL(v4l2_subdev_call_wrappers);\n> >\n> > +#if defined(CONFIG_V4L2_SUBDEV_API)\n> >  static long subdev_do_ioctl(struct file *file, unsigned int cmd, void *arg)\n> >  {\n> >  \tstruct video_device *vdev = video_devdata(file);\n> >  \tstruct v4l2_subdev *sd = vdev_to_v4l2_subdev(vdev);\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_SUBDEV_RO_DEVNODE, &vdev->flags);\n> > -#endif\n> >  \tint rval;\n> >\n> >  \tswitch (cmd) {\n> > @@ -466,7 +474,6 @@ static long subdev_do_ioctl(struct file *file, unsigned int cmd, void *arg)\n> >  \t\treturn ret;\n> >  \t}\n> >\n> > -#if defined(CONFIG_VIDEO_V4L2_SUBDEV_API)\n> >  \tcase VIDIOC_SUBDEV_G_FMT: {\n> >  \t\tstruct v4l2_subdev_format *format = arg;\n> >\n> > @@ -646,7 +653,7 @@ static long subdev_do_ioctl(struct file *file, unsigned int cmd, void *arg)\n> >\n> >  \tcase VIDIOC_SUBDEV_QUERYSTD:\n> >  \t\treturn v4l2_subdev_call(sd, video, querystd, arg);\n> > -#endif\n> > +\n> >  \tdefault:\n> >  \t\treturn v4l2_subdev_call(sd, core, ioctl, cmd, arg);\n> >  \t}\n> > @@ -686,6 +693,22 @@ static long subdev_compat_ioctl32(struct file *file, unsigned int cmd,\n> >  }\n> >  #endif\n> >\n> > +#else /* CONFIG_V4L2_SUBDEV_API */\n> > +static long subdev_ioctl(struct file *file, unsigned int cmd,\n> > +\tunsigned long arg)\n> > +{\n> > +\treturn 0;\n>\n> return -ENOTTY;\n>\n> > +}\n> > +\n> > +#ifdef CONFIG_COMPAT\n> > +static long subdev_compat_ioctl32(struct file *file, unsigned int cmd,\n> > +\tunsigned long arg)\n> > +{\n> > +\treturn 0;\n>\n> Ditto.\n>\n> > +}\n> > +#endif\n> > +#endif /* CONFIG_V4L2_SUBDEV_API */\n> > +\n> >  static __poll_t subdev_poll(struct file *file, poll_table *wait)\n> >  {\n> >  \tstruct video_device *vdev = video_devdata(file);\n>\n> --\n> Kind regards,\n>\n> Sakari Ailus","headers":{"Return-Path":"<jacopo@jmondi.org>","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 1EB8B603F7\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tWed, 29 Apr 2020 12:12:51 +0200 (CEST)","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 0CF4B4000C;\n\tWed, 29 Apr 2020 10:12:48 +0000 (UTC)"],"X-Originating-IP":"2.224.242.101","Date":"Wed, 29 Apr 2020 12:16:00 +0200","From":"Jacopo Mondi <jacopo@jmondi.org>","To":"Sakari Ailus <sakari.ailus@linux.intel.com>","Cc":"linux-media@vger.kernel.org, libcamera-devel@lists.libcamera.org,\n\tmchehab@kernel.org, hverkuil-cisco@xs4all.nl,\n\tandrey.konovalov@linaro.org, laurent.pinchart@ideasonboard.com","Message-ID":"<20200429101600.pkw3i5igq5lla5ev@uno.localdomain>","References":"<20200428210609.6793-5-jacopo@jmondi.org>\n\t<20200429085855.186273-1-jacopo@jmondi.org>\n\t<20200429094949.GD9190@paasikivi.fi.intel.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20200429094949.GD9190@paasikivi.fi.intel.com>","Subject":"Re: [libcamera-devel] [PATCH v5.1] media: v4l2-subdev: Guard whole\n\tfops and ioctl hdlr","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":"Wed, 29 Apr 2020 10:12:51 -0000"}},{"id":4645,"web_url":"https://patchwork.libcamera.org/comment/4645/","msgid":"<20200429110002.GE9190@paasikivi.fi.intel.com>","date":"2020-04-29T11:00:02","subject":"Re: [libcamera-devel] [PATCH v5.1] media: v4l2-subdev: Guard whole\n\tfops and ioctl hdlr","submitter":{"id":37,"url":"https://patchwork.libcamera.org/api/people/37/","name":"Sakari Ailus","email":"sakari.ailus@linux.intel.com"},"content":"On Wed, Apr 29, 2020 at 12:16:00PM +0200, Jacopo Mondi wrote:\n> Hi Sakari,\n> \n> On Wed, Apr 29, 2020 at 12:49:49PM +0300, Sakari Ailus wrote:\n> > Hi Jacopo,\n> >\n> > Thanks for the update.\n> >\n> > On Wed, Apr 29, 2020 at 10:58:55AM +0200, Jacopo Mondi wrote:\n> > > A sub-device device node can be registered in user space only if the\n> > > CONFIG_V4L2_SUBDEV_API Kconfig option is selected. Currently the\n> > > open/close file operations and the ioctl handler have some parts of their\n> > > implementations guarded by #if defined(CONFIG_V4L2_SUBDEV_API), while\n> > > they are actually not accessible without a video device node registered\n> > > to user space.\n> > >\n> > > Guard the whole open, close and ioctl handler and provide stubs if the\n> > > V4L2_SUBDEV_API Kconfig option is not selected.\n> > >\n> > > This slightly reduces the kernel size when the option is not selected\n> > > and simplifies the file ops and ioctl implementations.\n> > >\n> > > Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>\n> > > ---\n> > > A different approach compared to v5, which was anyway buggy and not a proper\n> > > solution.\n> > >\n> > > Sending out for comments, while waiting for consensus on v5 [5/6] (reserved\n> > > space in the ioctl argument vs versioning based on structure size)\n> > >\n> > > Compile tested with and without V4L2_SUBDEV_API Kconfig option enabled and\n> > > with drivers that depends on it built-in or as modules.\n> > >\n> > > ---\n> > >  drivers/media/v4l2-core/v4l2-subdev.c | 39 +++++++++++++++++++++------\n> > >  1 file changed, 31 insertions(+), 8 deletions(-)\n> > >\n> > > diff --git a/drivers/media/v4l2-core/v4l2-subdev.c b/drivers/media/v4l2-core/v4l2-subdev.c\n> > > index 1dc263c2ca0a..6fef52880c99 100644\n> > > --- a/drivers/media/v4l2-core/v4l2-subdev.c\n> > > +++ b/drivers/media/v4l2-core/v4l2-subdev.c\n> > > @@ -22,24 +22,22 @@\n> > >  #include <media/v4l2-fh.h>\n> > >  #include <media/v4l2-event.h>\n> > >\n> > > +#if defined(CONFIG_V4L2_SUBDEV_API)\n> > >  static int subdev_fh_init(struct v4l2_subdev_fh *fh, struct v4l2_subdev *sd)\n> > >  {\n> > > -#if defined(CONFIG_VIDEO_V4L2_SUBDEV_API)\n> > >  \tif (sd->entity.num_pads) {\n> > >  \t\tfh->pad = v4l2_subdev_alloc_pad_config(sd);\n> > >  \t\tif (fh->pad == NULL)\n> > >  \t\t\treturn -ENOMEM;\n> > >  \t}\n> > > -#endif\n> > > +\n> > >  \treturn 0;\n> > >  }\n> > >\n> > >  static void subdev_fh_free(struct v4l2_subdev_fh *fh)\n> > >  {\n> > > -#if defined(CONFIG_VIDEO_V4L2_SUBDEV_API)\n> > >  \tv4l2_subdev_free_pad_config(fh->pad);\n> > >  \tfh->pad = NULL;\n> > > -#endif\n> > >  }\n> > >\n> > >  static int subdev_open(struct file *file)\n> > > @@ -111,6 +109,17 @@ static int subdev_close(struct file *file)\n> > >\n> > >  \treturn 0;\n> > >  }\n> > > +#else /* CONFIG_V4L2_SUBDEV_API */\n> > > +static int subdev_open(struct file *file)\n> > > +{\n> > > +\treturn 0;\n> >\n> > Perhaps:\n> >\n> > return -ENODEV;\n> >\n> > And I'd use inline functions in the header.\n\nUm, as they're only used here, they indeed should remain here, not in\nthe header. I missed that while commenting.\n\n> \n> There is no way this functions gets called if no device node is\n> registered to userspace, I think. These are only here to please the\n> compiler. Am I mistaken ?\n\nThe fops struct is still there. It shouldn't be called, no, but if that\nhappens, then providing the right return value is helpful.\n\n> \n> Also, these function are not exported by any headers, but only the\n> file_operations structure that contains them is:\n> \n> include/media/v4l2-subdev.h\n>         extern const struct v4l2_file_operations v4l2_subdev_fops;\n\nRight, agreed.","headers":{"Return-Path":"<sakari.ailus@linux.intel.com>","Received":["from mga03.intel.com (mga03.intel.com [134.134.136.65])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 4FE4E603F7\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tWed, 29 Apr 2020 13:00:09 +0200 (CEST)","from orsmga004.jf.intel.com ([10.7.209.38])\n\tby orsmga103.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; \n\t29 Apr 2020 04:00:07 -0700","from paasikivi.fi.intel.com ([10.237.72.42])\n\tby orsmga004-auth.jf.intel.com with\n\tESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 29 Apr 2020 04:00:05 -0700","by paasikivi.fi.intel.com (Postfix, from userid 1000)\n\tid B2391202AE; Wed, 29 Apr 2020 14:00:02 +0300 (EEST)"],"IronPort-SDR":["hZsiIf35ZwxQS4wBIxcxrLMoGT/+jMS6g1GAmkUxfoON3SxA44c/1AlWmP+72cRiFN/B+Jkd0f\n\tU2c/qZ+sLKmg==","0d1/HVOJClYNeyj6JTmEp2xuWS76tsilnSrhnRkXiSGrdC0ZBQ3vVYtgZa1Ly7iMPhJELku0b8\n\t7Ms+15A1d6eA=="],"X-Amp-Result":"SKIPPED(no attachment in message)","X-Amp-File-Uploaded":"False","X-IronPort-AV":"E=Sophos;i=\"5.73,331,1583222400\"; d=\"scan'208\";a=\"405006917\"","Date":"Wed, 29 Apr 2020 14:00:02 +0300","From":"Sakari Ailus <sakari.ailus@linux.intel.com>","To":"Jacopo Mondi <jacopo@jmondi.org>","Cc":"linux-media@vger.kernel.org, libcamera-devel@lists.libcamera.org,\n\tmchehab@kernel.org, hverkuil-cisco@xs4all.nl,\n\tandrey.konovalov@linaro.org, laurent.pinchart@ideasonboard.com","Message-ID":"<20200429110002.GE9190@paasikivi.fi.intel.com>","References":"<20200428210609.6793-5-jacopo@jmondi.org>\n\t<20200429085855.186273-1-jacopo@jmondi.org>\n\t<20200429094949.GD9190@paasikivi.fi.intel.com>\n\t<20200429101600.pkw3i5igq5lla5ev@uno.localdomain>","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"<20200429101600.pkw3i5igq5lla5ev@uno.localdomain>","User-Agent":"Mutt/1.10.1 (2018-07-13)","Subject":"Re: [libcamera-devel] [PATCH v5.1] media: v4l2-subdev: Guard whole\n\tfops and ioctl hdlr","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":"Wed, 29 Apr 2020 11:00:09 -0000"}}]