examples: libmetal: amd_rpu: Fix dynamic config setup#102
examples: libmetal: amd_rpu: Fix dynamic config setup#102bentheredonethat wants to merge 7 commits into
Conversation
bentheredonethat
force-pushed
the
libmetal-rpu-shel-update
branch
from
7d6939c to
50b0d63
Compare
bentheredonethat
force-pushed
the
libmetal-rpu-shel-update
branch
from
50b0d63 to
b735d10
Compare
|
Hi @arnopo i have incorporated the machine context changes - thanks |
bentheredonethat
left a comment
bentheredonethat
left a comment
There was a problem hiding this comment.
see review comment -- i can fix cleanup
|
Hi @arnopo thanks for this cycle of review -- will fix each comment |
|
Hi @arnopo can i also send in a patch for linux side we have for internally working case where linux does not do lopper / SDT stuff? just have a script to setup linux side via a Device tree crawl + discovery script |
|
Ben will submit PR to libmetal library to avoid need for script |
|
related libmetal PR OpenAMP/libmetal#365 -- let us please review and agree on this then i can come back to pick this up |
|
Hi @arnopo @nathalie-ckc the libmetal PR OpenAMP/libmetal#365 (review) has been acked now by @arnopo - so let us please add this back to the queue of reviewable PRs. above review comments have been addressed or are moot as the current tree has moved |
Hi @bentheredonethat : it seems that there are some comments from my previous review that are still valid. could you cross-check ? |
Use members of channel struct when able as opposed to locals Signed-off-by: Ben Levinsky <ben.levinsky@amd.com>
bentheredonethat
force-pushed
the
libmetal-rpu-shel-update
branch
from
b735d10 to
4b05ce8
Compare
|
Hi @arnopo - apologies for the miscomm. Below are fixes in the latest branch. Hopefully all your previous review comments have now been picked up and addressed. Please let me know if anything is missing. Thank you for the continued review, |
arnopo
left a comment
arnopo
left a comment
There was a problem hiding this comment.
minor comments else look good to me
| #include <metal/device.h> | ||
| #include <metal/sys.h> | ||
| #include <metal/irq.h> | ||
| #include <metal/io.h> |
There was a problem hiding this comment.
seems to me that io.h is before irq.h in alphabetical order 😃
There was a problem hiding this comment.
will fix alphabetical includes throughout - thanks
| * | ||
| * Note this only applies for R52 based systems - hence the ifdef. | ||
| */ | ||
| if (Xil_SetMPURegion(0x0U, 0x7FFFFFFFU, |
There was a problem hiding this comment.
Would be more readable having definition than direct 0x0U and 0x7FFFFFFFU value
Do you have such definition in the Xil API?
There was a problem hiding this comment.
Hi @arnopo unfortunately no - the symbol name changed based on SOC and name can change in the future. is it ok if i provide a definition of this in the project in examples/libmetal/machine/remote/amd_rpu/common.h ?
Cosmetic change for remote side - have libraries ordered alphabetically Signed-off-by: Ben Levinsky <ben.levinsky@amd.com>
As the host application now uses 3 shared mem areas that can be non-contiguous ensure that remote side accounts for this too. Signed-off-by: Ben Levinsky <ben.levinsky@amd.com>
Set first 2G MPU Region as same flags as libmetal demo to ensure MPU regions dont get incorrectly created by BSP This will go away once overlapping region adjustment is fixed in BSP Signed-off-by: Ben Levinsky <ben.levinsky@amd.com>
Fix bug in call to memset(), pass struct channel_s *, not struct channel_s ** Signed-off-by: Ben Levinsky <ben.levinsky@amd.com>
Move the shared irq_shmem_demo channel definition into a demo-level header and keep platform-specific state behind a machine_ctx pointer. This removes the duplicated channel_s definitions from the host and remote OS headers and makes the separation between common transport state and platform-private state clearer. Also export the demo common include directory through CMake so the shared header can be included directly as irq_shmem_demo.h instead of using deep relative include paths. Signed-off-by: Ben Levinsky <ben.levinsky@amd.com>
Fix bug in call to memset(), pass struct channel_s *, not struct channel_s **. Also remove the duplicate metal_irq_unregister() call from platform_cleanup(). Signed-off-by: Ben Levinsky <ben.levinsky@amd.com>
bentheredonethat
force-pushed
the
libmetal-rpu-shel-update
branch
from
4b05ce8 to
b6c1a0c
Compare
arnopo
left a comment
arnopo
left a comment
There was a problem hiding this comment.
Look good to me, thanks!
3 participants
Linux side allows user to specify non-cotniguous descriptor and data regions
ensure RPU side does the same
also include some stylistic fixes and 1 MPU fix