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.

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

mcore-demos: Upgrade to 6.6.52_2.2.0 #2056

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

mcore-demos: Upgrade to 6.6.52_2.2.0 #2056

wants to merge 2 commits into from
+20 −27

Conversation

zelan-nxp
Copy link
Contributor

@zelan-nxp zelan-nxp commented Jan 7, 2025 •
edited
Loading

imx95 mocre: 2.16.001 -> 24.12.00
Install elf binary to /usr/lib/firmware/

otavio
otavio requested changes Jan 7, 2025
View reviewed changes
Copy link
Member

@otavio otavio left a comment

Choose a reason for hiding this comment

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

The changes look good, but please improve the commit log.

@otavio otavio mentioned this pull request Jan 7, 2025
Open
27 tasks
@zelan-nxp zelan-nxp force-pushed the mcore-L6.6.52 branch 2 times, most recently from 205ae8d to b1cdd3d Compare January 9, 2025 03:14
@otavio
Copy link
Member

otavio commented Jan 13, 2025

Please rebase this pull request because I made the change to remove the checksum across all the files to a single commit. So you will need also to remove this from this and other pull requests.

@otavio otavio requested a review from thochstein January 14, 2025 11:01
thochstein
thochstein approved these changes Jan 14, 2025
View reviewed changes
Copy link
Contributor

@thochstein thochstein left a comment

Choose a reason for hiding this comment

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

Changes are fine, but it does seem odd that the 1.0.1 recipe doesn't use the .inc file.

@zelan-nxp zelan-nxp force-pushed the mcore-L6.6.52 branch 2 times, most recently from 1882c08 to 540f161 Compare January 15, 2025 06:37
@zelan-nxp
Copy link
Contributor Author

Changes are fine, but it does seem odd that the 1.0.1 recipe doesn't use the .inc file.
Good catch. imx7d m4 demos binary name is imx7d-sabresd-m4-freertos-1.0.1.bin, no elf in package, Which special case than others. I update 1.0.1 recipe to use inc file and overwrite SRC_URI and let don't do install task.

otavio
otavio reviewed Jan 15, 2025
View reviewed changes
recipes-fsl/mcore-demos/imx-m4-demos_1.0.1.bb Outdated
SRC_URI = "${FSL_MIRROR}/${M4_SOC}-m4-freertos-${PV}.bin;fsl-eula=true"
S = "${WORKDIR}/${M4_SOC}-m4-freertos-${PV}"
SRC_URI = "${FSL_MIRROR}/${SOC}-m4-freertos-${PV}.bin;fsl-eula=true"
S = "${WORKDIR}/${SOC}-m4-freertos-${PV}.bin"
Copy link
Member

Choose a reason for hiding this comment

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

Have you tried that? Because this change seems completely wrong.

Also, there is no package being generated, so it is just not generating the package to allow it to be deployed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry, The S directory was typo to .bin Updated.
Yes, imx7d mcore package no elf，So no need install them to /lib/firmware

Copy link
Member

Choose a reason for hiding this comment

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

Why not? How would I load the firmware during the runtime of the system?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

These firmware was defined WKS_FILE_DEPENDS, they are installed in /run/media/boot-mmcblk1p1/mcore-demos when bootup. Although imx7d mcore no elf package, the bin package also generated and put in DEPLOYDIR.

Copy link
Member

Choose a reason for hiding this comment

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

I believe it is wrong. We should allow it to be installed as the user may want to load it manually.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@zelan-nxp could you, please, double check what were the files provided by the previous version of this package? Also, is there a list of which SOC should use which mcore-demos version?

Yes, imx7d mcore keep 1.0.0 version for a long time, the previous release we also install them like this ( written in boot partition). Other SOC mcore version you can check ([https://github.com/Freescale/meta-freescale/blob/master/recipes-fsl/mcore-demos/README])

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not sure how it works. Most of the archives install a .elf to the rootfs and a .bin to the .wic. For this recipe, there is no .elf. Should the .bin go in the rootfs? For all SOCs?

Mcore .elf is only used for developer debug, Generally, .elf shouldn't be install into rootfs for all SOC ,but developer require to debug their demo packages so we install them in the later release. Mcore pakcage is some feature(like audio) simple validation, they should write in correct core address, I think its's non-sense to install .bin package into standalone rootfs.

Copy link
Contributor

Choose a reason for hiding this comment

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

Need to clarify here, this is not a request to install the firmware to the rootfs, it is a request to install the firmware to the main package. It's up to the user whether to install the main package to the rootfs. (We install the main package to our rootfs because I guess we want to provide the .elf in the rootfs: nxp-imx/meta-imx@5c887ec.)

This seems a reasonable request with no downside, and in fact one of our own did implement a userspace utility for loading the firmware: https://github.com/nxp-imx/imx-m4fwloader.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I agree. Install main package is reasonable even though no .elf supply. I will pull it in layer.conf

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks, @zelan-nxp, for adding it for 7d, but it seems to me that this is a general feature request, and the firmware should be installed for the other SOCs as well.

Please confirm, @otavio.

@thochstein thochstein requested a review from junzhuimx January 17, 2025 22:34
@otavio otavio requested review from otavio and thochstein February 4, 2025 17:05
@zelan-nxp zelan-nxp force-pushed the mcore-L6.6.52 branch 2 times, most recently from 8586c9c to c35b1b0 Compare March 4, 2025 10:08
zelan-nxp added 2 commits March 4, 2025 11:17
@zelan-nxp
mcore-demos: Upgrade to 6.6.52_2.2.0
ba75520 
imx95: 2.16.001 -> 24.12.00

Signed-off-by: Zelan Zou <[email protected]>
@zelan-nxp
imx-base.inc: Install imx7d mcore to rootfs
d4c589d 
Signed-off-by: Zelan Zou <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@angolini angolini

@junzhuimx junzhuimx

@otavio otavio

@thochstein thochstein

Assignees
No one assigned
Labels
backport scarthgap
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@zelan-nxp @otavio @angolini @thochstein