Re: About usb_new_device() API

[Mayuresh Kulkarni <mkulkarni@opensource.cirrus.com>]

On Mon, 2019-08-05 at 10:07 -0400, Alan Stern wrote:
> On Mon, 5 Aug 2019, Mayuresh Kulkarni wrote:
> 
> > 
> > I think I found what is happening here:
> > - Looks like, the USB audio class driver is
> > calling usb_enable_autosuspend().
> The audio maintainers must have some good reason for doing that, but
> I 
> don't know what it is.  Normally kernel drivers are not supposed to 
> enable autosuspend.
> 

Thanks, please see my comment below.

> > 
> > - Please check $KERNEL_SRC/sound/usb, card.c, usb_audio_probe().
> > - It is using interface_to_usbdev() to get the parent of its
> > interface
> > device, which is the USB device allocated by the hub driver.
> > And hence, in above use-case, I can see L2 when our device is in
> > composite USB-audio mode.
> > 

Apologies, as I send the above response, without checking the latest
stable kernel tree.

In the kernel used on the platform I use (and also in Pixel-2 kernel),
the .probe() of USB-audio class driver calls usb_enable_autosuspend().

usb_enable_autosuspend() call is NOT present in latest stable kernel's
USB-audio class driver, which is in-sync with your comment.

> > Moreover, the HID-class driver doesn't seem to call
> > usb_enable_autosuspend() on its parent and hence I don't see L2 when
> > our
> > device operates as a vendor specific HID device.
> > So a simple fix would be to call usb_enable_autosuspend() from HID
> > class
> > driver as well.
> > 
> > With that said, what would be your recommendation here, Alan -
> > 1. Is it OK for USB-class drivers to call usb_enable_autosuspend()
> > on
> > their parent device to ensure low power state is entered?
> Generally it is _not_ okay.  Especially not for the HID class driver
> -- 
> there are far too many HID devices (like mice and keyboards) that
> don't 
> work correctly when they go to low power.
> 
> > 
> > OR
> > 2. Is it recommended to call usb_enable_autosuspend() from user-
> > space by
> > writing "auto" to "cat /sys/bus/usb/devices/.../power/control"?
> That is the recommended approach.
> 

This all makes sense now.
Thanks for all the comments and explainations.

> > 
> > In my opinion, both should be fine.
> Alan Stern
>