Skip to content

8mm evk a53 support #32694

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.

Sign up for GitHub

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

Jump to bottom
Closed
MrVan wants to merge 6 commits into from
Closed

8mm evk a53 support #32694

MrVan wants to merge 6 commits into from

Conversation

@MrVan
Copy link
Contributor

@MrVan MrVan commented Feb 26, 2021

This goal is to support Zephyr runs on top of Jailhouse hypervisor.

This patchset is the first step to make Zephyr runs on i.MX8MM A53 core from U-Boot.

@MrVan MrVan requested a review from hakehuang February 26, 2021 08:48
@Mani-Sadhasivam Mani-Sadhasivam self-requested a review February 26, 2021 09:30
hakehuang
hakehuang reviewed Feb 26, 2021
View reviewed changes
Copy link
Contributor

@hakehuang hakehuang left a comment

Choose a reason for hiding this comment

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

suggest to create an issues to explain on how to use this with uboot + jailhouse.

boards/arm/mimx8mm_evk_a53/board.cmake Outdated
Copy link
Contributor

@hakehuang hakehuang Feb 26, 2021

Choose a reason for hiding this comment

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

need add debugger support
`
board_set_debugger_ifnset(jlink)
board_set_flasher_ifnset(jlink)

board_runner_args(jlink "--device=MIMX8MD6_A53")
include(${ZEPHYR_BASE}/boards/common/jlink.board.cmake)
`

Copy link
Contributor Author

@MrVan MrVan Mar 8, 2021

Choose a reason for hiding this comment

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

I am not sure, but I not see this "./boards/arm/bcm958402m2_a72/board.cmake" and "boards/arm/qemu_cortex_a53/board.cmake", Is it really needed?

Copy link
Contributor

@hakehuang hakehuang Mar 11, 2021

Choose a reason for hiding this comment

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

without that you can not use west flash to download the application. how do you program the board now?

Copy link
Contributor Author

@MrVan MrVan Mar 11, 2021

Choose a reason for hiding this comment

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

Use U-Boot to load/kick the zephyr.bin or use Linux to load it for hypervisor usage. zephyr.bin is stored in sdcard fat partition.

Copy link
Contributor

@hakehuang hakehuang Mar 11, 2021

Choose a reason for hiding this comment

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

@MrVan the official way is to use west flash

Copy link
Contributor

@hakehuang hakehuang Mar 11, 2021

Choose a reason for hiding this comment

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

then user need extra steps to setup uboot, and as uboot is evovled overtime, this needs some extra documents and efforts.

Copy link
Contributor

@carlocaione carlocaione Mar 11, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

How to use U-Boot is already documented in this commit and that is enough. Fine to me if we can use west flash to flash the board as well but there is no such a thing as "official" way to flash or use the board. @MrVan is it a problem adding support for JLink?

Copy link
Contributor Author

@MrVan MrVan Mar 12, 2021

Choose a reason for hiding this comment

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

I am not familar with JLink. To me, we have sdcard to hold zephyr.bin, zephyr.bin is copied to the sdcard by mounting the sdcard to Linux PC. Could west flash support writing fat partition of a sdcard? Actually we have yocto Linux for image generating.

Copy link
Contributor

@hakehuang hakehuang Mar 12, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

@MrVan west does not support fat partition write, it is a different way. It may not be officially defined to use west flash, but basically we expect to have an easy to use solution. Besides if there no debuger tools supported, how users suppose to debug the program? IMHO, it is not a good enough enablement.

Copy link
Contributor

@carlocaione carlocaione Mar 12, 2021

Choose a reason for hiding this comment

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

@MrVan west does not support fat partition write, it is a different way. It may not be officially defined to use west flash, but basically we expect to have an easy to use solution. Besides if there no debuger tools supported, how users suppose to debug the program? IMHO, it is not a good enough enablement.

I disagree. U-Boot + SD is indeed an easy solution to use in this case (and pretty standard when dealing with AArch64 boards, much more common than having a J-Link available) and you cannot expect that everybody has access to a debugger / J-Link or that every board has a debug port easily accessible. So, this enablement is good enough for merging.

Now, that said @MrVan, can you add what suggested here #32694 (comment) so that west flash can be used when a debugger is available? This doesn't prevent the user from using U-Boot and the method you already described.

Please, also update the documentation describing both the methods.

carlescufi
carlescufi reviewed Feb 26, 2021
View reviewed changes
galak
galak reviewed Feb 26, 2021
View reviewed changes
@Mani-Sadhasivam
Copy link
Member

Mani-Sadhasivam commented Feb 26, 2021

@MrVan FWIW, I worked on this a while ago but never managed to clean it up and submit the PR:
https://github.com/Mani-Sadhasivam/zephyr/commits/jailhouse

I had to modify the GIC driver to support GIC v2.

carlocaione
carlocaione reviewed Feb 26, 2021
View reviewed changes
carlocaione
carlocaione reviewed Feb 26, 2021
View reviewed changes
carlocaione
carlocaione reviewed Feb 26, 2021
View reviewed changes
carlocaione
carlocaione reviewed Feb 26, 2021
View reviewed changes
carlocaione
carlocaione reviewed Feb 26, 2021
View reviewed changes
carlocaione
carlocaione reviewed Feb 26, 2021
View reviewed changes
@MrVan
Copy link
Contributor Author

MrVan commented Mar 8, 2021

@MrVan FWIW, I worked on this a while ago but never managed to clean it up and submit the PR:
https://github.com/Mani-Sadhasivam/zephyr/commits/jailhouse

I had to modify the GIC driver to support GIC v2.

Wow. what usecases you are making zephyr on Jailhouse for?

@MrVan MrVan force-pushed the 8mm-v1 branch 4 times, most recently from 542e890 to 90ca497 Compare March 9, 2021 07:13
@MrVan
Copy link
Contributor Author

MrVan commented Mar 9, 2021

updated, please help check.

@carlocaione
Copy link
Contributor

carlocaione commented Jan 24, 2022

@MrVan I don't think that's needed. Can you rebase? I'll try to push this.

@carlocaione carlocaione dismissed stale reviews from Mani-Sadhasivam and galak January 24, 2022 08:03

stale

@carlocaione carlocaione self-assigned this Jan 24, 2022
@MaureenHelm
Copy link
Member

MaureenHelm commented Jan 25, 2022

@galak,

@MaureenHelm we need to figure out how to handle the drivers for this. I don't think it makes sense to have duplicated HAL & non-HAL drivers. Doubles our support efforts.

Is this comment/block still valid?

This comment has been addressed

Since no response. I am not sure people still happy to review this one year pending PR. I would create a new PR to restart the work.

Agree with @carlocaione , please rebase this PR instead.

gmarull
gmarull requested changes Jan 25, 2022
View reviewed changes
Copy link
Member

@gmarull gmarull left a comment

Choose a reason for hiding this comment

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

A few questions

@MrVan MrVan force-pushed the 8mm-v1 branch 2 times, most recently from 93cfa87 to 0cbe1ea Compare February 8, 2022 03:53
@MrVan
Copy link
Contributor Author

MrVan commented Feb 8, 2022

@carlocaione @MaureenHelm sorry for late reply, just back from vocation. I have addressed all the comments.

@carlocaione carlocaione requested a review from gmarull February 8, 2022 08:29
gmarull
gmarull reviewed Feb 8, 2022
View reviewed changes
carlocaione
carlocaione reviewed Feb 8, 2022
View reviewed changes
gmarull
gmarull reviewed Feb 8, 2022
View reviewed changes
MrVan added 6 commits February 8, 2022 18:41
@MrVan
soc: arm64: nxp_imx: Add i.MX8M Mini SoC A53 support
5f80d7f 
Add minimal SoC support for the NXP i.MX8M Mini series Cortex-A53 Core.

Signed-off-by: Peng Fan <peng.fan@nxp.com>
@MrVan
dts: arm64: nxp: add i.MX8MM A53 device tree
ca78514 
Add i.MX8MM A53 device tree

Signed-off-by: Peng Fan <peng.fan@nxp.com>
@MrVan
board: arm64: add i.MX8MM EVK A53 board support
4f5a848 
Add i.MX8MM EVK A53 board support.
Zephyr is kicked from NS-EL1 U-Boot.

Signed-off-by: Peng Fan <peng.fan@nxp.com>
@MrVan
board: arm64: imx8mm_evk: add jailhouse hypervisor inmate support
c72ea41 
This is to let zephyr run in an inmate cell on top of Jailhouse
hypervisor.

The step as below:
jailhouse enable imx8mm.cell
ailhouse cell create imx8mm-zephyr.cell
jailhouse cell load 1 zephyr.bin -a 0x93c00000
jailhouse cell start 1

Currently we have not use loader to kick zephyr, later we will
use jailhouse loader to kick zephyr.

Signed-off-by: Peng Fan <peng.fan@nxp.com>
@MrVan
CODEOWNERS: update CODEOWNERS for i.MX8MM
21b2b64 
Update CODEOWNERS for i.MX8MM

Signed-off-by: Peng Fan <peng.fan@nxp.com>
@MrVan
board: arm64: imx8mm_evk: add a dummy file
120a53e 
Because we currently use RAM_CONSOLE for now, and there is actually
no device in the build, so there is twister build failure,
add a dummy file to avoid build failure.

Signed-off-by: Peng Fan <peng.fan@nxp.com>
@MrVan
Copy link
Contributor Author

MrVan commented Feb 9, 2022

@carlocaione @MaureenHelm Ready for merge now?

@carlocaione
Copy link
Contributor

carlocaione commented Feb 9, 2022

@carlocaione @MaureenHelm Ready for merge now?

Yes, but we are in stabilization period now. This will be merged when 3.0 will be released.

mbolivar-nordic
mbolivar-nordic suggested changes Feb 22, 2022
View reviewed changes
soc/arm64/nxp_imx/imx8m/mmu_regions.c
Comment on lines +34 to +41
DT_REG_ADDR(DT_INST(0, nxp_imx_ana)),
DT_REG_SIZE(DT_INST(0, nxp_imx_ana)),
MT_DEVICE_nGnRnE | MT_P_RW_U_RW | MT_NS),

MMU_REGION_FLAT_ENTRY("UART",
DT_REG_ADDR(DT_INST(1, nxp_imx_iuart)),
DT_REG_SIZE(DT_INST(1, nxp_imx_iuart)),
MT_DEVICE_nGnRnE | MT_P_RW_U_RW | MT_NS),
Copy link
Contributor

@mbolivar-nordic mbolivar-nordic Feb 22, 2022

Choose a reason for hiding this comment

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

Hard-coded instance numbers like this are incorrect to use. Please use an alternate mechanism to specify exactly the nodes you want. Read the DT_INST docstring for more details.

Copy link
Contributor Author

@MrVan MrVan Feb 28, 2022

Choose a reason for hiding this comment

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

Could we use a follow up patch to address this? I just follow qemu platform here. @carlocaione do you have any suggestions?

Copy link
Member

@Mani-Sadhasivam Mani-Sadhasivam Feb 28, 2022

Choose a reason for hiding this comment

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

Considering the amount of time this PR has been in an open state (1 year), I think we should merge this and follow up with PRs for fixing issues like this. @mbolivar-nordic what do you think?

Copy link
Contributor

@mbolivar-nordic mbolivar-nordic Mar 1, 2022

Choose a reason for hiding this comment

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

No, sorry, I don't want to see incorrect usage of the instance API merged.

Copy link
Contributor Author

@MrVan MrVan Mar 3, 2022

Choose a reason for hiding this comment

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

@mbolivar-nordic I am not experts in zephyr, so any suggestions what I should use?

@MrVan
Copy link
Contributor Author

MrVan commented Mar 4, 2022

A bit werid, I have updated my 8mm-v1 branch, but this PR not update automatically.

@MrVan
Copy link
Contributor Author

MrVan commented Mar 4, 2022

A bit werid, I have updated my 8mm-v1 branch, but this PR not update automatically.

I am not sure why PR not update automatically. what could I do here? Close this PR and reopen a new one?

@MrVan MrVan mentioned this pull request Mar 4, 2022
Closed
@stephanosio stephanosio closed this Mar 4, 2022
@stephanosio
Copy link
Member

stephanosio commented Mar 4, 2022

A bit werid, I have updated my 8mm-v1 branch, but this PR not update automatically.

I am not sure why PR not update automatically. what could I do here? Close this PR and reopen a new one?

@MrVan It looks like something broke on the GitHub side -- we have seen the same issue in other PRs lately. Please open a new PR.

@MrVan MrVan mentioned this pull request Mar 4, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@carlocaione carlocaione carlocaione approved these changes

@carlescufi carlescufi carlescufi left review comments

@hakehuang hakehuang hakehuang approved these changes

@MaureenHelm MaureenHelm MaureenHelm approved these changes

@gmarull gmarull gmarull approved these changes

@nashif nashif Awaiting requested review from nashif

@ioannisg ioannisg Awaiting requested review from ioannisg

@dleach02 dleach02 Awaiting requested review from dleach02

@mmahadevan108 mmahadevan108 Awaiting requested review from mmahadevan108

@galak galak Awaiting requested review from galak

@Mani-Sadhasivam Mani-Sadhasivam Awaiting requested review from Mani-Sadhasivam

+1 more reviewer

@mbolivar-nordic mbolivar-nordic mbolivar-nordic requested changes

Reviewers whose approvals may not affect merge requirements

Assignees

@carlocaione carlocaione

Labels

area: API Changes to public APIs area: Boards area: Devicetree Binding PR modifies or adds a Device Tree binding area: Devicetree area: Documentation DNM This PR should not be merged (Do Not Merge) manifest manifest-hal_nxp platform: NXP NXP

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.