Skip to content

Open amp refactoring and add more platforms support #17553

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

Merged
merged 7 commits into from
Mar 26, 2020

Conversation

karl-zh
Copy link
Collaborator

@karl-zh karl-zh commented Jul 16, 2019

openAMP is a multiple core IPC which supported in Zephyr. This PR is going to make it supported for more multiple core platforms rather than for lpc54 only.

Currently added the arm platform AN521 and Musca A, for further plan is to supporting Musca B1 when #16729 get merged.

@zephyrbot
Copy link
Collaborator

zephyrbot commented Jul 16, 2019

All checks are passing now.

Review history of this comment for details about previous failed status.
Note that some checks might have not completed yet.

@karl-zh karl-zh force-pushed the openAMP_refactoring branch 3 times, most recently from dea2528 to 079407e Compare July 16, 2019 06:48
@galak galak requested a review from arnopo July 16, 2019 15:38
@galak galak added the area: IPC Inter-Process Communication label Jul 16, 2019
@galak
Copy link
Collaborator

galak commented Jul 16, 2019

@arnop2 can you take a look at this PR. I'm not that familiar with how we should be conveying the shared memory region in DTS.

Copy link
Contributor

@dbkinder dbkinder left a comment

Choose a reason for hiding this comment

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

one doc question, otherwise looks good.

@karl-zh karl-zh force-pushed the openAMP_refactoring branch from 079407e to 3149819 Compare July 23, 2019 13:58
@karl-zh karl-zh requested review from dbkinder and arnopo July 24, 2019 02:51
Copy link
Contributor

@dbkinder dbkinder left a comment

Choose a reason for hiding this comment

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

+1 for doc changes, thanks.

@karl-zh karl-zh force-pushed the openAMP_refactoring branch 3 times, most recently from d92786d to d9d3241 Compare July 26, 2019 12:15
@karl-zh karl-zh requested a review from arnopo July 27, 2019 14:05
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.

Just 2 minor remarks remaining, then should be ok for me

@karl-zh karl-zh force-pushed the openAMP_refactoring branch from d9d3241 to 3c12b92 Compare July 29, 2019 13:40
@galak galak force-pushed the openAMP_refactoring branch from c65ba22 to a5254ca Compare September 19, 2019 18:18
defined(CONFIG_SOC_V2M_MUSCA_B1)
u32_t current_core = sse_200_platform_get_cpu_id();

ipm_send(ipm_handle, 0, current_core ? 0 : 1, 0, 1);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why does the data here mater?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The dummy data is not necessary here for MHU based hardware. But it needs to distinguish the current core No. for notifications.

@galak
Copy link
Collaborator

galak commented Sep 21, 2019

Ok, I tried this on musca-a1 and qemu-mps2_an521 and it doesn't seem to work. Also tried the samples/subsys/ipc/ipm_mhu_dual_core on those platforms and they didn't seem to pass. Something to debug this week.

@karl-zh
Copy link
Collaborator Author

karl-zh commented Sep 21, 2019

Ok, I tried this on musca-a1 and qemu-mps2_an521 and it doesn't seem to work. Also tried the samples/subsys/ipc/ipm_mhu_dual_core on those platforms and they didn't seem to pass. Something to debug this week.

Okay, I also need some refresh of memory to this context. Maybe some rebase conflicts or other changes need rebase. BTW, do you have some fail details? I just have a test pass on my side with the previous commit I had on AN521, here is the command line for you reference:

AN521:
~/zephyr/samples/subsys/ipc/openamp/build$ cmake -GNinja -DBOARD=mps2_an521 -DCONF_FILE=prj.conf ..; ninja -v; cp ./openamp_remote-prefix/src/openamp_remote-build/zephyr/zephyr.bin ../zephyr_ns.bin ; cp zephyr/zephyr.bin ../zephyr.bin

Config file:
TITLE: Versatile Express Images Configuration File

[IMAGES]
TOTALIMAGES: 2 ;Number of Images (Max: 32)

IMAGE0ADDRESS: 0x10000000 ;Please select the required executable program
IMAGE0FILE: \SW\zephyr.bin ; - selftest

IMAGE1ADDRESS: 0x100000
IMAGE1FILE: \SW\zephyr_ns.bin

@galak
Copy link
Collaborator

galak commented Sep 21, 2019

Okay, I also need some refresh of memory to this context. Maybe some rebase conflicts or other changes need rebase. BTW, do you have some fail details? I just have a test pass on my side with the previous commit I had on AN521, here is the command line for you reference:

AN521:
~/zephyr/samples/subsys/ipc/openamp/build$ cmake -GNinja -DBOARD=mps2_an521 -DCONF_FILE=prj.conf ..; ninja -v; cp ./openamp_remote-prefix/src/openamp_remote-build/zephyr/zephyr.bin ../zephyr_ns.bin ; cp zephyr/zephyr.bin ../zephyr.bin

Config file:
TITLE: Versatile Express Images Configuration File

[IMAGES]
TOTALIMAGES: 2 ;Number of Images (Max: 32)

IMAGE0ADDRESS: 0x10000000 ;Please select the required executable program
IMAGE0FILE: \SW\zephyr.bin ; - selftest

IMAGE1ADDRESS: 0x100000
IMAGE1FILE: \SW\zephyr_ns.bin

Hmm, need to parse this in more detail. However is it possible to run the test w/o SECURE/NON-SECURE?

@karl-zh
Copy link
Collaborator Author

karl-zh commented Sep 21, 2019

Okay, I also need some refresh of memory to this context. Maybe some rebase conflicts or other changes need rebase. BTW, do you have some fail details? I just have a test pass on my side with the previous commit I had on AN521, here is the command line for you reference:
AN521:
~/zephyr/samples/subsys/ipc/openamp/build$ cmake -GNinja -DBOARD=mps2_an521 -DCONF_FILE=prj.conf ..; ninja -v; cp ./openamp_remote-prefix/src/openamp_remote-build/zephyr/zephyr.bin ../zephyr_ns.bin ; cp zephyr/zephyr.bin ../zephyr.bin
Config file:
TITLE: Versatile Express Images Configuration File
[IMAGES]
TOTALIMAGES: 2 ;Number of Images (Max: 32)
IMAGE0ADDRESS: 0x10000000 ;Please select the required executable program
IMAGE0FILE: \SW\zephyr.bin ; - selftest
IMAGE1ADDRESS: 0x100000
IMAGE1FILE: \SW\zephyr_ns.bin

Hmm, need to parse this in more detail. However is it possible to run the test w/o SECURE/NON-SECURE?

I might using a wrong binary name here. They are following the start work for tfm integrating with Zpehyr.
The two binaries here are for different CPUs, \SW\zephyr.bin is for CPU0 and \SW\zephyr_ns.bin is for CPU1, the name is for configurations file only(images.txt).

@jhedberg
Copy link
Member

jhedberg commented Jan 7, 2020

This PR seems to require rebasing since there are conflicts. Is something still missing before it can be merged (e.g. approvals or some additional code changes)?

@karl-zh
Copy link
Collaborator Author

karl-zh commented Jan 8, 2020

This PR seems to require rebasing since there are conflicts. Is something still missing before it can be merged (e.g. approvals or some additional code changes)?

Hi Johan

I think this PR needs approval before the merge.

@jhedberg
Copy link
Member

I think this PR needs approval before the merge.

@karl-zh while waiting for approval, could you rebase and fix the conflict with samples/subsys/ipc/openamp/sample.yaml?

@karl-zh
Copy link
Collaborator Author

karl-zh commented Jan 19, 2020

I think this PR needs approval before the merge.

@karl-zh while waiting for approval, could you rebase and fix the conflict with samples/subsys/ipc/openamp/sample.yaml?

Hi
I was trying to resolve the conflict from github, but it seems CI fail. I am on leave for the Chinese New Year holiday recently and will rebase/resolve and try to make CI pass when back to the office.
The approval and review is required too.

Thanks for your comments.

@erwango
Copy link
Member

erwango commented Jan 20, 2020

@karl-zh, was this intentional to push a merge commit ?

@karl-zh karl-zh force-pushed the openAMP_refactoring branch from da83b2a to 8162050 Compare February 20, 2020 02:38
@galak galak force-pushed the openAMP_refactoring branch 2 times, most recently from 480a371 to ce90b7e Compare March 26, 2020 09:55
@zephyrbot
Copy link
Collaborator

zephyrbot commented Mar 26, 2020

All checks are passing now.

Tip: The bot edits this comment instead of posting a new one, so you can check the comment's history to see earlier messages.

galak and others added 7 commits March 26, 2020 05:12
Move board specific conf info boards/<BOARD>.conf to support other
board/SoCs with this sample.

Signed-off-by: Kumar Gala <[email protected]>
Add configuable shared memory address for openAMP samples. There is a
plan to add more platforms supported for openAMP in zephyr.

Each platform can specify the shared memory address and device by
device tree and add it's support in openAMP samples.

Signed-off-by: Karl Zhang <[email protected]>
Signed-off-by: Kumar Gala <[email protected]>
Update serial output for both master and remote.

Signed-off-by: Karl Zhang <[email protected]>
Set sse-200 subsystem using MHU (Message Handling Unit) as default.

Signed-off-by: Karl Zhang <[email protected]>
AN521 is a dual core FPGA on MPS2+ with both cores are CM33. Add openAMP
to support on it.

Core 0 is primary core, it runs as master, core 1 is remote, it runs
as slave.

Signed-off-by: Karl Zhang <[email protected]>
Musca A is a dual core SoC with both cores are CM33. Add
openAMP to support on it.

Signed-off-by: Karl Zhang <[email protected]>
Musca B1 is a dual core SoC with both cores are CM33. Add
openAMP to support on it.

Signed-off-by: Karl Zhang <[email protected]>
@galak galak force-pushed the openAMP_refactoring branch from ce90b7e to a21437f Compare March 26, 2020 10:12
@galak galak merged commit fbd8cff into zephyrproject-rtos:master Mar 26, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants