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

zephyr: fixup kv260_r5 DT memory until it can be fixed in Zephyr #29

Merged
merged 1 commit into from
Oct 29, 2023

Conversation

wmamills
Copy link
Collaborator

We are currently using Zephyr v3.5.0.
The memory definitions for kv260_r5 are inappropriate for general usage and for ours.

The current flash definition refers to the QSPI XIP area of the zynqmp. This is unlikely to be appropriate to use in the general case. The current usage also assume use at offset 0 in the QSPI which is even less likely to be appropriate. It is OK zynqmp_r5 to define this (perhaps with the name of qspi-xip) but it should not be selected as the default for flash. Let people that needs this opt into it.

For now we disable the zephyr,flash choice. This does not eliminate the memory report line but it at least shows a region size of 0.

Likewise for the definition of RAM. Zynqmp_r5 defines 64MB of RAM starting at offset 0. This would be a mix of TCM and DDR depending on the R5 mode split vs lockstep. In lockstep, DDR would be used for anything >=256K. For split mode DDR would be used for 64K to 128K-1 and for >=198K.

Using DDR like this is dangerous if another OS is running on the A53s. Linux for example does not reserve this early DDR memory for use by the the R5(s).

For now assume lockstep (only mode supported by Zephyr v3.5.0) as provide a new memory definition for combined TCM and use it for RAM.

@wmamills wmamills requested review from uLipe and tnmysh October 24, 2023 16:12
@wmamills wmamills self-assigned this Oct 24, 2023
@wmamills wmamills marked this pull request as draft October 24, 2023 16:12
@wmamills wmamills changed the title zephyr: fixup kv260_r5 DT memory until it can be fixed in Zephyr WIP: zephyr: fixup kv260_r5 DT memory until it can be fixed in Zephyr Oct 24, 2023
@wmamills
Copy link
Collaborator Author

I have built the rpmsg_multi_services example and this seems OK.
I have not run it and I have not run it with Linux.
@tnmysh can you test please?

Assuming this works Ok I will change qemu_cortex_r5 in our sample also. This is a defferent situation. The qemu_cortex_r5 is suppose to be a generic R5 simulator so using 64MB of TCM & DDR is fine. It is also Ok to use QSPI flash and assume offset 0 is usable.

However we are currently using qemu_cortex_r5 as a standin for the zcu102_r5. This is our problem and should be fixed. For now just do the same changes for qemu_cortex_r5 as we do for kv260_r5. Perhpas we can use an include file. I will look into it.

We are currently using Zephyr v3.5.0.
The memory definitions for kv260_r5 are inappropriate for general usage
and for ours. (This is partially our doing so not throwing stones.)

The current flash definition refers to the QSPI XIP area of the zynqmp.
This is unlikely to be appropriate to use in the general case.  The
current usage also assumes use at offset 0 in the QSPI which is even less
likely to be appropriate.  It is OK zynqmp_r5 to define this (perhaps with the name
of qspi-xip) but it should not be selected as the default for flash.
Let people that needs this opt into it.

For now we disable the zephyr,flash choice.  This does not eliminate the
memory report line but it at least shows a region size of 0.

Likewise for the definition of RAM.  Zynqmp_r5 defines 64MB of RAM
starting at offset 0.  This would be a mix of TCM and DDR depending on the R5 mode
split vs lockstep mode.

In lockstep, DDR would be used for anything >=256K.
For split mode, DDR would be used for 64K to 128K-1 and for >=192K.

Using DDR like this is dangerous if another OS is running on the A53s.
Linux for example does not reserve this early DDR memory for use by the
the R5(s).

For now assume lockstep (only mode supported by Zephyr v3.5.0) and provide
a new memory definition for combined TCM and use it for RAM.

Put all these overrides in an include file so they are easy to undo when
Zephyr upstream is fixed.

We are using qemu_cortex_r5 as stand-in for zcu102_r5 for right now so do
the same for that board.  However once we have zcu102_r5 we should leave
qemu_cortex_r5 alone as its purpose is a generic R5 target not a full
zynqmp system.

Signed-off-by: Bill Mills <bill.mills@linaro.org>
@wmamills
Copy link
Collaborator Author

I have pushed a new version that uses an include file and modifies qemu_cortex_r5 also.
@tnmysh Please review and test.

@wmamills wmamills changed the title WIP: zephyr: fixup kv260_r5 DT memory until it can be fixed in Zephyr zephyr: fixup kv260_r5 DT memory until it can be fixed in Zephyr Oct 24, 2023
@wmamills wmamills marked this pull request as ready for review October 24, 2023 19:24
Copy link
Collaborator

@tnmysh tnmysh left a comment

Choose a reason for hiding this comment

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

Tested on kv260 board. It is working.

Looks good to me.

@wmamills wmamills merged commit 5ade828 into OpenAMP:main Oct 29, 2023
1 check passed
@wmamills wmamills deleted the wip-kv260-dts branch October 29, 2023 20:23
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.

3 participants