samples/subsys/nvs support for atsame54_xpro

[asob]

Describe the bug samples/subsys/nvs does not compile for atsame54_xpro even though that flash drivers exists for sam0. See further down for potential fix.

To Reproduce

west build -b atsame54_xpro samples/subsys/nvs

Expected behavior Output as described in samples/subsys/nvs/README.rst

Impact Showstopper, can't use nvs subsystem

Logs and console output

-- west build: generating a build system
Including boilerplate (Zephyr base): /home/user/zephyrproject/zephyr/cmake/app/boilerplate.cmake
-- Application: /home/user/zephyrproject/zephyr/samples/subsys/nvs
-- Zephyr version: 2.4.99 (/home/user/zephyrproject/zephyr)
-- Found Python3: /usr/bin/python3.8 (found suitable exact version "3.8.5") found components: Interpreter
-- Found west (found suitable version "0.8.0", minimum required is "0.7.1")
-- Board: atsame54_xpro
-- Cache files will be written to: /home/user/.cache/zephyr
ZEPHYR_TOOLCHAIN_VARIANT not set, trying to locate Zephyr SDK
-- Found toolchain: zephyr (/home/user/zephyr-sdk-0.11.4)
-- Found dtc: /home/user/zephyr-sdk-0.11.4/sysroots/x86_64-pokysdk-linux/usr/bin/dtc (found suitable version "1.5.0", minimum required is "1.4.6")
-- Found BOARD.dts: /home/user/zephyrproject/zephyr/boards/arm/atsame54_xpro/atsame54_xpro.dts
atsame54_xpro.dts.pre.tmp:136.29-140.5: Warning (unique_unit_address_if_enabled): /soc/pinmux@41008000: duplicate unit-address (also used in node /soc/gpio@41008000)
atsame54_xpro.dts.pre.tmp:141.29-145.5: Warning (unique_unit_address_if_enabled): /soc/pinmux@41008080: duplicate unit-address (also used in node /soc/gpio@41008080)
atsame54_xpro.dts.pre.tmp:146.29-150.5: Warning (unique_unit_address_if_enabled): /soc/pinmux@41008100: duplicate unit-address (also used in node /soc/gpio@41008100)
atsame54_xpro.dts.pre.tmp:151.29-155.5: Warning (unique_unit_address_if_enabled): /soc/pinmux@41008180: duplicate unit-address (also used in node /soc/gpio@41008180)
-- Generated zephyr.dts: /home/user/zephyrproject/zephyr/build/zephyr/zephyr.dts
-- Generated devicetree_unfixed.h: /home/user/zephyrproject/zephyr/build/zephyr/include/generated/devicetree_unfixed.h
Parsing /home/user/zephyrproject/zephyr/Kconfig
Loaded configuration '/home/user/zephyrproject/zephyr/boards/arm/atsame54_xpro/atsame54_xpro_defconfig'
Merged configuration '/home/user/zephyrproject/zephyr/samples/subsys/nvs/prj.conf'
Configuration saved to '/home/user/zephyrproject/zephyr/build/zephyr/.config'
Kconfig header saved to '/home/user/zephyrproject/zephyr/build/zephyr/include/generated/autoconf.h'
-- The C compiler identification is GNU 9.2.0
-- The CXX compiler identification is GNU 9.2.0
-- The ASM compiler identification is GNU
-- Found assembler: /home/user/zephyr-sdk-0.11.4/arm-zephyr-eabi/bin/arm-zephyr-eabi-gcc
-- Configuring done
-- Generating done
-- Build files have been written to: /home/user/zephyrproject/zephyr/build
-- west build: building application
[1/137] Preparing syscall dependency handling

[89/137] Building C object CMakeFiles/app.dir/src/main.c.obj
FAILED: CMakeFiles/app.dir/src/main.c.obj
ccache /home/user/zephyr-sdk-0.11.4/arm-zephyr-eabi/bin/arm-zephyr-eabi-gcc -DBUILD_VERSION=zephyr-v2.4.0-2327-g4f355355395f -DKERNEL -D_FORTIFY_SOURCE=2 -D__PROGRAM_START -D__ZEPHYR__=1 -I/home/user/zephyrproject/zephyr/subsys/fs/nvs -I/home/user/zephyrproject/zephyr/include -Izephyr/include/generated -I/home/user/zephyrproject/zephyr/soc/arm/atmel_sam0/same54 -I/home/user/zephyrproject/zephyr/drivers -I/home/user/zephyrproject/zephyr/soc/arm/atmel_sam0/common/. -I/home/user/zephyrproject/modules/hal/cmsis/CMSIS/Core/Include -I/home/user/zephyrproject/modules/hal/atmel/asf/sam0/include/same54 -isystem /home/user/zephyrproject/zephyr/lib/libc/minimal/include -isystem /home/user/zephyr-sdk-0.11.4/arm-zephyr-eabi/bin/../lib/gcc/arm-zephyr-eabi/9.2.0/include -isystem /home/user/zephyr-sdk-0.11.4/arm-zephyr-eabi/bin/../lib/gcc/arm-zephyr-eabi/9.2.0/include-fixed -Os -imacros /home/user/zephyrproject/zephyr/build/zephyr/include/generated/autoconf.h -ffreestanding -fno-common -g -mcpu=cortex-m4 -mthumb -mabi=aapcs -imacros /home/user/zephyrproject/zephyr/include/toolchain/zephyr_stdint.h -Wall -Wformat -Wformat-security -Wno-format-zero-length -Wno-main -Wno-pointer-sign -Wpointer-arith -Wno-address-of-packed-member -Wno-unused-but-set-variable -Werror=implicit-int -fno-asynchronous-unwind-tables -fno-pie -fno-pic -fno-strict-overflow -fno-reorder-functions -fno-defer-pop -fmacro-prefix-map=/home/user/zephyrproject/zephyr/samples/subsys/nvs=CMAKE_SOURCE_DIR -fmacro-prefix-map=/home/user/zephyrproject/zephyr=ZEPHYR_BASE -fmacro-prefix-map=/home/user/zephyrproject=WEST_TOPDIR -ffunction-sections -fdata-sections -std=c99 -nostdinc -MD -MT CMakeFiles/app.dir/src/main.c.obj -MF CMakeFiles/app.dir/src/main.c.obj.d -o CMakeFiles/app.dir/src/main.c.obj   -c /home/user/zephyrproject/zephyr/samples/subsys/nvs/src/main.c
In file included from /home/user/zephyrproject/zephyr/include/arch/arm/aarch32/arch.h:20,
                 from /home/user/zephyrproject/zephyr/include/arch/cpu.h:19,
                 from /home/user/zephyrproject/zephyr/include/kernel_includes.h:33,
                 from /home/user/zephyrproject/zephyr/include/kernel.h:17,
                 from /home/user/zephyrproject/zephyr/include/zephyr.h:18,
                 from /home/user/zephyrproject/zephyr/samples/subsys/nvs/src/main.c:41:
/home/user/zephyrproject/zephyr/samples/subsys/nvs/src/main.c: In function 'main':
/home/user/zephyrproject/zephyr/include/devicetree/fixed-partitions.h:53:9: error: 'DT_COMPAT_fixed_partitions_LABEL_storage_REG_IDX_0_VAL_ADDRESS' undeclared (first use in this function)
   53 |  DT_CAT(DT_COMPAT_fixed_partitions_LABEL_, label)
      |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/home/user/zephyrproject/zephyr/include/devicetree.h:1962:24: note: in definition of macro 'DT_CAT'
 1962 | #define DT_CAT(a1, a2) a1 ## a2
      |                        ^~
/home/user/zephyrproject/zephyr/include/devicetree.h:959:30: note: in expansion of macro 'DT_REG_ADDR_BY_IDX'
  959 | #define DT_REG_ADDR(node_id) DT_REG_ADDR_BY_IDX(node_id, 0)
      |                              ^~~~~~~~~~~~~~~~~~
/home/user/zephyrproject/zephyr/include/storage/flash_map.h:266:2: note: in expansion of macro 'DT_REG_ADDR'
  266 |  DT_REG_ADDR(DT_NODE_BY_FIXED_PARTITION_LABEL(label))
      |  ^~~~~~~~~~~
/home/user/zephyrproject/zephyr/include/devicetree/fixed-partitions.h:53:2: note: in expansion of macro 'DT_CAT'
   53 |  DT_CAT(DT_COMPAT_fixed_partitions_LABEL_, label)
      |  ^~~~~~
/home/user/zephyrproject/zephyr/include/storage/flash_map.h:266:14: note: in expansion of macro 'DT_NODE_BY_FIXED_PARTITION_LABEL'
  266 |  DT_REG_ADDR(DT_NODE_BY_FIXED_PARTITION_LABEL(label))
      |              ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/home/user/zephyrproject/zephyr/samples/subsys/nvs/src/main.c:77:14: note: in expansion of macro 'FLASH_AREA_OFFSET'
   77 |  fs.offset = FLASH_AREA_OFFSET(storage);
      |              ^~~~~~~~~~~~~~~~~
/home/user/zephyrproject/zephyr/include/devicetree/fixed-partitions.h:53:9: note: each undeclared identifier is reported only once for each function it appears in
   53 |  DT_CAT(DT_COMPAT_fixed_partitions_LABEL_, label)
      |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/home/user/zephyrproject/zephyr/include/devicetree.h:1962:24: note: in definition of macro 'DT_CAT'
 1962 | #define DT_CAT(a1, a2) a1 ## a2
      |                        ^~
/home/user/zephyrproject/zephyr/include/devicetree.h:959:30: note: in expansion of macro 'DT_REG_ADDR_BY_IDX'
  959 | #define DT_REG_ADDR(node_id) DT_REG_ADDR_BY_IDX(node_id, 0)
      |                              ^~~~~~~~~~~~~~~~~~
/home/user/zephyrproject/zephyr/include/storage/flash_map.h:266:2: note: in expansion of macro 'DT_REG_ADDR'
  266 |  DT_REG_ADDR(DT_NODE_BY_FIXED_PARTITION_LABEL(label))
      |  ^~~~~~~~~~~
/home/user/zephyrproject/zephyr/include/devicetree/fixed-partitions.h:53:2: note: in expansion of macro 'DT_CAT'
   53 |  DT_CAT(DT_COMPAT_fixed_partitions_LABEL_, label)
      |  ^~~~~~
/home/user/zephyrproject/zephyr/include/storage/flash_map.h:266:14: note: in expansion of macro 'DT_NODE_BY_FIXED_PARTITION_LABEL'
  266 |  DT_REG_ADDR(DT_NODE_BY_FIXED_PARTITION_LABEL(label))
      |              ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/home/user/zephyrproject/zephyr/samples/subsys/nvs/src/main.c:77:14: note: in expansion of macro 'FLASH_AREA_OFFSET'
   77 |  fs.offset = FLASH_AREA_OFFSET(storage);
      |              ^~~~~~~~~~~~~~~~~
[98/137] Building C object zephyr/drivers/flash/CMakeFiles/drivers__flash.dir/flash_sam0.c.obj
ninja: build stopped: subcommand failed.
FATAL ERROR: command exited with status 1: /usr/bin/cmake --build /home/user/zephyrproject/zephyr/build

Environment (please complete the following information):

• OS: Linux DESKTOP-FJA506P 5.4.72-microsoft-standard-WSL2 #1 SMP Wed Oct 28 23:40:43 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux
• Toolchain : zephyr-sdk-0.11.4
• Commit SHA : 4f355355395f894ad52880266952808eed33ecf4

Additional context I've made the following changes

diff --git a/boards/arm/atsame54_xpro/atsame54_xpro.dts b/boards/arm/atsame54_xpro/atsame54_xpro.dts
index 73a922f407..a62555ae56 100644
--- a/boards/arm/atsame54_xpro/atsame54_xpro.dts
+++ b/boards/arm/atsame54_xpro/atsame54_xpro.dts
@@ -103,3 +103,37 @@
    status = "okay";
    zephyr,random-mac-address;
 };
+
+
+&flash0 {
+
+   partitions {
+       compatible = "fixed-partitions";
+       #address-cells = <1>;
+       #size-cells = <1>;
+
+       boot_partition: partition@0 {
+           label = "mcuboot";
+           reg = <0x00000000 0x00010000>;
+           read-only;
+       };
+
+       storage_partition: partition@1e000 {
+           label = "storage";
+           reg = <0x0001e000 0x00002000>;
+       };
+
+       slot0_partition: partition@20000 {
+           label = "image-0";
+           reg = <0x00020000 0x00060000>;
+       };
+       slot1_partition: partition@80000 {
+           label = "image-1";
+           reg = <0x00080000 0x00060000>;
+       };
+       scratch_partition: partition@e0000 {
+           label = "image-scratch";
+           reg = <0x000e0000 0x00020000>;
+       };
+   };
+};
\ No newline at end of file
diff --git a/drivers/flash/Kconfig.sam0 b/drivers/flash/Kconfig.sam0
index 5de7c50b5a..dc085d1b30 100644
--- a/drivers/flash/Kconfig.sam0
+++ b/drivers/flash/Kconfig.sam0
@@ -14,6 +14,7 @@ menuconfig SOC_FLASH_SAM0

 config SOC_FLASH_SAM0_EMULATE_BYTE_PAGES
    bool "Emulate byte-sized pages"
+   default y
    depends on SOC_FLASH_SAM0
    help
      Emulate a device with byte-sized pages by doing a

which results in nvs sample working properly

*** Booting Zephyr OS build zephyr-v2.4.0-2327-g4f355355395f  ***
No address found, adding 192.168.1.1 at id 1
No key found, adding it at id 2
No Reboot counter found, adding it at id 3
Id: 4 not found, adding it
Longarray not found, adding it as id 5
[00:00:00.008,000] <inf> fs_nvs: 3 Sectors of 8192 bytes
[00:00:00.008,000] <inf> fs_nvs: alloc wra: 0, 1ff0
[00:00:00.008,000] <inf> fs_nvs: data wra: 0, 0
Reboot counter history: ...0
Oldest reboot counter: 0
Rebooting in ...5...4...3...2...1
*** Booting Zephyr OS build zephyr-v2.4.0-2327-g4f355355395f  ***
Id: 1, Address: 192.168.1.1
Id: 2, Key: ff fe fd fc fb fa f9 f8
Id: 3, Reboot_counter: 1
Id: 4, Data: DATA
Id: 5, Longarray: 0 1 2 3 4 5 6 7 8 9 a b c d e f 10 11 12 13 14 15 16 17 18 19 1a 1b 1c 1d 1e 1f 20 21  22 23 24 25 26 27 28 29 2a 2b 2c 2d 2e 2f 30 31 32 33 34 35 36 37 38 39 3a 3b 3c 3d 3e 3f 40 41 42 43 44  45 46 47 48 49 4a 4b 4c 4d 4e 4f 50 51 52 53 54 55 56 57 58 59 5a 5b 5c 5d 5e 5f 60 61 62 63 64 65 66 6 7 68 69 6a 6b 6c 6d 6e 6f 70 71 72 73 74 75 76 77 78 79 7a 7b 7c 7d 7e 7f
[00:00:00.008,000] <inf> fs_nvs: 3 Sectors of 8192 bytes
[00:00:00.008,000] <inf> fs_nvs: alloc wra: 0, 1fc0
[00:00:00.008,000] <inf> fs_nvs: data wra: 0, a1
Reboot counter history: ...1...0
Oldest reboot counter: 0
Rebooting in ...5...4...3...2...1
*** Booting Zephyr OS build zephyr-v2.4.0-2327-g4f355355395f  ***
Id: 1, Address: 192.168.1.1
Id: 2, Key: ff fe fd fc fb fa f9 f8
Id: 3, Reboot_counter: 2
Id: 4, Data: DATA
Id: 5, Longarray: 0 1 2 3 4 5 6 7 8 9 a b c d e f 10 11 12 13 14 15 16 17 18 19 1a 1b 1c 1d 1e 1f 20 21  22 23 24 25 26 27 28 29 2a 2b 2c 2d 2e 2f 30 31 32 33 34 35 36 37 38 39 3a 3b 3c 3d 3e 3f 40 41 42 43 44  45 46 47 48 49 4a 4b 4c 4d 4e 4f 50 51 52 53 54 55 56 57 58 59 5a 5b 5c 5d 5e 5f 60 61 62 63 64 65 66 6 7 68 69 6a 6b 6c 6d 6e 6f 70 71 72 73 74 75 76 77 78 79 7a 7b 7c 7d 7e 7f
[00:00:00.008,000] <inf> fs_nvs: 3 Sectors of 8192 bytes
[00:00:00.008,000] <inf> fs_nvs: alloc wra: 0, 1fb8
[00:00:00.008,000] <inf> fs_nvs: data wra: 0, a5
Reboot counter history: ...2...1...0
Oldest reboot counter: 0
Rebooting in ...5...4...3...2...1

[Laczen]

@asob, thank you for reporting this.

Indeed without a valid storage partition defined the sample will not work. Your fix will enable to use the nvs subsystem for your application, but maybe we need to add some extra information in the README.rst (and/or documentation) stating that "a valid flash partitioning setup (including a storage partition) is required".

I wouldn't go as far as to call this a bug, the flash partitioning was just not setup. When new devices are added this is to be expected. Some hardware manufacturers are helping a lot in the zephyr development and they are monitoring so that their devices can be used to their full potential. For other cases there are users who are contributing without connection to the hardware manufacturer. These users might be a bit more conservative into adding features they have not tested, so sometimes things are not added.

[asob]

@Laczen: You're welcome. Chip shortage is forcing many of us to switch to devices that are actually available so it's no longer about eing able to pick the device that has best support but rather a device that is available and make it work.

Valid partitioning information is not enough to get this sample working, I found that SOC_FLASH_SAM0_EMULATE_BYTE_PAGES must be defaulted to yes in drivers/flash/Kconfig.sam0 in order to have this sample operate as expected.

[Laczen]

@asob, I guess that if you have to enable EMULATE_BYTE_PAGES that the write-block-size of the flash is larger than 32. If this is the case and the driver does not allow rewrites to a flash address DO NOT USE nvs. The EMULATE_BYTE_PAGES is writing one write-block-size at a time and the method works well if you only "linearly" write to flash. nvs however writes data at the beginning of a sector and a allocation entry at the end (so "jumping" around). The result will be (unless the driver allows rewrites) that for each write of a nvs entry at least two write-block-sizes will be taken, resulting in very bad flash utilization and fast flash wear. If this is the case it is better to use fcb for storage. This is not so fail safe but uses "linear" writes.

Maybe this is the reason why the developers have not partitioned the flash.

[asob]

@Laczen Ok, I understand. I see from atmel's datasheet that flash writing is a bit restrictive in terms of minimum size, so I played around with the idea of having a block-sized buffer in ram (8192 bytes) which is used when writes are perfomed, to have some sort of read-modify-write procedure.

So I re-wrote the flash_sam0_write in such a way that every time a write occurs (aligned/unaligned to block boundary), i copy the whole block or blocks into ram, modify and then write it to back to flash. So far it loooks like it's working on nvs, littlefs samples.

So far the only downside is that I have to have an 8k buffer in ram at all times, well unless I choose to allocate/free it on demand.

[nandojve]

Hi @asob , It will be nice if you can upstream your solution.

CC: @arvinf

[Laczen]

@Laczen Ok, I understand. I see from atmel's datasheet that flash writing is a bit restrictive in terms of minimum size, so I played around with the idea of having a block-sized buffer in ram (8192 bytes) which is used when writes are perfomed, to have some sort of read-modify-write procedure.

So I re-wrote the flash_sam0_write in such a way that every time a write occurs (aligned/unaligned to block boundary), i copy the whole block or blocks into ram, modify and then write it to back to flash. So far it loooks like it's working on nvs, littlefs samples.

So far the only downside is that I have to have an 8k buffer in ram at all times, well unless I choose to allocate/free it on demand.

@asob, if this works there is no need for such a large buffer, just one of the write block size should suffice.

[arvinf]

I have an update to the flash driver that I'll be submitting as a PR soon to deal with this issue. If the write size or alignment is smaller than a page (which is 512 bytes on the SAM53) then it uses WQW -- write quad word -- command to write the data.

[github-actions[bot]]

This issue has been marked as stale because it has been open (more than) 60 days with no activity. Remove the stale label or add a comment saying that you would like to have the label removed otherwise this issue will automatically be closed in 14 days. Note, that you can always re-open a closed issue at any time.