From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga02.intel.com (mga02.intel.com [134.134.136.20]) (using TLSv1 with cipher CAMELLIA256-SHA (256/256 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id 28E571A1E31 for ; Mon, 19 Sep 2016 18:39:48 -0700 (PDT) Received: from orsmga005.jf.intel.com ([10.7.209.41]) by orsmga101.jf.intel.com with ESMTP; 19 Sep 2016 18:39:48 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.30,365,1470726000"; d="scan'208";a="11205858" Received: from fmsmsx104.amr.corp.intel.com ([10.18.124.202]) by orsmga005.jf.intel.com with ESMTP; 19 Sep 2016 18:39:47 -0700 Received: from fmsmsx156.amr.corp.intel.com (10.18.116.74) by fmsmsx104.amr.corp.intel.com (10.18.124.202) with Microsoft SMTP Server (TLS) id 14.3.248.2; Mon, 19 Sep 2016 18:39:47 -0700 Received: from shsmsx104.ccr.corp.intel.com (10.239.4.70) by fmsmsx156.amr.corp.intel.com (10.18.116.74) with Microsoft SMTP Server (TLS) id 14.3.248.2; Mon, 19 Sep 2016 18:39:46 -0700 Received: from shsmsx101.ccr.corp.intel.com ([169.254.1.118]) by SHSMSX104.ccr.corp.intel.com ([169.254.5.101]) with mapi id 14.03.0248.002; Tue, 20 Sep 2016 09:39:44 +0800 From: "Tian, Feng" To: Leif Lindholm CC: "Zeng, Star" , "edk2-devel@lists.01.org" , "Tian, Feng" Thread-Topic: [edk2] [patch] MdeModulePkg/Xhci: add 1ms delay before access MMIO reg during reset Thread-Index: AQHSDiijBkxkvbxojUeAsAoIvhne5KB4b8OAgAkzGlA= Date: Tue, 20 Sep 2016 01:39:43 +0000 Message-ID: <7F1BAD85ADEA444D97065A60D2E97EE566E1B1D4@SHSMSX101.ccr.corp.intel.com> References: <5a0c21d0fe889950fda9e59aaf2272b66d429b8f.1473816993.git.feng.tian@intel.com> <20160914130833.GB16080@bivouac.eciton.net> In-Reply-To: <20160914130833.GB16080@bivouac.eciton.net> 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/Xhci: add 1ms delay before access MMIO reg during reset X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 20 Sep 2016 01:39:48 -0000 Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Lefi, This delay is just a workaround for some XHCI HCs, I will add such info int= o commit log. Thanks Feng -----Original Message----- From: Leif Lindholm [mailto:leif.lindholm@linaro.org]=20 Sent: Wednesday, September 14, 2016 9:09 PM To: Tian, Feng Cc: Zeng, Star ; edk2-devel@lists.01.org Subject: Re: [edk2] [patch] MdeModulePkg/Xhci: add 1ms delay before access = MMIO reg during reset On Wed, Sep 14, 2016 at 09:37:13AM +0800, Feng Tian wrote: > Some XHCI host controllers require to have extra 1ms delay before=20 > accessing any MMIO register during HC reset. Is this compliant with the XHCI specification or a bug workaround? I am not going to argue about the delay, but I would like to see it spelled= out in the commit message. > Cc: Star Zeng > Contributed-under: TianoCore Contribution Agreement 1.0 > Signed-off-by: Feng Tian > --- > MdeModulePkg/Bus/Pci/XhciDxe/XhciReg.c | 6 +++++- > 1 file changed, 5 insertions(+), 1 deletion(-) >=20 > diff --git a/MdeModulePkg/Bus/Pci/XhciDxe/XhciReg.c=20 > b/MdeModulePkg/Bus/Pci/XhciDxe/XhciReg.c > index d0f2205..cb822a6 100644 > --- a/MdeModulePkg/Bus/Pci/XhciDxe/XhciReg.c > +++ b/MdeModulePkg/Bus/Pci/XhciDxe/XhciReg.c > @@ -2,7 +2,7 @@ > =20 > The XHCI register operation routines. > =20 > -Copyright (c) 2011 - 2015, Intel Corporation. All rights=20 > reserved.
> +Copyright (c) 2011 - 2016, Intel Corporation. All rights=20 > +reserved.
> This program and the accompanying materials are licensed and made=20 > available under the terms and conditions of the BSD License which=20 > accompanies this distribution. The full text of the license may be=20 > found at @@ -687,6 +687,10 @@ XhcResetHC ( > if ((Xhc->DebugCapSupOffset =3D=3D 0xFFFFFFFF) || ((XhcReadExtCapReg (= Xhc, Xhc->DebugCapSupOffset) & 0xFF) !=3D XHC_CAP_USB_DEBUG) || > ((XhcReadExtCapReg (Xhc, Xhc->DebugCapSupOffset + XHC_DC_DCCTRL) &= BIT0) =3D=3D 0)) { > XhcSetOpRegBit (Xhc, XHC_USBCMD_OFFSET, XHC_USBCMD_RESET); > + // > + // some XHCI host controllers require to have extra 1ms delay before= accessing any MMIO register during reset. > + // > + gBS->Stall (XHC_1_MILLISECOND); OK, so this is not actually a comment on this patch, but why do we have so = many separate definitions of how many microseconds go in a milisecond or se= cond? USB_BUS_1_MILLISECOND (Pei and Dxe) XHC_1_MILLISECOND (Pei and Dxe) EHC_1_MILLISECOND (Pei and Dxe) UHC_1_MILLISECOND (Dxe, Pei defines STALL_1_MILLI_SECOND instead) And IdeBusPei/PciBusDxe do similar things. Could we add something common to Base.h instead, like the SIZE_ ones, dropp= ping a lot of duplication, gaining uniformity, and correcting spelling? / Leif > Status =3D XhcWaitOpRegBit (Xhc, XHC_USBCMD_OFFSET, XHC_USBCMD_RESET= , FALSE, Timeout); > } > =20 > -- > 2.7.1.windows.2 >=20 > _______________________________________________ > edk2-devel mailing list > edk2-devel@lists.01.org > https://lists.01.org/mailman/listinfo/edk2-devel