Re: [RFCv1 PATCH 2/8] v4l2-ctrls/event: remove struct v4l2_ctrl_fh, instead use v4l2_subscribed_event

[Laurent Pinchart]

Hi Hans,

Thanks for the patch.

On Tuesday 14 June 2011 17:22:27 Hans Verkuil wrote:
 From: Hans Verkuil hans.verk...@cisco.com
 
 The v4l2_ctrl_fh struct connected v4l2_ctrl with v4l2_fh so the control
 would know which filehandles subscribed to it. However, it is much easier
 to use struct v4l2_subscribed_event directly for that and get rid of that
 intermediate struct.

[snip]

 diff --git a/include/media/v4l2-event.h b/include/media/v4l2-event.h
 index 042b893..eda17f8 100644
 --- a/include/media/v4l2-event.h
 +++ b/include/media/v4l2-event.h
 @@ -38,9 +38,18 @@ struct v4l2_kevent {
  };
 
  struct v4l2_subscribed_event {
 + /* list node for the v4l2_fh-subscribed list */
   struct list_headlist;
 + /* event type */
   u32 type;
 + /* associated object ID (e.g. control ID) */
   u32 id;
 + /* copy of v4l2_event_subscription-flags */
 + u32 flags;
 + /* filehandle that subscribed to this event */
 + struct v4l2_fh  *fh;
 + /* list node that hooks into the object's event list (if there is one) 
 */
 + struct list_headnode;
  };
 
  int v4l2_event_alloc(struct v4l2_fh *fh, unsigned int n);

What about using kerneldoc style and document the structure in a single 
comment block right above it ? I find it easier to read.

-- 
Regards,

Laurent Pinchart
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[RFCv1 PATCH 2/8] v4l2-ctrls/event: remove struct v4l2_ctrl_fh, instead use v4l2_subscribed_event

[Hans Verkuil]

From: Hans Verkuil hans.verk...@cisco.com

The v4l2_ctrl_fh struct connected v4l2_ctrl with v4l2_fh so the control
would know which filehandles subscribed to it. However, it is much easier
to use struct v4l2_subscribed_event directly for that and get rid of that
intermediate struct.

Signed-off-by: Hans Verkuil hans.verk...@cisco.com
---
 drivers/media/video/v4l2-ctrls.c |   50 ++---
 drivers/media/video/v4l2-event.c |   34 +
 include/media/v4l2-ctrls.h   |   17 +---
 include/media/v4l2-event.h   |9 +++
 4 files changed, 47 insertions(+), 63 deletions(-)

diff --git a/drivers/media/video/v4l2-ctrls.c b/drivers/media/video/v4l2-ctrls.c
index f581910..079f952 100644
--- a/drivers/media/video/v4l2-ctrls.c
+++ b/drivers/media/video/v4l2-ctrls.c
@@ -581,15 +581,15 @@ static void fill_event(struct v4l2_event *ev, struct 
v4l2_ctrl *ctrl, u32 change
 static void send_event(struct v4l2_fh *fh, struct v4l2_ctrl *ctrl, u32 changes)
 {
struct v4l2_event ev;
-   struct v4l2_ctrl_fh *pos;
+   struct v4l2_subscribed_event *sev;
 
-   if (list_empty(ctrl-fhs))
+   if (list_empty(ctrl-ev_subs))
return;
fill_event(ev, ctrl, changes);
 
-   list_for_each_entry(pos, ctrl-fhs, node)
-   if (pos-fh != fh)
-   v4l2_event_queue_fh(pos-fh, ev);
+   list_for_each_entry(sev, ctrl-ev_subs, node)
+   if (sev-fh  sev-fh != fh)
+   v4l2_event_queue_fh(sev-fh, ev);
 }
 
 /* Helper function: copy the current control value back to the caller */
@@ -867,7 +867,7 @@ void v4l2_ctrl_handler_free(struct v4l2_ctrl_handler *hdl)
 {
struct v4l2_ctrl_ref *ref, *next_ref;
struct v4l2_ctrl *ctrl, *next_ctrl;
-   struct v4l2_ctrl_fh *ctrl_fh, *next_ctrl_fh;
+   struct v4l2_subscribed_event *sev, *next_sev;
 
if (hdl == NULL || hdl-buckets == NULL)
return;
@@ -881,10 +881,8 @@ void v4l2_ctrl_handler_free(struct v4l2_ctrl_handler *hdl)
/* Free all controls owned by the handler */
list_for_each_entry_safe(ctrl, next_ctrl, hdl-ctrls, node) {
list_del(ctrl-node);
-   list_for_each_entry_safe(ctrl_fh, next_ctrl_fh, ctrl-fhs, 
node) {
-   list_del(ctrl_fh-node);
-   kfree(ctrl_fh);
-   }
+   list_for_each_entry_safe(sev, next_sev, ctrl-ev_subs, node)
+   list_del(sev-node);
kfree(ctrl);
}
kfree(hdl-buckets);
@@ -1084,7 +1082,7 @@ static struct v4l2_ctrl *v4l2_ctrl_new(struct 
v4l2_ctrl_handler *hdl,
}
 
INIT_LIST_HEAD(ctrl-node);
-   INIT_LIST_HEAD(ctrl-fhs);
+   INIT_LIST_HEAD(ctrl-ev_subs);
ctrl-handler = hdl;
ctrl-ops = ops;
ctrl-id = id;
@@ -2027,41 +2025,31 @@ int v4l2_ctrl_s_ctrl(struct v4l2_ctrl *ctrl, s32 val)
 }
 EXPORT_SYMBOL(v4l2_ctrl_s_ctrl);
 
-void v4l2_ctrl_add_fh(struct v4l2_ctrl_handler *hdl,
-   struct v4l2_ctrl_fh *ctrl_fh,
-   struct v4l2_event_subscription *sub)
+void v4l2_ctrl_add_event(struct v4l2_ctrl *ctrl,
+   struct v4l2_subscribed_event *sev)
 {
-   struct v4l2_ctrl *ctrl = v4l2_ctrl_find(hdl, sub-id);
-
v4l2_ctrl_lock(ctrl);
-   list_add_tail(ctrl_fh-node, ctrl-fhs);
+   list_add_tail(sev-node, ctrl-ev_subs);
if (ctrl-type != V4L2_CTRL_TYPE_CTRL_CLASS 
-   (sub-flags  V4L2_EVENT_SUB_FL_SEND_INITIAL)) {
+   (sev-flags  V4L2_EVENT_SUB_FL_SEND_INITIAL)) {
struct v4l2_event ev;
 
fill_event(ev, ctrl, V4L2_EVENT_CTRL_CH_VALUE |
V4L2_EVENT_CTRL_CH_FLAGS);
-   v4l2_event_queue_fh(ctrl_fh-fh, ev);
+   v4l2_event_queue_fh(sev-fh, ev);
}
v4l2_ctrl_unlock(ctrl);
 }
-EXPORT_SYMBOL(v4l2_ctrl_add_fh);
+EXPORT_SYMBOL(v4l2_ctrl_add_event);
 
-void v4l2_ctrl_del_fh(struct v4l2_ctrl *ctrl, struct v4l2_fh *fh)
+void v4l2_ctrl_del_event(struct v4l2_ctrl *ctrl,
+   struct v4l2_subscribed_event *sev)
 {
-   struct v4l2_ctrl_fh *pos;
-
v4l2_ctrl_lock(ctrl);
-   list_for_each_entry(pos, ctrl-fhs, node) {
-   if (pos-fh == fh) {
-   list_del(pos-node);
-   kfree(pos);
-   break;
-   }
-   }
+   list_del(sev-node);
v4l2_ctrl_unlock(ctrl);
 }
-EXPORT_SYMBOL(v4l2_ctrl_del_fh);
+EXPORT_SYMBOL(v4l2_ctrl_del_event);
 
 int v4l2_ctrl_subscribe_fh(struct v4l2_fh *fh,
struct v4l2_event_subscription *sub, unsigned n)
diff --git a/drivers/media/video/v4l2-event.c b/drivers/media/video/v4l2-event.c
index 70fa82d..dc68f60 100644
--- a/drivers/media/video/v4l2-event.c
+++ b/drivers/media/video/v4l2-event.c
@@ -213,7 +213,6 @@ int v4l2_event_subscribe(struct