Modify the Kconfig name of the mtdconfig module 2 #3371
+7
−7
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Because the MTD_CONFIG configuration item in drivers/mtd/Kconfig has changed Signed-off-by: zhaoxingyu1 <zhaoxingyu1@xiaomi.com>
Because MTD_CONFIG configuration item in drivers/mtd/Kconfig has changed. Signed-off-by: zhaoxingyu1 <zhaoxingyu1@xiaomi.com>
Contributor
Author
Configuration/Tool: esp32c3-legacy-devkit/nvcfgdata
|
Contributor
|
@zhaoxingyu12 please include some testing results. Did you test CFGDATA too? Maybe we need to add CFGDATA to "SIM" or "QEMU" to make the test easy to do. Currently only two boards are using it |
acassis
requested review from
cederom,
jerpelea,
linguini1 and
xiaoxiang781216
Contributor
|
@zhaoxingyu12 I'm not sure that changing MTD_CONFIG to MTD_CONFIG_NONE is a good name change. I noticed your idea was to rename MTD_CONFIG_FAIL_SAFE to MTD_CONFIG_NVS, but it will be strange see CFGDATA depending on MTD_CONFIG_NONE. Could you please add more details in the git commit message about this change? It seems very strange |
Contributor
Author
now, CFGDATA depending on !MTD_CONFIG_NONE. （If either MTD_CONFIG or MTD_CONFIG_NVS is enabled, CFGDATA will be enabled.） Currently, the mtd_config implementations use a choice structure, where all implementations (such as MTD_CONFIG, MTD_CONFIG_NVS (Original name: MTD_CONFIG_FAIL_SAFE), and any future submodules) are defined as mutually exclusive options—only one can be selected at a time. By introducing MTD_CONFIG_NONE as a "no implementation" option, other submodules only need to depend on MTD_CONFIG_NONE=n, ensuring that their features are enabled only when a specific implementation is chosen. This approach not only avoids multiple dependencies and configuration conflicts, but also provides a clearer and more unified way to integrate additional mtd_config submodules in the future. it relates to this pr apache/nuttx#18120 choice config MTD_CONFIG config MTD_CONFIG_NVS config MTD_CONFIG_NONE endchoice |
Contributor
Author
I have conducted tests on QEMU, and the test results are in line with expectations.
|
Contributor
|
Thank you @zhaoxingyu12 :-) Ci failed.. probably needs a rebase? I agree with @acassis that |
| #ifndef CONFIG_DISABLE_MOUNTPOINT | ||
| # if defined(CONFIG_MTD_LOOP) && !defined(CONFIG_NSH_DISABLE_LOMTD) | ||
| # ifdef CONFIG_MTD_CONFIG | ||
| # ifndef CONFIG_MTD_CONFIG_NONE |
Contributor
There was a problem hiding this comment.
let's fix nvs test too
Contributor
_NONE suffix is used in many places, could we keep it: |
Contributor
|
Yup :-) Similar with |
Contributor
|
CI failed, PR needs a rebase? :-) |
Contributor
@zhaoxingyu12 please fix the ci break. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Note: Please adhere to Contributing Guidelines.
Summary
Modify the Kconfig name of the mtdconfig module 2
This pull request makes two main adjustments to align with recent changes in the NuttX mtd_config module Kconfig.
Impact
These changes ensure compatibility with the updated mtd_config Kconfig structure in NuttX. By standardizing the naming and clarifying dependency logic, the configuration becomes more robust and easier to maintain, reducing the risk of conflicts and making it simpler to integrate or extend mtd_config submodules in the future
Testing
test it by CICT together with apache/nuttx#18120
I have conducted tests with CFGDATA on QEMU, and the test results are in line with expectations.
enabled these configurations.
CONFIG_MTD_CONFIG_NVS=y
CONFIG_SYSTEM_CFGDATA=y
CONFIG_MTD_CONFIG_NAMED=y
register lomtdconfig
dd if=/dev/zero of=/data/nvs.img bs=8192 count=10
lomtd -c 2 /dev/config /data/nvs.img
test pass
goldfish-armv7a-ap> cfgdata print mykey
Error reading config entry 'mykey'
goldfish-armv7a-ap> cfgdata set mykey "myvalue"
goldfish-armv7a-ap>
goldfish-armv7a-ap> cfgdata print mykey
myvalue
goldfish-armv7a-ap> cfgdata all
Name Len Data
mykey 8 myvalue