-
Notifications
You must be signed in to change notification settings - Fork 19
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[QUESTIONS]projects:ad469x_iio: Added SPI DMA Support #85
base: main
Are you sure you want to change the base?
Conversation
Added SPI DMA support for the AD4696 Signed-off-by: Janani Sunil <[email protected]>
#if (INTERFACE_MODE == SPI_DMA) | ||
/* If SPI_DMA is enabled, this pwm is also | ||
* also used to trigger a spi tx dma transaction. | ||
* */ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
so cnv triggers tm8 in output compare mode with bytes per sample pulses (tim8_config)
but the pmw_desc is not used. (pmw_enable never called)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
return ret; | ||
} | ||
|
||
ret = no_os_pwm_disable(tx_trigger_desc); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
where is tx_trigger_desc enabled?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tx trigger should not be manually enabled, as it is being operated in slave mode, where the CNV timer (timer 1 channel 3) acts as a master.
}; | ||
|
||
/* AD469x DMA Descriptor */ | ||
struct no_os_dma_desc* ad469x_dma_desc; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
where is this used?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is not needed
#if (INTERFACE_MODE == SPI_DMA) | ||
/* PWM descriptor for controlling the CS pulse. | ||
*/ | ||
struct no_os_pwm_desc *cs_desc; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
where is this used?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is not needed
HAL_Init(); | ||
SystemClock_Config(); | ||
#if (INTERFACE_MODE == SPI_DMA) | ||
/* If SPI_DMA mode is enabled, we don't need DMA initialization |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you mean the opposite here... right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, you are right, its a typo here.
{ | ||
#if (INTERFACE_MODE == SPI_DMA) | ||
TIM1->EGR = TIM_EGR_UG;// Generate update event | ||
TIM1->CR2 = (TIM_CR2_MMS_COMPARE_PULSE * |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
multiplication for bit operations?
|
||
ret = no_os_dma_xfer_abort(sdesc->dma_desc, sdesc->rxdma_ch); | ||
if (ret) { | ||
return ret; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
return int when return type for function is void.
#endif | ||
} | ||
|
||
void tim8_config(void) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
these register mainpulations should be on drivers/platform/stm32/*?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A few bitfield configurations aren't really handled in the platform driver layer. Also, since the DMA implementation is timing critical, we would want to keep it as in direct reg acces mode
#define SAMPLING_RATE (62500) | ||
#define CONV_TRIGGER_DUTY_CYCLE_NSEC(x) (x / 10) | ||
#else | ||
#define SAMPLING_RATE (500000) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
was this tested with 1msps? or only up to 500ksps?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This has been tested only upto 500ksps
#define AD469x_DMA_NUM_CHANNELS 2 | ||
|
||
/* PWM configuration for 22.5MHz SPI clock */ | ||
#define TX_TRIGGER_PERIOD 406 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
so CS goes low in 300ns, stays low for 1700ns. (2000ns sample rate)
tx pulse at 406ns, and 812ns (2 bytes - 2 pulses per timer event)
is this correct?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes
Added SPI DMA support for the AD4696