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

RPI5: add scmi and wifi support #41

Merged
merged 7 commits into from
Sep 24, 2024
Merged

Conversation

GrygiriiS
Copy link
Collaborator

@GrygiriiS GrygiriiS commented Sep 5, 2024

  • Switch ARM TF and XEN on rpi5 dev branches with SCMI support.
  • added DT overlays for both SCMI and WIFI.
  • updated u-boot script
  • Updated rpi5.yaml to add option ENABLE_SCMI and add option to ENABLE_WIFI

Both ENABLE_SCMI and ENABLE_WIFI are disabled by default.

NOTE1. if ENABLE_SCMI is enabled it will change SRAM mapping for Zephyr Dom0 application, so to run, the rpi_5.overlay has to be edited to correct "sram0: memory@10000000" node with proper values.

@GrygiriiS
Copy link
Collaborator Author

@GrygiriiS GrygiriiS force-pushed the rpi5_scmi_wifi_v1 branch 2 times, most recently from 51bb31d to 95c6408 Compare September 6, 2024 06:37
@rshym
Copy link
Collaborator

rshym commented Sep 6, 2024

Acked-by: Ruslan Shymkevych <[email protected]>

@GrygiriiS
Copy link
Collaborator Author

This PR updated:

  • dropped private dev branches
  • dropped Zephyr Dom0 Scmi
    In general, it's ready, but we'ra waiting for Xen SCMI support to be finished.

Test branch to be used for integration can be found here:
https://github.com/GrygiriiS/meta-xt-prod-devel-rpi5/tree/rpi5_scmi_wifi_v1_dev

@oleksiimoisieiev
Copy link
Collaborator

I think README should be updated as well

@rshym
Copy link
Collaborator

rshym commented Sep 17, 2024

After the repush, the first commit message still mentions AUTOREV.
The second commit has 'No end-of-the-file" (Red crossed circle).

@@ -0,0 +1,2 @@
CONFIG_ARM_SCI=y
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is this change should be in this commit? Commit message does not mention enabling SCMI

# Current master branch
SRC_URI_TRUSTED_FIRMWARE_A = "git://github.com/xen-troops/arm-trusted-firmware.git;protocol=https"

SRCREV_tfa = "${AUTOREV}"
Copy link
Collaborator

Choose a reason for hiding this comment

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

As we are going to merge this into the main branch, we don't need AUTOREV here

@@ -0,0 +1,150 @@
// SPDX-License-Identifier: GPL-2.0
Copy link
Collaborator

Choose a reason for hiding this comment

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

It would be nice to have more extended commit message for this change


SCMI_PCIE1_DT_NAME_ADD = "${@bb.utils.contains("MACHINE_FEATURES", "domd_nvme", "${SCMI_DOMD_PCIE1_DT_NAME}.dtbo", "", d)}"

DOMD_OVERLAYS:append = "${@bb.utils.contains("MACHINE_FEATURES", "scmi", " ${SCMI_DOMD_DT_NAME}.dtbo", "", d)}"
DOMD_OVERLAYS:append = "${@bb.utils.contains("MACHINE_FEATURES", "scmi", " ${SCMI_PCIE1_DT_NAME_ADD}", "", d)}"

DOMD_OVERLAYS:append = "${@bb.utils.contains("MACHINE_FEATURES", "domd_wifi", " ${DOMD_WIFI_DT_NAME}.dtbo", "", d)}"
Copy link
Collaborator

Choose a reason for hiding this comment

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

IMO, this approach for adding dtbo is getting out of hand. Lots of almost duplicated code....

Also, commit message clearly mentions linux: but you are making changes not only to linux.

@@ -365,3 +365,23 @@ parameters:
conf:
- [DOMD_OVERLAYS, "%{SOC_FAMILY}-%{MACHINE}-pcie1.dtbo"]
- [MACHINE_FEATURES:append, " domd_nvme"]

ENABLE_WIFI:
Copy link
Collaborator

Choose a reason for hiding this comment

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

IMO, this patch can be folded into previous one

rpi5.yaml Outdated
"start4db.elf": "%{YOCTOS_WORK_DIR}/%{DOMD_BUILD_DIR}/tmp/deploy/images/%{MACHINE}/bootfiles/start4db.elf"
"start4cd.elf": "%{YOCTOS_WORK_DIR}/%{DOMD_BUILD_DIR}/tmp/deploy/images/%{MACHINE}/bootfiles/start4cd.elf"
"start4x.elf": "%{YOCTOS_WORK_DIR}/%{DOMD_BUILD_DIR}/tmp/deploy/images/%{MACHINE}/bootfiles/start4x.elf"
"/": "%{YOCTOS_WORK_DIR}/%{DOMD_BUILD_DIR}/tmp/deploy/images/%{MACHINE}/bootfiles"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Have you tested this? Does ninja correctly handles changes in the files?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

ninja has nothing to do with this - this is handled by Rouge tool. And it works for directory.

Copy link
Collaborator

Choose a reason for hiding this comment

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

ninja has everything to do with this, as it parses dependencies and decides what should be done before invoking the rouge tool. With your approach there is no guarantee that image file will include any bootfiles at all. If Yocto for some reason will not build them, rouge will pack just an empty directory. Also, ninja might not trigger image rebuild if some files inside directory were changed. We need to test this.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Have you tried by your self delete any file deploy/images/? run ninja it will succeed, but ninja full.img - will fail. So do not know what "ninja handles dependency" function you refer here to :(

Next in "build.ninja" there is below - which is just call of rouge with parameters:
build full.img: rouge yocto/build-domd/tmp/deploy/images/raspberrypi5/armstub8-2712.bin $
yocto/build-domd/tmp/deploy/images/raspberrypi5/bootfiles $
yocto/build-domd/tmp/deploy/images/raspberrypi5/Image.gz $

And the last, and most important, we DO NOT maintain bootfiles recipe. it come from meta-raspberrypi and can be changed there by adding or removing some bootloader files. Now if smth is removed - ok rouge will fail. But if added - the build will succeed (including image generation), but RPI5 boot will fail. It make no sense for us to maintain full list of RPI5 bootfiles here (and moreover It is not safe) as we do not own it. What is reasonable is to have dependency on one or two files and if it's satisfied then assume bootfiles build is ok and just copy ALL bootfiles in the rootfs image.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

And moulin documentation:
https://moulin.readthedocs.io/en/latest/user-reference.html
yocto builder https://moulin.readthedocs.io/en/latest/user-reference.html#yocto-builder

target_images - list of image files that should be generated by this component as a result of invoking $ bitbake {build_target}. Every component should generate at least one image file.
Note. Every component should generate at least one image file. So no need to list ALL binaries.

additional_deps - list of additional dependencies. This is basically target_images produced by other components. You can use those to implement build dependencies between components. For example, if your system needs to have DomU’s kernel image on Dom0 file system, you might want to add path to DomU’s kernel into additional_deps of Dom0’s config. This will ensure that Dom0 will be built after DomU.

Note. additional_deps description clearly mention that NO NEED to add dependencies for ALL images, just for key images.

Rouge docs: https://moulin.readthedocs.io/en/latest/rouge.html#filesystem-image-with-files
You can specify parent folders for remote and these folders will be created on the destination filesystem. You may specify not only files but directories also. If the local directory contains subdirectories, they will be created unders the remote directory.
As per above, just again. this change is valid usage of moulin/rouge.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Now if smth is removed - ok rouge will fail. But if added - the build will succeed (including image generation), but RPI5 boot will fail. It make no sense for us to maintain full list of RPI5 bootfiles here (and moreover It is not safe) as we do not own it.

In my opinion it is better to fail during the build process, not during runtime. This will save us a lot of headache in case of any issues in the future. This is another reason why I want to maintain the complete list of required files.


XEN_URL = "git://github.com/xen-troops/xen.git;protocol=https;branch=rpi5_dev"
XEN_REL = "4.19"
XEN_REV = "2214507e72c2c2217b3234478912be6581324fbf"
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think AUTOREV should be set here.

rpi5.yaml Outdated
@@ -271,25 +271,7 @@ images:
size: 512 MiB
items:
"armstub8-2712.bin": "%{YOCTOS_WORK_DIR}/%{DOMD_BUILD_DIR}/tmp/deploy/images/%{MACHINE}/armstub8-2712.bin"
"bootcode.bin": "%{YOCTOS_WORK_DIR}/%{DOMD_BUILD_DIR}/tmp/deploy/images/%{MACHINE}/bootfiles/bootcode.bin"
"config.txt": "%{YOCTOS_WORK_DIR}/%{DOMD_BUILD_DIR}/tmp/deploy/images/%{MACHINE}/bootfiles/config.txt"
"cmdline.txt": "%{YOCTOS_WORK_DIR}/%{DOMD_BUILD_DIR}/tmp/deploy/images/%{MACHINE}/bootfiles/cmdline.txt"
Copy link
Collaborator

Choose a reason for hiding this comment

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

This could be the fix of #3

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I have no idea

rpi5.yaml Outdated
@@ -271,25 +271,7 @@ images:
size: 512 MiB
items:
"armstub8-2712.bin": "%{YOCTOS_WORK_DIR}/%{DOMD_BUILD_DIR}/tmp/deploy/images/%{MACHINE}/armstub8-2712.bin"
Copy link
Collaborator

Choose a reason for hiding this comment

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

full.imag -> full.img in the commit descrition.

@oleksiimoisieiev
Copy link
Collaborator

This approach to add all scmi related stuff in meta-xt-prod-devel looks as the only acceptable solution. That's because there is a significant difference in Zephyr (Dom0) memory mapping with/w-o SCMI. So we can't make it completely optional (controlled by config param) without making 2 zephyr branches or using special snippets. In any case - every configuration will have it's specific memory-mapping. So when all configuration is on the top layer - it can be used as the reference for making project-specific top meta-layer with or without SCMI enabled.
This approach looks good so
Reviewed-by: Oleksii Moisieiev <[email protected]>
with a small comments.

@lorc
Copy link
Collaborator

lorc commented Sep 20, 2024

Thanks

Acked-by: Volodymyr Babchuk <[email protected]>

Grygorii Strashko and others added 7 commits September 24, 2024 11:23
Switch Xen to xen-troops/xen/rpi5_dev branch with SCMI support.

The SCMI is enabled if MACHINE_FEATURES contains "scmi".

Signed-off-by: Grygorii Strashko <[email protected]>
Acked-by: Ruslan Shymkevych <[email protected]>
Acked-by: Volodymyr Babchuk <[email protected]>
Switch trusted-firmware-a to xen-troops/arm-trusted-firmware/rpi5_dev
branch with scmi support.

The SCMI is enabled if MACHINE_FEATURES contains "scmi".

Signed-off-by: Grygorii Strashko <[email protected]>
Acked-by: Ruslan Shymkevych <[email protected]>
Acked-by: Volodymyr Babchuk <[email protected]>
Add SCMI DT overlays.

System/Xen DT overlay bcm2712-raspberrypi5-xen-scmi.dtso
- adds SCMI node with SMC/HVC transport, resets and pinctrl protocols
support, and access-controllers support.

DomD partial DT overlays
- bcm2712-raspberrypi5-domd-scmi.dtso
- bcm2712-raspberrypi5-pcie1-scmi.dtso
those add skeleton SCMI node and switch PCIE to use SCMI resets.

Signed-off-by: Grygorii Strashko <[email protected]>
Acked-by: Ruslan Shymkevych <[email protected]>
Acked-by: Volodymyr Babchuk <[email protected]>
Add *ENABLE_SCMI* option to enable ARM SCMI for RPI5.
ARM SCMI is *disabled* by default, as Zephyr Dom0 application will not
start with ENABLE_SCMI=yes due to memory map changes.

Signed-off-by: Grygorii Strashko <[email protected]>
Acked-by: Ruslan Shymkevych <[email protected]>
Acked-by: Volodymyr Babchuk <[email protected]>
Add wifi related packages if wifi support is needed.

Signed-off-by: Dmytro Semenets <[email protected]>
Signed-off-by: Grygorii Strashko <[email protected]>
Acked-by: Ruslan Shymkevych <[email protected]>
Acked-by: Volodymyr Babchuk <[email protected]>
The WIFI is supported only with SCMI.

Add passthrough nodes in xen dtb:
* sdio2
* pinctrl

Add dt overlay with appropriate nodes for Xen and domd's device trees.

Signed-off-by: Dmytro Semenets <[email protected]>
Signed-off-by: Grygorii Strashko <[email protected]>
Acked-by: Ruslan Shymkevych <[email protected]>
Acked-by: Volodymyr Babchuk <[email protected]>
Add option for wifi support in domd. It's disabled by default.

Signed-off-by: Dmytro Semenets <[email protected]>
Signed-off-by: Grygorii Strashko <[email protected]>
Acked-by: Ruslan Shymkevych <[email protected]>
Acked-by: Volodymyr Babchuk <[email protected]>
@lorc lorc merged commit c41b536 into xen-troops:main Sep 24, 2024
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.

5 participants