public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [platforms: PATCH v3 00/10] Armada 7k/8k - misc improvements pt.2
@ 2017-10-27 16:31 Marcin Wojtas
  2017-10-27 16:31 ` [platforms: PATCH v3 01/10] Marvell/Drivers: MvI2cDxe: Abort transaction immediately upon fail Marcin Wojtas
                   ` (10 more replies)
  0 siblings, 11 replies; 12+ messages in thread
From: Marcin Wojtas @ 2017-10-27 16:31 UTC (permalink / raw)
  To: edk2-devel
  Cc: leif.lindholm, ard.biesheuvel, nadavh, neta, kostap, jinghua, mw,
	jsd

Hi,

I send v3 with minor fixes, requested in latest review. Details
can be found in the changelog below.

The patches are available in the github:
https://github.com/MarvellEmbeddedProcessors/edk2-open-platform/commits/misc-upstream-r20171027-2

I'm looking forward to your comments or remarks.

Best regards,
Marcin

Changelog:
v2 -> v3:
7/10
  - simplify code by supporting the only real usecase (forcing link up)

9/10
  - print actual clock value in DumpCapabilityReg and inform about
    its mismatch with original register contents.
  - fix typo in functions' declarations

1/10, 2/10, 6/10, 8/10
  - add RBs

v1 -> v2:
1/10
  - remove unrelated style fix
  - fix style around modified functions calls

2/10
  - leave original EFI_SUCCESS assignment

6/10
  - use descriptively named temporary variable for pin index in a loop

7/10
  - use single flag for link up/down
  - simplify logic
  - correct style

8/10
  - mention missing SDR25 in a commit message

9/10
  - use new member of SD_MMC_HC_PRIVATE_DATA
    to set actual input clock speed and use it for the output
    clock configuration
  - rewrite commit message

3/10, 4/10, 5/10, 10/10
  - add RB's

Ard Biesheuvel (2):
  Marvell/Library: MppLib: Disable the stack protector
  Marvell/Library: MppLib: Take 0xFF placeholders into account

David Greeson (2):
  Marvell/Drivers: MvI2cDxe: Abort transaction immediately upon fail
  Marvell/Drivers: MvI2cDxe: Reduce bus occupation time

Joe Zhou (1):
  Marvell/Library: MppLib: Prevent overwriting PCD values

Marcin Wojtas (5):
  Marvell/Drivers: MvI2cDxe: Fix returning status in MvI2cStartRequest
  Marvell/Drivers: Pp2Dxe: Change settings for the always-up link
  Marvell/Drivers: XenonDxe: Fix UHS signalling mode setting
  Marvell/Drivers: XenonDxe: Allow overriding base clock frequency
  Marvell/Drivers: XenonDxe: Do not modify FIFO default values

 Platform/Marvell/Drivers/I2c/MvI2cDxe/MvI2cDxe.c        | 70 ++++++++++++--------
 Platform/Marvell/Drivers/I2c/MvI2cDxe/MvI2cDxe.h        |  2 +-
 Platform/Marvell/Drivers/Net/Pp2Dxe/Mvpp2Lib.c          | 19 ++++++
 Platform/Marvell/Drivers/Net/Pp2Dxe/Mvpp2Lib.h          |  5 ++
 Platform/Marvell/Drivers/Net/Pp2Dxe/Pp2Dxe.c            |  6 +-
 Platform/Marvell/Drivers/SdMmc/XenonDxe/EmmcDevice.c    |  6 +-
 Platform/Marvell/Drivers/SdMmc/XenonDxe/SdDevice.c      |  4 +-
 Platform/Marvell/Drivers/SdMmc/XenonDxe/SdMmcPciHcDxe.c | 15 +++--
 Platform/Marvell/Drivers/SdMmc/XenonDxe/SdMmcPciHcDxe.h |  6 ++
 Platform/Marvell/Drivers/SdMmc/XenonDxe/SdMmcPciHci.c   | 31 +++++----
 Platform/Marvell/Drivers/SdMmc/XenonDxe/SdMmcPciHci.h   | 16 +++--
 Platform/Marvell/Drivers/SdMmc/XenonDxe/XenonSdhci.c    | 16 -----
 Platform/Marvell/Library/MppLib/MppLib.c                | 35 +++++-----
 Platform/Marvell/Library/MppLib/MppLib.inf              |  3 +
 14 files changed, 144 insertions(+), 90 deletions(-)

-- 
2.7.4



^ permalink raw reply	[flat|nested] 12+ messages in thread

* [platforms: PATCH v3 01/10] Marvell/Drivers: MvI2cDxe: Abort transaction immediately upon fail
  2017-10-27 16:31 [platforms: PATCH v3 00/10] Armada 7k/8k - misc improvements pt.2 Marcin Wojtas
@ 2017-10-27 16:31 ` Marcin Wojtas
  2017-10-27 16:31 ` [platforms: PATCH v3 02/10] Marvell/Drivers: MvI2cDxe: Fix returning status in MvI2cStartRequest Marcin Wojtas
                   ` (9 subsequent siblings)
  10 siblings, 0 replies; 12+ messages in thread
From: Marcin Wojtas @ 2017-10-27 16:31 UTC (permalink / raw)
  To: edk2-devel
  Cc: leif.lindholm, ard.biesheuvel, nadavh, neta, kostap, jinghua, mw,
	jsd, David Greeson

From: David Greeson <dgreeson@cisco.com>

Although the I2C transaction routines were prepared to
return their status, they were never used. This could
cause bus lock-up e.g. in case of failing to send a
slave address, the data transfer was attempted to be
continued anyway.

This patch fixes faulty behavior by checking transaction
status and stopping it immediately, once the fail
is detected. On the occasion fix style around modified
functions calls.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: David Greeson <dgreeson@cisco.com>
[Style adjustment and cleanup]
Signed-off-by: Marcin Wojtas <mw@semihalf.com>
Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>
---
 Platform/Marvell/Drivers/I2c/MvI2cDxe/MvI2cDxe.c | 62 +++++++++++++-------
 1 file changed, 41 insertions(+), 21 deletions(-)

diff --git a/Platform/Marvell/Drivers/I2c/MvI2cDxe/MvI2cDxe.c b/Platform/Marvell/Drivers/I2c/MvI2cDxe/MvI2cDxe.c
index d85ee0b..b4599d2 100755
--- a/Platform/Marvell/Drivers/I2c/MvI2cDxe/MvI2cDxe.c
+++ b/Platform/Marvell/Drivers/I2c/MvI2cDxe/MvI2cDxe.c
@@ -565,6 +565,7 @@ MvI2cStartRequest (
   UINTN Transmitted;
   I2C_MASTER_CONTEXT *I2cMasterContext = I2C_SC_FROM_MASTER(This);
   EFI_I2C_OPERATION *Operation;
+  EFI_STATUS Status = EFI_SUCCESS;
 
   ASSERT (RequestPacket != NULL);
   ASSERT (I2cMasterContext != NULL);
@@ -574,33 +575,52 @@ MvI2cStartRequest (
     ReadMode = Operation->Flags & I2C_FLAG_READ;
 
     if (Count == 0) {
-      MvI2cStart ( I2cMasterContext,
-                   (SlaveAddress << 1) | ReadMode,
-                   I2C_TRANSFER_TIMEOUT
-                 );
+      Status = MvI2cStart (I2cMasterContext,
+                 (SlaveAddress << 1) | ReadMode,
+                 I2C_TRANSFER_TIMEOUT);
     } else if (!(Operation->Flags & I2C_FLAG_NORESTART)) {
-      MvI2cRepeatedStart ( I2cMasterContext,
-                           (SlaveAddress << 1) | ReadMode,
-                           I2C_TRANSFER_TIMEOUT
-                         );
+      Status = MvI2cRepeatedStart (I2cMasterContext,
+                 (SlaveAddress << 1) | ReadMode,
+                 I2C_TRANSFER_TIMEOUT);
     }
 
+    /* I2C transaction was aborted, so stop further transactions */
+    if (EFI_ERROR (Status)) {
+      MvI2cStop (I2cMasterContext);
+      break;
+    }
+
+    /*
+     * If sending the slave address was successful,
+     * proceed to read or write section.
+     */
     if (ReadMode) {
-      MvI2cRead ( I2cMasterContext,
-                  Operation->Buffer,
-                  Operation->LengthInBytes,
-                  &Transmitted,
-                  Count == 1,
-                  I2C_TRANSFER_TIMEOUT
-                 );
+      Status = MvI2cRead (I2cMasterContext,
+                 Operation->Buffer,
+                 Operation->LengthInBytes,
+                 &Transmitted,
+                 Count == 1,
+                 I2C_TRANSFER_TIMEOUT);
+      Operation->LengthInBytes = Transmitted;
     } else {
-      MvI2cWrite ( I2cMasterContext,
-                   Operation->Buffer,
-                   Operation->LengthInBytes,
-                   &Transmitted,
-                   I2C_TRANSFER_TIMEOUT
-                  );
+      Status = MvI2cWrite (I2cMasterContext,
+                 Operation->Buffer,
+                 Operation->LengthInBytes,
+                 &Transmitted,
+                 I2C_TRANSFER_TIMEOUT);
+      Operation->LengthInBytes = Transmitted;
     }
+
+    /*
+     * The I2C read or write transaction failed.
+     * Stop the I2C transaction.
+     */
+    if (EFI_ERROR (Status)) {
+      MvI2cStop (I2cMasterContext);
+      break;
+    }
+
+    /* Check if there is any more data to be sent */
     if (Count == RequestPacket->OperationCount - 1) {
       MvI2cStop ( I2cMasterContext );
     }
-- 
2.7.4



^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [platforms: PATCH v3 02/10] Marvell/Drivers: MvI2cDxe: Fix returning status in MvI2cStartRequest
  2017-10-27 16:31 [platforms: PATCH v3 00/10] Armada 7k/8k - misc improvements pt.2 Marcin Wojtas
  2017-10-27 16:31 ` [platforms: PATCH v3 01/10] Marvell/Drivers: MvI2cDxe: Abort transaction immediately upon fail Marcin Wojtas
@ 2017-10-27 16:31 ` Marcin Wojtas
  2017-10-27 16:31 ` [platforms: PATCH v3 03/10] Marvell/Drivers: MvI2cDxe: Reduce bus occupation time Marcin Wojtas
                   ` (8 subsequent siblings)
  10 siblings, 0 replies; 12+ messages in thread
From: Marcin Wojtas @ 2017-10-27 16:31 UTC (permalink / raw)
  To: edk2-devel
  Cc: leif.lindholm, ard.biesheuvel, nadavh, neta, kostap, jinghua, mw,
	jsd

In MvI2cStartRequest the status was assigned to the variable
without dereferencing a pointer. Fix it.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Marcin Wojtas <mw@semihalf.com>
Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>
---
 Platform/Marvell/Drivers/I2c/MvI2cDxe/MvI2cDxe.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Platform/Marvell/Drivers/I2c/MvI2cDxe/MvI2cDxe.c b/Platform/Marvell/Drivers/I2c/MvI2cDxe/MvI2cDxe.c
index b4599d2..a62383f 100755
--- a/Platform/Marvell/Drivers/I2c/MvI2cDxe/MvI2cDxe.c
+++ b/Platform/Marvell/Drivers/I2c/MvI2cDxe/MvI2cDxe.c
@@ -627,7 +627,7 @@ MvI2cStartRequest (
   }
 
   if (I2cStatus != NULL)
-    I2cStatus = EFI_SUCCESS;
+    *I2cStatus = EFI_SUCCESS;
   if (Event != NULL)
     gBS->SignalEvent(Event);
   return EFI_SUCCESS;
-- 
2.7.4



^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [platforms: PATCH v3 03/10] Marvell/Drivers: MvI2cDxe: Reduce bus occupation time
  2017-10-27 16:31 [platforms: PATCH v3 00/10] Armada 7k/8k - misc improvements pt.2 Marcin Wojtas
  2017-10-27 16:31 ` [platforms: PATCH v3 01/10] Marvell/Drivers: MvI2cDxe: Abort transaction immediately upon fail Marcin Wojtas
  2017-10-27 16:31 ` [platforms: PATCH v3 02/10] Marvell/Drivers: MvI2cDxe: Fix returning status in MvI2cStartRequest Marcin Wojtas
@ 2017-10-27 16:31 ` Marcin Wojtas
  2017-10-27 16:31 ` [platforms: PATCH v3 04/10] Marvell/Library: MppLib: Prevent overwriting PCD values Marcin Wojtas
                   ` (7 subsequent siblings)
  10 siblings, 0 replies; 12+ messages in thread
From: Marcin Wojtas @ 2017-10-27 16:31 UTC (permalink / raw)
  To: edk2-devel
  Cc: leif.lindholm, ard.biesheuvel, nadavh, neta, kostap, jinghua, mw,
	jsd, David Greeson

From: David Greeson <dgreeson@cisco.com>

During each transaction start, clearing the I2C_CONTROL_FLAG
was surrounded by 3 uncoditional stalls. This was not necessary,
so replace them with one busy-wait loop, whose polling
count could be also safely reduced.

Above improvements result in faster transfer initialization
and allow to reduce the I2C bus occupation.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: David Greeson <dgreeson@cisco.com>
Signed-off-by: Marcin Wojtas <mw@semihalf.com>
Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>
---
 Platform/Marvell/Drivers/I2c/MvI2cDxe/MvI2cDxe.c | 6 +-----
 Platform/Marvell/Drivers/I2c/MvI2cDxe/MvI2cDxe.h | 2 +-
 2 files changed, 2 insertions(+), 6 deletions(-)

diff --git a/Platform/Marvell/Drivers/I2c/MvI2cDxe/MvI2cDxe.c b/Platform/Marvell/Drivers/I2c/MvI2cDxe/MvI2cDxe.c
index a62383f..d6f590d 100755
--- a/Platform/Marvell/Drivers/I2c/MvI2cDxe/MvI2cDxe.c
+++ b/Platform/Marvell/Drivers/I2c/MvI2cDxe/MvI2cDxe.c
@@ -243,9 +243,8 @@ MvI2cClearIflg (
  IN I2C_MASTER_CONTEXT *I2cMasterContext
  )
 {
-  gBS->Stall(I2C_OPERATION_TIMEOUT);
+  MvI2cPollCtrl (I2cMasterContext, I2C_OPERATION_TIMEOUT, I2C_CONTROL_IFLG);
   MvI2cControlClear(I2cMasterContext, I2C_CONTROL_IFLG);
-  gBS->Stall(I2C_OPERATION_TIMEOUT);
 }
 
 /* Timeout is given in us */
@@ -295,9 +294,6 @@ MvI2cLockedStart (
     MvI2cClearIflg(I2cMasterContext);
   }
 
-  /* Without this delay we Timeout checking IFLG if the Timeout is 0 */
-  gBS->Stall(I2C_OPERATION_TIMEOUT);
-
   if (MvI2cPollCtrl(I2cMasterContext, Timeout, I2C_CONTROL_IFLG)) {
     DEBUG((DEBUG_ERROR, "MvI2cDxe: Timeout sending %sSTART condition\n",
         Mask == I2C_STATUS_START ? "" : "repeated "));
diff --git a/Platform/Marvell/Drivers/I2c/MvI2cDxe/MvI2cDxe.h b/Platform/Marvell/Drivers/I2c/MvI2cDxe/MvI2cDxe.h
index 028fd54..3c9beaf 100644
--- a/Platform/Marvell/Drivers/I2c/MvI2cDxe/MvI2cDxe.h
+++ b/Platform/Marvell/Drivers/I2c/MvI2cDxe/MvI2cDxe.h
@@ -68,7 +68,7 @@ SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 
 #define I2C_SOFT_RESET    0x1c
 #define I2C_TRANSFER_TIMEOUT 10000
-#define I2C_OPERATION_TIMEOUT 1000
+#define I2C_OPERATION_TIMEOUT 100
 
 #define I2C_UNKNOWN        0x0
 #define I2C_SLOW           0x1
-- 
2.7.4



^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [platforms: PATCH v3 04/10] Marvell/Library: MppLib: Prevent overwriting PCD values
  2017-10-27 16:31 [platforms: PATCH v3 00/10] Armada 7k/8k - misc improvements pt.2 Marcin Wojtas
                   ` (2 preceding siblings ...)
  2017-10-27 16:31 ` [platforms: PATCH v3 03/10] Marvell/Drivers: MvI2cDxe: Reduce bus occupation time Marcin Wojtas
@ 2017-10-27 16:31 ` Marcin Wojtas
  2017-10-27 16:31 ` [platforms: PATCH v3 05/10] Marvell/Library: MppLib: Disable the stack protector Marcin Wojtas
                   ` (6 subsequent siblings)
  10 siblings, 0 replies; 12+ messages in thread
From: Marcin Wojtas @ 2017-10-27 16:31 UTC (permalink / raw)
  To: edk2-devel
  Cc: leif.lindholm, ard.biesheuvel, nadavh, neta, kostap, jinghua, mw,
	jsd, Joe Zhou

From: Joe Zhou <shjzhou@marvell.com>

After enabling dynamic PCDs, it is possible to reconfigure
MPP during platform initialization. It occurred that due to
a faulty way of passing temporary values, information obtained
from PCDs was overwritten. This patch fixes the issue, which
on the occasion simplifies PcdToMppRegs function.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Joe Zhou <shjzhou@marvell.com>
Signed-off-by: Marcin Wojtas <mw@semihalf.com>
Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>
---
 Platform/Marvell/Library/MppLib/MppLib.c | 21 ++++++++------------
 1 file changed, 8 insertions(+), 13 deletions(-)

diff --git a/Platform/Marvell/Library/MppLib/MppLib.c b/Platform/Marvell/Library/MppLib/MppLib.c
index c09acf9..383c820 100644
--- a/Platform/Marvell/Library/MppLib/MppLib.c
+++ b/Platform/Marvell/Library/MppLib/MppLib.c
@@ -74,7 +74,7 @@ STATIC
 VOID
 SetRegisterValue (
   UINT8 RegCount,
-  UINT8 **MppRegPcd,
+  UINT8 MppRegPcd[][MPP_PINS_PER_REG],
   UINTN BaseAddr,
   BOOLEAN ReverseFlag
   )
@@ -99,10 +99,10 @@ STATIC
 UINT8
 PcdToMppRegs (
   UINTN PinCount,
-  UINT8 **MppRegPcd
+  UINT8 **MppRegPcd,
+  UINT8 MppRegPcdTmp[][MPP_PINS_PER_REG]
   )
 {
-  UINT8 MppRegPcdTmp[MPP_MAX_REGS][MPP_PINS_PER_REG];
   UINT8 PcdGroupCount, MppRegCount;
   UINTN i, j, k, l;
 
@@ -125,14 +125,7 @@ PcdToMppRegs (
     for (j = 0; j < PCD_PINS_PER_GROUP; j++) {
       k = (PCD_PINS_PER_GROUP * i + j) / MPP_PINS_PER_REG;
       l = (PCD_PINS_PER_GROUP * i + j) % MPP_PINS_PER_REG;
-      MppRegPcdTmp[k][l] = MppRegPcd[i][j];
-    }
-  }
-
-  /* Update input table */
-  for (i = 0; i < MppRegCount; i++) {
-    for (j = 0; j < MPP_PINS_PER_REG; j++) {
-      MppRegPcd[i][j] = MppRegPcdTmp[i][j];
+      MppRegPcdTmp[k][l] = (UINT8)MppRegPcd[i][j];
     }
   }
 
@@ -191,6 +184,7 @@ MppInitialize (
   BOOLEAN ReverseFlag[MAX_CHIPS];
   UINT8 *MppRegPcd[MAX_CHIPS][MPP_MAX_REGS];
   UINT32 i, ChipCount;
+  UINT8 TmpMppValue[MPP_MAX_REGS][MPP_PINS_PER_REG];
 
   ChipCount = PcdGet32 (PcdMppChipCount);
 
@@ -203,8 +197,9 @@ MppInitialize (
   for (i = 0; i < MAX_CHIPS; i++) {
     if (i == ChipCount)
       break;
-    RegCount = PcdToMppRegs (PinCount[i], MppRegPcd[i]);
-    SetRegisterValue (RegCount, MppRegPcd[i], BaseAddr[i], ReverseFlag[i]);
+
+    RegCount = PcdToMppRegs (PinCount[i], MppRegPcd[i], TmpMppValue);
+    SetRegisterValue (RegCount, TmpMppValue, BaseAddr[i], ReverseFlag[i]);
 
     /*
      * eMMC PHY IP has its own MPP configuration.
-- 
2.7.4



^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [platforms: PATCH v3 05/10] Marvell/Library: MppLib: Disable the stack protector
  2017-10-27 16:31 [platforms: PATCH v3 00/10] Armada 7k/8k - misc improvements pt.2 Marcin Wojtas
                   ` (3 preceding siblings ...)
  2017-10-27 16:31 ` [platforms: PATCH v3 04/10] Marvell/Library: MppLib: Prevent overwriting PCD values Marcin Wojtas
@ 2017-10-27 16:31 ` Marcin Wojtas
  2017-10-27 16:31 ` [platforms: PATCH v3 06/10] Marvell/Library: MppLib: Take 0xFF placeholders into account Marcin Wojtas
                   ` (5 subsequent siblings)
  10 siblings, 0 replies; 12+ messages in thread
From: Marcin Wojtas @ 2017-10-27 16:31 UTC (permalink / raw)
  To: edk2-devel
  Cc: leif.lindholm, ard.biesheuvel, nadavh, neta, kostap, jinghua, mw,
	jsd

From: Ard Biesheuvel <ard.biesheuvel@linaro.org>

MppLib may be used very early (in SEC), at which point stack protection
measures are more likely to cause harm than help, given that not even
the UART has been configured to the point where we can complain usefully.
So just disable it.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Signed-off-by: Marcin Wojtas <mw@semihalf.com>
Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>
---
 Platform/Marvell/Library/MppLib/MppLib.inf | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/Platform/Marvell/Library/MppLib/MppLib.inf b/Platform/Marvell/Library/MppLib/MppLib.inf
index 2de9cd0..1268542 100644
--- a/Platform/Marvell/Library/MppLib/MppLib.inf
+++ b/Platform/Marvell/Library/MppLib/MppLib.inf
@@ -106,3 +106,6 @@
   gMarvellTokenSpaceGuid.PcdChip3MppSel7
 
   gMarvellTokenSpaceGuid.PcdPciESdhci
+
+[BuildOptions]
+  *_*_*_CC_FLAGS = -fno-stack-protector
-- 
2.7.4



^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [platforms: PATCH v3 06/10] Marvell/Library: MppLib: Take 0xFF placeholders into account
  2017-10-27 16:31 [platforms: PATCH v3 00/10] Armada 7k/8k - misc improvements pt.2 Marcin Wojtas
                   ` (4 preceding siblings ...)
  2017-10-27 16:31 ` [platforms: PATCH v3 05/10] Marvell/Library: MppLib: Disable the stack protector Marcin Wojtas
@ 2017-10-27 16:31 ` Marcin Wojtas
  2017-10-27 16:31 ` [platforms: PATCH v3 07/10] Marvell/Drivers: Pp2Dxe: Change settings for the always-up link Marcin Wojtas
                   ` (4 subsequent siblings)
  10 siblings, 0 replies; 12+ messages in thread
From: Marcin Wojtas @ 2017-10-27 16:31 UTC (permalink / raw)
  To: edk2-devel
  Cc: leif.lindholm, ard.biesheuvel, nadavh, neta, kostap, jinghua, mw,
	jsd

From: Ard Biesheuvel <ard.biesheuvel@linaro.org>

The MppSel definition PCDs contain 0xFF placeholders for values that
should be left untouched. MppLib needs to be taught how to take those
into account.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Signed-off-by: Marcin Wojtas <mw@semihalf.com>
Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>
---
 Platform/Marvell/Library/MppLib/MppLib.c | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/Platform/Marvell/Library/MppLib/MppLib.c b/Platform/Marvell/Library/MppLib/MppLib.c
index 383c820..297725f 100644
--- a/Platform/Marvell/Library/MppLib/MppLib.c
+++ b/Platform/Marvell/Library/MppLib/MppLib.c
@@ -79,18 +79,24 @@ SetRegisterValue (
   BOOLEAN ReverseFlag
   )
 {
-  UINT32 i, j, CtrlVal;
+  UINT32 i, j, CtrlVal, CtrlMask, PinIndex;
   INTN Sign;
 
   Sign = ReverseFlag ? -1 : 1;
 
   for (i = 0; i < RegCount; i++) {
     CtrlVal = 0;
+    CtrlMask = 0;
     for (j = 0; j < MPP_PINS_PER_REG; j++) {
-      CtrlVal |= MPP_PIN_VAL(7 * (UINTN) ReverseFlag + j * Sign,
-        MppRegPcd[i][7 * (UINTN) ReverseFlag + j * Sign]);
+
+      PinIndex = 7 * (UINTN)ReverseFlag + j * Sign;
+
+      if (MppRegPcd[i][PinIndex] != 0xff) {
+        CtrlVal |= MPP_PIN_VAL(PinIndex, MppRegPcd[i][PinIndex]);
+        CtrlMask |= MPP_PIN_VAL(PinIndex, 0xf);
+      }
     }
-    MmioWrite32 (BaseAddr + 4 * i * Sign, CtrlVal);
+    MmioAndThenOr32 (BaseAddr + 4 * i * Sign, ~CtrlMask, CtrlVal);
   }
 }
 
-- 
2.7.4



^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [platforms: PATCH v3 07/10] Marvell/Drivers: Pp2Dxe: Change settings for the always-up link
  2017-10-27 16:31 [platforms: PATCH v3 00/10] Armada 7k/8k - misc improvements pt.2 Marcin Wojtas
                   ` (5 preceding siblings ...)
  2017-10-27 16:31 ` [platforms: PATCH v3 06/10] Marvell/Library: MppLib: Take 0xFF placeholders into account Marcin Wojtas
@ 2017-10-27 16:31 ` Marcin Wojtas
  2017-10-27 16:31 ` [platforms: PATCH v3 08/10] Marvell/Drivers: XenonDxe: Fix UHS signalling mode setting Marcin Wojtas
                   ` (3 subsequent siblings)
  10 siblings, 0 replies; 12+ messages in thread
From: Marcin Wojtas @ 2017-10-27 16:31 UTC (permalink / raw)
  To: edk2-devel
  Cc: leif.lindholm, ard.biesheuvel, nadavh, neta, kostap, jinghua, mw,
	jsd

Currently initial forcing link status happened for all ports, not only
marked as 'always-up'. Although this didn't actually matter for the MAC
settings, because MAC is automatically updated with PHY HW polling
feature of the controller, perform mv_gop110_fl_cfg only when
the appropriate flag is true. Also in such case, force the link as up,
using a new library routine.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Marcin Wojtas <mw@semihalf.com>
---
 Platform/Marvell/Drivers/Net/Pp2Dxe/Mvpp2Lib.c | 19 +++++++++++++++++++
 Platform/Marvell/Drivers/Net/Pp2Dxe/Mvpp2Lib.h |  5 +++++
 Platform/Marvell/Drivers/Net/Pp2Dxe/Pp2Dxe.c   |  6 +++++-
 3 files changed, 29 insertions(+), 1 deletion(-)

diff --git a/Platform/Marvell/Drivers/Net/Pp2Dxe/Mvpp2Lib.c b/Platform/Marvell/Drivers/Net/Pp2Dxe/Mvpp2Lib.c
index 53154db..0c9f00c 100644
--- a/Platform/Marvell/Drivers/Net/Pp2Dxe/Mvpp2Lib.c
+++ b/Platform/Marvell/Drivers/Net/Pp2Dxe/Mvpp2Lib.c
@@ -4804,6 +4804,25 @@ MvGop110PortEventsMask (
   return 0;
 }
 
+/*
+ * Sets "Force Link Pass" and clears "Do Not Force Link Fail" bits.
+ * This function should only be called when the port is disabled.
+ */
+VOID
+MvGop110GmacForceLinkUp (
+  IN PP2DXE_PORT *Port
+  )
+{
+  UINT32 RegVal;
+
+  RegVal = MvGop110GmacRead (Port, MVPP2_PORT_AUTO_NEG_CFG_REG);
+
+  RegVal |= MVPP2_PORT_AUTO_NEG_CFG_FORCE_LINK_UP_MASK;
+  RegVal &= ~MVPP2_PORT_AUTO_NEG_CFG_FORCE_LINK_DOWN_MASK;
+
+  MvGop110GmacWrite (Port, MVPP2_PORT_AUTO_NEG_CFG_REG, RegVal);
+}
+
 INT32
 MvGop110FlCfg (
   IN PP2DXE_PORT *Port
diff --git a/Platform/Marvell/Drivers/Net/Pp2Dxe/Mvpp2Lib.h b/Platform/Marvell/Drivers/Net/Pp2Dxe/Mvpp2Lib.h
index a7011f7..3dc9ecd 100644
--- a/Platform/Marvell/Drivers/Net/Pp2Dxe/Mvpp2Lib.h
+++ b/Platform/Marvell/Drivers/Net/Pp2Dxe/Mvpp2Lib.h
@@ -504,6 +504,11 @@ MvGop110XlgPortLinkEventMask (
   IN PP2DXE_PORT *Port
   );
 
+VOID
+MvGop110GmacForceLinkUp (
+  IN PP2DXE_PORT *Port
+  );
+
 INT32
 MvGop110FlCfg (
   IN PP2DXE_PORT *Port
diff --git a/Platform/Marvell/Drivers/Net/Pp2Dxe/Pp2Dxe.c b/Platform/Marvell/Drivers/Net/Pp2Dxe/Pp2Dxe.c
index 2827976..b0a38b3 100644
--- a/Platform/Marvell/Drivers/Net/Pp2Dxe/Pp2Dxe.c
+++ b/Platform/Marvell/Drivers/Net/Pp2Dxe/Pp2Dxe.c
@@ -1310,7 +1310,11 @@ Pp2DxeInitialiseController (
     NetCompConfig |= MvpPp2xGop110NetcCfgCreate(&Pp2Context->Port);
 
     MvGop110PortInit(&Pp2Context->Port);
-    MvGop110FlCfg(&Pp2Context->Port);
+
+    if (Pp2Context->Port.AlwaysUp == TRUE) {
+      MvGop110GmacForceLinkUp (&Pp2Context->Port);
+      MvGop110FlCfg (&Pp2Context->Port);
+    }
 
     Status = gBS->CreateEvent (
                  EVT_SIGNAL_EXIT_BOOT_SERVICES,
-- 
2.7.4



^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [platforms: PATCH v3 08/10] Marvell/Drivers: XenonDxe: Fix UHS signalling mode setting
  2017-10-27 16:31 [platforms: PATCH v3 00/10] Armada 7k/8k - misc improvements pt.2 Marcin Wojtas
                   ` (6 preceding siblings ...)
  2017-10-27 16:31 ` [platforms: PATCH v3 07/10] Marvell/Drivers: Pp2Dxe: Change settings for the always-up link Marcin Wojtas
@ 2017-10-27 16:31 ` Marcin Wojtas
  2017-10-27 16:31 ` [platforms: PATCH v3 09/10] Marvell/Drivers: XenonDxe: Allow overriding base clock frequency Marcin Wojtas
                   ` (2 subsequent siblings)
  10 siblings, 0 replies; 12+ messages in thread
From: Marcin Wojtas @ 2017-10-27 16:31 UTC (permalink / raw)
  To: edk2-devel
  Cc: leif.lindholm, ard.biesheuvel, nadavh, neta, kostap, jinghua, mw,
	jsd

This patch fixes incorrect settings for UHS mode in
SD_MMC_HC_HOST_CTRL2 register for SDR50 and SDR25, of which
the latter was missing. This field should be set to:

0x4 for DDR52
0x2 for SDR50
0x1 for SDR25
0x0 for others.

This way EmmcSwitchToHighSpeed function is on par with Linux
set_uhs_signaling routine in the Xenon driver.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Marcin Wojtas <mw@semihalf.com>
Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>
---
 Platform/Marvell/Drivers/SdMmc/XenonDxe/EmmcDevice.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Platform/Marvell/Drivers/SdMmc/XenonDxe/EmmcDevice.c b/Platform/Marvell/Drivers/SdMmc/XenonDxe/EmmcDevice.c
index 3f73194..4d4833f 100755
--- a/Platform/Marvell/Drivers/SdMmc/XenonDxe/EmmcDevice.c
+++ b/Platform/Marvell/Drivers/SdMmc/XenonDxe/EmmcDevice.c
@@ -772,6 +772,8 @@ EmmcSwitchToHighSpeed (
   if (IsDdr) {
     HostCtrl2 = BIT2;
   } else if (ClockFreq == 52) {
+    HostCtrl2 = BIT1;
+  } else if (ClockFreq == 26) {
     HostCtrl2 = BIT0;
   } else {
     HostCtrl2 = 0;
-- 
2.7.4



^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [platforms: PATCH v3 09/10] Marvell/Drivers: XenonDxe: Allow overriding base clock frequency
  2017-10-27 16:31 [platforms: PATCH v3 00/10] Armada 7k/8k - misc improvements pt.2 Marcin Wojtas
                   ` (7 preceding siblings ...)
  2017-10-27 16:31 ` [platforms: PATCH v3 08/10] Marvell/Drivers: XenonDxe: Fix UHS signalling mode setting Marcin Wojtas
@ 2017-10-27 16:31 ` Marcin Wojtas
  2017-10-27 16:31 ` [platforms: PATCH v3 10/10] Marvell/Drivers: XenonDxe: Do not modify FIFO default values Marcin Wojtas
  2017-10-29 16:51 ` [platforms: PATCH v3 00/10] Armada 7k/8k - misc improvements pt.2 Leif Lindholm
  10 siblings, 0 replies; 12+ messages in thread
From: Marcin Wojtas @ 2017-10-27 16:31 UTC (permalink / raw)
  To: edk2-devel
  Cc: leif.lindholm, ard.biesheuvel, nadavh, neta, kostap, jinghua, mw,
	jsd

Some SdMmc host controllers are run by clocks with different
frequency than it is reflected in Capabilities Register 1.
Because the bitfield is only 8 bits wide, a maximum value
that could be obtained from hardware is 255(MHz).

In case the actual frequency exceeds 255MHz, the 8-bit BaseClkFreq
member of SD_MMC_HC_SLOT_CAP structure occurs to be not sufficient
to be used for setting the clock speed in SdMmcHcClockSupply
function.

This patch adds new UINT32 array ('BaseClkFreq[]') to
SD_MMC_HC_PRIVATE_DATA structure for specifying
the input clock speed for each slot of the host controller.
All routines that are used for clock configuration are
updated accordingly.

Thanks to above the Xenon host controller driver
could be modified to configure clock speed relatively
to actual 400MHz input.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Marcin Wojtas <mw@semihalf.com>
---
 Platform/Marvell/Drivers/SdMmc/XenonDxe/EmmcDevice.c    |  4 +--
 Platform/Marvell/Drivers/SdMmc/XenonDxe/SdDevice.c      |  4 +--
 Platform/Marvell/Drivers/SdMmc/XenonDxe/SdMmcPciHcDxe.c | 15 ++++++----
 Platform/Marvell/Drivers/SdMmc/XenonDxe/SdMmcPciHcDxe.h |  6 ++++
 Platform/Marvell/Drivers/SdMmc/XenonDxe/SdMmcPciHci.c   | 31 ++++++++++++--------
 Platform/Marvell/Drivers/SdMmc/XenonDxe/SdMmcPciHci.h   | 16 ++++++----
 6 files changed, 48 insertions(+), 28 deletions(-)

diff --git a/Platform/Marvell/Drivers/SdMmc/XenonDxe/EmmcDevice.c b/Platform/Marvell/Drivers/SdMmc/XenonDxe/EmmcDevice.c
index 4d4833f..530a01c 100755
--- a/Platform/Marvell/Drivers/SdMmc/XenonDxe/EmmcDevice.c
+++ b/Platform/Marvell/Drivers/SdMmc/XenonDxe/EmmcDevice.c
@@ -705,7 +705,7 @@ EmmcSwitchClockFreq (
   //
   // Convert the clock freq unit from MHz to KHz.
   //
-  Status = SdMmcHcClockSupply (PciIo, Slot, ClockFreq * 1000, Private->Capability[Slot]);
+  Status = SdMmcHcClockSupply (PciIo, Slot, ClockFreq * 1000, Private->BaseClkFreq[Slot]);
 
   return Status;
 }
@@ -1007,7 +1007,7 @@ EmmcSetBusMode (
     return Status;
   }
 
-  ASSERT (Private->Capability[Slot].BaseClkFreq != 0);
+  ASSERT (Private->BaseClkFreq[Slot] != 0);
   //
   // Check if the Host Controller support 8bits bus width.
   //
diff --git a/Platform/Marvell/Drivers/SdMmc/XenonDxe/SdDevice.c b/Platform/Marvell/Drivers/SdMmc/XenonDxe/SdDevice.c
index 9122848..ea7eed7 100644
--- a/Platform/Marvell/Drivers/SdMmc/XenonDxe/SdDevice.c
+++ b/Platform/Marvell/Drivers/SdMmc/XenonDxe/SdDevice.c
@@ -972,7 +972,7 @@ SdCardSetBusMode (
     return Status;
   }
 
-  Status = SdMmcHcClockSupply (PciIo, Slot, ClockFreq * 1000, *Capability);
+  Status = SdMmcHcClockSupply (PciIo, Slot, ClockFreq * 1000, Private->BaseClkFreq[Slot]);
   if (EFI_ERROR (Status)) {
     return Status;
   }
@@ -1144,7 +1144,7 @@ SdCardIdentification (
         goto Error;
       }
 
-      SdMmcHcInitClockFreq (PciIo, Slot, Private->Capability[Slot]);
+      SdMmcHcInitClockFreq (PciIo, Slot, Private->BaseClkFreq[Slot]);
 
       gBS->Stall (1000);
 
diff --git a/Platform/Marvell/Drivers/SdMmc/XenonDxe/SdMmcPciHcDxe.c b/Platform/Marvell/Drivers/SdMmc/XenonDxe/SdMmcPciHcDxe.c
index 981eab5..80159a4 100644
--- a/Platform/Marvell/Drivers/SdMmc/XenonDxe/SdMmcPciHcDxe.c
+++ b/Platform/Marvell/Drivers/SdMmc/XenonDxe/SdMmcPciHcDxe.c
@@ -291,7 +291,10 @@ SdMmcPciHcEnumerateDevice (
         //
         // Reinitialize slot and restart identification process for the new attached device
         //
-        Status = SdMmcHcInitHost (Private->PciIo, Slot, Private->Capability[Slot]);
+        Status = SdMmcHcInitHost (Private->PciIo,
+                   Slot,
+                   Private->Capability[Slot],
+                   Private->BaseClkFreq[Slot]);
         if (EFI_ERROR (Status)) {
           continue;
         }
@@ -617,11 +620,13 @@ SdMmcPciHcDriverBindingStart (
   Private->Capability[Slot].Sdr50 = 0;
   Private->Capability[Slot].BusWidth8 = 0;
 
-  if (Private->Capability[Slot].BaseClkFreq == 0) {
-    Private->Capability[Slot].BaseClkFreq = 0xff;
-  }
+  //
+  // Override inappropriate base clock frequency from Capabilities Register 1.
+  // Actual clock speed of Xenon controller is 400MHz.
+  //
+  Private->BaseClkFreq[Slot] = XENON_MMC_MAX_CLK / 1000 / 1000;
 
-  DumpCapabilityReg (Slot, &Private->Capability[Slot]);
+  DumpCapabilityReg (Slot, &Private->Capability[Slot], Private->BaseClkFreq[Slot]);
 
   Status = SdMmcHcGetMaxCurrent (PciIo, Slot, &Private->MaxCurrent[Slot]);
   if (EFI_ERROR (Status)) {
diff --git a/Platform/Marvell/Drivers/SdMmc/XenonDxe/SdMmcPciHcDxe.h b/Platform/Marvell/Drivers/SdMmc/XenonDxe/SdMmcPciHcDxe.h
index 6a2a279..067b9ac 100644
--- a/Platform/Marvell/Drivers/SdMmc/XenonDxe/SdMmcPciHcDxe.h
+++ b/Platform/Marvell/Drivers/SdMmc/XenonDxe/SdMmcPciHcDxe.h
@@ -115,6 +115,12 @@ typedef struct {
   UINT64                              MaxCurrent[SD_MMC_HC_MAX_SLOT];
 
   UINT32                              ControllerVersion;
+
+  //
+  // Some controllers may require to override base clock frequency
+  // value stored in Capabilities Register 1.
+  //
+  UINT32                              BaseClkFreq[SD_MMC_HC_MAX_SLOT];
 } SD_MMC_HC_PRIVATE_DATA;
 
 #define SD_MMC_HC_TRB_SIG             SIGNATURE_32 ('T', 'R', 'B', 'T')
diff --git a/Platform/Marvell/Drivers/SdMmc/XenonDxe/SdMmcPciHci.c b/Platform/Marvell/Drivers/SdMmc/XenonDxe/SdMmcPciHci.c
index ccbf355..1f4abd1 100644
--- a/Platform/Marvell/Drivers/SdMmc/XenonDxe/SdMmcPciHci.c
+++ b/Platform/Marvell/Drivers/SdMmc/XenonDxe/SdMmcPciHci.c
@@ -22,12 +22,14 @@
 
   @param[in]  Slot            The slot number of the SD card to send the command to.
   @param[in]  Capability      The buffer to store the capability data.
+  @param[in]  BaseClkFreq     The base clock frequency of host controller in MHz.
 
 **/
 VOID
 DumpCapabilityReg (
   IN UINT8                Slot,
-  IN SD_MMC_HC_SLOT_CAP   *Capability
+  IN SD_MMC_HC_SLOT_CAP   *Capability,
+  IN UINT32               BaseClkFreq
   )
 {
   //
@@ -35,7 +37,10 @@ DumpCapabilityReg (
   //
   DEBUG ((DEBUG_INFO, " == Slot [%d] Capability is 0x%x ==\n", Slot, Capability));
   DEBUG ((DEBUG_INFO, "   Timeout Clk Freq  %d%a\n", Capability->TimeoutFreq, (Capability->TimeoutUnit) ? "MHz" : "KHz"));
-  DEBUG ((DEBUG_INFO, "   Base Clk Freq     %dMHz\n", Capability->BaseClkFreq));
+  if (Capability->BaseClkFreq != BaseClkFreq) {
+    DEBUG ((DEBUG_INFO, "   Controller register value overriden:\n"));
+  }
+  DEBUG ((DEBUG_INFO, "   Base Clk Freq     %dMHz\n", BaseClkFreq));
   DEBUG ((DEBUG_INFO, "   Max Blk Len       %dbytes\n", 512 * (1 << Capability->MaxBlkLen)));
   DEBUG ((DEBUG_INFO, "   8-bit Support     %a\n", Capability->BusWidth8 ? "TRUE" : "FALSE"));
   DEBUG ((DEBUG_INFO, "   ADMA2 Support     %a\n", Capability->Adma2 ? "TRUE" : "FALSE"));
@@ -678,7 +683,7 @@ SdMmcHcStopClock (
   @param[in] PciIo          The PCI IO protocol instance.
   @param[in] Slot           The slot number of the SD card to send the command to.
   @param[in] ClockFreq      The max clock frequency to be set. The unit is KHz.
-  @param[in] Capability     The capability of the slot.
+  @param[in] BaseClkFreq    The base clock frequency of host controller in MHz.
 
   @retval EFI_SUCCESS       The clock is supplied successfully.
   @retval Others            The clock isn't supplied successfully.
@@ -689,11 +694,10 @@ SdMmcHcClockSupply (
   IN EFI_PCI_IO_PROTOCOL    *PciIo,
   IN UINT8                  Slot,
   IN UINT64                 ClockFreq,
-  IN SD_MMC_HC_SLOT_CAP     Capability
+  IN UINT32                 BaseClkFreq
   )
 {
   EFI_STATUS                Status;
-  UINT32                    BaseClkFreq;
   UINT32                    SettingFreq;
   UINT32                    Divisor;
   UINT32                    Remainder;
@@ -703,9 +707,8 @@ SdMmcHcClockSupply (
   //
   // Calculate a divisor for SD clock frequency
   //
-  ASSERT (Capability.BaseClkFreq != 0);
+  ASSERT (BaseClkFreq != 0);
 
-  BaseClkFreq = Capability.BaseClkFreq;
   if (ClockFreq == 0) {
     return EFI_INVALID_PARAMETER;
   }
@@ -896,7 +899,7 @@ SdMmcHcSetBusWidth (
 
   @param[in] PciIo          The PCI IO protocol instance.
   @param[in] Slot           The slot number of the SD card to send the command to.
-  @param[in] Capability     The capability of the slot.
+  @param[in] BaseClkFreq    The base clock frequency of host controller in MHz.
 
   @retval EFI_SUCCESS       The clock is supplied successfully.
   @retval Others            The clock isn't supplied successfully.
@@ -906,7 +909,7 @@ EFI_STATUS
 SdMmcHcInitClockFreq (
   IN EFI_PCI_IO_PROTOCOL    *PciIo,
   IN UINT8                  Slot,
-  IN SD_MMC_HC_SLOT_CAP     Capability
+  IN UINT32                 BaseClkFreq
   )
 {
   EFI_STATUS                Status;
@@ -915,7 +918,7 @@ SdMmcHcInitClockFreq (
   //
   // Calculate a divisor for SD clock frequency
   //
-  if (Capability.BaseClkFreq == 0) {
+  if (BaseClkFreq == 0) {
     //
     // Don't support get Base Clock Frequency information via another method
     //
@@ -925,7 +928,7 @@ SdMmcHcInitClockFreq (
   // Supply 400KHz clock frequency at initialization phase.
   //
   InitFreq = 400;
-  Status = SdMmcHcClockSupply (PciIo, Slot, InitFreq, Capability);
+  Status = SdMmcHcClockSupply (PciIo, Slot, InitFreq, BaseClkFreq);
   return Status;
 }
 
@@ -1024,6 +1027,7 @@ SdMmcHcInitTimeoutCtrl (
   @param[in] PciIo          The PCI IO protocol instance.
   @param[in] Slot           The slot number of the SD card to send the command to.
   @param[in] Capability     The capability of the slot.
+  @param[in] BaseClkFreq    The base clock frequency of host controller in MHz.
 
   @retval EFI_SUCCESS       The host controller is initialized successfully.
   @retval Others            The host controller isn't initialized successfully.
@@ -1033,12 +1037,13 @@ EFI_STATUS
 SdMmcHcInitHost (
   IN EFI_PCI_IO_PROTOCOL    *PciIo,
   IN UINT8                  Slot,
-  IN SD_MMC_HC_SLOT_CAP     Capability
+  IN SD_MMC_HC_SLOT_CAP     Capability,
+  IN UINT32                 BaseClkFreq
   )
 {
   EFI_STATUS       Status;
 
-  Status = SdMmcHcInitClockFreq (PciIo, Slot, Capability);
+  Status = SdMmcHcInitClockFreq (PciIo, Slot, BaseClkFreq);
   if (EFI_ERROR (Status)) {
     return Status;
   }
diff --git a/Platform/Marvell/Drivers/SdMmc/XenonDxe/SdMmcPciHci.h b/Platform/Marvell/Drivers/SdMmc/XenonDxe/SdMmcPciHci.h
index fb62758..533f37c 100644
--- a/Platform/Marvell/Drivers/SdMmc/XenonDxe/SdMmcPciHci.h
+++ b/Platform/Marvell/Drivers/SdMmc/XenonDxe/SdMmcPciHci.h
@@ -140,12 +140,14 @@ typedef struct {
 
   @param[in]  Slot            The slot number of the SD card to send the command to.
   @param[in]  Capability      The buffer to store the capability data.
+  @param[in]  BaseClkFreq     The base clock frequency of host controller in MHz.
 
 **/
 VOID
 DumpCapabilityReg (
   IN UINT8                Slot,
-  IN SD_MMC_HC_SLOT_CAP   *Capability
+  IN SD_MMC_HC_SLOT_CAP   *Capability,
+  IN UINT32               BaseClkFreq
   );
 
 /**
@@ -414,7 +416,7 @@ SdMmcHcStopClock (
   @param[in] PciIo          The PCI IO protocol instance.
   @param[in] Slot           The slot number of the SD card to send the command to.
   @param[in] ClockFreq      The max clock frequency to be set. The unit is KHz.
-  @param[in] Capability     The capability of the slot.
+  @param[in] BaseClkFreq    The base clock frequency of host controller in MHz.
 
   @retval EFI_SUCCESS       The clock is supplied successfully.
   @retval Others            The clock isn't supplied successfully.
@@ -425,7 +427,7 @@ SdMmcHcClockSupply (
   IN EFI_PCI_IO_PROTOCOL    *PciIo,
   IN UINT8                  Slot,
   IN UINT64                 ClockFreq,
-  IN SD_MMC_HC_SLOT_CAP     Capability
+  IN UINT32                 BaseClkFreq
   );
 
 /**
@@ -473,7 +475,7 @@ SdMmcHcSetBusWidth (
 
   @param[in] PciIo          The PCI IO protocol instance.
   @param[in] Slot           The slot number of the SD card to send the command to.
-  @param[in] Capability     The capability of the slot.
+  @param[in] BaseClkFreq    The base clock frequency of host controller in MHz.
 
   @retval EFI_SUCCESS       The clock is supplied successfully.
   @retval Others            The clock isn't supplied successfully.
@@ -483,7 +485,7 @@ EFI_STATUS
 SdMmcHcInitClockFreq (
   IN EFI_PCI_IO_PROTOCOL    *PciIo,
   IN UINT8                  Slot,
-  IN SD_MMC_HC_SLOT_CAP     Capability
+  IN UINT32                 BaseClkFreq
   );
 
 /**
@@ -531,6 +533,7 @@ SdMmcHcInitTimeoutCtrl (
   @param[in] PciIo          The PCI IO protocol instance.
   @param[in] Slot           The slot number of the SD card to send the command to.
   @param[in] Capability     The capability of the slot.
+  @param[in] BaseClkFreq    The base clock frequency of host controller in MHz.
 
   @retval EFI_SUCCESS       The host controller is initialized successfully.
   @retval Others            The host controller isn't initialized successfully.
@@ -540,7 +543,8 @@ EFI_STATUS
 SdMmcHcInitHost (
   IN EFI_PCI_IO_PROTOCOL    *PciIo,
   IN UINT8                  Slot,
-  IN SD_MMC_HC_SLOT_CAP     Capability
+  IN SD_MMC_HC_SLOT_CAP     Capability,
+  IN UINT32                 BaseClkFreq
   );
 
 #endif
-- 
2.7.4



^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [platforms: PATCH v3 10/10] Marvell/Drivers: XenonDxe: Do not modify FIFO default values
  2017-10-27 16:31 [platforms: PATCH v3 00/10] Armada 7k/8k - misc improvements pt.2 Marcin Wojtas
                   ` (8 preceding siblings ...)
  2017-10-27 16:31 ` [platforms: PATCH v3 09/10] Marvell/Drivers: XenonDxe: Allow overriding base clock frequency Marcin Wojtas
@ 2017-10-27 16:31 ` Marcin Wojtas
  2017-10-29 16:51 ` [platforms: PATCH v3 00/10] Armada 7k/8k - misc improvements pt.2 Leif Lindholm
  10 siblings, 0 replies; 12+ messages in thread
From: Marcin Wojtas @ 2017-10-27 16:31 UTC (permalink / raw)
  To: edk2-devel
  Cc: leif.lindholm, ard.biesheuvel, nadavh, neta, kostap, jinghua, mw,
	jsd

Changing controller's FIFO default values is not necessary and
possibly can cause instabilities, when using some devices.
Disable the modification and rely on initial settings.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Marcin Wojtas <mw@semihalf.com>
Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>
---
 Platform/Marvell/Drivers/SdMmc/XenonDxe/XenonSdhci.c | 16 ----------------
 1 file changed, 16 deletions(-)

diff --git a/Platform/Marvell/Drivers/SdMmc/XenonDxe/XenonSdhci.c b/Platform/Marvell/Drivers/SdMmc/XenonDxe/XenonSdhci.c
index 31f207e..6bbe5bc 100755
--- a/Platform/Marvell/Drivers/SdMmc/XenonDxe/XenonSdhci.c
+++ b/Platform/Marvell/Drivers/SdMmc/XenonDxe/XenonSdhci.c
@@ -44,20 +44,6 @@ XenonReadVersion (
   SdMmcHcRwMmio (PciIo, SD_BAR_INDEX, SD_MMC_HC_CTRL_VER, TRUE, SDHC_REG_SIZE_2B, ControllerVersion);
 }
 
-STATIC
-VOID
-XenonSetFifo (
-  IN EFI_PCI_IO_PROTOCOL   *PciIo
-  )
-{
-  UINTN Data;
-
-  // Set FIFO_RTC, FIFO_WTC, FIFO_CS and FIFO_PDLVMC
-  Data = SDHC_SLOT_FIFO_DEFAULT_CONFIG;
-
-  SdMmcHcRwMmio (PciIo, SD_BAR_INDEX, SDHC_SLOT_FIFO_CTRL, FALSE, SDHC_REG_SIZE_4B, &Data);
-}
-
 // Auto Clock Gating
 STATIC
 VOID
@@ -634,8 +620,6 @@ XenonInit (
   // Read XENON version
   XenonReadVersion (PciIo, &Private->ControllerVersion);
 
-  XenonSetFifo (PciIo);
-
   // Disable auto clock generator
   XenonSetAcg (PciIo, FALSE);
 
-- 
2.7.4



^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [platforms: PATCH v3 00/10] Armada 7k/8k - misc improvements pt.2
  2017-10-27 16:31 [platforms: PATCH v3 00/10] Armada 7k/8k - misc improvements pt.2 Marcin Wojtas
                   ` (9 preceding siblings ...)
  2017-10-27 16:31 ` [platforms: PATCH v3 10/10] Marvell/Drivers: XenonDxe: Do not modify FIFO default values Marcin Wojtas
@ 2017-10-29 16:51 ` Leif Lindholm
  10 siblings, 0 replies; 12+ messages in thread
From: Leif Lindholm @ 2017-10-29 16:51 UTC (permalink / raw)
  To: Marcin Wojtas
  Cc: edk2-devel, ard.biesheuvel, nadavh, neta, kostap, jinghua, jsd

On Fri, Oct 27, 2017 at 06:31:43PM +0200, Marcin Wojtas wrote:
> Hi,
> 
> I send v3 with minor fixes, requested in latest review. Details
> can be found in the changelog below.
> 
> The patches are available in the github:
> https://github.com/MarvellEmbeddedProcessors/edk2-open-platform/commits/misc-upstream-r20171027-2
> 
> I'm looking forward to your comments or remarks.
> 
> Best regards,
> Marcin

Remaining patches:
Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>

Series pushed as 5fc9a86fb3..f79bce44ac.

/
    Leif

> Changelog:
> v2 -> v3:
> 7/10
>   - simplify code by supporting the only real usecase (forcing link up)
> 
> 9/10
>   - print actual clock value in DumpCapabilityReg and inform about
>     its mismatch with original register contents.
>   - fix typo in functions' declarations
> 
> 1/10, 2/10, 6/10, 8/10
>   - add RBs
> 
> v1 -> v2:
> 1/10
>   - remove unrelated style fix
>   - fix style around modified functions calls
> 
> 2/10
>   - leave original EFI_SUCCESS assignment
> 
> 6/10
>   - use descriptively named temporary variable for pin index in a loop
> 
> 7/10
>   - use single flag for link up/down
>   - simplify logic
>   - correct style
> 
> 8/10
>   - mention missing SDR25 in a commit message
> 
> 9/10
>   - use new member of SD_MMC_HC_PRIVATE_DATA
>     to set actual input clock speed and use it for the output
>     clock configuration
>   - rewrite commit message
> 
> 3/10, 4/10, 5/10, 10/10
>   - add RB's
> 
> Ard Biesheuvel (2):
>   Marvell/Library: MppLib: Disable the stack protector
>   Marvell/Library: MppLib: Take 0xFF placeholders into account
> 
> David Greeson (2):
>   Marvell/Drivers: MvI2cDxe: Abort transaction immediately upon fail
>   Marvell/Drivers: MvI2cDxe: Reduce bus occupation time
> 
> Joe Zhou (1):
>   Marvell/Library: MppLib: Prevent overwriting PCD values
> 
> Marcin Wojtas (5):
>   Marvell/Drivers: MvI2cDxe: Fix returning status in MvI2cStartRequest
>   Marvell/Drivers: Pp2Dxe: Change settings for the always-up link
>   Marvell/Drivers: XenonDxe: Fix UHS signalling mode setting
>   Marvell/Drivers: XenonDxe: Allow overriding base clock frequency
>   Marvell/Drivers: XenonDxe: Do not modify FIFO default values
> 
>  Platform/Marvell/Drivers/I2c/MvI2cDxe/MvI2cDxe.c        | 70 ++++++++++++--------
>  Platform/Marvell/Drivers/I2c/MvI2cDxe/MvI2cDxe.h        |  2 +-
>  Platform/Marvell/Drivers/Net/Pp2Dxe/Mvpp2Lib.c          | 19 ++++++
>  Platform/Marvell/Drivers/Net/Pp2Dxe/Mvpp2Lib.h          |  5 ++
>  Platform/Marvell/Drivers/Net/Pp2Dxe/Pp2Dxe.c            |  6 +-
>  Platform/Marvell/Drivers/SdMmc/XenonDxe/EmmcDevice.c    |  6 +-
>  Platform/Marvell/Drivers/SdMmc/XenonDxe/SdDevice.c      |  4 +-
>  Platform/Marvell/Drivers/SdMmc/XenonDxe/SdMmcPciHcDxe.c | 15 +++--
>  Platform/Marvell/Drivers/SdMmc/XenonDxe/SdMmcPciHcDxe.h |  6 ++
>  Platform/Marvell/Drivers/SdMmc/XenonDxe/SdMmcPciHci.c   | 31 +++++----
>  Platform/Marvell/Drivers/SdMmc/XenonDxe/SdMmcPciHci.h   | 16 +++--
>  Platform/Marvell/Drivers/SdMmc/XenonDxe/XenonSdhci.c    | 16 -----
>  Platform/Marvell/Library/MppLib/MppLib.c                | 35 +++++-----
>  Platform/Marvell/Library/MppLib/MppLib.inf              |  3 +
>  14 files changed, 144 insertions(+), 90 deletions(-)
> 
> -- 
> 2.7.4
> 


^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2017-10-29 16:47 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-10-27 16:31 [platforms: PATCH v3 00/10] Armada 7k/8k - misc improvements pt.2 Marcin Wojtas
2017-10-27 16:31 ` [platforms: PATCH v3 01/10] Marvell/Drivers: MvI2cDxe: Abort transaction immediately upon fail Marcin Wojtas
2017-10-27 16:31 ` [platforms: PATCH v3 02/10] Marvell/Drivers: MvI2cDxe: Fix returning status in MvI2cStartRequest Marcin Wojtas
2017-10-27 16:31 ` [platforms: PATCH v3 03/10] Marvell/Drivers: MvI2cDxe: Reduce bus occupation time Marcin Wojtas
2017-10-27 16:31 ` [platforms: PATCH v3 04/10] Marvell/Library: MppLib: Prevent overwriting PCD values Marcin Wojtas
2017-10-27 16:31 ` [platforms: PATCH v3 05/10] Marvell/Library: MppLib: Disable the stack protector Marcin Wojtas
2017-10-27 16:31 ` [platforms: PATCH v3 06/10] Marvell/Library: MppLib: Take 0xFF placeholders into account Marcin Wojtas
2017-10-27 16:31 ` [platforms: PATCH v3 07/10] Marvell/Drivers: Pp2Dxe: Change settings for the always-up link Marcin Wojtas
2017-10-27 16:31 ` [platforms: PATCH v3 08/10] Marvell/Drivers: XenonDxe: Fix UHS signalling mode setting Marcin Wojtas
2017-10-27 16:31 ` [platforms: PATCH v3 09/10] Marvell/Drivers: XenonDxe: Allow overriding base clock frequency Marcin Wojtas
2017-10-27 16:31 ` [platforms: PATCH v3 10/10] Marvell/Drivers: XenonDxe: Do not modify FIFO default values Marcin Wojtas
2017-10-29 16:51 ` [platforms: PATCH v3 00/10] Armada 7k/8k - misc improvements pt.2 Leif Lindholm

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox