:p
atchew
Login
Most of the drivers use the control framework or can use the superset version of these callbacks: vidioc_g/s_ext_ctrl and vidioc_query_ext_ctrl. Signed-off-by: Ricardo Ribalda <ribalda@chromium.org> --- Ricardo Ribalda (10): media: ioctl: Simulate v4l2_queryctrl with v4l2_query_ext_ctrl media: pvrusb2: Convert queryctrl to query_ext_ctrl media: pvrusb2: Remove g/s_ctrl callbacks media: uvcvideo: Remove vidioc_queryctrl media: atomisp: Replace queryctrl with query_ext_ctrl media: atomisp: Remove vidioc_g/s callback media: v4l2: Remove vidioc_queryctrl callback media: v4l2: Remove vidioc_g_ctrl callback media: cx231xx: Replace s_ctrl with s_ext_ctrls media: v4l2: Remove vidioc_s_ctrl callback drivers/media/usb/cx231xx/cx231xx-417.c | 21 ++++++++---- drivers/media/usb/pvrusb2/pvrusb2-v4l2.c | 40 +++++------------------ drivers/media/usb/uvc/uvc_v4l2.c | 10 ------ drivers/media/v4l2-core/v4l2-dev.c | 6 ++-- drivers/media/v4l2-core/v4l2-ioctl.c | 28 ++++++++++++---- drivers/staging/media/atomisp/pci/atomisp_ioctl.c | 36 ++++++++++---------- include/media/v4l2-ioctl.h | 12 ------- 7 files changed, 65 insertions(+), 88 deletions(-) --- base-commit: 6c10d1adae82e1c8da16e7ebd2320e69f20b9d6f change-id: 20241209-queryctrl-5c3632b7c857 Best regards, -- Ricardo Ribalda <ribalda@chromium.org>
v4l2_queryctrl is a subset of v4l2_query_ext_ctrl. If the driver does not implement v4l2_queryctrl we can implement it with v4l2_query_ext_ctrl. Suggested-by: Hans de Goede <hdegoede@redhat.com> Signed-off-by: Ricardo Ribalda <ribalda@chromium.org> --- drivers/media/v4l2-core/v4l2-dev.c | 3 ++- drivers/media/v4l2-core/v4l2-ioctl.c | 22 +++++++++++++++++++++- 2 files changed, 23 insertions(+), 2 deletions(-) diff --git a/drivers/media/v4l2-core/v4l2-dev.c b/drivers/media/v4l2-core/v4l2-dev.c index XXXXXXX..XXXXXXX 100644 --- a/drivers/media/v4l2-core/v4l2-dev.c +++ b/drivers/media/v4l2-core/v4l2-dev.c @@ -XXX,XX +XXX,XX @@ static void determine_valid_ioctls(struct video_device *vdev) and that can't be tested here. If the bit for these control ioctls is set, then the ioctl is valid. But if it is 0, then it can still be valid if the filehandle passed the control handler. */ - if (vdev->ctrl_handler || ops->vidioc_queryctrl) + if (vdev->ctrl_handler || ops->vidioc_queryctrl || + ops->vidioc_query_ext_ctrl) __set_bit(_IOC_NR(VIDIOC_QUERYCTRL), valid_ioctls); if (vdev->ctrl_handler || ops->vidioc_query_ext_ctrl) __set_bit(_IOC_NR(VIDIOC_QUERY_EXT_CTRL), valid_ioctls); diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c b/drivers/media/v4l2-core/v4l2-ioctl.c index XXXXXXX..XXXXXXX 100644 --- a/drivers/media/v4l2-core/v4l2-ioctl.c +++ b/drivers/media/v4l2-core/v4l2-ioctl.c @@ -XXX,XX +XXX,XX @@ static int v4l_queryctrl(const struct v4l2_ioctl_ops *ops, struct file *file, void *fh, void *arg) { struct video_device *vfd = video_devdata(file); + struct v4l2_query_ext_ctrl qec; struct v4l2_queryctrl *p = arg; struct v4l2_fh *vfh = test_bit(V4L2_FL_USES_V4L2_FH, &vfd->flags) ? fh : NULL; + int ret; if (vfh && vfh->ctrl_handler) return v4l2_queryctrl(vfh->ctrl_handler, p); @@ -XXX,XX +XXX,XX @@ static int v4l_queryctrl(const struct v4l2_ioctl_ops *ops, return v4l2_queryctrl(vfd->ctrl_handler, p); if (ops->vidioc_queryctrl) return ops->vidioc_queryctrl(file, fh, p); - return -ENOTTY; + if (!ops->vidioc_query_ext_ctrl) + return -ENOTTY; + + /* Simulate query_ext_ctr using query_ctrl. */ + qec.id = p->id; + ret = ops->vidioc_query_ext_ctrl(file, fh, &qec); + if (ret) + return ret; + + p->id = qec.id; + p->type = qec.type; + strscpy(p->name, qec.name, sizeof(p->name)); + p->minimum = qec.minimum; + p->maximum = qec.maximum; + p->step = qec.step; + p->default_value = qec.default_value; + p->flags = qec.flags; + + return 0; } static int v4l_query_ext_ctrl(const struct v4l2_ioctl_ops *ops, -- 2.47.0.338.g60cca15819-goog
The driver was missing support for query_ext_ctrl. Instead of adding a new callback for it, replace the current implementation of queryctrl and let the ioctl framework emulate the old function. Most of the fields are identical, so the change is pretty simple. Signed-off-by: Ricardo Ribalda <ribalda@chromium.org> --- drivers/media/usb/pvrusb2/pvrusb2-v4l2.c | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/drivers/media/usb/pvrusb2/pvrusb2-v4l2.c b/drivers/media/usb/pvrusb2/pvrusb2-v4l2.c index XXXXXXX..XXXXXXX 100644 --- a/drivers/media/usb/pvrusb2/pvrusb2-v4l2.c +++ b/drivers/media/usb/pvrusb2/pvrusb2-v4l2.c @@ -XXX,XX +XXX,XX @@ static int pvr2_streamoff(struct file *file, void *priv, enum v4l2_buf_type i) return pvr2_hdw_set_streaming(hdw, 0); } -static int pvr2_queryctrl(struct file *file, void *priv, - struct v4l2_queryctrl *vc) +static int pvr2_query_ext_ctrl(struct file *file, void *priv, + struct v4l2_query_ext_ctrl *vc) { struct pvr2_v4l2_fh *fh = file->private_data; struct pvr2_hdw *hdw = fh->channel.mc_head->hdw; @@ -XXX,XX +XXX,XX @@ static int pvr2_queryctrl(struct file *file, void *priv, } pvr2_trace(PVR2_TRACE_V4LIOCTL, - "QUERYCTRL id=0x%x mapping name=%s (%s)", + "QUERYEXTCTRL id=0x%x mapping name=%s (%s)", vc->id, pvr2_ctrl_get_name(cptr), pvr2_ctrl_get_desc(cptr)); strscpy(vc->name, pvr2_ctrl_get_desc(cptr), sizeof(vc->name)); vc->flags = pvr2_ctrl_get_v4lflags(cptr); pvr2_ctrl_get_def(cptr, &val); vc->default_value = val; + vc->nr_of_dims = 0; + vc->elems = 1; + vc->elem_size = 4; switch (pvr2_ctrl_get_type(cptr)) { case pvr2_ctl_enum: vc->type = V4L2_CTRL_TYPE_MENU; @@ -XXX,XX +XXX,XX @@ static int pvr2_queryctrl(struct file *file, void *priv, break; default: pvr2_trace(PVR2_TRACE_V4LIOCTL, - "QUERYCTRL id=0x%x name=%s not mappable", + "QUERYEXTCTRL id=0x%x name=%s not mappable", vc->id, pvr2_ctrl_get_name(cptr)); return -EINVAL; } @@ -XXX,XX +XXX,XX @@ static const struct v4l2_ioctl_ops pvr2_ioctl_ops = { .vidioc_try_fmt_vid_cap = pvr2_try_fmt_vid_cap, .vidioc_streamon = pvr2_streamon, .vidioc_streamoff = pvr2_streamoff, - .vidioc_queryctrl = pvr2_queryctrl, + .vidioc_query_ext_ctrl = pvr2_query_ext_ctrl, .vidioc_querymenu = pvr2_querymenu, .vidioc_g_ctrl = pvr2_g_ctrl, .vidioc_s_ctrl = pvr2_s_ctrl, -- 2.47.0.338.g60cca15819-goog
The ioctl helpers can emulate g/s_ctrl with g/s_ext_ctrl. Simplify the code. Signed-off-by: Ricardo Ribalda <ribalda@chromium.org> --- drivers/media/usb/pvrusb2/pvrusb2-v4l2.c | 27 --------------------------- 1 file changed, 27 deletions(-) diff --git a/drivers/media/usb/pvrusb2/pvrusb2-v4l2.c b/drivers/media/usb/pvrusb2/pvrusb2-v4l2.c index XXXXXXX..XXXXXXX 100644 --- a/drivers/media/usb/pvrusb2/pvrusb2-v4l2.c +++ b/drivers/media/usb/pvrusb2/pvrusb2-v4l2.c @@ -XXX,XX +XXX,XX @@ static int pvr2_querymenu(struct file *file, void *priv, struct v4l2_querymenu * return ret; } -static int pvr2_g_ctrl(struct file *file, void *priv, struct v4l2_control *vc) -{ - struct pvr2_v4l2_fh *fh = file->private_data; - struct pvr2_hdw *hdw = fh->channel.mc_head->hdw; - int val = 0; - int ret; - - ret = pvr2_ctrl_get_value(pvr2_hdw_get_ctrl_v4l(hdw, vc->id), - &val); - vc->value = val; - return ret; -} - -static int pvr2_s_ctrl(struct file *file, void *priv, struct v4l2_control *vc) -{ - struct pvr2_v4l2_fh *fh = file->private_data; - struct pvr2_hdw *hdw = fh->channel.mc_head->hdw; - int ret; - - ret = pvr2_ctrl_set_value(pvr2_hdw_get_ctrl_v4l(hdw, vc->id), - vc->value); - pvr2_hdw_commit_ctl(hdw); - return ret; -} - static int pvr2_g_ext_ctrls(struct file *file, void *priv, struct v4l2_ext_controls *ctls) { @@ -XXX,XX +XXX,XX @@ static const struct v4l2_ioctl_ops pvr2_ioctl_ops = { .vidioc_streamoff = pvr2_streamoff, .vidioc_query_ext_ctrl = pvr2_query_ext_ctrl, .vidioc_querymenu = pvr2_querymenu, - .vidioc_g_ctrl = pvr2_g_ctrl, - .vidioc_s_ctrl = pvr2_s_ctrl, .vidioc_g_ext_ctrls = pvr2_g_ext_ctrls, .vidioc_s_ext_ctrls = pvr2_s_ext_ctrls, .vidioc_try_ext_ctrls = pvr2_try_ext_ctrls, -- 2.47.0.338.g60cca15819-goog
It can be implemented by the v4l2 ioctl framework using vidioc_query_ext_ctrl. Signed-off-by: Ricardo Ribalda <ribalda@chromium.org> --- drivers/media/usb/uvc/uvc_v4l2.c | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/drivers/media/usb/uvc/uvc_v4l2.c b/drivers/media/usb/uvc/uvc_v4l2.c index XXXXXXX..XXXXXXX 100644 --- a/drivers/media/usb/uvc/uvc_v4l2.c +++ b/drivers/media/usb/uvc/uvc_v4l2.c @@ -XXX,XX +XXX,XX @@ static int uvc_ioctl_s_input(struct file *file, void *fh, unsigned int input) return ret; } -static int uvc_ioctl_queryctrl(struct file *file, void *fh, - struct v4l2_queryctrl *qc) -{ - struct uvc_fh *handle = fh; - struct uvc_video_chain *chain = handle->chain; - - return uvc_query_v4l2_ctrl(chain, qc); -} - static int uvc_ioctl_query_ext_ctrl(struct file *file, void *fh, struct v4l2_query_ext_ctrl *qec) { @@ -XXX,XX +XXX,XX @@ const struct v4l2_ioctl_ops uvc_ioctl_ops = { .vidioc_enum_input = uvc_ioctl_enum_input, .vidioc_g_input = uvc_ioctl_g_input, .vidioc_s_input = uvc_ioctl_s_input, - .vidioc_queryctrl = uvc_ioctl_queryctrl, .vidioc_query_ext_ctrl = uvc_ioctl_query_ext_ctrl, .vidioc_g_ext_ctrls = uvc_ioctl_g_ext_ctrls, .vidioc_s_ext_ctrls = uvc_ioctl_s_ext_ctrls, -- 2.47.0.338.g60cca15819-goog
The ioctl framework provides an emulator of queryctrl using query_ext_ctrl. Replace our implementation of queryctrl to support both. Now that we are at it: - Add comment about missing functionality. - Remove superfluous clear of reserved[0]. - Remove ret var. Signed-off-by: Ricardo Ribalda <ribalda@chromium.org> --- drivers/staging/media/atomisp/pci/atomisp_ioctl.c | 34 +++++++++++++---------- 1 file changed, 19 insertions(+), 15 deletions(-) diff --git a/drivers/staging/media/atomisp/pci/atomisp_ioctl.c b/drivers/staging/media/atomisp/pci/atomisp_ioctl.c index XXXXXXX..XXXXXXX 100644 --- a/drivers/staging/media/atomisp/pci/atomisp_ioctl.c +++ b/drivers/staging/media/atomisp/pci/atomisp_ioctl.c @@ -XXX,XX +XXX,XX @@ static const char *CARD = "ATOM ISP"; /* max size 31 */ * FIXME: ISP should not know beforehand all CIDs supported by sensor. * Instead, it needs to propagate to sensor unknown CIDs. */ -static struct v4l2_queryctrl ci_v4l2_controls[] = { +static struct v4l2_query_ext_ctrl ci_v4l2_controls[] = { { .id = V4L2_CID_AUTO_WHITE_BALANCE, .type = V4L2_CTRL_TYPE_BOOLEAN, @@ -XXX,XX +XXX,XX @@ static int atomisp_s_ctrl(struct file *file, void *fh, /* * To query the attributes of a control. - * applications set the id field of a struct v4l2_queryctrl and call the + * applications set the id field of a struct v4l2_query_ext_ctrl and call the * this ioctl with a pointer to this structure. The driver fills * the rest of the structure. */ -static int atomisp_queryctl(struct file *file, void *fh, - struct v4l2_queryctrl *qc) +static int atomisp_query_ext_ctrl(struct file *file, void *fh, + struct v4l2_query_ext_ctrl *qc) { - int i, ret = -EINVAL; + int i; + /* TODO: implement V4L2_CTRL_FLAG_NEXT_CTRL */ if (qc->id & V4L2_CTRL_FLAG_NEXT_CTRL) - return ret; + return -EINVAL; for (i = 0; i < ctrls_num; i++) { if (ci_v4l2_controls[i].id == qc->id) { - memcpy(qc, &ci_v4l2_controls[i], - sizeof(struct v4l2_queryctrl)); - qc->reserved[0] = 0; - ret = 0; - break; + memcpy(qc, &ci_v4l2_controls[i], sizeof(*qc)); + qc->nr_of_dims = 0; + qc->elems = 1; + qc->elem_size = 4; + return 0; } } - if (ret != 0) - qc->flags = V4L2_CTRL_FLAG_DISABLED; - return ret; + /* + * This is probably not needed, but this flag has been set for + * many kernel versions. Leave to avoid breaking any apps + */ + qc->flags = V4L2_CTRL_FLAG_DISABLED; + return -EINVAL; } static int atomisp_camera_g_ext_ctrls(struct file *file, void *fh, @@ -XXX,XX +XXX,XX @@ const struct v4l2_ioctl_ops atomisp_ioctl_ops = { .vidioc_enum_input = atomisp_enum_input, .vidioc_g_input = atomisp_g_input, .vidioc_s_input = atomisp_s_input, - .vidioc_queryctrl = atomisp_queryctl, + .vidioc_query_ext_ctrl = atomisp_query_ext_ctrl, .vidioc_s_ctrl = atomisp_s_ctrl, .vidioc_g_ctrl = atomisp_g_ctrl, .vidioc_s_ext_ctrls = atomisp_s_ext_ctrls, -- 2.47.0.338.g60cca15819-goog
The v4l2 ioctl framework can implement vidioc_g/s_ctrl with vidioc_g/s_ext_ctrl() and we provide those. These are the last references of vidioc_s/g_ctrl in the codebase. We can attempt to remove them now. Signed-off-by: Ricardo Ribalda <ribalda@chromium.org> --- drivers/staging/media/atomisp/pci/atomisp_ioctl.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/drivers/staging/media/atomisp/pci/atomisp_ioctl.c b/drivers/staging/media/atomisp/pci/atomisp_ioctl.c index XXXXXXX..XXXXXXX 100644 --- a/drivers/staging/media/atomisp/pci/atomisp_ioctl.c +++ b/drivers/staging/media/atomisp/pci/atomisp_ioctl.c @@ -XXX,XX +XXX,XX @@ const struct v4l2_ioctl_ops atomisp_ioctl_ops = { .vidioc_g_input = atomisp_g_input, .vidioc_s_input = atomisp_s_input, .vidioc_query_ext_ctrl = atomisp_query_ext_ctrl, - .vidioc_s_ctrl = atomisp_s_ctrl, - .vidioc_g_ctrl = atomisp_g_ctrl, .vidioc_s_ext_ctrls = atomisp_s_ext_ctrls, .vidioc_g_ext_ctrls = atomisp_g_ext_ctrls, .vidioc_enum_framesizes = atomisp_enum_framesizes, -- 2.47.0.338.g60cca15819-goog
All the drivers either use the control framework or provide a vidioc_query_ext_ctrl. We can remove this callback to reduce the temptation of new drivers to implement it. Signed-off-by: Ricardo Ribalda <ribalda@chromium.org> --- drivers/media/v4l2-core/v4l2-dev.c | 3 +-- drivers/media/v4l2-core/v4l2-ioctl.c | 2 -- include/media/v4l2-ioctl.h | 4 ---- 3 files changed, 1 insertion(+), 8 deletions(-) diff --git a/drivers/media/v4l2-core/v4l2-dev.c b/drivers/media/v4l2-core/v4l2-dev.c index XXXXXXX..XXXXXXX 100644 --- a/drivers/media/v4l2-core/v4l2-dev.c +++ b/drivers/media/v4l2-core/v4l2-dev.c @@ -XXX,XX +XXX,XX @@ static void determine_valid_ioctls(struct video_device *vdev) and that can't be tested here. If the bit for these control ioctls is set, then the ioctl is valid. But if it is 0, then it can still be valid if the filehandle passed the control handler. */ - if (vdev->ctrl_handler || ops->vidioc_queryctrl || - ops->vidioc_query_ext_ctrl) + if (vdev->ctrl_handler || ops->vidioc_query_ext_ctrl) __set_bit(_IOC_NR(VIDIOC_QUERYCTRL), valid_ioctls); if (vdev->ctrl_handler || ops->vidioc_query_ext_ctrl) __set_bit(_IOC_NR(VIDIOC_QUERY_EXT_CTRL), valid_ioctls); diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c b/drivers/media/v4l2-core/v4l2-ioctl.c index XXXXXXX..XXXXXXX 100644 --- a/drivers/media/v4l2-core/v4l2-ioctl.c +++ b/drivers/media/v4l2-core/v4l2-ioctl.c @@ -XXX,XX +XXX,XX @@ static int v4l_queryctrl(const struct v4l2_ioctl_ops *ops, return v4l2_queryctrl(vfh->ctrl_handler, p); if (vfd->ctrl_handler) return v4l2_queryctrl(vfd->ctrl_handler, p); - if (ops->vidioc_queryctrl) - return ops->vidioc_queryctrl(file, fh, p); if (!ops->vidioc_query_ext_ctrl) return -ENOTTY; diff --git a/include/media/v4l2-ioctl.h b/include/media/v4l2-ioctl.h index XXXXXXX..XXXXXXX 100644 --- a/include/media/v4l2-ioctl.h +++ b/include/media/v4l2-ioctl.h @@ -XXX,XX +XXX,XX @@ struct v4l2_fh; * :ref:`VIDIOC_G_OUTPUT <vidioc_g_output>` ioctl * @vidioc_s_output: pointer to the function that implements * :ref:`VIDIOC_S_OUTPUT <vidioc_g_output>` ioctl - * @vidioc_queryctrl: pointer to the function that implements - * :ref:`VIDIOC_QUERYCTRL <vidioc_queryctrl>` ioctl * @vidioc_query_ext_ctrl: pointer to the function that implements * :ref:`VIDIOC_QUERY_EXT_CTRL <vidioc_queryctrl>` ioctl * @vidioc_g_ctrl: pointer to the function that implements @@ -XXX,XX +XXX,XX @@ struct v4l2_ioctl_ops { int (*vidioc_s_output)(struct file *file, void *fh, unsigned int i); /* Control handling */ - int (*vidioc_queryctrl)(struct file *file, void *fh, - struct v4l2_queryctrl *a); int (*vidioc_query_ext_ctrl)(struct file *file, void *fh, struct v4l2_query_ext_ctrl *a); int (*vidioc_g_ctrl)(struct file *file, void *fh, -- 2.47.0.338.g60cca15819-goog
All the drivers either use the control framework or provide a vidioc_g_ext_ctrls callback. We can remove this callback. Thanks for your service! Signed-off-by: Ricardo Ribalda <ribalda@chromium.org> --- drivers/media/v4l2-core/v4l2-dev.c | 2 +- drivers/media/v4l2-core/v4l2-ioctl.c | 2 -- include/media/v4l2-ioctl.h | 4 ---- 3 files changed, 1 insertion(+), 7 deletions(-) diff --git a/drivers/media/v4l2-core/v4l2-dev.c b/drivers/media/v4l2-core/v4l2-dev.c index XXXXXXX..XXXXXXX 100644 --- a/drivers/media/v4l2-core/v4l2-dev.c +++ b/drivers/media/v4l2-core/v4l2-dev.c @@ -XXX,XX +XXX,XX @@ static void determine_valid_ioctls(struct video_device *vdev) __set_bit(_IOC_NR(VIDIOC_QUERYCTRL), valid_ioctls); if (vdev->ctrl_handler || ops->vidioc_query_ext_ctrl) __set_bit(_IOC_NR(VIDIOC_QUERY_EXT_CTRL), valid_ioctls); - if (vdev->ctrl_handler || ops->vidioc_g_ctrl || ops->vidioc_g_ext_ctrls) + if (vdev->ctrl_handler || ops->vidioc_g_ext_ctrls) __set_bit(_IOC_NR(VIDIOC_G_CTRL), valid_ioctls); if (vdev->ctrl_handler || ops->vidioc_s_ctrl || ops->vidioc_s_ext_ctrls) __set_bit(_IOC_NR(VIDIOC_S_CTRL), valid_ioctls); diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c b/drivers/media/v4l2-core/v4l2-ioctl.c index XXXXXXX..XXXXXXX 100644 --- a/drivers/media/v4l2-core/v4l2-ioctl.c +++ b/drivers/media/v4l2-core/v4l2-ioctl.c @@ -XXX,XX +XXX,XX @@ static int v4l_g_ctrl(const struct v4l2_ioctl_ops *ops, return v4l2_g_ctrl(vfh->ctrl_handler, p); if (vfd->ctrl_handler) return v4l2_g_ctrl(vfd->ctrl_handler, p); - if (ops->vidioc_g_ctrl) - return ops->vidioc_g_ctrl(file, fh, p); if (ops->vidioc_g_ext_ctrls == NULL) return -ENOTTY; diff --git a/include/media/v4l2-ioctl.h b/include/media/v4l2-ioctl.h index XXXXXXX..XXXXXXX 100644 --- a/include/media/v4l2-ioctl.h +++ b/include/media/v4l2-ioctl.h @@ -XXX,XX +XXX,XX @@ struct v4l2_fh; * :ref:`VIDIOC_S_OUTPUT <vidioc_g_output>` ioctl * @vidioc_query_ext_ctrl: pointer to the function that implements * :ref:`VIDIOC_QUERY_EXT_CTRL <vidioc_queryctrl>` ioctl - * @vidioc_g_ctrl: pointer to the function that implements - * :ref:`VIDIOC_G_CTRL <vidioc_g_ctrl>` ioctl * @vidioc_s_ctrl: pointer to the function that implements * :ref:`VIDIOC_S_CTRL <vidioc_g_ctrl>` ioctl * @vidioc_g_ext_ctrls: pointer to the function that implements @@ -XXX,XX +XXX,XX @@ struct v4l2_ioctl_ops { /* Control handling */ int (*vidioc_query_ext_ctrl)(struct file *file, void *fh, struct v4l2_query_ext_ctrl *a); - int (*vidioc_g_ctrl)(struct file *file, void *fh, - struct v4l2_control *a); int (*vidioc_s_ctrl)(struct file *file, void *fh, struct v4l2_control *a); int (*vidioc_g_ext_ctrls)(struct file *file, void *fh, -- 2.47.0.338.g60cca15819-goog
The v4l2 ioctl framework can provide support for s_ctrl. This the last driver implementing s_ctrl. Signed-off-by: Ricardo Ribalda <ribalda@chromium.org> --- drivers/media/usb/cx231xx/cx231xx-417.c | 21 ++++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-) diff --git a/drivers/media/usb/cx231xx/cx231xx-417.c b/drivers/media/usb/cx231xx/cx231xx-417.c index XXXXXXX..XXXXXXX 100644 --- a/drivers/media/usb/cx231xx/cx231xx-417.c +++ b/drivers/media/usb/cx231xx/cx231xx-417.c @@ -XXX,XX +XXX,XX @@ static int vidioc_s_std(struct file *file, void *priv, v4l2_std_id id) return 0; } -static int vidioc_s_ctrl(struct file *file, void *priv, - struct v4l2_control *ctl) +static int cx231xx_s_ext_ctrls(struct file *file, void *priv, + struct v4l2_ext_controls *ctls) { struct cx231xx *dev = video_drvdata(file); + struct v4l2_control ctl; struct v4l2_subdev *sd; + unsigned int i; - dprintk(3, "enter vidioc_s_ctrl()\n"); + dprintk(3, "enter vidioc_s_ext_ctrl()\n"); /* Update the A/V core */ - v4l2_device_for_each_subdev(sd, &dev->v4l2_dev) - v4l2_s_ctrl(NULL, sd->ctrl_handler, ctl); - dprintk(3, "exit vidioc_s_ctrl()\n"); + for (i = 0; i < ctls->count; i++) { + ctl.id = ctls->controls[i].id; + ctl.value = ctls->controls[i].value; + v4l2_device_for_each_subdev(sd, &dev->v4l2_dev) + v4l2_s_ctrl(NULL, sd->ctrl_handler, &ctl); + ctls->controls[i].value = ctl.value; + } + dprintk(3, "exit vidioc_s_ext_ctrl()\n"); return 0; } @@ -XXX,XX +XXX,XX @@ static const struct v4l2_ioctl_ops mpeg_ioctl_ops = { .vidioc_enum_input = cx231xx_enum_input, .vidioc_g_input = cx231xx_g_input, .vidioc_s_input = cx231xx_s_input, - .vidioc_s_ctrl = vidioc_s_ctrl, + .vidioc_s_ext_ctrls = cx231xx_s_ext_ctrls, .vidioc_g_pixelaspect = vidioc_g_pixelaspect, .vidioc_g_selection = vidioc_g_selection, .vidioc_querycap = cx231xx_querycap, -- 2.47.0.338.g60cca15819-goog
All the drivers either use the control framework or provide a vidiod_ext_ctrl. We can remove this callback. Signed-off-by: Ricardo Ribalda <ribalda@chromium.org> --- drivers/media/v4l2-core/v4l2-dev.c | 2 +- drivers/media/v4l2-core/v4l2-ioctl.c | 2 -- include/media/v4l2-ioctl.h | 4 ---- 3 files changed, 1 insertion(+), 7 deletions(-) diff --git a/drivers/media/v4l2-core/v4l2-dev.c b/drivers/media/v4l2-core/v4l2-dev.c index XXXXXXX..XXXXXXX 100644 --- a/drivers/media/v4l2-core/v4l2-dev.c +++ b/drivers/media/v4l2-core/v4l2-dev.c @@ -XXX,XX +XXX,XX @@ static void determine_valid_ioctls(struct video_device *vdev) __set_bit(_IOC_NR(VIDIOC_QUERY_EXT_CTRL), valid_ioctls); if (vdev->ctrl_handler || ops->vidioc_g_ext_ctrls) __set_bit(_IOC_NR(VIDIOC_G_CTRL), valid_ioctls); - if (vdev->ctrl_handler || ops->vidioc_s_ctrl || ops->vidioc_s_ext_ctrls) + if (vdev->ctrl_handler || ops->vidioc_s_ext_ctrls) __set_bit(_IOC_NR(VIDIOC_S_CTRL), valid_ioctls); if (vdev->ctrl_handler || ops->vidioc_g_ext_ctrls) __set_bit(_IOC_NR(VIDIOC_G_EXT_CTRLS), valid_ioctls); diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c b/drivers/media/v4l2-core/v4l2-ioctl.c index XXXXXXX..XXXXXXX 100644 --- a/drivers/media/v4l2-core/v4l2-ioctl.c +++ b/drivers/media/v4l2-core/v4l2-ioctl.c @@ -XXX,XX +XXX,XX @@ static int v4l_s_ctrl(const struct v4l2_ioctl_ops *ops, return v4l2_s_ctrl(vfh, vfh->ctrl_handler, p); if (vfd->ctrl_handler) return v4l2_s_ctrl(NULL, vfd->ctrl_handler, p); - if (ops->vidioc_s_ctrl) - return ops->vidioc_s_ctrl(file, fh, p); if (ops->vidioc_s_ext_ctrls == NULL) return -ENOTTY; diff --git a/include/media/v4l2-ioctl.h b/include/media/v4l2-ioctl.h index XXXXXXX..XXXXXXX 100644 --- a/include/media/v4l2-ioctl.h +++ b/include/media/v4l2-ioctl.h @@ -XXX,XX +XXX,XX @@ struct v4l2_fh; * :ref:`VIDIOC_S_OUTPUT <vidioc_g_output>` ioctl * @vidioc_query_ext_ctrl: pointer to the function that implements * :ref:`VIDIOC_QUERY_EXT_CTRL <vidioc_queryctrl>` ioctl - * @vidioc_s_ctrl: pointer to the function that implements - * :ref:`VIDIOC_S_CTRL <vidioc_g_ctrl>` ioctl * @vidioc_g_ext_ctrls: pointer to the function that implements * :ref:`VIDIOC_G_EXT_CTRLS <vidioc_g_ext_ctrls>` ioctl * @vidioc_s_ext_ctrls: pointer to the function that implements @@ -XXX,XX +XXX,XX @@ struct v4l2_ioctl_ops { /* Control handling */ int (*vidioc_query_ext_ctrl)(struct file *file, void *fh, struct v4l2_query_ext_ctrl *a); - int (*vidioc_s_ctrl)(struct file *file, void *fh, - struct v4l2_control *a); int (*vidioc_g_ext_ctrls)(struct file *file, void *fh, struct v4l2_ext_controls *a); int (*vidioc_s_ext_ctrls)(struct file *file, void *fh, -- 2.47.0.338.g60cca15819-goog
Most of the drivers use the control framework or can use the superset version of these callbacks: vidioc_g/s_ext_ctrl and vidioc_query_ext_ctrl. Signed-off-by: Ricardo Ribalda <ribalda@chromium.org> --- Changes in v2: - v4l2_query_ext_ctrl_to_v4l2_queryctrl - Fix conversion (Thanks Hans) - Link to v1: https://lore.kernel.org/r/20241209-queryctrl-v1-0-deff7acfcdcb@chromium.org --- Ricardo Ribalda (11): media: ioctl: Simulate v4l2_queryctrl with v4l2_query_ext_ctrl media: pvrusb2: Convert queryctrl to query_ext_ctrl media: pvrusb2: Remove g/s_ctrl callbacks media: uvcvideo: Remove vidioc_queryctrl media: atomisp: Replace queryctrl with query_ext_ctrl media: atomisp: Remove vidioc_g/s callback media: v4l2: Remove vidioc_queryctrl callback media: v4l2: Remove vidioc_g_ctrl callback media: cx231xx: Replace s_ctrl with s_ext_ctrls media: v4l2: Remove vidioc_s_ctrl callback media: v4l2-core: Introduce v4l2_query_ext_ctrl_to_v4l2_queryctrl drivers/media/usb/cx231xx/cx231xx-417.c | 21 ++++++---- drivers/media/usb/pvrusb2/pvrusb2-v4l2.c | 40 ++++-------------- drivers/media/usb/uvc/uvc_v4l2.c | 10 ----- drivers/media/v4l2-core/v4l2-ctrls-api.c | 51 +++++++++++++---------- drivers/media/v4l2-core/v4l2-dev.c | 6 +-- drivers/media/v4l2-core/v4l2-ioctl.c | 19 +++++---- drivers/staging/media/atomisp/pci/atomisp_ioctl.c | 35 ++++++++-------- include/media/v4l2-ctrls.h | 12 ++++++ include/media/v4l2-ioctl.h | 12 ------ 9 files changed, 97 insertions(+), 109 deletions(-) --- base-commit: 6c10d1adae82e1c8da16e7ebd2320e69f20b9d6f change-id: 20241209-queryctrl-5c3632b7c857 Best regards, -- Ricardo Ribalda <ribalda@chromium.org>
v4l2_queryctrl is a subset of v4l2_query_ext_ctrl. If the driver does not implement v4l2_queryctrl we can implement it with v4l2_query_ext_ctrl. Suggested-by: Hans de Goede <hdegoede@redhat.com> Signed-off-by: Ricardo Ribalda <ribalda@chromium.org> --- drivers/media/v4l2-core/v4l2-dev.c | 3 ++- drivers/media/v4l2-core/v4l2-ioctl.c | 37 +++++++++++++++++++++++++++++++++++- 2 files changed, 38 insertions(+), 2 deletions(-) diff --git a/drivers/media/v4l2-core/v4l2-dev.c b/drivers/media/v4l2-core/v4l2-dev.c index XXXXXXX..XXXXXXX 100644 --- a/drivers/media/v4l2-core/v4l2-dev.c +++ b/drivers/media/v4l2-core/v4l2-dev.c @@ -XXX,XX +XXX,XX @@ static void determine_valid_ioctls(struct video_device *vdev) and that can't be tested here. If the bit for these control ioctls is set, then the ioctl is valid. But if it is 0, then it can still be valid if the filehandle passed the control handler. */ - if (vdev->ctrl_handler || ops->vidioc_queryctrl) + if (vdev->ctrl_handler || ops->vidioc_queryctrl || + ops->vidioc_query_ext_ctrl) __set_bit(_IOC_NR(VIDIOC_QUERYCTRL), valid_ioctls); if (vdev->ctrl_handler || ops->vidioc_query_ext_ctrl) __set_bit(_IOC_NR(VIDIOC_QUERY_EXT_CTRL), valid_ioctls); diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c b/drivers/media/v4l2-core/v4l2-ioctl.c index XXXXXXX..XXXXXXX 100644 --- a/drivers/media/v4l2-core/v4l2-ioctl.c +++ b/drivers/media/v4l2-core/v4l2-ioctl.c @@ -XXX,XX +XXX,XX @@ static int v4l_queryctrl(const struct v4l2_ioctl_ops *ops, struct file *file, void *fh, void *arg) { struct video_device *vfd = video_devdata(file); + struct v4l2_query_ext_ctrl qec; struct v4l2_queryctrl *p = arg; struct v4l2_fh *vfh = test_bit(V4L2_FL_USES_V4L2_FH, &vfd->flags) ? fh : NULL; + int ret; if (vfh && vfh->ctrl_handler) return v4l2_queryctrl(vfh->ctrl_handler, p); @@ -XXX,XX +XXX,XX @@ static int v4l_queryctrl(const struct v4l2_ioctl_ops *ops, return v4l2_queryctrl(vfd->ctrl_handler, p); if (ops->vidioc_queryctrl) return ops->vidioc_queryctrl(file, fh, p); - return -ENOTTY; + if (!ops->vidioc_query_ext_ctrl) + return -ENOTTY; + + /* Simulate query_ext_ctr using query_ctrl. */ + qec.id = p->id; + ret = ops->vidioc_query_ext_ctrl(file, fh, &qec); + if (ret) + return ret; + + p->id = qec.id; + p->type = qec.type; + p->flags = qec.flags; + strscpy(p->name, qec.name, sizeof(p->name)); + switch (p->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: + p->minimum = qec.minimum; + p->maximum = qec.maximum; + p->step = qec.step; + p->default_value = qec.default_value; + break; + default: + p->minimum = 0; + p->maximum = 0; + p->step = 0; + p->default_value = 0; + break; + } + + return 0; } static int v4l_query_ext_ctrl(const struct v4l2_ioctl_ops *ops, -- 2.47.0.338.g60cca15819-goog
The driver was missing support for query_ext_ctrl. Instead of adding a new callback for it, replace the current implementation of queryctrl and let the ioctl framework emulate the old function. Most of the fields are identical, so the change is pretty simple. Signed-off-by: Ricardo Ribalda <ribalda@chromium.org> --- drivers/media/usb/pvrusb2/pvrusb2-v4l2.c | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/drivers/media/usb/pvrusb2/pvrusb2-v4l2.c b/drivers/media/usb/pvrusb2/pvrusb2-v4l2.c index XXXXXXX..XXXXXXX 100644 --- a/drivers/media/usb/pvrusb2/pvrusb2-v4l2.c +++ b/drivers/media/usb/pvrusb2/pvrusb2-v4l2.c @@ -XXX,XX +XXX,XX @@ static int pvr2_streamoff(struct file *file, void *priv, enum v4l2_buf_type i) return pvr2_hdw_set_streaming(hdw, 0); } -static int pvr2_queryctrl(struct file *file, void *priv, - struct v4l2_queryctrl *vc) +static int pvr2_query_ext_ctrl(struct file *file, void *priv, + struct v4l2_query_ext_ctrl *vc) { struct pvr2_v4l2_fh *fh = file->private_data; struct pvr2_hdw *hdw = fh->channel.mc_head->hdw; @@ -XXX,XX +XXX,XX @@ static int pvr2_queryctrl(struct file *file, void *priv, } pvr2_trace(PVR2_TRACE_V4LIOCTL, - "QUERYCTRL id=0x%x mapping name=%s (%s)", + "QUERYEXTCTRL id=0x%x mapping name=%s (%s)", vc->id, pvr2_ctrl_get_name(cptr), pvr2_ctrl_get_desc(cptr)); strscpy(vc->name, pvr2_ctrl_get_desc(cptr), sizeof(vc->name)); vc->flags = pvr2_ctrl_get_v4lflags(cptr); pvr2_ctrl_get_def(cptr, &val); vc->default_value = val; + vc->nr_of_dims = 0; + vc->elems = 1; + vc->elem_size = 4; switch (pvr2_ctrl_get_type(cptr)) { case pvr2_ctl_enum: vc->type = V4L2_CTRL_TYPE_MENU; @@ -XXX,XX +XXX,XX @@ static int pvr2_queryctrl(struct file *file, void *priv, break; default: pvr2_trace(PVR2_TRACE_V4LIOCTL, - "QUERYCTRL id=0x%x name=%s not mappable", + "QUERYEXTCTRL id=0x%x name=%s not mappable", vc->id, pvr2_ctrl_get_name(cptr)); return -EINVAL; } @@ -XXX,XX +XXX,XX @@ static const struct v4l2_ioctl_ops pvr2_ioctl_ops = { .vidioc_try_fmt_vid_cap = pvr2_try_fmt_vid_cap, .vidioc_streamon = pvr2_streamon, .vidioc_streamoff = pvr2_streamoff, - .vidioc_queryctrl = pvr2_queryctrl, + .vidioc_query_ext_ctrl = pvr2_query_ext_ctrl, .vidioc_querymenu = pvr2_querymenu, .vidioc_g_ctrl = pvr2_g_ctrl, .vidioc_s_ctrl = pvr2_s_ctrl, -- 2.47.0.338.g60cca15819-goog
The ioctl helpers can emulate g/s_ctrl with g/s_ext_ctrl. Simplify the code. Signed-off-by: Ricardo Ribalda <ribalda@chromium.org> --- drivers/media/usb/pvrusb2/pvrusb2-v4l2.c | 27 --------------------------- 1 file changed, 27 deletions(-) diff --git a/drivers/media/usb/pvrusb2/pvrusb2-v4l2.c b/drivers/media/usb/pvrusb2/pvrusb2-v4l2.c index XXXXXXX..XXXXXXX 100644 --- a/drivers/media/usb/pvrusb2/pvrusb2-v4l2.c +++ b/drivers/media/usb/pvrusb2/pvrusb2-v4l2.c @@ -XXX,XX +XXX,XX @@ static int pvr2_querymenu(struct file *file, void *priv, struct v4l2_querymenu * return ret; } -static int pvr2_g_ctrl(struct file *file, void *priv, struct v4l2_control *vc) -{ - struct pvr2_v4l2_fh *fh = file->private_data; - struct pvr2_hdw *hdw = fh->channel.mc_head->hdw; - int val = 0; - int ret; - - ret = pvr2_ctrl_get_value(pvr2_hdw_get_ctrl_v4l(hdw, vc->id), - &val); - vc->value = val; - return ret; -} - -static int pvr2_s_ctrl(struct file *file, void *priv, struct v4l2_control *vc) -{ - struct pvr2_v4l2_fh *fh = file->private_data; - struct pvr2_hdw *hdw = fh->channel.mc_head->hdw; - int ret; - - ret = pvr2_ctrl_set_value(pvr2_hdw_get_ctrl_v4l(hdw, vc->id), - vc->value); - pvr2_hdw_commit_ctl(hdw); - return ret; -} - static int pvr2_g_ext_ctrls(struct file *file, void *priv, struct v4l2_ext_controls *ctls) { @@ -XXX,XX +XXX,XX @@ static const struct v4l2_ioctl_ops pvr2_ioctl_ops = { .vidioc_streamoff = pvr2_streamoff, .vidioc_query_ext_ctrl = pvr2_query_ext_ctrl, .vidioc_querymenu = pvr2_querymenu, - .vidioc_g_ctrl = pvr2_g_ctrl, - .vidioc_s_ctrl = pvr2_s_ctrl, .vidioc_g_ext_ctrls = pvr2_g_ext_ctrls, .vidioc_s_ext_ctrls = pvr2_s_ext_ctrls, .vidioc_try_ext_ctrls = pvr2_try_ext_ctrls, -- 2.47.0.338.g60cca15819-goog
It can be implemented by the v4l2 ioctl framework using vidioc_query_ext_ctrl. Signed-off-by: Ricardo Ribalda <ribalda@chromium.org> --- drivers/media/usb/uvc/uvc_v4l2.c | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/drivers/media/usb/uvc/uvc_v4l2.c b/drivers/media/usb/uvc/uvc_v4l2.c index XXXXXXX..XXXXXXX 100644 --- a/drivers/media/usb/uvc/uvc_v4l2.c +++ b/drivers/media/usb/uvc/uvc_v4l2.c @@ -XXX,XX +XXX,XX @@ static int uvc_ioctl_s_input(struct file *file, void *fh, unsigned int input) return ret; } -static int uvc_ioctl_queryctrl(struct file *file, void *fh, - struct v4l2_queryctrl *qc) -{ - struct uvc_fh *handle = fh; - struct uvc_video_chain *chain = handle->chain; - - return uvc_query_v4l2_ctrl(chain, qc); -} - static int uvc_ioctl_query_ext_ctrl(struct file *file, void *fh, struct v4l2_query_ext_ctrl *qec) { @@ -XXX,XX +XXX,XX @@ const struct v4l2_ioctl_ops uvc_ioctl_ops = { .vidioc_enum_input = uvc_ioctl_enum_input, .vidioc_g_input = uvc_ioctl_g_input, .vidioc_s_input = uvc_ioctl_s_input, - .vidioc_queryctrl = uvc_ioctl_queryctrl, .vidioc_query_ext_ctrl = uvc_ioctl_query_ext_ctrl, .vidioc_g_ext_ctrls = uvc_ioctl_g_ext_ctrls, .vidioc_s_ext_ctrls = uvc_ioctl_s_ext_ctrls, -- 2.47.0.338.g60cca15819-goog
The ioctl framework provides an emulator of queryctrl using query_ext_ctrl. Replace our implementation of queryctrl to support both. Now that we are at it: - Add comment about missing functionality. - Remove superfluous clear of reserved[0]. - Remove ret var. Signed-off-by: Ricardo Ribalda <ribalda@chromium.org> --- drivers/staging/media/atomisp/pci/atomisp_ioctl.c | 33 ++++++++++++----------- 1 file changed, 18 insertions(+), 15 deletions(-) diff --git a/drivers/staging/media/atomisp/pci/atomisp_ioctl.c b/drivers/staging/media/atomisp/pci/atomisp_ioctl.c index XXXXXXX..XXXXXXX 100644 --- a/drivers/staging/media/atomisp/pci/atomisp_ioctl.c +++ b/drivers/staging/media/atomisp/pci/atomisp_ioctl.c @@ -XXX,XX +XXX,XX @@ static const char *CARD = "ATOM ISP"; /* max size 31 */ * FIXME: ISP should not know beforehand all CIDs supported by sensor. * Instead, it needs to propagate to sensor unknown CIDs. */ -static struct v4l2_queryctrl ci_v4l2_controls[] = { +static struct v4l2_query_ext_ctrl ci_v4l2_controls[] = { { .id = V4L2_CID_AUTO_WHITE_BALANCE, .type = V4L2_CTRL_TYPE_BOOLEAN, @@ -XXX,XX +XXX,XX @@ static int atomisp_s_ctrl(struct file *file, void *fh, /* * To query the attributes of a control. - * applications set the id field of a struct v4l2_queryctrl and call the + * applications set the id field of a struct v4l2_query_ext_ctrl and call the * this ioctl with a pointer to this structure. The driver fills * the rest of the structure. */ -static int atomisp_queryctl(struct file *file, void *fh, - struct v4l2_queryctrl *qc) +static int atomisp_query_ext_ctrl(struct file *file, void *fh, + struct v4l2_query_ext_ctrl *qc) { - int i, ret = -EINVAL; + int i; + /* TODO: implement V4L2_CTRL_FLAG_NEXT_CTRL */ if (qc->id & V4L2_CTRL_FLAG_NEXT_CTRL) - return ret; + return -EINVAL; for (i = 0; i < ctrls_num; i++) { if (ci_v4l2_controls[i].id == qc->id) { - memcpy(qc, &ci_v4l2_controls[i], - sizeof(struct v4l2_queryctrl)); - qc->reserved[0] = 0; - ret = 0; - break; + *qc = ci_v4l2_controls[i]; + qc->elems = 1; + qc->elem_size = 4; + return 0; } } - if (ret != 0) - qc->flags = V4L2_CTRL_FLAG_DISABLED; - return ret; + /* + * This is probably not needed, but this flag has been set for + * many kernel versions. Leave it to avoid breaking any apps. + */ + qc->flags = V4L2_CTRL_FLAG_DISABLED; + return -EINVAL; } static int atomisp_camera_g_ext_ctrls(struct file *file, void *fh, @@ -XXX,XX +XXX,XX @@ const struct v4l2_ioctl_ops atomisp_ioctl_ops = { .vidioc_enum_input = atomisp_enum_input, .vidioc_g_input = atomisp_g_input, .vidioc_s_input = atomisp_s_input, - .vidioc_queryctrl = atomisp_queryctl, + .vidioc_query_ext_ctrl = atomisp_query_ext_ctrl, .vidioc_s_ctrl = atomisp_s_ctrl, .vidioc_g_ctrl = atomisp_g_ctrl, .vidioc_s_ext_ctrls = atomisp_s_ext_ctrls, -- 2.47.0.338.g60cca15819-goog
The v4l2 ioctl framework can implement vidioc_g/s_ctrl with vidioc_g/s_ext_ctrl() and we provide those. These are the last references of vidioc_s/g_ctrl in the codebase. We can attempt to remove them now. Signed-off-by: Ricardo Ribalda <ribalda@chromium.org> --- drivers/staging/media/atomisp/pci/atomisp_ioctl.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/drivers/staging/media/atomisp/pci/atomisp_ioctl.c b/drivers/staging/media/atomisp/pci/atomisp_ioctl.c index XXXXXXX..XXXXXXX 100644 --- a/drivers/staging/media/atomisp/pci/atomisp_ioctl.c +++ b/drivers/staging/media/atomisp/pci/atomisp_ioctl.c @@ -XXX,XX +XXX,XX @@ const struct v4l2_ioctl_ops atomisp_ioctl_ops = { .vidioc_g_input = atomisp_g_input, .vidioc_s_input = atomisp_s_input, .vidioc_query_ext_ctrl = atomisp_query_ext_ctrl, - .vidioc_s_ctrl = atomisp_s_ctrl, - .vidioc_g_ctrl = atomisp_g_ctrl, .vidioc_s_ext_ctrls = atomisp_s_ext_ctrls, .vidioc_g_ext_ctrls = atomisp_g_ext_ctrls, .vidioc_enum_framesizes = atomisp_enum_framesizes, -- 2.47.0.338.g60cca15819-goog
All the drivers either use the control framework or provide a vidioc_query_ext_ctrl. We can remove this callback to reduce the temptation of new drivers to implement it. Signed-off-by: Ricardo Ribalda <ribalda@chromium.org> --- drivers/media/v4l2-core/v4l2-dev.c | 3 +-- drivers/media/v4l2-core/v4l2-ioctl.c | 2 -- include/media/v4l2-ioctl.h | 4 ---- 3 files changed, 1 insertion(+), 8 deletions(-) diff --git a/drivers/media/v4l2-core/v4l2-dev.c b/drivers/media/v4l2-core/v4l2-dev.c index XXXXXXX..XXXXXXX 100644 --- a/drivers/media/v4l2-core/v4l2-dev.c +++ b/drivers/media/v4l2-core/v4l2-dev.c @@ -XXX,XX +XXX,XX @@ static void determine_valid_ioctls(struct video_device *vdev) and that can't be tested here. If the bit for these control ioctls is set, then the ioctl is valid. But if it is 0, then it can still be valid if the filehandle passed the control handler. */ - if (vdev->ctrl_handler || ops->vidioc_queryctrl || - ops->vidioc_query_ext_ctrl) + if (vdev->ctrl_handler || ops->vidioc_query_ext_ctrl) __set_bit(_IOC_NR(VIDIOC_QUERYCTRL), valid_ioctls); if (vdev->ctrl_handler || ops->vidioc_query_ext_ctrl) __set_bit(_IOC_NR(VIDIOC_QUERY_EXT_CTRL), valid_ioctls); diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c b/drivers/media/v4l2-core/v4l2-ioctl.c index XXXXXXX..XXXXXXX 100644 --- a/drivers/media/v4l2-core/v4l2-ioctl.c +++ b/drivers/media/v4l2-core/v4l2-ioctl.c @@ -XXX,XX +XXX,XX @@ static int v4l_queryctrl(const struct v4l2_ioctl_ops *ops, return v4l2_queryctrl(vfh->ctrl_handler, p); if (vfd->ctrl_handler) return v4l2_queryctrl(vfd->ctrl_handler, p); - if (ops->vidioc_queryctrl) - return ops->vidioc_queryctrl(file, fh, p); if (!ops->vidioc_query_ext_ctrl) return -ENOTTY; diff --git a/include/media/v4l2-ioctl.h b/include/media/v4l2-ioctl.h index XXXXXXX..XXXXXXX 100644 --- a/include/media/v4l2-ioctl.h +++ b/include/media/v4l2-ioctl.h @@ -XXX,XX +XXX,XX @@ struct v4l2_fh; * :ref:`VIDIOC_G_OUTPUT <vidioc_g_output>` ioctl * @vidioc_s_output: pointer to the function that implements * :ref:`VIDIOC_S_OUTPUT <vidioc_g_output>` ioctl - * @vidioc_queryctrl: pointer to the function that implements - * :ref:`VIDIOC_QUERYCTRL <vidioc_queryctrl>` ioctl * @vidioc_query_ext_ctrl: pointer to the function that implements * :ref:`VIDIOC_QUERY_EXT_CTRL <vidioc_queryctrl>` ioctl * @vidioc_g_ctrl: pointer to the function that implements @@ -XXX,XX +XXX,XX @@ struct v4l2_ioctl_ops { int (*vidioc_s_output)(struct file *file, void *fh, unsigned int i); /* Control handling */ - int (*vidioc_queryctrl)(struct file *file, void *fh, - struct v4l2_queryctrl *a); int (*vidioc_query_ext_ctrl)(struct file *file, void *fh, struct v4l2_query_ext_ctrl *a); int (*vidioc_g_ctrl)(struct file *file, void *fh, -- 2.47.0.338.g60cca15819-goog
All the drivers either use the control framework or provide a vidioc_g_ext_ctrls callback. We can remove this callback. Thanks for your service! Signed-off-by: Ricardo Ribalda <ribalda@chromium.org> --- drivers/media/v4l2-core/v4l2-dev.c | 2 +- drivers/media/v4l2-core/v4l2-ioctl.c | 2 -- include/media/v4l2-ioctl.h | 4 ---- 3 files changed, 1 insertion(+), 7 deletions(-) diff --git a/drivers/media/v4l2-core/v4l2-dev.c b/drivers/media/v4l2-core/v4l2-dev.c index XXXXXXX..XXXXXXX 100644 --- a/drivers/media/v4l2-core/v4l2-dev.c +++ b/drivers/media/v4l2-core/v4l2-dev.c @@ -XXX,XX +XXX,XX @@ static void determine_valid_ioctls(struct video_device *vdev) __set_bit(_IOC_NR(VIDIOC_QUERYCTRL), valid_ioctls); if (vdev->ctrl_handler || ops->vidioc_query_ext_ctrl) __set_bit(_IOC_NR(VIDIOC_QUERY_EXT_CTRL), valid_ioctls); - if (vdev->ctrl_handler || ops->vidioc_g_ctrl || ops->vidioc_g_ext_ctrls) + if (vdev->ctrl_handler || ops->vidioc_g_ext_ctrls) __set_bit(_IOC_NR(VIDIOC_G_CTRL), valid_ioctls); if (vdev->ctrl_handler || ops->vidioc_s_ctrl || ops->vidioc_s_ext_ctrls) __set_bit(_IOC_NR(VIDIOC_S_CTRL), valid_ioctls); diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c b/drivers/media/v4l2-core/v4l2-ioctl.c index XXXXXXX..XXXXXXX 100644 --- a/drivers/media/v4l2-core/v4l2-ioctl.c +++ b/drivers/media/v4l2-core/v4l2-ioctl.c @@ -XXX,XX +XXX,XX @@ static int v4l_g_ctrl(const struct v4l2_ioctl_ops *ops, return v4l2_g_ctrl(vfh->ctrl_handler, p); if (vfd->ctrl_handler) return v4l2_g_ctrl(vfd->ctrl_handler, p); - if (ops->vidioc_g_ctrl) - return ops->vidioc_g_ctrl(file, fh, p); if (ops->vidioc_g_ext_ctrls == NULL) return -ENOTTY; diff --git a/include/media/v4l2-ioctl.h b/include/media/v4l2-ioctl.h index XXXXXXX..XXXXXXX 100644 --- a/include/media/v4l2-ioctl.h +++ b/include/media/v4l2-ioctl.h @@ -XXX,XX +XXX,XX @@ struct v4l2_fh; * :ref:`VIDIOC_S_OUTPUT <vidioc_g_output>` ioctl * @vidioc_query_ext_ctrl: pointer to the function that implements * :ref:`VIDIOC_QUERY_EXT_CTRL <vidioc_queryctrl>` ioctl - * @vidioc_g_ctrl: pointer to the function that implements - * :ref:`VIDIOC_G_CTRL <vidioc_g_ctrl>` ioctl * @vidioc_s_ctrl: pointer to the function that implements * :ref:`VIDIOC_S_CTRL <vidioc_g_ctrl>` ioctl * @vidioc_g_ext_ctrls: pointer to the function that implements @@ -XXX,XX +XXX,XX @@ struct v4l2_ioctl_ops { /* Control handling */ int (*vidioc_query_ext_ctrl)(struct file *file, void *fh, struct v4l2_query_ext_ctrl *a); - int (*vidioc_g_ctrl)(struct file *file, void *fh, - struct v4l2_control *a); int (*vidioc_s_ctrl)(struct file *file, void *fh, struct v4l2_control *a); int (*vidioc_g_ext_ctrls)(struct file *file, void *fh, -- 2.47.0.338.g60cca15819-goog
The v4l2 ioctl framework can provide support for s_ctrl. This the last driver implementing s_ctrl. Signed-off-by: Ricardo Ribalda <ribalda@chromium.org> --- drivers/media/usb/cx231xx/cx231xx-417.c | 21 ++++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-) diff --git a/drivers/media/usb/cx231xx/cx231xx-417.c b/drivers/media/usb/cx231xx/cx231xx-417.c index XXXXXXX..XXXXXXX 100644 --- a/drivers/media/usb/cx231xx/cx231xx-417.c +++ b/drivers/media/usb/cx231xx/cx231xx-417.c @@ -XXX,XX +XXX,XX @@ static int vidioc_s_std(struct file *file, void *priv, v4l2_std_id id) return 0; } -static int vidioc_s_ctrl(struct file *file, void *priv, - struct v4l2_control *ctl) +static int cx231xx_s_ext_ctrls(struct file *file, void *priv, + struct v4l2_ext_controls *ctls) { struct cx231xx *dev = video_drvdata(file); + struct v4l2_control ctl; struct v4l2_subdev *sd; + unsigned int i; - dprintk(3, "enter vidioc_s_ctrl()\n"); + dprintk(3, "enter vidioc_s_ext_ctrl()\n"); /* Update the A/V core */ - v4l2_device_for_each_subdev(sd, &dev->v4l2_dev) - v4l2_s_ctrl(NULL, sd->ctrl_handler, ctl); - dprintk(3, "exit vidioc_s_ctrl()\n"); + for (i = 0; i < ctls->count; i++) { + ctl.id = ctls->controls[i].id; + ctl.value = ctls->controls[i].value; + v4l2_device_for_each_subdev(sd, &dev->v4l2_dev) + v4l2_s_ctrl(NULL, sd->ctrl_handler, &ctl); + ctls->controls[i].value = ctl.value; + } + dprintk(3, "exit vidioc_s_ext_ctrl()\n"); return 0; } @@ -XXX,XX +XXX,XX @@ static const struct v4l2_ioctl_ops mpeg_ioctl_ops = { .vidioc_enum_input = cx231xx_enum_input, .vidioc_g_input = cx231xx_g_input, .vidioc_s_input = cx231xx_s_input, - .vidioc_s_ctrl = vidioc_s_ctrl, + .vidioc_s_ext_ctrls = cx231xx_s_ext_ctrls, .vidioc_g_pixelaspect = vidioc_g_pixelaspect, .vidioc_g_selection = vidioc_g_selection, .vidioc_querycap = cx231xx_querycap, -- 2.47.0.338.g60cca15819-goog
All the drivers either use the control framework or provide a vidiod_ext_ctrl. We can remove this callback. Signed-off-by: Ricardo Ribalda <ribalda@chromium.org> --- drivers/media/v4l2-core/v4l2-dev.c | 2 +- drivers/media/v4l2-core/v4l2-ioctl.c | 2 -- include/media/v4l2-ioctl.h | 4 ---- 3 files changed, 1 insertion(+), 7 deletions(-) diff --git a/drivers/media/v4l2-core/v4l2-dev.c b/drivers/media/v4l2-core/v4l2-dev.c index XXXXXXX..XXXXXXX 100644 --- a/drivers/media/v4l2-core/v4l2-dev.c +++ b/drivers/media/v4l2-core/v4l2-dev.c @@ -XXX,XX +XXX,XX @@ static void determine_valid_ioctls(struct video_device *vdev) __set_bit(_IOC_NR(VIDIOC_QUERY_EXT_CTRL), valid_ioctls); if (vdev->ctrl_handler || ops->vidioc_g_ext_ctrls) __set_bit(_IOC_NR(VIDIOC_G_CTRL), valid_ioctls); - if (vdev->ctrl_handler || ops->vidioc_s_ctrl || ops->vidioc_s_ext_ctrls) + if (vdev->ctrl_handler || ops->vidioc_s_ext_ctrls) __set_bit(_IOC_NR(VIDIOC_S_CTRL), valid_ioctls); if (vdev->ctrl_handler || ops->vidioc_g_ext_ctrls) __set_bit(_IOC_NR(VIDIOC_G_EXT_CTRLS), valid_ioctls); diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c b/drivers/media/v4l2-core/v4l2-ioctl.c index XXXXXXX..XXXXXXX 100644 --- a/drivers/media/v4l2-core/v4l2-ioctl.c +++ b/drivers/media/v4l2-core/v4l2-ioctl.c @@ -XXX,XX +XXX,XX @@ static int v4l_s_ctrl(const struct v4l2_ioctl_ops *ops, return v4l2_s_ctrl(vfh, vfh->ctrl_handler, p); if (vfd->ctrl_handler) return v4l2_s_ctrl(NULL, vfd->ctrl_handler, p); - if (ops->vidioc_s_ctrl) - return ops->vidioc_s_ctrl(file, fh, p); if (ops->vidioc_s_ext_ctrls == NULL) return -ENOTTY; diff --git a/include/media/v4l2-ioctl.h b/include/media/v4l2-ioctl.h index XXXXXXX..XXXXXXX 100644 --- a/include/media/v4l2-ioctl.h +++ b/include/media/v4l2-ioctl.h @@ -XXX,XX +XXX,XX @@ struct v4l2_fh; * :ref:`VIDIOC_S_OUTPUT <vidioc_g_output>` ioctl * @vidioc_query_ext_ctrl: pointer to the function that implements * :ref:`VIDIOC_QUERY_EXT_CTRL <vidioc_queryctrl>` ioctl - * @vidioc_s_ctrl: pointer to the function that implements - * :ref:`VIDIOC_S_CTRL <vidioc_g_ctrl>` ioctl * @vidioc_g_ext_ctrls: pointer to the function that implements * :ref:`VIDIOC_G_EXT_CTRLS <vidioc_g_ext_ctrls>` ioctl * @vidioc_s_ext_ctrls: pointer to the function that implements @@ -XXX,XX +XXX,XX @@ struct v4l2_ioctl_ops { /* Control handling */ int (*vidioc_query_ext_ctrl)(struct file *file, void *fh, struct v4l2_query_ext_ctrl *a); - int (*vidioc_s_ctrl)(struct file *file, void *fh, - struct v4l2_control *a); int (*vidioc_g_ext_ctrls)(struct file *file, void *fh, struct v4l2_ext_controls *a); int (*vidioc_s_ext_ctrls)(struct file *file, void *fh, -- 2.47.0.338.g60cca15819-goog
We use this logic in a couple of places. Refactor into a function. No functional change expected from this patch. Signed-off-by: Ricardo Ribalda <ribalda@chromium.org> --- drivers/media/v4l2-core/v4l2-ctrls-api.c | 51 +++++++++++++++++++------------- drivers/media/v4l2-core/v4l2-ioctl.c | 28 ++---------------- include/media/v4l2-ctrls.h | 12 ++++++++ 3 files changed, 44 insertions(+), 47 deletions(-) diff --git a/drivers/media/v4l2-core/v4l2-ctrls-api.c b/drivers/media/v4l2-core/v4l2-ctrls-api.c index XXXXXXX..XXXXXXX 100644 --- a/drivers/media/v4l2-core/v4l2-ctrls-api.c +++ b/drivers/media/v4l2-core/v4l2-ctrls-api.c @@ -XXX,XX +XXX,XX @@ int v4l2_query_ext_ctrl(struct v4l2_ctrl_handler *hdl, struct v4l2_query_ext_ctr } EXPORT_SYMBOL(v4l2_query_ext_ctrl); -/* Implement VIDIOC_QUERYCTRL */ -int v4l2_queryctrl(struct v4l2_ctrl_handler *hdl, struct v4l2_queryctrl *qc) +void v4l2_query_ext_ctrl_to_v4l2_queryctrl(struct v4l2_queryctrl *to, + const struct v4l2_query_ext_ctrl *from) { - struct v4l2_query_ext_ctrl qec = { qc->id }; - int rc; + to->id = from->id; + to->type = from->type; + to->flags = from->flags; + strscpy(to->name, from->name, sizeof(to->name)); - rc = v4l2_query_ext_ctrl(hdl, &qec); - if (rc) - return rc; - - qc->id = qec.id; - qc->type = qec.type; - qc->flags = qec.flags; - strscpy(qc->name, qec.name, sizeof(qc->name)); - switch (qc->type) { + switch (from->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.minimum; - qc->maximum = qec.maximum; - qc->step = qec.step; - qc->default_value = qec.default_value; + to->minimum = from->minimum; + to->maximum = from->maximum; + to->step = from->step; + to->default_value = from->default_value; break; default: - qc->minimum = 0; - qc->maximum = 0; - qc->step = 0; - qc->default_value = 0; + to->minimum = 0; + to->maximum = 0; + to->step = 0; + to->default_value = 0; break; } +} +EXPORT_SYMBOL(v4l2_query_ext_ctrl_to_v4l2_queryctrl); + +/* 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; + + rc = v4l2_query_ext_ctrl(hdl, &qec); + if (rc) + return rc; + + v4l2_query_ext_ctrl_to_v4l2_queryctrl(qc, &qec); + return 0; } EXPORT_SYMBOL(v4l2_queryctrl); diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c b/drivers/media/v4l2-core/v4l2-ioctl.c index XXXXXXX..XXXXXXX 100644 --- a/drivers/media/v4l2-core/v4l2-ioctl.c +++ b/drivers/media/v4l2-core/v4l2-ioctl.c @@ -XXX,XX +XXX,XX @@ static int v4l_queryctrl(const struct v4l2_ioctl_ops *ops, ret = ops->vidioc_query_ext_ctrl(file, fh, &qec); if (ret) return ret; - - p->id = qec.id; - p->type = qec.type; - p->flags = qec.flags; - strscpy(p->name, qec.name, sizeof(p->name)); - switch (p->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: - p->minimum = qec.minimum; - p->maximum = qec.maximum; - p->step = qec.step; - p->default_value = qec.default_value; - break; - default: - p->minimum = 0; - p->maximum = 0; - p->step = 0; - p->default_value = 0; - break; - } - - return 0; + v4l2_query_ext_ctrl_to_v4l2_queryctrl(p, &qec); + return ret; } static int v4l_query_ext_ctrl(const struct v4l2_ioctl_ops *ops, diff --git a/include/media/v4l2-ctrls.h b/include/media/v4l2-ctrls.h index XXXXXXX..XXXXXXX 100644 --- a/include/media/v4l2-ctrls.h +++ b/include/media/v4l2-ctrls.h @@ -XXX,XX +XXX,XX @@ v4l2_ctrl_request_hdl_ctrl_find(struct v4l2_ctrl_handler *hdl, u32 id); */ int v4l2_queryctrl(struct v4l2_ctrl_handler *hdl, struct v4l2_queryctrl *qc); +/** + * v4l2_query_ext_ctrl_to_v4l2_queryctrl - Convert a qec to qe. + * + * @to: The v4l2_queryctrl to write to. + * @from: The v4l2_query_ext_ctrl to read from. + * + * This function is a helper to convert a v4l2_query_ext_ctrl into a + * v4l2_queryctrl. + */ +void v4l2_query_ext_ctrl_to_v4l2_queryctrl(struct v4l2_queryctrl *to, + const struct v4l2_query_ext_ctrl *from); + /** * v4l2_query_ext_ctrl - Helper function to implement * :ref:`VIDIOC_QUERY_EXT_CTRL <vidioc_queryctrl>` ioctl -- 2.47.0.338.g60cca15819-goog