pkg/flashdb: enhance FAL config by fabian18 · Pull Request #20221 · RIOT-OS/RIOT

fabian18 · 2024-01-02T23:23:52Z

Contribution description

I wanted to split out the FlashDB fal_cfg partition offset fix from #19557.
This also adds a short test to see if all partitions initialize fine and it adds a configuration option for the sector size.
The test very much reminds of an auto_init module for FLashDB.

Testing procedure

Test output of BOARD=same54-xpro make -C tests/pkg/flashdb_fal_cfg flash term

2024-01-03 00:09:14,858 # main(): This is RIOT! (Version: 2024.01-devel-475-g2de30-pr/flashdb_cfg_enhancement)
2024-01-03 00:09:14,862 # Initializing FlashDB KVDB partition part0
2024-01-03 00:09:14,869 # [I/FAL] Flash Abstraction Layer (V0.5.99) initialize success.
2024-01-03 00:09:14,879 # Initializing FlashDB KVDB partition part1
2024-01-03 00:09:14,883 # Initializing FlashDB TSDB partition part2
2024-01-03 00:09:14,887 # Initializing FlashDB TSDB partition part3
2024-01-03 00:09:14,887 # SUCCESS

Issues/PRs references

benpicco · 2024-01-03T12:25:37Z

tests/pkg/flashdb_fal_cfg/Makefile

+ifeq (,$(filter periph_rtc,$(FEATURES_PROVIDED)))
+  FEATURES_REQUIRED += periph_rtt
+  USEMODULE += rtt_rtc
+  USEMODULE += ztimer_no_periph_rtt
+else
+  FEATURES_REQUIRED += periph_rtc
+endif


This won't work unfortunately.
You don't have the FEATURES_PROVIDED information at this point, so you will always select periph_rtt

This is the known issue with the make dependency resolution... Maybe DEFAULT_MODULE can somehow be used but that only gets resolved on the last iteration.

I did not figure out how to resolve this with DEFAULT_MODULE but FEATURES_REQUIRED_ANY seems to do a good job here.

When I compile with BOARD=same54-xpro make it compiles fine.
And when I remove FEATURES_PROVIDED += periph_rtc from boards/same54-xpro/Makefile.featuresit also compiles fine with "Using real-time-timer RTC".

riot-ci · 2024-01-03T14:04:09Z

Murdock results

✔️ PASSED

bf4bfdf tests/pkg/flashdb_fal_cfg: add Makefile.ci

Success	Failures	Total	Runtime
10011	0	10011	10m:33s

Artifacts

Documentation preview

benpicco · 2024-01-09T11:19:00Z

tests/pkg/flashdb_fal_cfg/Makefile

+  $(info Using real-time-timer RTC)
+  USEMODULE += rtt_rtc
+  USEMODULE += ztimer_no_periph_rtt
+endif


Does this work? I would expect that doing this after the dependency resolution won't have any effect.

I don´t know the magic behind it but I assume it works. When I remove FEATURES_PROVIDED += periph_rtc from boards/same54-xpro/Makefile.features and do an info-modules I see that rtt_rtc and ztimer_no_periph_rtt are in. When I try to build after I remove USEMODULE += rtt_rtc linking fails, as expected.

tests/pkg/flashdb_fal_cfg/main.c:99: undefined reference to 'rtc_get_time'

benpicco

Please squash

benpicco · 2024-01-30T12:20:02Z

tests/pkg/flashdb_fal_cfg/Makefile

+
+# handle RTC backend after inclusion of $(RIOTBASE)/Makefile.include
+ifeq (,$(filter periph_rtc,$(FEATURES_USED)))
+  $(info Using real-time-timer RTC)


Looks like this output messes up CI

The specified board Using does not exist.

I was not able to figure this out by the CI message.

fabian18 · 2024-02-07T10:45:24Z

pkg/flashdb/include/fal_cfg.h

+ * a KV with a length of 10K, you can use the control function to set the sector size to 12K or larger.
+ *
+ */
+#define CONFIG_FLASHDB_MIN_SECTOR_SIZE_EXP      12


I should say that this is just the default but it can be different for 2 databases.
For that I would rename this to CONFIG_FLASHDB_DEFAULT_MIN_SECTOR_SIZE_EXP.

A module M that initializes a database should have something like:

#ifndef M_FLASHDB_MIN_SECTOR_SIZE_EXP #define M_FLASHDB_MIN_SECTOR_SIZE_EXP CONFIG_FLASHDB_MIN_SECTOR_SIZE_EXP #endif

Do you agree?

Hmm and the fact that it can be 10K or 12 actually means that it is not necessarily a power of 2.
I would change it to a factor x KiB Default would be 4 for 4K

the fact that it can be 10K or 12

Ah ok so there are no assumptions about this being a power of two in the code then - where did you find the 10k sector?

In the documentation of now CONFIG_FLASHDB_MIN_SECTOR_SIZE_DEFAULT_FACTOR I copied something from the documentation of flashDB Sector size and block size.

benpicco · 2024-02-07T17:40:09Z

pkg/flashdb/include/fal_cfg.h


+#if !defined(CONFIG_FLASHDB_MIN_SECTOR_SIZE_DEFAULT_FACTOR) || defined(DOXYGEN)
+/**
+ * @brief   Virtual sector size factor in multiples of 1KiB for FlashDB


Suggested change

* @brief Virtual sector size factor in multiples of 1KiB for FlashDB

* @brief Minimal virtual sector size in KiB for FlashDB

benpicco · 2024-02-07T17:41:31Z

pkg/flashdb/include/fal_cfg.h

+ * a KV with a length of 10K, you can use the control function to set the sector size to 12K or larger.
+ *
+ */
+#define CONFIG_FLASHDB_MIN_SECTOR_SIZE_DEFAULT_FACTOR   4


Suggested change

#define CONFIG_FLASHDB_MIN_SECTOR_SIZE_DEFAULT_FACTOR 4

#define CONFIG_FLASHDB_MIN_SECTOR_SIZE_DEFAULT_KiB 4

Instead of FACTOR you can just use the unit.

The MIN DEFAULT is a bit confusion, maybe add a note that this is only for the automatic partition configuration.

benpicco · 2024-02-09T16:03:41Z

You might have to run make generate-Makefile.ci

fabian18 added 2 commits December 31, 2023 01:27

pkg/flashdb: have FAL MTD and defaults for partition 0 defined

6e1e78d

pkg/flashdb: fix FAL partition offsets

bd75060

fabian18 requested review from MichelRottleuthner, MrKevinWeiss, aabadie, jia200x and leandrolanzieri as code owners January 2, 2024 23:23

github-actions bot added Area: tests Area: tests and testing framework Area: pkg Area: External package ports Area: Kconfig Area: Kconfig integration labels Jan 2, 2024

benpicco added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Jan 3, 2024

benpicco reviewed Jan 3, 2024

View reviewed changes

fabian18 changed the title ~~Pr/flashdb cfg enhancement~~ pkg/flashdb: enhance FAL config Jan 5, 2024

benpicco reviewed Jan 9, 2024

View reviewed changes

benpicco approved these changes Jan 14, 2024

View reviewed changes

fabian18 force-pushed the pr/flashdb_cfg_enhancement branch from 6a8af08 to 94f074e Compare January 18, 2024 20:47

benpicco reviewed Jan 30, 2024

View reviewed changes

fabian18 commented Feb 7, 2024

View reviewed changes

benpicco reviewed Feb 7, 2024

View reviewed changes

fabian18 added 2 commits February 8, 2024 20:48

pkg/flashdb: add configurable sector size

3167d58

tests/pkg/flashdb: add test for FAL config initialization

057695b

fabian18 force-pushed the pr/flashdb_cfg_enhancement branch from e6b67ee to 057695b Compare February 8, 2024 19:48

benpicco added this pull request to the merge queue Feb 9, 2024

github-merge-queue bot removed this pull request from the merge queue due to failed status checks Feb 9, 2024

benpicco added this pull request to the merge queue Feb 9, 2024

github-merge-queue bot removed this pull request from the merge queue due to failed status checks Feb 9, 2024

benpicco added this pull request to the merge queue Feb 10, 2024

github-merge-queue bot removed this pull request from the merge queue due to failed status checks Feb 10, 2024

fabian18 added 2 commits February 10, 2024 23:20

tests/pkg/flashdb_mtd: add Makefile.ci

b671dd4

tests/pkg/flashdb_fal_cfg: add Makefile.ci

bf4bfdf

fabian18 force-pushed the pr/flashdb_cfg_enhancement branch from f702848 to bf4bfdf Compare February 10, 2024 22:22

benpicco enabled auto-merge February 10, 2024 22:27

benpicco added this pull request to the merge queue Feb 10, 2024

Merged via the queue into RIOT-OS:master with commit 270aa70 Feb 11, 2024

MrKevinWeiss added this to the Release 2024.04 milestone Apr 30, 2024

fabian18 deleted the pr/flashdb_cfg_enhancement branch January 11, 2025 15:25

	* @brief Virtual sector size factor in multiples of 1KiB for FlashDB
	* @brief Minimal virtual sector size in KiB for FlashDB

	#define CONFIG_FLASHDB_MIN_SECTOR_SIZE_DEFAULT_FACTOR 4
	#define CONFIG_FLASHDB_MIN_SECTOR_SIZE_DEFAULT_KiB 4

Conversation

fabian18 commented Jan 2, 2024

Contribution description

Testing procedure

Issues/PRs references

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

riot-ci commented Jan 3, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Murdock results

Artifacts

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

fabian18 Jan 14, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

benpicco left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

fabian18 Feb 7, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

benpicco commented Feb 9, 2024

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

riot-ci commented Jan 3, 2024 •

edited

Loading

fabian18 Jan 14, 2024 •

edited

Loading

fabian18 Feb 7, 2024 •

edited

Loading