From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wr0-x22e.google.com (mail-wr0-x22e.google.com [IPv6:2a00:1450:400c:c0c::22e]) (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 D3ACA21CE73F4 for ; Thu, 6 Jul 2017 08:20:55 -0700 (PDT) Received: by mail-wr0-x22e.google.com with SMTP id k67so7285169wrc.2 for ; Thu, 06 Jul 2017 08:22:36 -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=Jee1NZn7ZgTLWaR3nLJuENb93S6Hnm/YK9QEdq9zdhw=; b=b/atT9b7RYKPP/ORj4dhqPsjt9rgbJ36PrTW0atzrlg6iS5K0w6yk+BoBmmzC1411q FCy5w9w6PWwsBuFfs3I7cQNSoZn3l1AEZuVge+G67MMuNSBZHzUBewlFNJI6XCMh9lfV pWRkE+8QCvbGYZtOntztFuLgPyv5yRxTISOZA= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to:user-agent; bh=Jee1NZn7ZgTLWaR3nLJuENb93S6Hnm/YK9QEdq9zdhw=; b=SeEJnaY/sOhbQejw6F7IGO6SOhzzC8cjksuIQ6HNo0+tos7G4EUbVfR3YQ/dqJeQDj H3wmj5zQbnNn/FrtHCXcSHw6/OLz7PEuG9MXdsbhepeLqLxXD+hLO0l5ZqIgR31X9Y3p yBYrR/EzCQrOL7kplQf1DooZKw2nI27kfbPyEQO0bRyZaTCU5dokEtd+P9kUIiuOf59U gro6ugUEpqj8EqlcPbO6qUpTa8UgI7pWihLNIs/8wNIt7lN7OGRgI/g9O2LcBuQWZKh1 7tYCkupQKPSxTNCILZXmXgNXpwHQTXrbTKo/0HE5rsmkYS9nsGSxubifrlXfIyrJpo6t aqtA== X-Gm-Message-State: AKS2vOyPN9LyPZD2N8h5C9swpHb31RLLUP5hPYHiy71hkw5nTZfIJecU WKbzbMCiuWJVVGr5 X-Received: by 10.223.130.36 with SMTP id 33mr47248531wrb.111.1499354554761; Thu, 06 Jul 2017 08:22:34 -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 u80sm494187wrb.1.2017.07.06.08.22.33 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Thu, 06 Jul 2017 08:22:34 -0700 (PDT) Date: Thu, 6 Jul 2017 16:22:32 +0100 From: Leif Lindholm To: Jun Nie Cc: ard.biesheuvel@linaro.org, haojian.zhuang@linaro.org, edk2-devel@lists.01.org, shawn.guo@linaro.org, jason.liu@linaro.org Message-ID: <20170706152232.GQ26676@bivouac.eciton.net> References: <1499243228-1225-1-git-send-email-jun.nie@linaro.org> <1499243228-1225-2-git-send-email-jun.nie@linaro.org> MIME-Version: 1.0 In-Reply-To: <1499243228-1225-2-git-send-email-jun.nie@linaro.org> User-Agent: Mutt/1.5.23 (2014-03-12) Subject: Re: [PATCH v2 2/2] EmbeddedPkg/DwEmmc: Adjust FIFO threshold X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.22 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 06 Jul 2017 15:20:56 -0000 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline On Wed, Jul 05, 2017 at 04:27:08PM +0800, Jun Nie wrote: > Adjust FIFO threshold according to FIFO depth. Skip > the adjustment if we do not have FIFO depth info. > So, this is a big improvement in readability - but some of my generic style comments do not appear to have been addressed. > Contributed-under: TianoCore Contribution Agreement 1.0 > Signed-off-by: Jun Nie > --- > EmbeddedPkg/Drivers/DwEmmcDxe/DwEmmc.h | 6 ++++ > EmbeddedPkg/Drivers/DwEmmcDxe/DwEmmcDxe.c | 50 +++++++++++++++++++++++++++++ > EmbeddedPkg/Drivers/DwEmmcDxe/DwEmmcDxe.inf | 1 + > EmbeddedPkg/EmbeddedPkg.dec | 1 + > 4 files changed, 58 insertions(+) > > diff --git a/EmbeddedPkg/Drivers/DwEmmcDxe/DwEmmc.h b/EmbeddedPkg/Drivers/DwEmmcDxe/DwEmmc.h > index 055f1e0..90c7676 100644 > --- a/EmbeddedPkg/Drivers/DwEmmcDxe/DwEmmc.h > +++ b/EmbeddedPkg/Drivers/DwEmmcDxe/DwEmmc.h > @@ -38,7 +38,10 @@ > #define DWEMMC_RINTSTS ((UINT32)PcdGet32 (PcdDwEmmcDxeBaseAddress) + 0x044) > #define DWEMMC_STATUS ((UINT32)PcdGet32 (PcdDwEmmcDxeBaseAddress) + 0x048) > #define DWEMMC_FIFOTH ((UINT32)PcdGet32 (PcdDwEmmcDxeBaseAddress) + 0x04c) > +#define DWEMMC_TCBCNT ((UINT32)PcdGet32 (PcdDwEmmcDxeBaseAddress) + 0x05c) > +#define DWEMMC_TBBCNT ((UINT32)PcdGet32 (PcdDwEmmcDxeBaseAddress) + 0x060) > #define DWEMMC_DEBNCE ((UINT32)PcdGet32 (PcdDwEmmcDxeBaseAddress) + 0x064) > +#define DWEMMC_HCON ((UINT32)PcdGet32 (PcdDwEmmcDxeBaseAddress) + 0x070) > #define DWEMMC_UHSREG ((UINT32)PcdGet32 (PcdDwEmmcDxeBaseAddress) + 0x074) > #define DWEMMC_BMOD ((UINT32)PcdGet32 (PcdDwEmmcDxeBaseAddress) + 0x080) > #define DWEMMC_DBADDR ((UINT32)PcdGet32 (PcdDwEmmcDxeBaseAddress) + 0x088) > @@ -47,6 +50,7 @@ > #define DWEMMC_DSCADDR ((UINT32)PcdGet32 (PcdDwEmmcDxeBaseAddress) + 0x094) > #define DWEMMC_BUFADDR ((UINT32)PcdGet32 (PcdDwEmmcDxeBaseAddress) + 0x098) > #define DWEMMC_CARDTHRCTL ((UINT32)PcdGet32 (PcdDwEmmcDxeBaseAddress) + 0X100) > +#define DWEMMC_DATA ((UINT32)PcdGet32 (PcdDwEmmcDxeBaseAddress) + 0X200) > > #define CMD_UPDATE_CLK 0x80202000 > #define CMD_START_BIT (1 << 31) > @@ -124,4 +128,6 @@ > #define DWEMMC_CARD_RD_THR(x) ((x & 0xfff) << 16) > #define DWEMMC_CARD_RD_THR_EN (1 << 0) > > +#define DWEMMC_GET_HDATA_WIDTH(x) (((x) >> 7) & 0x7) > + > #endif // __DWEMMC_H__ > diff --git a/EmbeddedPkg/Drivers/DwEmmcDxe/DwEmmcDxe.c b/EmbeddedPkg/Drivers/DwEmmcDxe/DwEmmcDxe.c > index bb26b69..70e064d 100644 > --- a/EmbeddedPkg/Drivers/DwEmmcDxe/DwEmmcDxe.c > +++ b/EmbeddedPkg/Drivers/DwEmmcDxe/DwEmmcDxe.c > @@ -415,6 +415,55 @@ DwEmmcReceiveResponse ( > return EFI_SUCCESS; > } > > +VOID DwEmmcAdjustFifothreshold ( This still needs to be: VOID DwEmmcAdjustFifoThreshold ( > + VOID > + ) > +{ > + /* DMA multiple transaction size map to reg value as array index */ > + CONST UINT32 BurstSize[] = {1, 4, 8, 16, 32, 64, 128, 256}; > + UINT32 BlkDepthInFifo, Fifoth, FifoWidth, FifoDepth; Fifoth -> FifoThreshold. > + UINT32 BlkSize = DWEMMC_BLOCK_SIZE, Idx = 0, RxWmark = 1, TxWmark, TxWmarkInvers; RxWmark -> RxWaterMark. TxWmark -> TxWaterMark. TxWmarkInvers -> TxWaterMarkInverse. > + > + /* Skip FIFO adjustment if we do not have platform FIFO depth info */ > + FifoDepth = PcdGet32 (PcdDwEmmcDxeFifoDepth); > + if (!FifoDepth) { > + return; > + } > + > + TxWmark = FifoDepth / 2; > + TxWmarkInvers = FifoDepth - TxWmark; > + > + FifoWidth = DWEMMC_GET_HDATA_WIDTH (MmioRead32 (DWEMMC_HCON)); > + if (!FifoWidth) { > + FifoWidth = 2; > + } else if (FifoWidth == 2) { > + FifoWidth = 8; > + } else { > + FifoWidth = 4; > + } > + > + BlkDepthInFifo = BlkSize / FifoWidth; > + > + /* if BlkSize is not a multiple of the FIFO width */ This comment still only says exactly what the code says. Why are we done if BlkSize is not a multiple of FifoWidth? Is this situation possible? What does it indicate? / Leif > + if (BlkSize % FifoWidth) { > + goto done; > + } > + > + Idx = ARRAY_SIZE (BurstSize) - 1; > + while (Idx && ((BlkDepthInFifo % BurstSize[Idx]) || (TxWmarkInvers % BurstSize[Idx]))) { > + Idx--; > + } > + RxWmark = BurstSize[Idx] - 1; > + /* > + * If Idx is '0', it won't be tried > + * Thus, initial values are used > + */ > +done: > + Fifoth = DWEMMC_DMA_BURST_SIZE (Idx) | DWEMMC_FIFO_TWMARK (TxWmark) > + | DWEMMC_FIFO_RWMARK (RxWmark); > + MmioWrite32 (DWEMMC_FIFOTH, Fifoth); > +} > + > EFI_STATUS > PrepareDmaData ( > IN DWEMMC_IDMAC_DESCRIPTOR* IdmacDesc, > @@ -633,6 +682,7 @@ DwEmmcDxeInitialize ( > > Handle = NULL; > > + DwEmmcAdjustFifothreshold (); > gpIdmacDesc = (DWEMMC_IDMAC_DESCRIPTOR *)AllocatePages (DWEMMC_MAX_DESC_PAGES); > if (gpIdmacDesc == NULL) { > return EFI_BUFFER_TOO_SMALL; > diff --git a/EmbeddedPkg/Drivers/DwEmmcDxe/DwEmmcDxe.inf b/EmbeddedPkg/Drivers/DwEmmcDxe/DwEmmcDxe.inf > index 99b4f99..bc4413e 100644 > --- a/EmbeddedPkg/Drivers/DwEmmcDxe/DwEmmcDxe.inf > +++ b/EmbeddedPkg/Drivers/DwEmmcDxe/DwEmmcDxe.inf > @@ -49,6 +49,7 @@ > gEmbeddedTokenSpaceGuid.PcdDwEmmcDxeBaseAddress > gEmbeddedTokenSpaceGuid.PcdDwEmmcDxeClockFrequencyInHz > gEmbeddedTokenSpaceGuid.PcdDwEmmcDxeMaxClockFreqInHz > + gEmbeddedTokenSpaceGuid.PcdDwEmmcDxeFifoDepth > > [Depex] > TRUE > diff --git a/EmbeddedPkg/EmbeddedPkg.dec b/EmbeddedPkg/EmbeddedPkg.dec > index 2da9b2f..5f39d9d 100644 > --- a/EmbeddedPkg/EmbeddedPkg.dec > +++ b/EmbeddedPkg/EmbeddedPkg.dec > @@ -170,6 +170,7 @@ > gEmbeddedTokenSpaceGuid.PcdDwEmmcDxeBaseAddress|0x0|UINT32|0x00000035 > gEmbeddedTokenSpaceGuid.PcdDwEmmcDxeClockFrequencyInHz|0x0|UINT32|0x00000036 > gEmbeddedTokenSpaceGuid.PcdDwEmmcDxeMaxClockFreqInHz|0x0|UINT32|0x00000037 > + gEmbeddedTokenSpaceGuid.PcdDwEmmcDxeFifoDepth|0x0|UINT32|0x00000038 > > # > # Android FastBoot > -- > 1.9.1 >