Skip to content

builder: Fix false positive flash map errors #601

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

Merged
merged 1 commit into from
Jun 28, 2025

Conversation

m-gorecki
Copy link
Contributor

To allow conditional overrides of flash map it's necessary to resolve target, because otherwise the syscfgs that determine the choice of a different flash map will not be available. The problem was that every resolving was trying to detect errors in configuration (also in flash map). As the resolving had to be done before bsp package reloading, while detecting errors the default flash map is loaded. If this default flash map did not define some flash areas used in the target configuration, an error was reported even if the flash map that should override the default one was correct.

Now we don't try to detect error while doing this particular resolving. Instead we do this after reloading bsp package so the properly chosen flash map is used in error detection.

To allow conditional overrides of flash map it's necessary to
resolve target, because otherwise the syscfgs that determine
the choice of a different flash map will not be available.
The problem was that every resolving was trying to detect
errors in configuration (also in flash map). As the resolving
had to be done before bsp package reloading, while detecting
errors the default flash map is loaded. If this default flash
map did not define some flash areas used in the target configuration,
an error was reported even if the flash map that should override
the default one was correct.

Now we don't try to detect error while doing this particular
resolving. Instead we do this after reloading bsp package so
the properly chosen flash map is used in error detection.
@kasjer
Copy link
Contributor

kasjer commented Jun 27, 2025

I tested this on original blackpill411ce bsp.
With this change it builds when

syscfg.vals:
    SPIFLASH8M: 1

but does not when default flash map is supposed to be used

syscfg.vals:
    SPIFLASH8M: 0

Interestingly enough it does not build without this change at all regardless of syscfg value. (Which is strange, I think one of the versions built before).

@m-gorecki
Copy link
Contributor Author

I assume you get error "Setting NFFS_FLASH_AREA specifies unknown flash area: FLASH_AREA_NFFS".
If you are building the same target (which probably uses fs/nffs package) with just SPIFLASH8M disabled then this is expected behavior. blackpill411ce bsp sets syscfg:
NFFS_FLASH_AREA: FLASH_AREA_NFFS
and if FLASH_AREA_NFFS is not defined in the chosen flash map then it's an error and default flash map indeed does not define this area. It probably worked before because newt ignores set but undefined syscfgs. So in some other target, which did not use fs/nffs package this setting was ignored and the default flash map was fine.
This also explains why target doesn't build without this change regardless of syscfg value - without this PR default map was always used when detecting flash configuration errors.

@kasjer
Copy link
Contributor

kasjer commented Jun 28, 2025

@m-gorecki thanks that was it, it works as expected.
I was not aware of type: flash_owner in syscfg.defs:

syscfg.defs:
    NFFS_FLASH_AREA:
        description: 'Flash area to use for NFFS.'
        type: flash_owner
        value:
        restrictions:
            - $notnull

@m-gorecki m-gorecki merged commit 423bd89 into apache:master Jun 28, 2025
70 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants