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

[Hans de Goede]

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.

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





spl_

> +	if (!*mmc) {
> +#ifdef CONFIG_SPL_LIBCOMMON_SUPPORT
> +		puts("spl: mmc device not found\n");
> +#endif
> +		return -ENODEV;
> +	}
> +
> +	return 0;
> +}
> +#endif
> +
>   #ifdef CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_PARTITION
>   static int mmc_load_image_raw_partition(struct mmc *mmc, int partition)
>   {
> @@ -110,30 +163,10 @@ void spl_mmc_load_image(void)
>   	int err = 0;
>   	__maybe_unused int part;
>
> -#ifdef CONFIG_DM_MMC
> -	struct udevice *dev;
> -
> -	mmc_initialize(NULL);
> -	err = uclass_get_device(UCLASS_MMC, 0, &dev);
> -	mmc = NULL;
> -	if (!err)
> -		mmc = mmc_get_mmc_dev(dev);
> -#else
> -	mmc_initialize(gd->bd);
> -
> -	/* We register only one device. So, the dev id is always 0 */
> -	mmc = find_mmc_device(0);
> -	if (!mmc) {
> -#ifdef CONFIG_SPL_LIBCOMMON_SUPPORT
> -		puts("spl: mmc device not found\n");
> -#endif
> +	if (spl_mmc_find_device(&mmc))
>   		hang();
> -	}
> -#endif
> -
> -	if (!err)
> -		err = mmc_init(mmc);
>
> +	err = mmc_init(mmc);
>   	if (err) {
>   #ifdef CONFIG_SPL_LIBCOMMON_SUPPORT
>   		printf("spl: mmc init failed with error: %d\n", err);
>