Hi Peter,
Thank you for the patch.
On Thursday 06 April 2017 13:58:25 Peter Boström wrote:
> Permits distinguishing between two /dev/videoX entries from the same
> physical UVC device (that naturally share the same iProduct name).
>
> This change matches current Windows behavior by prioritizing iFunction
> over iInterface, but unlike Windows it displays both iProduct and
> iFunction/iInterface strings when both are available.
>
> Signed-off-by: Peter Boström <p...@google.com>
> ---
> drivers/media/usb/uvc/uvc_driver.c | 43 ++++++++++++++++++++++++++++-------
> drivers/media/usb/uvc/uvcvideo.h | 4 +++-
> 2 files changed, 39 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/media/usb/uvc/uvc_driver.c
> b/drivers/media/usb/uvc/uvc_driver.c index 04bf35063c4c..66adf8a77e56
> 100644
> --- a/drivers/media/usb/uvc/uvc_driver.c
> +++ b/drivers/media/usb/uvc/uvc_driver.c
> @@ -1998,6 +1998,8 @@ static int uvc_probe(struct usb_interface *intf,
> {
> struct usb_device *udev = interface_to_usbdev(intf);
> struct uvc_device *dev;
> + char additional_name_buf[UVC_DEVICE_NAME_SIZE];
> + const char *additional_name = NULL;
> int ret;
>
> if (id->idVendor && id->idProduct)
> @@ -2025,13 +2027,40 @@ static int uvc_probe(struct usb_interface *intf,
> dev->quirks = (uvc_quirks_param == -1)
> ? id->driver_info : uvc_quirks_param;
>
> - if (udev->product != NULL)
> - strlcpy(dev->name, udev->product, sizeof dev->name);
> - else
> - snprintf(dev->name, sizeof dev->name,
> - "UVC Camera (%04x:%04x)",
> - le16_to_cpu(udev->descriptor.idVendor),
> - le16_to_cpu(udev->descriptor.idProduct));
> + /*
> + * Add iFunction or iInterface to names when available as additional
> + * distinguishers between interfaces. iFunction is prioritized over
> + * iInterface which matches Windows behavior at the point of writing.
> + */
> + if (intf->intf_assoc && intf->intf_assoc->iFunction != 0) {
> + usb_string(udev, intf->intf_assoc->iFunction,
> + additional_name_buf, sizeof(additional_name_buf));
> + additional_name = additional_name_buf;
> + } else if (intf->cur_altsetting->desc.iInterface != 0) {
> + usb_string(udev, intf->cur_altsetting->desc.iInterface,
> + additional_name_buf, sizeof(additional_name_buf));
> + additional_name = additional_name_buf;
> + }
> +
> + if (additional_name) {
> + if (udev->product) {
> + snprintf(dev->name, sizeof(dev->name), "%s: %s",
> + udev->product, additional_name);
> + } else {
> + snprintf(dev->name, sizeof(dev->name),
> + "UVC Camera: %s (%04x:%04x)",
> + additional_name,
> + le16_to_cpu(udev->descriptor.idVendor),
> + le16_to_cpu(udev->descriptor.idProduct));
> + }
> + } else if (udev->product) {
> + strlcpy(dev->name, udev->product, sizeof(dev->name));
> + } else {
> + snprintf(dev->name, sizeof(dev->name),
> + "UVC Camera (%04x:%04x)",
> + le16_to_cpu(udev->descriptor.idVendor),
> + le16_to_cpu(udev->descriptor.idProduct));
> + }
This makes sense to me, but I think we could come up with a version of the
same code that wouldn't require the temporary 64 bytes buffer on the stack.
How about the following (untested) code ?
diff --git a/drivers/media/usb/uvc/uvc_driver.c
b/drivers/media/usb/uvc/uvc_driver.c
index 602256ffe14d..9b90a1ac5945 100644
--- a/drivers/media/usb/uvc/uvc_driver.c
+++ b/drivers/media/usb/uvc/uvc_driver.c
@@ -2013,6 +2013,7 @@ static int uvc_probe(struct usb_interface *intf,
{
struct usb_device *udev = interface_to_usbdev(intf);
struct uvc_device *dev;
+ int string;
int ret;
if (id->idVendor && id->idProduct)
@@ -2048,6 +2049,24 @@ static int uvc_probe(struct usb_interface *intf,
le16_to_cpu(udev->descriptor.idVendor),
le16_to_cpu(udev->descriptor.idProduct));
+ /*
+ * Add iFunction or iInterface to names when available as additional
+ * distinguishers between interfaces. iFunction is prioritized over
+ * iInterface which matches Windows behavior at the point of writing.
+ */
+ string = intf->intf_assoc ? intf->intf_assoc->iFunction
+ : intf->cur_altsetting->desc.iInterface;
+ if (string != 0) {
+ size_t len;
+
+ strlcat(dev->name, ": ", sizeof(dev->name));
+ len = strlen(dev->name);
+
+ /* usb_string() accounts for the terminating NULL character. */
+ usb_string(udev, string, dev->name + len,
+ sizeof(dev->name) - len);
+ }
+
/* Parse the Video Class control descriptor. */
if (uvc_parse_control(dev) < 0) {
uvc_trace(UVC_TRACE_PROBE, "Unable to parse UVC "
diff --git a/drivers/media/usb/uvc/uvcvideo.h b/drivers/media/usb/uvc/uvcvideo.h
index 15e415e32c7f..f5bb42c6b023 100644
--- a/drivers/media/usb/uvc/uvcvideo.h
+++ b/drivers/media/usb/uvc/uvcvideo.h
@@ -556,7 +556,7 @@ struct uvc_device {
unsigned long warnings;
__u32 quirks;
int intfnum;
- char name[32];
+ char name[64];
struct mutex lock; /* Protects users */
unsigned int users;
> /* Parse the Video Class control descriptor. */
> if (uvc_parse_control(dev) < 0) {
> diff --git a/drivers/media/usb/uvc/uvcvideo.h
> b/drivers/media/usb/uvc/uvcvideo.h index 4205e7a423f0..0cbedaee6e19 100644
> --- a/drivers/media/usb/uvc/uvcvideo.h
> +++ b/drivers/media/usb/uvc/uvcvideo.h
> @@ -541,13 +541,15 @@ struct uvc_streaming {
> } clock;
> };
>
> +#define UVC_DEVICE_NAME_SIZE 64
> +
> struct uvc_device {
> struct usb_device *udev;
> struct usb_interface *intf;
> unsigned long warnings;
> __u32 quirks;
> int intfnum;
> - char name[32];
> + char name[UVC_DEVICE_NAME_SIZE];
>
> struct mutex lock; /* Protects users */
> unsigned int users;
--
Regards,
Laurent Pinchart
