Re: [PATCH v1 2/2] USB: set hub's default autosuspend delay as 0

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Sun, 23 Sep 2012, Ming Lei wrote:

> > In fact, sometimes new connected device _does_ mean there will be
> > another new connection in the next few seconds.  This happens because
> 
> Looks I should have made it clearer, and I mean the physical device
> connection or disconnection, eg, device plugging and unplugging.

What difference does it make whether the connection was caused by 
manipulating a physical plug or by electrical signalling?  The end 
result is the same.

> > some devices have trouble initializing when they are first plugged in.
> > They disconnect themselves from the bus and then reconnect a second or
> > so later.
> 
> First, the situation should be less like to happen.
> 
> Secondly, the patch may only affect the case when the new
> connected device is a hub.

No, it can happen with any device.

> Finally, the patch might not affect it if the later reconnect is just
> logical disconnection and connection. If the device triggers
> physical disconnect/connect in bus(such as, D+ changes) during
> its initializing, the patch just makes one extra suspend/resume
> on the hub as did in plugging and unplugging device.

Yes.  My point is that what you wrote is not always correct.

> >> +      *
> >> +      * - The wakeup event can be handled completely in hub_resume()
> >> +      *   and khubd, and the submitted status URB is just to check
> >> +      *   future changes on hub downstream ports.
> >
> > I'm not sure what you mean here.  You seem to be saying that the hub
> > driver is smart enough not to allow another autosuspend until the
> > wakeup event has been processed.  This is true no matter how long or
> > short the autosuspend delay is.
> 
> For some drivers, the remote wakeup is handled by submitting new
> URBs(such as HID, to read the key pressed) in its resume callback, so
> changing autosuspend delay as 0 doesn't work, but that is not ture for
> hub, so I mentioned that  "the submitted status URB is just to check
> future changes on hub downstream ports.", and the submitted URB
> in resume() can be killed in latter auto suspend without any effect on
> handling the current remote wakeup.
> 
> IMO, this point is important wrt. the change in this patch.

I still don't fully understand.  _Any_ driver should be able to handle
an autosuspend delay of 0.  If it can't do all the necessary work in
its resume routine then it should increment the PM usage counter, so
that the device won't be autosuspended again until the counter is
decremented after the work is finished.

There's nothing special about hubs in this regard.

> >> +      *
> >> +      * - suppose there is one device which will remote wakeup
> >> +      *   every 2 seconds periodically and does't care the resume
> >> +      *   delay, the bus can't be suspended with previous default
> >> +      *   autosuspend delay at all, not mention in case of several
> >> +      *   hubs connected in the path from the device to root hub.
> >
> > This is true; however it is misleading.  What matters is the total
> > energy usage.  Does it take more energy to suspend and resume a hub
> > every 2 seconds or simply to leave it active the whole time?
> 
> In theory, the total energy should be decreased under the situation
> since the whole bus might be in suspend state at most of times(maybe
> more than 90%) in the situation.  According to '7.2.3 Power Control
> During Suspend/Resume' of USB 2.0 spec:
> 
>           The average current cannot exceed the average suspend current
>           limit (ICCSH or ICCSL, see Table 7-7) during any 1.0-second interval.

You are ignoring the energy required to make the transition between the 
active and suspended states, plus the extra energy used by the CPU to 
carry out a suspend and a resume.

Why do you think we have an autosuspend delay in the first place?  Why 
not always use a delay of 0 for every device?

> > Also, what reason is there to think that some device will wake up every
> > 2 seconds periodically?
> 
> I want to explain the patch may cause energy consumption decrease.

Well yes, that's a good point, but it has nothing to do with the 
behavior of child devices.  If you shave 2 seconds off the active time 
whenever a hub resumes, then of course you will save energy.

> > Did you consider restoring the autosuspend delay back to the default
> > when the hub driver unbinds?
> 
> Looks not necessary to do it.
> 
> If the hub driver is unbound, all usb devices connected to the hub
> will be disconnected and remote-wakeup won't be enabled, so
> it is OK to let the hub suspend immediately.
> 
> IMO,  maybe the autosuspend_delay of all USB devices should
> be set as zero after they are unbound since they won't be used
> before being bound again.

Remember that devices can be bound to usbfs as well as to their regular 
drivers.  usbfs does not support suspend/resume.

Alan Stern

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux