linux-usb.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* About usb_new_device() API
@ 2019-08-01 14:49 Mayuresh Kulkarni
  2019-08-01 17:51 ` Alan Stern
  0 siblings, 1 reply; 7+ messages in thread
From: Mayuresh Kulkarni @ 2019-08-01 14:49 UTC (permalink / raw)
  To: linux-usb

Hi All,

I am seeing a peculiar behaviour which I think *might* be 
caused by usb_new_device(). Since usb_new_device() is one of the core
APIs, I cannot explain how PM works for USB device at later point in
time.

In a particular use-case, our composite USB device
exposes HID interface with vendor report descriptor. Since the standard
HID-class driver's HID-input part is unable to decode this vendor report
descriptor, it is unable to bind itself to this interface.
After this, we don't see any L2 requests on USB bus analyser.
Obvious reason seems to be, since no driver is bound to interface, there
cannot be PM call-backs since driver has these call-backs.

But I am expecting that the USB device (which is parent of HID
interface) should see L2. The reason being, USB-core seems to properly
do runtime get/put wherever needed. And HID interface has no driver, so
from USB-core point of view, it is a USB device w/o any interface.
(please correct if this is incorrect expectation).

With that said, I am confused about usb_new_device() in this context. It
seems to call usb_disable_autosuspend() ==> pm_runtime_forbid() ==>
increment usage_count.
However, it never calls usb_enable_autosuspend() itself.
Now since USB PM (and L2) works fine at later point in time (i.e.: after
all the interfaces are bound to their appropriate drivers), I think
somewhere the equivalent of usb_enable_autosuspend() gets called for the
USB device and hence USB PM works fine.

I wonder this *may be* be an issue I am seeing with the use-case
mentioned above. But definitely confused about it and hence thought of
sending this email.

Does this description makes sense? Is it necessary to
call usb_enable_autosuspend() in usb_new_device()?

Thanks,

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: About usb_new_device() API
  2019-08-01 14:49 About usb_new_device() API Mayuresh Kulkarni
@ 2019-08-01 17:51 ` Alan Stern
  2019-08-02 15:43   ` Mayuresh Kulkarni
  0 siblings, 1 reply; 7+ messages in thread
From: Alan Stern @ 2019-08-01 17:51 UTC (permalink / raw)
  To: Mayuresh Kulkarni; +Cc: linux-usb

On Thu, 1 Aug 2019, Mayuresh Kulkarni wrote:

> Hi All,
> 
> I am seeing a peculiar behaviour which I think *might* be 
> caused by usb_new_device(). Since usb_new_device() is one of the core
> APIs, I cannot explain how PM works for USB device at later point in
> time.
> 
> In a particular use-case, our composite USB device
> exposes HID interface with vendor report descriptor. Since the standard
> HID-class driver's HID-input part is unable to decode this vendor report
> descriptor, it is unable to bind itself to this interface.
> After this, we don't see any L2 requests on USB bus analyser.
> Obvious reason seems to be, since no driver is bound to interface, there
> cannot be PM call-backs since driver has these call-backs.

There are other possible reasons.  For example, what is the setting 
stored in /sys/bus/usb/devices/.../power/control (fill in the "..." 
with the appropriate name for your device)?

If the file contains "on" then runtime PM is forbidden and the device 
will always remain at full power.  If the file contains "auto" then the 
device will be subject to normal runtime-PM suspends and resumes.

> But I am expecting that the USB device (which is parent of HID
> interface) should see L2. The reason being, USB-core seems to properly
> do runtime get/put wherever needed. And HID interface has no driver, so
> from USB-core point of view, it is a USB device w/o any interface.
> (please correct if this is incorrect expectation).

More accurately, it is a USB device with one interface which is not 
bound to a driver.

> With that said, I am confused about usb_new_device() in this context. It
> seems to call usb_disable_autosuspend() ==> pm_runtime_forbid() ==>
> increment usage_count.

Correct.  By default, all USB devices except hubs are forbidden to go 
into runtime suspend.  This setting can be changed by userspace (by 
writing to the sysfs file mentioned above).

> However, it never calls usb_enable_autosuspend() itself.
> Now since USB PM (and L2) works fine at later point in time (i.e.: after
> all the interfaces are bound to their appropriate drivers), I think
> somewhere the equivalent of usb_enable_autosuspend() gets called for the
> USB device and hence USB PM works fine.

There are programs, like powertop, which will automatically write
"auto" to the power/control sysfs file when a new device appears.  
Doing so calls pm_runtime_allow(), which decrements usage_count.

> I wonder this *may be* be an issue I am seeing with the use-case
> mentioned above. But definitely confused about it and hence thought of
> sending this email.
> 
> Does this description makes sense? Is it necessary to
> call usb_enable_autosuspend() in usb_new_device()?

It is not necessary.  Check that sysfs file and see what it contains.  
In fact, you can check the contents of all the files in the device's 
sysfs power/ subdirectory.

Alan Stern


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: About usb_new_device() API
  2019-08-01 17:51 ` Alan Stern
@ 2019-08-02 15:43   ` Mayuresh Kulkarni
  2019-08-02 16:27     ` Alan Stern
  0 siblings, 1 reply; 7+ messages in thread
From: Mayuresh Kulkarni @ 2019-08-02 15:43 UTC (permalink / raw)
  To: Alan Stern; +Cc: linux-usb

On Thu, 2019-08-01 at 13:51 -0400, Alan Stern wrote:
> On Thu, 1 Aug 2019, Mayuresh Kulkarni wrote:
> 
> > 
> > Hi All,
> > 
> > I am seeing a peculiar behaviour which I think *might* be 
> > caused by usb_new_device(). Since usb_new_device() is one of the
> > core
> > APIs, I cannot explain how PM works for USB device at later point in
> > time.
> > 
> > In a particular use-case, our composite USB device
> > exposes HID interface with vendor report descriptor. Since the
> > standard
> > HID-class driver's HID-input part is unable to decode this vendor
> > report
> > descriptor, it is unable to bind itself to this interface.
> > After this, we don't see any L2 requests on USB bus analyser.
> > Obvious reason seems to be, since no driver is bound to interface,
> > there
> > cannot be PM call-backs since driver has these call-backs.
> There are other possible reasons.  For example, what is the setting 
> stored in /sys/bus/usb/devices/.../power/control (fill in the "..." 
> with the appropriate name for your device)?
> 
> If the file contains "on" then runtime PM is forbidden and the device 
> will always remain at full power.  If the file contains "auto" then
> the 
> device will be subject to normal runtime-PM suspends and resumes.
> 

Hi Alan,

Thanks a lot for clearing out the confusion.

Our USB device can operate in 2 mutually exclusive modes: one is normal
composite USB audio mode and other is vendor specific HID device mode.

On the same platform (Android based):
- When the device is in normal composite USB audio mode,
"cat /sys/bus/usb/devices/.../power/control" show "auto".
- When the device is in vendor specific HID device mode,
"cat /sys/bus/usb/devices/.../power/control" show "on".

And hence as per your comment, I am unable to see USB-2.0 L2 for vendor
specific HID device mode.

I guess I need to find out "who" is setting the /power/control = "auto"
when composite USB audio device is detected. And explore if it could be
moved to a more generic place.

Is there any module parameter (or some other means) by which,
power/control (or deprecated power/level) will always be "auto", by
default?

> > 
> > But I am expecting that the USB device (which is parent of HID
> > interface) should see L2. The reason being, USB-core seems to
> > properly
> > do runtime get/put wherever needed. And HID interface has no driver,
> > so
> > from USB-core point of view, it is a USB device w/o any interface.
> > (please correct if this is incorrect expectation).
> More accurately, it is a USB device with one interface which is not 
> bound to a driver.
> 
> > 
> > With that said, I am confused about usb_new_device() in this
> > context. It
> > seems to call usb_disable_autosuspend() ==> pm_runtime_forbid() ==>
> > increment usage_count.
> Correct.  By default, all USB devices except hubs are forbidden to go 
> into runtime suspend.  This setting can be changed by userspace (by 
> writing to the sysfs file mentioned above).
> 
> > 
> > However, it never calls usb_enable_autosuspend() itself.
> > Now since USB PM (and L2) works fine at later point in time (i.e.:
> > after
> > all the interfaces are bound to their appropriate drivers), I think
> > somewhere the equivalent of usb_enable_autosuspend() gets called for
> > the
> > USB device and hence USB PM works fine.
> There are programs, like powertop, which will automatically write
> "auto" to the power/control sysfs file when a new device appears.  
> Doing so calls pm_runtime_allow(), which decrements usage_count.
> 

Cool, thanks for this info.

> > 
> > I wonder this *may be* be an issue I am seeing with the use-case
> > mentioned above. But definitely confused about it and hence thought
> > of
> > sending this email.
> > 
> > Does this description makes sense? Is it necessary to
> > call usb_enable_autosuspend() in usb_new_device()?
> It is not necessary.  Check that sysfs file and see what it
> contains.  
> In fact, you can check the contents of all the files in the device's 
> sysfs power/ subdirectory.

Thanks, the files under power/ have useful info (great for doing
diagnosis).

> 
> Alan Stern
> 

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: About usb_new_device() API
  2019-08-02 15:43   ` Mayuresh Kulkarni
@ 2019-08-02 16:27     ` Alan Stern
  2019-08-05  8:43       ` Mayuresh Kulkarni
  0 siblings, 1 reply; 7+ messages in thread
From: Alan Stern @ 2019-08-02 16:27 UTC (permalink / raw)
  To: Mayuresh Kulkarni; +Cc: linux-usb

On Fri, 2 Aug 2019, Mayuresh Kulkarni wrote:

> Hi Alan,
> 
> Thanks a lot for clearing out the confusion.
> 
> Our USB device can operate in 2 mutually exclusive modes: one is normal
> composite USB audio mode and other is vendor specific HID device mode.
> 
> On the same platform (Android based):
> - When the device is in normal composite USB audio mode,
> "cat /sys/bus/usb/devices/.../power/control" show "auto".
> - When the device is in vendor specific HID device mode,
> "cat /sys/bus/usb/devices/.../power/control" show "on".
> 
> And hence as per your comment, I am unable to see USB-2.0 L2 for vendor
> specific HID device mode.
> 
> I guess I need to find out "who" is setting the /power/control = "auto"
> when composite USB audio device is detected. And explore if it could be
> moved to a more generic place.

I'm pretty sure this is done by some program, not automatically done by 
the kernel.

> Is there any module parameter (or some other means) by which,
> power/control (or deprecated power/level) will always be "auto", by
> default?

No, there isn't.  The power/control (or power/level) attribute has to 
be set by the user or a program.

Alan Stern


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: About usb_new_device() API
  2019-08-02 16:27     ` Alan Stern
@ 2019-08-05  8:43       ` Mayuresh Kulkarni
  2019-08-05 14:07         ` Alan Stern
  0 siblings, 1 reply; 7+ messages in thread
From: Mayuresh Kulkarni @ 2019-08-05  8:43 UTC (permalink / raw)
  To: Alan Stern; +Cc: linux-usb

On Fri, 2019-08-02 at 12:27 -0400, Alan Stern wrote:
> On Fri, 2 Aug 2019, Mayuresh Kulkarni wrote:
> 
> > 
> > Hi Alan,
> > 
> > Thanks a lot for clearing out the confusion.
> > 
> > Our USB device can operate in 2 mutually exclusive modes: one is
> > normal
> > composite USB audio mode and other is vendor specific HID device
> > mode.
> > 
> > On the same platform (Android based):
> > - When the device is in normal composite USB audio mode,
> > "cat /sys/bus/usb/devices/.../power/control" show "auto".
> > - When the device is in vendor specific HID device mode,
> > "cat /sys/bus/usb/devices/.../power/control" show "on".
> > 
> > And hence as per your comment, I am unable to see USB-2.0 L2 for
> > vendor
> > specific HID device mode.
> > 
> > I guess I need to find out "who" is setting the /power/control =
> > "auto"
> > when composite USB audio device is detected. And explore if it could
> > be
> > moved to a more generic place.
> I'm pretty sure this is done by some program, not automatically done
> by 
> the kernel.

I think I found what is happening here:
- Looks like, the USB audio class driver is
calling usb_enable_autosuspend().
- 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.

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?
OR
2. Is it recommended to call usb_enable_autosuspend() from user-space by
writing "auto" to "cat /sys/bus/usb/devices/.../power/control"?

In my opinion, both should be fine.

Thanks,

> 
> > 
> > Is there any module parameter (or some other means) by which,
> > power/control (or deprecated power/level) will always be "auto", by
> > default?
> No, there isn't.  The power/control (or power/level) attribute has to 
> be set by the user or a program.
> 
> Alan Stern
> 

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: About usb_new_device() API
  2019-08-05  8:43       ` Mayuresh Kulkarni
@ 2019-08-05 14:07         ` Alan Stern
  2019-08-05 16:30           ` Mayuresh Kulkarni
  0 siblings, 1 reply; 7+ messages in thread
From: Alan Stern @ 2019-08-05 14:07 UTC (permalink / raw)
  To: Mayuresh Kulkarni; +Cc: linux-usb

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.

> - 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.
> 
> 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.

> In my opinion, both should be fine.

Alan Stern


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: About usb_new_device() API
  2019-08-05 14:07         ` Alan Stern
@ 2019-08-05 16:30           ` Mayuresh Kulkarni
  0 siblings, 0 replies; 7+ messages in thread
From: Mayuresh Kulkarni @ 2019-08-05 16:30 UTC (permalink / raw)
  To: Alan Stern; +Cc: linux-usb

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
> 

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2019-08-05 16:31 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-08-01 14:49 About usb_new_device() API Mayuresh Kulkarni
2019-08-01 17:51 ` Alan Stern
2019-08-02 15:43   ` Mayuresh Kulkarni
2019-08-02 16:27     ` Alan Stern
2019-08-05  8:43       ` Mayuresh Kulkarni
2019-08-05 14:07         ` Alan Stern
2019-08-05 16:30           ` Mayuresh Kulkarni

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).