Enable nxp MICFIL driver and Dialog DA7212 driver #96367

ZhaoxiangJin · 2025-09-22T12:42:52Z

Enabled DA7212 driver
Tested da7212 with i2s_codec example (CONFIG_USE_DMIC=n, you can hear the sine wave from the codec.)
Enabled MICFIL driver
Tested MICFIL with i2s_codec example (CONFIG_USE_DMIC=y, you can hear the voice played from the on-board micphone)

According to the reviewer's suggestion, this PR has been split into the following PRs:

TODO: There may be some problems with the NXP port driver that cause the MICFIL CLOCK and DATA pinmux to not be configured successfully. The current workaround is to write to the registers directly. The root cause will be found and solved later.

dts/bindings/audio/nxp,pdm.yaml

dbaluta · 2025-09-23T07:14:48Z

include/zephyr/audio/dmic.h

 	 * @name Parameters common to all PDM controllers
 	 *@{
 	 */
 	/** Minimum clock frequency supported by the mic */


Please add more context to this change. E.g Remove whitespaces to make code easier to read and follow the common sense coding style.

Update in the latest pr, hope you have see the change

@ZhaoxiangJin Changes in this file are just whitespace changes, from what I see. I don't think you're supposed to reformat code (albeit rightfully) that you haven't changed in any other way.

Personally, I think consistency in indentation style is an important part of the code, so this commit is necessary.

dbaluta · 2025-09-23T07:15:54Z

drivers/audio/CMakeLists.txt

-zephyr_library_sources_ifdef(CONFIG_AUDIO_CODEC_SHELL	codec_shell.c)
+zephyr_library_sources_ifdef(CONFIG_AUDIO_CODEC_SHELL codec_shell.c)
 zephyr_library_sources_ifdef(CONFIG_AUDIO_DMIC_MCUX dmic_mcux.c)
 zephyr_library_sources_ifdef(CONFIG_AUDIO_CODEC_WM8904 wm8904.c)


This does more than the commit message says. So, please add the style changes in a separate patch.

A patch should do one single logical change.

Updated, please review the latest pr

dbaluta · 2025-09-23T07:16:37Z

drivers/audio/Kconfig.dmic_nxp_micfil

+	select CLOCK_CONTROL
+	help
+	  Enable NXP PDM driver.
+


Please refer everywhere as DMIC PDM to avoid confusion with MICFIL PDM.

renamed the driver to micfil, please review

dbaluta · 2025-09-23T07:17:25Z

drivers/audio/CMakeLists.txt

@@ -1,18 +1,19 @@
 # SPDX-License-Identifier: Apache-2.0

 zephyr_library()


Please add a reference to documentation where one can read the datasheet of the the device. Please mention chapter and provide a link to the doc

you mean add link to the CMakeLists?

VitekST · 2025-09-25T13:19:15Z

drivers/audio/dmic_nxp_pdm.c

+	}
+
+	/* NXP PDM PCM width is 24bit. */
+	if (!(stream->pcm_width == 24U)) {


Just a small nitpick here - can't the non-equality operator be used here instead of checking for equality and negating the condition additionally?

VitekST · 2025-09-25T16:51:01Z

drivers/audio/dmic_nxp_pdm.c

+					index, &pdm_idx, &lr);
+
+		/* Map (pdm_idx, lr) to hardware DATACH index: 2*pdm_idx + (R?1:0). */
+		uint8_t hw_chan = (uint8_t)(2U * pdm_idx + (lr == PDM_CHAN_RIGHT ? 1U : 0U));


Left-right channel mapping was abolished in 1f69b91 for the nxp,dmic driver (instantiated on the i.MX RT500 and RT600) - do we really want to retain that here? I personally think it's better for the user to be able to map channels as one desires without having to map 2 consecutive channels.

Of course, that raises a question if really the left/right distinction is needed in the dmic API.

I am not sure whether this broken this constraint, but one thing i am ensure is the new map strategy broken dmic_api test.
Hello @hakehuang, do you now whether current mimxrt595_evk run dmic_api test case well? Thanks.

Hello @hakehuang, do you now whether current mimxrt595_evk run dmic_api test case well? Thanks.

sure, let me kick a test

@ZhaoxiangJin it fails at my side on frdm_mcxn236, and mimxrt595 is ok

** Booting Zephyr OS build v4.2.0-4646-g2c1449a5ed40 *** codec sample PCM output rate: 16000, channels: 2 start streams Failed to write data: -11 error -11

@ZhaoxiangJin On a second glance, I think you have a point.

The mentioned constraint isn't worded with needed clarity, but the dmic_nrfx_pdm.c driver from Nordic seems to be written in such a way that the left/right distinction is indeed relevant - what it calls a "channel" doesn't map to a singular PCM stream, but rather to a pair of those, which are then distinguished with the enum pdm_lr values. Each hardware channel index corresponds conceptually to a PDM data line (of course, carrying two channels, because of the DDR signalling).

...so I think that the way the nxp,dmic driver got modified does indeed deviate from the API as it was intended to be used and that change should've really been an API change with a proper RFC.

Now... not realising this, I authored some changes in the i2s_codec sample that build a 1:1 map of channels. These changes were merged, enabling the sample on mimxrt685_evk and mimxrt595_evk boards. This now constitutes a bit of a sticky situation, as there are now drivers with differing interpretations of the dmic API. The dmic_api is written against the original one, i2s_codec is written against the modified one.

I don't feel I am the right person to make decisions such as these, but what I would perhaps do is stick to the API as it is defined at the moment and just have the dmic_api test functional, with the i2s_codec sample being broken for the targets where the PDM/MICFIL peripheral exists. I can revert that particular commit if an RFC doesn't come through.

VitekST · 2025-09-26T06:27:13Z

drivers/audio/dmic_nxp_pdm.c

+	for (uint16_t frame = 0; frame < frames_to_read; frame++) {
+		for (uint8_t chan = 0; chan < data->channels; chan++) {
+			uint8_t hw_ch = data->hw_chan[chan];
+			/* Take high 24 bits: shift down by 8. Low 8 bits are zero in FIFO */


AFAIK, 24 bit PCM samples can be quite problematic when dealing with them later, for example not all I2S codecs support I2S traffic bearing such samples. 32 bits is usually the "greatest common divisor". I suggest that you either just copy from FIFO as it is (thus forcing 32 bits upon the user) or implement some logic that allows the user to select a bit width (16, 24 and 32 perhaps) and which truncates the samples as needed.

Hello @VitekST, thanks for your valuable suggestions! i just enabled DA7212 codec in FRDM-MCXN236, the next step is test whether pdm && sai && codec works well (I had originally planned to test on the RT700, but encountered some other issues that were difficult to resolve, so I switched to verifying on the MCXN236.). I will incorporate your suggestions to improve the driver. BTW, I'd like to know, have you resolved the issue where DMA can't support dual-channel on your side? If so, I can close this PR and use your PDM driver to support the MCXN236. Thanks!

Updated to use 32bit.
After testing, MICFIL, SAI, and CODEC (FRDM-MCXN236 on-board DA7212) can function properly together.

@ZhaoxiangJin Great!

No, sadly, my attempts at implementing an equivalent driver using the dma API (with the nxp,mcux-edma driver being the relevant implementation) have come to a halt as that particular driver seems to struggle at handling scatter-gather transfers with reloading. Don't mind me, your driver can be rewritten once this gets resolved.

Add `dialog` for Dialog Semiconductor. Signed-off-by: Zhaoxiang Jin <[email protected]>

Add dialog7212 driver. Signed-off-by: Zhaoxiang Jin <[email protected]>

Enable da7212 on frdm_mcxn236 Signed-off-by: Zhaoxiang Jin <[email protected]>

- Configure BLOCK count by using kconfig option 'EXTRA_BLOCKS' instad of fixed value. This allows user to customize the RAM usage based on their platform. - Keep the PCM data in flash to avoid large RAM usage. - Refine the sample to get the SAMPLE_BIT_WIDTH and BYTES_PER_SAMPLE from Kconfig options. Signed-off-by: Zhaoxiang Jin <[email protected]>

Enable i2s_codec samples for frdm_mcxn236. With 'CONFIG_USE_DMIC=n', you can hear the sine wave from the codec. Signed-off-by: Zhaoxiang Jin <[email protected]>

Remove whitespaces to make code easier to read and follow the common sense coding style. Signed-off-by: Zhaoxiang Jin <[email protected]>

Add NXP MICFIL driver base DMIC device driver model. Signed-off-by: Zhaoxiang Jin <[email protected]>

Enable MICFIL clock control for NXP MCUX SoCs. Signed-off-by: Zhaoxiang Jin <[email protected]>

1. Enable MICFIL on frdm_mcxn236 board. 2. MICFIL CLOCK and DATA Pins are conflict with flexcomm0_lpuart pins, so change flexcomm0_lpuart pins to 'FC0_P2_PIO0_6' and 'FC0_P3_PIO0_7'. Signed-off-by: Zhaoxiang Jin <[email protected]>

Enable nxp micfil test with 'CONFIG_USE_DMIC=y', you can hear the voice played from the on-board micphone. Signed-off-by: Zhaoxiang Jin <[email protected]>

sonarqubecloud · 2025-09-29T23:04:43Z

Quality Gate passed

Issues
11 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

dbaluta

@ZhaoxiangJin this driver looks oddly similar with this one:

https://github.com/zephyrproject-rtos/zephyr/blob/main/drivers/dai/nxp/micfil/micfil.c

Can't you use this? Also, you haven't answered to any of my comments.

dbaluta · 2025-10-02T12:48:31Z

drivers/audio/CMakeLists.txt

 zephyr_library_sources_ifdef(CONFIG_AUDIO_CODEC_WM8962 wm8962.c)
 zephyr_library_sources_ifdef(CONFIG_AUDIO_CODEC_CS43L22 cs43l22.c)
 zephyr_library_sources_ifdef(CONFIG_AUDIO_CODEC_PCM1681 pcm1681.c)
 zephyr_library_sources_ifdef(CONFIG_AUDIO_CODEC_MAX98091 max98091.c)


Please add a link to the reference manual so reviewers can analyze the hardware IP.

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.

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.

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.

"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:

Enable DA7212 driver

Refine i2s_codec example for DA7212 and MICFIL test

Enable DA7212 test

Format dmic.h

Enable MICFIL driver base DMIC device driver model

Enable MICFIL test

However, I recognize my oversight regarding reviewer burden. I will split this PR, please move your comments to new PRs

VitekST · 2025-10-02T13:05:28Z

@dbaluta That nxp,dai-micfil driver handles only the 1st PDM channel, as it exposes only the 1st (DATACH0) PCM FIFO. In addition, there's no sample for the dai API (the only one is, well, SOF) and it pushes the need to handle data transfers onto the application developer.

ZhaoxiangJin · 2025-10-02T13:05:38Z

@ZhaoxiangJin this driver looks oddly similar with this one:

https://github.com/zephyrproject-rtos/zephyr/blob/main/drivers/dai/nxp/micfil/micfil.c

Can't you use this? Also, you haven't answered to any of my comments.

I think DAI and DMIC should be different device driver models. Of course, I haven't closely examined exactly what kind of model DAI is.

dbaluta · 2025-10-02T13:18:39Z

@dbaluta That nxp,dai-micfil driver handles only the 1st PDM channel, as it exposes only the 1st (DATACH0) PCM FIFO. In addition, there's no sample for the dai API (the only one is, well, SOF) and it pushes the need to handle data transfers onto the application developer.

Actually, it handles 1-8 channels with SDMA scripts taking care of copying the data starting at DATACH0 + current_channel. I tested it with SOF and 8 channels.

I have no problem on you using the I2S interface. Looks like it's there for longer time then DAI interface. I want to understand if we can avoid duplicating the code and having two drivers for the same IP (not clear though it is the same IP) as the naming is confusing and havent seen yet the datasheet for this PRs IPs/

VitekST · 2025-10-02T13:33:35Z

@dbaluta That nxp,dai-micfil driver handles only the 1st PDM channel, as it exposes only the 1st (DATACH0) PCM FIFO. In addition, there's no sample for the dai API (the only one is, well, SOF) and it pushes the need to handle data transfers onto the application developer.

Actually, it handles 1-8 channels with SDMA scripts taking care of copying the data starting at DATACH0 + current_channel. I tested it with SOF and 8 channels.

I have no problem on you using the I2S interface. Looks like it's there for longer time then DAI interface. I want to understand if we can avoid duplicating the code and having two drivers for the same IP (not clear though it is the same IP) as the naming is confusing and havent seen yet the datasheet for this PRs IPs/

Okay, noted. Yeah, that's trivial, the FIFO registers are laid out consecutively.

The API in question is not i2s, it's dmic. The main difference is that the dmic API handles the task of transferring data to a queue of audio buffers - the application developer doesn't have to set up DMA or copy over the data by the means of processor horsepower, instead the application consumes buffers that have the data transferred. By extension, the API has a facility to remap channels to an arbitrary order, although to which extent is a matter in itself, about which I have started discussing just now.

The motivation is that there are samples for the dmic API (i2s_codec) as well as tests (dmic_api).

About the deduplication - one approach comes to mind. Each audio endpoint could be represented by a driver implementing the dai API (with perhaps the extension of being able to expose multiple FIFOs - so that channel remapping can be done), with an overlay driver that would handle DMA and buffer handling, presenting a nice, i2s-like API to the user. This would cover both use-cases, having audio that can be used straight as it is in Zephyr and still maintaining compatibility with SOF.

Of course, this travels towards something that can be called "a grand unifying theory of audio on Zephyr". As a result, I don't think that it can be decided just like that in this PR, it's a change that would touch practically all i2s, dmic and dai drivers if we were to do this the right way and thoroughly. So... at this time, I don't think we can avoid that if we don't want to break anything.

VitekST · 2025-10-02T14:10:24Z

drivers/audio/dmic_nxp_micfil.c

+	 * have information, and the other bits are always 0. We output 32-bit PCM
+	 * to keep alignment and simplify processing.
+	 */
+	if (!(stream->pcm_width == 32U)) {


I mentioned this earlier - I think it's best to use the inequality operator here.

VitekST · 2025-10-02T14:12:35Z

drivers/audio/dmic_nxp_micfil.c

+	case DMIC_TRIGGER_RELEASE: {
+		/* Check if we are in a state that can be started/released */
+		if (!(data->state == DMIC_STATE_CONFIGURED ||
+			data->state == DMIC_STATE_PAUSED)) {


I think that it's also best to use the inequality operator here - use that and replace the disjunction with conjuction.

dbaluta · 2025-10-02T14:22:03Z

Of course, this travels towards something that can be called "a grand unifying theory of audio on Zephyr". As a result, I don't think that it can be decided just like that in this PR, it's a change that would touch practically all i2s, dmic and dai drivers if we were to do this the right way and thoroughly. So... at this time, I don't think we can avoid that if we don't want to break anything.

Agree with you. Lets use use dmic interface for now for this.

ZhaoxiangJin force-pushed the enable-nxp-pdm branch 2 times, most recently from 0650738 to 37a4f90 Compare September 22, 2025 13:10

ZhaoxiangJin marked this pull request as ready for review September 22, 2025 13:15

zephyrbot added area: Clock Control platform: NXP Drivers NXP Semiconductors, drivers platform: NXP NXP area: Audio labels Sep 22, 2025

zephyrbot requested review from butok, dbaluta, decsny, DerekSnell, dleach02, EmilioCBen, kv2019i, lgirdwood, lyakh, manuargue, mmahadevan108 and nordic-krch September 22, 2025 13:17

zephyrbot assigned dleach02 and mmahadevan108 Sep 22, 2025

decsny reviewed Sep 22, 2025

View reviewed changes

dts/bindings/audio/nxp,pdm.yaml Outdated Show resolved Hide resolved

dbaluta reviewed Sep 23, 2025

View reviewed changes

VitekST reviewed Sep 25, 2025

View reviewed changes

VitekST reviewed Sep 26, 2025

View reviewed changes

ZhaoxiangJin force-pushed the enable-nxp-pdm branch from 37a4f90 to fa527b0 Compare September 29, 2025 11:29

ZhaoxiangJin requested a review from decsny September 29, 2025 15:02

decsny removed request for VitekST and decsny September 29, 2025 16:51

ZhaoxiangJin added 10 commits September 30, 2025 06:32

dts: bindings: add dialog vendor prefix

611f84a

Add `dialog` for Dialog Semiconductor. Signed-off-by: Zhaoxiang Jin <[email protected]>

drivers: audio: Add dialog7212 driver

3432318

Add dialog7212 driver. Signed-off-by: Zhaoxiang Jin <[email protected]>

boards: frdm_mcxn236: Enable da7212 on frdm_mcxn236

6f15a7f

Enable da7212 on frdm_mcxn236 Signed-off-by: Zhaoxiang Jin <[email protected]>

samples: i2s_codec: enable i2s_codec samples for frdm_mcxn236

ffa63a5

Enable i2s_codec samples for frdm_mcxn236. With 'CONFIG_USE_DMIC=n', you can hear the sine wave from the codec. Signed-off-by: Zhaoxiang Jin <[email protected]>

audio: dmic: Format dmic.h

af1fa22

Remove whitespaces to make code easier to read and follow the common sense coding style. Signed-off-by: Zhaoxiang Jin <[email protected]>

drivers: audio: Add NXP MICFIL driver

e1b8489

Add NXP MICFIL driver base DMIC device driver model. Signed-off-by: Zhaoxiang Jin <[email protected]>

drivers: mcux_syscon: enable MICFIL clock control

a2a730b

Enable MICFIL clock control for NXP MCUX SoCs. Signed-off-by: Zhaoxiang Jin <[email protected]>

board: frdm_mcxn236: Enable MICFIL on frdm_mcxn236

d5e9f21

1. Enable MICFIL on frdm_mcxn236 board. 2. MICFIL CLOCK and DATA Pins are conflict with flexcomm0_lpuart pins, so change flexcomm0_lpuart pins to 'FC0_P2_PIO0_6' and 'FC0_P3_PIO0_7'. Signed-off-by: Zhaoxiang Jin <[email protected]>

samples: i2s_codec: Enable nxp micfil test

2c1449a

Enable nxp micfil test with 'CONFIG_USE_DMIC=y', you can hear the voice played from the on-board micphone. Signed-off-by: Zhaoxiang Jin <[email protected]>

ZhaoxiangJin force-pushed the enable-nxp-pdm branch from 93d967b to 2c1449a Compare September 29, 2025 22:45

ZhaoxiangJin requested a review from VitekST September 29, 2025 22:46

dbaluta requested changes Oct 2, 2025

View reviewed changes

dbaluta reviewed Oct 2, 2025

View reviewed changes

VitekST mentioned this pull request Oct 2, 2025

drivers: audio: dmic_mcux: remove arbitrary mapping of pdm to dmic channel #90824

Merged

VitekST reviewed Oct 2, 2025

View reviewed changes

TomasBarakNXP mentioned this pull request Oct 3, 2025

Enable i2s_codec sample on mcxn5xx_evk #96954

Open

ZhaoxiangJin closed this Oct 3, 2025

This was referenced Oct 3, 2025

Enable dialog da7212 driver #96978

Open

Enable nxp micfil driver #96980

Open

		@@ -1,18 +1,19 @@
		# SPDX-License-Identifier: Apache-2.0

		zephyr_library()

Enable nxp MICFIL driver and Dialog DA7212 driver #96367

Enable nxp MICFIL driver and Dialog DA7212 driver #96367

Uh oh!

Conversation

ZhaoxiangJin commented Sep 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ZhaoxiangJin Oct 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

VitekST Sep 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

hakehuang Sep 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

VitekST Oct 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sonarqubecloud bot commented Sep 29, 2025

Quality Gate passed

Uh oh!

dbaluta left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

VitekST commented Oct 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ZhaoxiangJin commented Oct 2, 2025

Uh oh!

dbaluta commented Oct 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

VitekST commented Oct 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

ZhaoxiangJin commented Sep 22, 2025 •

edited

Loading

ZhaoxiangJin Oct 2, 2025 •

edited

Loading

VitekST Sep 25, 2025 •

edited

Loading

hakehuang Sep 30, 2025 •

edited

Loading

VitekST Oct 2, 2025 •

edited

Loading

dbaluta left a comment •

edited

Loading

VitekST commented Oct 2, 2025 •

edited

Loading

dbaluta commented Oct 2, 2025 •

edited

Loading

VitekST commented Oct 2, 2025 •

edited

Loading