nxp: imx8/imx8x: switch to Zephyr native drivers

[LaurentiuM1234]

Note: cleanup will be done in separate PRs after this is merged

[paulstelian97]

Hi there, I thought of checking what's happening around here. I noticed that there may be an opportunity to break bisection (though I may misunderstand something) between the last two commits (a place where you no longer initialise a DMA domain, yet the topology still specifies one). Did you consider swapping the last two commits to avoid this?

Other than that I like how this looks! (well except the build errors from the DT, but I guess you're already working on those)

[LaurentiuM1234]

I noticed that there may be an opportunity to break bisection (though I may misunderstand something) between the last two commits (a place where you no longer initialise a DMA domain, yet the topology still specifies one)

yep, you're right

Did you consider swapping the last two commits to avoid this?

that will also break git bisect since the timer domain doesn't work right without the native switch. I think the solution would be to just squash the last 2 commits.

[iuliana-prodan]

I noticed that there may be an opportunity to break bisection (though I may misunderstand something) between the last two commits (a place where you no longer initialise a DMA domain, yet the topology still specifies one)

yep, you're right

Did you consider swapping the last two commits to avoid this?

that will also break git bisect since the timer domain doesn't work right without the native switch. I think the solution would be to just squash the last 2 commits.

And use in commit message "switch to native drivers and timer domain", since a lot of changes are also related to timer domain.
IMO you should do the same in PR #8859

[lgirdwood]

LGTM, giving my +1 so NXP can schedule the merging after validation is complete.

[LaurentiuM1234]

V2 changes

1. Commit that changes the topologies to timer domain and commit that does transition to native drivers have been squashed into a single one.
2. Adapted to HWMv2.

[LaurentiuM1234]

V3 changes

1. Rebased
2. All Zephyr dependencies merged. Added zephyr hash bump.

Opening this up for reviews. Will remove the [DNM] tag when testing is done.

[marc-hb]

This PR introduces a small and surprising, imx8/zephyr.lst disassembly difference between the Linux and Windows build:

https://github.com/thesofproject/sof/actions/runs/8552063499?pr=8863

--- win/build-sof-staging/sof-info/imx8x/zephyr.lst	2024-04-04 09:13:44
+++ lin/build-sof-staging/sof-info/imx8x/zephyr.lst	2024-04-04 09:12:34
@@ -81127,20 +81127,26 @@
 92433b0e:	430c                          	movi.n	a3, 4
 #if !(defined(FSL_SDK_DISABLE_DRIVER_CLOCK_CONTROL) && FSL_SDK_DISABLE_DRIVER_CLOCK_CONTROL)
 
         uint32_t instance = LPUART_GetInstance(base);
 
         /* Enable lpuart clock */
         (void)CLOCK_EnableClock(s_lpuartClock[instance]);
 92433b10:	833441                      	l32r	a4, 924147e0 <_stext+0x1598> (92413034 <s_lpuartClock>)
 92433b13:	a03340                      	addx4	a3, a3, a4
 92433b16:	03a8                          	l32i.n	a10, a3, 0
+ * @param name  Which clock to enable, see \ref clock_ip_name_t.
+ * @return true for success, false for failure.
+ */
+static inline bool CLOCK_EnableClock(clock_ip_name_t name)
+{
+    return CLOCK_EnableClockExt(name, 0);
 92433b18:	31e9                          	s32i.n	a14, a1, 12
 92433b1a:	00a0b2                      	movi	a11, 0
 92433b1d:	ffcfa5                      	call8	92433818 <CLOCK_EnableClockExt>
     base->GLOBAL |= LPUART_GLOBAL_RST_MASK;
 92433b20:	0020c0                      	memw
 92433b23:	2238                          	l32i.n	a3, a2, 8
 92433b25:	240c                          	movi.n	a4, 2
 92433b27:	203340                      	or	a3, a3, a4
 92433b2a:	0020c0                      	memw
 92433b2d:	2239                          	s32i.n	a3, a2, 8

It is triggered by this PR because daily builds are ok:
https://github.com/thesofproject/sof/actions/runs/8563182403

In light of recent events I hope everything understands the importance of reproducible builds.

The first thing is to submit the west.yml separately:
https://docs.zephyrproject.org/latest/contribute/contributor_expectations.html#defining-smaller-prs
Not just for reviewers but also for Continous integration.

Then, if you don't have BOTH one Linux and one Windows build system, could you please submit draft/throw-away PRs with subsets of this (pretty big) PR to isolate the trigger?

[marc-hb]

The first thing is to submit the west.yml separately:

... unless there is a two-way dependency between this west.yml bump and some of the code changes in this PR. In that case they must be part of the same commit so every commit works.

[marc-hb]

Need to root-cause the reproducible builds issue

[LaurentiuM1234]

This PR introduces a small and surprising, imx8/zephyr.lst disassembly difference between the Linux and Windows build:

https://github.com/thesofproject/sof/actions/runs/8552063499?pr=8863

--- win/build-sof-staging/sof-info/imx8x/zephyr.lst	2024-04-04 09:13:44
+++ lin/build-sof-staging/sof-info/imx8x/zephyr.lst	2024-04-04 09:12:34
@@ -81127,20 +81127,26 @@
 92433b0e:	430c                          	movi.n	a3, 4
 #if !(defined(FSL_SDK_DISABLE_DRIVER_CLOCK_CONTROL) && FSL_SDK_DISABLE_DRIVER_CLOCK_CONTROL)
 
         uint32_t instance = LPUART_GetInstance(base);
 
         /* Enable lpuart clock */
         (void)CLOCK_EnableClock(s_lpuartClock[instance]);
 92433b10:	833441                      	l32r	a4, 924147e0 <_stext+0x1598> (92413034 <s_lpuartClock>)
 92433b13:	a03340                      	addx4	a3, a3, a4
 92433b16:	03a8                          	l32i.n	a10, a3, 0
+ * @param name  Which clock to enable, see \ref clock_ip_name_t.
+ * @return true for success, false for failure.
+ */
+static inline bool CLOCK_EnableClock(clock_ip_name_t name)
+{
+    return CLOCK_EnableClockExt(name, 0);
 92433b18:	31e9                          	s32i.n	a14, a1, 12
 92433b1a:	00a0b2                      	movi	a11, 0
 92433b1d:	ffcfa5                      	call8	92433818 <CLOCK_EnableClockExt>
     base->GLOBAL |= LPUART_GLOBAL_RST_MASK;
 92433b20:	0020c0                      	memw
 92433b23:	2238                          	l32i.n	a3, a2, 8
 92433b25:	240c                          	movi.n	a4, 2
 92433b27:	203340                      	or	a3, a3, a4
 92433b2a:	0020c0                      	memw
 92433b2d:	2239                          	s32i.n	a3, a2, 8

It is triggered by this PR because daily builds are ok: https://github.com/thesofproject/sof/actions/runs/8563182403

In light of recent events I hope everything understands the importance of reproducible builds.

The first thing is to submit the west.yml separately: https://docs.zephyrproject.org/latest/contribute/contributor_expectations.html#defining-smaller-prs Not just for reviewers but also for Continous integration.

Then, if you don't have BOTH one Linux and one Windows build system, could you please submit draft/throw-away PRs with subsets of this (pretty big) PR to isolate the trigger?

ACK, started with the west.yml change: #9010, which seems to be ok w.r.t the reproducible builds. Anyways, if I were to guess based on the log you've provided, I'd say the change that enables the serial interface causes this issue.

[LaurentiuM1234]

Rebased

[lgirdwood]

@LaurentiuM1234 is building on Windows priority for you, it seems the compiler has different behavior here for this platform only. I would say here that this is a second order test vs the 1st order runtime functional test for this platform.

I would suggest we comment this platform out from the OS reproducability test and comment in the test that the compiler is generating different code here on Windows vs Linux. You can reference the GH PR and BUG in the comment. We can then move on. @dbaluta this also good with you ?

[marc-hb]

Comparing Windows and Linux is the only way the reproducibility test work.

[lgirdwood]

Comparing Windows and Linux is the only way the reproducibility test work.

That fine when the tools are deterministsic and align when used on different OSes, in this case effort is being spent to workaround a compiler bug for a second order test. Today we need to comment out this platform in the test (or report and ignore result for this platform). The compiler will update over time and may align in a future version (when we can re-enable).

[marc-hb]

The fix is tested and ready:

• cmake: add control over inline source code disassembly zephyrproject-rtos/zephyr#71535

[LaurentiuM1234]

Rebased. Let's see the repro builds results after #9071.

[LaurentiuM1234]

Repro builds are green. Dropping the DNM tag.