From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail05.groups.io (mail05.groups.io [45.79.224.7]) by spool.mail.gandi.net (Postfix) with ESMTPS id 5A95F740034 for ; Thu, 1 Aug 2024 15:18:50 +0000 (UTC) DKIM-Signature: a=rsa-sha256; bh=EqruU+Yy/Lui2nF8d0Fxpz/gq+UjJg2b4QP9MzhreP8=; c=relaxed/simple; d=groups.io; h=Date:From:To:CC:Subject:Message-ID:References:MIME-Version:In-Reply-To:Precedence:List-Subscribe:List-Help:Sender:List-Id:Mailing-List:Delivered-To:Resent-Date:Resent-From:Reply-To:List-Unsubscribe-Post:List-Unsubscribe:Content-Type:Content-Disposition; s=20240206; t=1722525530; v=1; b=1FfpFEiOUFK1VWmvqV0uDJeGLxwnWbiTefTx1rJ05h9qbDTGDYecSQzHVVSd2914qv7vc59s /dv1c+MtVsdJClDzR+8AKmyH0ECN33njfMIENLU50z/tu7tV/vnEMOFI5nWmH8PMYeQUnaoh47q wCv+HnbGvHWxZ5Dw5QESpb9bH3RN7nzLsfmgHh+6shW7emmLt9JiFk87QHLeNebr/sfiN3pjwmJ g6ilaNa5bkukh7caZgr5UZI1cgk87JiFwkQnrXH39/G1FbVQcml2ep++SFAGMwb/inUd7gWSvte 2rfZhrDnNoO1ToqA5XhMYBKw5PRJFVK0ISpKrgAvAgUeg== X-Received: by 127.0.0.2 with SMTP id mVQ2YY7687511xIPpSvgaNg4; Thu, 01 Aug 2024 08:18:48 -0700 X-Received: from mx0a-0031df01.pphosted.com (mx0a-0031df01.pphosted.com [205.220.168.131]) by mx.groups.io with SMTP id smtpd.web10.71085.1722525528335823150 for ; Thu, 01 Aug 2024 08:18:48 -0700 X-Received: from pps.filterd (m0279864.ppops.net [127.0.0.1]) by mx0a-0031df01.pphosted.com (8.18.1.2/8.18.1.2) with ESMTP id 471DaccK019209; Thu, 1 Aug 2024 15:18:48 GMT X-Received: from nasanppmta05.qualcomm.com (i-global254.qualcomm.com [199.106.103.254]) by mx0a-0031df01.pphosted.com (PPS) with ESMTPS id 40pq529pat-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Thu, 01 Aug 2024 15:18:47 +0000 (GMT) X-Received: from nasanex01c.na.qualcomm.com (nasanex01c.na.qualcomm.com [10.45.79.139]) by NASANPPMTA05.qualcomm.com (8.17.1.19/8.17.1.19) with ESMTPS id 471FIlGt031216 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Thu, 1 Aug 2024 15:18:47 GMT X-Received: from qc-i7.hemma.eciton.net (10.80.80.8) by nasanex01c.na.qualcomm.com (10.45.79.139) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.2.1544.9; Thu, 1 Aug 2024 08:18:45 -0700 Date: Thu, 1 Aug 2024 16:18:42 +0100 From: "Leif Lindholm" To: , CC: , Subject: Re: [edk2-devel] [edk2-platforms][PATCH 1/5] AmpereAltraPkg/DwI2cLib: Add SmbusRead() function Message-ID: References: <20240801093618.191274-1-nhi@os.amperecomputing.com> <20240801093618.191274-2-nhi@os.amperecomputing.com> MIME-Version: 1.0 In-Reply-To: <20240801093618.191274-2-nhi@os.amperecomputing.com> X-Originating-IP: [10.80.80.8] X-ClientProxiedBy: nasanex01a.na.qualcomm.com (10.52.223.231) To nasanex01c.na.qualcomm.com (10.45.79.139) X-QCInternal: smtphost X-Proofpoint-GUID: GgRwlrxMKtX1y9L0jKkPwaQLg86ZAS2Y X-Proofpoint-ORIG-GUID: GgRwlrxMKtX1y9L0jKkPwaQLg86ZAS2Y Precedence: Bulk List-Subscribe: List-Help: Sender: devel@edk2.groups.io List-Id: Mailing-List: list devel@edk2.groups.io; contact devel+owner@edk2.groups.io Resent-Date: Thu, 01 Aug 2024 08:18:48 -0700 Resent-From: quic_llindhol@quicinc.com Reply-To: devel@edk2.groups.io,quic_llindhol@quicinc.com List-Unsubscribe-Post: List-Unsubscribe=One-Click List-Unsubscribe: X-Gm-Message-State: YN0TPrrBBAttQYO7RVO6obd1x7686176AA= Content-Type: text/plain; charset="us-ascii" Content-Disposition: inline X-GND-Status: LEGIT Authentication-Results: spool.mail.gandi.net; dkim=pass header.d=groups.io header.s=20240206 header.b=1FfpFEiO; dmarc=fail reason="SPF not aligned (relaxed), DKIM not aligned (relaxed)" header.from=quicinc.com (policy=none); spf=pass (spool.mail.gandi.net: domain of bounce@groups.io designates 45.79.224.7 as permitted sender) smtp.mailfrom=bounce@groups.io 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 > 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. This change also changes the API to pass the PecCheck parameter, without actuaklly using it for anyone. Regardless of my previous comment, I don't think that's very useful. So if you can't move it to the struct, can you move it to the later patch instead? / Leif > Signed-off-by: Nhi Pham > --- > 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, > + BOOLEAN PecCheck > ) > { > EFI_STATUS Status; > @@ -559,6 +612,7 @@ InternalI2cRead ( > UINTN Count; > UINTN ReadCount; > UINTN WriteCount; > + UINT32 ResponseLen; > > Status = EFI_SUCCESS; > Base = mI2cBusList[Bus].Base; > @@ -601,6 +655,36 @@ InternalI2cRead ( > } > > WriteCount = 0; > + if (IsSmbus) { > + // > + // Read Smbus Data Length, first byte of the Smbus response data. > + // > + Status = InternalSmbusReadDataLength (Bus, &ResponseLen); > + if (EFI_ERROR (Status)) { > + DEBUG ((DEBUG_ERROR, "%a: InternalSmbusReadDataLength failed\n", __func__)); > + goto Exit; > + } > + > + WriteCount++; > + Buf[ReadCount++] = ResponseLen; > + > + // > + // Abort the transaction when the requested length is shorter than the actual response data > + // or if there is no response data when PEC disabled. > + // > + if ((*Length < (ResponseLen + 2)) || (!PecCheck && ResponseLen == 0)) { > + MmioWrite32 (Base + DW_IC_DATA_CMD, DW_IC_DATA_CMD_CMD | DW_IC_DATA_CMD_STOP); > + I2cSync (); > + Status = EFI_INVALID_PARAMETER; > + goto Exit; > + } > + > + *Length = ResponseLen + 1; // Response Data Length + 8-bit Byte Count field > + if (PecCheck) { > + *Length += 1; // ++ 8-bit PEC field > + } > + } > + > while ((*Length - ReadCount) != 0) { > TxLimit = mI2cBusList[Bus].TxFifo - MmioRead32 (Base + DW_IC_TXFLR); > RxLimit = mI2cBusList[Bus].RxFifo - MmioRead32 (Base + DW_IC_RXFLR); > @@ -742,7 +826,50 @@ I2cRead ( > > I2cSetSlaveAddr (Bus, SlaveAddr); > > - return InternalI2cRead (Bus, BufCmd, CmdLength, Buf, ReadLength); > + return InternalI2cRead (Bus, BufCmd, CmdLength, Buf, ReadLength, FALSE, FALSE); > +} > + > +/** > + 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 > + ) > +{ > + if (Bus >= AC01_I2C_MAX_BUS_NUM > + || Buf == NULL > + || ReadLength == NULL) > + { > + return EFI_INVALID_PARAMETER; > + } > + > + I2cSetSlaveAddr (Bus, SlaveAddr); > + > + return InternalI2cRead (Bus, BufCmd, CmdLength, Buf, ReadLength, TRUE, PecCheck); > } > > /** > -- > 2.25.1 > > > > > > -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#120182): https://edk2.groups.io/g/devel/message/120182 Mute This Topic: https://groups.io/mt/107662232/7686176 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io] -=-=-=-=-=-=-=-=-=-=-=-