[PATCH v2 02/10] mmc: Add init() API

[Simon Goldschmidt]

On Fri, Jan 24, 2020 at 12:52 PM Faiz Abbas <faiz_abbas at ti.com> wrote:
>
> Add an init() API for platform specific init() operations.

Could you describe why this cannot be done in the probe callback? It's not
easily visible as the function you changed (mmc_get_op_cond) doesn't even have
a comment to describe what it does...

In general, I think commit messages could be more detailed than one line. If
only to make it easier in the future to recap why things have been done.

>
> Signed-off-by: Faiz Abbas <faiz_abbas at ti.com>
> Signed-off-by: Lokesh Vutla <lokeshvutla at ti.com>
> ---
>  drivers/mmc/mmc.c | 13 ++++++-------
>  include/mmc.h     |  7 +++++++
>  2 files changed, 13 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/mmc/mmc.c b/drivers/mmc/mmc.c
> index d43983d4a6..50df8c8626 100644
> --- a/drivers/mmc/mmc.c
> +++ b/drivers/mmc/mmc.c
> @@ -2787,14 +2787,13 @@ int mmc_get_op_cond(struct mmc *mmc)
>         }
>         if (err)
>                 return err;
> -
>  #if CONFIG_IS_ENABLED(DM_MMC)
> -       /* The device has already been probed ready for use */
> -#else
> -       /* made sure it's not NULL earlier */
> -       err = mmc->cfg->ops->init(mmc);
> -       if (err)
> -               return err;

You're removing the init code for non-DM MMC here and did not describe it in
the commit message. Is this change intended at all?

Regards,
Simon

> +       struct dm_mmc_ops *ops = mmc_get_ops(mmc->dev);
> +       if (ops->init) {
> +               err = ops->init(mmc->dev);
> +               if (err)
> +                       return err;
> +       }
>  #endif
>         mmc->ddr_mode = 0;
>
> diff --git a/include/mmc.h b/include/mmc.h
> index 2f21dbf1b7..6aef125f25 100644
> --- a/include/mmc.h
> +++ b/include/mmc.h
> @@ -406,6 +406,13 @@ struct mmc;
>
>  #if CONFIG_IS_ENABLED(DM_MMC)
>  struct dm_mmc_ops {
> +       /**
> +        * init() - platform specific initialization for the host controller
> +        *
> +        * @dev:        Device to init
> +        * @return 0 if Ok, -ve if error
> +        */
> +       int (*init)(struct udevice *dev);
>         /**
>          * send_cmd() - Send a command to the MMC device
>          *
> --
> 2.19.2
>