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

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel