From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
To: Yunke Cao <yunkec@google.com>
Cc: Hans Verkuil <hverkuil-cisco@xs4all.nl>,
Daniel Scally <dan.scally@ideasonboard.com>,
Tomasz Figa <tfiga@chromium.org>,
Sergey Senozhatsky <senozhatsky@chromium.org>,
Ricardo Ribalda <ribalda@chromium.org>,
linux-media@vger.kernel.org
Subject: Re: [PATCH v14 04/11] media: uvcvideo: Split uvc_control_mapping.size to v4l2 and data size
Date: Mon, 18 Dec 2023 05:17:47 +0200 [thread overview]
Message-ID: <20231218031747.GL5290@pendragon.ideasonboard.com> (raw)
In-Reply-To: <CANqU6FfgruB7tO6L_wP9zyrkOTEHf27XSuUdisAbocLD=_Eing@mail.gmail.com>
Hi Yunke,
On Tue, Dec 12, 2023 at 04:59:59PM +0900, Yunke Cao wrote:
> On Fri, Dec 8, 2023 at 11:15 PM Laurent Pinchart wrote:
> > On Fri, Dec 01, 2023 at 04:18:55PM +0900, Yunke Cao wrote:
> > > Rename the existing size to data_size to represent uvc control data size,
> > > add a separate field for v4l2 control size. v4l2 control size will be
> > > used the compound controls.
> >
> > s/uvc/UVC/ and s/v4l2/V4L2/ in the whole commit message.
> >
> > > Also modify the uvc driver documents to clarify the size in
> > > uvc_xu_control_mapping corresponds to the uvc control data size.
> > >
> > > Reviewed-by: Daniel Scally <dan.scally@ideasonboard.com>
> > > Signed-off-by: Yunke Cao <yunkec@google.com>
> > > ---
> > > Changelog since v11:
> > > - No change.
> > > Changelog since v10:
> > > - Added Reviewed-by from Daniel Scally.
> > > Changelog since v9:
> > > - No change.
> > > Changelog since v8:
> > > - No change.
> > > Changelog since v7:
> > > - Newly added patch.
> > >
> > > .../userspace-api/media/drivers/uvcvideo.rst | 2 +-
> > > drivers/media/usb/uvc/uvc_ctrl.c | 80 +++++++++----------
> > > drivers/media/usb/uvc/uvc_v4l2.c | 2 +-
> > > drivers/media/usb/uvc/uvcvideo.h | 6 +-
> > > 4 files changed, 47 insertions(+), 43 deletions(-)
> > >
> > > diff --git a/Documentation/userspace-api/media/drivers/uvcvideo.rst b/Documentation/userspace-api/media/drivers/uvcvideo.rst
> > > index a290f9fadae9..aab4304e6bb5 100644
> > > --- a/Documentation/userspace-api/media/drivers/uvcvideo.rst
> > > +++ b/Documentation/userspace-api/media/drivers/uvcvideo.rst
> > > @@ -157,7 +157,7 @@ Argument: struct uvc_xu_control_mapping
> > > __u8 name[32] V4L2 control name
> > > __u8 entity[16] UVC extension unit GUID
> > > __u8 selector UVC control selector
> > > - __u8 size V4L2 control size (in bits)
> > > + __u8 size UVC control data size (in bits)
> >
> > The V4L2 and UVC sizes are identical for all controls mapped through
> > this mechanism, right ?
>
> Yes, I think that's right in the current implementation.
> Do you think we need to support UVCIOC_CTRL_MAP for the compound controls?
No, I wouldn't do so. When I introduced the UVCIOC_CTRL_MAP ioctl, I was
envisioning vendors submitting descriptions for a large number of XU
controls. I thought it would be inconvenient to constantly update the
uvcvideo driver with new mappings.
Years have gone by, and the tsunami of XU control mappings never came.
In restrospect it may have been better to simply add mappings to the
kernel driver (or maybe today we would use BPF, it seems to be popular).
I wouldn't extend the mechanism to support compound controls unless
there's a reason to believe the situation will change and lots of XU
compound controls will need to be mapped.
> I wanted to change this because simply we assign
> + map->data_size = xmap->size;
> in uvc_ioctl_xu_ctrl_map().
>
> Is this okay?
I'm fine with that. Maybe the comment could state
V4L2 and UVC control data size (in bits)
?
> > > __u8 offset V4L2 control offset (in bits)
> >
> > If the size if the "UVC control data size", shouldn't this be the "UVC
> > control data offset" ?
>
> Ah, right. I will change this in the v15 if we keep the "UVC control data size".
>
> > > enum v4l2_ctrl_type
> > > v4l2_type V4L2 control type
> > > diff --git a/drivers/media/usb/uvc/uvc_ctrl.c b/drivers/media/usb/uvc/uvc_ctrl.c
> > > index 4a685532f7eb..98254b93eb46 100644
> > > --- a/drivers/media/usb/uvc/uvc_ctrl.c
> > > +++ b/drivers/media/usb/uvc/uvc_ctrl.c
> > > @@ -464,7 +464,7 @@ static const struct uvc_control_mapping uvc_ctrl_mappings[] = {
> > > .id = V4L2_CID_BRIGHTNESS,
> > > .entity = UVC_GUID_UVC_PROCESSING,
> > > .selector = UVC_PU_BRIGHTNESS_CONTROL,
> > > - .size = 16,
> > > + .data_size = 16,
> > > .offset = 0,
> > > .v4l2_type = V4L2_CTRL_TYPE_INTEGER,
> > > .data_type = UVC_CTRL_DATA_TYPE_SIGNED,
> > > @@ -473,7 +473,7 @@ static const struct uvc_control_mapping uvc_ctrl_mappings[] = {
> > > .id = V4L2_CID_CONTRAST,
> > > .entity = UVC_GUID_UVC_PROCESSING,
> > > .selector = UVC_PU_CONTRAST_CONTROL,
> > > - .size = 16,
> > > + .data_size = 16,
> > > .offset = 0,
> > > .v4l2_type = V4L2_CTRL_TYPE_INTEGER,
> > > .data_type = UVC_CTRL_DATA_TYPE_UNSIGNED,
> > > @@ -482,7 +482,7 @@ static const struct uvc_control_mapping uvc_ctrl_mappings[] = {
> > > .id = V4L2_CID_HUE,
> > > .entity = UVC_GUID_UVC_PROCESSING,
> > > .selector = UVC_PU_HUE_CONTROL,
> > > - .size = 16,
> > > + .data_size = 16,
> > > .offset = 0,
> > > .v4l2_type = V4L2_CTRL_TYPE_INTEGER,
> > > .data_type = UVC_CTRL_DATA_TYPE_SIGNED,
> > > @@ -493,7 +493,7 @@ static const struct uvc_control_mapping uvc_ctrl_mappings[] = {
> > > .id = V4L2_CID_SATURATION,
> > > .entity = UVC_GUID_UVC_PROCESSING,
> > > .selector = UVC_PU_SATURATION_CONTROL,
> > > - .size = 16,
> > > + .data_size = 16,
> > > .offset = 0,
> > > .v4l2_type = V4L2_CTRL_TYPE_INTEGER,
> > > .data_type = UVC_CTRL_DATA_TYPE_UNSIGNED,
> > > @@ -502,7 +502,7 @@ static const struct uvc_control_mapping uvc_ctrl_mappings[] = {
> > > .id = V4L2_CID_SHARPNESS,
> > > .entity = UVC_GUID_UVC_PROCESSING,
> > > .selector = UVC_PU_SHARPNESS_CONTROL,
> > > - .size = 16,
> > > + .data_size = 16,
> > > .offset = 0,
> > > .v4l2_type = V4L2_CTRL_TYPE_INTEGER,
> > > .data_type = UVC_CTRL_DATA_TYPE_UNSIGNED,
> > > @@ -511,7 +511,7 @@ static const struct uvc_control_mapping uvc_ctrl_mappings[] = {
> > > .id = V4L2_CID_GAMMA,
> > > .entity = UVC_GUID_UVC_PROCESSING,
> > > .selector = UVC_PU_GAMMA_CONTROL,
> > > - .size = 16,
> > > + .data_size = 16,
> > > .offset = 0,
> > > .v4l2_type = V4L2_CTRL_TYPE_INTEGER,
> > > .data_type = UVC_CTRL_DATA_TYPE_UNSIGNED,
> > > @@ -520,7 +520,7 @@ static const struct uvc_control_mapping uvc_ctrl_mappings[] = {
> > > .id = V4L2_CID_BACKLIGHT_COMPENSATION,
> > > .entity = UVC_GUID_UVC_PROCESSING,
> > > .selector = UVC_PU_BACKLIGHT_COMPENSATION_CONTROL,
> > > - .size = 16,
> > > + .data_size = 16,
> > > .offset = 0,
> > > .v4l2_type = V4L2_CTRL_TYPE_INTEGER,
> > > .data_type = UVC_CTRL_DATA_TYPE_UNSIGNED,
> > > @@ -529,7 +529,7 @@ static const struct uvc_control_mapping uvc_ctrl_mappings[] = {
> > > .id = V4L2_CID_GAIN,
> > > .entity = UVC_GUID_UVC_PROCESSING,
> > > .selector = UVC_PU_GAIN_CONTROL,
> > > - .size = 16,
> > > + .data_size = 16,
> > > .offset = 0,
> > > .v4l2_type = V4L2_CTRL_TYPE_INTEGER,
> > > .data_type = UVC_CTRL_DATA_TYPE_UNSIGNED,
> > > @@ -538,7 +538,7 @@ static const struct uvc_control_mapping uvc_ctrl_mappings[] = {
> > > .id = V4L2_CID_HUE_AUTO,
> > > .entity = UVC_GUID_UVC_PROCESSING,
> > > .selector = UVC_PU_HUE_AUTO_CONTROL,
> > > - .size = 1,
> > > + .data_size = 1,
> > > .offset = 0,
> > > .v4l2_type = V4L2_CTRL_TYPE_BOOLEAN,
> > > .data_type = UVC_CTRL_DATA_TYPE_BOOLEAN,
> > > @@ -548,7 +548,7 @@ static const struct uvc_control_mapping uvc_ctrl_mappings[] = {
> > > .id = V4L2_CID_EXPOSURE_AUTO,
> > > .entity = UVC_GUID_UVC_CAMERA,
> > > .selector = UVC_CT_AE_MODE_CONTROL,
> > > - .size = 4,
> > > + .data_size = 4,
> > > .offset = 0,
> > > .v4l2_type = V4L2_CTRL_TYPE_MENU,
> > > .data_type = UVC_CTRL_DATA_TYPE_BITMASK,
> > > @@ -561,7 +561,7 @@ static const struct uvc_control_mapping uvc_ctrl_mappings[] = {
> > > .id = V4L2_CID_EXPOSURE_AUTO_PRIORITY,
> > > .entity = UVC_GUID_UVC_CAMERA,
> > > .selector = UVC_CT_AE_PRIORITY_CONTROL,
> > > - .size = 1,
> > > + .data_size = 1,
> > > .offset = 0,
> > > .v4l2_type = V4L2_CTRL_TYPE_BOOLEAN,
> > > .data_type = UVC_CTRL_DATA_TYPE_BOOLEAN,
> > > @@ -570,7 +570,7 @@ static const struct uvc_control_mapping uvc_ctrl_mappings[] = {
> > > .id = V4L2_CID_EXPOSURE_ABSOLUTE,
> > > .entity = UVC_GUID_UVC_CAMERA,
> > > .selector = UVC_CT_EXPOSURE_TIME_ABSOLUTE_CONTROL,
> > > - .size = 32,
> > > + .data_size = 32,
> > > .offset = 0,
> > > .v4l2_type = V4L2_CTRL_TYPE_INTEGER,
> > > .data_type = UVC_CTRL_DATA_TYPE_UNSIGNED,
> > > @@ -581,7 +581,7 @@ static const struct uvc_control_mapping uvc_ctrl_mappings[] = {
> > > .id = V4L2_CID_AUTO_WHITE_BALANCE,
> > > .entity = UVC_GUID_UVC_PROCESSING,
> > > .selector = UVC_PU_WHITE_BALANCE_TEMPERATURE_AUTO_CONTROL,
> > > - .size = 1,
> > > + .data_size = 1,
> > > .offset = 0,
> > > .v4l2_type = V4L2_CTRL_TYPE_BOOLEAN,
> > > .data_type = UVC_CTRL_DATA_TYPE_BOOLEAN,
> > > @@ -591,7 +591,7 @@ static const struct uvc_control_mapping uvc_ctrl_mappings[] = {
> > > .id = V4L2_CID_WHITE_BALANCE_TEMPERATURE,
> > > .entity = UVC_GUID_UVC_PROCESSING,
> > > .selector = UVC_PU_WHITE_BALANCE_TEMPERATURE_CONTROL,
> > > - .size = 16,
> > > + .data_size = 16,
> > > .offset = 0,
> > > .v4l2_type = V4L2_CTRL_TYPE_INTEGER,
> > > .data_type = UVC_CTRL_DATA_TYPE_UNSIGNED,
> > > @@ -602,7 +602,7 @@ static const struct uvc_control_mapping uvc_ctrl_mappings[] = {
> > > .id = V4L2_CID_AUTO_WHITE_BALANCE,
> > > .entity = UVC_GUID_UVC_PROCESSING,
> > > .selector = UVC_PU_WHITE_BALANCE_COMPONENT_AUTO_CONTROL,
> > > - .size = 1,
> > > + .data_size = 1,
> > > .offset = 0,
> > > .v4l2_type = V4L2_CTRL_TYPE_BOOLEAN,
> > > .data_type = UVC_CTRL_DATA_TYPE_BOOLEAN,
> > > @@ -613,7 +613,7 @@ static const struct uvc_control_mapping uvc_ctrl_mappings[] = {
> > > .id = V4L2_CID_BLUE_BALANCE,
> > > .entity = UVC_GUID_UVC_PROCESSING,
> > > .selector = UVC_PU_WHITE_BALANCE_COMPONENT_CONTROL,
> > > - .size = 16,
> > > + .data_size = 16,
> > > .offset = 0,
> > > .v4l2_type = V4L2_CTRL_TYPE_INTEGER,
> > > .data_type = UVC_CTRL_DATA_TYPE_SIGNED,
> > > @@ -624,7 +624,7 @@ static const struct uvc_control_mapping uvc_ctrl_mappings[] = {
> > > .id = V4L2_CID_RED_BALANCE,
> > > .entity = UVC_GUID_UVC_PROCESSING,
> > > .selector = UVC_PU_WHITE_BALANCE_COMPONENT_CONTROL,
> > > - .size = 16,
> > > + .data_size = 16,
> > > .offset = 16,
> > > .v4l2_type = V4L2_CTRL_TYPE_INTEGER,
> > > .data_type = UVC_CTRL_DATA_TYPE_SIGNED,
> > > @@ -635,7 +635,7 @@ static const struct uvc_control_mapping uvc_ctrl_mappings[] = {
> > > .id = V4L2_CID_FOCUS_ABSOLUTE,
> > > .entity = UVC_GUID_UVC_CAMERA,
> > > .selector = UVC_CT_FOCUS_ABSOLUTE_CONTROL,
> > > - .size = 16,
> > > + .data_size = 16,
> > > .offset = 0,
> > > .v4l2_type = V4L2_CTRL_TYPE_INTEGER,
> > > .data_type = UVC_CTRL_DATA_TYPE_UNSIGNED,
> > > @@ -646,7 +646,7 @@ static const struct uvc_control_mapping uvc_ctrl_mappings[] = {
> > > .id = V4L2_CID_FOCUS_AUTO,
> > > .entity = UVC_GUID_UVC_CAMERA,
> > > .selector = UVC_CT_FOCUS_AUTO_CONTROL,
> > > - .size = 1,
> > > + .data_size = 1,
> > > .offset = 0,
> > > .v4l2_type = V4L2_CTRL_TYPE_BOOLEAN,
> > > .data_type = UVC_CTRL_DATA_TYPE_BOOLEAN,
> > > @@ -656,7 +656,7 @@ static const struct uvc_control_mapping uvc_ctrl_mappings[] = {
> > > .id = V4L2_CID_IRIS_ABSOLUTE,
> > > .entity = UVC_GUID_UVC_CAMERA,
> > > .selector = UVC_CT_IRIS_ABSOLUTE_CONTROL,
> > > - .size = 16,
> > > + .data_size = 16,
> > > .offset = 0,
> > > .v4l2_type = V4L2_CTRL_TYPE_INTEGER,
> > > .data_type = UVC_CTRL_DATA_TYPE_UNSIGNED,
> > > @@ -665,7 +665,7 @@ static const struct uvc_control_mapping uvc_ctrl_mappings[] = {
> > > .id = V4L2_CID_IRIS_RELATIVE,
> > > .entity = UVC_GUID_UVC_CAMERA,
> > > .selector = UVC_CT_IRIS_RELATIVE_CONTROL,
> > > - .size = 8,
> > > + .data_size = 8,
> > > .offset = 0,
> > > .v4l2_type = V4L2_CTRL_TYPE_INTEGER,
> > > .data_type = UVC_CTRL_DATA_TYPE_SIGNED,
> > > @@ -674,7 +674,7 @@ static const struct uvc_control_mapping uvc_ctrl_mappings[] = {
> > > .id = V4L2_CID_ZOOM_ABSOLUTE,
> > > .entity = UVC_GUID_UVC_CAMERA,
> > > .selector = UVC_CT_ZOOM_ABSOLUTE_CONTROL,
> > > - .size = 16,
> > > + .data_size = 16,
> > > .offset = 0,
> > > .v4l2_type = V4L2_CTRL_TYPE_INTEGER,
> > > .data_type = UVC_CTRL_DATA_TYPE_UNSIGNED,
> > > @@ -683,7 +683,7 @@ static const struct uvc_control_mapping uvc_ctrl_mappings[] = {
> > > .id = V4L2_CID_ZOOM_CONTINUOUS,
> > > .entity = UVC_GUID_UVC_CAMERA,
> > > .selector = UVC_CT_ZOOM_RELATIVE_CONTROL,
> > > - .size = 0,
> > > + .data_size = 0,
> > > .offset = 0,
> > > .v4l2_type = V4L2_CTRL_TYPE_INTEGER,
> > > .data_type = UVC_CTRL_DATA_TYPE_SIGNED,
> > > @@ -694,7 +694,7 @@ static const struct uvc_control_mapping uvc_ctrl_mappings[] = {
> > > .id = V4L2_CID_PAN_ABSOLUTE,
> > > .entity = UVC_GUID_UVC_CAMERA,
> > > .selector = UVC_CT_PANTILT_ABSOLUTE_CONTROL,
> > > - .size = 32,
> > > + .data_size = 32,
> > > .offset = 0,
> > > .v4l2_type = V4L2_CTRL_TYPE_INTEGER,
> > > .data_type = UVC_CTRL_DATA_TYPE_SIGNED,
> > > @@ -703,7 +703,7 @@ static const struct uvc_control_mapping uvc_ctrl_mappings[] = {
> > > .id = V4L2_CID_TILT_ABSOLUTE,
> > > .entity = UVC_GUID_UVC_CAMERA,
> > > .selector = UVC_CT_PANTILT_ABSOLUTE_CONTROL,
> > > - .size = 32,
> > > + .data_size = 32,
> > > .offset = 32,
> > > .v4l2_type = V4L2_CTRL_TYPE_INTEGER,
> > > .data_type = UVC_CTRL_DATA_TYPE_SIGNED,
> > > @@ -712,7 +712,7 @@ static const struct uvc_control_mapping uvc_ctrl_mappings[] = {
> > > .id = V4L2_CID_PAN_SPEED,
> > > .entity = UVC_GUID_UVC_CAMERA,
> > > .selector = UVC_CT_PANTILT_RELATIVE_CONTROL,
> > > - .size = 16,
> > > + .data_size = 16,
> > > .offset = 0,
> > > .v4l2_type = V4L2_CTRL_TYPE_INTEGER,
> > > .data_type = UVC_CTRL_DATA_TYPE_SIGNED,
> > > @@ -723,7 +723,7 @@ static const struct uvc_control_mapping uvc_ctrl_mappings[] = {
> > > .id = V4L2_CID_TILT_SPEED,
> > > .entity = UVC_GUID_UVC_CAMERA,
> > > .selector = UVC_CT_PANTILT_RELATIVE_CONTROL,
> > > - .size = 16,
> > > + .data_size = 16,
> > > .offset = 16,
> > > .v4l2_type = V4L2_CTRL_TYPE_INTEGER,
> > > .data_type = UVC_CTRL_DATA_TYPE_SIGNED,
> > > @@ -734,7 +734,7 @@ static const struct uvc_control_mapping uvc_ctrl_mappings[] = {
> > > .id = V4L2_CID_PRIVACY,
> > > .entity = UVC_GUID_UVC_CAMERA,
> > > .selector = UVC_CT_PRIVACY_CONTROL,
> > > - .size = 1,
> > > + .data_size = 1,
> > > .offset = 0,
> > > .v4l2_type = V4L2_CTRL_TYPE_BOOLEAN,
> > > .data_type = UVC_CTRL_DATA_TYPE_BOOLEAN,
> > > @@ -743,7 +743,7 @@ static const struct uvc_control_mapping uvc_ctrl_mappings[] = {
> > > .id = V4L2_CID_PRIVACY,
> > > .entity = UVC_GUID_EXT_GPIO_CONTROLLER,
> > > .selector = UVC_CT_PRIVACY_CONTROL,
> > > - .size = 1,
> > > + .data_size = 1,
> > > .offset = 0,
> > > .v4l2_type = V4L2_CTRL_TYPE_BOOLEAN,
> > > .data_type = UVC_CTRL_DATA_TYPE_BOOLEAN,
> > > @@ -754,7 +754,7 @@ const struct uvc_control_mapping uvc_ctrl_power_line_mapping_limited = {
> > > .id = V4L2_CID_POWER_LINE_FREQUENCY,
> > > .entity = UVC_GUID_UVC_PROCESSING,
> > > .selector = UVC_PU_POWER_LINE_FREQUENCY_CONTROL,
> > > - .size = 2,
> > > + .data_size = 2,
> > > .offset = 0,
> > > .v4l2_type = V4L2_CTRL_TYPE_MENU,
> > > .data_type = UVC_CTRL_DATA_TYPE_ENUM,
> > > @@ -766,7 +766,7 @@ const struct uvc_control_mapping uvc_ctrl_power_line_mapping_uvc11 = {
> > > .id = V4L2_CID_POWER_LINE_FREQUENCY,
> > > .entity = UVC_GUID_UVC_PROCESSING,
> > > .selector = UVC_PU_POWER_LINE_FREQUENCY_CONTROL,
> > > - .size = 2,
> > > + .data_size = 2,
> > > .offset = 0,
> > > .v4l2_type = V4L2_CTRL_TYPE_MENU,
> > > .data_type = UVC_CTRL_DATA_TYPE_ENUM,
> > > @@ -783,7 +783,7 @@ static const struct uvc_control_mapping uvc_ctrl_power_line_mapping_uvc15 = {
> > > .id = V4L2_CID_POWER_LINE_FREQUENCY,
> > > .entity = UVC_GUID_UVC_PROCESSING,
> > > .selector = UVC_PU_POWER_LINE_FREQUENCY_CONTROL,
> > > - .size = 2,
> > > + .data_size = 2,
> > > .offset = 0,
> > > .v4l2_type = V4L2_CTRL_TYPE_MENU,
> > > .data_type = UVC_CTRL_DATA_TYPE_ENUM,
> > > @@ -816,7 +816,7 @@ static inline void uvc_clear_bit(u8 *data, int bit)
> > > }
> > >
> > > /*
> > > - * Extract the bit string specified by mapping->offset and mapping->size
> > > + * Extract the bit string specified by mapping->offset and mapping->data_size
> > > * from the little-endian data stored at 'data' and return the result as
> > > * a signed 32bit integer. Sign extension will be performed if the mapping
> > > * references a signed data type.
> > > @@ -824,7 +824,7 @@ static inline void uvc_clear_bit(u8 *data, int bit)
> > > static s32 uvc_get_le_value(struct uvc_control_mapping *mapping,
> > > u8 query, const u8 *data)
> > > {
> > > - int bits = mapping->size;
> > > + int bits = mapping->data_size;
> > > int offset = mapping->offset;
> > > s32 value = 0;
> > > u8 mask;
> > > @@ -847,19 +847,19 @@ static s32 uvc_get_le_value(struct uvc_control_mapping *mapping,
> > >
> > > /* Sign-extend the value if needed. */
> > > if (mapping->data_type == UVC_CTRL_DATA_TYPE_SIGNED)
> > > - value |= -(value & (1 << (mapping->size - 1)));
> > > + value |= -(value & (1 << (mapping->data_size - 1)));
> > >
> > > return value;
> > > }
> > >
> > > /*
> > > - * Set the bit string specified by mapping->offset and mapping->size
> > > + * Set the bit string specified by mapping->offset and mapping->data_size
> > > * in the little-endian data stored at 'data' to the value 'value'.
> > > */
> > > static void uvc_set_le_value(struct uvc_control_mapping *mapping,
> > > s32 value, u8 *data)
> > > {
> > > - int bits = mapping->size;
> > > + int bits = mapping->data_size;
> > > int offset = mapping->offset;
> > > u8 mask;
> > >
> > > @@ -2039,7 +2039,7 @@ int uvc_ctrl_set(struct uvc_fh *handle,
> > > * needs to be loaded from the device to perform the read-modify-write
> > > * operation.
> > > */
> > > - if ((ctrl->info.size * 8) != mapping->size) {
> > > + if ((ctrl->info.size * 8) != mapping->data_size) {
> > > ret = __uvc_ctrl_load_cur(chain, ctrl);
> > > if (ret < 0)
> > > return ret;
> > > @@ -2546,8 +2546,8 @@ int uvc_ctrl_add_mapping(struct uvc_video_chain *chain,
> > > }
> > >
> > > /* Validate the user-provided bit-size and offset */
> > > - if (mapping->size > 32 ||
> > > - mapping->offset + mapping->size > ctrl->info.size * 8) {
> > > + if (mapping->data_size > 32 ||
> > > + mapping->offset + mapping->data_size > ctrl->info.size * 8) {
> > > ret = -EINVAL;
> > > goto done;
> > > }
> > > diff --git a/drivers/media/usb/uvc/uvc_v4l2.c b/drivers/media/usb/uvc/uvc_v4l2.c
> > > index 5a88847bfbfe..ff72caf7bc9e 100644
> > > --- a/drivers/media/usb/uvc/uvc_v4l2.c
> > > +++ b/drivers/media/usb/uvc/uvc_v4l2.c
> > > @@ -122,7 +122,7 @@ static int uvc_ioctl_xu_ctrl_map(struct uvc_video_chain *chain,
> > > }
> > > memcpy(map->entity, xmap->entity, sizeof(map->entity));
> > > map->selector = xmap->selector;
> > > - map->size = xmap->size;
> > > + map->data_size = xmap->size;
> > > map->offset = xmap->offset;
> > > map->v4l2_type = xmap->v4l2_type;
> > > map->data_type = xmap->data_type;
> > > diff --git a/drivers/media/usb/uvc/uvcvideo.h b/drivers/media/usb/uvc/uvcvideo.h
> > > index 5091085fcfb0..7bc41270ed94 100644
> > > --- a/drivers/media/usb/uvc/uvcvideo.h
> > > +++ b/drivers/media/usb/uvc/uvcvideo.h
> > > @@ -110,7 +110,11 @@ struct uvc_control_mapping {
> > > u8 entity[16];
> > > u8 selector;
> > >
> > > - u8 size;
> > > + /* Size of the v4l2 control. Required for compound controls. */
> > > + u8 v4l2_size;
> >
> > Let's introduce this field in the patch that uses it. The commit message
> > needs to be updated to explain that this patch renames the size field to
> > data_size to prepare for adding another size field for compound
> > controls.
>
> Sounds good.
>
> > > + /* UVC data size. Required for all controls. */
> >
> > "UVC data size" is not very clear. Let me attempt to write a more
> > precise description:
> >
> > /*
> > * Size of the control data in the payload of the UVC control GET and
> > * SET requests, expressed in bits.
> > */
> >
> > Is this correct ?
>
> Yes, that sounds much better indeed.
>
> > > + u8 data_size;
> > > +
> > > u8 offset;
> > > enum v4l2_ctrl_type v4l2_type;
> > > u32 data_type;
--
Regards,
Laurent Pinchart
next prev parent reply other threads:[~2023-12-18 3:17 UTC|newest]
Thread overview: 40+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-12-01 7:18 [PATCH v14 00/11] Implement UVC v1.5 ROI Yunke Cao
2023-12-01 7:18 ` [PATCH v14 01/11] media: v4l2_ctrl: Add V4L2_CTRL_TYPE_RECT Yunke Cao
2023-12-01 8:35 ` Hans Verkuil
2023-12-08 13:41 ` Laurent Pinchart
2023-12-12 1:33 ` Yunke Cao
2023-12-12 1:37 ` Laurent Pinchart
2023-12-01 7:18 ` [PATCH v14 02/11] media: uvcvideo: add uvc_ctrl_get_boundary for getting default value Yunke Cao
2023-12-08 15:13 ` Laurent Pinchart
2023-12-01 7:18 ` [PATCH v14 03/11] media: uvcvideo: introduce __uvc_ctrl_get_std() Yunke Cao
2023-12-08 15:12 ` Laurent Pinchart
2023-12-01 7:18 ` [PATCH v14 04/11] media: uvcvideo: Split uvc_control_mapping.size to v4l2 and data size Yunke Cao
2023-12-08 14:15 ` Laurent Pinchart
2023-12-12 7:59 ` Yunke Cao
2023-12-18 3:17 ` Laurent Pinchart [this message]
2023-12-01 7:18 ` [PATCH v14 05/11] media: uvcvideo: Add support for compound controls Yunke Cao
2023-12-06 5:45 ` Dan Carpenter
2023-12-08 15:07 ` Laurent Pinchart
2023-12-13 7:38 ` Yunke Cao
2023-12-18 3:27 ` Laurent Pinchart
2023-12-20 2:28 ` Yunke Cao
2023-12-01 7:18 ` [PATCH v14 06/11] v4l2-ctrls: add support for V4L2_CTRL_WHICH_MIN/MAX_VAL Yunke Cao
2023-12-01 8:32 ` Hans Verkuil
2023-12-05 22:30 ` kernel test robot
2023-12-06 4:34 ` kernel test robot
2023-12-06 5:59 ` kernel test robot
2023-12-08 15:20 ` Laurent Pinchart
2023-12-13 4:06 ` Yunke Cao
2023-12-14 17:34 ` kernel test robot
2023-12-01 7:18 ` [PATCH v14 07/11] media: vivid: Add an rectangle control Yunke Cao
2023-12-01 7:18 ` [PATCH v14 08/11] media: uvcvideo: support V4L2_CTRL_WHICH_MIN/MAX_VAL Yunke Cao
2023-12-08 15:26 ` Laurent Pinchart
2023-12-01 7:19 ` [PATCH v14 09/11] media: uvcvideo: implement UVC v1.5 ROI Yunke Cao
2023-12-08 15:43 ` Laurent Pinchart
2024-03-14 13:24 ` Gergo Koteles
2023-12-01 7:19 ` [PATCH v14 10/11] media: uvcvideo: initilaize ROI control to default value Yunke Cao
2023-12-08 15:50 ` Laurent Pinchart
2023-12-01 7:19 ` [PATCH v14 11/11] media: uvcvideo: document UVC v1.5 ROI Yunke Cao
2023-12-08 16:00 ` Laurent Pinchart
2023-12-12 4:45 ` Yunke Cao
2023-12-18 3:44 ` Laurent Pinchart
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20231218031747.GL5290@pendragon.ideasonboard.com \
--to=laurent.pinchart@ideasonboard.com \
--cc=dan.scally@ideasonboard.com \
--cc=hverkuil-cisco@xs4all.nl \
--cc=linux-media@vger.kernel.org \
--cc=ribalda@chromium.org \
--cc=senozhatsky@chromium.org \
--cc=tfiga@chromium.org \
--cc=yunkec@google.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox