[U-Boot] [linux-sunxi] [PATCH 1/6] sunxi: Add an option to disable MMC

[Hans de Goede <hdegoede@redhat.com>]

Hi,

First if all, thanks for the patches for this. I've a couple
of comments on a few of them (including this one) I'll reply to the one
I've comments on. no reply means I think it is fine :)

On 09/18/2015 08:06 AM, Maxime Ripard wrote:
> Some devices don't have any MMC devices, so it doesn't really make sense to
> enable the MMC related functions and options for them.
>
> Add an option to disable the MMC support entirely.
>
> Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
> ---
>   arch/arm/cpu/armv7/sunxi/board.c | 6 ++++++
>   include/configs/sunxi-common.h   | 7 +++++--
>   2 files changed, 11 insertions(+), 2 deletions(-)
>
> diff --git a/arch/arm/cpu/armv7/sunxi/board.c b/arch/arm/cpu/armv7/sunxi/board.c
> index b40198b36ee5..e6721feb4a4c 100644
> --- a/arch/arm/cpu/armv7/sunxi/board.c
> +++ b/arch/arm/cpu/armv7/sunxi/board.c
> @@ -135,7 +135,9 @@ DECLARE_GLOBAL_DATA_PTR;
>    */
>   u32 spl_boot_device(void)
>   {
> +#ifdef CONFIG_MMC
>   	struct mmc *mmc0, *mmc1;
> +#endif
>   	/*
>   	 * When booting from the SD card or NAND memory, the "eGON.BT0"
>   	 * signature is expected to be found in memory at the address 0x0004
> @@ -156,15 +158,18 @@ u32 spl_boot_device(void)
>   		return BOOT_DEVICE_BOARD;
>
>   	/* The BROM will try to boot from mmc0 first, so try that first. */
> +#ifdef CONFIG_MMC
>   	mmc_initialize(gd->bd);
>   	mmc0 = find_mmc_device(0);
>   	if (sunxi_mmc_has_egon_boot_signature(mmc0))
>   		return BOOT_DEVICE_MMC1;
> +#endif
>
>   	/* Fallback to booting NAND if enabled. */
>   	if (IS_ENABLED(CONFIG_SPL_NAND_SUPPORT))
>   		return BOOT_DEVICE_NAND;
>
> +#ifdef CONFIG_MMC
>   	if (CONFIG_MMC_SUNXI_SLOT_EXTRA == 2) {
>   		mmc1 = find_mmc_device(1);
>   		if (sunxi_mmc_has_egon_boot_signature(mmc1)) {
> @@ -178,6 +183,7 @@ u32 spl_boot_device(void)
>   			return BOOT_DEVICE_MMC2;
>   		}
>   	}
> +#endif
>
>   	panic("Could not determine boot source\n");
>   	return -1;		/* Never reached */
> diff --git a/include/configs/sunxi-common.h b/include/configs/sunxi-common.h
> index 48cc4ed6f629..4fde0d4371e4 100644
> --- a/include/configs/sunxi-common.h
> +++ b/include/configs/sunxi-common.h
> @@ -142,7 +142,7 @@
>   #endif
>
>   /* mmc config */
> -#if !defined(CONFIG_UART0_PORT_F)
> +#if !defined(CONFIG_UART0_PORT_F) && !defined(CONFIG_NO_MMC)

You're introducing a CONFIG_NO_MMC here, and setting that using
EXTRA_OPTIONS. EXTRA_OPTIONS has been deprecated, and adding
new options there is something which we do not want to do.

Instead I think it would be better to Kconfig-ify CONFIG_MMC,
the problem with doing this is that CONFIG_MMC gets used by
almost all SoCs supported by u-boot, and moving all of them
to use Kconfig for this at once is not ideal.

What u-boot has been doing so far for this is using a construct
like this:

config SYS_CLK_FREQ
         depends on ARC || ARCH_SUNXI
         int "CPU clock frequency"
         help
           TODO: Move CONFIG_SYS_CLK_FREQ for all the architecture

So for mmc we would get something like this in drivers/mmc/Kconfig:

config MMC
	depends on ARCH_SUNXI
	bool "Enable MMC support"
	help
	  TODO: Move CONFIG_MMC for all the architecture

And in board/sunxi/Kconfig

config MMC
	default y if ARCH_SUNXI && !UART0_PORT_F

We need the if ARCH_SUNXI to make "make savedefconfig" not cry out on
non SUNXI archs.

And then in sunxi-common.h we can just do:

#ifdef CONFIG_MMC
#define CONFIG_GENERIC_MMC
...

>   #define CONFIG_MMC
>   #define CONFIG_GENERIC_MMC
>   #define CONFIG_CMD_MMC
> @@ -199,7 +199,7 @@
>
>   #define CONFIG_SPL_LIBDISK_SUPPORT
>
> -#if !defined(CONFIG_UART0_PORT_F)
> +#if !defined(CONFIG_UART0_PORT_F) && !defined(CONFIG_NO_MMC)
>   #define CONFIG_SPL_MMC_SUPPORT
>   #endif
>

And this would become #ifdef CONFIG_MMC too

> @@ -360,9 +360,12 @@ extern int soft_i2c_gpio_scl;
>   #define CONFIG_FASTBOOT_BUF_SIZE	0x2000000
>
>   #define CONFIG_FASTBOOT_FLASH
> +
> +#ifdef CONFIG_MMC
>   #define CONFIG_FASTBOOT_FLASH_MMC_DEV	0
>   #define CONFIG_EFI_PARTITION
>   #endif
> +#endif
>
>   #ifdef CONFIG_USB_FUNCTION_MASS_STORAGE
>   #define CONFIG_CMD_USB_MASS_STORAGE
>

Regards,

Hans