From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Permerror (SPF Permanent Error: More than 10 MX records returned) identity=mailfrom; client-ip=192.55.52.88; helo=mga01.intel.com; envelope-from=dandan.bi@intel.com; receiver=edk2-devel@lists.01.org Received: from mga01.intel.com (mga01.intel.com [192.55.52.88]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id 29879221EA0B2 for ; Thu, 7 Dec 2017 21:17:48 -0800 (PST) Received: from orsmga007.jf.intel.com ([10.7.209.58]) by fmsmga101.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 07 Dec 2017 21:22:22 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.45,376,1508828400"; d="scan'208";a="1029139" Received: from fmsmsx107.amr.corp.intel.com ([10.18.124.205]) by orsmga007.jf.intel.com with ESMTP; 07 Dec 2017 21:22:20 -0800 Received: from fmsmsx123.amr.corp.intel.com (10.18.125.38) by fmsmsx107.amr.corp.intel.com (10.18.124.205) with Microsoft SMTP Server (TLS) id 14.3.319.2; Thu, 7 Dec 2017 21:22:21 -0800 Received: from shsmsx101.ccr.corp.intel.com (10.239.4.153) by fmsmsx123.amr.corp.intel.com (10.18.125.38) with Microsoft SMTP Server (TLS) id 14.3.319.2; Thu, 7 Dec 2017 21:22:21 -0800 Received: from shsmsx102.ccr.corp.intel.com ([169.254.2.175]) by SHSMSX101.ccr.corp.intel.com ([169.254.1.159]) with mapi id 14.03.0319.002; Fri, 8 Dec 2017 13:22:19 +0800 From: "Bi, Dandan" To: Pankaj Bansal , Leif Lindholm CC: "Kinney, Michael D" , "edk2-devel@lists.01.org" , "Gao, Liming" , "Ni, Ruiyu" Thread-Topic: [edk2] [RFC] MdePkg/Spi : Modify SPI Protocol strctures to make it more generic Thread-Index: AQHTbzZwT9L8S58D3kyrUflS00dNF6M3cHSAgAFxb2A= Date: Fri, 8 Dec 2017 05:22:18 +0000 Message-ID: <3C0D5C461C9E904E8F62152F6274C0BB3B9D93E1@shsmsx102.ccr.corp.intel.com> References: <1512635719-32003-1-git-send-email-pankaj.bansal@nxp.com> <20171207144851.ld2vbgo6az2shquy@bivouac.eciton.net> In-Reply-To: <20171207144851.ld2vbgo6az2shquy@bivouac.eciton.net> Accept-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [10.239.127.40] MIME-Version: 1.0 Subject: Re: [RFC] MdePkg/Spi : Modify SPI Protocol strctures to make it more generic X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.22 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 08 Dec 2017 05:17:49 -0000 Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Hi Pankaj Bansal, Current SPI header files follow PI 1.6 Spec, if you find any issue in PI s= pec, current process is to raise them to PIWG for discussion firstly, then = PIWG will discuss the issue and decide whether update the spec, at last upd= ate the code if needed. Thanks, Dandan -----Original Message----- From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Leif= Lindholm Sent: Thursday, December 7, 2017 10:49 PM To: Pankaj Bansal Cc: Kinney, Michael D ; edk2-devel@lists.01.org= ; Gao, Liming Subject: Re: [edk2] [RFC] MdePkg/Spi : Modify SPI Protocol strctures to mak= e it more generic Adding MdePkg maintainers: Mike and Liming. Please always cc package maintainers (listed in Maintainers.txt). / Leif On Thu, Dec 07, 2017 at 02:05:19PM +0530, Pankaj Bansal wrote: > Issue_1: Currenty SPI IO and SPI HC protocol strcture is not equipped=20 > to handle all type of SPI commands e.g. QuadIo Read Command (0xEBh) in=20 > spansion flash S25FS512S will not work. >=20 > Cause: Currenty SPI protocol describes a SPI transaction as=20 > combination of data and address and command, which would all be=20 > transmitted on same data bus (1 or 2 or 4). >=20 > BUT when we have a case in which command is send one single data bus=20 > and address and data is sent one 4 data bus (like 0xEB in spansion),=20 > then this will not work with current strcture. >=20 > Fix: Instead of making a single transaction, we can specify a group of=20 > transactions (called packet, terminology borrowed from I2c Protocol)=20 > in which each transaction specifies the data width, frequency, delay=20 > and read/write info. >=20 > Issue_2: Specify additional data about SPI transaction (i.e.=20 > transaction is of type cmd, address, data, dummy etc). some=20 > controllers need this info to communicate with SPI peripheral. others can= ignore this info. >=20 > Contributed-under: TianoCore Contribution Agreement 1.1 > Signed-off-by: Pankaj Bansal > --- > MdePkg/Include/Protocol/SpiHc.h | 15 +- =20 > MdePkg/Include/Protocol/SpiIo.h | 250 ++++++++++++------------------ > 2 files changed, 104 insertions(+), 161 deletions(-) >=20 > diff --git a/MdePkg/Include/Protocol/SpiHc.h=20 > b/MdePkg/Include/Protocol/SpiHc.h index 12fe5d2..65964bc 100644 > --- a/MdePkg/Include/Protocol/SpiHc.h > +++ b/MdePkg/Include/Protocol/SpiHc.h > @@ -110,8 +110,11 @@ typedef EFI_STATUS > status. > =20 > @param[in] This Pointer to an EFI_SPI_HC_PROTOCOL structure= . > - @param[in] BusTransaction Pointer to a EFI_SPI_BUS_ TRANSACTION conta= ining > - the description of the SPI transaction to p= erform. > + @param[in] SpiPeripheral The address of an EFI_SPI_PERIPHERAL data s= tructure > + describing the SPI peripheral from(to) whic= h > + read(write) transactions to be performed. > + @param[in] RequestPacket Pointer to a EFI_SPI_REQUEST_PACKET contain= ing > + the description of the SPI transactions to = perform. > =20 > @retval EFI_SUCCESS The transaction completed successfully > @retval EFI_BAD_BUFFER_SIZE The BusTransaction->WriteBytes value=20 > is invalid, @@ -124,7 +127,8 @@ typedef EFI_STATUS typedef EFI_STATUS =20 > (EFIAPI *EFI_SPI_HC_PROTOCOL_TRANSACTION) ( > IN CONST EFI_SPI_HC_PROTOCOL *This, > - IN EFI_SPI_BUS_TRANSACTION *BusTransaction > + IN CONST EFI_SPI_PERIPHERAL *SpiPeripheral; > + IN EFI_SPI_REQUEST_PACKET *RequestPacket > ); > =20 > /// > @@ -135,7 +139,6 @@ struct _EFI_SPI_HC_PROTOCOL { > /// Host control attributes, may have zero or more of the following se= t: > /// * HC_SUPPORTS_WRITE_ONLY_OPERATIONS > /// * HC_SUPPORTS_READ_ONLY_OPERATIONS > - /// * HC_SUPPORTS_WRITE_THEN_READ_OPERATIONS > /// * HC_TX_FRAME_IN_MOST_SIGNIFICANT_BITS > /// - The SPI host controller requires the transmit frame to be in m= ost > /// significant bits instead of least significant bits.The host dr= iver > @@ -149,10 +152,6 @@ struct _EFI_SPI_HC_PROTOCOL { > /// - The SPI controller supports a 2 - bit data bus > /// * HC_SUPPORTS_4_B1T_DATA_BUS_WIDTH > /// - The SPI controller supports a 4 - bit data bus > - /// * HC_TRANSFER_SIZE_INCLUDES_OPCODE > - /// - Transfer size includes the opcode byte > - /// * HC_TRANSFER_SIZE_INCLUDES_ADDRESS > - /// - Transfer size includes the 3 address bytes > /// The SPI host controller must support full - duplex (receive while > /// sending) operation.The SPI host controller must support a 1 - bit = bus > /// width. > diff --git a/MdePkg/Include/Protocol/SpiIo.h=20 > b/MdePkg/Include/Protocol/SpiIo.h index 43e8045..507abb5 100644 > --- a/MdePkg/Include/Protocol/SpiIo.h > +++ b/MdePkg/Include/Protocol/SpiIo.h > @@ -27,147 +27,33 @@ typedef struct _EFI_SPI_IO_PROTOCOL=20 > EFI_SPI_IO_PROTOCOL; /// Note: The UEFI PI 1.6 specification does not sp= ecify values for the > /// members below. The order matches the specification. > /// > +/// > +/// SPI peripheral transfer type > +/// > +/// The EFI_SPI_TRANSACTION_TYPE describes the type of SPI=20 > +transaction to either /// send or recievce from SPI peripheral. > +/// some SPI controllers need this information to complete a SPI=20 > +operation /// (which consists of one or many transactions) /// ///=20 > +for other controllers this field may be left 0 (SPI_TRANSACTION_NONE)=20 > +/// > typedef enum { > - /// > - /// Data flowing in both direction between the host and > - /// SPI peripheral.ReadBytes must equal WriteBytes and both=20 > ReadBuffer and > - /// WriteBuffer must be provided. > - /// > - SPI_TRANSACTION_FULL_DUPLEX, > - > - /// > - /// Data flowing from the host to the SPI peripheral.ReadBytes must=20 > be > - /// zero.WriteBytes must be non - zero and WriteBuffer must be provide= d. > - /// > - SPI_TRANSACTION_WRITE_ONLY, > - > - /// > - /// Data flowing from the SPI peripheral to the host.WriteBytes=20 > must be > - /// zero.ReadBytes must be non - zero and ReadBuffer must be provided. > - /// > - SPI_TRANSACTION_READ_ONLY, > - > - /// > - /// Data first flowing from the host to the SPI peripheral and then=20 > data > - /// flows from the SPI peripheral to the host.These types of=20 > operations get > - /// used for SPI flash devices when control data (opcode, address)=20 > must be > - /// passed to the SPI peripheral to specify the data to be read. > - /// > - SPI_TRANSACTION_WRITE_THEN_READ > -} EFI_SPI_TRANSACTION_TYPE; > - > -/** > - Initiate a SPI transaction between the host and a SPI peripheral. > - > - This routine must be called at or below TPL_NOTIFY. > - This routine works with the SPI bus layer to pass the SPI=20 > transaction to the > - SPI controller for execution on the SPI bus. There are four types=20 > of > - supported transactions supported by this routine: > - * Full Duplex: WriteBuffer and ReadBuffer are the same size. > - * Write Only: WriteBuffer contains data for SPI peripheral,=20 > ReadBytes =3D 0 > - * Read Only: ReadBuffer to receive data from SPI peripheral,=20 > WriteBytes =3D 0 > - * Write Then Read: WriteBuffer contains control data to write to SPI > - peripheral before data is placed into the ReadBuffe= r. > - Both WriteBytes and ReadBytes must be non-zero. > - > - @param[in] This Pointer to an EFI_SPI_IO_PROTOCOL struct= ure. > - @param[in] TransactionType Type of SPI transaction. > - @param[in] DebugTransaction Set TRUE only when debugging is desired. > - Debugging may be turned on for a single = SPI > - transaction. Only this transaction will = display > - debugging messages. All other transactio= ns with > - this value set to FALSE will not display= any > - debugging messages. > - @param[in] ClockHz Specify the ClockHz value as zero (0) to= use > - the maximum clock frequency supported by= the > - SPI controller and part. Specify a non-z= ero > - value only when a specific SPI transacti= on > - requires a reduced clock rate. > - @param[in] BusWidth Width of the SPI bus in bits: 1, 2, 4 > - @param[in] FrameSize Frame size in bits, range: 1 - 32 > - @param[in] WriteBytes The length of the WriteBuffer in bytes. > - Specify zero for read-only operations. > - @param[in] WriteBuffer The buffer containing data to be sent fr= om the > - host to the SPI chip. Specify NULL for r= ead > - only operations. > - * Frame sizes 1-8 bits: UINT8 (one byte)= per > - frame > - * Frame sizes 7-16 bits: UINT16 (two byt= es) per > - frame > - * Frame sizes 17-32 bits: UINT32 (four b= ytes) > - per frame The transmit frame is in the= least > - significant N bits. > - @param[in] ReadBytes The length of the ReadBuffer in bytes. > - Specify zero for write-only operations. > - @param[out] ReadBuffer The buffer to receeive data from the SPI= chip > - during the transaction. Specify NULL for= write > - only operations. > - * Frame sizes 1-8 bits: UINT8 (one byte)= per > - frame > - * Frame sizes 7-16 bits: UINT16 (two byt= es) per > - frame > - * Frame sizes 17-32 bits: UINT32 (four b= ytes) > - per frame The received frame is in the= least > - significant N bits. > - > - @retval EFI_SUCCESS The SPI transaction completed successfu= lly > - @retval EFI_BAD_BUFFER_SIZE The writeBytes value was invalid > - @retval EFI_BAD_BUFFER_SIZE The ReadBytes value was invalid > - @retval EFI_INVALID_PARAMETER TransactionType is not valid, > - or BusWidth not supported by SPI periph= eral or > - SPI host controller, > - or WriteBytes non-zero and WriteBuffer = is > - NULL, > - or ReadBytes non-zero and ReadBuffer is= NULL, > - or ReadBuffer !=3D WriteBuffer for full= -duplex > - type, > - or WriteBuffer was NULL, > - or TPL is too high > - @retval EFI_OUT_OF_RESOURCES Insufficient memory for SPI transaction > - @retval EFI_UNSUPPORTED The FrameSize is not supported by the S= PI bus > - layer or the SPI host controller > - @retval EFI_UNSUPPORTED The SPI controller was not able to supp= ort > - > -**/ > -typedef > -EFI_STATUS > -(EFIAPI *EFI_SPI_IO_PROTOCOL_TRANSACTION) ( > - IN CONST EFI_SPI_IO_PROTOCOL *This, > - IN EFI_SPI_TRANSACTION_TYPE TransactionType, > - IN BOOLEAN DebugTransaction, > - IN UINT32 ClockHz OPTIONAL, > - IN UINT32 BusWidth, > - IN UINT32 FrameSize, > - IN UINT32 WriteBytes, > - IN UINT8 *WriteBuffer, > - IN UINT32 ReadBytes, > - OUT UINT8 *ReadBuffer > - ); > + SPI_TRANSACTION_NONE =3D 0, > =20 > -/** > - Update the SPI peripheral associated with this SPI 10 instance. > + /// Command to send to SPI peripheral SPI_TRANSACTION_COMMAND, > =20 > - Support socketed SPI parts by allowing the SPI peripheral driver to=20 > replace > - the SPI peripheral after the connection is made. An example use is=20 > socketed > - SPI NOR flash parts, where the size and parameters change depending=20 > upon > - device is in the socket. > + /// Address from/to which data is to be read/written =20 > + SPI_TRANSACTION_ADDRESS, > =20 > - @param[in] This Pointer to an EFI_SPI_IO_PROTOCOL structure. > - @param[in] SpiPeripheral Pointer to an EFI_SPI_PERIPHERAL structure. > + /// Dummy cycles > + /// Some SPI peripherals need dummy clock cycles which can be used=20 > + by /// peripherals to have some delay between transfers. > + SPI_TRANSACTION_DUMMY, > =20 > - @retval EFI_SUCCESS The SPI peripheral was updated successf= ully > - @retval EFI_INVALID_PARAMETER The SpiPeripheral value is NULL, > - or the SpiPeripheral->SpiBus is NULL, > - or the SpiP eripheral - >SpiBus pointin= g at > - wrong bus, > - or the SpiP eripheral - >SpiPart is NUL= L > - > -**/ > -typedef EFI_STATUS > -(EFIAPI *EFI_SPI_IO_PROTOCOL_UPDATE_SPI_PERIPHERAL) ( > - IN CONST EFI_SPI_IO_PROTOCOL *This, > - IN CONST EFI_SPI_PERIPHERAL *SpiPeripheral > - ); > + /// Data Request (Read/write) > + SPI_TRANSACTION_DATA, > +} EFI_SPI_TRANSACTION_TYPE; > =20 > /// > /// The EFI_SPI_BUS_ TRANSACTION data structure contains the=20 > description of the @@ -175,11 +61,6 @@ typedef EFI_STATUS /// =20 > typedef struct _EFI_SPI_BUS_TRANSACTION { > /// > - /// Pointer to the SPI peripheral being manipulated. > - /// > - CONST EFI_SPI_PERIPHERAL *SpiPeripheral; > - > - /// > /// Type of transaction specified by one of the EFI_SPI_TRANSACTION_TY= PE > /// values. > /// > @@ -204,9 +85,9 @@ typedef struct _EFI_SPI_BUS_TRANSACTION { > UINT32 FrameSize; > =20 > /// > - /// Length of the write buffer in bytes > + /// Length of the buffer (read or write or both) in bytes > /// > - UINT32 WriteBytes; > + UINT32 Bytes; > =20 > /// > /// Buffer containing data to send to the SPI peripheral @@ -216,20=20 > +97,87 @@ typedef struct _EFI_SPI_BUS_TRANSACTION { > UINT8 *WriteBuffer; > =20 > /// > - /// Length of the read buffer in bytes > - /// > - UINT32 ReadBytes; > - > - /// > /// Buffer to receive the data from the SPI peripheral > /// * Frame sizes 1 - 8 bits: UINT8 (one byte) per frame > /// * Frame sizes 7 - 16 bits : UINT16 (two bytes) per frame > /// * Frame sizes 17 - 32 bits : UINT32 (four bytes) per frame > /// > UINT8 *ReadBuffer; > + > + /// delay afer transaction > + UINT32 DelayMicroSeconds; > + > + /// Specify the ClockHz value as zero (0) to use /// the maximum=20 > + clock frequency supported by the /// SPI controller and part.=20 > + Specify a non-zero /// value only when a specific SPI transaction =20 > + /// requires a reduced clock rate. > + UINT32 ClockHz; > } EFI_SPI_BUS_TRANSACTION; > =20 > /// > +/// SPI device request > +/// > +/// The EFI_SPI_REQUEST_PACKET describes single SPI operation, ///=20 > +between Chip Select deassert and Chip Select assert. > +/// Some operation will consist of a single transaction while others=20 > +will /// be two or more. > +/// > +typedef struct _EFI_SPI_REQUEST_PACKET { > + /// > + /// Number of elements in the operation array > + /// > + UINTN TransactionCount; > + /// > + /// Description of the SPI operation > + /// > + EFI_SPI_BUS_TRANSACTION Transaction [1]; > +} EFI_SPI_REQUEST_PACKET; > + > +/** > + Initiate a SPI transaction between the host and a SPI peripheral. > + > + This routine must be called at or below TPL_NOTIFY. > + This routine works with the SPI bus layer to pass the SPI=20 > + transaction to the SPI controller for execution on the SPI bus. > + > + @param[in] This Pointer to an EFI_SPI_IO_PROTOCOL struct= ure. > + @param[in] RequestPacket Pointer to an EFI_SPI_REQUEST_PACKET str= ucture > + describing the SPI transaction **/=20 > +typedef EFI_STATUS (EFIAPI *EFI_SPI_IO_PROTOCOL_TRANSACTION) ( > + IN CONST EFI_SPI_IO_PROTOCOL *This, > + IN EFI_SPI_REQUEST_PACKET *RequestPacket, > + ); > + > +/** > + Update the SPI peripheral associated with this SPI 10 instance. > + > + Support socketed SPI parts by allowing the SPI peripheral driver to=20 > + replace the SPI peripheral after the connection is made. An example=20 > + use is socketed SPI NOR flash parts, where the size and parameters=20 > + change depending upon device is in the socket. > + > + @param[in] This Pointer to an EFI_SPI_IO_PROTOCOL structure. > + @param[in] SpiPeripheral Pointer to an EFI_SPI_PERIPHERAL structure. > + > + @retval EFI_SUCCESS The SPI peripheral was updated successf= ully > + @retval EFI_INVALID_PARAMETER The SpiPeripheral value is NULL, > + or the SpiPeripheral->SpiBus is NULL, > + or the SpiP eripheral - >SpiBus pointin= g at > + wrong bus, > + or the SpiP eripheral - >SpiPart is=20 > + NULL > + > +**/ > +typedef EFI_STATUS > +(EFIAPI *EFI_SPI_IO_PROTOCOL_UPDATE_SPI_PERIPHERAL) ( > + IN CONST EFI_SPI_IO_PROTOCOL *This, > + IN CONST EFI_SPI_PERIPHERAL *SpiPeripheral > + ); > + > +/// > /// Support managed SPI data transactions between the SPI controller=20 > and a SPI /// chip. > /// > @@ -262,14 +210,10 @@ struct _EFI_SPI_IO_PROTOCOL { > =20 > /// > /// Transaction attributes: One or more from: > - /// * SPI_10_SUPPORTS_2_B1T_DATA_BUS_W1DTH > + /// * SPI_IO_SUPPORTS_2_B1T_DATA_BUS_W1DTH > /// - The SPI host and peripheral supports a 2-bit data bus > /// * SPI_IO_SUPPORTS_4_BIT_DATA_BUS_W1DTH > /// - The SPI host and peripheral supports a 4-bit data bus > - /// * SPI_IO_TRANSFER_SIZE_INCLUDES_OPCODE > - /// - Transfer size includes the opcode byte > - /// * SPI_IO_TRANSFER_SIZE_INCLUDES_ADDRESS > - /// - Transfer size includes the 3 address bytes > /// > UINT32 Attributes; > =20 > -- > 2.7.4 >=20 _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel