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) + udev->quirks &= ~USB_QUIRK_RESET_RESUME; 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, + .driver_info = UVC_INFO_QUIRK(UVC_QUIRK_FORCE_RESUME) }, /* Chicony CNF7129 (Asus EEE 100HE) */ { .match_flags = USB_DEVICE_ID_MATCH_DEVICE | USB_DEVICE_ID_MATCH_INT_INFO, 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
Best regards,
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,.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
One more thing, s/ucvideo/uvcvideo/ in the subject line.
On Thu, Apr 04, 2024 at 03:30:57AM +0300, Laurent Pinchart wrote:
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,.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
-- Regards,
Laurent Pinchart
linux-stable-mirror@lists.linaro.org
-
Laurent Pinchart -
Ricardo Ribalda