Re: [RFC] MdePkg/Spi : Modify SPI Protocol strctures to make it more generic

[Leif Lindholm <leif.lindholm@linaro.org>]

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
> to handle all type of SPI commands
> e.g. QuadIo Read Command (0xEBh) in spansion flash S25FS512S will not
> work.
> 
> Cause: Currenty SPI protocol describes a SPI transaction as combination
> of data and address and command, which would all be transmitted on
> same data bus (1 or 2 or 4).
> 
> BUT when we have a case in which command is send one single data bus and
> address and data is sent one 4 data bus (like 0xEB in spansion), then
> this will not work with current strcture.
> 
> Fix: Instead of making a single transaction, we can specify a group of
> transactions (called packet, terminology borrowed from I2c Protocol) in
> which each transaction specifies the data width, frequency, delay and
> read/write info.
> 
> Issue_2: Specify additional data about SPI transaction (i.e. transaction
> is of type cmd, address, data, dummy etc). some controllers need this
> info to communicate with SPI peripheral. others can ignore this info.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Pankaj Bansal <pankaj.bansal@nxp.com>
> ---
>  MdePkg/Include/Protocol/SpiHc.h |  15 +-
>  MdePkg/Include/Protocol/SpiIo.h | 250 ++++++++++++------------------
>  2 files changed, 104 insertions(+), 161 deletions(-)
> 
> diff --git a/MdePkg/Include/Protocol/SpiHc.h 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.
>  
>    @param[in] This            Pointer to an EFI_SPI_HC_PROTOCOL structure.
> -  @param[in] BusTransaction  Pointer to a EFI_SPI_BUS_ TRANSACTION containing
> -                             the description of the SPI transaction to perform.
> +  @param[in] SpiPeripheral   The address of an EFI_SPI_PERIPHERAL data structure
> +                             describing the SPI peripheral from(to) which
> +                             read(write) transactions to be performed.
> +  @param[in] RequestPacket   Pointer to a EFI_SPI_REQUEST_PACKET containing
> +                             the description of the SPI transactions to perform.
>  
>    @retval EFI_SUCCESS          The transaction completed successfully
>    @retval EFI_BAD_BUFFER_SIZE  The BusTransaction->WriteBytes value is invalid,
> @@ -124,7 +127,8 @@ typedef EFI_STATUS
>  typedef EFI_STATUS
>  (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
>    );
>  
>  ///
> @@ -135,7 +139,6 @@ struct _EFI_SPI_HC_PROTOCOL {
>    /// Host control attributes, may have zero or more of the following set:
>    /// * 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 most
>    ///     significant bits instead of least significant bits.The host driver
> @@ -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 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 EFI_SPI_IO_PROTOCOL;
>  /// Note: The UEFI PI 1.6 specification does not specify values for the
>  ///       members below. The order matches the specification.
>  ///
> +///
> +/// SPI peripheral transfer type
> +///
> +/// The EFI_SPI_TRANSACTION_TYPE describes the type of SPI transaction to either
> +/// send or recievce from SPI peripheral.
> +/// some SPI controllers need this information to complete a SPI operation
> +/// (which consists of one or many transactions)
> +///
> +/// for other controllers this field may be left 0 (SPI_TRANSACTION_NONE)
> +///
>  typedef enum {
> -  ///
> -  /// Data flowing in both direction between the host and
> -  /// SPI peripheral.ReadBytes must equal WriteBytes and both ReadBuffer and
> -  /// WriteBuffer must be provided.
> -  ///
> -  SPI_TRANSACTION_FULL_DUPLEX,
> -
> -  ///
> -  /// Data flowing from the host to the SPI peripheral.ReadBytes must be
> -  /// zero.WriteBytes must be non - zero and WriteBuffer must be provided.
> -  ///
> -  SPI_TRANSACTION_WRITE_ONLY,
> -
> -  ///
> -  /// Data flowing from the SPI peripheral to the host.WriteBytes 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 data
> -  /// flows from the SPI peripheral to the host.These types of operations get
> -  /// used for SPI flash devices when control data (opcode, address) 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 transaction to the
> -  SPI controller for execution on the SPI bus. There are four types of
> -  supported transactions supported by this routine:
> -  * Full Duplex: WriteBuffer and ReadBuffer are the same size.
> -  * Write Only: WriteBuffer contains data for SPI peripheral, ReadBytes = 0
> -  * Read Only: ReadBuffer to receive data from SPI peripheral, WriteBytes = 0
> -  * Write Then Read: WriteBuffer contains control data to write to SPI
> -                     peripheral before data is placed into the ReadBuffer.
> -                     Both WriteBytes and ReadBytes must be non-zero.
> -
> -  @param[in]  This              Pointer to an EFI_SPI_IO_PROTOCOL structure.
> -  @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 transactions 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-zero
> -                                value only when a specific SPI transaction
> -                                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 from the
> -                                host to the SPI chip. Specify NULL for read
> -                                only operations.
> -                                * 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 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 bytes) per
> -                                  frame
> -                                * Frame sizes 17-32 bits: UINT32 (four bytes)
> -                                  per frame The received frame is in the least
> -                                  significant N bits.
> -
> -  @retval EFI_SUCCESS            The SPI transaction completed successfully
> -  @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 peripheral or
> -                                 SPI host controller,
> -                                 or WriteBytes non-zero and WriteBuffer is
> -                                 NULL,
> -                                 or ReadBytes non-zero and ReadBuffer is NULL,
> -                                 or ReadBuffer != 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 SPI bus
> -                                 layer or the SPI host controller
> -  @retval EFI_UNSUPPORTED        The SPI controller was not able to support
> -
> -**/
> -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 = 0,
>  
> -/**
> -  Update the SPI peripheral associated with this SPI 10 instance.
> +  /// Command to send to SPI peripheral
> +  SPI_TRANSACTION_COMMAND,
>  
> -  Support socketed SPI parts by allowing the SPI peripheral driver to replace
> -  the SPI peripheral after the connection is made. An example use is socketed
> -  SPI NOR flash parts, where the size and parameters change depending upon
> -  device is in the socket.
> +  /// Address from/to which data is to be read/written
> +  SPI_TRANSACTION_ADDRESS,
>  
> -  @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 by
> +  /// peripherals to have some delay between transfers.
> +  SPI_TRANSACTION_DUMMY,
>  
> -  @retval EFI_SUCCESS            The SPI peripheral was updated successfully
> -  @retval EFI_INVALID_PARAMETER  The SpiPeripheral value is NULL,
> -                                 or the SpiPeripheral->SpiBus is NULL,
> -                                 or the SpiP eripheral - >SpiBus pointing at
> -                                 wrong bus,
> -                                 or the SpiP eripheral - >SpiPart is 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
> -  );
> +  /// Data Request (Read/write)
> +  SPI_TRANSACTION_DATA,
> +} EFI_SPI_TRANSACTION_TYPE;
>  
>  ///
>  /// The EFI_SPI_BUS_ TRANSACTION data structure contains the description of the
> @@ -175,11 +61,6 @@ typedef EFI_STATUS
>  ///
>  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_TYPE
>    /// values.
>    ///
> @@ -204,9 +85,9 @@ typedef struct _EFI_SPI_BUS_TRANSACTION {
>    UINT32                   FrameSize;
>  
>    ///
> -  /// Length of the write buffer in bytes
> +  /// Length of the buffer (read or write or both) in bytes
>    ///
> -  UINT32                   WriteBytes;
> +  UINT32                   Bytes;
>  
>    ///
>    /// Buffer containing data to send to the SPI peripheral
> @@ -216,20 +97,87 @@ typedef struct _EFI_SPI_BUS_TRANSACTION {
>    UINT8                    *WriteBuffer;
>  
>    ///
> -  /// 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 clock frequency supported by the
> +  /// SPI controller and part. Specify a non-zero
> +  /// value only when a specific SPI transaction
> +  /// requires a reduced clock rate.
> +  UINT32                   ClockHz;
>  } EFI_SPI_BUS_TRANSACTION;
>  
>  ///
> +/// SPI device request
> +///
> +/// The EFI_SPI_REQUEST_PACKET describes single SPI operation,
> +/// between Chip Select deassert and Chip Select assert.
> +/// Some operation will consist of a single transaction while others 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 transaction to the
> +  SPI controller for execution on the SPI bus.
> +
> +  @param[in]  This              Pointer to an EFI_SPI_IO_PROTOCOL structure.
> +  @param[in] RequestPacket      Pointer to an EFI_SPI_REQUEST_PACKET structure
> +                                describing the SPI transaction
> +**/
> +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 replace
> +  the SPI peripheral after the connection is made. An example use is socketed
> +  SPI NOR flash parts, where the size and parameters 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 successfully
> +  @retval EFI_INVALID_PARAMETER  The SpiPeripheral value is NULL,
> +                                 or the SpiPeripheral->SpiBus is NULL,
> +                                 or the SpiP eripheral - >SpiBus pointing at
> +                                 wrong bus,
> +                                 or the SpiP eripheral - >SpiPart is 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 and a SPI
>  /// chip.
>  ///
> @@ -262,14 +210,10 @@ struct _EFI_SPI_IO_PROTOCOL {
>  
>    ///
>    /// 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;
>  
> -- 
> 2.7.4
>