[RFC/PATCH v3 0/7] V4L2 subdev userspace API

[RFC/PATCH v3 0/7] V4L2 subdev userspace API

[Laurent Pinchart]

Hi everybody,

Here's the third version of the V4L2 subdev userspace API patches. Comments
received on the first and second versions have been incorporated, including the
video_usercopy usage. The generic ioctls support patch has been dropped and
will be resubmitted later with a use case.

Laurent Pinchart (6):
  v4l: Share code between video_usercopy and video_ioctl2
  v4l: subdev: Don't require core operations
  v4l: subdev: Merge v4l2_i2c_new_subdev_cfg and v4l2_i2c_new_subdev
  v4l: subdev: Add device node support
  v4l: subdev: Uninline the v4l2_subdev_init function
  v4l: subdev: Control ioctls support

Sakari Ailus (1):
  v4l: subdev: Events support

 Documentation/video4linux/v4l2-framework.txt |   52 ++++++
 drivers/media/radio/radio-si4713.c           |    2 +-
 drivers/media/video/Makefile                 |    2 +-
 drivers/media/video/soc_camera.c             |    2 +-
 drivers/media/video/v4l2-common.c            |   22 ++-
 drivers/media/video/v4l2-dev.c               |   27 ++--
 drivers/media/video/v4l2-device.c            |   25 +++-
 drivers/media/video/v4l2-ioctl.c             |  216 +++++++++-----------------
 drivers/media/video/v4l2-subdev.c            |  176 +++++++++++++++++++++
 include/media/v4l2-common.h                  |   21 +--
 include/media/v4l2-dev.h                     |   18 ++-
 include/media/v4l2-ioctl.h                   |    3 +
 include/media/v4l2-subdev.h                  |   40 +++--
 13 files changed, 398 insertions(+), 208 deletions(-)
 create mode 100644 drivers/media/video/v4l2-subdev.c

-- 
Regards,

Laurent Pinchart

[RFC/PATCH v3 1/7] v4l: Share code between video_usercopy and video_ioctl2

[Laurent Pinchart]

The two functions are mostly identical. They handle the copy_from_user
and copy_to_user operations related with V4L2 ioctls and call the real
ioctl handler.

Create a __video_usercopy function that implements the core of
video_usercopy and video_ioctl2, and call that function from both.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/media/video/v4l2-ioctl.c |  218 ++++++++++++-------------------------
 1 files changed, 71 insertions(+), 147 deletions(-)

diff --git a/drivers/media/video/v4l2-ioctl.c b/drivers/media/video/v4l2-ioctl.c
index 0eeceae..486eaba 100644
--- a/drivers/media/video/v4l2-ioctl.c
+++ b/drivers/media/video/v4l2-ioctl.c
@@ -373,35 +373,62 @@ video_fix_command(unsigned int cmd)
 }
 #endif
 
-/*
- * Obsolete usercopy function - Should be removed soon
- */
-long
-video_usercopy(struct file *file, unsigned int cmd, unsigned long arg,
+/* In some cases, only a few fields are used as input, i.e. when the app sets
+ * "index" and then the driver fills in the rest of the structure for the thing
+ * with that index.  We only need to copy up the first non-input field.  */
+static unsigned long cmd_input_size(unsigned int cmd)
+{
+	/* Size of structure up to and including 'field' */
+#define CMDINSIZE(cmd, type, field)				\
+	case VIDIOC_##cmd:					\
+		return offsetof(struct v4l2_##type, field) +	\
+			sizeof(((struct v4l2_##type *)0)->field);
+
+	switch (cmd) {
+		CMDINSIZE(ENUM_FMT,		fmtdesc,	type);
+		CMDINSIZE(G_FMT,		format,		type);
+		CMDINSIZE(QUERYBUF,		buffer,		type);
+		CMDINSIZE(G_PARM,		streamparm,	type);
+		CMDINSIZE(ENUMSTD,		standard,	index);
+		CMDINSIZE(ENUMINPUT,		input,		index);
+		CMDINSIZE(G_CTRL,		control,	id);
+		CMDINSIZE(G_TUNER,		tuner,		index);
+		CMDINSIZE(QUERYCTRL,		queryctrl,	id);
+		CMDINSIZE(QUERYMENU,		querymenu,	index);
+		CMDINSIZE(ENUMOUTPUT,		output,		index);
+		CMDINSIZE(G_MODULATOR,		modulator,	index);
+		CMDINSIZE(G_FREQUENCY,		frequency,	tuner);
+		CMDINSIZE(CROPCAP,		cropcap,	type);
+		CMDINSIZE(G_CROP,		crop,		type);
+		CMDINSIZE(ENUMAUDIO,		audio,		index);
+		CMDINSIZE(ENUMAUDOUT,		audioout,	index);
+		CMDINSIZE(ENCODER_CMD,		encoder_cmd,	flags);
+		CMDINSIZE(TRY_ENCODER_CMD,	encoder_cmd,	flags);
+		CMDINSIZE(G_SLICED_VBI_CAP,	sliced_vbi_cap,	type);
+		CMDINSIZE(ENUM_FRAMESIZES,	frmsizeenum,	pixel_format);
+		CMDINSIZE(ENUM_FRAMEINTERVALS,	frmivalenum,	height);
+	default:
+		return _IOC_SIZE(cmd);
+	}
+}
+
+static long
+__video_usercopy(struct file *file, unsigned int cmd, unsigned long arg,
 		v4l2_kioctl func)
 {
 	char	sbuf[128];
 	void    *mbuf = NULL;
-	void	*parg = NULL;
+	void	*parg = (void *)arg;
 	long	err  = -EINVAL;
 	int     is_ext_ctrl;
 	size_t  ctrls_size = 0;
 	void __user *user_ptr = NULL;
 
-#ifdef __OLD_VIDIOC_
-	cmd = video_fix_command(cmd);
-#endif
 	is_ext_ctrl = (cmd == VIDIOC_S_EXT_CTRLS || cmd == VIDIOC_G_EXT_CTRLS ||
 		       cmd == VIDIOC_TRY_EXT_CTRLS);
 
 	/*  Copy arguments into temp kernel buffer  */
-	switch (_IOC_DIR(cmd)) {
-	case _IOC_NONE:
-		parg = NULL;
-		break;
-	case _IOC_READ:
-	case _IOC_WRITE:
-	case (_IOC_WRITE | _IOC_READ):
+	if (_IOC_DIR(cmd) != _IOC_NONE) {
 		if (_IOC_SIZE(cmd) <= sizeof(sbuf)) {
 			parg = sbuf;
 		} else {
@@ -413,11 +440,21 @@ video_usercopy(struct file *file, unsigned int cmd, unsigned long arg,
 		}
 
 		err = -EFAULT;
-		if (_IOC_DIR(cmd) & _IOC_WRITE)
-			if (copy_from_user(parg, (void __user *)arg, _IOC_SIZE(cmd)))
+		if (_IOC_DIR(cmd) & _IOC_WRITE) {
+			unsigned long n = cmd_input_size(cmd);
+
+			if (copy_from_user(parg, (void __user *)arg, n))
 				goto out;
-		break;
+
+			/* zero out anything we don't copy from userspace */
+			if (n < _IOC_SIZE(cmd))
+				memset((u8 *)parg + n, 0, _IOC_SIZE(cmd) - n);
+		} else {
+			/* read-only ioctl */
+			memset(parg, 0, _IOC_SIZE(cmd));
+		}
 	}
+
 	if (is_ext_ctrl) {
 		struct v4l2_ext_controls *p = parg;
 
@@ -439,7 +476,7 @@ video_usercopy(struct file *file, unsigned int cmd, unsigned long arg,
 		}
 	}
 
-	/* call driver */
+	/* Handles IOCTL */
 	err = func(file, cmd, parg);
 	if (err == -ENOIOCTLCMD)
 		err = -EINVAL;
@@ -468,6 +505,19 @@ out:
 	kfree(mbuf);
 	return err;
 }
+
+/*
+ * Obsolete usercopy function - Should be removed soon
+ */
+long
+video_usercopy(struct file *file, unsigned int cmd, unsigned long arg,
+		v4l2_kioctl func)
+{
+#ifdef __OLD_VIDIOC_
+	cmd = video_fix_command(cmd);
+#endif
+	return __video_usercopy(file, cmd, arg, func);
+}
 EXPORT_SYMBOL(video_usercopy);
 
 static void dbgbuf(unsigned int cmd, struct video_device *vfd,
@@ -2021,138 +2071,12 @@ static long __video_do_ioctl(struct file *file,
 	return ret;
 }
 
-/* In some cases, only a few fields are used as input, i.e. when the app sets
- * "index" and then the driver fills in the rest of the structure for the thing
- * with that index.  We only need to copy up the first non-input field.  */
-static unsigned long cmd_input_size(unsigned int cmd)
-{
-	/* Size of structure up to and including 'field' */
-#define CMDINSIZE(cmd, type, field) 				\
-	case VIDIOC_##cmd: 					\
-		return offsetof(struct v4l2_##type, field) + 	\
-			sizeof(((struct v4l2_##type *)0)->field);
-
-	switch (cmd) {
-		CMDINSIZE(ENUM_FMT,		fmtdesc,	type);
-		CMDINSIZE(G_FMT,		format,		type);
-		CMDINSIZE(QUERYBUF,		buffer,		type);
-		CMDINSIZE(G_PARM,		streamparm,	type);
-		CMDINSIZE(ENUMSTD,		standard,	index);
-		CMDINSIZE(ENUMINPUT,		input,		index);
-		CMDINSIZE(G_CTRL,		control,	id);
-		CMDINSIZE(G_TUNER,		tuner,		index);
-		CMDINSIZE(QUERYCTRL,		queryctrl,	id);
-		CMDINSIZE(QUERYMENU,		querymenu,	index);
-		CMDINSIZE(ENUMOUTPUT,		output,		index);
-		CMDINSIZE(G_MODULATOR,		modulator,	index);
-		CMDINSIZE(G_FREQUENCY,		frequency,	tuner);
-		CMDINSIZE(CROPCAP,		cropcap,	type);
-		CMDINSIZE(G_CROP,		crop,		type);
-		CMDINSIZE(ENUMAUDIO,		audio, 		index);
-		CMDINSIZE(ENUMAUDOUT,		audioout, 	index);
-		CMDINSIZE(ENCODER_CMD,		encoder_cmd,	flags);
-		CMDINSIZE(TRY_ENCODER_CMD,	encoder_cmd,	flags);
-		CMDINSIZE(G_SLICED_VBI_CAP,	sliced_vbi_cap,	type);
-		CMDINSIZE(ENUM_FRAMESIZES,	frmsizeenum,	pixel_format);
-		CMDINSIZE(ENUM_FRAMEINTERVALS,	frmivalenum,	height);
-	default:
-		return _IOC_SIZE(cmd);
-	}
-}
-
 long video_ioctl2(struct file *file,
 	       unsigned int cmd, unsigned long arg)
 {
-	char	sbuf[128];
-	void    *mbuf = NULL;
-	void	*parg = (void *)arg;
-	long	err  = -EINVAL;
-	int     is_ext_ctrl;
-	size_t  ctrls_size = 0;
-	void __user *user_ptr = NULL;
-
 #ifdef __OLD_VIDIOC_
 	cmd = video_fix_command(cmd);
 #endif
-	is_ext_ctrl = (cmd == VIDIOC_S_EXT_CTRLS || cmd == VIDIOC_G_EXT_CTRLS ||
-		       cmd == VIDIOC_TRY_EXT_CTRLS);
-
-	/*  Copy arguments into temp kernel buffer  */
-	if (_IOC_DIR(cmd) != _IOC_NONE) {
-		if (_IOC_SIZE(cmd) <= sizeof(sbuf)) {
-			parg = sbuf;
-		} else {
-			/* too big to allocate from stack */
-			mbuf = kmalloc(_IOC_SIZE(cmd), GFP_KERNEL);
-			if (NULL == mbuf)
-				return -ENOMEM;
-			parg = mbuf;
-		}
-
-		err = -EFAULT;
-		if (_IOC_DIR(cmd) & _IOC_WRITE) {
-			unsigned long n = cmd_input_size(cmd);
-
-			if (copy_from_user(parg, (void __user *)arg, n))
-				goto out;
-
-			/* zero out anything we don't copy from userspace */
-			if (n < _IOC_SIZE(cmd))
-				memset((u8 *)parg + n, 0, _IOC_SIZE(cmd) - n);
-		} else {
-			/* read-only ioctl */
-			memset(parg, 0, _IOC_SIZE(cmd));
-		}
-	}
-
-	if (is_ext_ctrl) {
-		struct v4l2_ext_controls *p = parg;
-
-		/* In case of an error, tell the caller that it wasn't
-		   a specific control that caused it. */
-		p->error_idx = p->count;
-		user_ptr = (void __user *)p->controls;
-		if (p->count) {
-			ctrls_size = sizeof(struct v4l2_ext_control) * p->count;
-			/* Note: v4l2_ext_controls fits in sbuf[] so mbuf is still NULL. */
-			mbuf = kmalloc(ctrls_size, GFP_KERNEL);
-			err = -ENOMEM;
-			if (NULL == mbuf)
-				goto out_ext_ctrl;
-			err = -EFAULT;
-			if (copy_from_user(mbuf, user_ptr, ctrls_size))
-				goto out_ext_ctrl;
-			p->controls = mbuf;
-		}
-	}
-
-	/* Handles IOCTL */
-	err = __video_do_ioctl(file, cmd, parg);
-	if (err == -ENOIOCTLCMD)
-		err = -EINVAL;
-	if (is_ext_ctrl) {
-		struct v4l2_ext_controls *p = parg;
-
-		p->controls = (void *)user_ptr;
-		if (p->count && err == 0 && copy_to_user(user_ptr, mbuf, ctrls_size))
-			err = -EFAULT;
-		goto out_ext_ctrl;
-	}
-	if (err < 0)
-		goto out;
-
-out_ext_ctrl:
-	/*  Copy results into user buffer  */
-	switch (_IOC_DIR(cmd)) {
-	case _IOC_READ:
-	case (_IOC_WRITE | _IOC_READ):
-		if (copy_to_user((void __user *)arg, parg, _IOC_SIZE(cmd)))
-			err = -EFAULT;
-		break;
-	}
-
-out:
-	kfree(mbuf);
-	return err;
+	return __video_usercopy(file, cmd, arg, __video_do_ioctl);
 }
 EXPORT_SYMBOL(video_ioctl2);
-- 
1.7.1

Re: [RFC/PATCH v3 1/7] v4l: Share code between video_usercopy and video_ioctl2

[Hans Verkuil]

On Monday 12 July 2010 17:25:46 Laurent Pinchart wrote:
> The two functions are mostly identical. They handle the copy_from_user
> and copy_to_user operations related with V4L2 ioctls and call the real
> ioctl handler.
> 
> Create a __video_usercopy function that implements the core of
> video_usercopy and video_ioctl2, and call that function from both.

Acked-by: Hans Verkuil <hverkuil@xs4all.nl>

Two notes:

1) This change will clash with the multiplane patches.
2) Perhaps it is time that we remove the __OLD_VIDIOC_ support?

Regards,

	Hans

> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
>  drivers/media/video/v4l2-ioctl.c |  218 ++++++++++++-------------------------
>  1 files changed, 71 insertions(+), 147 deletions(-)
> 
> diff --git a/drivers/media/video/v4l2-ioctl.c b/drivers/media/video/v4l2-ioctl.c
> index 0eeceae..486eaba 100644
> --- a/drivers/media/video/v4l2-ioctl.c
> +++ b/drivers/media/video/v4l2-ioctl.c
> @@ -373,35 +373,62 @@ video_fix_command(unsigned int cmd)
>  }
>  #endif
>  
> -/*
> - * Obsolete usercopy function - Should be removed soon
> - */
> -long
> -video_usercopy(struct file *file, unsigned int cmd, unsigned long arg,
> +/* In some cases, only a few fields are used as input, i.e. when the app sets
> + * "index" and then the driver fills in the rest of the structure for the thing
> + * with that index.  We only need to copy up the first non-input field.  */
> +static unsigned long cmd_input_size(unsigned int cmd)
> +{
> +	/* Size of structure up to and including 'field' */
> +#define CMDINSIZE(cmd, type, field)				\
> +	case VIDIOC_##cmd:					\
> +		return offsetof(struct v4l2_##type, field) +	\
> +			sizeof(((struct v4l2_##type *)0)->field);
> +
> +	switch (cmd) {
> +		CMDINSIZE(ENUM_FMT,		fmtdesc,	type);
> +		CMDINSIZE(G_FMT,		format,		type);
> +		CMDINSIZE(QUERYBUF,		buffer,		type);
> +		CMDINSIZE(G_PARM,		streamparm,	type);
> +		CMDINSIZE(ENUMSTD,		standard,	index);
> +		CMDINSIZE(ENUMINPUT,		input,		index);
> +		CMDINSIZE(G_CTRL,		control,	id);
> +		CMDINSIZE(G_TUNER,		tuner,		index);
> +		CMDINSIZE(QUERYCTRL,		queryctrl,	id);
> +		CMDINSIZE(QUERYMENU,		querymenu,	index);
> +		CMDINSIZE(ENUMOUTPUT,		output,		index);
> +		CMDINSIZE(G_MODULATOR,		modulator,	index);
> +		CMDINSIZE(G_FREQUENCY,		frequency,	tuner);
> +		CMDINSIZE(CROPCAP,		cropcap,	type);
> +		CMDINSIZE(G_CROP,		crop,		type);
> +		CMDINSIZE(ENUMAUDIO,		audio,		index);
> +		CMDINSIZE(ENUMAUDOUT,		audioout,	index);
> +		CMDINSIZE(ENCODER_CMD,		encoder_cmd,	flags);
> +		CMDINSIZE(TRY_ENCODER_CMD,	encoder_cmd,	flags);
> +		CMDINSIZE(G_SLICED_VBI_CAP,	sliced_vbi_cap,	type);
> +		CMDINSIZE(ENUM_FRAMESIZES,	frmsizeenum,	pixel_format);
> +		CMDINSIZE(ENUM_FRAMEINTERVALS,	frmivalenum,	height);
> +	default:
> +		return _IOC_SIZE(cmd);
> +	}
> +}
> +
> +static long
> +__video_usercopy(struct file *file, unsigned int cmd, unsigned long arg,
>  		v4l2_kioctl func)
>  {
>  	char	sbuf[128];
>  	void    *mbuf = NULL;
> -	void	*parg = NULL;
> +	void	*parg = (void *)arg;
>  	long	err  = -EINVAL;
>  	int     is_ext_ctrl;
>  	size_t  ctrls_size = 0;
>  	void __user *user_ptr = NULL;
>  
> -#ifdef __OLD_VIDIOC_
> -	cmd = video_fix_command(cmd);
> -#endif
>  	is_ext_ctrl = (cmd == VIDIOC_S_EXT_CTRLS || cmd == VIDIOC_G_EXT_CTRLS ||
>  		       cmd == VIDIOC_TRY_EXT_CTRLS);
>  
>  	/*  Copy arguments into temp kernel buffer  */
> -	switch (_IOC_DIR(cmd)) {
> -	case _IOC_NONE:
> -		parg = NULL;
> -		break;
> -	case _IOC_READ:
> -	case _IOC_WRITE:
> -	case (_IOC_WRITE | _IOC_READ):
> +	if (_IOC_DIR(cmd) != _IOC_NONE) {
>  		if (_IOC_SIZE(cmd) <= sizeof(sbuf)) {
>  			parg = sbuf;
>  		} else {
> @@ -413,11 +440,21 @@ video_usercopy(struct file *file, unsigned int cmd, unsigned long arg,
>  		}
>  
>  		err = -EFAULT;
> -		if (_IOC_DIR(cmd) & _IOC_WRITE)
> -			if (copy_from_user(parg, (void __user *)arg, _IOC_SIZE(cmd)))
> +		if (_IOC_DIR(cmd) & _IOC_WRITE) {
> +			unsigned long n = cmd_input_size(cmd);
> +
> +			if (copy_from_user(parg, (void __user *)arg, n))
>  				goto out;
> -		break;
> +
> +			/* zero out anything we don't copy from userspace */
> +			if (n < _IOC_SIZE(cmd))
> +				memset((u8 *)parg + n, 0, _IOC_SIZE(cmd) - n);
> +		} else {
> +			/* read-only ioctl */
> +			memset(parg, 0, _IOC_SIZE(cmd));
> +		}
>  	}
> +
>  	if (is_ext_ctrl) {
>  		struct v4l2_ext_controls *p = parg;
>  
> @@ -439,7 +476,7 @@ video_usercopy(struct file *file, unsigned int cmd, unsigned long arg,
>  		}
>  	}
>  
> -	/* call driver */
> +	/* Handles IOCTL */
>  	err = func(file, cmd, parg);
>  	if (err == -ENOIOCTLCMD)
>  		err = -EINVAL;
> @@ -468,6 +505,19 @@ out:
>  	kfree(mbuf);
>  	return err;
>  }
> +
> +/*
> + * Obsolete usercopy function - Should be removed soon
> + */
> +long
> +video_usercopy(struct file *file, unsigned int cmd, unsigned long arg,
> +		v4l2_kioctl func)
> +{
> +#ifdef __OLD_VIDIOC_
> +	cmd = video_fix_command(cmd);
> +#endif
> +	return __video_usercopy(file, cmd, arg, func);
> +}
>  EXPORT_SYMBOL(video_usercopy);
>  
>  static void dbgbuf(unsigned int cmd, struct video_device *vfd,
> @@ -2021,138 +2071,12 @@ static long __video_do_ioctl(struct file *file,
>  	return ret;
>  }
>  
> -/* In some cases, only a few fields are used as input, i.e. when the app sets
> - * "index" and then the driver fills in the rest of the structure for the thing
> - * with that index.  We only need to copy up the first non-input field.  */
> -static unsigned long cmd_input_size(unsigned int cmd)
> -{
> -	/* Size of structure up to and including 'field' */
> -#define CMDINSIZE(cmd, type, field) 				\
> -	case VIDIOC_##cmd: 					\
> -		return offsetof(struct v4l2_##type, field) + 	\
> -			sizeof(((struct v4l2_##type *)0)->field);
> -
> -	switch (cmd) {
> -		CMDINSIZE(ENUM_FMT,		fmtdesc,	type);
> -		CMDINSIZE(G_FMT,		format,		type);
> -		CMDINSIZE(QUERYBUF,		buffer,		type);
> -		CMDINSIZE(G_PARM,		streamparm,	type);
> -		CMDINSIZE(ENUMSTD,		standard,	index);
> -		CMDINSIZE(ENUMINPUT,		input,		index);
> -		CMDINSIZE(G_CTRL,		control,	id);
> -		CMDINSIZE(G_TUNER,		tuner,		index);
> -		CMDINSIZE(QUERYCTRL,		queryctrl,	id);
> -		CMDINSIZE(QUERYMENU,		querymenu,	index);
> -		CMDINSIZE(ENUMOUTPUT,		output,		index);
> -		CMDINSIZE(G_MODULATOR,		modulator,	index);
> -		CMDINSIZE(G_FREQUENCY,		frequency,	tuner);
> -		CMDINSIZE(CROPCAP,		cropcap,	type);
> -		CMDINSIZE(G_CROP,		crop,		type);
> -		CMDINSIZE(ENUMAUDIO,		audio, 		index);
> -		CMDINSIZE(ENUMAUDOUT,		audioout, 	index);
> -		CMDINSIZE(ENCODER_CMD,		encoder_cmd,	flags);
> -		CMDINSIZE(TRY_ENCODER_CMD,	encoder_cmd,	flags);
> -		CMDINSIZE(G_SLICED_VBI_CAP,	sliced_vbi_cap,	type);
> -		CMDINSIZE(ENUM_FRAMESIZES,	frmsizeenum,	pixel_format);
> -		CMDINSIZE(ENUM_FRAMEINTERVALS,	frmivalenum,	height);
> -	default:
> -		return _IOC_SIZE(cmd);
> -	}
> -}
> -
>  long video_ioctl2(struct file *file,
>  	       unsigned int cmd, unsigned long arg)
>  {
> -	char	sbuf[128];
> -	void    *mbuf = NULL;
> -	void	*parg = (void *)arg;
> -	long	err  = -EINVAL;
> -	int     is_ext_ctrl;
> -	size_t  ctrls_size = 0;
> -	void __user *user_ptr = NULL;
> -
>  #ifdef __OLD_VIDIOC_
>  	cmd = video_fix_command(cmd);
>  #endif
> -	is_ext_ctrl = (cmd == VIDIOC_S_EXT_CTRLS || cmd == VIDIOC_G_EXT_CTRLS ||
> -		       cmd == VIDIOC_TRY_EXT_CTRLS);
> -
> -	/*  Copy arguments into temp kernel buffer  */
> -	if (_IOC_DIR(cmd) != _IOC_NONE) {
> -		if (_IOC_SIZE(cmd) <= sizeof(sbuf)) {
> -			parg = sbuf;
> -		} else {
> -			/* too big to allocate from stack */
> -			mbuf = kmalloc(_IOC_SIZE(cmd), GFP_KERNEL);
> -			if (NULL == mbuf)
> -				return -ENOMEM;
> -			parg = mbuf;
> -		}
> -
> -		err = -EFAULT;
> -		if (_IOC_DIR(cmd) & _IOC_WRITE) {
> -			unsigned long n = cmd_input_size(cmd);
> -
> -			if (copy_from_user(parg, (void __user *)arg, n))
> -				goto out;
> -
> -			/* zero out anything we don't copy from userspace */
> -			if (n < _IOC_SIZE(cmd))
> -				memset((u8 *)parg + n, 0, _IOC_SIZE(cmd) - n);
> -		} else {
> -			/* read-only ioctl */
> -			memset(parg, 0, _IOC_SIZE(cmd));
> -		}
> -	}
> -
> -	if (is_ext_ctrl) {
> -		struct v4l2_ext_controls *p = parg;
> -
> -		/* In case of an error, tell the caller that it wasn't
> -		   a specific control that caused it. */
> -		p->error_idx = p->count;
> -		user_ptr = (void __user *)p->controls;
> -		if (p->count) {
> -			ctrls_size = sizeof(struct v4l2_ext_control) * p->count;
> -			/* Note: v4l2_ext_controls fits in sbuf[] so mbuf is still NULL. */
> -			mbuf = kmalloc(ctrls_size, GFP_KERNEL);
> -			err = -ENOMEM;
> -			if (NULL == mbuf)
> -				goto out_ext_ctrl;
> -			err = -EFAULT;
> -			if (copy_from_user(mbuf, user_ptr, ctrls_size))
> -				goto out_ext_ctrl;
> -			p->controls = mbuf;
> -		}
> -	}
> -
> -	/* Handles IOCTL */
> -	err = __video_do_ioctl(file, cmd, parg);
> -	if (err == -ENOIOCTLCMD)
> -		err = -EINVAL;
> -	if (is_ext_ctrl) {
> -		struct v4l2_ext_controls *p = parg;
> -
> -		p->controls = (void *)user_ptr;
> -		if (p->count && err == 0 && copy_to_user(user_ptr, mbuf, ctrls_size))
> -			err = -EFAULT;
> -		goto out_ext_ctrl;
> -	}
> -	if (err < 0)
> -		goto out;
> -
> -out_ext_ctrl:
> -	/*  Copy results into user buffer  */
> -	switch (_IOC_DIR(cmd)) {
> -	case _IOC_READ:
> -	case (_IOC_WRITE | _IOC_READ):
> -		if (copy_to_user((void __user *)arg, parg, _IOC_SIZE(cmd)))
> -			err = -EFAULT;
> -		break;
> -	}
> -
> -out:
> -	kfree(mbuf);
> -	return err;
> +	return __video_usercopy(file, cmd, arg, __video_do_ioctl);
>  }
>  EXPORT_SYMBOL(video_ioctl2);
> 

-- 
Hans Verkuil - video4linux developer - sponsored by TANDBERG, part of Cisco

Re: [RFC/PATCH v3 1/7] v4l: Share code between video_usercopy and video_ioctl2

[Laurent Pinchart]

Hi Hans,

On Wednesday 04 August 2010 20:30:06 Hans Verkuil wrote:
> On Monday 12 July 2010 17:25:46 Laurent Pinchart wrote:
> > The two functions are mostly identical. They handle the copy_from_user
> > and copy_to_user operations related with V4L2 ioctls and call the real
> > ioctl handler.
> > 
> > Create a __video_usercopy function that implements the core of
> > video_usercopy and video_ioctl2, and call that function from both.
> 
> Acked-by: Hans Verkuil <hverkuil@xs4all.nl>

Thanks for the acks.

> Two notes:
> 
> 1) This change will clash with the multiplane patches.

Obviously, for this patch and the others, I will keep rebasing the code until 
it gets merged.

> 2) Perhaps it is time that we remove the __OLD_VIDIOC_ support?

Fine with me. We need to list that in feature-removal-schedule.txt.

-- 
Regards,

Laurent Pinchart

[RFC/PATCH v3 2/7] v4l: subdev: Don't require core operations

[Laurent Pinchart]

There's no reason to require subdevices to implement the core
operations. Remove the check for non-NULL core operations when
initializing the subdev.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 include/media/v4l2-subdev.h |    3 +--
 1 files changed, 1 insertions(+), 2 deletions(-)

diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h
index 02c6f4d..6088316 100644
--- a/include/media/v4l2-subdev.h
+++ b/include/media/v4l2-subdev.h
@@ -437,8 +437,7 @@ static inline void v4l2_subdev_init(struct v4l2_subdev *sd,
 					const struct v4l2_subdev_ops *ops)
 {
 	INIT_LIST_HEAD(&sd->list);
-	/* ops->core MUST be set */
-	BUG_ON(!ops || !ops->core);
+	BUG_ON(!ops);
 	sd->ops = ops;
 	sd->v4l2_dev = NULL;
 	sd->flags = 0;
-- 
1.7.1

Re: [RFC/PATCH v3 2/7] v4l: subdev: Don't require core operations

[Hans Verkuil]

On Monday 12 July 2010 17:25:47 Laurent Pinchart wrote:
> There's no reason to require subdevices to implement the core
> operations. Remove the check for non-NULL core operations when
> initializing the subdev.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

Acked-by: Hans Verkuil <hverkuil@xs4all.nl>

> ---
>  include/media/v4l2-subdev.h |    3 +--
>  1 files changed, 1 insertions(+), 2 deletions(-)
> 
> diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h
> index 02c6f4d..6088316 100644
> --- a/include/media/v4l2-subdev.h
> +++ b/include/media/v4l2-subdev.h
> @@ -437,8 +437,7 @@ static inline void v4l2_subdev_init(struct v4l2_subdev *sd,
>  					const struct v4l2_subdev_ops *ops)
>  {
>  	INIT_LIST_HEAD(&sd->list);
> -	/* ops->core MUST be set */
> -	BUG_ON(!ops || !ops->core);
> +	BUG_ON(!ops);
>  	sd->ops = ops;
>  	sd->v4l2_dev = NULL;
>  	sd->flags = 0;
> 

-- 
Hans Verkuil - video4linux developer - sponsored by TANDBERG, part of Cisco

[RFC/PATCH v3 3/7] v4l: subdev: Merge v4l2_i2c_new_subdev_cfg and v4l2_i2c_new_subdev

[Laurent Pinchart]

v4l2_i2c_new_subdev_cfg is called by v4l2_i2c_new_subdev only. Merge the
two functions into v4l2_i2c_new_subdev.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/media/video/v4l2-common.c |    7 ++-----
 include/media/v4l2-common.h       |   15 +--------------
 2 files changed, 3 insertions(+), 19 deletions(-)

diff --git a/drivers/media/video/v4l2-common.c b/drivers/media/video/v4l2-common.c
index 4e53b0b..bbda421 100644
--- a/drivers/media/video/v4l2-common.c
+++ b/drivers/media/video/v4l2-common.c
@@ -897,10 +897,9 @@ error:
 }
 EXPORT_SYMBOL_GPL(v4l2_i2c_new_subdev_board);
 
-struct v4l2_subdev *v4l2_i2c_new_subdev_cfg(struct v4l2_device *v4l2_dev,
+struct v4l2_subdev *v4l2_i2c_new_subdev(struct v4l2_device *v4l2_dev,
 		struct i2c_adapter *adapter,
 		const char *module_name, const char *client_type,
-		int irq, void *platform_data,
 		u8 addr, const unsigned short *probe_addrs)
 {
 	struct i2c_board_info info;
@@ -910,13 +909,11 @@ struct v4l2_subdev *v4l2_i2c_new_subdev_cfg(struct v4l2_device *v4l2_dev,
 	memset(&info, 0, sizeof(info));
 	strlcpy(info.type, client_type, sizeof(info.type));
 	info.addr = addr;
-	info.irq = irq;
-	info.platform_data = platform_data;
 
 	return v4l2_i2c_new_subdev_board(v4l2_dev, adapter, module_name,
 			&info, probe_addrs);
 }
-EXPORT_SYMBOL_GPL(v4l2_i2c_new_subdev_cfg);
+EXPORT_SYMBOL_GPL(v4l2_i2c_new_subdev);
 
 /* Return i2c client address of v4l2_subdev. */
 unsigned short v4l2_i2c_subdev_addr(struct v4l2_subdev *sd)
diff --git a/include/media/v4l2-common.h b/include/media/v4l2-common.h
index 98b3264..6fc3d7a 100644
--- a/include/media/v4l2-common.h
+++ b/include/media/v4l2-common.h
@@ -139,24 +139,11 @@ struct v4l2_subdev_ops;
 /* Load an i2c module and return an initialized v4l2_subdev struct.
    Only call request_module if module_name != NULL.
    The client_type argument is the name of the chip that's on the adapter. */
-struct v4l2_subdev *v4l2_i2c_new_subdev_cfg(struct v4l2_device *v4l2_dev,
+struct v4l2_subdev *v4l2_i2c_new_subdev(struct v4l2_device *v4l2_dev,
 		struct i2c_adapter *adapter,
 		const char *module_name, const char *client_type,
-		int irq, void *platform_data,
 		u8 addr, const unsigned short *probe_addrs);
 
-/* Load an i2c module and return an initialized v4l2_subdev struct.
-   Only call request_module if module_name != NULL.
-   The client_type argument is the name of the chip that's on the adapter. */
-static inline struct v4l2_subdev *v4l2_i2c_new_subdev(struct v4l2_device *v4l2_dev,
-		struct i2c_adapter *adapter,
-		const char *module_name, const char *client_type,
-		u8 addr, const unsigned short *probe_addrs)
-{
-	return v4l2_i2c_new_subdev_cfg(v4l2_dev, adapter, module_name,
-				client_type, 0, NULL, addr, probe_addrs);
-}
-
 struct i2c_board_info;
 
 struct v4l2_subdev *v4l2_i2c_new_subdev_board(struct v4l2_device *v4l2_dev,
-- 
1.7.1

Re: [RFC/PATCH v3 3/7] v4l: subdev: Merge v4l2_i2c_new_subdev_cfg and v4l2_i2c_new_subdev

[Hans Verkuil]

On Monday 12 July 2010 17:25:48 Laurent Pinchart wrote:
> v4l2_i2c_new_subdev_cfg is called by v4l2_i2c_new_subdev only. Merge the
> two functions into v4l2_i2c_new_subdev.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

Acked-by: Hans Verkuil <hverkuil@xs4all.nl>

> ---
>  drivers/media/video/v4l2-common.c |    7 ++-----
>  include/media/v4l2-common.h       |   15 +--------------
>  2 files changed, 3 insertions(+), 19 deletions(-)
> 
> diff --git a/drivers/media/video/v4l2-common.c b/drivers/media/video/v4l2-common.c
> index 4e53b0b..bbda421 100644
> --- a/drivers/media/video/v4l2-common.c
> +++ b/drivers/media/video/v4l2-common.c
> @@ -897,10 +897,9 @@ error:
>  }
>  EXPORT_SYMBOL_GPL(v4l2_i2c_new_subdev_board);
>  
> -struct v4l2_subdev *v4l2_i2c_new_subdev_cfg(struct v4l2_device *v4l2_dev,
> +struct v4l2_subdev *v4l2_i2c_new_subdev(struct v4l2_device *v4l2_dev,
>  		struct i2c_adapter *adapter,
>  		const char *module_name, const char *client_type,
> -		int irq, void *platform_data,
>  		u8 addr, const unsigned short *probe_addrs)
>  {
>  	struct i2c_board_info info;
> @@ -910,13 +909,11 @@ struct v4l2_subdev *v4l2_i2c_new_subdev_cfg(struct v4l2_device *v4l2_dev,
>  	memset(&info, 0, sizeof(info));
>  	strlcpy(info.type, client_type, sizeof(info.type));
>  	info.addr = addr;
> -	info.irq = irq;
> -	info.platform_data = platform_data;
>  
>  	return v4l2_i2c_new_subdev_board(v4l2_dev, adapter, module_name,
>  			&info, probe_addrs);
>  }
> -EXPORT_SYMBOL_GPL(v4l2_i2c_new_subdev_cfg);
> +EXPORT_SYMBOL_GPL(v4l2_i2c_new_subdev);
>  
>  /* Return i2c client address of v4l2_subdev. */
>  unsigned short v4l2_i2c_subdev_addr(struct v4l2_subdev *sd)
> diff --git a/include/media/v4l2-common.h b/include/media/v4l2-common.h
> index 98b3264..6fc3d7a 100644
> --- a/include/media/v4l2-common.h
> +++ b/include/media/v4l2-common.h
> @@ -139,24 +139,11 @@ struct v4l2_subdev_ops;
>  /* Load an i2c module and return an initialized v4l2_subdev struct.
>     Only call request_module if module_name != NULL.
>     The client_type argument is the name of the chip that's on the adapter. */
> -struct v4l2_subdev *v4l2_i2c_new_subdev_cfg(struct v4l2_device *v4l2_dev,
> +struct v4l2_subdev *v4l2_i2c_new_subdev(struct v4l2_device *v4l2_dev,
>  		struct i2c_adapter *adapter,
>  		const char *module_name, const char *client_type,
> -		int irq, void *platform_data,
>  		u8 addr, const unsigned short *probe_addrs);
>  
> -/* Load an i2c module and return an initialized v4l2_subdev struct.
> -   Only call request_module if module_name != NULL.
> -   The client_type argument is the name of the chip that's on the adapter. */
> -static inline struct v4l2_subdev *v4l2_i2c_new_subdev(struct v4l2_device *v4l2_dev,
> -		struct i2c_adapter *adapter,
> -		const char *module_name, const char *client_type,
> -		u8 addr, const unsigned short *probe_addrs)
> -{
> -	return v4l2_i2c_new_subdev_cfg(v4l2_dev, adapter, module_name,
> -				client_type, 0, NULL, addr, probe_addrs);
> -}
> -
>  struct i2c_board_info;
>  
>  struct v4l2_subdev *v4l2_i2c_new_subdev_board(struct v4l2_device *v4l2_dev,
> 

-- 
Hans Verkuil - video4linux developer - sponsored by TANDBERG, part of Cisco

[RFC/PATCH v3 4/7] v4l: subdev: Add device node support

[Laurent Pinchart]

Create a device node named subdevX for every registered subdev.

As the device node is registered before the subdev core::s_config
function is called, return -EGAIN on open until initialization
completes.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Vimarsh Zutshi <vimarsh.zutshi@nokia.com>
---
 Documentation/video4linux/v4l2-framework.txt |   18 +++++++
 drivers/media/radio/radio-si4713.c           |    2 +-
 drivers/media/video/Makefile                 |    2 +-
 drivers/media/video/soc_camera.c             |    2 +-
 drivers/media/video/v4l2-common.c            |   15 +++++-
 drivers/media/video/v4l2-dev.c               |   27 ++++------
 drivers/media/video/v4l2-device.c            |   25 +++++++++-
 drivers/media/video/v4l2-ioctl.c             |    2 +-
 drivers/media/video/v4l2-subdev.c            |   65 ++++++++++++++++++++++++++
 include/media/v4l2-common.h                  |    6 ++-
 include/media/v4l2-dev.h                     |   18 ++++++-
 include/media/v4l2-ioctl.h                   |    3 +
 include/media/v4l2-subdev.h                  |   16 ++++++-
 13 files changed, 170 insertions(+), 31 deletions(-)
 create mode 100644 drivers/media/video/v4l2-subdev.c

diff --git a/Documentation/video4linux/v4l2-framework.txt b/Documentation/video4linux/v4l2-framework.txt
index e831aac..164bb0f 100644
--- a/Documentation/video4linux/v4l2-framework.txt
+++ b/Documentation/video4linux/v4l2-framework.txt
@@ -314,6 +314,24 @@ controlled through GPIO pins. This distinction is only relevant when setting
 up the device, but once the subdev is registered it is completely transparent.
 
 
+V4L2 sub-device userspace API
+-----------------------------
+
+Beside exposing a kernel API through the v4l2_subdev_ops structure, V4L2
+sub-devices can also be controlled directly by userspace applications.
+
+When a sub-device is registered, a device node named v4l-subdevX can be created
+in /dev. If the sub-device supports direct userspace configuration it must set
+the V4L2_SUBDEV_FL_HAS_DEVNODE flag before being registered.
+
+For I2C and SPI sub-devices, the v4l2_device driver can disable registration of
+the device node if it wants to control the sub-device on its own. In that case
+it must set the v4l2_i2c_new_subdev_board or v4l2_spi_new_subdev enable_devnode
+argument to 0. Setting the argument to 1 will only enable device node
+registration if the sub-device driver has set the V4L2_SUBDEV_FL_HAS_DEVNODE
+flag.
+
+
 I2C sub-device drivers
 ----------------------
 
diff --git a/drivers/media/radio/radio-si4713.c b/drivers/media/radio/radio-si4713.c
index 13554ab..58dd199 100644
--- a/drivers/media/radio/radio-si4713.c
+++ b/drivers/media/radio/radio-si4713.c
@@ -292,7 +292,7 @@ static int radio_si4713_pdriver_probe(struct platform_device *pdev)
 	}
 
 	sd = v4l2_i2c_new_subdev_board(&rsdev->v4l2_dev, adapter, "si4713_i2c",
-					pdata->subdev_board_info, NULL);
+					pdata->subdev_board_info, NULL, 0);
 	if (!sd) {
 		dev_err(&pdev->dev, "Cannot get v4l2 subdevice\n");
 		rval = -ENODEV;
diff --git a/drivers/media/video/Makefile b/drivers/media/video/Makefile
index cc93859..c9c07e5 100644
--- a/drivers/media/video/Makefile
+++ b/drivers/media/video/Makefile
@@ -11,7 +11,7 @@ stkwebcam-objs	:=	stk-webcam.o stk-sensor.o
 omap2cam-objs	:=	omap24xxcam.o omap24xxcam-dma.o
 
 videodev-objs	:=	v4l2-dev.o v4l2-ioctl.o v4l2-device.o v4l2-fh.o \
-			v4l2-event.o
+			v4l2-event.o v4l2-subdev.o
 
 # V4L2 core modules
 
diff --git a/drivers/media/video/soc_camera.c b/drivers/media/video/soc_camera.c
index 475757b..10fae27 100644
--- a/drivers/media/video/soc_camera.c
+++ b/drivers/media/video/soc_camera.c
@@ -895,7 +895,7 @@ static int soc_camera_init_i2c(struct soc_camera_device *icd,
 	icl->board_info->platform_data = icd;
 
 	subdev = v4l2_i2c_new_subdev_board(&ici->v4l2_dev, adap,
-				icl->module_name, icl->board_info, NULL);
+				icl->module_name, icl->board_info, NULL, 0);
 	if (!subdev)
 		goto ei2cnd;
 
diff --git a/drivers/media/video/v4l2-common.c b/drivers/media/video/v4l2-common.c
index bbda421..4265562 100644
--- a/drivers/media/video/v4l2-common.c
+++ b/drivers/media/video/v4l2-common.c
@@ -838,7 +838,8 @@ EXPORT_SYMBOL_GPL(v4l2_i2c_subdev_init);
 /* Load an i2c sub-device. */
 struct v4l2_subdev *v4l2_i2c_new_subdev_board(struct v4l2_device *v4l2_dev,
 		struct i2c_adapter *adapter, const char *module_name,
-		struct i2c_board_info *info, const unsigned short *probe_addrs)
+		struct i2c_board_info *info, const unsigned short *probe_addrs,
+		int enable_devnode)
 {
 	struct v4l2_subdev *sd = NULL;
 	struct i2c_client *client;
@@ -868,9 +869,12 @@ struct v4l2_subdev *v4l2_i2c_new_subdev_board(struct v4l2_device *v4l2_dev,
 	if (!try_module_get(client->driver->driver.owner))
 		goto error;
 	sd = i2c_get_clientdata(client);
+	if (!enable_devnode)
+		sd->flags &= ~V4L2_SUBDEV_FL_HAS_DEVNODE;
 
 	/* Register with the v4l2_device which increases the module's
 	   use count as well. */
+	sd->initialized = 0;
 	if (v4l2_device_register_subdev(v4l2_dev, sd))
 		sd = NULL;
 	/* Decrease the module use count to match the first try_module_get. */
@@ -885,6 +889,8 @@ struct v4l2_subdev *v4l2_i2c_new_subdev_board(struct v4l2_device *v4l2_dev,
 		if (err && err != -ENOIOCTLCMD) {
 			v4l2_device_unregister_subdev(sd);
 			sd = NULL;
+		} else {
+			sd->initialized = 1;
 		}
 	}
 
@@ -911,7 +917,7 @@ struct v4l2_subdev *v4l2_i2c_new_subdev(struct v4l2_device *v4l2_dev,
 	info.addr = addr;
 
 	return v4l2_i2c_new_subdev_board(v4l2_dev, adapter, module_name,
-			&info, probe_addrs);
+			&info, probe_addrs, 0);
 }
 EXPORT_SYMBOL_GPL(v4l2_i2c_new_subdev);
 
@@ -981,7 +987,8 @@ void v4l2_spi_subdev_init(struct v4l2_subdev *sd, struct spi_device *spi,
 EXPORT_SYMBOL_GPL(v4l2_spi_subdev_init);
 
 struct v4l2_subdev *v4l2_spi_new_subdev(struct v4l2_device *v4l2_dev,
-		struct spi_master *master, struct spi_board_info *info)
+		struct spi_master *master, struct spi_board_info *info,
+		int enable_devnode)
 {
 	struct v4l2_subdev *sd = NULL;
 	struct spi_device *spi = NULL;
@@ -1000,6 +1007,8 @@ struct v4l2_subdev *v4l2_spi_new_subdev(struct v4l2_device *v4l2_dev,
 		goto error;
 
 	sd = spi_get_drvdata(spi);
+	if (!enable_devnode)
+		sd->flags &= ~V4L2_SUBDEV_FL_HAS_DEVNODE;
 
 	/* Register with the v4l2_device which increases the module's
 	   use count as well. */
diff --git a/drivers/media/video/v4l2-dev.c b/drivers/media/video/v4l2-dev.c
index 0ca7ec9..bcd47a0 100644
--- a/drivers/media/video/v4l2-dev.c
+++ b/drivers/media/video/v4l2-dev.c
@@ -376,13 +376,14 @@ static int get_index(struct video_device *vdev)
 }
 
 /**
- *	video_register_device - register video4linux devices
+ *	__video_register_device - register video4linux devices
  *	@vdev: video device structure we want to register
  *	@type: type of device to register
  *	@nr:   which device node number (0 == /dev/video0, 1 == /dev/video1, ...
  *             -1 == first free)
  *	@warn_if_nr_in_use: warn if the desired device node number
  *	       was already in use and another number was chosen instead.
+ *	@owner: module that owns the video device node
  *
  *	The registration code assigns minor numbers and device node numbers
  *	based on the requested type and registers the new device node with
@@ -401,9 +402,11 @@ static int get_index(struct video_device *vdev)
  *	%VFL_TYPE_VBI - Vertical blank data (undecoded)
  *
  *	%VFL_TYPE_RADIO - A radio card
+ *
+ *	%VFL_TYPE_SUBDEV - A subdevice
  */
-static int __video_register_device(struct video_device *vdev, int type, int nr,
-		int warn_if_nr_in_use)
+int __video_register_device(struct video_device *vdev, int type, int nr,
+		int warn_if_nr_in_use, struct module *owner)
 {
 	int i = 0;
 	int ret;
@@ -439,6 +442,9 @@ static int __video_register_device(struct video_device *vdev, int type, int nr,
 	case VFL_TYPE_RADIO:
 		name_base = "radio";
 		break;
+	case VFL_TYPE_SUBDEV:
+		name_base = "v4l-subdev";
+		break;
 	default:
 		printk(KERN_ERR "%s called with unknown type: %d\n",
 		       __func__, type);
@@ -525,7 +531,7 @@ static int __video_register_device(struct video_device *vdev, int type, int nr,
 		vdev->cdev->ops = &v4l2_unlocked_fops;
 	else
 		vdev->cdev->ops = &v4l2_fops;
-	vdev->cdev->owner = vdev->fops->owner;
+	vdev->cdev->owner = owner;
 	ret = cdev_add(vdev->cdev, MKDEV(VIDEO_MAJOR, vdev->minor), 1);
 	if (ret < 0) {
 		printk(KERN_ERR "%s: cdev_add failed\n", __func__);
@@ -574,18 +580,7 @@ cleanup:
 	vdev->minor = -1;
 	return ret;
 }
-
-int video_register_device(struct video_device *vdev, int type, int nr)
-{
-	return __video_register_device(vdev, type, nr, 1);
-}
-EXPORT_SYMBOL(video_register_device);
-
-int video_register_device_no_warn(struct video_device *vdev, int type, int nr)
-{
-	return __video_register_device(vdev, type, nr, 0);
-}
-EXPORT_SYMBOL(video_register_device_no_warn);
+EXPORT_SYMBOL(__video_register_device);
 
 /**
  *	video_unregister_device - unregister a video4linux device
diff --git a/drivers/media/video/v4l2-device.c b/drivers/media/video/v4l2-device.c
index 5a7dc4a..b287aaa 100644
--- a/drivers/media/video/v4l2-device.c
+++ b/drivers/media/video/v4l2-device.c
@@ -115,18 +115,38 @@ EXPORT_SYMBOL_GPL(v4l2_device_unregister);
 int v4l2_device_register_subdev(struct v4l2_device *v4l2_dev,
 						struct v4l2_subdev *sd)
 {
+	struct video_device *vdev;
+	int ret = 0;
+
 	/* Check for valid input */
 	if (v4l2_dev == NULL || sd == NULL || !sd->name[0])
 		return -EINVAL;
+
 	/* Warn if we apparently re-register a subdev */
 	WARN_ON(sd->v4l2_dev != NULL);
+
 	if (!try_module_get(sd->owner))
 		return -ENODEV;
+
 	sd->v4l2_dev = v4l2_dev;
 	spin_lock(&v4l2_dev->lock);
 	list_add_tail(&sd->list, &v4l2_dev->subdevs);
 	spin_unlock(&v4l2_dev->lock);
-	return 0;
+
+	/* Register the device node. */
+	vdev = &sd->devnode;
+	strlcpy(vdev->name, sd->name, sizeof(vdev->name));
+	vdev->parent = v4l2_dev->dev;
+	vdev->fops = &v4l2_subdev_fops;
+	vdev->release = video_device_release_empty;
+	if (sd->flags & V4L2_SUBDEV_FL_HAS_DEVNODE) {
+		ret = __video_register_device(vdev, VFL_TYPE_SUBDEV, -1, 1,
+					      sd->owner);
+		if (ret < 0)
+			v4l2_device_unregister_subdev(sd);
+	}
+
+	return ret;
 }
 EXPORT_SYMBOL_GPL(v4l2_device_register_subdev);
 
@@ -135,10 +155,13 @@ void v4l2_device_unregister_subdev(struct v4l2_subdev *sd)
 	/* return if it isn't registered */
 	if (sd == NULL || sd->v4l2_dev == NULL)
 		return;
+
 	spin_lock(&sd->v4l2_dev->lock);
 	list_del(&sd->list);
 	spin_unlock(&sd->v4l2_dev->lock);
 	sd->v4l2_dev = NULL;
+
 	module_put(sd->owner);
+	video_unregister_device(&sd->devnode);
 }
 EXPORT_SYMBOL_GPL(v4l2_device_unregister_subdev);
diff --git a/drivers/media/video/v4l2-ioctl.c b/drivers/media/video/v4l2-ioctl.c
index 486eaba..e49ace8 100644
--- a/drivers/media/video/v4l2-ioctl.c
+++ b/drivers/media/video/v4l2-ioctl.c
@@ -412,7 +412,7 @@ static unsigned long cmd_input_size(unsigned int cmd)
 	}
 }
 
-static long
+long
 __video_usercopy(struct file *file, unsigned int cmd, unsigned long arg,
 		v4l2_kioctl func)
 {
diff --git a/drivers/media/video/v4l2-subdev.c b/drivers/media/video/v4l2-subdev.c
new file mode 100644
index 0000000..424c9c2
--- /dev/null
+++ b/drivers/media/video/v4l2-subdev.c
@@ -0,0 +1,65 @@
+/*
+ *  V4L2 subdevice support.
+ *
+ *  Copyright (C) 2010  Laurent Pinchart <laurent.pinchart@ideasonboard.com>
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License as published by
+ *  the Free Software Foundation; either version 2 of the License, or
+ *  (at your option) any later version.
+ *
+ *  This program is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License
+ *  along with this program; if not, write to the Free Software
+ *  Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
+ */
+
+#include <linux/types.h>
+#include <linux/ioctl.h>
+#include <linux/videodev2.h>
+
+#include <media/v4l2-device.h>
+#include <media/v4l2-ioctl.h>
+
+static int subdev_open(struct file *file)
+{
+	struct video_device *vdev = video_devdata(file);
+	struct v4l2_subdev *sd = vdev_to_v4l2_subdev(vdev);
+
+	if (!sd->initialized)
+		return -EAGAIN;
+
+	return 0;
+}
+
+static int subdev_close(struct file *file)
+{
+	return 0;
+}
+
+static long subdev_do_ioctl(struct file *file, unsigned int cmd, void *arg)
+{
+	switch (cmd) {
+	default:
+		return -ENOIOCTLCMD;
+	}
+
+	return 0;
+}
+
+static long subdev_ioctl(struct file *file, unsigned int cmd,
+	unsigned long arg)
+{
+	return __video_usercopy(file, cmd, arg, subdev_do_ioctl);
+}
+
+const struct v4l2_file_operations v4l2_subdev_fops = {
+	.owner = THIS_MODULE,
+	.open = subdev_open,
+	.unlocked_ioctl = subdev_ioctl,
+	.release = subdev_close,
+};
diff --git a/include/media/v4l2-common.h b/include/media/v4l2-common.h
index 6fc3d7a..496d158 100644
--- a/include/media/v4l2-common.h
+++ b/include/media/v4l2-common.h
@@ -148,7 +148,8 @@ struct i2c_board_info;
 
 struct v4l2_subdev *v4l2_i2c_new_subdev_board(struct v4l2_device *v4l2_dev,
 		struct i2c_adapter *adapter, const char *module_name,
-		struct i2c_board_info *info, const unsigned short *probe_addrs);
+		struct i2c_board_info *info, const unsigned short *probe_addrs,
+		int enable_devnode);
 
 /* Initialize an v4l2_subdev with data from an i2c_client struct */
 void v4l2_i2c_subdev_init(struct v4l2_subdev *sd, struct i2c_client *client,
@@ -181,7 +182,8 @@ struct spi_device;
 /* Load an spi module and return an initialized v4l2_subdev struct.
    The client_type argument is the name of the chip that's on the adapter. */
 struct v4l2_subdev *v4l2_spi_new_subdev(struct v4l2_device *v4l2_dev,
-		struct spi_master *master, struct spi_board_info *info);
+		struct spi_master *master, struct spi_board_info *info,
+		int enable_devnode);
 
 /* Initialize an v4l2_subdev with data from an spi_device struct */
 void v4l2_spi_subdev_init(struct v4l2_subdev *sd, struct spi_device *spi,
diff --git a/include/media/v4l2-dev.h b/include/media/v4l2-dev.h
index bebe44b..195fa56 100644
--- a/include/media/v4l2-dev.h
+++ b/include/media/v4l2-dev.h
@@ -22,7 +22,8 @@
 #define VFL_TYPE_VBI		1
 #define VFL_TYPE_RADIO		2
 #define VFL_TYPE_VTX		3
-#define VFL_TYPE_MAX		4
+#define VFL_TYPE_SUBDEV		4
+#define VFL_TYPE_MAX		5
 
 struct v4l2_ioctl_callbacks;
 struct video_device;
@@ -98,15 +99,26 @@ struct video_device
 /* dev to video-device */
 #define to_video_device(cd) container_of(cd, struct video_device, dev)
 
+int __must_check __video_register_device(struct video_device *vdev, int type,
+		int nr, int warn_if_nr_in_use, struct module *owner);
+
 /* Register video devices. Note that if video_register_device fails,
    the release() callback of the video_device structure is *not* called, so
    the caller is responsible for freeing any data. Usually that means that
    you call video_device_release() on failure. */
-int __must_check video_register_device(struct video_device *vdev, int type, int nr);
+static inline int __must_check video_register_device(struct video_device *vdev,
+		int type, int nr)
+{
+	return __video_register_device(vdev, type, nr, 1, vdev->fops->owner);
+}
 
 /* Same as video_register_device, but no warning is issued if the desired
    device node number was already in use. */
-int __must_check video_register_device_no_warn(struct video_device *vdev, int type, int nr);
+static inline int __must_check video_register_device_no_warn(
+		struct video_device *vdev, int type, int nr)
+{
+	return __video_register_device(vdev, type, nr, 0, vdev->fops->owner);
+}
 
 /* Unregister video devices. Will do nothing if vdev == NULL or
    video_is_registered() returns false. */
diff --git a/include/media/v4l2-ioctl.h b/include/media/v4l2-ioctl.h
index 06daa6e..abb64d0 100644
--- a/include/media/v4l2-ioctl.h
+++ b/include/media/v4l2-ioctl.h
@@ -316,6 +316,9 @@ extern long v4l2_compat_ioctl32(struct file *file, unsigned int cmd,
 				unsigned long arg);
 #endif
 
+extern long __video_usercopy(struct file *file, unsigned int cmd,
+				unsigned long arg, v4l2_kioctl func);
+
 /* Include support for obsoleted stuff */
 extern long video_usercopy(struct file *file, unsigned int cmd,
 				unsigned long arg, v4l2_kioctl func);
diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h
index 6088316..dc0ccd3 100644
--- a/include/media/v4l2-subdev.h
+++ b/include/media/v4l2-subdev.h
@@ -22,6 +22,7 @@
 #define _V4L2_SUBDEV_H
 
 #include <media/v4l2-common.h>
+#include <media/v4l2-dev.h>
 #include <media/v4l2-mediabus.h>
 
 /* generic v4l2_device notify callback notification values */
@@ -402,9 +403,11 @@ struct v4l2_subdev_ops {
 #define V4L2_SUBDEV_NAME_SIZE 32
 
 /* Set this flag if this subdev is a i2c device. */
-#define V4L2_SUBDEV_FL_IS_I2C (1U << 0)
+#define V4L2_SUBDEV_FL_IS_I2C			(1U << 0)
 /* Set this flag if this subdev is a spi device. */
-#define V4L2_SUBDEV_FL_IS_SPI (1U << 1)
+#define V4L2_SUBDEV_FL_IS_SPI			(1U << 1)
+/* Set this flag if this subdev needs a device node. */
+#define V4L2_SUBDEV_FL_HAS_DEVNODE		(1U << 2)
 
 /* Each instance of a subdev driver should create this struct, either
    stand-alone or embedded in a larger struct.
@@ -421,8 +424,16 @@ struct v4l2_subdev {
 	u32 grp_id;
 	/* pointer to private data */
 	void *priv;
+	/* subdev device node */
+	struct video_device devnode;
+	unsigned int initialized;
 };
 
+#define vdev_to_v4l2_subdev(vdev) \
+	container_of(vdev, struct v4l2_subdev, devnode)
+
+extern const struct v4l2_file_operations v4l2_subdev_fops;
+
 static inline void v4l2_set_subdevdata(struct v4l2_subdev *sd, void *p)
 {
 	sd->priv = p;
@@ -444,6 +455,7 @@ static inline void v4l2_subdev_init(struct v4l2_subdev *sd,
 	sd->name[0] = '\0';
 	sd->grp_id = 0;
 	sd->priv = NULL;
+	sd->initialized = 1;
 }
 
 /* Call an ops of a v4l2_subdev, doing the right checks against
-- 
1.7.1

Re: [RFC/PATCH v3 4/7] v4l: subdev: Add device node support

[Hans Verkuil]

On Monday 12 July 2010 17:25:49 Laurent Pinchart wrote:
> Create a device node named subdevX for every registered subdev.
> 
> As the device node is registered before the subdev core::s_config
> function is called, return -EGAIN on open until initialization
> completes.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Signed-off-by: Vimarsh Zutshi <vimarsh.zutshi@nokia.com>

Acked-by: Hans Verkuil <hverkuil@xs4all.nl>

Note: on Friday I hope to post a patch that removes the VTX support. This was
scheduled for removal in 2.6.35, but I never got around to that, so I hope
to remove it in 2.6.36 instead. That change will clash with this patch though
since the VTX type is removed.

It may (or may not) be a good idea to use the freed range of minors for these
new subdev nodes.

Regards,

	Hans

> ---
>  Documentation/video4linux/v4l2-framework.txt |   18 +++++++
>  drivers/media/radio/radio-si4713.c           |    2 +-
>  drivers/media/video/Makefile                 |    2 +-
>  drivers/media/video/soc_camera.c             |    2 +-
>  drivers/media/video/v4l2-common.c            |   15 +++++-
>  drivers/media/video/v4l2-dev.c               |   27 ++++------
>  drivers/media/video/v4l2-device.c            |   25 +++++++++-
>  drivers/media/video/v4l2-ioctl.c             |    2 +-
>  drivers/media/video/v4l2-subdev.c            |   65 ++++++++++++++++++++++++++
>  include/media/v4l2-common.h                  |    6 ++-
>  include/media/v4l2-dev.h                     |   18 ++++++-
>  include/media/v4l2-ioctl.h                   |    3 +
>  include/media/v4l2-subdev.h                  |   16 ++++++-
>  13 files changed, 170 insertions(+), 31 deletions(-)
>  create mode 100644 drivers/media/video/v4l2-subdev.c
> 
> diff --git a/Documentation/video4linux/v4l2-framework.txt b/Documentation/video4linux/v4l2-framework.txt
> index e831aac..164bb0f 100644
> --- a/Documentation/video4linux/v4l2-framework.txt
> +++ b/Documentation/video4linux/v4l2-framework.txt
> @@ -314,6 +314,24 @@ controlled through GPIO pins. This distinction is only relevant when setting
>  up the device, but once the subdev is registered it is completely transparent.
>  
>  
> +V4L2 sub-device userspace API
> +-----------------------------
> +
> +Beside exposing a kernel API through the v4l2_subdev_ops structure, V4L2
> +sub-devices can also be controlled directly by userspace applications.
> +
> +When a sub-device is registered, a device node named v4l-subdevX can be created
> +in /dev. If the sub-device supports direct userspace configuration it must set
> +the V4L2_SUBDEV_FL_HAS_DEVNODE flag before being registered.
> +
> +For I2C and SPI sub-devices, the v4l2_device driver can disable registration of
> +the device node if it wants to control the sub-device on its own. In that case
> +it must set the v4l2_i2c_new_subdev_board or v4l2_spi_new_subdev enable_devnode
> +argument to 0. Setting the argument to 1 will only enable device node
> +registration if the sub-device driver has set the V4L2_SUBDEV_FL_HAS_DEVNODE
> +flag.
> +
> +
>  I2C sub-device drivers
>  ----------------------
>  
> diff --git a/drivers/media/radio/radio-si4713.c b/drivers/media/radio/radio-si4713.c
> index 13554ab..58dd199 100644
> --- a/drivers/media/radio/radio-si4713.c
> +++ b/drivers/media/radio/radio-si4713.c
> @@ -292,7 +292,7 @@ static int radio_si4713_pdriver_probe(struct platform_device *pdev)
>  	}
>  
>  	sd = v4l2_i2c_new_subdev_board(&rsdev->v4l2_dev, adapter, "si4713_i2c",
> -					pdata->subdev_board_info, NULL);
> +					pdata->subdev_board_info, NULL, 0);
>  	if (!sd) {
>  		dev_err(&pdev->dev, "Cannot get v4l2 subdevice\n");
>  		rval = -ENODEV;
> diff --git a/drivers/media/video/Makefile b/drivers/media/video/Makefile
> index cc93859..c9c07e5 100644
> --- a/drivers/media/video/Makefile
> +++ b/drivers/media/video/Makefile
> @@ -11,7 +11,7 @@ stkwebcam-objs	:=	stk-webcam.o stk-sensor.o
>  omap2cam-objs	:=	omap24xxcam.o omap24xxcam-dma.o
>  
>  videodev-objs	:=	v4l2-dev.o v4l2-ioctl.o v4l2-device.o v4l2-fh.o \
> -			v4l2-event.o
> +			v4l2-event.o v4l2-subdev.o
>  
>  # V4L2 core modules
>  
> diff --git a/drivers/media/video/soc_camera.c b/drivers/media/video/soc_camera.c
> index 475757b..10fae27 100644
> --- a/drivers/media/video/soc_camera.c
> +++ b/drivers/media/video/soc_camera.c
> @@ -895,7 +895,7 @@ static int soc_camera_init_i2c(struct soc_camera_device *icd,
>  	icl->board_info->platform_data = icd;
>  
>  	subdev = v4l2_i2c_new_subdev_board(&ici->v4l2_dev, adap,
> -				icl->module_name, icl->board_info, NULL);
> +				icl->module_name, icl->board_info, NULL, 0);
>  	if (!subdev)
>  		goto ei2cnd;
>  
> diff --git a/drivers/media/video/v4l2-common.c b/drivers/media/video/v4l2-common.c
> index bbda421..4265562 100644
> --- a/drivers/media/video/v4l2-common.c
> +++ b/drivers/media/video/v4l2-common.c
> @@ -838,7 +838,8 @@ EXPORT_SYMBOL_GPL(v4l2_i2c_subdev_init);
>  /* Load an i2c sub-device. */
>  struct v4l2_subdev *v4l2_i2c_new_subdev_board(struct v4l2_device *v4l2_dev,
>  		struct i2c_adapter *adapter, const char *module_name,
> -		struct i2c_board_info *info, const unsigned short *probe_addrs)
> +		struct i2c_board_info *info, const unsigned short *probe_addrs,
> +		int enable_devnode)
>  {
>  	struct v4l2_subdev *sd = NULL;
>  	struct i2c_client *client;
> @@ -868,9 +869,12 @@ struct v4l2_subdev *v4l2_i2c_new_subdev_board(struct v4l2_device *v4l2_dev,
>  	if (!try_module_get(client->driver->driver.owner))
>  		goto error;
>  	sd = i2c_get_clientdata(client);
> +	if (!enable_devnode)
> +		sd->flags &= ~V4L2_SUBDEV_FL_HAS_DEVNODE;
>  
>  	/* Register with the v4l2_device which increases the module's
>  	   use count as well. */
> +	sd->initialized = 0;
>  	if (v4l2_device_register_subdev(v4l2_dev, sd))
>  		sd = NULL;
>  	/* Decrease the module use count to match the first try_module_get. */
> @@ -885,6 +889,8 @@ struct v4l2_subdev *v4l2_i2c_new_subdev_board(struct v4l2_device *v4l2_dev,
>  		if (err && err != -ENOIOCTLCMD) {
>  			v4l2_device_unregister_subdev(sd);
>  			sd = NULL;
> +		} else {
> +			sd->initialized = 1;
>  		}
>  	}
>  
> @@ -911,7 +917,7 @@ struct v4l2_subdev *v4l2_i2c_new_subdev(struct v4l2_device *v4l2_dev,
>  	info.addr = addr;
>  
>  	return v4l2_i2c_new_subdev_board(v4l2_dev, adapter, module_name,
> -			&info, probe_addrs);
> +			&info, probe_addrs, 0);
>  }
>  EXPORT_SYMBOL_GPL(v4l2_i2c_new_subdev);
>  
> @@ -981,7 +987,8 @@ void v4l2_spi_subdev_init(struct v4l2_subdev *sd, struct spi_device *spi,
>  EXPORT_SYMBOL_GPL(v4l2_spi_subdev_init);
>  
>  struct v4l2_subdev *v4l2_spi_new_subdev(struct v4l2_device *v4l2_dev,
> -		struct spi_master *master, struct spi_board_info *info)
> +		struct spi_master *master, struct spi_board_info *info,
> +		int enable_devnode)
>  {
>  	struct v4l2_subdev *sd = NULL;
>  	struct spi_device *spi = NULL;
> @@ -1000,6 +1007,8 @@ struct v4l2_subdev *v4l2_spi_new_subdev(struct v4l2_device *v4l2_dev,
>  		goto error;
>  
>  	sd = spi_get_drvdata(spi);
> +	if (!enable_devnode)
> +		sd->flags &= ~V4L2_SUBDEV_FL_HAS_DEVNODE;
>  
>  	/* Register with the v4l2_device which increases the module's
>  	   use count as well. */
> diff --git a/drivers/media/video/v4l2-dev.c b/drivers/media/video/v4l2-dev.c
> index 0ca7ec9..bcd47a0 100644
> --- a/drivers/media/video/v4l2-dev.c
> +++ b/drivers/media/video/v4l2-dev.c
> @@ -376,13 +376,14 @@ static int get_index(struct video_device *vdev)
>  }
>  
>  /**
> - *	video_register_device - register video4linux devices
> + *	__video_register_device - register video4linux devices
>   *	@vdev: video device structure we want to register
>   *	@type: type of device to register
>   *	@nr:   which device node number (0 == /dev/video0, 1 == /dev/video1, ...
>   *             -1 == first free)
>   *	@warn_if_nr_in_use: warn if the desired device node number
>   *	       was already in use and another number was chosen instead.
> + *	@owner: module that owns the video device node
>   *
>   *	The registration code assigns minor numbers and device node numbers
>   *	based on the requested type and registers the new device node with
> @@ -401,9 +402,11 @@ static int get_index(struct video_device *vdev)
>   *	%VFL_TYPE_VBI - Vertical blank data (undecoded)
>   *
>   *	%VFL_TYPE_RADIO - A radio card
> + *
> + *	%VFL_TYPE_SUBDEV - A subdevice
>   */
> -static int __video_register_device(struct video_device *vdev, int type, int nr,
> -		int warn_if_nr_in_use)
> +int __video_register_device(struct video_device *vdev, int type, int nr,
> +		int warn_if_nr_in_use, struct module *owner)
>  {
>  	int i = 0;
>  	int ret;
> @@ -439,6 +442,9 @@ static int __video_register_device(struct video_device *vdev, int type, int nr,
>  	case VFL_TYPE_RADIO:
>  		name_base = "radio";
>  		break;
> +	case VFL_TYPE_SUBDEV:
> +		name_base = "v4l-subdev";
> +		break;
>  	default:
>  		printk(KERN_ERR "%s called with unknown type: %d\n",
>  		       __func__, type);
> @@ -525,7 +531,7 @@ static int __video_register_device(struct video_device *vdev, int type, int nr,
>  		vdev->cdev->ops = &v4l2_unlocked_fops;
>  	else
>  		vdev->cdev->ops = &v4l2_fops;
> -	vdev->cdev->owner = vdev->fops->owner;
> +	vdev->cdev->owner = owner;
>  	ret = cdev_add(vdev->cdev, MKDEV(VIDEO_MAJOR, vdev->minor), 1);
>  	if (ret < 0) {
>  		printk(KERN_ERR "%s: cdev_add failed\n", __func__);
> @@ -574,18 +580,7 @@ cleanup:
>  	vdev->minor = -1;
>  	return ret;
>  }
> -
> -int video_register_device(struct video_device *vdev, int type, int nr)
> -{
> -	return __video_register_device(vdev, type, nr, 1);
> -}
> -EXPORT_SYMBOL(video_register_device);
> -
> -int video_register_device_no_warn(struct video_device *vdev, int type, int nr)
> -{
> -	return __video_register_device(vdev, type, nr, 0);
> -}
> -EXPORT_SYMBOL(video_register_device_no_warn);
> +EXPORT_SYMBOL(__video_register_device);
>  
>  /**
>   *	video_unregister_device - unregister a video4linux device
> diff --git a/drivers/media/video/v4l2-device.c b/drivers/media/video/v4l2-device.c
> index 5a7dc4a..b287aaa 100644
> --- a/drivers/media/video/v4l2-device.c
> +++ b/drivers/media/video/v4l2-device.c
> @@ -115,18 +115,38 @@ EXPORT_SYMBOL_GPL(v4l2_device_unregister);
>  int v4l2_device_register_subdev(struct v4l2_device *v4l2_dev,
>  						struct v4l2_subdev *sd)
>  {
> +	struct video_device *vdev;
> +	int ret = 0;
> +
>  	/* Check for valid input */
>  	if (v4l2_dev == NULL || sd == NULL || !sd->name[0])
>  		return -EINVAL;
> +
>  	/* Warn if we apparently re-register a subdev */
>  	WARN_ON(sd->v4l2_dev != NULL);
> +
>  	if (!try_module_get(sd->owner))
>  		return -ENODEV;
> +
>  	sd->v4l2_dev = v4l2_dev;
>  	spin_lock(&v4l2_dev->lock);
>  	list_add_tail(&sd->list, &v4l2_dev->subdevs);
>  	spin_unlock(&v4l2_dev->lock);
> -	return 0;
> +
> +	/* Register the device node. */
> +	vdev = &sd->devnode;
> +	strlcpy(vdev->name, sd->name, sizeof(vdev->name));
> +	vdev->parent = v4l2_dev->dev;
> +	vdev->fops = &v4l2_subdev_fops;
> +	vdev->release = video_device_release_empty;
> +	if (sd->flags & V4L2_SUBDEV_FL_HAS_DEVNODE) {
> +		ret = __video_register_device(vdev, VFL_TYPE_SUBDEV, -1, 1,
> +					      sd->owner);
> +		if (ret < 0)
> +			v4l2_device_unregister_subdev(sd);
> +	}
> +
> +	return ret;
>  }
>  EXPORT_SYMBOL_GPL(v4l2_device_register_subdev);
>  
> @@ -135,10 +155,13 @@ void v4l2_device_unregister_subdev(struct v4l2_subdev *sd)
>  	/* return if it isn't registered */
>  	if (sd == NULL || sd->v4l2_dev == NULL)
>  		return;
> +
>  	spin_lock(&sd->v4l2_dev->lock);
>  	list_del(&sd->list);
>  	spin_unlock(&sd->v4l2_dev->lock);
>  	sd->v4l2_dev = NULL;
> +
>  	module_put(sd->owner);
> +	video_unregister_device(&sd->devnode);
>  }
>  EXPORT_SYMBOL_GPL(v4l2_device_unregister_subdev);
> diff --git a/drivers/media/video/v4l2-ioctl.c b/drivers/media/video/v4l2-ioctl.c
> index 486eaba..e49ace8 100644
> --- a/drivers/media/video/v4l2-ioctl.c
> +++ b/drivers/media/video/v4l2-ioctl.c
> @@ -412,7 +412,7 @@ static unsigned long cmd_input_size(unsigned int cmd)
>  	}
>  }
>  
> -static long
> +long
>  __video_usercopy(struct file *file, unsigned int cmd, unsigned long arg,
>  		v4l2_kioctl func)
>  {
> diff --git a/drivers/media/video/v4l2-subdev.c b/drivers/media/video/v4l2-subdev.c
> new file mode 100644
> index 0000000..424c9c2
> --- /dev/null
> +++ b/drivers/media/video/v4l2-subdev.c
> @@ -0,0 +1,65 @@
> +/*
> + *  V4L2 subdevice support.
> + *
> + *  Copyright (C) 2010  Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> + *
> + *  This program is free software; you can redistribute it and/or modify
> + *  it under the terms of the GNU General Public License as published by
> + *  the Free Software Foundation; either version 2 of the License, or
> + *  (at your option) any later version.
> + *
> + *  This program is distributed in the hope that it will be useful,
> + *  but WITHOUT ANY WARRANTY; without even the implied warranty of
> + *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + *  GNU General Public License for more details.
> + *
> + *  You should have received a copy of the GNU General Public License
> + *  along with this program; if not, write to the Free Software
> + *  Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
> + */
> +
> +#include <linux/types.h>
> +#include <linux/ioctl.h>
> +#include <linux/videodev2.h>
> +
> +#include <media/v4l2-device.h>
> +#include <media/v4l2-ioctl.h>
> +
> +static int subdev_open(struct file *file)
> +{
> +	struct video_device *vdev = video_devdata(file);
> +	struct v4l2_subdev *sd = vdev_to_v4l2_subdev(vdev);
> +
> +	if (!sd->initialized)
> +		return -EAGAIN;
> +
> +	return 0;
> +}
> +
> +static int subdev_close(struct file *file)
> +{
> +	return 0;
> +}
> +
> +static long subdev_do_ioctl(struct file *file, unsigned int cmd, void *arg)
> +{
> +	switch (cmd) {
> +	default:
> +		return -ENOIOCTLCMD;
> +	}
> +
> +	return 0;
> +}
> +
> +static long subdev_ioctl(struct file *file, unsigned int cmd,
> +	unsigned long arg)
> +{
> +	return __video_usercopy(file, cmd, arg, subdev_do_ioctl);
> +}
> +
> +const struct v4l2_file_operations v4l2_subdev_fops = {
> +	.owner = THIS_MODULE,
> +	.open = subdev_open,
> +	.unlocked_ioctl = subdev_ioctl,
> +	.release = subdev_close,
> +};
> diff --git a/include/media/v4l2-common.h b/include/media/v4l2-common.h
> index 6fc3d7a..496d158 100644
> --- a/include/media/v4l2-common.h
> +++ b/include/media/v4l2-common.h
> @@ -148,7 +148,8 @@ struct i2c_board_info;
>  
>  struct v4l2_subdev *v4l2_i2c_new_subdev_board(struct v4l2_device *v4l2_dev,
>  		struct i2c_adapter *adapter, const char *module_name,
> -		struct i2c_board_info *info, const unsigned short *probe_addrs);
> +		struct i2c_board_info *info, const unsigned short *probe_addrs,
> +		int enable_devnode);
>  
>  /* Initialize an v4l2_subdev with data from an i2c_client struct */
>  void v4l2_i2c_subdev_init(struct v4l2_subdev *sd, struct i2c_client *client,
> @@ -181,7 +182,8 @@ struct spi_device;
>  /* Load an spi module and return an initialized v4l2_subdev struct.
>     The client_type argument is the name of the chip that's on the adapter. */
>  struct v4l2_subdev *v4l2_spi_new_subdev(struct v4l2_device *v4l2_dev,
> -		struct spi_master *master, struct spi_board_info *info);
> +		struct spi_master *master, struct spi_board_info *info,
> +		int enable_devnode);
>  
>  /* Initialize an v4l2_subdev with data from an spi_device struct */
>  void v4l2_spi_subdev_init(struct v4l2_subdev *sd, struct spi_device *spi,
> diff --git a/include/media/v4l2-dev.h b/include/media/v4l2-dev.h
> index bebe44b..195fa56 100644
> --- a/include/media/v4l2-dev.h
> +++ b/include/media/v4l2-dev.h
> @@ -22,7 +22,8 @@
>  #define VFL_TYPE_VBI		1
>  #define VFL_TYPE_RADIO		2
>  #define VFL_TYPE_VTX		3
> -#define VFL_TYPE_MAX		4
> +#define VFL_TYPE_SUBDEV		4
> +#define VFL_TYPE_MAX		5
>  
>  struct v4l2_ioctl_callbacks;
>  struct video_device;
> @@ -98,15 +99,26 @@ struct video_device
>  /* dev to video-device */
>  #define to_video_device(cd) container_of(cd, struct video_device, dev)
>  
> +int __must_check __video_register_device(struct video_device *vdev, int type,
> +		int nr, int warn_if_nr_in_use, struct module *owner);
> +
>  /* Register video devices. Note that if video_register_device fails,
>     the release() callback of the video_device structure is *not* called, so
>     the caller is responsible for freeing any data. Usually that means that
>     you call video_device_release() on failure. */
> -int __must_check video_register_device(struct video_device *vdev, int type, int nr);
> +static inline int __must_check video_register_device(struct video_device *vdev,
> +		int type, int nr)
> +{
> +	return __video_register_device(vdev, type, nr, 1, vdev->fops->owner);
> +}
>  
>  /* Same as video_register_device, but no warning is issued if the desired
>     device node number was already in use. */
> -int __must_check video_register_device_no_warn(struct video_device *vdev, int type, int nr);
> +static inline int __must_check video_register_device_no_warn(
> +		struct video_device *vdev, int type, int nr)
> +{
> +	return __video_register_device(vdev, type, nr, 0, vdev->fops->owner);
> +}
>  
>  /* Unregister video devices. Will do nothing if vdev == NULL or
>     video_is_registered() returns false. */
> diff --git a/include/media/v4l2-ioctl.h b/include/media/v4l2-ioctl.h
> index 06daa6e..abb64d0 100644
> --- a/include/media/v4l2-ioctl.h
> +++ b/include/media/v4l2-ioctl.h
> @@ -316,6 +316,9 @@ extern long v4l2_compat_ioctl32(struct file *file, unsigned int cmd,
>  				unsigned long arg);
>  #endif
>  
> +extern long __video_usercopy(struct file *file, unsigned int cmd,
> +				unsigned long arg, v4l2_kioctl func);
> +
>  /* Include support for obsoleted stuff */
>  extern long video_usercopy(struct file *file, unsigned int cmd,
>  				unsigned long arg, v4l2_kioctl func);
> diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h
> index 6088316..dc0ccd3 100644
> --- a/include/media/v4l2-subdev.h
> +++ b/include/media/v4l2-subdev.h
> @@ -22,6 +22,7 @@
>  #define _V4L2_SUBDEV_H
>  
>  #include <media/v4l2-common.h>
> +#include <media/v4l2-dev.h>
>  #include <media/v4l2-mediabus.h>
>  
>  /* generic v4l2_device notify callback notification values */
> @@ -402,9 +403,11 @@ struct v4l2_subdev_ops {
>  #define V4L2_SUBDEV_NAME_SIZE 32
>  
>  /* Set this flag if this subdev is a i2c device. */
> -#define V4L2_SUBDEV_FL_IS_I2C (1U << 0)
> +#define V4L2_SUBDEV_FL_IS_I2C			(1U << 0)
>  /* Set this flag if this subdev is a spi device. */
> -#define V4L2_SUBDEV_FL_IS_SPI (1U << 1)
> +#define V4L2_SUBDEV_FL_IS_SPI			(1U << 1)
> +/* Set this flag if this subdev needs a device node. */
> +#define V4L2_SUBDEV_FL_HAS_DEVNODE		(1U << 2)
>  
>  /* Each instance of a subdev driver should create this struct, either
>     stand-alone or embedded in a larger struct.
> @@ -421,8 +424,16 @@ struct v4l2_subdev {
>  	u32 grp_id;
>  	/* pointer to private data */
>  	void *priv;
> +	/* subdev device node */
> +	struct video_device devnode;
> +	unsigned int initialized;
>  };
>  
> +#define vdev_to_v4l2_subdev(vdev) \
> +	container_of(vdev, struct v4l2_subdev, devnode)
> +
> +extern const struct v4l2_file_operations v4l2_subdev_fops;
> +
>  static inline void v4l2_set_subdevdata(struct v4l2_subdev *sd, void *p)
>  {
>  	sd->priv = p;
> @@ -444,6 +455,7 @@ static inline void v4l2_subdev_init(struct v4l2_subdev *sd,
>  	sd->name[0] = '\0';
>  	sd->grp_id = 0;
>  	sd->priv = NULL;
> +	sd->initialized = 1;
>  }
>  
>  /* Call an ops of a v4l2_subdev, doing the right checks against
> 

-- 
Hans Verkuil - video4linux developer - sponsored by TANDBERG, part of Cisco

[RFC/PATCH v3 5/7] v4l: subdev: Uninline the v4l2_subdev_init function

[Laurent Pinchart]

The function isn't small or performance sensitive enough to be inlined.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/media/video/v4l2-subdev.c |   14 ++++++++++++++
 include/media/v4l2-subdev.h       |   15 ++-------------
 2 files changed, 16 insertions(+), 13 deletions(-)

diff --git a/drivers/media/video/v4l2-subdev.c b/drivers/media/video/v4l2-subdev.c
index 424c9c2..052dc9c 100644
--- a/drivers/media/video/v4l2-subdev.c
+++ b/drivers/media/video/v4l2-subdev.c
@@ -63,3 +63,17 @@ const struct v4l2_file_operations v4l2_subdev_fops = {
 	.unlocked_ioctl = subdev_ioctl,
 	.release = subdev_close,
 };
+
+void v4l2_subdev_init(struct v4l2_subdev *sd, const struct v4l2_subdev_ops *ops)
+{
+	INIT_LIST_HEAD(&sd->list);
+	BUG_ON(!ops);
+	sd->ops = ops;
+	sd->v4l2_dev = NULL;
+	sd->flags = 0;
+	sd->name[0] = '\0';
+	sd->grp_id = 0;
+	sd->priv = NULL;
+	sd->initialized = 1;
+}
+EXPORT_SYMBOL(v4l2_subdev_init);
diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h
index dc0ccd3..9ee45c8 100644
--- a/include/media/v4l2-subdev.h
+++ b/include/media/v4l2-subdev.h
@@ -444,19 +444,8 @@ static inline void *v4l2_get_subdevdata(const struct v4l2_subdev *sd)
 	return sd->priv;
 }
 
-static inline void v4l2_subdev_init(struct v4l2_subdev *sd,
-					const struct v4l2_subdev_ops *ops)
-{
-	INIT_LIST_HEAD(&sd->list);
-	BUG_ON(!ops);
-	sd->ops = ops;
-	sd->v4l2_dev = NULL;
-	sd->flags = 0;
-	sd->name[0] = '\0';
-	sd->grp_id = 0;
-	sd->priv = NULL;
-	sd->initialized = 1;
-}
+void v4l2_subdev_init(struct v4l2_subdev *sd,
+		      const struct v4l2_subdev_ops *ops);
 
 /* Call an ops of a v4l2_subdev, doing the right checks against
    NULL pointers.
-- 
1.7.1

Re: [RFC/PATCH v3 5/7] v4l: subdev: Uninline the v4l2_subdev_init function

[Hans Verkuil]

On Monday 12 July 2010 17:25:50 Laurent Pinchart wrote:
> The function isn't small or performance sensitive enough to be inlined.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

Acked-by: Hans Verkuil <hverkuil@xs4all.nl>

> ---
>  drivers/media/video/v4l2-subdev.c |   14 ++++++++++++++
>  include/media/v4l2-subdev.h       |   15 ++-------------
>  2 files changed, 16 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/media/video/v4l2-subdev.c b/drivers/media/video/v4l2-subdev.c
> index 424c9c2..052dc9c 100644
> --- a/drivers/media/video/v4l2-subdev.c
> +++ b/drivers/media/video/v4l2-subdev.c
> @@ -63,3 +63,17 @@ const struct v4l2_file_operations v4l2_subdev_fops = {
>  	.unlocked_ioctl = subdev_ioctl,
>  	.release = subdev_close,
>  };
> +
> +void v4l2_subdev_init(struct v4l2_subdev *sd, const struct v4l2_subdev_ops *ops)
> +{
> +	INIT_LIST_HEAD(&sd->list);
> +	BUG_ON(!ops);
> +	sd->ops = ops;
> +	sd->v4l2_dev = NULL;
> +	sd->flags = 0;
> +	sd->name[0] = '\0';
> +	sd->grp_id = 0;
> +	sd->priv = NULL;
> +	sd->initialized = 1;
> +}
> +EXPORT_SYMBOL(v4l2_subdev_init);
> diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h
> index dc0ccd3..9ee45c8 100644
> --- a/include/media/v4l2-subdev.h
> +++ b/include/media/v4l2-subdev.h
> @@ -444,19 +444,8 @@ static inline void *v4l2_get_subdevdata(const struct v4l2_subdev *sd)
>  	return sd->priv;
>  }
>  
> -static inline void v4l2_subdev_init(struct v4l2_subdev *sd,
> -					const struct v4l2_subdev_ops *ops)
> -{
> -	INIT_LIST_HEAD(&sd->list);
> -	BUG_ON(!ops);
> -	sd->ops = ops;
> -	sd->v4l2_dev = NULL;
> -	sd->flags = 0;
> -	sd->name[0] = '\0';
> -	sd->grp_id = 0;
> -	sd->priv = NULL;
> -	sd->initialized = 1;
> -}
> +void v4l2_subdev_init(struct v4l2_subdev *sd,
> +		      const struct v4l2_subdev_ops *ops);
>  
>  /* Call an ops of a v4l2_subdev, doing the right checks against
>     NULL pointers.
> 

-- 
Hans Verkuil - video4linux developer - sponsored by TANDBERG, part of Cisco

[RFC/PATCH v3 6/7] v4l: subdev: Control ioctls support

[Laurent Pinchart]

Pass the control-related ioctls to the subdev driver through the core
operations.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 Documentation/video4linux/v4l2-framework.txt |   16 ++++++++++++++++
 drivers/media/video/v4l2-subdev.c            |   24 ++++++++++++++++++++++++
 2 files changed, 40 insertions(+), 0 deletions(-)

diff --git a/Documentation/video4linux/v4l2-framework.txt b/Documentation/video4linux/v4l2-framework.txt
index 164bb0f..9c3f33c 100644
--- a/Documentation/video4linux/v4l2-framework.txt
+++ b/Documentation/video4linux/v4l2-framework.txt
@@ -331,6 +331,22 @@ argument to 0. Setting the argument to 1 will only enable device node
 registration if the sub-device driver has set the V4L2_SUBDEV_FL_HAS_DEVNODE
 flag.
 
+The device node handles a subset of the V4L2 API.
+
+VIDIOC_QUERYCTRL
+VIDIOC_QUERYMENU
+VIDIOC_G_CTRL
+VIDIOC_S_CTRL
+VIDIOC_G_EXT_CTRLS
+VIDIOC_S_EXT_CTRLS
+VIDIOC_TRY_EXT_CTRLS
+
+	The controls ioctls are identical to the ones defined in V4L2. They
+	behave identically, with the only exception that they deal only with
+	controls implemented in the sub-device. Depending on the driver, those
+	controls can be also be accessed through one (or several) V4L2 device
+	nodes.
+
 
 I2C sub-device drivers
 ----------------------
diff --git a/drivers/media/video/v4l2-subdev.c b/drivers/media/video/v4l2-subdev.c
index 052dc9c..ea3941a 100644
--- a/drivers/media/video/v4l2-subdev.c
+++ b/drivers/media/video/v4l2-subdev.c
@@ -43,7 +43,31 @@ static int subdev_close(struct file *file)
 
 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);
+
 	switch (cmd) {
+	case VIDIOC_QUERYCTRL:
+		return v4l2_subdev_call(sd, core, queryctrl, arg);
+
+	case VIDIOC_QUERYMENU:
+		return v4l2_subdev_call(sd, core, querymenu, arg);
+
+	case VIDIOC_G_CTRL:
+		return v4l2_subdev_call(sd, core, g_ctrl, arg);
+
+	case VIDIOC_S_CTRL:
+		return v4l2_subdev_call(sd, core, s_ctrl, arg);
+
+	case VIDIOC_G_EXT_CTRLS:
+		return v4l2_subdev_call(sd, core, g_ext_ctrls, arg);
+
+	case VIDIOC_S_EXT_CTRLS:
+		return v4l2_subdev_call(sd, core, s_ext_ctrls, arg);
+
+	case VIDIOC_TRY_EXT_CTRLS:
+		return v4l2_subdev_call(sd, core, try_ext_ctrls, arg);
+
 	default:
 		return -ENOIOCTLCMD;
 	}
-- 
1.7.1

Re: [RFC/PATCH v3 6/7] v4l: subdev: Control ioctls support

[Hans Verkuil]

On Monday 12 July 2010 17:25:51 Laurent Pinchart wrote:
> Pass the control-related ioctls to the subdev driver through the core
> operations.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

Acked-by: Hans Verkuil <hverkuil@xs4all.nl>

Note: if the control framework is merged first, then this code will
no doubt have to change.

> ---
>  Documentation/video4linux/v4l2-framework.txt |   16 ++++++++++++++++
>  drivers/media/video/v4l2-subdev.c            |   24 ++++++++++++++++++++++++
>  2 files changed, 40 insertions(+), 0 deletions(-)
> 
> diff --git a/Documentation/video4linux/v4l2-framework.txt b/Documentation/video4linux/v4l2-framework.txt
> index 164bb0f..9c3f33c 100644
> --- a/Documentation/video4linux/v4l2-framework.txt
> +++ b/Documentation/video4linux/v4l2-framework.txt
> @@ -331,6 +331,22 @@ argument to 0. Setting the argument to 1 will only enable device node
>  registration if the sub-device driver has set the V4L2_SUBDEV_FL_HAS_DEVNODE
>  flag.
>  
> +The device node handles a subset of the V4L2 API.
> +
> +VIDIOC_QUERYCTRL
> +VIDIOC_QUERYMENU
> +VIDIOC_G_CTRL
> +VIDIOC_S_CTRL
> +VIDIOC_G_EXT_CTRLS
> +VIDIOC_S_EXT_CTRLS
> +VIDIOC_TRY_EXT_CTRLS
> +
> +	The controls ioctls are identical to the ones defined in V4L2. They
> +	behave identically, with the only exception that they deal only with
> +	controls implemented in the sub-device. Depending on the driver, those
> +	controls can be also be accessed through one (or several) V4L2 device
> +	nodes.
> +
>  
>  I2C sub-device drivers
>  ----------------------
> diff --git a/drivers/media/video/v4l2-subdev.c b/drivers/media/video/v4l2-subdev.c
> index 052dc9c..ea3941a 100644
> --- a/drivers/media/video/v4l2-subdev.c
> +++ b/drivers/media/video/v4l2-subdev.c
> @@ -43,7 +43,31 @@ static int subdev_close(struct file *file)
>  
>  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);
> +
>  	switch (cmd) {
> +	case VIDIOC_QUERYCTRL:
> +		return v4l2_subdev_call(sd, core, queryctrl, arg);
> +
> +	case VIDIOC_QUERYMENU:
> +		return v4l2_subdev_call(sd, core, querymenu, arg);
> +
> +	case VIDIOC_G_CTRL:
> +		return v4l2_subdev_call(sd, core, g_ctrl, arg);
> +
> +	case VIDIOC_S_CTRL:
> +		return v4l2_subdev_call(sd, core, s_ctrl, arg);
> +
> +	case VIDIOC_G_EXT_CTRLS:
> +		return v4l2_subdev_call(sd, core, g_ext_ctrls, arg);
> +
> +	case VIDIOC_S_EXT_CTRLS:
> +		return v4l2_subdev_call(sd, core, s_ext_ctrls, arg);
> +
> +	case VIDIOC_TRY_EXT_CTRLS:
> +		return v4l2_subdev_call(sd, core, try_ext_ctrls, arg);
> +
>  	default:
>  		return -ENOIOCTLCMD;
>  	}
> 

-- 
Hans Verkuil - video4linux developer - sponsored by TANDBERG, part of Cisco

[RFC/PATCH v3 7/7] v4l: subdev: Events support

[Laurent Pinchart]

From: Sakari Ailus <sakari.ailus@maxwell.research.nokia.com>

Provide v4l2_subdevs with v4l2_event support. Subdev drivers only need very
little to support events.

Signed-off-by: Sakari Ailus <sakari.ailus@maxwell.research.nokia.com>
Signed-off-by: David Cohen <david.cohen@nokia.com>
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 Documentation/video4linux/v4l2-framework.txt |   18 ++++++
 drivers/media/video/v4l2-subdev.c            |   75 +++++++++++++++++++++++++-
 include/media/v4l2-subdev.h                  |   10 ++++
 3 files changed, 102 insertions(+), 1 deletions(-)

diff --git a/Documentation/video4linux/v4l2-framework.txt b/Documentation/video4linux/v4l2-framework.txt
index 9c3f33c..89bd881 100644
--- a/Documentation/video4linux/v4l2-framework.txt
+++ b/Documentation/video4linux/v4l2-framework.txt
@@ -347,6 +347,24 @@ VIDIOC_TRY_EXT_CTRLS
 	controls can be also be accessed through one (or several) V4L2 device
 	nodes.
 
+VIDIOC_DQEVENT
+VIDIOC_SUBSCRIBE_EVENT
+VIDIOC_UNSUBSCRIBE_EVENT
+
+	The events ioctls are identical to the ones defined in V4L2. They
+	behave identically, with the only exception that they deal only with
+	events generated by the sub-device. Depending on the driver, those
+	events can also be reported by one (or several) V4L2 device nodes.
+
+	Sub-device drivers that want to use events need to set the
+	V4L2_SUBDEV_USES_EVENTS v4l2_subdev::flags and initialize
+	v4l2_subdev::nevents to events queue depth before registering the
+	sub-device. After registration events can be queued as usual on the
+	v4l2_subdev::devnode device node.
+
+	To properly support events, the poll() file operation is also
+	implemented.
+
 
 I2C sub-device drivers
 ----------------------
diff --git a/drivers/media/video/v4l2-subdev.c b/drivers/media/video/v4l2-subdev.c
index ea3941a..b063195 100644
--- a/drivers/media/video/v4l2-subdev.c
+++ b/drivers/media/video/v4l2-subdev.c
@@ -18,26 +18,68 @@
  *  Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
  */
 
-#include <linux/types.h>
 #include <linux/ioctl.h>
+#include <linux/slab.h>
+#include <linux/types.h>
 #include <linux/videodev2.h>
 
 #include <media/v4l2-device.h>
 #include <media/v4l2-ioctl.h>
+#include <media/v4l2-fh.h>
+#include <media/v4l2-event.h>
 
 static int subdev_open(struct file *file)
 {
 	struct video_device *vdev = video_devdata(file);
 	struct v4l2_subdev *sd = vdev_to_v4l2_subdev(vdev);
+	struct v4l2_fh *vfh;
+	int ret;
 
 	if (!sd->initialized)
 		return -EAGAIN;
 
+	if (sd->flags & V4L2_SUBDEV_FL_HAS_EVENTS) {
+		vfh = kzalloc(sizeof(*vfh), GFP_KERNEL);
+		if (vfh == NULL)
+			return -ENOMEM;
+
+		ret = v4l2_fh_init(vfh, vdev);
+		if (ret)
+			goto err;
+
+		ret = v4l2_event_init(vfh);
+		if (ret)
+			goto err;
+
+		ret = v4l2_event_alloc(vfh, sd->nevents);
+		if (ret)
+			goto err;
+
+		v4l2_fh_add(vfh);
+		file->private_data = vfh;
+	}
+
 	return 0;
+
+err:
+	if (vfh != NULL) {
+		v4l2_fh_exit(vfh);
+		kfree(vfh);
+	}
+
+	return ret;
 }
 
 static int subdev_close(struct file *file)
 {
+	struct v4l2_fh *vfh = file->private_data;
+
+	if (vfh != NULL) {
+		v4l2_fh_del(vfh);
+		v4l2_fh_exit(vfh);
+		kfree(vfh);
+	}
+
 	return 0;
 }
 
@@ -45,6 +87,7 @@ 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 *fh = file->private_data;
 
 	switch (cmd) {
 	case VIDIOC_QUERYCTRL:
@@ -68,6 +111,18 @@ static long subdev_do_ioctl(struct file *file, unsigned int cmd, void *arg)
 	case VIDIOC_TRY_EXT_CTRLS:
 		return v4l2_subdev_call(sd, core, try_ext_ctrls, arg);
 
+	case VIDIOC_DQEVENT:
+		if (!(sd->flags & V4L2_SUBDEV_FL_HAS_EVENTS))
+			return -ENOIOCTLCMD;
+
+		return v4l2_event_dequeue(fh, arg, file->f_flags & O_NONBLOCK);
+
+	case VIDIOC_SUBSCRIBE_EVENT:
+		return v4l2_subdev_call(sd, core, subscribe_event, fh, arg);
+
+	case VIDIOC_UNSUBSCRIBE_EVENT:
+		return v4l2_subdev_call(sd, core, unsubscribe_event, fh, arg);
+
 	default:
 		return -ENOIOCTLCMD;
 	}
@@ -81,11 +136,29 @@ static long subdev_ioctl(struct file *file, unsigned int cmd,
 	return __video_usercopy(file, cmd, arg, subdev_do_ioctl);
 }
 
+static unsigned int subdev_poll(struct file *file, poll_table *wait)
+{
+	struct video_device *vdev = video_devdata(file);
+	struct v4l2_subdev *sd = vdev_to_v4l2_subdev(vdev);
+	struct v4l2_fh *fh = file->private_data;
+
+	if (!(sd->flags & V4L2_SUBDEV_FL_HAS_EVENTS))
+		return POLLERR;
+
+	poll_wait(file, &fh->events->wait, wait);
+
+	if (v4l2_event_pending(fh))
+		return POLLPRI;
+
+	return 0;
+}
+
 const struct v4l2_file_operations v4l2_subdev_fops = {
 	.owner = THIS_MODULE,
 	.open = subdev_open,
 	.unlocked_ioctl = subdev_ioctl,
 	.release = subdev_close,
+	.poll = subdev_poll,
 };
 
 void v4l2_subdev_init(struct v4l2_subdev *sd, const struct v4l2_subdev_ops *ops)
diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h
index 9ee45c8..55a8c93 100644
--- a/include/media/v4l2-subdev.h
+++ b/include/media/v4l2-subdev.h
@@ -36,6 +36,8 @@
 #define V4L2_SUBDEV_IR_TX_FIFO_SERVICE_REQ	0x00000001
 
 struct v4l2_device;
+struct v4l2_event_subscription;
+struct v4l2_fh;
 struct v4l2_subdev;
 struct tuner_setup;
 
@@ -134,6 +136,10 @@ struct v4l2_subdev_core_ops {
 	int (*s_register)(struct v4l2_subdev *sd, struct v4l2_dbg_register *reg);
 #endif
 	int (*s_power)(struct v4l2_subdev *sd, int on);
+	int (*subscribe_event)(struct v4l2_subdev *sd, struct v4l2_fh *fh,
+			       struct v4l2_event_subscription *sub);
+	int (*unsubscribe_event)(struct v4l2_subdev *sd, struct v4l2_fh *fh,
+				 struct v4l2_event_subscription *sub);
 };
 
 /* s_mode: switch the tuner to a specific tuner mode. Replacement of s_radio.
@@ -408,6 +414,8 @@ struct v4l2_subdev_ops {
 #define V4L2_SUBDEV_FL_IS_SPI			(1U << 1)
 /* Set this flag if this subdev needs a device node. */
 #define V4L2_SUBDEV_FL_HAS_DEVNODE		(1U << 2)
+/* Set this flag if this subdev generates events. */
+#define V4L2_SUBDEV_FL_HAS_EVENTS		(1U << 3)
 
 /* Each instance of a subdev driver should create this struct, either
    stand-alone or embedded in a larger struct.
@@ -427,6 +435,8 @@ struct v4l2_subdev {
 	/* subdev device node */
 	struct video_device devnode;
 	unsigned int initialized;
+	/* number of events to be allocated on open */
+	unsigned int nevents;
 };
 
 #define vdev_to_v4l2_subdev(vdev) \
-- 
1.7.1

Re: [RFC/PATCH v3 7/7] v4l: subdev: Events support

[Hans Verkuil]

On Monday 12 July 2010 17:25:52 Laurent Pinchart wrote:
> From: Sakari Ailus <sakari.ailus@maxwell.research.nokia.com>
> 
> Provide v4l2_subdevs with v4l2_event support. Subdev drivers only need very
> little to support events.
> 
> Signed-off-by: Sakari Ailus <sakari.ailus@maxwell.research.nokia.com>
> Signed-off-by: David Cohen <david.cohen@nokia.com>
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

Acked-by: Hans Verkuil <hverkuil@xs4all.nl>

> ---
>  Documentation/video4linux/v4l2-framework.txt |   18 ++++++
>  drivers/media/video/v4l2-subdev.c            |   75 +++++++++++++++++++++++++-
>  include/media/v4l2-subdev.h                  |   10 ++++
>  3 files changed, 102 insertions(+), 1 deletions(-)
> 
> diff --git a/Documentation/video4linux/v4l2-framework.txt b/Documentation/video4linux/v4l2-framework.txt
> index 9c3f33c..89bd881 100644
> --- a/Documentation/video4linux/v4l2-framework.txt
> +++ b/Documentation/video4linux/v4l2-framework.txt
> @@ -347,6 +347,24 @@ VIDIOC_TRY_EXT_CTRLS
>  	controls can be also be accessed through one (or several) V4L2 device
>  	nodes.
>  
> +VIDIOC_DQEVENT
> +VIDIOC_SUBSCRIBE_EVENT
> +VIDIOC_UNSUBSCRIBE_EVENT
> +
> +	The events ioctls are identical to the ones defined in V4L2. They
> +	behave identically, with the only exception that they deal only with
> +	events generated by the sub-device. Depending on the driver, those
> +	events can also be reported by one (or several) V4L2 device nodes.
> +
> +	Sub-device drivers that want to use events need to set the
> +	V4L2_SUBDEV_USES_EVENTS v4l2_subdev::flags and initialize
> +	v4l2_subdev::nevents to events queue depth before registering the
> +	sub-device. After registration events can be queued as usual on the
> +	v4l2_subdev::devnode device node.
> +
> +	To properly support events, the poll() file operation is also
> +	implemented.
> +
>  
>  I2C sub-device drivers
>  ----------------------
> diff --git a/drivers/media/video/v4l2-subdev.c b/drivers/media/video/v4l2-subdev.c
> index ea3941a..b063195 100644
> --- a/drivers/media/video/v4l2-subdev.c
> +++ b/drivers/media/video/v4l2-subdev.c
> @@ -18,26 +18,68 @@
>   *  Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
>   */
>  
> -#include <linux/types.h>
>  #include <linux/ioctl.h>
> +#include <linux/slab.h>
> +#include <linux/types.h>
>  #include <linux/videodev2.h>
>  
>  #include <media/v4l2-device.h>
>  #include <media/v4l2-ioctl.h>
> +#include <media/v4l2-fh.h>
> +#include <media/v4l2-event.h>
>  
>  static int subdev_open(struct file *file)
>  {
>  	struct video_device *vdev = video_devdata(file);
>  	struct v4l2_subdev *sd = vdev_to_v4l2_subdev(vdev);
> +	struct v4l2_fh *vfh;
> +	int ret;
>  
>  	if (!sd->initialized)
>  		return -EAGAIN;
>  
> +	if (sd->flags & V4L2_SUBDEV_FL_HAS_EVENTS) {
> +		vfh = kzalloc(sizeof(*vfh), GFP_KERNEL);
> +		if (vfh == NULL)
> +			return -ENOMEM;
> +
> +		ret = v4l2_fh_init(vfh, vdev);
> +		if (ret)
> +			goto err;
> +
> +		ret = v4l2_event_init(vfh);
> +		if (ret)
> +			goto err;
> +
> +		ret = v4l2_event_alloc(vfh, sd->nevents);
> +		if (ret)
> +			goto err;
> +
> +		v4l2_fh_add(vfh);
> +		file->private_data = vfh;
> +	}
> +
>  	return 0;
> +
> +err:
> +	if (vfh != NULL) {
> +		v4l2_fh_exit(vfh);
> +		kfree(vfh);
> +	}
> +
> +	return ret;
>  }
>  
>  static int subdev_close(struct file *file)
>  {
> +	struct v4l2_fh *vfh = file->private_data;
> +
> +	if (vfh != NULL) {
> +		v4l2_fh_del(vfh);
> +		v4l2_fh_exit(vfh);
> +		kfree(vfh);
> +	}
> +
>  	return 0;
>  }
>  
> @@ -45,6 +87,7 @@ 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 *fh = file->private_data;
>  
>  	switch (cmd) {
>  	case VIDIOC_QUERYCTRL:
> @@ -68,6 +111,18 @@ static long subdev_do_ioctl(struct file *file, unsigned int cmd, void *arg)
>  	case VIDIOC_TRY_EXT_CTRLS:
>  		return v4l2_subdev_call(sd, core, try_ext_ctrls, arg);
>  
> +	case VIDIOC_DQEVENT:
> +		if (!(sd->flags & V4L2_SUBDEV_FL_HAS_EVENTS))
> +			return -ENOIOCTLCMD;
> +
> +		return v4l2_event_dequeue(fh, arg, file->f_flags & O_NONBLOCK);
> +
> +	case VIDIOC_SUBSCRIBE_EVENT:
> +		return v4l2_subdev_call(sd, core, subscribe_event, fh, arg);
> +
> +	case VIDIOC_UNSUBSCRIBE_EVENT:
> +		return v4l2_subdev_call(sd, core, unsubscribe_event, fh, arg);
> +
>  	default:
>  		return -ENOIOCTLCMD;
>  	}
> @@ -81,11 +136,29 @@ static long subdev_ioctl(struct file *file, unsigned int cmd,
>  	return __video_usercopy(file, cmd, arg, subdev_do_ioctl);
>  }
>  
> +static unsigned int subdev_poll(struct file *file, poll_table *wait)
> +{
> +	struct video_device *vdev = video_devdata(file);
> +	struct v4l2_subdev *sd = vdev_to_v4l2_subdev(vdev);
> +	struct v4l2_fh *fh = file->private_data;
> +
> +	if (!(sd->flags & V4L2_SUBDEV_FL_HAS_EVENTS))
> +		return POLLERR;
> +
> +	poll_wait(file, &fh->events->wait, wait);
> +
> +	if (v4l2_event_pending(fh))
> +		return POLLPRI;
> +
> +	return 0;
> +}
> +
>  const struct v4l2_file_operations v4l2_subdev_fops = {
>  	.owner = THIS_MODULE,
>  	.open = subdev_open,
>  	.unlocked_ioctl = subdev_ioctl,
>  	.release = subdev_close,
> +	.poll = subdev_poll,
>  };
>  
>  void v4l2_subdev_init(struct v4l2_subdev *sd, const struct v4l2_subdev_ops *ops)
> diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h
> index 9ee45c8..55a8c93 100644
> --- a/include/media/v4l2-subdev.h
> +++ b/include/media/v4l2-subdev.h
> @@ -36,6 +36,8 @@
>  #define V4L2_SUBDEV_IR_TX_FIFO_SERVICE_REQ	0x00000001
>  
>  struct v4l2_device;
> +struct v4l2_event_subscription;
> +struct v4l2_fh;
>  struct v4l2_subdev;
>  struct tuner_setup;
>  
> @@ -134,6 +136,10 @@ struct v4l2_subdev_core_ops {
>  	int (*s_register)(struct v4l2_subdev *sd, struct v4l2_dbg_register *reg);
>  #endif
>  	int (*s_power)(struct v4l2_subdev *sd, int on);
> +	int (*subscribe_event)(struct v4l2_subdev *sd, struct v4l2_fh *fh,
> +			       struct v4l2_event_subscription *sub);
> +	int (*unsubscribe_event)(struct v4l2_subdev *sd, struct v4l2_fh *fh,
> +				 struct v4l2_event_subscription *sub);
>  };
>  
>  /* s_mode: switch the tuner to a specific tuner mode. Replacement of s_radio.
> @@ -408,6 +414,8 @@ struct v4l2_subdev_ops {
>  #define V4L2_SUBDEV_FL_IS_SPI			(1U << 1)
>  /* Set this flag if this subdev needs a device node. */
>  #define V4L2_SUBDEV_FL_HAS_DEVNODE		(1U << 2)
> +/* Set this flag if this subdev generates events. */
> +#define V4L2_SUBDEV_FL_HAS_EVENTS		(1U << 3)
>  
>  /* Each instance of a subdev driver should create this struct, either
>     stand-alone or embedded in a larger struct.
> @@ -427,6 +435,8 @@ struct v4l2_subdev {
>  	/* subdev device node */
>  	struct video_device devnode;
>  	unsigned int initialized;
> +	/* number of events to be allocated on open */
> +	unsigned int nevents;
>  };
>  
>  #define vdev_to_v4l2_subdev(vdev) \
> 

-- 
Hans Verkuil - video4linux developer - sponsored by TANDBERG, part of Cisco

Re: [RFC/PATCH v3 0/7] V4L2 subdev userspace API

[Laurent Pinchart]

Hi,

On Monday 12 July 2010 17:25:45 Laurent Pinchart wrote:
> Hi everybody,
> 
> Here's the third version of the V4L2 subdev userspace API patches. Comments
> received on the first and second versions have been incorporated, including
> the video_usercopy usage. The generic ioctls support patch has been
> dropped and will be resubmitted later with a use case.

Mauro, is there a chance those patches could get in 2.6.36 ?

Hans, you mentioned you had no more comments, could you ack them ?

-- 
Regards,

Laurent Pinchart

Re: [RFC/PATCH v3 0/7] V4L2 subdev userspace API

[Mauro Carvalho Chehab]

Em 04-08-2010 09:46, Laurent Pinchart escreveu:
> Hi,
> 
> On Monday 12 July 2010 17:25:45 Laurent Pinchart wrote:
>> Hi everybody,
>>
>> Here's the third version of the V4L2 subdev userspace API patches. Comments
>> received on the first and second versions have been incorporated, including
>> the video_usercopy usage. The generic ioctls support patch has been
>> dropped and will be resubmitted later with a use case.
> 
> Mauro, is there a chance those patches could get in 2.6.36 ?

Unfortunately, the changes are not high. 

I still have lots of patches ready to merge that I received before the start of the 
merge window waiting for me to handle. As you know, we should first send the patches 
to linux-next, and wait for a while, before sending them upstream. I won't doubt that 
this time, we'll have only a 7-days window before -rc1.

There are also some other dead lines for this week, including the review of LPC proposals.

Finally, one requirement for merging API additions is to have a driver using it. 
In the past, we had bad experiences of adding things at the kernel API, but waiting
for a very long time for a kernel driver using it, as the ones that pushed hard for
adding the new API's didn't submitted their drivers timely (on some cased it ended by 
having a driver using the new API's several kernel versions later).
So, even considering that subdev API is ready, we still need to wait for drivers needing
it to be submitted. So, the better is to analyse and apply it after the end of the merge window,
on a separate branch, merging it at the main branch after receiving a driver needing
the new API.

Cheers,
Mauro.

Re: [RFC/PATCH v3 0/7] V4L2 subdev userspace API

[Laurent Pinchart]

Hi Mauro,

On Wednesday 04 August 2010 17:38:04 Mauro Carvalho Chehab wrote:
> Em 04-08-2010 09:46, Laurent Pinchart escreveu:
> > On Monday 12 July 2010 17:25:45 Laurent Pinchart wrote:
> >> Hi everybody,
> >> 
> >> Here's the third version of the V4L2 subdev userspace API patches.
> >> Comments received on the first and second versions have been
> >> incorporated, including the video_usercopy usage. The generic ioctls
> >> support patch has been dropped and will be resubmitted later with a use
> >> case.
> > 
> > Mauro, is there a chance those patches could get in 2.6.36 ?
> 
> Unfortunately, the changes are not high.
> 
> I still have lots of patches ready to merge that I received before the
> start of the merge window waiting for me to handle. As you know, we should
> first send the patches to linux-next, and wait for a while, before sending
> them upstream. I won't doubt that this time, we'll have only a 7-days
> window before -rc1.
> 
> There are also some other dead lines for this week, including the review of
> LPC proposals.
> 
> Finally, one requirement for merging API additions is to have a driver
> using it. In the past, we had bad experiences of adding things at the
> kernel API, but waiting for a very long time for a kernel driver using it,
> as the ones that pushed hard for adding the new API's didn't submitted
> their drivers timely (on some cased it ended by having a driver using the
> new API's several kernel versions later). So, even considering that subdev
> API is ready, we still need to wait for drivers needing it to be
> submitted. So, the better is to analyse and apply it after the end of the
> merge window, on a separate branch, merging it at the main branch after
> receiving a driver needing the new API.

OK, understood.

I would still like to get your comments on the patches, if any. Hans has acked 
them, and I'd like to make sure you're fine with them as well before pushing 
them internally (the less I break the API/ABI, the better for our userspace 
developers :-)). I will then keep the patches up-to-date with the mainline 
kernel until the OMAP3 ISP driver (and the media controller) is ready for 
submission.

-- 
Regards,

Laurent Pinchart

Re: [RFC/PATCH v3 0/7] V4L2 subdev userspace API

[Mauro Carvalho Chehab]

Em 04-08-2010 17:29, Laurent Pinchart escreveu:
> Hi Mauro,
> 
> On Wednesday 04 August 2010 17:38:04 Mauro Carvalho Chehab wrote:
>> Em 04-08-2010 09:46, Laurent Pinchart escreveu:
>>> On Monday 12 July 2010 17:25:45 Laurent Pinchart wrote:
>>>> Hi everybody,
>>>>
>>>> Here's the third version of the V4L2 subdev userspace API patches.
>>>> Comments received on the first and second versions have been
>>>> incorporated, including the video_usercopy usage. The generic ioctls
>>>> support patch has been dropped and will be resubmitted later with a use
>>>> case.
>>>
>>> Mauro, is there a chance those patches could get in 2.6.36 ?
>>
>> Unfortunately, the changes are not high.
		      =======
			chances - unfortunately typo ;)
>>
>> I still have lots of patches ready to merge that I received before the
>> start of the merge window waiting for me to handle. As you know, we should
>> first send the patches to linux-next, and wait for a while, before sending
>> them upstream. I won't doubt that this time, we'll have only a 7-days
>> window before -rc1.
>>
>> There are also some other dead lines for this week, including the review of
>> LPC proposals.
>>
>> Finally, one requirement for merging API additions is to have a driver
>> using it. In the past, we had bad experiences of adding things at the
>> kernel API, but waiting for a very long time for a kernel driver using it,
>> as the ones that pushed hard for adding the new API's didn't submitted
>> their drivers timely (on some cased it ended by having a driver using the
>> new API's several kernel versions later). So, even considering that subdev
>> API is ready, we still need to wait for drivers needing it to be
>> submitted. So, the better is to analyse and apply it after the end of the
>> merge window, on a separate branch, merging it at the main branch after
>> receiving a driver needing the new API.
> 
> OK, understood.
> 
> I would still like to get your comments on the patches, if any. Hans has acked 
> them, and I'd like to make sure you're fine with them as well before pushing 
> them internally (the less I break the API/ABI, the better for our userspace 
> developers :-)). I will then keep the patches up-to-date with the mainline 
> kernel until the OMAP3 ISP driver (and the media controller) is ready for 
> submission.

Sure, I'll carefully review it and provide my comments. This is
on my TODO list.

Cheers,
Mauro