zynqmp_r5: replace Xil_SetMPURegion with Xil_MemMap

[kernelchuk]

The metal_machine_io_mem_map() used to call Xil_SetMPURegion directly
and could get into an infinite loop. Replace the faulty code in
metal_machine_io_mem_map() with Xil_MemMap() call and assert it succeeds.

Signed-off-by: Sergei Korneichuk sergei.korneichuk@amd.com

fixes: 51e08c6

[tnmysh]

@arnopo, @edmooring

#51: FILE: lib/system/freertos/zynqmp_r5/sys.c:69:
[23](https://github.com/OpenAMP/libmetal/runs/8046721200?check_suite_focus=true#step:6:24)
+	void *__attribute__((unused)) physaddr = Xil_MemMap(pa, size, flags);
[24](https://github.com/OpenAMP/libmetal/runs/8046721200?check_suite_focus=true#step:6:25)

Above warning is false positive I believe. As it is not function definition, but Xil_MemMap is function use.
However, would you recommend not to use attribute() in the same line where variable is initialized?
I don't see any downside of it, but I may be wrong.

[arnopo]

@arnopo, @edmooring

#51: FILE: lib/system/freertos/zynqmp_r5/sys.c:69:
[23](https://github.com/OpenAMP/libmetal/runs/8046721200?check_suite_focus=true#step:6:24)
+	void *__attribute__((unused)) physaddr = Xil_MemMap(pa, size, flags);
[24](https://github.com/OpenAMP/libmetal/runs/8046721200?check_suite_focus=true#step:6:25)

Above warning is false positive I believe. As it is not function definition, but Xil_MemMap is function use. However, would you recommend not to use attribute() in the same line where variable is initialized? I don't see any downside of it, but I may be wrong.

Following code seeems not generate this false positive:

void *metal_machine_io_mem_map(void *va, metal_phys_addr_t pa,
			       size_t size, unsigned int flags)
{
	void *__attribute__((unused)) physaddr;

	physaddr = Xil_MemMap(pa, size, flags);
	metal_assert(physaddr == (void *)pa);
	return va;
}

[kernelchuk]

Thank you Arnaud, I fixed the code, I should have done it when it was flagged before sending the first PR. Not sure why Zephyr build fails. Any hints ?

[arnopo]

Thank you Arnaud, I fixed the code, I should have done it when it was flagged before sending the first PR. Not sure why Zephyr build fails. Any hints ?

No idea yet, probably a problem in the IC. I tried to rerun the action without success. I need to look deeper. Just ignore the check for now.

[arnopo]

@bentheredonethat
Any feedback on this PR?
Thanks

[bentheredonethat]

@arnopo i am in support of this patch. AMD BSP provides this so we should leverage as proposed by Sergei.

[edmooring]

Looks good to me.

[arnopo]

Please could you rebase on main to rerun the Ci? CI issue should be fixed in #219

[kernelchuk]

Hi Arnaud, Ed,
I have about 30 commits with bug fixes in our AMD/Xilinx internal repos and I'd like to upstream them. I can

1. Create PRs for each of them
2. Combine all or some of them in a one PR

What is your preferred method of up-steaming these commits?

[arnopo]

just an update for the copyright else LGTM

[arnopo]

Hi Arnaud, Ed, I have about 30 commits with bug fixes in our AMD/Xilinx internal repos and I'd like to upstream them. I can

1. Create PRs for each of them
2. Combine all or some of them in a one PR

What is your preferred method of up-steaming these commits?

it depends on the commits. I would say obvious commits in a package, and a PR per commit that might trigger more discussion.

[edmooring]

Agreed on the 30 commits. As a rough guideline, combining commits that don't overlap and only affect one file each is fine. A multi-file commit probably deserves its own PR.