[U-Boot] [PATCH 05/12] spl: mmc: get rid of #ifdef CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_PARTITION check

Nikita Kiryanov nikita at compulab.co.il
Thu Oct 22 15:40:47 CEST 2015


On Thu, Oct 22, 2015 at 02:47:29PM +0200, Hans de Goede wrote:
Hi Hans,
> Hi,
> 
> On 22-10-15 14:01, Nikita Kiryanov wrote:
> >Implement defaults for the raw partition image loading so that the #ifdef
> >CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_PARTITION in spl_mmc_load_image() will no
> >longer be necessary.
> >
> >This change makes it possible for mmc_load_image_raw_partition() and
> >mmc_load_image_raw_sector() to coexist.
> >
> >Signed-off-by: Nikita Kiryanov <nikita at compulab.co.il>
> >Cc: Igor Grinberg <grinberg at compulab.co.il>
> >Cc: Paul Kocialkowski <contact at paulk.fr>
> >Cc: Pantelis Antoniou <panto at antoniou-consulting.com>
> >Cc: Tom Rini <trini at konsulko.com>
> >Cc: Simon Glass <sjg at chromium.org>
> 
> Same remark as with the previous patch, I'm not happy to see all these
> patches removing #ifdef-s given that spl is severely size constrained
> on some devices (e.g. recently we had a patchset for the rockchip 3036,
> which has only 8k space for the SPL).
> 
> And I really do not see a need for this, boards which want to use
> multiple methods can simple define the CONFIG_SPL_FOO for all of them.

The goal of the first part of the series is to make the spl_mmc file
easier to maintain. I do not believe that the function stubs would add
much to the binary size, but if you're concerned about bloat, I can do
a buildman comparison.

> 
> Likewise this is also not necessary to convert things to an array
> of image loading functions to try one by one, which your ultimate
> goal seems to be (which is a good goal). You can simply put
> #ifdef's around the array initializers which are not always there.

True, the first part is not necessary for the second part, I just saw
an opportunity. If the consensus would be to reject the first part I'll
rework and resubmit the second part for the current spl_mmc.c

> 
> Regards,
> 
> Hans
> 
> 
> 
> >---
> >  common/spl/spl_mmc.c | 13 +++++++++----
> >  1 file changed, 9 insertions(+), 4 deletions(-)
> >
> >diff --git a/common/spl/spl_mmc.c b/common/spl/spl_mmc.c
> >index f0c4d56..fbdcf0d 100644
> >--- a/common/spl/spl_mmc.c
> >+++ b/common/spl/spl_mmc.c
> >@@ -133,6 +133,12 @@ static int mmc_load_image_raw_partition(struct mmc *mmc, int partition)
> >  	return mmc_load_image_raw_sector(mmc, info.start);
> >  #endif
> >  }
> >+#else
> >+#define CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_PARTITION -1
> >+static int mmc_load_image_raw_partition(struct mmc *mmc, int partition)
> >+{
> >+	return -ENOSYS;
> >+}
> >  #endif
> >
> >  #ifdef CONFIG_SPL_OS_BOOT
> >@@ -193,12 +199,12 @@ void spl_mmc_load_image(void)
> >  			if (!err)
> >  				return;
> >  		}
> >-#if defined(CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_PARTITION)
> >+
> >  		err = mmc_load_image_raw_partition(mmc,
> >  			CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_PARTITION);
> >  		if (!err)
> >  			return;
> >-#elif defined(CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_SECTOR)
> >+#if defined(CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_SECTOR)
> >  		err = mmc_load_image_raw_sector(mmc,
> >  			CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_SECTOR);
> >  		if (!err)
> >@@ -265,12 +271,11 @@ void spl_mmc_load_image(void)
> >  			if (!err)
> >  				return;
> >  		}
> >-#if defined(CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_PARTITION)
> >  		err = mmc_load_image_raw_partition(mmc,
> >  			CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_PARTITION);
> >  		if (!err)
> >  			return;
> >-#elif defined(CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_SECTOR)
> >+#if defined(CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_SECTOR)
> >  		err = mmc_load_image_raw_sector(mmc,
> >  			CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_SECTOR);
> >  		if (!err)
> >
> 


More information about the U-Boot mailing list