Re: [PATCH] MdeModulePkg/EhciDxe: fix host controller reset condition in BindingStart

["Zeng, Star" <star.zeng@intel.com>]

Good finding.

There is another place at https://github.com/tianocore/edk2/blob/master/MdeModulePkg/Bus/Pci/EhciDxe/Ehci.c#L150 which has similar logic, please update it also.


Thanks,
Star
-----Original Message-----
From: Laszlo Ersek [mailto:lersek@redhat.com] 
Sent: Friday, August 31, 2018 10:04 PM
To: edk2-devel-01 <edk2-devel@lists.01.org>
Cc: Gerd Hoffmann <kraxel@redhat.com>; Wang, Jian J <jian.j.wang@intel.com>; Ni, Ruiyu <ruiyu.ni@intel.com>; Zeng, Star <star.zeng@intel.com>; Shi, Steven <steven.shi@intel.com>
Subject: [PATCH] MdeModulePkg/EhciDxe: fix host controller reset condition in BindingStart

Commit 09943f5ecc0f ("MdeModulePkg: Skip to manage usb debug port in EDKII EHCI driver if it's used by usb debug port driver", 2012-04-28) made the host controller reset in EhcDriverBindingStart() conditional. The intent was to avoid the reset when
- one of the USB ports on the host controller was a Debug Port, AND
- the Debug Port was in use.

This translates to the following positive condition: reset the controller only if:
- no USB port on the host controller is a Debug Port, OR
- the Debug Port is not in use.

Commit 09943f5ecc0f failed to implement the first subcondition, and thus the result is too strict now (for resetting the host controller). Relax it to the correct condition.

This issue was found by Steven Shi on QEMU. QEMU's EHCI device model does not have a Debug Port. In repeated disconnect / connect cycles, the controller is never reset in EhcDriverBindingStart(), therefore the devices on the controller are never re-enumerated after a disconnect.

Cc: Gerd Hoffmann <kraxel@redhat.com>
Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Ruiyu Ni <ruiyu.ni@intel.com>
Cc: Star Zeng <star.zeng@intel.com>
Cc: Steven Shi <steven.shi@intel.com>
Reported-by: Steven Shi <steven.shi@intel.com>
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=1129
Fixes: 09943f5ecc0fbc0c98c511c82703a0ba3b2b5819
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
---

Notes:
    Repo:   https://github.com/lersek/edk2.git
    Branch: ehci_start_reset_1129

 MdeModulePkg/Bus/Pci/EhciDxe/Ehci.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/MdeModulePkg/Bus/Pci/EhciDxe/Ehci.c b/MdeModulePkg/Bus/Pci/EhciDxe/Ehci.c
index 30ad2b2ffeef..89ed034b9093 100644
--- a/MdeModulePkg/Bus/Pci/EhciDxe/Ehci.c
+++ b/MdeModulePkg/Bus/Pci/EhciDxe/Ehci.c
@@ -1916,11 +1916,13 @@ EhcDriverBindingStart (
   //
   if (FeaturePcdGet (PcdTurnOffUsbLegacySupport)) {
     EhcClearLegacySupport (Ehc);
   }
 
-  if (Ehc->DebugPortNum != 0) {
+  if (Ehc->DebugPortNum == 0) {
+    EhcResetHC (Ehc, EHC_RESET_TIMEOUT);  } else {
     State = EhcReadDbgRegister(Ehc, 0);
     if ((State & (USB_DEBUG_PORT_IN_USE | USB_DEBUG_PORT_OWNER)) != (USB_DEBUG_PORT_IN_USE | USB_DEBUG_PORT_OWNER)) {
       EhcResetHC (Ehc, EHC_RESET_TIMEOUT);
     }
   }
--
2.14.1.3.gb7cf6e02401b