When the device suspends, it keeps power-cycling.
The user notices it because the LED constanct oscillate between
blue (ready) and no LED (off).
<6>[95202.128542] usb 3-3-port4: attempt power cycle
<6>[95206.070120] usb 3-3.4: new high-speed USB device number 49 using xhci_hcd
<6>[95206.212027] usb 3-3.4: New USB device found, idVendor=2e1a, idProduct=4c01, bcdDevice= 2.00
<6>[95206.212044] usb 3-3.4: New USB device strings: Mfr=1, Product=2, SerialNumber=<Serial: 1>
<6>[95206.212050] usb 3-3.4: Product: Insta360 Link
<6>[95206.212075] usb 3-3.4: Manufacturer: Amba
<7>[95206.214862] usb 3-3.4: GPIO lookup for consumer privacy
<7>[95206.214866] usb 3-3.4: using lookup tables for GPIO lookup
<7>[95206.214869] usb 3-3.4: No GPIO consumer privacy found
<6>[95206.214871] usb 3-3.4: Found UVC 1.10 device Insta360 Link (2e1a:4c01)
<3>[95206.217113] usb 3-3.4: Failed to query (GET_INFO) UVC control 14 on unit 1: -32 (exp. 1).
<3>[95206.217733] usb 3-3.4: Failed to query (GET_INFO) UVC control 16 on unit 1: -32 (exp. 1).
<4>[95206.223544] usb 3-3.4: Warning! Unlikely big volume range (=32767), cval->res is probably wrong.
<4>[95206.223554] usb 3-3.4: [9] FU [Mic Capture Volume] ch = 1, val = -32768/-1/1
<6>[95210.698990] usb 3-3.4: USB disconnect, device number 49
<6>[95211.963090] usb 3-3.4: new high-speed USB device number 50 using xhci_hcd
<6>[95212.657061] usb 3-3.4: new full-speed USB device number 51 using xhci_hcd
<3>[95212.783119] usb 3-3.4: device descriptor read/64, error -32
<3>[95213.015076] usb 3-3.4: device descriptor read/64, error -32
<6>[95213.120358] usb 3-3-port4: attempt power cycle
Bus 001 Device 009: ID 2e1a:4c01 Amba Insta360 Link
Device Descriptor:
bLength 18
bDescriptorType 1
bcdUSB 2.00
bDeviceClass 239 Miscellaneous Device
bDeviceSubClass 2
bDeviceProtocol 1 Interface Association
bMaxPacketSize0 64
idVendor 0x2e1a
idProduct 0x4c01
bcdDevice 2.00
iManufacturer 1 Amba
iProduct 2 Insta360 Link
iSerial 0
bNumConfigurations 1
Signed-off-by: Ricardo Ribalda <[email protected]>
---
media: uvcvideo: Disable autosuspend for Insta360
The device does not handle properly the USB suspend and makes it barely usable.
To: Laurent Pinchart <[email protected]>
To: Mauro Carvalho Chehab <[email protected]>
Cc: [email protected]
Cc: [email protected]
---
drivers/media/usb/uvc/uvc_driver.c | 13 ++++++++++++-
drivers/media/usb/uvc/uvcvideo.h | 1 +
2 files changed, 13 insertions(+), 1 deletion(-)
diff --git a/drivers/media/usb/uvc/uvc_driver.c b/drivers/media/usb/uvc/uvc_driver.c
index 215fb483efb0..ad95c7599863 100644
--- a/drivers/media/usb/uvc/uvc_driver.c
+++ b/drivers/media/usb/uvc/uvc_driver.c
@@ -2223,7 +2223,9 @@ static int uvc_probe(struct usb_interface *intf,
}
uvc_dbg(dev, PROBE, "UVC device initialized\n");
- usb_enable_autosuspend(udev);
+ if (!(dev->quirks & UVC_QUIRK_DISABLE_AUTOSUSPEND))
+ usb_enable_autosuspend(udev);
+
return 0;
error:
@@ -2967,6 +2969,15 @@ static const struct usb_device_id uvc_ids[] = {
.bInterfaceSubClass = 1,
.bInterfaceProtocol = 0,
.driver_info = (kernel_ulong_t)&uvc_quirk_force_y8 },
+ /* Insta360 Link */
+ { .match_flags = USB_DEVICE_ID_MATCH_DEVICE
+ | USB_DEVICE_ID_MATCH_INT_INFO,
+ .idVendor = 0x2e1a,
+ .idProduct = 0x4c01,
+ .bInterfaceClass = USB_CLASS_VIDEO,
+ .bInterfaceSubClass = 1,
+ .bInterfaceProtocol = 0,
+ .driver_info = UVC_INFO_QUIRK(UVC_QUIRK_DISABLE_AUTOSUSPEND) },
/* GEO Semiconductor GC6500 */
{ .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 df93db259312..47c86c7c6346 100644
--- a/drivers/media/usb/uvc/uvcvideo.h
+++ b/drivers/media/usb/uvc/uvcvideo.h
@@ -74,6 +74,7 @@
#define UVC_QUIRK_RESTORE_CTRLS_ON_INIT 0x00000400
#define UVC_QUIRK_FORCE_Y8 0x00000800
#define UVC_QUIRK_FORCE_BPP 0x00001000
+#define UVC_QUIRK_DISABLE_AUTOSUSPEND 0x00002000
/* Format flags */
#define UVC_FMT_FLAG_COMPRESSED 0x00000001
---
base-commit: 23758867219c8d84c8363316e6dd2f9fd7ae3049
change-id: 20221101-instal-9a77ba1cc448
Best regards,
--
Ricardo Ribalda <[email protected]>
Hi Ricardo,
Thank you for the patch.
On Fri, Dec 02, 2022 at 05:48:52PM +0100, Ricardo Ribalda wrote:
> When the device suspends, it keeps power-cycling.
>
> The user notices it because the LED constanct oscillate between
> blue (ready) and no LED (off).
>
> <6>[95202.128542] usb 3-3-port4: attempt power cycle
> <6>[95206.070120] usb 3-3.4: new high-speed USB device number 49 using xhci_hcd
> <6>[95206.212027] usb 3-3.4: New USB device found, idVendor=2e1a, idProduct=4c01, bcdDevice= 2.00
> <6>[95206.212044] usb 3-3.4: New USB device strings: Mfr=1, Product=2, SerialNumber=<Serial: 1>
> <6>[95206.212050] usb 3-3.4: Product: Insta360 Link
> <6>[95206.212075] usb 3-3.4: Manufacturer: Amba
> <7>[95206.214862] usb 3-3.4: GPIO lookup for consumer privacy
> <7>[95206.214866] usb 3-3.4: using lookup tables for GPIO lookup
> <7>[95206.214869] usb 3-3.4: No GPIO consumer privacy found
> <6>[95206.214871] usb 3-3.4: Found UVC 1.10 device Insta360 Link (2e1a:4c01)
> <3>[95206.217113] usb 3-3.4: Failed to query (GET_INFO) UVC control 14 on unit 1: -32 (exp. 1).
> <3>[95206.217733] usb 3-3.4: Failed to query (GET_INFO) UVC control 16 on unit 1: -32 (exp. 1).
> <4>[95206.223544] usb 3-3.4: Warning! Unlikely big volume range (=32767), cval->res is probably wrong.
> <4>[95206.223554] usb 3-3.4: [9] FU [Mic Capture Volume] ch = 1, val = -32768/-1/1
> <6>[95210.698990] usb 3-3.4: USB disconnect, device number 49
> <6>[95211.963090] usb 3-3.4: new high-speed USB device number 50 using xhci_hcd
> <6>[95212.657061] usb 3-3.4: new full-speed USB device number 51 using xhci_hcd
> <3>[95212.783119] usb 3-3.4: device descriptor read/64, error -32
> <3>[95213.015076] usb 3-3.4: device descriptor read/64, error -32
> <6>[95213.120358] usb 3-3-port4: attempt power cycle
>
> Bus 001 Device 009: ID 2e1a:4c01 Amba Insta360 Link
> Device Descriptor:
> bLength 18
> bDescriptorType 1
> bcdUSB 2.00
> bDeviceClass 239 Miscellaneous Device
> bDeviceSubClass 2
> bDeviceProtocol 1 Interface Association
> bMaxPacketSize0 64
> idVendor 0x2e1a
> idProduct 0x4c01
> bcdDevice 2.00
> iManufacturer 1 Amba
> iProduct 2 Insta360 Link
> iSerial 0
> bNumConfigurations 1
>
> Signed-off-by: Ricardo Ribalda <[email protected]>
> ---
> media: uvcvideo: Disable autosuspend for Insta360
>
> The device does not handle properly the USB suspend and makes it barely usable.
Isn't this best handled with a quirk in the USB core ? Autosuspend is a
device feature, not an interface feature, so if the USB sound driver is
loaded but uvcvideo isn't, the kernel may still attempt to autosuspend
the device.
> To: Laurent Pinchart <[email protected]>
> To: Mauro Carvalho Chehab <[email protected]>
> Cc: [email protected]
> Cc: [email protected]
> ---
> drivers/media/usb/uvc/uvc_driver.c | 13 ++++++++++++-
> drivers/media/usb/uvc/uvcvideo.h | 1 +
> 2 files changed, 13 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/media/usb/uvc/uvc_driver.c b/drivers/media/usb/uvc/uvc_driver.c
> index 215fb483efb0..ad95c7599863 100644
> --- a/drivers/media/usb/uvc/uvc_driver.c
> +++ b/drivers/media/usb/uvc/uvc_driver.c
> @@ -2223,7 +2223,9 @@ static int uvc_probe(struct usb_interface *intf,
> }
>
> uvc_dbg(dev, PROBE, "UVC device initialized\n");
> - usb_enable_autosuspend(udev);
> + if (!(dev->quirks & UVC_QUIRK_DISABLE_AUTOSUSPEND))
> + usb_enable_autosuspend(udev);
> +
> return 0;
>
> error:
> @@ -2967,6 +2969,15 @@ static const struct usb_device_id uvc_ids[] = {
> .bInterfaceSubClass = 1,
> .bInterfaceProtocol = 0,
> .driver_info = (kernel_ulong_t)&uvc_quirk_force_y8 },
> + /* Insta360 Link */
> + { .match_flags = USB_DEVICE_ID_MATCH_DEVICE
> + | USB_DEVICE_ID_MATCH_INT_INFO,
> + .idVendor = 0x2e1a,
> + .idProduct = 0x4c01,
> + .bInterfaceClass = USB_CLASS_VIDEO,
> + .bInterfaceSubClass = 1,
> + .bInterfaceProtocol = 0,
> + .driver_info = UVC_INFO_QUIRK(UVC_QUIRK_DISABLE_AUTOSUSPEND) },
> /* GEO Semiconductor GC6500 */
> { .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 df93db259312..47c86c7c6346 100644
> --- a/drivers/media/usb/uvc/uvcvideo.h
> +++ b/drivers/media/usb/uvc/uvcvideo.h
> @@ -74,6 +74,7 @@
> #define UVC_QUIRK_RESTORE_CTRLS_ON_INIT 0x00000400
> #define UVC_QUIRK_FORCE_Y8 0x00000800
> #define UVC_QUIRK_FORCE_BPP 0x00001000
> +#define UVC_QUIRK_DISABLE_AUTOSUSPEND 0x00002000
>
> /* Format flags */
> #define UVC_FMT_FLAG_COMPRESSED 0x00000001
>
> ---
> base-commit: 23758867219c8d84c8363316e6dd2f9fd7ae3049
> change-id: 20221101-instal-9a77ba1cc448
--
Regards,
Laurent Pinchart
Hi Laurent
We are back to this issue.
On Thu, 29 Dec 2022 at 03:22, Laurent Pinchart
<[email protected]> wrote:
>
> Hi Ricardo,
>
> Thank you for the patch.
>
> On Fri, Dec 02, 2022 at 05:48:52PM +0100, Ricardo Ribalda wrote:
> > When the device suspends, it keeps power-cycling.
> >
> > The user notices it because the LED constanct oscillate between
> > blue (ready) and no LED (off).
> >
> > <6>[95202.128542] usb 3-3-port4: attempt power cycle
> > <6>[95206.070120] usb 3-3.4: new high-speed USB device number 49 using xhci_hcd
> > <6>[95206.212027] usb 3-3.4: New USB device found, idVendor=2e1a, idProduct=4c01, bcdDevice= 2.00
> > <6>[95206.212044] usb 3-3.4: New USB device strings: Mfr=1, Product=2, SerialNumber=<Serial: 1>
> > <6>[95206.212050] usb 3-3.4: Product: Insta360 Link
> > <6>[95206.212075] usb 3-3.4: Manufacturer: Amba
> > <7>[95206.214862] usb 3-3.4: GPIO lookup for consumer privacy
> > <7>[95206.214866] usb 3-3.4: using lookup tables for GPIO lookup
> > <7>[95206.214869] usb 3-3.4: No GPIO consumer privacy found
> > <6>[95206.214871] usb 3-3.4: Found UVC 1.10 device Insta360 Link (2e1a:4c01)
> > <3>[95206.217113] usb 3-3.4: Failed to query (GET_INFO) UVC control 14 on unit 1: -32 (exp. 1).
> > <3>[95206.217733] usb 3-3.4: Failed to query (GET_INFO) UVC control 16 on unit 1: -32 (exp. 1).
> > <4>[95206.223544] usb 3-3.4: Warning! Unlikely big volume range (=32767), cval->res is probably wrong.
> > <4>[95206.223554] usb 3-3.4: [9] FU [Mic Capture Volume] ch = 1, val = -32768/-1/1
> > <6>[95210.698990] usb 3-3.4: USB disconnect, device number 49
> > <6>[95211.963090] usb 3-3.4: new high-speed USB device number 50 using xhci_hcd
> > <6>[95212.657061] usb 3-3.4: new full-speed USB device number 51 using xhci_hcd
> > <3>[95212.783119] usb 3-3.4: device descriptor read/64, error -32
> > <3>[95213.015076] usb 3-3.4: device descriptor read/64, error -32
> > <6>[95213.120358] usb 3-3-port4: attempt power cycle
> >
> > Bus 001 Device 009: ID 2e1a:4c01 Amba Insta360 Link
> > Device Descriptor:
> > bLength 18
> > bDescriptorType 1
> > bcdUSB 2.00
> > bDeviceClass 239 Miscellaneous Device
> > bDeviceSubClass 2
> > bDeviceProtocol 1 Interface Association
> > bMaxPacketSize0 64
> > idVendor 0x2e1a
> > idProduct 0x4c01
> > bcdDevice 2.00
> > iManufacturer 1 Amba
> > iProduct 2 Insta360 Link
> > iSerial 0
> > bNumConfigurations 1
> >
> > Signed-off-by: Ricardo Ribalda <[email protected]>
> > ---
> > media: uvcvideo: Disable autosuspend for Insta360
> >
> > The device does not handle properly the USB suspend and makes it barely usable.
>
> Isn't this best handled with a quirk in the USB core ? Autosuspend is a
> device feature, not an interface feature, so if the USB sound driver is
> loaded but uvcvideo isn't, the kernel may still attempt to autosuspend
> the device.
>
Seems like USB_QUIRK_NO_AUTOSUSPEND was gone for a long time
https://lore.kernel.org/lkml/[email protected]/
under the assumption that autosuspend was off by default and user
space should only enable autosuspend on the devices that support it
(if I understand it correctly).
There are two other quirks still available: USB_QUIRK_RESET_RESUME and
USB_QUIRK_DISCONNECT_SUSPEND, but they do not seem to work for this
device (Yunke, thanks for looking into this)
If we are explicitly enabling autosuspend on the driver, shouldn't we
make sure that the device supports it?
Regards!
> > To: Laurent Pinchart <[email protected]>
> > To: Mauro Carvalho Chehab <[email protected]>
> > Cc: [email protected]
> > Cc: [email protected]
> > ---
> > drivers/media/usb/uvc/uvc_driver.c | 13 ++++++++++++-
> > drivers/media/usb/uvc/uvcvideo.h | 1 +
> > 2 files changed, 13 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/media/usb/uvc/uvc_driver.c b/drivers/media/usb/uvc/uvc_driver.c
> > index 215fb483efb0..ad95c7599863 100644
> > --- a/drivers/media/usb/uvc/uvc_driver.c
> > +++ b/drivers/media/usb/uvc/uvc_driver.c
> > @@ -2223,7 +2223,9 @@ static int uvc_probe(struct usb_interface *intf,
> > }
> >
> > uvc_dbg(dev, PROBE, "UVC device initialized\n");
> > - usb_enable_autosuspend(udev);
> > + if (!(dev->quirks & UVC_QUIRK_DISABLE_AUTOSUSPEND))
> > + usb_enable_autosuspend(udev);
> > +
> > return 0;
> >
> > error:
> > @@ -2967,6 +2969,15 @@ static const struct usb_device_id uvc_ids[] = {
> > .bInterfaceSubClass = 1,
> > .bInterfaceProtocol = 0,
> > .driver_info = (kernel_ulong_t)&uvc_quirk_force_y8 },
> > + /* Insta360 Link */
> > + { .match_flags = USB_DEVICE_ID_MATCH_DEVICE
> > + | USB_DEVICE_ID_MATCH_INT_INFO,
> > + .idVendor = 0x2e1a,
> > + .idProduct = 0x4c01,
> > + .bInterfaceClass = USB_CLASS_VIDEO,
> > + .bInterfaceSubClass = 1,
> > + .bInterfaceProtocol = 0,
> > + .driver_info = UVC_INFO_QUIRK(UVC_QUIRK_DISABLE_AUTOSUSPEND) },
> > /* GEO Semiconductor GC6500 */
> > { .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 df93db259312..47c86c7c6346 100644
> > --- a/drivers/media/usb/uvc/uvcvideo.h
> > +++ b/drivers/media/usb/uvc/uvcvideo.h
> > @@ -74,6 +74,7 @@
> > #define UVC_QUIRK_RESTORE_CTRLS_ON_INIT 0x00000400
> > #define UVC_QUIRK_FORCE_Y8 0x00000800
> > #define UVC_QUIRK_FORCE_BPP 0x00001000
> > +#define UVC_QUIRK_DISABLE_AUTOSUSPEND 0x00002000
> >
> > /* Format flags */
> > #define UVC_FMT_FLAG_COMPRESSED 0x00000001
> >
> > ---
> > base-commit: 23758867219c8d84c8363316e6dd2f9fd7ae3049
> > change-id: 20221101-instal-9a77ba1cc448
>
> --
> Regards,
>
> Laurent Pinchart
--
Ricardo Ribalda
On Wed, 1 Mar 2023 at 10:04, Ricardo Ribalda <[email protected]> wrote:
>
> Hi Laurent
>
> We are back to this issue.
>
>
> On Thu, 29 Dec 2022 at 03:22, Laurent Pinchart
> <[email protected]> wrote:
> >
> > Hi Ricardo,
> >
> > Thank you for the patch.
> >
> > On Fri, Dec 02, 2022 at 05:48:52PM +0100, Ricardo Ribalda wrote:
> > > When the device suspends, it keeps power-cycling.
> > >
> > > The user notices it because the LED constanct oscillate between
> > > blue (ready) and no LED (off).
> > >
> > > <6>[95202.128542] usb 3-3-port4: attempt power cycle
> > > <6>[95206.070120] usb 3-3.4: new high-speed USB device number 49 using xhci_hcd
> > > <6>[95206.212027] usb 3-3.4: New USB device found, idVendor=2e1a, idProduct=4c01, bcdDevice= 2.00
> > > <6>[95206.212044] usb 3-3.4: New USB device strings: Mfr=1, Product=2, SerialNumber=<Serial: 1>
> > > <6>[95206.212050] usb 3-3.4: Product: Insta360 Link
> > > <6>[95206.212075] usb 3-3.4: Manufacturer: Amba
> > > <7>[95206.214862] usb 3-3.4: GPIO lookup for consumer privacy
> > > <7>[95206.214866] usb 3-3.4: using lookup tables for GPIO lookup
> > > <7>[95206.214869] usb 3-3.4: No GPIO consumer privacy found
> > > <6>[95206.214871] usb 3-3.4: Found UVC 1.10 device Insta360 Link (2e1a:4c01)
> > > <3>[95206.217113] usb 3-3.4: Failed to query (GET_INFO) UVC control 14 on unit 1: -32 (exp. 1).
> > > <3>[95206.217733] usb 3-3.4: Failed to query (GET_INFO) UVC control 16 on unit 1: -32 (exp. 1).
> > > <4>[95206.223544] usb 3-3.4: Warning! Unlikely big volume range (=32767), cval->res is probably wrong.
> > > <4>[95206.223554] usb 3-3.4: [9] FU [Mic Capture Volume] ch = 1, val = -32768/-1/1
> > > <6>[95210.698990] usb 3-3.4: USB disconnect, device number 49
> > > <6>[95211.963090] usb 3-3.4: new high-speed USB device number 50 using xhci_hcd
> > > <6>[95212.657061] usb 3-3.4: new full-speed USB device number 51 using xhci_hcd
> > > <3>[95212.783119] usb 3-3.4: device descriptor read/64, error -32
> > > <3>[95213.015076] usb 3-3.4: device descriptor read/64, error -32
> > > <6>[95213.120358] usb 3-3-port4: attempt power cycle
> > >
> > > Bus 001 Device 009: ID 2e1a:4c01 Amba Insta360 Link
> > > Device Descriptor:
> > > bLength 18
> > > bDescriptorType 1
> > > bcdUSB 2.00
> > > bDeviceClass 239 Miscellaneous Device
> > > bDeviceSubClass 2
> > > bDeviceProtocol 1 Interface Association
> > > bMaxPacketSize0 64
> > > idVendor 0x2e1a
> > > idProduct 0x4c01
> > > bcdDevice 2.00
> > > iManufacturer 1 Amba
> > > iProduct 2 Insta360 Link
> > > iSerial 0
> > > bNumConfigurations 1
> > >
> > > Signed-off-by: Ricardo Ribalda <[email protected]>
> > > ---
> > > media: uvcvideo: Disable autosuspend for Insta360
> > >
> > > The device does not handle properly the USB suspend and makes it barely usable.
> >
> > Isn't this best handled with a quirk in the USB core ? Autosuspend is a
> > device feature, not an interface feature, so if the USB sound driver is
> > loaded but uvcvideo isn't, the kernel may still attempt to autosuspend
> > the device.
> >
>
> Seems like USB_QUIRK_NO_AUTOSUSPEND was gone for a long time
>
> https://lore.kernel.org/lkml/[email protected]/
>
> under the assumption that autosuspend was off by default and user
> space should only enable autosuspend on the devices that support it
> (if I understand it correctly).
>
> There are two other quirks still available: USB_QUIRK_RESET_RESUME and
> USB_QUIRK_DISCONNECT_SUSPEND, but they do not seem to work for this
> device (Yunke, thanks for looking into this)
>
> If we are explicitly enabling autosuspend on the driver, shouldn't we
> make sure that the device supports it?
>
Alan, Greg, any idea about what is the best way to proceed here from a
USB perspective?
Thanks!
>
> > > To: Laurent Pinchart <[email protected]>
> > > To: Mauro Carvalho Chehab <[email protected]>
> > > Cc: [email protected]
> > > Cc: [email protected]
> > > ---
> > > drivers/media/usb/uvc/uvc_driver.c | 13 ++++++++++++-
> > > drivers/media/usb/uvc/uvcvideo.h | 1 +
> > > 2 files changed, 13 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/media/usb/uvc/uvc_driver.c b/drivers/media/usb/uvc/uvc_driver.c
> > > index 215fb483efb0..ad95c7599863 100644
> > > --- a/drivers/media/usb/uvc/uvc_driver.c
> > > +++ b/drivers/media/usb/uvc/uvc_driver.c
> > > @@ -2223,7 +2223,9 @@ static int uvc_probe(struct usb_interface *intf,
> > > }
> > >
> > > uvc_dbg(dev, PROBE, "UVC device initialized\n");
> > > - usb_enable_autosuspend(udev);
> > > + if (!(dev->quirks & UVC_QUIRK_DISABLE_AUTOSUSPEND))
> > > + usb_enable_autosuspend(udev);
> > > +
> > > return 0;
> > >
> > > error:
> > > @@ -2967,6 +2969,15 @@ static const struct usb_device_id uvc_ids[] = {
> > > .bInterfaceSubClass = 1,
> > > .bInterfaceProtocol = 0,
> > > .driver_info = (kernel_ulong_t)&uvc_quirk_force_y8 },
> > > + /* Insta360 Link */
> > > + { .match_flags = USB_DEVICE_ID_MATCH_DEVICE
> > > + | USB_DEVICE_ID_MATCH_INT_INFO,
> > > + .idVendor = 0x2e1a,
> > > + .idProduct = 0x4c01,
> > > + .bInterfaceClass = USB_CLASS_VIDEO,
> > > + .bInterfaceSubClass = 1,
> > > + .bInterfaceProtocol = 0,
> > > + .driver_info = UVC_INFO_QUIRK(UVC_QUIRK_DISABLE_AUTOSUSPEND) },
> > > /* GEO Semiconductor GC6500 */
> > > { .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 df93db259312..47c86c7c6346 100644
> > > --- a/drivers/media/usb/uvc/uvcvideo.h
> > > +++ b/drivers/media/usb/uvc/uvcvideo.h
> > > @@ -74,6 +74,7 @@
> > > #define UVC_QUIRK_RESTORE_CTRLS_ON_INIT 0x00000400
> > > #define UVC_QUIRK_FORCE_Y8 0x00000800
> > > #define UVC_QUIRK_FORCE_BPP 0x00001000
> > > +#define UVC_QUIRK_DISABLE_AUTOSUSPEND 0x00002000
> > >
> > > /* Format flags */
> > > #define UVC_FMT_FLAG_COMPRESSED 0x00000001
> > >
> > > ---
> > > base-commit: 23758867219c8d84c8363316e6dd2f9fd7ae3049
> > > change-id: 20221101-instal-9a77ba1cc448
> >
> > --
> > Regards,
> >
> > Laurent Pinchart
>
>
>
> --
> Ricardo Ribalda
--
Ricardo Ribalda
On Wed, Mar 08, 2023 at 11:43:09PM +0100, Ricardo Ribalda wrote:
> On Wed, 1 Mar 2023 at 10:04, Ricardo Ribalda <[email protected]> wrote:
> >
> > Hi Laurent
> >
> > We are back to this issue.
> >
> >
> > On Thu, 29 Dec 2022 at 03:22, Laurent Pinchart
> > <[email protected]> wrote:
> > >
> > > Hi Ricardo,
> > >
> > > Thank you for the patch.
> > >
> > > On Fri, Dec 02, 2022 at 05:48:52PM +0100, Ricardo Ribalda wrote:
> > > > When the device suspends, it keeps power-cycling.
> > > >
> > > > The user notices it because the LED constanct oscillate between
> > > > blue (ready) and no LED (off).
> > > > Signed-off-by: Ricardo Ribalda <[email protected]>
> > > > ---
> > > > media: uvcvideo: Disable autosuspend for Insta360
> > > >
> > > > The device does not handle properly the USB suspend and makes it barely usable.
> > >
> > > Isn't this best handled with a quirk in the USB core ? Autosuspend is a
> > > device feature, not an interface feature, so if the USB sound driver is
> > > loaded but uvcvideo isn't, the kernel may still attempt to autosuspend
> > > the device.
> > >
> >
> > Seems like USB_QUIRK_NO_AUTOSUSPEND was gone for a long time
> >
> > https://lore.kernel.org/lkml/[email protected]/
> >
> > under the assumption that autosuspend was off by default and user
> > space should only enable autosuspend on the devices that support it
> > (if I understand it correctly).
> >
> > There are two other quirks still available: USB_QUIRK_RESET_RESUME and
> > USB_QUIRK_DISCONNECT_SUSPEND, but they do not seem to work for this
> > device (Yunke, thanks for looking into this)
> >
> > If we are explicitly enabling autosuspend on the driver, shouldn't we
> > make sure that the device supports it?
> >
>
> Alan, Greg, any idea about what is the best way to proceed here from a
> USB perspective?
How is autosuspend getting enabled for this device? It is disabled by
default for non-hub USB devices.
If the uvcvideo or USB sound driver is enabling autosuspend, the driver
should be fixed. Perhaps by adding a quirk bit for this purpose.
If userspace is enabling autosuspend, then any misbehavior isn't the
kernel's fault. :-)
Alan Stern
Hi Alan
On Thu, 9 Mar 2023 at 00:02, Alan Stern <[email protected]> wrote:
>
> On Wed, Mar 08, 2023 at 11:43:09PM +0100, Ricardo Ribalda wrote:
> > On Wed, 1 Mar 2023 at 10:04, Ricardo Ribalda <[email protected]> wrote:
> > >
> > > Hi Laurent
> > >
> > > We are back to this issue.
> > >
> > >
> > > On Thu, 29 Dec 2022 at 03:22, Laurent Pinchart
> > > <[email protected]> wrote:
> > > >
> > > > Hi Ricardo,
> > > >
> > > > Thank you for the patch.
> > > >
> > > > On Fri, Dec 02, 2022 at 05:48:52PM +0100, Ricardo Ribalda wrote:
> > > > > When the device suspends, it keeps power-cycling.
> > > > >
> > > > > The user notices it because the LED constanct oscillate between
> > > > > blue (ready) and no LED (off).
>
> > > > > Signed-off-by: Ricardo Ribalda <[email protected]>
> > > > > ---
> > > > > media: uvcvideo: Disable autosuspend for Insta360
> > > > >
> > > > > The device does not handle properly the USB suspend and makes it barely usable.
> > > >
> > > > Isn't this best handled with a quirk in the USB core ? Autosuspend is a
> > > > device feature, not an interface feature, so if the USB sound driver is
> > > > loaded but uvcvideo isn't, the kernel may still attempt to autosuspend
> > > > the device.
> > > >
> > >
> > > Seems like USB_QUIRK_NO_AUTOSUSPEND was gone for a long time
> > >
> > > https://lore.kernel.org/lkml/[email protected]/
> > >
> > > under the assumption that autosuspend was off by default and user
> > > space should only enable autosuspend on the devices that support it
> > > (if I understand it correctly).
> > >
> > > There are two other quirks still available: USB_QUIRK_RESET_RESUME and
> > > USB_QUIRK_DISCONNECT_SUSPEND, but they do not seem to work for this
> > > device (Yunke, thanks for looking into this)
> > >
> > > If we are explicitly enabling autosuspend on the driver, shouldn't we
> > > make sure that the device supports it?
> > >
> >
> > Alan, Greg, any idea about what is the best way to proceed here from a
> > USB perspective?
>
> How is autosuspend getting enabled for this device? It is disabled by
> default for non-hub USB devices.
It is enabled on the driver via usb_enable_autosuspend()
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/media/usb/uvc/uvc_driver.c#n2211
>
> If the uvcvideo or USB sound driver is enabling autosuspend, the driver
> should be fixed. Perhaps by adding a quirk bit for this purpose.
This is what I tried with this patch :). Laurent, could you please
take a second look to it?
Thanks!
>
> If userspace is enabling autosuspend, then any misbehavior isn't the
> kernel's fault. :-)
>
> Alan Stern
--
Ricardo Ribalda
Hi Ricardo,
On Thu, Mar 09, 2023 at 12:12:27AM +0100, Ricardo Ribalda wrote:
> On Thu, 9 Mar 2023 at 00:02, Alan Stern wrote:
> > On Wed, Mar 08, 2023 at 11:43:09PM +0100, Ricardo Ribalda wrote:
> > > On Wed, 1 Mar 2023 at 10:04, Ricardo Ribalda wrote:
> > > > On Thu, 29 Dec 2022 at 03:22, Laurent Pinchart wrote:
> > > > > On Fri, Dec 02, 2022 at 05:48:52PM +0100, Ricardo Ribalda wrote:
> > > > > > When the device suspends, it keeps power-cycling.
> > > > > >
> > > > > > The user notices it because the LED constanct oscillate between
> > > > > > blue (ready) and no LED (off).
> >
> > > > > > Signed-off-by: Ricardo Ribalda <[email protected]>
> > > > > > ---
> > > > > > media: uvcvideo: Disable autosuspend for Insta360
> > > > > >
> > > > > > The device does not handle properly the USB suspend and makes it barely usable.
> > > > >
> > > > > Isn't this best handled with a quirk in the USB core ? Autosuspend is a
> > > > > device feature, not an interface feature, so if the USB sound driver is
> > > > > loaded but uvcvideo isn't, the kernel may still attempt to autosuspend
> > > > > the device.
> > > > >
> > > >
> > > > Seems like USB_QUIRK_NO_AUTOSUSPEND was gone for a long time
> > > >
> > > > https://lore.kernel.org/lkml/[email protected]/
> > > >
> > > > under the assumption that autosuspend was off by default and user
> > > > space should only enable autosuspend on the devices that support it
> > > > (if I understand it correctly).
> > > >
> > > > There are two other quirks still available: USB_QUIRK_RESET_RESUME and
> > > > USB_QUIRK_DISCONNECT_SUSPEND, but they do not seem to work for this
> > > > device (Yunke, thanks for looking into this)
> > > >
> > > > If we are explicitly enabling autosuspend on the driver, shouldn't we
> > > > make sure that the device supports it?
> > > >
> > >
> > > Alan, Greg, any idea about what is the best way to proceed here from a
> > > USB perspective?
> >
> > How is autosuspend getting enabled for this device? It is disabled by
> > default for non-hub USB devices.
>
> It is enabled on the driver via usb_enable_autosuspend()
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/media/usb/uvc/uvc_driver.c#n2211
>
> > If the uvcvideo or USB sound driver is enabling autosuspend, the driver
> > should be fixed. Perhaps by adding a quirk bit for this purpose.
>
> This is what I tried with this patch :). Laurent, could you please
> take a second look to it?
> Thanks!
Done, and
Reviewed-by: Laurent Pinchart <[email protected]>
> > If userspace is enabling autosuspend, then any misbehavior isn't the
> > kernel's fault. :-)
--
Regards,
Laurent Pinchart