[v10,02/11,RESEND] media: uvcvideo: add uvc_ctrl_get_boundary for getting default value
Commit Message
Introduce uvc_ctrl_get_boundary(), making it easier to extend to
support compound controls and V4L2_CTRL_WHICH_MIN/MAX_VAL in the
following patches.
For now, it simply returns EINVAL for compound controls and calls
__query_v4l2_ctrl() for non-compound controls.
Reviewed-by: Ricardo Ribalda <ribalda@chromium.org>
Signed-off-by: Yunke Cao <yunkec@google.com>
---
Changelog since v9:
- Make __uvc_ctrl_get_boundary_std() static.
Changelog since v8:
- No Change.
Changelog since v7:
- Rename uvc_ctrl_get_fixed() to uvc_ctrl_get_boundary().
- Move refactor (introduce __uvc_ctrl_get_boundary_std()) in this patch
instead of in the patch where we implement compound control.
- Fix locking. uvc_ctrl_get_boundary() now acquires ctrl_mutex.
__uvc_ctrl_get_boundary_std() calls __uvc_query_v4l2_ctrl() while
holding the mutex.
- Define a uvc_ctrl_mapping_is_compound() for readability.
drivers/media/usb/uvc/uvc_ctrl.c | 49 ++++++++++++++++++++++++++++++++
drivers/media/usb/uvc/uvc_v4l2.c | 6 +---
drivers/media/usb/uvc/uvcvideo.h | 2 ++
3 files changed, 52 insertions(+), 5 deletions(-)
Comments
On (22/12/01 11:31), Yunke Cao wrote:
> Introduce uvc_ctrl_get_boundary(), making it easier to extend to
> support compound controls and V4L2_CTRL_WHICH_MIN/MAX_VAL in the
> following patches.
>
> For now, it simply returns EINVAL for compound controls and calls
> __query_v4l2_ctrl() for non-compound controls.
>
> Reviewed-by: Ricardo Ribalda <ribalda@chromium.org>
> Signed-off-by: Yunke Cao <yunkec@google.com>
Reviewed-by: Sergey Senozhatsky <senozhatsky@chromium.org>
Hi Yunke,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on linus/master]
[also build test WARNING on v6.1-rc7 next-20221201]
[cannot apply to media-tree/master]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Yunke-Cao/media-Implement-UVC-v1-5-ROI/20221201-103422
patch link: https://lore.kernel.org/r/20221201023204.2177458-3-yunkec%40google.com
patch subject: [PATCH v10 02/11 RESEND] media: uvcvideo: add uvc_ctrl_get_boundary for getting default value
config: m68k-allyesconfig
compiler: m68k-linux-gcc (GCC) 12.1.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/intel-lab-lkp/linux/commit/bb57965a09e65c4ed0cc01d63e158bba810648b4
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Yunke-Cao/media-Implement-UVC-v1-5-ROI/20221201-103422
git checkout bb57965a09e65c4ed0cc01d63e158bba810648b4
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=m68k SHELL=/bin/bash drivers/media/usb/uvc/
If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>
All warnings (new ones prefixed by >>):
drivers/media/usb/uvc/uvc_ctrl.c: In function 'uvc_ctrl_get_boundary':
>> drivers/media/usb/uvc/uvc_ctrl.c:1788:73: warning: passing argument 4 of '__uvc_ctrl_get_boundary_std' makes integer from pointer without a cast [-Wint-conversion]
1788 | ret = __uvc_ctrl_get_boundary_std(chain, ctrl, mapping, xctrl);
| ^~~~~
| |
| struct v4l2_ext_control *
drivers/media/usb/uvc/uvc_ctrl.c:1755:44: note: expected 'u32' {aka 'unsigned int'} but argument is of type 'struct v4l2_ext_control *'
1755 | u32 v4l2_which,
| ~~~~^~~~~~~~~~
drivers/media/usb/uvc/uvc_ctrl.c:1788:23: error: too few arguments to function '__uvc_ctrl_get_boundary_std'
1788 | ret = __uvc_ctrl_get_boundary_std(chain, ctrl, mapping, xctrl);
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/media/usb/uvc/uvc_ctrl.c:1752:12: note: declared here
1752 | static int __uvc_ctrl_get_boundary_std(struct uvc_video_chain *chain,
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~
vim +/__uvc_ctrl_get_boundary_std +1788 drivers/media/usb/uvc/uvc_ctrl.c
1768
1769 int uvc_ctrl_get_boundary(struct uvc_video_chain *chain,
1770 struct v4l2_ext_control *xctrl)
1771 {
1772 struct uvc_control *ctrl;
1773 struct uvc_control_mapping *mapping;
1774 int ret;
1775
1776 if (mutex_lock_interruptible(&chain->ctrl_mutex))
1777 return -ERESTARTSYS;
1778
1779 ctrl = uvc_find_control(chain, xctrl->id, &mapping);
1780 if (!ctrl) {
1781 ret = -EINVAL;
1782 goto done;
1783 }
1784
1785 if (uvc_ctrl_mapping_is_compound(mapping))
1786 ret = -EINVAL;
1787 else
> 1788 ret = __uvc_ctrl_get_boundary_std(chain, ctrl, mapping, xctrl);
1789
1790 done:
1791 mutex_unlock(&chain->ctrl_mutex);
1792 return ret;
1793 }
1794
Hi!
Thanks for the review.
On Thu, Dec 1, 2022 at 4:16 PM kernel test robot <lkp@intel.com> wrote:
>
> Hi Yunke,
>
> Thank you for the patch! Perhaps something to improve:
>
> [auto build test WARNING on linus/master]
> [also build test WARNING on v6.1-rc7 next-20221201]
> [cannot apply to media-tree/master]
> [If your patch is applied to the wrong git tree, kindly drop us a note.
> And when submitting patch, we suggest to use '--base' as documented in
> https://git-scm.com/docs/git-format-patch#_base_tree_information]
>
> url: https://github.com/intel-lab-lkp/linux/commits/Yunke-Cao/media-Implement-UVC-v1-5-ROI/20221201-103422
> patch link: https://lore.kernel.org/r/20221201023204.2177458-3-yunkec%40google.com
> patch subject: [PATCH v10 02/11 RESEND] media: uvcvideo: add uvc_ctrl_get_boundary for getting default value
> config: m68k-allyesconfig
> compiler: m68k-linux-gcc (GCC) 12.1.0
> reproduce (this is a W=1 build):
> wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
> chmod +x ~/bin/make.cross
> # https://github.com/intel-lab-lkp/linux/commit/bb57965a09e65c4ed0cc01d63e158bba810648b4
> git remote add linux-review https://github.com/intel-lab-lkp/linux
> git fetch --no-tags linux-review Yunke-Cao/media-Implement-UVC-v1-5-ROI/20221201-103422
> git checkout bb57965a09e65c4ed0cc01d63e158bba810648b4
> # save the config file
> mkdir build_dir && cp config build_dir/.config
> COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=m68k SHELL=/bin/bash drivers/media/usb/uvc/
>
> If you fix the issue, kindly add following tag where applicable
> | Reported-by: kernel test robot <lkp@intel.com>
>
> All warnings (new ones prefixed by >>):
>
> drivers/media/usb/uvc/uvc_ctrl.c: In function 'uvc_ctrl_get_boundary':
> >> drivers/media/usb/uvc/uvc_ctrl.c:1788:73: warning: passing argument 4 of '__uvc_ctrl_get_boundary_std' makes integer from pointer without a cast [-Wint-conversion]
> 1788 | ret = __uvc_ctrl_get_boundary_std(chain, ctrl, mapping, xctrl);
> | ^~~~~
> | |
> | struct v4l2_ext_control *
> drivers/media/usb/uvc/uvc_ctrl.c:1755:44: note: expected 'u32' {aka 'unsigned int'} but argument is of type 'struct v4l2_ext_control *'
> 1755 | u32 v4l2_which,
> | ~~~~^~~~~~~~~~
1755 | u32 v4l2_which,
This parameter should have been added as part of patch 10/11 instead
of here. Sorry!
Will fix it in the next version.
> drivers/media/usb/uvc/uvc_ctrl.c:1788:23: error: too few arguments to function '__uvc_ctrl_get_boundary_std'
> 1788 | ret = __uvc_ctrl_get_boundary_std(chain, ctrl, mapping, xctrl);
> | ^~~~~~~~~~~~~~~~~~~~~~~~~~~
> drivers/media/usb/uvc/uvc_ctrl.c:1752:12: note: declared here
> 1752 | static int __uvc_ctrl_get_boundary_std(struct uvc_video_chain *chain,
> | ^~~~~~~~~~~~~~~~~~~~~~~~~~~
>
>
> vim +/__uvc_ctrl_get_boundary_std +1788 drivers/media/usb/uvc/uvc_ctrl.c
>
> 1768
> 1769 int uvc_ctrl_get_boundary(struct uvc_video_chain *chain,
> 1770 struct v4l2_ext_control *xctrl)
> 1771 {
> 1772 struct uvc_control *ctrl;
> 1773 struct uvc_control_mapping *mapping;
> 1774 int ret;
> 1775
> 1776 if (mutex_lock_interruptible(&chain->ctrl_mutex))
> 1777 return -ERESTARTSYS;
> 1778
> 1779 ctrl = uvc_find_control(chain, xctrl->id, &mapping);
> 1780 if (!ctrl) {
> 1781 ret = -EINVAL;
> 1782 goto done;
> 1783 }
> 1784
> 1785 if (uvc_ctrl_mapping_is_compound(mapping))
> 1786 ret = -EINVAL;
> 1787 else
> > 1788 ret = __uvc_ctrl_get_boundary_std(chain, ctrl, mapping, xctrl);
> 1789
> 1790 done:
> 1791 mutex_unlock(&chain->ctrl_mutex);
> 1792 return ret;
> 1793 }
> 1794
>
> --
> 0-DAY CI Kernel Test Service
> https://01.org/lkp
@@ -837,6 +837,12 @@ static void uvc_set_le_value(struct uvc_control_mapping *mapping,
}
}
+static bool
+uvc_ctrl_mapping_is_compound(const struct uvc_control_mapping *mapping)
+{
+ return mapping->v4l2_type >= V4L2_CTRL_COMPOUND_TYPES;
+}
+
/* ------------------------------------------------------------------------
* Terminal and unit management
*/
@@ -1743,6 +1749,49 @@ int uvc_ctrl_get(struct uvc_video_chain *chain,
return __uvc_ctrl_get(chain, ctrl, mapping, &xctrl->value);
}
+static int __uvc_ctrl_get_boundary_std(struct uvc_video_chain *chain,
+ struct uvc_control *ctrl,
+ struct uvc_control_mapping *mapping,
+ u32 v4l2_which,
+ struct v4l2_ext_control *xctrl)
+{
+ struct v4l2_queryctrl qc = { .id = xctrl->id };
+
+ int ret = __uvc_query_v4l2_ctrl(chain, ctrl, mapping, &qc);
+
+ if (ret < 0)
+ return ret;
+
+ xctrl->value = qc.default_value;
+ return 0;
+}
+
+int uvc_ctrl_get_boundary(struct uvc_video_chain *chain,
+ struct v4l2_ext_control *xctrl)
+{
+ struct uvc_control *ctrl;
+ struct uvc_control_mapping *mapping;
+ int ret;
+
+ if (mutex_lock_interruptible(&chain->ctrl_mutex))
+ return -ERESTARTSYS;
+
+ ctrl = uvc_find_control(chain, xctrl->id, &mapping);
+ if (!ctrl) {
+ ret = -EINVAL;
+ goto done;
+ }
+
+ if (uvc_ctrl_mapping_is_compound(mapping))
+ ret = -EINVAL;
+ else
+ ret = __uvc_ctrl_get_boundary_std(chain, ctrl, mapping, xctrl);
+
+done:
+ mutex_unlock(&chain->ctrl_mutex);
+ return ret;
+}
+
int uvc_ctrl_set(struct uvc_fh *handle,
struct v4l2_ext_control *xctrl)
{
@@ -1046,15 +1046,11 @@ static int uvc_ioctl_g_ext_ctrls(struct file *file, void *fh,
if (ctrls->which == V4L2_CTRL_WHICH_DEF_VAL) {
for (i = 0; i < ctrls->count; ++ctrl, ++i) {
- struct v4l2_queryctrl qc = { .id = ctrl->id };
-
- ret = uvc_query_v4l2_ctrl(chain, &qc);
+ ret = uvc_ctrl_get_boundary(chain, ctrl);
if (ret < 0) {
ctrls->error_idx = i;
return ret;
}
-
- ctrl->value = qc.default_value;
}
return 0;
@@ -759,6 +759,8 @@ static inline int uvc_ctrl_rollback(struct uvc_fh *handle)
}
int uvc_ctrl_get(struct uvc_video_chain *chain, struct v4l2_ext_control *xctrl);
+int uvc_ctrl_get_boundary(struct uvc_video_chain *chain,
+ struct v4l2_ext_control *xctrl);
int uvc_ctrl_set(struct uvc_fh *handle, struct v4l2_ext_control *xctrl);
int uvc_ctrl_is_accessible(struct uvc_video_chain *chain, u32 v4l2_id,
bool read);