SPI API "padding" functionality breaks the application

[Brandon-Hurst]

The struct-based API for SPI describes the function of automatically padding the transaction with the appropriate bytes if if rxLen > txLen.

On both MAX32670 and MAX78000 under the latest SDK, I have found that the SPIMasterTransaction function actually breaks my application by sending it into an infinite while loop whenever txLen != rxLen. I also noticed some strange issues when this was happening, such as the order of transmitted bytes being reversed, etc. Will share scope shots later today.

Example function:

// call max31723_spi_init(MAX31723_SPI) first
// assign the return value of this function to a uint8_t variable
uint8_t max31723_read_reg(uint8_t reg) 
{
    int err;
    mxc_spi_req_t r_req;
    reg &= ~0x80; //Clear the write bit.
    uint8_t t_data[2] = {reg, 0xFF}; //0xFF --> DUMMY BYTE
    uint8_t r_data[2] = {0x00, 0x00};

    r_req.spi           = MAX31723_SPI;
    r_req.txData        = t_data; //must pass a pointer to the data arrays 
    r_req.rxData        = r_data; //same for r_data 
    r_req.txLen         = 2;
    r_req.rxLen         = 2;
    r_req.ssIdx         = 0;
    r_req.ssDeassert    = 1;
    r_req.txCnt         = 0;
    r_req.rxCnt         = 0;
    r_req.completeCB    = NULL;

    err = MXC_SPI_MasterTransaction(&r_req);
    if (err != E_NO_ERROR)
    {
        printf("Issue reading SPI peripheral: errno %d", err);
    }
    //Second byte of rx array contains read data
    return r_data[1]; 
}

int max31723_spi_init(mxc_spi_regs_t* spi)
{
    int err;
    // spi, mastermode, quadmode, num_slaves, ss_polarity, speed, pins
    err = MXC_SPI_Init(spi, MASTERMODE, QUAD_OFF, NUM_PERIPHS, SS_ACTIVE_HI, SPI_SPEED); 

    MXC_SPI_SetMode(spi, 3); // mode3
    MXC_SPI_SetDataSize(spi, 8); // 8-bit character size
    MXC_SPI_SetWidth(spi, SPI_WIDTH_STANDARD); // width of data lines is standard MOSI/MISO; not quad

    //Write MAX31723 reg0 to 0x00
    max31723_write_reg(0x00, 0x00);
    return err;
}

Valid Read:

Invalid Read (failed auto-padding): In this case, an infinite while loop is reached.

[Jake-Carter]

The struct-based API for SPI describes the function of automatically padding the transaction with the appropriate bytes if if rxLen > txLen

We have two versions of the SPI drivers: "V1" (spi_reva1.c) and "V2" (spi_reva2.c).

"V1" requires you to pad the data manually as you've done above, whereas in "V2" the drivers handle the padding automatically. Right now the only devices that support "V2" are the MAX32572, MAX32690, and MAX78002.

The exact verbiage in the comment might be misleading. "These actions wil be performed..." doesn't apply to those sup-qualifiers on step 2. Those are requirements that must be done by the user.

("V1" header)

[Brandon-Hurst]

Understood, thanks. I would say the API's HTML documentation is what misled me here:

The highlighted portion is what I was looking at. It would suggest to me that this action is to be performed by the API, not the user. The above comment would most likely have misled me in a similar way. Additionally -- under proper configurations the SS line can be asserted and de-asserted by the API/SPI peripheral (preferable), not by the developer via manual GPIO. So that would mean different logic for the different sub-items above.

I'm assuming the same logic should be the case for operating as a slave/sub node -- is that correct?

[Jake-Carter]

Yes, the wording and implementation is misleading. Looks like the only time we actually do the automatic padding in "V1" is when txData is NULL. (https://github.com/analogdevicesinc/msdk/blob/f2c2c8b03ffb8e92b01947a267c9fe6e18dd5ecc/Libraries/PeriphDrivers/Source/SPI/spi_reva1.c#L737)

In which case we re-use the RX buffer for the dummy bytes... I can see that being problematic... But yes, the same is done for slave transactions as well.

"V1" is generally a mess and we're porting the rest of the micros to "V2", though it's relatively slow going

[Brandon-Hurst]

Okay, thanks again for the input here, it's helpful in the short term. We can continue this dialog outside this issue as needed.