feat(Examples): Add generic I2C RX/TX DMA vector flexibility to I2C examples for all parts #693
Conversation
github-actions
bot
added
MAX32670
|
@sihyung-maxim Would the DMA i2c writes only occur once (regardless of the number of calls) because the states[i2cNum].writeDone flag is never reset in MXC_I2C_RevA_MasterTransactionDMA (i2c_reva.c)? |
Yes, you're correct. It should be cleared before the transaction starts. |
|
/clang-format-run |
github-actions
bot
added
MAX32520
sihyung-maxim
changed the title
sihyung-maxim
requested review from
lorne-maxim,
Jake-Carter and
Jacob-Scheiffler
|
|
||
| i2cNum = MXC_I2C_GET_IDX((mxc_i2c_regs_t *)i2c); | ||
|
|
||
| i2c->ctrl = 0; |
There was a problem hiding this comment.
Does setting the Reset bit in the GCR regs not clear all the I2C registers?
There was a problem hiding this comment.
No, unfortunately, not all registers are cleared when you set the reset bit in the GCR.
| int8_t i2cNum; | ||
| int8_t rxChannel; | ||
| int8_t txChannel; | ||
| mxc_dma_config_t rxConfig; |
There was a problem hiding this comment.
Might be worth considering adding either a parameter or another function to handle TX and RX separately. It seems like there are a fair number of applications which would either want to use DMA for TX or RX, but not necessarily both. If that is the case then it would be kind of silly to take two DMA channels when the user only needs one.
| states[i2cNum].channelTx = txChannel; | ||
| states[i2cNum].channelRx = rxChannel; | ||
|
|
||
| states[i2cNum].dma_initialized = true; |
There was a problem hiding this comment.
Consider just using the channelTx/Rx to determine if the DMA has been initialized. Like, "if(channelTx == E_NO_DEVICE)..." If you choose to handle TX and RX separately that would eliminate the need for a second boolean as well.
|
/clang-format-run |
| txChannel = MXC_DMA_AcquireChannel(); | ||
| #endif | ||
|
|
||
| txConfig.ch = txChannel; |
There was a problem hiding this comment.
Looks like txConfig and rxConfig are set but never used?
There was a problem hiding this comment.
They're used when setting the appropriate register settings, but I removed them.
| config.dstinc_en = 1; | ||
|
|
||
| srcdst.ch = channel; | ||
| srcdst.ch = states[i2cNum].channelRx; |
There was a problem hiding this comment.
Check channelRx/Tx is configured before using?
There was a problem hiding this comment.
The change is in there. GitHub bugged out and didn't show the changes from the latest commit.
Examples/MAX32572/I2C/main.c
Outdated
| // Serve as a 16 byte loopback, returning data*2 | ||
| for (int i = 0; i < I2C_BYTES; i++) { | ||
| Stxdata[i] = i; | ||
| Stxdata[i] = Srxdata[i]; |
There was a problem hiding this comment.
This is redundant since Stxdata is already initialized in main. Ditto for other I2C examples.
- Merge remote-tracking branch 'remotes/upstream/main' into feat/me15_i2c_dma
…xamples for all parts (#693) Co-authored-by: sihyung-maxim <[email protected]> Co-authored-by: Jake Carter <[email protected]>
Description
MXC_I2C_DMA_Init(...)to initialize the DMA and acquire DMA channels before callingMXC_I2C_MasterTransactionDMA(...). This allows you to set up generic DMA channel vectors during run-time rather than defining specific DMA channel interrupt handlers beforehand (not always going to acquire DMA channels 0 and 1).MXC_I2C_DMA_GetRXChannel(...)andMXC_I2C_DMA_GetTXChannel(...)to grab the acquired RX/TX channels for a specific I2C instance (if DMA is used).MXC_I2C_DMA_Init(...)to initialize the I2C DMA beforehand, then the `MXC_I2C_MasterTransactionDMA(...) function will initialize the DMA like how it was previously done. This is just an added option to set up generic DMA channel vectors since channels 0 and 1 won't always be acquired. These changes won't affect anyone's current projects if they choose to not use this option.Checklist Before Requesting Review