fix(Examples)!: Require DMA Channel as Input Argument for ADC DMA Conversions, Fix ADC DMA Transfer Bug for Channels > 0 #643
Conversation
Vignesh-VVadivel
requested review from
Jake-Carter,
sihyung-maxim and
Jacob-Scheiffler
| @@ -246,6 +246,7 @@ typedef struct { | |||
| mxc_adc_avg_t avg_number; ///< no of samples to average | |||
| mxc_adc_div_lpmode_t lpmode_divder; ///< Divide by 2 control in lpmode | |||
| uint8_t num_slots; ///< num of slots in the sequence | |||
| int8_t dma_channel; ///< DMA channel number for DMA transfer | |||
There was a problem hiding this comment.
We should consider updating this comment to say that users should call MXC_DMA_AcquireChannel to assign the value for this field.
There was a problem hiding this comment.
Updated that comment in the code.
| @@ -214,7 +214,7 @@ int MXC_ADC_RevB_StartConversionDMA(mxc_adc_revb_regs_t *adc, mxc_adc_conversion | |||
|
|
|||
| num_bytes = (req->num_slots + 1) * 4; //Support 8 slots (32 bytes) only. (TODO) | |||
|
|
|||
| channel = MXC_DMA_AcquireChannel(); | |||
| channel = req->dma_channel; | |||
There was a problem hiding this comment.
This change will break builds for the other chips which use ADC Rev. B. If we proceed with this implementation these changes will need to be applied to the ME12 and ME18 as well.
There was a problem hiding this comment.
Yup sure,
I updated these changes for both ME12 and ME18.
I have also implemented these changes for MAX78002, since it uses ADC Rev B.
| @@ -202,6 +202,7 @@ typedef struct { | |||
| mxc_adc_avg_t avg_number; ///< no of samples to average | |||
| mxc_adc_div_lpmode_t lpmode_divder; ///< Divide by 2 control in lpmode | |||
| uint8_t num_slots; ///< num of slots in the sequence | |||
| int8_t dma_channel; ///< User should call MXC_DMA_AcquireChannel() to assign DMA Channel number | |||
There was a problem hiding this comment.
I would suggest changing this comment to indicate that the field is ignored for non-DMA transactions.
///<The channel to use for DMA-enabled transactions. Users should call \ref MXC_DMA_AcquireChannel to assign the DMA channel number. For non-DMA transactions, this field is ignored.It looks like we also need to improve our Doxygen headers for the TransactionDMA function in general (see here). Can you improve the adc.h files to fix the following issues?
- The Doxygen for
mxc_adc_req_tis broken (no link/documentation generated) - Update the
MXC_ADC_StartConversionDMAfunction brief to inform the user that they must acquire a DMA channel and pass it in via the "req" input struct - Update the
MXC_ADC_StartConversionDMAfunction brief to inform the user that they must enable the correct DMA IRQ call theMXC_DMA_Handlerfunction from the ISR. - Add a "Detailed Description" section for the adc.h file
github-actions
bot
added
the
documentation
There was a problem hiding this comment.
Thanks @Vignesh-VVadivel,
Final task for you on this: mxc_adc_req_t and mxc_adc_slot_req_t are also missing Doxygen @brief sections for:
- MAX32662
adc.h - MAX32672
adc.h - MAX32690
adc.h - MAX78002
adc.h
We should add them so that these structs also show up in the structs section:
sihyung-maxim
changed the title
Jake-Carter
added
MAX32672
Jake-Carter
changed the title
github-actions
bot
removed
MAX32690
Jake-Carter
added
API Change
…versions, Fix ADC DMA Transfer Bug for Channels > 0 (analogdevicesinc#643)
…versions, Fix ADC DMA Transfer Bug for Channels > 0 (analogdevicesinc#643)
Fixed the DMA transfer over multiple channels for ADC example in the MAX32672 board, and the corresponding JIRA ticket is here
This PR contains the fix for all example codes that uses ADC Rev B, which includes MAX32662, MAX32672, MAX32690, MAX78002.