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 <mrj...@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

Reply via email to