Hi Ricardo, Thank you for the patch.
On Thu, Mar 11, 2021 at 01:20:36PM +0100, Ricardo Ribalda wrote: > Create a virtual entity that holds all the class control. Isn't this making control classes too complicated ? Can't they be handled explicitly in uvc_query_v4l2_ctrl(), as that's the only place where it should matter ? When registering V4L2 controls you'd set a bitmask to track which classes are used (similarly to 10/10, but with the bitmask stored in the chain, not an entity), and uvc_query_v4l2_ctrl() would then handle the classes explicitly. > Fixes v4l2-compliance: > Control ioctls (Input 0): > fail: v4l2-test-controls.cpp(216): missing control class for > class 00980000 > fail: v4l2-test-controls.cpp(253): missing control class for > class 009a0000 > test VIDIOC_QUERY_EXT_CTRL/QUERYMENU: FAIL > > Signed-off-by: Ricardo Ribalda <[email protected]> > --- > drivers/media/usb/uvc/uvc_ctrl.c | 3 ++ > drivers/media/usb/uvc/uvc_driver.c | 52 +++++++++++++++++++++++++++--- > drivers/media/usb/uvc/uvc_entity.c | 1 + > drivers/media/usb/uvc/uvcvideo.h | 10 ++++++ > 4 files changed, 61 insertions(+), 5 deletions(-) > > diff --git a/drivers/media/usb/uvc/uvc_ctrl.c > b/drivers/media/usb/uvc/uvc_ctrl.c > index fd4d5ad098b9..273eccc136b8 100644 > --- a/drivers/media/usb/uvc/uvc_ctrl.c > +++ b/drivers/media/usb/uvc/uvc_ctrl.c > @@ -2354,6 +2354,9 @@ int uvc_ctrl_init_device(struct uvc_device *dev) > } else if (UVC_ENTITY_TYPE(entity) == UVC_EXT_GPIO_UNIT) { > bmControls = entity->gpio.bmControls; > bControlSize = entity->gpio.bControlSize; > + } else if (UVC_ENTITY_TYPE(entity) == UVC_CTRL_CLASS_UNIT) { > + bmControls = entity->ctrl_class.bmControls; > + bControlSize = entity->ctrl_class.bControlSize; > } > > /* Remove bogus/blacklisted controls */ > diff --git a/drivers/media/usb/uvc/uvc_driver.c > b/drivers/media/usb/uvc/uvc_driver.c > index 30ef2a3110f7..996e8bd06ac5 100644 > --- a/drivers/media/usb/uvc/uvc_driver.c > +++ b/drivers/media/usb/uvc/uvc_driver.c > @@ -1025,6 +1025,7 @@ static int uvc_parse_streaming(struct uvc_device *dev, > } > > static const u8 uvc_camera_guid[16] = UVC_GUID_UVC_CAMERA; > +static const u8 uvc_ctrl_class_guid[16] = UVC_GUID_CTRL_CLASS; > static const u8 uvc_gpio_guid[16] = UVC_GUID_EXT_GPIO_CONTROLLER; > static const u8 uvc_media_transport_input_guid[16] = > UVC_GUID_UVC_MEDIA_TRANSPORT_INPUT; > @@ -1057,6 +1058,9 @@ static struct uvc_entity *uvc_alloc_entity(u16 type, > u16 id, > * is initialized by the caller. > */ > switch (type) { > + case UVC_CTRL_CLASS_UNIT: > + memcpy(entity->guid, uvc_ctrl_class_guid, 16); > + break; > case UVC_EXT_GPIO_UNIT: > memcpy(entity->guid, uvc_gpio_guid, 16); > break; > @@ -1474,6 +1478,39 @@ static int uvc_parse_control(struct uvc_device *dev) > return 0; > } > > +/* > ----------------------------------------------------------------------------- > + * Control Class > + */ > + > +static int uvc_ctrl_class_get_info(struct uvc_device *dev, > + struct uvc_entity *entity, > + u8 cs, u8 *caps) > +{ > + *caps = 0; > + return 0; > +} > + > +static int uvc_ctrl_class_parse(struct uvc_device *dev) > +{ > + struct uvc_entity *unit; > + > + unit = uvc_alloc_entity(UVC_CTRL_CLASS_UNIT, > + UVC_CTRL_CLASS_UNIT_ID, 0, 1); > + if (!unit) > + return -ENOMEM; > + > + unit->ctrl_class.bControlSize = 1; > + unit->ctrl_class.bmControls = (u8 *)unit + sizeof(*unit); > + unit->ctrl_class.bmControls[0] = (1 << (UVC_CC_LAST_CLASS + 1)) - 1; > + unit->get_info = uvc_ctrl_class_get_info; > + strncpy(unit->name, "Control Class", sizeof(unit->name) - 1); > + > + list_add_tail(&unit->list, &dev->entities); > + dev->ctrl_class_unit = unit; > + > + return 0; > +} > + > /* > ----------------------------------------------------------------------------- > * Privacy GPIO > */ > @@ -2054,12 +2091,11 @@ static int uvc_scan_device(struct uvc_device *dev) > return -1; > } > > - /* Add GPIO entity to the first chain. */ > - if (dev->gpio_unit) { > - chain = list_first_entry(&dev->chains, > - struct uvc_video_chain, list); > + /* Add virtual entities to the first chain. */ > + chain = list_first_entry(&dev->chains, struct uvc_video_chain, list); > + list_add_tail(&dev->ctrl_class_unit->chain, &chain->entities); > + if (dev->gpio_unit) > list_add_tail(&dev->gpio_unit->chain, &chain->entities); > - } > > return 0; > } > @@ -2399,6 +2435,12 @@ static int uvc_probe(struct usb_interface *intf, > goto error; > } > > + /* Parse the control class. */ > + if (uvc_ctrl_class_parse(dev) < 0) { > + uvc_dbg(dev, PROBE, "Unable to parse UVC CTRL CLASS\n"); > + goto error; > + } > + > dev_info(&dev->udev->dev, "Found UVC %u.%02x device %s (%04x:%04x)\n", > dev->uvc_version >> 8, dev->uvc_version & 0xff, > udev->product ? udev->product : "<unnamed>", > diff --git a/drivers/media/usb/uvc/uvc_entity.c > b/drivers/media/usb/uvc/uvc_entity.c > index 7c4d2f93d351..5285030a738c 100644 > --- a/drivers/media/usb/uvc/uvc_entity.c > +++ b/drivers/media/usb/uvc/uvc_entity.c > @@ -106,6 +106,7 @@ static int uvc_mc_init_entity(struct uvc_video_chain > *chain, > case UVC_OTT_MEDIA_TRANSPORT_OUTPUT: > case UVC_EXTERNAL_VENDOR_SPECIFIC: > case UVC_EXT_GPIO_UNIT: > + case UVC_CTRL_CLASS_UNIT: > default: > function = MEDIA_ENT_F_V4L2_SUBDEV_UNKNOWN; > break; > diff --git a/drivers/media/usb/uvc/uvcvideo.h > b/drivers/media/usb/uvc/uvcvideo.h > index 5792232ed312..1d59ac10c2eb 100644 > --- a/drivers/media/usb/uvc/uvcvideo.h > +++ b/drivers/media/usb/uvc/uvcvideo.h > @@ -41,6 +41,9 @@ > #define UVC_EXT_GPIO_UNIT 0x7ffe > #define UVC_EXT_GPIO_UNIT_ID 0x100 > > +#define UVC_CTRL_CLASS_UNIT 0x7ffd > +#define UVC_CTRL_CLASS_UNIT_ID 0x101 > + > /* ------------------------------------------------------------------------ > * GUIDs > */ > @@ -183,6 +186,7 @@ > */ > #define UVC_CC_CAMERA_CLASS 0 > #define UVC_CC_USER_CLASS 1 > +#define UVC_CC_LAST_CLASS UVC_CC_USER_CLASS > > /* ------------------------------------------------------------------------ > * Driver specific constants. > @@ -375,6 +379,11 @@ struct uvc_entity { > struct gpio_desc *gpio_privacy; > int irq; > } gpio; > + > + struct { > + u8 bControlSize; > + u8 *bmControls; > + } ctrl_class; > }; > > u8 bNrInPins; > @@ -715,6 +724,7 @@ struct uvc_device { > } async_ctrl; > > struct uvc_entity *gpio_unit; > + struct uvc_entity *ctrl_class_unit; > }; > > enum uvc_handle_state { -- Regards, Laurent Pinchart
