From mboxrd@z Thu Jan 1 00:00:00 1970 From: Hans de Goede Date: Thu, 22 Oct 2015 14:42:22 +0200 Subject: [U-Boot] [PATCH 03/12] spl: mmc: refactor device location code to its own function In-Reply-To: <1445515280-21389-4-git-send-email-nikita@compulab.co.il> References: <1445515280-21389-1-git-send-email-nikita@compulab.co.il> <1445515280-21389-4-git-send-email-nikita@compulab.co.il> Message-ID: <5628D9AE.2020006@redhat.com> List-Id: 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: > 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 > Cc: Igor Grinberg > Cc: Paul Kocialkowski > Cc: Pantelis Antoniou > Cc: Tom Rini > Cc: Simon Glass > --- > 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 > #include > #include > +#include > #include > #include > > @@ -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); >