Enable nxp MICFIL driver and Dialog DA7212 driver #96367
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,22 @@ | ||
| # Copyright 2025 NXP | ||
| # | ||
| # SPDX-License-Identifier: Apache-2.0 | ||
|
|
||
| config AUDIO_DMIC_NXP_MICFIL | ||
| bool "NXP MICFIL driver" | ||
| default y | ||
| depends on DT_HAS_NXP_MICFIL_ENABLED | ||
| select HAS_MCUX | ||
| select CLOCK_CONTROL | ||
| help | ||
| Enable NXP MICFIL driver. | ||
|
|
||
|
There was a problem hiding this comment. Please refer everywhere as DMIC PDM to avoid confusion with MICFIL PDM. There was a problem hiding this comment. renamed the driver to micfil, please review |
||
| if AUDIO_DMIC_NXP_MICFIL | ||
|
|
||
| config DMIC_NXP_MICFIL_QUEUE_SIZE | ||
| int "Message queue depth" | ||
| default 4 | ||
| help | ||
| Depth of the message queue used to pass filled buffers to the app. | ||
|
|
||
| endif # AUDIO_DMIC_NXP_MICFIL | ||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Please add a link to the reference manual so reviewers can analyze the hardware IP.
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.
I will add it when i back to the office, but i'd like to know where the link should be added? I noticed that you made a comment in the CMakeLists file, so do I need to put link in the CMakeLists file? This seems rather strange.
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.
The link should be added in the commit message.
Also, this PR is a mix of everything adding a codec, adding a dmic driver, enhancements to samples, whitespace fixes to DMIC.
This is a nightmare to review. Let's pause a bit and organize everything in a more logical way so that we can have good code quality and have meaningful reviews.
Let's split this into multiple PRs like this:
PR1: Add support for Dialog7212 codec driver
This will include following patches:
ffa63a5 samples: i2s_codec: enable i2s_codec samples for frdm_mcxn236
6f15a7f boards: frdm_mcxn236: Enable da7212 on frdm_mcxn236
3432318 drivers: audio: Add dialog7212 driver
611f84a dts: bindings: add dialog vendor prefix
PR2: Small enhancements to audio
d7fc5b1 samples: i2s_codec: Refine sample for samll RAM platforms
af1fa22 audio: dmic: Format dmic.h
PR3: Add NXP DMIC MICFIL support
1449a5ed4 samples: i2s_codec: Enable nxp micfil test
d5e9f21 board: frdm_mcxn236: Enable MICFIL on frdm_mcxn236
a2a730b drivers: mcux_syscon: enable MICFIL clock control
e1b8489 drivers: audio: Add NXP MICFIL driver
This 3 PRs cand be sent separately are independent and can be reviewed in parallel. We go merge PR2 very fast.
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.
One thing to be careful of is the "refine sample for small RAM platforms".
It is unclear to me which set of enablement needed this fix. If it was needed for the da7212 enablement then put it before that set of commits. If needed for the MICFIL support then it is in the right place based on the recommendation.
The high-level guidance here is that each commit needs to stand on its own and be bisectable. There should be a logical grouping of commits. And don't overload a PR with a bunch of significant enablement because of the challenge it puts on the reviewers.
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.
"refine sample for small RAM platforms" should placed before i2s_codec example enablement for DA7212 and MICFIL, that is before ffa63a5 and 2c1449a.
From a technical/logical perspective, the commit sequence in this PR is well-structured and bisectable:
However, I recognize my oversight regarding reviewer burden. I will split this PR, please move your comments to new PRs