Received-SPF: pass (google.com: domain of linux-kernel+bounces-111404-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45e3:2400::1 as permitted sender) client-ip=2604:1380:45e3:2400::1;
Date: Fri, 22 Mar 2024 14:08:10 +0200
From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
To: Ricardo Ribalda <ribalda@chromium.org>
Cc: Alan Stern <stern@rowland.harvard.edu>, Yunke Cao <yunkec@chromium.org>,
	Mauro Carvalho Chehab <mchehab@kernel.org>,
	linux-kernel@vger.kernel.org, linux-media@vger.kernel.org,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Subject: Re: [PATCH RESEND] media: uvcvideo: Disable autosuspend for Insta360
 Link
Message-ID: <20240322120810.GA648@pendragon.ideasonboard.com>
References: <20221101-instal-v1-0-d13d1331c4b5@chromium.org>
 <Y6z55cXTt5yXjIZj@pendragon.ideasonboard.com>
 <CANiDSCvbQ7uMgoxDsXDOphjANNZ2EuoV-Dra+wtT9LLL6USstQ@mail.gmail.com>
 <CANiDSCveK6QRbYM==XN9jggy8DHfS3QJawDWpFLy0jcmmHeduA@mail.gmail.com>
 <1298c83f-695a-4356-aa2e-22b0e6629610@rowland.harvard.edu>
 <CANiDSCtfPO+O=uLTAAiLDrmvcRigi1weAih_+et5iwFy5Joamw@mail.gmail.com>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
In-Reply-To: <CANiDSCtfPO+O=uLTAAiLDrmvcRigi1weAih_+et5iwFy5Joamw@mail.gmail.com>

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 <ribalda@chromium.org>
> > > > > > ---
> > > > > > 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/20071115064457.GU19218@kroah.com/
> > > >
> > > > 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 <laurent.pinchart@ideasonboard.com>

> > If userspace is enabling autosuspend, then any misbehavior isn't the
> > kernel's fault.  :-)

-- 
Regards,

Laurent Pinchart