[DO NOT MERGE] Fix usb suspend reset #27415
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Due to an unfortunate pin conflict, IOA2 is used both to trigger SPI rescue and to control the USB VBUS sense buffer on the teacup board. To work around this, introduce a strapping that set IOA2 as input to prevent interference. Signed-off-by: Amaury Pouly <amaury.pouly@lowrisc.org>
Apply the strapping that disables IOA2 in all USB tests to avoid intereference on the VBUS sense buffer. Signed-off-by: Amaury Pouly <amaury.pouly@lowrisc.org>
Instead of printing an error message saying that several devices were found, actually provide the list of devices found. Signed-off-by: Amaury Pouly <amaury.pouly@lowrisc.org>
pamaury
force-pushed
the
fix_usb_suspend_reset
branch
from
e089473 to
263c3a5
Compare
See explanation in the code. Signed-off-by: Amaury Pouly <amaury.pouly@lowrisc.org>
Per the USB hub specification, when performing port operations, we should look at the port status to make sure that the operation was performed. This replaces the artificial 100ms delay which does not correspond to any real specification timing. This commit also introduces stricter checks on the port status: an error will occur if trying to suspend/resume a port with incompatible status (though even stricter checks are possible such as making sure that a device is connected). Signed-off-by: Amaury Pouly <amaury.pouly@lowrisc.org>
The timeout accounts for all operations (port status check, ...) and since the spec only requires at most 50ms per request, 100ms seems a bit low (although it works in practice). Bump to 1sec just to avoid stupid failures. Signed-off-by: Amaury Pouly <amaury.pouly@lowrisc.org>
Signed-off-by: Amaury Pouly <amaury.pouly@lowrisc.org>
pamaury
force-pushed
the
fix_usb_suspend_reset
branch
from
941f496 to
947f7a2
Compare
pamaury
force-pushed
the
fix_usb_suspend_reset
branch
from
7802937 to
dd46344
Compare
pamaury
force-pushed
the
fix_usb_suspend_reset
branch
from
1444920 to
e2b4def
Compare
pamaury
force-pushed
the
fix_usb_suspend_reset
branch
from
cb432ee to
188a4ef
Compare
When the device enters the shutdown phase, it will very quickly configure the usbdev interface, try to connect and very shortly afterwards disable the interface. This means that the device may or may not appear on the bus (most likely not). Meanwhile, the host is trying to suspend the device in this phase which is incorrect. Signed-off-by: Amaury Pouly <amaury.pouly@lowrisc.org>
When enabling/disable VBUS sense, it is necessary to wait for a while for things to stabilize. Make this delay configurable. Signed-off-by: Amaury Pouly <amaury.pouly@lowrisc.org>
It seems that 100ms is too low on certain boards. It is not clear why but there is no real harm in increasing this delay. Signed-off-by: Amaury Pouly <amaury.pouly@lowrisc.org>
Signed-off-by: Amaury Pouly <amaury.pouly@lowrisc.org>
pamaury
force-pushed
the
fix_usb_suspend_reset
branch
from
188a4ef to
0673806
Compare
Signed-off-by: Amaury Pouly <amaury.pouly@lowrisc.org>
Signed-off-by: Amaury Pouly <amaury.pouly@lowrisc.org>
pamaury
force-pushed
the
fix_usb_suspend_reset
branch
from
0673806 to
b894444
Compare
Signed-off-by: Amaury Pouly <amaury.pouly@lowrisc.org>
When the host fails due to some error (like hub status failure), the UART output will not be printed. This can make debugging the test challenging since some crucial information about the device state is lost. Change the test so that in case of failure, the UART is drained and displayed. Signed-off-by: Amaury Pouly <amaury.pouly@lowrisc.org>
Currently there is a sequence where the devices prints a message, suspends and then waits for a resume. On the host side, we need to wait for the UART message, send a request to the hub to suspend, observe that the suspend was successfull and then resume. The current timeout between suspend and resume is only 40ms which can be quite tight since hub operations can take a long time. Increase this delay to avoid flakiness. Signed-off-by: Amaury Pouly <amaury.pouly@lowrisc.org>
See explanation in the code. Signed-off-by: Amaury Pouly <amaury.pouly@lowrisc.org>
Signed-off-by: Amaury Pouly <amaury.pouly@lowrisc.org>
pamaury
force-pushed
the
fix_usb_suspend_reset
branch
from
8e5ed0f to
f50a818
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The usb suspend tests use hub operations to reset the device but doing so bypasses the kernel which will not try to configure the device afterwards. Includes #27337