On Mon, Sep 24, 2012 at 12:51 AM, Alan Stern <stern@xxxxxxxxxxxxxxxxxxx> wrote:
> 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.
The patch just changes the hub's autosuspend delay as 0, so
the default 2 sec delay of other device won't make the hub
suspend immediately.
>
>> 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.
OK, I will fix it.
>
>> >> + *
>> >> + * - 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.
Maybe not.
If you change usb hid device's autosuspend delay as 0, you will
find the mouse can't work basically.
The driver may not know when the URB for receiving data will
complete, and increasing the PM counter under the situation may not
let the device suspend. And changing the autosuspend delay as
0 will kill the URB immediately which is not completed, so
it doesn't work.
>
> There's nothing special about hubs in this regard.
For hub, it needn't to submit a new URB to handle the current
wakeup event, and it is different with USB HID.
>
>> >> + *
>> >> + * - 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,
The current(Ct) during transition can't go above the hub's downstream port limit
(100mA or 500mA) per '7.2.1' of USB 2.0 spec, and suppose the below:
- the transition from active to suspend current is Cw
- the working(active state) current is Cw
- the transition time is Tt( <= 10ms, per '7.1.7.6 Suspending')
- the suspend time is Ts(suppose 2900ms)
- the suspend current is Cs(2.5mA)
- suppose VBUS voltage(V) not change
So the extra energy consumed during the transition is that E1 = V*(Ct-Cw)*Tt,
and the saved energy during suspend period is E2 = V*(Cw-Cs)*Ts. We can
easily get that E1 < E2 suppose Cw = 1/3 Ct or less.
> plus the extra energy used by the CPU to
> carry out a suspend and a resume.
As we discussed before, for most of cases, no extra suspend/resume is
introduced with the patch, so we can ignore the energy used by CPU.
In fact, it is a common problem for runtime PM, especially for
auto-suspend(extra timer is introduced), CPU have to be involved with
device's state change(active<->suspended).
>
> Why do you think we have an autosuspend delay in the first place? Why
> not always use a delay of 0 for every device?
See the above USB HID example.
>
>> > 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.
usbfs always wake up device first in the open(), then the device can be
accessed, so keeping the autosuspend delay as zero can't cause problem
for the case, can it?
Thanks,
--
Ming Lei
--
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
