fix(PeriphDrivers): Fix SPI RevA1 DMA Callback for Read-Only or Asymmetrical Transactions

[Jake-Carter]

Description

This PR updates the SPI Rev A1 DMA drivers to work with read-only and/or asymmetrical transactions (txLen == 0 or txLen < rxLen).

Reported by internal Jira MSDK-1264

SPI receive operation in 3-wire mode does not work when DMA is used (MASTERDMA). There is no problem with its operation when DMA is not used (MASTERSYNC or MASTERASYNC). When DMA is used, the DMA rececive channel's callback functions is not called, so the CPU cannot learn that the SPI transaction has finished.

The problematic logic seems to be the txrx_req logic.

If txLen is 0, then .txrx_req is set to 0 and the logical condition for releasing both DMA channels is satisfied even if the RX channel's CTZ has not fired. .mtMode also never seems to be set to 1 at all in the drivers.

Previous implementation...

void MXC_SPI_RevA1_DMACallback(int ch, int error)
{
    mxc_spi_reva_req_t *temp_req;

    for (int i = 0; i < MXC_SPI_INSTANCES; i++) {
        if (states[i].req != NULL) {
            if (states[i].channelTx == ch) {
                states[i].req_done++;
            } else if (states[i].channelRx == ch) {
                states[i].req_done++;
                //save the request
                temp_req = states[i].req;

                if (MXC_SPI_GetDataSize((mxc_spi_regs_t *)temp_req->spi) > 8) {
                    MXC_SPI_RevA1_SwapByte(temp_req->rxData, temp_req->rxLen);
                }
            }

            if (!states[i].txrx_req || (states[i].txrx_req && states[i].req_done == 2)) { 
                // ^^^ !!! Consider .txrx_req == 0, we fall in here...

                //save the request
                temp_req = states[i].req;
                MXC_FreeLock((uint32_t *)&states[i].req);
                // Callback if not NULL
                if (temp_req->completeCB != NULL) {
                    temp_req->completeCB(temp_req, E_NO_ERROR);
                }
                if (states[i].mtMode == 0) {
                    // ^^^ !!! Always seems to be set to 0...  Blindly releasing both channels

                    // release any acquired DMA channels
                    if (states[i].channelTx >= 0) {
                        MXC_DMA_RevA_ReleaseChannel(states[i].channelTx);
                        states[i].channelTx = E_NO_DEVICE;
                    }
                    if (states[i].channelRx >= 0) {
                        MXC_DMA_RevA_ReleaseChannel(states[i].channelRx);
                        states[i].channelRx = E_NO_DEVICE;
                    }
                }
                break;
            }
        }
    }
}

Checklist Before Requesting Review

• [ ] PR Title follows correct guidelines.
• [ ] Description of changes and all other relevant information.
• [ ] (Optional) Link any related GitHub issues using a keyword
• [ ] (Optional) Provide info on any relevant functional testing/validation. For API changes or significant features, this is not optional.

[Jake-Carter]

@kenan-balci please test this fix against your findings from MSDK-1264

[kenan-balci]

@Jake-Carter My tests failed under some conditions.

• This update succeeds in 3-wire SPI mode when the received byte count is smaller than 32 bytes.
• In 3-wire SPI mode, when the received byte count is greater than 32 bytes, the first 32 bytes are received correctly; the next bytes cannot be received.
• Standard SPI mode tests succeeded. Code and output error are below: main.txt

[Jake-Carter]

Thanks for the detailed tests @kenan-balci. Failing at 32 bytes is suspicious in that it's exactly the depth of the SPI FIFO.

I notice in the test code that the master (RX) transaction is started after the slave (TX). I wonder if the overhead for setting up the master DMA transaction simply longer than the TX transmission time. If that were the case, then data beyond the depth of the FIFO would be missed because the FIFO is completely full.

Can you try launching the RX side before the TX? ex:

// ...
        // Slave sends - Master receives
        master_req.txLen = 0;
        master_req.rxLen = DATA_LEN;
        slave_req.txLen = DATA_LEN;
        slave_req.rxLen = 0;

        /***** Perform Transaction *****/
        MXC_GPIO_OutClr(gpio_p0_7.port, gpio_p0_7.mask);

#if MASTERDMA
        MXC_DMA_ReleaseChannel(0);
        MXC_DMA_ReleaseChannel(1);

        NVIC_EnableIRQ(DMA0_IRQn);
        NVIC_EnableIRQ(DMA1_IRQn);
        MXC_SPI_MasterTransactionDMA(&master_req, MXC_DMA0); // Launch RX to prepare for TX data
#endif

        MXC_SPI_SlaveTransactionAsync(&slave_req); // Launch TX

#if MASTERSYNC
    MXC_SPI_MasterTransaction(&master_req);
#endif

#if MASTERDMA
    while (DMA_FLAG == 0) {}
    DMA_FLAG = 0;
#endif
// ...

[kenan-balci]

@Jake-Carter Changing the test code did not work. Sending more than 32 bytes succeeds in the MASTERSYNC configuration, but it fails with MASTERDMA.

[Jake-Carter]

Strange... it seems like data past 32 bytes is at least being read in, but now it's corrupted...

Are you able to capture a logic analyzer trace of the pins? I think the slave should not start transmitting data until the master drives both SS and SCK low. The slave's data transmission should be completely clocked out by SCK. Considering this I guess it would make sense that the second test is corrupted, but I'm not sure why it would stop after the FIFO depth for the first test...

It would be helpful to see what's happening on the bus to continue troubleshooting

[kenan-balci]

@Jake-Carter Logic analyzer output for 3 wire SPI receive op. with DMA: Logic analyzer output for a 3-wire SPI receive op. without DMA: It seems that the slave sends incorrect data after 32 bytes. Master is reading the values it receives correctly. It's a strange bug, I'll try to debug it when I have time.

[github-actions[bot]]

This pull request is stale because it has been open for 30 days with no activity. Remove stale label, commit, or comment or this will be closed in 7 days.

[github-actions[bot]]

This pull request is stale because it has been open for 30 days with no activity. Remove stale label, commit, or comment or this will be closed in 7 days.

[github-actions[bot]]

This pull request was closed because it has been stalled for 7 days with no activity.