TL;DR: The two functions are only used in the SPL MMC driver so I think
their names should reflect that.
spl_boot_mode() has caused a bit of trouble to me and others because
some of its implementations miss what it is meant to do in its current
form. This is in part due to history and I suspect in part due to its
name being a bit misleading.
As a quick summary: spl_boot_mode() is solely used by the SPL MMC driver
to check how it should load U-Boot from the currently attempted
boot-source. There are three possibilities:
1. MMCSD_MODE_FS: The MMC device contains a filesystem (FAT or EXT4).
2. MMCSD_MODE_EMMCBOOT: U-Boot should be loaded from an eMMC
boot-partition.
3. MMCSD_MODE_RAW: U-Boot should be loaded from a raw offset or
a partition on the MMC device.
spl_boot_mode() should, for each MMC boot device, return one of those
3 modes. The boot_device parameter tells which device is queried.
Historically, the boot_device parameter did not exist. Because of this,
a lot of implementations relied on checking where SPL was loaded from
(spl_boot_device()) as it is common to load U-Boot from the same device.
However, nowadays it is possible to attempt loading U-Boot from multiple
MMC devices (as a fallback), for example using board_boot_order().
This means, spl_boot_mode() needs to properly tell the MMC driver the
mode of each device. This was the reason for introducing the
boot_device parameter in commit 2b1cdafa9fdd ("common: Pass the boot
device into spl_boot_mode()")).
I am bringing all this up because some existing implementations don't
handle this properly and new code has been merged which also doesn't.
To make it more clear what the function and its cousin,
spl_boot_partition(), are supposed to do, I suggest renaming them to
spl_mmc_boot_mode() and spl_mmc_boot_partition() respectively.
I will also send a second series to fix the implementation which is
affecting me (arch/arm/mach-imx/spl.c).
Harald Seiler (2):
spl: mmc: Rename spl_boot_mode() to spl_mmc_boot_mode()
spl: mmc: Rename spl_boot_partition() to spl_mmc_boot_partition()
include/spl.h | 32 ++++++++++++++++++++++++--
common/spl/spl_mmc.c | 9 ++++----
arch/arm/mach-imx/spl.c | 2 +-
arch/arm/mach-k3/am6_init.c | 2 +-
arch/arm/mach-k3/j721e_init.c | 2 +-
arch/arm/mach-omap2/boot-common.c | 2 +-
arch/arm/mach-rockchip/spl.c | 2 +-
arch/arm/mach-socfpga/spl_a10.c | 2 +-
arch/arm/mach-socfpga/spl_agilex.c | 2 +-
arch/arm/mach-socfpga/spl_gen5.c | 2 +-
arch/arm/mach-socfpga/spl_s10.c | 2 +-
arch/arm/mach-stm32mp/spl.c | 4 ++--
arch/arm/mach-uniphier/mmc-boot-mode.c | 2 +-
13 files changed, 46 insertions(+), 19 deletions(-)
TL;DR: The two functions are only used in the SPL MMC driver so I think their names should reflect that. spl_boot_mode() has caused a bit of trouble to me and others because some of its implementations miss what it is meant to do in its current form. This is in part due to history and I suspect in part due to its name being a bit misleading. As a quick summary: spl_boot_mode() is solely used by the SPL MMC driver to check how it should load U-Boot from the currently attempted boot-source. There are three possibilities: 1. MMCSD_MODE_FS: The MMC device contains a filesystem (FAT or EXT4). 2. MMCSD_MODE_EMMCBOOT: U-Boot should be loaded from an eMMC boot-partition. 3. MMCSD_MODE_RAW: U-Boot should be loaded from a raw offset or a partition on the MMC device. spl_boot_mode() should, for each MMC boot device, return one of those 3 modes. The boot_device parameter tells which device is queried. Historically, the boot_device parameter did not exist. Because of this, a lot of implementations relied on checking where SPL was loaded from (spl_boot_device()) as it is common to load U-Boot from the same device. However, nowadays it is possible to attempt loading U-Boot from multiple MMC devices (as a fallback), for example using board_boot_order(). This means, spl_boot_mode() needs to properly tell the MMC driver the mode of each device. This was the reason for introducing the boot_device parameter in commit 2b1cdafa9fdd ("common: Pass the boot device into spl_boot_mode()")). I am bringing all this up because some existing implementations don't handle this properly and new code has been merged which also doesn't. To make it more clear what the function and its cousin, spl_boot_partition(), are supposed to do, I suggest renaming them to spl_mmc_boot_mode() and spl_mmc_boot_partition() respectively. I will also send a second series to fix the implementation which is affecting me (arch/arm/mach-imx/spl.c). Harald Seiler (2): spl: mmc: Rename spl_boot_mode() to spl_mmc_boot_mode() spl: mmc: Rename spl_boot_partition() to spl_mmc_boot_partition() include/spl.h | 32 ++++++++++++++++++++++++-- common/spl/spl_mmc.c | 9 ++++---- arch/arm/mach-imx/spl.c | 2 +- arch/arm/mach-k3/am6_init.c | 2 +- arch/arm/mach-k3/j721e_init.c | 2 +- arch/arm/mach-omap2/boot-common.c | 2 +- arch/arm/mach-rockchip/spl.c | 2 +- arch/arm/mach-socfpga/spl_a10.c | 2 +- arch/arm/mach-socfpga/spl_agilex.c | 2 +- arch/arm/mach-socfpga/spl_gen5.c | 2 +- arch/arm/mach-socfpga/spl_s10.c | 2 +- arch/arm/mach-stm32mp/spl.c | 4 ++-- arch/arm/mach-uniphier/mmc-boot-mode.c | 2 +- 13 files changed, 46 insertions(+), 19 deletions(-)