* [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 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
* 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
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