On 2020-01-12 03:49, Baruch Siach wrote:
Hi Joel,
On Sat, Jan 11 2020, Joel Johnson wrote:
Remove MMC booting assumptions from clearfog_defconfig. Their presence in starting config files conflicts with default ENV_IS_IN selection logic based on boot source, since the "select" option can't distinguish between user-overridden and defconfig selected.
As I mentioned in a previous patch review, I believe that SPI boot support should be in a separate defconfig.
As I responded in a previous patch, I like to specifically avoid that, to enable (not by default) mixed media boot and storage locations. The current set I have makes the mixed mode almost achievable, except needing to explicit set CONFIG_ENV_IS_IN_MMC=n when enabling SPI boot, otherwise it boots from SPI and tries to load env from MMC. That's a use case I'm after, but isn't what should be a reasonable default given the surprise factor if not intended.
One more comment below.
Signed-off-by: Joel Johnson mrjoel@lixil.net
The real objective is to remove CONFIG_MVEBU_SPL_BOOT_DEVICE_MMC as well but I couldn't identify an available mechanism to do so. Requiring it as enabled in the defconfig results in additional items being unneccessarily included if switching boot device after starting with the defconfig, as well as ENV_IS_IN_MMC being undesirably residually defined.
arch/arm/mach-mvebu/Kconfig | 1 + board/solidrun/clearfog/Kconfig | 5 +++++ configs/clearfog_defconfig | 3 --- 3 files changed, 6 insertions(+), 3 deletions(-)
diff --git a/arch/arm/mach-mvebu/Kconfig b/arch/arm/mach-mvebu/Kconfig index b3239b2b01..3f9aa4b26e 100644 --- a/arch/arm/mach-mvebu/Kconfig +++ b/arch/arm/mach-mvebu/Kconfig @@ -248,6 +248,7 @@ config MVEBU_SPL_BOOT_DEVICE_MMC select SPL_DM_GPIO select SPL_DM_MMC select SPL_LIBDISK_SUPPORT
- select SPL_MMC_SUPPORT
config MVEBU_SPL_BOOT_DEVICE_SATA bool "SATA" diff --git a/board/solidrun/clearfog/Kconfig b/board/solidrun/clearfog/Kconfig index 253b925ba1..bd51df8750 100644 --- a/board/solidrun/clearfog/Kconfig +++ b/board/solidrun/clearfog/Kconfig @@ -52,6 +52,11 @@ config ENV_SECT_SIZE # Use optimistic 64 KiB erase block, will vary between actual media default 0x10000 if MVEBU_SPL_BOOT_DEVICE_MMC
+config SYS_MMCSD_RAW_MODE_U_BOOT_SECTOR
- hex "Address on the MMC to load U-Boot from"
- default 0x141
- depends on MVEBU_SPL_BOOT_DEVICE_MMC
You might want to take a look at the series linked below, which conflicts with this patch:
https://patchwork.ozlabs.org/project/uboot/list/?series=121894
Indeed, the dynamic runtime approach seems much nicer for handling the MMCSD offset business, including relative vs. absolute device offset. I only have non-eMMC units so can't test it, but I'm all for dropping the SYS_MMCSD_RAW_MODE_U_BOOT_SECTOR portion of this patch in favor of that patch, in which leaving the defconfig entry does no harm. Any indication of when that patch will be included? It seems to have been stagnant for a while.
Joel