Skip to content

mcux-sdk: edma: support memory offset for different master and correct ELINKNO#528

Closed
yongxu-wang15 wants to merge 3 commits intozephyrproject-rtos:masterfrom
nxp-upstream:mcu-sdk-support-memory-offest-for-edma-rev2-driver
Closed

mcux-sdk: edma: support memory offset for different master and correct ELINKNO#528
yongxu-wang15 wants to merge 3 commits intozephyrproject-rtos:masterfrom
nxp-upstream:mcu-sdk-support-memory-offest-for-edma-rev2-driver

Conversation

@yongxu-wang15
Copy link
Copy Markdown
Contributor

No description provided.

@@ -8,6 +8,9 @@
#include <stdbool.h>

#include "fsl_edma_rev2.h"
Copy link
Copy Markdown
Contributor

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?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sure, have updated

@yongxu-wang15 yongxu-wang15 force-pushed the mcu-sdk-support-memory-offest-for-edma-rev2-driver branch 2 times, most recently from 91d4818 to 8e2196d Compare April 17, 2025 07:43
@dbaluta
Copy link
Copy Markdown
Contributor

dbaluta commented Apr 17, 2025

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.

Copy link
Copy Markdown
Contributor

@LaurentiuM1234 LaurentiuM1234 left a 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.

#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
Copy link
Copy Markdown
Contributor

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?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sure, have updated


#include "fsl_edma_rev2.h"
#if defined FSL_FEATURE_MEMORY_HAS_ADDRESS_OFFSET && FSL_FEATURE_MEMORY_HAS_ADDRESS_OFFSET
#include "fsl_memory.h"
Copy link
Copy Markdown
Contributor

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.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, for examples In imx95, DTCM has different memory maps for M33 or M7core, which are defined in modules/hal/nxp/mcux/mcux-sdk/devices/MIMX9596, and other devices.
image

}

#if defined FSL_FEATURE_MEMORY_HAS_ADDRESS_OFFSET && FSL_FEATURE_MEMORY_HAS_ADDRESS_OFFSET
saddr = MEMORY_ConvertMemoryMapAddress((uint32_t)(saddr), kMEMORY_Local2DMA);
Copy link
Copy Markdown
Contributor

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

@dleach02
Copy link
Copy Markdown
Member

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>
@yongxu-wang15 yongxu-wang15 force-pushed the mcu-sdk-support-memory-offest-for-edma-rev2-driver branch from 8e2196d to 57e9a25 Compare April 22, 2025 03:19
@yongxu-wang15
Copy link
Copy Markdown
Contributor Author

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.
and I want to merge this pr after the another zephyr patch can be merged.

@mmahadevan108
Copy link
Copy Markdown
Collaborator

@yongxu-wang15 , please submit this patch to the SDK repo as shared earlier.

@mmahadevan108
Copy link
Copy Markdown
Collaborator

@yongxu-wang15 , any update on submitting this patch to the SDK team.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants