From mboxrd@z Thu Jan  1 00:00:00 1970
From: Hans de Goede <hdegoede@redhat.com>
Date: Thu, 22 Oct 2015 14:37:11 +0200
Subject: [U-Boot] [PATCH 02/12] spl: mmc: add break statements in
 spl_mmc_load_image()
In-Reply-To: <1445515280-21389-3-git-send-email-nikita@compulab.co.il>
References: <1445515280-21389-1-git-send-email-nikita@compulab.co.il>
 <1445515280-21389-3-git-send-email-nikita@compulab.co.il>
Message-ID: <5628D877.6080403@redhat.com>
List-Id: <u-boot.lists.denx.de>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
To: u-boot@lists.denx.de

Hi,

On 22-10-15 14:01, Nikita Kiryanov wrote:
> The original intention of the mmc load_image() function was to try multiple
> boot modes before failing. This is evident by the lack of break statements
> in the switch, and the following line in the default case:
> puts("spl: mmc: no boot mode left to try\n");
>
> This implementation is problematic because:
> - The availability of alternative boot modes is very arbitrary since it
> depends on the specific order of the switch cases. If your boot mode happens to
> be the first case, then you'll have a bunch of other boot modes as alternatives.
> If it happens to be the last case, then you have none.
> - Opting in/out is tied to config options, so the only way for you to prevent an
> alternative boot mode from being attempted is to give up on the feature completely.
> - This implementation makes the code more complicated and difficult to
> understand.
>
> Address these issues by inserting a break statements between the cases to make the
> function try only one boot mode.
>
> Signed-off-by: Nikita Kiryanov <nikita@compulab.co.il>
> Cc: Igor Grinberg <grinberg@compulab.co.il>
> Cc: Paul Kocialkowski <contact@paulk.fr>
> Cc: Pantelis Antoniou <panto@antoniou-consulting.com>
> Cc: Tom Rini <trini@konsulko.com>
> Cc: Simon Glass <sjg@chromium.org>
> ---
>   common/spl/spl_mmc.c | 10 ++++++----
>   1 file changed, 6 insertions(+), 4 deletions(-)
>
> diff --git a/common/spl/spl_mmc.c b/common/spl/spl_mmc.c
> index ce58c58..e831970 100644
> --- a/common/spl/spl_mmc.c
> +++ b/common/spl/spl_mmc.c
> @@ -164,6 +164,7 @@ void spl_mmc_load_image(void)
>   		if (!err)
>   			return;
>   #endif
> +		break;
>   	case MMCSD_MODE_FS:
>   		debug("spl: mmc boot mode: fs\n");
>
> @@ -203,6 +204,7 @@ void spl_mmc_load_image(void)
>   #endif
>   #endif
>   #endif
> +		break;
>   #ifdef CONFIG_SUPPORT_EMMC_BOOT
>   	case MMCSD_MODE_EMMCBOOT:
>   		/*
> @@ -240,15 +242,15 @@ void spl_mmc_load_image(void)
>   		if (!err)
>   			return;
>   #endif
> +		break;
>   #endif
>   	case MMCSD_MODE_UNDEFINED:
>   	default:
>   #ifdef CONFIG_SPL_LIBCOMMON_SUPPORT
> -		if (err)
> -			puts("spl: mmc: no boot mode left to try\n");
> -		else
> -			puts("spl: mmc: wrong boot mode\n");
> +		puts("spl: mmc: wrong boot mode\n");
>   #endif
>   		hang();

The above hang() can be removed now.

>   	}
> +
> +	hang();
>   }
>

Regards,

Hans