Update dma sdk with new macro definitions to handle different DMA parameters #563
Conversation
|
@JuanSapriza @JoseCalero can you please review this PR? |
…on strong inline)
There was a problem hiding this comment.
Left some comments and made a PR with some proposed changes. You can pick which ones to keep and which ones not.
here: LuigiGiuffrida98#1
| // from here we wake up even if we did not jump to the ISR | ||
| } | ||
| CSR_SET_BITS(CSR_REG_MSTATUS, 0x8); | ||
| volatile dma *the_dma = dma_peri(channel); |
There was a problem hiding this comment.
the_dma? hahaha- If
the_dmais being enforced to be volatile, having a separate function to start it does not make much sense. If it's only for clarity, maybe consider using__attribute__((always_inline))?
There was a problem hiding this comment.
Note that inline is a suggestion to the compiler only
There was a problem hiding this comment.
if we are having a separate function, maybe there is no need to have the volatile, allowing the compiler to accelerate a bit the copying of the data.
I think its a small detail. For what it concerns me, it can be left like this (maybe not with the_dma tho hahaha)
sw/device/lib/drivers/dma/dma.h
Outdated
| @@ -107,6 +107,9 @@ extern "C" { | |||
| */ | |||
| #define DMA_DATA_TYPE_2_SIZE(type) (0b00000100 >> (type) ) | |||
|
|
|||
| #define DMA_REGISTER_SIZE_BYTES sizeof(int) | |||
There was a problem hiding this comment.
This is re-defined in the .c. I dont think any of the two are necessary, as they are used only on the write_register function were we are already calling the peripheral as a uint32_t*, meaning that we are already forcing the 32 bits (even safer than using int)
|
This is still unstable, I'm still fixing it. |
|
this PR is functional? @LuigiGiuffrida98 @JuanSapriza |
|
Yes |
|
what is the status of this? @JuanSapriza |
|
@davideschiavone this PR should be ready to be reviewed and eventually merged. |
|
i will check it today and let you guys know, but was already good when I last saw it |
OK thx let me know |
|
Ok, i tested it and everything looks ok. Maybe one day @LuigiGiuffrida98 can document the sdk functions and/or add them to the documentation? :) |
No description provided.