From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Permerror (SPF Permanent Error: Two or more type TXT spf records found.) identity=mailfrom; client-ip=192.55.52.136; helo=mga12.intel.com; envelope-from=star.zeng@intel.com; receiver=edk2-devel@lists.01.org Received: from mga12.intel.com (mga12.intel.com [192.55.52.136]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id D98B3210FCF5C for ; Mon, 3 Sep 2018 01:36:02 -0700 (PDT) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga003.fm.intel.com ([10.253.24.29]) by fmsmga106.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 03 Sep 2018 01:36:01 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.53,324,1531810800"; d="scan'208";a="77560674" Received: from fmsmsx106.amr.corp.intel.com ([10.18.124.204]) by FMSMGA003.fm.intel.com with ESMTP; 03 Sep 2018 01:35:30 -0700 Received: from shsmsx103.ccr.corp.intel.com (10.239.4.69) by FMSMSX106.amr.corp.intel.com (10.18.124.204) with Microsoft SMTP Server (TLS) id 14.3.319.2; Mon, 3 Sep 2018 01:35:30 -0700 Received: from shsmsx102.ccr.corp.intel.com ([169.254.2.226]) by SHSMSX103.ccr.corp.intel.com ([169.254.4.240]) with mapi id 14.03.0319.002; Mon, 3 Sep 2018 16:35:29 +0800 From: "Zeng, Star" To: Laszlo Ersek , edk2-devel-01 CC: Gerd Hoffmann , "Wang, Jian J" , "Ni, Ruiyu" , "Shi, Steven" , "Zeng, Star" Thread-Topic: [PATCH] MdeModulePkg/EhciDxe: fix host controller reset condition in BindingStart Thread-Index: AQHUQTNuAy95qhbdEkexZhGyLikIpqTeP02A Date: Mon, 3 Sep 2018 08:35:28 +0000 Message-ID: <0C09AFA07DD0434D9E2A0C6AEB0483103BBB4C09@shsmsx102.ccr.corp.intel.com> References: <20180831140330.26888-1-lersek@redhat.com> In-Reply-To: <20180831140330.26888-1-lersek@redhat.com> Accept-Language: zh-CN, en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [10.239.127.40] MIME-Version: 1.0 Subject: Re: [PATCH] MdeModulePkg/EhciDxe: fix host controller reset condition in BindingStart X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 03 Sep 2018 08:36:03 -0000 Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Good finding. There is another place at https://github.com/tianocore/edk2/blob/master/Mde= ModulePkg/Bus/Pci/EhciDxe/Ehci.c#L150 which has similar logic, please updat= e it also. Thanks, Star -----Original Message----- From: Laszlo Ersek [mailto:lersek@redhat.com]=20 Sent: Friday, August 31, 2018 10:04 PM To: edk2-devel-01 Cc: Gerd Hoffmann ; Wang, Jian J = ; Ni, Ruiyu ; Zeng, Star ; Shi, St= even 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 ho= st 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 o= nly 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 th= e 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 n= ot have a Debug Port. In repeated disconnect / connect cycles, the controll= er is never reset in EhcDriverBindingStart(), therefore the devices on the = controller are never re-enumerated after a disconnect. Cc: Gerd Hoffmann Cc: Jian J Wang Cc: Ruiyu Ni Cc: Star Zeng Cc: Steven Shi Reported-by: Steven Shi Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=3D1129 Fixes: 09943f5ecc0fbc0c98c511c82703a0ba3b2b5819 Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Laszlo Ersek --- 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/Ehc= iDxe/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); } =20 - if (Ehc->DebugPortNum !=3D 0) { + if (Ehc->DebugPortNum =3D=3D 0) { + EhcResetHC (Ehc, EHC_RESET_TIMEOUT); } else { State =3D EhcReadDbgRegister(Ehc, 0); if ((State & (USB_DEBUG_PORT_IN_USE | USB_DEBUG_PORT_OWNER)) !=3D (USB= _DEBUG_PORT_IN_USE | USB_DEBUG_PORT_OWNER)) { EhcResetHC (Ehc, EHC_RESET_TIMEOUT); } } -- 2.14.1.3.gb7cf6e02401b