[U-Boot] [PATCH V2 03/13] spl: mmc: refactor device location code to its own function
Simon Glass
sjg at chromium.org
Thu Oct 29 18:19:16 CET 2015
Hi Nikita,
On 28 October 2015 at 03:23, Nikita Kiryanov <nikita at compulab.co.il> 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>
> ---
> Changes in V2:
> - No changes.
>
> common/spl/spl_mmc.c | 77 +++++++++++++++++++++++++++++++++++++---------------
> 1 file changed, 55 insertions(+), 22 deletions(-)
Reviewed-by: Simon Glass <sjg at chromium.org>
But can we only have one spl_mmc_find_device() function, with the
#ifdef CONFIG_DM_MMC inside it?
>
> diff --git a/common/spl/spl_mmc.c b/common/spl/spl_mmc.c
> index 6011f77..044be52 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);
> + 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);
> --
> 1.9.1
>
Regards,
Simon
More information about the U-Boot
mailing list