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 613C6D8118D for ; Thu, 1 Aug 2024 15:14:09 +0000 (UTC) DKIM-Signature: a=rsa-sha256; bh=jX7oWKSeLHGzJevbN4cwT5wrOw1Xczzaf6JOotN66qk=; 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=1722525249; v=1; b=1naiSzcyQ9cVw+lYs9I9MvILWLUWA1JhGsh7AVNf1oWIXekNbBVjk5EpxKfNwkNrcwiLVMz5 NRM/l+OscJRxYiClCs8SQhIXX5jfsJ3XoxK+5ySDS/lrUahvacT6Zj4vajLQsxNOVqoicmEQCbV s0jTZSCm+dEP61+D0pK7JqMBBDgNIBlLXPaRzFuafG05A0kulqJlodTyQc8l/peZjY2Tx7o0uMg ZqUxlRLyJeykPo8Xhfax9zkYNGnJn1qb77VmJSd+Z+JbnUAvctAsKvAtM+J+hkhKx72I0aHTa7m z2WiaGdezb84Rwap7hRs/YCd5Xyr9hvwD/WGxveM7YB5w== X-Received: by 127.0.0.2 with SMTP id p1foYY7687511xMV1WcJZEq9; Thu, 01 Aug 2024 08:14:07 -0700 X-Received: from mx0b-0031df01.pphosted.com (mx0b-0031df01.pphosted.com [205.220.180.131]) by mx.groups.io with SMTP id smtpd.web11.70847.1722525247261968953 for ; Thu, 01 Aug 2024 08:14:07 -0700 X-Received: from pps.filterd (m0279872.ppops.net [127.0.0.1]) by mx0a-0031df01.pphosted.com (8.18.1.2/8.18.1.2) with ESMTP id 471Dae5v024611; Thu, 1 Aug 2024 15:14:06 GMT X-Received: from nasanppmta04.qualcomm.com (i-global254.qualcomm.com [199.106.103.254]) by mx0a-0031df01.pphosted.com (PPS) with ESMTPS id 40qjpjcxsu-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Thu, 01 Aug 2024 15:14:06 +0000 (GMT) X-Received: from nasanex01c.na.qualcomm.com (nasanex01c.na.qualcomm.com [10.45.79.139]) by NASANPPMTA04.qualcomm.com (8.17.1.19/8.17.1.19) with ESMTPS id 471FE40O032464 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Thu, 1 Aug 2024 15:14:05 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:14:03 -0700 Date: Thu, 1 Aug 2024 16:14:00 +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: vU8tU3sLDdkaBP6JZRbH4BUVNg2DgPMa X-Proofpoint-ORIG-GUID: vU8tU3sLDdkaBP6JZRbH4BUVNg2DgPMa 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:14:07 -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: 4kpsa6Co2JzH46Xiqrhg95OFx7686176AA= 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=1naiSzcy; spf=pass (spool.mail.gandi.net: domain of bounce@groups.io designates 45.79.224.7 as permitted sender) smtp.mailfrom=bounce@groups.io; dmarc=fail reason="SPF not aligned (relaxed), DKIM not aligned (relaxed)" header.from=quicinc.com (policy=none) 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 > 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. > 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, 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 > ) > { > 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 (#120181): https://edk2.groups.io/g/devel/message/120181 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] -=-=-=-=-=-=-=-=-=-=-=-