All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jacopo Mondi <jacopo@jmondi.org>
To: linux-media@vger.kernel.org, libcamera-devel@lists.libcamera.org
Cc: Jacopo Mondi <jacopo@jmondi.org>,
	mchehab@kernel.org, hverkuil-cisco@xs4all.nl,
	sakari.ailus@linux.intel.com, andrey.konovalov@linaro.org,
	laurent.pinchart@ideasonboard.com
Subject: [PATCH v5.1] media: v4l2-subdev: Guard whole fops and ioctl hdlr
Date: Wed, 29 Apr 2020 10:58:55 +0200	[thread overview]
Message-ID: <20200429085855.186273-1-jacopo@jmondi.org> (raw)
In-Reply-To: <20200428210609.6793-5-jacopo@jmondi.org>

A sub-device device node can be registered in user space only if the
CONFIG_V4L2_SUBDEV_API Kconfig option is selected. Currently the
open/close file operations and the ioctl handler have some parts of their
implementations guarded by #if defined(CONFIG_V4L2_SUBDEV_API), while
they are actually not accessible without a video device node registered
to user space.

Guard the whole open, close and ioctl handler and provide stubs if the
V4L2_SUBDEV_API Kconfig option is not selected.

This slightly reduces the kernel size when the option is not selected
and simplifies the file ops and ioctl implementations.

Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
---
A different approach compared to v5, which was anyway buggy and not a proper
solution.

Sending out for comments, while waiting for consensus on v5 [5/6] (reserved
space in the ioctl argument vs versioning based on structure size)

Compile tested with and without V4L2_SUBDEV_API Kconfig option enabled and
with drivers that depends on it built-in or as modules.

---
 drivers/media/v4l2-core/v4l2-subdev.c | 39 +++++++++++++++++++++------
 1 file changed, 31 insertions(+), 8 deletions(-)

diff --git a/drivers/media/v4l2-core/v4l2-subdev.c b/drivers/media/v4l2-core/v4l2-subdev.c
index 1dc263c2ca0a..6fef52880c99 100644
--- a/drivers/media/v4l2-core/v4l2-subdev.c
+++ b/drivers/media/v4l2-core/v4l2-subdev.c
@@ -22,24 +22,22 @@
 #include <media/v4l2-fh.h>
 #include <media/v4l2-event.h>

+#if defined(CONFIG_V4L2_SUBDEV_API)
 static int subdev_fh_init(struct v4l2_subdev_fh *fh, struct v4l2_subdev *sd)
 {
-#if defined(CONFIG_VIDEO_V4L2_SUBDEV_API)
 	if (sd->entity.num_pads) {
 		fh->pad = v4l2_subdev_alloc_pad_config(sd);
 		if (fh->pad == NULL)
 			return -ENOMEM;
 	}
-#endif
+
 	return 0;
 }

 static void subdev_fh_free(struct v4l2_subdev_fh *fh)
 {
-#if defined(CONFIG_VIDEO_V4L2_SUBDEV_API)
 	v4l2_subdev_free_pad_config(fh->pad);
 	fh->pad = NULL;
-#endif
 }

 static int subdev_open(struct file *file)
@@ -111,6 +109,17 @@ static int subdev_close(struct file *file)

 	return 0;
 }
+#else /* CONFIG_V4L2_SUBDEV_API */
+static int subdev_open(struct file *file)
+{
+	return 0;
+}
+
+static int subdev_close(struct file *file)
+{
+	return 0;
+}
+#endif /* CONFIG_V4L2_SUBDEV_API */

 static inline int check_which(u32 which)
 {
@@ -324,15 +333,14 @@ const struct v4l2_subdev_ops v4l2_subdev_call_wrappers = {
 };
 EXPORT_SYMBOL(v4l2_subdev_call_wrappers);

+#if defined(CONFIG_V4L2_SUBDEV_API)
 static long subdev_do_ioctl(struct file *file, unsigned int cmd, void *arg)
 {
 	struct video_device *vdev = video_devdata(file);
 	struct v4l2_subdev *sd = vdev_to_v4l2_subdev(vdev);
 	struct v4l2_fh *vfh = file->private_data;
-#if defined(CONFIG_VIDEO_V4L2_SUBDEV_API)
 	struct v4l2_subdev_fh *subdev_fh = to_v4l2_subdev_fh(vfh);
 	bool ro_subdev = test_bit(V4L2_FL_SUBDEV_RO_DEVNODE, &vdev->flags);
-#endif
 	int rval;

 	switch (cmd) {
@@ -466,7 +474,6 @@ static long subdev_do_ioctl(struct file *file, unsigned int cmd, void *arg)
 		return ret;
 	}

-#if defined(CONFIG_VIDEO_V4L2_SUBDEV_API)
 	case VIDIOC_SUBDEV_G_FMT: {
 		struct v4l2_subdev_format *format = arg;

@@ -646,7 +653,7 @@ static long subdev_do_ioctl(struct file *file, unsigned int cmd, void *arg)

 	case VIDIOC_SUBDEV_QUERYSTD:
 		return v4l2_subdev_call(sd, video, querystd, arg);
-#endif
+
 	default:
 		return v4l2_subdev_call(sd, core, ioctl, cmd, arg);
 	}
@@ -686,6 +693,22 @@ static long subdev_compat_ioctl32(struct file *file, unsigned int cmd,
 }
 #endif

+#else /* CONFIG_V4L2_SUBDEV_API */
+static long subdev_ioctl(struct file *file, unsigned int cmd,
+	unsigned long arg)
+{
+	return 0;
+}
+
+#ifdef CONFIG_COMPAT
+static long subdev_compat_ioctl32(struct file *file, unsigned int cmd,
+	unsigned long arg)
+{
+	return 0;
+}
+#endif
+#endif /* CONFIG_V4L2_SUBDEV_API */
+
 static __poll_t subdev_poll(struct file *file, poll_table *wait)
 {
 	struct video_device *vdev = video_devdata(file);
--
2.26.1


  parent reply	other threads:[~2020-04-29  8:55 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-04-28 21:06 [PATCH v5 0/6] media: Register read-only sub-dev devnode Jacopo Mondi
2020-04-28 21:06 ` [PATCH v5 1/6] Documentation: media: Update sub-device API intro Jacopo Mondi
2020-04-28 21:06 ` [PATCH v5 2/6] Documentation: media: Document read-only subdevice Jacopo Mondi
2020-04-28 21:06 ` [PATCH v5 3/6] media: v4l2-dev: Add v4l2_device_register_ro_subdev_node() Jacopo Mondi
2020-04-28 21:06 ` [PATCH v5 4/6] media: v4l2-subdev: Assume V4L2_SUBDEV_API is selected Jacopo Mondi
2020-04-28 21:26   ` Sakari Ailus
2020-04-29  7:02     ` Jacopo Mondi
2020-04-29  8:27       ` Sakari Ailus
2020-04-29  8:43         ` Jacopo Mondi
2020-04-28 23:44   ` kbuild test robot
2020-04-28 23:44     ` kbuild test robot
2020-04-29  7:04     ` Jacopo Mondi
2020-04-29  8:58   ` Jacopo Mondi [this message]
2020-04-29  9:49     ` [PATCH v5.1] media: v4l2-subdev: Guard whole fops and ioctl hdlr Sakari Ailus
2020-04-29 10:16       ` Jacopo Mondi
2020-04-29 11:00         ` Sakari Ailus
2020-04-28 21:06 ` [PATCH v5 5/6] v4l2-subdev: add VIDIOC_SUBDEV_QUERYCAP ioctl Jacopo Mondi
2020-04-28 21:28   ` Sakari Ailus
2020-04-29  8:09     ` Jacopo Mondi
2020-04-29  8:18       ` Sakari Ailus
2020-05-06 13:29         ` Hans Verkuil
2020-05-06 18:34           ` Sakari Ailus
2020-05-07  7:14             ` Hans Verkuil
2020-04-28 21:06 ` [PATCH v5 6/6] v4l: document VIDIOC_SUBDEV_QUERYCAP Jacopo Mondi

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200429085855.186273-1-jacopo@jmondi.org \
    --to=jacopo@jmondi.org \
    --cc=andrey.konovalov@linaro.org \
    --cc=hverkuil-cisco@xs4all.nl \
    --cc=laurent.pinchart@ideasonboard.com \
    --cc=libcamera-devel@lists.libcamera.org \
    --cc=linux-media@vger.kernel.org \
    --cc=mchehab@kernel.org \
    --cc=sakari.ailus@linux.intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.