* [PATCH v2 1/2] EmbeddedPkg/DwEmmcDxe: limit max clock for platform @ 2017-07-05 8:27 Jun Nie 2017-07-05 8:27 ` [PATCH v2 2/2] EmbeddedPkg/DwEmmc: Adjust FIFO threshold Jun Nie 2017-07-06 15:24 ` [PATCH v2 1/2] EmbeddedPkg/DwEmmcDxe: limit max clock for platform Leif Lindholm 0 siblings, 2 replies; 5+ messages in thread From: Jun Nie @ 2017-07-05 8:27 UTC (permalink / raw) To: ard.biesheuvel, leif.lindholm, haojian.zhuang, edk2-devel Cc: shawn.guo, jason.liu, Jun Nie Some boards may have max clock limitation. Add a Pcd to notify driver. Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Jun Nie <jun.nie@linaro.org> --- EmbeddedPkg/Drivers/DwEmmcDxe/DwEmmcDxe.c | 4 ++++ EmbeddedPkg/Drivers/DwEmmcDxe/DwEmmcDxe.inf | 1 + EmbeddedPkg/EmbeddedPkg.dec | 1 + 3 files changed, 6 insertions(+) diff --git a/EmbeddedPkg/Drivers/DwEmmcDxe/DwEmmcDxe.c b/EmbeddedPkg/Drivers/DwEmmcDxe/DwEmmcDxe.c index fe23d11..bb26b69 100644 --- a/EmbeddedPkg/Drivers/DwEmmcDxe/DwEmmcDxe.c +++ b/EmbeddedPkg/Drivers/DwEmmcDxe/DwEmmcDxe.c @@ -560,6 +560,10 @@ DwEmmcSetIos ( EFI_STATUS Status = EFI_SUCCESS; UINT32 Data; + if ((PcdGet32 (PcdDwEmmcDxeMaxClockFreqInHz) != 0) && + (BusClockFreq > PcdGet32 (PcdDwEmmcDxeMaxClockFreqInHz))) { + return EFI_UNSUPPORTED; + } if (TimingMode != EMMCBACKWARD) { Data = MmioRead32 (DWEMMC_UHSREG); switch (TimingMode) { diff --git a/EmbeddedPkg/Drivers/DwEmmcDxe/DwEmmcDxe.inf b/EmbeddedPkg/Drivers/DwEmmcDxe/DwEmmcDxe.inf index e3c8313..99b4f99 100644 --- a/EmbeddedPkg/Drivers/DwEmmcDxe/DwEmmcDxe.inf +++ b/EmbeddedPkg/Drivers/DwEmmcDxe/DwEmmcDxe.inf @@ -48,6 +48,7 @@ [Pcd] gEmbeddedTokenSpaceGuid.PcdDwEmmcDxeBaseAddress gEmbeddedTokenSpaceGuid.PcdDwEmmcDxeClockFrequencyInHz + gEmbeddedTokenSpaceGuid.PcdDwEmmcDxeMaxClockFreqInHz [Depex] TRUE diff --git a/EmbeddedPkg/EmbeddedPkg.dec b/EmbeddedPkg/EmbeddedPkg.dec index 5ea2f22..2da9b2f 100644 --- a/EmbeddedPkg/EmbeddedPkg.dec +++ b/EmbeddedPkg/EmbeddedPkg.dec @@ -169,6 +169,7 @@ # DwEmmc Driver PCDs gEmbeddedTokenSpaceGuid.PcdDwEmmcDxeBaseAddress|0x0|UINT32|0x00000035 gEmbeddedTokenSpaceGuid.PcdDwEmmcDxeClockFrequencyInHz|0x0|UINT32|0x00000036 + gEmbeddedTokenSpaceGuid.PcdDwEmmcDxeMaxClockFreqInHz|0x0|UINT32|0x00000037 # # Android FastBoot -- 1.9.1 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH v2 2/2] EmbeddedPkg/DwEmmc: Adjust FIFO threshold 2017-07-05 8:27 [PATCH v2 1/2] EmbeddedPkg/DwEmmcDxe: limit max clock for platform Jun Nie @ 2017-07-05 8:27 ` Jun Nie 2017-07-06 15:22 ` Leif Lindholm 2017-07-06 15:24 ` [PATCH v2 1/2] EmbeddedPkg/DwEmmcDxe: limit max clock for platform Leif Lindholm 1 sibling, 1 reply; 5+ messages in thread From: Jun Nie @ 2017-07-05 8:27 UTC (permalink / raw) To: ard.biesheuvel, leif.lindholm, haojian.zhuang, edk2-devel Cc: shawn.guo, jason.liu, Jun Nie Adjust FIFO threshold according to FIFO depth. Skip the adjustment if we do not have FIFO depth info. Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Jun Nie <jun.nie@linaro.org> --- 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 ( + 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; + UINT32 BlkSize = DWEMMC_BLOCK_SIZE, Idx = 0, RxWmark = 1, TxWmark, TxWmarkInvers; + + /* 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 */ + 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 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH v2 2/2] EmbeddedPkg/DwEmmc: Adjust FIFO threshold 2017-07-05 8:27 ` [PATCH v2 2/2] EmbeddedPkg/DwEmmc: Adjust FIFO threshold Jun Nie @ 2017-07-06 15:22 ` Leif Lindholm 2017-07-07 9:13 ` Jun Nie 0 siblings, 1 reply; 5+ messages in thread From: Leif Lindholm @ 2017-07-06 15:22 UTC (permalink / raw) To: Jun Nie; +Cc: ard.biesheuvel, haojian.zhuang, edk2-devel, shawn.guo, jason.liu 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 <jun.nie@linaro.org> > --- > 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 > ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2 2/2] EmbeddedPkg/DwEmmc: Adjust FIFO threshold 2017-07-06 15:22 ` Leif Lindholm @ 2017-07-07 9:13 ` Jun Nie 0 siblings, 0 replies; 5+ messages in thread From: Jun Nie @ 2017-07-07 9:13 UTC (permalink / raw) To: Leif Lindholm Cc: Ard Biesheuvel, Haojian Zhuang, edk2-devel, Shawn Guo, Jason Liu 2017-07-06 23:22 GMT+08:00 Leif Lindholm <leif.lindholm@linaro.org>: > 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 <jun.nie@linaro.org> >> --- >> 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. My fault. The style is totally different with kernel and I am not fully customized yet. > >> + >> + /* 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? You find the key point. As this driver is dedicated for EMMC, not shared with MMC, this check shall never be true. We can ignore it. Jun > > / > 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 >> ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2 1/2] EmbeddedPkg/DwEmmcDxe: limit max clock for platform 2017-07-05 8:27 [PATCH v2 1/2] EmbeddedPkg/DwEmmcDxe: limit max clock for platform Jun Nie 2017-07-05 8:27 ` [PATCH v2 2/2] EmbeddedPkg/DwEmmc: Adjust FIFO threshold Jun Nie @ 2017-07-06 15:24 ` Leif Lindholm 1 sibling, 0 replies; 5+ messages in thread From: Leif Lindholm @ 2017-07-06 15:24 UTC (permalink / raw) To: Jun Nie; +Cc: ard.biesheuvel, haojian.zhuang, edk2-devel, shawn.guo, jason.liu On Wed, Jul 05, 2017 at 04:27:07PM +0800, Jun Nie wrote: > Some boards may have max clock limitation. Add a Pcd to notify > driver. > > Contributed-under: TianoCore Contribution Agreement 1.0 > Signed-off-by: Jun Nie <jun.nie@linaro.org> Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org> (I'll hold off on pushing this one until 2/2 is ready. No need to resend this.) > --- > EmbeddedPkg/Drivers/DwEmmcDxe/DwEmmcDxe.c | 4 ++++ > EmbeddedPkg/Drivers/DwEmmcDxe/DwEmmcDxe.inf | 1 + > EmbeddedPkg/EmbeddedPkg.dec | 1 + > 3 files changed, 6 insertions(+) > > diff --git a/EmbeddedPkg/Drivers/DwEmmcDxe/DwEmmcDxe.c b/EmbeddedPkg/Drivers/DwEmmcDxe/DwEmmcDxe.c > index fe23d11..bb26b69 100644 > --- a/EmbeddedPkg/Drivers/DwEmmcDxe/DwEmmcDxe.c > +++ b/EmbeddedPkg/Drivers/DwEmmcDxe/DwEmmcDxe.c > @@ -560,6 +560,10 @@ DwEmmcSetIos ( > EFI_STATUS Status = EFI_SUCCESS; > UINT32 Data; > > + if ((PcdGet32 (PcdDwEmmcDxeMaxClockFreqInHz) != 0) && > + (BusClockFreq > PcdGet32 (PcdDwEmmcDxeMaxClockFreqInHz))) { > + return EFI_UNSUPPORTED; > + } > if (TimingMode != EMMCBACKWARD) { > Data = MmioRead32 (DWEMMC_UHSREG); > switch (TimingMode) { > diff --git a/EmbeddedPkg/Drivers/DwEmmcDxe/DwEmmcDxe.inf b/EmbeddedPkg/Drivers/DwEmmcDxe/DwEmmcDxe.inf > index e3c8313..99b4f99 100644 > --- a/EmbeddedPkg/Drivers/DwEmmcDxe/DwEmmcDxe.inf > +++ b/EmbeddedPkg/Drivers/DwEmmcDxe/DwEmmcDxe.inf > @@ -48,6 +48,7 @@ > [Pcd] > gEmbeddedTokenSpaceGuid.PcdDwEmmcDxeBaseAddress > gEmbeddedTokenSpaceGuid.PcdDwEmmcDxeClockFrequencyInHz > + gEmbeddedTokenSpaceGuid.PcdDwEmmcDxeMaxClockFreqInHz > > [Depex] > TRUE > diff --git a/EmbeddedPkg/EmbeddedPkg.dec b/EmbeddedPkg/EmbeddedPkg.dec > index 5ea2f22..2da9b2f 100644 > --- a/EmbeddedPkg/EmbeddedPkg.dec > +++ b/EmbeddedPkg/EmbeddedPkg.dec > @@ -169,6 +169,7 @@ > # DwEmmc Driver PCDs > gEmbeddedTokenSpaceGuid.PcdDwEmmcDxeBaseAddress|0x0|UINT32|0x00000035 > gEmbeddedTokenSpaceGuid.PcdDwEmmcDxeClockFrequencyInHz|0x0|UINT32|0x00000036 > + gEmbeddedTokenSpaceGuid.PcdDwEmmcDxeMaxClockFreqInHz|0x0|UINT32|0x00000037 > > # > # Android FastBoot > -- > 1.9.1 > ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2017-07-07 9:11 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2017-07-05 8:27 [PATCH v2 1/2] EmbeddedPkg/DwEmmcDxe: limit max clock for platform Jun Nie 2017-07-05 8:27 ` [PATCH v2 2/2] EmbeddedPkg/DwEmmc: Adjust FIFO threshold Jun Nie 2017-07-06 15:22 ` Leif Lindholm 2017-07-07 9:13 ` Jun Nie 2017-07-06 15:24 ` [PATCH v2 1/2] EmbeddedPkg/DwEmmcDxe: limit max clock for platform Leif Lindholm
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox