Re: [PATCH] MdeModulePkg/EhciDxe: factor out EhcIsDebugPortInUse()

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

Thanks for the quick following up. :)

Reviewed-by: Star Zeng <star.zeng@intel.com>

Star
-----Original Message-----
From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Laszlo Ersek
Sent: Thursday, September 6, 2018 3:45 AM
To: edk2-devel-01 <edk2-devel@lists.01.org>
Cc: Ni, Ruiyu <ruiyu.ni@intel.com>; Zeng, Star <star.zeng@intel.com>
Subject: [edk2] [PATCH] MdeModulePkg/EhciDxe: factor out EhcIsDebugPortInUse()

The EhcReset(), EhcGetRootHubPortStatus() and EhcDriverBindingStart() functions need to see whether the host controller (or a specific port on the host controller) can be accessed, dependent on the controller having (or the specific port being) an in-use debug port. Because the condition isn't simple, extract it to a separate function.

Cc: Ruiyu Ni <ruiyu.ni@intel.com>
Cc: Star Zeng <star.zeng@intel.com>
Suggested-by: Star Zeng <star.zeng@intel.com>
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_dbgport_in_use

 MdeModulePkg/Bus/Pci/EhciDxe/Ehci.h    |  2 +
 MdeModulePkg/Bus/Pci/EhciDxe/EhciReg.h | 27 ++++++----
 MdeModulePkg/Bus/Pci/EhciDxe/Ehci.c    | 26 +++------
 MdeModulePkg/Bus/Pci/EhciDxe/EhciReg.c | 55 +++++++++++++++++++-
 4 files changed, 80 insertions(+), 30 deletions(-)

diff --git a/MdeModulePkg/Bus/Pci/EhciDxe/Ehci.h b/MdeModulePkg/Bus/Pci/EhciDxe/Ehci.h
index f7556754f8ea..d7fbecb43003 100644
--- a/MdeModulePkg/Bus/Pci/EhciDxe/Ehci.h
+++ b/MdeModulePkg/Bus/Pci/EhciDxe/Ehci.h
@@ -77,6 +77,8 @@ typedef struct _USB2_HC_DEV  USB2_HC_DEV;
 #define USB_DEBUG_PORT_IN_USE        BIT10
 #define USB_DEBUG_PORT_ENABLE        BIT28
 #define USB_DEBUG_PORT_OWNER         BIT30
+#define USB_DEBUG_PORT_IN_USE_MASK   (USB_DEBUG_PORT_IN_USE | \
+                                      USB_DEBUG_PORT_OWNER)
 
 //
 // EHC raises TPL to TPL_NOTIFY to serialize all its operations diff --git a/MdeModulePkg/Bus/Pci/EhciDxe/EhciReg.h b/MdeModulePkg/Bus/Pci/EhciDxe/EhciReg.h
index 2347ee125fa7..1ee12455b8d0 100644
--- a/MdeModulePkg/Bus/Pci/EhciDxe/EhciReg.h
+++ b/MdeModulePkg/Bus/Pci/EhciDxe/EhciReg.h
@@ -137,19 +137,28 @@ EhcReadCapRegister (
   );
 
 /**
-  Read EHCI debug port register.
+  Check whether the host controller has an in-use debug port.
 
-  @param  Ehc          The EHCI device.
-  @param  Offset       Debug port register address.
+  @param[in] Ehc         The Enhanced Host Controller to query.
 
-  @return The register content read.
-  @retval If err, return 0xffff.
+  @param[in] PortNumber  If PortNumber is not NULL, then query whether
+                         PortNumber is an in-use debug port on Ehc. (PortNumber
+                         is taken in UEFI notation, i.e., zero-based.)
+                         Otherwise, query whether Ehc has any in-use debug
+                         port.
 
+  @retval TRUE   PortNumber is an in-use debug port on Ehc (if PortNumber is
+                 not NULL), or some port on Ehc is an in-use debug port
+                 (otherwise).
+
+  @retval FALSE  PortNumber is not an in-use debug port on Ehc (if PortNumber
+                 is not NULL), or no port on Ehc is an in-use debug port
+                 (otherwise).
 **/
-UINT32
-EhcReadDbgRegister (
-  IN  USB2_HC_DEV         *Ehc,
-  IN  UINT32              Offset
+BOOLEAN
+EhcIsDebugPortInUse (
+  IN CONST USB2_HC_DEV *Ehc,
+  IN CONST UINT8       *PortNumber OPTIONAL
   );
 
 /**
diff --git a/MdeModulePkg/Bus/Pci/EhciDxe/Ehci.c b/MdeModulePkg/Bus/Pci/EhciDxe/Ehci.c
index 89ed034b9093..50b5598df4fb 100644
--- a/MdeModulePkg/Bus/Pci/EhciDxe/Ehci.c
+++ b/MdeModulePkg/Bus/Pci/EhciDxe/Ehci.c
@@ -121,7 +121,6 @@ EhcReset (
   USB2_HC_DEV             *Ehc;
   EFI_TPL                 OldTpl;
   EFI_STATUS              Status;
-  UINT32                  DbgCtrlStatus;
 
   Ehc = EHC_FROM_THIS (This);
 
@@ -147,12 +146,9 @@ EhcReset (
     //
     // Host Controller must be Halt when Reset it
     //
-    if (Ehc->DebugPortNum != 0) {
-      DbgCtrlStatus = EhcReadDbgRegister(Ehc, 0);
-      if ((DbgCtrlStatus & (USB_DEBUG_PORT_IN_USE | USB_DEBUG_PORT_OWNER)) == (USB_DEBUG_PORT_IN_USE | USB_DEBUG_PORT_OWNER)) {
-        Status = EFI_SUCCESS;
-        goto ON_EXIT;
-      }
+    if (EhcIsDebugPortInUse (Ehc, NULL)) {
+      Status = EFI_SUCCESS;
+      goto ON_EXIT;
     }
 
     if (!EhcIsHalt (Ehc)) {
@@ -345,7 +341,6 @@ EhcGetRootHubPortStatus (
   UINTN                   Index;
   UINTN                   MapSize;
   EFI_STATUS              Status;
-  UINT32                  DbgCtrlStatus;
 
   if (PortStatus == NULL) {
     return EFI_INVALID_PARAMETER;
@@ -367,11 +362,8 @@ EhcGetRootHubPortStatus (
   PortStatus->PortStatus        = 0;
   PortStatus->PortChangeStatus  = 0;
 
-  if ((Ehc->DebugPortNum != 0) && (PortNumber == (Ehc->DebugPortNum - 1))) {
-    DbgCtrlStatus = EhcReadDbgRegister(Ehc, 0);
-    if ((DbgCtrlStatus & (USB_DEBUG_PORT_IN_USE | USB_DEBUG_PORT_OWNER)) == (USB_DEBUG_PORT_IN_USE | USB_DEBUG_PORT_OWNER)) {
-      goto ON_EXIT;
-    }
+  if (EhcIsDebugPortInUse (Ehc, &PortNumber)) {
+    goto ON_EXIT;
   }
 
   State                         = EhcReadOpReg (Ehc, Offset);
@@ -1696,7 +1688,6 @@ EhcDriverBindingStart (
   UINTN                   EhciBusNumber;
   UINTN                   EhciDeviceNumber;
   UINTN                   EhciFunctionNumber;
-  UINT32                  State;
   EFI_DEVICE_PATH_PROTOCOL  *HcDevicePath;
 
   //
@@ -1918,13 +1909,8 @@ EhcDriverBindingStart (
     EhcClearLegacySupport (Ehc);
   }
 
-  if (Ehc->DebugPortNum == 0) {
+  if (!EhcIsDebugPortInUse (Ehc, NULL)) {
     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);
-    }
   }
 
   Status = EhcInitHC (Ehc);
diff --git a/MdeModulePkg/Bus/Pci/EhciDxe/EhciReg.c b/MdeModulePkg/Bus/Pci/EhciDxe/EhciReg.c
index 59752d1bdc64..11c36132fd4f 100644
--- a/MdeModulePkg/Bus/Pci/EhciDxe/EhciReg.c
+++ b/MdeModulePkg/Bus/Pci/EhciDxe/EhciReg.c
@@ -65,7 +65,7 @@ EhcReadCapRegister (
 **/
 UINT32
 EhcReadDbgRegister (
-  IN  USB2_HC_DEV         *Ehc,
+  IN  CONST USB2_HC_DEV   *Ehc,
   IN  UINT32              Offset
   )
 {
@@ -90,6 +90,59 @@ EhcReadDbgRegister (
 }
 
 
+/**
+  Check whether the host controller has an in-use debug port.
+
+  @param[in] Ehc         The Enhanced Host Controller to query.
+
+  @param[in] PortNumber  If PortNumber is not NULL, then query whether
+                         PortNumber is an in-use debug port on Ehc. (PortNumber
+                         is taken in UEFI notation, i.e., zero-based.)
+                         Otherwise, query whether Ehc has any in-use debug
+                         port.
+
+  @retval TRUE   PortNumber is an in-use debug port on Ehc (if PortNumber is
+                 not NULL), or some port on Ehc is an in-use debug port
+                 (otherwise).
+
+  @retval FALSE  PortNumber is not an in-use debug port on Ehc (if PortNumber
+                 is not NULL), or no port on Ehc is an in-use debug port
+                 (otherwise).
+**/
+BOOLEAN
+EhcIsDebugPortInUse (
+  IN CONST USB2_HC_DEV *Ehc,
+  IN CONST UINT8       *PortNumber OPTIONAL
+  )
+{
+  UINT32 State;
+
+  if (Ehc->DebugPortNum == 0) {
+    //
+    // The host controller has no debug port.
+    //
+    return FALSE;
+  }
+
+  //
+  // The Debug Port Number field in HCSPARAMS is one-based.
+  //
+  if (PortNumber != NULL && *PortNumber != Ehc->DebugPortNum - 1) {
+    //
+    // The caller specified a port, but it's not the debug port of the host
+    // controller.
+    //
+    return FALSE;
+  }
+
+  //
+  // Deduce usage from the Control Register.
+  //
+  State = EhcReadDbgRegister(Ehc, 0);
+  return (State & USB_DEBUG_PORT_IN_USE_MASK) == 
+USB_DEBUG_PORT_IN_USE_MASK; }
+
+
 /**
   Read EHCI Operation register.
 
--
2.14.1.3.gb7cf6e02401b

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel