LeafLabs & Friends,
A few weeks ago I put in a pull request that enhanced the DMA Interrupt handler interface and removed unnecessary ISR clears. This pull request was denied & closed because it modifies an existing interface signature without a deprecation period -- which breaks a promise made as of 0.0.10's release.
I've spent some time making the changes mbolivar suggested in the pull request and commit comments, as well as rewriting the changes to allow for a deprecation period -- though I have not committed or pushed any changes to github yet. Throughout making the modifications I had trouble figuring out an elegant way of implementing them. I recently was on a week long camping trip, and while staring at a beautiful camp fire I had a realization: A wirish DMA class would be very handy. The proposed changes I made in my initial pull request would still be great to have, however they can be delayed until a later release to allow for a deprecation period where a non-elegant temporary "hack" is introduced to facilitate a transition.
Because I have little experience with writing publicly used APIs, I'd like to share what I have planned for feedback on better practices and use this thread for brainstorming.
Like mbolivar mentioned in the pull request comments, storing the latest
dma_irq_cause within the handler config is probably the best option. This cause can then be accessed via a getter function, i.e. the existing
dma_get_irq_cause. This will allow the current interface to remain the same and not break any existing applications that utilize dma interrupt handlers, while giving benefit to removing unnecessary
dma_clear_isr_bit calls. The irq cause can be set within the handler dispatcher, similar to how my initial pull request handled it except that the config variable will be seat instead of passing it to the user's handler.
Changes Post-Deprecation Period
Once a deprecation period has passed, the
dma_irq_cause can then be passed to the handler & the signatures can be modified. In addition, the
dma_irq_cause from within the handler config can be removed as it is no longer needed (no deprecation period for this should be required as no one else but
dma.c should be accessing that variable).
dma_get_irq_cause can be reverted to its previous functionality of reading the isr bits & applying the masks, however it will no longer have the right of clearing the isr bits. This will be done separately within the dispatch handler.
More Elegant Solution
A more elegant solution would be to always set the interrupt enable bits for every channel configured. This will make the dispatcher trigger, allowing for the
dma_irq_cause to be set every time - even if no user handler is setup. This would add "overhead" to uses where no handlers are used, however it would insure the correct irq cause is always available to the user. This method would use changes mentioned in the
Transitional Changes but would not require the
dma_get_irq_cause internal functionality to be reverted post-deprecation period, as the irq cause within the handler config would still be used.
The problem with this method is if the user modifies the CR2 bits manually, they may disable the interrupts which will break the irq_cause calculation for users not using the interrupt handlers.
Wirish DMA Class
Besides having getter/setter functionality for various aspects of the DMA controller settings, I feel the most important feature to have is the ability to very quickly reconfigure a particular DMA channel. This is handy because a particular channel can only serve to one peripheral at a time. I can see uses where a single channel can be reconfigured multiple times throughout the runtime of an application to communicate with multiple different peripherals on a single channel, for example communication with multiple different SPI devices, then being reconfigured to do UART transfer, etc. While different non-interfering channels should be used, you don't always get a choice making this a good feature to have.
The configuration should be handled via a struct, or a special configuration class, that contains the parameters you wish to use. An instance of this configuration struct/class can be made for each channel-peripheral combo you plan to use. Passing a reference to this struct/class to the DMA object will have it call the corresponding libmaple c functions and modify the DMA controller to these new settings. This will allow easy configuration between multiple peripherals/configurations on a single channel. Multiple levels of how complex the configuration should be can be used for improved performance. For example, only the # of transfers and memory addresses used, channel + full reconfiguration, etc. This will prevent every aspect of the DMA controller being changes when unnecessary.
As far as implementation details, the DMA class should represent one controller (DMAx, as per RM0008 notation) with
DMA2 pre-defined for the corresponding maple boards. The class, along with every Wirish class that represents a hardware peripheral, should be a singleton to prevent multiple instances and register changes between instances (shouldn't be a problem with good programming practices, but safety built-in rarely hurts).
Some things I have questions about best practices for:
- Should the user allocate the RAM & provide the DMA class a pointer to it, or allow the DMA class to manage the memory?
- If the DMA class should manage the memory, how should the user get access to it after peripheral to RAM transfer is completed? Give the user the raw pointer, or provide an iterator #define macro? (faster than a function, and "safer" than giving a raw pointer... ?)
That's about it for now. I'd love to hear what ya'll have to think. I really love my Maple and feel the DMA controller is a very important hardware feature and should be taken advantage of with an easy-to-use interface like
HardwareSerial makes SPI and Serial simple!
I'd love to start work on this, unless there' feedback this/these are bad ideas.