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@xxxxxxxxxxxxxxxx>
> > > Signed-off-by: Yunke Cao <yunkec@xxxxxxxxxx>
> > > ---
> > > 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
