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

boot: zephyr: nxp: Add NXP platforms to the allow list #2076

Merged
merged 1 commit into from
Dec 11, 2024

Conversation

butok
Copy link
Contributor

@butok butok commented Sep 25, 2024

Adds NXP platforms to the boot zephyr allow list.

@de-nordic de-nordic added the area: zephyr Affects the Zephyr port label Oct 2, 2024
Comment on lines 9 to 39
- frdm_k22f
- frdm_k64f
- frdm_k82f
- frdm_ke17z
- frdm_ke17z512
- rddrone_fmuk66
- twr_ke18f
- twr_kv58f220m
- frdm_mcxn947/mcxn947/cpu0
- lpcxpresso55s06
- lpcxpresso55s16
- lpcxpresso55s28
- lpcxpresso55s36
- lpcxpresso55s69/lpc55s69/cpu0
- mimxrt1010_evk
- mimxrt1015_evk
- mimxrt1020_evk
- mimxrt1024_evk
- mimxrt1040_evk
- mimxrt1050_evk
- mimxrt1060_evk
- mimxrt1062_fmurt6
- mimxrt1064_evk
- mimxrt1160_evk/mimxrt1166/cm7
- mimxrt1170_evk/mimxrt1176/cm7
- vmu_rt1170/mimxrt1176/cm7
- mimxrt595_evk/mimxrt595s/cm33
- mimxrt685_evk/mimxrt685s/cm33
- rd_rw612_bga
- nrf52840dk/nrf52840
- disco_l475_iot1
Copy link
Collaborator

Choose a reason for hiding this comment

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

Since this is reworked... can we have these in alphabetical order?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Since this is reworked... can we have these in alphabetical order?

I tried to have it per vendor, not alphabetical.

BTW: Just an idea.
Can we add "mcuboot" to the board's .yaml "supported:" section and avoid adding boards to the allow list?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Since this is reworked... can we have these in alphabetical order?

I tried to have it per vendor, not alphabetical.

Yeah, so then you have primary by vendor, secondary by alphabet, or primary by vendor secondary by chaos.
Or just everything goes by alphabet.

Anyway, up to you, but I am in favor of everything alphabetically.

BTW: Just an idea. Can we add "mcuboot" to the board's .yaml "supported:" section and avoid adding boards to the allow list?

Haven't thought about that.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, so then you have primary by vendor, secondary by alphabet, or primary by vendor secondary by chaos.
Or just everything goes by alphabet.

Not so easy;)
By vendor, then by family (kinetis, RT, LPC...), so it may look like a bit chaotic but better for real searching.

Haven't thought about that.

It's not convenient to update all these lists in different tests/samples.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, so then you have primary by vendor, secondary by alphabet, or primary by vendor secondary by chaos.
Or just everything goes by alphabet.

@de-nordic
According your request, it has been updated by vendor and after by alphabet.

BTW:
CI is broken.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

CI is broken.

Very strange.
According error logs:

Should I close the PR and try to create a new one?

@butok butok force-pushed the add_nxp_mcuboot_zephyr branch from 385456e to c81e0f6 Compare November 26, 2024 09:25
@butok butok requested a review from nordicjm as a code owner November 26, 2024 09:25
@butok butok force-pushed the add_nxp_mcuboot_zephyr branch from c81e0f6 to 26be491 Compare November 26, 2024 09:37
@nordicjm
Copy link
Collaborator

I manually got the branch and amended it to Signed-off-by: Andrej Butok <[email protected]> and it passes

@butok butok force-pushed the add_nxp_mcuboot_zephyr branch from 26be491 to 102100c Compare November 26, 2024 10:01
@butok
Copy link
Contributor Author

butok commented Nov 26, 2024

I manually got the branch and amended it to Signed-off-by: Andrej Butok <[email protected]> and it passes

OK, Amend again to the original Signed-off-by: Andrej Butok [email protected]
CI failed.
Again wants "Missing "Signed-off-by: Andrej Butok [email protected]"
Funny ;)

@nordicjm
Copy link
Collaborator

Hmm when I run it locally it passes:

102100c2 (HEAD -> add_nxp_mcuboot_zephyr, origin/add_nxp_mcuboot_zephyr) boot: zephyr: nxp: Add NXP platforms to the allow list
8a2e2edb (origin/main, origin/HEAD, main) ci: Upgrade artifact actions

./ci/check-signed-off-by.sh 8a2e2edba596de7471139ec9d9011d2dd8cbd86e

@nordicjm
Copy link
Collaborator

I think you have some conflict with your local git setup, it is doing: git show -s --format="%an <%ae>" 102100c2800a678ee63d479ea04b68e0aba852de and git show -s --format="%cn <%ce>" 102100c2800a678ee63d479ea04b68e0aba852de. The first gets the git author, which is Andrej Butok <[email protected]>, this is correct, the second gets the git committer, which is Andrej Butok <[email protected]>, would check your local git config for these settings then do git commit --amend --reset-author and check that both output the same

@butok butok force-pushed the add_nxp_mcuboot_zephyr branch from 102100c to d36970f Compare November 26, 2024 11:08
@butok
Copy link
Contributor Author

butok commented Nov 26, 2024

I think you have some conflict with your local git setup, it is doing: git show -s --format="%an <%ae>" 102100c2800a678ee63d479ea04b68e0aba852de and git show -s --format="%cn <%ce>" 102100c2800a678ee63d479ea04b68e0aba852de. The first gets the git author, which is Andrej Butok <[email protected]>, this is correct, the second gets the git committer, which is Andrej Butok <[email protected]>, would check your local git config for these settings then do git commit --amend --reset-author and check that both output the same

Thank you for pointing to the root cause.
The PR was continued on a different PC with wrong settings.

Adds NXP platforms, sorted alphabetically,
to the boot zephyr allow list.

Signed-off-by: Andrej Butok <[email protected]>
@butok butok force-pushed the add_nxp_mcuboot_zephyr branch from d36970f to c320f54 Compare December 6, 2024 12:58
@butok
Copy link
Contributor Author

butok commented Dec 6, 2024

Rebased.

@nordicjm nordicjm merged commit 7ba0e55 into mcu-tools:main Dec 11, 2024
58 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: zephyr Affects the Zephyr port
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants