SDK 2.11: fsl_sai fifo combine mode on Rx has several bugs

cancel
Showing results for 
Show  only  | Search instead for 
Did you mean: 

SDK 2.11: fsl_sai fifo combine mode on Rx has several bugs

1,289 Views
caleb
Contributor III

There are a few bugs in the fsl_sai.c and fsl_sai_edma.c when using combine mode.

Attached are two patches that fix the issues I found:

the file 0001-fsl_sai_edma.c-SAI_TransferReceiveLoopEDMA-and-SAI_T.patch  fixes fsl_sai.c to properly set TCR3 and RCR3 in SAI_TransferSendLoopEDMA and  SAI_TransferReceiveLoopEDMA respectively.

specifically: 

- base->TCR3 |= I2S_TCR3_TCE(1UL << handle->channel);
+ base->TCR3 |= I2S_TCR3_TCE(handle->channelMask);

and

- base->RCR3 |= I2S_RCR3_RCE(1UL << handle->channel);
+ base->RCR3 |= I2S_RCR3_RCE(handle->channelMask);

 

the 0002-fsl_sai-fsl_sai_edma-incorrect-settings-for-combine-.patch fixes a fundamental incorrect assumption in fsl_sai.h where it assumes that FCOMB meanings are the same between TCR4.FCOMB and RCR4.FCOMB.  The reality is that the meanings of Read and Write have different values in different registers.  This necessitates changing fsl_sai.h to account for that.

Specifically, add the following defs in fsl_sai.h

-/*! @brief sai fifo combine mode definition */
-typedef enum _sai_fifo_combine
+/*! @brief sai fifo combine mode definition for RCR4 */
+typedef enum _sai_tx_fifo_combine
{
- kSAI_FifoCombineDisabled = 0U, /*!< sai fifo combine mode disabled */
- kSAI_FifoCombineModeEnabledOnRead, /*!< sai fifo combine mode enabled on FIFO reads */
- kSAI_FifoCombineModeEnabledOnWrite, /*!< sai fifo combine mode enabled on FIFO write */
- kSAI_FifoCombineModeEnabledOnReadWrite, /*!< sai fifo combined mode enabled on FIFO read/writes */
+ kSAI_FifoCombineTxDisabled = 0U, /*!< sai fifo combine mode disabled */
+ kSAI_FifoCombineTxModeEnabledOnRead, /*!< sai fifo combine mode enabled on FIFO reads */
+ kSAI_FifoCombineTxModeEnabledOnWrite, /*!< sai fifo combine mode enabled on FIFO write */
+ kSAI_FifoCombineTxModeEnabledOnReadWrite, /*!< sai fifo combined mode enabled on FIFO read/writes */
+} sai_tx_fifo_combine_t;
+/*! @brief sai fifo combine mode definition for RCR4 */
+typedef enum _sai_rx_fifo_combine
+{
+ kSAI_FifoCombineRxDisabled = 0U, /*!< sai fifo combine mode disabled */
+ kSAI_FifoCombineRxModeEnabledOnWrite, /*!< sai fifo combine mode enabled on FIFO write */
+ kSAI_FifoCombineRxModeEnabledOnRead, /*!< sai fifo combine mode enabled on FIFO read */
+ kSAI_FifoCombineRxModeEnabledOnReadWrite, /*!< sai fifo combined mode enabled on FIFO read/writes */
+} sai_rx_fifo_combine_t;
+typedef union {
+ sai_tx_fifo_combine_t tx; // used only for tx fifos
+ sai_rx_fifo_combine_t rx; // used only for rx fifos

update fsl_sai.c accordingly:

- tcr4 |= I2S_TCR4_FCOMB(config->fifoCombine);
+ tcr4 |= I2S_TCR4_FCOMB(config->fifoCombine.tx);

- rcr4 |= I2S_RCR4_FCOMB(config->fifoCombine);
+ rcr4 |= I2S_RCR4_FCOMB(config->fifoCombine.rx);

and finally update fsl_sai_edma.c as well:

 assert(
(saiConfig->channelNums <= 1U) ||
- ((saiConfig->channelNums > 1U) && ((saiConfig->fifo.fifoCombine == kSAI_FifoCombineModeEnabledOnWrite) ||
- (saiConfig->fifo.fifoCombine == kSAI_FifoCombineModeEnabledOnReadWrite))));
+ ((saiConfig->channelNums > 1U) && ((saiConfig->fifo.fifoCombine.tx == kSAI_FifoCombineTxModeEnabledOnWrite) ||
+ (saiConfig->fifo.fifoCombine.tx == kSAI_FifoCombineTxModeEnabledOnReadWrite))));

assert(
(saiConfig->channelNums <= 1U) ||
- ((saiConfig->channelNums > 1U) && ((saiConfig->fifo.fifoCombine == kSAI_FifoCombineModeEnabledOnRead) ||
- (saiConfig->fifo.fifoCombine == kSAI_FifoCombineModeEnabledOnReadWrite))));
+ ((saiConfig->channelNums > 1U) && ((saiConfig->fifo.fifoCombine.rx == kSAI_FifoCombineRxModeEnabledOnRead) ||
+ (saiConfig->fifo.fifoCombine.rx == kSAI_FifoCombineRxModeEnabledOnReadWrite))));

The attached patches apply to the 2.11 SDK.

Of course, any use code that references combine mode will need updating a bit. 

0 Kudos
5 Replies

1,251 Views
kerryzhou
NXP TechSupport
NXP TechSupport

Hi @caleb ,

  About item 1, our AE also confirm it, so I will report it to our SDK team directly, modify:

status_t SAI_TransferSendLoopEDMA(I2S_Type *base, sai_edma_handle_t *handle, sai_transfer_t *xfer, uint32_t loopTransferCount)

{

...

base->TCR3 |= I2S_TCR3_TCE(handle->channelMask);

...

}

 

status_t SAI_TransferReceiveLoopEDMA(I2S_Type *base, sai_edma_handle_t *handle, sai_transfer_t *xfer,
uint32_t loopTransferCount)

{

...

base->RCR3 |= I2S_RCR3_RCE(handle->channelMask);

...

}

Thanks for your contribution.

Best Regards,

Kerry

 

0 Kudos

1,252 Views
kerryzhou
NXP TechSupport
NXP TechSupport

Hi @caleb ,

  Thanks for your updated information.

  I have checked your mentioned point:

1. set TCR3 and RCR3 FIFO

My understand is, when use several channel, it should use handle->channelMask:

base->RCR3 |= I2S_RCR3_RCE(handle->channelMask);
base->TCR3 |= I2S_TCR3_TCE(handle->channelMask);

In fact, even to the 1UL << handle->channel, if the 1UL << handle->channel can matches to the TCE or RCE, it's also OK. Just make sure handle->channel indicate all your enabled channels.

Anyway, I will check with our AE, then give you reply later.

 

2. TCR4.FCOMB and RCR4.FCOMB

I don't think, you need to modify the driver, as you totally can seperately define the tx and rx config:

sai_transceiver_t SAI2_Tx_config = {

.fifo.fifoCombine = xxx,

}

sai_transceiver_t SAI2_Rx_config = {

.fifo.fifoCombine = xxx,

}

Then use the related calling code:

SAI_TransferTxSetConfigEDMA(SAI2_PERIPHERAL, &SAI2_SAI_Tx_eDMA_Handle, &SAI2_Tx_config);
/* Configures SAI Rx sub-module functionality */
SAI_TransferRxSetConfigEDMA(SAI2_PERIPHERAL, &SAI2_SAI_Rx_eDMA_Handle, &SAI2_Rx_config);

Now, you don't need to modify the  fsl_sai.h, fsl_sai.c, etc.

If you check the CFG sai generated code, you will find, that code also set the TX and RX seperately.

 

Wish it helps you!

Best Regards,

Kerrry

 

0 Kudos

1,161 Views
caleb
Contributor III

2. TCR4.FCOMB and RCR4.FCOMB

I don't think, you need to modify the driver, as you totally can seperately define the tx and rx config:

The name kSAI_FifoCombineModeEnabledOnWrite (currently set to 2) has no meaning.

 

If you're in TX mode, then the correct EnabledOnWrite == 2.  If you're in RX mode the correct , EnabledOnWrite == 1, as shown in the patch.

kSAI_FifoCombineTxModeEnabledOnRead = 1
kSAI_FifoCombineTxModeEnabledOnWrite = 2
kSAI_FifoCombineRxModeEnabledOnWrite = 1
kSAI_FifoCombineRxModeEnabledOnRead = 2

If you look at the two asserts in the fsl_sai_edma driver code:

 assert(
(saiConfig->channelNums <= 1U) ||
((saiConfig->channelNums > 1U) && ((saiConfig->fifo.fifoCombine == kSAI_FifoCombineModeEnabledOnWrite) ||
(saiConfig->fifo.fifoCombine == kSAI_FifoCombineModeEnabledOnReadWrite))));

assert(
(saiConfig->channelNums <= 1U) ||
((saiConfig->channelNums > 1U) && ((saiConfig->fifo.fifoCombine == kSAI_FifoCombineModeEnabledOnRead) ||
(saiConfig->fifo.fifoCombine == kSAI_FifoCombineModeEnabledOnReadWrite))));

This simplifies to

assert(
(saiConfig->channelNums <= 1U) ||
((saiConfig->channelNums > 1U) && ((saiConfig->fifo.fifoCombine == 2) ||
(saiConfig->fifo.fifoCombine == 3))));

assert(
(saiConfig->channelNums <= 1U) ||
((saiConfig->channelNums > 1U) && ((saiConfig->fifo.fifoCombine == 1) ||
(saiConfig->fifo.fifoCombine == 3))));

HOWEVER THE CORRECT CODE IS:

assert(
(saiConfig->channelNums <= 1U) ||
((saiConfig->channelNums > 1U) && ((saiConfig->fifo.fifoCombine == 2) ||
(saiConfig->fifo.fifoCombine == 3))));

assert(
(saiConfig->channelNums <= 1U) ||
((saiConfig->channelNums > 1U) && ((saiConfig->fifo.fifoCombine == 2) ||
(saiConfig->fifo.fifoCombine == 3))));

The current EDMA code CANNOT work no matter what configuration code you pass it if you wish to use fifo combine mode in the RX pathway.

 

If you don't believe me, please try it in actual hardware.  you will see for yourself.  Then apply my patch, and see it magically work.  

 

Thanks,

 -Caleb

 

0 Kudos

1,277 Views
kerryzhou
NXP TechSupport
NXP TechSupport

Hi @caleb ,

  Thank you for your interest in the NXP MIMXRT product, I would like to provide service for you.
Please tell me which RT chip you are using, and which detail SDK version.
Eg: SDK_2_11_1_EVKB-IMXRT1050
Then I can help to check the details, if it is really having issues, I will report to our SDK team internally.
Please help to confirm the chip and SDK version at first, thanks.

 

Best Regard,

Kerry

0 Kudos

1,259 Views
caleb
Contributor III

Hi @kerryzhou ,  It's sdk-2.11.1.MIMXRT1060_EVK.

 

Thanks,

 -Caleb

 

0 Kudos