[4/8] V4L: Events: Support event handling in do_ioctl

Message ID 1265479331-20595-4-git-send-email-sakari.ailus@maxwell.research.nokia.com (mailing list archive)
State Superseded, archived
Headers

Commit Message

Sakari Ailus Feb. 6, 2010, 6:02 p.m. UTC
  Add support for event handling to do_ioctl.

Signed-off-by: Sakari Ailus <sakari.ailus@maxwell.research.nokia.com>
---
 drivers/media/video/Makefile     |    2 +-
 drivers/media/video/v4l2-ioctl.c |   48 ++++++++++++++++++++++++++++++++++++++
 include/media/v4l2-ioctl.h       |    9 +++++++
 3 files changed, 58 insertions(+), 1 deletions(-)
  

Comments

Sakari Ailus Feb. 7, 2010, 12:15 p.m. UTC | #1
Sakari Ailus wrote:
> Add support for event handling to do_ioctl.
> 
> Signed-off-by: Sakari Ailus <sakari.ailus@maxwell.research.nokia.com>
...
> @@ -239,6 +241,13 @@ struct v4l2_ioctl_ops {
>  	int (*vidioc_enum_frameintervals) (struct file *file, void *fh,
>  					   struct v4l2_frmivalenum *fival);
>  
> +	int (*vidioc_dqevent)	       (struct v4l2_fh *fh,
> +					struct v4l2_event *ev);
> +	int (*vidioc_subscribe_event)  (struct v4l2_fh *fh,
> +					struct v4l2_event_subscription *sub);
> +	int (*vidioc_unsubscribe_event) (struct v4l2_fh *fh,
> +					 struct v4l2_event_subscription *sub);
> +
>  	/* For other private ioctls */
>  	long (*vidioc_default)	       (struct file *file, void *fh,
>  					int cmd, void *arg);

Replying to myself, there seems to be valid use for the struct file as
an argument to the function fields. That is a way to get the video
device pointer using video_devdata(). Otherwise the video device pointer
has to be stored in the file handle instead.

So I'm going to add the file pointers as first arguments here as they
are in other functions unless there are objections. The type of second
argument is still going to be struct v4l2_fh *.

Regards,
  
Hans Verkuil Feb. 7, 2010, 12:22 p.m. UTC | #2
On Sunday 07 February 2010 13:15:12 Sakari Ailus wrote:
> Sakari Ailus wrote:
> > Add support for event handling to do_ioctl.
> > 
> > Signed-off-by: Sakari Ailus <sakari.ailus@maxwell.research.nokia.com>
> ...
> > @@ -239,6 +241,13 @@ struct v4l2_ioctl_ops {
> >  	int (*vidioc_enum_frameintervals) (struct file *file, void *fh,
> >  					   struct v4l2_frmivalenum *fival);
> >  
> > +	int (*vidioc_dqevent)	       (struct v4l2_fh *fh,
> > +					struct v4l2_event *ev);
> > +	int (*vidioc_subscribe_event)  (struct v4l2_fh *fh,
> > +					struct v4l2_event_subscription *sub);
> > +	int (*vidioc_unsubscribe_event) (struct v4l2_fh *fh,
> > +					 struct v4l2_event_subscription *sub);
> > +
> >  	/* For other private ioctls */
> >  	long (*vidioc_default)	       (struct file *file, void *fh,
> >  					int cmd, void *arg);
> 
> Replying to myself, there seems to be valid use for the struct file as
> an argument to the function fields. That is a way to get the video
> device pointer using video_devdata(). Otherwise the video device pointer
> has to be stored in the file handle instead.
> 
> So I'm going to add the file pointers as first arguments here as they
> are in other functions unless there are objections. The type of second
> argument is still going to be struct v4l2_fh *.

No, instead v4l2_fh should have a pointer to video_device. Much cleaner and
consistent with the other v4l2 internal structures.

And see also my review I'm posting soon.

Regards,

	Hans

> 
> Regards,
> 
>
  
Sakari Ailus Feb. 7, 2010, 6:30 p.m. UTC | #3
Hans Verkuil wrote:
> On Sunday 07 February 2010 13:15:12 Sakari Ailus wrote:
>> Sakari Ailus wrote:
>>> Add support for event handling to do_ioctl.
>>>
>>> Signed-off-by: Sakari Ailus <sakari.ailus@maxwell.research.nokia.com>
>> ...
>>> @@ -239,6 +241,13 @@ struct v4l2_ioctl_ops {
>>>  	int (*vidioc_enum_frameintervals) (struct file *file, void *fh,
>>>  					   struct v4l2_frmivalenum *fival);
>>>  
>>> +	int (*vidioc_dqevent)	       (struct v4l2_fh *fh,
>>> +					struct v4l2_event *ev);
>>> +	int (*vidioc_subscribe_event)  (struct v4l2_fh *fh,
>>> +					struct v4l2_event_subscription *sub);
>>> +	int (*vidioc_unsubscribe_event) (struct v4l2_fh *fh,
>>> +					 struct v4l2_event_subscription *sub);
>>> +
>>>  	/* For other private ioctls */
>>>  	long (*vidioc_default)	       (struct file *file, void *fh,
>>>  					int cmd, void *arg);
>>
>> Replying to myself, there seems to be valid use for the struct file as
>> an argument to the function fields. That is a way to get the video
>> device pointer using video_devdata(). Otherwise the video device pointer
>> has to be stored in the file handle instead.
>>
>> So I'm going to add the file pointers as first arguments here as they
>> are in other functions unless there are objections. The type of second
>> argument is still going to be struct v4l2_fh *.
> 
> No, instead v4l2_fh should have a pointer to video_device. Much cleaner and
> consistent with the other v4l2 internal structures.

Right. Fixed that. There's now struct video_device pointer in struct
v4l2_fh.
  

Patch

diff --git a/drivers/media/video/Makefile b/drivers/media/video/Makefile
index b888ad1..68253d6 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-subdev.o \
-			v4l2-fh.o
+			v4l2-fh.o v4l2-event.o
 
 # V4L2 core modules
 
diff --git a/drivers/media/video/v4l2-ioctl.c b/drivers/media/video/v4l2-ioctl.c
index bfc4696..6964bcc 100644
--- a/drivers/media/video/v4l2-ioctl.c
+++ b/drivers/media/video/v4l2-ioctl.c
@@ -1797,7 +1797,55 @@  static long __video_do_ioctl(struct file *file,
 		}
 		break;
 	}
+	case VIDIOC_DQEVENT:
+	{
+		struct v4l2_event *ev = arg;
+
+		if (!ops->vidioc_dqevent)
+			break;
+
+		ret = ops->vidioc_dqevent(file->private_data, ev);
+		if (ret < 0) {
+			dbgarg(cmd, "no pending events?");
+			break;
+		}
+		dbgarg(cmd,
+		       "count=%d, type=0x%8.8x, sequence=%d, "
+		       "timestamp=%lu.%9.9lu ",
+		       ev->count, ev->type, ev->sequence,
+		       ev->timestamp.tv_sec, ev->timestamp.tv_nsec);
+		break;
+	}
+	case VIDIOC_SUBSCRIBE_EVENT:
+	{
+		struct v4l2_event_subscription *sub = arg;
 
+		if (!ops->vidioc_subscribe_event)
+			break;
+
+		ret = ops->vidioc_subscribe_event(file->private_data, sub);
+		if (ret < 0) {
+			dbgarg(cmd, "failed, ret=%ld", ret);
+			break;
+		}
+		dbgarg(cmd, "type=0x%8.8x", sub->type);
+		break;
+	}
+	case VIDIOC_UNSUBSCRIBE_EVENT:
+	{
+		struct v4l2_event_subscription *sub = arg;
+
+		if (!ops->vidioc_unsubscribe_event)
+			break;
+
+		ret = ops->vidioc_unsubscribe_event(file->private_data, sub);
+		if (ret < 0) {
+			dbgarg(cmd, "failed, ret=%ld", ret);
+			break;
+		}
+		dbgarg(cmd, "type=0x%8.8x", sub->type);
+		break;
+	}
 	default:
 	{
 		if (!ops->vidioc_default)
diff --git a/include/media/v4l2-ioctl.h b/include/media/v4l2-ioctl.h
index 7a4529d..29dd64b 100644
--- a/include/media/v4l2-ioctl.h
+++ b/include/media/v4l2-ioctl.h
@@ -21,6 +21,8 @@ 
 #include <linux/videodev2.h>
 #endif
 
+struct v4l2_fh;
+
 struct v4l2_ioctl_ops {
 	/* ioctl callbacks */
 
@@ -239,6 +241,13 @@  struct v4l2_ioctl_ops {
 	int (*vidioc_enum_frameintervals) (struct file *file, void *fh,
 					   struct v4l2_frmivalenum *fival);
 
+	int (*vidioc_dqevent)	       (struct v4l2_fh *fh,
+					struct v4l2_event *ev);
+	int (*vidioc_subscribe_event)  (struct v4l2_fh *fh,
+					struct v4l2_event_subscription *sub);
+	int (*vidioc_unsubscribe_event) (struct v4l2_fh *fh,
+					 struct v4l2_event_subscription *sub);
+
 	/* For other private ioctls */
 	long (*vidioc_default)	       (struct file *file, void *fh,
 					int cmd, void *arg);