[PATCH 10/10] arm: mvebu: clearfog: don't assume MMC booting

[Joel Johnson]

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 at 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