Murdock results

✔️ PASSED

0908b86 boards/esp32-wrover-kit: cleanup of MTD dependencies

Artifacts

• Documentation preview

Hm, it doesn't seem to be as easy as I though. Enabling mtd_sdcard after including Makefile.include doesn't pull in sdcard_spi automatically 🤔 It seems to be a chicken-egg problem. Testing for any enabled mtd_*_default module before including Makefile.include doesn't work because boards make dependencies are then resolved afterwards.

I had to change the approach to clean up the dependencies. Now, the makefile variable BOARD_WITH_MTD_SDMMC defines the boards that have the SD Card connected to a SD/MMC Controller and for which the MTD driver module mtd_sdmmc should be used for the tests/pkg/fatfs* test applications.

The same functionality is already implemented in drivers/mtd:

Just removing the following lines from the tests/pkg/fatfs/Makefile fixed the issue for me:

ifneq (,$(filter native native32 native64,$(BOARD)))
#overwrite default mtd_native-config to use fat image as flash device
CFLAGS += -DMTD_NATIVE_FILENAME=\"./bin/riot_fatfs_disk.img\"
CFLAGS += -DFATFS_IMAGE_FILE_SIZE_MIB=$(FATFS_IMAGE_FILE_SIZE_MIB)
CFLAGS += -DMTD_SECTOR_NUM=\(\(\(FATFS_IMAGE_FILE_SIZE_MIB\)*1024*1024\)/MTD_SECTOR_SIZE\)
- else
- # for actual hardware use mtd_sdcard as storage device
- USEMODULE += mtd_sdcard
endif

This is the console log (also works for devices with mtd_sdcard):

chris@W11nMate:~/RIOTstuff/riot-gettingstarted/RIOT/tests/pkg/fatfs$ BOARD=stm32f746g-disco make info-modules | grep  mtd
fatfs_diskio_mtd
mtd
mtd_sdmmc
mtd_sdmmc_default

chris@W11nMate:~/RIOTstuff/riot-gettingstarted/RIOT/tests/pkg/fatfs$ BOARD=sensebox_samd21 make info-modules | grep mtd
fatfs_diskio_mtd
mtd
mtd_sdcard
mtd_sdcard_default

Just removing the following lines from the tests/pkg/fatfs/Makefile fixed the issue for me:

🤔 that's interesting but makes the life much more easier.

Just removing the following lines from the tests/pkg/fatfs/Makefile fixed the issue for me:

Dependencies for MTDs are just bit tricky. Some years ago I had a discussion with @benpicco whether it would make sense to define the used SD Card interface as a feature of the board. At that time, we decided not to define according features to be able to wire any SD Card breakout module for testing anytime.

In hindsight it would make sense to define the feature by the board. Even when wiring the SD card manually, that requires a change to the board configuration, so adding the feature would be no big hassle.

Also back then we didn't have many boards with proper / native SD card interfaces, so manual wiring was the way to go.

We should probably also remove the sam0_sdhc driver, not much point in having two drivers for the same hardware.

Just removing the following lines from the tests/pkg/fatfs/Makefile fixed the issue for me:

ifneq (,$(filter native native32 native64,$(BOARD)))
#overwrite default mtd_native-config to use fat image as flash device
CFLAGS += -DMTD_NATIVE_FILENAME=\"./bin/riot_fatfs_disk.img\"
CFLAGS += -DFATFS_IMAGE_FILE_SIZE_MIB=$(FATFS_IMAGE_FILE_SIZE_MIB)
CFLAGS += -DMTD_SECTOR_NUM=\(\(\(FATFS_IMAGE_FILE_SIZE_MIB\)*1024*1024\)/MTD_SECTOR_SIZE\)
- else
- # for actual hardware use mtd_sdcard as storage device
- USEMODULE += mtd_sdcard
endif

Not really. It works for boards that have an SDMMC or SPI SD Card interfac and enable mtd_sdmmc_default or mtd_sdcard_default, respectively. But it doesn't work for other boards that should use mtd_sdcard by default even though they don't have it:

BOARD=atmega256rfr2-xpro make -j4 -C tests/pkg/fatfs info-modules | grep -E "mtd|sdcard"
fatfs_diskio_mtd
mtd

Therefore, I dropped the last two fixups.

We could use a tests/pkg/fatfs/Makefile.board.dep to handle the board specific dependencies. This file is (re-)evaluated during the dependency resolution, unlike the tests/pkg/fatfs/Makefile or tests/pkg/fatfs/Makefile.include, which are only evaluated once.

ifeq (,$(filter mtd_sdcard mtd_sdmmc,$(USEMODULE)))
  # if no SD interface is specified, use mtd_sdcard by default
  USEMODULE += mtd_sdcard
endif

This results in the following output:

cbuec@W11nMate:~/RIOTstuff/riot-gettingstarted/RIOT$ BOARD=atmega256rfr2-xpro make -j4 -C tests/pkg/fatfs info-modules | grep -E "mtd|sdcard"
shell fatfs_diskio_mtd mtd
fatfs_diskio_mtd
mtd
mtd_sdcard
sdcard_spi

But for boards with SD Card connected to SDMMC, we have the problem as before:

BOARD=stm32f746g-disco make -j4 -C tests/pkg/fatfs_vfs info-modules | grep -E "mtd|sdcard"
fatfs_diskio_mtd
mtd
mtd_sdcard
mtd_sdmmc
mtd_sdmmc_default
sdcard_spi

Makefile.board.dep is evaluated at the beginning, even before the Makefile.dep of the board is included, so that mtd_sdmm_default is not yet set in the first pass of dependency resolution and thus mtd_sdcard is set in any case.

Yes indeed. While I generally consider this behavior a bug of #12755, I'm not sure if changing that behavior won't introduce other issues in other applications...

Consider this Makefile.board.dep, which tracks the dependency resolution rounds and only evaluates the USEMODULE after the second round.

Note that Make does not support incrementing a counter variable, therefore I used a string. Not the prettiest thing, but more elegant than a load of shell calls IMO.

# The dependency resolution is called multiple times, but not all the
# dependencies are resolved in the first round, such as the board deps.
# To make sure that the board deps are resolved before running, we
# create a temporary variable that stores the rounds and adds a `+` to
# the string (make does not support integer incrementation).
TEST_FATFS_DEP_ROUND := $(TEST_FATFS_DEP_ROUND)+

ifeq (++,$(TEST_FATFS_DEP_ROUND))
  ifeq (,$(filter mtd_sdcard mtd_sdmmc,$(USEMODULE)))
    # if no SD interface is specified, use mtd_sdcard by default
    USEMODULE += mtd_sdcard
  endif
endif

This leads to this output:

cbuec@W11nMate:~/RIOTstuff/riot-gettingstarted/RIOT$ BOARD=stm32f746g-disco make -j4 -C tests/pkg/fatfs info-modules | grep -E "mtd
|sdcard"
fatfs_diskio_mtd
mtd
mtd_sdmmc
mtd_sdmmc_default
cbuec@W11nMate:~/RIOTstuff/riot-gettingstarted/RIOT$ BOARD=atmega256rfr2-xpro make -j4 -C tests/pkg/fatfs info-modules | grep -E "mtd|sdcard"
fatfs_diskio_mtd
mtd
mtd_sdcard
sdcard_spi

Consider this Makefile.board.dep, which tracks the dependency resolution rounds and only evaluates the USEMODULE after the second round.

Nice hack 😉

Murdock seems to be happy 👍

@crasbe Thanks for reviewing and the test application "hack".