lowRISC / opentitan

OpenTitan: Open source silicon root of trust
https://www.opentitan.org
Apache License 2.0
2.51k stars 746 forks source link

[usbdev] Make sure wakeup and resume time is within spec #9972

Closed msfschaffner closed 1 year ago

msfschaffner commented 2 years ago

We should assess whether the wakeup and resume timing is within the USB spec. This will probably have to be a chip-level test, since it depends on a variety of system properties and not only on the usbdev alone.

This bug is to make sure we don't forget provisioning such a test in our testplans (it might already be there, in which case we can resolve this issue).

See also #9905 for reference.

CC @a-will

estimate 16 remaining 2023-03-23 8

vogelpi commented 2 years ago

Thanks for opening the issue. We have an item in the testplan to check the wakeup and resume feature but it doesn't state to measure the timing.

msfschaffner commented 2 years ago

Ok, thanks for checking. Can you copy paste the name of the item here? @kosta-kojdic can we update that item accordingly?

tjaychen commented 2 years ago

it should be this one chip_sw_usb_suspend. The test does not explicitly point out the suspend timing, it describes more the internal mechanism.

We should add a line to indicate this needs to be spec compliant during testing.

kosta-kojdic commented 2 years ago

@msfschaffner Can you point me to the test plan you are referring to? For usbdev I am not aware of existing extensive testplan in OpenTitan hjson format.

msfschaffner commented 2 years ago

I believe this one is a chip-level integration test: https://docs.opentitan.org/hw/top_earlgrey/doc/dv/

msfschaffner commented 1 year ago

@msfschaffner

johngt commented 1 year ago

Also tagging @alees24 on this.

alees24 commented 1 year ago

estimate 16 remaining 2023-03-23 8

alees24 commented 1 year ago

The USB host controller is required to perform at least 20ms of sustained Resume Signaling and at least 50ms of Reset Signaling (of bursts lasting of no less than 10ms).

Resume from Deep Sleep to software regaining control of the USB is observed to take 2ms in t-l ASIC simulation from the point of the usbdev_aon_wake module detecting any of Resume signaling, Disconnection or Bus Reset conditions.

This affords software ample time to reconfigure the usbdev as required, deactivate the aon/wake module and resume control of the bus. (Caveat: this is with current test software; obviously the onus is on production software to become responsive in time.)

Work on t-l tests continues, but the only undesirable behavior thus far observed is recorded in #18562 - In the event of a bus disconnection being detected it would be preferable for the pull ups to be deasserted by the aon_wake module itself to ensure a clean break. There is no guarantee that the USB host will also detect a disconnect/reconnect. It is suggested that software force a deliberate, sustained disconnect before attempting to reconnect, otherwise a brief interruption could lead to inconsistent state. Since the software regains control of the USB in as little as 2ms, this will not be seen as a result of physically disconnecting and reinserting a connector, but an electrical/cabling disruption could perhaps be handled better.

WIP draft PR #18564

johngt commented 1 year ago

Note for @moidx / @jesultra - there will need to be software workarounds based on @alees24 comments

a-will commented 1 year ago

Agreed that it would be better to have the aon_wake_module handle it. It doesn't seem like a terribly dire situation, but it'd be nice to not have additional software delays added to boot time, however small. :)

Just to put the optimization into perspective: The device has 10 seconds to disconnect the pull-up after VBUS is removed (section 7.2.1), then afterwards, the software part requires at least 2.5 us of having the pull-ups disconnected (the max time allowed before a hub is required by spec to detect disconnection events).

alees24 commented 1 year ago

From my understanding when we awaken to a 'Disconnect' report from aon/wake we need only remove the pull up for the few microseconds that you mention, to ensure that the hub/host spots it.

It's worth noting that we should probably be doing that anyway, because a break in the VBUS/SENSE signal doesn't necessarily mean that a pull up deassertion has been seen by the host; suppose there's an electrical issue/contact problem on VBUS but not on D+/D-; then we could be under the impression that we've been disconnected, but the host thinks otherwise, and we thus become unresponsive, having returned to the Powered state.

moidx commented 1 year ago

@johngt can you update timing estimates on this issue to determine what work is remaining for M2.5.2? Thanks

alees24 commented 1 year ago

Nothing more required for M2.5.2

estimate 0

johngt commented 1 year ago

Per @alees24 comment - closing this issue for this current MS. Can re-open / create a new issue post the current milestone.