[U-Boot] [PATCH v2] misc: fs_loader: Add support for initializing MMC

[Simon Glass]

Hi Tien Fong,

On Sat, 29 Dec 2018 at 18:36, <tien.fong.chee at intel.com> wrote:
>
> From: Tien Fong Chee <tien.fong.chee at intel.com>
>
> Firmware loader would encounter problem if the MMC is accessed before
> initializing it. This patch would adding the support of probing block
> device and initializing MMC before the MMC is accessed by firmware loader.
>
> Signed-off-by: Tien Fong Chee <tien.fong.chee at intel.com>
> ---
>
> Changes in v2:
> - Initializing MMC through probing the blk device
> ---
>  drivers/misc/fs_loader.c | 33 +++++++++++++++++++++++++++++++++
>  1 file changed, 33 insertions(+)
>
> diff --git a/drivers/misc/fs_loader.c b/drivers/misc/fs_loader.c
> index 57a14a3..5858073 100644
> --- a/drivers/misc/fs_loader.c
> +++ b/drivers/misc/fs_loader.c
> @@ -12,6 +12,7 @@
>  #include <linux/string.h>
>  #include <mapmem.h>
>  #include <malloc.h>
> +#include <mmc.h>
>  #include <spl.h>
>
>  DECLARE_GLOBAL_DATA_PTR;
> @@ -252,6 +253,38 @@ static int fs_loader_ofdata_to_platdata(struct udevice *dev)
>
>  static int fs_loader_probe(struct udevice *dev)
>  {
> +#if defined(CONFIG_DM_MMC) && defined(CONFIG_BLK)
> +       int ret;
> +       struct device_platdata *plat = dev->platdata;
> +
> +       ret = mmc_initialize(NULL);

You should not need this since the MMC device should init itself when probed.

> +       if (ret) {
> +               debug("MMC: could not initialize mmc. error: %d\n", ret);
> +
> +               return ret;
> +       }
> +
> +       if (plat->phandlepart.phandle) {
> +               ofnode node = ofnode_get_by_phandle(plat->phandlepart.phandle);
> +
> +               struct udevice *mmc_dev = NULL;
> +
> +               ret = device_get_global_by_ofnode(node, &mmc_dev);

You could use this?

uclass_get_device_by_ofnode(UCLASS_MMC, node, &mmc_dev)

But actually I think you should drop any mention of MMC so that your
code can work for any storage uclass.

> +               if (!ret) {
> +                       struct mmc *mmc = mmc_get_mmc_dev(mmc_dev);
> +                       struct udevice *dev;
> +
> +                       ret = blk_get_from_parent(mmc->dev, &dev);

You are going from mmc_dev -> mmc -> mmc->dev, but unless I am
mistaken, mmc_dev == mmc->dev. So you should be able to simplify this
to:

                       ret = blk_get_from_parent(mmc_dev, &dev);

Also you should be able to rename mmc to something more generic, e.g.
to make this work with SATA.

> +                       if (ret) {
> +                               debug("MMC: No block device: %d\n",
> +                                       ret);
> +
> +                               return ret;
> +                       }
> +               }
> +       }
> +#endif
> +
>         return 0;
>  };
>
> --
> 2.2.0
>

Regards,
Simon