Hi Ricardo,
Thank you for the patch.
On Tue, Apr 02, 2024 at 12:09:29PM +0000, Ricardo Ribalda wrote:
Logitech Rally Bar devices, despite behaving as UVC cameras, have a different power management system that the other cameras from Logitech.
USB_QUIRK_RESET_RESUME is applied to all the UVC cameras from Logitech at the usb core. Unfortunately, USB_QUIRK_RESET_RESUME causes undesired USB disconnects in the Rally Bar that make them completely unusable.
There is an open discussion about if we should fix this in the core or add a quirk in the UVC driver. In order to enable this hardware, let's land this patch first, and we can revert it later if there is a different conclusion.
Fixes: e387ef5c47dd ("usb: Add USB_QUIRK_RESET_RESUME for all Logitech UVC webcams") Cc: stable@vger.kernel.org Cc: Greg Kroah-Hartman gregkh@linuxfoundation.org Cc: Alan Stern stern@rowland.harvard.edu Cc: Oliver Neukum oneukum@suse.com Acked-by: Greg Kroah-Hartman gregkh@linuxfoundation.org Reviewed-by: Devinder Khroad dkhroad@logitech.com Reviewed-by: Sergey Senozhatsky senozhatsky@chromium.org Reviewed-by: Laurent Pinchart laurent.pinchart@ideasonboard.com Signed-off-by: Ricardo Ribalda ribalda@chromium.org
Tested with a Rallybar Mini with an Acer Chromebook Spin 513
Changes in v5:
- Update commit message to describe that this is a temp solution.
- Link to v4: https://lore.kernel.org/r/20240108-rallybar-v4-1-a7450641e41b@chromium.org
Changes in v4:
- Include Logi Rally Bar Huddle (Thanks Kyle!)
- Link to v3: https://lore.kernel.org/r/20240102-rallybar-v3-1-0ab197ce4aa2@chromium.org
Changes in v3:
- Move quirk to uvc driver
- Link to v2: https://lore.kernel.org/r/20231222-rallybar-v2-1-5849d62a9514@chromium.org
Changes in v2:
- Add Fixes tag
- Add UVC maintainer as Cc
- Link to v1: https://lore.kernel.org/r/20231222-rallybar-v1-1-82b2a4d3106f@chromium.org
drivers/media/usb/uvc/uvc_driver.c | 30 ++++++++++++++++++++++++++++++ drivers/media/usb/uvc/uvcvideo.h | 1 + 2 files changed, 31 insertions(+)
diff --git a/drivers/media/usb/uvc/uvc_driver.c b/drivers/media/usb/uvc/uvc_driver.c index 08fcd2ffa727b..9663bcac68438 100644 --- a/drivers/media/usb/uvc/uvc_driver.c +++ b/drivers/media/usb/uvc/uvc_driver.c @@ -14,6 +14,7 @@ #include <linux/module.h> #include <linux/slab.h> #include <linux/usb.h> +#include <linux/usb/quirks.h> #include <linux/usb/uvc.h> #include <linux/videodev2.h> #include <linux/vmalloc.h> @@ -2233,6 +2234,8 @@ static int uvc_probe(struct usb_interface *intf, } uvc_dbg(dev, PROBE, "UVC device initialized\n");
- if (dev->quirks & UVC_QUIRK_FORCE_RESUME)
The quirk isn't really about forcing resume, but about not resetting on resume. Can we name it UVC_QUIRK_NO_RESET_RESUME ?
udev->quirks &= ~USB_QUIRK_RESET_RESUME;
Let's move this before the uvc_dbg().
With that, the patch looks good to me. I can apply those changes locally if you don't want to submit a v6.
usb_enable_autosuspend(udev); return 0; @@ -2574,6 +2577,33 @@ static const struct usb_device_id uvc_ids[] = { .bInterfaceSubClass = 1, .bInterfaceProtocol = 0, .driver_info = UVC_INFO_QUIRK(UVC_QUIRK_RESTORE_CTRLS_ON_INIT) },
- /* Logitech Rally Bar Huddle */
- { .match_flags = USB_DEVICE_ID_MATCH_DEVICE
| USB_DEVICE_ID_MATCH_INT_INFO,
.idVendor = 0x046d,
.idProduct = 0x087c,
.bInterfaceClass = USB_CLASS_VIDEO,
.bInterfaceSubClass = 1,
.bInterfaceProtocol = 0,
.driver_info = UVC_INFO_QUIRK(UVC_QUIRK_FORCE_RESUME) },
- /* Logitech Rally Bar */
- { .match_flags = USB_DEVICE_ID_MATCH_DEVICE
| USB_DEVICE_ID_MATCH_INT_INFO,
.idVendor = 0x046d,
.idProduct = 0x089b,
.bInterfaceClass = USB_CLASS_VIDEO,
.bInterfaceSubClass = 1,
.bInterfaceProtocol = 0,
.driver_info = UVC_INFO_QUIRK(UVC_QUIRK_FORCE_RESUME) },
- /* Logitech Rally Bar Mini */
- { .match_flags = USB_DEVICE_ID_MATCH_DEVICE
| USB_DEVICE_ID_MATCH_INT_INFO,
.idVendor = 0x046d,
.idProduct = 0x08d3,
.bInterfaceClass = USB_CLASS_VIDEO,
.bInterfaceSubClass = 1,
.bInterfaceProtocol = 0,
/* Chicony CNF7129 (Asus EEE 100HE) */ { .match_flags = USB_DEVICE_ID_MATCH_DEVICE | USB_DEVICE_ID_MATCH_INT_INFO,.driver_info = UVC_INFO_QUIRK(UVC_QUIRK_FORCE_RESUME) },
diff --git a/drivers/media/usb/uvc/uvcvideo.h b/drivers/media/usb/uvc/uvcvideo.h index 6fb0a78b1b009..fa59a21d2a289 100644 --- a/drivers/media/usb/uvc/uvcvideo.h +++ b/drivers/media/usb/uvc/uvcvideo.h @@ -73,6 +73,7 @@ #define UVC_QUIRK_FORCE_Y8 0x00000800 #define UVC_QUIRK_FORCE_BPP 0x00001000 #define UVC_QUIRK_WAKE_AUTOSUSPEND 0x00002000 +#define UVC_QUIRK_FORCE_RESUME 0x00004000 /* Format flags */ #define UVC_FMT_FLAG_COMPRESSED 0x00000001
base-commit: c0f65a7c112b3cfa691cead54bcf24d6cc2182b5 change-id: 20231222-rallybar-19ce0c64d5e6