-
Notifications
You must be signed in to change notification settings - Fork 147
mcux-sdk: edma: support memory offset for different master and correct ELINKNO #528
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
base: master
Are you sure you want to change the base?
Conversation
@@ -8,6 +8,9 @@ | |||
#include <stdbool.h> | |||
|
|||
#include "fsl_edma_rev2.h" |
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.
Can you add a short description for this commit explaining what is the issue this is trying to tackle?
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.
sure, have updated
91d4818
to
8e2196d
Compare
I'm fine with this but keep in mind to separate patches per logical change. Once you feel the need to add a list of things that this commit is doing that's a clear sign that you need to split the PR into multiple patches. I won't insist on this one because it looks pretty simple and straightforward. |
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.
very good catches, thank you! can you please add a commit description detailing the address translation issue? I think it's extremely important to have this logged for future reference.
@@ -74,7 +74,7 @@ extern "C" { | |||
#define EDMA_TCD_ATTR_DSIZE(x) ((x) & EDMA_TCD_ATTR_SSIZE_DSIZE_MASK) | |||
#define EDMA_TCD_ATTR_SSIZE(x) (((x) & EDMA_TCD_ATTR_SSIZE_DSIZE_MASK) << 8) | |||
|
|||
#define EDMA_TCD_CITER_ELINKNO_MASK 0xf |
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.
good catch, thx! mind also doing this change for EDMA_TCD_BITER_ELINKNO_MASK?
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.
sure, have updated
@@ -8,6 +8,9 @@ | |||
#include <stdbool.h> | |||
|
|||
#include "fsl_edma_rev2.h" | |||
#if defined FSL_FEATURE_MEMORY_HAS_ADDRESS_OFFSET && FSL_FEATURE_MEMORY_HAS_ADDRESS_OFFSET | |||
#include "fsl_memory.h" |
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.
would you mind making this e separate commit with a more detailed description? I'd be interested in learning more about this.
is this related to the fact that DTCM is mapped at different addresses for M7 and EDMA and thus we require a translation? if so, please mention this in the commit description.
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.
@@ -237,6 +240,11 @@ status_t EDMA_ConfigureTransfer(edma_config_t *cfg, int channel, | |||
break; | |||
} | |||
|
|||
#if defined FSL_FEATURE_MEMORY_HAS_ADDRESS_OFFSET && FSL_FEATURE_MEMORY_HAS_ADDRESS_OFFSET | |||
saddr = MEMORY_ConvertMemoryMapAddress((uint32_t)(saddr), kMEMORY_Local2DMA); |
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.
might be useful to add a comment here stating that translation will only be performed if saddr/daddr are from DTCM
Can you clarify which platforms are impacted by this? The changes are made to the legacy driver side of things and not SDK-NG Also, there isn't a Zephyr side PR so how or when do you need this integrated? |
- This api convert the address between system mapped address and native mapped address. - There maybe offset between subsystem native address and system address for some memory (such as iMX95 DTCM) Signed-off-by: Yongxu Wang <yongxu.wang@nxp.com>
Signed-off-by: Yongxu Wang <yongxu.wang@nxp.com>
Signed-off-by: Yongxu Wang <yongxu.wang@nxp.com>
8e2196d
to
57e9a25
Compare
hi @dleach02 , this patch is for dma_nxp_edma driver, and mainly to support m2m test case for imx95 board, the another pull request is zephyrproject-rtos/zephyr#88748. |
No description provided.