On 8/1/2024 10:14 PM, Leif Lindholm wrote:
On Thu, Aug 01, 2024 at 16:36:14 +0700, Nhi Pham via groups.io wrote:
This adds the SmbusRead() function designed for SMBUS transaction to
support the extraction of the data lenth byte from the initial byte of

length

I will fix, thanks.


the SMBUS Block Read, allowing the I2C master to accurately read the
SMBUS response with the exact data length. This addresses the issue
where the SmbusLib:SmBusReadBlock() function consistently reads a
32-byte block of data.

Doh, I managed to start reviewing on 2/5 instead 1/5.
Regardless, since this is new code, please update to controller/target
terminology to match current versions of protocol specifications,
throughout the set.

I will update, thanks.


Signed-off-by: Nhi Pham <n...@os.amperecomputing.com>
---
  Silicon/Ampere/AmpereAltraPkg/Include/Library/I2cLib.h    |  31 +++++
  Silicon/Ampere/AmpereAltraPkg/Library/DwI2cLib/DwI2cLib.c | 137 
+++++++++++++++++++-
  2 files changed, 163 insertions(+), 5 deletions(-)

diff --git a/Silicon/Ampere/AmpereAltraPkg/Include/Library/I2cLib.h 
b/Silicon/Ampere/AmpereAltraPkg/Include/Library/I2cLib.h
index f13794171029..d460f49efccb 100644
--- a/Silicon/Ampere/AmpereAltraPkg/Include/Library/I2cLib.h
+++ b/Silicon/Ampere/AmpereAltraPkg/Include/Library/I2cLib.h
@@ -65,6 +65,37 @@ I2cRead (
    IN OUT UINT32 *ReadLength
    );
+/**
+  SMBUS block read.
+
+  @param[in]     Bus          I2C bus Id.
+  @param[in]     SlaveAddr    The address of slave device on the bus.
+  @param[in]     BufCmd       Buffer where to send the command.
+  @param[in]     CmdLength    Length of BufCmd.
+  @param[in,out] Buf          Buffer where to put the read data to.
+  @param[in,out] ReadLength   Pointer to length of buffer.
+  @param[in]     PecCheck     If Packet Error Code (PEC) checking is required 
for this operation.
+
+  @retval EFI_SUCCESS            Read successfully.
+  @retval EFI_INVALID_PARAMETER  A parameter is invalid.
+  @retval EFI_UNSUPPORTED        The bus is not supported.
+  @retval EFI_NOT_READY          The device/bus is not ready.
+  @retval EFI_TIMEOUT            Timeout when transferring data.
+  @retval EFI_CRC_ERROR          There are errors on receiving data.
+
+**/
+EFI_STATUS
+EFIAPI
+SmbusRead (
+  IN     UINT32  Bus,
+  IN     UINT32  SlaveAddr,
+  IN     UINT8   *BufCmd,
+  IN     UINT32  CmdLength,
+  IN OUT UINT8   *Buf,
+  IN OUT UINT32  *ReadLength,
+  IN     BOOLEAN PecCheck
+  );
+
  /**
   Setup new transaction with I2C slave device.
diff --git a/Silicon/Ampere/AmpereAltraPkg/Library/DwI2cLib/DwI2cLib.c b/Silicon/Ampere/AmpereAltraPkg/Library/DwI2cLib/DwI2cLib.c
index 669ba2ea98a4..9e52ae69e7cd 100644
--- a/Silicon/Ampere/AmpereAltraPkg/Library/DwI2cLib/DwI2cLib.c
+++ b/Silicon/Ampere/AmpereAltraPkg/Library/DwI2cLib/DwI2cLib.c
@@ -337,6 +337,11 @@ I2cWaitTxData (
        DEBUG ((DEBUG_ERROR, "%a: Timeout waiting for TX buffer available\n", 
__FUNCTION__));
        return EFI_TIMEOUT;
      }
+
+    if ((I2cCheckErrors (Bus) & DW_IC_INTR_TX_ABRT) != 0) {
+      return EFI_ABORTED;
+    }
+
      MicroSecondDelay (mI2cBusList[Bus].PollingTime);
    }
@@ -542,13 +547,61 @@ InternalI2cWrite (
    return Status;
  }
+EFI_STATUS
+InternalSmbusReadDataLength (
+  UINT32  Bus,
+  UINT32 *Length
+  )
+{
+  EFI_STATUS Status;
+  UINTN      Base;
+  UINT32     CmdSend;
+
+  Base = mI2cBusList[Bus].Base;
+
+  CmdSend = DW_IC_DATA_CMD_CMD;
+  MmioWrite32 (Base + DW_IC_DATA_CMD, CmdSend);
+  I2cSync ();
+
+  if (I2cCheckErrors (Bus) != 0) {
+    DEBUG ((DEBUG_ERROR, "%a: Sending reading command error\n", __func__));
+    return EFI_CRC_ERROR;
+  }
+
+  Status = I2cWaitRxData (Bus);
+  if (EFI_ERROR (Status)) {
+    DEBUG ((DEBUG_ERROR,
+      "%a: Reading Smbus data length failed to wait data\n",
+      __func__
+      ));
+
+    if (Status != EFI_ABORTED) {
+      MmioWrite32 (Base + DW_IC_DATA_CMD, DW_IC_DATA_CMD_STOP);
+      I2cSync ();
+    }
+
+    return Status;
+  }
+
+  *Length = MmioRead32 (Base + DW_IC_DATA_CMD) & DW_IC_DATA_CMD_DAT_MASK;
+  I2cSync ();
+
+  if (I2cCheckErrors (Bus) != 0) {
+    DEBUG ((DEBUG_ERROR, "%a: Sending reading command error\n", __func__));
+    return EFI_CRC_ERROR;
+  }
+  return EFI_SUCCESS;
+}
+
  EFI_STATUS
  InternalI2cRead (
    UINT32  Bus,
-  UINT8  *BufCmd,
-  UINT32 CmdLength,
-  UINT8  *Buf,
-  UINT32 *Length
+  UINT8   *BufCmd,
+  UINT32  CmdLength,
+  UINT8   *Buf,
+  UINT32  *Length,
+  BOOLEAN IsSmbus,

Can this really change on a per-transaction basis?
If not, can it move to a state held in DW_I2C_CONTEXT_T?

+  BOOLEAN PecCheck

I guess the same question as above really.

/
     Leif

That makes sense, thanks Leif. I will pass IsSmbus and PecCheck in I2cProbe() and cache in the DW_I2C_CONTEXT_T.

Regards,
Nhi


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#120220): https://edk2.groups.io/g/devel/message/120220
Mute This Topic: https://groups.io/mt/107662232/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-


Reply via email to