Re: [PATCH] MdeModulePkg/XhciDxe: Retry device slot init on failure


Wu, Hao A
 

Hello Heng,

It looks to me that the patch is addressing a similar issue you met on the UpXtreme board mentioned in:
https://bugzilla.tianocore.org/show_bug.cgi?id=3007

Could you help to check if such retry during the slot initialization helps for the USB device on your board?
Thanks in advance.

Best Regards,
Hao Wu

-----Original Message-----
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Jeff
Brasen
Sent: Wednesday, October 21, 2020 11:45 PM
To: devel@edk2.groups.io
Cc: Wu, Hao A <hao.a.wu@...>; Ni, Ray <ray.ni@...>; Jon
Hunter <jonathanh@...>; Jeff Brasen <jbrasen@...>
Subject: [edk2-devel] [PATCH] MdeModulePkg/XhciDxe: Retry device slot
init on failure

From: Jon Hunter <jonathanh@...>

With some super-speed USB mass storage devices it has been observed that
a USB transaction error may occur when attempting the set the device
address during enumeration.

According the the xHCI specification (section 4.6.5) ...

"A USB Transaction ErrorCompletion Code for an Address Device Command
may be due to a Stall response from a device. Software should issue a
Disable Slot Commandfor the Device Slot then an Enable Slot Command to
recover from this error."

To fix this, retry the device slot initialization if it fails due to a device error.

Signed-off-by: Jeff Brasen <jbrasen@...>
---
MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.c | 71 ++++++++++++++--------
--
1 file changed, 42 insertions(+), 29 deletions(-)

diff --git a/MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.c
b/MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.c
index 9cb115363c..1a16864205 100644
--- a/MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.c
+++ b/MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.c
@@ -1717,9 +1717,11 @@ XhcPollPortStatusChange (
EFI_STATUS Status;
UINT8 Speed;
UINT8 SlotId;
+ UINT8 Retries;
USB_DEV_ROUTE RouteChart;

Status = EFI_SUCCESS;
+ Retries = 1;

if ((PortState->PortChangeStatus & (USB_PORT_STAT_C_CONNECTION |
USB_PORT_STAT_C_ENABLE | USB_PORT_STAT_C_OVERCURRENT |
USB_PORT_STAT_C_RESET)) == 0) {
return EFI_SUCCESS;
@@ -1739,40 +1741,51 @@ XhcPollPortStatusChange (
RouteChart.Route.TierNum = ParentRouteChart.Route.TierNum + 1;
}

- SlotId = XhcRouteStringToSlotId (Xhc, RouteChart);
- if (SlotId != 0) {
- if (Xhc->HcCParams.Data.Csz == 0) {
- Status = XhcDisableSlotCmd (Xhc, SlotId);
- } else {
- Status = XhcDisableSlotCmd64 (Xhc, SlotId);
- }
- }
-
- if (((PortState->PortStatus & USB_PORT_STAT_ENABLE) != 0) &&
- ((PortState->PortStatus & USB_PORT_STAT_CONNECTION) != 0)) {
- //
- // Has a device attached, Identify device speed after port is enabled.
- //
- Speed = EFI_USB_SPEED_FULL;
- if ((PortState->PortStatus & USB_PORT_STAT_LOW_SPEED) != 0) {
- Speed = EFI_USB_SPEED_LOW;
- } else if ((PortState->PortStatus & USB_PORT_STAT_HIGH_SPEED) != 0) {
- Speed = EFI_USB_SPEED_HIGH;
- } else if ((PortState->PortStatus & USB_PORT_STAT_SUPER_SPEED) != 0) {
- Speed = EFI_USB_SPEED_SUPER;
- }
- //
- // Execute Enable_Slot cmd for attached device, initialize device context
and assign device address.
- //
+ do {
SlotId = XhcRouteStringToSlotId (Xhc, RouteChart);
- if ((SlotId == 0) && ((PortState->PortChangeStatus &
USB_PORT_STAT_C_RESET) != 0)) {
+ if (SlotId != 0) {
if (Xhc->HcCParams.Data.Csz == 0) {
- Status = XhcInitializeDeviceSlot (Xhc, ParentRouteChart, Port,
RouteChart, Speed);
+ Status = XhcDisableSlotCmd (Xhc, SlotId);
} else {
- Status = XhcInitializeDeviceSlot64 (Xhc, ParentRouteChart, Port,
RouteChart, Speed);
+ Status = XhcDisableSlotCmd64 (Xhc, SlotId);
}
}
- }
+
+ if (((PortState->PortStatus & USB_PORT_STAT_ENABLE) != 0) &&
+ ((PortState->PortStatus & USB_PORT_STAT_CONNECTION) != 0)) {
+ //
+ // Has a device attached, Identify device speed after port is enabled.
+ //
+ Speed = EFI_USB_SPEED_FULL;
+ if ((PortState->PortStatus & USB_PORT_STAT_LOW_SPEED) != 0) {
+ Speed = EFI_USB_SPEED_LOW;
+ } else if ((PortState->PortStatus & USB_PORT_STAT_HIGH_SPEED) != 0) {
+ Speed = EFI_USB_SPEED_HIGH;
+ } else if ((PortState->PortStatus & USB_PORT_STAT_SUPER_SPEED) != 0)
{
+ Speed = EFI_USB_SPEED_SUPER;
+ }
+ //
+ // Execute Enable_Slot cmd for attached device, initialize device context
and assign device address.
+ //
+ SlotId = XhcRouteStringToSlotId (Xhc, RouteChart);
+ if ((SlotId == 0) && ((PortState->PortChangeStatus &
USB_PORT_STAT_C_RESET) != 0)) {
+ if (Xhc->HcCParams.Data.Csz == 0) {
+ Status = XhcInitializeDeviceSlot (Xhc, ParentRouteChart, Port,
RouteChart, Speed);
+ } else {
+ Status = XhcInitializeDeviceSlot64 (Xhc, ParentRouteChart, Port,
RouteChart, Speed);
+ }
+ }
+ }
+
+ //
+ // According to the xHCI specification (section 4.6.5), "a USB Transaction
+ // Error Completion Code for an Address Device Command may be due to
a Stall
+ // response from a device. Software should issue a Disable Slot Command
for
+ // the Device Slot then an Enable Slot Command to recover from this
error."
+ // Therefore, retry the device slot initialization if it fails due to a
+ // device error.
+ //
+ } while ((Status == EFI_DEVICE_ERROR) && (Retries--));

return Status;
}
--
2.25.1




Join devel@edk2.groups.io to automatically receive all group messages.