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

[Maxime Ripard]

Hi Hans,

On Fri, Sep 18, 2015 at 10:52:37AM -0400, Hans de Goede wrote:
> 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 at 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

That sounds great. I'll switch to that.

Thanks!
Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20150922/eebefde2/attachment.sig>