Skip to content

Conversation

Vge0rge
Copy link
Contributor

@Vge0rge Vge0rge commented Aug 19, 2025

See commit messages

@Vge0rge Vge0rge requested review from a team as code owners August 19, 2025 13:01
@github-actions github-actions bot added manifest changelog-entry-required Update changelog before merge. Remove label if entry is not needed or already added. ble mesh Label for ble mesh PRbot. Add this if PR is related to ble mesh and you need to get review. labels Aug 19, 2025
@NordicBuilder
Copy link
Contributor

NordicBuilder commented Aug 19, 2025

The following west manifest projects have changed revision in this Pull Request:

Name Old Revision New Revision Diff
zephyr nrfconnect/sdk-zephyr@20d89eb nrfconnect/sdk-zephyr@dc15a79 (main) nrfconnect/[email protected]

All manifest checks OK

Note: This message is automatically posted and updated by the Manifest GitHub Action.

@NordicBuilder
Copy link
Contributor

NordicBuilder commented Aug 19, 2025

CI Information

To view the history of this post, clich the 'edited' button above
Build number: 14

Inputs:

Sources:

sdk-nrf: PR head: d5540857c408bd5c4c88063edec3d50e63b2ad5d
zephyr: PR head: dc15a79bfeb842bfb503fd63017e8cb371050ddf

more details

sdk-nrf:

PR head: d5540857c408bd5c4c88063edec3d50e63b2ad5d
merge base: 652d8a49a427e27bc9bc749f9b266552965b9ef0
target head (main): 652d8a49a427e27bc9bc749f9b266552965b9ef0
Diff

zephyr:

PR head: dc15a79bfeb842bfb503fd63017e8cb371050ddf
merge base: 20d89eb11a79ec9ca6ee80372af2a725028765ac
Diff

Github labels

Enabled Name Description
ci-disabled Disable the ci execution
ci-all-test Run all of ci, no test spec filtering will be done
ci-force-downstream Force execution of downstream even if twister fails
ci-run-twister Force run twister
ci-run-zephyr-twister Force run zephyr twister
List of changed files detected by CI (10)
applications
│  ├── machine_learning
│  │  │ sample.yaml
doc
│  ├── nrf
│  │  ├── releases_and_maturity
│  │  │  ├── releases
│  │  │  │  │ release-notes-changelog.rst
modules
│  ├── trusted-firmware-m
│  │  │ Kconfig.tfm.defconfig
samples
│  ├── bluetooth
│  │  ├── peripheral_lbs
│  │  │  │ sample.yaml
│  │  ├── peripheral_status
│  │  │  │ sample.yaml
│  │  ├── peripheral_uart
│  │  │  │ sample.yaml
subsys
│  ├── bluetooth
│  │  ├── mesh
│  │  │  │ Kconfig
│  ├── net
│  │  ├── openthread
│  │  │  │ Kconfig
west.yml
zephyr
│  ├── tests
│  │  ├── bluetooth
│  │  │  ├── bt_crypto
│  │  │  │  ├── boards
│  │  │  │  │  │ nrf5340dk_nrf5340_cpuapp_ns.conf

Outputs:

Toolchain

Version: 53a57c4d75
Build docker image: docker-dtr.nordicsemi.no/sw-production/ncs-build:53a57c4d75_bba2ea5f2e

Test Spec & Results: ✅ Success; ❌ Failure; 🟠 Queued; 🟡 Progress; ◻️ Skipped; ⚠️ Quarantine

  • ◻️ Toolchain - Skipped: existing toolchain is used
  • ✅ Build twister
    • sdk-nrf test count: 27
    • sdk-zephyr test count: 4
  • ✅ Integration tests
    • ✅ test-fw-nrfconnect-apps
    • ✅ test-fw-nrfconnect-ble_mesh
    • ✅ test-fw-nrfconnect-ble_samples
    • ✅ test-fw-nrfconnect-chip
    • ✅ test-fw-nrfconnect-nrf_crypto
    • ✅ test-fw-nrfconnect-tfm
    • ✅ test-fw-nrfconnect-thread-main
    • ✅ test-sdk-find-my
    • ✅ test-sdk-mcuboot
    • ✅ test-sdk-dfu
Disabled integration tests
    • test-fw-nrfconnect-nrf_lrcs_mosh
    • test-fw-nrfconnect-nrf_lrcs_positioning
    • desktop52_verification
    • doc-internal
    • test_ble_nrf_config
    • test-fw-nrfconnect-fem
    • test-fw-nrfconnect-nfc
    • test-fw-nrfconnect-nrf-iot_libmodem-nrf
    • test-fw-nrfconnect-nrf-iot_lwm2m
    • test-fw-nrfconnect-nrf-iot_samples
    • test-fw-nrfconnect-nrf-iot_serial_lte_modem
    • test-fw-nrfconnect-nrf-iot_thingy91
    • test-fw-nrfconnect-nrf-iot_zephyr_lwm2m
    • test-fw-nrfconnect-proprietary_esb
    • test-fw-nrfconnect-ps-main
    • test-fw-nrfconnect-rpc
    • test-fw-nrfconnect-rs
    • test-low-level
    • test-sdk-audio
    • test-sdk-pmic-samples
    • test-sdk-wifi
    • test-secdom-samples-public

Note: This message is automatically posted and updated by the CI

Copy link

github-actions bot commented Aug 19, 2025

You can find the documentation preview for this PR here.

Preview links for modified nRF Connect SDK documents:

https://ncsdoc.z6.web.core.windows.net/PR-24059/nrf/releases_and_maturity/releases/release-notes-changelog.html

@Vge0rge Vge0rge force-pushed the change_tfm_profile branch from 875616a to f925bef Compare August 19, 2025 14:13
Copy link
Contributor

@alxelax alxelax left a comment

Choose a reason for hiding this comment

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

From mesh point of view the changes are Ok.
However, since dependency on TEST_BT_FAST_PAIR_CRYPTO_PSA macro was removed, then changes from downstream shouldn't be reverted. Right?

@Vge0rge
Copy link
Contributor Author

Vge0rge commented Aug 20, 2025

From mesh point of view the changes are Ok. However, since dependency on TEST_BT_FAST_PAIR_CRYPTO_PSA macro was removed, then changes from downstream shouldn't be reverted. Right?

@alxelax I am not sure that I follow, I removed the dependency to the TEST_BT_FAST_PAIR_CRYPTO_PSA because of CI issues and reverted my original change to the Kconfig file which enabled the correct TF-M profile for this test. Am I missing something here?

@alxelax
Copy link
Contributor

alxelax commented Aug 20, 2025

From mesh point of view the changes are Ok. However, since dependency on TEST_BT_FAST_PAIR_CRYPTO_PSA macro was removed, then changes from downstream shouldn't be reverted. Right?

@alxelax I am not sure that I follow, I removed the dependency to the TEST_BT_FAST_PAIR_CRYPTO_PSA because of CI issues and reverted my original change to the Kconfig file which enabled the correct TF-M profile for this test. Am I missing something here?

Probably I missed something. @PavelVPV, could you confirm that after reverting your fix from downstream(nrfconnect/sdk-zephyr@319b359) and applying of these changes the test(ble crypto test that you fixed) still passes?

@PavelVPV
Copy link
Contributor

From mesh point of view the changes are Ok. However, since dependency on TEST_BT_FAST_PAIR_CRYPTO_PSA macro was removed, then changes from downstream shouldn't be reverted. Right?

@alxelax I am not sure that I follow, I removed the dependency to the TEST_BT_FAST_PAIR_CRYPTO_PSA because of CI issues and reverted my original change to the Kconfig file which enabled the correct TF-M profile for this test. Am I missing something here?

Probably I missed something. @PavelVPV, could you confirm that after reverting your fix from downstream(nrfconnect/sdk-zephyr@319b359) and applying of these changes the test(ble crypto test that you fixed) still passes?

This revert was discussed internally. It was passing locally on @Vge0rge machine, and we discussed how to run CI. But if needed I can double check locally.

@alxelax
Copy link
Contributor

alxelax commented Aug 20, 2025

From mesh point of view the changes are Ok. However, since dependency on TEST_BT_FAST_PAIR_CRYPTO_PSA macro was removed, then changes from downstream shouldn't be reverted. Right?

@alxelax I am not sure that I follow, I removed the dependency to the TEST_BT_FAST_PAIR_CRYPTO_PSA because of CI issues and reverted my original change to the Kconfig file which enabled the correct TF-M profile for this test. Am I missing something here?

Probably I missed something. @PavelVPV, could you confirm that after reverting your fix from downstream(nrfconnect/sdk-zephyr@319b359) and applying of these changes the test(ble crypto test that you fixed) still passes?

This revert was discussed internally. It was passing locally on @Vge0rge machine, and we discussed how to run CI. But if needed I can double check locally.

Not needed. If you run and passed it, then this is Ok. I'll approve PR after it passes CI steps.

@Vge0rge Vge0rge requested a review from a team as a code owner August 20, 2025 13:24
@Vge0rge
Copy link
Contributor Author

Vge0rge commented Aug 21, 2025

ping @nrfconnect/ncs-si-muffin @nrfconnect/ncs-code-owners @nrfconnect/ncs-co-build-system

@PavelVPV
Copy link
Contributor

@Vge0rge , do you plan to backport this to 3.1.1?

@Vge0rge
Copy link
Contributor Author

Vge0rge commented Aug 25, 2025

@Vge0rge , do you plan to backport this to 3.1.1?

I didn't have that in mind, no.

@grochu
Copy link
Contributor

grochu commented Aug 28, 2025

@Vge0rge, please add relevant notes in the NCS changelog in documentation, so the user have it clear we will no longer support them.

@Vge0rge Vge0rge requested a review from a team as a code owner August 28, 2025 13:00
@github-actions github-actions bot added doc-required PR must not be merged without tech writer approval. and removed changelog-entry-required Update changelog before merge. Remove label if entry is not needed or already added. labels Aug 28, 2025
@Vge0rge Vge0rge force-pushed the change_tfm_profile branch from c9a130d to 8caafdc Compare August 28, 2025 13:40
@NordicBuilder
Copy link
Contributor

NordicBuilder commented Aug 28, 2025

Memory footprint analysis revealed the following potential issues

sample.find_my.switchable_networks.release.ui_switch[nrf54lm20dk/nrf54lm20a/cpuapp]: ROM size increased by 4080[B] in comparison to the main[94521ce] branch. - link (cc: @nrfconnect/ncs-si-bluebagel)
sample.matter.template.debug[nrf5340dk/nrf5340/cpuapp]: ROM size increased by 2156[B] in comparison to the main[94521ce] branch. - link (cc: @kkasperczyk-no @ArekBalysNordic @markaj-nordic)
sample.matter.template.release[nrf5340dk/nrf5340/cpuapp]: ROM size increased by 1596[B] in comparison to the main[94521ce] branch. - link (cc: @kkasperczyk-no @ArekBalysNordic @markaj-nordic)
sample.matter.template.release[nrf52840dk/nrf52840]: ROM size increased by 1648[B] in comparison to the main[94521ce] branch. - link (cc: @kkasperczyk-no @ArekBalysNordic @markaj-nordic)
sample.find_my.locator_tag.release[nrf54lm20dk/nrf54lm20a/cpuapp]: ROM size increased by 2520[B] in comparison to the main[94521ce] branch. - link (cc: @nrfconnect/ncs-si-bluebagel)
sample.matter.template.debug[nrf52840dk/nrf52840]: ROM size increased by 2156[B] in comparison to the main[94521ce] branch. - link (cc: @kkasperczyk-no @ArekBalysNordic @markaj-nordic)
sample.bluetooth.fast_pair.locator_tag.release[nrf54lm20dk/nrf54lm20a/cpuapp]: ROM size increased by 4024[B] in comparison to the main[94521ce] branch. - link (cc: @nrfconnect/ncs-si-bluebagel)

Note: This message is automatically posted and updated by the CI (latest/sdk-nrf/PR-24059/14)

Copy link
Contributor

@greg-fer greg-fer left a comment

Choose a reason for hiding this comment

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

One preferential suggestion.

@Vge0rge Vge0rge force-pushed the change_tfm_profile branch from e225391 to 1fdb91e Compare August 29, 2025 07:07
@Vge0rge Vge0rge requested a review from peknis August 29, 2025 07:07
@Vge0rge Vge0rge force-pushed the change_tfm_profile branch from 1fdb91e to 7e8093c Compare August 29, 2025 07:10
@Vge0rge Vge0rge force-pushed the change_tfm_profile branch 2 times, most recently from a9f4728 to ad978e7 Compare August 29, 2025 07:40
@Vge0rge Vge0rge force-pushed the change_tfm_profile branch from ad978e7 to b6bd24d Compare August 29, 2025 12:03
@github-actions github-actions bot removed the DNM label Aug 29, 2025
Vge0rge and others added 7 commits August 29, 2025 14:07
Change the default profile for TF-M for all devices which
use the bluetooth crypto, bluetooth mesh crypto and openthread.

Instead of the minimal profile of TF-M which doesn't provide
crypto operations apart from rng use the PROFILE_TYPE_NOT_SET
which allows crypto operations based on the enabled PSA_WANTs.

This is preparation work, the profile will be PROFILE_TYPE_NOT_SET
for all devices and use cases in the future.

Signed-off-by: Georgios Vasilakis <[email protected]>
Automatically created by Github Action

Signed-off-by: Nordic Builder <[email protected]>
Remove the choice of the TF-M profile in bluetoooth
subsystem since this is now handled by the TF-M
Kconfig configurations.

Signed-off-by: Georgios Vasilakis <[email protected]>
Remove the choice of the TF-M profile in openthread
subsystem since this is now handled by the TF-M
Kconfig configurations.

Signed-off-by: Georgios Vasilakis <[email protected]>
Remove the thingy53 non secure target from several bluetooth
samples which enable BT_CRYPTO.

Recently the configuration of TF-M was updated to take into
account the BT_CRYPTO symbol. Meaning that when the BT_CRYPTO
is enabled TF-M configuration makes sure to provide support
for the BT_CRYPTO requirements. This was not happening before
resulting to invalid configurations where the samples failed
at runtime.

The thingy 53 non secure target has a flash overflow when trying
to build these samples with a valid TF-M configuration and thus
I removed them from the yaml files.

Signed-off-by: Georgios Vasilakis <[email protected]>
Remove the thingy53 non secure target from the machine learning
application since it enables BT_CRYPTO.

Recently the configuration of TF-M was updated to take into
account the BT_CRYPTO symbol. Meaning that when the BT_CRYPTO
is enabled TF-M configuration makes sure to provide support
for the BT_CRYPTO requirements. This was not happening before
resulting to invalid configurations where the samples failed
at runtime.

The thingy 53 non secure target has a flash overflow when trying
to build this application with a valid TF-M configuration and thus
I removed it from the yaml file.

Signed-off-by: Georgios Vasilakis <[email protected]>
The thingy53 non-secure targets are no longer supported
in the machine learning application and in the bluetooth
peripheral status/uart/lbs samples.

Signed-off-by: Georgios Vasilakis <[email protected]>
@Vge0rge Vge0rge force-pushed the change_tfm_profile branch from b6bd24d to d554085 Compare August 29, 2025 12:08
@anangl anangl closed this Aug 29, 2025
@anangl anangl reopened this Aug 29, 2025
@anangl anangl merged commit 5bbd6fa into nrfconnect:main Aug 29, 2025
28 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ble mesh Label for ble mesh PRbot. Add this if PR is related to ble mesh and you need to get review. doc-required PR must not be merged without tech writer approval. manifest manifest-zephyr
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants