From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wm0-x22b.google.com (mail-wm0-x22b.google.com [IPv6:2a00:1450:400c:c09::22b]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id 5C9191A1E62 for ; Wed, 14 Sep 2016 06:08:39 -0700 (PDT) Received: by mail-wm0-x22b.google.com with SMTP id b187so46408604wme.1 for ; Wed, 14 Sep 2016 06:08:39 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to:user-agent; bh=J9RjlErHM6OxONOYWvTLj1Y4rIPrIbtNy3YP59L5eUo=; b=TzaHqQWUFqh8ZceT1lsNXXGKQOI4f6ubqkYOdaz232ldIM/Bp9DKZLDs2HAn9SMHIS Yuwf5ewaiehyXq+Hnj0oySlCddzg9KagiA9p9GUKUHTKQGS8lDRzm/ef/mLMB0fKx7tk VBPoOuTc9xfIKhtsDX0jyNak5p7dRBoGEpEp8= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to:user-agent; bh=J9RjlErHM6OxONOYWvTLj1Y4rIPrIbtNy3YP59L5eUo=; b=ceGEQQbi32WQBNRHPImWI1c5aJ5JRPSvBHTGLIDwC05rBbhxXLibftnMFF+iURcnQf 9Y6Oe9/o8B/uTBRpccWnUKm3yOHn8i7Njf2vNFWL62qgX26YMBv6aJ4p4rF5txlV+Eky XuuYie0hy3pt4GdkmXvir0pg3bf+NZ53XL7jBMDkoSeRs+b6V6GuOKCp1ZbN1XP8y5+I UUpN12Fm6XCOvYaiD16mwYFitzTQZQCOkl98ZIyRtiKxNtg10qCpQv9o7TLFCw/nskwd Ez4pfvcqRL2rdjSZoOvhZ0xgGn8nvpJL9xk12aw+S2ZUhOEc7OioN5qO07Jp1PWXzRpj kzzg== X-Gm-Message-State: AE9vXwNEkK678pU5/1424wsHX22gws/WpRhHS3yEhdPW/7BGYPROJ6VNqZaeSmcbaA8glhF9 X-Received: by 10.28.209.76 with SMTP id i73mr959003wmg.104.1473858516438; Wed, 14 Sep 2016 06:08:36 -0700 (PDT) Received: from bivouac.eciton.net (bivouac.eciton.net. [2a00:1098:0:86:1000:23:0:2]) by smtp.gmail.com with ESMTPSA id s6sm4148733wjm.25.2016.09.14.06.08.35 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Wed, 14 Sep 2016 06:08:35 -0700 (PDT) Date: Wed, 14 Sep 2016 14:08:33 +0100 From: Leif Lindholm To: Feng Tian Cc: star.zeng@intel.com, edk2-devel@lists.01.org Message-ID: <20160914130833.GB16080@bivouac.eciton.net> References: <5a0c21d0fe889950fda9e59aaf2272b66d429b8f.1473816993.git.feng.tian@intel.com> MIME-Version: 1.0 In-Reply-To: <5a0c21d0fe889950fda9e59aaf2272b66d429b8f.1473816993.git.feng.tian@intel.com> User-Agent: Mutt/1.5.23 (2014-03-12) 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: Wed, 14 Sep 2016 13:08:39 -0000 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline On Wed, Sep 14, 2016 at 09:37:13AM +0800, Feng Tian wrote: > Some XHCI host controllers require to have extra 1ms delay before > 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(-) > > diff --git a/MdeModulePkg/Bus/Pci/XhciDxe/XhciReg.c 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 @@ > > The XHCI register operation 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 > @@ -687,6 +687,10 @@ XhcResetHC ( > if ((Xhc->DebugCapSupOffset == 0xFFFFFFFF) || ((XhcReadExtCapReg (Xhc, Xhc->DebugCapSupOffset) & 0xFF) != XHC_CAP_USB_DEBUG) || > ((XhcReadExtCapReg (Xhc, Xhc->DebugCapSupOffset + XHC_DC_DCCTRL) & BIT0) == 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 second? 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, droppping a lot of duplication, gaining uniformity, and correcting spelling? / Leif > Status = XhcWaitOpRegBit (Xhc, XHC_USBCMD_OFFSET, XHC_USBCMD_RESET, FALSE, Timeout); > } > > -- > 2.7.1.windows.2 > > _______________________________________________ > edk2-devel mailing list > edk2-devel@lists.01.org > https://lists.01.org/mailman/listinfo/edk2-devel