[PATCH] spl: mmc: Use correct MMC device when loading image

[Harald Seiler]

Hi,

On Mon, 2022-07-11 at 12:18 +0200, Quentin Schulz wrote:
> Hi Harald,
> 
> On 7/6/22 12:58, Harald Seiler wrote:
> > When attempting to load images from multiple MMC devices in sequence,
> > spl_mmc_load() chooses the wrong device from the second attempt onwards.
> > 
> > The reason is that MMC initialization is only done on its first call and
> > spl_mmc_load() will then continue using this same device for all future
> > calls.
> > 
> > Fix this by checking the devnum of the "cached" device struct against
> > the one which is requested.  If they match, use the cached one but if
> > they do not match, initialize the new device.
> > 
> > This fixes specifying multiple MMC devices in the SPL's boot order to
> > fall back when U-Boot Proper is corrupted or missing on the first
> > attempted MMC device.
> > 
> > Fixes: e1eb6ada4e38 ("spl: Make image loader infrastructure more universal")
> > Signed-off-by: Harald Seiler <hws at denx.de>
> > ---
> >   common/spl/spl_mmc.c | 6 ++++--
> >   1 file changed, 4 insertions(+), 2 deletions(-)
> > 
> > diff --git a/common/spl/spl_mmc.c b/common/spl/spl_mmc.c
> > index e1a7d25bd0..22c8a8097c 100644
> > --- a/common/spl/spl_mmc.c
> > +++ b/common/spl/spl_mmc.c
> > @@ -371,9 +371,11 @@ int spl_mmc_load(struct spl_image_info *spl_image,
> >   	u32 boot_mode;
> >   	int err = 0;
> >   	__maybe_unused int part = 0;
> > +	int mmc_dev;
> >   
> > -	/* Perform peripheral init only once */
> > -	if (!mmc) {
> > +	/* Perform peripheral init only once for an mmc device */
> > +	mmc_dev = spl_mmc_get_device_index(bootdev->boot_device);
> > +	if (!mmc || mmc->block_dev.devnum != mmc_dev) {
> 
> block_dev field only exists when CONFIG_BLK is not defined, c.f. 
> https://elixir.bootlin.com/u-boot/latest/source/include/mmc.h#L705
> 
> Considering the commit introducing this, c.f. 
> https://source.denx.de/u-boot/u-boot/-/commit/33fb211dd2706e666db4008801dc0d5903fd82f6, 
> I can suggest the following diff (which makes it compile with 
> rk3399-puma_defconfig):
> 
> diff --git a/common/spl/spl_mmc.c b/common/spl/spl_mmc.c
> index e0e1a80536..c40b436a11 100644
> --- a/common/spl/spl_mmc.c
> +++ b/common/spl/spl_mmc.c
> @@ -409,10 +409,17 @@ int spl_mmc_load(struct spl_image_info *spl_image,
>          int err = 0;
>          __maybe_unused int part = 0;
>          int mmc_dev;
> +       struct blk_desc *block_dev;
> 
>          /* Perform peripheral init only once for an mmc device */
>          mmc_dev = spl_mmc_get_device_index(bootdev->boot_device);
> -       if (!mmc || mmc->block_dev.devnum != mmc_dev) {
> +#if !CONFIG_IS_ENABLED(BLK)
> +       block_dev = &mmc->block_dev;
> +#else
> +       block_dev = dev_get_uclass_plat(mmc->dev);
> +#endif
> +
> +       if (!mmc || block_dev->devnum != mmc_dev) {
>                  err = spl_mmc_find_device(&mmc, bootdev->boot_device);
>                  if (err)
>                          return err;
> 
> Note: Only build tested.

Thanks for reporting.  It's not quite that simple because `mmc` is NULL
on first call to spl_mmc_load().  I'll send a v2 to fix this.

-- 
Harald