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 support for i.MX8M Plus #53

Merged
merged 5 commits into from
Oct 2, 2024

Conversation

iuliana-prodan
Copy link
Contributor

Add support for i.MX8M Plus for rpmsg_multi_services sample

Move sample.yaml file outside of source folder.
While here, fix some typos.

Signed-off-by: Iuliana Prodan <[email protected]>
In src/ folder should only be source files, therefore remove
the others.
Actually all these files (README, CMakeLists and prj.conf)
are already in sample's folder, where they belong.

Signed-off-by: Iuliana Prodan <[email protected]>
Copy link
Collaborator

@arnopo arnopo left a comment

Choose a reason for hiding this comment

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

minor comments then LGTM

@@ -0,0 +1 @@
CONFIG_PLATFORM_SPECIFIC_INIT=n
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do we really need to add it to the board? It seems that we could just remove it from prj.conf.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You're right! Now that I looked better I see that CONFIG_PLATFORM_SPECIFIC_INIT is bool and the default is implicitly n - https://github.com/zephyrproject-rtos/zephyr/blob/v3.6-branch/arch/arm/core/Kconfig#L168
So, there's no need to set it again.

I compiled tested it and is all fine, for all platforms, but since I cannot test it on hw I thought is safer to let it there, but is redundant.

The CONFIG_PLATFORM_SPECIFIC_INIT is bool, so the
default value is implicitly n.
Therefore, is redundant to set it again.

Also, this is arch  specific, and it should be no
restriction to run the sample on other,
non-arm, platforms.

Signed-off-by: Iuliana Prodan <[email protected]>
While testing rpmsg_multi_services example on HiFi4 DSP from
i.MX8M Plus, realized the stack is not enough.
Increase the size based on Thread Analyzer measurements:
...
<dbg> openamp_rsc_table: mailbox_notify: mailbox_notify: msg received

OpenAMP Linux sample client responder ended
Thread analyze:
 0x9240d5b0          : STACK: unused 688 usage 1360 / 2048 (66 %); CPU: 0 %
      : Total CPU cycles used: 25065
 0x9240d638          : STACK: unused 704 usage 1344 / 2048 (65 %); CPU: 0 %
      : Total CPU cycles used: 19662
 0x9240d748          : STACK: unused 328 usage 696 / 1024 (67 %); CPU: 0 %
      : Total CPU cycles used: 1234863
 0x9240d7d0          : STACK: unused 272 usage 752 / 1024 (73 %); CPU: 0 %
      : Total CPU cycles used: 53216
 0x9240d858          : STACK: unused 168 usage 856 / 1024 (83 %); CPU: 98 %
      : Total CPU cycles used: 136995186
 0x9240d920          : STACK: unused 936 usage 88 / 1024 (8 %); CPU: 0 %
      : Total CPU cycles used: 0
 ISR0                : STACK: unused 1584 usage 464 / 2048 (22 %)

Signed-off-by: Iuliana Prodan <[email protected]>
Add the dts and config overlay for nxp_adsp_imx8m board
in order to have the rpmsg_multi_services sample working on
HiFi4 DSP from i.MX 8M Plus.

Signed-off-by: Iuliana Prodan <[email protected]>
@iuliana-prodan
Copy link
Contributor Author

Changes since previous version:

  • remove CONFIG_PLATFORM_SPECIFIC_INIT since is redundant;
  • remove unneeded comment.

@oamporg
Copy link

oamporg commented Sep 18, 2024

I did look through this and it looks good to me.
I did not setup to build it.

@wmamills
Copy link
Collaborator

[Now as me] I did look through this and it looks good to me.
I did not setup to build it.

Copy link
Collaborator

@wmamills wmamills left a comment

Choose a reason for hiding this comment

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

Looks ok.

@glneo
Copy link
Contributor

glneo commented Sep 18, 2024

The rpmsg_multi_services is effectively just a downstream fork of the openamp_rsc_table example in upstream Zephyr. We should simply remove this example from system-reference to avoid any confusion (#41).

Looks like support for this platform is already in openamp_rsc_table also, not sure why it needs duplicated here. That is the problem with having a downstream, repeated work. All the more reason to finish (#41).

@arnopo arnopo merged commit 5386c24 into OpenAMP:main Oct 2, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants