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

manifest: sdk-zephyr: ZMS a new memory storage system #17502

Merged
merged 8 commits into from
Oct 16, 2024

Conversation

rghaddab
Copy link
Contributor

includes changes from this :

nrfconnect/sdk-zephyr#2045

@rghaddab rghaddab requested a review from a team as a code owner September 26, 2024 10:27
@CLAassistant
Copy link

CLAassistant commented Sep 26, 2024

CLA assistant check
All committers have signed the CLA.

@github-actions github-actions bot added manifest changelog-entry-required Update changelog before merge. Remove label if entry is not needed or already added. labels Sep 26, 2024
@NordicBuilder
Copy link
Contributor

NordicBuilder commented Sep 26, 2024

The following west manifest projects have been modified in this Pull Request:

Name Old Revision New Revision Diff
matter nrfconnect/sdk-connectedhomeip@bdec5fd nrfconnect/sdk-connectedhomeip@9a4ea8e (master) nrfconnect/[email protected]
zephyr nrfconnect/sdk-zephyr@2a679b9 (sdsc-bundle-2.8.0-rc1) nrfconnect/sdk-zephyr@98662fc (main) nrfconnect/[email protected]

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

@rghaddab rghaddab force-pushed the rghaddab/pr-zms branch 2 times, most recently from c1a0cb1 to 6db1ded Compare September 26, 2024 10:44
@NordicBuilder
Copy link
Contributor

You can find the documentation preview for this PR at this link. It will be updated about 10 minutes after the documentation build succeeds.

Note: This comment is automatically posted by the Documentation Publishing GitHub Action.

@NordicBuilder
Copy link
Contributor

NordicBuilder commented Sep 26, 2024

CI Information

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

Inputs:

Sources:

sdk-nrf: PR head: 7eaa1f439ee5e1ffa5c35199dc8f41dbd25310b0
matter: PR head: 2bd04ee997f3a3eebe85ac247b80d6df6884a6d1
zephyr: PR head: df1b6886fc4477b06c27839ce9c70935a149a2da

more details

sdk-nrf:

PR head: 7eaa1f439ee5e1ffa5c35199dc8f41dbd25310b0
merge base: c4f11427d275fad0ffc8b6d7b5c4efeb28f591e0
target head (main): c4f11427d275fad0ffc8b6d7b5c4efeb28f591e0
Diff

matter:

PR head: 2bd04ee997f3a3eebe85ac247b80d6df6884a6d1
merge base: bdec5fd36f56f445c673ac14f9f09377b66ba49b
target head (master): 3567e538367bfa58e83b9d7c05aeace93f505194
Diff

zephyr:

PR head: df1b6886fc4477b06c27839ce9c70935a149a2da
merge base: 2a679b95faef5dd1e5a46d6710d7059b6042758c
target head (main): 2a679b95faef5dd1e5a46d6710d7059b6042758c
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 (114)
doc
│  ├── nrf
│  │  ├── protocols
│  │  │  ├── matter
│  │  │  │  ├── getting_started
│  │  │  │  │  │ advanced_kconfigs.rst
│  │  ├── releases_and_maturity
│  │  │  ├── releases
│  │  │  │  │ release-notes-changelog.rst
include
│  │ flash_map_pm.h
modules
│  ├── lib
│  │  ├── matter
│  │  │  ├── config
│  │  │  │  ├── nrfconnect
│  │  │  │  │  ├── chip-module
│  │  │  │  │  │  ├── CMakeLists.txt
│  │  │  │  │  │  ├── Kconfig
│  │  │  │  │  │  │ Kconfig.defaults
│  │  │  │  ├── nxp
│  │  │  │  │  ├── chip-module
│  │  │  │  │  │  │ Kconfig
│  │  │  │  ├── telink
│  │  │  │  │  ├── chip-module
│  │  │  │  │  │  │ Kconfig
│  │  │  │  ├── zephyr
│  │  │  │  │  │ Kconfig
│  │  │  ├── docs
│  │  │  │  ├── guides
│  │  │  │  │  │ nrfconnect_examples_configuration.md
│  │  │  ├── src
│  │  │  │  ├── platform
│  │  │  │  │  ├── Zephyr
│  │  │  │  │  │  │ ConfigurationManagerImpl.cpp
│  │  │  │  │  ├── nrfconnect
│  │  │  │  │  │  │ FactoryDataProvider.cpp
samples
│  ├── bluetooth
│  │  ├── central_and_peripheral_hr
│  │  │  ├── Kconfig
│  │  │  │ prj.conf
│  │  ├── central_bas
│  │  │  ├── Kconfig
│  │  │  │ prj.conf
│  │  ├── central_hids
│  │  │  ├── Kconfig
│  │  │  │ prj.conf
│  │  ├── central_nfc_pairing
│  │  │  ├── Kconfig
│  │  │  │ prj.conf
│  │  ├── central_uart
│  │  │  ├── Kconfig
│  │  │  │ prj.conf
│  │  ├── enocean
│  │  │  ├── Kconfig
│  │  │  │ prj.conf
│  │  ├── fast_pair
│  │  │  ├── input_device
│  │  │  │  ├── Kconfig
│  │  │  │  │ prj.conf
│  │  │  ├── locator_tag
│  │  │  │  ├── Kconfig
│  │  │  │  ├── configuration
│  │  │  │  │  ├── prj.conf
│  │  │  │  │  │ prj_release.conf
│  │  ├── peripheral_ams_client
│  │  │  ├── Kconfig
│  │  │  │ prj.conf
│  │  ├── peripheral_ancs_client
│  │  │  ├── Kconfig
│  │  │  │ prj.conf
│  │  ├── peripheral_bms
│  │  │  ├── Kconfig
│  │  │  │ prj.conf
│  │  ├── peripheral_cgms
│  │  │  ├── Kconfig
│  │  │  │ prj.conf
│  │  ├── peripheral_cts_client
│  │  │  ├── Kconfig
│  │  │  │ prj.conf
│  │  ├── peripheral_hids_keyboard
│  │  │  ├── Kconfig
│  │  │  │ prj.conf
│  │  ├── peripheral_hids_mouse
│  │  │  │ Kconfig
│  │  ├── peripheral_lbs
│  │  │  │ Kconfig
│  │  ├── peripheral_mds
│  │  │  ├── Kconfig
│  │  │  │ prj.conf
│  │  ├── peripheral_nfc_pairing
│  │  │  ├── Kconfig
│  │  │  │ prj.conf
│  │  ├── peripheral_power_profiling
│  │  │  ├── Kconfig
│  │  │  │ prj.conf
│  │  ├── peripheral_rscs
│  │  │  │ Kconfig
│  │  ├── peripheral_status
│  │  │  │ Kconfig
│  │  ├── peripheral_uart
│  │  │  ├── Kconfig
│  │  │  ├── prj.conf
│  │  │  │ prj_minimal.conf
│  │  ├── rpc_host
│  │  │  ├── Kconfig
│  │  │  │ prj.conf
│  ├── matter
│  │  ├── common
│  │  │  ├── src
│  │  │  │  ├── Kconfig
│  │  │  │  ├── persistent_storage
│  │  │  │  │  │ persistent_storage_shell.cpp
│  │  ├── light_bulb
│  │  │  ├── boards
│  │  │  │  ├── nrf54l15dk_nrf54l15_cpuapp.conf
│  │  │  │  │ nrf54l15pdk_nrf54l15_cpuapp_ns.conf
│  │  ├── light_switch
│  │  │  ├── boards
│  │  │  │  ├── nrf54l15dk_nrf54l15_cpuapp.conf
│  │  │  │  │ nrf54l15pdk_nrf54l15_cpuapp_ns.conf
│  │  ├── lock
│  │  │  ├── Kconfig
│  │  │  ├── boards
│  │  │  │  ├── nrf54l15dk_nrf54l15_cpuapp.conf
│  │  │  │  │ nrf54l15pdk_nrf54l15_cpuapp_ns.conf
│  │  │  ├── prj_thread_wifi_switched.conf
│  │  │  ├── src
│  │  │  │  ├── access
│  │  │  │  │  │ access_storage.cpp
│  │  ├── smoke_co_alarm
│  │  │  ├── boards
│  │  │  │  │ nrf54l15dk_nrf54l15_cpuapp.conf
│  │  ├── template
│  │  │  ├── boards
│  │  │  │  ├── nrf54l15dk_nrf54l15_cpuapp.conf
│  │  │  │  ├── nrf54l15dk_nrf54l15_cpuapp_internal.conf
│  │  │  │  │ nrf54l15pdk_nrf54l15_cpuapp_ns.conf
│  │  ├── thermostat
│  │  │  ├── boards
│  │  │  │  ├── nrf54l15dk_nrf54l15_cpuapp.conf
│  │  │  │  │ nrf54l15pdk_nrf54l15_cpuapp_ns.conf
│  │  ├── window_covering
│  │  │  ├── boards
│  │  │  │  ├── nrf54l15dk_nrf54l15_cpuapp.conf
│  │  │  │  │ nrf54l15pdk_nrf54l15_cpuapp_ns.conf
│  ├── openthread
│  │  ├── cli
│  │  │  ├── boards
│  │  │  │  │ nrf54l15dk_nrf54l15_cpuapp.conf
│  │  ├── coprocessor
│  │  │  ├── boards
│  │  │  │  │ nrf54l15dk_nrf54l15_cpuapp.conf
subsys
│  ├── net
│  │  ├── CMakeLists.txt
│  │  ├── openthread
│  │  │  ├── settings_check
│  │  │  │  │ CMakeLists.txt
│  ├── partition_manager
│  │  ├── CMakeLists.txt
│  │  ├── Kconfig
│  │  │ pm.yml.zms
│  ├── trusted_storage
│  │  │ Kconfig
west.yml
zephyr
│  ├── doc
│  │  ├── services
│  │  │  ├── storage
│  │  │  │  ├── index.rst
│  │  │  │  ├── zms
│  │  │  │  │  │ zms.rst
│  ├── include
│  │  ├── zephyr
│  │  │  ├── fs
│  │  │  │  │ zms.h
│  ├── samples
│  │  ├── subsys
│  │  │  ├── fs
│  │  │  │  ├── zms
│  │  │  │  │  ├── CMakeLists.txt
│  │  │  │  │  ├── README.rst
│  │  │  │  │  ├── prj.conf
│  │  │  │  │  ├── sample.yaml
│  │  │  │  │  ├── src
│  │  │  │  │  │  │ main.c
│  ├── subsys
│  │  ├── fs
│  │  │  ├── CMakeLists.txt
│  │  │  ├── Kconfig
│  │  │  ├── zms
│  │  │  │  ├── CMakeLists.txt
│  │  │  │  ├── Kconfig
│  │  │  │  ├── zms.c
│  │  │  │  │ zms_priv.h
│  │  ├── settings
│  │  │  ├── Kconfig
│  │  │  ├── include
│  │  │  │  ├── settings
│  │  │  │  │  │ settings_zms.h
│  │  │  ├── src
│  │  │  │  ├── CMakeLists.txt
│  │  │  │  │ settings_zms.c
│  ├── tests
│  │  ├── subsys
│  │  │  ├── fs
│  │  │  │  ├── zms
│  │  │  │  │  ├── CMakeLists.txt
│  │  │  │  │  ├── boards
│  │  │  │  │  │  ├── native_sim.overlay
│  │  │  │  │  │  │ qemu_x86_ev_0x00.overlay
│  │  │  │  │  ├── prj.conf
│  │  │  │  │  ├── src
│  │  │  │  │  │  │ main.c
│  │  │  │  │  │ testcase.yaml
│  │  │  ├── settings
│  │  │  │  ├── zms
│  │  │  │  │  ├── CMakeLists.txt
│  │  │  │  │  ├── prj.conf
│  │  │  │  │  ├── src
│  │  │  │  │  │  ├── CMakeLists.txt
│  │  │  │  │  │  ├── settings_test.h
│  │  │  │  │  │  │ settings_test_zms.c
│  │  │  │  │  │ testcase.yaml

Outputs:

Toolchain

Version: 3dd8985b56
Build docker image: docker-dtr.nordicsemi.no/sw-production/ncs-build:3dd8985b56_81ed5a52d6

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

  • ◻️ Toolchain - Skipped: existing toolchain is used
  • ❌ Build twister
  • ❌ Integration tests
    • ✅ test-sdk-audio
    • ❌ desktop52_verification
    • ❌ test-fw-nrfconnect-boot
    • ❌ test-fw-nrfconnect-apps
    • ❌ test_ble_nrf_config
    • ✅ test-fw-nrfconnect-ble_mesh
    • ✅ test-fw-nrfconnect-ble_samples
    • ❌ test-fw-nrfconnect-chip
    • ❌ test-fw-nrfconnect-nfc - Error: Error starting job: null
    • ✅ test-fw-nrfconnect-nrf-iot_cloud
    • ❌ test-fw-nrfconnect-nrf-iot_libmodem-nrf
    • ✅ test-fw-nrfconnect-nrf-iot_serial_lte_modem
    • ✅ test-fw-nrfconnect-nrf-iot_zephyr_lwm2m
    • ✅ test-fw-nrfconnect-nrf-iot_samples
    • ✅ test-fw-nrfconnect-nrf-iot_lwm2m
    • ✅ doc-internal
    • ❌ test-fw-nrfconnect-nrf-iot_thingy91
    • ✅ test-fw-nrfconnect-nrf_crypto
    • ❌ test-fw-nrfconnect-proprietary_esb
    • ✅ test-fw-nrfconnect-rpc
    • ❌ test-fw-nrfconnect-rs
    • ✅ test-fw-nrfconnect-fem
    • ✅ test-fw-nrfconnect-tfm
    • ❌ test-fw-nrfconnect-thread
    • ❌ test-fw-nrfconnect-zigbee
    • ❌ test-sdk-find-my
    • ❌ test-fw-nrfconnect-nrf-iot_mosh
    • ❌ test-fw-nrfconnect-nrf-iot_positioning
    • ✅ test-sdk-sidewalk
    • ❌ test-sdk-wifi
    • ✅ test-low-level
    • ❌ test-fw-nrfconnect-nrf-iot_nrf_provisioning
    • ✅ test-sdk-pmic-samples
    • ✅ test-sdk-mcuboot
    • ✅ test-sdk-dfu
    • ❌ test-fw-nrfconnect-ps
    • ✅ test-secdom-samples-public
    • ⚠️ test-fw-nrfconnect-fw-update

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

@rghaddab rghaddab force-pushed the rghaddab/pr-zms branch 6 times, most recently from 8cb2764 to 2cbf0a8 Compare October 3, 2024 20:19
@rghaddab rghaddab requested review from a team, nordicjm and tejlmand as code owners October 3, 2024 20:19
@rghaddab rghaddab force-pushed the rghaddab/pr-zms branch 4 times, most recently from 217b8ca to bc25fcc Compare October 9, 2024 08:59
@pdunaj
Copy link
Contributor

pdunaj commented Oct 9, 2024

Hi @rghaddab , I have added commit selecting ZMS in few ble samples which work with KConfig. Other will require few more changes these will follow.
Hi @ns-tolu , you can most probably use these for your verification.

@pdunaj pdunaj requested review from a team as code owners October 10, 2024 11:00
@pdunaj
Copy link
Contributor

pdunaj commented Oct 10, 2024

Conversions of some remaining ble samples. fyi @miha-nordic

I have not covered mesh nor anything beyond samples/bluetooth. I have not checked if it works on my desktop.
I have check it builds on most samples. I know that one (I recall it was central_nfc_pairing fyi @dchat-nordic ) is not building properly.

@rghaddab rghaddab requested review from a team as code owners October 16, 2024 10:12
@rlubos
Copy link
Contributor

rlubos commented Oct 16, 2024

@nrfconnect/ncs-co-build-system @nrfconnect/ncs-si-muffin @nrfconnect/ncs-thread Ping, please prioritize this.

@rlubos
Copy link
Contributor

rlubos commented Oct 16, 2024

@rghaddab Subrepo PRs are merged, please update SHAs.

@rghaddab rghaddab removed the DNM label Oct 16, 2024
# In nRF54L15 we place the TF-M non-secure storage partitions after the
# TF-M non-secure application to avoid splitting the secure/non-secure
# partitions more than necessary.
#if defined(CONFIG_SOC_NRF54L15_ENGA_CPUAPP) || defined(CONFIG_SOC_NRF54L15_CPUAPP)
Copy link
Contributor

Choose a reason for hiding this comment

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

CONFIG_SOC_NRF54L15_ENGA_CPUAPP can go

#endif
size: CONFIG_PM_PARTITION_SIZE_ZMS_STORAGE


Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change

Comment on lines +19 to +20
select ZMS if SOC_FLASH_NRF_RRAM
select NVS if !SOC_FLASH_NRF_RRAM
Copy link
Contributor

Choose a reason for hiding this comment

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

Should be imply so it can be changed to allow for relocating to/from external storage

Comment on lines +9 to +10
config SETTINGS
default y
Copy link
Contributor

Choose a reason for hiding this comment

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

this isn't needed

@@ -31,8 +31,6 @@ CONFIG_BT_SETTINGS=y
CONFIG_FLASH=y
CONFIG_FLASH_PAGE_LAYOUT=y
CONFIG_FLASH_MAP=y
CONFIG_NVS=y
CONFIG_SETTINGS=y
Copy link
Contributor

Choose a reason for hiding this comment

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

add CONFIG_SETTINGS=y back here

Comment on lines +21 to +25
#ifdef CONFIG_NVS
nvs_fs *sStorage = nullptr;
#elif CONFIG_ZMS
zms_fs *sStorage = nullptr;
#endif
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
#ifdef CONFIG_NVS
nvs_fs *sStorage = nullptr;
#elif CONFIG_ZMS
zms_fs *sStorage = nullptr;
#endif
#if defined(CONFIG_NVS)
...
#elif defined(CONFIG_ZMS)
...
#endif

and apply throughout

Copy link
Contributor

Choose a reason for hiding this comment

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

Applied as a follow-up here: #17939 please have a look.

if (CONFIG_SOC_SERIES_NRF54LX AND CONFIG_SETTINGS_NVS)
message(WARNING " ###################################################################################\n"
" # #\n"
" # Your application uses NVS backend for the settings storage that is not #\n"
Copy link
Contributor

Choose a reason for hiding this comment

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

get rid of the excess spacing on the right here. Also seems a bit odd that this is being added for openthread?

Copy link
Contributor

Choose a reason for hiding this comment

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

Why is it odd? This is because the Zephyr OpenThread L2 layer implies NVS and we cannot fix it reasonably fast now. The ZMS was enabled in all OpenThread samples, but still someone may have an own application copy that will not be automatically aligned, so the check will be done for all applications running OpenThread.

Copy link
Contributor

Choose a reason for hiding this comment

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

It should be a global thing for any code so users can know if they've got something set up that will kill their device in half the time as one would expect

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, I agree, but I was asked only to handle integration with OpenThread samples, so I've taken care only about this use case. Maybe it could be moved and used in some generic place if others agree on that.

# This script checks if NVS settings backend is used with nRF54L SoC target and prints
# the warning that the recommended backend is ZMS due to the RRAM design.

if (CONFIG_SOC_SERIES_NRF54LX AND CONFIG_SETTINGS_NVS)
Copy link
Contributor

Choose a reason for hiding this comment

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

does not take into consideration if NVS is being placed on external flash

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, because it does consider only OpenThread module use case, which uses only settings. This is not generic warning for all application use cases.

Copy link
Contributor

Choose a reason for hiding this comment

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

it does not work though because if I use NVS on external flash with the Kconfig enabled for that, I wrongly get this giant warning when NVS isn't on the internal RRAM, it's on an external flash

Copy link
Contributor

Choose a reason for hiding this comment

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

Ok, so do you mean settings placed on the external flash or NVS used for some custom use case on the external flash?

NordicBuilder and others added 8 commits October 16, 2024 14:25
Automatically created by Github Action

Signed-off-by: Nordic Builder <[email protected]>
This adds the support of ZMS for the partition manager

Signed-off-by: Riadh Ghaddab <[email protected]>
makes the partition manager take into account ZMS storage partition when
its CONFIG is enabled.

Signed-off-by: Riadh Ghaddab <[email protected]>
if the settings backend is enabled for the trusted storage, it can also
select SETTINGS_ZMS as a backend for settings

Signed-off-by: Riadh Ghaddab <[email protected]>
When RRAM device is being used select ZMS rather then NVS
as a file system for settings.

Signed-off-by: Pawel Dunaj <[email protected]>
Select settings and fs in kconfig instead of prj.conf.
Select file system depending on NV memory type.

Signed-off-by: Pawel Dunaj <[email protected]>
Enabled ZMS fs backend for all Matter samples that work on
devices which use RRAM, like nRF54l15.

Signed-off-by: Arkadiusz Balys <[email protected]>
The settings backend for the nRF54L SoC family should be
selected to the ZMS, so it was applied for all OpenThread samples.

Additionally, added a cmake check that prints warning in case
someone uses NVS with nRF54L SoC and informs about
the consequences.

Signed-off-by: Kamil Kasperczyk <[email protected]>
@ns-tolu
Copy link

ns-tolu commented Oct 16, 2024

cannot wait with this PR anymore. It was decided to merge it as it is.

@rghaddab
Copy link
Contributor Author

@rghaddab Subrepo PRs are merged, please update SHAs.

@rlubos I updated the SHA

@rlubos
Copy link
Contributor

rlubos commented Oct 16, 2024

@rghaddab Thanks, please address any remaining review comments in a separate PR.

@rlubos rlubos merged commit 3ad9997 into nrfconnect:main Oct 16, 2024
11 of 13 checks passed
@carlescufi
Copy link
Contributor

cannot wait with this PR anymore. It was decided to merge it as it is.

@rghaddab please do follow-up with @nordicjm's comments in a follow-up PR

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.