Makefile.include: include Makefile.features after $(CPU)/Makefile.include

[fjmolinas]

Contribution description

Currently in base Makefile.include, Makefile.fatures for a BOARD is included after $(RIOTCPU)/$(CPU)/Makefile.include.

RIOT/Makefile.include

Lines 247 to 258 in 9a64731

	# process provided features
	include $(RIOTBOARD)/$(BOARD)/Makefile.features
	# mandatory includes!
	include $(RIOTMAKE)/pseudomodules.inc.mk
	include $(RIOTMAKE)/defaultmodules.inc.mk
	# Include Board and CPU configuration
	INCLUDES += $(addprefix -I,$(wildcard $(RIOTBOARD)/$(BOARD)/include))
	include $(RIOTBOARD)/$(BOARD)/Makefile.include
	INCLUDES += -I$(RIOTCPU)/$(CPU)/include
	include $(RIOTCPU)/$(CPU)/Makefile.include

This causes the below statement to have no effect since KINETIS_SERIES isn't defined at this point. But more generaly means none of the variables defined are in $(RIOTCPU)/$(CPU)/Makefile.include are yet available.

RIOT/cpu/kinetis/Makefile.features

Lines 5 to 9 in 9a64731

	ifeq (EA,$(KINETIS_SERIES))
	FEATURES_PROVIDED += periph_ics
	else
	FEATURES_PROVIDED += periph_mcg
	endif

Moving Makfile.features below include $(RIOTCPU)/$(CPU)/Makefile.include. would allow using the SERIES, FAMILY, etc. to include some features for the specific group.

I'm not sure of the impact of this change, I would like @cladmi insight on the matter. Based on my inspection of the build system there would be no negative impact since FEATURES_PROVIDED are first used by Makefile.dep. Also Makefile.features files only reference themselves so this doesn't affect the include $(RIOTBOARD)/$(BOARD)/Makefile.include and $(RIOTCPU)/$(CPU)/Makefile.include.

Testing procedure

All applications for all boards should still compile as expected.

To see this fixes #11423 add $(info Kinetis series: $(KINETIS_SERIES)) just before the above the code below and call make -C examples/hello-world/ BOARD=pba-d-01-kw2x.

RIOT/cpu/kinetis/Makefile.features

Lines 5 to 9 in 9a64731

	ifeq (EA,$(KINETIS_SERIES))
	FEATURES_PROVIDED += periph_ics
	else
	FEATURES_PROVIDED += periph_mcg
	endif

Without this PR:

make: Entering directory '/home/francisco/workspace/RIOT/examples/hello-world'
Kinetis series: W
Building application "hello-world" for "pba-d-01-kw2x" with MCU "kinetis".

Without this PR:

make: Entering directory '/home/francisco/workspace/RIOT/examples/hello-world'
Kinetis series:
Building application "hello-world" for "pba-d-01-kw2x" with MCU "kinetis".

Issues/PRs references

Could fix #11423.
Related to #8713

[kaspar030]

features are also evaluated by the "info-boards-supported" machinery, which in turn cannot include cpu/Makefile.features, as this happens in a single Makefile run. That machinery would break with the proposed change.

[cladmi]

The issue is also present in the efm32 cpu which depends on a variable defined by Makefile.include

RIOT/cpu/efm32/Makefile.features

Lines 9 to 11 in d16d911

	ifeq (1,$(EFM32_TNRG))
	FEATURES_PROVIDED += periph_hwrng
	endif

(where there is even a typo in TRNG but fixing it changes nothing).

The problem should however be solved the other way around, with these variables all defined by Makefile.features.

It cannot currently be done as CPU is defined by board/*/Makefile.include, I described the issue in #9913 where the next step would be to move all the CPU/CPU_FAM/CPU_FAMILY/CPU_MODEL variables to Makeflie.features.
I welcome feedback in the tracking issue if I should PR a change in this direction or how to handle it otherwise.

And indeed, there is also this file https:/RIOT-OS/RIOT/blob/master/makefiles/info-global.inc.mk#L10 that does the evaluation in a different way for getting the dependencies.
Moving these BSP specific variables would require to list them for cleaning them between each evaluation or change this handling but may not be directly possible.

The two way of doing the parsing already leads to inconsistencies:

make info-boards-supported | tr ' ' '\n' | grep mips-malta
mips-malta

BOARD=mips-malta make
There are unsatisfied feature requirements: periph_uart


EXPECT ERRORS!

[fjmolinas]

@cladmi @kaspar030 Thanks for your insight into the build system!

I understand even though this is a valid problem this PR is not the propper way of fixing the issue. I will close this PR and raise the issue in #9913 (I had missed it when opening this PR).