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.

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

Add MCXN947 sai module support #77534

Merged

Conversation

mcuxted
Copy link
Contributor

@mcuxted mcuxted commented Aug 26, 2024

  1. Add sai instances on MCXN947.
  2. Update sai driver.
  3. Port i2s_speed case on FRDM-mCXN947 and test passed locally.
  4. support sai clock for syscon.

Copy link

Hello @mcuxted, and thank you very much for your first pull request to the Zephyr project!
Our Continuous Integration pipeline will execute a series of checks on your Pull Request commit messages and code, and you are expected to address any failures by updating the PR. Please take a look at our commit message guidelines to find out how to format your commit messages, and at our contribution workflow to understand how to update your Pull Request. If you haven't already, please make sure to review the project's Contributor Expectations and update (by amending and force-pushing the commits) your pull request if necessary.
If you are stuck or need help please join us on Discord and ask your question there. Additionally, you can escalate the review when applicable. 😊

@mcuxted
Copy link
Contributor Author

mcuxted commented Aug 26, 2024

Thanks to @hakehuang for helping debug i2s_speed to find the i2s driver problem and fix it.

@mcuxted mcuxted force-pushed the add_mcxn10_sai_module_support branch from ec29530 to c465de2 Compare August 26, 2024 03:22
@hakehuang
Copy link
Collaborator

need add i2s in boards/nxp/frdm_mcxn947_mcxn947_cpu0.yaml, supports list

@mcuxted mcuxted force-pushed the add_mcxn10_sai_module_support branch 3 times, most recently from 9c540b1 to a30544e Compare August 29, 2024 08:04
@carlescufi carlescufi assigned DerekSnell and mmahadevan108 and unassigned anangl Aug 29, 2024
@DerekSnell DerekSnell removed their assignment Aug 30, 2024
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.

Thank you @mcuxted ,
Can you please update the doc page?

boards/nxp/frdm_mcxn947/doc/index.rst Outdated Show resolved Hide resolved
@DerekSnell
Copy link
Contributor

What is left too be able to merge this PR?
Hi @ehughes ,
The merge window is closed right now because the repo is in the stabilization phase for the upcoming v4.0 release. This PR passes all the compliance checks and has the approvals needed. So it should merge quickly after the v4.0 release.

Best regards

{
const struct i2s_mcux_config *dev_cfg = dev->config;

#ifdef CONFIG_I2S_HAS_PLL_SETTING
Copy link
Collaborator

Choose a reason for hiding this comment

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

Not a blocking comment, just a general concern- we probably need to consider a more generic way to set the MCLK pin direction here. The block here seems to be specific to the RT1xxx series parts, but configuring the MCLK pin is an operation generic across I2S peripherals.

Perhaps we need an SOC specific function to handle this? @DerekSnell are you aware, does the MCLK direction setting end up being very SOC-specific? IE would it make the most sense to implement it as SOC specific code?

Copy link
Contributor

Choose a reason for hiding this comment

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

Hi @danieldegrasse and @mcuxted ,
Sorry, I do not have experience with these SAI SOC differences. I think it would be best to refer to the MCUXpresso SDK driver examples for the different SOCs. It appears these are the MCUs supported in Zephyr today with the SAI: RT10xx, RT11xx, RT700, MCXC, MCXN, Kinetis K like K22F12.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hi @DerekSnell, I have updated it according to the latest code, adding pinmuxes to uniformly modify the direction of mclk

@kartben
Copy link
Collaborator

kartben commented Dec 10, 2024

@mcuxted needs rebase to resolve merge conflict

@mcuxted mcuxted dismissed stale reviews from hakehuang and DerekSnell via f76b784 December 12, 2024 06:10
@mcuxted mcuxted force-pushed the add_mcxn10_sai_module_support branch 2 times, most recently from f76b784 to e3291f9 Compare December 12, 2024 08:11
@hakehuang hakehuang self-requested a review December 12, 2024 10:13
hakehuang
hakehuang previously approved these changes Dec 12, 2024
status = "okay";
};
&sai0 {
status = "okay";
Copy link
Collaborator

Choose a reason for hiding this comment

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

This should be added to the _qspi variant as well.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Moved configuration to frdm_mcxn947_mcxn947_cpu0.dtsi

.pllmdiv = SCG_SPLLMDIV_MDIV(10U),
.pllRate = 80000000U
};
#if DT_NODE_HAS_STATUS_OKAY(DT_NODELABEL(sai0)) || DT_NODE_HAS_STATUS_OKAY(DT_NODELABEL(sai1))
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is the FlexCAN clock setup deleted?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The clock source of flexcan has been switched to kFRO_HF_to_FLEXCAN0

Copy link
Collaborator

Choose a reason for hiding this comment

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

However I don't see flexcan in this list?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The original clk source of flexcan is pll1. Due to the conflict with sai mclk clock source, the clock source of flexcan is switched from kPLL1_CLK0_to_FLEXCAN0 to kFRO_HF_to_FLEXCAN0 to ensure the use of flexcan(CLOCK_AttachClk(kFRO_HF_to_FLEXCAN0);).

Copy link
Collaborator

Choose a reason for hiding this comment

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

@mcuxted , can you please look at this code?

Copy link
Collaborator

Choose a reason for hiding this comment

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

@hakehuang , can we confirm that this change does not break Flexcan support on this board.

Copy link
Collaborator

@hakehuang hakehuang Jan 16, 2025

Choose a reason for hiding this comment

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

@mmahadevan108 good point. looks OK for all timings

2025-01-16 19:18:10,194 - twister - DEBUG - DEVICE: *** Booting Zephyr OS build v4.0.0-2200-g227d86b63ed2 ***
2025-01-16 19:18:10,197 - twister - DEBUG - DEVICE: Running TESTSUITE can_timing
2025-01-16 19:18:10,203 - twister - DEBUG - DEVICE: ===================================================================
2025-01-16 19:18:10,207 - twister - DEBUG - DEVICE: testing on device can@d4000 @ 48000000 Hz
2025-01-16 19:18:10,208 - twister - DEBUG - DEVICE: START - test_timing
2025-01-16 19:18:10,221 - twister - DEBUG - DEVICE: testing bitrate 10000, sample point 87.5%: sjw = 1, prop_seg = 8, phase_seg1 = 8, phase_seg2 = 3, prescaler = 240 OK, sample point error 2.5%
2025-01-16 19:18:10,233 - twister - DEBUG - DEVICE: testing bitrate 20000, sample point 87.5%: sjw = 1, prop_seg = 6, phase_seg1 = 7, phase_seg2 = 2, prescaler = 150 OK, sample point error 0.0%
2025-01-16 19:18:10,245 - twister - DEBUG - DEVICE: testing bitrate 50000, sample point 87.5%: sjw = 1, prop_seg = 6, phase_seg1 = 7, phase_seg2 = 2, prescaler = 60 OK, sample point error 0.0%
2025-01-16 19:18:10,258 - twister - DEBUG - DEVICE: testing bitrate 125000, sample point 87.5%: sjw = 1, prop_seg = 6, phase_seg1 = 7, phase_seg2 = 2, prescaler = 24 OK, sample point error 0.0%
2025-01-16 19:18:10,269 - twister - DEBUG - -- west flash: using runner jlink
2025-01-16 19:18:10,269 - twister - DEBUG - DEVICE: testing bitrate 250000, sample point 87.5%: sjw = 1, prop_seg = 6, phase_seg1 = 7, phase_seg2 = 2, prescaler = 12 OK, sample point error 0.0%
2025-01-16 19:18:10,282 - twister - DEBUG - DEVICE: testing bitrate 500000, sample point 87.5%: sjw = 1, prop_seg = 6, phase_seg1 = 7, phase_seg2 = 2, prescaler = 6 OK, sample point error 0.0%
2025-01-16 19:18:10,295 - twister - DEBUG - DEVICE: testing bitrate 800000, sample point 80.0%: sjw = 2, prop_seg = 7, phase_seg1 = 8, phase_seg2 = 4, prescaler = 3 OK, sample point error 0.0%
2025-01-16 19:18:10,307 - twister - DEBUG - DEVICE: testing bitrate 1000000, sample point 75.0%: sjw = 2, prop_seg = 5, phase_seg1 = 6, phase_seg2 = 4, prescaler = 3 OK, sample point error 0.0%
2025-01-16 19:18:10,310 - twister - DEBUG - DEVICE: PASS - test_timing in 0.100 seconds
2025-01-16 19:18:10,316 - twister - DEBUG - DEVICE: ===================================================================
2025-01-16 19:18:10,319 - twister - DEBUG - DEVICE: START - test_timing_data
2025-01-16 19:18:10,322 - twister - DEBUG - DEVICE: SKIP - test_timing_data in 0.001 seconds
2025-01-16 19:18:10,328 - twister - DEBUG - DEVICE: ===================================================================
2025-01-16 19:18:10,331 - twister - DEBUG - DEVICE: TESTSUITE can_timing succeeded
2025-01-16 19:18:10,335 - twister - DEBUG - DEVICE: ------ TESTSUITE SUMMARY START ------
2025-01-16 19:18:10,343 - twister - DEBUG - DEVICE: SUITE PASS - 100.00% [can_timing]: pass = 1, fail = 0, skip = 1, total = 2 duration = 0.101 seconds
2025-01-16 19:18:10,348 - twister - DEBUG - DEVICE: - PASS - [can_timing.test_timing] duration = 0.100 seconds
2025-01-16 19:18:10,354 - twister - DEBUG - DEVICE: - SKIP - [can_timing.test_timing_data] duration = 0.001 seconds
2025-01-16 19:18:10,357 - twister - DEBUG - DEVICE: ------ TESTSUITE SUMMARY END ------
2025-01-16 19:18:10,364 - twister - DEBUG - DEVICE: ===================================================================
2025-01-16 19:18:10,367 - twister - DEBUG - DEVICE: RunID: e34ce6648f488d5bfb13441d421780dd
2025-01-16 19:18:10,370 - twister - DEBUG - DEVICE: PROJECT EXECUTION SUCCESSFUL

Copy link
Collaborator

Choose a reason for hiding this comment

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

Thanks @hakehuang . Can you please approve this PR if it looks good to you per your testing.


pinmux-cells:
- pin
- function
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why is this added?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In some devices, the core will set the direction of mclk with dts. MCXN9 does not have relevant register settings, but there is an mclk direction setting inside the driver, so add relevant configuration.

@@ -1065,6 +1066,7 @@ static void i2s_mcux_isr(void *arg)

static void audio_clock_settings(const struct device *dev)
{
#ifdef CONFIG_I2S_HAS_PLL_SETTING
Copy link
Collaborator

@mmahadevan108 mmahadevan108 Jan 8, 2025

Choose a reason for hiding this comment

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

Instead of adding a Kconfig, ideally we should move this code out of the driver and into the soc.c file. This may not be required as part of this PR. If you can do it then that would be great.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I also agree that the PLL configuration should be moved out of the i2s driver and configured using other methods,This may also cause the pll configuration to be overwritten.
In MCXN9, we can use the SDK to configure the PLL in the board.h. Is it a way to control the PLL outside the driver on other devices?

@mcuxted mcuxted force-pushed the add_mcxn10_sai_module_support branch from b884cb8 to 227d86b Compare January 10, 2025 02:56
Add sai clock support for syscon.

Signed-off-by: Qiang Zhang <qiang.zhang_6@nxp.com>
  i2s driver have not suooprt frdm_mcxn947 pll clk set.
  so add macro CONFIG_I2S_HAS_PLL_SETTING to control pll init.

Signed-off-by: Qiang Zhang <qiang.zhang_6@nxp.com>
Add sai nodes for NXP mcxn94x

Signed-off-by: Qiang Zhang <qiang.zhang_6@nxp.com>
Support sai for NXP frdm_mcxn947.

Signed-off-by: Qiang Zhang <qiang.zhang_6@nxp.com>
Support i2s example for NXP frdm_mcxn947

Signed-off-by: Qiang Zhang <qiang.zhang_6@nxp.com>
@mmahadevan108 mmahadevan108 force-pushed the add_mcxn10_sai_module_support branch from 227d86b to a959efd Compare January 16, 2025 17:14
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.

i2s_speed test passes on my FRDM-MCXN947 board, and this LGTM. Thank you, @mcuxted

@kartben kartben merged commit 97f4838 into zephyrproject-rtos:main Jan 17, 2025
27 checks passed
Copy link

Hi @mcuxted!
Congratulations on getting your very first Zephyr pull request merged 🎉🥳. This is a fantastic achievement, and we're thrilled to have you as part of our community!

To celebrate this milestone and showcase your contribution, we'd love to award you the Zephyr Technical Contributor badge. If you're interested, please claim your badge by filling out this form: Claim Your Zephyr Badge.

Thank you for your valuable input, and we look forward to seeing more of your contributions in the future! 🪁

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

Successfully merging this pull request may close these issues.

10 participants