[U-Boot] [PATCH 03/12] spl: mmc: refactor device location code to its own function

[Nikita Kiryanov]

Hi,
On Thu, Oct 22, 2015 at 02:42:22PM +0200, Hans de Goede wrote:
> Hi,
> 
> On 22-10-15 14:01, Nikita Kiryanov wrote:
> >Simplify spl_mmc_load_image() code by moving the part that finds the mmc device
> >into its own function spl_mmc_find_device(), available in two flavors: DM and
> >non-DM.
> >
> >This refactor fixes a bug in which an error in the device location sequence
> >does not necessarily aborts the rest of the code. With this refactor, we fail
> >the moment there is an error.
> >
> >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>
> >---
> >  common/spl/spl_mmc.c | 77 +++++++++++++++++++++++++++++++++++++---------------
> >  1 file changed, 55 insertions(+), 22 deletions(-)
> >
> >diff --git a/common/spl/spl_mmc.c b/common/spl/spl_mmc.c
> >index e831970..cfbda1a 100644
> >--- a/common/spl/spl_mmc.c
> >+++ b/common/spl/spl_mmc.c
> >@@ -11,6 +11,7 @@
> >  #include <spl.h>
> >  #include <linux/compiler.h>
> >  #include <asm/u-boot.h>
> >+#include <errno.h>
> >  #include <mmc.h>
> >  #include <image.h>
> >
> >@@ -59,6 +60,58 @@ end:
> >  	return 0;
> >  }
> >
> >+#ifdef CONFIG_DM_MMC
> >+static int spl_mmc_find_device(struct mmc **mmc)
> >+{
> >+	struct udevice *dev;
> >+	int err;
> >+
> >+	err = mmc_initialize(NULL);
> >+	if (err) {
> >+#ifdef CONFIG_SPL_LIBCOMMON_SUPPORT
> >+		printf("spl: could not initialize mmc. error: %d\n", err);
> >+#endif
> >+		return err;
> >+	}
> >+
> >+	err = uclass_get_device(UCLASS_MMC, 0, &dev);
> >+	if (err) {
> >+#ifdef CONFIG_SPL_LIBCOMMON_SUPPORT
> >+		puts("spl: could not find mmc device. error: %d\n", err);
> >+#endif
> >+		return err;
> >+	}
> >+
> >+	*mmc = NULL;
> >+	*mmc = mmc_get_mmc_dev(dev);
> >+	return *mmc != NULL ? 0 : -ENODEV;
> >+}
> >+#else
> >+static int spl_mmc_find_device(struct mmc **mmc)
> >+{
> >+	int err;
> >+
> >+	err = mmc_initialize(gd->bd);
> >+	if (err) {
> >+#ifdef CONFIG_SPL_LIBCOMMON_SUPPORT
> >+		printf("spl: could not initialize mmc. error: %d\n", err);
> >+#endif
> >+		return err;
> >+	}
> >+
> >+	/* We register only one device. So, the dev id is always 0 */
> >+	*mmc = find_mmc_device(0);
> 
> This does not work when there are 2 mmc devices, 1 main and one alternate
> boot device (sunxi has this). spl_boot_device() can return:
> BOOT_DEVICE_MMC1 and/or BOOT_DEVICE_MMC2 currently the number in this gets
> completely ignored by the spl mmc code, this patch-set seems like a good
> moment to fix this.

I'll look into incorporating it into a V2 (though my time constraints
may force me to do otherwise).

> 
> This will allow cleaning up hacks like this one:
> 
>         if (CONFIG_MMC_SUNXI_SLOT_EXTRA == 2) {
>                 mmc1 = find_mmc_device(1);
>                 if (sunxi_mmc_has_egon_boot_signature(mmc1)) {
>                         /*
>                          * spl_mmc.c: spl_mmc_load_image() is hard-coded to
>                          * use find_mmc_device(0), no matter what we
>                          * return. Swap mmc0 and mmc2 to make this work.
>                          */
>                         mmc0->block_dev.dev = 1;
>                         mmc1->block_dev.dev = 0;
>                         return BOOT_DEVICE_MMC2;
>                 }
> 	}
> 
> Regards,
> 
> Hans
>