From mboxrd@z Thu Jan 1 00:00:00 1970 From: Hans de Goede Date: Fri, 18 Sep 2015 10:52:37 -0400 Subject: [U-Boot] [linux-sunxi] [PATCH 1/6] sunxi: Add an option to disable MMC In-Reply-To: <1442577980-26373-2-git-send-email-maxime.ripard@free-electrons.com> References: <1442577980-26373-1-git-send-email-maxime.ripard@free-electrons.com> <1442577980-26373-2-git-send-email-maxime.ripard@free-electrons.com> Message-ID: <55FC2535.8080906@redhat.com> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de 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 > --- > 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