Em Tue, 11 Mar 2014 21:43:47 +0100 Hans Verkuil <[email protected]> escreveu:
> Hi Mauro, > > On 03/11/2014 09:14 PM, Mauro Carvalho Chehab wrote: > > Em Mon, 17 Feb 2014 10:57:21 +0100 > > Hans Verkuil <[email protected]> escreveu: > > > >> From: Hans Verkuil <[email protected]> > >> > >> This patch implements initial support for complex types. > >> > >> For the most part the changes are fairly obvious (basic support for is_ptr > >> types, the type_is_int function is replaced by a is_int bitfield, and > >> v4l2_query_ext_ctrl is added), but one change needs more explanation: > >> > >> The v4l2_ctrl struct adds a 'new' field and a 'stores' array at the end > >> of the struct. This is in preparation for future patches where each control > >> can have multiple configuration stores. The idea is that stores[0] is the > >> current > >> control value, stores[1] etc. are the control values for each > >> configuration store > >> and the 'new' value can be accessed through 'stores[-1]', i.e. the 'new' > >> field. > >> However, for now only stores[-1] and stores[0] is used. > >> > >> Drivers are not expected to use the stores array (at least not while > >> support > >> for configuration stores is not yet fully added), but the stores array will > >> be used inside the control framework. > >> > >> These new fields use the v4l2_ctrl_ptr union, which is a pointer to a > >> control > >> value. > >> > >> Signed-off-by: Hans Verkuil <[email protected]> > >> Reviewed-by: Sylwester Nawrocki <[email protected]> > >> --- > >> drivers/media/v4l2-core/v4l2-ctrls.c | 236 > >> ++++++++++++++++++++++++++--------- > >> include/media/v4l2-ctrls.h | 38 +++++- > >> 2 files changed, 210 insertions(+), 64 deletions(-) > >> > >> diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c > >> b/drivers/media/v4l2-core/v4l2-ctrls.c > >> index df8ed0a..5d1eeea 100644 > >> --- a/drivers/media/v4l2-core/v4l2-ctrls.c > >> +++ b/drivers/media/v4l2-core/v4l2-ctrls.c > >> @@ -1095,20 +1095,6 @@ void v4l2_ctrl_fill(u32 id, const char **name, > >> const char **unit, > >> } > >> EXPORT_SYMBOL(v4l2_ctrl_fill); > >> > >> -/* Helper function to determine whether the control type is compatible > >> with > >> - VIDIOC_G/S_CTRL. */ > >> -static bool type_is_int(const struct v4l2_ctrl *ctrl) > >> -{ > >> - switch (ctrl->type) { > >> - case V4L2_CTRL_TYPE_INTEGER64: > >> - case V4L2_CTRL_TYPE_STRING: > >> - /* Nope, these need v4l2_ext_control */ > >> - return false; > >> - default: > >> - return true; > >> - } > >> -} > >> - > >> static void fill_event(struct v4l2_event *ev, struct v4l2_ctrl *ctrl, u32 > >> changes) > >> { > >> memset(ev->reserved, 0, sizeof(ev->reserved)); > >> @@ -1117,7 +1103,7 @@ static void fill_event(struct v4l2_event *ev, struct > >> v4l2_ctrl *ctrl, u32 change > >> ev->u.ctrl.changes = changes; > >> ev->u.ctrl.type = ctrl->type; > >> ev->u.ctrl.flags = ctrl->flags; > >> - if (ctrl->type == V4L2_CTRL_TYPE_STRING) > >> + if (ctrl->is_ptr) > >> ev->u.ctrl.value64 = 0; > >> else > >> ev->u.ctrl.value64 = ctrl->cur.val64; > >> @@ -1152,6 +1138,9 @@ static int cur_to_user(struct v4l2_ext_control *c, > >> { > >> u32 len; > >> > >> + if (ctrl->is_ptr && !ctrl->is_string) > >> + return copy_to_user(c->p, ctrl->cur.p, ctrl->elem_size); > >> + > >> switch (ctrl->type) { > >> case V4L2_CTRL_TYPE_STRING: > >> len = strlen(ctrl->cur.string); > >> @@ -1179,6 +1168,9 @@ static int user_to_new(struct v4l2_ext_control *c, > >> u32 size; > >> > >> ctrl->is_new = 1; > >> + if (ctrl->is_ptr && !ctrl->is_string) > >> + return copy_from_user(ctrl->p, c->p, ctrl->elem_size); > >> + > > > > It is a little hard to follow all copy_*_user and memcpy/memcpy cases. > > > > On all such calls, the elem_size data is coming from the driver? > > Absolutely. > > > It doesn't seem right to tust on userspace for that, if this could > > be a border case. > > > >> switch (ctrl->type) { > >> case V4L2_CTRL_TYPE_INTEGER64: > >> ctrl->val64 = c->value64; > >> @@ -1213,6 +1205,9 @@ static int new_to_user(struct v4l2_ext_control *c, > >> { > >> u32 len; > >> > >> + if (ctrl->is_ptr && !ctrl->is_string) > >> + return copy_to_user(c->p, ctrl->p, ctrl->elem_size); > >> + > >> switch (ctrl->type) { > >> case V4L2_CTRL_TYPE_STRING: > >> len = strlen(ctrl->string); > >> @@ -1239,6 +1234,7 @@ static void new_to_cur(struct v4l2_fh *fh, struct > >> v4l2_ctrl *ctrl, u32 ch_flags) > >> > >> if (ctrl == NULL) > >> return; > >> + > >> switch (ctrl->type) { > >> case V4L2_CTRL_TYPE_BUTTON: > >> changed = true; > >> @@ -1253,8 +1249,13 @@ static void new_to_cur(struct v4l2_fh *fh, struct > >> v4l2_ctrl *ctrl, u32 ch_flags) > >> ctrl->cur.val64 = ctrl->val64; > >> break; > >> default: > >> - changed = ctrl->val != ctrl->cur.val; > >> - ctrl->cur.val = ctrl->val; > >> + if (ctrl->is_ptr) { > >> + changed = memcmp(ctrl->p, ctrl->cur.p, ctrl->elem_size); > >> + memcpy(ctrl->cur.p, ctrl->p, ctrl->elem_size); > >> + } else { > >> + changed = ctrl->val != ctrl->cur.val; > >> + ctrl->cur.val = ctrl->val; > >> + } > >> break; > >> } > >> if (ch_flags & V4L2_EVENT_CTRL_CH_FLAGS) { > >> @@ -1294,7 +1295,10 @@ static void cur_to_new(struct v4l2_ctrl *ctrl) > >> ctrl->val64 = ctrl->cur.val64; > >> break; > >> default: > >> - ctrl->val = ctrl->cur.val; > >> + if (ctrl->is_ptr) > >> + memcpy(ctrl->p, ctrl->cur.p, ctrl->elem_size); > >> + else > >> + ctrl->val = ctrl->cur.val; > >> break; > >> } > >> } > >> @@ -1507,7 +1511,7 @@ static struct v4l2_ctrl_ref *find_private_ref( > >> VIDIOC_G/S_CTRL. */ > >> if (V4L2_CTRL_ID2CLASS(ref->ctrl->id) == V4L2_CTRL_CLASS_USER && > >> V4L2_CTRL_DRIVER_PRIV(ref->ctrl->id)) { > >> - if (!type_is_int(ref->ctrl)) > >> + if (!ref->ctrl->is_int) > >> continue; > >> if (id == 0) > >> return ref; > >> @@ -1577,8 +1581,12 @@ static int handler_new_ref(struct v4l2_ctrl_handler > >> *hdl, > >> u32 class_ctrl = V4L2_CTRL_ID2CLASS(id) | 1; > >> int bucket = id % hdl->nr_of_buckets; /* which bucket to use */ > >> > >> - /* Automatically add the control class if it is not yet present. */ > >> - if (id != class_ctrl && find_ref_lock(hdl, class_ctrl) == NULL) > >> + /* > >> + * Automatically add the control class if it is not yet present and > >> + * the new control is not hidden. > >> + */ > >> + if (!(ctrl->flags & V4L2_CTRL_FLAG_HIDDEN) && > >> + id != class_ctrl && find_ref_lock(hdl, class_ctrl) == NULL) > >> if (!v4l2_ctrl_new_std(hdl, NULL, class_ctrl, 0, 0, 0, 0)) > >> return hdl->error; > >> > >> @@ -1640,23 +1648,36 @@ static struct v4l2_ctrl *v4l2_ctrl_new(struct > >> v4l2_ctrl_handler *hdl, > >> u32 id, const char *name, const char *unit, > >> enum v4l2_ctrl_type type, > >> s64 min, s64 max, u64 step, s64 def, > >> + u32 elem_size, > >> u32 flags, const char * const *qmenu, > >> const s64 *qmenu_int, void *priv) > >> { > >> struct v4l2_ctrl *ctrl; > >> - unsigned sz_extra = 0; > >> + unsigned sz_extra; > >> + void *data; > >> int err; > >> > >> if (hdl->error) > >> return NULL; > >> > >> + if (type == V4L2_CTRL_TYPE_INTEGER64) > >> + elem_size = sizeof(s64); > >> + else if (type == V4L2_CTRL_TYPE_STRING) > >> + elem_size = max + 1; > >> + else if (type < V4L2_CTRL_COMPLEX_TYPES) > >> + elem_size = sizeof(s32); > >> + > >> /* Sanity checks */ > >> if (id == 0 || name == NULL || id >= V4L2_CID_PRIVATE_BASE || > >> + elem_size == 0 || > >> (type == V4L2_CTRL_TYPE_MENU && qmenu == NULL) || > >> (type == V4L2_CTRL_TYPE_INTEGER_MENU && qmenu_int == NULL)) { > >> handler_set_err(hdl, -ERANGE); > >> return NULL; > >> } > >> + /* Complex controls are always hidden */ > >> + if (type >= V4L2_CTRL_COMPLEX_TYPES) > >> + flags |= V4L2_CTRL_FLAG_HIDDEN; > >> err = check_range(type, min, max, step, def); > >> if (err) { > >> handler_set_err(hdl, err); > >> @@ -1667,12 +1688,13 @@ static struct v4l2_ctrl *v4l2_ctrl_new(struct > >> v4l2_ctrl_handler *hdl, > >> return NULL; > >> } > >> > >> + sz_extra = sizeof(union v4l2_ctrl_ptr); > >> if (type == V4L2_CTRL_TYPE_BUTTON) > >> flags |= V4L2_CTRL_FLAG_WRITE_ONLY; > >> else if (type == V4L2_CTRL_TYPE_CTRL_CLASS) > >> flags |= V4L2_CTRL_FLAG_READ_ONLY; > >> - else if (type == V4L2_CTRL_TYPE_STRING) > >> - sz_extra += 2 * (max + 1); > >> + else if (type == V4L2_CTRL_TYPE_STRING || type >= > >> V4L2_CTRL_COMPLEX_TYPES) > >> + sz_extra += 2 * elem_size; > >> > >> ctrl = kzalloc(sizeof(*ctrl) + sz_extra, GFP_KERNEL); > >> if (ctrl == NULL) { > >> @@ -1692,18 +1714,31 @@ static struct v4l2_ctrl *v4l2_ctrl_new(struct > >> v4l2_ctrl_handler *hdl, > >> ctrl->minimum = min; > >> ctrl->maximum = max; > >> ctrl->step = step; > >> + ctrl->default_value = def; > >> + ctrl->is_string = type == V4L2_CTRL_TYPE_STRING; > >> + ctrl->is_ptr = type >= V4L2_CTRL_COMPLEX_TYPES || ctrl->is_string; > >> + ctrl->is_int = !ctrl->is_ptr && type != V4L2_CTRL_TYPE_INTEGER64; > >> + ctrl->elem_size = elem_size; > >> if (type == V4L2_CTRL_TYPE_MENU) > >> ctrl->qmenu = qmenu; > >> else if (type == V4L2_CTRL_TYPE_INTEGER_MENU) > >> ctrl->qmenu_int = qmenu_int; > >> ctrl->priv = priv; > >> - ctrl->cur.val = ctrl->val = ctrl->default_value = def; > >> + ctrl->cur.val = ctrl->val = def; > >> + data = &ctrl->stores[1]; > >> + > >> + if (ctrl->is_string) { > >> + ctrl->string = ctrl->new.p_char = data; > >> + ctrl->stores[0].p_char = data + elem_size; > >> > >> - if (ctrl->type == V4L2_CTRL_TYPE_STRING) { > >> - ctrl->cur.string = (char *)&ctrl[1] + sz_extra - (max + 1); > >> - ctrl->string = (char *)&ctrl[1] + sz_extra - 2 * (max + 1); > >> if (ctrl->minimum) > >> memset(ctrl->cur.string, ' ', ctrl->minimum); > >> + } else if (ctrl->is_ptr) { > >> + ctrl->p = ctrl->new.p = data; > >> + ctrl->stores[0].p = data + elem_size; > >> + } else { > >> + ctrl->new.p = &ctrl->val; > >> + ctrl->stores[0].p = &ctrl->cur.val; > >> } > >> if (handler_new_ref(hdl, ctrl)) { > >> kfree(ctrl); > >> @@ -1752,7 +1787,8 @@ struct v4l2_ctrl *v4l2_ctrl_new_custom(struct > >> v4l2_ctrl_handler *hdl, > >> ctrl = v4l2_ctrl_new(hdl, cfg->ops, cfg->id, name, unit, > >> type, min, max, > >> is_menu ? cfg->menu_skip_mask : step, > >> - def, flags, qmenu, qmenu_int, priv); > >> + def, cfg->elem_size, > >> + flags, qmenu, qmenu_int, priv); > >> if (ctrl) > >> ctrl->is_private = cfg->is_private; > >> return ctrl; > >> @@ -1770,13 +1806,15 @@ struct v4l2_ctrl *v4l2_ctrl_new_std(struct > >> v4l2_ctrl_handler *hdl, > >> u32 flags; > >> > >> v4l2_ctrl_fill(id, &name, &unit, &type, &min, &max, &step, &def, > >> &flags); > >> - if (type == V4L2_CTRL_TYPE_MENU > >> - || type == V4L2_CTRL_TYPE_INTEGER_MENU) { > >> + if (type == V4L2_CTRL_TYPE_MENU || > >> + type == V4L2_CTRL_TYPE_INTEGER_MENU || > >> + type >= V4L2_CTRL_COMPLEX_TYPES) { > >> handler_set_err(hdl, -EINVAL); > >> return NULL; > >> } > >> return v4l2_ctrl_new(hdl, ops, id, name, unit, type, > >> - min, max, step, def, flags, NULL, NULL, NULL); > >> + min, max, step, def, 0, > >> + flags, NULL, NULL, NULL); > >> } > >> EXPORT_SYMBOL(v4l2_ctrl_new_std); > >> > >> @@ -1809,7 +1847,8 @@ struct v4l2_ctrl *v4l2_ctrl_new_std_menu(struct > >> v4l2_ctrl_handler *hdl, > >> return NULL; > >> } > >> return v4l2_ctrl_new(hdl, ops, id, name, unit, type, > >> - 0, max, mask, def, flags, qmenu, qmenu_int, NULL); > >> + 0, max, mask, def, 0, > >> + flags, qmenu, qmenu_int, NULL); > >> } > >> EXPORT_SYMBOL(v4l2_ctrl_new_std_menu); > >> > >> @@ -1841,7 +1880,7 @@ struct v4l2_ctrl > >> *v4l2_ctrl_new_std_menu_items(struct v4l2_ctrl_handler *hdl, > >> return NULL; > >> } > >> return v4l2_ctrl_new(hdl, ops, id, name, unit, type, 0, max, mask, def, > >> - flags, qmenu, NULL, NULL); > >> + 0, flags, qmenu, NULL, NULL); > >> > >> } > >> EXPORT_SYMBOL(v4l2_ctrl_new_std_menu_items); > >> @@ -1866,7 +1905,8 @@ struct v4l2_ctrl *v4l2_ctrl_new_int_menu(struct > >> v4l2_ctrl_handler *hdl, > >> return NULL; > >> } > >> return v4l2_ctrl_new(hdl, ops, id, name, unit, type, > >> - 0, max, 0, def, flags, NULL, qmenu_int, NULL); > >> + 0, max, 0, def, 0, > >> + flags, NULL, qmenu_int, NULL); > >> } > >> EXPORT_SYMBOL(v4l2_ctrl_new_int_menu); > >> > >> @@ -2154,9 +2194,10 @@ int v4l2_ctrl_handler_setup(struct > >> v4l2_ctrl_handler *hdl) > >> } > >> EXPORT_SYMBOL(v4l2_ctrl_handler_setup); > >> > >> -/* Implement VIDIOC_QUERYCTRL */ > >> -int v4l2_queryctrl(struct v4l2_ctrl_handler *hdl, struct v4l2_queryctrl > >> *qc) > >> +/* Implement VIDIOC_QUERY_EXT_CTRL */ > >> +int v4l2_query_ext_ctrl(struct v4l2_ctrl_handler *hdl, struct > >> v4l2_query_ext_ctrl *qc) > >> { > >> + const unsigned next_flags = V4L2_CTRL_FLAG_NEXT_CTRL | > >> V4L2_CTRL_FLAG_NEXT_HIDDEN; > >> u32 id = qc->id & V4L2_CTRL_ID_MASK; > >> struct v4l2_ctrl_ref *ref; > >> struct v4l2_ctrl *ctrl; > >> @@ -2169,7 +2210,22 @@ int v4l2_queryctrl(struct v4l2_ctrl_handler *hdl, > >> struct v4l2_queryctrl *qc) > >> /* Try to find it */ > >> ref = find_ref(hdl, id); > >> > >> - if ((qc->id & V4L2_CTRL_FLAG_NEXT_CTRL) && > >> !list_empty(&hdl->ctrl_refs)) { > >> + if ((qc->id & next_flags) && !list_empty(&hdl->ctrl_refs)) { > >> + unsigned mask; > >> + unsigned match; > >> + > >> + if ((qc->id & next_flags) == V4L2_CTRL_FLAG_NEXT_HIDDEN) { > >> + /* Match any hidden control */ > >> + mask = match = V4L2_CTRL_FLAG_HIDDEN; > >> + } else if ((qc->id & next_flags) == next_flags) { > >> + /* Match any control, hidden or not */ > >> + mask = match = 0; > >> + } else { > >> + /* Match any control that is not hidden */ > >> + mask = V4L2_CTRL_FLAG_HIDDEN; > >> + match = 0; > >> + } > >> + > >> /* Find the next control with ID > qc->id */ > >> > >> /* Did we reach the end of the control list? */ > >> @@ -2177,19 +2233,28 @@ int v4l2_queryctrl(struct v4l2_ctrl_handler *hdl, > >> struct v4l2_queryctrl *qc) > >> ref = NULL; /* Yes, so there is no next control */ > >> } else if (ref) { > >> /* We found a control with the given ID, so just get > >> - the next one in the list. */ > >> - ref = list_entry(ref->node.next, typeof(*ref), node); > >> + the next valid one in the list. */ > >> + list_for_each_entry_continue(ref, &hdl->ctrl_refs, node) > >> + if (id < ref->ctrl->id && > >> + (ref->ctrl->flags & mask) == match) > >> + break; > >> + if (&ref->node == &hdl->ctrl_refs) > >> + ref = NULL; > >> } else { > >> /* No control with the given ID exists, so start > >> searching for the next largest ID. We know there > >> is one, otherwise the first 'if' above would have > >> been true. */ > >> list_for_each_entry(ref, &hdl->ctrl_refs, node) > >> - if (id < ref->ctrl->id) > >> + if (id < ref->ctrl->id && > >> + (ref->ctrl->flags & mask) == match) > >> break; > >> + if (&ref->node == &hdl->ctrl_refs) > >> + ref = NULL; > >> } > >> } > >> mutex_unlock(hdl->lock); > >> + > >> if (!ref) > >> return -EINVAL; > >> > >> @@ -2200,23 +2265,66 @@ int v4l2_queryctrl(struct v4l2_ctrl_handler *hdl, > >> struct v4l2_queryctrl *qc) > >> else > >> qc->id = ctrl->id; > >> strlcpy(qc->name, ctrl->name, sizeof(qc->name)); > >> - qc->minimum = ctrl->minimum; > >> - qc->maximum = ctrl->maximum; > >> - qc->default_value = ctrl->default_value; > >> + qc->flags = ctrl->flags; > >> + qc->type = ctrl->type; > >> + if (ctrl->is_ptr) > >> + qc->flags |= V4L2_CTRL_FLAG_IS_PTR; > >> + if (ctrl->unit) > >> + strlcpy(qc->unit, ctrl->unit, sizeof(qc->unit)); > >> + qc->elem_size = ctrl->elem_size; > >> + qc->min.val = ctrl->minimum; > >> + qc->max.val = ctrl->maximum; > >> + qc->def.val = ctrl->default_value; > >> + qc->cols = qc->rows = 1; > >> if (ctrl->type == V4L2_CTRL_TYPE_MENU > >> || ctrl->type == V4L2_CTRL_TYPE_INTEGER_MENU) > >> - qc->step = 1; > >> + qc->step.val = 1; > >> else > >> - qc->step = ctrl->step; > >> - qc->flags = ctrl->flags; > >> - qc->type = ctrl->type; > >> + qc->step.val = ctrl->step; > >> + return 0; > >> +} > >> +EXPORT_SYMBOL(v4l2_query_ext_ctrl); > >> + > >> +/* Implement VIDIOC_QUERYCTRL */ > >> +int v4l2_queryctrl(struct v4l2_ctrl_handler *hdl, struct v4l2_queryctrl > >> *qc) > >> +{ > >> + struct v4l2_query_ext_ctrl qec = { qc->id }; > >> + int rc; > >> + > >> + /* VIDIOC_QUERYCTRL cannot be used to enumerate hidden controls */ > > > > Why not? > > Good question, actually. QUERYCTRL cannot properly represent the full > information > about compound types (let's call it that for now), so I sort of assumed you > wouldn't want to use it for this anyway. But on the other hand, you can just > fill in what you can and leave it at that. Userspace can call QUERY_EXT_CTRL > if > it wants the full information. > > I think it would be a good improvement to allow this for queryctrl as well. Well, I would remove that limit. I mean: if the user is explicitly using VIDIOC_QUERYCTRL to see the hidden controls, why not exposing them? They may not be a complex element that would require the extra data provided via the EXT_CTRL call. > > > > >> + if (qc->id & V4L2_CTRL_FLAG_NEXT_HIDDEN) > >> + return -EINVAL; > >> + rc = v4l2_query_ext_ctrl(hdl, &qec); > >> + if (rc) > >> + return rc; > >> + > >> + /* VIDIOC_QUERYCTRL is not allowed to see hidden controls */ > >> + if (qc->flags & V4L2_CTRL_FLAG_HIDDEN) > >> + return -EINVAL; > >> + qc->id = qec.id; > >> + qc->type = qec.type; > >> + qc->flags = qec.flags; > >> + strlcpy(qc->name, qec.name, sizeof(qc->name)); > >> + switch (qc->type) { > >> + case V4L2_CTRL_TYPE_INTEGER: > >> + case V4L2_CTRL_TYPE_BOOLEAN: > >> + case V4L2_CTRL_TYPE_MENU: > >> + case V4L2_CTRL_TYPE_INTEGER_MENU: > >> + case V4L2_CTRL_TYPE_STRING: > >> + case V4L2_CTRL_TYPE_BITMASK: > >> + qc->minimum = qec.min.val; > >> + qc->maximum = qec.max.val; > >> + qc->step = qec.step.val; > >> + qc->default_value = qec.def.val; > >> + break; > >> + } > >> return 0; > >> } > >> EXPORT_SYMBOL(v4l2_queryctrl); > >> > >> int v4l2_subdev_queryctrl(struct v4l2_subdev *sd, struct v4l2_queryctrl > >> *qc) > >> { > >> - if (qc->id & V4L2_CTRL_FLAG_NEXT_CTRL) > >> + if (qc->id & (V4L2_CTRL_FLAG_NEXT_CTRL | V4L2_CTRL_FLAG_NEXT_HIDDEN)) > >> return -EINVAL; > >> return v4l2_queryctrl(sd->ctrl_handler, qc); > >> } > >> @@ -2316,7 +2424,8 @@ EXPORT_SYMBOL(v4l2_subdev_querymenu); > >> Find the controls in the control array and do some basic checks. */ > >> static int prepare_ext_ctrls(struct v4l2_ctrl_handler *hdl, > >> struct v4l2_ext_controls *cs, > >> - struct v4l2_ctrl_helper *helpers) > >> + struct v4l2_ctrl_helper *helpers, > >> + bool get) > >> { > >> struct v4l2_ctrl_helper *h; > >> bool have_clusters = false; > >> @@ -2348,6 +2457,13 @@ static int prepare_ext_ctrls(struct > >> v4l2_ctrl_handler *hdl, > >> have_clusters = true; > >> if (ctrl->cluster[0] != ctrl) > >> ref = find_ref_lock(hdl, ctrl->cluster[0]->id); > >> + if (ctrl->is_ptr && !ctrl->is_string && c->size < > >> ctrl->elem_size) { > >> + if (get) { > >> + c->size = ctrl->elem_size; > >> + return -ENOSPC; > >> + } > >> + return -EFAULT; > >> + } > >> /* Store the ref to the master control of the cluster */ > >> h->mref = ref; > >> h->ctrl = ctrl; > >> @@ -2428,7 +2544,7 @@ int v4l2_g_ext_ctrls(struct v4l2_ctrl_handler *hdl, > >> struct v4l2_ext_controls *cs > >> return -ENOMEM; > >> } > >> > >> - ret = prepare_ext_ctrls(hdl, cs, helpers); > >> + ret = prepare_ext_ctrls(hdl, cs, helpers, true); > >> cs->error_idx = cs->count; > >> > >> for (i = 0; !ret && i < cs->count; i++) > >> @@ -2490,11 +2606,11 @@ static int get_ctrl(struct v4l2_ctrl *ctrl, struct > >> v4l2_ext_control *c) > >> int ret = 0; > >> int i; > >> > >> - /* String controls are not supported. The new_to_user() and > >> + /* Complex controls are not supported. The new_to_user() and > >> * cur_to_user() calls below would need to be modified not to access > >> * userspace memory when called from get_ctrl(). > >> */ > >> - if (ctrl->type == V4L2_CTRL_TYPE_STRING) > >> + if (!ctrl->is_int) > >> return -EINVAL; > >> > >> if (ctrl->flags & V4L2_CTRL_FLAG_WRITE_ONLY) > >> @@ -2520,7 +2636,7 @@ int v4l2_g_ctrl(struct v4l2_ctrl_handler *hdl, > >> struct v4l2_control *control) > >> struct v4l2_ext_control c; > >> int ret; > >> > >> - if (ctrl == NULL || !type_is_int(ctrl)) > >> + if (ctrl == NULL || !ctrl->is_int) > >> return -EINVAL; > >> ret = get_ctrl(ctrl, &c); > >> control->value = c.value; > >> @@ -2539,7 +2655,7 @@ s32 v4l2_ctrl_g_ctrl(struct v4l2_ctrl *ctrl) > >> struct v4l2_ext_control c; > >> > >> /* It's a driver bug if this happens. */ > >> - WARN_ON(!type_is_int(ctrl)); > >> + WARN_ON(!ctrl->is_int); > >> c.value = 0; > >> get_ctrl(ctrl, &c); > >> return c.value; > >> @@ -2675,7 +2791,7 @@ static int try_set_ext_ctrls(struct v4l2_fh *fh, > >> struct v4l2_ctrl_handler *hdl, > >> if (!helpers) > >> return -ENOMEM; > >> } > >> - ret = prepare_ext_ctrls(hdl, cs, helpers); > >> + ret = prepare_ext_ctrls(hdl, cs, helpers, false); > >> if (!ret) > >> ret = validate_ctrls(cs, helpers, set); > >> if (ret && set) > >> @@ -2780,11 +2896,11 @@ static int set_ctrl(struct v4l2_fh *fh, struct > >> v4l2_ctrl *ctrl, > >> struct v4l2_ctrl *master = ctrl->cluster[0]; > >> int i; > >> > >> - /* String controls are not supported. The user_to_new() and > >> + /* Complex controls are not supported. The user_to_new() and > >> * cur_to_user() calls below would need to be modified not to access > >> * userspace memory when called from set_ctrl(). > >> */ > > > > The new text also has an error. From the code below, what's not supported > > below are actually non-integer controls. > > Well spotted: > > >> - if (ctrl->type == V4L2_CTRL_TYPE_STRING) > >> + if (!ctrl->is_int) > > This should have been 'if (ctrl->is_ptr)' > > >> return -EINVAL; > >> > >> /* Reset the 'is_new' flags of the cluster */ > >> @@ -2826,7 +2942,7 @@ int v4l2_s_ctrl(struct v4l2_fh *fh, struct > >> v4l2_ctrl_handler *hdl, > >> struct v4l2_ext_control c; > >> int ret; > >> > >> - if (ctrl == NULL || !type_is_int(ctrl)) > >> + if (ctrl == NULL || !ctrl->is_int) > >> return -EINVAL; > >> > >> if (ctrl->flags & V4L2_CTRL_FLAG_READ_ONLY) > >> @@ -2850,7 +2966,7 @@ int v4l2_ctrl_s_ctrl(struct v4l2_ctrl *ctrl, s32 val) > >> struct v4l2_ext_control c; > >> > >> /* It's a driver bug if this happens. */ > >> - WARN_ON(!type_is_int(ctrl)); > >> + WARN_ON(!ctrl->is_int); > >> c.value = val; > >> return set_ctrl_lock(NULL, ctrl, &c); > >> } > >> diff --git a/include/media/v4l2-ctrls.h b/include/media/v4l2-ctrls.h > >> index 3998049..515c1ba 100644 > >> --- a/include/media/v4l2-ctrls.h > >> +++ b/include/media/v4l2-ctrls.h > >> @@ -36,6 +36,19 @@ struct v4l2_subscribed_event; > >> struct v4l2_fh; > >> struct poll_table_struct; > >> > >> +/** union v4l2_ctrl_ptr - A pointer to a control value. > >> + * @p_s32: Pointer to a 32-bit signed value. > >> + * @p_s64: Pointer to a 64-bit signed value. > >> + * @p_char: Pointer to a string. > >> + * @p: Pointer to a complex value. > >> + */ > >> +union v4l2_ctrl_ptr { > >> + s32 *p_s32; > >> + s64 *p_s64; > >> + char *p_char; > >> + void *p; > >> +}; > > > > I can't see where the above is used. > > It's used at the end of the v4l2_ctrl struct. > > > > >> + > >> /** struct v4l2_ctrl_ops - The control operations that the driver has to > >> provide. > >> * @g_volatile_ctrl: Get a new value for this control. Generally only > >> relevant > >> * for volatile (and usually read-only) controls such as a > >> control > >> @@ -73,6 +86,12 @@ typedef void (*v4l2_ctrl_notify_fnc)(struct v4l2_ctrl > >> *ctrl, void *priv); > >> * members are in 'automatic' mode or 'manual' mode. This > >> is > >> * used for autogain/gain type clusters. Drivers should > >> never > >> * set this flag directly. > >> + * @is_int: If set, then this control has a simple integer value > >> (i.e. it > >> + * uses ctrl->val). > >> + * @is_string: If set, then this control has type V4L2_CTRL_TYPE_STRING. > >> + * @is_ptr: If set, then this control is a matrix and/or has type > >> >= V4L2_CTRL_COMPLEX_TYPES > >> + * and/or has type V4L2_CTRL_TYPE_STRING. In other words, > >> struct > >> + * v4l2_ext_control uses field p to point to the data. > >> * @has_volatiles: If set, then one or more members of the cluster are > >> volatile. > >> * Drivers should never touch this flag. > >> * @call_notify: If set, then call the handler's notify function > >> whenever the > >> @@ -91,6 +110,7 @@ typedef void (*v4l2_ctrl_notify_fnc)(struct v4l2_ctrl > >> *ctrl, void *priv); > >> * @maximum: The control's maximum value. > >> * @default_value: The control's default value. > >> * @step: The control's step value for non-menu controls. > >> + * @elem_size: The size in bytes of the control. > >> * @menu_skip_mask: The control's skip mask for menu controls. This > >> makes it > >> * easy to skip menu items that are not valid. If bit X is > >> set, > >> * then menu item X is skipped. Of course, this only works > >> for > >> @@ -105,7 +125,6 @@ typedef void (*v4l2_ctrl_notify_fnc)(struct v4l2_ctrl > >> *ctrl, void *priv); > >> * @cur: The control's current value. > >> * @val: The control's new s32 value. > >> * @val64: The control's new s64 value. > >> - * @string: The control's new string value. > >> * @priv: The control's private pointer. For use by the driver. > >> It is > >> * untouched by the control framework. Note that this > >> pointer is > >> * not freed when the control is deleted. Should this be > >> needed > >> @@ -124,6 +143,9 @@ struct v4l2_ctrl { > >> unsigned int is_new:1; > >> unsigned int is_private:1; > >> unsigned int is_auto:1; > >> + unsigned int is_int:1; > >> + unsigned int is_string:1; > >> + unsigned int is_ptr:1; > >> unsigned int has_volatiles:1; > >> unsigned int call_notify:1; > >> unsigned int manual_mode_value:8; > >> @@ -134,6 +156,7 @@ struct v4l2_ctrl { > >> const char *unit; > >> enum v4l2_ctrl_type type; > >> s64 minimum, maximum, default_value; > >> + u32 elem_size; > >> union { > >> u64 step; > >> u64 menu_skip_mask; > >> @@ -143,17 +166,21 @@ struct v4l2_ctrl { > >> const s64 *qmenu_int; > >> }; > >> unsigned long flags; > >> + void *priv; > >> union { > >> s32 val; > >> s64 val64; > >> char *string; > >> - } cur; > >> + void *p; > >> + }; > >> union { > >> s32 val; > >> s64 val64; > >> char *string; > >> - }; > >> - void *priv; > >> + void *p; > >> + } cur; > >> + union v4l2_ctrl_ptr new; > >> + union v4l2_ctrl_ptr stores[]; > > ^^^^^^^^^^^^^^^^^^^^ > > Here! You're not using it here... you're just declaring it. You're actually using it on a latter patch ;) Btw, the declaration for stores[] seems weird or wrong on the above, as this is actually allocating space for just a pointer. IMO, you should, instead, be declaring it as either: size_t stores_size; union v4l2_ctrl_ptr *stores; or size_t stores_size; union v4l2_ctrl_ptr stores[MAX_STORES]; > > >> }; > >> > >> /** struct v4l2_ctrl_ref - The control reference. > >> @@ -215,6 +242,7 @@ struct v4l2_ctrl_handler { > >> * @max: The control's maximum value. > >> * @step: The control's step value for non-menu controls. > >> * @def: The control's default value. > >> + * @elem_size: The size in bytes of the control. > >> * @flags: The control's flags. > >> * @menu_skip_mask: The control's skip mask for menu controls. This > >> makes it > >> * easy to skip menu items that are not valid. If bit X is > >> set, > >> @@ -239,6 +267,7 @@ struct v4l2_ctrl_config { > >> s64 max; > >> u64 step; > >> s64 def; > >> + u32 elem_size; > >> u32 flags; > >> u64 menu_skip_mask; > >> const char * const *qmenu; > >> @@ -664,6 +693,7 @@ unsigned int v4l2_ctrl_poll(struct file *file, struct > >> poll_table_struct *wait); > >> > >> /* Helpers for ioctl_ops. If hdl == NULL then they will all return > >> -EINVAL. */ > >> int v4l2_queryctrl(struct v4l2_ctrl_handler *hdl, struct v4l2_queryctrl > >> *qc); > >> +int v4l2_query_ext_ctrl(struct v4l2_ctrl_handler *hdl, struct > >> v4l2_query_ext_ctrl *qc); > >> int v4l2_querymenu(struct v4l2_ctrl_handler *hdl, struct v4l2_querymenu > >> *qm); > >> int v4l2_g_ctrl(struct v4l2_ctrl_handler *hdl, struct v4l2_control *ctrl); > >> int v4l2_s_ctrl(struct v4l2_fh *fh, struct v4l2_ctrl_handler *hdl, > > > > > > Regards, > > Hans -- Regards, Mauro -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
