Skip to content
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.

Sign up for GitHub

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

Jump to bottom

drivers: dma: Introduce driver for NXP's eDMA IP #65671

Merged
merged 2 commits into from Jan 23, 2024
Merged

drivers: dma: Introduce driver for NXP's eDMA IP #65671

merged 2 commits into from Jan 23, 2024

Conversation

LaurentiuM1234
Copy link

@LaurentiuM1234 LaurentiuM1234 commented Nov 23, 2023 •
edited

Please check commit c63a3e7's description for reason why this driver is used instead of dma_mcux_edma.

@zephyrbot
Copy link
Collaborator

zephyrbot commented Nov 23, 2023 •
edited

The following west manifest projects have been modified in this Pull Request:

Name Old Revision New Revision Diff
hal_nxp zephyrproject-rtos/hal_nxp@9b2693c zephyrproject-rtos/hal_nxp@d6d43ca (master) zephyrproject-rtos/hal_nxp@9b2693c7..d6d43cab

Note: This message is automatically posted and updated by the Manifest GitHub Action.

@zephyrbot zephyrbot added manifest manifest-hal_nxp DNM This PR should not be merged (Do Not Merge) labels Nov 23, 2023
@LaurentiuM1234 LaurentiuM1234 marked this pull request as ready for review November 23, 2023 12:19
@zephyrbot zephyrbot added platform: NXP NXP area: Devicetree Binding PR modifies or adds a Device Tree binding area: DMA Direct Memory Access labels Nov 23, 2023
@teburd
Copy link
Collaborator

teburd commented Nov 23, 2023

I’m ok with having another eDMA, what I did with the designware dma is try to create a common set of functions and reuse them where possible as there are many users of the designware gpdma. It would be nice if eDMA did something similar if possible. Like if there’s common transfer block definitions and configuration registers for example some helpers could be made to reuse between the two.

@LaurentiuM1234
Copy link
Author

I’m ok with having another eDMA, what I did with the designware dma is try to create a common set of functions and reuse them where possible as there are many users of the designware gpdma. It would be nice if eDMA did something similar if possible. Like if there’s common transfer block definitions and configuration registers for example some helpers could be made to reuse between the two.

ACK. If everyone's ok with this, I'd like to have this in first as-is before making such clean-ups. It would make it a bit easier for me and I'm thinking it would also ease the review process a bit.

@LaurentiuM1234 LaurentiuM1234 mentioned this pull request Nov 24, 2023
Merged
@LaurentiuM1234
Copy link
Author

V4 updates

  1. Changed naming to dma_nxp_edma. No functional change.

@LaurentiuM1234
Copy link
Author

@DerekSnell could you please review this again? Thanks!

@LaurentiuM1234 LaurentiuM1234 mentioned this pull request Jan 17, 2024
Merged
dbaluta
dbaluta previously approved these changes Jan 17, 2024
View reviewed changes
decsny
decsny previously requested changes Jan 17, 2024
View reviewed changes
Copy link
Member

@decsny decsny left a comment

Choose a reason for hiding this comment

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

I do not think it is efficicient to maintain 2 EDMA drivers if it's not necessary. Can you please be more concrete about why we cannot reuse code for the legacy EDMA versions, because I do not understand from your comments.

@decsny decsny dismissed their stale review January 17, 2024 20:12

removed for internal discussion about edma roadmap

DerekSnell
DerekSnell previously requested changes Jan 17, 2024
View reviewed changes
Copy link
Contributor

@DerekSnell DerekSnell left a comment

Choose a reason for hiding this comment

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

Hi @LaurentiuM1234,

Thank you for updating this PR. There are still some references to i.MX I want removed, since this driver could be used on other SOCs. But once you remove those, I will remove my block. Thank you

drivers/dma/Kconfig.nxp_edma Outdated Show resolved Hide resolved
dts/bindings/dma/nxp,edma.yaml Outdated Show resolved Hide resolved
drivers/dma/Kconfig.nxp_edma Outdated Show resolved Hide resolved
drivers/dma/dma_nxp_edma.c Outdated Show resolved Hide resolved
drivers/dma/dma_nxp_edma.h Outdated Show resolved Hide resolved
dts/bindings/dma/nxp,edma.yaml Outdated Show resolved Hide resolved
dts/bindings/dma/nxp,edma.yaml Outdated Show resolved Hide resolved
@LaurentiuM1234
Copy link
Author

Hi @LaurentiuM1234,

Thank you for updating this PR. There are still some references to i.MX I want removed, since this driver could be used on other SOCs. But once you remove those, I will remove my block. Thank you

Should be fixed now

dbaluta
dbaluta previously approved these changes Jan 18, 2024
View reviewed changes
@LaurentiuM1234 LaurentiuM1234 mentioned this pull request Jan 18, 2024
Merged
iuliana-prodan
iuliana-prodan reviewed Jan 18, 2024
View reviewed changes
drivers/dma/dma_nxp_edma.c Show resolved Hide resolved
drivers/dma/dma_nxp_edma.c Outdated Show resolved Hide resolved
drivers/dma/dma_nxp_edma.c Show resolved Hide resolved
iuliana-prodan
iuliana-prodan previously approved these changes Jan 18, 2024
View reviewed changes
@DerekSnell DerekSnell dismissed their stale review January 18, 2024 22:23

Thank you @LaurentiuM1234, for addressing my comments. I have removed my block

Laurentiu Mihalcea added 2 commits January 22, 2024 22:45
manifest: Bump up hal_nxp revision
3892e6e 
Bump up hal_nxp revision to contain the EDMA REV2-related
changes from NXP HAL.

Signed-off-by: Laurentiu Mihalcea <laurentiu.mihalcea@nxp.com>
drivers: dma: Introduce driver for NXP's eDMA IP
7f4b007 
This commit introduces a driver for NXP's eDMA IP.

The main reasons for introducing a new driver are the following:

	1) The HAL EDMA wrappers don't support well different
	eDMA versions (e.g: i.MX93 and i.MX8QM). As such, a new
	revision had to be introduced, thus requiring a new Zephyr
	driver.

	2) The eDMA versions found on i.MX93, i.MX8QM, and i.MX8QXP
	don't use the DMAMUX IP (instead, channel MUX-ing is performed
	through an eDMA register in the case of i.MX93).

Signed-off-by: Laurentiu Mihalcea <laurentiu.mihalcea@nxp.com>
@zephyrbot zephyrbot added platform: NXP Drivers NXP Semiconductors, drivers and removed DNM This PR should not be merged (Do Not Merge) labels Jan 22, 2024
@LaurentiuM1234
Copy link
Author

dependency merged, should be good to go as well once CI is green

@dbaluta
Copy link
Collaborator

dbaluta commented Jan 23, 2024

@teburd @dleach02 good to merge. dependency merged and SHA updated.

@nashif nashif merged commit 6abc592 into zephyrproject-rtos:main Jan 23, 2024
22 of 24 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dbaluta dbaluta dbaluta approved these changes

@teburd teburd teburd approved these changes

@iuliana-prodan iuliana-prodan iuliana-prodan approved these changes

@DerekSnell DerekSnell DerekSnell left review comments

@decsny decsny decsny left review comments

@mmahadevan108 mmahadevan108 Awaiting requested review from mmahadevan108 mmahadevan108 is a code owner

@dleach02 dleach02 Awaiting requested review from dleach02 dleach02 is a code owner

@bperseghetti bperseghetti Awaiting requested review from bperseghetti

@danieldegrasse danieldegrasse Awaiting requested review from danieldegrasse

@Dat-NguyenDuy Dat-NguyenDuy Awaiting requested review from Dat-NguyenDuy

@EmilioCBen EmilioCBen Awaiting requested review from EmilioCBen

@galak galak Awaiting requested review from galak

@manuargue manuargue Awaiting requested review from manuargue

@PetervdPerk-NXP PetervdPerk-NXP Awaiting requested review from PetervdPerk-NXP

@yvanderv yvanderv Awaiting requested review from yvanderv

Assignees

@teburd teburd

Labels
area: Devicetree Binding PR modifies or adds a Device Tree binding area: DMA Direct Memory Access manifest manifest-hal_nxp platform: NXP Drivers NXP Semiconductors, drivers platform: NXP NXP
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
9 participants
@LaurentiuM1234 @zephyrbot @teburd @danieldegrasse @dbaluta @iuliana-prodan @DerekSnell @decsny @nashif