Re: [edk2] [patch] MdeModulePkg/Xhci: Add 10ms delay before sending SendAddr cmd to dev

2016-11-22 Thread Zeng, Star 
Reviewed-by: Star Zeng <star.z...@intel.com>

-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Feng Tian
Sent: Wednesday, November 23, 2016 9:58 AM
To: edk2-devel@lists.01.org
Cc: Baraneedharan Anbazhagan <anbazha...@hp.com>; Zeng, Star 
<star.z...@intel.com>
Subject: [edk2] [patch] MdeModulePkg/Xhci: Add 10ms delay before sending 
SendAddr cmd to dev

We send ADDRESS DEVICE CMD in XhcInitializeDeviceSlot(), which will cause XHC 
issue a USB SET_ADDRESS request to the USB Device.

According to USB spec, there should have a 10ms delay before this operation 
after resetting a given port.

But in original code, there is a possible path which may have no such 10ms 
delay:
UsbHubResetPort()->UsbHubSetPortFeature()->Stall(20)->UsbHubGetPortSt
atus()->XhcPollPortStatusChange()->(if RESET_C bit is set)-> 
XhcInitializeDeviceSlot()->(if RESET_C bit is set)->Stall(10)

So this patch is used to fix above issue.

Cc: Star Zeng <star.z...@intel.com>
Cc: Baraneedharan Anbazhagan <anbazha...@hp.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Feng Tian <feng.t...@intel.com>
Tested-by: Baraneedharan Anbazhagan <anbazha...@hp.com>
---
 MdeModulePkg/Bus/Pci/XhciDxe/Xhci.h  |  7 ++-
 MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.c | 10 +-
 MdeModulePkg/Bus/Pci/XhciPei/XhcPeim.h   |  8 +++-
 MdeModulePkg/Bus/Pci/XhciPei/XhciSched.c | 10 +-
 4 files changed, 31 insertions(+), 4 deletions(-)

diff --git a/MdeModulePkg/Bus/Pci/XhciDxe/Xhci.h 
b/MdeModulePkg/Bus/Pci/XhciDxe/Xhci.h
index 06cc73c..28e2402 100644
--- a/MdeModulePkg/Bus/Pci/XhciDxe/Xhci.h
+++ b/MdeModulePkg/Bus/Pci/XhciDxe/Xhci.h
@@ -2,7 +2,7 @@
 
   Provides some data structure definitions used by the XHCI host controller 
driver.
 
-Copyright (c) 2011 - 2015, Intel Corporation. All rights reserved.
+Copyright (c) 2011 - 2016, Intel Corporation. All rights reserved.
 This program and the accompanying materials  are licensed and made available 
under the terms and conditions of the BSD License  which accompanies this 
distribution.  The full text of the license may be found at
@@ -61,6 +61,11 @@ typedef struct _USB_DEV_CONTEXT  USB_DEV_CONTEXT;
 //
 #define XHC_RESET_TIMEOUT(1000)
 //
+// TRSTRCY delay requirement in usb 2.0 spec chapter 7.1.7.5.
+// The unit is microsecond, setting it as 10ms.
+//
+#define XHC_RESET_RECOVERY_DELAY (10 * 1000)
+//
 // XHC async transfer timer interval, set by experience.
 // The unit is 100us, takes 1ms as interval.
 //
diff --git a/MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.c 
b/MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.c
index e37f674..4bec76a 100644
--- a/MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.c
+++ b/MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.c
@@ -2,7 +2,7 @@
 
   XHCI transfer scheduling routines.
 
-Copyright (c) 2011 - 2015, Intel Corporation. All rights reserved.
+Copyright (c) 2011 - 2016, Intel Corporation. All rights reserved.
 This program and the accompanying materials  are licensed and made available 
under the terms and conditions of the BSD License  which accompanies this 
distribution.  The full text of the license may be found at @@ -2115,6 +2115,10 
@@ XhcInitializeDeviceSlot (
   // 8) Issue an Address Device Command for the Device Slot, where the command 
points to the Input
   //Context data structure described above.
   //
+  // Delay 10ms to meet TRSTRCY delay requirement in usb 2.0 spec 
+ chapter 7.1.7.5 before sending SetAddress() request  // to device.
+  //
+  gBS->Stall (XHC_RESET_RECOVERY_DELAY);
   ZeroMem (, sizeof (CmdTrbAddr));
   PhyAddr = UsbHcGetPciAddrForHostAddr (Xhc->MemPool, 
Xhc->UsbDevContext[SlotId].InputContext, sizeof (INPUT_CONTEXT));
   CmdTrbAddr.PtrLo= XHC_LOW_32BIT (PhyAddr);
@@ -2321,6 +2325,10 @@ XhcInitializeDeviceSlot64 (
   // 8) Issue an Address Device Command for the Device Slot, where the command 
points to the Input
   //Context data structure described above.
   //
+  // Delay 10ms to meet TRSTRCY delay requirement in usb 2.0 spec 
+ chapter 7.1.7.5 before sending SetAddress() request  // to device.
+  //
+  gBS->Stall (XHC_RESET_RECOVERY_DELAY);
   ZeroMem (, sizeof (CmdTrbAddr));
   PhyAddr = UsbHcGetPciAddrForHostAddr (Xhc->MemPool, 
Xhc->UsbDevContext[SlotId].InputContext, sizeof (INPUT_CONTEXT_64));
   CmdTrbAddr.PtrLo= XHC_LOW_32BIT (PhyAddr);
diff --git a/MdeModulePkg/Bus/Pci/XhciPei/XhcPeim.h 
b/MdeModulePkg/Bus/Pci/XhciPei/XhcPeim.h
index ccf4dc2..99f0396 100644
--- a/MdeModulePkg/Bus/Pci/XhciPei/XhcPeim.h
+++ b/MdeModulePkg/Bus/Pci/XhciPei/XhcPeim.h
@@ -1,7 +1,7 @@
 /** @file
 Private Header file for Usb Host Controller PEIM
 
-Copyright (c) 2014 - 2015, Intel Corporation. All rights reserved.
+Copyright (c) 2014 - 2016, Intel Corporation. All rights reserved.
 
 This program and the accompanying materials  are licensed and made available 
under the term

[edk2] [patch] MdeModulePkg/Xhci: Add 10ms delay before sending SendAddr cmd to dev

2016-11-22 Thread Feng Tian 
We send ADDRESS DEVICE CMD in XhcInitializeDeviceSlot(), which will
cause XHC issue a USB SET_ADDRESS request to the USB Device.

According to USB spec, there should have a 10ms delay before this
operation after resetting a given port.

But in original code, there is a possible path which may have no such
10ms delay:
UsbHubResetPort()->UsbHubSetPortFeature()->Stall(20)->UsbHubGetPortSt
atus()->XhcPollPortStatusChange()->(if RESET_C bit is set)->
XhcInitializeDeviceSlot()->(if RESET_C bit is set)->Stall(10)

So this patch is used to fix above issue.

Cc: Star Zeng 
Cc: Baraneedharan Anbazhagan 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Feng Tian 
Tested-by: Baraneedharan Anbazhagan 
---
 MdeModulePkg/Bus/Pci/XhciDxe/Xhci.h  |  7 ++-
 MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.c | 10 +-
 MdeModulePkg/Bus/Pci/XhciPei/XhcPeim.h   |  8 +++-
 MdeModulePkg/Bus/Pci/XhciPei/XhciSched.c | 10 +-
 4 files changed, 31 insertions(+), 4 deletions(-)

diff --git a/MdeModulePkg/Bus/Pci/XhciDxe/Xhci.h 
b/MdeModulePkg/Bus/Pci/XhciDxe/Xhci.h
index 06cc73c..28e2402 100644
--- a/MdeModulePkg/Bus/Pci/XhciDxe/Xhci.h
+++ b/MdeModulePkg/Bus/Pci/XhciDxe/Xhci.h
@@ -2,7 +2,7 @@
 
   Provides some data structure definitions used by the XHCI host controller 
driver.
 
-Copyright (c) 2011 - 2015, Intel Corporation. All rights reserved.
+Copyright (c) 2011 - 2016, Intel Corporation. All rights reserved.
 This program and the accompanying materials
 are licensed and made available under the terms and conditions of the BSD 
License
 which accompanies this distribution.  The full text of the license may be 
found at
@@ -61,6 +61,11 @@ typedef struct _USB_DEV_CONTEXT  USB_DEV_CONTEXT;
 //
 #define XHC_RESET_TIMEOUT(1000)
 //
+// TRSTRCY delay requirement in usb 2.0 spec chapter 7.1.7.5.
+// The unit is microsecond, setting it as 10ms.
+//
+#define XHC_RESET_RECOVERY_DELAY (10 * 1000)
+//
 // XHC async transfer timer interval, set by experience.
 // The unit is 100us, takes 1ms as interval.
 //
diff --git a/MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.c 
b/MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.c
index e37f674..4bec76a 100644
--- a/MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.c
+++ b/MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.c
@@ -2,7 +2,7 @@
 
   XHCI transfer scheduling routines.
 
-Copyright (c) 2011 - 2015, Intel Corporation. All rights reserved.
+Copyright (c) 2011 - 2016, Intel Corporation. All rights reserved.
 This program and the accompanying materials
 are licensed and made available under the terms and conditions of the BSD 
License
 which accompanies this distribution.  The full text of the license may be 
found at
@@ -2115,6 +2115,10 @@ XhcInitializeDeviceSlot (
   // 8) Issue an Address Device Command for the Device Slot, where the command 
points to the Input
   //Context data structure described above.
   //
+  // Delay 10ms to meet TRSTRCY delay requirement in usb 2.0 spec chapter 
7.1.7.5 before sending SetAddress() request
+  // to device.
+  //
+  gBS->Stall (XHC_RESET_RECOVERY_DELAY);
   ZeroMem (, sizeof (CmdTrbAddr));
   PhyAddr = UsbHcGetPciAddrForHostAddr (Xhc->MemPool, 
Xhc->UsbDevContext[SlotId].InputContext, sizeof (INPUT_CONTEXT));
   CmdTrbAddr.PtrLo= XHC_LOW_32BIT (PhyAddr);
@@ -2321,6 +2325,10 @@ XhcInitializeDeviceSlot64 (
   // 8) Issue an Address Device Command for the Device Slot, where the command 
points to the Input
   //Context data structure described above.
   //
+  // Delay 10ms to meet TRSTRCY delay requirement in usb 2.0 spec chapter 
7.1.7.5 before sending SetAddress() request
+  // to device.
+  //
+  gBS->Stall (XHC_RESET_RECOVERY_DELAY);
   ZeroMem (, sizeof (CmdTrbAddr));
   PhyAddr = UsbHcGetPciAddrForHostAddr (Xhc->MemPool, 
Xhc->UsbDevContext[SlotId].InputContext, sizeof (INPUT_CONTEXT_64));
   CmdTrbAddr.PtrLo= XHC_LOW_32BIT (PhyAddr);
diff --git a/MdeModulePkg/Bus/Pci/XhciPei/XhcPeim.h 
b/MdeModulePkg/Bus/Pci/XhciPei/XhcPeim.h
index ccf4dc2..99f0396 100644
--- a/MdeModulePkg/Bus/Pci/XhciPei/XhcPeim.h
+++ b/MdeModulePkg/Bus/Pci/XhciPei/XhcPeim.h
@@ -1,7 +1,7 @@
 /** @file
 Private Header file for Usb Host Controller PEIM
 
-Copyright (c) 2014 - 2015, Intel Corporation. All rights reserved.
+Copyright (c) 2014 - 2016, Intel Corporation. All rights reserved.
 
 This program and the accompanying materials
 are licensed and made available under the terms and conditions
@@ -53,6 +53,12 @@ typedef struct _USB_DEV_CONTEXT USB_DEV_CONTEXT;
 #define XHC_RESET_TIMEOUT   (1000)
 
 //
+// TRSTRCY delay requirement in usb 2.0 spec chapter 7.1.7.5.
+// The unit is microsecond, setting it as 10ms.
+//
+#define XHC_RESET_RECOVERY_DELAY (10 * 1000)
+
+//
 // Wait for root port state stable.
 //
 #define XHC_ROOT_PORT_STATE_STABLE  (200 * XHC_1_MILLISECOND)
diff --git a/MdeModulePkg/Bus/Pci/XhciPei/XhciSched.c